feat: add StateComposition plugin — real-time state metrics with incremental tracking

[AnkushinDaniil]

Summary

Nethermind plugin that collects real-time state composition metrics — account/contract counts, trie node distribution, byte sizes, depth distribution, branch occupancy, code size tracking, and per-contract storage inspection. Designed for bloatnet benchmarking and gas limit research.

Single operating mode: bootstrap scan → per-block incremental diffs → shutdown snapshot → warm restart from snapshot.

• 35 production files (4 600 LOC), 17 test files (3 400 LOC), 81 tests
• 3 RPC endpoints: statecomp_get, statecomp_cancelScan, statecomp_inspectContract
• 30+ Prometheus metrics (gauges + counters) with labeled per-depth breakdowns
• Geth trie/inspect.go parity — JSON field names match Geth's Short/Full/Value convention

Architecture

Component Map

graph TB
    subgraph Plugin Layer
        Plugin["StateCompositionPlugin<br/><i>INethermindPlugin</i>"]
        Module["StateCompositionModule<br/><i>Autofac IModule</i>"]
        Config["IStateCompositionConfig"]
    end

    subgraph Service Layer
        Service["StateCompositionService<br/><i>IStoppableService</i>"]
        Holder["StateCompositionStateHolder<br/>thread-safe state container"]
    end

    subgraph Scan Path
        Visitor["StateCompositionVisitor<br/><i>ITreeVisitor · parallel</i>"]
        Counters["ThreadLocal&lt;VisitorCounters&gt;<br/>lock-free per-thread"]
        Depth["DepthCounter ×16"]
        TopN["TopNTracker"]
        LevelStats["LevelStatsBuilder"]
        SingleContract["SingleContractVisitor"]
    end

    subgraph Diff Path
        DiffWalker["TrieDiffWalker<br/>5 partial classes"]
        TrieDiff["TrieDiff<br/>exact add/remove delta"]
        DepthDelta["CumulativeDepthStats<br/>depth distribution delta"]
    end

    subgraph Persistence
        SnapshotStore["StateCompositionSnapshotStore<br/><i>RocksDB</i>"]
        Decoder["StateCompositionSnapshotDecoder<br/><i>RLP codec</i>"]
    end

    subgraph RPC
        RpcModule["StateCompositionRpcModule"]
        RpcGet["statecomp_get"]
        RpcCancel["statecomp_cancelScan"]
        RpcInspect["statecomp_inspectContract"]
    end

    subgraph Observability
        Metrics["Metrics<br/>30+ Prometheus gauges/counters"]
    end

    Plugin --> Module
    Plugin --> Config
    Module --> Service
    Module --> Holder
    Module --> SnapshotStore
    Module --> RpcModule

    Service --> Visitor
    Service --> DiffWalker
    Service --> SingleContract
    Service --> Holder
    Service --> SnapshotStore

    Visitor --> Counters --> Depth
    Counters --> TopN
    Counters --> LevelStats

    DiffWalker --> TrieDiff
    DiffWalker --> DepthDelta

    Holder --> Metrics

    RpcModule --> Holder
    RpcModule --> Service
    RpcModule --> RpcGet
    RpcModule --> RpcCancel
    RpcModule --> RpcInspect

    SnapshotStore --> Decoder

Loading

Operating Mode Lifecycle

stateDiagram-v2
    [*] --> PluginInit: Node starts

    state PluginInit {
        [*] --> CheckSnapshot
        CheckSnapshot --> BootstrapScan: No snapshot / stale
        CheckSnapshot --> RestoreSnapshot: Valid snapshot
        RestoreSnapshot --> IncrementalReady
        BootstrapScan --> IncrementalReady: Scan completes
    }

    IncrementalReady --> IncrementalDiff: NewHeadBlock event

    state IncrementalDiff {
        [*] --> ComputeDiff: TrieDiffWalker
        ComputeDiff --> ApplyDiff: StateHolder.ApplyIncrementalDiffAndUpdate
        ApplyDiff --> UpdateMetrics: Prometheus gauges
    }

    IncrementalDiff --> IncrementalReady: Next block

    IncrementalDiff --> Recovery: MissingTrieNodeException
    state Recovery {
        [*] --> InvalidateBaseline
        InvalidateBaseline --> AutoRescan: ScheduleBaselineRescan
        AutoRescan --> [*]: Re-seeds incremental state
    }
    Recovery --> IncrementalReady

    IncrementalReady --> Shutdown: IStoppableService.StopAsync
    state Shutdown {
        [*] --> Unsubscribe: NewHeadBlock -=
        Unsubscribe --> CancelScan
        CancelScan --> DrainLocks: _scanLock → _diffLock
        DrainLocks --> FlushSnapshot: Force-write to RocksDB
    }
    Shutdown --> [*]

