Fix FlatDB persistence edge cases from the signed/unsigned type unification

[wurdum]

Fix FlatDB persistence edge cases from the signed→unsigned type unification

Changes

Two follow-up fixes to Nethermind.State.Flat persistence, both regressions introduced by the long → ulong / PreGenesis sentinel migration. The root cause in each is that StateId.PreGenesis.BlockNumber is now ulong.MaxValue, which collides with unsigned arithmetic that used to be safe when the sentinel was -1.

• Anchor the compaction schedule at genesis when nothing is persisted.

  • ICompactionSchedule.NextFullCompactionAfter now takes in StateId from instead of ulong from, and CompactionSchedule maps PreGenesis → block 0 internally (removing the if (from == ulong.MaxValue) return ulong.MaxValue; short-circuit).
  • PersistenceManager passes the StateId (in currentPersistedState) at both call sites (DetermineSnapshotToPersist, FlushToPersistence).
  • Why: on a fresh DB (currentPersistedState == PreGenesis), feeding the sentinel's ulong.MaxValue block number into the schedule made nextCompactedBoundary == ulong.MaxValue, which is > finalizedBlockNumber for any real chain. That forced the first persist down the unfinalized/head-ancestor branch, delaying it from depth MinReorgDepth + CompactSize until depth exceeded MaxReorgDepth, and emitting a misleading "Very long unfinalized state" warning on a healthy node. Anchoring at genesis lets the branch be chosen by real finalization status. Self-correcting and one-time per database lifetime, but noisy and memory-heavy at cold start.

• Saturate inMemoryStateDepth to avoid a ulong underflow.

  • PersistenceManager.DetermineSnapshotToPersist computes the non-PreGenesis depth with lastSnapshotNumber.SaturatingSub(currentPersistedState.BlockNumber) instead of a raw ulong subtraction.
  • Why: if the latest snapshot is below the persisted block — reachable only via a deep reorg below a force-persisted unfinalized block — the result wrapped to ≈2⁶⁴, defeating both keep-in-memory early-returns and emitting two spurious warnings before returning null anyway. Saturating the subtraction floors the depth at 0 so the existing guard returns null early and silently, matching the old signed behavior.

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

If yes, did you write tests?

• Yes
• No

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

Remarks

Both fixes are scoped to FlatDB cold-start / persistence-edge behavior; neither changes consensus-visible behavior, on-disk format, or steady-state operation.

[claude]

Claude finished @wurdum's task in 3m 15s —— View job

---

PR Review: FlatDB persistence edge cases from the signed→unsigned unification

• Read changed files & surrounding context
• Analyze correctness, edge cases, regressions
• Review tests
• Post review findings

Both fixes are well-reasoned, minimal, and correctly target the ulong.MaxValue sentinel collisions introduced by the long → ulong migration. I traced the logic and it holds up; one finding worth addressing before merge.

Findings

Medium — second fix (SaturatingSub) lacks a regression test and is in tension with the Debug.Assert · PersistenceManager.cs:124-128
The saturation only alters behavior when lastSnapshotNumber < currentPersistedState.BlockNumber — precisely the case the Debug.Assert one line above declares impossible. So in Debug/test builds the assert fires first and the saturated path is only reachable in Release. Consequences:

• The underflow fix has no regression test, while AGENTS.md/CONTRIBUTING require one per bug fix (the compaction-anchoring fix is well covered). A test can't easily be added while the assert stands — it would throw in a Debug run before reaching the saturated branch.
• The assert message now contradicts the reachable scenario the PR describes (deep reorg below a force-persisted unfinalized block).

Recommendation: relax the assert to a debug log and add a regression test driving currentPersistedState.BlockNumber > latestSnapshot.BlockNumber (asserting an early, silent null), or add a short comment that the saturation is the Release-only safety net for the rare reorg the assert documents. Details in the inline comment.

Low — NextFullCompactionAfter_PreGenesis_AnchorsAtGenesis only asserts equivalence to block 0, not a concrete value. That's acceptable since NextFullCompactionAfter_VariousOffsets pins exact values for block 0, but pinning the expected boundary (e.g. 16 for offset 0) would make the intent self-contained.

What I verified (no issues)

• PreGenesis → 0 anchoring is correct. On a fresh DB the boundary now resolves to CompactSize (16 for offset 0) instead of ulong.MaxValue, so the finalized-vs-unfinalized branch is chosen by real finalization status, eliminating the spurious "Very long unfinalized state" warning and early-cold-start memory bloat. When finalized < boundary the unfinalized branch still correctly keeps state in memory until finalization catches up.
• All call sites updated (DetermineSnapshotToPersist, FlushToPersistence); no stray ulong-overload callers remain.
• FlushToPersistence loop still terminates and now prefers a compacted PreGenesis→16 snapshot before falling back to block-0 non-compacted — an improvement, with no infinite-loop risk (each iteration advances currentPersistedState).
• Sentinel safety: only the exact PreGenesis record value maps to 0; StateId.Sync (MaxValue-1) and any other input behave identically to the pre-fix code (overflow guard returns ulong.MaxValue), so no behavioral regression for those.
• Doc comment on ICompactionSchedule.NextFullCompactionAfter was updated to document the PreGenesis semantics and corrected the long.MaxValue → ulong.MaxValue reference.

Verdict

Solid, scoped fixes with correct reasoning. The one blocking item is the missing regression test for the SaturatingSub underflow path (and the related assert tension) — addressing or explicitly acknowledging it would clear the review.