Fixed PR_REVIEW notes, added helpers for tracing transfer

app/app.go

@@ -631,12 +631,7 @@ func New(
  }
 
  if app.evmRPCConfig.LiveEVMTracer != "" {
- // PR_REVIEW_NOTE: So I moved this code from `ProcessBlock` and there, I had access to `ctx` so the code was actually looking
- // like `evmtypes.DefaultChainConfig().EthereumConfig(app.EvmKeeper.ChainID(ctx))`. But here, I don't have access to `ctx`
- // Is there another mean to get the EVM chainID from here? I need it to call `OnSeiBlockchainInit` on the logger,
- // so another solution would be to call this one later when EVM chainID is known. Last resort, we have a sync.Once
- // that we can use to call it only once.
- chainConfig := evmtypes.DefaultChainConfig().EthereumConfig(big.NewInt(int64(app.evmRPCConfig.LiveEVMTracerChainID)))
+ chainConfig := evmtypes.DefaultChainConfig().EthereumConfig(app.EvmKeeper.ChainID())
  evmTracer, err := evmtracers.NewBlockchainTracer(evmtracers.GlobalLiveTracerRegistry, app.evmRPCConfig.LiveEVMTracer, chainConfig)
  if err != nil {
  panic(fmt.Sprintf("error creating EVM tracer due to %s", err))

evmrpc/config.go

@@ -56,19 +56,9 @@ type Config struct {
  // The EVM tracer to use when doing node synchronization, applies to
  // all block produced but traces only EVM transactions.
  //
- // Refer to <TBC> for registered tracers
- //
- // PR_REVIEW_NOTE: It his an acceptable way of documenting the available tracers?
- // PR_REVIEW_NOTE: This section renders as `[evm]` in config but is named EVMRPC on top,
- // is live tracing of block synchronization should be here? Maybe "higher"
- // in the config hierarchy? We might think also about a way that later, we could
- // different trace active for Cosmos related function and EVM related function.
+ // Refer to x/evm/tracers/registry.go#GlobalLiveTracerRegistry for registered tracers.
  LiveEVMTracer string `mapstructure:"live_evm_tracer"`
 
- // PR_REVIEW_NOTE: This is a hackish workaround because I didn't how to get it in `app/app.go#New`,
- // this will not be part of the final PR.
- LiveEVMTracerChainID int `mapstructure:"live_evm_tracer_chain_id"`
-
  // list of CORS allowed origins, separated by comma
  CORSOrigins string `mapstructure:"cors_origins"`
 
@@ -108,6 +98,7 @@ var DefaultConfig = Config{
  IdleTimeout: rpc.DefaultHTTPTimeouts.IdleTimeout,
  SimulationGasLimit: 10_000_000, // 10M
  SimulationEVMTimeout: 60 * time.Second,
+ LiveEVMTracer: "",
  CORSOrigins: "*",
  WSOrigins: "*",
  FilterTimeout: 120 * time.Second,
@@ -130,6 +121,7 @@ const (
  flagIdleTimeout = "evm.idle_timeout"
  flagSimulationGasLimit = "evm.simulation_gas_limit"
  flagSimulationEVMTimeout = "evm.simulation_evm_timeout"
+ flagLiveEVMTracer = "evm.live_evm_tracer"
  flagCORSOrigins = "evm.cors_origins"
  flagWSOrigins = "evm.ws_origins"
  flagFilterTimeout = "evm.filter_timeout"
@@ -139,9 +131,6 @@ const (
  flagDenyList = "evm.deny_list"
  flagMaxLogNoBlock = "evm.max_log_no_block"
  flagMaxBlocksForLog = "evm.max_blocks_for_log"
- flagLiveEVMTracer = "evm.live_evm_tracer"
- // PR_REVIEW_NOTE: This is going to go away, temporary hack
- flagLiveEVMTracerChainID = "evm.live_evm_tracer_chain_id"
 )
 
 func ReadConfig(opts servertypes.AppOptions) (Config, error) {
@@ -197,6 +186,11 @@ func ReadConfig(opts servertypes.AppOptions) (Config, error) {
  return cfg, err
  }
  }
+ if v := opts.Get(flagLiveEVMTracer); v != nil {
+ if cfg.LiveEVMTracer, err = cast.ToStringE(v); err != nil {
+ return cfg, err
+ }
+ }
  if v := opts.Get(flagCORSOrigins); v != nil {
  if cfg.CORSOrigins, err = cast.ToStringE(v); err != nil {
  return cfg, err
@@ -242,16 +236,6 @@ func ReadConfig(opts servertypes.AppOptions) (Config, error) {
  return cfg, err
  }
  }
- if v := opts.Get(flagLiveEVMTracer); v != nil {
- if cfg.LiveEVMTracer, err = cast.ToStringE(v); err != nil {
- return cfg, err
- }
- }
- if v := opts.Get(flagLiveEVMTracerChainID); v != nil {
- if cfg.LiveEVMTracerChainID, err = cast.ToIntE(v); err != nil {
- return cfg, err
- }
- }
 
  return cfg, nil
 }

evmrpc/config_test.go

@@ -20,6 +20,7 @@ type opts struct {
  idleTimeout interface{}
  simulationGasLimit interface{}
  simulationEVMTimeout interface{}
+ liveEVMTracer interface{}
  corsOrigins interface{}
  wsOrigins interface{}
  filterTimeout interface{}
@@ -29,8 +30,6 @@ type opts struct {
  denyList interface{}
  maxLogNoBlock interface{}
  maxBlocksForLog interface{}
- liveEVMTracer interface{}
- liveEVMTracerChainID interface{}
 }
 
 func (o *opts) Get(k string) interface{} {
@@ -64,6 +63,9 @@ func (o *opts) Get(k string) interface{} {
  if k == "evm.simulation_evm_timeout" {
  return o.simulationEVMTimeout
  }
+ if k == "evm.live_evm_tracer" {
+ return o.liveEVMTracer
+ }
  if k == "evm.cors_origins" {
  return o.corsOrigins
  }
@@ -91,12 +93,6 @@ func (o *opts) Get(k string) interface{} {
  if k == "evm.max_blocks_for_log" {
  return o.maxBlocksForLog
  }
- if k == "evm.live_evm_tracer" {
- return o.liveEVMTracer
- }
- if k == "evm.live_evm_tracer_chain_id" {
- return o.liveEVMTracerChainID
- }
  panic(fmt.Errorf("unknown key: %s", k))
 }
 
@@ -114,15 +110,14 @@ func TestReadConfig(t *testing.T) {
  time.Duration(60),
  "",
  "",
+ "",
  time.Duration(5),
  time.Duration(5),
  1000,
  false,
  make([]string, 0),
  20000,
  1000,
- "",
- 0,
  }
  _, err := evmrpc.ReadConfig(&goodOpts)
  require.Nil(t, err)

precompiles/bank/bank.go

@@ -10,7 +10,6 @@ import (
  sdk "github.com/cosmos/cosmos-sdk/types"
  "github.com/ethereum/go-ethereum/accounts/abi"
  "github.com/ethereum/go-ethereum/common"
- "github.com/ethereum/go-ethereum/core/tracing"
  "github.com/ethereum/go-ethereum/core/vm"
  pcommon "github.com/sei-protocol/sei-chain/precompiles/common"
  "github.com/sei-protocol/sei-chain/utils"
@@ -225,46 +224,28 @@ func (p Precompile) sendNative(ctx sdk.Context, method *abi.Method, args []inter
  return nil, err
  }
 
- usei, wei, err := pcommon.HandlePaymentUseiWei(ctx, p.evmKeeper.GetSeiAddressOrDefault(ctx, p.address), senderSeiAddr, value, p.bankKeeper)
+ precompiledSeiAddr := p.evmKeeper.GetSeiAddressOrDefault(ctx, p.address)
+
+ usei, wei, err := pcommon.HandlePaymentUseiWei(ctx, precompiledSeiAddr, senderSeiAddr, value, p.bankKeeper)
  if err != nil {
  return nil, err
  }
 
- if hooks := tracers.GetCtxEthTracingHooks(ctx); hooks != nil && hooks.OnBalanceChange != nil && !wei.IsZero() {
- // Precompile address got wei removed from it
- newBalance := p.bankKeeper.GetWeiBalance(ctx, p.evmKeeper.GetSeiAddressOrDefault(ctx, p.address)).BigInt()
- oldBalance := new(big.Int).Add(newBalance, wei.BigInt())
-
- hooks.OnBalanceChange(p.address, oldBalance, newBalance, tracing.BalanceChangeTransfer)
-
- // Sender received wei from the precompile address
- newBalance = p.bankKeeper.GetWeiBalance(ctx, senderSeiAddr).BigInt()
- oldBalance = new(big.Int).Sub(newBalance, wei.BigInt())
-
- hooks.OnBalanceChange(caller, oldBalance, newBalance, tracing.BalanceChangeTransfer)
+ if hooks := tracers.GetCtxEthTracingHooks(ctx); hooks != nil && hooks.OnBalanceChange != nil && (value.Sign() != 0) {
+ tracers.TraceTransferEVMValue(ctx, hooks, p.bankKeeper, precompiledSeiAddr, p.address, senderSeiAddr, caller, value)
  }
 
  if err := p.bankKeeper.SendCoinsAndWei(ctx, senderSeiAddr, receiverSeiAddr, usei, wei); err != nil {
  return nil, err
  }
 
- if hooks := tracers.GetCtxEthTracingHooks(ctx); hooks != nil && hooks.OnBalanceChange != nil && !wei.IsZero() {
- // Sender address got wei removed from it
- newBalance := p.bankKeeper.GetWeiBalance(ctx, senderSeiAddr).BigInt()
- oldBalance := new(big.Int).Add(newBalance, wei.BigInt())
-
- hooks.OnBalanceChange(caller, oldBalance, newBalance, tracing.BalanceChangeTransfer)
-
- // Receiver received wei from the sender address
- evmReceiverAddr, err := p.evmKeeper.GetEVMAddressFromBech32OrDefault(ctx, receiverAddr)
- if err != nil {
- panic(fmt.Errorf("failed to get EVM address from Sei bech32 address input %q, this shouldn't happen at this point since SendCoinsAndWei above worked: %w", receiverAddr, err))
+ if hooks := tracers.GetCtxEthTracingHooks(ctx); hooks != nil && hooks.OnBalanceChange != nil && (value.Sign() != 0) {
+ receveirEvmAddr, found := p.evmKeeper.GetEVMAddress(ctx, receiverSeiAddr)
+ if !found {
+ return nil, fmt.Errorf("sei address %s is not associated, this shouldn't happen at this point since SendCoinsAndWei above worked", receiverSeiAddr)
  }
 
- newBalance = p.bankKeeper.GetWeiBalance(ctx, receiverSeiAddr).BigInt()
- oldBalance = new(big.Int).Sub(newBalance, wei.BigInt())
-
- hooks.OnBalanceChange(evmReceiverAddr, oldBalance, newBalance, tracing.BalanceChangeTransfer)
+ tracers.TraceTransferEVMValue(ctx, hooks, p.bankKeeper, senderSeiAddr, caller, receiverSeiAddr, receveirEvmAddr, value)
  }
 
  return method.Outputs.Pack(true)

precompiles/bank/bank_test.go

@@ -12,6 +12,7 @@ import (
  "github.com/cosmos/cosmos-sdk/types/tx/signing"
  banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
  "github.com/ethereum/go-ethereum/common"
+ ethtracing "github.com/ethereum/go-ethereum/core/tracing"
  ethtypes "github.com/ethereum/go-ethereum/core/types"
  "github.com/ethereum/go-ethereum/core/vm"
  "github.com/ethereum/go-ethereum/crypto"
@@ -20,6 +21,8 @@ import (
  "github.com/sei-protocol/sei-chain/x/evm/ante"
  "github.com/sei-protocol/sei-chain/x/evm/keeper"
  "github.com/sei-protocol/sei-chain/x/evm/state"
+ "github.com/sei-protocol/sei-chain/x/evm/tracers"
+ "github.com/sei-protocol/sei-chain/x/evm/tracing"
  "github.com/sei-protocol/sei-chain/x/evm/types"
  "github.com/sei-protocol/sei-chain/x/evm/types/ethtx"
  "github.com/stretchr/testify/require"
@@ -39,7 +42,18 @@ func (tx mockTx) GetSignaturesV2() ([]signing.SignatureV2, error) { return nil,
 
 func TestRun(t *testing.T) {
  testApp := testkeeper.EVMTestApp
+
+ var balanceChanges []balanceChange
+
  ctx := testApp.NewContext(false, tmtypes.Header{}).WithBlockHeight(2)
+ ctx = tracers.SetCtxBlockchainTracer(ctx, &tracing.Hooks{
+ Hooks: &ethtracing.Hooks{
+ OnBalanceChange: func(addr common.Address, prev, new *big.Int, reason ethtracing.BalanceChangeReason) {
+ balanceChanges = append(balanceChanges, balanceChange{prev.String(), new.String()})
+ },
+ },
+ })
+
  k := &testApp.EvmKeeper
 
  // Setup sender addresses and environment
@@ -137,6 +151,15 @@ func TestRun(t *testing.T) {
  require.Nil(t, err)
  require.Empty(t, res.VmError)
 
+ // Test balance changes, there is 8 but we care about the first 4 here
+ require.Equal(t, 8, len(balanceChanges))
+ require.Equal(t, []balanceChange{
+ {"9800000000000000000", "9799989999999999900"},
+ {"0", "10000000000100"},
+ {"10000000000100", "0"},
+ {"9799989999999999900", "9800000000000000000"},
+ }, balanceChanges[0:4], "balance changes do not match, actual are:\n\n%s", balanceChangesValues(balanceChanges[0:4]))
+
  evts := ctx.EventManager().ABCIEvents()
 
  for _, evt := range evts {
@@ -289,3 +312,19 @@ func TestAddress(t *testing.T) {
  require.Nil(t, err)
  require.Equal(t, common.HexToAddress(bank.BankAddress), p.Address())
 }
+
+type balanceChange struct {
+ // We use string to avoid big.Int equality issues
+ old string
+ new string
+}
+
+func balanceChangesValues(changes []balanceChange) string {
+ out := make([]string, len(changes))
+ for i, change := range changes {
+ out[i] = fmt.Sprintf("{%q, %q}", change.old, change.new)
+ }
+
+ return strings.Join(out, "\n")
+
+}

x/evm/module.go

@@ -4,13 +4,11 @@ import (
  "encoding/json"
  "fmt"
  "math"
- "math/big"
 
  // this line is used by starport scaffolding # 1
 
  "github.com/ethereum/go-ethereum/common"
  "github.com/ethereum/go-ethereum/core"
- "github.com/ethereum/go-ethereum/core/tracing"
  ethtypes "github.com/ethereum/go-ethereum/core/types"
  "github.com/ethereum/go-ethereum/core/vm"
  "github.com/gorilla/mux"
@@ -237,13 +235,11 @@ func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.Val
  panic(err)
  }
 
- if evmHooks != nil && evmHooks.OnBalanceChange != nil && !weiBalance.IsZero() {
- // Only if the corresponding EVM address exists that we tracer the EVM balance change
- evmAddress := am.keeper.GetEVMAddressOrDefault(ctx, coinbaseAddress)
- newBalance := am.keeper.BankKeeper().GetWeiBalance(ctx, coinbaseAddress).BigInt()
- oldBalance := new(big.Int).Sub(newBalance, weiBalance.BigInt())
+ if evmHooks != nil && evmHooks.OnBalanceChange != nil {
+ fromEVMAddr := am.keeper.GetEVMAddressOrDefault(ctx, coinbaseAddress)
+ toEVMAddr := am.keeper.GetEVMAddressOrDefault(ctx, coinbase)
 
- evmHooks.OnBalanceChange(evmAddress, oldBalance, newBalance, tracing.BalanceIncreaseRewardTransactionFee)
+ tracers.TraceTransferUseiAndWei(ctx, evmHooks, am.keeper.BankKeeper(), coinbaseAddress, fromEVMAddr, coinbase, toEVMAddr, balance, weiBalance)
  }
  }
  surplus = surplus.Add(deferredInfo.Surplus)
@@ -259,17 +255,13 @@ func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.Val
  if surplusWei.GT(sdk.ZeroInt()) {
  if err := am.keeper.BankKeeper().AddWei(ctx, evmModuleAddress, surplusWei); err != nil {
  ctx.Logger().Error("failed to send wei surplus of %s to EVM module account", surplusWei)
- } else {
- if evmHooks != nil && evmHooks.OnBalanceChange != nil {
- // Only if the corresponding EVM address exists that we tracer the EVM balance change
- evmAddress := am.keeper.GetEVMAddressOrDefault(ctx, evmModuleAddress)
- newBalance := am.keeper.BankKeeper().GetWeiBalance(ctx, evmModuleAddress).BigInt()
- oldBalance := new(big.Int).Sub(newBalance, surplusWei.BigInt())
-
- evmHooks.OnBalanceChange(evmAddress, oldBalance, newBalance, tracing.BalanceIncreaseRewardMineBlock)
- }
  }
  }
+
+ if evmHooks != nil && evmHooks.OnBalanceChange != nil && (surplusUsei.GT(sdk.ZeroInt()) || surplusWei.GT(sdk.ZeroInt())) {
+ tracers.TraceBlockReward(ctx, evmHooks, am.keeper.BankKeeper(), evmModuleAddress, am.keeper.GetEVMAddressOrDefault(ctx, evmModuleAddress), sdk.NewCoin(am.keeper.GetBaseDenom(ctx), surplusUsei), surplusWei)
+ }
+
  am.keeper.SetTxHashesOnHeight(ctx, ctx.BlockHeight(), utils.Map(evmTxDeferredInfoList, func(i keeper.EvmTxDeferredInfo) common.Hash { return i.TxHash }))
  am.keeper.SetBlockBloom(ctx, ctx.BlockHeight(), utils.Map(evmTxDeferredInfoList, func(i keeper.EvmTxDeferredInfo) ethtypes.Bloom { return i.TxBloom }))
  return []abci.ValidatorUpdate{}

x/evm/state/balance.go

@@ -76,7 +76,6 @@ func (s *DBImpl) AddBalance(evmAddr common.Address, amt *big.Int, reason tracing
  }
 
  if s.logger != nil && s.logger.OnBalanceChange != nil {
- // We could modify AddWei instead so it returns us the old/new balance directly.
  newBalance := s.GetBalance(evmAddr)
  oldBalance := new(big.Int).Sub(newBalance, amt)