Loading

Incremental Diff Data Flow

sequenceDiagram
    participant BT as BlockTree
    participant Svc as Service
    participant DW as TrieDiffWalker
    participant SH as StateHolder
    participant M as Metrics

    BT->>Svc: NewHeadBlock(block)
    Note over Svc: Skip if no baseline<br/>or same root

    Svc->>Svc: Task.Run(RunIncrementalDiff)
    Svc->>Svc: _diffLock.TryEnter() (coalescing)

    Svc->>DW: ComputeDiff(prevRoot, newRoot, resolver)
    Note over DW: Walk both tries in parallel<br/>Branch/Extension/Leaf diffing<br/>+ storage subtree diffs

    DW-->>Svc: TrieDiff (nodes ± bytes ± slots ±<br/>CodeHashChanges, SlotCountChanges,<br/>DepthDelta)

    Svc->>SH: ApplyIncrementalDiffAndUpdate(diff)
    Note over SH: lock(_lock)<br/>1. CumulativeTrieStats.ApplyDiff<br/>2. Code hash refcount tracker<br/>3. Slot histogram re-bucketing<br/>4. Depth distribution merge

    SH-->>Svc: Updated CumulativeTrieStats

    Svc->>M: UpdateFromCumulativeStats
    Svc->>M: UpdateDepthDistribution

Loading

Full Scan Data Flow

sequenceDiagram
    participant P as Plugin/Recovery
    participant Svc as Service
    participant V as Visitor (parallel)
    participant TL as ThreadLocal Counters
    participant SH as StateHolder
    participant SS as SnapshotStore

    P->>Svc: AnalyzeAsync(header, ct)
    Note over Svc: _scanLock (fail-fast)

    Svc->>V: RunTreeVisitor(visitor, header, options)
    Note over V: Parallel walk with<br/>MaxDegreeOfParallelism threads

    loop Each trie node
        V->>TL: .Value! (thread-local counters)
        Note over TL: Count nodes, bytes, depth,<br/>contracts, slots, code sizes
    end

    V->>V: GetStats() — merge all ThreadLocal values
    V-->>Svc: StateCompositionStats + TrieDepthDistribution

    Svc->>SH: SetBaseline + MarkScanCompleted
    Svc->>SH: InitializeIncremental(CumulativeTrieStats,<br/>tracker maps)

    Svc->>SS: WriteSnapshotForHead (under _diffLock)

Loading

Data Model

Type	Role
StateCompositionStats	Raw output from full scan (Geth vocabulary)
CumulativeTrieStats	Immutable record updated per diff via ApplyDiff (MPT vocabulary)
TrieDiff	Exact add/remove delta between two state roots
CumulativeDepthStats	Per-depth node counts with AddInPlace merge
StateCompositionReport	RPC response from statecomp_get
StateCompositionSnapshot	RLP-persisted state for warm restart
TopContractEntry	Per-contract storage trie detail (Geth jsonStorageSnapshot parity)
TrieLevelStat	Per-depth level stats (Geth jsonLevel parity)

Node Naming Convention

The plugin uses two node vocabularies by design:

Layer	Vocabulary	Why
JSON-facing types (TrieLevelStat, TopContractEntry, StateCompositionStats)	Geth (FullNode=branch, ShortNode=extension+leaf, ValueNode=leaf)	Matches go-ethereum/trie/inspect.go:jsonLevel for cross-client tooling
Internal types (CumulativeTrieStats, TrieDiff, TrieDiffWalker)	Standard MPT (Branch, Extension, Leaf)	Clearer for diff/cumulative arithmetic

