diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e7732b74e..4c6502c0f66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### 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. diff --git a/types/errors/errors.go b/types/errors/errors.go index c1a207f78f4..52cd584b209 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -161,6 +161,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 should only be set when we recovering from a panic ErrPanic = errorsmod.ErrPanic ) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index e2c3ef14d48..fb433241f25 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -37,6 +37,15 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKee } 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") + } + fee, priority, err := dfd.txFeeChecker(ctx, tx) if err != nil { return ctx, err diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index 357b5ca226d..c8ad40f3f51 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -8,66 +8,96 @@ import ( "github.com/cosmos/cosmos-sdk/x/bank/testutil" ) -func (suite *AnteTestSuite) TestEnsureMempoolFees() { - suite.SetupTest(true) // setup - suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() +func (s *AnteTestSuite) TestDeductFeeDecorator_ZeroGas() { + s.SetupTest(true) // setup + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() - mfd := ante.NewDeductFeeDecorator(suite.accountKeeper, suite.bankKeeper, suite.feeGrantKeeper, nil) + mfd := ante.NewDeductFeeDecorator(s.accountKeeper, s.bankKeeper, s.feeGrantKeeper, nil) antehandler := sdk.ChainAnteDecorators(mfd) // keys and addresses priv1, _, addr1 := testdata.KeyTestPubAddr() coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(300))) - testutil.FundAccount(suite.bankKeeper, suite.ctx, addr1, coins) + testutil.FundAccount(s.bankKeeper, s.ctx, addr1, coins) + + // msg and signatures + msg := testdata.NewTestMsg(addr1) + s.Require().NoError(s.txBuilder.SetMsgs(msg)) + + // set zero gas + s.txBuilder.SetGasLimit(0) + + 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) + + _, err = antehandler(s.ctx, tx, false) + s.Require().Error(err) +} + +func (s *AnteTestSuite) TestEnsureMempoolFees() { + s.SetupTest(true) // setup + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() + + mfd := ante.NewDeductFeeDecorator(s.accountKeeper, s.bankKeeper, s.feeGrantKeeper, nil) + antehandler := sdk.ChainAnteDecorators(mfd) + + // keys and addresses + priv1, _, addr1 := testdata.KeyTestPubAddr() + coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(300))) + testutil.FundAccount(s.bankKeeper, s.ctx, addr1, coins) // msg and signatures msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) - suite.txBuilder.SetFeeAmount(feeAmount) - suite.txBuilder.SetGasLimit(gasLimit) + s.Require().NoError(s.txBuilder.SetMsgs(msg)) + s.txBuilder.SetFeeAmount(feeAmount) + s.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) - suite.Require().NoError(err) + 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", sdk.NewDec(200).Quo(sdk.NewDec(100000))) highGasPrice := []sdk.DecCoin{atomPrice} - suite.ctx = suite.ctx.WithMinGasPrices(highGasPrice) + s.ctx = s.ctx.WithMinGasPrices(highGasPrice) // Set IsCheckTx to true - suite.ctx = suite.ctx.WithIsCheckTx(true) + s.ctx = s.ctx.WithIsCheckTx(true) // antehandler errors with insufficient fees - _, err = antehandler(suite.ctx, tx, false) - suite.Require().NotNil(err, "Decorator should have errored on too low fee for local gasPrice") + _, err = antehandler(s.ctx, tx, false) + s.Require().NotNil(err, "Decorator should have errored on too low fee for local gasPrice") // Set IsCheckTx to false - suite.ctx = suite.ctx.WithIsCheckTx(false) + s.ctx = s.ctx.WithIsCheckTx(false) // antehandler should not error since we do not check minGasPrice in DeliverTx - _, err = antehandler(suite.ctx, tx, false) - suite.Require().Nil(err, "MempoolFeeDecorator returned error in DeliverTx") + _, err = antehandler(s.ctx, tx, false) + s.Require().Nil(err, "MempoolFeeDecorator returned error in DeliverTx") // Set IsCheckTx back to true for testing sufficient mempool fee - suite.ctx = suite.ctx.WithIsCheckTx(true) + s.ctx = s.ctx.WithIsCheckTx(true) atomPrice = sdk.NewDecCoinFromDec("atom", sdk.NewDec(0).Quo(sdk.NewDec(100000))) lowGasPrice := []sdk.DecCoin{atomPrice} - suite.ctx = suite.ctx.WithMinGasPrices(lowGasPrice) + s.ctx = s.ctx.WithMinGasPrices(lowGasPrice) - newCtx, err := antehandler(suite.ctx, tx, false) - suite.Require().Nil(err, "Decorator should not have errored on fee higher than local gasPrice") + newCtx, err := antehandler(s.ctx, tx, false) + 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. - suite.Require().Equal(feeAmount.AmountOf("atom").Int64(), newCtx.Priority()) + s.Require().Equal(feeAmount.AmountOf("atom").Int64(), newCtx.Priority()) } -func (suite *AnteTestSuite) TestDeductFees() { - suite.SetupTest(false) // setup - suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() +func (s *AnteTestSuite) TestDeductFees() { + s.SetupTest(false) // setup + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() // keys and addresses priv1, _, addr1 := testdata.KeyTestPubAddr() @@ -76,34 +106,34 @@ func (suite *AnteTestSuite) TestDeductFees() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) - suite.txBuilder.SetFeeAmount(feeAmount) - suite.txBuilder.SetGasLimit(gasLimit) + s.Require().NoError(s.txBuilder.SetMsgs(msg)) + s.txBuilder.SetFeeAmount(feeAmount) + s.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) - suite.Require().NoError(err) + tx, err := s.CreateTestTx(privs, accNums, accSeqs, s.ctx.ChainID()) + s.Require().NoError(err) // Set account with insufficient funds - acc := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr1) - suite.accountKeeper.SetAccount(suite.ctx, acc) + acc := s.accountKeeper.NewAccountWithAddress(s.ctx, addr1) + s.accountKeeper.SetAccount(s.ctx, acc) coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(10))) - err = testutil.FundAccount(suite.bankKeeper, suite.ctx, addr1, coins) - suite.Require().NoError(err) + err = testutil.FundAccount(s.bankKeeper, s.ctx, addr1, coins) + s.Require().NoError(err) - dfd := ante.NewDeductFeeDecorator(suite.accountKeeper, suite.bankKeeper, nil, nil) + dfd := ante.NewDeductFeeDecorator(s.accountKeeper, s.bankKeeper, nil, nil) antehandler := sdk.ChainAnteDecorators(dfd) - _, err = antehandler(suite.ctx, tx, false) + _, err = antehandler(s.ctx, tx, false) - suite.Require().NotNil(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 - suite.accountKeeper.SetAccount(suite.ctx, acc) - err = testutil.FundAccount(suite.bankKeeper, suite.ctx, addr1, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(200)))) - suite.Require().NoError(err) + s.accountKeeper.SetAccount(s.ctx, acc) + err = testutil.FundAccount(s.bankKeeper, s.ctx, addr1, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(200)))) + s.Require().NoError(err) - _, err = antehandler(suite.ctx, tx, false) + _, err = antehandler(s.ctx, tx, false) - suite.Require().Nil(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") } diff --git a/x/auth/ante/testutil_test.go b/x/auth/ante/testutil_test.go index 9c16f583c6e..eb5683ea0cf 100644 --- a/x/auth/ante/testutil_test.go +++ b/x/auth/ante/testutil_test.go @@ -50,6 +50,10 @@ type AnteTestSuite struct { feeGrantKeeper feegrantkeeper.Keeper } +func TestAnteTestSuite(t *testing.T) { + suite.Run(t, new(AnteTestSuite)) +} + // SetupTest setups a new test, with new app, context, and anteHandler. func (suite *AnteTestSuite) SetupTest(isCheckTx bool) { var ( @@ -209,7 +213,3 @@ func (suite *AnteTestSuite) RunTestCase(privs []cryptotypes.PrivKey, msgs []sdk. } }) } - -func TestAnteTestSuite(t *testing.T) { - suite.Run(t, new(AnteTestSuite)) -}