Fixed PR_REVIEW notes, added helpers for tracing transfer

Author: maoueh

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)