From 9161ea9dfa68157546b091a510ad07bbf3b7bea2 Mon Sep 17 00:00:00 2001 From: yihuang Date: Sat, 23 Oct 2021 01:21:03 +0800 Subject: [PATCH] evm: refactor dup state transition code (#674) * Problem: state transition code is duplicated Closes: #672 Solution: - move gas refund out from ApplyMessage - move check into ApplyMessage - move evm construction into ApplyMessage - ensure context stack is clean after ApplyMessage return fix unit tests undo rename add underflow check * improve performance - don't duplicate params loading - passing EVMConfig around as pointer --- app/ante/eth.go | 10 +- .../namespaces/eth/filters/filters.go | 2 +- x/evm/keeper/grpc_query.go | 53 +--- x/evm/keeper/grpc_query_test.go | 14 +- x/evm/keeper/keeper.go | 8 + x/evm/keeper/state_transition.go | 245 ++++++++---------- .../keeper/state_transition_benchmark_test.go | 8 +- x/evm/types/config.go | 14 + x/evm/types/errors.go | 4 + 9 files changed, 166 insertions(+), 192 deletions(-) create mode 100644 x/evm/types/config.go diff --git a/app/ante/eth.go b/app/ante/eth.go index 4056821ccb..45860d64ec 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -18,7 +18,6 @@ import ( "github.com/ethereum/go-ethereum/core" ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" - "github.com/ethereum/go-ethereum/params" ) // EVMKeeper defines the expected keeper interface used on the Eth AnteHandler @@ -29,7 +28,7 @@ type EVMKeeper interface { GetParams(ctx sdk.Context) evmtypes.Params WithContext(ctx sdk.Context) ResetRefundTransient(ctx sdk.Context) - NewEVM(msg core.Message, config *params.ChainConfig, params evmtypes.Params, coinbase common.Address, tracer vm.Tracer) *vm.EVM + NewEVM(msg core.Message, cfg *evmtypes.EVMConfig, tracer vm.Tracer) *vm.EVM GetCodeHash(addr common.Address) common.Hash DeductTxCostsFromUserBalance( ctx sdk.Context, msgEthTx evmtypes.MsgEthereumTx, txData evmtypes.TxData, denom string, homestead, istanbul bool, @@ -364,7 +363,12 @@ func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate } // NOTE: pass in an empty coinbase address and nil tracer as we don't need them for the check below - evm := ctd.evmKeeper.NewEVM(coreMsg, ethCfg, params, common.Address{}, evmtypes.NewNoOpTracer()) + cfg := &evmtypes.EVMConfig{ + ChainConfig: ethCfg, + Params: params, + CoinBase: common.Address{}, + } + evm := ctd.evmKeeper.NewEVM(coreMsg, cfg, evmtypes.NewNoOpTracer()) // check that caller has enough balance to cover asset transfer for **topmost** call // NOTE: here the gas consumed is from the context with the infinite gas meter diff --git a/rpc/ethereum/namespaces/eth/filters/filters.go b/rpc/ethereum/namespaces/eth/filters/filters.go index 613f87e124..16c6f69df9 100644 --- a/rpc/ethereum/namespaces/eth/filters/filters.go +++ b/rpc/ethereum/namespaces/eth/filters/filters.go @@ -177,7 +177,7 @@ func (f *Filter) blockLogs(header *ethtypes.Header) ([]*ethtypes.Log, error) { return []*ethtypes.Log{}, errors.Wrapf(err, "failed to fetch logs block number %d", header.Number.Int64()) } - var unfiltered []*ethtypes.Log + unfiltered := make([]*ethtypes.Log, 0) for _, logs := range logsList { unfiltered = append(unfiltered, logs...) } diff --git a/x/evm/keeper/grpc_query.go b/x/evm/keeper/grpc_query.go index b731d0c23f..24c1c76c65 100644 --- a/x/evm/keeper/grpc_query.go +++ b/x/evm/keeper/grpc_query.go @@ -223,20 +223,7 @@ func (k Keeper) EthCall(c context.Context, req *types.EthCallRequest) (*types.Ms msg := args.ToMessage(req.GasCap) - params := k.GetParams(ctx) - ethCfg := params.ChainConfig.EthereumConfig(k.eip155ChainID) - - coinbase, err := k.GetCoinbaseAddress(ctx) - if err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } - - tracer := types.NewTracer(k.tracer, msg, ethCfg, k.Ctx().BlockHeight(), k.debug) - evm := k.NewEVM(msg, ethCfg, params, coinbase, tracer) - - // pass true means execute in query mode, which don't do actual gas refund. - res, err := k.ApplyMessage(evm, msg, ethCfg, true) - k.ctxStack.RevertAll() + res, err := k.ApplyMessage(msg, nil, false) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -291,12 +278,9 @@ func (k Keeper) EstimateGas(c context.Context, req *types.EthCallRequest) (*type } cap = hi - params := k.GetParams(ctx) - ethCfg := params.ChainConfig.EthereumConfig(k.eip155ChainID) - - coinbase, err := k.GetCoinbaseAddress(ctx) + cfg, err := k.EVMConfig(ctx) if err != nil { - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Error(codes.Internal, "failed to load evm config") } // Create a helper to check if a gas allowance results in an executable transaction @@ -308,13 +292,7 @@ func (k Keeper) EstimateGas(c context.Context, req *types.EthCallRequest) (*type msg := args.ToMessage(req.GasCap) - tracer := types.NewTracer(k.tracer, msg, ethCfg, k.Ctx().BlockHeight(), k.debug) - evm := k.NewEVM(msg, ethCfg, params, coinbase, tracer) - // pass true means execute in query mode, which don't do actual gas refund. - rsp, err := k.ApplyMessage(evm, msg, ethCfg, true) - - k.ctxStack.RevertAll() - + rsp, err := k.ApplyMessageWithConfig(msg, nil, false, cfg) if err != nil { if errors.Is(stacktrace.RootCause(err), core.ErrIntrinsicGas) { return true, nil, nil // Special case, raise gas limit @@ -364,11 +342,6 @@ func (k Keeper) TraceTx(c context.Context, req *types.QueryTraceTxRequest) (*typ ctx = ctx.WithHeaderHash(common.Hex2Bytes(req.BlockHash)) k.WithContext(ctx) - coinbase, err := k.GetCoinbaseAddress(ctx) - if err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } - params := k.GetParams(ctx) ethCfg := params.ChainConfig.EthereumConfig(k.eip155ChainID) signer := ethtypes.MakeSigner(ethCfg, big.NewInt(ctx.BlockHeight())) @@ -379,18 +352,17 @@ func (k Keeper) TraceTx(c context.Context, req *types.QueryTraceTxRequest) (*typ if err != nil { continue } - evm := k.NewEVM(msg, ethCfg, params, coinbase, types.NewNoOpTracer()) k.SetTxHashTransient(ethTx.Hash()) k.SetTxIndexTransient(uint64(i)) - _, err = k.ApplyMessage(evm, msg, ethCfg, true) + _, err = k.ApplyMessage(msg, types.NewNoOpTracer(), true) if err != nil { continue } } tx := req.Msg.AsTransaction() - result, err := k.traceTx(ctx, coinbase, signer, req.TxIndex, params, ethCfg, tx, req.TraceConfig) + result, err := k.traceTx(ctx, signer, req.TxIndex, ethCfg, tx, req.TraceConfig) if err != nil { // error will be returned with detail status from traceTx return nil, err @@ -431,15 +403,10 @@ func (k Keeper) TraceBlock(c context.Context, req *types.QueryTraceBlockRequest) txsLength := len(req.Txs) results := make([]*types.TxTraceResult, 0, txsLength) - coinbase, err := k.GetCoinbaseAddress(ctx) - if err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } - for i, tx := range req.Txs { result := types.TxTraceResult{} ethTx := tx.AsTransaction() - traceResult, err := k.traceTx(ctx, coinbase, signer, uint64(i), params, ethCfg, ethTx, req.TraceConfig) + traceResult, err := k.traceTx(ctx, signer, uint64(i), ethCfg, ethTx, req.TraceConfig) if err != nil { result.Error = err.Error() continue @@ -460,10 +427,8 @@ func (k Keeper) TraceBlock(c context.Context, req *types.QueryTraceBlockRequest) func (k *Keeper) traceTx( ctx sdk.Context, - coinbase common.Address, signer ethtypes.Signer, txIndex uint64, - params types.Params, ethCfg *ethparams.ChainConfig, tx *ethtypes.Transaction, traceConfig *types.TraceConfig, @@ -522,12 +487,10 @@ func (k *Keeper) traceTx( tracer = types.NewTracer(types.TracerStruct, msg, ethCfg, ctx.BlockHeight(), true) } - evm := k.NewEVM(msg, ethCfg, params, coinbase, tracer) - k.SetTxHashTransient(tx.Hash()) k.SetTxIndexTransient(txIndex) - res, err := k.ApplyMessage(evm, msg, ethCfg, true) + res, err := k.ApplyMessage(msg, tracer, true) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } diff --git a/x/evm/keeper/grpc_query_test.go b/x/evm/keeper/grpc_query_test.go index 0e54e25ec5..1d16044bc0 100644 --- a/x/evm/keeper/grpc_query_test.go +++ b/x/evm/keeper/grpc_query_test.go @@ -629,9 +629,9 @@ func (suite *KeeperTestSuite) TestTraceTx() { predecessors = append(predecessors, firstTx) }, expPass: true, - traceResponse: []byte{0x7b, 0x22, 0x67, 0x61, 0x73, 0x22, 0x3a, 0x32, 0x35, 0x32, 0x32, 0x38, 0x2c, 0x22, 0x66, 0x61, 0x69, 0x6c, 0x65, 0x64, 0x22, 0x3a, 0x66, 0x61, 0x6c, 0x73, 0x65, 0x2c, 0x22, 0x72, 0x65, 0x74, 0x75, 0x72, 0x6e, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x22, 0x22, 0x2c, 0x22, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x4c, 0x6f, 0x67, 0x73, 0x22, 0x3a, 0x5b, 0x5d, 0x7d}, - }, - } + traceResponse: []byte{0x7b, 0x22, 0x67, 0x61, 0x73, 0x22, 0x3a, 0x33, 0x30, 0x38, 0x32, 0x38, 0x2c, 0x22, 0x66, 0x61, 0x69, 0x6c, 0x65, 0x64, 0x22, 0x3a, 0x66, 0x61, 0x6c, 0x73, 0x65, 0x2c, 0x22, 0x72, 0x65, 0x74, 0x75, 0x72, 0x6e, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x22, 0x22, 0x2c, 0x22, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x4c, 0x6f, 0x67, 0x73, 0x22, 0x3a, 0x5b, 0x5d, 0x7d}, + }, + } for _, tc := range testCases { suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { @@ -732,14 +732,15 @@ func (suite *KeeperTestSuite) TestTraceBlock() { // create multiple transactions in the same block firstTx := suite.TransferERC20Token(suite.T(), contractAddr, suite.address, common.HexToAddress("0x378c50D9264C63F3F92B806d4ee56E9D86FfB3Ec"), sdk.NewIntWithDecimal(1, 18).BigInt()) secondTx := suite.TransferERC20Token(suite.T(), contractAddr, suite.address, common.HexToAddress("0x378c50D9264C63F3F92B806d4ee56E9D86FfB3Ec"), sdk.NewIntWithDecimal(1, 18).BigInt()) + ctx = sdk.WrapSDKContext(suite.ctx) suite.Commit() // overwrite txs to include only the ones on new block txs = append([]*types.MsgEthereumTx{}, firstTx, secondTx) }, expPass: true, - traceResponse: []byte{0x5b, 0x7b, 0x22, 0x72, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x22, 0x3a, 0x7b, 0x22, 0x67, 0x61, 0x73, 0x22, 0x3a, 0x33, 0x34, 0x38, 0x32, 0x38, 0x2c, 0x22, 0x66, 0x61, 0x69, 0x6c, 0x65, 0x64, 0x22, 0x3a, 0x66, 0x61, 0x6c, 0x73, 0x65, 0x2c, 0x22, 0x72, 0x65, 0x74, 0x75, 0x72, 0x6e, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x22, 0x22, 0x2c, 0x22, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x4c, 0x6f, 0x67, 0x73, 0x22, 0x3a, 0x5b, 0x5d, 0x7d, 0x7d, 0x2c, 0x7b, 0x22, 0x72, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x22, 0x3a, 0x7b, 0x22, 0x67, 0x61, 0x73, 0x22, 0x3a, 0x32, 0x35, 0x32, 0x32, 0x38, 0x2c, 0x22, 0x66, 0x61, 0x69, 0x6c, 0x65, 0x64, 0x22, 0x3a, 0x66, 0x61, 0x6c, 0x73, 0x65, 0x2c, 0x22, 0x72, 0x65, 0x74, 0x75, 0x72, 0x6e, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x22, 0x22, 0x2c, 0x22, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x4c, 0x6f, 0x67, 0x73, 0x22, 0x3a, 0x5b, 0x5d, 0x7d, 0x7d, 0x5d}, - }, - } + traceResponse: []byte{0x5b, 0x7b, 0x22, 0x72, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x22, 0x3a, 0x7b, 0x22, 0x67, 0x61, 0x73, 0x22, 0x3a, 0x33, 0x34, 0x38, 0x32, 0x38, 0x2c, 0x22, 0x66, 0x61, 0x69, 0x6c, 0x65, 0x64, 0x22, 0x3a, 0x66, 0x61, 0x6c, 0x73, 0x65, 0x2c, 0x22, 0x72, 0x65, 0x74, 0x75, 0x72, 0x6e, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x22, 0x22, 0x2c, 0x22, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x4c, 0x6f, 0x67, 0x73, 0x22, 0x3a, 0x5b, 0x5d, 0x7d, 0x7d, 0x2c, 0x7b, 0x22, 0x72, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x22, 0x3a, 0x7b, 0x22, 0x67, 0x61, 0x73, 0x22, 0x3a, 0x33, 0x30, 0x38, 0x32, 0x38, 0x2c, 0x22, 0x66, 0x61, 0x69, 0x6c, 0x65, 0x64, 0x22, 0x3a, 0x66, 0x61, 0x6c, 0x73, 0x65, 0x2c, 0x22, 0x72, 0x65, 0x74, 0x75, 0x72, 0x6e, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x22, 0x22, 0x2c, 0x22, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x4c, 0x6f, 0x67, 0x73, 0x22, 0x3a, 0x5b, 0x5d, 0x7d, 0x7d, 0x5d}, + }, + } for _, tc := range testCases { suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { @@ -750,6 +751,7 @@ func (suite *KeeperTestSuite) TestTraceBlock() { suite.Commit() // Generate token transfer transaction txMsg := suite.TransferERC20Token(suite.T(), contractAddr, suite.address, common.HexToAddress("0x378c50D9264C63F3F92B806d4ee56E9D86FfB3Ec"), sdk.NewIntWithDecimal(1, 18).BigInt()) + ctx = sdk.WrapSDKContext(suite.ctx) suite.Commit() txs = append(txs, txMsg) diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 9b74753beb..04b16f4b0c 100644 --- a/x/evm/keeper/keeper.go +++ b/x/evm/keeper/keeper.go @@ -9,10 +9,13 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core" ethtypes "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/vm" "github.com/palantir/stacktrace" "github.com/tendermint/tendermint/libs/log" + "github.com/ethereum/go-ethereum/params" ethermint "github.com/tharsis/ethermint/types" "github.com/tharsis/ethermint/x/evm/types" ) @@ -373,3 +376,8 @@ func (k *Keeper) PostTxProcessing(txHash common.Hash, logs []*ethtypes.Log) erro } return k.hooks.PostTxProcessing(k.Ctx(), txHash, logs) } + +// Tracer return a default vm.Tracer based on current keeper state +func (k Keeper) Tracer(msg core.Message, ethCfg *params.ChainConfig) vm.Tracer { + return types.NewTracer(k.tracer, msg, ethCfg, k.Ctx().BlockHeight(), k.debug) +} diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index 522f68f0aa..e7aa7ab6cd 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -21,22 +21,38 @@ import ( "github.com/ethereum/go-ethereum/params" ) +// EVMConfig creates the EVMConfig based on current state +func (k *Keeper) EVMConfig(ctx sdk.Context) (*types.EVMConfig, error) { + params := k.GetParams(ctx) + ethCfg := params.ChainConfig.EthereumConfig(k.eip155ChainID) + + // get the coinbase address from the block proposer + coinbase, err := k.GetCoinbaseAddress(ctx) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to obtain coinbase address") + } + + return &types.EVMConfig{ + Params: params, + ChainConfig: ethCfg, + CoinBase: coinbase, + }, nil +} + // NewEVM generates a go-ethereum VM from the provided Message fields and the chain parameters // (ChainConfig and module Params). It additionally sets the validator operator address as the // coinbase address to make it available for the COINBASE opcode, even though there is no // beneficiary of the coinbase transaction (since we're not mining). func (k *Keeper) NewEVM( msg core.Message, - config *params.ChainConfig, - params types.Params, - coinbase common.Address, + cfg *types.EVMConfig, tracer vm.Tracer, ) *vm.EVM { blockCtx := vm.BlockContext{ CanTransfer: core.CanTransfer, Transfer: core.Transfer, GetHash: k.GetHashFn(), - Coinbase: coinbase, + Coinbase: cfg.CoinBase, GasLimit: ethermint.BlockGasLimit(k.Ctx()), BlockNumber: big.NewInt(k.Ctx().BlockHeight()), Time: big.NewInt(k.Ctx().BlockHeader().Time.Unix()), @@ -44,9 +60,11 @@ func (k *Keeper) NewEVM( } txCtx := core.NewEVMTxContext(msg) - vmConfig := k.VMConfig(msg, params, tracer) - - return vm.NewEVM(blockCtx, txCtx, k, config, vmConfig) + if tracer == nil { + tracer = k.Tracer(msg, cfg.ChainConfig) + } + vmConfig := k.VMConfig(msg, cfg.Params, tracer) + return vm.NewEVM(blockCtx, txCtx, k, cfg.ChainConfig, vmConfig) } // VMConfig creates an EVM configuration from the debug setting and the extra EIPs enabled on the @@ -137,89 +155,69 @@ func (k Keeper) GetHashFn() vm.GetHashFunc { // For relevant discussion see: https://github.com/cosmos/cosmos-sdk/discussions/9072 func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumTxResponse, error) { ctx := k.Ctx() - params := k.GetParams(ctx) // ensure keeper state error is cleared defer k.ClearStateError() - // return error if contract creation or call are disabled through governance - if !params.EnableCreate && tx.To() == nil { - return nil, stacktrace.Propagate(types.ErrCreateDisabled, "failed to create new contract") - } else if !params.EnableCall && tx.To() != nil { - return nil, stacktrace.Propagate(types.ErrCallDisabled, "failed to call contract") + cfg, err := k.EVMConfig(ctx) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to load evm config") } - ethCfg := params.ChainConfig.EthereumConfig(k.eip155ChainID) - // get the latest signer according to the chain rules from the config - signer := ethtypes.MakeSigner(ethCfg, big.NewInt(ctx.BlockHeight())) + signer := ethtypes.MakeSigner(cfg.ChainConfig, big.NewInt(ctx.BlockHeight())) msg, err := tx.AsMessage(signer) if err != nil { return nil, stacktrace.Propagate(err, "failed to return ethereum transaction as core message") } - // get the coinbase address from the block proposer - coinbase, err := k.GetCoinbaseAddress(ctx) - if err != nil { - return nil, stacktrace.Propagate(err, "failed to obtain coinbase address") - } - - // create an ethereum EVM instance and run the message - tracer := types.NewTracer(k.tracer, msg, ethCfg, ctx.BlockHeight(), k.debug) - evm := k.NewEVM(msg, ethCfg, params, coinbase, tracer) - txHash := tx.Hash() // set the transaction hash and index to the impermanent (transient) block state so that it's also // available on the StateDB functions (eg: AddLog) k.SetTxHashTransient(txHash) - if !k.ctxStack.IsEmpty() { - panic("context stack shouldn't be dirty before apply message") - } - - // revision is -1 for empty stack - revision := -1 + // snapshot to contain the tx processing and post processing in same scope + var commit func() if k.hooks != nil { - // snapshot to contain the tx processing and post processing in same scope - revision = k.Snapshot() + // Create a cache context to revert state when tx hooks fails, + // the cache context is only committed when both tx and hooks executed successfully. + // Didn't use `Snapshot` because the context stack has exponential complexity on certain operations, + // thus restricted to be used only inside `ApplyMessage`. + var cacheCtx sdk.Context + cacheCtx, commit = ctx.CacheContext() + k.WithContext(cacheCtx) + defer (func() { + k.WithContext(ctx) + })() } - // pass false to execute in real mode, which do actual gas refunding - res, err := k.ApplyMessage(evm, msg, ethCfg, false) + res, err := k.ApplyMessageWithConfig(msg, nil, true, cfg) if err != nil { return nil, stacktrace.Propagate(err, "failed to apply ethereum core message") } - res.Hash = txHash.Hex() - // The state is reverted (i.e `RevertToSnapshot`) for the VM error cases, so it's safe to call the commit here. - // NOTE: revision is >= 0 only when the EVM hooks are not empty - if revision >= 0 { - // Flatten the cache contexts to improve the efficiency of following DB operations. - // Only commit the cache layers created by the EVM contract execution - // FIXME: some operations under deep context stack are extremely slow, - // see `benchmark_test.go:BenchmarkDeepContextStack13`. - if err = k.ctxStack.CommitToRevision(revision); err != nil { - return nil, stacktrace.Propagate(err, "failed to commit ethereum core message") - } - } else { - // All cache layers are created by the EVM contract execution. So it is safe to commit them all - k.CommitCachedContexts() + // refund gas prior to handling the vm error in order to match the Ethereum gas consumption instead of the default SDK one. + err = k.RefundGas(msg, msg.Gas()-res.GasUsed, cfg.Params.EvmDenom) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From()) } + res.Hash = txHash.Hex() + logs := k.GetTxLogsTransient(txHash) if !res.Failed() { // Only call hooks if tx executed successfully. if err = k.PostTxProcessing(txHash, logs); err != nil { // If hooks return error, revert the whole tx. - k.RevertToSnapshot(revision) res.VmError = types.ErrPostTxProcessing.Error() - k.Logger(ctx).Error("tx post processing failed", "error", err) - } else { - // PostTxProcessing is successful, commit the leftover contexts - k.CommitCachedContexts() + k.Logger(k.Ctx()).Error("tx post processing failed", "error", err) + } else if commit != nil { + // PostTxProcessing is successful, commit the cache context + commit() + ctx.EventManager().EmitEvents(k.Ctx().EventManager().Events()) } } @@ -238,14 +236,21 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT return res, nil } -// ApplyMessage computes the new state by applying the given message against the existing state. +// ApplyMessageWithConfig computes the new state by applying the given message against the existing state. // If the message fails, the VM execution error with the reason will be returned to the client // and the transaction won't be committed to the store. // // Reverted state // -// The transaction is never "reverted" since there is no snapshot + rollback performed on the StateDB. -// Only successful transactions are written to the store during DeliverTx mode. +// The snapshot and rollback are supported by the `ContextStack`, which should be only used inside `ApplyMessage`, +// because some operations has exponential computational complexity with deep stack. +// +// Different Callers +// +// It's called in three scenarios: +// 1. `ApplyTransaction`, in the transaction processing flow. +// 2. `EthCall/EthEstimateGas` grpc query handler. +// 3. Called by other native modules directly. // // Prechecks and Preprocessing // @@ -263,23 +268,39 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT // // 1. set up the initial access list (iff fork > Berlin) // -// Query mode +// Tracer parameter +// +// It should be a `vm.Tracer` object or nil, if pass `nil`, it'll create a default one based on keeper options. // -// The gRPC query endpoint from 'eth_call' calls this method in query mode, and since the query handler don't call AnteHandler, -// so we don't do real gas refund in that case. -func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainConfig, query bool) (*types.MsgEthereumTxResponse, error) { +// Commit parameter +// +// If commit is true, the cache context stack will be committed, otherwise discarded. +func (k *Keeper) ApplyMessageWithConfig(msg core.Message, tracer vm.Tracer, commit bool, cfg *types.EVMConfig) (*types.MsgEthereumTxResponse, error) { var ( ret []byte // return bytes from evm execution vmErr error // vm errors do not effect consensus and are therefore not assigned to err ) + if !k.ctxStack.IsEmpty() { + panic("context stack shouldn't be dirty before apply message") + } + + evm := k.NewEVM(msg, cfg, tracer) + // ensure keeper state error is cleared defer k.ClearStateError() + // return error if contract creation or call are disabled through governance + if !cfg.Params.EnableCreate && msg.To() == nil { + return nil, stacktrace.Propagate(types.ErrCreateDisabled, "failed to create new contract") + } else if !cfg.Params.EnableCall && msg.To() != nil { + return nil, stacktrace.Propagate(types.ErrCallDisabled, "failed to call contract") + } + sender := vm.AccountRef(msg.From()) contractCreation := msg.To() == nil - intrinsicGas, err := k.GetEthIntrinsicGas(msg, cfg, contractCreation) + intrinsicGas, err := k.GetEthIntrinsicGas(msg, cfg.ChainConfig, contractCreation) if err != nil { // should have already been checked on Ante Handler return nil, stacktrace.Propagate(err, "intrinsic gas failed") @@ -293,7 +314,7 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo // access list preparaion is moved from ante handler to here, because it's needed when `ApplyMessage` is called // under contexts where ante handlers are not run, for example `eth_call` and `eth_estimateGas`. - if rules := cfg.Rules(big.NewInt(k.Ctx().BlockHeight())); rules.IsBerlin { + if rules := cfg.ChainConfig.Rules(big.NewInt(k.Ctx().BlockHeight())); rules.IsBerlin { k.PrepareAccessList(msg.From(), msg.To(), vm.ActivePrecompiles(rules), msg.AccessList()) } @@ -305,19 +326,16 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo refundQuotient := uint64(2) - if query { - // gRPC query handlers don't go through the AnteHandler to deduct the gas fee from the sender or have access historical state. - // We don't refund gas to the sender. - // For more info, see: https://github.com/tharsis/ethermint/issues/229 and https://github.com/cosmos/cosmos-sdk/issues/9636 - gasConsumed := msg.Gas() - leftoverGas - leftoverGas += k.GasToRefund(gasConsumed, refundQuotient) - } else { - // refund gas prior to handling the vm error in order to match the Ethereum gas consumption instead of the default SDK one. - leftoverGas, err = k.RefundGas(msg, leftoverGas, refundQuotient) - if err != nil { - return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From()) - } + // calculate gas refund + if msg.Gas() < leftoverGas { + return nil, stacktrace.Propagate(types.ErrGasOverflow, "apply message") + } + gasUsed := msg.Gas() - leftoverGas + refund := k.GasToRefund(gasUsed, refundQuotient) + if refund > gasUsed { + return nil, stacktrace.Propagate(types.ErrGasOverflow, "apply message") } + gasUsed -= refund // EVM execution error needs to be available for the JSON-RPC client var vmError string @@ -325,7 +343,14 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo vmError = vmErr.Error() } - gasUsed := msg.Gas() - leftoverGas + // The context stack is designed specifically for `StateDB` interface, it should only be used in `ApplyMessage`, + // after return, the stack should be clean, the cached states are either committed or discarded. + if commit { + k.CommitCachedContexts() + } else { + k.ctxStack.RevertAll() + } + return &types.MsgEthereumTxResponse{ GasUsed: gasUsed, VmError: vmError, @@ -333,36 +358,13 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo }, nil } -// ApplyNativeMessage executes an ethereum message on the EVM. It is meant to be called from an internal -// native Cosmos SDK module. -func (k *Keeper) ApplyNativeMessage(msg core.Message) (*types.MsgEthereumTxResponse, error) { - // TODO: clean up and remove duplicate code. - - ctx := k.Ctx() - params := k.GetParams(ctx) - // return error if contract creation or call are disabled through governance - if !params.EnableCreate && msg.To() == nil { - return nil, stacktrace.Propagate(types.ErrCreateDisabled, "failed to create new contract") - } else if !params.EnableCall && msg.To() != nil { - return nil, stacktrace.Propagate(types.ErrCallDisabled, "failed to call contract") - } - - ethCfg := params.ChainConfig.EthereumConfig(k.eip155ChainID) - - // get the coinbase address from the block proposer - coinbase, err := k.GetCoinbaseAddress(ctx) - if err != nil { - return nil, stacktrace.Propagate(err, "failed to obtain coinbase address") - } - - evm := k.NewEVM(msg, ethCfg, params, coinbase, nil) - - ret, err := k.ApplyMessage(evm, msg, ethCfg, true) +// ApplyMessage calls ApplyMessageWithConfig with default EVMConfig +func (k *Keeper) ApplyMessage(msg core.Message, tracer vm.Tracer, commit bool) (*types.MsgEthereumTxResponse, error) { + cfg, err := k.EVMConfig(k.Ctx()) if err != nil { - return nil, err + return nil, stacktrace.Propagate(err, "failed to load evm config") } - k.CommitCachedContexts() - return ret, nil + return k.ApplyMessageWithConfig(msg, tracer, commit, cfg) } // GetEthIntrinsicGas returns the intrinsic gas cost for the transaction @@ -390,53 +392,30 @@ func (k *Keeper) GasToRefund(gasConsumed, refundQuotient uint64) uint64 { // consumed in the transaction. Additionally, the function sets the total gas consumed to the value // returned by the EVM execution, thus ignoring the previous intrinsic gas consumed during in the // AnteHandler. -func (k *Keeper) RefundGas(msg core.Message, leftoverGas, refundQuotient uint64) (uint64, error) { - // safety check: leftover gas after execution should never exceed the gas limit defined on the message - if leftoverGas > msg.Gas() { - return leftoverGas, stacktrace.Propagate( - sdkerrors.Wrapf(types.ErrInconsistentGas, "leftover gas cannot be greater than gas limit (%d > %d)", leftoverGas, msg.Gas()), - "failed to update gas consumed after refund of leftover gas", - ) - } - - gasConsumed := msg.Gas() - leftoverGas - - // calculate available gas to refund and add it to the leftover gas amount - refund := k.GasToRefund(gasConsumed, refundQuotient) - leftoverGas += refund - - // safety check: leftover gas after refund should never exceed the gas limit defined on the message - if leftoverGas > msg.Gas() { - return leftoverGas, stacktrace.Propagate( - sdkerrors.Wrapf(types.ErrInconsistentGas, "leftover gas cannot be greater than gas limit (%d > %d)", leftoverGas, msg.Gas()), - "failed to update gas consumed after refund of %d gas", refund, - ) - } - +func (k *Keeper) RefundGas(msg core.Message, leftoverGas uint64, denom string) error { // Return EVM tokens for remaining gas, exchanged at the original rate. remaining := new(big.Int).Mul(new(big.Int).SetUint64(leftoverGas), msg.GasPrice()) switch remaining.Sign() { case -1: // negative refund errors - return leftoverGas, sdkerrors.Wrapf(types.ErrInvalidRefund, "refunded amount value cannot be negative %d", remaining.Int64()) + return sdkerrors.Wrapf(types.ErrInvalidRefund, "refunded amount value cannot be negative %d", remaining.Int64()) case 1: // positive amount refund - params := k.GetParams(k.Ctx()) - refundedCoins := sdk.Coins{sdk.NewCoin(params.EvmDenom, sdk.NewIntFromBigInt(remaining))} + refundedCoins := sdk.Coins{sdk.NewCoin(denom, sdk.NewIntFromBigInt(remaining))} // refund to sender from the fee collector module account, which is the escrow account in charge of collecting tx fees err := k.bankKeeper.SendCoinsFromModuleToAccount(k.Ctx(), authtypes.FeeCollectorName, msg.From().Bytes(), refundedCoins) if err != nil { err = sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "fee collector account failed to refund fees: %s", err.Error()) - return leftoverGas, stacktrace.Propagate(err, "failed to refund %d leftover gas (%s)", leftoverGas, refundedCoins.String()) + return stacktrace.Propagate(err, "failed to refund %d leftover gas (%s)", leftoverGas, refundedCoins.String()) } default: // no refund, consume gas and update the tx gas meter } - return leftoverGas, nil + return nil } // resetGasMeterAndConsumeGas reset first the gas meter consumed value to zero and set it back to the new value diff --git a/x/evm/keeper/state_transition_benchmark_test.go b/x/evm/keeper/state_transition_benchmark_test.go index 54ac09c89d..e16865d6f2 100644 --- a/x/evm/keeper/state_transition_benchmark_test.go +++ b/x/evm/keeper/state_transition_benchmark_test.go @@ -153,7 +153,7 @@ func BenchmarkApplyTransactionWithLegacyTx(b *testing.B) { } } -func BenchmarkApplyNativeMessage(b *testing.B) { +func BenchmarkApplyMessage(b *testing.B) { suite := KeeperTestSuite{} suite.DoSetupTest(b) @@ -178,7 +178,7 @@ func BenchmarkApplyNativeMessage(b *testing.B) { require.NoError(b, err) b.StartTimer() - resp, err := suite.app.EvmKeeper.ApplyNativeMessage(m) + resp, err := suite.app.EvmKeeper.ApplyMessage(m, nil, true) b.StopTimer() require.NoError(b, err) @@ -186,7 +186,7 @@ func BenchmarkApplyNativeMessage(b *testing.B) { } } -func BenchmarkApplyNativeMessageWithLegacyTx(b *testing.B) { +func BenchmarkApplyMessageWithLegacyTx(b *testing.B) { suite := KeeperTestSuite{} suite.DoSetupTest(b) @@ -211,7 +211,7 @@ func BenchmarkApplyNativeMessageWithLegacyTx(b *testing.B) { require.NoError(b, err) b.StartTimer() - resp, err := suite.app.EvmKeeper.ApplyNativeMessage(m) + resp, err := suite.app.EvmKeeper.ApplyMessage(m, nil, true) b.StopTimer() require.NoError(b, err) diff --git a/x/evm/types/config.go b/x/evm/types/config.go new file mode 100644 index 0000000000..e7c41b00d2 --- /dev/null +++ b/x/evm/types/config.go @@ -0,0 +1,14 @@ +package types + +import ( + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/params" +) + +// EVMConfig encapulates common parameters needed to create an EVM to execute a message +// It's mainly to reduce the number of method parameters +type EVMConfig struct { + Params Params + ChainConfig *params.ChainConfig + CoinBase common.Address +} diff --git a/x/evm/types/errors.go b/x/evm/types/errors.go index 495216a8c9..a8cdad5f89 100644 --- a/x/evm/types/errors.go +++ b/x/evm/types/errors.go @@ -30,6 +30,7 @@ const ( codeErrInconsistentGas codeErrInvalidGasCap codeErrInvalidBaseFee + codeErrGasOverflow ) var ErrPostTxProcessing = errors.New("failed to execute post processing") @@ -85,6 +86,9 @@ var ( // ErrInvalidBaseFee returns an error if a the base fee cap value is invalid ErrInvalidBaseFee = sdkerrors.Register(ModuleName, codeErrInvalidBaseFee, "invalid base fee") + + // ErrGasOverflow returns an error if gas computation overlow/underflow + ErrGasOverflow = sdkerrors.Register(ModuleName, codeErrGasOverflow, "gas computation overflow/underflow") ) // NewExecErrorWithReason unpacks the revert return bytes and returns a wrapped error