chore: verify fees refactor (#1496)

* chore: verify fees refactor

* adjust call structure in rest of repo after splitting up DeductTxCostsFromUserBalance

* adjust test logic after splitting DeductTxCostsFromUserBalance up

* remove outdated TODO

* address PR comments - remove import name for evm keeper

* remove misleading comment

* address review comments - only handover boolean instead of context

* remove TODO

Co-authored-by: MalteHerrmann <[email protected]>
Co-authored-by: MalteHerrmann <[email protected]>

Author: fedekunze

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")
 		}

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

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)

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