fix: Add validation on create gentx (backport #11693) (#11698)

cosmos · Apr 20, 2022 · 13d78de · 13d78de
1 parent ffef0ba
commit 13d78de
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 88 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* [\#11693](https://github.com/cosmos/cosmos-sdk/pull/11693) Add validation for gentx cmd.
 * [\#11686](https://github.com/cosmos/cosmos-sdk/pull/11686) Update the min required Golang version to `1.17`.
 * (x/auth/vesting) [\#11652](https://github.com/cosmos/cosmos-sdk/pull/11652) Add util functions for `Period(s)`
 

diff --git 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
@@ -162,6 +162,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 = authclient.PrintUnsignedStdTx(txBldr, clientCtx, []sdk.Msg{msg}); err != nil {
 				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
@@ -1,18 +1,16 @@
 package testutil
 
 import (
-	"context"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"os"
 	"path/filepath"
 
 	"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,84 +46,97 @@ 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))
-	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)
-	s.Require().NoError(err)
-
-	// validate generated transaction.
-	open, err := os.Open(genTxFile)
-	s.Require().NoError(err)
-
-	all, err := ioutil.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().Equal([]sdk.AccAddress{val.Address}, msgs[0].GetSigners())
-	s.Require().Equal(amount, msgs[0].(*types.MsgCreateValidator).Value)
-	s.Require().NoError(tx.ValidateBasic())
-}
-
-func (s *IntegrationTestSuite) TestGenTxCmdPubkey() {
-	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)
-
-	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))
+	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
@@ -64,7 +64,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 := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 25)})
+	err := simapp.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)
@@ -230,7 +230,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(