fix!: prevent 0 gas txs (backport cosmos#12416) (cosmos#12432)

CHANGELOG.md

@@ -55,6 +55,7 @@ It is strongly recommended to upgrade to these releases as well.
 
 ### Bug Fixes
 
+* [#12416](https://github.com/cosmos/cosmos-sdk/pull/12416) Prevent zero gas transactions in the `DeductFeeDecorator` AnteHandler decorator.
 * (x/mint) [#12384](https://github.com/cosmos/cosmos-sdk/pull/12384) Ensure `GoalBonded` must be positive when performing `x/mint` parameter validation.
 * (x/auth) [#12261](https://github.com/cosmos/cosmos-sdk/pull/12261) Deprecate pagination in GetTxsEventRequest/Response in favor of page and limit to align with tendermint `SignClient.TxSearch`
 * (vesting) [#12190](https://github.com/cosmos/cosmos-sdk/pull/12190) Replace https://github.com/cosmos/cosmos-sdk/pull/12190 to use `NewBaseAccountWithAddress` in all vesting account message handlers.

types/errors/errors.go

@@ -138,6 +138,10 @@ var (
 	// ErrAppConfig defines an error occurred if min-gas-prices field in BaseConfig is empty.
 	ErrAppConfig = Register(RootCodespace, 40, "error in app.toml")
 
+	// ErrInvalidGasLimit defines an error when an invalid GasWanted value is
+	// supplied.
+	ErrInvalidGasLimit = Register(RootCodespace, 41, "invalid gas limit")
+
 	// ErrPanic is only set when we recover from a panic, so we know to
 	// redact potentially sensitive system info
 	ErrPanic = Register(UndefinedCodespace, 111222, "panic")

x/auth/ante/fee.go

@@ -46,15 +46,17 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKee
 	return dfd
 }
 
-// SetMinGasPrices sets the minimum-gas-prices value in the state of DeductFeeDecorator
-func (dfd *DeductFeeDecorator) SetMinGasPrices(minGasPrices sdk.DecCoins) {
-	dfd.minGasPrices = minGasPrices
-}
+func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
+	feeTx, ok := tx.(sdk.FeeTx)
+	if !ok {
+		return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
+	}
+
+	if ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 {
+		return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas")
+	}
 
-// AnteHandle implements an AnteHandler decorator for the DeductFeeDecorator
-func (dfd *DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) {
-	dfd.minGasPrices = ctx.MinGasPrices()
-	txPriority, err := dfd.innerValidateTx(ctx, tx)
+	fee, priority, err := dfd.txFeeChecker(ctx, tx)
 	if err != nil {
 		return ctx, err
 	}

x/auth/ante/fee_test.go

@@ -17,101 +17,95 @@ import (
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 )
 
-func TestDeductFeeDecorator_ZeroGas(t *testing.T) {
-	s := SetupTestSuite(t, true)
+func (s *AnteTestSuite) TestDeductFeeDecorator_ZeroGas() {
+	s.SetupTest(true) // setup
 	s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder()
 
-	mfd := ante.NewDeductFeeDecorator(s.accountKeeper, s.bankKeeper, s.feeGrantKeeper, nil)
+	mfd := ante.NewDeductFeeDecorator(s.app.AccountKeeper, s.app.BankKeeper, s.app.FeeGrantKeeper, nil)
 	antehandler := sdk.ChainAnteDecorators(mfd)
 
 	// keys and addresses
-	accs := s.CreateTestAccounts(1)
+	priv1, _, addr1 := testdata.KeyTestPubAddr()
+	coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(300)))
+	testutil.FundAccount(s.app.BankKeeper, s.ctx, addr1, coins)
 
 	// msg and signatures
-	msg := testdata.NewTestMsg(accs[0].acc.GetAddress())
-	require.NoError(t, s.txBuilder.SetMsgs(msg))
+	msg := testdata.NewTestMsg(addr1)
+	s.Require().NoError(s.txBuilder.SetMsgs(msg))
 
 	// set zero gas
 	s.txBuilder.SetGasLimit(0)
 
-	privs, accNums, accSeqs := []cryptotypes.PrivKey{accs[0].priv}, []uint64{0}, []uint64{0}
-	tx, err := s.CreateTestTx(s.ctx, privs, accNums, accSeqs, s.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT)
-	require.NoError(t, err)
+	privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0}
+	tx, err := s.CreateTestTx(privs, accNums, accSeqs, s.ctx.ChainID())
+	s.Require().NoError(err)
 
 	// Set IsCheckTx to true
 	s.ctx = s.ctx.WithIsCheckTx(true)
 
-	// Set current block height in headerInfo
-	headerInfo := s.ctx.HeaderInfo()
-	headerInfo.Height = s.ctx.BlockHeight()
-	s.ctx = s.ctx.WithHeaderInfo(headerInfo)
-
 	_, err = antehandler(s.ctx, tx, false)
-	require.Error(t, err)
-
-	// zero gas is accepted in simulation mode
-	s.ctx = s.ctx.WithExecMode(sdk.ExecModeSimulate)
-	_, err = antehandler(s.ctx, tx, true)
-	require.NoError(t, err)
+	s.Require().Error(err)
 }
 
-func TestEnsureMempoolFees(t *testing.T) {
-	s := SetupTestSuite(t, true) // setup
+func (s *AnteTestSuite) TestEnsureMempoolFees() {
+	s.SetupTest(true) // setup
 	s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder()
 
-	mfd := ante.NewDeductFeeDecorator(s.accountKeeper, s.bankKeeper, s.feeGrantKeeper, nil)
+	mfd := ante.NewDeductFeeDecorator(s.app.AccountKeeper, s.app.BankKeeper, s.app.FeeGrantKeeper, nil)
 	antehandler := sdk.ChainAnteDecorators(mfd)
 
 	// keys and addresses
-	accs := s.CreateTestAccounts(1)
+	priv1, _, addr1 := testdata.KeyTestPubAddr()
+	coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(300)))
+	testutil.FundAccount(s.app.BankKeeper, s.ctx, addr1, coins)
 
 	// msg and signatures
 	msg := testdata.NewTestMsg(accs[0].acc.GetAddress())
 	feeAmount := testdata.NewTestFeeAmount()
-	gasLimit := uint64(15)
-	require.NoError(t, s.txBuilder.SetMsgs(msg))
+	gasLimit := testdata.NewTestGasLimit()
+	s.Require().NoError(s.txBuilder.SetMsgs(msg))
 	s.txBuilder.SetFeeAmount(feeAmount)
 	s.txBuilder.SetGasLimit(gasLimit)
 
-	s.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), accs[0].acc.GetAddress(), authtypes.FeeCollectorName, feeAmount).Return(nil).Times(3)
-
-	privs, accNums, accSeqs := []cryptotypes.PrivKey{accs[0].priv}, []uint64{0}, []uint64{0}
-	tx, err := s.CreateTestTx(s.ctx, privs, accNums, accSeqs, s.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT)
-	require.NoError(t, err)
+	privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0}
+	tx, err := s.CreateTestTx(privs, accNums, accSeqs, s.ctx.ChainID())
+	s.Require().NoError(err)
 
 	// Set high gas price so standard test fee fails
 	atomPrice := sdk.NewDecCoinFromDec("atom", math.LegacyNewDec(20))
 	highGasPrice := []sdk.DecCoin{atomPrice}
 	s.ctx = s.ctx.WithMinGasPrices(highGasPrice)
 
+	// Set IsCheckTx to true
+	s.ctx = s.ctx.WithIsCheckTx(true)
+
 	// antehandler errors with insufficient fees
 	_, err = antehandler(s.ctx, tx, false)
-	require.NotNil(t, err, "Decorator should have errored on too low fee for local gasPrice")
+	s.Require().NotNil(err, "Decorator should have errored on too low fee for local gasPrice")
 
-	// antehandler should not error since we do not check minGasPrice in simulation mode
-	cacheCtx, _ := s.ctx.CacheContext()
-	cacheCtx = cacheCtx.WithExecMode(sdk.ExecModeSimulate)
-	_, err = antehandler(cacheCtx, tx, true)
-	require.Nil(t, err, "Decorator should not have errored in simulation mode")
+	// Set IsCheckTx to false
+	s.ctx = s.ctx.WithIsCheckTx(false)
 
 	// antehandler should not error since we do not check minGasPrice in DeliverTx
-	s.ctx = s.ctx.WithExecMode(sdk.ExecModeFinalize)
 	_, err = antehandler(s.ctx, tx, false)
-	require.Nil(t, err, "MempoolFeeDecorator returned error in DeliverTx")
+	s.Require().Nil(err, "MempoolFeeDecorator returned error in DeliverTx")
+
+	// Set IsCheckTx back to true for testing sufficient mempool fee
+	s.ctx = s.ctx.WithIsCheckTx(true)
 
 	atomPrice = sdk.NewDecCoinFromDec("atom", math.LegacyNewDec(0).Quo(math.LegacyNewDec(100000)))
 	lowGasPrice := []sdk.DecCoin{atomPrice}
 	s.ctx = s.ctx.WithMinGasPrices(lowGasPrice)
 
 	newCtx, err := antehandler(s.ctx, tx, false)
-	require.Nil(t, err, "Decorator should not have errored on fee higher than local gasPrice")
-	// Priority is the smallest gas price amount in any denom. Since we have only 1 gas price
-	// of 10atom, the priority here is 10.
-	require.Equal(t, int64(10), newCtx.Priority())
+	s.Require().Nil(err, "Decorator should not have errored on fee higher than local gasPrice")
+	// Priority is the smallest amount in any denom. Since we have only 1 fee
+	// of 150atom, the priority here is 150.
+	s.Require().Equal(feeAmount.AmountOf("atom").Int64(), newCtx.Priority())
 }
 
