refactor: remove query by events for x/gov deposits (cosmos#9519)

<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

Closes: cosmos#9419 

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review. -->

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [x] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [x] reviewed API design and naming
- [x] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

CHANGELOG.md

@@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (client/keys) [\#9407](https://github.com/cosmos/cosmos-sdk/pull/9601) Added `keys rename` CLI command and `Keyring.Rename` interface method to rename a key in the keyring.
 * (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` renamed to `DeleteAndBurnDeposits`, `RefundDeposits` renamed to `RefundAndDeleteDeposits` 
 * (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`.
 * [\#9418](https://github.com/cosmos/cosmos-sdk/pull/9418) `sdk.Msg`'s  `GetSigners()` method updated to return `[]string`.

x/gov/abci.go

@@ -16,10 +16,10 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) {
 
 	logger := keeper.Logger(ctx)
 
-	// delete inactive proposal from store and its deposits
+	// delete dead proposals from store and burn theirs deposits. A proposal is dead when it's inactive and didn't get enough deposit on time to get into voting phase.
 	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)
@@ -50,9 +50,9 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) {
 		passes, burnDeposits, tallyResults := keeper.Tally(ctx, proposal)
 
 		if burnDeposits {
-			keeper.DeleteDeposits(ctx, proposal.ProposalId)
+			keeper.DeleteAndBurnDeposits(ctx, proposal.ProposalId)
 		} else {
-			keeper.RefundDeposits(ctx, proposal.ProposalId)
+			keeper.RefundAndDeleteDeposits(ctx, proposal.ProposalId)
 		}
 
 		if passes {

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/cli_test.go

@@ -1,5 +1,3 @@
-// +build norace
-
 package testutil
 
 import (

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,98 @@ 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]
 
-func (s *DepositTestSuite) TearDownSuite() {
-	s.T().Log("tearing down test suite")
-	s.network.Cleanup()
+	deposits := sdk.Coins{
+		sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(0)),
+		sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(50))),
+	}
+	s.deposits = deposits
+
+	// create 2 proposals for testing
+	for i := 0; i < len(deposits); i++ {
+		id := i + 1
+		deposit := deposits[i]
+
+		s.createProposal(val, deposit, id)
+		s.proposalIDs = append(s.proposalIDs, fmt.Sprintf("%d", id))
+	}
 }
 
-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()
+func (s *DepositTestSuite) SetupNewSuite() {
+	s.T().Log("setting up new test suite")
 
-	// 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))
+	var err error
+	s.network, err = network.New(s.T(), s.T().TempDir(), s.cfg)
 	s.Require().NoError(err)
 
-	// deposit more amount
-	_, err = MsgDeposit(clientCtx, val.Address.String(), "1", sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(50)).String())
+	_, err = s.network.WaitForHeight(1)
 	s.Require().NoError(err)
+}
 
-	// waiting for voting period to end
-	time.Sleep(20 * time.Second)
+func (s *DepositTestSuite) createProposal(val *network.Validator, initialDeposit sdk.Coin, id int) {
+	var exactArgs []string
 
-	// query deposit & verify initial deposit
-	deposit := s.queryDeposit(val, "1", false)
-	s.Require().Equal(deposit.Amount.String(), initialDeposit)
+	if !initialDeposit.IsZero() {
+		exactArgs = append(exactArgs, fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit.String()))
+	}
 
-	// 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)
+	_, 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)
+}
+
+func (s *DepositTestSuite) TearDownSuite() {
+	s.T().Log("tearing down test suite")
+	s.network.Cleanup()
 }
 
 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,74 +135,81 @@ 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() {
+	// resetting state required (proposal is getting removed from state and proposalID is not in sequence)
+	s.TearDownSuite()
+	s.SetupNewSuite()
+
 	val := s.network.Validators[0]
 	clientCtx := val.ClientCtx
 	initialDeposit := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens)
+	id := 1
+	proposalID := fmt.Sprintf("%d", id)
 
-	// 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)
+	s.createProposal(val, initialDeposit, id)
 
 	// 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)
 	s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(out.Bytes(), &deposits))
 	s.Require().Equal(len(deposits.Deposits), 1)
 	// verify initial deposit
-	s.Require().Equal(deposits.Deposits[0].Amount.String(), sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens).String())
+	s.Require().Equal(deposits.Deposits[0].Amount.String(), initialDeposit.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/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,8 +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
-func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) {
+// RefundAndDeleteDeposits refunds and deletes all the deposits on a specific proposal.
+func (keeper Keeper) RefundAndDeleteDeposits(ctx sdk.Context, proposalID uint64) {
 	store := ctx.KVStore(keeper.storeKey)
 
 	keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool {