Skip to content

KAD/discv4 debug logs#11811

Merged
flcl42 merged 1 commit into
masterfrom
kad-log
May 28, 2026
Merged

KAD/discv4 debug logs#11811
flcl42 merged 1 commit into
masterfrom
kad-log

Conversation

@flcl42

@flcl42 flcl42 commented May 28, 2026

Copy link
Copy Markdown
Contributor

Changes

  • Don't bother info level with debug logs

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @flcl42's task in 42s —— View job


Review

  • Gather context and read changed file
  • Analyze changes
  • Post review

Summary: Two logging fixes in KBucketTree<TNode>, both correct and unambiguously safe.

LogTreeStatistics() — guard hoisted before the tree traversal

The if (!_logger.IsDebug) return; guard is now at the top of the method, before the expensive tree traversal. Previously the traversal ran unconditionally and only the log call was gated. This is a meaningful performance win: in production (where Debug is off) the entire O(n) tree walk is skipped.

LogTreeStructure()InfoTrace, guard added

_logger.Info(...) was clearly wrong — printing the full tree structure at Info level would spam production logs on every call to LogDebugInfo(). Dropping it to Trace and adding the early-return guard before the StringBuilder allocation and traversal is the right fix.


Findings:

No new issues introduced by this PR.

Pre-existing (out of scope): (double)totalItems / totalBuckets in LogTreeStatistics() will produce NaN if totalBuckets == 0 (empty tree). This is already present on master and only affects debug-level output, so it is not a blocker.


Verdict: Changes are correct, safe to merge. No Critical/High/Medium findings.

