x/authz pruning and gas

[aaronc]

Following up from https://github.com/cosmos/cosmos-sdk/pull/7105#discussion_r505643003, as improvements to the core x/authz implementations it would be desirable to:

• DONE ~support some method for pruning expired authorizations~
• have a gas fee or some other financial mechanism to prevent lots of spurious authorizations from polluting state

[robert-zaremba]

Do we have any framework, guidelines or simulation about gas costs?

[robert-zaremba]

My proposal for storage is following:

1. charge small amount of gas
2. and require a stake, which will be released once we delete the storage.

This is what Ethereum simulations and research are suggesting.

[clevinson]

Let's put this in the icebox until we have a better solution for deposits for messages like this.

[yun-yeo]

@aaronc The auth module has been added as sdk module, so this will be done before v0.43.0 release?

[aaronc]

@YunSuk-Yeo actually the latest status of this conversation is that this should be done more generally with storage deposits/bonds (see #8929). As a result we have not prioritized just deleting expired grants because it only covers a subset of pruning, and instead are planning to do a more general solution in the next release. That general solution though could be a hybrid between storage deposits and expiration-based pruning.

[yun-yeo]

@aaronc so next release means v0.43.0?

Btw there are some issues

Simulation

https://github.com/cosmos/cosmos-sdk/blob/eb7d939f86c6cd7b4218492364cdda3f649f06b5/x/authz/simulation/operations.go#L252

https://github.com/cosmos/cosmos-sdk/blob/eb7d939f86c6cd7b4218492364cdda3f649f06b5/x/authz/simulation/operations.go#L258

Both must use {}.!IsAllGTE(coins) to check the balance properly. {}.IsAllLTE(coins) only works for simulation with single stake coin. If there are more coins like atom then the comparison always return false.

Implementation

There is an attempt to delete a expired Grant, but if GetCleanAuthorization returns nil when the Grant has been expired.

https://github.com/cosmos/cosmos-sdk/blob/f2cea6a137ce19ad8987fa8a0cb99f4b37c4484d/x/authz/keeper/keeper.go#L181-L184

but, DispatchActions revert all state when the returned authorization is nil.

https://github.com/cosmos/cosmos-sdk/blob/f2cea6a137ce19ad8987fa8a0cb99f4b37c4484d/x/authz/keeper/keeper.go#L86-L89

so don't need to call k.DeleteGrant(ctx, grantee, granter, msgType) in func GetCleanAuthoization

[aaronc]

@aaronc so next release means v0.43.0?

No a generalized pruning solution will be in the next release - v0.44. 0.43 is the current release which we're hoping to finalize ASAP.

Thanks for identifying those issues @amaurym @robert-zaremba can you triage?

[robert-zaremba]

So, the issue of "reverting" expired grant deletion caused a bug: https://github.com/cosmos/cosmos-sdk/issues/10611. Ah, we didn't put enough attention to triage this task. I'm discussing a potential solution here: https://github.com/cosmos/cosmos-sdk/pull/10633 (right now, the PR removes grant deletion).

[amaury1093]

Pruning has been done in #10714.

Only item left here is gas consumption to avoid polluting state. We currently didn't decide on a solution for this.

[alpe]

I am working on authz support for wasmd and was wondering about the gas consumption, too. It is too cheap currently as the time the date is stored is not taken into account. There is no incentive to optimize the grants (for contracts).

[giansalex]

Is it possible to add a MsgReject where a grantee can reject an authorization granted?

For example, in restake apps (granter is a third party account) there are many delegators who have re-delegated or undelegated all their tokens, but they do not revoke authorization. This may become relevant in cosmos-sdk 46 where authorizations may not expire.

If it is feasible, I could work on it.

[robert-zaremba]

@giansalex I think this is a good idea, and it should be rather easy to do. I don't see any security concern.