From ccc6f5b53db4702f10eeb3bec384c9a2187547a0 Mon Sep 17 00:00:00 2001 From: yihuang Date: Wed, 15 Dec 2021 10:17:03 +0800 Subject: [PATCH] fix: use `EffectiveGasPrice` in ante handler for dynamic fee tx (#817) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 | 1 + app/ante/ante.go | 4 +-- app/ante/ante_test.go | 11 +++--- app/ante/eth.go | 55 ++++++++++++++++++++++++++++++ rpc/ethereum/namespaces/eth/api.go | 8 +++++ x/evm/keeper/utils.go | 16 +++------ x/evm/keeper/utils_test.go | 19 +++++------ x/evm/types/access_list_tx.go | 10 ++++++ x/evm/types/dynamic_fee_tx.go | 18 +++++++++- x/evm/types/legacy_tx.go | 10 ++++++ x/evm/types/msg.go | 18 ++++++++++ x/evm/types/tx_data.go | 6 ++++ 12 files changed, 145 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d56be1c..16c3ac75 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app/ante/ante.go b/app/ante/ante.go index fbb59016..656bb194 100644 --- a/app/ante/ante.go +++ b/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), diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index 6111383b..b01edddc 100644 --- a/app/ante/ante_test.go +++ b/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) diff --git a/app/ante/eth.go b/app/ante/eth.go index c51b53fb..870d81be 100644 --- a/app/ante/eth.go +++ b/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) +} diff --git a/rpc/ethereum/namespaces/eth/api.go b/rpc/ethereum/namespaces/eth/api.go index d0a230e2..3b4d3af9 100644 --- a/rpc/ethereum/namespaces/eth/api.go +++ b/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 } diff --git a/x/evm/keeper/utils.go b/x/evm/keeper/utils.go index 6a8b4e88..75303632 100644 --- a/x/evm/keeper/utils.go +++ b/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 diff --git a/x/evm/keeper/utils_test.go b/x/evm/keeper/utils_test.go index 6a3ba463..e4da6149 100644 --- a/x/evm/keeper/utils_test.go +++ b/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: ðtypes.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, ) diff --git a/x/evm/types/access_list_tx.go b/x/evm/types/access_list_tx.go index edfa857b..d318c426 100644 --- a/x/evm/types/access_list_tx.go +++ b/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() +} diff --git a/x/evm/types/dynamic_fee_tx.go b/x/evm/types/dynamic_fee_tx.go index bc4f6ae7..76c62172 100644 --- a/x/evm/types/dynamic_fee_tx.go +++ b/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()) +} diff --git a/x/evm/types/legacy_tx.go b/x/evm/types/legacy_tx.go index 888bd66f..1846d7be 100644 --- a/x/evm/types/legacy_tx.go +++ b/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() +} diff --git a/x/evm/types/msg.go b/x/evm/types/msg.go index ae03e251..469fcd8a 100644 --- a/x/evm/types/msg.go +++ b/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 { diff --git a/x/evm/types/tx_data.go b/x/evm/types/tx_data.go index 5ec2fa66..1f78ad26 100644 --- a/x/evm/types/tx_data.go +++ b/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) {