Subkeys 2 #28
Subkeys 2 #28
Conversation
|
Still need to test |
| &initial_expirations, | ||
| ); | ||
|
|
||
| // Check allowances work for accounts with balances |
|
|
||
| // Add a third (new) admin | ||
| let msg = HandleMsg::UpdateAdmins { | ||
| admins: [&config.admins[..], &[admin3.clone()]].concat(), |
There was a problem hiding this comment.
Nice use of the spread and concat operators. But somehow, I would prefer to see this more explicit.
let new_admins = vec![owner.clone(), admin2.clone(), admin3.clone()]
let msg = HandleMsg::UpdateAdmins { admins: new_admins.clone() };
// ...
assert_eq!(config, ConfigResponse { admins: new_admins, mutable: true });This is the exact same functionality you have written but easier to read. And key in test cases is legibility (performance is not an issue)
|
Going through this code again, not the PR, I notice there is no test for execute, that the allowances work as they are supposed to. |
ethanfrey
dismissed
their stale review
I realized key execute tests were missing. Cannot approve yet
|
Yes. I'll write the |
| e => panic!("unexpected error: {}", e), | ||
| } | ||
|
|
||
| // Owner / admins can do anything (at the contract level) |
There was a problem hiding this comment.
Good test case.
Can you add another where the spender (with allowance) tries to send a message besides BankMsg::Send. Anything else to show it fails.
Just one more test and this is 👍
Improved test cases after previous PR comments.
Added the two remaining test cases.