Enforce Module permissions modifications to be routed through supply keeper

[colin-axner]

Summary

Add functions to supply keeper such as AddPermission and RemovePermission.

Problem Definition

Currently there is little structure around how adding/removing permissions for a module account. A module account has AddPermission and RemovePermission functions that aren't exported. The only way we can do contextual validation on the permissions called in AddPermission is to use the ValidatePermissions function in supply keeper.

Proposal

Add AddPermission and RemovePermission to supply keeper that calls ValidatePermissions before calling the account's add/remove permission functions

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[fedekunze]

I believe @rigelrozanski had a very compelling argument to only grant permissions at initialization. If we support adding permissions what prevents for eg a module account to add the Minter Permission and start minting coins ?

[colin-axner]

So in this case all possible permissions are declared at initialization, ie a module account cannot be granted Minter Permission unless it is added as a possible permission initially.

Perhaps one way of making this stricter is unidirectional permission modifications. Give module accounts the full permissions declared at initialization and only allow for removal of those permissions and no additions. One example of usage of this is in coinswap if governance decided to disable a trading pair, the subaccounts minting permission could be removed.

[alexanderbez]

I believe @rigelrozanski had a very compelling argument to only grant permissions at initialization. If we support adding permissions what prevents for eg a module account to add the Minter Permission and start minting coins ?

It would have to go through governance. There may very well be situations where we want to grant permissions to existing module accounts or remove them (although I haven't thought of any).

[rigelrozanski]

YO

so this is what it is - a module can be granted the permission to modify permissions. Note that this isn't referring to a module-account permission, but a new class of permission for a module to make these modifications - it would ideally just live in supply and never get explicitly passed to other modules but if it did this would be defined at application initialization.

In our situation I reckon it would look like the supply module is passes this special new permission to a Proposal Type which is registered with governance at app initialization.

[colin-axner]

Sidenote: When implementing #4679, I had a misunderstanding of how permissions were managed. AddPermission and RemovePermission can be removed from the code, since they do not actually modify permissions tracked by the Supply Keeper. They just modify the ModuleAccount instance being referenced.

[rigelrozanski]

Open up a PR?

[alexanderbez]

Closed by #4798