Skip to content

Commit

Permalink
refactor: optimize AnteHandler gas consumption (evmos#1388)
Browse files Browse the repository at this point in the history
* refactor: antehandler order and params optimization

* removed additional individual params for the feemarket module

* typo fix

* Apply suggestions from code review - Fede

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* typo fix

* formatted using gofumpt

* typo fix - missed negate operator

* missed to negate conditions

* added unit tests for the new param getter methods

* updated changelog

* Apply suggestions from code review

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* moved to improvements

* Converted unit tests into table-driven tests

* added Require().Equal() to test case

Co-authored-by: Federico Kunze Küllmer <[email protected]>
  • Loading branch information
Vvaradinov and fedekunze authored Oct 19, 2022
1 parent 5879750 commit 83e509b
Show file tree
Hide file tree
Showing 19 changed files with 258 additions and 66 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (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
* (feemarket) [\#1165](https://github.com/evmos/ethermint/pull/1165) Add hint in specs about different gas terminology in Cosmos and Ethereum.
* (cli) [#1226](https://github.com/evmos/ethermint/pull/1226) Add custom app db backend flag.
Expand Down
2 changes: 1 addition & 1 deletion app/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
coinAmount := sdk.NewCoin(evmtypes.DefaultEVMDenom, sdk.NewInt(20))
gasAmount := sdk.NewCoins(coinAmount)
gas := uint64(200000)
//reusing the gasAmount for deposit
// reusing the gasAmount for deposit
deposit := sdk.NewCoins(coinAmount)
txBuilder := suite.CreateTestEIP712SubmitProposal(from, privKey, "ethermint_9000-1", gas, gasAmount, deposit)
return txBuilder.GetTx()
Expand Down
36 changes: 19 additions & 17 deletions app/ante/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,8 @@ func NewEthSigVerificationDecorator(ek EVMKeeper) EthSigVerificationDecorator {
// won't see the error message.
func (esvd EthSigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
chainID := esvd.evmKeeper.ChainID()

params := esvd.evmKeeper.GetParams(ctx)

ethCfg := params.ChainConfig.EthereumConfig(chainID)
chainCfg := esvd.evmKeeper.GetChainConfig(ctx)
ethCfg := chainCfg.EthereumConfig(chainID)
blockNum := big.NewInt(ctx.BlockHeight())
signer := ethtypes.MakeSigner(ethCfg, blockNum)

Expand All @@ -53,8 +51,9 @@ func (esvd EthSigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, s
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}

allowUnprotectedTxs := esvd.evmKeeper.GetAllowUnprotectedTxs(ctx)
ethTx := msgEthTx.AsTransaction()
if !params.AllowUnprotectedTxs && !ethTx.Protected() {
if !allowUnprotectedTxs && !ethTx.Protected() {
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrNotSupported,
"rejected unprotected Ethereum txs. Please EIP155 sign your transaction to protect it against replay-attacks")
Expand Down Expand Up @@ -176,15 +175,13 @@ func NewEthGasConsumeDecorator(
// - transaction or block gas meter runs out of gas
// - sets the gas meter limit
func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
params := egcd.evmKeeper.GetParams(ctx)

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

blockHeight := big.NewInt(ctx.BlockHeight())
homestead := ethCfg.IsHomestead(blockHeight)
istanbul := ethCfg.IsIstanbul(blockHeight)
london := ethCfg.IsLondon(blockHeight)
evmDenom := params.EvmDenom
gasWanted := uint64(0)
var events sdk.Events

Expand Down Expand Up @@ -213,6 +210,7 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
gasWanted += txData.GetGas()
}

evmDenom := egcd.evmKeeper.GetEVMDenom(ctx)
fees, priority, err := egcd.evmKeeper.DeductTxCostsFromUserBalance(
ctx,
*msgEthTx,
Expand Down Expand Up @@ -436,9 +434,9 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
txFee := sdk.Coins{}
txGasLimit := uint64(0)

params := vbd.evmKeeper.GetParams(ctx)
chainCfg := vbd.evmKeeper.GetChainConfig(ctx)
chainID := vbd.evmKeeper.ChainID()
ethCfg := params.ChainConfig.EthereumConfig(chainID)
ethCfg := chainCfg.EthereumConfig(chainID)
baseFee := vbd.evmKeeper.GetBaseFee(ctx, ethCfg)

for _, msg := range protoTx.GetMsgs() {
Expand All @@ -460,17 +458,21 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
}

// return error if contract creation or call are disabled through governance
if !params.EnableCreate && txData.GetTo() == nil {
enableCreate := vbd.evmKeeper.GetEnableCreate(ctx)
enableCall := vbd.evmKeeper.GetEnableCall(ctx)

if !enableCreate && txData.GetTo() == nil {
return ctx, sdkerrors.Wrap(evmtypes.ErrCreateDisabled, "failed to create new contract")
} else if !params.EnableCall && txData.GetTo() != nil {
} else if !enableCall && txData.GetTo() != nil {
return ctx, sdkerrors.Wrap(evmtypes.ErrCallDisabled, "failed to call contract")
}

if baseFee == nil && txData.TxType() == ethtypes.DynamicFeeTxType {
return ctx, sdkerrors.Wrap(ethtypes.ErrTxTypeNotSupported, "dynamic fee tx not supported")
}

txFee = txFee.Add(sdk.NewCoin(params.EvmDenom, sdkmath.NewIntFromBigInt(txData.Fee())))
evmDenom := vbd.evmKeeper.GetEVMDenom(ctx)
txFee = txFee.Add(sdk.NewCoin(evmDenom, sdkmath.NewIntFromBigInt(txData.Fee())))
}

authInfo := protoTx.AuthInfo
Expand Down Expand Up @@ -546,8 +548,8 @@ func NewEthMempoolFeeDecorator(ek EVMKeeper) EthMempoolFeeDecorator {
// 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 {
params := mfd.evmKeeper.GetParams(ctx)
ethCfg := params.ChainConfig.EthereumConfig(mfd.evmKeeper.ChainID())
chainCfg := mfd.evmKeeper.GetChainConfig(ctx)
ethCfg := chainCfg.EthereumConfig(mfd.evmKeeper.ChainID())
baseFee := mfd.evmKeeper.GetBaseFee(ctx, ethCfg)
if baseFee == nil {
for _, msg := range tx.GetMsgs() {
Expand All @@ -556,7 +558,7 @@ func (mfd EthMempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulat
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}

evmDenom := params.EvmDenom
evmDenom := mfd.evmKeeper.GetEVMDenom(ctx)
feeAmt := ethMsg.GetFee()
glDec := sdk.NewDec(int64(ethMsg.GetGas()))
requiredFee := ctx.MinGasPrices().AmountOf(evmDenom).Mul(glDec)
Expand Down
8 changes: 4 additions & 4 deletions app/ante/fee_market.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ func NewGasWantedDecorator(
}

func (gwd GasWantedDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
params := gwd.evmKeeper.GetParams(ctx)
ethCfg := params.ChainConfig.EthereumConfig(gwd.evmKeeper.ChainID())
chainCfg := gwd.evmKeeper.GetChainConfig(ctx)
ethCfg := chainCfg.EthereumConfig(gwd.evmKeeper.ChainID())

blockHeight := big.NewInt(ctx.BlockHeight())
isLondon := ethCfg.IsLondon(blockHeight)
Expand All @@ -39,10 +39,10 @@ func (gwd GasWantedDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo
}

gasWanted := feeTx.GetGas()
feeMktParams := gwd.feeMarketKeeper.GetParams(ctx)
isBaseFeeEnabled := gwd.feeMarketKeeper.GetBaseFeeEnabled(ctx)

// Add total gasWanted to cumulative in block transientStore in FeeMarket module
if feeMktParams.IsBaseFeeEnabled(ctx.BlockHeight()) {
if isBaseFeeEnabled {
if _, err := gwd.feeMarketKeeper.AddTransientGasWanted(ctx, gasWanted); err != nil {
return ctx, sdkerrors.Wrapf(err, "failed to add gas wanted to transient store")
}
Expand Down
8 changes: 4 additions & 4 deletions app/ante/fees.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ func (mpd MinGasPriceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
return next(ctx, tx, simulate)
}

evmParams := mpd.evmKeeper.GetParams(ctx)
evmDenom := mpd.evmKeeper.GetEVMDenom(ctx)
minGasPrices := sdk.DecCoins{
{
Denom: evmParams.EvmDenom,
Denom: evmDenom,
Amount: minGasPrice,
},
}
Expand Down Expand Up @@ -93,8 +93,8 @@ func (empd EthMinGasPriceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul
return next(ctx, tx, simulate)
}

paramsEvm := empd.evmKeeper.GetParams(ctx)
ethCfg := paramsEvm.ChainConfig.EthereumConfig(empd.evmKeeper.ChainID())
chainCfg := empd.evmKeeper.GetChainConfig(ctx)
ethCfg := chainCfg.EthereumConfig(empd.evmKeeper.ChainID())
baseFee := empd.evmKeeper.GetBaseFee(ctx, ethCfg)

for _, msg := range tx.GetMsgs() {
Expand Down
6 changes: 3 additions & 3 deletions app/ante/handler_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type HandlerOptions struct {
AccountKeeper evmtypes.AccountKeeper
BankKeeper evmtypes.BankKeeper
IBCKeeper *ibckeeper.Keeper
FeeMarketKeeper evmtypes.FeeMarketKeeper
FeeMarketKeeper FeeMarketKeeper
EvmKeeper EVMKeeper
FeegrantKeeper ante.FeegrantKeeper
SignModeHandler authsigning.SignModeHandler
Expand Down Expand Up @@ -70,9 +70,9 @@ func newCosmosAnteHandler(options HandlerOptions) sdk.AnteHandler {
RejectMessagesDecorator{}, // reject MsgEthereumTxs
ante.NewSetUpContextDecorator(),
ante.NewExtensionOptionsDecorator(options.ExtensionOptionChecker),
NewMinGasPriceDecorator(options.FeeMarketKeeper, options.EvmKeeper),
ante.NewValidateBasicDecorator(),
ante.NewTxTimeoutHeightDecorator(),
NewMinGasPriceDecorator(options.FeeMarketKeeper, options.EvmKeeper),
ante.NewValidateMemoDecorator(options.AccountKeeper),
ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
ante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
Expand All @@ -93,9 +93,9 @@ func newCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler {
ante.NewSetUpContextDecorator(),
// NOTE: extensions option decorator removed
// ante.NewRejectExtensionOptionsDecorator(),
NewMinGasPriceDecorator(options.FeeMarketKeeper, options.EvmKeeper),
ante.NewValidateBasicDecorator(),
ante.NewTxTimeoutHeightDecorator(),
NewMinGasPriceDecorator(options.FeeMarketKeeper, options.EvmKeeper),
ante.NewValidateMemoDecorator(options.AccountKeeper),
ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
ante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
Expand Down
6 changes: 6 additions & 0 deletions app/ante/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ type EVMKeeper interface {
GetBalance(ctx sdk.Context, addr common.Address) *big.Int
ResetTransientGasUsed(ctx sdk.Context)
GetTxIndexTransient(ctx sdk.Context) uint64
GetChainConfig(ctx sdk.Context) evmtypes.ChainConfig
GetEVMDenom(ctx sdk.Context) string
GetEnableCreate(ctx sdk.Context) bool
GetEnableCall(ctx sdk.Context) bool
GetAllowUnprotectedTxs(ctx sdk.Context) bool
}

type protoTxProvider interface {
Expand All @@ -44,4 +49,5 @@ type protoTxProvider interface {
type FeeMarketKeeper interface {
GetParams(ctx sdk.Context) (params feemarkettypes.Params)
AddTransientGasWanted(ctx sdk.Context, gasWanted uint64) (uint64, error)
GetBaseFeeEnabled(ctx sdk.Context) bool
}
8 changes: 5 additions & 3 deletions ethereum/eip712/preprocess_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ import (
)

// Testing Constants
var chainId = "ethermint_9000-1"
var ctx = client.Context{}.WithTxConfig(
encoding.MakeConfig(app.ModuleBasics).TxConfig,
var (
chainId = "ethermint_9000-1"
ctx = client.Context{}.WithTxConfig(
encoding.MakeConfig(app.ModuleBasics).TxConfig,
)
)
var feePayerAddress = "ethm17xpfvakm2amg962yls6f84z3kell8c5lthdzgl"

Expand Down
5 changes: 3 additions & 2 deletions tests/rpc/ws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ package rpc
import (
"encoding/json"
"fmt"
"github.com/gorilla/websocket"
"github.com/stretchr/testify/require"
"net/url"
"os"
"strings"
"testing"
"time"

"github.com/gorilla/websocket"
"github.com/stretchr/testify/require"
)

var (
Expand Down
35 changes: 35 additions & 0 deletions x/evm/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,38 @@ func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
k.paramSpace.SetParamSet(ctx, &params)
}

// GetChainConfig returns the chain configuration parameter.
func (k Keeper) GetChainConfig(ctx sdk.Context) types.ChainConfig {
chainCfg := types.ChainConfig{}
k.paramSpace.GetIfExists(ctx, types.ParamStoreKeyChainConfig, &chainCfg)
return chainCfg
}

// GetEVMDenom returns the EVM denom.
func (k Keeper) GetEVMDenom(ctx sdk.Context) string {
evmDenom := ""
k.paramSpace.GetIfExists(ctx, types.ParamStoreKeyEVMDenom, &evmDenom)
return evmDenom
}

// GetEnableCall returns true if the EVM Call operation is enabled.
func (k Keeper) GetEnableCall(ctx sdk.Context) bool {
enableCall := false
k.paramSpace.GetIfExists(ctx, types.ParamStoreKeyEnableCall, &enableCall)
return enableCall
}

// GetEnableCreate returns true if the EVM Create contract operation is enabled.
func (k Keeper) GetEnableCreate(ctx sdk.Context) bool {
enableCreate := false
k.paramSpace.GetIfExists(ctx, types.ParamStoreKeyEnableCreate, &enableCreate)
return enableCreate
}

// GetAllowUnprotectedTxs returns true if unprotected txs (i.e non-replay protected as per EIP-155) are supported by the chain.
func (k Keeper) GetAllowUnprotectedTxs(ctx sdk.Context) bool {
allowUnprotectedTx := false
k.paramSpace.GetIfExists(ctx, types.ParamStoreKeyAllowUnprotectedTxs, &allowUnprotectedTx)
return allowUnprotectedTx
}
89 changes: 85 additions & 4 deletions x/evm/keeper/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,94 @@ package keeper_test

import (
"github.com/evmos/ethermint/x/evm/types"
"reflect"
)

func (suite *KeeperTestSuite) TestParams() {
params := suite.app.EvmKeeper.GetParams(suite.ctx)
suite.Require().Equal(types.DefaultParams(), params)
params.EvmDenom = "inj"
suite.app.EvmKeeper.SetParams(suite.ctx, params)
newParams := suite.app.EvmKeeper.GetParams(suite.ctx)
suite.Require().Equal(newParams, params)
testCases := []struct {
name string
paramsFun func() interface{}
getFun func() interface{}
expected bool
}{
{
"success - Checks if the default params are set correctly",
func() interface{} {
return types.DefaultParams()
},
func() interface{} {
return suite.app.EvmKeeper.GetParams(suite.ctx)
},
true,
},
{
"success - EvmDenom param is set to \"inj\" and can be retrieved correctly",
func() interface{} {
params.EvmDenom = "inj"
suite.app.EvmKeeper.SetParams(suite.ctx, params)
return params.EvmDenom
},
func() interface{} {
return suite.app.EvmKeeper.GetEVMDenom(suite.ctx)
},
true,
},
{
"success - Check EnableCreate param is set to false and can be retrieved correctly",
func() interface{} {
params.EnableCreate = false
suite.app.EvmKeeper.SetParams(suite.ctx, params)
return params.EnableCreate
},
func() interface{} {
return suite.app.EvmKeeper.GetEnableCreate(suite.ctx)
},
true,
},
{
"success - Check EnableCall param is set to false and can be retrieved correctly",
func() interface{} {
params.EnableCall = false
suite.app.EvmKeeper.SetParams(suite.ctx, params)
return params.EnableCall
},
func() interface{} {
return suite.app.EvmKeeper.GetEnableCall(suite.ctx)
},
true,
},
{
"success - Check AllowUnprotectedTxs param is set to false and can be retrieved correctly",
func() interface{} {
params.AllowUnprotectedTxs = false
suite.app.EvmKeeper.SetParams(suite.ctx, params)
return params.AllowUnprotectedTxs
},
func() interface{} {
return suite.app.EvmKeeper.GetAllowUnprotectedTxs(suite.ctx)
},
true,
},
{
"success - Check ChainConfig param is set to the default value and can be retrieved correctly",
func() interface{} {
params.ChainConfig = types.DefaultChainConfig()
suite.app.EvmKeeper.SetParams(suite.ctx, params)
return params.ChainConfig
},
func() interface{} {
return suite.app.EvmKeeper.GetChainConfig(suite.ctx)
},
true,
},
}
for _, tc := range testCases {
suite.Run(tc.name, func() {
outcome := reflect.DeepEqual(tc.paramsFun(), tc.getFun())
suite.Require().Equal(tc.expected, outcome)
})
}

}
2 changes: 1 addition & 1 deletion x/evm/types/access_list_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (suite *TxDataTestSuite) TestAccessListTxGetGasFeeCap() {
func (suite *TxDataTestSuite) TestEmptyAccessList() {
testCases := []struct {
name string
tx AccessListTx
tx AccessListTx
}{
{
"empty access list tx",
Expand Down
Loading

0 comments on commit 83e509b

Please sign in to comment.