@flcl42 flcl42 merged commit 790174c into master May 28, 2026
965 of 973 checks passed
@flcl42 flcl42 deleted the kad-log branch May 28, 2026 11:59
LukaszRozmej added a commit to divi2806/nethermind that referenced this pull request May 28, 2026
- Restore NethermindEth#11811 KBucketTree.cs early-return guards and Trace level reverted by stale rebase
- Make FullPruner OldestStateBlock advance atomic with DB swap: write the marker before pruning.Commit() so FullPruningDb mirrors it into the cloning DB
- Drop unsafe (IStateBoundaryWriter) cast in FullPrunerFactory by injecting IStateBoundaryWriter from DI
- Log backward OldestStateBlock writes at Warn instead of dropping them silently
- Restructure BuildState with single early-return to remove the redundant retention null check
- Restore ReadOnlyBlockTree.cs blank line removed in rebase
- Rename Eth_capabilities_* tests to match the project's PascalCase prefix convention
LukaszRozmej added a commit that referenced this pull request May 29, 2026
…ry (#11438)

* Add eth_capabilities RPC method and related data structures

This commit introduces the `eth_capabilities` method to the Eth RPC module, which returns the node's historical data availability and head block information. It includes the new `EthCapabilitiesResult` class and associated classes (`CapabilityHead`, `CapabilityResource`, and `CapabilityDeleteStrategy`) to structure the response. Additionally, tests for the new functionality have been added to ensure correct behavior across various configurations, including archive and pruned nodes.

* Enhance eth_capabilities method to handle pruning modes and improve state reporting

This commit updates the `eth_capabilities` method in the Eth RPC module to correctly report the availability of blocks and receipts based on the configured pruning mode. It ensures that the `OldestBlock` field is set to null for full pruning scenarios, preventing misleading information. Additionally, tests have been added to verify the behavior of the method across different configurations, including archive nodes and various pruning settings, ensuring compliance with the expected JSON structure.

* Add EthCapabilitiesProvider class and refactor eth_capabilities method

This commit introduces the `EthCapabilitiesProvider` class, which encapsulates the logic for retrieving the capabilities of the Ethereum node, including block and receipt availability based on the current sync and pruning configurations. The `eth_capabilities` method in the `EthRpcModule` is refactored to utilize this new provider, streamlining the code and improving maintainability. Additionally, the interface for the `eth_capabilities` method is updated to include an example response for better clarity in usage.

* Refactor EthCapabilitiesResult and EthCapabilitiesProvider to use long and Hash256 types

This commit updates the `EthCapabilitiesResult` and `EthCapabilitiesProvider` classes to replace string representations of block numbers and hashes with their appropriate types: `long` for block numbers and `Hash256` for block hashes. This change enhances type safety and consistency across the codebase. Additionally, the related tests have been updated to reflect these changes, ensuring accurate validation of capabilities reporting.

* refactor: constructor-based capability records and dedup capability tests

- Convert EthCapabilitiesResult, CapabilityHead, CapabilityResource, and
  CapabilityDeleteStrategy to primary-constructor records; preserve conditional
  JSON omission via [property: JsonIgnore(...)] attribute targeting.
- Update EthCapabilitiesProvider to construct records via constructors instead
  of object initializers.
- De-duplicate EthRpcModuleTests.Capabilities: merge the two default-build
  smoke tests; consolidate archive/full-pruning/no-receipts cases under a
  TestCaseSource that asserts full CapabilityResource equality.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: introduce IEthCapabilitiesProvider, register via DI, rename types

- Add IEthCapabilitiesProvider interface; register via Autofac (singleton).
- EthRpcModule now takes IEthCapabilitiesProvider instead of constructing it
  from IBlockFinder + sync/pruning configs; OptimismEthRpcModule and the
  factories pass the injected instance through.
- Rename EthCapabilitiesResult -> EthCapabilities, CapabilityHead -> ChainHead,
  CapabilityResource -> ResourceAvailability, CapabilityDeleteStrategy ->
  DeleteStrategy. File renamed accordingly.
- Replace eth_capabilities key-set test with a JSON-schema-based test using
  NJsonSchema (added to JsonRpc.Test). Schema mirrors ethereum/execution-apis#755
  and enforces additionalProperties=false plus required fields and hex patterns.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(eth_capabilities): account for history pruning; alias Tx/Logs to Receipts

- EthCapabilitiesProvider now consumes IHistoryConfig and IHistoryPruner.
  Blocks/Tx/Logs/Receipts.OldestBlock is lifted to historyPruner.OldestBlockHeader
  when history pruning is active, and Rolling mode produces a window DeleteStrategy
  with retention = RetentionEpochs * 32 blocks.
- EthCapabilities.Tx and .Logs become computed properties returning Receipts —
  the three resources share storage and pruning policy in Nethermind today.
  JsonPropertyOrder preserves the spec's canonical key order on the wire; the
  fields can be decoupled later if a future spec change requires it.
- Add Nethermind.History project reference to JsonRpc.
- Fold the three new history-pruning tests into the existing TestCaseSource;
  Blocks is now part of the parametric assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): align with spec — integer retentionBlocks; share Resource helper

Reviewed against ethereum/execution-apis#755 schema and test fixtures:

- DeleteStrategy.RetentionBlocks must be a JSON integer per spec
  ("retentionBlocks":90000), but Nethermind's default long converter writes
  hex. Apply [JsonConverter(typeof(LongRawJsonConverter))] to override.
- Tighten the JSON-schema test: retentionBlocks is integer; deleteStrategy.type
  is enum ["window"] (the spec's only variant — others may be added via oneOf).
- Extract a single Resource(enabled, oldestBlock, deleteStrategy?) helper used
  for all four resources. Enforces the spec invariant "disabled: true ⇒ no
  other fields" in one place.
- Promote HistoryPruner.SlotsPerEpoch from private to public; reference it
  from EthCapabilitiesProvider so the constant has a single source of truth.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: regenerate packages.lock.json after JsonRpc->History reference

CI runs `dotnet restore --locked-mode`; adding the Nethermind.History
project reference to Nethermind.JsonRpc changed the dep graph, which the
locked-mode restore detected as inconsistent with packages.lock.json.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): wire state availability through IWorldStateManager

