Guibescos/transition to permissions #297
Conversation
guibescos
force-pushed
the
guibescos/transition-to-permissions
branch
from
f0fc2b9 to
ead116a
Compare
jayantk
left a comment
jayantk
left a comment
There was a problem hiding this comment.
LGTM, couple minor comments but feel free to merge and address in follow-ups.
| next_mapping_account.clone(), | ||
| permissions_account.clone() | ||
| ], | ||
| bytes_of::<CommandHeader>(&AddMapping.into()) |
There was a problem hiding this comment.
is this supposed to be InitMapping ?
| ) | ||
| .unwrap(); | ||
|
|
||
| assert_eq!( |
There was a problem hiding this comment.
I'm confused about the tests below. I thought that funding_account is permissioned to do these actions, and the attacker_account isn't. It looks like every test below is a permission violation
Maybe leave some code comments explaining what the tests are supposed to be testing.
There was a problem hiding this comment.
oh I looked into the code and realized that only InitMapping is allowed to be done by the permissions key. suggest leaving a comment to that effect and that these tests will change as more permissions are granted.
Update all instructions that require account signatures to having the
PermissionAccountchecks.It is backwards compatible and this PR doesn't try to define the permissions.