execution/commitment: route trie trace through an io.Writer#21859
Merged
Conversation
AskAlexSharov
approved these changes
Jun 17, 2026
awskii
force-pushed
the
awskii/trace-writer-v2
branch
2 times, most recently
from
76198a3 to
f334960
Compare
Replace HexPatriciaHashed's `trace bool` + `fmt.Printf` pattern with a single `traceW io.Writer`. `SetTrace(bool)` becomes `SetTraceWriter(io.Writer)` on the Trie interface; nil disables tracing. The `if traceW != nil` guard is kept, so disabled tracing stays free. Trace output is caller-controlled: pass os.Stderr, a file, or a bytes.Buffer to capture for assertions. The old SetCapture/GetCapture ([]string) mechanism is removed — capturing is just handing the trie a writer. The commitment context drives tracing with an io.Writer (SetTraceWriter), re-applied to the trie on each ComputeCommitment. The per-key domain line is already emitted to the writer as a `[proc]` line, so the separate `traceDomain bool`/stdout dump is dropped (left as a commented-out fmt.Println for ad-hoc debugging). SharedDomains no longer carries trace state. The concurrent trie wraps the writer in a mutex-guarded syncWriter and fans it out to the root and all mounts so cross-goroutine trace writes are safe. GenerateWitness resets traceW to nil to preserve the prior trace-off behavior.
awskii
force-pushed
the
awskii/trace-writer-v2
branch
from
f334960 to
aa144e8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors commitment-trie tracing from boolean flags + stdout printing to a caller-provided io.Writer, enabling flexible trace routing (stderr/file/buffer) and making trace output testable without maintaining a separate capture mechanism.
Changes:
- Replaces
SetTrace(bool)/SetTraceDomain(bool)/ capture APIs withSetTraceWriter(io.Writer)across commitment trie implementations and callers. - Updates commitment context to store and re-apply a trace writer per
ComputeCommitment, and adds tests assertingnildisables tracing andbytes.Buffercaptures output. - Adds a mutex-guarded writer wrapper in the concurrent trie to serialize cross-goroutine trace writes to a shared destination.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rpc/jsonrpc/eth_call.go | Removes legacy commitment trace toggles around SeekCommitment. |
| execution/tests/blockgen/chain_makers.go | Removes legacy commitment trace toggles during block generation. |
| execution/stagedsync/exec3_serial.go | Removes legacy commitment trace toggles in serial executor path. |
| execution/commitment/trie_reader_test.go | Updates tests to use SetTraceWriter(nil) instead of SetTrace(false). |
| execution/commitment/hex_patricia_hashed.go | Core migration: trace output now guarded by traceW != nil and written via fmt.Fprintf(traceW, ...); capture removed. |
| execution/commitment/hex_patricia_hashed_test.go | Updates existing tests to new API and adds new tests for writer-based tracing. |
| execution/commitment/hex_patricia_hashed_fuzz_test.go | Updates fuzz tests to disable tracing via SetTraceWriter(nil). |
| execution/commitment/hex_concurrent_patricia_hashed.go | Adds SetTraceWriter propagation with a mutex-wrapped writer for concurrency safety. |
| execution/commitment/commitmentdb/commitment_context.go | Replaces stored trace bool with stored io.Writer and re-applies writer each ComputeCommitment. |
| execution/commitment/commitment.go | Updates Trie interface to expose SetTraceWriter(io.Writer) only. |
| execution/commitment/commitment_bench_test.go | Updates benchmark setup to disable tracing via SetTraceWriter(nil). |
| execution/commitment/backtester/backtester.go | Enables commitment trace by wiring trace writer to os.Stderr based on logger trace level. |
| db/test/domains_restart_test.go | Removes legacy domains.SetTrace(...) usage. |
| db/test/aggregator_ext_test.go | Removes commented legacy domains.SetTrace(...) usage. |
| db/state/execctx/domain_shared.go | Drops stored trace/capture state and removes legacy SetTrace/Trace/CommitmentCapture APIs. |
| db/integrity/commitment_integrity.go | Enables commitment trace by wiring trace writer to os.Stderr based on logger trace level. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…race Address Copilot review on the trace-writer PR: - SharedDomainsCommitmentContext.SetTraceWriter now propagates the writer to the patricia trie immediately (not only on the next ComputeCommitment), so tracing is active for non-ComputeCommitment trie ops such as SeekCommitment/restore. ComputeCommitment still re-applies it (needed after GenerateWitness clears it). - exec3_serial: dbg.TraceBlock once again enables commitment tracing for the block — it routes the trie trace to os.Stderr around ComputeCommitment and resets it afterwards, so trace doesn't leak into later blocks.
- syncWriter.Write uses defer for the mutex unlock (panic-safe, idiomatic). - Rename TestSetTraceWriter_NilDisablesTrace -> TestSetTraceWriter_NilWriterSafe and fix its comment: it verifies a nil writer is safe (no nil-deref), not that output is suppressed (the buffer test already covers writer output).
Reconcile the trie-trace io.Writer refactor with main's parallel/streaming trie topology (the old ConcurrentPatriciaHashed was replaced by ParallelPatriciaHashed + StreamingCommitter + the deep-storage fold), and extend the refactor to cover the parallel path: - HexPatriciaHashed, ParallelPatriciaHashed, StreamingCommitter and the deep storage fold all route trace through traceW io.Writer via SetTraceWriter; SetTrace/SetTraceDomain/SetCapture/GetCapture and the SharedDomains trace flags are removed. - Concurrent writes are serialized by a shared syncWriter. Deep-storage fold workers tag every line with the parent account address so one account's fold is grep-able out of the interleaved parallel output; account/split workers tag by nibble. - TrieContext.trace -> traceW, wired from the commitment context, so branch reads and writes are traceable as [SDC] lines on the same writer.
awskii
requested review from
lupin012,
mh0lt,
sudeepdino008,
taratorio and
yperbasis
as code owners
… tracing off, handle short writes - foldKeys/foldSplit/mount/deep-storage workers only format the [nib]/[addr] trace tag when the writer is non-nil, and clear pooled workers' traceW when off, so disabled tracing stays allocation-free. - prefixWriter.Write returns io.ErrShortWrite on a partial underlying write instead of silently claiming success.
# Conflicts: # execution/commitment/hex_patricia_hashed.go
# Conflicts: # execution/commitment/hex_patricia_hashed.go
mh0lt
added a commit
that referenced
this pull request
Jul 3, 2026
Bring the cache stack current with main (parallel/streaming commitment correctness fixes #22184/#22113, nibblized-keccak cache #22185, erigondb.toml commitment referencing #21452, trie io.Writer trace #21859, etc.). One conflict in commitment_context.go's trieContext: keep both this branch's probeSd/probeTx (adaptive trunk-pin probe) and main's traceW (io.Writer trace).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reimplements #20316 on current
main(that branch was ~800 commits behind and conflicting); supersedes it. Replaces the commitment trie'strace/traceDomain/capture []string+fmt.Printftracing with a singletraceW io.Writer.Changes
Trieinterface:SetTrace/SetTraceDomain/SetCapture/GetCapturecollapse toSetTraceWriter(io.Writer);nildisables tracing, and thetraceW != nilguard keeps disabled tracing free (no perf change).SharedDomainsno longer carries trace state. Callers passos.Stderr, a file, or abytes.Bufferto capture.HexPatriciaHashed,ParallelPatriciaHashed,StreamingCommitter, and the deep-storage fold.TrieContext.tracealso becomes anio.Writer, wired from the commitment context, so branch reads and writes trace as[SDC]lines on the same writer.