Skip to content

Commit 69a814e

Browse files
execution/commitment: route trie trace through an io.Writer (#21859)
Reimplements #20316 on current `main` (that branch was ~800 commits behind and conflicting); supersedes it. Replaces the commitment trie's `trace`/`traceDomain`/`capture []string` + `fmt.Printf` tracing with a single `traceW io.Writer`. ## Changes - `Trie` interface: `SetTrace`/`SetTraceDomain`/`SetCapture`/`GetCapture` collapse to `SetTraceWriter(io.Writer)`; `nil` disables tracing, and the `traceW != nil` guard keeps disabled tracing free (no perf change). `SharedDomains` no longer carries trace state. Callers pass `os.Stderr`, a file, or a `bytes.Buffer` to capture. - Covers every trie engine: `HexPatriciaHashed`, `ParallelPatriciaHashed`, `StreamingCommitter`, and the deep-storage fold. - Parallel/streaming trace is attributable: concurrent workers share one mutex-guarded writer, so whole lines never interleave. Deep-storage fold workers prefix each line with the parent account address — grep one address to follow that account's whole storage fold out of the interleaved output; account and split workers prefix by nibble. - `TrieContext.trace` also becomes an `io.Writer`, wired from the commitment context, so branch reads and writes trace as `[SDC]` lines on the same writer. --------- Co-authored-by: Alexey Sharov <AskAlexSharov@gmail.com>
1 parent 1f4d702 commit 69a814e

26 files changed

Lines changed: 513 additions & 291 deletions

db/integrity/commitment_integrity.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"encoding/hex"
2424
"errors"
2525
"fmt"
26+
"os"
2627
"path/filepath"
2728
"runtime"
2829
"sort"
@@ -214,7 +215,11 @@ func checkCommitmentRootViaSd(ctx context.Context, tx kv.TemporalTx, f state.Vis
214215
if err != nil {
215216
return nil, err
216217
}
217-
sd.GetCommitmentCtx().SetTrace(logger.Enabled(ctx, log.LvlTrace))
218+
if logger.Enabled(ctx, log.LvlTrace) {
219+
sd.GetCommitmentCtx().SetTraceWriter(os.Stderr)
220+
} else {
221+
sd.GetCommitmentCtx().SetTraceWriter(nil)
222+
}
218223
sd.GetCommitmentCtx().SetStateReader(commitmentdb.NewFilesOnlyStateReader(tx, maxTxNum))
219224
latestTxNum, _, err := sd.SeekCommitment(ctx, tx) // seek commitment again to use the new state reader instead
220225
if err != nil {
@@ -1038,7 +1043,11 @@ func checkCommitmentHistAtBlkWithIdx(ctx context.Context, tx kv.TemporalTx, sd *
10381043
// plain state data view: as of end of the block
10391044
splitStateReader := commitmentdb.NewSplitHistoryReader(tx, commitmentAsOf, toTxNum, true /* withHistory */)
10401045
sd.GetCommitmentCtx().SetStateReader(splitStateReader)
1041-
sd.GetCommitmentCtx().SetTrace(logger.Enabled(ctx, log.LvlTrace))
1046+
if logger.Enabled(ctx, log.LvlTrace) {
1047+
sd.GetCommitmentCtx().SetTraceWriter(os.Stderr)
1048+
} else {
1049+
sd.GetCommitmentCtx().SetTraceWriter(nil)
1050+
}
10421051
latestTxNum, latestBlockNum, err := sd.SeekCommitment(ctx, tx) // seek commitment again with new history state reader
10431052
if err != nil {
10441053
return err

db/state/execctx/domain_shared.go

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,8 @@ type SharedDomains struct {
101101

102102
logger log.Logger
103103

104-
txNum uint64
105-
currentStep kv.Step
106-
trace bool //nolint
107-
commitmentCapture bool
104+
txNum uint64
105+
currentStep kv.Step
108106
// disableInlineTouchKey when true, DomainPut skips the TouchKey call.
109107
// Used when the commitment calculator goroutine owns the Updates buffer
110108
// and feeds touches via TouchPlainKeyDirect from the fan-out channel.
@@ -191,8 +189,7 @@ func NewSharedDomains(ctx context.Context, tx kv.TemporalTx, logger log.Logger,
191189
trieCfg := o.trieCfg
192190

193191
sd := &SharedDomains{
194-
logger: logger,
195-
//trace: true,
192+
logger: logger,
196193
metrics: kvmetrics.DomainMetrics{Domains: map[kv.Domain]*kvmetrics.DomainIOMetrics{}},
197194
stepSize: tx.Debug().StepSize(),
198195
}
@@ -672,14 +669,6 @@ func (sd *SharedDomains) Unwind(txNumUnwindTo uint64, changeset *[kv.DomainLen][
672669
}
673670
}
674671

675-
func (sd *SharedDomains) Trace() bool {
676-
return sd.trace
677-
}
678-
679-
func (sd *SharedDomains) CommitmentCapture() bool {
680-
return sd.commitmentCapture
681-
}
682-
683672
func (sd *SharedDomains) GetMemBatch() kv.TemporalMemBatch { return sd.mem }
684673
func (sd *SharedDomains) SetInMemHistoryReads(v bool) { sd.mem.SetInMemHistoryReads(v) }
685674
func (sd *SharedDomains) InMemHistoryReads() bool { return sd.mem.InMemHistoryReads() }
@@ -836,12 +825,6 @@ func (sd *SharedDomains) InlineTouchKeyDisabled() bool {
836825
return sd.disableInlineTouchKey
837826
}
838827

839-
func (sd *SharedDomains) SetTrace(b, capture bool) []string {
840-
sd.trace = b
841-
sd.commitmentCapture = capture
842-
return sd.sdCtx.GetCapture(true)
843-
}
844-
845828
func (sd *SharedDomains) HasPrefix(domain kv.Domain, prefix []byte, roTx kv.Tx) ([]byte, []byte, bool, error) {
846829
return sd.mem.HasPrefix(domain, prefix, roTx)
847830
}

db/test/aggregator_ext_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,6 @@ func generateSharedDomainsUpdates(t *testing.T, domains *execctx.SharedDomains,
812812
usedKeys[k] = struct{}{}
813813
}
814814
if txNum%commitEvery == 0 {
815-
// domains.SetTrace(true)
816815
stateRootHash, err := domains.ComputeCommitment(t.Context(), tx, true, txNum/commitEvery, txNum, "", nil)
817816
require.NoErrorf(t, err, "txNum=%d", txNum)
818817
_ = stateRootHash

db/test/domains_restart_test.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,6 @@ func TestCommit(t *testing.T) {
480480
require.NoError(t, err)
481481
}
482482

483-
domains.SetTrace(false, false)
484483
domainsHash, err := domains.ComputeCommitment(ctx, tx, true, blockNum, txNum, "", nil)
485484
require.NoError(t, err)
486485
err = domains.Flush(ctx, tx)
@@ -489,3 +488,35 @@ func TestCommit(t *testing.T) {
489488
require.Equal(t, common.BytesToHash(common.FromHex("0xfe81cd91357cd915cae7c02b5a4771e903c16b29dec582818076954be3741030")), common.BytesToHash(domainsHash))
490489

491490
}
491+
492+
// The commitment context routes both the trie trace and the branch read/write
493+
// trace through one io.Writer; branch writes surface as [SDC] lines.
494+
func TestCommitmentContextTraceWriter(t *testing.T) {
495+
ctx := t.Context()
496+
db, _, _ := testDbAndAggregatorv3(t, t.TempDir(), uint64(100))
497+
tx, err := db.BeginTemporalRw(ctx)
498+
require.NoError(t, err)
499+
defer tx.Rollback()
500+
501+
domains, err := execctx.NewSharedDomains(ctx, tx, log.New())
502+
require.NoError(t, err)
503+
defer domains.Close()
504+
505+
acc := accounts.Account{Balance: u256.U64(7), CodeHash: accounts.EmptyCodeHash, Incarnation: 1}
506+
val := accounts.SerialiseV3(&acc)
507+
addr := common.Hex2Bytes("8e5476fc5990638a4fb0b5fd3f61bb4b5c5f395e")
508+
for i := 1; i < 12; i++ { // diverging first nibbles force a branch node
509+
addr[0] = byte(i * 16)
510+
require.NoError(t, domains.DomainPut(kv.AccountsDomain, tx, common.Copy(addr), val, 0, nil))
511+
}
512+
513+
var trace strings.Builder
514+
domains.GetCommitmentCtx().SetTraceWriter(&trace)
515+
_, err = domains.ComputeCommitment(ctx, tx, true, 0, 0, "", nil)
516+
require.NoError(t, err)
517+
domains.GetCommitmentCtx().SetTraceWriter(nil)
518+
519+
out := trace.String()
520+
require.Contains(t, out, "[SDC] PutBranch", "branch writes must trace through the writer")
521+
require.Contains(t, out, "[proc]", "trie must trace processed keys through the same writer")
522+
}

execution/commitment/backtester/backtester.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,11 @@ func (bt Backtester) backtestBlock(ctx context.Context, tx kv.TemporalTx, block
217217
// - commitment data as-of the beginning of the block
218218
// - account/storage/code data as-of the end of the block
219219
sd.GetCommitmentCtx().SetStateReader(commitmentdb.NewSplitHistoryReader(tx, fromTxNum, toTxNum /* withHistory */, false))
220-
sd.GetCommitmentCtx().SetTrace(bt.logger.Enabled(ctx, log.LvlTrace))
220+
if bt.logger.Enabled(ctx, log.LvlTrace) {
221+
sd.GetCommitmentCtx().SetTraceWriter(os.Stderr)
222+
} else {
223+
sd.GetCommitmentCtx().SetTraceWriter(nil)
224+
}
221225
latestTxNum, latestBlockNum, err := sd.SeekCommitment(ctx, tx)
222226
if err != nil {
223227
return err

execution/commitment/commitment.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"encoding/binary"
2323
"errors"
2424
"fmt"
25+
"io"
2526
"math/bits"
2627
"reflect"
2728
"slices"
@@ -93,12 +94,8 @@ type Trie interface {
9394
// RootHash produces root hash of the trie
9495
RootHash() (hash []byte, err error)
9596

96-
// Makes trie more verbose
97-
SetTrace(bool)
98-
// Trace domain writes only (no filding etc)
99-
SetTraceDomain(bool)
100-
SetCapture(capture []string)
101-
GetCapture(truncate bool) []string
97+
// SetTraceWriter sets the trace writer; nil disables tracing
98+
SetTraceWriter(io.Writer)
10299
EnableCsvMetrics(filePathPrefix string)
103100

104101
// Variant returns commitment trie variant

execution/commitment/commitment_bench_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func BenchmarkHexPatriciaHashedFold(b *testing.B) {
3131
ctx := context.Background()
3232
ms := NewMockState(b)
3333
hph := NewHexPatriciaHashed(1, ms, DefaultTrieConfig())
34-
hph.SetTrace(false)
34+
hph.SetTraceWriter(nil)
3535

3636
// Build a trie with accounts and storage to exercise all fold paths
3737
plainKeys, updates := NewUpdateBuilder().

execution/commitment/commitmentdb/commitment_context.go

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"encoding/hex"
88
"errors"
99
"fmt"
10+
"io"
1011
"runtime"
1112
"sync"
1213
"sync/atomic"
@@ -43,9 +44,6 @@ type sd interface {
4344
// read), tagged with source.
4445
MergeMetrics(source kvmetrics.Source, wm *kvmetrics.DomainMetrics)
4546
StepSize() uint64
46-
Trace() bool
47-
CommitmentCapture() bool
48-
4947
// ProbeReadLayers samples sd.mem, parent.mem and tx-direct (MDBX) for one
5048
// key — BranchCache divergence-detection probe. Read-only.
5149
ProbeReadLayers(domain kv.Domain, tx kv.TemporalTx, key []byte) (mem, parentMem, mdbx []byte, memOk, parentOk bool)
@@ -64,7 +62,7 @@ type SharedDomainsCommitmentContext struct {
6462
patriciaTrie commitment.Trie
6563
variant commitment.TrieVariant // selected trie engine, for the [commitment] log (updates.Mode() is ModeParallel for both parallel and streaming)
6664
justRestored atomic.Bool // set to true when commitment trie was just restored from snapshot
67-
trace bool
65+
traceW io.Writer
6866
stateReader StateReader
6967
paraTrieDB kv.TemporalRoDB // DB used for para trie and/or parallel trie warmup
7068
// warmupBase holds the construction-time portion of the per-call WarmupConfig.
@@ -170,14 +168,11 @@ func (sdc *SharedDomainsCommitmentContext) SetCustomHistoryStateReader(stateRead
170168
sdc.SetStateReader(stateReader)
171169
}
172170

173-
func (sdc *SharedDomainsCommitmentContext) SetTrace(trace bool) {
174-
sdc.trace = trace
175-
sdc.patriciaTrie.SetTrace(trace)
176-
}
177-
178-
// SetCapture enables/disables trie operation capture for diagnosis.
179-
func (sdc *SharedDomainsCommitmentContext) SetCapture(capture []string) {
180-
sdc.patriciaTrie.SetCapture(capture)
171+
func (sdc *SharedDomainsCommitmentContext) SetTraceWriter(w io.Writer) {
172+
// Wrap once so the main and per-worker TrieContexts share one mutex-guarded
173+
// writer: concurrent workers trace branch reads/writes without racing.
174+
sdc.traceW = commitment.NewSyncWriter(w)
175+
sdc.patriciaTrie.SetTraceWriter(sdc.traceW)
181176
}
182177

183178
// GetUpdates returns the current updates buffer. Used by the commitment
@@ -225,6 +220,7 @@ func (sdc *SharedDomainsCommitmentContext) trieContext(tx kv.TemporalTx, blockNu
225220
stepSize: sdc.sharedDomains.StepSize(),
226221
txNum: txNum,
227222
blockNum: blockNum,
223+
traceW: sdc.traceW,
228224
}
229225
if sdc.stateReader != nil {
230226
mainTtx.stateReader = sdc.stateReader.CloneForWorker(readCtx, tx)
@@ -246,10 +242,6 @@ func (sdc *SharedDomainsCommitmentContext) Reset() {
246242
}
247243
}
248244

249-
func (sdc *SharedDomainsCommitmentContext) GetCapture(truncate bool) []string {
250-
return sdc.patriciaTrie.GetCapture(truncate)
251-
}
252-
253245
func (sdc *SharedDomainsCommitmentContext) ClearRam() {
254246
sdc.updates.Reset()
255247
sdc.Reset()
@@ -421,21 +413,17 @@ func (sdc *SharedDomainsCommitmentContext) ComputeCommitment(ctx context.Context
421413
}
422414
log.Debug("[commitment] processed", "block", blockNum, "txNum", txNum, "keys", common.PrettyCounter(updateCount), "keys/s", common.PrettyCounter(keysPerSec), "mode", sdc.variant, "bufmode", sdc.updates.Mode(), "spent", took, "rootHash", hex.EncodeToString(rootHash))
423415
}()
416+
// Re-apply the trace writer before any trie operations (including the early-return
417+
// RootHash below); GenerateWitness clears it, so it must be restored on each call.
418+
sdc.patriciaTrie.SetTraceWriter(sdc.traceW)
419+
424420
if updateCount == 0 {
425421
rootHash, err = sdc.patriciaTrie.RootHash()
426422
return rootHash, err
427423
}
428424

429425
// data accessing functions should be set when domain is opened/shared context updated
430426

431-
sdc.patriciaTrie.SetTrace(sdc.trace)
432-
sdc.patriciaTrie.SetTraceDomain(sdc.sharedDomains.Trace())
433-
if sdc.sharedDomains.CommitmentCapture() {
434-
if sdc.patriciaTrie.GetCapture(false) == nil {
435-
sdc.patriciaTrie.SetCapture([]string{})
436-
}
437-
}
438-
439427
// Per-ComputeCommitment metrics accumulator for the main (root-fold) reads.
440428
// The fold is single-goroutine, so this lock-free accumulator is owned
441429
// exclusively here; merged into the shared metrics when commitment finishes.
@@ -626,6 +614,7 @@ func (sdc *SharedDomainsCommitmentContext) trieContextFactory(ctx context.Contex
626614
putter: sdc.sharedDomains.AsPutDel(roTx),
627615
stepSize: stepSize,
628616
txNum: txNum,
617+
traceW: sdc.traceW,
629618
}
630619
if sdc.stateReader != nil {
631620
warmupCtx.stateReader = sdc.stateReader.CloneForWorker(workerCtx, roTx)
@@ -672,6 +661,7 @@ func (sdc *SharedDomainsCommitmentContext) concurrentTrieContextFactory(ctx cont
672661
stepSize: stepSize,
673662
txNum: txNum,
674663
localCollector: collector,
664+
traceW: sdc.traceW,
675665
}
676666
if sdc.stateReader != nil {
677667
warmupCtx.stateReader = sdc.stateReader.CloneForWorker(workerCtx, roTx)
@@ -887,7 +877,7 @@ func (sdc *SharedDomainsCommitmentContext) restorePatriciaState(value []byte) (u
887877
return 0, 0, fmt.Errorf("failed restore state : %w", err)
888878
}
889879
sdc.justRestored.Store(true) // to prevent double reset
890-
if sdc.trace {
880+
if sdc.traceW != nil {
891881
rootHash, err := hext.RootHash()
892882
if err != nil {
893883
return 0, 0, fmt.Errorf("failed to get root hash after state restore: %w", err)
@@ -904,7 +894,7 @@ type TrieContext struct {
904894
blockNum uint64
905895

906896
stepSize uint64
907-
trace bool
897+
traceW io.Writer // nil = disabled; traces branch reads/writes (see [SDC] lines)
908898
stateReader StateReader
909899
localCollector *etl.Collector // per-goroutine collector for concurrent PutBranch
910900
}
@@ -925,15 +915,18 @@ func (sdc *TrieContext) Branch(pref []byte) ([]byte, kv.Step, error) {
925915
// underlying state cache / getter aliases shared storage that another goroutine
926916
// (concurrent commitment workers) can recycle. Own the bytes at the trie-context
927917
// boundary so all downstream consumers are safe.
918+
if sdc.traceW != nil {
919+
fmt.Fprintf(sdc.traceW, "[SDC] Branch read %x => %x\n", pref, enc)
920+
}
928921
return common.Copy(enc), step, nil
929922
}
930923

931924
func (sdc *TrieContext) PutBranch(prefix []byte, data []byte, prevData []byte) error {
932925
if sdc.stateReader.WithHistory() { // do not store branches if explicitly operate on history
933926
return nil
934927
}
935-
if sdc.trace {
936-
fmt.Printf("[SDC] PutBranch: %x: %x\n", prefix, data)
928+
if sdc.traceW != nil {
929+
fmt.Fprintf(sdc.traceW, "[SDC] PutBranch %x: %x\n", prefix, data)
937930
}
938931
if sdc.localCollector != nil {
939932
return sdc.localCollector.Collect(prefix, data)

0 commit comments

Comments
 (0)