CumulativeTrieStats.FromScanStats() is the boundary: Branches = FullNodes, Extensions = ShortNodes − ValueNodes, Leaves = ValueNodes.

Metrics (Prometheus)

Metric	Type	Description
statecomp_accounts_total	Gauge	Total accounts
statecomp_contracts_total	Gauge	Total contracts
statecomp_storage_slots_total	Gauge	Total storage slots
statecomp_account_trie_{branches,extensions,leaves,bytes}	Gauge	Account trie structure
statecomp_storage_trie_{branches,extensions,leaves,bytes}	Gauge	Storage trie structure
statecomp_contracts_with_storage	Gauge	Contracts with non-empty storage
statecomp_empty_accounts	Gauge	EOAs without storage
statecomp_code_bytes_total	Gauge	Deduplicated bytecode size
statecomp_trie_depth_nodes{trie,depth,kind}	Labeled Gauge	Per-depth node counts
statecomp_trie_depth_bytes{trie,depth}	Labeled Gauge	Per-depth byte sizes
statecomp_account_branch_occupancy{children}	Labeled Gauge	Branch child distribution
statecomp_slot_count_histogram{bucket}	Labeled Gauge	Log2 slot-count buckets
statecomp_incremental_block	Gauge	Latest diff block
statecomp_diffs_since_baseline	Gauge	Diffs since last scan
statecomp_scan_duration_seconds	Gauge	Last scan duration
statecomp_scans_completed	Counter	Total scans
statecomp_diffs_applied	Counter	Total diffs
statecomp_diff_errors	Counter	Diff failures
statecomp_baseline_invalidations	Counter	Stale-root recoveries

Configuration

{
  "StateComposition": {
    "Enabled": true,
    "ScanParallelism": 4,
    "ScanMemoryBudgetBytes": 1000000000,
    "TopNContracts": 20,
    "ExcludeStorage": false,
    "PersistSnapshots": true,
    "TrackDepthIncrementally": true
  }
}

Types of changes

• New feature (a non-breaking change that adds functionality)

Testing

• 81 unit tests covering:
  • Visitor: parallel scan, ThreadLocal aggregation, depth distribution, code size dedup, slot histogram, Geth convention
  • TrieDiffWalker: branch/extension/leaf diffing, storage subtree diffs, slot count changes, code hash changes, depth tracking, mismatched node types
  • Service: scan lifecycle, scan cancellation, scan-inspect concurrency, baseline invalidation + auto-recovery, incremental diff application
  • StateHolder: code hash refcount tracker (add/share/drop/swap), slot histogram bucketing (add/grow/empty/CoW)
  • Snapshots: RLP round-trip with tracker maps, depth stats, schema versioning
  • Metrics: labeled gauge publishing, code bytes + slot histogram via Prometheus
  • RPC: statecomp_get response shape, inspectContract lifecycle
  • CumulativeTrieStats: freeze semantics for code bytes and histogram across diffs

[LukaszRozmej]

@claude review

[claude]

Claude finished @LukaszRozmej's task in 3m 47s —— View job

---

Code Review — StateComposition Plugin

• Read repo rules (coding-style, di-patterns, robustness, test-infrastructure)
• Review plugin/module/config files
• Review service layer (StateCompositionService, StateHolder)
• Review diff walker (TrieDiffWalker partial classes)
• Review visitor components
• Review RPC module
• Review snapshots/persistence
• Review metrics
• Review tests
• Post final review

---

This is an ambitious plugin with clear engineering investment — the architecture, docstrings, and test coverage are above average. Below is a prioritized list of issues found.

---

Critical

1. IStoppableService never registered — graceful shutdown flush is dead code

StateCompositionModule.cs:

builder.RegisterType<StateCompositionService>()
    .AsSelf()
    .SingleInstance()
    .AutoActivate();

There is no .As<IStoppableService>(). Nethermind's IServiceStopper discovers stoppable services via IEnumerable<IStoppableService>, which Autofac resolves from registrations. Because the service is only registered AsSelf(), StopAsync() — whose entire purpose is the "force-flush to RocksDB on graceful shutdown" — will never be called. The PersistSnapshots feature is silently broken at every clean shutdown.

