From 6ade175b1199244c590619320facd48944add2a9 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Wed, 5 Feb 2025 15:24:25 +0100 Subject: [PATCH 1/5] core/tracing: auto upgrade deprecated hooks --- core/state/statedb_hooked.go | 2 -- core/state/statedb_hooked_test.go | 2 +- core/tracing/hooks.go | 36 +++++++++++++++++++++++++++++++ core/tracing/journal.go | 23 ++++++++++---------- core/tracing/journal_test.go | 6 ++++++ eth/tracers/dir.go | 14 +++++++++++- eth/tracers/live.go | 8 ++++++- eth/tracers/live/supply.go | 4 ++++ 8 files changed, 78 insertions(+), 17 deletions(-) diff --git a/core/state/statedb_hooked.go b/core/state/statedb_hooked.go index a2fdfe9a21..b0501ce64d 100644 --- a/core/state/statedb_hooked.go +++ b/core/state/statedb_hooked.go @@ -184,8 +184,6 @@ func (s *hookedStateDB) SetNonce(address common.Address, nonce uint64, reason tr s.inner.SetNonce(address, nonce, reason) if s.hooks.OnNonceChangeV2 != nil { s.hooks.OnNonceChangeV2(address, prev, nonce, reason) - } else if s.hooks.OnNonceChange != nil { - s.hooks.OnNonceChange(address, prev, nonce) } } diff --git a/core/state/statedb_hooked_test.go b/core/state/statedb_hooked_test.go index ce42e96409..0f47ddcf8f 100644 --- a/core/state/statedb_hooked_test.go +++ b/core/state/statedb_hooked_test.go @@ -98,7 +98,7 @@ func TestHooks(t *testing.T) { OnBalanceChange: func(addr common.Address, prev, new *big.Int, reason tracing.BalanceChangeReason) { emitF("%v.balance: %v->%v (%v)", addr, prev, new, reason) }, - OnNonceChange: func(addr common.Address, prev, new uint64) { + OnNonceChangeV2: func(addr common.Address, prev, new uint64, reason tracing.NonceChangeReason) { emitF("%v.nonce: %v->%v", addr, prev, new) }, OnCodeChange: func(addr common.Address, prevCodeHash common.Hash, prevCode []byte, codeHash common.Hash, code []byte) { diff --git a/core/tracing/hooks.go b/core/tracing/hooks.go index 4002b57207..3ad7ed3d38 100644 --- a/core/tracing/hooks.go +++ b/core/tracing/hooks.go @@ -25,6 +25,7 @@ package tracing import ( + "errors" "math/big" "github.com/ethereum/go-ethereum/common" @@ -33,6 +34,11 @@ import ( "github.com/holiman/uint256" ) +var ( + // ErrHookConflict is returned when conflicting hooks are exposed. + ErrHookConflict = errors.New("hook conflict") +) + // OpContext provides the context at which the opcode is being // executed in, including the memory, stack and various contract-level information. type OpContext interface { @@ -217,6 +223,36 @@ type Hooks struct { OnBlockHashRead BlockHashReadHook } +// Upgrade will upgrade the methods to their most recent versions +// when possible, e.g. use `OnNonceChangeV2` instead of `OnNonceChange`. +func Upgrade(hooks *Hooks) (*Hooks, error) { + if hooks == nil { + return &Hooks{}, nil + } + if hooks.OnNonceChange != nil && hooks.OnNonceChangeV2 != nil { + return nil, ErrHookConflict + } + if hooks.OnSystemCallStart != nil && hooks.OnSystemCallStartV2 != nil { + return nil, ErrHookConflict + } + // Shallow copy + tmp := *hooks + internal := &tmp + if hooks.OnNonceChange != nil { + hooks.OnNonceChangeV2 = func(addr common.Address, prev, new uint64, reason NonceChangeReason) { + internal.OnNonceChange(addr, prev, new) + } + hooks.OnNonceChange = nil + } + if hooks.OnSystemCallStart != nil { + hooks.OnSystemCallStartV2 = func(vm *VMContext) { + internal.OnSystemCallStart() + } + hooks.OnSystemCallStart = nil + } + return hooks, nil +} + // BalanceChangeReason is used to indicate the reason for a balance change, useful // for tracing and reporting. type BalanceChangeReason byte diff --git a/core/tracing/journal.go b/core/tracing/journal.go index 8937d4c5ae..0917cc65fc 100644 --- a/core/tracing/journal.go +++ b/core/tracing/journal.go @@ -41,12 +41,17 @@ func WrapWithJournal(hooks *Hooks) (*Hooks, error) { if hooks == nil { return nil, fmt.Errorf("wrapping nil tracer") } - // No state change to journal, return the wrapped hooks as is - if hooks.OnBalanceChange == nil && hooks.OnNonceChange == nil && hooks.OnNonceChangeV2 == nil && hooks.OnCodeChange == nil && hooks.OnStorageChange == nil { - return hooks, nil + + // Upgrade hooks so we don't have to bother with old versions. + var err error + hooks, err = Upgrade(hooks) + if err != nil { + return nil, err } - if hooks.OnNonceChange != nil && hooks.OnNonceChangeV2 != nil { - return nil, fmt.Errorf("cannot have both OnNonceChange and OnNonceChangeV2") + + // No state change to journal, return the wrapped hooks as is + if hooks.OnBalanceChange == nil && hooks.OnNonceChangeV2 == nil && hooks.OnCodeChange == nil && hooks.OnStorageChange == nil { + return hooks, nil } // Create a new Hooks instance and copy all hooks @@ -62,12 +67,10 @@ func WrapWithJournal(hooks *Hooks) (*Hooks, error) { if hooks.OnBalanceChange != nil { wrapped.OnBalanceChange = j.OnBalanceChange } - if hooks.OnNonceChange != nil || hooks.OnNonceChangeV2 != nil { + if hooks.OnNonceChangeV2 != nil { // Regardless of which hook version is used in the tracer, // the journal will want to capture the nonce change reason. wrapped.OnNonceChangeV2 = j.OnNonceChangeV2 - // A precaution to ensure EVM doesn't call both hooks. - wrapped.OnNonceChange = nil } if hooks.OnCodeChange != nil { wrapped.OnCodeChange = j.OnCodeChange @@ -156,8 +159,6 @@ func (j *journal) OnNonceChangeV2(addr common.Address, prev, new uint64, reason } if j.hooks.OnNonceChangeV2 != nil { j.hooks.OnNonceChangeV2(addr, prev, new, reason) - } else if j.hooks.OnNonceChange != nil { - j.hooks.OnNonceChange(addr, prev, new) } } @@ -219,8 +220,6 @@ func (b balanceChange) revert(hooks *Hooks) { func (n nonceChange) revert(hooks *Hooks) { if hooks.OnNonceChangeV2 != nil { hooks.OnNonceChangeV2(n.addr, n.new, n.prev, NonceChangeRevert) - } else if hooks.OnNonceChange != nil { - hooks.OnNonceChange(n.addr, n.new, n.prev) } } diff --git a/core/tracing/journal_test.go b/core/tracing/journal_test.go index d9616a2ce8..9b75b3da21 100644 --- a/core/tracing/journal_test.go +++ b/core/tracing/journal_test.go @@ -297,7 +297,9 @@ func newTracerAllHooks() *tracerAllHooks { for i := 0; i < hooksType.NumField(); i++ { t.hooksCalled[hooksType.Field(i).Name] = false } + // Deprecated methods delete(t.hooksCalled, "OnNonceChange") + delete(t.hooksCalled, "OnSystemCallStart") return t } @@ -322,9 +324,13 @@ func (t *tracerAllHooks) hooks() *Hooks { hooksValue := reflect.ValueOf(h).Elem() for i := 0; i < hooksValue.NumField(); i++ { field := hooksValue.Type().Field(i) + // Skip deprecated methods if field.Name == "OnNonceChange" { continue } + if field.Name == "OnSystemCallStart" { + continue + } hookMethod := reflect.MakeFunc(field.Type, func(args []reflect.Value) []reflect.Value { t.hooksCalled[field.Name] = true return nil diff --git a/eth/tracers/dir.go b/eth/tracers/dir.go index 1cdfab5454..4ec4967b41 100644 --- a/eth/tracers/dir.go +++ b/eth/tracers/dir.go @@ -45,6 +45,14 @@ type Tracer struct { Stop func(err error) } +func (t *Tracer) upgrade() (*Tracer, error) { + upgraded, err := tracing.Upgrade(t.Hooks) + if err != nil { + return nil, err + } + return &Tracer{Hooks: upgraded, GetResult: t.GetResult, Stop: t.Stop}, nil +} + type ctorFn func(*Context, json.RawMessage, *params.ChainConfig) (*Tracer, error) type jsCtorFn func(string, *Context, json.RawMessage, *params.ChainConfig) (*Tracer, error) @@ -84,7 +92,11 @@ func (d *directory) New(name string, ctx *Context, cfg json.RawMessage, chainCon cfg = json.RawMessage("{}") } if elem, ok := d.elems[name]; ok { - return elem.ctor(ctx, cfg, chainConfig) + t, err := elem.ctor(ctx, cfg, chainConfig) + if err != nil { + return nil, err + } + return t.upgrade() } // Assume JS code return d.jsEval(name, ctx, cfg, chainConfig) diff --git a/eth/tracers/live.go b/eth/tracers/live.go index 8b222d2e6c..c785b576d4 100644 --- a/eth/tracers/live.go +++ b/eth/tracers/live.go @@ -19,6 +19,7 @@ package tracers import ( "encoding/json" "errors" + "fmt" "github.com/ethereum/go-ethereum/core/tracing" ) @@ -36,6 +37,7 @@ type liveDirectory struct { // Register registers a tracer constructor by name. func (d *liveDirectory) Register(name string, f ctorFunc) { d.elems[name] = f + fmt.Printf("Registered tracer %q\n", name) } // New instantiates a tracer by name. @@ -44,7 +46,11 @@ func (d *liveDirectory) New(name string, config json.RawMessage) (*tracing.Hooks config = json.RawMessage("{}") } if f, ok := d.elems[name]; ok { - return f(config) + hooks, err := f(config) + if err != nil { + return nil, err + } + return tracing.Upgrade(hooks) } return nil, errors.New("not found") } diff --git a/eth/tracers/live/supply.go b/eth/tracers/live/supply.go index bae7445cb4..8abdfed869 100644 --- a/eth/tracers/live/supply.go +++ b/eth/tracers/live/supply.go @@ -120,6 +120,7 @@ func newSupplyTracer(cfg json.RawMessage) (*tracing.Hooks, error) { OnGenesisBlock: t.onGenesisBlock, OnTxStart: t.onTxStart, OnBalanceChange: t.onBalanceChange, + OnNonceChange: t.onNonceChange, OnEnter: t.onEnter, OnExit: t.onExit, OnClose: t.onClose, @@ -326,3 +327,6 @@ func (s *supplyTracer) write(data any) { log.Warn("failed to write to supply tracer log file", "error", err) } } +func (t *supplyTracer) onNonceChange(addr common.Address, prev, new uint64) { + fmt.Printf("Nonce change for %s: %d -> %d\n", addr, prev, new) +} From b2693ab8e2c32ceb4c05a024e72d94e7d173c1a6 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Wed, 5 Feb 2025 15:35:01 +0100 Subject: [PATCH 2/5] rm debug code --- eth/tracers/live/supply.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/eth/tracers/live/supply.go b/eth/tracers/live/supply.go index 8abdfed869..bae7445cb4 100644 --- a/eth/tracers/live/supply.go +++ b/eth/tracers/live/supply.go @@ -120,7 +120,6 @@ func newSupplyTracer(cfg json.RawMessage) (*tracing.Hooks, error) { OnGenesisBlock: t.onGenesisBlock, OnTxStart: t.onTxStart, OnBalanceChange: t.onBalanceChange, - OnNonceChange: t.onNonceChange, OnEnter: t.onEnter, OnExit: t.onExit, OnClose: t.onClose, @@ -327,6 +326,3 @@ func (s *supplyTracer) write(data any) { log.Warn("failed to write to supply tracer log file", "error", err) } } -func (t *supplyTracer) onNonceChange(addr common.Address, prev, new uint64) { - fmt.Printf("Nonce change for %s: %d -> %d\n", addr, prev, new) -} From 0ccd62465c85798c653e531c42415ffad085bf1c Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Wed, 5 Feb 2025 16:00:37 +0100 Subject: [PATCH 3/5] simplify systemcall start calling --- core/state_processor.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/core/state_processor.go b/core/state_processor.go index 902ff582e8..7041a26932 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -23,7 +23,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/consensus/misc" "github.com/ethereum/go-ethereum/core/state" - "github.com/ethereum/go-ethereum/core/tracing" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/crypto" @@ -213,7 +212,9 @@ func ApplyTransaction(evm *vm.EVM, gp *GasPool, statedb *state.StateDB, header * // contract. This method is exported to be used in tests. func ProcessBeaconBlockRoot(beaconRoot common.Hash, evm *vm.EVM) { if tracer := evm.Config.Tracer; tracer != nil { - onSystemCallStart(tracer, evm.GetVMContext()) + if tracer.OnSystemCallStartV2 != nil { + tracer.OnSystemCallStartV2(evm.GetVMContext()) + } if tracer.OnSystemCallEnd != nil { defer tracer.OnSystemCallEnd() } @@ -237,7 +238,9 @@ func ProcessBeaconBlockRoot(beaconRoot common.Hash, evm *vm.EVM) { // as per EIP-2935/7709. func ProcessParentBlockHash(prevHash common.Hash, evm *vm.EVM) { if tracer := evm.Config.Tracer; tracer != nil { - onSystemCallStart(tracer, evm.GetVMContext()) + if tracer.OnSystemCallStartV2 != nil { + tracer.OnSystemCallStartV2(evm.GetVMContext()) + } if tracer.OnSystemCallEnd != nil { defer tracer.OnSystemCallEnd() } @@ -277,7 +280,9 @@ func ProcessConsolidationQueue(requests *[][]byte, evm *vm.EVM) { func processRequestsSystemCall(requests *[][]byte, evm *vm.EVM, requestType byte, addr common.Address) { if tracer := evm.Config.Tracer; tracer != nil { - onSystemCallStart(tracer, evm.GetVMContext()) + if tracer.OnSystemCallStartV2 != nil { + tracer.OnSystemCallStartV2(evm.GetVMContext()) + } if tracer.OnSystemCallEnd != nil { defer tracer.OnSystemCallEnd() } @@ -323,11 +328,3 @@ func ParseDepositLogs(requests *[][]byte, logs []*types.Log, config *params.Chai } return nil } - -func onSystemCallStart(tracer *tracing.Hooks, ctx *tracing.VMContext) { - if tracer.OnSystemCallStartV2 != nil { - tracer.OnSystemCallStartV2(ctx) - } else if tracer.OnSystemCallStart != nil { - tracer.OnSystemCallStart() - } -} From 6879bbf7dedb380e155d366b20f7859a20c508ff Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Wed, 5 Feb 2025 16:01:24 +0100 Subject: [PATCH 4/5] various --- cmd/evm/internal/t8ntool/file_tracer.go | 43 +++++-------------------- eth/tracers/logger/logger_json.go | 24 +++++++------- 2 files changed, 20 insertions(+), 47 deletions(-) diff --git a/cmd/evm/internal/t8ntool/file_tracer.go b/cmd/evm/internal/t8ntool/file_tracer.go index 38fc35bd32..b1e642b70d 100644 --- a/cmd/evm/internal/t8ntool/file_tracer.go +++ b/cmd/evm/internal/t8ntool/file_tracer.go @@ -20,7 +20,6 @@ import ( "encoding/json" "fmt" "io" - "math/big" "os" "path/filepath" @@ -115,38 +114,12 @@ func (l *fileWritingTracer) OnTxEnd(receipt *types.Receipt, err error) { } func (l *fileWritingTracer) hooks() *tracing.Hooks { - return &tracing.Hooks{ - OnTxStart: l.OnTxStart, - OnTxEnd: l.OnTxEnd, - OnEnter: func(depth int, typ byte, from common.Address, to common.Address, input []byte, gas uint64, value *big.Int) { - if l.inner != nil && l.inner.OnEnter != nil { - l.inner.OnEnter(depth, typ, from, to, input, gas, value) - } - }, - OnExit: func(depth int, output []byte, gasUsed uint64, err error, reverted bool) { - if l.inner != nil && l.inner.OnExit != nil { - l.inner.OnExit(depth, output, gasUsed, err, reverted) - } - }, - OnOpcode: func(pc uint64, op byte, gas, cost uint64, scope tracing.OpContext, rData []byte, depth int, err error) { - if l.inner != nil && l.inner.OnOpcode != nil { - l.inner.OnOpcode(pc, op, gas, cost, scope, rData, depth, err) - } - }, - OnFault: func(pc uint64, op byte, gas, cost uint64, scope tracing.OpContext, depth int, err error) { - if l.inner != nil && l.inner.OnFault != nil { - l.inner.OnFault(pc, op, gas, cost, scope, depth, err) - } - }, - OnSystemCallStart: func() { - if l.inner != nil && l.inner.OnSystemCallStart != nil { - l.inner.OnSystemCallStart() - } - }, - OnSystemCallEnd: func() { - if l.inner != nil && l.inner.OnSystemCallEnd != nil { - l.inner.OnSystemCallEnd() - } - }, - } + // Shallow copy + tmp := *l.inner + hooks := &tmp + + hooks.OnTxStart = l.OnTxStart + hooks.OnTxEnd = l.OnTxEnd + + return hooks } diff --git a/eth/tracers/logger/logger_json.go b/eth/tracers/logger/logger_json.go index 52ac3945d4..0bd11d7993 100644 --- a/eth/tracers/logger/logger_json.go +++ b/eth/tracers/logger/logger_json.go @@ -69,11 +69,11 @@ func NewJSONLogger(cfg *Config, writer io.Writer) *tracing.Hooks { l.cfg = &Config{} } l.hooks = &tracing.Hooks{ - OnTxStart: l.OnTxStart, - OnSystemCallStart: l.onSystemCallStart, - OnExit: l.OnExit, - OnOpcode: l.OnOpcode, - OnFault: l.OnFault, + OnTxStart: l.OnTxStart, + OnSystemCallStartV2: l.onSystemCallStartV2, + OnExit: l.OnExit, + OnOpcode: l.OnOpcode, + OnFault: l.OnFault, } return l.hooks } @@ -86,12 +86,12 @@ func NewJSONLoggerWithCallFrames(cfg *Config, writer io.Writer) *tracing.Hooks { l.cfg = &Config{} } l.hooks = &tracing.Hooks{ - OnTxStart: l.OnTxStart, - OnSystemCallStart: l.onSystemCallStart, - OnEnter: l.OnEnter, - OnExit: l.OnExit, - OnOpcode: l.OnOpcode, - OnFault: l.OnFault, + OnTxStart: l.OnTxStart, + OnSystemCallStartV2: l.onSystemCallStartV2, + OnEnter: l.OnEnter, + OnExit: l.OnExit, + OnOpcode: l.OnOpcode, + OnFault: l.OnFault, } return l.hooks } @@ -127,7 +127,7 @@ func (l *jsonLogger) OnOpcode(pc uint64, op byte, gas, cost uint64, scope tracin l.encoder.Encode(log) } -func (l *jsonLogger) onSystemCallStart() { +func (l *jsonLogger) onSystemCallStartV2(ctx *tracing.VMContext) { // Process no events while in system call. hooks := *l.hooks *l.hooks = tracing.Hooks{ From 0a73826f0acfc943fb40d6100f71bc4f165449f0 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Wed, 5 Feb 2025 16:09:49 +0100 Subject: [PATCH 5/5] rm debug code --- eth/tracers/live.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/eth/tracers/live.go b/eth/tracers/live.go index c785b576d4..b577cb2eb8 100644 --- a/eth/tracers/live.go +++ b/eth/tracers/live.go @@ -19,7 +19,6 @@ package tracers import ( "encoding/json" "errors" - "fmt" "github.com/ethereum/go-ethereum/core/tracing" ) @@ -37,7 +36,6 @@ type liveDirectory struct { // Register registers a tracer constructor by name. func (d *liveDirectory) Register(name string, f ctorFunc) { d.elems[name] = f - fmt.Printf("Registered tracer %q\n", name) } // New instantiates a tracer by name.