(feat): tracing support for state-sync transactions#2172
Conversation
Add live tracing and refactor tracing methods for state-sync transactions. Co-authored-by: Milan Došlić <milan.doslic@tenderly.co>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
@claude review |
|
Claude finished @manav2401's task in 3m 51s —— View job Code Review: State-Sync Tracing Support
SummaryGood PR overall. The refactoring consolidates state-sync tracing into the standard tracing codepaths (removing the separate Below are the findings, organized by severity. Issues1. [HIGH]
|
There was a problem hiding this comment.
Pull request overview
Adds Bor-specific tracing support so state-sync (bridge) transactions are included in VM/live traces, and refactors tracer codepaths to treat state-sync txs as first-class transactions (post-Madhugiri). Also updates consensus “system tx” execution to propagate VM tracer config and adds end-to-end tests that replay state-sync events via the StateReceiver contract.
Changes:
- Extend tracing APIs to replay and trace post-Madhugiri
StateSyncTxexecution, including correct log indexing/cumulative context plumbing. - Propagate
vmtraceVM config into Bor consensus system calls (span/state commits) so live tracing covers bridge processing. - Add E2E state-sync tracing tests using actual StateReceiver bytecode and update docs/config defaults for VM tracing.
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/bor/mocks/IHeimdallClient.go | Regenerated mocks (mockgen metadata/any updates). |
| tests/bor/helper.go | Update mock expectations for CommitSpan signature change. |
| tests/bor/bor_test.go | Update mock expectations for CommitSpan signature change. |
| params/protocol_params.go | Introduce params.BorSystemAddress for Bor system transactions. |
| internal/cli/server/service.go | Switch DebugBlock tracing implementation to tracer API’s TraceBlockByNumber. |
| internal/cli/server/flags.go | Clarify vmtrace flag usage text. |
| internal/cli/server/config.go | Extend/reorder defaults (vmtrace/jsonconfig, rpc limits, sync/gc defaults, etc.). |
| eth/tracers/internal/tracetest/supply_test.go | Update expected supply trace fields (issuance reward). |
| eth/tracers/dir.go | Add Bor-specific context fields for state-sync tracing (cumulative gas/log index). |
| eth/tracers/api.go | Major tracer refactor: state-sync tx replay/tracing, new traceTx return signature, removal of BorTraceEnabled paths. |
| eth/tracers/api_test.go | Remove IO tracing dump test and related helpers. |
| eth/tracers/api_statesync_test.go | Add E2E tests for tracing state-sync txs via StateReceiver + target contract. |
| eth/tracers/api_bor.go | Remove Bor-only tracing API (TraceBorBlock) implementation. |
| eth/state_accessor.go | Add bounds check for txIndex in stateAtTransaction. |
| eth/filters/IIterator.go | Regenerated mocks (mockgen metadata/structure). |
| eth/filters/IDatabase.go | Regenerated mocks + parameter naming cleanup/any updates. |
| eth/filters/IBatch.go | Regenerated mocks + parameter naming cleanup/any updates. |
| eth/filters/IBackend.go | Regenerated mocks + interface additions reflected in mock. |
| eth/ethconfig/config.go | Use centralized Bor ABI package for validator set ABI. |
| eth/backend.go | Pass VM tracer config into Bor engine for system tx tracing (SetVMConfig). |
| docs/cli/server.md | Document updated vmtrace / vmtrace.jsonconfig defaults. |
| docs/cli/example_config.toml | Add VM tracing example config entries. |
| docs/cli/default_config.toml | Default vmtrace.jsonconfig to {} instead of empty string. |
| core/types/transaction.go | Add Transaction.GetStateSyncData() helper for StateSyncTx. |
| core/state_processor.go | Fire tracer hooks around state-sync tx processing and pass hooked state into Finalize. |
| core/parallel_state_processor.go | Fire tracer hooks around state-sync tx processing in parallel processor. |
| consensus/bor/statefull/processor.go | Add replay helper ApplyStateSyncEvents and plumb VM config into system calls. |
| consensus/bor/span.go | Extend Spanner.CommitSpan signature to accept vm.Config. |
| consensus/bor/span_mock.go | Regenerated mock for updated Spanner interface. |
| consensus/bor/heimdall/span/spanner.go | Pass VM config through to system call execution; use centralized SystemTxGas constant. |
| consensus/bor/genesis.go | Extend GenesisContract CommitState signature to accept vm.Config. |
| consensus/bor/genesis_contract_mock.go | Regenerated mock for updated GenesisContract interface. |
| consensus/bor/contract/client.go | Move ABI + SystemTxGas constants to consensus/bor/abi. |
| consensus/bor/bor.go | Store VM config in Bor engine and use it for span/state system transactions. |
| consensus/bor/bor_test.go | Update fakes/mocks for new CommitSpan / CommitState signatures. |
| consensus/bor/abi/common.go | New centralized Bor ABI + SystemTxGas definitions. |
| cmd/keeper/go.sum | Dependency checksum updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code ReviewFound 3 issues: 1. Bug: Duplicate import causes compilation failure - consensus/bor/heimdall/span/spanner.go:13-14 The same package consensus/bor/abi is imported twice (once as abi, once as borabi). Go does not allow this. Fix: Remove the unaliased import and use borabi consistently. 2. Bug: OnTxEnd called without guard - panics on empty blocks - core/state_processor.go:176-181, core/parallel_state_processor.go:448-453 OnTxEnd is inside the IsMadhugiri block but NOT guarded by appliedNewStateSyncReceipt. On empty blocks, receipts[len(receipts)-1] panics with index out of range. On blocks without state-sync tx, OnTxEnd fires without matching OnTxStart. Fix: Move OnTxEnd inside the appliedNewStateSyncReceipt guard. 3. Bug: time.Now() produces non-deterministic traces - consensus/bor/statefull/processor.go:181 ApplyStateSyncEvents uses time.Now().Unix() as syncTime, but production path uses deterministic event.Time from Heimdall. Makes trace results non-reproducible. Root cause: StateSyncData lacks a Time field. Fix: Add Time field to StateSyncData and use it instead of time.Now(). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 37 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (31.80%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #2172 +/- ##
===========================================
- Coverage 52.29% 52.23% -0.07%
===========================================
Files 885 886 +1
Lines 155727 155726 -1
===========================================
- Hits 81441 81339 -102
- Misses 69055 69172 +117
+ Partials 5231 5215 -16
... and 19 files with indirect coverage changes
🚀 New features to boost your workflow:
|
|
@claude review |
|
Bug [eth/tracers/api.go:904] — Same missing nil guard on .Bor as above. api.backend.ChainConfig().Bor.StateReceiverContract is accessed without checking if Bor is nil in standardTraceBlockToFile, which is also RPC-exposed. Fix: same pattern — 'if borConfig := api.backend.ChainConfig().Bor; borConfig != nil { stateReceiverAddress = borConfig.StateReceiverContract }'. |
|
Bug [core/state_processor.go:159] — Unbalanced OnTxStart/OnTxEnd tracer lifecycle. OnTxStart fires unconditionally (line 157-159) when the last tx is StateSyncTxType, but OnTxEnd is only called when appliedNewStateSyncReceipt is true (line 176-180). If Finalize does not produce a receipt (stateSyncData is empty), the tx/receipt count mismatch at line 171 causes an error return without ever calling OnTxEnd. This leaves tracers like callTracer in an inconsistent state. Same issue exists in parallel_state_processor.go lines 425-453. Fix: move OnTxStart after Finalize succeeds, or call OnTxEnd before the error return at line 172. |
|
This PR is stale because it has been open 21 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
|
@claude review this PR |
|
Claude finished @manav2401's task in 4m 8s —— View job Code Review: State-Sync Tracing Support
I reviewed the full PR (commit Below are the remaining findings. Issues1. [HIGH]
|
|
@claude regarding your review findings.
Review the PR again. |
|
Claude finished @manav2401's task in 4m 58s —— View job Code Review: State-Sync Tracing Support (Re-review)
I reviewed the full PR at commit Acknowledgment of Author's Responses
Remaining Findings1. [MEDIUM] Live tracing:
|
|
@claude regarding your findings.
|
|
Claude finished @manav2401's task in 48s —— View job Acknowledging Review Responses
Thanks for the responses, @manav2401. Finding #1 (live tracing Finding #2 ( No further action needed on this PR from my side. |
|
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 40 changed files in this pull request and generated 2 comments.
Files not reviewed (6)
- consensus/bor/genesis_contract_mock.go: Language not supported
- consensus/bor/span_mock.go: Language not supported
- eth/filters/IBackend.go: Language not supported
- eth/filters/IBatch.go: Language not supported
- eth/filters/IDatabase.go: Language not supported
- eth/filters/IIterator.go: Language not supported
Description
Extension of #2062 from @milando12.
This PR
debug_traceBlockByNumber,debug_traceBlockByHash,debug_traceChain, etc).The reason why we add custom hooks is because state-sync transactions can contain multiple bridge events and all of them are executed as separate EVM calls which are packed into 1 transaction. This breaks assumptions for certain tracer e.g. in
callTracer, it should only have 1 root call frame per transaction. This would be invalid for state-sync transactions with multiple state-sync events (because it would have multiple independent root call frames). Hence, referencing the erigon implementation, this PR adds wrapper hooks which crafts a single synthetic root call frame and all bridge event calls are nested sub-calls inside it.Changes
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it
Nodes audience
In case this PR includes changes that must be applied only to a subset of nodes, please specify how you handled it (e.g. by adding a flag with a default value...)
Checklist
Cross repository changes
Testing
Manual tests
Please complete this section with the steps you performed if you ran manual tests for this functionality, otherwise delete it
Additional comments
Please post additional comments in this section if you have them, otherwise delete it