Fix: add .As<IStoppableService>() to the registration. Fix this →

---

Significant

2. catch (Exception ex) when (logger.IsError) — silently swallows exceptions when log level is off

StateCompositionPlugin.cs:135 and StateCompositionService.Incremental.cs:128:

catch (Exception ex) when (logger.IsError)   // ← BUG
{
    logger.Error("...", ex);
}

In C#, an exception filter is evaluated before the stack is unwound. If logger.IsError is false, the filter evaluates to false, the catch block is skipped entirely, and the exception propagates to the Task.Run continuation as an unobserved task exception. In both callers the task result is discarded (_ = Task.Run(...)), so the exception is silently lost. The scan can fail invisibly with no log output and no recovery.

Correct pattern:

catch (Exception ex)
{
    if (_logger.IsError)
        _logger.Error("StateComposition: bootstrap scan failed", ex);
}

Fix this →

---

3. SnapshotBlocksToKeep is documented but never used

StateCompositionConfig.cs declares SnapshotBlocksToKeep { get; set; } = 10_000 with description "Number of historical snapshot blocks to keep". But StateCompositionSnapshotStore.WriteSnapshot removes the previous snapshot unconditionally, keeping at most one snapshot at all times, and PurgeOldEntries removes all entries except the latest. The config option is dead. Either implement the eviction logic or remove the option to avoid operator confusion.

---

4. statecomp_inspectContract passes CancellationToken.None — no timeout

StateCompositionRpcModule.cs:46:

Result<TopContractEntry?> result = await service.InspectContractAsync(
    address, head.Header, CancellationToken.None).ConfigureAwait(false);

A contract with a very large storage trie can block this RPC call indefinitely. The RPC framework's per-request timeout (if any) won't propagate here. At minimum the request's cancellation token should be threaded through; ideally a bounded timeout should be applied (e.g. cts.CancelAfter(TimeSpan.FromSeconds(30))).

---

5. TrieDiff.DepthDelta is a shared mutable reference, not a copy

TrieDiffWalker.cs:77:

DepthDelta: _depthDelta,   // reference to mutable field

