Enforce default code permissions #850
Conversation
Codecov Report
@@ Coverage Diff @@
## main #850 +/- ##
==========================================
+ Coverage 58.89% 58.97% +0.07%
==========================================
Files 52 52
Lines 6165 6177 +12
==========================================
+ Hits 3631 3643 +12
Misses 2266 2266
Partials 268 268
|
|
This looks good to me, will solve current issues with the gov system. We may need to add some migration notes besides Stargaze and Osmosis I'm not aware of which other chains have permissioned setup. But when upgrading chains will have to add a migration that: |
|
Yes, we should use some migrate notes. That said, the old settings will continue to work, we just provide a second (more gas-efficient) way to achieve this. And the chain can adjust when they are ready |
| defaultPermssion: types.AccessTypeNobody, | ||
| requestedPermission: nil, | ||
| grantedPermission: types.AccessConfig{Permission: types.AccessTypeNobody}, | ||
| }, |
There was a problem hiding this comment.
nit: you don't have a case where requestedPermission < defaultPermission .
There was a problem hiding this comment.
I do... that is what "override everyone" tests (request more restrictive)
and "cannot override nobody" (request less restrictive)
I can add more, but these cover the general cases, and the actual compare is heavily tested elsewhere.
If you have a concrete default/requested I should add, let me know. I will just add one for equals
| requestedPermission: nil, | ||
| grantedPermission: types.AccessConfig{Permission: types.AccessTypeEverybody}, | ||
| }, | ||
| "cannot override nobody": { |
ethanfrey
force-pushed
the
840-enforce-default-code-permissions
branch
from
7484090 to
27d3051
Compare
Closes #840
Closes #761
We now define the
InstantiateDefaultPermissionas a restriction on the contracts, not a "helpful default".That means, it is used if you do not set a permission explicitly, but it also provide a "maximum bound" on how permissive you can be.
That is, if
InstantiateDefaultPermissionisEverybody, you can set any permission you wish (permissionless cosmwasm)But if
InstantiateDefaultPermissionisNobody, you can only set permission to Nobody on upload and must request governance to update that to allow instantiation (permissioned cosmwasm)This minor logic change now allows us to open up code upload to everybody (which cost $$$ gas when it was a proposal) and just have a vote on updating the instantiate permissions (implemented in #796), which is much cheaper to vote on. Thus resolving the issue of very expensive votes on permssioned chains #761 while still allowing them to control what code is executed.
@jhernandezb Happy for feedback here as well