Skip to content

Commit

Permalink
fix(group)!: Don't re-tally proposals after VP end (#14071)
Browse files Browse the repository at this point in the history
  • Loading branch information
amaury1093 authored Nov 30, 2022
1 parent e395e43 commit 4ebe3aa
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,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
Original file line number Diff line number Diff line change
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
// REJECTED.
}
return nil
}
72 changes: 72 additions & 0 deletions x/group/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3263,3 +3263,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) })
}

0 comments on commit 4ebe3aa

Please sign in to comment.