Update group module to tally at the end of voting

[blushi]

Summary

Update group module to tally at the end.

Problem Definition

Currently, proposals get unvalidated as soon as the corresponding group or group policy gets updated (e.g. if group weights change).

Proposal

Instead, we should change the x/group tally behavior to be based on the group weights at the end of voting.

Some pending questions: how do we deal with the time difference between when a voting window ends and MsgExec arrives? Do we maybe need to switch to end blocker based processing? If so then how do we deal with gas and fees? Those would need to be added to the proposal and/or some dedicated pay fees msg.

cc/ @hxrts @nooomski @aaronc @cmwaters

---

For Admin Use

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

[cmwaters]

I think the idea was to remove MsgExec entirely and rely on EndBlocker to execute the message automatically when the voting period has finished (exactly as what happens in gov). Obviously, the challenge is to make sure that the proposer includes enough of a fee to cover the gas of the execution. I'm honestly not familiar with how that is done but I'd assume there is some way that we can calculate the gas in MsgSubmitProposal especially as we're iterating over the messages in the proposal anyway and we'd consume the gas then and there that would roughly equate to the gas needed to execute the messages in EndBlock.

[aaronc]

So if we take the EndBlocker approach which is probably correct here, we can add:

• a Fee message which includes Coins and gas_limit
• contribute_to_fee options on MsgSubmitProposal and MsgVote
• a standalone MsgPayFee
• a MsgSetGasPrice that gov calls to set the gas price

I'm not sure we want to deal with refunding unused gas, but if we do, then we should just have one MsgPayFee per proposal and refund it to that payer. I don't think we want to deal with multiple payers, although if that's a desired UX I guess it's not terrible. If we take the one fee payer approach then if MsgPayFee is called twice it should replace the fee and refund existing fee to the original payer, but only allow replacement if the new fee + gas limit are >= the original one.

[nooomski]

Imo the proposer should just pay for the fee. Can't really see a strong case for anything else really.

[hxrts]

Agree, proposer-only fee is simple. Fee grant can be used for alternative payment setup.

I think I'd prefer refunding unused gas, then have a consistent mechanism for spam prevention that covers gov/groups/authz to be implemented later. Deposit module is one option. Another would be rate limiting proposals per account or globally per unit time with exponential fee or deposit amount, something Dean suggested a while back that I quite like.

[amaury1093]

If we go with proposer paying for execution fee, then we can do as @cmwaters suggested: on MsgSubmitProposal, actually execute the proposal, but in a cached context (ie. don't write to state), which gives a good estimate of gas.

Note that there were some concerns about this, namely that a proposal's Msgs can fail during submission, but pass during execution. Or vice-versa. See #10868 (comment).

I think one-proposer gas refunding is doable. We basically need to store the proposer's initial tx fee and gas_limit somewhere, so that execution EndBlocker can refund the difference. Instead of storing them in the proposal, maybe we can just store in state on MsgSubmitProposal, under a new key proposal_id -> {fee, gas_limit}? It'd be transparent for users.

[cmwaters]

I think one-proposer gas refunding is doable. We basically need to store the proposer's initial tx fee and gas_limit somewhere, so that execution EndBlocker can refund the difference. Instead of storing them in the proposal, maybe we can just store in state on MsgSubmitProposal, under a new key proposal_id -> {fee, gas_limit}? It'd be transparent for users.

This is all great but I don't think we should endeavour to solve refunding gas for this release. There may be incongruences between the gas at submission and execution but we'd hope it's negligible. We don't have to turn this into an exact science. I think for now it's okay if a proposer occasionally overspends on gas.

[cmwaters]

Roundup on the discussion in today's call:

• For this release, we have decided to keep MsgExec as solving the gas problem is seemingly too much work given our time constraints. To allow for changing weights throughout the duration of a proposal, we will still use the Endblocker at the end of the proposal to calculate the tally and persist the result. MsgExec can only happen after the election has finished and won't tally the results but simply execute the messages if the proposal status is "passed". The account executing the message thus pays the gas.
• Ideally we remove the MsgExec to improve the UX of group users (so they don't have to come back to execute something). For the following release, we may want to implement a protocol in which an environment can be set up for tracking gas in EndBlock. The groups module could then do one of two things when executing a proposal:
  1. Use a constant set by app developers on how much gas can be expended in a proposal
  2. Have a variable set by the proposer themselves as to how much they are willing to spend in gas
     The problem with this option is that it's bad UX if a user submits a proposal with x amount of fees to pay for the gas and that proposal fails because x is not enough. Ideally there is some guarantee that as long as the proposal isn't grossly expensive it should have enough gas to execute.

Another idea that was thrown around is to have a deposit module in which the group declares a connected account which can be drained to cover the gas needed to execute the proposal. We can continue to think about this later,

[amaury1093]

I'll create a PR with the following changes:

• distinguish 2 windows for group proposals: the voting window, and the execution window
• instead of doing tallying after each vote (current version), we do tallying once, at the end of the voting window

[nooomski]

Thanks @AmauryM ! Would that execution window be a global param? Or part of the decision policy?

[amaury1093]

I'm leaning towards part of the decision policy (more flexible, and not much more code), see the proto files in #11198.

Are we okay to leave it optional too, which would mean the execution window is infinite?

[nooomski]

Sounds good to me. During the discussion in the gov / groups call I was under the impression the execution window was going to be infinite anyway :)

[cmwaters]

If garbage collection of finished proposals is not important then it's fine to not have a window (i.e. make it infinite). If we do expect some garbage collection then there should be an upper limit. I would tend to think that this execution window would actually be a global parameter. I can't see why you'd want to have it different from group to group