Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(group)!: Don't re-tally proposals after VP end #14071

Merged
merged 10 commits into from Nov 30, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -100,6 +100,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### State Machine Breaking

* (x/group) [#13742](https://github.com/cosmos/cosmos-sdk/pull/13742) Migrate group policy account from module accounts to base account.
* (x/group) [#14071](https://github.com/cosmos/cosmos-sdk/pull/14071) Don't re-tally proposal after voting period end if they have been marked as ACCEPTED or REJECTED.
* (codec) [#13307](https://github.com/cosmos/cosmos-sdk/pull/13307) Register all modules' `Msg`s with group's ModuleCdc so that Amino sign bytes are correctly generated.
* (codec) [#13196](https://github.com/cosmos/cosmos-sdk/pull/13196) Register all modules' `Msg`s with gov's ModuleCdc so that Amino sign bytes are correctly generated.
* (group) [#13592](https://github.com/cosmos/cosmos-sdk/pull/13592) Fix group types registration with Amino.
Expand Down
4 changes: 3 additions & 1 deletion x/group/keeper/keeper.go
Expand Up @@ -407,7 +407,7 @@ func (k Keeper) TallyProposalsAtVPEnd(ctx sdk.Context) error {
if err := k.pruneVotes(ctx, proposalID); err != nil {
return err
}
} else {
} else if proposal.Status == group.PROPOSAL_STATUS_SUBMITTED {
if err := k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo); err != nil {
return sdkerrors.Wrap(err, "doTallyAndUpdate")
}
Expand All @@ -416,6 +416,8 @@ func (k Keeper) TallyProposalsAtVPEnd(ctx sdk.Context) error {
return sdkerrors.Wrap(err, "proposal update")
}
}
// Note: we do nothing if the proposal has been marked as ACCEPTED or
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
// REJECTED.
}
return nil
}
72 changes: 72 additions & 0 deletions x/group/keeper/keeper_test.go
Expand Up @@ -3277,3 +3277,75 @@ func (s *TestSuite) TestTallyProposalsAtVPEnd() {
s.Require().NoError(s.groupKeeper.TallyProposalsAtVPEnd(ctx))
s.NotPanics(func() { module.EndBlocker(ctx, s.groupKeeper) })
}

// TestTallyProposalsAtVPEnd_GroupMemberLeaving test that the node doesn't
// panic if a member leaves after the voting period end.
func (s *TestSuite) TestTallyProposalsAtVPEnd_GroupMemberLeaving() {
addrs := s.addrs
addr1 := addrs[0]
addr2 := addrs[1]
addr3 := addrs[2]
votingPeriod := time.Duration(4 * time.Minute)
minExecutionPeriod := votingPeriod + group.DefaultConfig().MaxExecutionPeriod

groupMsg := &group.MsgCreateGroupWithPolicy{
Admin: addr1.String(),
Members: []group.MemberRequest{
{Address: addr1.String(), Weight: "0.3"},
{Address: addr2.String(), Weight: "7"},
{Address: addr3.String(), Weight: "0.6"},
},
}
policy := group.NewThresholdDecisionPolicy(
"3",
votingPeriod,
minExecutionPeriod,
)
s.Require().NoError(groupMsg.SetDecisionPolicy(policy))

s.setNextAccount()
groupRes, err := s.groupKeeper.CreateGroupWithPolicy(s.ctx, groupMsg)
s.Require().NoError(err)
accountAddr := groupRes.GetGroupPolicyAddress()
groupPolicy, err := sdk.AccAddressFromBech32(accountAddr)
s.Require().NoError(err)
s.Require().NotNil(groupPolicy)

proposalRes, err := s.groupKeeper.SubmitProposal(s.ctx, &group.MsgSubmitProposal{
GroupPolicyAddress: accountAddr,
Proposers: []string{addr1.String()},
Messages: nil,
})
s.Require().NoError(err)

// group members vote
_, err = s.groupKeeper.Vote(s.ctx, &group.MsgVote{
ProposalId: proposalRes.ProposalId,
Voter: addr1.String(),
Option: group.VOTE_OPTION_NO,
})
s.Require().NoError(err)
_, err = s.groupKeeper.Vote(s.ctx, &group.MsgVote{
ProposalId: proposalRes.ProposalId,
Voter: addr2.String(),
Option: group.VOTE_OPTION_NO,
})
s.Require().NoError(err)

// move forward in time
ctx := s.sdkCtx.WithBlockTime(s.sdkCtx.BlockTime().Add(votingPeriod + 1))

// Tally the result. This saves the tally result to state.
s.Require().NoError(s.groupKeeper.TallyProposalsAtVPEnd(ctx))
s.NotPanics(func() { module.EndBlocker(ctx, s.groupKeeper) })

// member 2 (high weight) leaves group.
_, err = s.groupKeeper.LeaveGroup(ctx, &group.MsgLeaveGroup{
Address: addr2.String(),
GroupId: groupRes.GroupId,
})
s.Require().NoError(err)

s.Require().NoError(s.groupKeeper.TallyProposalsAtVPEnd(ctx))
s.NotPanics(func() { module.EndBlocker(ctx, s.groupKeeper) })
}