Pruning config only describes the trie-based store; flat-DB state has its own
retention model. Move the state-availability report behind IWorldStateManager
so each implementation reports its own semantics, and address the test review
by collapsing the standalone memory-pruned and json-schema fixtures into a
single mock-based scenario suite plus one end-to-end JSON schema test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): support state proofs within trie pruning window

Trie nodes resolve by hash, so eth_getProof works wherever State works —
not only on archive nodes. Memory/Hybrid/Full trie modes now report
StateProofsSupported = true; only flat-DB (no by-hash lookup) keeps it off.
Stateproofs OldestBlock and DeleteStrategy now mirror State so callers see
the same retention bounds. Comments trimmed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): drop StateProofsSupported; flat supports proofs

Flat-DB serves state proofs via RunTreeVisitor over ReadOnlySnapshotBundle —
HashServer is only the snap-server hook, not the proof path. With both
production managers always supporting proofs, the flag is dead. Stateproofs
now mirrors State.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(eth_capabilities): persist OldestStateBlock metadata

Adds an absolute floor for historical state availability, recorded in the
metadata DB and exposed via IBlockTree.OldestStateBlock. It's set:
- after fast/snap sync completes (= pivot block), via a new
  RecordOldestStateBlockOnStateSyncFinished hook on ITreeSync.SyncCompleted;
- after full pruning completes (= copied state's block), in FullPruner.

EthCapabilitiesProvider combines this floor with the rolling window
retention from IWorldStateManager, so a fast-synced archive node now
correctly reports OldestBlock = pivot, and a full-pruned node reports
OldestBlock = last successful prune target.

The Archive flag on StateAvailability is dropped — the floor + window are
sufficient to express all retention shapes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): inline StateAvailability as GetOldestStateBlock(head)

Drops the StateAvailability struct (only one field left) and exposes the
manager's contribution as a method returning the oldest block given the
chain head. Mirrors IBlockTree.OldestStateBlock naming. Trie pruning manager
returns max(0, head - PruningBoundary) for memory/hybrid, null otherwise;
flat returns null (its retention is via the IBlockTree.OldestStateBlock
floor). The provider combines this with the floor and derives DeleteStrategy
from the actual span retained.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: regenerate packages.lock.json with Nethermind.History reference

The post-merge lock file was missing the JsonRpc → History dependency added
on this branch, causing dotnet restore --locked-mode to fail in CI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): use raw barriers; honor body availability; handle no-head

Use min(pivot, AncientBodiesBarrier) and min(pivot, max(bodiesBarrier,
receiptsBarrier)) directly. Drops AncientReceiptsBarrierCalc's Math.Max(1, ...)
clamp, fixing the off-by-one for archive (PivotNumber=0) and fast-sync with
default barriers. Receipts now correctly report oldestBlock=0 from genesis
when nothing is gated.

Blocks resource was reporting LowestInsertedHeader for body availability.
A fast-synced node with non-zero AncientBodiesBarrier or DownloadBodiesInFastSync
=false has headers but no bodies below the cap, so Blocks must reflect bodies:
- lowestBlock floors to the bodies barrier
- enabled = headersAvailable && bodiesSynced
- receiptsSynced now also requires bodiesSynced

Suppress state.deleteStrategy when a higher floor (post-fast-sync pivot, full-
prune mark) dominates the rolling window. Keeps oldestBlock and head-retentionBlocks
internally consistent.

Return all-disabled with head=(0, Keccak.Zero) when blockTree.Head is null
(node warming up, no chain reconstructed yet).

Tests: 4 new scenarios cover fast-sync default barriers, ancient bodies barrier,
disabled bodies, and the no-head path; floor-dominated case now asserts the
suppressed strategy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): use a valid 64-char hash in ExampleResponse

The previous example response contained "0xabc" as the head hash, which is
3 hex chars instead of the 64 the spec schema requires. The ExampleResponse
is documentation-only, but a malformed example breaks any tooling that
parses or validates the auto-generated docs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): gate body/receipt download flags on fast-sync state

