evm: refactor dup state transition code (evmos#674)

* Problem: state transition code is duplicated

Closes: evmos#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

@@ -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

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

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

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)

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