Skip to content

Commit

Permalink
Revert "imp(ante): refactor AnteHandler (evmos#1455)"
Browse files Browse the repository at this point in the history 
This reverts commit feed84b.
  • Loading branch information
@mmsqe
mmsqe committed Nov 28, 2022
1 parent 30b0c61 commit 41b9bc2
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 89 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (ante) [#1455](https://github.com/evmos/ethermint/pull/1455) Refactor `AnteHandler` logic
* (evm) [#1444](https://github.com/evmos/ethermint/pull/1444) Improve performance of `eth_estimateGas`
* (ante) [\#1388](https://github.com/evmos/ethermint/pull/1388) Optimize AnteHandler gas consumption
* (lint) [#1298](https://github.com/evmos/ethermint/pull/1298) 150 character line length limit, `gofumpt`, and linting
Expand Down
140 changes: 55 additions & 85 deletions app/ante/eth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func NewEthGasConsumeDecorator(
// (during CheckTx only) and that the sender has enough balance to pay for the gas cost.
//
// Intrinsic gas for a transaction is the amount of gas that the transaction uses before the
// transaction is executed. The gas is a constant value plus any cost incurred by additional bytes
// transaction is executed. The gas is a constant value plus any cost inccured by additional bytes
// of data supplied with the transaction.
//
// This AnteHandler decorator will fail if:
Expand All @@ -175,14 +175,7 @@ func NewEthGasConsumeDecorator(
// - user doesn't have enough balance to deduct the transaction fees (gas_limit * gas_price)
// - transaction or block gas meter runs out of gas
// - sets the gas meter limit
// - gas limit is greater than the block gas meter limit
func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
// gas consumption limit already checked during CheckTx so there's no need to
// verify it again during ReCheckTx
if ctx.IsReCheckTx() {
return next(ctx, tx, simulate)
}

chainCfg := egcd.evmKeeper.GetChainConfig(ctx)
ethCfg := chainCfg.EthereumConfig(egcd.evmKeeper.ChainID())

Expand Down Expand Up @@ -232,45 +225,32 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
return ctx, sdkerrors.Wrapf(err, "failed to deduct transaction costs from user balance")
}

events = append(events,
sdk.NewEvent(
sdk.EventTypeTx,
sdk.NewAttribute(sdk.AttributeKeyFee, fees.String()),
),
)

events = append(events, sdk.NewEvent(sdk.EventTypeTx, sdk.NewAttribute(sdk.AttributeKeyFee, fees.String())))
if priority < minPriority {
minPriority = priority
}
}

// TODO: change to typed events
ctx.EventManager().EmitEvents(events)

// TODO: deprecate after https://github.com/cosmos/cosmos-sdk/issues/9514 is fixed on SDK
blockGasLimit := ethermint.BlockGasLimit(ctx)

// return error if the tx gas is greater than the block limit (max gas)

// NOTE: it's important here to use the gas wanted instead of the gas consumed
// from the tx gas pool. The later only has the value so far since the
// EthSetupContextDecorator so it will never exceed the block gas limit.
if gasWanted > blockGasLimit {
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrOutOfGas,
"tx gas (%d) exceeds block gas limit (%d)",
gasWanted,
blockGasLimit,
)
// NOTE: safety check
if blockGasLimit > 0 {
// generate a copy of the gas pool (i.e block gas meter) to see if we've run out of gas for this block
// if current gas consumed is greater than the limit, this funcion panics and the error is recovered on the Baseapp
gasPool := sdk.NewGasMeter(blockGasLimit)
gasPool.ConsumeGas(ctx.GasMeter().GasConsumedToLimit(), "gas pool check")
}

// Set tx GasMeter with a limit of GasWanted (i.e gas limit from the Ethereum tx).
// The gas consumed will be then reset to the gas used by the state transition
// in the EVM.
// Set ctx.GasMeter with a limit of GasWanted (gasLimit)
gasConsumed := ctx.GasMeter().GasConsumed()
ctx = ctx.WithGasMeter(ethermint.NewInfiniteGasMeterWithLimit(gasWanted))
ctx.GasMeter().ConsumeGas(gasConsumed, "copy gas consumed")

// FIXME: use a custom gas configuration that doesn't add any additional gas and only
// takes into account the gas consumed at the end of the EVM transaction.
newCtx := ctx.
WithGasMeter(ethermint.NewInfiniteGasMeterWithLimit(gasWanted)).
WithPriority(minPriority)
newCtx := ctx.WithPriority(minPriority)

// we know that we have enough gas on the pool to cover the intrinsic gas
return next(newCtx, tx, simulate)
Expand Down Expand Up @@ -312,22 +292,6 @@ func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
)
}

if evmtypes.IsLondon(ethCfg, ctx.BlockHeight()) {
if baseFee == nil {
return ctx, sdkerrors.Wrap(
evmtypes.ErrInvalidBaseFee,
"base fee is supported but evm block context value is nil",
)
}
if coreMsg.GasFeeCap().Cmp(baseFee) < 0 {
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrInsufficientFee,
"max fee per gas less than block base fee (%s < %s)",
coreMsg.GasFeeCap(), baseFee,
)
}
}

// NOTE: pass in an empty coinbase address and nil tracer as we don't need them for the check below
cfg := &evmtypes.EVMConfig{
ChainConfig: ethCfg,
Expand All @@ -348,6 +312,22 @@ func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
coreMsg.From(),
)
}