DownloadBodiesInFastSync and DownloadReceiptsInFastSync only affect data
availability when the node actually fast-synced. A full-sync node downloads
bodies and receipts as part of forward processing regardless of the flags.
Treating them as authoritative previously caused full-sync nodes that happened
to leave the flags off to falsely report Blocks/Receipts as disabled.

The flags now only disable the resources when FastSync || SnapSync is on.
Tests updated: the receipts/bodies-disabled scenarios are now expressed as
fast-sync configs (which actually exercise the flags), and two new scenarios
pin the full-sync-ignores-the-flag behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(eth_capabilities): note window-vs-floor selection rationale

One-liner explaining why DeleteStrategy is suppressed when the static floor
dominates the rolling window — addresses the relevant claude[bot] review
finding. The two transient-during-sync findings (genesis-state advertised
pre-pivot, Receipts.OldestBlock < Blocks.OldestBlock during header download)
self-resolve once sync completes and don't need code commentary.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): track actual sync progress mid-sync

Use ISyncPointers.LowestInserted{Body,Receipt}Number so Blocks.OldestBlock
and Receipts.OldestBlock reflect what the node currently retains, not just
the eventual post-sync barrier. During fast sync the pointers track above
the barrier; once sync completes they collapse onto it.

State and Stateproofs are gated on OldestStateBlock when fast-sync is
configured — until StateSyncRunner finalises and writes the pivot floor we
report State as disabled rather than advertising historical state from
genesis.

Two new scenarios cover the mid-progress and pre-state-finalised cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): apply review fixes

Provider:
- Required ctor params (drop nullable defaults; tests pass Substitute.For)
- IReadOnlyBlockTree instead of IBlockTree (read-only intent)
- sealed
- Drop misleading head.Hash ?? Keccak.Zero (BlockTree invariant guarantees non-null)
- Inline Resource helper into BuildState/BuildResource/BuildWindow
- Suppress window descriptor when retentionBlocks <= 0 (genesis edge)
- Hoist Disabled to a static field
- Extract BuildState to absorb the multi-line "why" comment

BlockTree:
- OldestStateBlock setter no-op when value unchanged

BlockTreeOverlay:
- OldestStateBlock falls back to base tree (matches Head/Finalized/Safe pattern)

FullPruner:
- CopyTrie returns bool committed; OldestStateBlock write gated on the explicit
  return rather than re-reading the cancellation token

Tests:
- Rename test method to method_scenario shape
- New: BlockTreeTests.OldestStateBlock_round_trips_through_metadata_db

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): address review feedback from asdacap & stavros

- Move OldestStateBlock off IBlockFinder: now declared on IBlockTree
  (writer side) and IWorldStateManager (external consumers). BlockTree
  implements a narrow IOldestStateBlockStore so WorldStateManager can
  delegate without taking a Nethermind.Blockchain dependency.
- Gate Stateproofs separately from State via IWorldStateManager.SupportsTrieProofs:
  trie-backed managers return true, FlatWorldStateManager returns false to
  avoid advertising O(state-size) or snapshot-limited proof reconstruction.
- Encapsulate the epoch->blocks conversion behind IHistoryPruner.GetRetentionBlocks;
  SlotsPerEpoch becomes private const again.
- Clarify XML doc: Tx/Logs aliasing Receipts (with LogIndex caveat),
  JsonPropertyOrder is example-payload parity, not a wire requirement.
- Replace Keccak.Zero in ExampleResponse with a realistic 32-byte hash.
- Add CapabilitiesScenario row covering AncientReceiptsBarrier > AncientBodiesBarrier.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): own OldestStateBlock persistence in WorldStateManager

- Move the absolute oldest-state-block floor out of BlockTree entirely.
  WorldStateManager now owns the metadata-DB persistence (same key) via a
  small OldestStateBlockStore helper; FlatWorldStateManager does the same.
  IBlockTree/IBlockFinder no longer expose OldestStateBlock.
