From 23f88315e84d8de74679db02171aee489b30b808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Mon, 6 Jun 2022 10:15:10 +0200 Subject: [PATCH] ante: refactor (#1113) --- CHANGELOG.md | 24 +++---- app/ante/fee_market.go | 21 ++++--- app/ante/fees.go | 121 ++++++++++++++++++++++-------------- app/ante/handler_options.go | 2 +- 4 files changed, 99 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f354042..3c2e028f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,25 +40,25 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking -- (feemarket) [tharsis#1105](https://github.com/tharsis/ethermint/pull/1105) Update `BaseFee` calculation based on `GasWanted` instead of `GasUsed`. +* (feemarket) [tharsis#1105](https://github.com/tharsis/ethermint/pull/1105) Update `BaseFee` calculation based on `GasWanted` instead of `GasUsed`. ### API Breaking -- (feemarket) [tharsis#1104](https://github.com/tharsis/ethermint/pull/1104) Enforce a minimum gas price for Cosmos and EVM transactions through the `MinGasPrice` parameter. -- (rpc) [tharsis#1081](https://github.com/tharsis/ethermint/pull/1081) Deduplicate some json-rpc logic codes, cleanup several dead functions. -- (ante) [tharsis#1062](https://github.com/tharsis/ethermint/pull/1062) Emit event of eth tx hash in ante handler to support query failed transactions. -- (analytics) [tharsis#1106](https://github.com/tharsis/ethermint/pull/1106) Update telemetry to Ethermint modules. -- (rpc) [tharsis#1108](https://github.com/tharsis/ethermint/pull/1108) Update GetGasPrice RPC endpoint with global `MinGasPrice` +* (feemarket) [tharsis#1104](https://github.com/tharsis/ethermint/pull/1104) Enforce a minimum gas price for Cosmos and EVM transactions through the `MinGasPrice` parameter. +* (rpc) [tharsis#1081](https://github.com/tharsis/ethermint/pull/1081) Deduplicate some json-rpc logic codes, cleanup several dead functions. +* (ante) [tharsis#1062](https://github.com/tharsis/ethermint/pull/1062) Emit event of eth tx hash in ante handler to support query failed transactions. +* (analytics) [tharsis#1106](https://github.com/tharsis/ethermint/pull/1106) Update telemetry to Ethermint modules. +* (rpc) [tharsis#1108](https://github.com/tharsis/ethermint/pull/1108) Update GetGasPrice RPC endpoint with global `MinGasPrice` ### Improvements -* (cli) [tharsis#1086](https://github.com/tharsis/ethermint/pull/1086) Add rollback command. -* (specs) [tharsis#1095](https://github.com/tharsis/ethermint/pull/1095) Add more evm specs concepts. +* (cli) [tharsis#1086](https://github.com/tharsis/ethermint/pull/1086) Add rollback command. +* (specs) [tharsis#1095](https://github.com/tharsis/ethermint/pull/1095) Add more evm specs concepts. * (evm) [tharsis#1101](https://github.com/tharsis/ethermint/pull/1101) Add tx_type, gas and counter telemetry for ethereum txs. ### Bug Fixes -* (rpc) [tharsis#1082](https://github.com/tharsis/ethermint/pull/1082) fix gas price returned in getTransaction api. +* (rpc) [tharsis#1082](https://github.com/tharsis/ethermint/pull/1082) fix gas price returned in getTransaction api. * (evm) [tharsis#1088](https://github.com/tharsis/ethermint/pull/1088) Fix ability to append log in tx post processing. * (rpc) [tharsis#1081](https://github.com/tharsis/ethermint/pull/1081) fix `debug_getBlockRlp`/`debug_printBlock` don't filter failed transactions. * (ante) [tharsis#1111](https://github.com/tharsis/ethermint/pull/1111) Move CanTransfer decorator before GasConsume decorator @@ -279,9 +279,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (rpc) [tharsis#661](https://github.com/tharsis/ethermint/pull/661) Fix OOM bug when creating too many filters using JSON-RPC. * (evm) [tharsis#660](https://github.com/tharsis/ethermint/pull/660) Fix `nil` pointer panic in `ApplyNativeMessage`. * (evm, test) [tharsis#649](https://github.com/tharsis/ethermint/pull/649) Test DynamicFeeTx. -* (evm) [tharsis#702](https://github.com/tharsis/ethermint/pull/702) Fix panic in web3 RPC handlers -* (rpc) [tharsis#720](https://github.com/tharsis/ethermint/pull/720) Fix `debug_traceTransaction` failure -* (rpc) [tharsis#741](https://github.com/tharsis/ethermint/pull/741) Fix `eth_getBlockByNumberAndHash` return with non eth txs +* (evm) [tharsis#702](https://github.com/tharsis/ethermint/pull/702) Fix panic in web3 RPC handlers +* (rpc) [tharsis#720](https://github.com/tharsis/ethermint/pull/720) Fix `debug_traceTransaction` failure +* (rpc) [tharsis#741](https://github.com/tharsis/ethermint/pull/741) Fix `eth_getBlockByNumberAndHash` return with non eth txs * (rpc) [tharsis#743](https://github.com/tharsis/ethermint/pull/743) Fix debug JSON RPC handler crash on non-existing block ### Improvements diff --git a/app/ante/fee_market.go b/app/ante/fee_market.go index 565f2570..2f82b506 100644 --- a/app/ante/fee_market.go +++ b/app/ante/fee_market.go @@ -31,17 +31,20 @@ func (gwd GasWantedDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo ethCfg := params.ChainConfig.EthereumConfig(gwd.evmKeeper.ChainID()) blockHeight := big.NewInt(ctx.BlockHeight()) - london := ethCfg.IsLondon(blockHeight) + isLondon := ethCfg.IsLondon(blockHeight) feeTx, ok := tx.(sdk.FeeTx) - if ok && london { - gasWanted := feeTx.GetGas() - feeMktParams := gwd.feeMarketKeeper.GetParams(ctx) - // Add total gasWanted to cumulative in block transientStore in FeeMarket module - if feeMktParams.IsBaseFeeEnabled(ctx.BlockHeight()) { - if _, err := gwd.feeMarketKeeper.AddTransientGasWanted(ctx, gasWanted); err != nil { - return ctx, sdkerrors.Wrapf(err, "failed to add gas wanted to transient store") - } + if !ok || !isLondon { + return next(ctx, tx, simulate) + } + + gasWanted := feeTx.GetGas() + feeMktParams := gwd.feeMarketKeeper.GetParams(ctx) + + // Add total gasWanted to cumulative in block transientStore in FeeMarket module + if feeMktParams.IsBaseFeeEnabled(ctx.BlockHeight()) { + if _, err := gwd.feeMarketKeeper.AddTransientGasWanted(ctx, gasWanted); err != nil { + return ctx, sdkerrors.Wrapf(err, "failed to add gas wanted to transient store") } } diff --git a/app/ante/fees.go b/app/ante/fees.go index ae94101b..3aab89db 100644 --- a/app/ante/fees.go +++ b/app/ante/fees.go @@ -1,6 +1,8 @@ package ante import ( + "math/big" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -23,36 +25,46 @@ func NewMinGasPriceDecorator(fk FeeMarketKeeper, ek EVMKeeper) MinGasPriceDecora } func (mpd MinGasPriceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - minGasPrice := mpd.feesKeeper.GetParams(ctx).MinGasPrice - minGasPrices := sdk.DecCoins{sdk.DecCoin{ - Denom: mpd.evmKeeper.GetParams(ctx).EvmDenom, - Amount: minGasPrice, - }} - feeTx, ok := tx.(sdk.FeeTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } + minGasPrice := mpd.feesKeeper.GetParams(ctx).MinGasPrice + + // short-circuit if min gas price is 0 + if minGasPrice.IsZero() { + return next(ctx, tx, simulate) + } + + evmParams := mpd.evmKeeper.GetParams(ctx) + minGasPrices := sdk.DecCoins{ + { + Denom: evmParams.EvmDenom, + Amount: minGasPrice, + }, + } + feeCoins := feeTx.GetFee() gas := feeTx.GetGas() - if !minGasPrices.IsZero() { - requiredFees := make(sdk.Coins, len(minGasPrices)) + requiredFees := make(sdk.Coins, 0) - // Determine the required fees by multiplying each required minimum gas - // price by the gas limit, where fee = ceil(minGasPrice * gasLimit). - gasLimit := sdk.NewDec(int64(gas)) - for i, gp := range minGasPrices { - fee := gp.Amount.Mul(gasLimit) - requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) - } + // Determine the required fees by multiplying each required minimum gas + // price by the gas limit, where fee = ceil(minGasPrice * gasLimit). + gasLimit := sdk.NewDecFromBigInt(new(big.Int).SetUint64(gas)) - if !feeCoins.IsAnyGTE(requiredFees) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "provided fee < minimum global fee (%s < %s). Please increase the gas price.", feeCoins, requiredFees) + for _, gp := range minGasPrices { + fee := gp.Amount.Mul(gasLimit).Ceil().RoundInt() + if fee.IsPositive() { + requiredFees = requiredFees.Add(sdk.Coin{Denom: gp.Denom, Amount: fee}) } } + if !feeCoins.IsAnyGTE(requiredFees) { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "provided fee < minimum global fee (%s < %s). Please increase the gas price.", feeCoins, requiredFees) + } + return next(ctx, tx, simulate) } @@ -73,40 +85,55 @@ func NewEthMinGasPriceDecorator(fk FeeMarketKeeper, ek EVMKeeper) EthMinGasPrice func (empd EthMinGasPriceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { minGasPrice := empd.feesKeeper.GetParams(ctx).MinGasPrice - if !minGasPrice.IsZero() { - for _, msg := range tx.GetMsgs() { - ethMsg, ok := msg.(*evmtypes.MsgEthereumTx) - if !ok { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil)) - } + // short-circuit if min gas price is 0 + if minGasPrice.IsZero() { + return next(ctx, tx, simulate) + } - feeAmt := ethMsg.GetFee() + paramsEvm := empd.evmKeeper.GetParams(ctx) + ethCfg := paramsEvm.ChainConfig.EthereumConfig(empd.evmKeeper.ChainID()) + baseFee := empd.evmKeeper.GetBaseFee(ctx, ethCfg) - // For dynamic transactions, GetFee() uses the GasFeeCap value, which - // is the maximum gas price that the signer can pay. In practice, the - // signer can pay less, if the block's BaseFee is lower. So, in this case, - // we use the EffectiveFee. If the feemarket formula results in a BaseFee - // that lowers EffectivePrice until it is < MinGasPrices, the users must - // increase the GasTipCap (priority fee) until EffectivePrice > MinGasPrices. - // Transactions with MinGasPrices * gasUsed < tx fees < EffectiveFee are rejected - // by the feemarket AnteHandle - txData, err := evmtypes.UnpackTxData(ethMsg.Data) - if err != nil { - return ctx, sdkerrors.Wrapf(err, "failed to unpack tx data %s", ethMsg.Hash) - } - if txData.TxType() != ethtypes.LegacyTxType { - paramsEvm := empd.evmKeeper.GetParams(ctx) - ethCfg := paramsEvm.ChainConfig.EthereumConfig(empd.evmKeeper.ChainID()) - baseFee := empd.evmKeeper.GetBaseFee(ctx, ethCfg) - feeAmt = ethMsg.GetEffectiveFee(baseFee) - } + for _, msg := range tx.GetMsgs() { + ethMsg, ok := msg.(*evmtypes.MsgEthereumTx) + if !ok { + return ctx, sdkerrors.Wrapf( + sdkerrors.ErrUnknownRequest, + "invalid message type %T, expected %T", + msg, (*evmtypes.MsgEthereumTx)(nil), + ) + } - gasLimit := sdk.NewDec(int64(ethMsg.GetGas())) - requiredFee := minGasPrice.Mul(gasLimit) + feeAmt := ethMsg.GetFee() - if sdk.NewDecFromBigInt(feeAmt).LT(requiredFee) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "provided fee < minimum global fee (%s < %s). Please increase the priority tip (for EIP-1559 txs) or the gas prices (for access list or legacy txs)", feeAmt, requiredFee) - } + // For dynamic transactions, GetFee() uses the GasFeeCap value, which + // is the maximum gas price that the signer can pay. In practice, the + // signer can pay less, if the block's BaseFee is lower. So, in this case, + // we use the EffectiveFee. If the feemarket formula results in a BaseFee + // that lowers EffectivePrice until it is < MinGasPrices, the users must + // increase the GasTipCap (priority fee) until EffectivePrice > MinGasPrices. + // Transactions with MinGasPrices * gasUsed < tx fees < EffectiveFee are rejected + // by the feemarket AnteHandle + + txData, err := evmtypes.UnpackTxData(ethMsg.Data) + if err != nil { + return ctx, sdkerrors.Wrapf(err, "failed to unpack tx data %s", ethMsg.Hash) + } + + if txData.TxType() != ethtypes.LegacyTxType { + feeAmt = ethMsg.GetEffectiveFee(baseFee) + } + + gasLimit := sdk.NewDecFromBigInt(new(big.Int).SetUint64(ethMsg.GetGas())) + requiredFee := minGasPrice.Mul(gasLimit) + fee := sdk.NewDecFromBigInt(feeAmt) + + if fee.LT(requiredFee) { + return ctx, sdkerrors.Wrapf( + sdkerrors.ErrInsufficientFee, + "provided fee < minimum global fee (%s < %s). Please increase the priority tip (for EIP-1559 txs) or the gas prices (for access list or legacy txs)", + fee, requiredFee, + ) } } diff --git a/app/ante/handler_options.go b/app/ante/handler_options.go index c5261330..7a24125d 100644 --- a/app/ante/handler_options.go +++ b/app/ante/handler_options.go @@ -58,8 +58,8 @@ func newEthAnteHandler(options HandlerOptions) sdk.AnteHandler { NewCanTransferDecorator(options.EvmKeeper), NewEthGasConsumeDecorator(options.EvmKeeper, options.MaxTxGasWanted), NewEthIncrementSenderSequenceDecorator(options.AccountKeeper), // innermost AnteDecorator. - NewEthEmitEventDecorator(options.EvmKeeper), // emit eth tx hash and index at the very last ante handler. NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper), + NewEthEmitEventDecorator(options.EvmKeeper), // emit eth tx hash and index at the very last ante handler. ) }