From 3bed587d37227f2e74b5949b10d52efc7d7453a1 Mon Sep 17 00:00:00 2001 From: Matthieu Vachon Date: Wed, 17 Apr 2024 14:21:11 -0400 Subject: [PATCH] Fixed PR_REVIEW notes, added helpers for tracing transfer # Conflicts: # evmrpc/config.go # evmrpc/config_test.go # precompiles/bank/bank.go # x/evm/module.go --- app/app.go | 7 +--- evmrpc/config.go | 32 ++++---------- evmrpc/config_test.go | 15 +++---- precompiles/bank/bank.go | 39 +++++------------- precompiles/bank/bank_test.go | 39 ++++++++++++++++++ x/evm/module.go | 30 +++++--------- x/evm/state/balance.go | 1 - x/evm/tracers/balances.go | 78 +++++++++++++++++++++++++++++++++++ x/evm/tracers/registry.go | 3 -- x/evm/tracers/tracing.go | 8 +--- 10 files changed, 154 insertions(+), 98 deletions(-) create mode 100644 x/evm/tracers/balances.go diff --git a/app/app.go b/app/app.go index 138791497..f77dcdd50 100644 --- a/app/app.go +++ b/app/app.go @@ -625,12 +625,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)) diff --git a/evmrpc/config.go b/evmrpc/config.go index 52cfe7497..d6afe1f10 100644 --- a/evmrpc/config.go +++ b/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 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"` @@ -102,6 +92,7 @@ var DefaultConfig = Config{ IdleTimeout: rpc.DefaultHTTPTimeouts.IdleTimeout, SimulationGasLimit: 10_000_000, // 10M SimulationEVMTimeout: 60 * time.Second, + LiveEVMTracer: "", CORSOrigins: "*", WSOrigins: "*", FilterTimeout: 120 * time.Second, @@ -122,6 +113,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" @@ -129,9 +121,6 @@ const ( flagCheckTxTimeout = "evm.checktx_timeout" flagSlow = "evm.slow" flagDenyList = "evm.deny_list" - 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) { @@ -187,6 +176,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 @@ -222,16 +216,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 } diff --git a/evmrpc/config_test.go b/evmrpc/config_test.go index 72d64f25f..b2c82669c 100644 --- a/evmrpc/config_test.go +++ b/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{} @@ -27,8 +28,6 @@ type opts struct { maxTxPoolTxs interface{} slow interface{} denyList interface{} - liveEVMTracer interface{} - liveEVMTracerChainID interface{} } func (o *opts) Get(k string) interface{} { @@ -62,6 +61,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 } @@ -83,12 +85,6 @@ func (o *opts) Get(k string) interface{} { if k == "evm.deny_list" { return o.denyList } - 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)) } @@ -106,13 +102,12 @@ func TestReadConfig(t *testing.T) { time.Duration(60), "", "", + "", time.Duration(5), time.Duration(5), 1000, false, make([]string, 0), - "", - 0, } _, err := evmrpc.ReadConfig(&goodOpts) require.Nil(t, err) diff --git a/precompiles/bank/bank.go b/precompiles/bank/bank.go index 9fff25eed..df9d8f0a8 100644 --- a/precompiles/bank/bank.go +++ b/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" @@ -214,46 +213,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 bech32, this shouldn't happen because SendCoinsAndWei worked: %w", 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) diff --git a/precompiles/bank/bank_test.go b/precompiles/bank/bank_test.go index c4a16dd8b..313b40e41 100644 --- a/precompiles/bank/bank_test.go +++ b/precompiles/bank/bank_test.go @@ -10,6 +10,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" 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" @@ -18,6 +19,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" @@ -26,7 +29,18 @@ import ( 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: ðtracing.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 @@ -116,6 +130,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 { @@ -243,3 +266,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") + +} diff --git a/x/evm/module.go b/x/evm/module.go index 9e8625eef..08da5b027 100644 --- a/x/evm/module.go +++ b/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" @@ -235,38 +233,32 @@ func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.Val if err := am.keeper.BankKeeper().SendCoinsAndWei(ctx, coinbaseAddress, coinbase, balance.Amount, weiBalance); err != nil { 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()) - evmHooks.OnBalanceChange(evmAddress, oldBalance, newBalance, tracing.BalanceIncreaseRewardTransactionFee) + if evmHooks != nil && evmHooks.OnBalanceChange != nil { + fromEVMAddr := am.keeper.GetEVMAddressOrDefault(ctx, coinbaseAddress) + toEVMAddr := am.keeper.GetEVMAddressOrDefault(ctx, coinbase) + + tracers.TraceTransferUseiAndWei(ctx, evmHooks, am.keeper.BankKeeper(), coinbaseAddress, fromEVMAddr, coinbase, toEVMAddr, balance, weiBalance) } } surplus = surplus.Add(deferredInfo.Surplus) } surplusUsei, surplusWei := state.SplitUseiWeiAmount(surplus.BigInt()) + evmModuleAddress := am.keeper.AccountKeeper().GetModuleAddress(types.ModuleName) if surplusUsei.GT(sdk.ZeroInt()) { - evmModuleAddress := am.keeper.AccountKeeper().GetModuleAddress(types.ModuleName) - if err := am.keeper.BankKeeper().AddCoins(ctx, evmModuleAddress, sdk.NewCoins(sdk.NewCoin(am.keeper.GetBaseDenom(ctx), surplusUsei)), true); err != nil { ctx.Logger().Error("failed to send usei surplus of %s to EVM module account", surplusUsei) } 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{} diff --git a/x/evm/state/balance.go b/x/evm/state/balance.go index 376826586..1ed182263 100644 --- a/x/evm/state/balance.go +++ b/x/evm/state/balance.go @@ -85,7 +85,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) diff --git a/x/evm/tracers/balances.go b/x/evm/tracers/balances.go new file mode 100644 index 000000000..e6ef27dfd --- /dev/null +++ b/x/evm/tracers/balances.go @@ -0,0 +1,78 @@ +package tracers + +import ( + "math/big" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/tracing" + "github.com/sei-protocol/sei-chain/x/evm/state" +) + +type BankBalanceKeeper interface { + GetBalance(sdk.Context, sdk.AccAddress, string) sdk.Coin + GetWeiBalance(ctx sdk.Context, addr sdk.AccAddress) sdk.Int +} + +func TraceTransferUseiAndWei( + ctx sdk.Context, + hooks *tracing.Hooks, + bankKeeper BankBalanceKeeper, + fromSeiAddr sdk.AccAddress, + fromEVMAddr common.Address, + toSeiAddr sdk.AccAddress, + toEVMAddr common.Address, + usei sdk.Coin, + wei sdk.Int, +) { + value := usei.Amount.Mul(state.SdkUseiToSweiMultiplier).Add(wei).BigInt() + TraceTransferEVMValue(ctx, hooks, bankKeeper, fromSeiAddr, fromEVMAddr, toSeiAddr, toEVMAddr, value) +} + +func TraceTransferEVMValue( + ctx sdk.Context, + hooks *tracing.Hooks, + bankKeeper BankBalanceKeeper, + fromSeiAddr sdk.AccAddress, + fromEVMAddr common.Address, + toSeiAddr sdk.AccAddress, + toEVMAddr common.Address, + value *big.Int, +) { + // From address got value removed from it + newBalance := getEVMBalance(ctx, bankKeeper, fromSeiAddr) + oldBalance := new(big.Int).Add(newBalance, value) + + hooks.OnBalanceChange(fromEVMAddr, oldBalance, newBalance, tracing.BalanceChangeTransfer) + + // To received valye from the sender + newBalance = getEVMBalance(ctx, bankKeeper, toSeiAddr) + oldBalance = new(big.Int).Sub(newBalance, value) + + hooks.OnBalanceChange(toEVMAddr, oldBalance, newBalance, tracing.BalanceChangeTransfer) +} + +func TraceBlockReward( + ctx sdk.Context, + hooks *tracing.Hooks, + bankKeeper BankBalanceKeeper, + toSeiAddr sdk.AccAddress, + toEVMAddr common.Address, + usei sdk.Coin, + wei sdk.Int, +) { + value := usei.Amount.Mul(state.SdkUseiToSweiMultiplier).Add(wei).BigInt() + + // To received value + newBalance := getEVMBalance(ctx, bankKeeper, toSeiAddr) + oldBalance := new(big.Int).Sub(newBalance, value) + + hooks.OnBalanceChange(toEVMAddr, oldBalance, newBalance, tracing.BalanceIncreaseRewardMineBlock) +} + +func getEVMBalance(ctx sdk.Context, bankKeeper BankBalanceKeeper, addr sdk.AccAddress) *big.Int { + swei := bankKeeper.GetBalance(ctx, addr, sdk.MustGetBaseDenom()).Amount.Mul(state.SdkUseiToSweiMultiplier) + wei := bankKeeper.GetWeiBalance(ctx, addr) + + return swei.Add(wei).BigInt() +} diff --git a/x/evm/tracers/registry.go b/x/evm/tracers/registry.go index 4c90641ba..71731343d 100644 --- a/x/evm/tracers/registry.go +++ b/x/evm/tracers/registry.go @@ -1,8 +1,5 @@ package tracers -// PR_REVIEW_NOTE: I defined a global registry to make it easy to register tracer. -// Let me know if you guys prefer to not have a global registry and instead register -// tracers explicitly, maybe in the `app/app.go` file. var GlobalLiveTracerRegistry = NewLiveTracerRegistry() type LiveTracerRegistry interface { diff --git a/x/evm/tracers/tracing.go b/x/evm/tracers/tracing.go index 8a5c1001d..6bfb7be97 100644 --- a/x/evm/tracers/tracing.go +++ b/x/evm/tracers/tracing.go @@ -18,12 +18,8 @@ import ( // by the registry. type BlockchainTracerFactory = func(tracerURL *url.URL) (*tracing.Hooks, error) -// PR_REVIEW_NOTE: I defined the tracer identifier to be either a plain string or an URL of the form ://, -// -// this way a tracer can be configured for example using some query parameters as "config" value. We use that in a lot -// of our project and found it's a pretty good way to configure "generic" dependency. -// -// We could switch to plain string if you prefer. +// NewBlockchainTracer creates a new [tracing.Hooks] struct that is used to trace blocks and transactions +// for EVM needs. The tracer is instansiated by the provided URL and registered in the registry. func NewBlockchainTracer(registry LiveTracerRegistry, tracerIdentifier string, chainConfig *params.ChainConfig) (*tracing.Hooks, error) { tracerURL, err := url.Parse(tracerIdentifier) if err != nil {