if evmtypes.IsLondon(ethCfg, ctx.BlockHeight()) {
if baseFee == nil {
return ctx, sdkerrors.Wrap(
evmtypes.ErrInvalidBaseFee,
"base fee is supported but evm block context value is nil",
)
}
if coreMsg.GasFeeCap().Cmp(baseFee) < 0 {
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrInsufficientFee,
"max fee per gas less than block base fee (%s < %s)",
coreMsg.GasFeeCap(), baseFee,
)
}
}
}

return next(ctx, tx, simulate)
Expand Down Expand Up @@ -566,41 +546,31 @@ func NewEthMempoolFeeDecorator(ek EVMKeeper) EthMempoolFeeDecorator {
}
}

// AnteHandle ensures that the provided fees meet a minimum threshold for the validator.
// This check only for local mempool purposes, and thus it is only run on (Re)CheckTx.
// The logic is also skipped if the London hard fork and EIP-1559 are enabled.
// 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.
// It only do the check if london hardfork not enabled or feemarket not enabled, because in that case feemarket will take over the task.
func (mfd EthMempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
if !ctx.IsCheckTx() || simulate {
return next(ctx, tx, simulate)
}
chainCfg := mfd.evmKeeper.GetChainConfig(ctx)
ethCfg := chainCfg.EthereumConfig(mfd.evmKeeper.ChainID())

baseFee := mfd.evmKeeper.GetBaseFee(ctx, ethCfg)
// skip check as the London hard fork and EIP-1559 are enabled
if baseFee != nil {
return next(ctx, tx, simulate)
}

evmDenom := mfd.evmKeeper.GetEVMDenom(ctx)
minGasPrice := ctx.MinGasPrices().AmountOf(evmDenom)

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

fee := sdk.NewDecFromBigInt(ethMsg.GetFee())
gasLimit := sdk.NewDecFromBigInt(new(big.Int).SetUint64(ethMsg.GetGas()))
requiredFee := minGasPrice.Mul(gasLimit)

if fee.LT(requiredFee) {
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrInsufficientFee,
"insufficient fees; got: %s required: %s",
fee, requiredFee,
)
if ctx.IsCheckTx() && !simulate {
chainCfg := mfd.evmKeeper.GetChainConfig(ctx)
ethCfg := chainCfg.EthereumConfig(mfd.evmKeeper.ChainID())
baseFee := mfd.evmKeeper.GetBaseFee(ctx, ethCfg)
evmDenom := mfd.evmKeeper.GetEVMDenom(ctx)

if baseFee == nil {
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))
}

feeAmt := ethMsg.GetFee()
glDec := sdk.NewDec(int64(ethMsg.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)
}
}
}
}

Expand Down
3 changes: 0 additions & 3 deletions scripts/integration-test-all.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,12 @@ fi
echo "compiling ethermint"
make build


# PID array declaration
arr=()

init_func() {
"$PWD"/build/ethermintd keys add $KEY"$i" --keyring-backend test --home "$DATA_DIR$i" --no-backup --algo "eth_secp256k1"
"$PWD"/build/ethermintd init $MONIKER --chain-id $CHAINID --home "$DATA_DIR$i"
# Set gas limit in genesis
cat $DATA_DIR$i/config/genesis.json | jq '.consensus_params["block"]["max_gas"]="10000000"' > $DATA_DIR$i/config/tmp_genesis.json && mv $DATA_DIR$i/config/tmp_genesis.json $DATA_DIR$i/config/genesis.json
"$PWD"/build/ethermintd add-genesis-account \
"$("$PWD"/build/ethermintd keys show "$KEY$i" --keyring-backend test -a --home "$DATA_DIR$i")" 1000000000000000000aphoton,1000000000000000000stake \
--keyring-backend test --home "$DATA_DIR$i"
Expand Down

0 comments on commit 41b9bc2

Please sign in to comment.