-func TestDeductFees(t *testing.T) {
-	s := SetupTestSuite(t, false)
+func (s *AnteTestSuite) TestDeductFees() {
+	s.SetupTest(false) // setup
 	s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder()
 
 	// keys and addresses
@@ -121,24 +115,34 @@ func TestDeductFees(t *testing.T) {
 	msg := testdata.NewTestMsg(accs[0].acc.GetAddress())
 	feeAmount := testdata.NewTestFeeAmount()
 	gasLimit := testdata.NewTestGasLimit()
-	require.NoError(t, s.txBuilder.SetMsgs(msg))
+	s.Require().NoError(s.txBuilder.SetMsgs(msg))
 	s.txBuilder.SetFeeAmount(feeAmount)
 	s.txBuilder.SetGasLimit(gasLimit)
 
-	privs, accNums, accSeqs := []cryptotypes.PrivKey{accs[0].priv}, []uint64{0}, []uint64{0}
-	tx, err := s.CreateTestTx(s.ctx, privs, accNums, accSeqs, s.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT)
-	require.NoError(t, err)
+	privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0}
+	tx, err := s.CreateTestTx(privs, accNums, accSeqs, s.ctx.ChainID())
+	s.Require().NoError(err)
+
+	// Set account with insufficient funds
+	acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr1)
+	s.app.AccountKeeper.SetAccount(s.ctx, acc)
+	coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(10)))
+	err = testutil.FundAccount(s.app.BankKeeper, s.ctx, addr1, coins)
+	s.Require().NoError(err)
 
-	dfd := ante.NewDeductFeeDecorator(s.accountKeeper, s.bankKeeper, nil, nil)
+	dfd := ante.NewDeductFeeDecorator(s.app.AccountKeeper, s.app.BankKeeper, nil, nil)
 	antehandler := sdk.ChainAnteDecorators(dfd)
-	s.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(sdkerrors.ErrInsufficientFunds)
 
 	_, err = antehandler(s.ctx, tx, false)
 
-	require.NotNil(t, err, "Tx did not error when fee payer had insufficient funds")
+	s.Require().NotNil(err, "Tx did not error when fee payer had insufficient funds")
+
+	// Set account with sufficient funds
+	s.app.AccountKeeper.SetAccount(s.ctx, acc)
+	err = testutil.FundAccount(s.app.BankKeeper, s.ctx, addr1, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(200))))
+	s.Require().NoError(err)
 
-	s.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
 	_, err = antehandler(s.ctx, tx, false)
 
-	require.Nil(t, err, "Tx errored after account has been set with sufficient funds")
+	s.Require().Nil(err, "Tx errored after account has been set with sufficient funds")
 }