- Route FullPruner and StateSyncRunner writes through
  IWorldStateManager.OldestStateBlock (proper layering).
- FullPrunerFactory.Create now takes IWorldStateManager (not IStateReader)
  so the pruner can write the floor without going through the block tree.
- Make CopyTrie synchronous (it has no async work; was already returning
  Task.FromResult).
- Reformat multi-arg EthCapabilitiesProvider constructor calls one-per-line.
- Drop IOldestStateBlockStore glue interface no longer needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): alias stateproofs to state again

Flat storage retains trie nodes in path-keyed columns (see
BaseTriePersistence: StateNodesTop, StateNodes, StorageNodes,
FallbackNodes) and serves eth_getProof in O(trie-depth) for any block
where the flat snapshot is present — same complexity as trie storage.
The previous SupportsTrieProofs gate was based on a wrong assumption.
Drop the property; stateproofs availability matches state availability
for both Nethermind backends.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(eth_capabilities): discard stale state-availability markers at startup

Adds a one-shot consistency check at world-state-manager activation:
- If OldestStateBlock points at a block whose state root is not present
  in the state DB, clear it.
- Same for IBlockTree.BestPersistedState.

This handles the scenario where a user wipes the state directory without
touching MetadataDb / BlockInfoDb — without the check, eth_capabilities
would over-report state availability until sync rewrites the floor.

The check uses GlobalStateReader.HasStateForBlock so it's meaningful
(verifies the root, not just "is the DB empty"). Wired via
OnActivate<IWorldStateManager> in both WorldStateModule and
FlatWorldStateModule.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(state-metadata): cover stale-floor discard at startup

Move StateMetadataValidator from Nethermind.Init to Nethermind.Blockchain
(where IBlockTree lives) so it can be tested in Nethermind.Blockchain.Test.
Use explicit BlockTreeLookupOptions overload so NSubstitute can intercept
the FindHeader call without going through the default interface method.

Tests cover:
- OldestStateBlock cleared when state root missing
- OldestStateBlock kept when state root present
- OldestStateBlock kept when header unknown (defensive)
- No-op when OldestStateBlock is null
- BestPersistedState clear/keep symmetric to OldestStateBlock
- Markers independent (one stale, other ok)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): delete metadata key when OldestStateBlock is cleared

- Address claude-bot review (pass 6): OldestStateBlockStore setter now
  deletes the metadata DB entry on null, so a stale floor isn't reloaded
  on every restart after StateMetadataValidator clears it.
- Remove unused `using Nethermind.Blockchain.Find;` from
  StateMetadataValidator and its tests — BlockTreeLookupOptions lives in
  Nethermind.Blockchain. Fixes IDE0005 lint failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): guard descending-pointer pre-insert state and stabilize retentionBlocks

- Block/Receipt resources are now Disabled while fast sync is configured to download but the
  descending pointer is still null. Reporting the barrier (eventual floor) as oldestBlock
  before any insertion over-claimed availability.
- Replace IWorldStateManager.GetOldestStateBlock(head) with RetentionWindowBlocks. Provider
  derives windowOldest itself and emits the configured retention so retentionBlocks stays
  correct when head < retention (fresh chain) instead of rounding down to head.
- Lock OldestStateBlockStore reads/writes: long? is two fields, the RPC path reads concurrently
  with sync runners and the full pruner.
- Add a regression-scenario test for the pre-batch fast-sync window and update existing
  scenarios to set descending pointers consistent with their post-sync framing.

* refactor(eth_capabilities): narrow OldestStateBlockStore deps, dedup tests

- Promote OldestStateBlockStore to a DI singleton (registered in BlockTreeModule
  via IDbProvider.MetadataDb). FullPruner, StateSyncRunner, EthCapabilitiesProvider,
  StateMetadataValidator now depend on the narrow store instead of IWorldStateManager.
  Removes OldestStateBlock from IWorldStateManager.
