refactor(test): de-duplicate StateComposition tests - #11256
Merged
LukaszRozmej merged 5 commits intoApr 20, 2026
Merged
Conversation
Extract shared TestDataBuilders (EmptyBaseline, BuildStats, CreateTestConfig, AssertAccountCumulativeEquals) to eliminate copy-pasted CumulativeTrieStats/IStateCompositionConfig setup across service, metrics, and diff-walker tests. Parameterise the slot-bucket check in SlotCountHistogramTests with TestCase and collapse the shared BeginStorageTrie+TrackStorageNode+Flush cycle into a single helper. Net: +45/-177 LOC in tests (+108 helper), 81 → 85 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Visitor_ClassifiesContracts and Visitor_TracksContractsWithStorage ran the same SimulateAccounts-twice-then-inspect pattern with different inputs. Collapse into one TestCase-parameterized Visitor_ClassifiesAccounts method. Each case now asserts all three totals (AccountsTotal, ContractsTotal, ContractsWithStorage) instead of two — widens coverage slightly as a side-effect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the [TestCase(\"Depth\")]+switch-on-string dispatch with a TestCaseSource that carries typed TopContractEntry pairs and the comparer method group directly. Each case gets a readable SetName so failure diagnostics point at the specific comparator under test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @LukaszRozmej's task in 3m 32s —— View job PR Review —
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
AnkushinDaniil
approved these changes
Apr 20, 2026
- Rename AssertAccountCumulativeEquals → AssertAccountTrieFieldsEqual and document that it intentionally skips storage-trie fields, so future callers don't mistake partial coverage for full coverage. - Drop the one-liner EmptyBaseline shim in StateCompositionStateHolderTests and route the 8 call sites directly to TestDataBuilders.EmptyBaseline — removes an indirection that defeats the single-source-of-truth goal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@claude re-review |
Contributor
|
Claude finished @LukaszRozmej's task in 2m 14s —— View job PR Re-Review —
|
Left over from the earlier inline BuildStats that was replaced with TestDataBuilders.BuildStats — tripped IDE0005 in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LukaszRozmej
merged commit Apr 20, 2026
279eace
into
feature/state-composition-plugin
406 checks passed
AnkushinDaniil
pushed a commit
that referenced
this pull request
Apr 21, 2026
* refactor(test): de-duplicate StateComposition test helpers Extract shared TestDataBuilders (EmptyBaseline, BuildStats, CreateTestConfig, AssertAccountCumulativeEquals) to eliminate copy-pasted CumulativeTrieStats/IStateCompositionConfig setup across service, metrics, and diff-walker tests. Parameterise the slot-bucket check in SlotCountHistogramTests with TestCase and collapse the shared BeginStorageTrie+TrackStorageNode+Flush cycle into a single helper. Net: +45/-177 LOC in tests (+108 helper), 81 → 85 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): merge two visitor classification tests via TestCase Visitor_ClassifiesContracts and Visitor_TracksContractsWithStorage ran the same SimulateAccounts-twice-then-inspect pattern with different inputs. Collapse into one TestCase-parameterized Visitor_ClassifiesAccounts method. Each case now asserts all three totals (AccountsTotal, ContractsTotal, ContractsWithStorage) instead of two — widens coverage slightly as a side-effect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): TestCaseSource for Comparator_DeterministicTiebreaking Replace the [TestCase(\"Depth\")]+switch-on-string dispatch with a TestCaseSource that carries typed TopContractEntry pairs and the comparer method group directly. Each case gets a readable SetName so failure diagnostics point at the specific comparator under test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): address claude-review nits on test-dedup PR - Rename AssertAccountCumulativeEquals → AssertAccountTrieFieldsEqual and document that it intentionally skips storage-trie fields, so future callers don't mistake partial coverage for full coverage. - Drop the one-liner EmptyBaseline shim in StateCompositionStateHolderTests and route the 8 call sites directly to TestDataBuilders.EmptyBaseline — removes an indirection that defeats the single-source-of-truth goal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): drop unused System.Collections.Immutable using Left over from the earlier inline BuildStats that was replaced with TestDataBuilders.BuildStats — tripped IDE0005 in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AnkushinDaniil
added a commit
that referenced
this pull request
Apr 24, 2026
…cremental tracking (#10995) * feat: add StateComposition plugin with parallel trie visitor Introduce Nethermind.StateComposition plugin for state composition metrics collection (bloatnet benchmarking). Implements a parallel ITreeVisitor with ThreadLocal<VisitorCounters> for lock-free scaling to 64+ cores. Plugin provides: - 17 composition metrics (accounts, contracts, storage slots, trie node counts/bytes, branch occupancy) - Per-depth distribution tracking (16 levels, account + storage) - Progress callback every 1M accounts - All data models, config, and service interfaces for future PRs Includes 33 unit tests covering visitor behavior, data model correctness, ThreadLocal aggregation, and JSON round-trip. * feat: add statecomp_ RPC namespace with 6 endpoints and runner config Add JSON-RPC module for state composition metrics: - statecomp_getStats: full state scan at head block - statecomp_getScanProgress: scan progress with ETA - statecomp_getCachedStats: cached stats with staleness indicator - statecomp_getCacheMetadata: scan metadata (freshness, duration) - statecomp_getTrieDistribution: trie depth distribution - statecomp_getModuleInfo: API discovery endpoint Supporting infrastructure: - StateCompositionService: orchestrates scans via IStateReader with SemaphoreSlim concurrency guard and progress reporting - StateCompositionStateHolder: thread-safe baseline cache with BlocksSinceBaseline staleness tracking - CachedStatsResponse, ModuleInfo/EndpointInfo response types - statecomp-mainnet.json runner config (archive mode, 4GB memory) - Updated StateCompositionModule DI registrations * refactor: simplify StateComposition plugin per code review - Strip IStateCompositionConfig to 4 used fields (Enabled, ScanQueueTimeoutSeconds, ScanParallelism, ScanMemoryBudget) - Remove unused FromTrieStats, ModuleInfo, ScanProgressResult, progress plumbing - Extract MaxTrackedDepth constant, remove hardcoded magic numbers - Remove dead DI registration (InstancePerDependency visitor) - Remove BlocksSinceBaseline, UpdateHeadBlock (premature for this PR) - Simplify AnalyzeAsync (remove catch block, keep try/finally for semaphore) - Strip RPC to 4 endpoints: getStats, getCachedStats, getCacheMetadata, getTrieDistribution - Add projects to Nethermind.slnx under /Plugins/StateComposition/ - Update statecomp-mainnet.json config to match stripped config - 26/26 tests pass, 0 warnings, 0 errors * fix: address co-design review findings and add Geth feature parity Fix all critical and high severity issues from the multi-perspective co-design review. Add per-contract storage trie tracking with Top-N rankings and depth histogram to match Geth's inspect-trie functionality. Key changes: - Remove abstract from RpcModule, add [RpcModule(ModuleType.Statecomp)] - Add CancellationToken propagation and statecomp_cancelScan endpoint - Fix race condition in StateHolder.MarkScanStarted with proper locking - Fix semaphore release with acquired-flag pattern in service - Add config validation rejecting invalid values in service constructor - Remove uncollectable TotalCodeSize and duplicate byte-size fields - Add per-contract storage tracking: TopN by depth/nodes/slots - Add StorageMaxDepthHistogram for storage trie depth distribution - Convert CachedStatsResponse to readonly record struct - Inject IStateCompositionStateHolder interface instead of concrete type - GetTrieDistributionAsync now throws when not initialized - Cache aggregated counters to avoid duplicate computation - Add 15 new tests (TopN, histogram, cancellation, service, RPC) * feat: add Geth inspect-trie feature parity and remove Geth references - Add TopContractEntry with Owner, Levels[16], Summary fields - Add deterministic multi-field comparators (depth, totalNodes, valueNodes) - Add per-contract depth counters and storage max-depth histogram - Add InspectContract RPC endpoint for single-contract trie analysis - Add ExcludeStorage config option to skip storage traversal - Add scan cooldown (H2) and volatile CTS race fix (H1) - Add progress reporting via 8-second PeriodicTimer - Rename fields to Short/Full/Value terminology - Remove all Geth references and review-issue comments - Expand PluginBootstrapTests with comparator and TopN tests * feat: achieve 100% Geth inspect-trie parity with StateCompositionContext Introduce StateCompositionContext as custom INodeContext<T> that combines TreePath tracking with Level/IsStorage/BranchChildIndex fields. This closes the three remaining Geth parity gaps: - Owner hash: extract keccak256(address) from accumulated nibble path at VisitAccount time instead of passing default ValueHash256 - Comparator direction: fix Owner tiebreaker to match Geth's bytes.Compare (ascending) instead of inverted descending - Storage depth: reset Level to 0 in AddStorage so per-contract Levels[16] uses relative depth matching Geth's approach All 44 tests pass. * fix: security hardening and expanded test coverage for StateComposition - Fail-fast semaphore in AnalyzeAsync and InspectContractAsync - Move cooldown check inside critical section to prevent bypass - Fix CancelScan TOCTOU race via local variable capture - Add IDisposable on StateCompositionService for semaphore cleanup - Remove volatile+lock hybrid in StateHolder (lock-only) - Extract SingleContractVisitor to top-level class - Add custom StateCompositionException type - Add XML doc warning on mutable DepthCounter struct Add 14 new tests (58 total, 0 failures) covering AnalyzeAsync integration, InspectContract edge cases, ExcludeStorage mode, Owner hash preservation, cooldown/semaphore rejection, TopN eviction, and deterministic comparator tiebreaking. * fix: match Geth inspect-trie node counting conventions exactly Align NM reporting layer with three Geth shortNode/valueNode conventions: 1. Short = Extension + Leaf (Geth's shortNode covers both) 2. Value per-depth at depth+1 (Geth counts valueNode one level deeper than its parent leaf shortNode; Size stays at leaf's actual depth) 3. MaxDepth +1 and TotalNodes = physical nodes + leaves (Geth counts valueNode as an extra depth level and an extra node) Changes are reporting-only — internal DepthCounter tracking remains separated (ShortNodes=ext, ValueNodes=leaf) for correctness. Verified: 58/58 tests pass, 65/65 comparison checks at block 500K achieve 100% parity with Geth inspect-trie across all metrics. * feat: add 6 research distribution metrics to StateComposition Collect balance, nonce, storage-slot, and branch-occupancy distributions plus empty account count and top-contracts-by-size ranking from the existing trie walk with zero additional DB reads. New fields in TrieDepthDistribution: - BalanceDistribution (8 buckets: 0 | <0.01 ETH | ... | 10K+) - NonceDistribution (6 buckets: 0 | 1 | 2-10 | ... | 1K+) - StorageSlotDistribution (7 buckets: 1 | 2-10 | ... | 100K+) - BranchOccupancyDistribution (16 entries, children 1..16) New fields in StateCompositionStats: - EmptyAccounts - TopContractsBySize Includes 7 new tests covering all distribution buckets, boundary values, empty account counting, and top-by-size ranking. * refactor: apply co-design review fixes to StateComposition plugin - Refactor service layer to return Result<T> instead of throwing exceptions, matching Nethermind's DebugRpcModule pattern - Update RPC module to deconstruct Result<T> with proper error codes - Extract TopNTracker from VisitorCounters for SRP (H3) - Make VisitorCounters internal (L5) - Add address null-check on inspectContract endpoint (H2) - Dynamic ScanParallelism default: ProcessorCount/2 clamped 1-16 (M4) - Document TopContractsBySize as Nethermind extension (M2) - Revert exception subtypes to base class only - Add 16 new tests: Geth convention regressions (7), multi-threaded merge (2), SingleContractVisitor (5), cancellation semantics (2) - Fix stale SingleContractContext type reference in service tests All 58 tests passing. * refactor: clean up StateComposition plugin after co-design review - Remove ScanCooldownSeconds config and cooldown logic - Remove unused IsScanning property from state holder - Remove unused StateCompositionException class - Remove Balance/Nonce/StorageSlot distribution metrics (not in Geth) - Fix GetTrieDistributionAsync to use cached data (no params needed) - Add CODEOWNERS entries for StateComposition plugin - Split test classes into 1-class-per-file (8 test files, 77 tests) - Modernize assertions: Assert.EnterMultipleScope, collection expressions - Modernize NSubstitute: null! instead of default! for ref args All 77 tests passing. Build: 0 warnings, 0 errors. Verified 100% metric parity with Geth inspect-trie at block 500k (2,064/2,064 per-contract fields match across all 3 rankings). * format * feat: add live scan progress metrics to periodic log Report accounts/s, slots/s, nodes/s and data throughput every 8s during state composition scan. Uses ScanSnapshot with mid-scan ThreadLocal counter aggregation via Volatile.Read. Follows Nethermind conventions: SizeExtensions.SizeToString for bytes, VisitorProgressTracker-style M/K formatting for counts. * test: add scan consistency tests for content-addressed trie isolation Validate that StateComposition scans produce correct results regardless of concurrent block processing, leveraging the content-addressed trie's natural snapshot isolation. Tests cover: - Scan at older root returns original counts after new commits - Isolation after in-place account modifications - Trie node counts reflect original structure per root - Multiple historical roots all scan correctly - Concurrent scan and commit complete without deadlock - Service-layer isolation through full scan pipeline - Sequential scans update state holder correctly * feat: add TrieDiffWalker for exact incremental state composition diffs Replaces the flawed ITrieStoreListener approach (monotonic overcounting) with a recursive diff walker that walks both old and new state roots to compute exact adds AND removes — zero approximation. - TrieDiff.cs: immutable result struct with separate Added/Removed fields - CumulativeSizeStats.cs: cumulative stats with ApplyDiff and FromScanStats - TrieDiffWalker.cs: recursive diff algorithm, skips identical subtrees by hash - TrieDiffWalkerTests.cs: 26 tests including multi-block scan/diff/scan verification Key fix: FromScanStats correctly maps Extensions = ShortNodes - ValueNodes (ShortNodes in Nethermind's visitor includes both extensions AND leaves). * feat: add RocksDB persistence for incremental state composition stats Persist CumulativeSizeStats snapshots to a dedicated stateComposition RocksDB database for warm restart and historical queries. On startup, the plugin restores incremental tracking from the latest valid snapshot, eliminating the need for a fresh 30+ min scan after node restart. - StateCompositionSnapshot: persisted record with stats, block, root - StateCompositionSnapshotDecoder: RLP encode/decode (~140 bytes/entry) - StateCompositionSnapshotStore: DB access with sentinel key for O(1) latest - StateCompositionSnapshotPruner: prunes entries older than configurable window - Plugin warm restart: validates snapshot root against canonical chain - statecomp_getStatsAtBlock RPC: query historical stats by block number - Config: PersistSnapshots, SnapshotBlocksToKeep (10k), SnapshotInterval * feat: add Prometheus metrics for state composition plugin 19 metrics auto-discovered by Nethermind's monitoring system: - 11 cumulative state gauges (accounts, contracts, slots, trie nodes/bytes) - 4 operational gauges (incremental block, diffs count, scan duration/block) - 2 counters (scans completed, diffs applied, diff errors) - 2 scan-only gauges (contracts with storage, empty accounts) Metrics updated on scan completion, each incremental diff, and warm restart. * chore: regenerate packages.lock.json after Nethermind.Init reference * fix(state-composition): force-resolve service to wire NewHeadBlock subscription StateCompositionService and StateCompositionSnapshotPruner subscribe to IBlockTree.NewHeadBlock in their constructors, but Autofac registers them as lazy singletons. Until something resolved them (e.g. an RPC call), the constructors never ran and the event handlers were never wired. As a result incremental diff metrics never updated and the snapshot pruner never ran. Force-resolve both in InitRpcModules so the subscriptions are active from node startup, regardless of whether snapshot persistence is enabled. * feat(state-composition): track ContractsWithStorage and EmptyAccounts incrementally Both metrics were previously updated only on full scans, leaving them frozen between scans. Promote them to first-class CumulativeSizeStats fields so the trie-diff walker maintains them on every new head block. - CumulativeSizeStats: add ContractsWithStorage / EmptyAccounts fields, applied via TrieDiff and seeded from full-scan stats. - TrieDiff: add *Added/*Removed counters and Net* helpers. - TrieDiffWalker: replace DecodeAccountHashes with TryDecodeStruct and count HasStorage / IsTotallyEmpty transitions, matching the visitor semantics. CollectLeaf increments the counters for new/removed leaves. - Metrics.UpdateFromCumulativeStats now wires both gauges, so the service no longer needs the explicit scan-only assignments. - Snapshot RLP gains 2 longs; legacy snapshots fail to decode and the plugin catches the exception, discards them, and triggers a fresh scan to rebuild the baseline with the new schema. * feat(state-composition): expose trie depth distribution as Prometheus gauges Add 149 new [GaugeMetric] properties covering: - 5 scalars: avg/max account/storage depth, avg branch occupancy - 64 account trie per-depth gauges (full/short/value nodes + bytes, depths 0..15) - 64 storage trie per-depth gauges (same layout) - 16 branch occupancy histogram buckets (1..16 children) Metrics are populated on full scan completion from the existing TrieDepthDistribution cached by the visitor. Between scans values stay flat -- incremental per-block tracking lands in a follow-up. All properties use explicit underscore separators around depth digits (e.g. Depth_7_FullNodes) because MetricsController's PascalCase to snake_case conversion only triggers on lowercase to uppercase transitions, which would otherwise fuse the digit into adjacent words. Snapshot restore path leaves depth gauges at zero with a comment -- the current snapshot schema does not persist TrieDepthDistribution. * feat(state-composition): track trie depth distribution incrementally via diff walker Phase B of live trie-depth metrics. TrieDiffWalker now threads depth through its recursive descent and emits a DepthDelta per block, which StateCompositionStateHolder applies to a new CumulativeDepthStats. The 149 depth gauges added in Phase A are now refreshed on every new head block instead of only on full scans. - CumulativeDepthStats: mutable per-depth arrays (account/storage Full/Short/Value/Bytes + BranchOccupancy) seeded from scan via SeedFromScan (reverses Geth +1 shift) and applied in place under the state-holder lock. - DepthDelta: reusable per-block delta, cleared between diffs. - TrieDiffWalker: threads int depth through DiffSubtree/DiffNodes/ DiffBranches/DiffExtensions/DiffLeaves/CollectSubtree; branch children depth = d+1, extension children depth = d + key.Length; storage tries reset to 0. - Metrics.UpdateFromDepthStats: applies Geth conventions at read time (ValueNodes[d] reads AccountValueNodes[d-1]; MaxStorageDepth += 1). - TrackDepthIncrementally config flag (default true) gates the walker overhead for benchmarks. Tests: 127 pass (+13 new). CumulativeDepthStatsTests covers seed/apply/reset/clone parity with scan-derived gauges. TrieDiffWalker tests assert depth delta is null when disabled, per-depth buckets shift correctly on leaf/branch add/remove, and ShortNodes honors the Extension+Leaf Geth convention. * fix(state-composition): persist depth stats in snapshot and gate cold-start deltas Prevent negative per-depth gauges that appeared after restart when a pre-Phase-B snapshot was restored: the depth arrays were zero-seeded and the first incremental diffs that removed nodes pushed gauges below zero. - CumulativeDepthStats.IsSeeded flag: ApplyDelta is a no-op until a baseline is installed via SeedFromScan or SeedFromSnapshot. Metrics.UpdateFromDepthStats short-circuits on unseeded input so gauges stay at their cold-start zero until a fresh scan or a depth-carrying snapshot is loaded. - StateCompositionSnapshot gains an optional CumulativeDepthStats payload; the RLP encoder writes a leading present/absent marker followed by 162 longs when seeded. Legacy snapshots fail to decode and are discarded by the existing plugin try/catch, triggering a fresh scan that seeds with the new schema. - StateCompositionService persists the current depth stats on both the scan-complete and periodic snapshot writes; the plugin's restore path replays them into the state holder and calls UpdateFromDepthStats so gauges come up correct across restarts with no zero-window. - Tests: new NewSeededEmpty() helper so ApplyDelta/Clone tests continue to exercise delta arithmetic on a (now-required) seeded baseline. * refactor(state-composition): tighten plugin after multi-agent review Tranche A — delete dead abstractions and unused tests: - remove IStateCompositionService, IStateCompositionStateHolder interfaces (single impl each, blocked test doubles without adding value) - fold StateCompositionSnapshotPruner into StateCompositionService (one caller, tight coupling to snapshot write cadence) - replace throw-on-invalid-config with clamp + warn log; node should not fail to start over plugin config nits - drop try/catch in StateCompositionVisitor.VisitBranch (underlying TrieNode child loop cannot throw on well-formed RLP); replace with null-guard - remove spurious Volatile.Read in GetSnapshot (Task.WaitAll happens- before already guarantees visibility) - delete 9 trivial plugin-bootstrap tests, 5 redundant visitor tests, and the diagnostic "isolate extension undercount" region now that the bug it was chasing is fixed - move 59-line depth-gauge reset into MetricsDepthGaugesHelper Tranche B — perf + missing coverage: - switch TrieDiffWalker.DiffMismatchedNodes dictionary key from Hash256 to ValueHash256, and read leaf full-path as ValueHash256 directly (eliminates per-leaf Hash256 allocation on the hot diff path) - add cancellation-mid-scan test with ManualResetEventSlim gating - add reorg-rollback test (forward diff then backward diff must restore exact baseline across all 9 cumulative fields) - add cross-semaphore test documenting that InspectContractAsync runs independently of a blocked AnalyzeAsync Tranche C — hot-path perf wins without framework changes: - cache VisitorCounters on StateCompositionContext so per-node ThreadLocal.Value lookup fires once per root/worker instead of once per visited node - replace BuildSortedTopN lambda-captured comparer with a DescendingComparer struct (avoids delegate + closure per scan) - lazy-allocate TrieLevelStat scratch in VisitorCounters and short-circuit the per-contract finalize via TopNTracker.WouldInsert so only ranking contracts pay the ImmutableArray freeze cost - DepthDelta.IsEmpty() early-out skips UpdateFromDepthStats when the diff walker produced no depth changes - misc: RLP long-array encode/decode helpers replace the 162-field unrolling in StateCompositionSnapshotDecoder; CumulativeDepthStats exposes MarkSeeded() so the decoder no longer round-trips a sentinel Net: -474 LOC, 112 tests passing (+3 new), 0 build warnings. No behavioural change except config clamping and faster hot paths. * refactor(state-composition): reorganize plugin into functional subdirectories Move 19 files into Data/, Rpc/, Visitors/, Diff/, Service/, Snapshots/ subdirectories with folder-matching namespaces to align with sibling Nethermind plugins (Merge.Plugin, JsonRpc, Optimism). Mirror the same layout under Nethermind.StateComposition.Test. Split the two largest files via partial class: - TrieDiffWalker.cs (881 lines) → core + Branches/Extensions/Leaves/ Collection/Depth partials - StateCompositionService.cs → core + Incremental partial Reduce complexity hotspots via pure Extract Method refactors: - DiffBranches: extract DiffBranchChild + CollectBranchSlotSide (CC ~18 → ~6, nesting 5 → 3) - AnalyzeAsync: extract ResolveScanOptions, StartProgressLogging, PublishScanResults - OnNewHeadBlock: extract RunIncrementalDiff + MaybeWriteSnapshot - FinalizeCurrentStorageTrie: extract BuildCurrentStorageLevels + RankCurrentContract Merge IStateCompositionConfig.cs into StateCompositionConfig.cs (-1 file). Tighten StateCompositionRpcModule to internal sealed to match the internal service/state-holder it depends on. No behavior change. 112/112 tests passing. * refactor(state-composition): strip trivial and redundant comments Remove ~82 comment lines across 22 files: divider bars in Metrics.DepthGauges.cs, field-decoration restatements, obvious control-flow narration, and redundant XML <summary> blocks that just echoed method names. Preserved Geth convention notes, concurrency/lock invariants, and non-obvious WHY comments. No behavior change. 112/112 tests passing. * feat(state-composition): add CodeBytesTotal and per-contract slot histogram Aggregate on-chain bytecode deduplicated by codeHash (proxies and minimal clones contribute once) and a log-bucketed per-contract slot-count histogram. Both are produced by the full-scan visitor, fanned out as Prometheus gauges, and persisted through the snapshot schema so restarts resume the last baseline instead of dropping to zero. Freeze pattern: neither field can be maintained by the incremental diff walker without a refcount map, so ApplyDiff uses `this with { ... }` to carry them forward unchanged until the next scan refreshes them. SlotHistogramLength is defined once on CumulativeSizeStats and shared by producer and decoder so their wire length cannot drift. * refactor(state-composition): collapse depth metrics into labeled gauges Replace the 149 flat per-depth Prometheus properties and 16 per-bucket slot-count properties with four [KeyIsLabel] dictionaries, matching Nethermind's native labeled-gauge pattern. Fan-out is now driven by a single UpdateDepthDistribution publish instead of UpdateFromDepthStats + UpdateFromDistribution. Behavior is preserved: the IsSeeded cold-start gate, Geth +1 ValueNode presentation shift, and slot/code histogram exposure all carry over. * refactor(state-composition): lint cleanup and fix cancellation test - Remove unused usings in service and incremental partial classes - Test file modernization: using-order, cts.CancelAsync, spelling - Fix AnalyzeAsync_CancelledMidScan: mock now waits on cts.Token directly instead of a stale CancellationToken.None snapshot that never signaled * feat(state-composition): incremental updates for CodeBytesTotal and slot histogram Thread per-account payloads (SlotCountChanges, CodeHashChanges) through TrieDiff so the state holder can refcount CodeBytesTotal and move contracts between slot-histogram buckets on every NewHeadBlock. Full scans seed the trackers; snapshots persist them across restarts. Loading a snapshot without trackers triggers a fresh rescan. Adds 11 unit tests covering refcount edge cases (shared bytecode add / drop, swap) and snapshot round-trip for the three tracker maps. * fix(state-composition): enable plugin via primary-ctor config injection Default to disabled and inject IStateCompositionConfig via primary constructor so PluginLoader sees the real Enabled value at enumeration time instead of null (pattern match failed, plugin never initialized). Matches the TraceStorePlugin pattern. * fix(state-composition): use structural depth in TrieDiffWalker DiffExtensions and CollectSubtree incremented depth by the extension key length, while the baseline visitor's StateCompositionContext.Add uses Level+1 regardless of path length. The mismatch routed diff bytes into nibble-depth buckets while the baseline seeded structural-depth buckets, drifting the Trie Depth Distribution histogram (observed as a negative 6-byte value at account depth 13 with a 172-node bucket). Aligns both Extension paths in TrieDiffWalker to depth+1. * fix(state-composition): auto-recover when baseline root is pruned On restart, the persisted snapshot seeds LastProcessedStateRoot with the old block's root. If the container was stopped longer than the pruning window, that root is no longer in the trie DB, so every OnNewHeadBlock hit MissingTrieNodeException via TrieDiffWalker.ComputeDiff and spammed diff_errors indefinitely until an operator manually re-ran statecomp_getStats to reseed. Narrow the catch in RunIncrementalDiff: on MissingTrieNodeException, invalidate the baseline (LastProcessedStateRoot=null silences the null gate in OnNewHeadBlock), bump a dedicated StateCompBaselineInvalidations counter, and fire-and-forget AnalyzeAsync — the existing _scanLock coalesces back-to-back triggers, so at most one real scan runs. Generic exceptions keep the old diff_errors path unchanged so alerting still surfaces real bugs. Tests cover the narrow holder invalidation, the MissingTrieNode recovery path (counter bump + auto-rescan reseed), and a regression guard for generic exceptions keeping the legacy counter behaviour. * refactor(state-composition): atomic BuildSnapshot + require init StateCompositionStateHolder.BuildSnapshot captures stats, depth stats, and the three tracker dictionaries under a single lock entry so the persisted snapshot cannot tear against a concurrent InitializeIncremental or diff application. Replaces three separate Clone* accessors (each taking the lock individually) and collapses six lock entries in MaybeWriteSnapshot / PublishScanResults to one. Also removes the unused HasIncrementalTrackers probe and sets MustInitialize => true so the host enforces plugin startup order. * refactor(state-composition): drop redundant getCacheMetadata RPC statecomp_getCacheMetadata returned the same ScanMetadata? already exposed as CachedStatsResponse.LastScanMetadata on statecomp_getCachedStats. Delete the redundant endpoint and its test; callers migrate by reading the LastScanMetadata field on the cached-stats response. Also collapse the getCachedStats builder: four separate holder lock entries (IncrementalStats, IncrementalBlock, DiffsSinceBaseline, LastScanMetadata) become a single BuildCachedStatsResponse() accessor that captures all four under one lock, matching the atomic-by-design pattern introduced for BuildSnapshot. Document AnalyzeAsync's two legal call sites (operator RPC + MissingTrieNodeException recovery) so future readers cannot accidentally add a third scan dispatcher. * refactor(state-composition): transfer visitor maps on GetStats StateCompositionVisitor is internal sealed, IDisposable, and used exactly once under a `using` in StateCompositionService. After GetStats returns, the visitor is disposed and the holder has already taken its own defensive copy inside InitializeIncremental, so the two deep-copy blocks for _codeHashSizes and agg.CodeHashRefcounts were defending against a second GetStats call that cannot happen. Hand those maps through directly — the fields on StateCompositionStats are typed as IReadOnlyDictionary<>, so ConcurrentDictionary<> flows through without an intermediate materialization. SlotCountsByOwner still needs the list→dict foreach loop: the zero-owner sentinel can appear more than once across worker threads, and `new Dictionary(list)` throws on duplicate keys. * refactor(state-composition): extract LevelStatsBuilder helper Three sites hand-rolled the DepthCounter[] → TrieLevelStat[] conversion, each carrying its own copy of the Geth +1 valueNode depth shift: - StateCompositionVisitor.BuildLevelStats (filtered → ImmutableArray) - VisitorCounters.BuildCurrentStorageLevels (fill scratch + summary) - SingleContractVisitor.GetResult (fill + summary inlined) Consolidate the row-construction + depth shift into LevelStatsBuilder with two entry points: Fill(depths, dest) for fixed-buffer callers that also need the summary row, and BuildCompact(depths) for the RPC depth distribution which filters out empty levels. SingleContractVisitor now allocates a concrete array and wraps it via ImmutableCollectionsMarshal.AsImmutableArray — same no-copy handoff pattern already used by BuildSortedTopN. * refactor(state-composition): collapse decoder map helpers Fold the three parallel (slot-count, int-refcount, int-size) map encode/length/decode helper triples into single generic helpers parameterised by value type, and inline the per-depth-array encode loops behind a shared DepthArrays() list. Nullable map handling stays inside the generic helpers (count=0 + skip) so legacy null-tracker snapshots still round-trip, but the three call sites collapse from nine hand-rolled helpers to three. * refactor(state-composition): merge DepthDelta into CumulativeDepthStats The two types had identical layout (9 long[16] + 2 scalars). Fold DepthDelta into CumulativeDepthStats by replacing ApplyDelta(DepthDelta) with AddInPlace(CumulativeDepthStats) and retype TrieDiff.DepthDelta. Delete DepthDelta.cs entirely. Snapshot schema is unchanged (same field layout, same decoder). Plan M6 — net -40 LOC. * refactor(state-composition): back CumulativeDepthStats with long[9][16] + DepthSlot enum Replace nine parallel long[16] fields with a single jagged long[9][16] row array indexed by a new DepthSlot enum. Every cumulative operation (Reset, Clone, AddInPlace, IsEmpty, SeedFromSnapshot) collapses from nine field-by-field statements to a single loop. The snapshot decoder drops the DepthArrays helper and iterates ByDepth directly. Pass-through properties (AccountFullNodes, BranchOccupancy, etc.) are kept so Metrics.UpdateDepthDistribution, TrieDiffWalker.Depth, and the test suite still read via the named rows. Schema-compatible: the RLP layout is unchanged because DepthSlot pins the same numeric order as the old field list. Plan M7 — jagged layout gate; 136/136 tests pass in 6.9s (no regression). * refactor(state-composition): share trie walker across collect paths CollectSubtree and CollectSubtreeForDiff duplicated the entire branch/ extension traversal including depth and RecordNode bookkeeping — only the leaf handling differed. Extract the shared walker into WalkStructure<TH> parameterised by a struct ILeafHandler so the JIT specialises per handler type with no delegate allocation on the hot path. SemanticLeafHandler counts accounts/contracts/slots and recurses into storage tries; DictionaryLeafHandler stores leaves for deferred matching. The two public entry points shrink to ~5-line wrappers. Plan M8 — 136/136 tests pass in 6.9s (no regression). * refactor(state-composition): strip neuroslop xmldoc (H8) Remove xmldoc blocks that restate method/class names without documenting WHY. Keep summaries that pin non-obvious invariants: IsSeeded rationale, Prometheus label naming, BranchOccupancyDistribution. * refactor(state-composition): drop DescendingComparer struct (H4) * refactor(state-composition): collapse TryInsert/WouldInsert duplication (H5) * refactor(state-composition): extract ClampWithWarn helper for scan option clamping (H7) * refactor(state-composition): flatten redundant HasCode/HasStorage guard in CollectLeaf (M8) * refactor(state-composition): bind progress logger to linked CTS token (H6) * refactor(state-composition): use Dictionary for per-contract slot counts (H3) * refactor(state-composition): collapse TrieDiffWalker parallel counters into 2D tables (H1) * refactor(state-composition): single-pass snapshot encoder via EncodeOrLength helper (H2) * feat(state-composition): register RLP snapshot decoder via InitTxTypesAndRlpDecoders (V1) * feat(state-composition): prepend schema version byte to snapshot encoding (V2) * refactor(state-composition): volatile scan CTS and AutoActivate service (V4, V7) * test(state-composition): trim VisitorCounters tests and move under Visitors namespace Rename PluginBootstrapTests.cs -> Visitors/VisitorCountersTests.cs to match the namespace of the type under test. Delete four tests covered by existing broader assertions (trivial DepthCounter getters, Flush no-op, and the by-value-nodes Top-N test which is now covered in aggregate by the merge and insert tests). Collapse three Comparator_*_DeterministicTiebreaking tests into one [TestCase]-parameterized method. * test(state-composition): drop tests subsumed by broader diff coverage Remove seven tests whose invariants are covered by existing broader tests: BothRootsNull/BothRootsEmptyTreeHash (SameRoot covers zero-diff semantics), EmptyToSingleAccount/SingleAccountToEmpty (add/remove is exercised by the multiple-account and ModifyAccountBalance tests), Walker_CanBeReused_ForMultipleDiffs (reuse is verified every block by MultiBlockIncremental_MatchesFullScan), and the two trivial DepthDelta null/not-null toggles (the AddOneLeaf test already asserts non-null when trackDepth=true). * test(state-composition): drop CancellationTests white-box ShouldVisit probes These two tests peek at the visitor's ShouldVisit gate after cancelling a CancellationTokenSource. The end-to-end cancellation behaviour (RPC CancelScan + service scan abort) is already covered by StateCompositionServiceTests and StateCompositionRpcModuleTests, so the low-level probes are pure maintenance cost. * test(state-composition): parameterize constructor zero-config tests Collapse the three Constructor_ClampsZero{Parallelism,MemoryBudget,TopN} tests into one [TestCase]-parameterized test. The constructor does not actually clamp — clamping happens lazily inside ResolveScanOptions on AnalyzeAsync — so the three tests all verified the same thing: that the constructor accepts a zero without throwing. * test(state-composition): drop trivially subsumed visitor tests Visitor_ShouldVisit_AlwaysReturnsTrue is a single-line sanity check with no coverage value beyond what the non-cancelled branch of the remaining ShouldVisit tests already exercises. Visitor_CountsAccountsCorrectly is fully subsumed by Visitor_ClassifiesContracts, which asserts the same AccountsTotal invariant on a richer input. * test(state-composition): drop chained freeze-fields regression ApplyDiff_Chained_StillPreservesFreezeFields iterates the preserved freeze semantics three times over the top of what ApplyDiff_PreservesCodeBytesAndSlotHistogram already establishes. If a single ApplyDiff leaves CodeBytesTotal and SlotCountHistogram untouched then chaining N diffs cannot break that — the assertion is structurally redundant. * refactor(state-composition): delete statecomp_getStats RPC Plan pass 2 commit 1 — collapse to single operating mode by removing the only operator-initiated rescan entry point. statecomp_getCachedStats remains as the read-only stats endpoint. Also adds a tripwire doc comment above AnalyzeAsync naming the two legal callers (plugin bootstrap + RunIncrementalDiff recovery) so a future contributor adding a third caller has to delete the comment explicitly. * feat(state-composition): flush snapshot on graceful shutdown Plan pass 2 commit 2 — stop persisting a snapshot on every block. - StateCompositionService now implements IStoppableService. StopAsync cancels any in-flight scan, acquires _scanLock, and force-flushes the latest incremental state through a new WriteSnapshotForHead helper that every snapshot write routes through. - Default SnapshotInterval bumped from 1 to 1024. Per-interval writes are a crash-safety fallback; IServiceStopper guarantees StopAsync runs before the snapshot RocksDB is disposed on SIGTERM/docker stop. - ServiceStopperMiddleware auto-registers any singleton implementing IStoppableService, so StateCompositionModule needs no change. * refactor(state-composition): kill defensive copies on the holder hot paths Plan pass 2 commit 3 — eliminate copies that were defending against mutation that cannot happen with the single-writer invariant. - StateCompositionStateHolder.BuildSnapshot hands ownership of the slot, refcount, and code-size dictionaries directly to the snapshot record; the snapshot is persisted and discarded with no downstream reader, so the three defensive Dictionary copies are pure waste. - StateCompositionStateHolder.InitializeIncremental drops the conditional-copy-if-not-null branches on the snapshot dictionaries — the decoder always materialises fresh Dictionary instances. - StateCompositionStats and StateCompositionSnapshot expose the concrete Dictionary type for the three ownership-transfer hand-off fields so the holder can take ownership without an IReadOnlyDictionary wrapper allocation. CumulativeDepthStats.Clone is removed (no live callers). - StateCompositionVisitor returns the visitor's owned dictionary refs directly instead of cloning during GetStats; the visitor is one-shot and disposed immediately after. * refactor(state-composition): kill optional-field nullability with sentinels Replace nullable cold-start gates with explicit sentinels and flags so metric semantics stay intact while the surface area shrinks: - StateCompositionStateHolder: _lastScanMetadata, _incrementalStats, _lastProcessedStateRoot become non-nullable; new IsIncrementalSeeded bool flag gates incremental presence; LastProcessedStateRoot returns Hash256.Zero when invalidated. - ScanMetadata.IsComplete becomes the single freshness gate (record default has IsComplete=false, replacing the ScanMetadata? wrapper). - TrieDiff.DepthDelta / SlotCountChanges / CodeHashChanges become required positional members; introduce TrieDiff.Empty as the no-op early-return value so the walker no longer constructs nulls. - StateCompositionSnapshot drops nullable wrappers around DepthStats and the three tracker dictionaries; decoder always materialises an unseeded CumulativeDepthStats and empty maps when fields are absent. - IsSeeded on CumulativeDepthStats is preserved — it's a load-bearing guard against negative metrics on cold replay, not a cold-start gate. Tests updated: SnapshotRoundTripTests gains a BuildSnapshot helper that fills the new required positional args; assertions against now-non-null fields drop the redundant null checks; TrieDiffWalkerTests SameRoot_ReturnsZeroDiff compares against TrieDiff.Empty instead of default(TrieDiff). * test(state-composition): trim redundant single-node probes Drop white-box TrieDiffWalker probes (sections 1, 2, 4-7, 9) and the DepthDelta unit probes: all covered end-to-end by the integration fixtures (MultiBlock_ScanDiffScan_CumulativeMatchesFreshScan, LargeTrie_IncrementalMatchesFullScan, StorageTrieIncremental_MatchesFullScan, ReorgRollback_ForwardThenBackward). Drop no-state StateCompositionService probes (constructor zero-arg, GetTrieDistribution cold-start, CancelScan no-op, InspectContract no-data) — no behavior worth guarding; the concurrency and happy-path tests remain. Symmetry, ApplyDiff_RoundTrips, FromScanStats, and the storage-trie sanity tests stay. Tests: 96 passed, 0 failed (was 117). * refactor(state-composition): collapse lock-wrapped property getters Shrink seven single-statement `lock (_lock) return _field` getters in StateCompositionStateHolder from four lines each to one. Pure cosmetic pass — no behavior change, 21 LOC saved. * fix(state-composition): close shutdown-flush race and wire bootstrap scan Two concerns surfaced by reviewer on top of the shrinkage pass: 1) StopAsync wrote a torn snapshot under load. The capture sequence read IncrementalStats, IncrementalBlock, and LastProcessedStateRoot under separate holder-lock acquisitions, and BuildSnapshot then handed the live tracker dictionaries to the RLP encoder while the diff path could still mutate them through _diffLock. Fix: - Unsubscribe NewHeadBlock at the top of StopAsync to stop new diff dispatches (delegate -= is idempotent so Dispose is safe). - Acquire _diffLock around the read+write in StopAsync to drain any in-flight diff and exclude the encoder race. - Wrap InitializeIncremental + WriteSnapshotForHead in PublishScanResults with the same lock — the same race exists between the scan baseline install and the first head block. Lock order is _scanLock -> _diffLock everywhere; the diff path only takes _diffLock, so the nested acquire is deadlock-free. 2) StateCompositionPlugin.Init never called AnalyzeAsync, so a cold-start node with no persisted snapshot stayed at zero forever once statecomp_getStats was deleted. Wire ScheduleBootstrapScan in Init for the no-snapshot, stale-snapshot, and no-PersistSnapshots paths and update the AnalyzeAsync tripwire doc to match. * chore(state-composition): drop refactor-history comment Remove the trailing 'now part of CumulativeSizeStats' note on the PublishScanResults metric publish — the call to UpdateFromCumulativeStats on the next line documents itself. * fix(state-composition): write snapshot only on shutdown, purge old entries Snapshot data was accumulating unbounded on mainnet (~2 TB) because periodic writes every N blocks retained thousands of large entries. - Remove MaybeWriteSnapshot (periodic interval writer) and the scan-completion write from PublishScanResults - StopAsync is now the sole snapshot write path - WriteSnapshot deletes the previous entry before writing the new one so only one snapshot ever exists in the DB - Add PurgeOldEntries() called on startup to clean up legacy data - Remove trivial comments that duplicate self-explanatory code * chore(state-composition): remove trivial comments from test files Drop region tags, section labels, and comments that restate self-explanatory code across 8 test files. Kept XML docs explaining Geth conventions, isolation properties, freeze semantics, and race-condition design decisions. * test(state-composition): delete ScanConsistencyTests These tests validated content-addressed trie isolation — a property of Nethermind's StateTree, not the plugin. The plugin never scans at historical roots; it always operates on the current head. * test(state-composition): remove debug report file and impossible-scenario test Remove hardcoded /private/tmp/claude/ report-file writing from MultiBlock_ScanDiffScan test — TestContext.Out and Assert already cover the same output. Delete UpdateDepthDistribution_UnseededStats_Noop since UpdateDepthDistribution is never called with unseeded stats in production (all call sites are post-scan). * test(state-composition): merge 4 snapshot round-trip tests into one Consolidate RoundTrip_PreservesCodeBytesTotal, PreservesSlotCountHistogram, PreservesSlotCountByAddressTracker, and PreservesCodeHashTrackers into a single RoundTrip_PreservesAllFields test with EnterMultipleScope. Adds coverage for BlockNumber, DiffsSinceBaseline, and ScanBlockNumber fields that were previously untested. * test(state-composition): delete redundant Geth convention tests Remove Convention3, Convention4, Convention5, and Convention1_PerDepthLevels tests — all fully covered by AllConventions_StorageTrie_Comprehensive. Keep Convention1 and Convention2 account-trie tests (different code path through StateCompositionVisitor vs VisitorCounters). * refactor(state-composition): tighten visibility and seal internal types Make 8 types internal (StateCompositionStats, TrieDiff, SlotCountChange, CodeHashChange, StateCompositionContext, VisitorCounters, DepthCounter, StateCompositionSnapshotStore). Make ApplyDiff, FromScanStats, UpdateFromCumulativeStats, and UpdateDepthDistribution internal. Seal StateCompositionService and StateCompositionSnapshotStore. Remove unnecessary virtual on CancelScan and replace FakeService test subclass with a factory method. * fix(state-composition): use DiffMismatchedNodes for extension prefix mismatch, reuse walker - Extension prefix mismatch now routes through DiffMismatchedNodes instead of independent CollectSubtree calls, eliminating spurious CodeHashChange and SlotCountChange events for leaves shared across both subtrees - Move ITrieNodeResolver from TrieDiffWalker constructor to ComputeDiff parameter, allowing the walker to be reused across blocks without reallocating internal lists, counter tables, and depth stats arrays - Store a single TrieDiffWalker instance on StateCompositionService, protected by _diffLock - Fix two broken [with(...)] collection expressions left by a previous linter pass in StateCompositionSnapshotDecoder and StateCompositionVisitor * refactor(state-composition): delete statecomp_getStatsAtBlock RPC Historical snapshot lookup has no production consumer — the plugin only needs the latest snapshot for restart. Removes the endpoint from the interface and module, drops the snapshotStore dependency from the RPC module constructor. * refactor(state-composition): merge RPC endpoints into statecomp_get Unite statecomp_getCachedStats and statecomp_getTrieDistribution into a single statecomp_get endpoint. CachedStatsResponse now includes the TrieDepthDistribution field. Delete the separate GetTrieDistribution service method. * docs: clarify Geth vocabulary parity in trie node naming Replace cryptic "Short=Extension, Full=Branch, Value=Leaf" one-liners with proper doc comments that reference go-ethereum/trie/inspect.go and explain the dual vocabulary is intentional Geth parity, not confusion. * refactor: rename confusing types and fields for clarity - CachedStatsResponse → StateCompositionReport (not "cached", always live) - CumulativeSizeStats → CumulativeTrieStats (tracks counts + bytes, not just sizes) - CurrentStats → TrieStats (in report) - DiffsSinceLastScan → DiffsSinceBaseline (matches internal field name) - IsInitialized → HasScanBaseline (clarifies what it gates) - IsIncrementalSeeded → HasIncrementalBaseline (clarifies what it gates) - BuildCachedStatsResponse() → BuildReport() - CumulativeDepthStats.SlotCount → CategoryCount (avoids storage slot clash) * fix(state-composition): address PR review feedback Address findings from Claude bot review on PR #10995: - Register StateCompositionService as IStoppableService so graceful shutdown actually flushes the snapshot (StateCompositionModule.cs). - Remove catch-when(logger.IsError) anti-pattern in bootstrap scan and auto-rescan paths; guard the log call inside the catch so exceptions are never rethrown into the unobserved-task pipeline. - Drop the dead SnapshotBlocksToKeep config option. - Bound statecomp_inspectContract with a per-call CancellationTokenSource using the new InspectContractTimeoutSeconds config (default 30s) so long storage-trie walks don't pin RPC workers indefinitely. - Defensive CloneAsDelta() on TrieDiff.DepthDelta so consumers don't share a mutable reference with the walker. - Document LatestKey sentinel collision window in SnapshotStore. * style(state-composition): fix whitespace around spread operator Add the required space after '..' in collection spreads so 'dotnet format whitespace --verify-no-changes' passes the CI gate. * style(state-composition): trim waste in two comments Drop the lines that just restated code: the sidebar about SlotCountChanges/CodeHashChanges in TrieDiffWalker.CreateDiff, and the 'sentinel key holding the latest committed block number' lead-in above LatestKey (the field name already says that). Keep the non-obvious bits (clone rationale, 3.5-trillion-year collision window). * fix(state-composition): address second-round PR review findings - Snapshot write: put new → update LatestKey → remove old, so a crash between ops leaves the old entry reachable and PurgeOldEntries keeps it. - PublishScanResults: move depth-gauge publish inside _diffLock so an OnNewHeadBlock dispatch cannot tear the 9×16 table between seed and read. - StopAsync: bound _scanLock wait to 10s with warn log instead of blocking shutdown indefinitely on an uncooperative scan. - TrieDiff account leaves: switch to TryDecodeAccount; skip semantic diff and classification on length-1 empty stubs instead of treating them as real accounts / empty-account transitions. - Surface missing-node observations: new StateCompScanMissingNodes counter, visitor latch propagated into ScanMetadata.IsComplete, scan log flags incomplete runs explicitly. - Remove dead SnapshotInterval config knob (no readers). * fix(state-composition): guard shutdown race and harden parallel counter - Gate RunIncrementalDiff with a volatile _shuttingDown flag set at the top of StopAsync so diffs queued between `NewHeadBlock -=` and disposal cannot race IWorldStateManager/IDb teardown. - Convert StateCompScanMissingNodes to a get-only property backed by an Interlocked counter; concurrent tree-visitor workers now increment it safely via Metrics.IncrementScanMissingNodes(). - Cache Keccak.OfAnEmptyString.ValueHash256 in TrieDiffWalker so the hot RecordCodeHashChange path skips the property chain per call. - Drop two code-restating line comments in OnNewHeadBlock. * fix(state-composition): close scan-publish metrics race and rate-limit miss log - PublishScanResults: extend _diffLock to cover UpdateFromCumulativeStats and the scalar StateComp* writes. Prior scope left a window where a concurrent incremental diff for block N+1 could publish its metrics between InitializeIncremental and the scanner's post-lock writes, so the fresh baseline would clobber the newer N+1 values. - VisitMissingNode: latch-gate the warn log on the first miss using the existing _missingNodesObserved flag. The counter metric still increments on every miss, but the warn line fires once per scan so a pruning-window eviction cannot flood the log. * refactor(state-composition): drop redundant child-path copies in DiffBranchChild GetChildWithChildPath does not mutate its ref TreePath for inline children (verified in TrieNode.ResolveChildWithChildPath, switch arms for null/keccak/inline RLP never touch childPath). The child's own key is appended by the child's handler (DiffExtensions.path.AppendMut(Key), leaf handler appends for reconstruction), so the local oldChildPath / newChildPath copies were always equal to path on return — misleading dead locals. Passing ref path directly matches the existing pattern in CollectBranchSlotSide and WalkStructure (same file group). * test(state-composition): wire service tests via DI per test-infrastructure.md StateCompositionServiceTests constructed the service with Substitute.For<IBlockTree>() and Substitute.For<IWorldStateManager>(). test-infrastructure.md forbids mocking what production modules wire; use PseudoNethermindModule + TestEnvironmentModule and override only IStateReader (the boundary where tests must inject RunTreeVisitor side effects for semaphore/cancellation semantics). * fix(test): remove unused Nethermind.Trie.Pruning using * fix(statecomp): statecomp_cancelScan returns whether a scan was active CancelScan() now reports whether a cancellation signal was issued so RPC callers can distinguish "scan cancelled" from "no scan was running" rather than unconditionally returning true. * fix(statecomp): guard SlotCountHistogram update against default ImmutableArray Mirrors the existing IsDefault guard in StateCompositionSnapshotDecoder. Length throws NullReferenceException on a default-constructed ImmutableArray<long>; all current callers pass initialized arrays but the consistency matters for future call sites. * fix(statecomp): GetAddressHash returns null for leaves with missing Key A resolved leaf without a Key is corrupt; hashing the partial path would silently drift the per-contract code-hash and slot-count trackers under a wrong address. Callers now skip the record instead. * refactor(test): de-duplicate StateComposition tests (#11256) * refactor(test): de-duplicate StateComposition test helpers Extract shared TestDataBuilders (EmptyBaseline, BuildStats, CreateTestConfig, AssertAccountCumulativeEquals) to eliminate copy-pasted CumulativeTrieStats/IStateCompositionConfig setup across service, metrics, and diff-walker tests. Parameterise the slot-bucket check in SlotCountHistogramTests with TestCase and collapse the shared BeginStorageTrie+TrackStorageNode+Flush cycle into a single helper. Net: +45/-177 LOC in tests (+108 helper), 81 → 85 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): merge two visitor classification tests via TestCase Visitor_ClassifiesContracts and Visitor_TracksContractsWithStorage ran the same SimulateAccounts-twice-then-inspect pattern with different inputs. Collapse into one TestCase-parameterized Visitor_ClassifiesAccounts method. Each case now asserts all three totals (AccountsTotal, ContractsTotal, ContractsWithStorage) instead of two — widens coverage slightly as a side-effect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): TestCaseSource for Comparator_DeterministicTiebreaking Replace the [TestCase(\"Depth\")]+switch-on-string dispatch with a TestCaseSource that carries typed TopContractEntry pairs and the comparer method group directly. Each case gets a readable SetName so failure diagnostics point at the specific comparator under test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): address claude-review nits on test-dedup PR - Rename AssertAccountCumulativeEquals → AssertAccountTrieFieldsEqual and document that it intentionally skips storage-trie fields, so future callers don't mistake partial coverage for full coverage. - Drop the one-liner EmptyBaseline shim in StateCompositionStateHolderTests and route the 8 call sites directly to TestDataBuilders.EmptyBaseline — removes an indirection that defeats the single-source-of-truth goal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): drop unused System.Collections.Immutable using Left over from the earlier inline BuildStats that was replaced with TestDataBuilders.BuildStats — tripped IDE0005 in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * perf(statecomp): inline VisitorCounters fixed-size arrays Replace six managed arrays (DepthCounter[16] ×3, long[16] ×3) on VisitorCounters and the single-contract scratch buffer on SingleContractVisitor with InlineArray-backed structs (Long16, DepthCounter16). Each worker-local VisitorCounters now drops six heap allocations plus their GC tracking, and depth rows live contiguously with the enclosing struct for better cache locality on the per-node hot path. MaxTrackedDepth, Long16.Length, and CumulativeTrieStats.SlotHistogramLength all equal 16, so MergeFrom collapses into a single unrolled loop. * perf(statecomp): replace TrieDiffWalker byte-total arrays with scalars Replace the two long[2] tables (_trieBytesAdded, _trieBytesRemoved) with four named scalar fields: _accountBytesAdded, _accountBytesRemoved, _storageBytesAdded, _storageBytesRemoved. Drops two heap allocations per TrieDiffWalker and two bounds-checked indexer calls per node in the diff hot path, at the cost of one extra branch on isStorage in RecordNode. * perf(statecomp): inline CumulativeDepthStats per-depth rows Back the 9×16 counter matrix with a nested InlineArray (DepthRows9 of Long16) instead of a jagged long[9][16]. The full 144-long payload now lives contiguously inside the class instance with no per-row arrays, no array headers, and no GC tracking for the inner rows. - Drops ByDepth public accessor in favor of Span<long>-returning per-category accessors and GetRow(int) for indexed iteration (used by the snapshot decoder). - CloneAsDelta collapses from 10 allocations + 9 Array.Copy calls to a single struct copy (copy._rows = _rows). - AddInPlace / IsEmpty operate on a flat 144-long span instead of nine jagged array loops. - Metrics helpers take ReadOnlySpan<long> to match the new accessor type (no callsite changes; Span<long> implicitly converts). Wire format unchanged — the 9×16 rows still round-trip as 144 longs in the same slot order via DepthSlot / GetRow. Tests: 87/87 pass. * perf(statecomp): collapse refcount dict accesses with GetValueRefOrAddDefault Replaces the TryGetValue+indexer-set pair on CodeHashRefcounts with a single CollectionsMarshal.GetValueRefOrAddDefault call that yields a ref to the slot. One hash lookup per increment instead of two (hot path: VisitAccount per HasCode account; merge path: per code hash across all worker threads), and no boxing/temporary int. * perf(statecomp): collapse ApplyCodeHashChange increment with GetValueRefOrAddDefault Same optimisation as the visitor-side refcount collapse, applied to the incremental holder path. ApplyCodeHashChange runs once per account that gains code in each block diff, so the three dict operations (TryGetValue + indexer-set when existing, or +2 extra sets on first reference) collapse to one GetValueRefOrAddDefault lookup plus a ref increment. The `exists` flag replaces the newRefcount == 0 test to detect first-reference. * docs(statecomp): drop redundant comments on the new inline-storage types The XML docs on Long16/DepthCounter16 and the per-field comments on CumulativeDepthStats restated what the type signatures already said, and the new refcount sites had one-line comments narrating what GetValueRefOrAddDefault does (the name carries that). Keep only the WHY and the non-obvious invariants. * fix(statecomp): close BuildReport torn-state race and harden adjacent accessors - Coalesce SetBaseline/MarkScanCompleted/InitializeIncremental into a single PublishScanBaseline(...) holder method under one _lock acquisition so BuildReport cannot observe a half-published scan state (new distribution and metadata visible while _incrementalStats still reflects the pre-scan cumulative state). - Use Volatile.Read in StateCompositionVisitor.GetSnapshot so the progress logger cannot observe a torn 64-bit counter on non-x64 runtimes. - Narrow CurrentDepthStats from public to internal and document that callers must hold _diffLock for the duration they read the returned instance. - Add TryGetShutdownSnapshot so StopAsync reads HasIncrementalBaseline / LastProcessedStateRoot / IncrementalStats / IncrementalBlock under one lock. --------- Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
asdacap
pushed a commit
that referenced
this pull request
Apr 25, 2026
…cremental tracking (#10995) * feat: add StateComposition plugin with parallel trie visitor Introduce Nethermind.StateComposition plugin for state composition metrics collection (bloatnet benchmarking). Implements a parallel ITreeVisitor with ThreadLocal<VisitorCounters> for lock-free scaling to 64+ cores. Plugin provides: - 17 composition metrics (accounts, contracts, storage slots, trie node counts/bytes, branch occupancy) - Per-depth distribution tracking (16 levels, account + storage) - Progress callback every 1M accounts - All data models, config, and service interfaces for future PRs Includes 33 unit tests covering visitor behavior, data model correctness, ThreadLocal aggregation, and JSON round-trip. * feat: add statecomp_ RPC namespace with 6 endpoints and runner config Add JSON-RPC module for state composition metrics: - statecomp_getStats: full state scan at head block - statecomp_getScanProgress: scan progress with ETA - statecomp_getCachedStats: cached stats with staleness indicator - statecomp_getCacheMetadata: scan metadata (freshness, duration) - statecomp_getTrieDistribution: trie depth distribution - statecomp_getModuleInfo: API discovery endpoint Supporting infrastructure: - StateCompositionService: orchestrates scans via IStateReader with SemaphoreSlim concurrency guard and progress reporting - StateCompositionStateHolder: thread-safe baseline cache with BlocksSinceBaseline staleness tracking - CachedStatsResponse, ModuleInfo/EndpointInfo response types - statecomp-mainnet.json runner config (archive mode, 4GB memory) - Updated StateCompositionModule DI registrations * refactor: simplify StateComposition plugin per code review - Strip IStateCompositionConfig to 4 used fields (Enabled, ScanQueueTimeoutSeconds, ScanParallelism, ScanMemoryBudget) - Remove unused FromTrieStats, ModuleInfo, ScanProgressResult, progress plumbing - Extract MaxTrackedDepth constant, remove hardcoded magic numbers - Remove dead DI registration (InstancePerDependency visitor) - Remove BlocksSinceBaseline, UpdateHeadBlock (premature for this PR) - Simplify AnalyzeAsync (remove catch block, keep try/finally for semaphore) - Strip RPC to 4 endpoints: getStats, getCachedStats, getCacheMetadata, getTrieDistribution - Add projects to Nethermind.slnx under /Plugins/StateComposition/ - Update statecomp-mainnet.json config to match stripped config - 26/26 tests pass, 0 warnings, 0 errors * fix: address co-design review findings and add Geth feature parity Fix all critical and high severity issues from the multi-perspective co-design review. Add per-contract storage trie tracking with Top-N rankings and depth histogram to match Geth's inspect-trie functionality. Key changes: - Remove abstract from RpcModule, add [RpcModule(ModuleType.Statecomp)] - Add CancellationToken propagation and statecomp_cancelScan endpoint - Fix race condition in StateHolder.MarkScanStarted with proper locking - Fix semaphore release with acquired-flag pattern in service - Add config validation rejecting invalid values in service constructor - Remove uncollectable TotalCodeSize and duplicate byte-size fields - Add per-contract storage tracking: TopN by depth/nodes/slots - Add StorageMaxDepthHistogram for storage trie depth distribution - Convert CachedStatsResponse to readonly record struct - Inject IStateCompositionStateHolder interface instead of concrete type - GetTrieDistributionAsync now throws when not initialized - Cache aggregated counters to avoid duplicate computation - Add 15 new tests (TopN, histogram, cancellation, service, RPC) * feat: add Geth inspect-trie feature parity and remove Geth references - Add TopContractEntry with Owner, Levels[16], Summary fields - Add deterministic multi-field comparators (depth, totalNodes, valueNodes) - Add per-contract depth counters and storage max-depth histogram - Add InspectContract RPC endpoint for single-contract trie analysis - Add ExcludeStorage config option to skip storage traversal - Add scan cooldown (H2) and volatile CTS race fix (H1) - Add progress reporting via 8-second PeriodicTimer - Rename fields to Short/Full/Value terminology - Remove all Geth references and review-issue comments - Expand PluginBootstrapTests with comparator and TopN tests * feat: achieve 100% Geth inspect-trie parity with StateCompositionContext Introduce StateCompositionContext as custom INodeContext<T> that combines TreePath tracking with Level/IsStorage/BranchChildIndex fields. This closes the three remaining Geth parity gaps: - Owner hash: extract keccak256(address) from accumulated nibble path at VisitAccount time instead of passing default ValueHash256 - Comparator direction: fix Owner tiebreaker to match Geth's bytes.Compare (ascending) instead of inverted descending - Storage depth: reset Level to 0 in AddStorage so per-contract Levels[16] uses relative depth matching Geth's approach All 44 tests pass. * fix: security hardening and expanded test coverage for StateComposition - Fail-fast semaphore in AnalyzeAsync and InspectContractAsync - Move cooldown check inside critical section to prevent bypass - Fix CancelScan TOCTOU race via local variable capture - Add IDisposable on StateCompositionService for semaphore cleanup - Remove volatile+lock hybrid in StateHolder (lock-only) - Extract SingleContractVisitor to top-level class - Add custom StateCompositionException type - Add XML doc warning on mutable DepthCounter struct Add 14 new tests (58 total, 0 failures) covering AnalyzeAsync integration, InspectContract edge cases, ExcludeStorage mode, Owner hash preservation, cooldown/semaphore rejection, TopN eviction, and deterministic comparator tiebreaking. * fix: match Geth inspect-trie node counting conventions exactly Align NM reporting layer with three Geth shortNode/valueNode conventions: 1. Short = Extension + Leaf (Geth's shortNode covers both) 2. Value per-depth at depth+1 (Geth counts valueNode one level deeper than its parent leaf shortNode; Size stays at leaf's actual depth) 3. MaxDepth +1 and TotalNodes = physical nodes + leaves (Geth counts valueNode as an extra depth level and an extra node) Changes are reporting-only — internal DepthCounter tracking remains separated (ShortNodes=ext, ValueNodes=leaf) for correctness. Verified: 58/58 tests pass, 65/65 comparison checks at block 500K achieve 100% parity with Geth inspect-trie across all metrics. * feat: add 6 research distribution metrics to StateComposition Collect balance, nonce, storage-slot, and branch-occupancy distributions plus empty account count and top-contracts-by-size ranking from the existing trie walk with zero additional DB reads. New fields in TrieDepthDistribution: - BalanceDistribution (8 buckets: 0 | <0.01 ETH | ... | 10K+) - NonceDistribution (6 buckets: 0 | 1 | 2-10 | ... | 1K+) - StorageSlotDistribution (7 buckets: 1 | 2-10 | ... | 100K+) - BranchOccupancyDistribution (16 entries, children 1..16) New fields in StateCompositionStats: - EmptyAccounts - TopContractsBySize Includes 7 new tests covering all distribution buckets, boundary values, empty account counting, and top-by-size ranking. * refactor: apply co-design review fixes to StateComposition plugin - Refactor service layer to return Result<T> instead of throwing exceptions, matching Nethermind's DebugRpcModule pattern - Update RPC module to deconstruct Result<T> with proper error codes - Extract TopNTracker from VisitorCounters for SRP (H3) - Make VisitorCounters internal (L5) - Add address null-check on inspectContract endpoint (H2) - Dynamic ScanParallelism default: ProcessorCount/2 clamped 1-16 (M4) - Document TopContractsBySize as Nethermind extension (M2) - Revert exception subtypes to base class only - Add 16 new tests: Geth convention regressions (7), multi-threaded merge (2), SingleContractVisitor (5), cancellation semantics (2) - Fix stale SingleContractContext type reference in service tests All 58 tests passing. * refactor: clean up StateComposition plugin after co-design review - Remove ScanCooldownSeconds config and cooldown logic - Remove unused IsScanning property from state holder - Remove unused StateCompositionException class - Remove Balance/Nonce/StorageSlot distribution metrics (not in Geth) - Fix GetTrieDistributionAsync to use cached data (no params needed) - Add CODEOWNERS entries for StateComposition plugin - Split test classes into 1-class-per-file (8 test files, 77 tests) - Modernize assertions: Assert.EnterMultipleScope, collection expressions - Modernize NSubstitute: null! instead of default! for ref args All 77 tests passing. Build: 0 warnings, 0 errors. Verified 100% metric parity with Geth inspect-trie at block 500k (2,064/2,064 per-contract fields match across all 3 rankings). * format * feat: add live scan progress metrics to periodic log Report accounts/s, slots/s, nodes/s and data throughput every 8s during state composition scan. Uses ScanSnapshot with mid-scan ThreadLocal counter aggregation via Volatile.Read. Follows Nethermind conventions: SizeExtensions.SizeToString for bytes, VisitorProgressTracker-style M/K formatting for counts. * test: add scan consistency tests for content-addressed trie isolation Validate that StateComposition scans produce correct results regardless of concurrent block processing, leveraging the content-addressed trie's natural snapshot isolation. Tests cover: - Scan at older root returns original counts after new commits - Isolation after in-place account modifications - Trie node counts reflect original structure per root - Multiple historical roots all scan correctly - Concurrent scan and commit complete without deadlock - Service-layer isolation through full scan pipeline - Sequential scans update state holder correctly * feat: add TrieDiffWalker for exact incremental state composition diffs Replaces the flawed ITrieStoreListener approach (monotonic overcounting) with a recursive diff walker that walks both old and new state roots to compute exact adds AND removes — zero approximation. - TrieDiff.cs: immutable result struct with separate Added/Removed fields - CumulativeSizeStats.cs: cumulative stats with ApplyDiff and FromScanStats - TrieDiffWalker.cs: recursive diff algorithm, skips identical subtrees by hash - TrieDiffWalkerTests.cs: 26 tests including multi-block scan/diff/scan verification Key fix: FromScanStats correctly maps Extensions = ShortNodes - ValueNodes (ShortNodes in Nethermind's visitor includes both extensions AND leaves). * feat: add RocksDB persistence for incremental state composition stats Persist CumulativeSizeStats snapshots to a dedicated stateComposition RocksDB database for warm restart and historical queries. On startup, the plugin restores incremental tracking from the latest valid snapshot, eliminating the need for a fresh 30+ min scan after node restart. - StateCompositionSnapshot: persisted record with stats, block, root - StateCompositionSnapshotDecoder: RLP encode/decode (~140 bytes/entry) - StateCompositionSnapshotStore: DB access with sentinel key for O(1) latest - StateCompositionSnapshotPruner: prunes entries older than configurable window - Plugin warm restart: validates snapshot root against canonical chain - statecomp_getStatsAtBlock RPC: query historical stats by block number - Config: PersistSnapshots, SnapshotBlocksToKeep (10k), SnapshotInterval * feat: add Prometheus metrics for state composition plugin 19 metrics auto-discovered by Nethermind's monitoring system: - 11 cumulative state gauges (accounts, contracts, slots, trie nodes/bytes) - 4 operational gauges (incremental block, diffs count, scan duration/block) - 2 counters (scans completed, diffs applied, diff errors) - 2 scan-only gauges (contracts with storage, empty accounts) Metrics updated on scan completion, each incremental diff, and warm restart. * chore: regenerate packages.lock.json after Nethermind.Init reference * fix(state-composition): force-resolve service to wire NewHeadBlock subscription StateCompositionService and StateCompositionSnapshotPruner subscribe to IBlockTree.NewHeadBlock in their constructors, but Autofac registers them as lazy singletons. Until something resolved them (e.g. an RPC call), the constructors never ran and the event handlers were never wired. As a result incremental diff metrics never updated and the snapshot pruner never ran. Force-resolve both in InitRpcModules so the subscriptions are active from node startup, regardless of whether snapshot persistence is enabled. * feat(state-composition): track ContractsWithStorage and EmptyAccounts incrementally Both metrics were previously updated only on full scans, leaving them frozen between scans. Promote them to first-class CumulativeSizeStats fields so the trie-diff walker maintains them on every new head block. - CumulativeSizeStats: add ContractsWithStorage / EmptyAccounts fields, applied via TrieDiff and seeded from full-scan stats. - TrieDiff: add *Added/*Removed counters and Net* helpers. - TrieDiffWalker: replace DecodeAccountHashes with TryDecodeStruct and count HasStorage / IsTotallyEmpty transitions, matching the visitor semantics. CollectLeaf increments the counters for new/removed leaves. - Metrics.UpdateFromCumulativeStats now wires both gauges, so the service no longer needs the explicit scan-only assignments. - Snapshot RLP gains 2 longs; legacy snapshots fail to decode and the plugin catches the exception, discards them, and triggers a fresh scan to rebuild the baseline with the new schema. * feat(state-composition): expose trie depth distribution as Prometheus gauges Add 149 new [GaugeMetric] properties covering: - 5 scalars: avg/max account/storage depth, avg branch occupancy - 64 account trie per-depth gauges (full/short/value nodes + bytes, depths 0..15) - 64 storage trie per-depth gauges (same layout) - 16 branch occupancy histogram buckets (1..16 children) Metrics are populated on full scan completion from the existing TrieDepthDistribution cached by the visitor. Between scans values stay flat -- incremental per-block tracking lands in a follow-up. All properties use explicit underscore separators around depth digits (e.g. Depth_7_FullNodes) because MetricsController's PascalCase to snake_case conversion only triggers on lowercase to uppercase transitions, which would otherwise fuse the digit into adjacent words. Snapshot restore path leaves depth gauges at zero with a comment -- the current snapshot schema does not persist TrieDepthDistribution. * feat(state-composition): track trie depth distribution incrementally via diff walker Phase B of live trie-depth metrics. TrieDiffWalker now threads depth through its recursive descent and emits a DepthDelta per block, which StateCompositionStateHolder applies to a new CumulativeDepthStats. The 149 depth gauges added in Phase A are now refreshed on every new head block instead of only on full scans. - CumulativeDepthStats: mutable per-depth arrays (account/storage Full/Short/Value/Bytes + BranchOccupancy) seeded from scan via SeedFromScan (reverses Geth +1 shift) and applied in place under the state-holder lock. - DepthDelta: reusable per-block delta, cleared between diffs. - TrieDiffWalker: threads int depth through DiffSubtree/DiffNodes/ DiffBranches/DiffExtensions/DiffLeaves/CollectSubtree; branch children depth = d+1, extension children depth = d + key.Length; storage tries reset to 0. - Metrics.UpdateFromDepthStats: applies Geth conventions at read time (ValueNodes[d] reads AccountValueNodes[d-1]; MaxStorageDepth += 1). - TrackDepthIncrementally config flag (default true) gates the walker overhead for benchmarks. Tests: 127 pass (+13 new). CumulativeDepthStatsTests covers seed/apply/reset/clone parity with scan-derived gauges. TrieDiffWalker tests assert depth delta is null when disabled, per-depth buckets shift correctly on leaf/branch add/remove, and ShortNodes honors the Extension+Leaf Geth convention. * fix(state-composition): persist depth stats in snapshot and gate cold-start deltas Prevent negative per-depth gauges that appeared after restart when a pre-Phase-B snapshot was restored: the depth arrays were zero-seeded and the first incremental diffs that removed nodes pushed gauges below zero. - CumulativeDepthStats.IsSeeded flag: ApplyDelta is a no-op until a baseline is installed via SeedFromScan or SeedFromSnapshot. Metrics.UpdateFromDepthStats short-circuits on unseeded input so gauges stay at their cold-start zero until a fresh scan or a depth-carrying snapshot is loaded. - StateCompositionSnapshot gains an optional CumulativeDepthStats payload; the RLP encoder writes a leading present/absent marker followed by 162 longs when seeded. Legacy snapshots fail to decode and are discarded by the existing plugin try/catch, triggering a fresh scan that seeds with the new schema. - StateCompositionService persists the current depth stats on both the scan-complete and periodic snapshot writes; the plugin's restore path replays them into the state holder and calls UpdateFromDepthStats so gauges come up correct across restarts with no zero-window. - Tests: new NewSeededEmpty() helper so ApplyDelta/Clone tests continue to exercise delta arithmetic on a (now-required) seeded baseline. * refactor(state-composition): tighten plugin after multi-agent review Tranche A — delete dead abstractions and unused tests: - remove IStateCompositionService, IStateCompositionStateHolder interfaces (single impl each, blocked test doubles without adding value) - fold StateCompositionSnapshotPruner into StateCompositionService (one caller, tight coupling to snapshot write cadence) - replace throw-on-invalid-config with clamp + warn log; node should not fail to start over plugin config nits - drop try/catch in StateCompositionVisitor.VisitBranch (underlying TrieNode child loop cannot throw on well-formed RLP); replace with null-guard - remove spurious Volatile.Read in GetSnapshot (Task.WaitAll happens- before already guarantees visibility) - delete 9 trivial plugin-bootstrap tests, 5 redundant visitor tests, and the diagnostic "isolate extension undercount" region now that the bug it was chasing is fixed - move 59-line depth-gauge reset into MetricsDepthGaugesHelper Tranche B — perf + missing coverage: - switch TrieDiffWalker.DiffMismatchedNodes dictionary key from Hash256 to ValueHash256, and read leaf full-path as ValueHash256 directly (eliminates per-leaf Hash256 allocation on the hot diff path) - add cancellation-mid-scan test with ManualResetEventSlim gating - add reorg-rollback test (forward diff then backward diff must restore exact baseline across all 9 cumulative fields) - add cross-semaphore test documenting that InspectContractAsync runs independently of a blocked AnalyzeAsync Tranche C — hot-path perf wins without framework changes: - cache VisitorCounters on StateCompositionContext so per-node ThreadLocal.Value lookup fires once per root/worker instead of once per visited node - replace BuildSortedTopN lambda-captured comparer with a DescendingComparer struct (avoids delegate + closure per scan) - lazy-allocate TrieLevelStat scratch in VisitorCounters and short-circuit the per-contract finalize via TopNTracker.WouldInsert so only ranking contracts pay the ImmutableArray freeze cost - DepthDelta.IsEmpty() early-out skips UpdateFromDepthStats when the diff walker produced no depth changes - misc: RLP long-array encode/decode helpers replace the 162-field unrolling in StateCompositionSnapshotDecoder; CumulativeDepthStats exposes MarkSeeded() so the decoder no longer round-trips a sentinel Net: -474 LOC, 112 tests passing (+3 new), 0 build warnings. No behavioural change except config clamping and faster hot paths. * refactor(state-composition): reorganize plugin into functional subdirectories Move 19 files into Data/, Rpc/, Visitors/, Diff/, Service/, Snapshots/ subdirectories with folder-matching namespaces to align with sibling Nethermind plugins (Merge.Plugin, JsonRpc, Optimism). Mirror the same layout under Nethermind.StateComposition.Test. Split the two largest files via partial class: - TrieDiffWalker.cs (881 lines) → core + Branches/Extensions/Leaves/ Collection/Depth partials - StateCompositionService.cs → core + Incremental partial Reduce complexity hotspots via pure Extract Method refactors: - DiffBranches: extract DiffBranchChild + CollectBranchSlotSide (CC ~18 → ~6, nesting 5 → 3) - AnalyzeAsync: extract ResolveScanOptions, StartProgressLogging, PublishScanResults - OnNewHeadBlock: extract RunIncrementalDiff + MaybeWriteSnapshot - FinalizeCurrentStorageTrie: extract BuildCurrentStorageLevels + RankCurrentContract Merge IStateCompositionConfig.cs into StateCompositionConfig.cs (-1 file). Tighten StateCompositionRpcModule to internal sealed to match the internal service/state-holder it depends on. No behavior change. 112/112 tests passing. * refactor(state-composition): strip trivial and redundant comments Remove ~82 comment lines across 22 files: divider bars in Metrics.DepthGauges.cs, field-decoration restatements, obvious control-flow narration, and redundant XML <summary> blocks that just echoed method names. Preserved Geth convention notes, concurrency/lock invariants, and non-obvious WHY comments. No behavior change. 112/112 tests passing. * feat(state-composition): add CodeBytesTotal and per-contract slot histogram Aggregate on-chain bytecode deduplicated by codeHash (proxies and minimal clones contribute once) and a log-bucketed per-contract slot-count histogram. Both are produced by the full-scan visitor, fanned out as Prometheus gauges, and persisted through the snapshot schema so restarts resume the last baseline instead of dropping to zero. Freeze pattern: neither field can be maintained by the incremental diff walker without a refcount map, so ApplyDiff uses `this with { ... }` to carry them forward unchanged until the next scan refreshes them. SlotHistogramLength is defined once on CumulativeSizeStats and shared by producer and decoder so their wire length cannot drift. * refactor(state-composition): collapse depth metrics into labeled gauges Replace the 149 flat per-depth Prometheus properties and 16 per-bucket slot-count properties with four [KeyIsLabel] dictionaries, matching Nethermind's native labeled-gauge pattern. Fan-out is now driven by a single UpdateDepthDistribution publish instead of UpdateFromDepthStats + UpdateFromDistribution. Behavior is preserved: the IsSeeded cold-start gate, Geth +1 ValueNode presentation shift, and slot/code histogram exposure all carry over. * refactor(state-composition): lint cleanup and fix cancellation test - Remove unused usings in service and incremental partial classes - Test file modernization: using-order, cts.CancelAsync, spelling - Fix AnalyzeAsync_CancelledMidScan: mock now waits on cts.Token directly instead of a stale CancellationToken.None snapshot that never signaled * feat(state-composition): incremental updates for CodeBytesTotal and slot histogram Thread per-account payloads (SlotCountChanges, CodeHashChanges) through TrieDiff so the state holder can refcount CodeBytesTotal and move contracts between slot-histogram buckets on every NewHeadBlock. Full scans seed the trackers; snapshots persist them across restarts. Loading a snapshot without trackers triggers a fresh rescan. Adds 11 unit tests covering refcount edge cases (shared bytecode add / drop, swap) and snapshot round-trip for the three tracker maps. * fix(state-composition): enable plugin via primary-ctor config injection Default to disabled and inject IStateCompositionConfig via primary constructor so PluginLoader sees the real Enabled value at enumeration time instead of null (pattern match failed, plugin never initialized). Matches the TraceStorePlugin pattern. * fix(state-composition): use structural depth in TrieDiffWalker DiffExtensions and CollectSubtree incremented depth by the extension key length, while the baseline visitor's StateCompositionContext.Add uses Level+1 regardless of path length. The mismatch routed diff bytes into nibble-depth buckets while the baseline seeded structural-depth buckets, drifting the Trie Depth Distribution histogram (observed as a negative 6-byte value at account depth 13 with a 172-node bucket). Aligns both Extension paths in TrieDiffWalker to depth+1. * fix(state-composition): auto-recover when baseline root is pruned On restart, the persisted snapshot seeds LastProcessedStateRoot with the old block's root. If the container was stopped longer than the pruning window, that root is no longer in the trie DB, so every OnNewHeadBlock hit MissingTrieNodeException via TrieDiffWalker.ComputeDiff and spammed diff_errors indefinitely until an operator manually re-ran statecomp_getStats to reseed. Narrow the catch in RunIncrementalDiff: on MissingTrieNodeException, invalidate the baseline (LastProcessedStateRoot=null silences the null gate in OnNewHeadBlock), bump a dedicated StateCompBaselineInvalidations counter, and fire-and-forget AnalyzeAsync — the existing _scanLock coalesces back-to-back triggers, so at most one real scan runs. Generic exceptions keep the old diff_errors path unchanged so alerting still surfaces real bugs. Tests cover the narrow holder invalidation, the MissingTrieNode recovery path (counter bump + auto-rescan reseed), and a regression guard for generic exceptions keeping the legacy counter behaviour. * refactor(state-composition): atomic BuildSnapshot + require init StateCompositionStateHolder.BuildSnapshot captures stats, depth stats, and the three tracker dictionaries under a single lock entry so the persisted snapshot cannot tear against a concurrent InitializeIncremental or diff application. Replaces three separate Clone* accessors (each taking the lock individually) and collapses six lock entries in MaybeWriteSnapshot / PublishScanResults to one. Also removes the unused HasIncrementalTrackers probe and sets MustInitialize => true so the host enforces plugin startup order. * refactor(state-composition): drop redundant getCacheMetadata RPC statecomp_getCacheMetadata returned the same ScanMetadata? already exposed as CachedStatsResponse.LastScanMetadata on statecomp_getCachedStats. Delete the redundant endpoint and its test; callers migrate by reading the LastScanMetadata field on the cached-stats response. Also collapse the getCachedStats builder: four separate holder lock entries (IncrementalStats, IncrementalBlock, DiffsSinceBaseline, LastScanMetadata) become a single BuildCachedStatsResponse() accessor that captures all four under one lock, matching the atomic-by-design pattern introduced for BuildSnapshot. Document AnalyzeAsync's two legal call sites (operator RPC + MissingTrieNodeException recovery) so future readers cannot accidentally add a third scan dispatcher. * refactor(state-composition): transfer visitor maps on GetStats StateCompositionVisitor is internal sealed, IDisposable, and used exactly once under a `using` in StateCompositionService. After GetStats returns, the visitor is disposed and the holder has already taken its own defensive copy inside InitializeIncremental, so the two deep-copy blocks for _codeHashSizes and agg.CodeHashRefcounts were defending against a second GetStats call that cannot happen. Hand those maps through directly — the fields on StateCompositionStats are typed as IReadOnlyDictionary<>, so ConcurrentDictionary<> flows through without an intermediate materialization. SlotCountsByOwner still needs the list→dict foreach loop: the zero-owner sentinel can appear more than once across worker threads, and `new Dictionary(list)` throws on duplicate keys. * refactor(state-composition): extract LevelStatsBuilder helper Three sites hand-rolled the DepthCounter[] → TrieLevelStat[] conversion, each carrying its own copy of the Geth +1 valueNode depth shift: - StateCompositionVisitor.BuildLevelStats (filtered → ImmutableArray) - VisitorCounters.BuildCurrentStorageLevels (fill scratch + summary) - SingleContractVisitor.GetResult (fill + summary inlined) Consolidate the row-construction + depth shift into LevelStatsBuilder with two entry points: Fill(depths, dest) for fixed-buffer callers that also need the summary row, and BuildCompact(depths) for the RPC depth distribution which filters out empty levels. SingleContractVisitor now allocates a concrete array and wraps it via ImmutableCollectionsMarshal.AsImmutableArray — same no-copy handoff pattern already used by BuildSortedTopN. * refactor(state-composition): collapse decoder map helpers Fold the three parallel (slot-count, int-refcount, int-size) map encode/length/decode helper triples into single generic helpers parameterised by value type, and inline the per-depth-array encode loops behind a shared DepthArrays() list. Nullable map handling stays inside the generic helpers (count=0 + skip) so legacy null-tracker snapshots still round-trip, but the three call sites collapse from nine hand-rolled helpers to three. * refactor(state-composition): merge DepthDelta into CumulativeDepthStats The two types had identical layout (9 long[16] + 2 scalars). Fold DepthDelta into CumulativeDepthStats by replacing ApplyDelta(DepthDelta) with AddInPlace(CumulativeDepthStats) and retype TrieDiff.DepthDelta. Delete DepthDelta.cs entirely. Snapshot schema is unchanged (same field layout, same decoder). Plan M6 — net -40 LOC. * refactor(state-composition): back CumulativeDepthStats with long[9][16] + DepthSlot enum Replace nine parallel long[16] fields with a single jagged long[9][16] row array indexed by a new DepthSlot enum. Every cumulative operation (Reset, Clone, AddInPlace, IsEmpty, SeedFromSnapshot) collapses from nine field-by-field statements to a single loop. The snapshot decoder drops the DepthArrays helper and iterates ByDepth directly. Pass-through properties (AccountFullNodes, BranchOccupancy, etc.) are kept so Metrics.UpdateDepthDistribution, TrieDiffWalker.Depth, and the test suite still read via the named rows. Schema-compatible: the RLP layout is unchanged because DepthSlot pins the same numeric order as the old field list. Plan M7 — jagged layout gate; 136/136 tests pass in 6.9s (no regression). * refactor(state-composition): share trie walker across collect paths CollectSubtree and CollectSubtreeForDiff duplicated the entire branch/ extension traversal including depth and RecordNode bookkeeping — only the leaf handling differed. Extract the shared walker into WalkStructure<TH> parameterised by a struct ILeafHandler so the JIT specialises per handler type with no delegate allocation on the hot path. SemanticLeafHandler counts accounts/contracts/slots and recurses into storage tries; DictionaryLeafHandler stores leaves for deferred matching. The two public entry points shrink to ~5-line wrappers. Plan M8 — 136/136 tests pass in 6.9s (no regression). * refactor(state-composition): strip neuroslop xmldoc (H8) Remove xmldoc blocks that restate method/class names without documenting WHY. Keep summaries that pin non-obvious invariants: IsSeeded rationale, Prometheus label naming, BranchOccupancyDistribution. * refactor(state-composition): drop DescendingComparer struct (H4) * refactor(state-composition): collapse TryInsert/WouldInsert duplication (H5) * refactor(state-composition): extract ClampWithWarn helper for scan option clamping (H7) * refactor(state-composition): flatten redundant HasCode/HasStorage guard in CollectLeaf (M8) * refactor(state-composition): bind progress logger to linked CTS token (H6) * refactor(state-composition): use Dictionary for per-contract slot counts (H3) * refactor(state-composition): collapse TrieDiffWalker parallel counters into 2D tables (H1) * refactor(state-composition): single-pass snapshot encoder via EncodeOrLength helper (H2) * feat(state-composition): register RLP snapshot decoder via InitTxTypesAndRlpDecoders (V1) * feat(state-composition): prepend schema version byte to snapshot encoding (V2) * refactor(state-composition): volatile scan CTS and AutoActivate service (V4, V7) * test(state-composition): trim VisitorCounters tests and move under Visitors namespace Rename PluginBootstrapTests.cs -> Visitors/VisitorCountersTests.cs to match the namespace of the type under test. Delete four tests covered by existing broader assertions (trivial DepthCounter getters, Flush no-op, and the by-value-nodes Top-N test which is now covered in aggregate by the merge and insert tests). Collapse three Comparator_*_DeterministicTiebreaking tests into one [TestCase]-parameterized method. * test(state-composition): drop tests subsumed by broader diff coverage Remove seven tests whose invariants are covered by existing broader tests: BothRootsNull/BothRootsEmptyTreeHash (SameRoot covers zero-diff semantics), EmptyToSingleAccount/SingleAccountToEmpty (add/remove is exercised by the multiple-account and ModifyAccountBalance tests), Walker_CanBeReused_ForMultipleDiffs (reuse is verified every block by MultiBlockIncremental_MatchesFullScan), and the two trivial DepthDelta null/not-null toggles (the AddOneLeaf test already asserts non-null when trackDepth=true). * test(state-composition): drop CancellationTests white-box ShouldVisit probes These two tests peek at the visitor's ShouldVisit gate after cancelling a CancellationTokenSource. The end-to-end cancellation behaviour (RPC CancelScan + service scan abort) is already covered by StateCompositionServiceTests and StateCompositionRpcModuleTests, so the low-level probes are pure maintenance cost. * test(state-composition): parameterize constructor zero-config tests Collapse the three Constructor_ClampsZero{Parallelism,MemoryBudget,TopN} tests into one [TestCase]-parameterized test. The constructor does not actually clamp — clamping happens lazily inside ResolveScanOptions on AnalyzeAsync — so the three tests all verified the same thing: that the constructor accepts a zero without throwing. * test(state-composition): drop trivially subsumed visitor tests Visitor_ShouldVisit_AlwaysReturnsTrue is a single-line sanity check with no coverage value beyond what the non-cancelled branch of the remaining ShouldVisit tests already exercises. Visitor_CountsAccountsCorrectly is fully subsumed by Visitor_ClassifiesContracts, which asserts the same AccountsTotal invariant on a richer input. * test(state-composition): drop chained freeze-fields regression ApplyDiff_Chained_StillPreservesFreezeFields iterates the preserved freeze semantics three times over the top of what ApplyDiff_PreservesCodeBytesAndSlotHistogram already establishes. If a single ApplyDiff leaves CodeBytesTotal and SlotCountHistogram untouched then chaining N diffs cannot break that — the assertion is structurally redundant. * refactor(state-composition): delete statecomp_getStats RPC Plan pass 2 commit 1 — collapse to single operating mode by removing the only operator-initiated rescan entry point. statecomp_getCachedStats remains as the read-only stats endpoint. Also adds a tripwire doc comment above AnalyzeAsync naming the two legal callers (plugin bootstrap + RunIncrementalDiff recovery) so a future contributor adding a third caller has to delete the comment explicitly. * feat(state-composition): flush snapshot on graceful shutdown Plan pass 2 commit 2 — stop persisting a snapshot on every block. - StateCompositionService now implements IStoppableService. StopAsync cancels any in-flight scan, acquires _scanLock, and force-flushes the latest incremental state through a new WriteSnapshotForHead helper that every snapshot write routes through. - Default SnapshotInterval bumped from 1 to 1024. Per-interval writes are a crash-safety fallback; IServiceStopper guarantees StopAsync runs before the snapshot RocksDB is disposed on SIGTERM/docker stop. - ServiceStopperMiddleware auto-registers any singleton implementing IStoppableService, so StateCompositionModule needs no change. * refactor(state-composition): kill defensive copies on the holder hot paths Plan pass 2 commit 3 — eliminate copies that were defending against mutation that cannot happen with the single-writer invariant. - StateCompositionStateHolder.BuildSnapshot hands ownership of the slot, refcount, and code-size dictionaries directly to the snapshot record; the snapshot is persisted and discarded with no downstream reader, so the three defensive Dictionary copies are pure waste. - StateCompositionStateHolder.InitializeIncremental drops the conditional-copy-if-not-null branches on the snapshot dictionaries — the decoder always materialises fresh Dictionary instances. - StateCompositionStats and StateCompositionSnapshot expose the concrete Dictionary type for the three ownership-transfer hand-off fields so the holder can take ownership without an IReadOnlyDictionary wrapper allocation. CumulativeDepthStats.Clone is removed (no live callers). - StateCompositionVisitor returns the visitor's owned dictionary refs directly instead of cloning during GetStats; the visitor is one-shot and disposed immediately after. * refactor(state-composition): kill optional-field nullability with sentinels Replace nullable cold-start gates with explicit sentinels and flags so metric semantics stay intact while the surface area shrinks: - StateCompositionStateHolder: _lastScanMetadata, _incrementalStats, _lastProcessedStateRoot become non-nullable; new IsIncrementalSeeded bool flag gates incremental presence; LastProcessedStateRoot returns Hash256.Zero when invalidated. - ScanMetadata.IsComplete becomes the single freshness gate (record default has IsComplete=false, replacing the ScanMetadata? wrapper). - TrieDiff.DepthDelta / SlotCountChanges / CodeHashChanges become required positional members; introduce TrieDiff.Empty as the no-op early-return value so the walker no longer constructs nulls. - StateCompositionSnapshot drops nullable wrappers around DepthStats and the three tracker dictionaries; decoder always materialises an unseeded CumulativeDepthStats and empty maps when fields are absent. - IsSeeded on CumulativeDepthStats is preserved — it's a load-bearing guard against negative metrics on cold replay, not a cold-start gate. Tests updated: SnapshotRoundTripTests gains a BuildSnapshot helper that fills the new required positional args; assertions against now-non-null fields drop the redundant null checks; TrieDiffWalkerTests SameRoot_ReturnsZeroDiff compares against TrieDiff.Empty instead of default(TrieDiff). * test(state-composition): trim redundant single-node probes Drop white-box TrieDiffWalker probes (sections 1, 2, 4-7, 9) and the DepthDelta unit probes: all covered end-to-end by the integration fixtures (MultiBlock_ScanDiffScan_CumulativeMatchesFreshScan, LargeTrie_IncrementalMatchesFullScan, StorageTrieIncremental_MatchesFullScan, ReorgRollback_ForwardThenBackward). Drop no-state StateCompositionService probes (constructor zero-arg, GetTrieDistribution cold-start, CancelScan no-op, InspectContract no-data) — no behavior worth guarding; the concurrency and happy-path tests remain. Symmetry, ApplyDiff_RoundTrips, FromScanStats, and the storage-trie sanity tests stay. Tests: 96 passed, 0 failed (was 117). * refactor(state-composition): collapse lock-wrapped property getters Shrink seven single-statement `lock (_lock) return _field` getters in StateCompositionStateHolder from four lines each to one. Pure cosmetic pass — no behavior change, 21 LOC saved. * fix(state-composition): close shutdown-flush race and wire bootstrap scan Two concerns surfaced by reviewer on top of the shrinkage pass: 1) StopAsync wrote a torn snapshot under load. The capture sequence read IncrementalStats, IncrementalBlock, and LastProcessedStateRoot under separate holder-lock acquisitions, and BuildSnapshot then handed the live tracker dictionaries to the RLP encoder while the diff path could still mutate them through _diffLock. Fix: - Unsubscribe NewHeadBlock at the top of StopAsync to stop new diff dispatches (delegate -= is idempotent so Dispose is safe). - Acquire _diffLock around the read+write in StopAsync to drain any in-flight diff and exclude the encoder race. - Wrap InitializeIncremental + WriteSnapshotForHead in PublishScanResults with the same lock — the same race exists between the scan baseline install and the first head block. Lock order is _scanLock -> _diffLock everywhere; the diff path only takes _diffLock, so the nested acquire is deadlock-free. 2) StateCompositionPlugin.Init never called AnalyzeAsync, so a cold-start node with no persisted snapshot stayed at zero forever once statecomp_getStats was deleted. Wire ScheduleBootstrapScan in Init for the no-snapshot, stale-snapshot, and no-PersistSnapshots paths and update the AnalyzeAsync tripwire doc to match. * chore(state-composition): drop refactor-history comment Remove the trailing 'now part of CumulativeSizeStats' note on the PublishScanResults metric publish — the call to UpdateFromCumulativeStats on the next line documents itself. * fix(state-composition): write snapshot only on shutdown, purge old entries Snapshot data was accumulating unbounded on mainnet (~2 TB) because periodic writes every N blocks retained thousands of large entries. - Remove MaybeWriteSnapshot (periodic interval writer) and the scan-completion write from PublishScanResults - StopAsync is now the sole snapshot write path - WriteSnapshot deletes the previous entry before writing the new one so only one snapshot ever exists in the DB - Add PurgeOldEntries() called on startup to clean up legacy data - Remove trivial comments that duplicate self-explanatory code * chore(state-composition): remove trivial comments from test files Drop region tags, section labels, and comments that restate self-explanatory code across 8 test files. Kept XML docs explaining Geth conventions, isolation properties, freeze semantics, and race-condition design decisions. * test(state-composition): delete ScanConsistencyTests These tests validated content-addressed trie isolation — a property of Nethermind's StateTree, not the plugin. The plugin never scans at historical roots; it always operates on the current head. * test(state-composition): remove debug report file and impossible-scenario test Remove hardcoded /private/tmp/claude/ report-file writing from MultiBlock_ScanDiffScan test — TestContext.Out and Assert already cover the same output. Delete UpdateDepthDistribution_UnseededStats_Noop since UpdateDepthDistribution is never called with unseeded stats in production (all call sites are post-scan). * test(state-composition): merge 4 snapshot round-trip tests into one Consolidate RoundTrip_PreservesCodeBytesTotal, PreservesSlotCountHistogram, PreservesSlotCountByAddressTracker, and PreservesCodeHashTrackers into a single RoundTrip_PreservesAllFields test with EnterMultipleScope. Adds coverage for BlockNumber, DiffsSinceBaseline, and ScanBlockNumber fields that were previously untested. * test(state-composition): delete redundant Geth convention tests Remove Convention3, Convention4, Convention5, and Convention1_PerDepthLevels tests — all fully covered by AllConventions_StorageTrie_Comprehensive. Keep Convention1 and Convention2 account-trie tests (different code path through StateCompositionVisitor vs VisitorCounters). * refactor(state-composition): tighten visibility and seal internal types Make 8 types internal (StateCompositionStats, TrieDiff, SlotCountChange, CodeHashChange, StateCompositionContext, VisitorCounters, DepthCounter, StateCompositionSnapshotStore). Make ApplyDiff, FromScanStats, UpdateFromCumulativeStats, and UpdateDepthDistribution internal. Seal StateCompositionService and StateCompositionSnapshotStore. Remove unnecessary virtual on CancelScan and replace FakeService test subclass with a factory method. * fix(state-composition): use DiffMismatchedNodes for extension prefix mismatch, reuse walker - Extension prefix mismatch now routes through DiffMismatchedNodes instead of independent CollectSubtree calls, eliminating spurious CodeHashChange and SlotCountChange events for leaves shared across both subtrees - Move ITrieNodeResolver from TrieDiffWalker constructor to ComputeDiff parameter, allowing the walker to be reused across blocks without reallocating internal lists, counter tables, and depth stats arrays - Store a single TrieDiffWalker instance on StateCompositionService, protected by _diffLock - Fix two broken [with(...)] collection expressions left by a previous linter pass in StateCompositionSnapshotDecoder and StateCompositionVisitor * refactor(state-composition): delete statecomp_getStatsAtBlock RPC Historical snapshot lookup has no production consumer — the plugin only needs the latest snapshot for restart. Removes the endpoint from the interface and module, drops the snapshotStore dependency from the RPC module constructor. * refactor(state-composition): merge RPC endpoints into statecomp_get Unite statecomp_getCachedStats and statecomp_getTrieDistribution into a single statecomp_get endpoint. CachedStatsResponse now includes the TrieDepthDistribution field. Delete the separate GetTrieDistribution service method. * docs: clarify Geth vocabulary parity in trie node naming Replace cryptic "Short=Extension, Full=Branch, Value=Leaf" one-liners with proper doc comments that reference go-ethereum/trie/inspect.go and explain the dual vocabulary is intentional Geth parity, not confusion. * refactor: rename confusing types and fields for clarity - CachedStatsResponse → StateCompositionReport (not "cached", always live) - CumulativeSizeStats → CumulativeTrieStats (tracks counts + bytes, not just sizes) - CurrentStats → TrieStats (in report) - DiffsSinceLastScan → DiffsSinceBaseline (matches internal field name) - IsInitialized → HasScanBaseline (clarifies what it gates) - IsIncrementalSeeded → HasIncrementalBaseline (clarifies what it gates) - BuildCachedStatsResponse() → BuildReport() - CumulativeDepthStats.SlotCount → CategoryCount (avoids storage slot clash) * fix(state-composition): address PR review feedback Address findings from Claude bot review on PR #10995: - Register StateCompositionService as IStoppableService so graceful shutdown actually flushes the snapshot (StateCompositionModule.cs). - Remove catch-when(logger.IsError) anti-pattern in bootstrap scan and auto-rescan paths; guard the log call inside the catch so exceptions are never rethrown into the unobserved-task pipeline. - Drop the dead SnapshotBlocksToKeep config option. - Bound statecomp_inspectContract with a per-call CancellationTokenSource using the new InspectContractTimeoutSeconds config (default 30s) so long storage-trie walks don't pin RPC workers indefinitely. - Defensive CloneAsDelta() on TrieDiff.DepthDelta so consumers don't share a mutable reference with the walker. - Document LatestKey sentinel collision window in SnapshotStore. * style(state-composition): fix whitespace around spread operator Add the required space after '..' in collection spreads so 'dotnet format whitespace --verify-no-changes' passes the CI gate. * style(state-composition): trim waste in two comments Drop the lines that just restated code: the sidebar about SlotCountChanges/CodeHashChanges in TrieDiffWalker.CreateDiff, and the 'sentinel key holding the latest committed block number' lead-in above LatestKey (the field name already says that). Keep the non-obvious bits (clone rationale, 3.5-trillion-year collision window). * fix(state-composition): address second-round PR review findings - Snapshot write: put new → update LatestKey → remove old, so a crash between ops leaves the old entry reachable and PurgeOldEntries keeps it. - PublishScanResults: move depth-gauge publish inside _diffLock so an OnNewHeadBlock dispatch cannot tear the 9×16 table between seed and read. - StopAsync: bound _scanLock wait to 10s with warn log instead of blocking shutdown indefinitely on an uncooperative scan. - TrieDiff account leaves: switch to TryDecodeAccount; skip semantic diff and classification on length-1 empty stubs instead of treating them as real accounts / empty-account transitions. - Surface missing-node observations: new StateCompScanMissingNodes counter, visitor latch propagated into ScanMetadata.IsComplete, scan log flags incomplete runs explicitly. - Remove dead SnapshotInterval config knob (no readers). * fix(state-composition): guard shutdown race and harden parallel counter - Gate RunIncrementalDiff with a volatile _shuttingDown flag set at the top of StopAsync so diffs queued between `NewHeadBlock -=` and disposal cannot race IWorldStateManager/IDb teardown. - Convert StateCompScanMissingNodes to a get-only property backed by an Interlocked counter; concurrent tree-visitor workers now increment it safely via Metrics.IncrementScanMissingNodes(). - Cache Keccak.OfAnEmptyString.ValueHash256 in TrieDiffWalker so the hot RecordCodeHashChange path skips the property chain per call. - Drop two code-restating line comments in OnNewHeadBlock. * fix(state-composition): close scan-publish metrics race and rate-limit miss log - PublishScanResults: extend _diffLock to cover UpdateFromCumulativeStats and the scalar StateComp* writes. Prior scope left a window where a concurrent incremental diff for block N+1 could publish its metrics between InitializeIncremental and the scanner's post-lock writes, so the fresh baseline would clobber the newer N+1 values. - VisitMissingNode: latch-gate the warn log on the first miss using the existing _missingNodesObserved flag. The counter metric still increments on every miss, but the warn line fires once per scan so a pruning-window eviction cannot flood the log. * refactor(state-composition): drop redundant child-path copies in DiffBranchChild GetChildWithChildPath does not mutate its ref TreePath for inline children (verified in TrieNode.ResolveChildWithChildPath, switch arms for null/keccak/inline RLP never touch childPath). The child's own key is appended by the child's handler (DiffExtensions.path.AppendMut(Key), leaf handler appends for reconstruction), so the local oldChildPath / newChildPath copies were always equal to path on return — misleading dead locals. Passing ref path directly matches the existing pattern in CollectBranchSlotSide and WalkStructure (same file group). * test(state-composition): wire service tests via DI per test-infrastructure.md StateCompositionServiceTests constructed the service with Substitute.For<IBlockTree>() and Substitute.For<IWorldStateManager>(). test-infrastructure.md forbids mocking what production modules wire; use PseudoNethermindModule + TestEnvironmentModule and override only IStateReader (the boundary where tests must inject RunTreeVisitor side effects for semaphore/cancellation semantics). * fix(test): remove unused Nethermind.Trie.Pruning using * fix(statecomp): statecomp_cancelScan returns whether a scan was active CancelScan() now reports whether a cancellation signal was issued so RPC callers can distinguish "scan cancelled" from "no scan was running" rather than unconditionally returning true. * fix(statecomp): guard SlotCountHistogram update against default ImmutableArray Mirrors the existing IsDefault guard in StateCompositionSnapshotDecoder. Length throws NullReferenceException on a default-constructed ImmutableArray<long>; all current callers pass initialized arrays but the consistency matters for future call sites. * fix(statecomp): GetAddressHash returns null for leaves with missing Key A resolved leaf without a Key is corrupt; hashing the partial path would silently drift the per-contract code-hash and slot-count trackers under a wrong address. Callers now skip the record instead. * refactor(test): de-duplicate StateComposition tests (#11256) * refactor(test): de-duplicate StateComposition test helpers Extract shared TestDataBuilders (EmptyBaseline, BuildStats, CreateTestConfig, AssertAccountCumulativeEquals) to eliminate copy-pasted CumulativeTrieStats/IStateCompositionConfig setup across service, metrics, and diff-walker tests. Parameterise the slot-bucket check in SlotCountHistogramTests with TestCase and collapse the shared BeginStorageTrie+TrackStorageNode+Flush cycle into a single helper. Net: +45/-177 LOC in tests (+108 helper), 81 → 85 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): merge two visitor classification tests via TestCase Visitor_ClassifiesContracts and Visitor_TracksContractsWithStorage ran the same SimulateAccounts-twice-then-inspect pattern with different inputs. Collapse into one TestCase-parameterized Visitor_ClassifiesAccounts method. Each case now asserts all three totals (AccountsTotal, ContractsTotal, ContractsWithStorage) instead of two — widens coverage slightly as a side-effect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): TestCaseSource for Comparator_DeterministicTiebreaking Replace the [TestCase(\"Depth\")]+switch-on-string dispatch with a TestCaseSource that carries typed TopContractEntry pairs and the comparer method group directly. Each case gets a readable SetName so failure diagnostics point at the specific comparator under test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(test): address claude-review nits on test-dedup PR - Rename AssertAccountCumulativeEquals → AssertAccountTrieFieldsEqual and document that it intentionally skips storage-trie fields, so future callers don't mistake partial coverage for full coverage. - Drop the one-liner EmptyBaseline shim in StateCompositionStateHolderTests and route the 8 call sites directly to TestDataBuilders.EmptyBaseline — removes an indirection that defeats the single-source-of-truth goal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): drop unused System.Collections.Immutable using Left over from the earlier inline BuildStats that was replaced with TestDataBuilders.BuildStats — tripped IDE0005 in CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * perf(statecomp): inline VisitorCounters fixed-size arrays Replace six managed arrays (DepthCounter[16] ×3, long[16] ×3) on VisitorCounters and the single-contract scratch buffer on SingleContractVisitor with InlineArray-backed structs (Long16, DepthCounter16). Each worker-local VisitorCounters now drops six heap allocations plus their GC tracking, and depth rows live contiguously with the enclosing struct for better cache locality on the per-node hot path. MaxTrackedDepth, Long16.Length, and CumulativeTrieStats.SlotHistogramLength all equal 16, so MergeFrom collapses into a single unrolled loop. * perf(statecomp): replace TrieDiffWalker byte-total arrays with scalars Replace the two long[2] tables (_trieBytesAdded, _trieBytesRemoved) with four named scalar fields: _accountBytesAdded, _accountBytesRemoved, _storageBytesAdded, _storageBytesRemoved. Drops two heap allocations per TrieDiffWalker and two bounds-checked indexer calls per node in the diff hot path, at the cost of one extra branch on isStorage in RecordNode. * perf(statecomp): inline CumulativeDepthStats per-depth rows Back the 9×16 counter matrix with a nested InlineArray (DepthRows9 of Long16) instead of a jagged long[9][16]. The full 144-long payload now lives contiguously inside the class instance with no per-row arrays, no array headers, and no GC tracking for the inner rows. - Drops ByDepth public accessor in favor of Span<long>-returning per-category accessors and GetRow(int) for indexed iteration (used by the snapshot decoder). - CloneAsDelta collapses from 10 allocations + 9 Array.Copy calls to a single struct copy (copy._rows = _rows). - AddInPlace / IsEmpty operate on a flat 144-long span instead of nine jagged array loops. - Metrics helpers take ReadOnlySpan<long> to match the new accessor type (no callsite changes; Span<long> implicitly converts). Wire format unchanged — the 9×16 rows still round-trip as 144 longs in the same slot order via DepthSlot / GetRow. Tests: 87/87 pass. * perf(statecomp): collapse refcount dict accesses with GetValueRefOrAddDefault Replaces the TryGetValue+indexer-set pair on CodeHashRefcounts with a single CollectionsMarshal.GetValueRefOrAddDefault call that yields a ref to the slot. One hash lookup per increment instead of two (hot path: VisitAccount per HasCode account; merge path: per code hash across all worker threads), and no boxing/temporary int. * perf(statecomp): collapse ApplyCodeHashChange increment with GetValueRefOrAddDefault Same optimisation as the visitor-side refcount collapse, applied to the incremental holder path. ApplyCodeHashChange runs once per account that gains code in each block diff, so the three dict operations (TryGetValue + indexer-set when existing, or +2 extra sets on first reference) collapse to one GetValueRefOrAddDefault lookup plus a ref increment. The `exists` flag replaces the newRefcount == 0 test to detect first-reference. * docs(statecomp): drop redundant comments on the new inline-storage types The XML docs on Long16/DepthCounter16 and the per-field comments on CumulativeDepthStats restated what the type signatures already said, and the new refcount sites had one-line comments narrating what GetValueRefOrAddDefault does (the name carries that). Keep only the WHY and the non-obvious invariants. * fix(statecomp): close BuildReport torn-state race and harden adjacent accessors - Coalesce SetBaseline/MarkScanCompleted/InitializeIncremental into a single PublishScanBaseline(...) holder method under one _lock acquisition so BuildReport cannot observe a half-published scan state (new distribution and metadata visible while _incrementalStats still reflects the pre-scan cumulative state). - Use Volatile.Read in StateCompositionVisitor.GetSnapshot so the progress logger cannot observe a torn 64-bit counter on non-x64 runtimes. - Narrow CurrentDepthStats from public to internal and document that callers must hold _diffLock for the duration they read the returned instance. - Add TryGetShutdownSnapshot so StopAsync reads HasIncrementalBaseline / LastProcessedStateRoot / IncrementalStats / IncrementalBlock under one lock. --------- Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Stacked on top of #10995 — targets that branch, not master. Merge #10995 first (or rebase this onto master after) before this can land on master.
Changes
TestDataBuildershelper (EmptyBaseline,BuildStats,CreateTestConfig,AssertAccountCumulativeEquals) — eliminates copy-pastedCumulativeTrieStats/IStateCompositionConfigfixtures across service, metrics, and diff-walker tests.SlotCountHistogramTestswith[TestCase]; extract the sharedBeginStorageTrie+TrackStorageNode+Flushcycle into one helper.Visitor_ClassifiesContracts+Visitor_TracksContractsWithStorageinto a single[TestCase]-parameterisedVisitor_ClassifiesAccounts— broadens per-case coverage to three totals.Comparator_DeterministicTiebreaking's[TestCase("string")]+ switch dispatch with a proper[TestCaseSource]that carries typedTopContractEntrypairs and the comparer method group directly.CumulativeTrieStatsassertions inTrieDiffWalkerTestsintoTestDataBuilders.AssertAccountCumulativeEquals.Net: +201 / −234 LOC across 10 files (new helper + 9 test edits). Tests: 81 → 85 passing (extra count is from
[TestCase]expansion, not new tests).Types of changes
What types of changes does your code introduce?
Testing
Requires testing
Notes on testing
Pure test-file refactor. Existing 85 tests pass (
dotnet test --project Nethermind.StateComposition.Test ... -c release). No production code touched.Documentation
Requires documentation update
Requires explanation in Release Notes