From 6340f6d049fa0a915ae0aaf012610a26baaa2570 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 29 Nov 2022 16:27:09 +0100 Subject: [PATCH 1/4] fix(group)!: Don't re-tally proposals after VP end --- x/group/keeper/keeper.go | 4 +- x/group/keeper/keeper_test.go | 72 +++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/x/group/keeper/keeper.go b/x/group/keeper/keeper.go index 3c4b4baa191b..6956bfe84553 100644 --- a/x/group/keeper/keeper.go +++ b/x/group/keeper/keeper.go @@ -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") } @@ -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 } diff --git a/x/group/keeper/keeper_test.go b/x/group/keeper/keeper_test.go index 0e9197cb2715..f27b175998b8 100644 --- a/x/group/keeper/keeper_test.go +++ b/x/group/keeper/keeper_test.go @@ -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) }) +} From 21f276da6064d260b0e4b95ef4f0a6806bb3e64e Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 29 Nov 2022 16:33:41 +0100 Subject: [PATCH 2/4] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c4be6e24e32..f49ee37b438b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. From a501ec76a9bd00c98c0bea9dc562289ccf28c227 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 29 Nov 2022 16:42:05 +0100 Subject: [PATCH 3/4] typo --- x/group/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/group/keeper/keeper.go b/x/group/keeper/keeper.go index 6956bfe84553..9be57877c754 100644 --- a/x/group/keeper/keeper.go +++ b/x/group/keeper/keeper.go @@ -407,7 +407,7 @@ func (k Keeper) TallyProposalsAtVPEnd(ctx sdk.Context) error { if err := k.pruneVotes(ctx, proposalID); err != nil { return err } - } else if proposal.Status != group.PROPOSAL_STATUS_SUBMITTED { + } else if proposal.Status == group.PROPOSAL_STATUS_SUBMITTED { if err := k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo); err != nil { return sdkerrors.Wrap(err, "doTallyAndUpdate") } From 66bedfb9299fdc99245b39576cd97733c1506d17 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 29 Nov 2022 22:02:47 +0100 Subject: [PATCH 4/4] Update x/group/keeper/keeper.go Co-authored-by: Aleksandr Bezobchuk --- x/group/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/group/keeper/keeper.go b/x/group/keeper/keeper.go index 9be57877c754..0b6dcbfed0f3 100644 --- a/x/group/keeper/keeper.go +++ b/x/group/keeper/keeper.go @@ -416,7 +416,7 @@ 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 + // Note: We do nothing if the proposal has been marked as ACCEPTED or // REJECTED. } return nil