fix: delete all votes when proposal is cancelled (#16824)

cosmos · Jul 5, 2023 · 9dcf870 · 9dcf870
1 parent 3138a47
commit 9dcf870
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 31 deletions.
diff --git a/x/gov/keeper/proposal_test.go b/x/gov/keeper/proposal_test.go
@@ -178,72 +178,120 @@ func (suite *KeeperTestSuite) TestCancelProposal() {
 	tp := v1beta1.TextProposal{Title: "title", Description: "description"}
 	prop, err := v1.NewLegacyContent(&tp, govAcct)
 	suite.Require().NoError(err)
-	proposalResp, err := suite.govKeeper.SubmitProposal(suite.ctx, []sdk.Msg{prop}, "", "title", "summary", suite.addrs[0], false)
+	proposal, err := suite.govKeeper.SubmitProposal(suite.ctx, []sdk.Msg{prop}, "", "title", "summary", suite.addrs[0], false)
 	suite.Require().NoError(err)
-	proposalID := proposalResp.Id
+	proposalID := proposal.Id
 
-	proposal2Resp, err := suite.govKeeper.SubmitProposal(suite.ctx, []sdk.Msg{prop}, "", "title", "summary", suite.addrs[1], true)
-	proposal2ID := proposal2Resp.Id
-	makeProposalPass := func() {
-		proposal2, err := suite.govKeeper.Proposals.Get(suite.ctx, proposal2ID)
-		suite.Require().Nil(err)
+	proposal2, err := suite.govKeeper.SubmitProposal(suite.ctx, []sdk.Msg{prop}, "", "title", "summary", suite.addrs[1], true)
+	suite.Require().NoError(err)
+	proposal2ID := proposal2.Id
 
-		proposal2.Status = v1.ProposalStatus_PROPOSAL_STATUS_PASSED
-		suite.govKeeper.SetProposal(suite.ctx, proposal2)
-	}
+	// proposal3 is only used to check the votes for proposals which doesn't go through `CancelProposal` are still present in state
+	proposal3, err := suite.govKeeper.SubmitProposal(suite.ctx, []sdk.Msg{prop}, "", "title", "summary", suite.addrs[2], false)
+	suite.Require().NoError(err)
+	proposal3ID := proposal3.Id
+
+	// add votes for proposal 3
+	suite.Require().NoError(suite.govKeeper.ActivateVotingPeriod(suite.ctx, proposal3))
+
+	proposal3, err = suite.govKeeper.Proposals.Get(suite.ctx, proposal3ID)
+	suite.Require().Nil(err)
+	suite.Require().True(proposal3.VotingStartTime.Equal(suite.ctx.BlockHeader().Time))
+	// add vote
+	voteOptions := []*v1.WeightedVoteOption{{Option: v1.OptionYes, Weight: "1.0"}}
+	err = suite.govKeeper.AddVote(suite.ctx, proposal3ID, suite.addrs[0], voteOptions, "")
+	suite.Require().NoError(err)
 
 	testCases := []struct {
 		name        string
+		malleate    func() (proposalID uint64, proposer string)
 		proposalID  uint64
 		proposer    string
 		expectedErr bool
 	}{
 		{
-			name:        "without proposer",
-			proposalID:  1,
-			proposer:    "",
+			name: "without proposer",
+			malleate: func() (uint64, string) {
+				return 1, ""
+			},
 			expectedErr: true,
 		},
 		{
-			name:        "invalid proposal id",
-			proposalID:  1,
-			proposer:    string(suite.addrs[0]),
+			name: "invalid proposal id",
+			malleate: func() (uint64, string) {
+				return 1, suite.addrs[1].String()
+			},
 			expectedErr: true,
 		},
 		{
-			name:        "valid proposalID but invalid proposer",
-			proposalID:  proposalID,
-			proposer:    suite.addrs[1].String(),
+			name: "valid proposalID but invalid proposer",
+			malleate: func() (uint64, string) {
+				return proposalID, suite.addrs[1].String()
+			},
 			expectedErr: true,
 		},
 		{
-			name:        "valid proposalID but invalid proposal which has already passed",
-			proposalID:  proposal2ID,
-			proposer:    suite.addrs[1].String(),
+			name: "valid proposalID but invalid proposal which has already passed",
+			malleate: func() (uint64, string) {
+				// making proposal status pass
+				proposal2, err := suite.govKeeper.Proposals.Get(suite.ctx, proposal2ID)
+				suite.Require().Nil(err)
+
+				proposal2.Status = v1.ProposalStatus_PROPOSAL_STATUS_PASSED
+				suite.govKeeper.SetProposal(suite.ctx, proposal2)
+
+				return proposal2ID, suite.addrs[1].String()
+			},
 			expectedErr: true,
 		},
 		{
-			name:        "valid proposer and proposal id",
-			proposalID:  proposalID,
-			proposer:    suite.addrs[0].String(),
+			name: "valid proposer and proposal id",
+			malleate: func() (uint64, string) {
+				return proposalID, suite.addrs[0].String()
+			},
+			expectedErr: false,
+		},
+		{
+			name: "valid case with deletion of votes",
+			malleate: func() (uint64, string) {
+				suite.Require().NoError(suite.govKeeper.ActivateVotingPeriod(suite.ctx, proposal))
+
+				proposal, err = suite.govKeeper.Proposals.Get(suite.ctx, proposal.Id)
+				suite.Require().Nil(err)
+				suite.Require().True(proposal.VotingStartTime.Equal(suite.ctx.BlockHeader().Time))
+
+				// add vote
+				voteOptions := []*v1.WeightedVoteOption{{Option: v1.OptionYes, Weight: "1.0"}}
+				err = suite.govKeeper.AddVote(suite.ctx, proposalID, suite.addrs[0], voteOptions, "")
+				suite.Require().NoError(err)
+				vote, err := suite.govKeeper.Votes.Get(suite.ctx, collections.Join(proposalID, suite.addrs[0]))
+				suite.Require().NoError(err)
+				suite.Require().NotNil(vote)
+
+				return proposalID, suite.addrs[0].String()
+			},
 			expectedErr: false,
 		},
 	}
 
 	for _, tc := range testCases {
 		suite.Run(tc.name, func() {
-			if tc.proposalID == proposal2ID {
-				// making proposal status pass
-				makeProposalPass()
-			}
-			err = suite.govKeeper.CancelProposal(suite.ctx, tc.proposalID, tc.proposer)
+			pID, proposer := tc.malleate()
+			err = suite.govKeeper.CancelProposal(suite.ctx, pID, proposer)
 			if tc.expectedErr {
 				suite.Require().Error(err)
 			} else {
 				suite.Require().NoError(err)
 			}
 		})
 	}
+	_, err = suite.govKeeper.Votes.Get(suite.ctx, collections.Join(proposalID, suite.addrs[0]))
+	suite.Require().ErrorContains(err, collections.ErrNotFound.Error())
+
+	// check that proposal 3 votes are still present in the state
+	votes, err := suite.govKeeper.Votes.Get(suite.ctx, collections.Join(proposal3ID, suite.addrs[0]))
+	suite.Require().NoError(err)
+	suite.Require().NotNil(votes)
 }
 
 func TestMigrateProposalMessages(t *testing.T) {

diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go
@@ -58,6 +58,11 @@ func (keeper Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr s
 
 // deleteVotes deletes the all votes from a given proposalID.
 func (keeper Keeper) deleteVotes(ctx context.Context, proposalID uint64) error {
-	// TODO(tip): fix https://github.com/cosmos/cosmos-sdk/issues/16162
+	rng := collections.NewPrefixedPairRange[uint64, sdk.AccAddress](proposalID)
+	err := keeper.Votes.Clear(ctx, rng)
+	if err != nil {
+		return err
+	}
+
 	return nil
 }