From 68cd8c30f8c0ee86fa928143aa7924b48d0795eb Mon Sep 17 00:00:00 2001 From: Anil Kumar Kammari Date: Wed, 20 Apr 2022 20:24:29 +0530 Subject: [PATCH] fix: Add validation on create gentx (#11693) (cherry picked from commit df6114203601604d0686237c37d06110005ca7c2) # Conflicts: # CHANGELOG.md # x/genutil/client/cli/gentx.go # x/genutil/client/testutil/suite.go # x/genutil/gentx_test.go --- CHANGELOG.md | 5 ++ types/coin.go | 8 +- x/genutil/client/cli/gentx.go | 8 ++ x/genutil/client/testutil/suite.go | 135 ++++++++++++++++++++--------- x/genutil/gentx_test.go | 6 +- 5 files changed, 118 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec9773107ed4..975497ad737e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +<<<<<<< HEAD +======= +* [\#11693](https://github.com/cosmos/cosmos-sdk/pull/11693) Add validation for gentx cmd. +* [\#11558](https://github.com/cosmos/cosmos-sdk/pull/11558) Fix `--dry-run` not working when using tx command. +>>>>>>> df6114203 (fix: Add validation on create gentx (#11693)) * [\#11354](https://github.com/cosmos/cosmos-sdk/pull/11355) Added missing pagination flag for `bank q total` query. * [\#11197](https://github.com/cosmos/cosmos-sdk/pull/11197) Signing with multisig now works with multisig address which is not in the keyring. * (client) [\#11283](https://github.com/cosmos/cosmos-sdk/issues/11283) Support multiple keys for tx simulation and setting automatic gas for txs. diff --git a/types/coin.go b/types/coin.go index af8ef0ae7cc3..20ae56fead2e 100644 --- a/types/coin.go +++ b/types/coin.go @@ -814,13 +814,13 @@ func ParseCoinNormalized(coinStr string) (coin Coin, err error) { return coin, nil } -// ParseCoinsNormalized will parse out a list of coins separated by commas, and normalize them by converting to smallest -// unit. If the parsing is successuful, the provided coins will be sanitized by removing zero coins and sorting the coin +// ParseCoinsNormalized will parse out a list of coins separated by commas, and normalize them by converting to the smallest +// unit. If the parsing is successful, the provided coins will be sanitized by removing zero coins and sorting the coin // set. Lastly a validation of the coin set is executed. If the check passes, ParseCoinsNormalized will return the // sanitized coins. -// Otherwise it will return an error. +// Otherwise, it will return an error. // If an empty string is provided to ParseCoinsNormalized, it returns nil Coins. -// ParseCoinsNormalized supports decimal coins as inputs, and truncate them to int after converted to smallest unit. +// ParseCoinsNormalized supports decimal coins as inputs, and truncate them to int after converted to the smallest unit. // Expected format: "{amount0}{denomination},...,{amountN}{denominationN}" func ParseCoinsNormalized(coinStr string) (Coins, error) { coins, err := ParseDecCoins(coinStr) diff --git a/x/genutil/client/cli/gentx.go b/x/genutil/client/cli/gentx.go index be347b745b05..0239672dff6f 100644 --- a/x/genutil/client/cli/gentx.go +++ b/x/genutil/client/cli/gentx.go @@ -162,7 +162,15 @@ $ %s gentx my-key-name 1000000stake --home=/path/to/home/dir --keyring-backend=o w := bytes.NewBuffer([]byte{}) clientCtx = clientCtx.WithOutput(w) +<<<<<<< HEAD if err = authclient.PrintUnsignedStdTx(txBldr, clientCtx, []sdk.Msg{msg}); err != nil { +======= + if err = msg.ValidateBasic(); err != nil { + return err + } + + if err = txBldr.PrintUnsignedTx(clientCtx, msg); err != nil { +>>>>>>> df6114203 (fix: Add validation on create gentx (#11693)) return errors.Wrap(err, "failed to print unsigned std tx") } diff --git a/x/genutil/client/testutil/suite.go b/x/genutil/client/testutil/suite.go index 084a5f530f8d..bdd26a1618da 100644 --- a/x/genutil/client/testutil/suite.go +++ b/x/genutil/client/testutil/suite.go @@ -1,7 +1,6 @@ package testutil import ( - "context" "fmt" "io/ioutil" "os" @@ -9,10 +8,9 @@ import ( "github.com/stretchr/testify/suite" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/simapp" - "github.com/cosmos/cosmos-sdk/testutil" + clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" "github.com/cosmos/cosmos-sdk/testutil/network" sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" @@ -48,19 +46,9 @@ func (s *IntegrationTestSuite) TearDownSuite() { func (s *IntegrationTestSuite) TestGenTxCmd() { val := s.network.Validators[0] - dir := s.T().TempDir() - - cmd := cli.GenTxCmd( - simapp.ModuleBasics, - val.ClientCtx.TxConfig, banktypes.GenesisBalancesIterator{}, val.ClientCtx.HomeDir) - - _, out := testutil.ApplyMockIO(cmd) - clientCtx := val.ClientCtx.WithOutput(out) - - ctx := context.Background() - ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - + clientCtx := val.ClientCtx amount := sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(12)) +<<<<<<< HEAD genTxFile := filepath.Join(dir, "myTx") cmd.SetArgs([]string{ fmt.Sprintf("--%s=%s", flags.FlagChainID, s.network.Config.ChainID), @@ -104,28 +92,97 @@ func (s *IntegrationTestSuite) TestGenTxCmdPubkey() { _, out := testutil.ApplyMockIO(cmd) clientCtx := val.ClientCtx.WithOutput(out) - - ctx := context.Background() - ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - - amount := sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(12)) - genTxFile := filepath.Join(dir, "myTx") - - cmd.SetArgs([]string{ - fmt.Sprintf("--%s=%s", flags.FlagChainID, s.network.Config.ChainID), - fmt.Sprintf("--%s=%s", flags.FlagOutputDocument, genTxFile), - fmt.Sprintf("--%s={\"key\":\"BOIkjkFruMpfOFC9oNPhiJGfmY2pHF/gwHdLDLnrnS0=\"}", stakingcli.FlagPubKey), - val.Moniker, - amount.String(), - }) - s.Require().Error(cmd.ExecuteContext(ctx)) - - cmd.SetArgs([]string{ - fmt.Sprintf("--%s=%s", flags.FlagChainID, s.network.Config.ChainID), - fmt.Sprintf("--%s=%s", flags.FlagOutputDocument, genTxFile), - fmt.Sprintf("--%s={\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"BOIkjkFruMpfOFC9oNPhiJGfmY2pHF/gwHdLDLnrnS0=\"}", stakingcli.FlagPubKey), - val.Moniker, - amount.String(), - }) - s.Require().NoError(cmd.ExecuteContext(ctx)) +======= +>>>>>>> df6114203 (fix: Add validation on create gentx (#11693)) + + tests := []struct { + name string + args []string + expError bool + }{ + { + name: "invalid commission rate returns error", + args: []string{ + fmt.Sprintf("--%s=%s", flags.FlagChainID, s.network.Config.ChainID), + fmt.Sprintf("--%s=1", stakingcli.FlagCommissionRate), + val.Moniker, + amount.String(), + }, + expError: true, + }, + { + name: "valid gentx", + args: []string{ + fmt.Sprintf("--%s=%s", flags.FlagChainID, s.network.Config.ChainID), + val.Moniker, + amount.String(), + }, + expError: false, + }, + { + name: "invalid pubkey", + args: []string{ + fmt.Sprintf("--%s=%s", flags.FlagChainID, s.network.Config.ChainID), + fmt.Sprintf("--%s={\"key\":\"BOIkjkFruMpfOFC9oNPhiJGfmY2pHF/gwHdLDLnrnS0=\"}", stakingcli.FlagPubKey), + val.Moniker, + amount.String(), + }, + expError: true, + }, + { + name: "valid pubkey flag", + args: []string{ + fmt.Sprintf("--%s=%s", flags.FlagChainID, s.network.Config.ChainID), + fmt.Sprintf("--%s={\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"BOIkjkFruMpfOFC9oNPhiJGfmY2pHF/gwHdLDLnrnS0=\"}", stakingcli.FlagPubKey), + val.Moniker, + amount.String(), + }, + expError: false, + }, + } + + for _, tc := range tests { + tc := tc + + dir := s.T().TempDir() + genTxFile := filepath.Join(dir, "myTx") + tc.args = append(tc.args, fmt.Sprintf("--%s=%s", flags.FlagOutputDocument, genTxFile)) + + s.Run(tc.name, func() { + cmd := cli.GenTxCmd( + simapp.ModuleBasics, + val.ClientCtx.TxConfig, + banktypes.GenesisBalancesIterator{}, + val.ClientCtx.HomeDir) + + out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args) + + if tc.expError { + s.Require().Error(err) + + _, err = os.Open(genTxFile) + s.Require().Error(err) + } else { + s.Require().NoError(err, "test: %s\noutput: %s", tc.name, out.String()) + + // validate generated transaction. + open, err := os.Open(genTxFile) + s.Require().NoError(err) + + all, err := io.ReadAll(open) + s.Require().NoError(err) + + tx, err := val.ClientCtx.TxConfig.TxJSONDecoder()(all) + s.Require().NoError(err) + + msgs := tx.GetMsgs() + s.Require().Len(msgs, 1) + + s.Require().Equal(sdk.MsgTypeURL(&types.MsgCreateValidator{}), sdk.MsgTypeURL(msgs[0])) + s.Require().True(val.Address.Equals(msgs[0].GetSigners()[0])) + s.Require().Equal(amount, msgs[0].(*types.MsgCreateValidator).Value) + s.Require().NoError(tx.ValidateBasic()) + } + }) + } } diff --git a/x/genutil/gentx_test.go b/x/genutil/gentx_test.go index 8de5bc4afe41..fd48425e7b12 100644 --- a/x/genutil/gentx_test.go +++ b/x/genutil/gentx_test.go @@ -64,7 +64,11 @@ func (suite *GenTxTestSuite) setAccountBalance(addr sdk.AccAddress, amount int64 acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr) suite.app.AccountKeeper.SetAccount(suite.ctx, acc) +<<<<<<< HEAD err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 25)}) +======= + err := testutil.FundAccount(suite.app.BankKeeper, suite.ctx, addr, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, amount)}) +>>>>>>> df6114203 (fix: Add validation on create gentx (#11693)) suite.Require().NoError(err) bankGenesisState := suite.app.BankKeeper.ExportGenesis(suite.ctx) @@ -230,7 +234,7 @@ func (suite *GenTxTestSuite) TestDeliverGenTxs() { "success", func() { _ = suite.setAccountBalance(addr1, 50) - _ = suite.setAccountBalance(addr2, 0) + _ = suite.setAccountBalance(addr2, 1) msg := banktypes.NewMsgSend(addr1, addr2, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 1)}) tx, err := helpers.GenTx(