From 71040f42e441a4547e9df4d794aefbc8b837af89 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 15 Jun 2021 11:09:59 +0530 Subject: [PATCH 01/25] refactor: remove query by events for deposits --- x/gov/abci.go | 4 +- x/gov/client/cli/query.go | 60 +++++------ x/gov/client/testutil/cli_test.go | 2 - x/gov/client/testutil/deposits.go | 165 +++++++++++++++++++----------- x/gov/keeper/deposit.go | 16 ++- 5 files changed, 154 insertions(+), 93 deletions(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index 04d6cedf5ac9..ed7b800be7df 100644 --- a/x/gov/abci.go +++ b/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) @@ -50,7 +50,7 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) { passes, burnDeposits, tallyResults := keeper.Tally(ctx, proposal) if burnDeposits { - keeper.DeleteDeposits(ctx, proposal.ProposalId) + keeper.BurnDeposits(ctx, proposal.ProposalId) } else { keeper.RefundDeposits(ctx, proposal.ProposalId) } diff --git a/x/gov/client/cli/query.go b/x/gov/client/cli/query.go index 34d4665a99d4..c96814f9fd54 100644 --- a/x/gov/client/cli/query.go +++ b/x/gov/client/cli/query.go @@ -366,7 +366,7 @@ $ %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}, ) @@ -374,22 +374,22 @@ $ %s query gov deposit 1 cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err) } - depositorAddr, err := sdk.AccAddressFromBech32(args[1]) - if err != nil { - return 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.JSONCodec.MustUnmarshalJSON(resByTxQuery, &deposit) - return clientCtx.PrintProto(&deposit) - } + // 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.JSONCodec.MustUnmarshalJSON(resByTxQuery, &deposit) + // return clientCtx.PrintProto(&deposit) + // } res, err := queryClient.Deposit( ctx, @@ -439,7 +439,7 @@ $ %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}, ) @@ -447,21 +447,21 @@ $ %s query gov deposits 1 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 - } + // 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) + // 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) - } + // return clientCtx.PrintObjectLegacy(dep) + // } pageReq, err := client.ReadPageRequest(cmd.Flags()) if err != nil { diff --git a/x/gov/client/testutil/cli_test.go b/x/gov/client/testutil/cli_test.go index 713c33f12182..1376be6c2b21 100644 --- a/x/gov/client/testutil/cli_test.go +++ b/x/gov/client/testutil/cli_test.go @@ -1,5 +1,3 @@ -// +build norace - package testutil import ( diff --git a/x/gov/client/testutil/deposits.go b/x/gov/client/testutil/deposits.go index 57c3c9363330..96d6ba787b00 100644 --- a/x/gov/client/testutil/deposits.go +++ b/x/gov/client/testutil/deposits.go @@ -4,9 +4,13 @@ import ( "fmt" "time" + "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/crypto/hd" + "github.com/cosmos/cosmos-sdk/crypto/keyring" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" "github.com/cosmos/cosmos-sdk/testutil/network" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" "github.com/cosmos/cosmos-sdk/x/gov/client/cli" "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/stretchr/testify/suite" @@ -16,9 +20,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 { @@ -29,11 +34,42 @@ func (s *DepositTestSuite) SetupSuite() { s.T().Log("setting up test suite") s.network = network.New(s.T(), s.cfg) - _, 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, types.DefaultMinDepositTokens.Sub(sdk.NewInt(20))), + 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 4 proposals for testing + for i := 0; i < 4; 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() { @@ -45,73 +81,86 @@ 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() + proposalId := s.proposalIDs[0] + + commonArgs := []string{ + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10))).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)) + info, _, err := val.ClientCtx.Keyring.NewMnemonic("grantee", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + s.Require().NoError(err) + acc := sdk.AccAddress(info.GetPubKey().Address()) + + sendAmount := sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(150)) + _, err = testutil.MsgSendExec(val.ClientCtx, val.Address, acc, sendAmount, commonArgs...) + s.Require().NoError(err) + + _, err = s.network.WaitForHeight(1) s.Require().NoError(err) // deposit more amount - _, err = MsgDeposit(clientCtx, val.Address.String(), "1", sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(50)).String()) + extraDeposit := sendAmount.Sub(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(50))) + _, err = MsgDeposit(clientCtx, acc.String(), proposalId, extraDeposit.String()) s.Require().NoError(err) // waiting for voting period to end - time.Sleep(20 * time.Second) + // time.Sleep(20 * time.Second) + _, err = s.network.WaitForHeight(3) + s.Require().NoError(err) // query deposit & verify initial deposit - deposit := s.queryDeposit(val, "1", false) + deposit := s.queryDeposit(val, proposalId, false, "") + s.Require().NotNil(deposit) s.Require().Equal(deposit.Amount.String(), initialDeposit) // query deposits - deposits := s.queryDeposits(val, "1", false) - s.Require().Equal(len(deposits), 2) + deposits := s.queryDeposits(val, proposalId, false, "") + s.Require().NotNil(deposits) + s.Require().Len(deposits.Deposits, 2) // verify initial deposit - s.Require().Equal(deposits[0].Amount.String(), initialDeposit) + s.Require().Equal(deposits.Deposits[0].Amount.String(), initialDeposit) } func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() { val := s.network.Validators[0] + // val2 := s.network.Validators[1] 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[1] // 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) + // 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[2] // 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 @@ -120,23 +169,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[3] + proposalId := s.proposalIDs[3] // 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) @@ -146,48 +190,55 @@ 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) + + // time.Sleep(20 * time.Second) - 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 } diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 9275fbff40dc..0e560eef8b55 100644 --- a/x/gov/keeper/deposit.go +++ b/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 burns all the deposits on a specific proposal without refunding them +func (keeper Keeper) DeleteAndBurnDeposits(ctx sdk.Context, proposalID uint64) { store := ctx.KVStore(keeper.storeKey) keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { @@ -72,6 +72,18 @@ func (keeper Keeper) DeleteDeposits(ctx sdk.Context, proposalID uint64) { }) } +// BurnDeposits burns all the deposits on a specific proposal +func (keeper Keeper) BurnDeposits(ctx sdk.Context, proposalID uint64) { + keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { + err := keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, deposit.Amount) + if err != nil { + panic(err) + } + + return false + }) +} + // IterateAllDeposits iterates over the all the stored deposits and performs a callback function func (keeper Keeper) IterateAllDeposits(ctx sdk.Context, cb func(deposit types.Deposit) (stop bool)) { store := ctx.KVStore(keeper.storeKey) From 098d86f1bdd4958fc1585d1528cab127400bfe00 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 15 Jun 2021 11:17:58 +0530 Subject: [PATCH 02/25] revert norace --- x/gov/client/testutil/cli_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/gov/client/testutil/cli_test.go b/x/gov/client/testutil/cli_test.go index 1376be6c2b21..713c33f12182 100644 --- a/x/gov/client/testutil/cli_test.go +++ b/x/gov/client/testutil/cli_test.go @@ -1,3 +1,5 @@ +// +build norace + package testutil import ( From 322c6ac524ad030cdec8cac7ee6fd7e94bcc2c01 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 15 Jun 2021 11:42:09 +0530 Subject: [PATCH 03/25] fix lint --- x/gov/client/testutil/deposits.go | 32 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/x/gov/client/testutil/deposits.go b/x/gov/client/testutil/deposits.go index 96d6ba787b00..389c7b9b3d74 100644 --- a/x/gov/client/testutil/deposits.go +++ b/x/gov/client/testutil/deposits.go @@ -81,7 +81,7 @@ 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() - proposalId := s.proposalIDs[0] + proposalID := s.proposalIDs[0] commonArgs := []string{ fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), @@ -102,7 +102,7 @@ func (s *DepositTestSuite) TestQueryDepositsInitialDeposit() { // deposit more amount extraDeposit := sendAmount.Sub(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(50))) - _, err = MsgDeposit(clientCtx, acc.String(), proposalId, extraDeposit.String()) + _, err = MsgDeposit(clientCtx, acc.String(), proposalID, extraDeposit.String()) s.Require().NoError(err) // waiting for voting period to end @@ -111,12 +111,12 @@ func (s *DepositTestSuite) TestQueryDepositsInitialDeposit() { s.Require().NoError(err) // query deposit & verify initial deposit - deposit := s.queryDeposit(val, proposalId, false, "") + deposit := s.queryDeposit(val, proposalID, false, "") s.Require().NotNil(deposit) s.Require().Equal(deposit.Amount.String(), initialDeposit) // query deposits - deposits := s.queryDeposits(val, proposalId, false, "") + deposits := s.queryDeposits(val, proposalID, false, "") s.Require().NotNil(deposits) s.Require().Len(deposits.Deposits, 2) // verify initial deposit @@ -127,11 +127,11 @@ func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() { val := s.network.Validators[0] // val2 := s.network.Validators[1] clientCtx := val.ClientCtx - proposalId := s.proposalIDs[1] + proposalID := s.proposalIDs[1] // deposit amount depositAmount := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String() - _, err := MsgDeposit(clientCtx, val.Address.String(), proposalId, depositAmount) + _, err := MsgDeposit(clientCtx, val.Address.String(), proposalID, depositAmount) s.Require().NoError(err) // waiting for voting period to end @@ -140,12 +140,12 @@ func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() { s.Require().NoError(err) // query deposit - deposit := s.queryDeposit(val, proposalId, false, "") + deposit := s.queryDeposit(val, proposalID, false, "") s.Require().NotNil(deposit) s.Require().Equal(deposit.Amount.String(), depositAmount) // query deposits - deposits := s.queryDeposits(val, proposalId, false, "") + deposits := s.queryDeposits(val, proposalID, false, "") s.Require().NotNil(deposits) s.Require().Len(deposits.Deposits, 1) // verify initial deposit @@ -155,10 +155,10 @@ func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() { func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() { val := s.network.Validators[0] clientCtx := val.ClientCtx - proposalId := s.proposalIDs[2] + proposalID := s.proposalIDs[2] // query proposal - args := []string{proposalId, 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) @@ -169,18 +169,18 @@ func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() { // query proposal _, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args) s.Require().Error(err) - s.Require().Contains(err.Error(), fmt.Sprintf("proposal %s doesn't exist", proposalId)) + 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 := s.deposits[3] - proposalId := s.proposalIDs[3] + proposalID := s.proposalIDs[3] // query deposits var deposits types.QueryDepositsResponse - args := []string{proposalId, 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) @@ -190,7 +190,7 @@ 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(), proposalId, "no") + _, err = MsgVote(clientCtx, val.Address.String(), proposalID, "no") s.Require().NoError(err) _, err = s.network.WaitForHeight(3) @@ -198,13 +198,13 @@ func (s *DepositTestSuite) TestRejectedProposalDeposits() { // time.Sleep(20 * time.Second) - args = []string{proposalId, 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, proposalId, false, "") + depositsRes := s.queryDeposits(val, proposalID, false, "") s.Require().NotNil(depositsRes) s.Require().Len(depositsRes.Deposits, 1) // verify initial deposit From 45bdadfd9b2d5c059f02148914ed7d26055a21b7 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 15 Jun 2021 11:56:58 +0530 Subject: [PATCH 04/25] fix test --- x/gov/keeper/deposit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 0c8e370681aa..534cf0fe2da5 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -104,7 +104,7 @@ func TestDeposits(t *testing.T) { // Test delete deposits _, 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) } From 296b61f9a6bc2bea790ddb38900d6bdf91ff879d Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 21 Jun 2021 15:23:52 +0530 Subject: [PATCH 05/25] update refund logic --- go.mod | 2 +- go.sum | 6 ++++-- x/gov/abci.go | 6 ++---- x/gov/client/testutil/deposits.go | 11 +++++++---- x/gov/keeper/deposit.go | 20 ++++---------------- x/gov/keeper/deposit_test.go | 2 +- 6 files changed, 19 insertions(+), 28 deletions(-) diff --git a/go.mod b/go.mod index aa3b80eb3332..173d0975863a 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ require ( github.com/prometheus/common v0.29.0 github.com/rakyll/statik v0.1.7 github.com/regen-network/cosmos-proto v0.3.1 - github.com/rs/zerolog v1.22.0 + github.com/rs/zerolog v1.23.0 github.com/spf13/afero v1.3.4 // indirect github.com/spf13/cast v1.3.1 github.com/spf13/cobra v1.1.3 diff --git a/go.sum b/go.sum index a957981684f9..266b5b8b61ee 100644 --- a/go.sum +++ b/go.sum @@ -151,6 +151,7 @@ github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3Ee github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= +github.com/coreos/go-systemd/v22 v22.3.2/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d/go.mod h1:tSxLoYXyBmiFeKpvmq4dzayMdCjCnu8uqmCysIGBT2Y= @@ -267,6 +268,7 @@ github.com/gobwas/ws v1.0.2 h1:CoAavW/wd/kulfZmSIBt6p24n4j7tHgNVCjsfHVNUbo= github.com/gobwas/ws v1.0.2/go.mod h1:szmBTxLgaFppYjEmNtny/v3w89xOydFnnZMcgRRu/EM= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 h1:ZpnhV/YsD2/4cESfV5+Hoeu/iUR3ruzNvZ+yQfO03a0= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4= +github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/gogo/gateway v1.1.0 h1:u0SuhL9+Il+UbjM9VIE3ntfRujKbvVpFvNB4HbjeVQ0= github.com/gogo/gateway v1.1.0/go.mod h1:S7rR8FRQyG3QFESeSv4l2WnsyzlCLG0CzBbUUo/mbic= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= @@ -649,8 +651,8 @@ github.com/rs/cors v1.7.0 h1:+88SsELBHx5r+hZ8TCkggzSstaWNbDvThkVK8H6f9ik= github.com/rs/cors v1.7.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU= github.com/rs/xhandler v0.0.0-20160618193221-ed27b6fd6521/go.mod h1:RvLn4FgxWubrpZHtQLnOf6EwhN2hEMusxZOhcW9H3UQ= github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ= -github.com/rs/zerolog v1.22.0 h1:XrVUjV4K+izZpKXZHlPrYQiDtmdGiCylnT4i43AAWxg= -github.com/rs/zerolog v1.22.0/go.mod h1:ZPhntP/xmq1nnND05hhpAh2QMhSsA4UN3MGZ6O2J3hM= +github.com/rs/zerolog v1.23.0 h1:UskrK+saS9P9Y789yNNulYKdARjPZuS35B8gJF2x60g= +github.com/rs/zerolog v1.23.0/go.mod h1:6c7hFfxPOy7TacJc4Fcdi24/J0NKYGzjG8FWRI916Qo= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= diff --git a/x/gov/abci.go b/x/gov/abci.go index ed7b800be7df..fc5309e6a843 100644 --- a/x/gov/abci.go +++ b/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.DeleteAndBurnDeposits(ctx, proposal.ProposalId) + keeper.DeleteDeposits(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.BurnDeposits(ctx, proposal.ProposalId) - } else { + if !burnDeposits { keeper.RefundDeposits(ctx, proposal.ProposalId) } diff --git a/x/gov/client/testutil/deposits.go b/x/gov/client/testutil/deposits.go index 389c7b9b3d74..1f9d08d76e4f 100644 --- a/x/gov/client/testutil/deposits.go +++ b/x/gov/client/testutil/deposits.go @@ -40,7 +40,7 @@ func (s *DepositTestSuite) SetupSuite() { val := s.network.Validators[0] deposits := sdk.Coins{ - sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(20))), + sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(120))), 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), @@ -80,7 +80,7 @@ func (s *DepositTestSuite) TearDownSuite() { 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() + initialDeposit := s.deposits[0].String() proposalID := s.proposalIDs[0] commonArgs := []string{ @@ -89,11 +89,14 @@ func (s *DepositTestSuite) TestQueryDepositsInitialDeposit() { fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10))).String()), } - info, _, err := val.ClientCtx.Keyring.NewMnemonic("grantee", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + info, _, err := val.ClientCtx.Keyring.NewMnemonic("acc", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) s.Require().NoError(err) acc := sdk.AccAddress(info.GetPubKey().Address()) - sendAmount := sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(150)) + _, err = s.network.WaitForHeight(1) + s.Require().NoError(err) + + sendAmount := sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(200)) _, err = testutil.MsgSendExec(val.ClientCtx, val.Address, acc, sendAmount, commonArgs...) s.Require().NoError(err) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 0e560eef8b55..6a0544044ad5 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -53,8 +53,8 @@ func (keeper Keeper) GetDeposits(ctx sdk.Context, proposalID uint64) (deposits t return } -// DeleteAndBurnDeposits deletes and burns all the deposits on a specific proposal without refunding them -func (keeper Keeper) DeleteAndBurnDeposits(ctx sdk.Context, proposalID uint64) { +// DeleteDeposits deletes all the deposits on a specific proposal without refunding them +func (keeper Keeper) DeleteDeposits(ctx sdk.Context, proposalID uint64) { store := ctx.KVStore(keeper.storeKey) keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { @@ -72,18 +72,6 @@ func (keeper Keeper) DeleteAndBurnDeposits(ctx sdk.Context, proposalID uint64) { }) } -// BurnDeposits burns all the deposits on a specific proposal -func (keeper Keeper) BurnDeposits(ctx sdk.Context, proposalID uint64) { - keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { - err := keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, deposit.Amount) - if err != nil { - panic(err) - } - - return false - }) -} - // IterateAllDeposits iterates over the all the stored deposits and performs a callback function func (keeper Keeper) IterateAllDeposits(ctx sdk.Context, cb func(deposit types.Deposit) (stop bool)) { store := ctx.KVStore(keeper.storeKey) @@ -180,7 +168,7 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd // RefundDeposits refunds and deletes all the deposits on a specific proposal func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) { - store := ctx.KVStore(keeper.storeKey) + // store := ctx.KVStore(keeper.storeKey) keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { depositor, err := sdk.AccAddressFromBech32(deposit.Depositor) @@ -193,7 +181,7 @@ func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) { panic(err) } - store.Delete(types.DepositKey(proposalID, depositor)) + // store.Delete(types.DepositKey(proposalID, depositor)) return false }) } diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 534cf0fe2da5..0c8e370681aa 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -104,7 +104,7 @@ func TestDeposits(t *testing.T) { // Test delete deposits _, err = app.GovKeeper.AddDeposit(ctx, proposalID, TestAddrs[0], fourStake) require.NoError(t, err) - app.GovKeeper.DeleteAndBurnDeposits(ctx, proposalID) + app.GovKeeper.DeleteDeposits(ctx, proposalID) deposits = app.GovKeeper.GetDeposits(ctx, proposalID) require.Len(t, deposits, 0) } From d3f21e94a950d6f3a1d59006be86eac24309d722 Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 21 Jun 2021 16:46:06 +0530 Subject: [PATCH 06/25] fix tests --- x/gov/client/testutil/deposits.go | 54 ------------------------------- 1 file changed, 54 deletions(-) diff --git a/x/gov/client/testutil/deposits.go b/x/gov/client/testutil/deposits.go index 1f9d08d76e4f..50484c5975d2 100644 --- a/x/gov/client/testutil/deposits.go +++ b/x/gov/client/testutil/deposits.go @@ -4,13 +4,9 @@ import ( "fmt" "time" - "github.com/cosmos/cosmos-sdk/client/flags" - "github.com/cosmos/cosmos-sdk/crypto/hd" - "github.com/cosmos/cosmos-sdk/crypto/keyring" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" "github.com/cosmos/cosmos-sdk/testutil/network" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" "github.com/cosmos/cosmos-sdk/x/gov/client/cli" "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/stretchr/testify/suite" @@ -41,7 +37,6 @@ func (s *DepositTestSuite) SetupSuite() { deposits := sdk.Coins{ sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(120))), - 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), } @@ -77,55 +72,6 @@ func (s *DepositTestSuite) TearDownSuite() { s.network.Cleanup() } -func (s *DepositTestSuite) TestQueryDepositsInitialDeposit() { - val := s.network.Validators[0] - clientCtx := val.ClientCtx - initialDeposit := s.deposits[0].String() - proposalID := s.proposalIDs[0] - - commonArgs := []string{ - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10))).String()), - } - - info, _, err := val.ClientCtx.Keyring.NewMnemonic("acc", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) - s.Require().NoError(err) - acc := sdk.AccAddress(info.GetPubKey().Address()) - - _, err = s.network.WaitForHeight(1) - s.Require().NoError(err) - - sendAmount := sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(200)) - _, err = testutil.MsgSendExec(val.ClientCtx, val.Address, acc, sendAmount, commonArgs...) - s.Require().NoError(err) - - _, err = s.network.WaitForHeight(1) - s.Require().NoError(err) - - // deposit more amount - extraDeposit := sendAmount.Sub(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(50))) - _, err = MsgDeposit(clientCtx, acc.String(), proposalID, extraDeposit.String()) - s.Require().NoError(err) - - // waiting for voting period to end - // time.Sleep(20 * time.Second) - _, err = s.network.WaitForHeight(3) - s.Require().NoError(err) - - // query deposit & verify initial deposit - deposit := s.queryDeposit(val, proposalID, false, "") - s.Require().NotNil(deposit) - s.Require().Equal(deposit.Amount.String(), initialDeposit) - - // query deposits - deposits := s.queryDeposits(val, proposalID, false, "") - s.Require().NotNil(deposits) - s.Require().Len(deposits.Deposits, 2) - // verify initial deposit - s.Require().Equal(deposits.Deposits[0].Amount.String(), initialDeposit) -} - func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() { val := s.network.Validators[0] // val2 := s.network.Validators[1] From c1a790bfd6b69de4389c10836136fea0dee4213a Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 21 Jun 2021 17:05:25 +0530 Subject: [PATCH 07/25] fix tests --- x/gov/client/testutil/deposits.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/x/gov/client/testutil/deposits.go b/x/gov/client/testutil/deposits.go index 50484c5975d2..af902df71c98 100644 --- a/x/gov/client/testutil/deposits.go +++ b/x/gov/client/testutil/deposits.go @@ -36,14 +36,14 @@ func (s *DepositTestSuite) SetupSuite() { val := s.network.Validators[0] deposits := sdk.Coins{ - sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(120))), + 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 4 proposals for testing - for i := 0; i < 4; i++ { + for i := 0; i < len(deposits); i++ { var exactArgs []string id := i + 1 @@ -74,9 +74,8 @@ func (s *DepositTestSuite) TearDownSuite() { func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() { val := s.network.Validators[0] - // val2 := s.network.Validators[1] clientCtx := val.ClientCtx - proposalID := s.proposalIDs[1] + proposalID := s.proposalIDs[0] // deposit amount depositAmount := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String() @@ -84,7 +83,6 @@ func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() { s.Require().NoError(err) // waiting for voting period to end - // time.Sleep(20 * time.Second) _, err = s.network.WaitForHeight(2) s.Require().NoError(err) @@ -104,7 +102,7 @@ func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() { func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() { val := s.network.Validators[0] clientCtx := val.ClientCtx - proposalID := s.proposalIDs[2] + proposalID := s.proposalIDs[1] // query proposal args := []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)} @@ -124,8 +122,8 @@ func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() { func (s *DepositTestSuite) TestRejectedProposalDeposits() { val := s.network.Validators[0] clientCtx := val.ClientCtx - initialDeposit := s.deposits[3] - proposalID := s.proposalIDs[3] + initialDeposit := s.deposits[2] + proposalID := s.proposalIDs[2] // query deposits var deposits types.QueryDepositsResponse @@ -145,8 +143,6 @@ func (s *DepositTestSuite) TestRejectedProposalDeposits() { _, err = s.network.WaitForHeight(3) s.Require().NoError(err) - // time.Sleep(20 * time.Second) - args = []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)} cmd = cli.GetCmdQueryProposal() _, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args) From 330dcbc4462c27630380250d1745785e25356da0 Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 23 Jun 2021 13:28:48 +0530 Subject: [PATCH 08/25] update invariants --- x/gov/abci.go | 2 +- x/gov/genesis.go | 2 +- x/gov/keeper/deposit.go | 21 ++++++++++++++++++++- x/gov/keeper/deposit_test.go | 4 ++-- x/gov/keeper/invariants.go | 8 ++++---- 5 files changed, 28 insertions(+), 9 deletions(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index fc5309e6a843..6c64af2598b5 100644 --- a/x/gov/abci.go +++ b/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) diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 62a194d9c0a9..7cff0175c7dd 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -48,7 +48,7 @@ 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) { + if !balance.IsAllLTE(totalDeposits) { panic(fmt.Sprintf("expected module account was %s but we got %s", balance.String(), totalDeposits.String())) } } diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 6a0544044ad5..6ba281bb2bc1 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -53,7 +53,26 @@ func (keeper Keeper) GetDeposits(ctx sdk.Context, proposalID uint64) (deposits t return } -// DeleteDeposits deletes all the deposits on a specific proposal without refunding them +// DeleteAndBurnDeposits deletes all the deposits on a specific proposal without refunding them. +func (keeper Keeper) DeleteAndBurnDeposits(ctx sdk.Context, proposalID uint64) { + store := ctx.KVStore(keeper.storeKey) + + keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { + err := keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, deposit.Amount) + if err != nil { + panic(err) + } + + depositor, err := sdk.AccAddressFromBech32(deposit.Depositor) + if err != nil { + panic(err) + } + store.Delete(types.DepositKey(proposalID, depositor)) + return false + }) +} + +// DeleteDeposits deletes all the deposits. func (keeper Keeper) DeleteDeposits(ctx sdk.Context, proposalID uint64) { store := ctx.KVStore(keeper.storeKey) diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 0c8e370681aa..f4b89610c1f7 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -97,14 +97,14 @@ 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) require.Equal(t, addr0Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[0])) require.Equal(t, addr1Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[1])) // Test delete deposits _, 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) } diff --git a/x/gov/keeper/invariants.go b/x/gov/keeper/invariants.go index d0ec4dbfacc8..fab6a2d283c9 100644 --- a/x/gov/keeper/invariants.go +++ b/x/gov/keeper/invariants.go @@ -25,19 +25,19 @@ func AllInvariants(keeper Keeper, bk types.BankKeeper) sdk.Invariant { // deposit amounts held on store func ModuleAccountInvariant(keeper Keeper, bk types.BankKeeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { - var expectedDeposits sdk.Coins + var deposits sdk.Coins keeper.IterateAllDeposits(ctx, func(deposit types.Deposit) bool { - expectedDeposits = expectedDeposits.Add(deposit.Amount...) + deposits = deposits.Add(deposit.Amount...) return false }) macc := keeper.GetGovernanceAccount(ctx) balances := bk.GetAllBalances(ctx, macc.GetAddress()) - broken := !balances.IsEqual(expectedDeposits) + broken := !balances.IsAllLTE(deposits) return sdk.FormatInvariant(types.ModuleName, "deposits", fmt.Sprintf("\tgov ModuleAccount coins: %s\n\tsum of deposit amounts: %s\n", - balances, expectedDeposits)), broken + balances, deposits)), broken } } From e7eb3dffb023b57a74e803406a9b04a9eeb0ae98 Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 24 Jun 2021 15:30:53 +0530 Subject: [PATCH 09/25] fix tests --- x/gov/keeper/deposit.go | 6 ------ x/gov/keeper/deposit_test.go | 6 +++++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 6ba281bb2bc1..7463a2f3dee2 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -77,11 +77,6 @@ func (keeper Keeper) DeleteDeposits(ctx sdk.Context, proposalID uint64) { store := ctx.KVStore(keeper.storeKey) keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { - err := keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, deposit.Amount) - if err != nil { - panic(err) - } - depositor, err := sdk.AccAddressFromBech32(deposit.Depositor) if err != nil { panic(err) @@ -200,7 +195,6 @@ func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) { panic(err) } - // store.Delete(types.DepositKey(proposalID, depositor)) return false }) } diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index f4b89610c1f7..3386373eff9d 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "testing" "time" @@ -24,6 +25,8 @@ func TestDeposits(t *testing.T) { fourStake := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, app.StakingKeeper.TokensFromConsensusPower(ctx, 4))) fiveStake := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, app.StakingKeeper.TokensFromConsensusPower(ctx, 5))) + fmt.Println("stake amount", fourStake) + fmt.Println("stake amount", fiveStake) addr0Initial := app.BankKeeper.GetAllBalances(ctx, TestAddrs[0]) addr1Initial := app.BankKeeper.GetAllBalances(ctx, TestAddrs[1]) @@ -96,8 +99,9 @@ func TestDeposits(t *testing.T) { require.True(t, found) require.Equal(t, fourStake, deposit.Amount) app.GovKeeper.RefundDeposits(ctx, proposalID) + app.GovKeeper.DeleteDeposits(ctx, proposalID) deposit, found = app.GovKeeper.GetDeposit(ctx, proposalID, TestAddrs[1]) - require.True(t, found) + require.False(t, found) require.Equal(t, addr0Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[0])) require.Equal(t, addr1Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[1])) From 4189b990c733a5521bd759b308a9af08bc626fc3 Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 28 Jun 2021 18:04:09 +0530 Subject: [PATCH 10/25] remove commented code --- x/gov/client/cli/query.go | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/x/gov/client/cli/query.go b/x/gov/client/cli/query.go index 6c55dff6c5b9..7c9ac93b76f6 100644 --- a/x/gov/client/cli/query.go +++ b/x/gov/client/cli/query.go @@ -374,23 +374,6 @@ $ %s query gov deposit 1 cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk 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.JSONCodec.MustUnmarshalJSON(resByTxQuery, &deposit) - // return clientCtx.PrintProto(&deposit) - // } - res, err := queryClient.Deposit( ctx, &types.QueryDepositRequest{ProposalId: proposalID, Depositor: args[1]}, @@ -447,22 +430,6 @@ $ %s query gov deposits 1 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 From ff753527af69352b28b0a345e4adcf4e82780553 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 29 Jun 2021 13:32:35 +0530 Subject: [PATCH 11/25] update invariants --- x/gov/genesis.go | 4 ++-- x/gov/keeper/invariants.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 7cff0175c7dd..455e63921eea 100644 --- a/x/gov/genesis.go +++ b/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.IsAllLTE(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 should less than %s but we got %s", totalDeposits.String(), balance.String())) } } diff --git a/x/gov/keeper/invariants.go b/x/gov/keeper/invariants.go index fab6a2d283c9..a0fc650a331e 100644 --- a/x/gov/keeper/invariants.go +++ b/x/gov/keeper/invariants.go @@ -34,7 +34,7 @@ func ModuleAccountInvariant(keeper Keeper, bk types.BankKeeper) sdk.Invariant { macc := keeper.GetGovernanceAccount(ctx) balances := bk.GetAllBalances(ctx, macc.GetAddress()) - broken := !balances.IsAllLTE(deposits) + broken := deposits.IsAllGT(balances) return sdk.FormatInvariant(types.ModuleName, "deposits", fmt.Sprintf("\tgov ModuleAccount coins: %s\n\tsum of deposit amounts: %s\n", From ebf9ef4ec717beb5889245e39dac9457f7f229b7 Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 30 Jun 2021 12:59:33 +0530 Subject: [PATCH 12/25] update tests --- x/gov/keeper/deposit.go | 16 +--------------- x/gov/keeper/deposit_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 7463a2f3dee2..3323619f8880 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -72,20 +72,6 @@ func (keeper Keeper) DeleteAndBurnDeposits(ctx sdk.Context, proposalID uint64) { }) } -// DeleteDeposits deletes all the deposits. -func (keeper Keeper) DeleteDeposits(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 { - panic(err) - } - store.Delete(types.DepositKey(proposalID, depositor)) - return false - }) -} - // IterateAllDeposits iterates over the all the stored deposits and performs a callback function func (keeper Keeper) IterateAllDeposits(ctx sdk.Context, cb func(deposit types.Deposit) (stop bool)) { store := ctx.KVStore(keeper.storeKey) @@ -180,7 +166,7 @@ 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) diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 3386373eff9d..f96a4275534a 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -1,7 +1,6 @@ package keeper_test import ( - "fmt" "testing" "time" @@ -25,8 +24,6 @@ func TestDeposits(t *testing.T) { fourStake := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, app.StakingKeeper.TokensFromConsensusPower(ctx, 4))) fiveStake := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, app.StakingKeeper.TokensFromConsensusPower(ctx, 5))) - fmt.Println("stake amount", fourStake) - fmt.Println("stake amount", fiveStake) addr0Initial := app.BankKeeper.GetAllBalances(ctx, TestAddrs[0]) addr1Initial := app.BankKeeper.GetAllBalances(ctx, TestAddrs[1]) @@ -99,16 +96,19 @@ func TestDeposits(t *testing.T) { require.True(t, found) require.Equal(t, fourStake, deposit.Amount) app.GovKeeper.RefundDeposits(ctx, proposalID) - app.GovKeeper.DeleteDeposits(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.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])) } From 0b965941da7d68d119c4c7b05f2b5b64b203ed0f Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 30 Jun 2021 13:58:41 +0530 Subject: [PATCH 13/25] updated docs --- CHANGELOG.md | 1 + x/gov/spec/01_concepts.md | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c13f59bec64..3a13e5578586 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (client/tx) [\#9421](https://github.com/cosmos/cosmos-sdk/pull/9421/) `BuildUnsignedTx`, `BuildSimTx`, `PrintUnsignedStdTx` functions are moved to the Tx Factory as methods. * [\#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. ### CLI Breaking Changes diff --git a/x/gov/spec/01_concepts.md b/x/gov/spec/01_concepts.md index 29e582990b09..797edbdcb114 100644 --- a/x/gov/spec/01_concepts.md +++ b/x/gov/spec/01_concepts.md @@ -64,10 +64,10 @@ Once the proposal's deposit reaches `MinDeposit`, it enters voting period. If pr ### Deposit refund and burn -When a the a proposal finalized, the coins from the deposit are either refunded or burned, according to the final tally of the proposal: +When a proposal finalized, the coins from the deposit are either refunded or burned, according to the final tally of the proposal: -- If the proposal is approved or if it's rejected but _not_ vetoed, deposits will automatically be refunded to their respective depositor (transferred from the governance `ModuleAccount`). -- When the proposal is vetoed with a supermajority, deposits be burned from the governance `ModuleAccount`. +- If the proposal is approved or if it's rejected but _not_ vetoed, deposits will automatically be refunded to their respective depositor (transferred from the governance `ModuleAccount`) and keeps the deposits information in state. +- When the proposal is vetoed with a supermajority, deposits will be burned from the governance `ModuleAccount` along with that proposal info and it's deposits info get removed from the state. ## Vote From b9127b867984625866f8ea6d59518eef5ae7c6c4 Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 5 Jul 2021 13:28:48 +0530 Subject: [PATCH 14/25] address review changes --- x/gov/genesis.go | 2 +- x/gov/keeper/deposit.go | 2 -- x/gov/spec/01_concepts.md | 6 +++--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 455e63921eea..1e1124aaaadf 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -49,7 +49,7 @@ 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 totalDeposits.IsAllGT(balance) { - panic(fmt.Sprintf("expected module account should less than %s but we got %s", totalDeposits.String(), balance.String())) + panic(fmt.Sprintf("expected module account balance to be less than %s but got %s", totalDeposits.String(), balance.String())) } } diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 3323619f8880..6f72d259d3b0 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -168,8 +168,6 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd // 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 { diff --git a/x/gov/spec/01_concepts.md b/x/gov/spec/01_concepts.md index 797edbdcb114..5934e917155d 100644 --- a/x/gov/spec/01_concepts.md +++ b/x/gov/spec/01_concepts.md @@ -64,10 +64,10 @@ Once the proposal's deposit reaches `MinDeposit`, it enters voting period. If pr ### Deposit refund and burn -When a proposal finalized, the coins from the deposit are either refunded or burned, according to the final tally of the proposal: +When a proposal is finalized, the coins from the deposit are either refunded or burned, according to the final tally of the proposal: -- If the proposal is approved or if it's rejected but _not_ vetoed, deposits will automatically be refunded to their respective depositor (transferred from the governance `ModuleAccount`) and keeps the deposits information in state. -- When the proposal is vetoed with a supermajority, deposits will be burned from the governance `ModuleAccount` along with that proposal info and it's deposits info get removed from the state. +- If the proposal is approved or rejected but _not_ vetoed, each deposit will be automatically refunded to its respective depositor (transferred from the governance `ModuleAccount`) and the deposit information will remain in state. +- When the proposal is vetoed with a supermajority, deposits will be burned from the governance `ModuleAccount` and the proposal information along with its deposit information will be removed from state. ## Vote From b7d97cdf87e02f611e1849124df17ed9c8109a75 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 6 Jul 2021 13:49:27 +0530 Subject: [PATCH 15/25] fix lint --- x/gov/keeper/vote.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index cb58c95da5ed..d80a884ddf93 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -78,8 +78,8 @@ func (keeper Keeper) GetVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A // SetVote sets a Vote to the gov store func (keeper Keeper) SetVote(ctx sdk.Context, vote types.Vote) { // vote.Option is a deprecated field, we don't set it in state - if vote.Option != types.OptionEmpty { //nolint - vote.Option = types.OptionEmpty //nolint + if vote.Option != types.OptionEmpty { + vote.Option = types.OptionEmpty } store := ctx.KVStore(keeper.storeKey) @@ -135,6 +135,6 @@ func (keeper Keeper) deleteVote(ctx sdk.Context, proposalID uint64, voterAddr sd // there's only 1 VoteOption. func populateLegacyOption(vote *types.Vote) { if len(vote.Options) == 1 && vote.Options[0].Weight.Equal(sdk.MustNewDecFromStr("1.0")) { - vote.Option = vote.Options[0].Option //nolint + vote.Option = vote.Options[0].Option } } From 95be8f41ab88b7c62d56f42bb023085758baf27e Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 6 Jul 2021 13:52:56 +0530 Subject: [PATCH 16/25] review changes --- x/gov/keeper/deposit.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 6f72d259d3b0..0af76472259f 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -53,7 +53,7 @@ func (keeper Keeper) GetDeposits(ctx sdk.Context, proposalID uint64) (deposits t return } -// DeleteAndBurnDeposits deletes all the deposits on a specific proposal without refunding them. +// 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) @@ -166,7 +166,7 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd return activatedVotingPeriod, nil } -// RefundDeposits refunds 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) { keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { depositor, err := sdk.AccAddressFromBech32(deposit.Depositor) From 8c8549dda7e737687390c05a7e5733ff6368a5f2 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 6 Jul 2021 14:02:55 +0530 Subject: [PATCH 17/25] fix lint --- x/gov/keeper/vote.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index d80a884ddf93..7dfeb4404b30 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -78,8 +78,8 @@ func (keeper Keeper) GetVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A // SetVote sets a Vote to the gov store func (keeper Keeper) SetVote(ctx sdk.Context, vote types.Vote) { // vote.Option is a deprecated field, we don't set it in state - if vote.Option != types.OptionEmpty { - vote.Option = types.OptionEmpty + if vote.Option != types.OptionEmpty { // nolint + vote.Option = types.OptionEmpty // nolint } store := ctx.KVStore(keeper.storeKey) @@ -135,6 +135,6 @@ func (keeper Keeper) deleteVote(ctx sdk.Context, proposalID uint64, voterAddr sd // there's only 1 VoteOption. func populateLegacyOption(vote *types.Vote) { if len(vote.Options) == 1 && vote.Options[0].Weight.Equal(sdk.MustNewDecFromStr("1.0")) { - vote.Option = vote.Options[0].Option + vote.Option = vote.Options[0].Option // nolint } } From 34d653e23bf1cb299557f87986ec521236e2e3f5 Mon Sep 17 00:00:00 2001 From: atheeshp <59333759+atheeshp@users.noreply.github.com> Date: Mon, 12 Jul 2021 21:56:54 +0530 Subject: [PATCH 18/25] Update x/gov/abci.go Co-authored-by: Robert Zaremba --- x/gov/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index 6c64af2598b5..8d9c48069546 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -16,7 +16,7 @@ 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.DeleteAndBurnDeposits(ctx, proposal.ProposalId) From 0b47c1e938c5e9426f9d5c8b937319c97b766155 Mon Sep 17 00:00:00 2001 From: atheesh Date: Fri, 16 Jul 2021 23:50:41 +0530 Subject: [PATCH 19/25] update active proposals endblocker --- x/gov/abci.go | 4 +++- x/gov/keeper/deposit.go | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index 8d9c48069546..bf995cec0da4 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -49,7 +49,9 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) { passes, burnDeposits, tallyResults := keeper.Tally(ctx, proposal) - if !burnDeposits { + if burnDeposits { + keeper.BurnDeposits(ctx, proposal.ProposalId) + } else { keeper.RefundDeposits(ctx, proposal.ProposalId) } diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 0af76472259f..9b4fcd49abbe 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -182,3 +182,15 @@ func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) { return false }) } + +// BurnDeposits burns all the deposits on a specific proposal. +func (keeper Keeper) BurnDeposits(ctx sdk.Context, proposalID uint64) { + keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { + err := keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, deposit.Amount) + if err != nil { + panic(err) + } + + return false + }) +} From 8645610e8fcb868eb6b38079f0a535ffe21d3d9d Mon Sep 17 00:00:00 2001 From: atheesh Date: Sat, 17 Jul 2021 00:06:30 +0530 Subject: [PATCH 20/25] revert --- x/gov/abci.go | 4 +--- x/gov/keeper/deposit.go | 12 ------------ 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index bf995cec0da4..8d9c48069546 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -49,9 +49,7 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) { passes, burnDeposits, tallyResults := keeper.Tally(ctx, proposal) - if burnDeposits { - keeper.BurnDeposits(ctx, proposal.ProposalId) - } else { + if !burnDeposits { keeper.RefundDeposits(ctx, proposal.ProposalId) } diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 9b4fcd49abbe..0af76472259f 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -182,15 +182,3 @@ func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) { return false }) } - -// BurnDeposits burns all the deposits on a specific proposal. -func (keeper Keeper) BurnDeposits(ctx sdk.Context, proposalID uint64) { - keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool { - err := keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, deposit.Amount) - if err != nil { - panic(err) - } - - return false - }) -} From 8ae0f05affd2275cae019fa339695e162fad4fa6 Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 22 Jul 2021 17:16:34 +0530 Subject: [PATCH 21/25] update tests --- x/gov/abci.go | 6 ++- x/gov/client/testutil/cli_test.go | 2 - x/gov/client/testutil/deposits.go | 67 +++++++++++++++++++++---------- x/gov/client/testutil/helpers.go | 1 + x/gov/genesis.go | 4 +- x/gov/keeper/deposit.go | 7 +++- x/gov/keeper/deposit_test.go | 4 +- x/gov/keeper/invariants.go | 8 ++-- x/gov/spec/01_concepts.md | 2 +- 9 files changed, 64 insertions(+), 37 deletions(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index 8d9c48069546..c05c261d2d22 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -49,8 +49,10 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) { passes, burnDeposits, tallyResults := keeper.Tally(ctx, proposal) - if !burnDeposits { - keeper.RefundDeposits(ctx, proposal.ProposalId) + if burnDeposits { + keeper.DeleteAndBurnDeposits(ctx, proposal.ProposalId) + } else { + keeper.RefundAndDeleteDeposits(ctx, proposal.ProposalId) } if passes { diff --git a/x/gov/client/testutil/cli_test.go b/x/gov/client/testutil/cli_test.go index 713c33f12182..1376be6c2b21 100644 --- a/x/gov/client/testutil/cli_test.go +++ b/x/gov/client/testutil/cli_test.go @@ -1,5 +1,3 @@ -// +build norace - package testutil import ( diff --git a/x/gov/client/testutil/deposits.go b/x/gov/client/testutil/deposits.go index 4bb3585395f6..43d6b8a44856 100644 --- a/x/gov/client/testutil/deposits.go +++ b/x/gov/client/testutil/deposits.go @@ -41,35 +41,51 @@ func (s *DepositTestSuite) SetupSuite() { 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 + // create 2 proposals for testing for i := 0; i < len(deposits); i++ { - var exactArgs []string id := i + 1 + deposit := deposits[i] - 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.createProposal(val, deposit, id) s.proposalIDs = append(s.proposalIDs, fmt.Sprintf("%d", id)) } } +func (s *DepositTestSuite) SetupNewSuite() { + s.T().Log("setting up new test suite") + + var err error + s.network, err = network.New(s.T(), s.T().TempDir(), s.cfg) + s.Require().NoError(err) + + _, err = s.network.WaitForHeight(1) + s.Require().NoError(err) +} + +func (s *DepositTestSuite) createProposal(val *network.Validator, initialDeposit sdk.Coin, id int) { + var exactArgs []string + + if !initialDeposit.IsZero() { + exactArgs = append(exactArgs, fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit.String())) + } + + _, 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() @@ -123,10 +139,17 @@ func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() { } 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 := s.deposits[2] - proposalID := s.proposalIDs[2] + initialDeposit := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens) + id := 1 + proposalID := fmt.Sprintf("%d", id) + + s.createProposal(val, initialDeposit, id) // query deposits var deposits types.QueryDepositsResponse @@ -137,7 +160,7 @@ func (s *DepositTestSuite) TestRejectedProposalDeposits() { 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(), proposalID, "no") diff --git a/x/gov/client/testutil/helpers.go b/x/gov/client/testutil/helpers.go index e16ac0e98e83..7fa13272ae44 100644 --- a/x/gov/client/testutil/helpers.go +++ b/x/gov/client/testutil/helpers.go @@ -27,6 +27,7 @@ func MsgSubmitProposal(clientCtx client.Context, from, title, description, propo }, commonArgs...) args = append(args, extraArgs...) + fmt.Println("args", args) return clitestutil.ExecTestCLICmd(clientCtx, govcli.NewCmdSubmitProposal(), args) } diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 1e1124aaaadf..62a194d9c0a9 100644 --- a/x/gov/genesis.go +++ b/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 totalDeposits.IsAllGT(balance) { - panic(fmt.Sprintf("expected module account balance to be less than %s but got %s", totalDeposits.String(), balance.String())) + if !balance.IsEqual(totalDeposits) { + panic(fmt.Sprintf("expected module account was %s but we got %s", balance.String(), totalDeposits.String())) } } diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 0af76472259f..10eca5d511b2 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -166,8 +166,10 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd return activatedVotingPeriod, nil } -// RefundDeposits refunds 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 { depositor, err := sdk.AccAddressFromBech32(deposit.Depositor) if err != nil { @@ -179,6 +181,7 @@ func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) { panic(err) } + store.Delete(types.DepositKey(proposalID, depositor)) return false }) } diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index f96a4275534a..d781973ec008 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -95,9 +95,9 @@ func TestDeposits(t *testing.T) { deposit, found = app.GovKeeper.GetDeposit(ctx, proposalID, TestAddrs[1]) require.True(t, found) require.Equal(t, fourStake, deposit.Amount) - app.GovKeeper.RefundDeposits(ctx, proposalID) + app.GovKeeper.RefundAndDeleteDeposits(ctx, proposalID) deposit, found = app.GovKeeper.GetDeposit(ctx, proposalID, TestAddrs[1]) - require.True(t, found) // deposits will be refunded but not deleted from store + require.False(t, found) require.Equal(t, addr0Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[0])) require.Equal(t, addr1Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[1])) diff --git a/x/gov/keeper/invariants.go b/x/gov/keeper/invariants.go index a0fc650a331e..d0ec4dbfacc8 100644 --- a/x/gov/keeper/invariants.go +++ b/x/gov/keeper/invariants.go @@ -25,19 +25,19 @@ func AllInvariants(keeper Keeper, bk types.BankKeeper) sdk.Invariant { // deposit amounts held on store func ModuleAccountInvariant(keeper Keeper, bk types.BankKeeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { - var deposits sdk.Coins + var expectedDeposits sdk.Coins keeper.IterateAllDeposits(ctx, func(deposit types.Deposit) bool { - deposits = deposits.Add(deposit.Amount...) + expectedDeposits = expectedDeposits.Add(deposit.Amount...) return false }) macc := keeper.GetGovernanceAccount(ctx) balances := bk.GetAllBalances(ctx, macc.GetAddress()) - broken := deposits.IsAllGT(balances) + broken := !balances.IsEqual(expectedDeposits) return sdk.FormatInvariant(types.ModuleName, "deposits", fmt.Sprintf("\tgov ModuleAccount coins: %s\n\tsum of deposit amounts: %s\n", - balances, deposits)), broken + balances, expectedDeposits)), broken } } diff --git a/x/gov/spec/01_concepts.md b/x/gov/spec/01_concepts.md index 68edcfe0e9a5..fd0d56267e80 100644 --- a/x/gov/spec/01_concepts.md +++ b/x/gov/spec/01_concepts.md @@ -69,7 +69,7 @@ The deposit is kept in escrow and held by the governance `ModuleAccount` until t When a proposal is finalized, the coins from the deposit are either refunded or burned, according to the final tally of the proposal: -- If the proposal is approved or rejected but _not_ vetoed, each deposit will be automatically refunded to its respective depositor (transferred from the governance `ModuleAccount`) and the deposit information will remain in state. +- If the proposal is approved or rejected but _not_ vetoed, each deposit will be automatically refunded to its respective depositor (transferred from the governance `ModuleAccount`). All the deposits will be removed from state after refunding to the depositors. - When the proposal is vetoed with a supermajority, deposits will be burned from the governance `ModuleAccount` and the proposal information along with its deposit information will be removed from state. ## Vote From d4461f36e942634495c0ae06f893480ff50ed37a Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 22 Jul 2021 17:17:06 +0530 Subject: [PATCH 22/25] remove log --- x/gov/client/testutil/helpers.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/gov/client/testutil/helpers.go b/x/gov/client/testutil/helpers.go index 7fa13272ae44..e16ac0e98e83 100644 --- a/x/gov/client/testutil/helpers.go +++ b/x/gov/client/testutil/helpers.go @@ -27,7 +27,6 @@ func MsgSubmitProposal(clientCtx client.Context, from, title, description, propo }, commonArgs...) args = append(args, extraArgs...) - fmt.Println("args", args) return clitestutil.ExecTestCLICmd(clientCtx, govcli.NewCmdSubmitProposal(), args) } From 41fb2ccc0547905c80970b3afd47cecf481fefba Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 22 Jul 2021 17:20:11 +0530 Subject: [PATCH 23/25] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a305dd0a3d6a..f3385e8cd078 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,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` now renamed to `DeleteAndBurnDeposits`, also now deposits won't be removed from state for completed proposals unless a proposal is vetoed. +* [\#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`. From d95df5fbcb79e80975a8d5b8d251f04124201fec Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 22 Jul 2021 17:25:28 +0530 Subject: [PATCH 24/25] update docs --- x/gov/spec/01_concepts.md | 1 + 1 file changed, 1 insertion(+) diff --git a/x/gov/spec/01_concepts.md b/x/gov/spec/01_concepts.md index fd0d56267e80..c5e5aa70e997 100644 --- a/x/gov/spec/01_concepts.md +++ b/x/gov/spec/01_concepts.md @@ -71,6 +71,7 @@ When a proposal is finalized, the coins from the deposit are either refunded or - If the proposal is approved or rejected but _not_ vetoed, each deposit will be automatically refunded to its respective depositor (transferred from the governance `ModuleAccount`). All the deposits will be removed from state after refunding to the depositors. - When the proposal is vetoed with a supermajority, deposits will be burned from the governance `ModuleAccount` and the proposal information along with its deposit information will be removed from state. +- NOTE: The proposals which completed the voting period, cannot return the deposits when queried. ## Vote From 20913a1d77563c51a2cd2c9fd2e80205f08bf5ea Mon Sep 17 00:00:00 2001 From: atheeshp <59333759+atheeshp@users.noreply.github.com> Date: Tue, 27 Jul 2021 21:18:00 +0530 Subject: [PATCH 25/25] Update x/gov/spec/01_concepts.md Co-authored-by: Robert Zaremba --- x/gov/spec/01_concepts.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/gov/spec/01_concepts.md b/x/gov/spec/01_concepts.md index c5e5aa70e997..dde2bd0b1071 100644 --- a/x/gov/spec/01_concepts.md +++ b/x/gov/spec/01_concepts.md @@ -69,8 +69,9 @@ The deposit is kept in escrow and held by the governance `ModuleAccount` until t When a proposal is finalized, the coins from the deposit are either refunded or burned, according to the final tally of the proposal: -- If the proposal is approved or rejected but _not_ vetoed, each deposit will be automatically refunded to its respective depositor (transferred from the governance `ModuleAccount`). All the deposits will be removed from state after refunding to the depositors. +- If the proposal is approved or rejected but _not_ vetoed, each deposit will be automatically refunded to its respective depositor (transferred from the governance `ModuleAccount`). - When the proposal is vetoed with a supermajority, deposits will be burned from the governance `ModuleAccount` and the proposal information along with its deposit information will be removed from state. +- All refunded or burned deposits are removed from the state. Events are issued when burning or refunding a deposit. - NOTE: The proposals which completed the voting period, cannot return the deposits when queried. ## Vote