fix: use EffectiveGasPrice in ante handler for dynamic fee tx (#817)

* Use effectiveGasPrice in ante handler for dynamic fee tx

Closes: #814

Solution:
- use effectiveGasPrice when check minimal-gas-prices, and deduct fee in ante handler
- implement an EthMempoolFeeDecorator

* add effectiveGasPrice to tx receipt

* changelog

* fix unit test

* fix comments

* add comments

* Apply suggestions from code review

Co-authored-by: Thomas Nguy <81727899+thomas-nguy@users.noreply.github.com>

* review suggestions

Co-authored-by: Thomas Nguy <81727899+thomas-nguy@users.noreply.github.com>
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

CHANGELOG.md

@@ -41,6 +41,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 - (evm) [tharsis#840](https://github.com/tharsis/ethermint/pull/840) Store empty topics as empty array rather than nil.
 - (feemarket) [tharsis#822](https://github.com/tharsis/ethermint/pull/822) Update EIP1559 base fee in `BeginBlock`.
+- (evm) [tharsis#817](https://github.com/tharsis/ethermint/pull/817) Use `effectiveGasPrice` in ante handler, add `effectiveGasPrice` to tx receipt.
 
 ### Improvements
 

app/ante/ante.go

@@ -53,8 +53,8 @@ func NewAnteHandler(
 					// handle as *evmtypes.MsgEthereumTx
 
 					anteHandler = sdk.ChainAnteDecorators(
-						NewEthSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
-						authante.NewMempoolFeeDecorator(),
+						NewEthSetUpContextDecorator(),                         // outermost AnteDecorator. SetUpContext must be called first
+						NewEthMempoolFeeDecorator(evmKeeper, feeMarketKeeper), // Check eth effective gas price against minimal-gas-prices
 						authante.NewTxTimeoutHeightDecorator(),
 						authante.NewValidateMemoDecorator(ak),
 						NewEthValidateBasicDecorator(evmKeeper),

app/ante/ante_test.go

@@ -193,7 +193,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 				// bigger than MaxGasWanted
 				txBuilder.SetGasLimit(uint64(1 << 63))
 				return txBuilder.GetTx()
-			}, false, true, false,
+			}, true, false, false,
 		},
 		{
 			"fail - CheckTx (memo too long)",
@@ -286,7 +286,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 
 	for _, tc := range testCases {
 		suite.Run(tc.name, func() {
-			suite.ctx = suite.ctx.WithIsCheckTx(tc.reCheckTx).WithIsReCheckTx(tc.reCheckTx)
+			suite.ctx = suite.ctx.WithIsCheckTx(tc.checkTx).WithIsReCheckTx(tc.reCheckTx)
 
 			// expConsumed := params.TxGasContractCreation + params.TxGas
 			_, err := suite.anteHandler(suite.ctx, tc.txFn(), false)
@@ -314,8 +314,6 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
 	suite.Require().NoError(acc.SetSequence(1))
 	suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
 
-	suite.app.EvmKeeper.AddBalance(addr, big.NewInt((ethparams.InitialBaseFee+10)*100000))
-
 	testCases := []struct {
 		name      string
 		txFn      func() sdk.Tx
@@ -501,7 +499,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
 				// bigger than MaxGasWanted
 				txBuilder.SetGasLimit(uint64(1 << 63))
 				return txBuilder.GetTx()
-			}, false, true, false,
+			}, true, false, false,
 		},
 		{
 			"fail - CheckTx (memo too long)",
@@ -530,7 +528,8 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
 
 	for _, tc := range testCases {
 		suite.Run(tc.name, func() {
-			suite.ctx = suite.ctx.WithIsCheckTx(tc.reCheckTx).WithIsReCheckTx(tc.reCheckTx)
+			suite.ctx = suite.ctx.WithIsCheckTx(tc.checkTx).WithIsReCheckTx(tc.reCheckTx)
+			suite.app.EvmKeeper.AddBalance(addr, big.NewInt((ethparams.InitialBaseFee+10)*100000))
 			_, err := suite.anteHandler(suite.ctx, tc.txFn(), false)
 			if tc.expPass {
 				suite.Require().NoError(err)

app/ante/eth.go

@@ -543,3 +543,58 @@ func (esc EthSetupContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul
 	newCtx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	return next(newCtx, tx, simulate)
 }
+
+// EthMempoolFeeDecorator will check if the transaction's effective fee is at least as large
+// as the local validator's minimum gasFee (defined in validator config).
+// If fee is too low, decorator returns error and tx is rejected from mempool.
+// Note this only applies when ctx.CheckTx = true
+// If fee is high enough or not CheckTx, then call next AnteHandler
+// CONTRACT: Tx must implement FeeTx to use MempoolFeeDecorator
+type EthMempoolFeeDecorator struct {
+	feemarketKeeper evmtypes.FeeMarketKeeper
+	evmKeeper       EVMKeeper
+}
+
+func NewEthMempoolFeeDecorator(ek EVMKeeper, fmk evmtypes.FeeMarketKeeper) EthMempoolFeeDecorator {
+	return EthMempoolFeeDecorator{
+		feemarketKeeper: fmk,
+		evmKeeper:       ek,
+	}
+}
+
+// AnteHandle ensures that the provided fees meet a minimum threshold for the validator,
+// if this is a CheckTx. This is only for local mempool purposes, and thus
+// is only ran on check tx.
+func (mfd EthMempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
+	if ctx.IsCheckTx() && !simulate {
+		if len(tx.GetMsgs()) != 1 {
+			return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "only 1 ethereum msg supported per tx")
+		}
+		msg, ok := tx.GetMsgs()[0].(*evmtypes.MsgEthereumTx)
+		if !ok {
+			return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type %T, expected %T", tx, (*evmtypes.MsgEthereumTx)(nil))
+		}
+
+		var feeAmt *big.Int
+
+		feeMktParams := mfd.feemarketKeeper.GetParams(ctx)
+		params := mfd.evmKeeper.GetParams(ctx)
+		chainID := mfd.evmKeeper.ChainID()
+		ethCfg := params.ChainConfig.EthereumConfig(chainID)
+		evmDenom := params.EvmDenom
+		if evmtypes.IsLondon(ethCfg, ctx.BlockHeight()) && !feeMktParams.NoBaseFee {
+			baseFee := mfd.feemarketKeeper.GetBaseFee(ctx)
+			feeAmt = msg.GetEffectiveFee(baseFee)
+		} else {
+			feeAmt = msg.GetFee()
+		}
+
+		glDec := sdk.NewDec(int64(msg.GetGas()))
+		requiredFee := ctx.MinGasPrices().AmountOf(evmDenom).Mul(glDec)
+		if sdk.NewDecFromBigInt(feeAmt).LT(requiredFee) {
+			return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeAmt, requiredFee)
+		}
+	}
+
+	return next(ctx, tx, simulate)
+}

rpc/ethereum/namespaces/eth/api.go

@@ -843,6 +843,14 @@ func (e *PublicAPI) GetTransactionReceipt(hash common.Hash) (map[string]interfac
 		receipt["contractAddress"] = crypto.CreateAddress(from, txData.GetNonce())
 	}
 
+	if dynamicTx, ok := txData.(*evmtypes.DynamicFeeTx); ok {
+		baseFee, err := e.backend.BaseFee(res.Height)
+		if err != nil {
+			return nil, err
+		}
+		receipt["effectiveGasPrice"] = hexutil.Big(*dynamicTx.GetEffectiveGasPrice(baseFee))
+	}
+
 	return receipt, nil
 }
 

x/evm/keeper/utils.go

@@ -9,7 +9,6 @@ import (
 
 	evmtypes "github.com/tharsis/ethermint/x/evm/types"
 
-	cmath "github.com/ethereum/go-ethereum/common/math"
 	"github.com/ethereum/go-ethereum/core"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 )
@@ -54,24 +53,19 @@ func (k Keeper) DeductTxCostsFromUserBalance(
 		)
 	}
 
-	// calculate the fees paid to validators based on the effective tip and price
-	effectiveTip := txData.GetGasPrice()
+	var feeAmt *big.Int
 
 	feeMktParams := k.feeMarketKeeper.GetParams(ctx)
-
 	if london && !feeMktParams.NoBaseFee && txData.TxType() == ethtypes.DynamicFeeTxType {
 		baseFee := k.feeMarketKeeper.GetBaseFee(ctx)
-		gasFeeGap := new(big.Int).Sub(txData.GetGasFeeCap(), baseFee)
-		if gasFeeGap.Sign() == -1 {
+		if txData.GetGasFeeCap().Cmp(baseFee) < 0 {
 			return nil, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "the tx gasfeecap is lower than the tx baseFee: %s (gasfeecap), %s (basefee) ", txData.GetGasFeeCap(), baseFee)
 		}
-
-		effectiveTip = cmath.BigMin(txData.GetGasTipCap(), gasFeeGap)
+		feeAmt = txData.EffectiveFee(baseFee)
+	} else {
+		feeAmt = txData.Fee()
 	}
 
-	gasUsed := new(big.Int).SetUint64(txData.GetGas())
-	feeAmt := new(big.Int).Mul(gasUsed, effectiveTip)
-
 	fees := sdk.Coins{sdk.NewCoin(denom, sdk.NewIntFromBigInt(feeAmt))}
 
 	// deduct the full gas cost from the user balance

x/evm/keeper/utils_test.go

@@ -5,7 +5,6 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/ethereum/go-ethereum/common"
-	cmath "github.com/ethereum/go-ethereum/common/math"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	ethparams "github.com/ethereum/go-ethereum/params"
 	evmkeeper "github.com/tharsis/ethermint/x/evm/keeper"
@@ -257,7 +256,9 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() {
 	oneInt := sdk.NewInt(1)
 	fiveInt := sdk.NewInt(5)
 	fiftyInt := sdk.NewInt(50)
-	hundredBaseFeeInt := sdk.NewInt(ethparams.InitialBaseFee * 100)
+
+	// should be enough to cover all test cases
+	initBalance := sdk.NewInt((ethparams.InitialBaseFee + 10) * 105)
 
 	testCases := []struct {
 		name         string
@@ -331,15 +332,14 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() {
 			expectPass:   false,
 			dynamicTxFee: true,
 		},
-		// TODO: is this case valid?
 		{
-			name:         "empty fee failed to deduct",
+			name:         "empty tip fee is valid to deduct",
 			gasLimit:     10,
 			gasFeeCap:    big.NewInt(ethparams.InitialBaseFee),
 			gasTipCap:    big.NewInt(1),
 			cost:         &oneInt,
 			accessList:   &ethtypes.AccessList{},
-			expectPass:   false,
+			expectPass:   true,
 			dynamicTxFee: true,
 		},
 		{
@@ -382,9 +382,9 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() {
 				} else {
 					gasTipCap = tc.gasTipCap
 				}
-				suite.app.EvmKeeper.AddBalance(suite.address, hundredBaseFeeInt.BigInt())
+				suite.app.EvmKeeper.AddBalance(suite.address, initBalance.BigInt())
 				balance := suite.app.EvmKeeper.GetBalance(suite.address)
-				suite.Require().Equal(balance, hundredBaseFeeInt.BigInt())
+				suite.Require().Equal(balance, initBalance.BigInt())
 			} else {
 				if tc.gasPrice != nil {
 					gasPrice = tc.gasPrice.BigInt()
@@ -414,13 +414,10 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() {
 				suite.Require().NoError(err, "valid test %d failed", i)
 				if tc.dynamicTxFee {
 					baseFee := suite.app.FeeMarketKeeper.GetBaseFee(suite.ctx)
-					gasFeeGap := new(big.Int).Sub(txData.GetGasFeeCap(), baseFee)
-					effectiveTip := cmath.BigMin(txData.GetGasTipCap(), gasFeeGap)
-
 					suite.Require().Equal(
 						fees,
 						sdk.NewCoins(
-							sdk.NewCoin(evmtypes.DefaultEVMDenom, sdk.NewIntFromBigInt(effectiveTip).Mul(sdk.NewIntFromUint64(tc.gasLimit))),
+							sdk.NewCoin(evmtypes.DefaultEVMDenom, sdk.NewIntFromBigInt(txData.EffectiveFee(baseFee))),
 						),
 						"valid test %d failed, fee value is wrong ", i,
 					)

x/evm/types/access_list_tx.go

@@ -229,3 +229,13 @@ func (tx AccessListTx) Fee() *big.Int {
 func (tx AccessListTx) Cost() *big.Int {
 	return cost(tx.Fee(), tx.GetValue())
 }
+
+// EffectiveFee is the same as Fee for AccessListTx
+func (tx AccessListTx) EffectiveFee(baseFee *big.Int) *big.Int {
+	return tx.Fee()
+}
+
+// EffectiveCost is the same as Cost for AccessListTx
+func (tx AccessListTx) EffectiveCost(baseFee *big.Int) *big.Int {
+	return tx.Cost()
+}

x/evm/types/dynamic_fee_tx.go

@@ -7,6 +7,7 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/math"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 
 	"github.com/tharsis/ethermint/types"
@@ -256,10 +257,25 @@ func (tx DynamicFeeTx) Validate() error {
 
 // Fee returns gasprice * gaslimit.
 func (tx DynamicFeeTx) Fee() *big.Int {
-	return fee(tx.GetGasPrice(), tx.GasLimit)
+	return fee(tx.GetGasFeeCap(), tx.GasLimit)
 }
 
 // Cost returns amount + gasprice * gaslimit.
 func (tx DynamicFeeTx) Cost() *big.Int {
 	return cost(tx.Fee(), tx.GetValue())
 }
+
+// GetEffectiveGasPrice returns the effective gas price
+func (tx *DynamicFeeTx) GetEffectiveGasPrice(baseFee *big.Int) *big.Int {
+	return math.BigMin(new(big.Int).Add(tx.GasTipCap.BigInt(), baseFee), tx.GasFeeCap.BigInt())
+}
+
+// EffectiveFee returns effective_gasprice * gaslimit.
+func (tx DynamicFeeTx) EffectiveFee(baseFee *big.Int) *big.Int {
+	return fee(tx.GetEffectiveGasPrice(baseFee), tx.GasLimit)
+}
+
+// EffectiveCost returns amount + effective_gasprice * gaslimit.
+func (tx DynamicFeeTx) EffectiveCost(baseFee *big.Int) *big.Int {
+	return cost(tx.EffectiveFee(baseFee), tx.GetValue())
+}

x/evm/types/legacy_tx.go

@@ -200,3 +200,13 @@ func (tx LegacyTx) Fee() *big.Int {
 func (tx LegacyTx) Cost() *big.Int {
 	return cost(tx.Fee(), tx.GetValue())
 }
+
+// EffectiveFee is the same as Fee for LegacyTx
+func (tx LegacyTx) EffectiveFee(baseFee *big.Int) *big.Int {
+	return tx.Fee()
+}
+
+// EffectiveCost is the same as Cost for LegacyTx
+func (tx LegacyTx) EffectiveCost(baseFee *big.Int) *big.Int {
+	return tx.Cost()
+}

x/evm/types/msg.go

@@ -245,6 +245,24 @@ func (msg MsgEthereumTx) GetGas() uint64 {
 	return txData.GetGas()
 }
 
+// GetFee returns the fee for non dynamic fee tx
+func (msg MsgEthereumTx) GetFee() *big.Int {
+	txData, err := UnpackTxData(msg.Data)
+	if err != nil {
+		return nil
+	}
+	return txData.Fee()
+}
+
+// GetEffectiveFee returns the fee for dynamic fee tx
+func (msg MsgEthereumTx) GetEffectiveFee(baseFee *big.Int) *big.Int {
+	txData, err := UnpackTxData(msg.Data)
+	if err != nil {
+		return nil
+	}
+	return txData.EffectiveFee(baseFee)
+}
+
 // GetFrom loads the ethereum sender address from the sigcache and returns an
 // sdk.AccAddress from its bytes
 func (msg *MsgEthereumTx) GetFrom() sdk.AccAddress {

x/evm/types/tx_data.go

@@ -36,8 +36,14 @@ type TxData interface {
 
 	AsEthereumData() ethtypes.TxData
 	Validate() error
+
+	// static fee
 	Fee() *big.Int
 	Cost() *big.Int
+
+	// effective fee according to current base fee
+	EffectiveFee(baseFee *big.Int) *big.Int
+	EffectiveCost(baseFee *big.Int) *big.Int
 }
 
 func NewTxDataFromTx(tx *ethtypes.Transaction) (TxData, error) {