fix: Add validation on create gentx (#11693)

CHANGELOG.md

@@ -209,6 +209,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+* [\#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.
 * [\#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.

types/coin.go

@@ -834,13 +834,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
+// ParseCoinsNormalized will parse out a list of coins separated by commas, and normalize them by converting to the smallest
-// unit. If the parsing is successuful, the provided coins will be sanitized by removing zero coins and sorting the coin
+// 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)

x/genutil/client/cli/gentx.go

@@ -167,6 +167,10 @@ $ %s gentx my-key-name 1000000stake --home=/path/to/home/dir --keyring-backend=o
 			w := bytes.NewBuffer([]byte{})
 			clientCtx = clientCtx.WithOutput(w)
 
+			if err = msg.ValidateBasic(); err != nil {
+				return err
+			}
+
 			if err = txBldr.PrintUnsignedTx(clientCtx, msg); err != nil {
 				return errors.Wrap(err, "failed to print unsigned std tx")
 			}

x/genutil/client/testutil/suite.go

@@ -1,7 +1,6 @@
 package testutil
 
 import (
-	"context"
 	"fmt"
 	"io"
 	"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"
@@ -50,84 +48,97 @@ func (s *IntegrationTestSuite) TearDownSuite() {
 
 func (s *IntegrationTestSuite) TestGenTxCmd() {
 	val := s.network.Validators[0]
-	dir := s.T().TempDir()
+	clientCtx := val.ClientCtx
-
-	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)
-
 	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),
-		val.Moniker,
-		amount.String(),
-	})
 
-	err := cmd.ExecuteContext(ctx)
+	tests := []struct {
-	s.Require().NoError(err)
+		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,
+		},
+	}
 
-	// validate generated transaction.
+	for _, tc := range tests {
-	open, err := os.Open(genTxFile)
+		tc := tc
-	s.Require().NoError(err)
 
-	all, err := io.ReadAll(open)
+		dir := s.T().TempDir()
-	s.Require().NoError(err)
+		genTxFile := filepath.Join(dir, "myTx")
+		tc.args = append(tc.args, fmt.Sprintf("--%s=%s", flags.FlagOutputDocument, genTxFile))
 
-	tx, err := val.ClientCtx.TxConfig.TxJSONDecoder()(all)
+		s.Run(tc.name, func() {
-	s.Require().NoError(err)
+			cmd := cli.GenTxCmd(
+				simapp.ModuleBasics,
+				val.ClientCtx.TxConfig,
+				banktypes.GenesisBalancesIterator{},
+				val.ClientCtx.HomeDir)
 
-	msgs := tx.GetMsgs()
+			out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
-	s.Require().Len(msgs, 1)
 
-	s.Require().Equal(sdk.MsgTypeURL(&types.MsgCreateValidator{}), sdk.MsgTypeURL(msgs[0]))
+			if tc.expError {
-	s.Require().True(val.Address.Equals(msgs[0].GetSigners()[0]))
+				s.Require().Error(err)
-	s.Require().Equal(amount, msgs[0].(*types.MsgCreateValidator).Value)
+
-	s.Require().NoError(tx.ValidateBasic())
+				_, err = os.Open(genTxFile)
-}
+				s.Require().Error(err)
-
+			} else {
-func (s *IntegrationTestSuite) TestGenTxCmdPubkey() {
+				s.Require().NoError(err, "test: %s\noutput: %s", tc.name, out.String())
-	val := s.network.Validators[0]
+
-	dir := s.T().TempDir()
+				// validate generated transaction.
-
+				open, err := os.Open(genTxFile)
-	cmd := cli.GenTxCmd(
+				s.Require().NoError(err)
-		simapp.ModuleBasics,
+
-		val.ClientCtx.TxConfig,
+				all, err := io.ReadAll(open)
-		banktypes.GenesisBalancesIterator{},
+				s.Require().NoError(err)
-		val.ClientCtx.HomeDir,
+
-	)
+				tx, err := val.ClientCtx.TxConfig.TxJSONDecoder()(all)
-
+				s.Require().NoError(err)
-	_, out := testutil.ApplyMockIO(cmd)
+
-	clientCtx := val.ClientCtx.WithOutput(out)
+				msgs := tx.GetMsgs()
-
+				s.Require().Len(msgs, 1)
-	ctx := context.Background()
+
-	ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx)
+				s.Require().Equal(sdk.MsgTypeURL(&types.MsgCreateValidator{}), sdk.MsgTypeURL(msgs[0]))
-
+				s.Require().True(val.Address.Equals(msgs[0].GetSigners()[0]))
-	amount := sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(12))
+				s.Require().Equal(amount, msgs[0].(*types.MsgCreateValidator).Value)
-	genTxFile := filepath.Join(dir, "myTx")
+				s.Require().NoError(tx.ValidateBasic())
-
+			}
-	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))
 }

x/genutil/gentx_test.go

@@ -65,7 +65,7 @@ func (suite *GenTxTestSuite) setAccountBalance(addr sdk.AccAddress, amount int64
 	acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr)
 	suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
 
-	err := testutil.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)})
 	suite.Require().NoError(err)
 
 	bankGenesisState := suite.app.BankKeeper.ExportGenesis(suite.ctx)
@@ -231,7 +231,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(