diff --git a/x/bank/app_test.go b/x/bank/app_test.go index 86f9f9ff0a5f..19a33d2a6461 100644 --- a/x/bank/app_test.go +++ b/x/bank/app_test.go @@ -349,14 +349,14 @@ func TestMsgSetSendEnabled(t *testing.T) { s := createTestSuite(t, genAccs) ctx := s.App.BaseApp.NewContext(false, tmproto.Header{}) - require.NoError(t, testutil.FundAccount(s.BankKeeper, ctx, addr1, sdk.NewCoins(sdk.NewInt64Coin("foocoin", 101)))) + require.NoError(t, testutil.FundAccount(s.BankKeeper, ctx, addr1, sdk.NewCoins(sdk.NewInt64Coin("stake", 101)))) addr1Str := addr1.String() govAddr := s.BankKeeper.GetAuthority() goodGovProp, err := govv1.NewMsgSubmitProposal( []sdk.Msg{ types.NewMsgSetSendEnabled(govAddr, nil, nil), }, - sdk.Coins{{"foocoin", sdk.NewInt(5)}}, + sdk.Coins{{"stake", sdk.NewInt(5)}}, addr1Str, "set default send enabled to true", "Change send enabled", diff --git a/x/gov/README.md b/x/gov/README.md index ca87ccad6067..c6a3acdff070 100644 --- a/x/gov/README.md +++ b/x/gov/README.md @@ -480,6 +480,7 @@ All `sdk.Msgs` passed into the `messages` field of a `MsgSubmitProposal` message must be registered in the app's `MsgServiceRouter`. Each of these messages must have one signer, namely the gov module account. And finally, the metadata length must not be larger than the `maxMetadataLen` config passed into the gov keeper. +The `initialDeposit` must be strictly positive and conform to the accepted denom of the `MinDeposit` param. **State modifications:** @@ -539,10 +540,15 @@ upon receiving txGovSubmitProposal from sender do ### Deposit -Once a proposal is submitted, if -`Proposal.TotalDeposit < ActiveParam.MinDeposit`, Atom holders can send +Once a proposal is submitted, if `Proposal.TotalDeposit < ActiveParam.MinDeposit`, Atom holders can send `MsgDeposit` transactions to increase the proposal's deposit. +A deposit is accepted if: + +* The proposal exists +* The proposal is not in the voting period +* The deposited coins are conform to the accepted denom from the `MinDeposit` param + ```protobuf reference https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/proto/cosmos/gov/v1/tx.proto#L134-L147 ``` diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 62b90bd46d5f..fbbbd0b0a86b 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -118,6 +118,12 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd return false, sdkerrors.Wrapf(types.ErrInactiveProposal, "%d", proposalID) } + // Check coins to be deposited match the proposal's deposit params + params := keeper.GetParams(ctx) + if err := keeper.validateDepositDenom(ctx, params, depositAmount); err != nil { + return false, err + } + // update the governance module's account coins pool err := keeper.bankKeeper.SendCoinsFromAccountToModule(ctx, depositorAddr, types.ModuleName, depositAmount) if err != nil { @@ -131,7 +137,7 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd // Check if deposit has provided sufficient total funds to transition the proposal into the voting period activatedVotingPeriod := false - if proposal.Status == v1.StatusDepositPeriod && sdk.NewCoins(proposal.TotalDeposit...).IsAllGTE(keeper.GetParams(ctx).MinDeposit) { + if proposal.Status == v1.StatusDepositPeriod && sdk.NewCoins(proposal.TotalDeposit...).IsAllGTE(params.MinDeposit) { keeper.ActivateVotingPeriod(ctx, proposal) activatedVotingPeriod = true @@ -182,8 +188,7 @@ func (keeper Keeper) RefundAndDeleteDeposits(ctx sdk.Context, proposalID uint64) // validateInitialDeposit validates if initial deposit is greater than or equal to the minimum // required at the time of proposal submission. This threshold amount is determined by // the deposit parameters. Returns nil on success, error otherwise. -func (keeper Keeper) validateInitialDeposit(ctx sdk.Context, initialDeposit sdk.Coins) error { - params := keeper.GetParams(ctx) +func (keeper Keeper) validateInitialDeposit(ctx sdk.Context, params v1.Params, initialDeposit sdk.Coins) error { minInitialDepositRatio, err := sdk.NewDecFromStr(params.MinInitialDepositRatio) if err != nil { return err @@ -200,3 +205,21 @@ func (keeper Keeper) validateInitialDeposit(ctx sdk.Context, initialDeposit sdk. } return nil } + +// validateDepositDenom validates if the deposit denom is accepted by the governance module. +func (keeper Keeper) validateDepositDenom(ctx sdk.Context, params v1.Params, depositAmount sdk.Coins) error { + denoms := []string{} + acceptedDenoms := make(map[string]bool, len(params.MinDeposit)) + for _, coin := range params.MinDeposit { + acceptedDenoms[coin.Denom] = true + denoms = append(denoms, coin.Denom) + } + + for _, coin := range depositAmount { + if _, ok := acceptedDenoms[coin.Denom]; !ok { + return sdkerrors.Wrapf(types.ErrInvalidDepositDenom, "deposited %s, but gov accepts only the following denom(s): %v", coin, denoms) + } + } + + return nil +} diff --git a/x/gov/keeper/export_test.go b/x/gov/keeper/export_test.go index 83ec9e273d37..bada822866cc 100644 --- a/x/gov/keeper/export_test.go +++ b/x/gov/keeper/export_test.go @@ -5,5 +5,5 @@ import sdk "github.com/cosmos/cosmos-sdk/types" // ValidateInitialDeposit is a helper function used only in deposit tests which returns the same // functionality of validateInitialDeposit private function. func (k Keeper) ValidateInitialDeposit(ctx sdk.Context, initialDeposit sdk.Coins) error { - return k.validateInitialDeposit(ctx, initialDeposit) + return k.validateInitialDeposit(ctx, k.GetParams(ctx), initialDeposit) } diff --git a/x/gov/keeper/msg_server.go b/x/gov/keeper/msg_server.go index 00a7186a629f..b242b7d21676 100644 --- a/x/gov/keeper/msg_server.go +++ b/x/gov/keeper/msg_server.go @@ -29,7 +29,13 @@ func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitPropos initialDeposit := msg.GetInitialDeposit() - if err := k.validateInitialDeposit(ctx, initialDeposit); err != nil { + params := k.Keeper.GetParams(ctx) + + if err := k.validateInitialDeposit(ctx, params, initialDeposit); err != nil { + return nil, err + } + + if err := k.validateDepositDenom(ctx, params, initialDeposit); err != nil { return nil, err } diff --git a/x/gov/keeper/msg_server_test.go b/x/gov/keeper/msg_server_test.go index 613896ab164f..9c60f5220f5a 100644 --- a/x/gov/keeper/msg_server_test.go +++ b/x/gov/keeper/msg_server_test.go @@ -88,6 +88,34 @@ func (suite *KeeperTestSuite) TestSubmitProposalReq() { expErr: true, expErrMsg: "proposal message not recognized by router", }, + "invalid deposited coin": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + []sdk.Coin{sdk.NewCoin("invalid", sdk.NewInt(100))}, + proposer.String(), + "", + "Proposal", + "description of proposal", + ) + }, + expErr: true, + expErrMsg: "deposited 100invalid, but gov accepts only the following denom(s): [stake]: invalid deposit denom", + }, + "invalid deposited coin (multiple)": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + append(initialDeposit, sdk.NewCoin("invalid", sdk.NewInt(100))), + proposer.String(), + "", + "Proposal", + "description of proposal", + ) + }, + expErr: true, + expErrMsg: "deposited 100invalid, but gov accepts only the following denom(s): [stake]: invalid deposit denom", + }, "all good": { preRun: func() (*v1.MsgSubmitProposal, error) { return v1.NewMsgSubmitProposal( @@ -416,6 +444,24 @@ func (suite *KeeperTestSuite) TestDepositReq() { expErr: true, options: v1.NewNonSplitVoteOption(v1.OptionYes), }, + "invalid deposited coin ": { + preRun: func() uint64 { + return pId + }, + depositor: proposer, + deposit: []sdk.Coin{sdk.NewCoin("ibc/badcoin", sdk.NewInt(1000))}, + expErr: true, + options: v1.NewNonSplitVoteOption(v1.OptionYes), + }, + "invalid deposited coin (multiple)": { + preRun: func() uint64 { + return pId + }, + depositor: proposer, + deposit: append(minDeposit, sdk.NewCoin("ibc/badcoin", sdk.NewInt(1000))), + expErr: true, + options: v1.NewNonSplitVoteOption(v1.OptionYes), + }, "all good": { preRun: func() uint64 { return pId diff --git a/x/gov/types/errors.go b/x/gov/types/errors.go index eb14067caa1d..5caaffec8c82 100644 --- a/x/gov/types/errors.go +++ b/x/gov/types/errors.go @@ -22,4 +22,6 @@ var ( ErrInvalidSignalMsg = sdkerrors.Register(ModuleName, 14, "signal message is invalid") ErrMetadataTooLong = sdkerrors.Register(ModuleName, 15, "metadata too long") ErrMinDepositTooSmall = sdkerrors.Register(ModuleName, 16, "minimum deposit is too small") + // Error 23 is added through backport of #18189 PR + ErrInvalidDepositDenom = sdkerrors.Register(ModuleName, 23, "invalid deposit denom") )