- Drop IDbProvider from FlatWorldStateManager (the store is injected directly).
- Extract MetadataLongStore reusable base class taking IDb + key; OldestStateBlockStore
  becomes a thin sealed wrapper hardcoding MetadataDbKeys.OldestStateBlock.
- IFullPrunerFactory.Create signature: IWorldStateManager → IStateReader (mirrors
  what FullPruner actually needs); factory pulls the store via DI.
- IsDescendingResourceDownloaded as single expression.
- Drop the stateproofs-aliases-state comment.
- Deduplicate StateMetadataValidatorTests with [TestCase] parameterization
  across (Marker × initial × StateAt × shouldClear); test count: 9 (8 cases + 1
  independent-markers).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): IStateBoundary on IWorldStateManager; per-backend state-DB-resident storage

Addresses @asdacap review on `fabd507183`:
- Re-add `OldestStateBlock` (+ `RetentionWindowBlocks`) on a new narrow
  `IStateBoundary` interface; `IWorldStateManager : IStateBoundary`.
- Trie `WorldStateManager`: floor persists in `dbProvider.StateDb` under
  `Keccak("OldestStateBlock")` (collision-free vs 32-byte hash keys and
  HalfPath section bytes 0/1/2).
- `FlatWorldStateManager`: floor persists in `FlatDbColumns.Metadata` under
  the same keccak key, alongside the existing `CurrentState`/`Layout` keys.
- Co-locating the floor with state nodes means wiping the state directory
  drops the floor automatically — no startup validator needed.
- Drop `StateMetadataValidator` (+ tests), `OldestStateBlockStore`,
  `MetadataLongStore`, `MetadataDbKeys.OldestStateBlock` (the metadata-DB
  storage was never deployed), and the duplicated `OnActivate` validator
  wiring in both world-state modules.
- `FullPruner` / `StateSyncRunner` / `EthCapabilitiesProvider` depend on
  the narrow `IStateBoundary` (auto-resolved via `IWorldStateManager`).
- `IFullPrunerFactory.Create` signature returns to `IWorldStateManager`.
- Capabilities test mock simplifies to a single `IStateBoundary` substitute.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: move IStateBoundary into its own file

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): expose IWorldStateManager as IStateBoundary in DI

EthCapabilitiesProvider depends on IStateBoundary, but no module
registered that service — Autofac does not auto-resolve base interfaces
for factory-registered singletons. Add an explicit Map so the manager
is resolvable through both contracts. Surfaced by
RpcModuleProviderTests.ModuleFactory_FromDI_IsLazy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): rename CopyTrie to TryCopyTrie

Now that the method returns bool indicating whether the prune committed,
the Try-prefix matches the standard .NET convention and makes the
caller's `if (TryCopyTrie(...))` branch self-documenting.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(full-pruning): account for OldestStateBlock metadata in subset check

FullPruner now records the state-availability floor in the new state DB
on a successful prune, so the post-prune snapshot legitimately contains
one entry that's absent from the pre-prune snapshot. Verify the boundary
key was written (closing a coverage gap for the FullPruner → IStateBoundary
write), then exclude it before asserting the rest of the contents are a
subset of the pre-prune values.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): reject backward OldestStateBlock writes

The floor is meant to be monotonically non-decreasing: state sync writes
the pivot, then full pruning writes the (later) copied block. The setter
now rejects backward non-null writes so a stale caller can't regress the
reported availability. Null reset remains allowed for explicit recovery
(e.g. wiping a corrupt state DB).

Also adds StateBoundaryStoreTests covering initial-null read, round-trip
persistence, idempotent re-set, the monotonic guard, and the null-reset
escape hatch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(history-pruner): cover GetRetentionBlocks contract

Locks in the epochs→blocks conversion (×32) so future refactors can't
silently change the slot-per-epoch constant without test coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(eth_capabilities): persist before caching in StateBoundaryStore