The TrieDiff record holds a reference to _depthDelta, which is reset by the next ComputeDiff call (_depthDelta.Reset()). This is currently safe because _diffLock serializes access, but it violates caller expectations (a returned value shouldn't mutate under you) and is a footgun for anyone adding logging, metrics, or diagnostic code that captures the diff past the lock scope. The DepthDelta should be copied before being placed into the returned struct.

---

Minor / Nits

6. DI anti-pattern in tests — manual construction instead of production modules

StateCompositionServiceTests.cs:

StateCompositionService service = new(
    stateReader, Substitute.For<IWorldStateManager>(), ...);

Per test-infrastructure.md, tests should use production DI modules with overrides rather than manually constructing components. This makes the tests fragile when the constructor adds a new dependency. Consider using PseudoNethermindModule + TestEnvironmentModule for integration-style service tests.

---

7. LatestKey sentinel can collide with block long.MaxValue

StateCompositionSnapshotStore.cs:29:

private static readonly byte[] LatestKey = [0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF];

This value equals long.MaxValue when read as big-endian Int64. If a snapshot is ever written at block 9,223,372,036,854,775,807, the sentinel will be overwritten. Extremely unlikely, but an off-by-one (e.g. 0xFF…FE) or a different encoding space would eliminate the ambiguity entirely.

---

8. Inline branch child path not advanced before DiffNodes

TrieDiffWalker.Branches.cs:73-81: when both branch children are inline (hash is null, IsChildNull is false), GetChildWithChildPath advances local path copies (oldChildPath/newChildPath) but then DiffNodes is called with the original ref path. For inline extension children of a branch this means path does not include the inline node's own key when recursing, which could produce incorrect path reconstructions in leaf handling. This path is rare in Ethereum's MPT but worth a targeted test.

---

9. Reviewer's open comment (already addressed)

The comment "You can inject the IStateCompositionConfig straight at the constructor" was addressed — the current code already does this (StateCompositionPlugin(IStateCompositionConfig config)). No action needed here.

---

Summary

The most impactful fixes required before merging are #1 (IStoppableService registration) and #2 (exception filter anti-pattern). Both are behavioral bugs that silently break core promises of the plugin (crash-safety snapshots and error observability). Issues #3–#5 are design-level concerns that should be addressed or explicitly called out in follow-up issues.

[AnkushinDaniil]

@claude review it again and check if the issues from the previous review have been resolved

[claude]

Claude finished @AnkushinDaniil's task in 1m 58s —— View job

---

Re-review: Checking Previous Issues

• Read relevant files
• Check issue Feature/jsonrpc #1: IStoppableService registration in StateCompositionModule.cs
• Check issue Feature/networking #2: catch (Exception ex) when (logger.IsError) anti-pattern
• Check issue Netcore #3: SnapshotBlocksToKeep unused config option
• Check issue Hive #4: CancellationToken.None in statecomp_inspectContract
• Check issue Patricia alt 2 #5: TrieDiff.DepthDelta shared mutable reference
• Check issue Perf0602 #6: DI anti-pattern in tests
• Check issue Rlpstreams #7: LatestKey sentinel collision
• Check issue Vmspans #8: Inline branch child path not advanced
• Post final review

---

Results

All critical and significant issues from the previous review have been resolved. Two minor items remain, discussed below.

---

✅ Issue #1 — IStoppableService registration (Critical) — FIXED

StateCompositionModule.cs:35 now has:

builder.RegisterType<StateCompositionService>()
    .AsSelf()
    .As<IStoppableService>()   // ← added
    .SingleInstance()
    .AutoActivate();

The comment explaining the intent is a nice addition.

---

✅ Issue #2 — catch when (logger.IsError) anti-pattern (Significant) — FIXED

Both StateCompositionPlugin.cs:133-139 and StateCompositionService.Incremental.cs:127-134 now use the correct pattern:

catch (Exception ex)
{
    if (_logger.IsError)
        _logger.Error("...", ex);
}

The explanatory comment ("Guard the log call itself, not the catch...") is helpful for reviewers.

---

✅ Issue #3 — SnapshotBlocksToKeep dead config (Significant) — FIXED

The option has been removed entirely from IStateCompositionConfig. InspectContractTimeoutSeconds was added in its place (fixing issue #4 at the same time), which is the better design.

---

✅ Issue #4 — CancellationToken.None in statecomp_inspectContract (Significant) — FIXED

StateCompositionRpcModule.cs:44-63 now creates a CancellationTokenSource with the configured timeout and propagates it through InspectContractAsync. Timeout and non-timeout cancellation produce distinct RPC error messages, which is a good UX choice.

---

✅ Issue #5 — TrieDiff.DepthDelta mutable shared reference (Significant) — FIXED

TrieDiffWalker.cs:81:

DepthDelta: _depthDelta.CloneAsDelta(),

The TrieDiff now holds an independent copy, safe to read after the next ComputeDiff reset.

---

⚠️ Issue #6 — DI anti-pattern in tests (Minor) — Not addressed

StateCompositionServiceTests.cs still constructs StateCompositionService directly via new(...). This was flagged as a minor issue and is acceptable for now, but means any new constructor parameter will silently break all test fixtures at compile time rather than being automatically wired. Worth tracking as a follow-up.

---

⚠️ Issue #7 — LatestKey sentinel (Minor) — Acknowledged, not changed

The value [0xFF×8] (= long.MaxValue big-endian) is unchanged, but a comment has been added to StateCompositionSnapshotStore.cs:31:

"long.MaxValue big-endian: theoretical collision with a real block-number key lands ~3.5 trillion years out at 12s blocks, so no schema migration needed."

The reasoning is sound. Acceptable as-is.

---

⚠️ Issue #8 — Inline branch child path not advanced (Minor) — Not fixed

TrieDiffWalker.Branches.cs:72-81 still calls DiffNodes with the original ref path instead of the advanced path copies:

TreePath oldChildPath = path;
TrieNode? oldChild = oldBranch.GetChildWithChildPath(resolver, ref oldChildPath, i);
TreePath newChildPath = path;
TrieNode? newChild = newBranch.GetChildWithChildPath(resolver, ref newChildPath, i);

// path is not advanced through the inline node's key
DiffNodes(oldChild, newChild, ref path, resolver, isStorage, childDepth);

If the inline child is an extension node, its key nibbles should be appended to path before recursing — oldChildPath and newChildPath have these nibbles but path does not. This can produce incorrect path reconstructions in leaf handling for that rare case. The fix would be to pick one of the two advanced paths (e.g., oldChildPath, since both sides advanced through the same inline slot) and pass it to DiffNodes.

Fix this →

---

Summary

All 5 critical/significant issues are resolved. The two remaining minor items (#6, #8) are pre-existing and low-risk — #8 is technically still present but only affects the rare inline-extension-child edge case in Ethereum's MPT. The PR is in good shape to merge once the team is comfortable with those minor outstanding points.

[svlachakis]

@claude are you sure there are no race conditions at StateCompositionService?

[claude]

Claude finished @svlachakis's task in 8m 9s —— View job

---

Race Condition Analysis — StateCompositionService

• Read repo rules (robustness, coding-style)
• Read StateCompositionService.cs (main class)
• Read StateCompositionService.Incremental.cs
• Read StateCompositionStateHolder.cs
• Read TrieDiffWalker.cs and partial classes
• Analyze locking model and concurrent access patterns
• Post findings

---

The locking model is mostly sound, but there are three genuine issues and one latent footgun. Here's the full analysis.

---

The overall concurrency model

Four synchronization primitives:

• _scanLock (SemaphoreSlim(1,1)) — serializes full scans; async-awaitable
• _diffLock (Lock) — coalesces incremental diffs; TryEnter() drops duplicates
• _inspectLock (SemaphoreSlim(1,1)) — serializes InspectContractAsync
• _stateHolder._lock (Lock) — guards all holder fields

Lock ordering: _scanLock → _diffLock → _stateHolder._lock. Held consistently — no deadlock risk.

---

Issue 1 (Medium) — BuildReport() observes a torn state during scan completion

PublishScanResults performs a three-phase, non-atomic state transition:

_stateHolder.SetBaseline(stats, dist);        // phase 1: _currentDistribution = NEW
_stateHolder.MarkScanCompleted(...);           // phase 2: _lastScanMetadata = NEW
// ← no _diffLock held here
lock (_diffLock)
{
    _stateHolder.InitializeIncremental(...);   // phase 3: _incrementalStats = NEW
}

Between phases 2 and 3, a concurrent RPC call to statecomp_get → BuildReport() returns:

TrieDistribution  = NEW scan distribution  (from SetBaseline)
LastScanMetadata  = NEW scan metadata      (from MarkScanCompleted)
TrieStats         = OLD incremental stats  (not yet reset)
BlockNumber       = OLD incremental block

A caller during this window gets a response where the trie distribution claims to belong to the new scan but TrieStats still reflects the pre-scan cumulative state. This is visible to Prometheus/Grafana dashboards during a rescan (which can take 30+ minutes on large state).

Fix: move SetBaseline and MarkScanCompleted inside the existing lock (_diffLock) block before InitializeIncremental, so all three phases are atomic from BuildReport()'s perspective.

Fix this →

---

Issue 2 (Low) — GetSnapshot() is a data race on VisitorCounters fields

StartProgressLogging fires a timer task every 8 seconds that calls visitor.GetSnapshot():

// timer task (separate thread pool thread):
ScanSnapshot snap = visitor.GetSnapshot();
// GetSnapshot iterates _localCounters.Values and reads:
//   c.AccountsTotal, c.ContractsTotal, c.StorageSlotsTotal, ...

Meanwhile, scan threads write to those same VisitorCounters fields (c.AccountsTotal++, etc.) with no synchronization. These are plain long fields — not volatile, no Interlocked. By the C# memory model this is a data race; a reader can observe stale values or a partially-written 64-bit value on 32-bit runtimes.

In practice this is benign: the values are used only for progress logging (approximate counts, not correctness-critical), and on x64 aligned 64-bit reads are atomic in hardware. But it is technically undefined behavior under the C# memory model and would be flagged by a race detector. Adding volatile to the long fields in VisitorCounters that GetSnapshot() reads, or marking GetSnapshot() with a comment acknowledging the relaxed-consistency intent, would close this.

---

Issue 3 (Low) — CurrentDepthStats exposes a mutable reference without enforcing _diffLock

// StateCompositionStateHolder:
public CumulativeDepthStats CurrentDepthStats => _currentDepthStats; // raw mutable ref

_currentDepthStats is mutated by ApplyIncrementalDiffAndUpdate and InitializeIncremental, both of which run under _diffLock. The two callers of CurrentDepthStats — in RunIncrementalDiff and PublishScanResults — both hold _diffLock, so the current usage is safe.

However, the property is public and returns the live reference without any lock. There is nothing preventing a new caller (diagnostic endpoint, test, snapshot path) from capturing the reference and reading it outside _diffLock. That caller would race with the mutation path.

The comment documents the "single-writer invariant" and "readers iterate immediately after receiving the ref" but doesn't state the required lock. At minimum document the required guard; ideally expose a CopyDepthStats() method that returns a snapshot under _lock and make CurrentDepthStats internal.

---

Issue 4 (Low) — HasIncrementalBaseline and LastProcessedStateRoot are read under separate lock acquisitions in StopAsync

After InvalidateBaseline() the holder is in state {_hasIncrementalBaseline=true, _lastProcessedStateRoot=Hash256.Zero}. In StopAsync:

lock (_diffLock)
{
    if (!_stateHolder.HasIncrementalBaseline) return;        // acquires _stateHolder._lock, sees true
    Hash256 stateRoot = _stateHolder.LastProcessedStateRoot; // second separate _stateHolder._lock acquire
    if (stateRoot == Hash256.Zero) return;                   // catches the gap
    ...
}

The two reads are individually consistent but not atomic with each other. Currently handled correctly — the Hash256.Zero guard on the second check catches the post-invalidation state. But it's fragile: if someone adds code between the two checks that assumes a valid baseline exists, they will be surprised. A single lock (_stateHolder._lock) acquiring both values together would make the invariant explicit.

---

What IS correctly handled

• _diffLock coalescing: TryEnter() in RunIncrementalDiff correctly drops concurrent blocks — only one diff runs at a time.
• Scan + diff interaction: PublishScanResults holds _diffLock while calling InitializeIncremental, so a running diff cannot clobber the fresh baseline.
• TrieDiff.DepthDelta: fixed in round 3 — CloneAsDelta() prevents cross-diff mutation.
• BuildSnapshot with live refs: called under _diffLock, so ApplyIncrementalDiffAndUpdate (the only other writer to the tracker dicts) cannot run concurrently — safe.
• _currentScanCts TOCTOU: ObjectDisposedException in CancelScan() handles the window where the field is non-null but the CTS is already disposed.
• _shuttingDown flag: set before unsubscribing; all queued diff tasks that pass the volatile-read check will TryEnter and miss the lock held by StopAsync's flush — no diff can run after the snapshot write.
• Lock ordering _scanLock → _diffLock: consistent throughout, no deadlock risk.

---

Summary

Issue	Severity	Status
BuildReport() torn state during scan completion	Medium	⚠️ Unresolved
GetSnapshot() progress logger data race	Low	⚠️ Unresolved (benign on x64)
CurrentDepthStats mutable ref without lock documentation	Low	⚠️ Unresolved (latent footgun)
HasIncrementalBaseline + LastProcessedStateRoot separate reads	Low	⚠️ Unresolved (handled, but fragile)

The most impactful fix before merge is Issue 1 (BuildReport() inconsistency) — it's observable via the API and affects a 30-minute window during a bloatnet rescan. Issues 2–4 are minor and can be tracked as follow-ups.

[AnkushinDaniil]

@claude re-verify on 16f8e5210b.

• Issue 1: new PublishScanBaseline(...) coalesces SetBaseline + MarkScanCompleted + InitializeIncremental under a single _lock. (Your proposed fix — moving calls into _diffLock — wouldn't have helped; BuildReport contends on _lock, not _diffLock.)
• Issue 2: GetSnapshot uses Volatile.Read per counter; write path unchanged.
• Issue 3: CurrentDepthStats narrowed to internal; doc now names _diffLock as the required reader guard.
• Issue 4: StopAsync uses new TryGetShutdownSnapshot — one _lock, one call.

[claude]

Claude finished @AnkushinDaniil's task in 2m 32s —— View job

---

Re-verification: Race Condition Fixes on 16f8e521

• Read repo rules
• Verify Issue 1: PublishScanBaseline coalesces state transition under single _lock
• Verify Issue 2: GetSnapshot uses Volatile.Read per counter
• Verify Issue 3: CurrentDepthStats narrowed to internal; doc names _diffLock
• Verify Issue 4: StopAsync uses TryGetShutdownSnapshot — one _lock, one call

---

All four fixes are verified correct. Your note on the mechanism for Issue 1 is also correct — the previous review's proposed fix (moving calls into _diffLock) would not have worked because BuildReport contends on _lock, not _diffLock. Here is the detailed verification:

✅ Issue 1 — Torn BuildReport() state — FIXED CORRECTLY

PublishScanBaseline (lines 152–191 of StateCompositionStateHolder.cs) coalesces all three formerly-separate mutations (_currentStats, _currentDistribution, _hasScanBaseline, _lastScanMetadata, _incrementalStats, _hasIncrementalBaseline, _incrementalBlock, _diffsSinceBaseline, _lastProcessedStateRoot, _currentDepthStats) under a single lock (_lock). BuildReport reads the same fields under lock (_lock), so it now sees either the pre-scan or fully-published post-scan state — the half-published window is closed.

PublishScanResults in StateCompositionService.cs wraps PublishScanBaseline inside lock (_diffLock) for the metrics update, maintaining the established _diffLock → _lock ordering. No deadlock risk.

The correction to the prior suggestion is valid: the torn state was a _lock-level problem (BuildReport does not touch _diffLock), so moving the calls into _diffLock would have provided no mutual exclusion against a concurrent BuildReport.

✅ Issue 2 — GetSnapshot() progress-logger data race — FIXED CORRECTLY

GetSnapshot() (line 245–264 of StateCompositionVisitor.cs) now issues Volatile.Read(ref long) for each of the 8 per-thread counter fields (AccountsTotal, ContractsTotal, ContractsWithStorage, StorageSlotsTotal, AccountFullNodes, AccountShortNodes, AccountValueNodes, AccountNodeBytes, StorageValueNodes, StorageNodeBytes). All are public long fields — not properties — so Volatile.Read(ref long) is valid and guarantees a torn-free 64-bit load on every .NET runtime, not just x64.

The docstring correctly documents the remaining intentional imprecision: values are best-effort (no happens-before ordering between counters) and suitable for log-line rendering, not correctness-critical totals. The write path is unchanged by design, which is the correct trade-off: adding Volatile.Write to every hot-path counter increment would be a measurable cost for a progress logger.

✅ Issue 3 — CurrentDepthStats mutable reference without lock documentation — FIXED CORRECTLY

CurrentDepthStats is now internal (not public), and the XML doc comment precisely names the required guard:

"Callers MUST hold StateCompositionService._diffLock for the whole duration they read the returned instance; that is the only thing excluding the two writers, ApplyIncrementalDiffAndUpdate and PublishScanBaseline, which themselves run inside _diffLock critical sections in RunIncrementalDiff and PublishScanResults."

Both current callers (RunIncrementalDiff and PublishScanResults) hold _diffLock across the CurrentDepthStats read, which is consistent with this contract.

✅ Issue 4 — Fragile two-read pattern in StopAsync — FIXED CORRECTLY

TryGetShutdownSnapshot reads _hasIncrementalBaseline, _lastProcessedStateRoot, _incrementalBlock, and _incrementalStats under a single lock (_lock) and returns them atomically via out parameters. StopAsync then calls this inside lock (_diffLock), which is the correct outer lock — it serializes against any concurrent incremental diff that would otherwise be mutating those same fields.

---

All four race conditions from the previous round are now closed. The PR is ready to merge.