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 (backport #14071) #14091

Merged
merged 2 commits into from Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -42,6 +42,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (deps) Bump Tendermint version to [v0.34.24](https://github.com/tendermint/tendermint/releases/tag/v0.34.24).
* [#13651](https://github.com/cosmos/cosmos-sdk/pull/13651) Update `server/config/config.GetConfig` function.

### State Machine Breaking

* (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.

### Bug Fixes

* (baseapp) [#14049](https://github.com/cosmos/cosmos-sdk/pull/14049) Fix state sync when interval is zero.
Expand Down
4 changes: 3 additions & 1 deletion x/group/keeper/keeper.go
Expand Up @@ -401,7 +401,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 @@ -410,6 +410,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
// REJECTED.
}
return nil
}
71 changes: 71 additions & 0 deletions x/group/keeper/keeper_test.go
Expand Up @@ -3098,3 +3098,74 @@ func (s *TestSuite) TestTallyProposalsAtVPEnd() {
s.Require().NoError(s.app.GroupKeeper.TallyProposalsAtVPEnd(ctx))
s.NotPanics(func() { module.EndBlocker(ctx, s.app.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))

groupRes, err := s.app.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.app.GroupKeeper.SubmitProposal(s.ctx, &group.MsgSubmitProposal{
GroupPolicyAddress: accountAddr,
Proposers: []string{addr1.String()},
Messages: nil,
})
s.Require().NoError(err)

// group members vote
_, err = s.app.GroupKeeper.Vote(s.ctx, &group.MsgVote{
ProposalId: proposalRes.ProposalId,
Voter: addr1.String(),
Option: group.VOTE_OPTION_NO,
})
s.Require().NoError(err)
_, err = s.app.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.app.GroupKeeper.TallyProposalsAtVPEnd(ctx))
s.NotPanics(func() { module.EndBlocker(ctx, s.app.GroupKeeper) })

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

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