If the kv write threw after _value had already been overwritten, the
in-memory snapshot would diverge from disk until the next forward write
or process restart. Swap the order so a failed persistence leaves both
caches at the previous value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): split IStateBoundary into reader and writer

EthCapabilitiesProvider only needs to read the floor; StateSyncRunner and
FullPruner are the only legitimate writers. Splitting the contract makes
the write surface explicit instead of leaking through every consumer of
IWorldStateManager, which still implements both halves. Also promotes
StateBoundaryStore.OldestStateBlockKey to internal so the full-pruning
disk test can reference the key constant directly rather than recomputing
the keccak inline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): trim redundant comments, dedup boundary tests

- Drop inline comments restating what IStateBoundary's XML doc already
  says (co-location, "no rolling window for flat", etc.).
- Compress EthCapabilities docs by moving the Tx/Logs aliasing rationale
  into <remarks> and tightening to one sentence.
- Drop the implementation-detail tail of the RetentionBlocks param doc.
- Collapse the three single-action StateBoundaryStore tests
  (forward/backward/equal) into one [TestCase]-parameterized test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(eth_capabilities): address asdacap review on boundary plumbing

- IStateBoundaryWriter is trie-specific: it's no longer on IWorldStateManager.
  Only the trie WorldStateManager implements it; FlatWorldStateManager exposes
  only the reader half.
- Move the OldestStateBlock write from StateSyncRunner (wrong layer — sync
  runner orchestrates rounds and shouldn't know about the floor) to
  PatriciaTreeSyncStore.FinalizeSync, which is the trie-specific
  finalization hook that already has the pivot header.
- FlatWorldStateManager no longer owns a StateBoundaryStore. Flat state
  tracking lives in PersistenceManager already, so OldestStateBlock just
  reads CurrentPersistedStateId.BlockNumber from there. No setter on flat.
- DI: WorldStateModule keeps the IStateBoundary map (works for both
  backends since IWorldStateManager : IStateBoundary). IStateBoundaryWriter
  moves to PruningTrieStoreModule, mapped from the trie factory output so
  it stays unresolved when flat is the active backend.
- FullPruner's worldStateManager argument is cast to IStateBoundaryWriter
  at the FullPrunerFactory boundary (safe — full pruning is trie-only).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(state-boundary): correct StateBoundaryStore doc after flat split

The store now serves only the trie backend — flat reads from
PersistenceManager directly — so the dual-backend wording was misleading.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(state-boundary): convert StateBoundaryStoreTests to NUnit assertions

Master removed FluentAssertions (#11567 "Unify assertions"); the merged
StateBoundaryStoreTests still used .Should() via Core.Test's transitive
package reference and no longer builds. Convert to Assert.That to match
the unified style.

* fix(eth_capabilities): address review feedback from svlachakis

- Restore #11811 KBucketTree.cs early-return guards and Trace level reverted by stale rebase
- Make FullPruner OldestStateBlock advance atomic with DB swap: write the marker before pruning.Commit() so FullPruningDb mirrors it into the cloning DB
- Drop unsafe (IStateBoundaryWriter) cast in FullPrunerFactory by injecting IStateBoundaryWriter from DI
- Log backward OldestStateBlock writes at Warn instead of dropping them silently
- Restructure BuildState with single early-return to remove the redundant retention null check
- Restore ReadOnlyBlockTree.cs blank line removed in rebase
- Rename Eth_capabilities_* tests to match the project's PascalCase prefix convention

* fix(eth_capabilities): restore IStateBoundaryWriter cast in FullPrunerFactory

Injecting IStateBoundaryWriter created a DI cycle: PruningTrieStateFactory
→ FullPrunerFactory → IStateBoundaryWriter (mapped from
PruningTrieStateFactoryOutput, which is built by PruningTrieStateFactory).
Revert to the runtime cast — it is safe in practice because Create only
runs for trie storage, which is always WorldStateManager.

---------

Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants