refactor: remove query by events for x/gov deposits

CHANGELOG.md

@@ -51,6 +51,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
   the Tx Factory as methods.
 * (x/slashing) [\#9458](https://github.com/cosmos/cosmos-sdk/pull/9458) Coins burned from slashing is now returned from Slash function and included in Slash event.
 * [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error.
+* [\#9519](https://github.com/cosmos/cosmos-sdk/pull/9519) `DeleteDeposits` now renamed to `DeleteAndBurnDeposits`, also now deposits won't be removed from state for completed proposals unless a proposal is vetoed.
 * (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Removed deprecated `clientCtx.JSONCodec` from `client.Context`.
 * (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Rename `EncodingConfig.Marshaler` to `Codec`.
 

x/gov/abci.go

@@ -19,7 +19,7 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) {
 	// delete inactive proposal from store and its deposits
 	keeper.IterateInactiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal types.Proposal) bool {
 		keeper.DeleteProposal(ctx, proposal.ProposalId)
-		keeper.DeleteDeposits(ctx, proposal.ProposalId)
+		keeper.DeleteAndBurnDeposits(ctx, proposal.ProposalId)
 
 		// called when proposal become inactive
 		keeper.AfterProposalFailedMinDeposit(ctx, proposal.ProposalId)
@@ -49,9 +49,7 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) {
 
 		passes, burnDeposits, tallyResults := keeper.Tally(ctx, proposal)
 
-		if burnDeposits {
-			keeper.DeleteDeposits(ctx, proposal.ProposalId)
-		} else {
+		if !burnDeposits {
 			keeper.RefundDeposits(ctx, proposal.ProposalId)
 		}
 

x/gov/client/cli/query.go

@@ -366,31 +366,14 @@ $ %s query gov deposit 1 cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk
 
 			// check to see if the proposal is in the store
 			ctx := cmd.Context()
-			proposalRes, err := queryClient.Proposal(
+			_, err = queryClient.Proposal(
 				ctx,
 				&types.QueryProposalRequest{ProposalId: proposalID},
 			)
 			if err != nil {
 				return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
 			}
 
-			depositorAddr, err := sdk.AccAddressFromBech32(args[1])
-			if err != nil {
-				return err
-			}
-
-			var deposit types.Deposit
-			propStatus := proposalRes.Proposal.Status
-			if !(propStatus == types.StatusVotingPeriod || propStatus == types.StatusDepositPeriod) {
-				params := types.NewQueryDepositParams(proposalID, depositorAddr)
-				resByTxQuery, err := gcutils.QueryDepositByTxQuery(clientCtx, params)
-				if err != nil {
-					return err
-				}
-				clientCtx.Codec.MustUnmarshalJSON(resByTxQuery, &deposit)
-				return clientCtx.PrintProto(&deposit)
-			}
-
 			res, err := queryClient.Deposit(
 				ctx,
 				&types.QueryDepositRequest{ProposalId: proposalID, Depositor: args[1]},
@@ -439,30 +422,14 @@ $ %s query gov deposits 1
 
 			// check to see if the proposal is in the store
 			ctx := cmd.Context()
-			proposalRes, err := queryClient.Proposal(
+			_, err = queryClient.Proposal(
 				ctx,
 				&types.QueryProposalRequest{ProposalId: proposalID},
 			)
 			if err != nil {
 				return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
 			}
 
-			propStatus := proposalRes.GetProposal().Status
-			if !(propStatus == types.StatusVotingPeriod || propStatus == types.StatusDepositPeriod) {
-				params := types.NewQueryProposalParams(proposalID)
-				resByTxQuery, err := gcutils.QueryDepositsByTxQuery(clientCtx, params)
-				if err != nil {
-					return err
-				}
-
-				var dep types.Deposits
-				// TODO migrate to use JSONCodec (implement MarshalJSONArray
-				// or wrap lists of proto.Message in some other message)
-				clientCtx.LegacyAmino.MustUnmarshalJSON(resByTxQuery, &dep)
-
-				return clientCtx.PrintObjectLegacy(dep)
-			}
-
 			pageReq, err := client.ReadPageRequest(cmd.Flags())
 			if err != nil {
 				return err

x/gov/client/testutil/deposits.go

@@ -16,9 +16,10 @@ import (
 type DepositTestSuite struct {
 	suite.Suite
 
-	cfg     network.Config
-	network *network.Network
-	fees    string
+	cfg         network.Config
+	network     *network.Network
+	deposits    sdk.Coins
+	proposalIDs []string
 }
 
 func NewDepositTestSuite(cfg network.Config) *DepositTestSuite {
@@ -34,86 +35,82 @@ func (s *DepositTestSuite) SetupSuite() {
 
 	_, err = s.network.WaitForHeight(1)
 	s.Require().NoError(err)
-	s.fees = sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20))).String()
 
+	val := s.network.Validators[0]
+
+	deposits := sdk.Coins{
+		sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(0)),
+		sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(50))),
+		sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens),
+	}
+	s.deposits = deposits
+
+	// create 3 proposals for testing
+	for i := 0; i < len(deposits); i++ {
+		var exactArgs []string
+		id := i + 1
+
+		if !deposits[i].IsZero() {
+			exactArgs = append(exactArgs, fmt.Sprintf("--%s=%s", cli.FlagDeposit, deposits[i]))
+		}
+
+		_, err := MsgSubmitProposal(
+			val.ClientCtx,
+			val.Address.String(),
+			fmt.Sprintf("Text Proposal %d", id),
+			"Where is the title!?",
+			types.ProposalTypeText,
+			exactArgs...,
+		)
+
+		s.Require().NoError(err)
+		_, err = s.network.WaitForHeight(1)
+		s.Require().NoError(err)
+		s.proposalIDs = append(s.proposalIDs, fmt.Sprintf("%d", id))
+	}
 }
 
 func (s *DepositTestSuite) TearDownSuite() {
 	s.T().Log("tearing down test suite")
 	s.network.Cleanup()
 }
 
-func (s *DepositTestSuite) TestQueryDepositsInitialDeposit() {
-	val := s.network.Validators[0]
-	clientCtx := val.ClientCtx
-	initialDeposit := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(20))).String()
-
-	// create a proposal with deposit
-	_, err := MsgSubmitProposal(val.ClientCtx, val.Address.String(),
-		"Text Proposal 1", "Where is the title!?", types.ProposalTypeText,
-		fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit))
-	s.Require().NoError(err)
-
-	// deposit more amount
-	_, err = MsgDeposit(clientCtx, val.Address.String(), "1", sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(50)).String())
-	s.Require().NoError(err)
-
-	// waiting for voting period to end
-	time.Sleep(20 * time.Second)
-
-	// query deposit & verify initial deposit
-	deposit := s.queryDeposit(val, "1", false)
-	s.Require().Equal(deposit.Amount.String(), initialDeposit)
-
-	// query deposits
-	deposits := s.queryDeposits(val, "1", false)
-	s.Require().Equal(len(deposits), 2)
-	// verify initial deposit
-	s.Require().Equal(deposits[0].Amount.String(), initialDeposit)
-}
-
 func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() {
 	val := s.network.Validators[0]
 	clientCtx := val.ClientCtx
-
-	// create a proposal without deposit
-	_, err := MsgSubmitProposal(val.ClientCtx, val.Address.String(),
-		"Text Proposal 2", "Where is the title!?", types.ProposalTypeText)
-	s.Require().NoError(err)
+	proposalID := s.proposalIDs[0]
 
 	// deposit amount
-	_, err = MsgDeposit(clientCtx, val.Address.String(), "2", sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String())
+	depositAmount := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String()
+	_, err := MsgDeposit(clientCtx, val.Address.String(), proposalID, depositAmount)
 	s.Require().NoError(err)
 
 	// waiting for voting period to end
-	time.Sleep(20 * time.Second)
+	_, err = s.network.WaitForHeight(2)
+	s.Require().NoError(err)
 
 	// query deposit
-	deposit := s.queryDeposit(val, "2", false)
-	s.Require().Equal(deposit.Amount.String(), sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String())
+	deposit := s.queryDeposit(val, proposalID, false, "")
+	s.Require().NotNil(deposit)
+	s.Require().Equal(deposit.Amount.String(), depositAmount)
 
 	// query deposits
-	deposits := s.queryDeposits(val, "2", false)
-	s.Require().Equal(len(deposits), 1)
+	deposits := s.queryDeposits(val, proposalID, false, "")
+	s.Require().NotNil(deposits)
+	s.Require().Len(deposits.Deposits, 1)
 	// verify initial deposit
-	s.Require().Equal(deposits[0].Amount.String(), sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String())
+	s.Require().Equal(deposits.Deposits[0].Amount.String(), depositAmount)
 }
 
 func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() {
 	val := s.network.Validators[0]
 	clientCtx := val.ClientCtx
-	initialDeposit := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(2000))).String()
-
-	// create a proposal with deposit
-	_, err := MsgSubmitProposal(val.ClientCtx, val.Address.String(),
-		"Text Proposal 3", "Where is the title!?", types.ProposalTypeText,
-		fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit))
-	s.Require().NoError(err)
+	proposalID := s.proposalIDs[1]
 
 	// query proposal
-	args := []string{"3", fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
+	args := []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
 	cmd := cli.GetCmdQueryProposal()
-	_, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
+	_, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
 	s.Require().NoError(err)
 
 	// waiting for deposit period to end
@@ -122,23 +119,18 @@ func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() {
 	// query proposal
 	_, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
 	s.Require().Error(err)
-	s.Require().Contains(err.Error(), "proposal 3 doesn't exist")
+	s.Require().Contains(err.Error(), fmt.Sprintf("proposal %s doesn't exist", proposalID))
 }
 
 func (s *DepositTestSuite) TestRejectedProposalDeposits() {
 	val := s.network.Validators[0]
 	clientCtx := val.ClientCtx
-	initialDeposit := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens)
-
-	// create a proposal with deposit
-	_, err := MsgSubmitProposal(clientCtx, val.Address.String(),
-		"Text Proposal 4", "Where is the title!?", types.ProposalTypeText,
-		fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit))
-	s.Require().NoError(err)
+	initialDeposit := s.deposits[2]
+	proposalID := s.proposalIDs[2]
 
 	// query deposits
 	var deposits types.QueryDepositsResponse
-	args := []string{"4", fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
+	args := []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
 	cmd := cli.GetCmdQueryDeposits()
 	out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
 	s.Require().NoError(err)
@@ -148,48 +140,53 @@ func (s *DepositTestSuite) TestRejectedProposalDeposits() {
 	s.Require().Equal(deposits.Deposits[0].Amount.String(), sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens).String())
 
 	// vote
-	_, err = MsgVote(clientCtx, val.Address.String(), "4", "no")
+	_, err = MsgVote(clientCtx, val.Address.String(), proposalID, "no")
 	s.Require().NoError(err)
 
-	time.Sleep(20 * time.Second)
+	_, err = s.network.WaitForHeight(3)
+	s.Require().NoError(err)
 
-	args = []string{"4", fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
+	args = []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
 	cmd = cli.GetCmdQueryProposal()
 	_, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
 	s.Require().NoError(err)
 
 	// query deposits
-	depositsRes := s.queryDeposits(val, "4", false)
-	s.Require().Equal(len(depositsRes), 1)
+	depositsRes := s.queryDeposits(val, proposalID, false, "")
+	s.Require().NotNil(depositsRes)
+	s.Require().Len(depositsRes.Deposits, 1)
 	// verify initial deposit
-	s.Require().Equal(depositsRes[0].Amount.String(), initialDeposit.String())
-
+	s.Require().Equal(depositsRes.Deposits[0].Amount.String(), initialDeposit.String())
 }
 
-func (s *DepositTestSuite) queryDeposits(val *network.Validator, proposalID string, exceptErr bool) types.Deposits {
+func (s *DepositTestSuite) queryDeposits(val *network.Validator, proposalID string, exceptErr bool, message string) *types.QueryDepositsResponse {
 	args := []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
-	var depositsRes types.Deposits
+	var depositsRes *types.QueryDepositsResponse
 	cmd := cli.GetCmdQueryDeposits()
 	out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
+
 	if exceptErr {
 		s.Require().Error(err)
+		s.Require().Contains(err.Error(), message)
 		return nil
 	}
+
 	s.Require().NoError(err)
 	s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(out.Bytes(), &depositsRes))
 	return depositsRes
 }
 
-func (s *DepositTestSuite) queryDeposit(val *network.Validator, proposalID string, exceptErr bool) *types.Deposit {
+func (s *DepositTestSuite) queryDeposit(val *network.Validator, proposalID string, exceptErr bool, message string) *types.Deposit {
 	args := []string{proposalID, val.Address.String(), fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
-	var depositRes types.Deposit
+	var depositRes *types.Deposit
 	cmd := cli.GetCmdQueryDeposit()
 	out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
 	if exceptErr {
 		s.Require().Error(err)
+		s.Require().Contains(err.Error(), message)
 		return nil
 	}
 	s.Require().NoError(err)
 	s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(out.Bytes(), &depositRes))
-	return &depositRes
+	return depositRes
 }

x/gov/genesis.go

@@ -48,8 +48,8 @@ func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k
 	}
 
 	// check if total deposits equals balance, if it doesn't panic because there were export/import errors
-	if !balance.IsEqual(totalDeposits) {
-		panic(fmt.Sprintf("expected module account was %s but we got %s", balance.String(), totalDeposits.String()))
+	if totalDeposits.IsAllGT(balance) {
+		panic(fmt.Sprintf("expected module account balance to be less than %s but got %s", totalDeposits.String(), balance.String()))
 	}
 }
 

x/gov/keeper/deposit.go

@@ -53,8 +53,8 @@ func (keeper Keeper) GetDeposits(ctx sdk.Context, proposalID uint64) (deposits t
 	return
 }
 
-// DeleteDeposits deletes all the deposits on a specific proposal without refunding them
-func (keeper Keeper) DeleteDeposits(ctx sdk.Context, proposalID uint64) {
+// DeleteAndBurnDeposits deletes and burn all the deposits on a specific proposal.
+func (keeper Keeper) DeleteAndBurnDeposits(ctx sdk.Context, proposalID uint64) {
 	store := ctx.KVStore(keeper.storeKey)
 
 	keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool {
@@ -166,10 +166,8 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd
 	return activatedVotingPeriod, nil
 }
 
-// RefundDeposits refunds and deletes all the deposits on a specific proposal
+// RefundDeposits refunds all the deposits on a specific proposal.
 func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) {
-	store := ctx.KVStore(keeper.storeKey)
-
 	keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool {
 		depositor, err := sdk.AccAddressFromBech32(deposit.Depositor)
 		if err != nil {
@@ -181,7 +179,6 @@ func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) {
 			panic(err)
 		}
 
-		store.Delete(types.DepositKey(proposalID, depositor))
 		return false
 	})
 }

x/gov/keeper/deposit_test.go

@@ -97,14 +97,18 @@ func TestDeposits(t *testing.T) {
 	require.Equal(t, fourStake, deposit.Amount)
 	app.GovKeeper.RefundDeposits(ctx, proposalID)
 	deposit, found = app.GovKeeper.GetDeposit(ctx, proposalID, TestAddrs[1])
-	require.False(t, found)
+	require.True(t, found) // deposits will be refunded but not deleted from store
 	require.Equal(t, addr0Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[0]))
 	require.Equal(t, addr1Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[1]))
 
-	// Test delete deposits
+	// Test delete and burn deposits
+	proposal, err = app.GovKeeper.SubmitProposal(ctx, tp)
+	require.NoError(t, err)
+	proposalID = proposal.ProposalId
 	_, err = app.GovKeeper.AddDeposit(ctx, proposalID, TestAddrs[0], fourStake)
 	require.NoError(t, err)
-	app.GovKeeper.DeleteDeposits(ctx, proposalID)
+	app.GovKeeper.DeleteAndBurnDeposits(ctx, proposalID)
 	deposits = app.GovKeeper.GetDeposits(ctx, proposalID)
 	require.Len(t, deposits, 0)
+	require.Equal(t, addr0Initial.Sub(fourStake), app.BankKeeper.GetAllBalances(ctx, TestAddrs[0]))
 }