diff --git a/app/ante/eth.go b/app/ante/eth.go index 684dd8f96b..c2f9fae80a 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -11,7 +11,7 @@ import ( errortypes "github.com/cosmos/cosmos-sdk/types/errors" ethermint "github.com/evmos/ethermint/types" - evmkeeper "github.com/evmos/ethermint/x/evm/keeper" + "github.com/evmos/ethermint/x/evm/keeper" "github.com/evmos/ethermint/x/evm/statedb" evmtypes "github.com/evmos/ethermint/x/evm/types" @@ -79,7 +79,7 @@ func (avd EthAccountVerificationDecorator) AnteHandle( "the sender is not EOA: address %s, codeHash <%s>", fromAddr, acct.CodeHash) } - if err := evmkeeper.CheckSenderBalance(sdkmath.NewIntFromBigInt(acct.Balance), txData); err != nil { + if err := keeper.CheckSenderBalance(sdkmath.NewIntFromBigInt(acct.Balance), txData); err != nil { return ctx, errorsmod.Wrap(err, "failed to check sender balance") } } @@ -132,7 +132,6 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula blockHeight := big.NewInt(ctx.BlockHeight()) homestead := ethCfg.IsHomestead(blockHeight) istanbul := ethCfg.IsIstanbul(blockHeight) - london := ethCfg.IsLondon(blockHeight) gasWanted := uint64(0) var events sdk.Events @@ -164,16 +163,12 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula evmDenom := egcd.evmKeeper.GetEVMDenom(ctx) - fees, err := egcd.evmKeeper.DeductTxCostsFromUserBalance( - ctx, - *msgEthTx, - txData, - evmDenom, - baseFee, - homestead, - istanbul, - london, - ) + fees, err := keeper.VerifyFee(txData, evmDenom, baseFee, homestead, istanbul, ctx.IsCheckTx()) + if err != nil { + return ctx, errorsmod.Wrapf(err, "failed to verify the fees") + } + + err = egcd.evmKeeper.DeductTxCostsFromUserBalance(ctx, fees, common.HexToAddress(msgEthTx.From)) if err != nil { return ctx, errorsmod.Wrapf(err, "failed to deduct transaction costs from user balance") } diff --git a/app/ante/interfaces.go b/app/ante/interfaces.go index b0d62039e4..3619b223df 100644 --- a/app/ante/interfaces.go +++ b/app/ante/interfaces.go @@ -28,9 +28,7 @@ type EVMKeeper interface { DynamicFeeEVMKeeper NewEVM(ctx sdk.Context, msg core.Message, cfg *evmtypes.EVMConfig, tracer vm.EVMLogger, stateDB vm.StateDB) evm.EVM - DeductTxCostsFromUserBalance( - ctx sdk.Context, msgEthTx evmtypes.MsgEthereumTx, txData evmtypes.TxData, denom string, baseFee *big.Int, homestead, istanbul, london bool, - ) (fees sdk.Coins, err error) + DeductTxCostsFromUserBalance(ctx sdk.Context, fees sdk.Coins, from common.Address) error GetBalance(ctx sdk.Context, addr common.Address) *big.Int ResetTransientGasUsed(ctx sdk.Context) GetTxIndexTransient(ctx sdk.Context) uint64 diff --git a/x/evm/handler_test.go b/x/evm/handler_test.go index 55ae77a935..923cbb6c43 100644 --- a/x/evm/handler_test.go +++ b/x/evm/handler_test.go @@ -2,6 +2,7 @@ package evm_test import ( "errors" + "github.com/evmos/ethermint/x/evm/keeper" "math/big" "testing" "time" @@ -599,7 +600,9 @@ func (suite *EvmTestSuite) TestERC20TransferReverted() { txData, err := types.UnpackTxData(tx.Data) suite.Require().NoError(err) - _, err = k.DeductTxCostsFromUserBalance(suite.ctx, *tx, txData, "aphoton", baseFee, true, true, true) + fees, err := keeper.VerifyFee(txData, "aphoton", baseFee, true, true, suite.ctx.IsCheckTx()) + suite.Require().NoError(err) + err = k.DeductTxCostsFromUserBalance(suite.ctx, fees, common.HexToAddress(tx.From)) suite.Require().NoError(err) res, err := k.EthereumTx(sdk.WrapSDKContext(suite.ctx), tx) diff --git a/x/evm/keeper/utils.go b/x/evm/keeper/utils.go index beffb5e883..bc9ca6492b 100644 --- a/x/evm/keeper/utils.go +++ b/x/evm/keeper/utils.go @@ -11,19 +11,41 @@ import ( evmtypes "github.com/evmos/ethermint/x/evm/types" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" ethtypes "github.com/ethereum/go-ethereum/core/types" ) -// DeductTxCostsFromUserBalance it calculates the tx costs and deducts the fees -func (k Keeper) DeductTxCostsFromUserBalance( +// DeductTxCostsFromUserBalance deducts the fees from the user balance. Returns an +// error if the specified sender address does not exist or the account balance is not sufficient. +func (k *Keeper) DeductTxCostsFromUserBalance( ctx sdk.Context, - msgEthTx evmtypes.MsgEthereumTx, + fees sdk.Coins, + from common.Address, +) error { + // fetch sender account + signerAcc, err := authante.GetSignerAcc(ctx, k.accountKeeper, from.Bytes()) + if err != nil { + return errorsmod.Wrapf(err, "account not found for sender %s", from) + } + + // deduct the full gas cost from the user balance + if err := authante.DeductFees(k.bankKeeper, ctx, signerAcc, fees); err != nil { + return errorsmod.Wrapf(err, "failed to deduct full gas cost %s from the user %s balance", fees, from) + } + + return nil +} + +// VerifyFee is used to return the fee for the given transaction data in sdk.Coins. It checks that the +// gas limit is not reached, the gas limit is higher than the intrinsic gas and that the +// base fee is higher than the gas fee cap. +func VerifyFee( txData evmtypes.TxData, denom string, baseFee *big.Int, - homestead, istanbul, london bool, -) (fees sdk.Coins, err error) { + homestead, istanbul, isCheckTx bool, +) (sdk.Coins, error) { isContractCreation := txData.GetTo() == nil gasLimit := txData.GetGas() @@ -43,7 +65,7 @@ func (k Keeper) DeductTxCostsFromUserBalance( } // intrinsic gas verification during CheckTx - if ctx.IsCheckTx() && gasLimit < intrinsicGas { + if isCheckTx && gasLimit < intrinsicGas { return nil, errorsmod.Wrapf( errortypes.ErrOutOfGas, "gas limit too low: %d (gas limit) < %d (intrinsic gas)", gasLimit, intrinsicGas, @@ -63,24 +85,7 @@ func (k Keeper) DeductTxCostsFromUserBalance( return sdk.Coins{}, nil } - fees = sdk.Coins{{Denom: denom, Amount: sdkmath.NewIntFromBigInt(feeAmt)}} - - // fetch sender account from signature - signerAcc, err := authante.GetSignerAcc(ctx, k.accountKeeper, msgEthTx.GetFrom()) - if err != nil { - return nil, errorsmod.Wrapf(err, "account not found for sender %s", msgEthTx.From) - } - - // deduct the full gas cost from the user balance - if err := authante.DeductFees(k.bankKeeper, ctx, signerAcc, fees); err != nil { - return nil, errorsmod.Wrapf( - err, - "failed to deduct full gas cost %s from the user %s balance", - fees, msgEthTx.From, - ) - } - - return fees, nil + return sdk.Coins{{Denom: denom, Amount: sdkmath.NewIntFromBigInt(feeAmt)}}, nil } // CheckSenderBalance validates that the tx cost value is positive and that the diff --git a/x/evm/keeper/utils_test.go b/x/evm/keeper/utils_test.go index 00a52a73bc..c55868addb 100644 --- a/x/evm/keeper/utils_test.go +++ b/x/evm/keeper/utils_test.go @@ -8,7 +8,7 @@ import ( "github.com/ethereum/go-ethereum/common" ethtypes "github.com/ethereum/go-ethereum/core/types" ethparams "github.com/ethereum/go-ethereum/params" - evmkeeper "github.com/evmos/ethermint/x/evm/keeper" + "github.com/evmos/ethermint/x/evm/keeper" evmtypes "github.com/evmos/ethermint/x/evm/types" ) @@ -207,7 +207,8 @@ func (suite *KeeperTestSuite) TestCheckSenderBalance() { vmdb.AddBalance(suite.address, hundredInt.BigInt()) balance := vmdb.GetBalance(suite.address) suite.Require().Equal(balance, hundredInt.BigInt()) - vmdb.Commit() + err := vmdb.Commit() + suite.Require().NoError(err, "Unexpected error while committing to vmdb: %d", err) for i, tc := range testCases { suite.Run(tc.name, func() { @@ -237,7 +238,7 @@ func (suite *KeeperTestSuite) TestCheckSenderBalance() { txData, _ := evmtypes.UnpackTxData(tx.Data) acct := suite.app.EvmKeeper.GetAccountOrEmpty(suite.ctx, suite.address) - err := evmkeeper.CheckSenderBalance( + err := keeper.CheckSenderBalance( sdkmath.NewIntFromBigInt(acct.Balance), txData, ) @@ -251,7 +252,13 @@ func (suite *KeeperTestSuite) TestCheckSenderBalance() { } } -func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { +// TestVerifyFeeAndDeductTxCostsFromUserBalance is a test method for both the VerifyFee +// function and the DeductTxCostsFromUserBalance method. +// +// NOTE: This method combines testing for both functions, because these used to be +// in one function and share a lot of the same setup. +// In practice, the two tested functions will also be sequentially executed. +func (suite *KeeperTestSuite) TestVerifyFeeAndDeductTxCostsFromUserBalance() { hundredInt := sdkmath.NewInt(100) zeroInt := sdk.ZeroInt() oneInt := sdkmath.NewInt(1) @@ -262,126 +269,138 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { initBalance := sdkmath.NewInt((ethparams.InitialBaseFee + 10) * 105) testCases := []struct { - name string - gasLimit uint64 - gasPrice *sdkmath.Int - gasFeeCap *big.Int - gasTipCap *big.Int - cost *sdkmath.Int - accessList *ethtypes.AccessList - expectPass bool - enableFeemarket bool - from string - malleate func() + name string + gasLimit uint64 + gasPrice *sdkmath.Int + gasFeeCap *big.Int + gasTipCap *big.Int + cost *sdkmath.Int + accessList *ethtypes.AccessList + expectPassVerify bool + expectPassDeduct bool + enableFeemarket bool + from string + malleate func() }{ { - name: "Enough balance", - gasLimit: 10, - gasPrice: &oneInt, - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: true, - from: suite.address.String(), + name: "Enough balance", + gasLimit: 10, + gasPrice: &oneInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: true, + from: suite.address.String(), }, { - name: "Equal balance", - gasLimit: 100, - gasPrice: &oneInt, - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: true, - from: suite.address.String(), + name: "Equal balance", + gasLimit: 100, + gasPrice: &oneInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: true, + from: suite.address.String(), }, { - name: "Higher gas limit, not enough balance", - gasLimit: 105, - gasPrice: &oneInt, - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: false, - from: suite.address.String(), + name: "Higher gas limit, not enough balance", + gasLimit: 105, + gasPrice: &oneInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: false, + from: suite.address.String(), }, { - name: "Higher gas price, enough balance", - gasLimit: 20, - gasPrice: &fiveInt, - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: true, - from: suite.address.String(), + name: "Higher gas price, enough balance", + gasLimit: 20, + gasPrice: &fiveInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: true, + from: suite.address.String(), }, { - name: "Higher gas price, not enough balance", - gasLimit: 20, - gasPrice: &fiftyInt, - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: false, - from: suite.address.String(), + name: "Higher gas price, not enough balance", + gasLimit: 20, + gasPrice: &fiftyInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: false, + from: suite.address.String(), }, // This case is expected to be true because the fees can be deducted, but the tx // execution is going to fail because there is no more balance to pay the cost { - name: "Higher cost, enough balance", - gasLimit: 100, - gasPrice: &oneInt, - cost: &fiftyInt, - accessList: ðtypes.AccessList{}, - expectPass: true, - from: suite.address.String(), + name: "Higher cost, enough balance", + gasLimit: 100, + gasPrice: &oneInt, + cost: &fiftyInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: true, + from: suite.address.String(), }, // testcases with enableFeemarket enabled. { - name: "Invalid gasFeeCap w/ enableFeemarket", - gasLimit: 10, - gasFeeCap: big.NewInt(1), - gasTipCap: big.NewInt(1), - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: false, - enableFeemarket: true, - from: suite.address.String(), + name: "Invalid gasFeeCap w/ enableFeemarket", + gasLimit: 10, + gasFeeCap: big.NewInt(1), + gasTipCap: big.NewInt(1), + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: false, + expectPassDeduct: true, + enableFeemarket: true, + from: suite.address.String(), }, { - name: "empty tip fee is valid to deduct", - gasLimit: 10, - gasFeeCap: big.NewInt(ethparams.InitialBaseFee), - gasTipCap: big.NewInt(1), - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: true, - enableFeemarket: true, - from: suite.address.String(), + name: "empty tip fee is valid to deduct", + gasLimit: 10, + gasFeeCap: big.NewInt(ethparams.InitialBaseFee), + gasTipCap: big.NewInt(1), + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: true, + enableFeemarket: true, + from: suite.address.String(), }, { - name: "effectiveTip equal to gasTipCap", - gasLimit: 100, - gasFeeCap: big.NewInt(ethparams.InitialBaseFee + 2), - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: true, - enableFeemarket: true, - from: suite.address.String(), + name: "effectiveTip equal to gasTipCap", + gasLimit: 100, + gasFeeCap: big.NewInt(ethparams.InitialBaseFee + 2), + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: true, + enableFeemarket: true, + from: suite.address.String(), }, { - name: "effectiveTip equal to (gasFeeCap - baseFee)", - gasLimit: 105, - gasFeeCap: big.NewInt(ethparams.InitialBaseFee + 1), - gasTipCap: big.NewInt(2), - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: true, - enableFeemarket: true, - from: suite.address.String(), + name: "effectiveTip equal to (gasFeeCap - baseFee)", + gasLimit: 105, + gasFeeCap: big.NewInt(ethparams.InitialBaseFee + 1), + gasTipCap: big.NewInt(2), + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: true, + enableFeemarket: true, + from: suite.address.String(), }, { - name: "Invalid from address", - gasLimit: 10, - gasPrice: &oneInt, - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: false, - from: "", + name: "Invalid from address", + gasLimit: 10, + gasPrice: &oneInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: true, + expectPassDeduct: false, + from: "abcdef", }, { name: "Enough balance - with access list", @@ -394,17 +413,19 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { StorageKeys: []common.Hash{}, }, }, - expectPass: true, - from: suite.address.String(), + expectPassVerify: true, + expectPassDeduct: true, + from: suite.address.String(), }, { - name: "gasLimit < intrinsicGas during IsCheckTx", - gasLimit: 1, - gasPrice: &oneInt, - cost: &oneInt, - accessList: ðtypes.AccessList{}, - expectPass: false, - from: suite.address.String(), + name: "gasLimit < intrinsicGas during IsCheckTx", + gasLimit: 1, + gasPrice: &oneInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPassVerify: false, + expectPassDeduct: true, + from: suite.address.String(), malleate: func() { suite.ctx = suite.ctx.WithIsCheckTx(true) }, @@ -446,7 +467,8 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { balance := vmdb.GetBalance(suite.address) suite.Require().Equal(balance, hundredInt.BigInt()) } - vmdb.Commit() + err := vmdb.Commit() + suite.Require().NoError(err, "Unexpected error while committing to vmdb: %d", err) tx := evmtypes.NewTx(zeroInt.BigInt(), 1, &suite.address, amount, tc.gasLimit, gasPrice, gasFeeCap, gasTipCap, nil, tc.accessList) tx.From = tc.from @@ -457,19 +479,9 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { baseFee := suite.app.EvmKeeper.GetBaseFee(suite.ctx, ethCfg) priority := evmtypes.GetTxPriority(txData, baseFee) - fees, err := suite.app.EvmKeeper.DeductTxCostsFromUserBalance( - suite.ctx, - *tx, - txData, - evmtypes.DefaultEVMDenom, - baseFee, - false, - false, - suite.enableFeemarket, // london - ) - - if tc.expectPass { - suite.Require().NoError(err, "valid test %d failed", i) + fees, err := keeper.VerifyFee(txData, evmtypes.DefaultEVMDenom, baseFee, false, false, suite.ctx.IsCheckTx()) + if tc.expectPassVerify { + suite.Require().NoError(err, "valid test %d failed - '%s'", i, tc.name) if tc.enableFeemarket { baseFee := suite.app.FeeMarketKeeper.GetBaseFee(suite.ctx) suite.Require().Equal( @@ -477,7 +489,7 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { sdk.NewCoins( sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewIntFromBigInt(txData.EffectiveFee(baseFee))), ), - "valid test %d failed, fee value is wrong ", i, + "valid test %d failed, fee value is wrong - '%s'", i, tc.name, ) suite.Require().Equal(int64(0), priority) } else { @@ -486,12 +498,19 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { sdk.NewCoins( sdk.NewCoin(evmtypes.DefaultEVMDenom, tc.gasPrice.Mul(sdkmath.NewIntFromUint64(tc.gasLimit))), ), - "valid test %d failed, fee value is wrong ", i, + "valid test %d failed, fee value is wrong - '%s'", i, tc.name, ) } } else { - suite.Require().Error(err, "invalid test %d passed", i) - suite.Require().Nil(fees, "invalid test %d passed. fees value must be nil", i) + suite.Require().Error(err, "invalid test %d passed - '%s'", i, tc.name) + suite.Require().Nil(fees, "invalid test %d passed. fees value must be nil - '%s'", i, tc.name) + } + + err = suite.app.EvmKeeper.DeductTxCostsFromUserBalance(suite.ctx, fees, common.HexToAddress(tx.From)) + if tc.expectPassDeduct { + suite.Require().NoError(err, "valid test %d failed - '%s'", i, tc.name) + } else { + suite.Require().Error(err, "invalid test %d passed - '%s'", i, tc.name) } }) }