Skip to content

collapse lazy NonFinalizedState instantiation #1096

Description

@zancas

Summary

NodeBackedChainIndex stores its non-finalized state as
Arc<ArcSwapOption<NonFinalizedState<Source>>>
(packages/zaino-state/src/chain_index.rs:624, :877). The slot starts
empty and is populated only after the sync loop has driven the
finalized DB up to validator_best_height − 100. Until that moment,
snapshot_nonfinalized_state returns
ChainIndexSnapshot::StillSyncingFinalizedState { validator_finalized_height }
and every read path falls back to validator passthrough.

This issue tracks the complexity that lazy instantiation has imposed.

Symptoms / costs

  1. Two-level Option/Arc wrapping. Arc<ArcSwapOption<NFS>> exists
    purely so the slot can flip from NoneSome once. The inner
    NFS already holds an ArcSwap<NonfinalizedBlockCacheSnapshot> for
    atomic snapshot rotation; the outer ArcSwapOption only encodes
    "not built yet."

  2. Special-case branch in the sync loop.
    chain_index.rs:809-827 carries a match *intermediate_nfs_for_scoping
    to lazily call NonFinalizedState::initialize and nfs.store(...).
    The branch contains two .expect("todo") calls, in violation of
    CLAUDE.md — no .unwrap() rule.

  3. Stop-the-world load contortion. The same block must keep an
    Arc<NFS> borrow live across the init branch, hence
    let intermediate_nfs_for_scoping = nfs.load(); … &nfs.load_full().expect("just set to Some")
    followed by a literal std::mem::drop(intermediate_nfs_for_scoping)
    to release the guard.

  4. Implicit single-writer invariant. "Load None → call
    initializestore(Some)" is non-atomic. The code is correct
    only because the sync loop is the sole writer; this constraint is
    nowhere documented or enforced.

  5. 12+ consumer match arms. Every ChainIndex query in
    chain_index.rs repeats the
    match snapshot { NonFinalizedStateExists { … } | StillSyncingFinalizedState { … } }
    shape (sites at lines 1025, 1063, 1103, 1128, 1304, 1608, 1668,
    1763, 1845, 2023, 2133, 2143, 2153). Many of the
    StillSyncingFinalizedState arms simply re-implement validator
    passthrough; some return Ok(None) with a TODO (e.g.
    chain_index.rs:1332-1336 already says
    "Once we make chainwork an option field we should be able to
    support passthrough for this"
    ).

  6. Conflated "not built" vs "built but not yet authoritative." The
    one bit Some/None cannot distinguish "we have a partially-warm
    cache pointing at the validator's claimed finalized tip" from
    "Zaino's finalized DB has fully caught up to that tip and the
    cache's cumulative chainwork is trustworthy." Today both are
    represented as Some(NFS) once the slot flips.

Proposal

Eagerly construct the NonFinalizedState at chain-index creation;
collapse the slot to Arc<NonFinalizedState<Source>>. Mark the per-block
fields whose meaning depends on a fully-synced finalized state as
Provisional / Resolved:

  • cumulative chainworkProvisional until Zaino's finalized DB
    has the seed block, then Resolved (monotonic).
  • the property that parent_hash resolves to an in-cache parent —
    same lifecycle.

Replace the ChainIndexSnapshot enum with a single struct carrying:

  • non_finalized_snapshot: Arc<NonfinalizedBlockCacheSnapshot> (always present)
  • availability: SnapshotAvailability { validator_finalized_height, state: Provisional | Resolved }

validator_finalized_height stays — it's the passthrough cutoff that
get_block_height_passthrough already needs. No blocks_remaining or
ETA fields on the snapshot: those are stale-by-design once the snapshot
is held, and are better served by a live reader on the chain index
itself (e.g. a finalized_sync_progress() method on the subscriber)
that callers can poll on demand.

Resolution flips at update-time inside non_finalised_state.rs:529:
after the CAS succeeds, walk the snapshot's blocks and rewrite
Available::Provisional → Available::Resolved once the seed block is
in finalized_db. Tag flip only — the cumulative chainwork numbers
don't have to change at the boundary.

The 12 consumer match arms collapse: most paths read the snapshot
directly; the ~2 paths that actually need trustworthy cumulative work
(chain-tips selection across forks) call a require_resolved()
choke-point and fall back to passthrough on Provisional.

Out of scope

  • Mempool lifecycle.
  • Sync-loop backoff/timing (already covered by SyncTimings).
  • The non_finalised vs NonFinalized spelling split.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions