chain_index: finalized_height(chain_height) conflates chain-derived reorg floor with monotonic finalized-DB tip

[zancas]

Subtlety

chain_index::finalized_height(chain_height: u32) -> types::Height computes
chain_height.saturating_sub(NON_FINALIZED_DEPTH). The name suggests
"zaino's authoritative finalized tip," but the value is a chain-tip-
derived lower bound, not the actual finalized-DB tip.

The two agree in steady state — each iter advances fs.db_height to track
this value. They diverge after a reorg that shortens the best-known-
chain tip below the prior finalization boundary: zaino's finalized DB
is monotonic (once a block is added, it stays), so fs.db_height can
sit above chain_height - NON_FINALIZED_DEPTH.

We do assume reorgs cannot pass NON_FINALIZED_DEPTH blocks deep
(Zcash protocol bound via coinbase maturity), so every block zaino has
finalized was safe by protocol at the time of finalization. The
divergence is purely about the chain-derived lower bound falling
below the (monotonic) actual tip after the chain shortens — not about
"reorg-unsafe finalization."

Scenario

1. Best-known-chain tip = 200; zaino has finalized to fs.db_height = 100.
   finalized_height(200) = 100. Function value == fs tip. Consistent.
2. Validator reorgs ~50 blocks (within the protocol bound). New
   best-known-chain tip = 180.
3. finalized_height(180) = 80. But fs.db_height is still 100 —
   those finalized blocks at heights 1..=100 weren't touched by the
   reorg (it only reached heights 151..=200).

Why current callers tolerate it

• fs.sync_to_height(finalized_height(chain_height), &source) iterates
  (db_height + 1)..=target. If target < db_height, the range is empty
  → no-op. Correct outcome by accident; the name suggests the call
  conveys "the actual finalized tip is now this value," but really it's
  "advance the DB toward this value if we haven't already passed it."

• ChainIndexSnapshot::StillSyncingFinalizedState { validator_finalized_height }
  is used for read-routing — queries at heights ≤ this value are passed
  through to the finalized DB. Under-reporting (returning 80 when fs is
  at 100) means heights 81..=100 get routed to the non-existent NFS
  instead of the finalized DB. Probably manifests as "no result" rather
  than wrong data, but the snapshot is misleading about what zaino has
  actually finalized.

Proposed rename

Rename to finalized_height_floor(chain_tip). The _floor suffix cues
the reader that the actual finalized tip is max(fs.db_height, finalized_height_floor(chain_tip)) and that fs.db_height() is the
right call when you want "what zaino has actually finalized."

Whichever name lands, also: the
ChainIndexSnapshot::StillSyncingFinalizedState { validator_finalized_height }
field at chain_index.rs:1290 currently stores this lower-bound value
under a name that suggests "what the validator has actually finalized."
That's the read-routing under-reporting site. Worth re-naming and/or
sourcing it from fs.db_height for the read-routing use.

Related

• zebra_state::MAX_BLOCK_REORG_HEIGHT = MIN_TRANSPARENT_COINBASE_MATURITY - 1 = 99
  — Zebra's protocol-derived constant; zaino currently uses 100 (one
  extra block of safety margin), already noted in the doc on
  NON_FINALIZED_DEPTH.
• chain_index sync iter: NFS sync's unbounded while loop overshoots iter's chain_height #1126 (race fix that introduced the iter-level commitment to a single
  chain_height snapshot — the function is called once per iter and the
  value is treated as immutable; this PR didn't change the semantic,
  just named it).