From 7ec528f1c23107912dc2be10f970bce3e420309a Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 26 May 2026 13:36:39 -0700 Subject: [PATCH 01/31] =?UTF-8?q?=20=20test:=20add=20red=20driver=20for=20?= =?UTF-8?q?#1096=20=E2=80=94=20best=5Fchaintip=20must=20derive=20from=20NF?= =?UTF-8?q?S=20snapshot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the target invariant for the lazy-NonFinalizedState collapse (#1096): best_chaintip must read the chain tip from the non-finalized snapshot in every availability state, never via a validator passthrough. The test constructs the cold-start ChainIndexSnapshot::StillSyncingFinalizedState variant and asserts best_chaintip reports the real chain tip. It fails today: that variant has no snapshot tip, so the StillSyncingFinalizedState arm round- trips to the validator and returns the finalized floor (validator_finalized_height) instead of the tip — the same fallible path that can surface database_hole. After #1096 there is no still-syncing variant; the always-present snapshot carries best_tip, read directly with no validator call, and this test will be rewritten against a real snapshot_nonfinalized_state() result. This is a failing-on-purpose driver, not a surviving characterization test — it pins cold-start shape precisely to drive that variant's elimination, so the churn it incurs at the fix is intended. The module doc flags it as the one exception to this file's "don't pin the still-syncing variant" rule. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../chain_index/tests/non_finalised_state.rs | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index 9d474a5c2..f264ef168 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -19,8 +19,17 @@ //! Tests of the cold-start "still-syncing" variant are deliberately //! omitted: that variant is being eliminated, and pinning its shape //! would create immediate test churn at the refactor PR. +//! +//! The one exception is the trailing **red driver** for #1096 +//! (`best_chaintip_derives_tip_from_nfs_snapshot_not_validator_passthrough`). +//! Unlike the characterization tests above — which pin behavior that must +//! survive the refactor unchanged — that test is *failing on purpose* and is +//! expected to be rewritten when the still-syncing variant is removed. It +//! pins cold-start shape precisely because it is driving that variant's +//! elimination, so the churn it incurs is the point, not an accident. use super::{load_test_vectors_and_sync_chain_index, poll::poll_until, MockchainMode}; +use crate::chain_index::non_finalised_state::ChainIndexSnapshot; use crate::chain_index::{finalized_height_floor, ChainIndex}; use std::time::Duration; use tokio::time::sleep; @@ -286,3 +295,59 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi pre_mine_finalized_height.0, ); } + +/// **Red driver for #1096** (NOT a surviving characterization test — see the +/// module-level doc; this one is *failing on purpose* and is expected to be +/// rewritten when the still-syncing variant is removed). +/// +/// Target invariant: `best_chaintip` must derive the chain tip from the +/// non-finalized snapshot in *every* availability state — it must never fall +/// back to a validator passthrough. +/// +/// Today the lazy design hands out +/// [`ChainIndexSnapshot::StillSyncingFinalizedState`] during the cold-start +/// window, before the NFS slot is populated. In that variant `best_chaintip` +/// (`chain_index.rs`, the `StillSyncingFinalizedState` arm) has no snapshot +/// tip to read, so it round-trips to the validator and reports the *finalized +/// floor* (`validator_finalized_height`) as the tip — a stale height, and a +/// fallible call that surfaces `database_hole` if the validator can't serve +/// the floor block. +/// +/// After #1096 there is no still-syncing variant: the snapshot always carries +/// an NFS `best_tip`, so `best_chaintip` reads it directly and reports the +/// real tip with no validator call. The test will then be rewritten to assert +/// the invariant over a snapshot returned by `snapshot_nonfinalized_state()`. +/// +/// multi_thread: depends on the harness's background sync loop advancing the +/// NFS concurrently with the setup's poll-until-ready loop. +#[tokio::test(flavor = "multi_thread")] +async fn best_chaintip_derives_tip_from_nfs_snapshot_not_validator_passthrough() { + let (_blocks, _indexer, index_reader, mockchain) = + load_test_vectors_and_sync_chain_index(MockchainMode::Active).await; + + // The chain tip the always-present NFS snapshot reports: the harness syncs + // the NFS to exactly the source's active height before returning. + let chain_tip = mockchain.active_height(); + + // The cold-start variant the lazy design can hand out instead. Its + // `validator_finalized_height` is the true floor — exactly what + // `snapshot_nonfinalized_state()` computes while the NFS slot is `None`. + let cold_start_snapshot = ChainIndexSnapshot::StillSyncingFinalizedState { + validator_finalized_height: finalized_height_floor(chain_tip), + }; + + let tip = index_reader + .best_chaintip(&cold_start_snapshot) + .await + .expect("best_chaintip resolves against a still-syncing snapshot"); + + assert_eq!( + tip.height.0, + chain_tip, + "best_chaintip must derive the tip from the NFS snapshot and report the \ + chain tip ({chain_tip}) in every availability state; today the cold-start \ + variant passes through to the validator and reports the finalized floor \ + ({}) instead (#1096)", + finalized_height_floor(chain_tip).0, + ); +} From e5e0665a8349124edf854ce842008cb1ebb986ad Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 26 May 2026 17:02:36 -0700 Subject: [PATCH 02/31] zaino-state: add ProvisionalBlock and relative-work type for #1096 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Groundwork for the non-finalized-state rework: introduce the block representation the NFS will hold once it is built eagerly from the validator, ahead of wiring it into the snapshot. A non-finalized block cannot carry absolute cumulative chainwork until the finalized DB catches up to the seam — the validator does not expose chainwork (both backends drop the field) and zaino accumulates it in the FS. So: - ProvisionalBlock: the NFS's block. Mirrors IndexedBlock's payload but has no absolute chainwork field; it carries cumulative work measured relative to the seam (header-derived) and becomes an IndexedBlock via into_indexed() once the seam's absolute base is known. Its parent_hash is documented untrusted while provisional — the linkage is unvalidated until the seam connects. - ProvisionalCumulativeWork(ChainWork): a type distinct from absolute ChainWork, so passing relative work where absolute is required — or into an IndexedBlock's chainwork field — is a compile error rather than a naming convention. This is the misattribution guard. - block_to_provisional_block / provisional_block_from_parts build it by reusing the BlockWithMetadata extractors, now pub(crate): block_work, extract_block_data, extract_transactions, create_commitment_tree_data. The header-work step is factored into block_work() and shared with the absolute path. IndexedBlock's shape is unchanged. Not yet wired into NonfinalizedBlockCacheSnapshot (which still stores IndexedBlock), so these items are currently unused; the storage swap and the sync/query rewrite land in the following commit. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/chain_index/non_finalised_state.rs | 220 +++++++++++++++++- .../src/chain_index/types/helpers.rs | 29 ++- 2 files changed, 238 insertions(+), 11 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 3952a35f5..e7b1b39d5 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -1,7 +1,12 @@ use super::{finalised_state::ZainoDB, source::BlockchainSource, NON_FINALIZED_DEPTH}; +use crate::chain_index::types::db::{ + legacy::{BlockData, CompactTxData}, + CommitmentTreeData, +}; use crate::{ chain_index::types::{ - self, BlockHash, BlockIndex, BlockMetadata, BlockWithMetadata, Height, TreeRootData, + self, BlockContext, BlockHash, BlockIndex, BlockMetadata, BlockWithMetadata, Height, + TreeRootData, }, error::FinalisedStateError, ChainWork, IndexedBlock, @@ -94,6 +99,142 @@ pub(crate) struct NonfinalizedBlockCacheSnapshot { pub best_tip: BlockIndex, } +/// Cumulative work measured *relative to the seam*, header-derived. +/// +/// A distinct type from [`ChainWork`] (which is ABSOLUTE) precisely so the two +/// cannot be confused at a call site: passing relative work where absolute is +/// required — or writing it into an [`IndexedBlock`]'s `chainwork` field — is a +/// type error, not merely a naming convention. This is the misattribution +/// guard in the type system. +/// +/// Relative work is a sound best-tip ordering within the non-finalized window: +/// under the assumption that no reorg exceeds [`NON_FINALIZED_DEPTH`], the seam +/// is a stable common ancestor of every competing chain, so relative ordering +/// matches absolute ordering. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) struct ProvisionalCumulativeWork(ChainWork); + +impl ProvisionalCumulativeWork { + /// Relative work at the seam: zero by definition (the seam is the base). + pub(crate) fn seam() -> Self { + Self(ChainWork::from_u256(U256::zero())) + } + + /// Accumulate one block's own (header-derived) work onto the running + /// relative total. + pub(crate) fn add_block_work(&self, block_work: &ChainWork) -> Self { + Self(self.0.add(block_work)) + } + + /// Resolve to ABSOLUTE chainwork given the seam's absolute base + /// (`absolute = seam_base + relative`). Only callable once the base is + /// known — i.e. at the resolution boundary. + pub(crate) fn resolve(&self, seam_base: &ChainWork) -> ChainWork { + seam_base.add(&self.0) + } +} + +/// A non-finalized block as held by the NFS. +/// +/// It is deliberately **not** an [`IndexedBlock`]. "Indexed" means the block +/// has been placed in the validated chain with its *absolute* cumulative +/// chainwork — and that absolute value is unknowable while the finalized +/// state has not yet caught up to the seam (the validator does not expose +/// chainwork, and the finalized DB has not reached the floor). A provisional +/// block instead carries [`Self::provisional_cumulative_work`]: cumulative +/// work measured *relative to the seam*, derived from block headers alone. +/// +/// Relative work is sufficient to choose the best tip within the +/// non-finalized window: under the assumption that no reorg exceeds +/// [`NON_FINALIZED_DEPTH`], the seam is a stable common ancestor of every +/// competing window chain, so ordering by relative work matches ordering by +/// absolute work. +/// +/// A provisional block becomes an [`IndexedBlock`] only at the resolution +/// boundary, via [`Self::into_indexed`], when the seam's absolute work is +/// known: `absolute = seam_base + provisional_cumulative_work`. The relative +/// value lives only here and is never written into +/// [`IndexedBlock`]'s absolute `chainwork` field, so the two cannot be +/// misattributed. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ProvisionalBlock { + /// Height + hash of this block. + index: BlockIndex, + /// Parent block hash as *claimed by this block's header*. + /// + /// UNTRUSTED while the snapshot is provisional: the linkage it asserts is + /// not validated against the finalized chain until the seam connects, so + /// it must not be used for trusted chain-walks (e.g. fork-point recursion) + /// during the provisional stage. + parent_hash: BlockHash, + /// Cumulative work relative to the seam, header-derived. Never absolute — + /// the type enforces that (see [`ProvisionalCumulativeWork`]). + provisional_cumulative_work: ProvisionalCumulativeWork, + /// Header and auxiliary block data. + data: BlockData, + /// Compact representations of the block's transactions. + transactions: Vec, + /// Commitment tree data for the chain after this block is applied. + commitment_tree_data: CommitmentTreeData, +} + +impl ProvisionalBlock { + /// The block hash. + pub(crate) fn hash(&self) -> &BlockHash { + &self.index.hash + } + + /// The block height. + pub(crate) fn height(&self) -> Height { + self.index.height + } + + /// The parent block hash as claimed by the header. UNTRUSTED while + /// provisional — see the field doc; do not use for validated chain-walks. + pub(crate) fn parent_hash(&self) -> &BlockHash { + &self.parent_hash + } + + /// Cumulative work relative to the seam. Use this — never an absolute + /// chainwork — for best-tip selection within the non-finalized window. + pub(crate) fn provisional_cumulative_work(&self) -> &ProvisionalCumulativeWork { + &self.provisional_cumulative_work + } + + /// The compact transactions in this block. + pub(crate) fn transactions(&self) -> &[CompactTxData] { + &self.transactions + } + + /// Header and auxiliary block data. + pub(crate) fn data(&self) -> &BlockData { + &self.data + } + + /// Commitment tree data after this block. + pub(crate) fn commitment_tree_data(&self) -> &CommitmentTreeData { + &self.commitment_tree_data + } + + /// Promote to an [`IndexedBlock`] once the seam's absolute cumulative work + /// is known (the resolution boundary). The absolute chainwork is + /// `seam_base + provisional_cumulative_work`. + pub(crate) fn into_indexed(self, seam_base: &ChainWork) -> IndexedBlock { + let chainwork = self.provisional_cumulative_work.resolve(seam_base); + IndexedBlock::new( + BlockContext::new( + self.index.hash, + self.parent_hash, + chainwork, + self.index.height, + ), + self.data, + self.transactions, + self.commitment_tree_data, + ) + } +} + #[derive(Debug)] /// Could not connect to a validator pub enum NodeConnectionError { @@ -623,6 +764,83 @@ impl NonFinalizedState { self.current.load_full() } + /// Build a [`ProvisionalBlock`] from a source block, accumulating its + /// header work onto the parent's relative total. Mirrors + /// [`Self::block_to_chainblock`] but produces a provisional (not indexed) + /// block: no absolute chainwork is computed or required. + async fn block_to_provisional_block( + &self, + parent_work: &ProvisionalCumulativeWork, + block: &zebra_chain::block::Block, + ) -> Result { + let tree_roots = self + .get_tree_roots_from_source(block.hash().into()) + .await + .map_err(|e| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( + Box::new(InvalidData(format!("{}", e))), + )) + })?; + + Self::provisional_block_from_parts(block, &tree_roots, parent_work, self.network.clone()) + .map_err(|e| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( + Box::new(InvalidData(e)), + )) + }) + } + + /// Assemble a [`ProvisionalBlock`] from already-fetched parts. Reuses the + /// shared `BlockWithMetadata` extractors (`extract_block_data`, + /// `extract_transactions`, `create_commitment_tree_data`, `block_work`); + /// the only divergence from the indexed path is that work is accumulated + /// *relative to the seam* instead of into an absolute `BlockContext`. + fn provisional_block_from_parts( + block: &zebra_chain::block::Block, + tree_roots: &TreeRootData, + parent_work: &ProvisionalCumulativeWork, + network: Network, + ) -> Result { + let (sapling_root, sapling_size, orchard_root, orchard_size) = + tree_roots.clone().extract_with_defaults(); + + // `parent_chainwork` is unused for a provisional block (it feeds only + // `create_block_context`, which we never call here); a zero placeholder + // keeps the throwaway `BlockMetadata` shape without touching absolute + // work. The relative total is tracked separately, below. + let metadata = BlockMetadata::new( + sapling_root, + sapling_size as u32, + orchard_root, + orchard_size as u32, + ChainWork::from_u256(U256::zero()), + network, + ); + let block_with_metadata = BlockWithMetadata::new(block, metadata); + + let data = block_with_metadata.extract_block_data()?; + let transactions = block_with_metadata.extract_transactions()?; + let commitment_tree_data = block_with_metadata.create_commitment_tree_data(); + let provisional_cumulative_work = + parent_work.add_block_work(&block_with_metadata.block_work()?); + + let hash = BlockHash::from(block.hash()); + let parent_hash = BlockHash::from(block.header.previous_block_hash); + let height = block + .coinbase_height() + .map(|height| Height(height.0)) + .ok_or_else(|| String::from("Any valid block has a coinbase height"))?; + + Ok(ProvisionalBlock { + index: BlockIndex { height, hash }, + parent_hash, + provisional_cumulative_work, + data, + transactions, + commitment_tree_data, + }) + } + async fn block_to_chainblock( &self, prev_block: &IndexedBlock, diff --git a/packages/zaino-state/src/chain_index/types/helpers.rs b/packages/zaino-state/src/chain_index/types/helpers.rs index 9d0b24819..4d19a7039 100644 --- a/packages/zaino-state/src/chain_index/types/helpers.rs +++ b/packages/zaino-state/src/chain_index/types/helpers.rs @@ -129,8 +129,23 @@ impl<'a> BlockWithMetadata<'a> { Self { block, metadata } } + /// This block's own work, derived from its header difficulty target. + /// Header-only, so it is shared by both absolute chainwork accumulation + /// (`create_block_context`) and relative/provisional accumulation. + pub(crate) fn block_work(&self) -> Result { + let work = self + .block + .header + .difficulty_threshold + .to_work() + .ok_or_else(|| { + "Failed to calculate block work from difficulty threshold".to_string() + })?; + Ok(ChainWork::from(U256::from(work.as_u128()))) + } + /// Extract block header data - fn extract_block_data(&self) -> Result { + pub(crate) fn extract_block_data(&self) -> Result { let block = self.block; let network = &self.metadata.network; @@ -150,7 +165,7 @@ impl<'a> BlockWithMetadata<'a> { } /// Extract and process all transactions in the block - fn extract_transactions(&self) -> Result, String> { + pub(crate) fn extract_transactions(&self) -> Result, String> { let mut transactions = Vec::new(); for (i, txn) in self.block.transactions.iter().enumerate() { @@ -278,19 +293,13 @@ impl<'a> BlockWithMetadata<'a> { .map(|height| Height(height.0)) .ok_or_else(|| String::from("Any valid block has a coinbase height"))?; - let block_work = block.header.difficulty_threshold.to_work().ok_or_else(|| { - "Failed to calculate block work from difficulty threshold".to_string() - })?; - let chainwork = self - .metadata - .parent_chainwork - .add(&ChainWork::from(U256::from(block_work.as_u128()))); + let chainwork = self.metadata.parent_chainwork.add(&self.block_work()?); Ok(BlockContext::new(hash, parent_hash, chainwork, height)) } /// Create commitment tree data from metadata - fn create_commitment_tree_data(&self) -> super::db::CommitmentTreeData { + pub(crate) fn create_commitment_tree_data(&self) -> super::db::CommitmentTreeData { let commitment_tree_roots = super::db::CommitmentTreeRoots::new( <[u8; 32]>::from(self.metadata.sapling_root), <[u8; 32]>::from(self.metadata.orchard_root), From a5a2b29caa9ff27b1ccdb2c0898c8024b3999146 Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 26 May 2026 17:51:58 -0700 Subject: [PATCH 03/31] ignore red test --- .../zaino-state/src/chain_index/tests/non_finalised_state.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index f264ef168..339920e52 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -321,6 +321,11 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi /// multi_thread: depends on the harness's background sync loop advancing the /// NFS concurrently with the setup's poll-until-ready loop. #[tokio::test(flavor = "multi_thread")] +#[ignore = "red driver for #1096: fails on purpose against today's still-syncing \ + variant, which passes through to the validator and reports the \ + finalized floor instead of the tip. Pins the target invariant to \ + drive that variant's elimination; rewritten against \ + snapshot_nonfinalized_state() once #1096 lands."] async fn best_chaintip_derives_tip_from_nfs_snapshot_not_validator_passthrough() { let (_blocks, _indexer, index_reader, mockchain) = load_test_vectors_and_sync_chain_index(MockchainMode::Active).await; From d97d617b6d14ce278068ce204549fe63db1cf70a Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 26 May 2026 18:16:36 -0700 Subject: [PATCH 04/31] zaino-state: store ProvisionalBlock in the NFS (#1096 wiring) Wire the validator-sourced, in-memory-only ProvisionalBlock through the non-finalized state, replacing IndexedBlock as the NFS's block type. The NFS now tracks work *relative to the seam* and never depends on absolute chainwork, which is unavailable until the finalized DB catches up. NFS internals (non_finalised_state.rs): - NonfinalizedBlockCacheSnapshot.blocks: HashMap<_, ProvisionalBlock>. - from_initial_block seeds the seam anchor via ProvisionalBlock::from_indexed_seam (relative work = 0); add_block/add_block_new_chaintip take ProvisionalBlock. - The Block reorg-builder trait gains to_provisional_block (replacing to_indexed_block); impl moves from IndexedBlock to ProvisionalBlock. - sync, handle_reorg, add_nonbest_block build provisional blocks; update's best-tip max_by_key keys on provisional_cumulative_work. Query surface (chain_index.rs): - NonFinalizedSnapshot::get_chainblock_by_* return &ProvisionalBlock; the trait is now pub(crate) (in-crate only, not re-exported). - ProvisionalBlock::to_compact_block and IndexedBlock::to_compact_block share one chainwork-free assembly, compact_block_from_parts (no duplication). - get_indexed_block_by_*/blocks_containing_transaction unify the finalized (IndexedBlock) and non-finalized (ProvisionalBlock) layers behind a ChainBlock view exposing hash/height/data. Interim, resolved by the follow-up Availability step (#1096): ChainBlock and the get_indexed_block_by_* returns become IndexedBlock once the FS is fully synced (Resolved); ProvisionalBlock::into_indexed / ProvisionalCumulativeWork::resolve are the resolution bridge and are currently unused. The red-driver test stays intentionally failing until best_chaintip reads best_tip. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/zaino-state/src/chain_index.rs | 104 ++++++++++---- .../src/chain_index/non_finalised_state.rs | 131 ++++++++++++------ .../chain_index/tests/proptest_blockgen.rs | 5 +- .../src/chain_index/types/db/legacy.rs | 74 ++++++---- 4 files changed, 214 insertions(+), 100 deletions(-) diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index 969207fcc..01ec771d8 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -26,7 +26,7 @@ use std::{sync::Arc, time::Duration}; use arc_swap::ArcSwapOption; use futures::{FutureExt, Stream}; use hex::FromHex as _; -use non_finalised_state::NonfinalizedBlockCacheSnapshot; +use non_finalised_state::{NonfinalizedBlockCacheSnapshot, ProvisionalBlock}; use source::{BlockchainSource, ValidatorConnector}; use tokio_stream::StreamExt; use tokio_util::sync::CancellationToken; @@ -95,7 +95,7 @@ pub(crate) fn chain_tips_from_nonfinalized_snapshot( let parent_hashes = snapshot .blocks .values() - .map(|block| *block.context.parent_hash()) + .map(|block| *block.parent_hash()) .collect::>(); let mut tip_hashes = snapshot @@ -142,7 +142,7 @@ pub(crate) fn chain_tips_from_nonfinalized_snapshot( fn branch_len_to_active_chain( snapshot: &NonfinalizedBlockCacheSnapshot, - block: &IndexedBlock, + block: &ProvisionalBlock, ) -> u32 { let mut branch_len = 0; let mut current = block; @@ -154,7 +154,7 @@ fn branch_len_to_active_chain( branch_len += 1; - let parent_hash = current.context.parent_hash(); + let parent_hash = current.parent_hash(); let Some(parent) = snapshot.blocks.get(parent_hash) else { return branch_len; }; @@ -162,6 +162,43 @@ fn branch_len_to_active_chain( } } +/// A block from either chain layer: the finalized state (an [`IndexedBlock`], +/// carrying absolute chainwork) or the non-finalized state (a +/// [`ProvisionalBlock`], carrying relative work). Unifies the two for query +/// methods that may resolve a block from either layer while the NFS holds +/// provisional blocks. +/// +/// Interim shape: once the Availability/Resolved work lands, a resolved NFS +/// block promotes to `IndexedBlock` (via `into_indexed`), and these methods +/// return `IndexedBlock` directly — see issue #1096. +pub(crate) enum ChainBlock { + Finalized(IndexedBlock), + NonFinalized(ProvisionalBlock), +} + +impl ChainBlock { + pub(crate) fn hash(&self) -> &types::BlockHash { + match self { + ChainBlock::Finalized(block) => block.hash(), + ChainBlock::NonFinalized(block) => block.hash(), + } + } + + pub(crate) fn height(&self) -> types::Height { + match self { + ChainBlock::Finalized(block) => block.height(), + ChainBlock::NonFinalized(block) => block.height(), + } + } + + pub(crate) fn data(&self) -> &types::db::legacy::BlockData { + match self { + ChainBlock::Finalized(block) => block.data(), + ChainBlock::NonFinalized(block) => block.data(), + } + } +} + /// The interface to the chain index. /// /// `ChainIndex` provides a unified interface for querying blockchain data from different @@ -326,11 +363,15 @@ pub trait ChainIndex { /// /// **NOTE: This Method is currently not "passthrough aware", cumulative /// chain work must be made optional to enable this.** + // Interim return type while the NFS holds provisional blocks: a finalized + // hit is an `IndexedBlock`, a non-finalized hit a `ProvisionalBlock`. Once + // the FS is fully synced (Resolved), these resolve to `IndexedBlock` (#1096). + #[allow(private_interfaces)] fn get_indexed_block_by_hash( &self, snapshot: &Self::Snapshot, target_hash: &types::BlockHash, - ) -> impl std::future::Future, Self::Error>>; + ) -> impl std::future::Future, Self::Error>>; /// Returns Some(IndexedBlock) for the given block height.in the best chain. /// @@ -338,11 +379,12 @@ pub trait ChainIndex { /// /// **NOTE: This Method is currently not "passthrough aware", cumulative /// chain work must be made optional to enable this.** + #[allow(private_interfaces)] fn get_indexed_block_by_height( &self, snapshot: &Self::Snapshot, target_height: &types::Height, - ) -> impl std::future::Future, Self::Error>>; + ) -> impl std::future::Future, Self::Error>>; /// Given inclusive start and end heights, stream all blocks /// between the given heights. @@ -1093,7 +1135,7 @@ impl NodeBackedChainIndexSubscriber { .values() .find(|h| **h == hash) // Canonical height is None for blocks not on the best chain - .map(|_| block.context.index.height)), + .map(|_| block.height())), None => self // ChainIndex step 4: .finalized_state @@ -1113,7 +1155,7 @@ impl NodeBackedChainIndexSubscriber { &'self_lt self, snapshot: &'snapshot NonfinalizedBlockCacheSnapshot, txid: [u8; 32], - ) -> Result + use<'iter, Source>, FinalisedStateError> + ) -> Result + use<'iter, Source>, FinalisedStateError> where 'snapshot: 'iter, 'self_lt: 'iter, @@ -1131,12 +1173,13 @@ impl NodeBackedChainIndexSubscriber { None => None, } - .into_iter(); + .into_iter() + .map(ChainBlock::Finalized); let non_finalized_blocks_containing_transaction = snapshot.blocks.values().filter_map(move |block| { block.transactions().iter().find_map(|transaction| { if transaction.txid().0 == txid { - Some(block.clone()) + Some(ChainBlock::NonFinalized(block.clone())) } else { None } @@ -1342,14 +1385,15 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result, Self::Error> { + ) -> Result, Self::Error> { match snapshot.get_chainblock_by_hash(target_hash) { - Some(block) => Ok(Some(block.clone())), + Some(block) => Ok(Some(ChainBlock::NonFinalized(block.clone()))), None => match self.get_block_height(snapshot, *target_hash).await { Ok(Some(height)) => Ok(self .finalized_state .get_chain_block_by_height(height) - .await?), + .await? + .map(ChainBlock::Finalized)), Ok(None) => Ok(None), Err(e) => Err(e), }, @@ -1366,13 +1410,14 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result, Self::Error> { + ) -> Result, Self::Error> { match snapshot.get_chainblock_by_height(target_height) { - Some(block) => Ok(Some(block.clone())), + Some(block) => Ok(Some(ChainBlock::NonFinalized(block.clone()))), None => Ok(self .finalized_state .get_chain_block_by_height(*target_height) - .await?), + .await? + .map(ChainBlock::Finalized)), } } @@ -1742,7 +1787,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber block.context.index.height.into(), + Some(block) => block.height().into(), // As above Ok(None) None => return Ok(None), } @@ -2024,9 +2069,10 @@ impl ChainIndex for NodeBackedChainIndexSubscriber NonFinalizedSnapshot for Arc where T: NonFinalizedSnapshot, { - fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&IndexedBlock> { + fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock> { self.as_ref().get_chainblock_by_hash(target_hash) } - fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&IndexedBlock> { + fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock> { self.as_ref().get_chainblock_by_height(target_height) } @@ -2446,17 +2492,17 @@ where } /// A snapshot of the non-finalized state, for consistent queries -pub trait NonFinalizedSnapshot { +pub(crate) trait NonFinalizedSnapshot { /// Hash -> block - fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&IndexedBlock>; + fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock>; /// Height -> block - fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&IndexedBlock>; + fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock>; /// The maximum height that this snapshot can serve data for. fn max_serviceable_height(&self) -> &types::Height; } impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { - fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&IndexedBlock> { + fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock> { self.blocks.iter().find_map(|(hash, chainblock)| { if hash == target_hash { Some(chainblock) @@ -2465,7 +2511,7 @@ impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { } }) } - fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&IndexedBlock> { + fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock> { self.heights_to_hashes.iter().find_map(|(height, hash)| { if height == target_height { self.get_chainblock_by_hash(hash) @@ -2481,7 +2527,7 @@ impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { } impl NonFinalizedSnapshot for ChainIndexSnapshot { - fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&IndexedBlock> { + fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock> { match self { ChainIndexSnapshot::NonFinalizedStateExists { non_finalized_snapshot, @@ -2491,7 +2537,7 @@ impl NonFinalizedSnapshot for ChainIndexSnapshot { } } - fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&IndexedBlock> { + fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock> { match self { ChainIndexSnapshot::NonFinalizedStateExists { non_finalized_snapshot, diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index e7b1b39d5..cd0246ca2 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -1,6 +1,6 @@ use super::{finalised_state::ZainoDB, source::BlockchainSource, NON_FINALIZED_DEPTH}; use crate::chain_index::types::db::{ - legacy::{BlockData, CompactTxData}, + legacy::{compact_block_from_parts, BlockData, CompactTxData}, CommitmentTreeData, }; use crate::{ @@ -88,7 +88,7 @@ pub(crate) struct NonfinalizedBlockCacheSnapshot { /// this includes all blocks on-chain, as well as /// all blocks known to have been on-chain before being /// removed by a reorg. Blocks reorged away have no height. - pub blocks: HashMap, + pub blocks: HashMap, /// hashes indexed by height /// Hashes in this map are part of the best chain. pub heights_to_hashes: HashMap, @@ -216,6 +216,36 @@ impl ProvisionalBlock { &self.commitment_tree_data } + /// The compact-block representation. Compact blocks carry no cumulative + /// chainwork, so this is identical to an indexed block's; both delegate to + /// [`compact_block_from_parts`]. + pub(crate) fn to_compact_block(&self) -> zaino_proto::proto::compact_formats::CompactBlock { + compact_block_from_parts( + self.height(), + self.hash(), + self.parent_hash(), + self.data().time(), + self.transactions(), + self.commitment_tree_data(), + ) + } + + /// Build the seam anchor from a finalized [`IndexedBlock`]. The anchor's + /// relative work is zero by definition ([`ProvisionalCumulativeWork::seam`]); + /// the block's absolute chainwork is intentionally dropped — the NFS tracks + /// work relative to this seam, and the absolute base is reattached only at + /// resolution. + pub(crate) fn from_indexed_seam(indexed: IndexedBlock) -> Self { + Self { + index: indexed.context.index, + parent_hash: indexed.context.parent_hash, + provisional_cumulative_work: ProvisionalCumulativeWork::seam(), + data: indexed.data, + transactions: indexed.transactions, + commitment_tree_data: indexed.commitment_tree_data, + } + } + /// Promote to an [`IndexedBlock`] once the seam's absolute cumulative work /// is known (the resolution boundary). The absolute chainwork is /// `seam_base + provisional_cumulative_work`. @@ -348,7 +378,7 @@ impl NonfinalizedBlockCacheSnapshot { let mut blocks = HashMap::new(); let mut heights_to_hashes = HashMap::new(); - blocks.insert(hash, block); + blocks.insert(hash, ProvisionalBlock::from_indexed_seam(block)); heights_to_hashes.insert(height, hash); Self { @@ -358,7 +388,7 @@ impl NonfinalizedBlockCacheSnapshot { } } - fn add_block_new_chaintip(&mut self, block: IndexedBlock) { + fn add_block_new_chaintip(&mut self, block: ProvisionalBlock) { self.best_tip = BlockIndex { height: block.height(), hash: *block.hash(), @@ -366,7 +396,10 @@ impl NonfinalizedBlockCacheSnapshot { self.add_block(block) } - fn get_block_by_hash_bytes_in_serialized_order(&self, hash: [u8; 32]) -> Option<&IndexedBlock> { + fn get_block_by_hash_bytes_in_serialized_order( + &self, + hash: [u8; 32], + ) -> Option<&ProvisionalBlock> { self.blocks .values() .find(|block| block.hash_bytes_serialized_order() == hash) @@ -381,7 +414,7 @@ impl NonfinalizedBlockCacheSnapshot { .retain(|height, _hash| height >= &finalized_height); } - fn add_block(&mut self, block: IndexedBlock) { + fn add_block(&mut self, block: ProvisionalBlock) { self.heights_to_hashes.insert(block.height(), *block.hash()); self.blocks.insert(*block.hash(), block); } @@ -558,24 +591,29 @@ impl NonFinalizedState { let parent_hash = BlockHash::from(block.header.previous_block_hash); if parent_hash == working_snapshot.best_tip.hash { // Normal chain progression - let prev_block = working_snapshot - .blocks - .get(&working_snapshot.best_tip.hash) - .ok_or_else(|| { - SyncError::ReorgFailure(format!( - "found blocks {:?}, expected block {:?}", - working_snapshot - .blocks - .values() - .map(|block| (block.context.index.hash, block.context.index.height)) - .collect::>(), - working_snapshot.best_tip - )) - })?; - let chainblock = self.block_to_chainblock(prev_block, &block).await?; + let parent_work = { + let prev_block = working_snapshot + .blocks + .get(&working_snapshot.best_tip.hash) + .ok_or_else(|| { + SyncError::ReorgFailure(format!( + "found blocks {:?}, expected block {:?}", + working_snapshot + .blocks + .values() + .map(|block| (*block.hash(), block.height())) + .collect::>(), + working_snapshot.best_tip + )) + })?; + *prev_block.provisional_cumulative_work() + }; + let chainblock = self + .block_to_provisional_block(&parent_work, &block) + .await?; info!( height = (working_snapshot.best_tip.height + 1).0, - hash = %chainblock.context.index.hash, + hash = %chainblock.hash(), "Syncing block" ); working_snapshot.add_block_new_chaintip(chainblock); @@ -610,7 +648,7 @@ impl NonFinalizedState { &self, working_snapshot: &mut NonfinalizedBlockCacheSnapshot, block: &impl Block, - ) -> Result { + ) -> Result { let prev_block = match working_snapshot .get_block_by_hash_bytes_in_serialized_order(block.prev_hash_bytes_serialized_order()) .cloned() @@ -648,9 +686,10 @@ impl NonFinalizedState { Box::pin(self.handle_reorg(working_snapshot, &*prev_block)).await? } }; - let indexed_block = block.to_indexed_block(&prev_block, self).await?; - working_snapshot.add_block_new_chaintip(indexed_block.clone()); - Ok(indexed_block) + let parent_work = *prev_block.provisional_cumulative_work(); + let provisional_block = block.to_provisional_block(&parent_work, self).await?; + working_snapshot.add_block_new_chaintip(provisional_block.clone()); + Ok(provisional_block) } /// Handle non-finalized change listener events @@ -708,7 +747,7 @@ impl NonFinalizedState { let best_block = &new_snapshot .blocks .values() - .max_by_key(|block| block.chainwork()) + .max_by_key(|block| block.provisional_cumulative_work()) .cloned() .expect("empty snapshot impossible"); self.handle_reorg(&mut new_snapshot, best_block) @@ -913,7 +952,7 @@ impl NonFinalizedState { &self, working_snapshot: &mut NonfinalizedBlockCacheSnapshot, block: &impl Block, - ) -> Result { + ) -> Result { let prev_block = match working_snapshot .get_block_by_hash_bytes_in_serialized_order(block.prev_hash_bytes_serialized_order()) .cloned() @@ -941,9 +980,10 @@ impl NonFinalizedState { Box::pin(self.add_nonbest_block(working_snapshot, &*prev_block)).await? } }; - let indexed_block = block.to_indexed_block(&prev_block, self).await?; - working_snapshot.add_block(indexed_block.clone()); - Ok(indexed_block) + let parent_work = *prev_block.provisional_cumulative_work(); + let provisional_block = block.to_provisional_block(&parent_work, self).await?; + working_snapshot.add_block(provisional_block.clone()); + Ok(provisional_block) } } @@ -969,27 +1009,32 @@ pub enum UpdateError { trait Block { fn hash_bytes_serialized_order(&self) -> [u8; 32]; fn prev_hash_bytes_serialized_order(&self) -> [u8; 32]; - async fn to_indexed_block( + /// Produce the [`ProvisionalBlock`] for this block, accumulating its work + /// onto `parent_work` (relative to the seam). Replaces the former + /// `to_indexed_block`: the NFS holds provisional, not indexed, blocks. + async fn to_provisional_block( &self, - prev_block: &IndexedBlock, + parent_work: &ProvisionalCumulativeWork, nfs: &NonFinalizedState, - ) -> Result; + ) -> Result; } -impl Block for IndexedBlock { +impl Block for ProvisionalBlock { fn hash_bytes_serialized_order(&self) -> [u8; 32] { self.hash().0 } fn prev_hash_bytes_serialized_order(&self) -> [u8; 32] { - self.context.parent_hash.0 + self.parent_hash().0 } - async fn to_indexed_block( + async fn to_provisional_block( &self, - _prev_block: &IndexedBlock, + _parent_work: &ProvisionalCumulativeWork, _nfs: &NonFinalizedState, - ) -> Result { + ) -> Result { + // Identity: a block already in the snapshot keeps the relative work it + // was assigned when first added; `parent_work` is irrelevant here. Ok(self.clone()) } } @@ -1002,11 +1047,11 @@ impl Block for zebra_chain::block::Block { self.header.previous_block_hash.bytes_in_serialized_order() } - async fn to_indexed_block( + async fn to_provisional_block( &self, - prev_block: &IndexedBlock, + parent_work: &ProvisionalCumulativeWork, nfs: &NonFinalizedState, - ) -> Result { - nfs.block_to_chainblock(prev_block, self).await + ) -> Result { + nfs.block_to_provisional_block(parent_work, self).await } } diff --git a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs index 51740fe0a..f4cd18337 100644 --- a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs +++ b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs @@ -420,7 +420,10 @@ fn make_chain() { .unwrap(); for (hash, block) in &non_finalized_snapshot.blocks { if hash != &best_tip_hash { - assert!(block.chainwork().to_u256() <= best_tip_block.chainwork().to_u256()); + assert!( + block.provisional_cumulative_work() + <= best_tip_block.provisional_cumulative_work() + ); if non_finalized_snapshot.heights_to_hashes.get(&block.height()) == Some(block.hash()) { assert_eq!(index_reader.find_fork_point(&snapshot, hash).await.unwrap().unwrap().0, *hash); } else { diff --git a/packages/zaino-state/src/chain_index/types/db/legacy.rs b/packages/zaino-state/src/chain_index/types/db/legacy.rs index f0f970b0e..4b7e3c8cf 100644 --- a/packages/zaino-state/src/chain_index/types/db/legacy.rs +++ b/packages/zaino-state/src/chain_index/types/db/legacy.rs @@ -1179,33 +1179,53 @@ impl IndexedBlock { /// with tx data being added selectively here. pub fn to_compact_block(&self) -> zaino_proto::proto::compact_formats::CompactBlock { // NOTE: Returns u64::MAX if the block is not in the best chain. - let height: u64 = self.height().0.into(); - - let hash = self.hash().0.to_vec(); - let prev_hash = self.context.parent_hash.0.to_vec(); - - let vtx: Vec = self - .transactions() - .iter() - .map(|tx| tx.to_compact_tx(None)) - .collect(); - - let sapling_commitment_tree_size = self.commitment_tree_data().sizes().sapling(); - let orchard_commitment_tree_size = self.commitment_tree_data().sizes().orchard(); - - zaino_proto::proto::compact_formats::CompactBlock { - proto_version: 4, - height, - hash, - prev_hash, - time: self.data().time() as u32, - header: vec![], - vtx, - chain_metadata: Some(zaino_proto::proto::compact_formats::ChainMetadata { - sapling_commitment_tree_size, - orchard_commitment_tree_size, - }), - } + compact_block_from_parts( + self.height(), + self.hash(), + &self.context.parent_hash, + self.data().time(), + self.transactions(), + self.commitment_tree_data(), + ) + } +} + +/// Build a proto-v4 `CompactBlock` from the parts shared by [`IndexedBlock`] +/// and the non-finalized `ProvisionalBlock`. Compact-block construction needs +/// no cumulative chainwork, so both block types produce identical output via +/// this one assembly — no duplication. +pub(crate) fn compact_block_from_parts( + height: Height, + hash: &BlockHash, + parent_hash: &BlockHash, + time: i64, + transactions: &[CompactTxData], + commitment_tree_data: &CommitmentTreeData, +) -> zaino_proto::proto::compact_formats::CompactBlock { + let height: u64 = height.0.into(); + let hash = hash.0.to_vec(); + let prev_hash = parent_hash.0.to_vec(); + + let vtx: Vec = transactions + .iter() + .map(|tx| tx.to_compact_tx(None)) + .collect(); + + let sapling_commitment_tree_size = commitment_tree_data.sizes().sapling(); + let orchard_commitment_tree_size = commitment_tree_data.sizes().orchard(); + + zaino_proto::proto::compact_formats::CompactBlock { + proto_version: 4, + height, + hash, + prev_hash, + time: time as u32, + header: vec![], + vtx, + chain_metadata: Some(zaino_proto::proto::compact_formats::ChainMetadata { + sapling_commitment_tree_size, + orchard_commitment_tree_size, + }), } } From 366350f41fb01b3958c1e91a76eb9fd74d34ec29 Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 26 May 2026 18:25:26 -0700 Subject: [PATCH 05/31] rm dead code --- .../src/chain_index/non_finalised_state.rs | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index cd0246ca2..8e31e1cda 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -880,33 +880,6 @@ impl NonFinalizedState { }) } - async fn block_to_chainblock( - &self, - prev_block: &IndexedBlock, - block: &zebra_chain::block::Block, - ) -> Result { - let tree_roots = self - .get_tree_roots_from_source(block.hash().into()) - .await - .map_err(|e| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( - Box::new(InvalidData(format!("{}", e))), - )) - })?; - - Self::create_indexed_block_with_optional_roots( - block, - &tree_roots, - *prev_block.chainwork(), - self.network.clone(), - ) - .map_err(|e| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( - InvalidData(e), - ))) - }) - } - /// Get commitment tree roots from the blockchain source async fn get_tree_roots_from_source( &self, From 64b32090a203b50ab462690bebe5785d3d17b336 Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 26 May 2026 18:28:32 -0700 Subject: [PATCH 06/31] zaino-state: remove dead code left by the ProvisionalBlock wiring After the NFS switched to ProvisionalBlock storage, three items have no remaining callers: - block_to_chainblock: the old IndexedBlock sync-builder, superseded by block_to_provisional_block. - ProvisionalBlock::into_indexed / ProvisionalCumulativeWork::resolve: the forward Provisional -> absolute-chainwork conversion. It has no consumer until the NFS query surface resolves to IndexedBlock once the finalized state is fully synced (the Availability step, #1096); re-added there. The inverse, from_indexed_seam, stays (it builds the seam anchor and is live). - the now-unused BlockContext import (only into_indexed referenced it). No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/chain_index/non_finalised_state.rs | 39 +++---------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 8e31e1cda..6b65400c7 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -5,8 +5,7 @@ use crate::chain_index::types::db::{ }; use crate::{ chain_index::types::{ - self, BlockContext, BlockHash, BlockIndex, BlockMetadata, BlockWithMetadata, Height, - TreeRootData, + self, BlockHash, BlockIndex, BlockMetadata, BlockWithMetadata, Height, TreeRootData, }, error::FinalisedStateError, ChainWork, IndexedBlock, @@ -125,13 +124,6 @@ impl ProvisionalCumulativeWork { pub(crate) fn add_block_work(&self, block_work: &ChainWork) -> Self { Self(self.0.add(block_work)) } - - /// Resolve to ABSOLUTE chainwork given the seam's absolute base - /// (`absolute = seam_base + relative`). Only callable once the base is - /// known — i.e. at the resolution boundary. - pub(crate) fn resolve(&self, seam_base: &ChainWork) -> ChainWork { - seam_base.add(&self.0) - } } /// A non-finalized block as held by the NFS. @@ -150,12 +142,11 @@ impl ProvisionalCumulativeWork { /// competing window chain, so ordering by relative work matches ordering by /// absolute work. /// -/// A provisional block becomes an [`IndexedBlock`] only at the resolution -/// boundary, via [`Self::into_indexed`], when the seam's absolute work is -/// known: `absolute = seam_base + provisional_cumulative_work`. The relative -/// value lives only here and is never written into -/// [`IndexedBlock`]'s absolute `chainwork` field, so the two cannot be -/// misattributed. +/// A provisional block is promoted to an [`IndexedBlock`] only at the +/// resolution boundary, when the seam's absolute work becomes known +/// (`absolute = seam_base + provisional_cumulative_work`). The relative value +/// lives only here and is never written into [`IndexedBlock`]'s absolute +/// `chainwork` field, so the two cannot be misattributed. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct ProvisionalBlock { /// Height + hash of this block. @@ -245,24 +236,6 @@ impl ProvisionalBlock { commitment_tree_data: indexed.commitment_tree_data, } } - - /// Promote to an [`IndexedBlock`] once the seam's absolute cumulative work - /// is known (the resolution boundary). The absolute chainwork is - /// `seam_base + provisional_cumulative_work`. - pub(crate) fn into_indexed(self, seam_base: &ChainWork) -> IndexedBlock { - let chainwork = self.provisional_cumulative_work.resolve(seam_base); - IndexedBlock::new( - BlockContext::new( - self.index.hash, - self.parent_hash, - chainwork, - self.index.height, - ), - self.data, - self.transactions, - self.commitment_tree_data, - ) - } } #[derive(Debug)] From cd078289c47ea3d8b1787f55f6082d8ad9b0bed3 Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 26 May 2026 18:38:41 -0700 Subject: [PATCH 07/31] =?UTF-8?q?=20=20Introduce=20the=20Provisional/Resol?= =?UTF-8?q?ved=20availability=20the=20snapshot-collapse=20step=20=20=20wil?= =?UTF-8?q?l=20key=20on.=20It=20rides=20inside=20NonfinalizedBlockCacheSna?= =?UTF-8?q?pshot=20so=20it=20flips=20=20=20atomically=20with=20the=20CAS?= =?UTF-8?q?=20(compare-and-swap)=20that=20publishes=20the=20snapshot=20?= =?UTF-8?q?=E2=80=94=20=20=20resolution=20state=20and=20block=20contents?= =?UTF-8?q?=20become=20visible=20in=20one=20indivisible=20step.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SnapshotAvailability::Provisional { validator_finalized_height }: the finalized DB has not reached the seam; the window's prev-hash linkage is unvalidated and its blocks have no absolute chainwork, so finalized-range reads fall back to the validator up to validator_finalized_height. - SnapshotAvailability::Resolved { cumulative_chainwork_base }: the finalized DB has reached the seam; the seam block's absolute cumulative work is the base from which any window block's absolute chainwork derives (absolute = base + relative). Lifecycle: from_initial_block seeds Provisional; update() flips to Resolved exactly when the trimmed window still contains a block at the finalized-DB tip (seam overlap), stamping that FS block's absolute chainwork as the base — otherwise stays Provisional. The flip is computed before the CAS (compare-and-swap) swap, so it publishes with the snapshot. Foundation only: the availability fields are written but not yet read (a transient dead_code "field never read"), consumed by the following enum-collapse step that makes the query surface availability-aware. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/chain_index/non_finalised_state.rs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 6b65400c7..b2c36d0ff 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -80,6 +80,34 @@ impl ChainIndexSnapshot { } } +/// Whether a published snapshot's non-finalized window is anchored to the +/// validated finalized chain yet. +/// +/// The snapshot always exists and always carries blocks; this says whether the +/// finalized DB has caught up to the seam. Flipped to `Resolved` inside +/// `update`, atomically with the `compare_and_swap` that publishes the +/// snapshot (the value rides in the snapshot, so the flip is not separable +/// from the block contents). +#[derive(Debug, Clone, Copy)] +pub(crate) enum SnapshotAvailability { + /// The finalized DB has not reached the seam. The window's prev-hash + /// linkage is unvalidated and its blocks have no absolute chainwork; reads + /// that need finalized data fall back to the validator, serving up to + /// `validator_finalized_height`. + Provisional { + /// The finalized-tip height passthrough may serve up to. + validator_finalized_height: Height, + }, + /// The finalized DB has reached the seam: the window is validated, and each + /// block's absolute chainwork is `cumulative_chainwork_base + relative` + /// (the base is the seam block's absolute cumulative work, from the FS). + Resolved { + /// The seam's absolute cumulative work, the base for resolving any + /// window block's absolute chainwork. + cumulative_chainwork_base: ChainWork, + }, +} + #[derive(Debug, Clone)] /// A snapshot of the nonfinalized state as it existed when this was created. pub(crate) struct NonfinalizedBlockCacheSnapshot { @@ -96,6 +124,10 @@ pub(crate) struct NonfinalizedBlockCacheSnapshot { // best_tip is a BestTip, which contains // a Height, and a BlockHash as named fields. pub best_tip: BlockIndex, + /// Whether the finalized DB has caught up to this window's seam, and (when + /// it has) the seam's absolute-chainwork base. Set atomically with the + /// snapshot publish in `update`. + pub availability: SnapshotAvailability, } /// Cumulative work measured *relative to the seam*, header-derived. @@ -358,6 +390,11 @@ impl NonfinalizedBlockCacheSnapshot { blocks, heights_to_hashes, best_tip, + // Newly seeded from the seam block: the finalized DB has not yet + // caught up to it. `update` flips this to `Resolved` once it has. + availability: SnapshotAvailability::Provisional { + validator_finalized_height: height, + }, } } @@ -727,6 +764,32 @@ impl NonFinalizedState { .await .map_err(|_e| UpdateError::DatabaseHole)?; + // Resolve availability atomically with the publish below. The finalized + // DB has caught up iff the trimmed window still contains a block at the + // FS tip (the seam overlaps); when it does, the FS holds that block's + // absolute cumulative work, which is the base for the window's + // relative work. Until then the window floats free of the finalized + // chain — Provisional. + new_snapshot.availability = if new_snapshot + .heights_to_hashes + .contains_key(&finalized_height) + { + let cumulative_chainwork_base = finalized_db + .to_reader() + .get_chain_block_by_height(finalized_height) + .await + .map_err(|_e| UpdateError::FinalizedStateCorruption)? + .map(|seam_block| *seam_block.chainwork()) + .ok_or(UpdateError::FinalizedStateCorruption)?; + SnapshotAvailability::Resolved { + cumulative_chainwork_base, + } + } else { + SnapshotAvailability::Provisional { + validator_finalized_height: finalized_height, + } + }; + // Need to get best hash at some point in this process let stored = self .current From 2b992f9fd35a721b7152e5b5f4d507de2a6e9f89 Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 26 May 2026 19:07:40 -0700 Subject: [PATCH 08/31] Eliminate the StillSyncingFinalizedState path. The non-finalized state now always exists; a snapshot always carries blocks plus a SnapshotAvailability saying whether the finalized DB has caught up to its seam. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Data model: - ChainIndexSnapshot: enum -> struct { non_finalized_snapshot }. The availability rides inside NonfinalizedBlockCacheSnapshot, so it flips with the snapshot's CAS (compare-and-swap) publish in `update` — resolution state and block contents become visible in one indivisible step. - SnapshotAvailability { Provisional { validator_finalized_height } | Resolved }. from_initial_block seeds Provisional; update() flips to Resolved when the trimmed window still contains a block at the finalized-DB tip (seam overlap). Resolved is a unit variant for now — the seam's absolute-chainwork base (and its reader) return with the resolution-promotion step. - non_finalized_state: Arc> -> Arc, constructed eagerly in new_with_sync_timings. The sync loop's lazy `.expect("todo")` init and its dead `network` captures are gone; it just calls nfs.sync(...). - snapshot_nonfinalized_state is now infallible (the NFS is always present). Accessors / consumers: - get_nfs_snapshot() -> &NonfinalizedBlockCacheSnapshot (non-optional); is_resolved(); availability(); resolved_nfs_snapshot() -> Option<&...> (Some only when Resolved) for the "bail unless authoritative" call sites. - The ~16 match arms are availability-keyed: Resolved reads the NFS, Provisional keeps the validator passthrough. Arms that returned None *only* because the old design lacked an NFS (get_compact_block, get_mempool_height) now serve from the always-present NFS in both states; the finalized-gap remains the #1066 passthrough site. - best_chaintip reads best_tip from the snapshot directly in every state — no validator round-trip, no database_hole. The former #1096 RED driver is rewritten as a passing regression (best_chaintip_derives_tip_from_nfs_snapshot). The pub ChainIndex trait's surface references the in-crate ChainBlock / NonFinalizedSnapshot types; that lint is #[allow]ed (interim) rather than narrowing the public trait, since it's a re-exported API reachable by crates outside this workspace. Deferred: resolution promotion — re-add the seam base + into_indexed so get_indexed_block_by_*/ChainBlock return IndexedBlock when Resolved. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/zaino-state/src/backends/fetch.rs | 27 +- packages/zaino-state/src/backends/state.rs | 14 +- packages/zaino-state/src/chain_index.rs | 311 +++++++----------- .../src/chain_index/non_finalised_state.rs | 98 +++--- packages/zaino-state/src/chain_index/tests.rs | 2 +- .../src/chain_index/tests/mockchain_tests.rs | 6 +- .../chain_index/tests/non_finalised_state.rs | 95 ++---- .../chain_index/tests/proptest_blockgen.rs | 6 +- .../src/chain_index/tests/sync_loop.rs | 4 +- 9 files changed, 220 insertions(+), 343 deletions(-) diff --git a/packages/zaino-state/src/backends/fetch.rs b/packages/zaino-state/src/backends/fetch.rs index 8e4d6e874..c6047fb0c 100644 --- a/packages/zaino-state/src/backends/fetch.rs +++ b/packages/zaino-state/src/backends/fetch.rs @@ -75,7 +75,7 @@ use crate::{ BackendType, }; use crate::{ - chain_index::{non_finalised_state::ChainIndexSnapshot, NonFinalizedSnapshot}, + chain_index::{non_finalised_state::SnapshotAvailability, NonFinalizedSnapshot}, ChainIndex, NodeBackedChainIndex, NodeBackedChainIndexSubscriber, }; @@ -500,7 +500,7 @@ impl ZcashIndexer for FetchServiceSubscriber { async fn get_chain_tips(&self) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { return Ok(self.fetcher.get_chain_tips().await?); }; @@ -881,11 +881,12 @@ impl ZcashIndexer for FetchServiceSubscriber { impl LightWalletIndexer for FetchServiceSubscriber { /// Return the height of the tip of the best chain async fn get_latest_block(&self) -> Result { - match self.indexer.snapshot_nonfinalized_state().await? { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => Ok(non_finalized_snapshot.best_tip.to_wire()), - ChainIndexSnapshot::StillSyncingFinalizedState { .. } => { + let snapshot = self.indexer.snapshot_nonfinalized_state().await?; + match snapshot.availability() { + SnapshotAvailability::Resolved { .. } => { + Ok(snapshot.get_nfs_snapshot().best_tip.to_wire()) + } + SnapshotAvailability::Provisional { .. } => { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of reporting // the genesis block @@ -923,7 +924,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { } }; - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -999,7 +1000,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { } } }; - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -1096,7 +1097,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { let timeout_result = timeout( time::Duration::from_secs((service_timeout * 4) as u64), async { - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -1229,7 +1230,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { let timeout_result = timeout( time::Duration::from_secs((service_timeout * 4) as u64), async { - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -1366,7 +1367,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { // Zebra returns None for mempool transactions, convert to `Mempool Height`. None => { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -1722,7 +1723,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { let timeout = timeout( time::Duration::from_secs((service_timeout * 6) as u64), async { - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block diff --git a/packages/zaino-state/src/backends/state.rs b/packages/zaino-state/src/backends/state.rs index 61ad6f918..d559eda23 100644 --- a/packages/zaino-state/src/backends/state.rs +++ b/packages/zaino-state/src/backends/state.rs @@ -572,7 +572,7 @@ impl StateServiceSubscriber { time::Duration::from_secs((service_timeout * 4) as u64), async { // This method does not support passthrough. Just return. - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else {return}; + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else {return}; let chain_height = non_finalized_snapshot.best_tip.height.0; match state_service_clone @@ -1481,7 +1481,7 @@ impl ZcashIndexer for StateServiceSubscriber { /// tags: blockchain async fn get_block_count(&self) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -1493,7 +1493,7 @@ impl ZcashIndexer for StateServiceSubscriber { async fn get_chain_tips(&self) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { return Ok(self.rpc_client.get_chain_tips().await?); }; @@ -1878,7 +1878,7 @@ impl LightWalletIndexer for StateServiceSubscriber { /// Return the height of the tip of the best chain async fn get_latest_block(&self) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -1919,7 +1919,7 @@ impl LightWalletIndexer for StateServiceSubscriber { { Ok(Some(block)) => Ok(block), Ok(None) => { - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -1939,7 +1939,7 @@ impl LightWalletIndexer for StateServiceSubscriber { } } Err(e) => { - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block @@ -2373,7 +2373,7 @@ impl LightWalletIndexer for StateServiceSubscriber { let (channel_tx, channel_rx) = mpsc::channel(self.config.common.service.channel_size as usize); let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.get_nfs_snapshot() else { + let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of // acting as if we are only synced to the genesis block diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index 01ec771d8..56efc9915 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -11,19 +11,18 @@ //! - b. Build trasparent tx indexes efficiently //! - NOTE: Full transaction and block data is served from the backend finalizer. -use crate::chain_index::non_finalised_state::ChainIndexSnapshot; +use crate::chain_index::non_finalised_state::{ChainIndexSnapshot, SnapshotAvailability}; use crate::chain_index::source::GetTransactionLocation; use crate::chain_index::types::db::metadata::MempoolInfo; use crate::chain_index::types::BlockIndex; use crate::chain_index::types::{BestChainLocation, NonBestChainLocation}; use crate::error::{ChainIndexError, ChainIndexErrorKind, FinalisedStateError}; use crate::status::Status; -use crate::{CompactBlockStream, NamedAtomicStatus, NonFinalizedState, StatusType, SyncError}; +use crate::{CompactBlockStream, NamedAtomicStatus, StatusType, SyncError}; use crate::{IndexedBlock, Outpoint, TransactionHash, TxOutCompact}; use std::collections::HashSet; use std::{sync::Arc, time::Duration}; -use arc_swap::ArcSwapOption; use futures::{FutureExt, Stream}; use hex::FromHex as _; use non_finalised_state::{NonfinalizedBlockCacheSnapshot, ProvisionalBlock}; @@ -321,6 +320,12 @@ impl ChainBlock { /// /// When a call asks for info (e.g. a block), Zaino selects sources in this order: #[doc = simple_mermaid::mermaid!("chain_index_passthrough.mmd")] +// Interim (#1096): this pub trait's surface references in-crate types — the +// `Snapshot: NonFinalizedSnapshot` bound and the `ChainBlock` returned by +// `get_indexed_block_by_*` are `pub(crate)`. Keeping those types in-crate is +// the intended minimum visibility; the lint resolves when the resolution +// promotion turns `ChainBlock` into `IndexedBlock`. +#[allow(private_interfaces, private_bounds)] pub trait ChainIndex { /// A snapshot of the nonfinalized state, needed for atomic access type Snapshot: NonFinalizedSnapshot; @@ -363,10 +368,6 @@ pub trait ChainIndex { /// /// **NOTE: This Method is currently not "passthrough aware", cumulative /// chain work must be made optional to enable this.** - // Interim return type while the NFS holds provisional blocks: a finalized - // hit is an `IndexedBlock`, a non-finalized hit a `ProvisionalBlock`. Once - // the FS is fully synced (Resolved), these resolve to `IndexedBlock` (#1096). - #[allow(private_interfaces)] fn get_indexed_block_by_hash( &self, snapshot: &Self::Snapshot, @@ -379,7 +380,6 @@ pub trait ChainIndex { /// /// **NOTE: This Method is currently not "passthrough aware", cumulative /// chain work must be made optional to enable this.** - #[allow(private_interfaces)] fn get_indexed_block_by_height( &self, snapshot: &Self::Snapshot, @@ -696,7 +696,7 @@ pub trait ChainIndex { pub struct NodeBackedChainIndex { #[allow(dead_code)] mempool: std::sync::Arc>, - non_finalized_state: Arc>>, + non_finalized_state: Arc>, finalized_db: std::sync::Arc, sync_loop_handle: Option>>, status: NamedAtomicStatus, @@ -790,13 +790,21 @@ impl NodeBackedChainIndex { .map_err(crate::InitError::MempoolInitialzationError) .await?; + let network = config.network.to_zebra_network(); + // The non-finalized state is built eagerly so it always exists. It + // seeds Provisional from the validator (genesis); the sync loop extends + // it, and the finalized DB catching up to the seam flips it to Resolved. + let non_finalized_state = Arc::new( + crate::NonFinalizedState::initialize(source.clone(), network.clone(), None).await?, + ); + let mut chain_index = Self { mempool: std::sync::Arc::new(mempool_state), - non_finalized_state: Arc::new(ArcSwapOption::empty()), + non_finalized_state, finalized_db, sync_loop_handle: None, status: NamedAtomicStatus::new("ChainIndex", StatusType::Spawning), - network: config.network.to_zebra_network(), + network, source, sync_timings, cancel_token: CancellationToken::new(), @@ -859,7 +867,6 @@ impl NodeBackedChainIndex { let fs = self.finalized_db.clone(); let status = self.status.clone(); let source = self.source.clone(); - let network = self.network.clone(); let timings = self.sync_timings; let cancel_token = self.cancel_token.clone(); @@ -875,7 +882,6 @@ impl NodeBackedChainIndex { loop { let source = source.clone(); - let network = network.clone(); if cancel_token.is_cancelled() { return Ok(()); } @@ -916,34 +922,12 @@ impl NodeBackedChainIndex { .await .map_err(source_error)?; - let intermediate_nfs_for_scoping = nfs.load(); - let non_finalized_state = match *intermediate_nfs_for_scoping { - Some(ref nfs) => nfs, - None => { - nfs.store(Some(Arc::new( - NonFinalizedState::initialize( - source, - network, - fs.to_reader() - .get_chain_block_by_height(finalised_height) - .await - .expect("todo"), - ) - .await - .expect("todo"), - ))); - &nfs.load_full().expect("just set to Some") - } - }; - - // Sync nfs to the iter-committed `chain_height`, trimming - // blocks to finalized tip. Passing `chain_height` rather - // than letting NFS extend until `get_block` returns None - // bounds the iter against mid-iter source advances (#1126). - non_finalized_state - .sync(fs.clone(), chain_height.into()) - .await?; - std::mem::drop(intermediate_nfs_for_scoping); + // Sync the (always-present) nfs to the iter-committed + // `chain_height`, trimming blocks to the finalized tip. + // Passing `chain_height` rather than letting the NFS extend + // until `get_block` returns None bounds the iter against + // mid-iter source advances (#1126). + nfs.sync(fs.clone(), chain_height.into()).await?; Ok(()) } => r, @@ -1030,7 +1014,7 @@ impl Drop for NodeBackedChainIndex { #[derive(Clone, Debug)] pub struct NodeBackedChainIndexSubscriber { mempool: mempool::MempoolSubscriber, - non_finalized_state: Arc>>, + non_finalized_state: Arc>, finalized_state: finalised_state::reader::DbReader, status: NamedAtomicStatus, network: ZebraNetwork, @@ -1231,14 +1215,10 @@ impl NodeBackedChainIndexSubscriber { // Get the height of the mempool fn get_mempool_height(&self, snapshot: &ChainIndexSnapshot) -> Option { - let ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } = snapshot - else { - return None; - }; - - non_finalized_snapshot + // The mempool tip is a recent block; if it's in the always-present NFS + // window we can report its height (no absolute chainwork needed). + snapshot + .get_nfs_snapshot() .blocks .iter() .find(|(hash, _block)| **hash == self.mempool.mempool_chain_tip()) @@ -1259,6 +1239,9 @@ impl Status for NodeBackedChainIndexSubscriber } } +// Interim (#1096): `get_indexed_block_by_*` return the in-crate `ChainBlock` +// (collapses to `IndexedBlock` once resolution promotion lands). See the trait. +#[allow(private_interfaces)] impl ChainIndex for NodeBackedChainIndexSubscriber { type Snapshot = ChainIndexSnapshot; type Error = ChainIndexError; @@ -1269,26 +1252,11 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result { - match self.non_finalized_state.load().as_ref() { - Some(non_finalised_state) => Ok(ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot: non_finalised_state.get_snapshot(), - }), - None => { - let height = self - .source - .get_best_block_height() - .await - .map_err(ChainIndexError::backing_validator)? - .ok_or(ChainIndexError::database_hole( - "validator has no best block", - None, - ))?; - let validator_finalized_height = finalized_height_floor(height.0); - Ok(ChainIndexSnapshot::StillSyncingFinalizedState { - validator_finalized_height, - }) - } - } + // The non-finalized state always exists; a snapshot is infallible. Its + // own `availability` says whether the window is validated yet. + Ok(ChainIndexSnapshot::new( + self.non_finalized_state.get_snapshot(), + )) } // ********** Block methods ********** @@ -1308,17 +1276,15 @@ impl ChainIndex for NodeBackedChainIndexSubscriber { - self.get_indexed_block_height(non_finalized_snapshot, hash) + match snapshot.availability() { + SnapshotAvailability::Resolved => { + self.get_indexed_block_height(snapshot.get_nfs_snapshot(), hash) .await } - ChainIndexSnapshot::StillSyncingFinalizedState { + SnapshotAvailability::Provisional { validator_finalized_height, } => { - self.get_block_height_passthrough(validator_finalized_height, hash) + self.get_block_height_passthrough(&validator_finalized_height, hash) .await } // ChainIndex step 5 } @@ -1333,10 +1299,9 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result, Self::Error> { // First check non-finalised state. - match snapshot { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => match non_finalized_snapshot + match snapshot.availability() { + SnapshotAvailability::Resolved => match snapshot + .get_nfs_snapshot() .heights_to_hashes .get(&height) .copied() @@ -1350,10 +1315,10 @@ impl ChainIndex for NodeBackedChainIndexSubscriber { - if height <= *validator_finalized_height { + if height <= validator_finalized_height { // If still syncing try to fetch from backing validator (*passthrough*). // // Note this requires fetching the full block from the backing node. @@ -1511,41 +1476,30 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result, Self::Error> { - match snapshot { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => { - if height <= non_finalized_snapshot.best_tip.height { - Ok(Some(match snapshot.get_chainblock_by_height(&height) { - Some(block) => compact_block_with_pool_types( - block.to_compact_block(), - &pool_types.to_pool_types_vector(), - ), - None => match self - .finalized_state - .get_compact_block(height, pool_types) - .await - { - Ok(block) => block, - Err(e) => { - return Err(ChainIndexError::database_hole( - height, - Some(Box::new(e)), - )) - } - }, - })) - } else { - Ok(None) - } - } - - ChainIndexSnapshot::StillSyncingFinalizedState { - validator_finalized_height: _, - //TODO: Once we make chainwork an option field we should be able to - // support passthrougth for this - } => Ok(None), + // Compact blocks carry no absolute chainwork, so this serves from the + // always-present NFS regardless of availability. Only a height below + // the NFS floor that the finalized DB has not ingested yet (possible + // while Provisional) falls through to the finalized DB. + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); + if height > non_finalized_snapshot.best_tip.height { + return Ok(None); } + Ok(Some(match snapshot.get_chainblock_by_height(&height) { + Some(block) => compact_block_with_pool_types( + block.to_compact_block(), + &pool_types.to_pool_types_vector(), + ), + None => match self + .finalized_state + .get_compact_block(height, pool_types) + .await + { + Ok(block) => block, + // Finalized-gap while Provisional: #1066 — fall back to the + // validator here once passthrough lands. + Err(e) => return Err(ChainIndexError::database_hole(height, Some(Box::new(e)))), + }, + })) } /// Streams *compact* blocks for an inclusive height range. @@ -1775,12 +1729,13 @@ impl ChainIndex for NodeBackedChainIndexSubscriber ChainIndex for NodeBackedChainIndexSubscriber Result<(Option, HashSet), ChainIndexError> { - match snapshot { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => { + match snapshot.availability() { + SnapshotAvailability::Resolved => { + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); let blocks_containing_transaction = self .blocks_containing_transaction(non_finalized_snapshot, txid.0) .await? @@ -1876,10 +1830,11 @@ impl ChainIndex for NodeBackedChainIndexSubscriber ChainIndex for NodeBackedChainIndexSubscriber { if let Some((_transaction, GetTransactionLocation::BestChain(height))) = self @@ -1905,7 +1860,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber ChainIndex for NodeBackedChainIndexSubscriber, ) -> Option, Self::Error>>> { let non_finalized_snapshot = match snapshot { - Some(s) => match s { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => Some(non_finalized_snapshot), - // If we're still syncing the finalized state, the chain tip - // is newer than the snapshot's tip. Return None. - ChainIndexSnapshot::StillSyncingFinalizedState { .. } => return None, + Some(s) => match s.availability() { + SnapshotAvailability::Resolved => Some(s.get_nfs_snapshot()), + // While Provisional the chain tip is newer than the snapshot's + // tip, so there's no settled tip to anchor the stream to. + SnapshotAvailability::Provisional { .. } => return None, }, None => None, }; @@ -2052,10 +2005,9 @@ impl ChainIndex for NodeBackedChainIndexSubscriber { + match snapshot.availability() { + SnapshotAvailability::Resolved => { + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); match non_finalized_snapshot.get_chainblock_by_hash(hash) { Some(block) => { // At this point, we know that @@ -2093,7 +2045,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber { // We're not fully synced, so we pass through. @@ -2115,7 +2067,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber { // The VALIDATOR returned a block with a height. // However, there is as of yet no guaranteed the Block is FINALIZED - if height <= *validator_finalized_height { + if height <= validator_finalized_height { Ok(Some(( types::BlockHash::from(block.hash()), types::Height::from(height), @@ -2231,36 +2183,9 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result { - Ok(match snapshot { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => non_finalized_snapshot.best_tip, - - ChainIndexSnapshot::StillSyncingFinalizedState { - validator_finalized_height, - } => { - BlockIndex { - height: *validator_finalized_height, - hash: self - .source() - // TODO: do something more efficient than getting the whole block - .get_block(HashOrHeight::Height((*validator_finalized_height).into())) - .await - .map_err(|e| { - ChainIndexError::database_hole( - validator_finalized_height, - Some(Box::new(e)), - ) - })? - .ok_or(ChainIndexError::database_hole( - validator_finalized_height, - None, - ))? - .hash() - .into(), - } - } - }) + // The best tip is read directly from the always-present NFS in every + // availability state — no validator round-trip, so no database_hole. + Ok(snapshot.get_nfs_snapshot().best_tip) } async fn get_tx_out_set_info(&self) -> Result { @@ -2273,11 +2198,9 @@ impl ChainIndex for NodeBackedChainIndexSubscriber non_finalized_snapshot, - ChainIndexSnapshot::StillSyncingFinalizedState { .. } => { + let non_finalized_snapshot = match snapshot.availability() { + SnapshotAvailability::Resolved => snapshot.get_nfs_snapshot(), + SnapshotAvailability::Provisional { .. } => { // Accumulator invariants are not established until the finalised state catches // up. Match zcashd's "stats collection failed" empty-object shape. return Ok(GetTxOutSetInfoResponse::Empty(EmptyTxOutSetInfo {})); @@ -2528,32 +2451,22 @@ impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { impl NonFinalizedSnapshot for ChainIndexSnapshot { fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock> { - match self { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => non_finalized_snapshot.get_chainblock_by_hash(target_hash), - - ChainIndexSnapshot::StillSyncingFinalizedState { .. } => None, - } + // The NFS always exists; block lookup needs no absolute chainwork. + self.get_nfs_snapshot().get_chainblock_by_hash(target_hash) } fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock> { - match self { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => non_finalized_snapshot.get_chainblock_by_height(target_height), - - ChainIndexSnapshot::StillSyncingFinalizedState { .. } => None, - } + self.get_nfs_snapshot() + .get_chainblock_by_height(target_height) } fn max_serviceable_height(&self) -> &types::Height { - match self { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => non_finalized_snapshot.max_serviceable_height(), - - ChainIndexSnapshot::StillSyncingFinalizedState { + let nfs = self.get_nfs_snapshot(); + match &nfs.availability { + // Resolved: serve up to the non-finalized tip. + SnapshotAvailability::Resolved => &nfs.best_tip.height, + // Provisional: only the finalized range is serviceable via passthrough. + SnapshotAvailability::Provisional { validator_finalized_height, } => validator_finalized_height, } diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index b2c36d0ff..3c116d25c 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -41,43 +41,51 @@ pub struct NonFinalizedState { } #[derive(Debug, Clone)] -/// A snapshot of the chain index +/// A consistent snapshot of the chain index's non-finalized state. /// -/// If zaino has synced above the validator's finalized tip, -/// this contains a snapshot of the non-finalized state. -/// -/// If zaino is still syncing, this contains only the height -/// of the validator's finalized tip as of snapshot creation, -/// which is used to determine how high we can pass through -/// calls to the backing validator without serving nonfinalized -/// data. -pub enum ChainIndexSnapshot { - /// Zaino is ready to serve non-finalized data. - NonFinalizedStateExists { - /// The snapshot of the non_finalized state. - #[allow(private_interfaces)] - // Rust doesn't support private fields of enum variants - // The type of this field being private gives us something like it, though - non_finalized_snapshot: Arc, - }, - /// Zaino is not ready to serve non-finalized data. - StillSyncingFinalizedState { - /// The height the validater had last finalized as of snapshot creation. - validator_finalized_height: Height, - }, +/// The non-finalized state always exists — it is built eagerly at chain-index +/// creation — so a snapshot always carries a [`NonfinalizedBlockCacheSnapshot`]. +/// Whether that window has been validated against the finalized chain yet (the +/// finalized DB has reached its seam) is its [`SnapshotAvailability`]: while +/// `Provisional`, reads that need finalized data pass through to the validator. +pub struct ChainIndexSnapshot { + #[allow(private_interfaces)] + non_finalized_snapshot: Arc, } impl ChainIndexSnapshot { - /// Convenience fn to go from ChainIndexSnapshot to Option, - /// throwing away the validator_finalized_height in the None case. For ease of mapping, etc. - pub(crate) fn get_nfs_snapshot(&self) -> Option<&NonfinalizedBlockCacheSnapshot> { - match self { - ChainIndexSnapshot::NonFinalizedStateExists { - non_finalized_snapshot, - } => Some(non_finalized_snapshot), - ChainIndexSnapshot::StillSyncingFinalizedState { .. } => None, + pub(crate) fn new(non_finalized_snapshot: Arc) -> Self { + Self { + non_finalized_snapshot, } } + + /// The non-finalized snapshot. Always present: the NFS is eagerly + /// constructed and never absent. + pub(crate) fn get_nfs_snapshot(&self) -> &NonfinalizedBlockCacheSnapshot { + &self.non_finalized_snapshot + } + + /// Whether the finalized DB has caught up to this window's seam, and (when + /// it has) the seam's absolute-chainwork base. + pub(crate) fn availability(&self) -> SnapshotAvailability { + self.non_finalized_snapshot.availability + } + + /// True once a sync pass has validated this window against the finalized + /// chain (the finalized DB reached the seam). While false, reads needing + /// finalized data must pass through to the validator. + pub(crate) fn is_resolved(&self) -> bool { + matches!(self.availability(), SnapshotAvailability::Resolved) + } + + /// The non-finalized snapshot, but only once it is `Resolved` (validated + /// against the finalized chain). `None` while `Provisional`, so callers + /// that need authoritative data fall back (e.g. to the validator) then. + /// Distinct from [`Self::get_nfs_snapshot`], which is unconditional. + pub(crate) fn resolved_nfs_snapshot(&self) -> Option<&NonfinalizedBlockCacheSnapshot> { + self.is_resolved().then(|| self.get_nfs_snapshot()) + } } /// Whether a published snapshot's non-finalized window is anchored to the @@ -98,14 +106,12 @@ pub(crate) enum SnapshotAvailability { /// The finalized-tip height passthrough may serve up to. validator_finalized_height: Height, }, - /// The finalized DB has reached the seam: the window is validated, and each - /// block's absolute chainwork is `cumulative_chainwork_base + relative` - /// (the base is the seam block's absolute cumulative work, from the FS). - Resolved { - /// The seam's absolute cumulative work, the base for resolving any - /// window block's absolute chainwork. - cumulative_chainwork_base: ChainWork, - }, + /// The finalized DB has reached the seam: the window is validated against + /// the finalized chain. (The seam's absolute-chainwork base — needed to + /// resolve window blocks' *absolute* chainwork = `base + relative` — is + /// reattached by the resolution-promotion step, #1096, alongside its + /// reader; it isn't carried yet.) + Resolved, } #[derive(Debug, Clone)] @@ -774,16 +780,10 @@ impl NonFinalizedState { .heights_to_hashes .contains_key(&finalized_height) { - let cumulative_chainwork_base = finalized_db - .to_reader() - .get_chain_block_by_height(finalized_height) - .await - .map_err(|_e| UpdateError::FinalizedStateCorruption)? - .map(|seam_block| *seam_block.chainwork()) - .ok_or(UpdateError::FinalizedStateCorruption)?; - SnapshotAvailability::Resolved { - cumulative_chainwork_base, - } + // Seam overlap: the finalized DB has reached the window's floor. + // (The seam's absolute-chainwork base is fetched by the resolution + // promotion, #1096, when its reader exists.) + SnapshotAvailability::Resolved } else { SnapshotAvailability::Provisional { validator_finalized_height: finalized_height, diff --git a/packages/zaino-state/src/chain_index/tests.rs b/packages/zaino-state/src/chain_index/tests.rs index 85a415ffd..6e9661e83 100644 --- a/packages/zaino-state/src/chain_index/tests.rs +++ b/packages/zaino-state/src/chain_index/tests.rs @@ -155,7 +155,7 @@ async fn load_with_settings( loop { let nfs_ready = match index_reader.snapshot_nonfinalized_state().await { Ok(snap) => snap - .get_nfs_snapshot() + .resolved_nfs_snapshot() .is_some_and(|nfs| nfs.best_tip.height.0 == expected_nfs_tip), Err(_) => false, }; diff --git a/packages/zaino-state/src/chain_index/tests/mockchain_tests.rs b/packages/zaino-state/src/chain_index/tests/mockchain_tests.rs index 05c949757..aa321f0b2 100644 --- a/packages/zaino-state/src/chain_index/tests/mockchain_tests.rs +++ b/packages/zaino-state/src/chain_index/tests/mockchain_tests.rs @@ -39,7 +39,7 @@ async fn wait_for_indexer_tip( .snapshot_nonfinalized_state() .await .ok()? - .get_nfs_snapshot()? + .resolved_nfs_snapshot()? .best_tip .height .0; @@ -176,7 +176,7 @@ async fn sync_blocks_after_startup() { .snapshot_nonfinalized_state() .await .unwrap() - .get_nfs_snapshot() + .resolved_nfs_snapshot() .unwrap() .best_tip ) @@ -195,7 +195,7 @@ async fn sync_blocks_after_startup() { .snapshot_nonfinalized_state() .await .unwrap() - .get_nfs_snapshot() + .resolved_nfs_snapshot() .unwrap() .best_tip ) diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index f264ef168..cbe1f6b5c 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -16,20 +16,13 @@ //! absence (the slot does not flip back to "still syncing"). //! - **G**: `shutdown()` causes the sync loop to terminate cleanly. //! -//! Tests of the cold-start "still-syncing" variant are deliberately -//! omitted: that variant is being eliminated, and pinning its shape -//! would create immediate test churn at the refactor PR. -//! -//! The one exception is the trailing **red driver** for #1096 -//! (`best_chaintip_derives_tip_from_nfs_snapshot_not_validator_passthrough`). -//! Unlike the characterization tests above — which pin behavior that must -//! survive the refactor unchanged — that test is *failing on purpose* and is -//! expected to be rewritten when the still-syncing variant is removed. It -//! pins cold-start shape precisely because it is driving that variant's -//! elimination, so the churn it incurs is the point, not an accident. +//! The cold-start "still-syncing" variant has been eliminated (#1096): the NFS +//! is always present and carries a `Provisional`/`Resolved` availability. The +//! trailing `best_chaintip_derives_tip_from_nfs_snapshot` test — formerly the +//! red driver for that elimination — is now a passing regression that pins +//! `best_chaintip` reading the snapshot's `best_tip` directly. use super::{load_test_vectors_and_sync_chain_index, poll::poll_until, MockchainMode}; -use crate::chain_index::non_finalised_state::ChainIndexSnapshot; use crate::chain_index::{finalized_height_floor, ChainIndex}; use std::time::Duration; use tokio::time::sleep; @@ -45,7 +38,7 @@ async fn nfs_lowest_block_matches_finalized_db_tip() { let snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); let nfs = snapshot - .get_nfs_snapshot() + .resolved_nfs_snapshot() .expect("NFS exists after harness completes finalized sync"); let seam_height = finalized_height_floor(mockchain.active_height()); @@ -83,7 +76,7 @@ async fn block_is_evicted_from_nfs_when_finalized_advances_past_it() { let initial_snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); let initial_nfs = initial_snapshot - .get_nfs_snapshot() + .resolved_nfs_snapshot() .expect("NFS exists after harness"); let target_hash = *initial_nfs .heights_to_hashes @@ -109,7 +102,7 @@ async fn block_is_evicted_from_nfs_when_finalized_advances_past_it() { Duration::from_millis(25), || async { let snapshot = index_reader.snapshot_nonfinalized_state().await.ok()?; - let nfs = snapshot.get_nfs_snapshot()?; + let nfs = snapshot.resolved_nfs_snapshot()?; (nfs.best_tip.height.0 == post_mine_active_height).then_some(()) }, ) @@ -117,7 +110,7 @@ async fn block_is_evicted_from_nfs_when_finalized_advances_past_it() { let later_snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); let later_nfs = later_snapshot - .get_nfs_snapshot() + .resolved_nfs_snapshot() .expect("NFS still exists after advance"); assert!( @@ -145,7 +138,7 @@ async fn nfs_slot_is_monotonic_post_init() { for i in 0..10 { let snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); assert!( - snapshot.get_nfs_snapshot().is_some(), + snapshot.resolved_nfs_snapshot().is_some(), "iteration {i}: post-init snapshot must contain an NFS", ); sleep(Duration::from_millis(100)).await; @@ -179,7 +172,7 @@ async fn shutdown_terminates_sync_loop_cleanly() { Duration::from_millis(50), || async { let snapshot = index_reader.snapshot_nonfinalized_state().await.ok()?; - let nfs = snapshot.get_nfs_snapshot()?; + let nfs = snapshot.resolved_nfs_snapshot()?; (nfs.best_tip.height.0 == target_tip).then_some(()) }, ) @@ -247,7 +240,7 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi let initial_snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); let initial_nfs = initial_snapshot - .get_nfs_snapshot() + .resolved_nfs_snapshot() .expect("NFS exists after harness"); let target_hash = *initial_nfs .heights_to_hashes @@ -276,7 +269,7 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi Duration::from_millis(25), || async { let snapshot = index_reader.snapshot_nonfinalized_state().await.ok()?; - let nfs = snapshot.get_nfs_snapshot()?; + let nfs = snapshot.resolved_nfs_snapshot()?; (nfs.best_tip.height.0 == post_mine_active).then_some(()) }, ) @@ -284,7 +277,7 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi let later_snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); let later_nfs = later_snapshot - .get_nfs_snapshot() + .resolved_nfs_snapshot() .expect("NFS still exists after advance"); assert!( @@ -296,58 +289,28 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi ); } -/// **Red driver for #1096** (NOT a surviving characterization test — see the -/// module-level doc; this one is *failing on purpose* and is expected to be -/// rewritten when the still-syncing variant is removed). -/// -/// Target invariant: `best_chaintip` must derive the chain tip from the -/// non-finalized snapshot in *every* availability state — it must never fall -/// back to a validator passthrough. -/// -/// Today the lazy design hands out -/// [`ChainIndexSnapshot::StillSyncingFinalizedState`] during the cold-start -/// window, before the NFS slot is populated. In that variant `best_chaintip` -/// (`chain_index.rs`, the `StillSyncingFinalizedState` arm) has no snapshot -/// tip to read, so it round-trips to the validator and reports the *finalized -/// floor* (`validator_finalized_height`) as the tip — a stale height, and a -/// fallible call that surfaces `database_hole` if the validator can't serve -/// the floor block. +/// Regression for #1096: `best_chaintip` derives the tip from the +/// always-present NFS snapshot, never via a validator passthrough. /// -/// After #1096 there is no still-syncing variant: the snapshot always carries -/// an NFS `best_tip`, so `best_chaintip` reads it directly and reports the -/// real tip with no validator call. The test will then be rewritten to assert -/// the invariant over a snapshot returned by `snapshot_nonfinalized_state()`. +/// Before #1096, the cold-start `StillSyncingFinalizedState` variant had no +/// snapshot tip, so `best_chaintip` round-tripped to the validator and reported +/// the finalized *floor* (and could surface `database_hole`). Now the snapshot +/// always carries an NFS `best_tip` and `best_chaintip` reads it directly. This +/// pins that `best_chaintip` equals the snapshot's `best_tip` — which, after the +/// harness's sync, is the real chain tip — with no validator call. /// /// multi_thread: depends on the harness's background sync loop advancing the /// NFS concurrently with the setup's poll-until-ready loop. #[tokio::test(flavor = "multi_thread")] -async fn best_chaintip_derives_tip_from_nfs_snapshot_not_validator_passthrough() { +async fn best_chaintip_derives_tip_from_nfs_snapshot() { let (_blocks, _indexer, index_reader, mockchain) = load_test_vectors_and_sync_chain_index(MockchainMode::Active).await; - // The chain tip the always-present NFS snapshot reports: the harness syncs - // the NFS to exactly the source's active height before returning. - let chain_tip = mockchain.active_height(); - - // The cold-start variant the lazy design can hand out instead. Its - // `validator_finalized_height` is the true floor — exactly what - // `snapshot_nonfinalized_state()` computes while the NFS slot is `None`. - let cold_start_snapshot = ChainIndexSnapshot::StillSyncingFinalizedState { - validator_finalized_height: finalized_height_floor(chain_tip), - }; - - let tip = index_reader - .best_chaintip(&cold_start_snapshot) - .await - .expect("best_chaintip resolves against a still-syncing snapshot"); + let snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); + let tip = index_reader.best_chaintip(&snapshot).await.unwrap(); - assert_eq!( - tip.height.0, - chain_tip, - "best_chaintip must derive the tip from the NFS snapshot and report the \ - chain tip ({chain_tip}) in every availability state; today the cold-start \ - variant passes through to the validator and reports the finalized floor \ - ({}) instead (#1096)", - finalized_height_floor(chain_tip).0, - ); + // best_chaintip reads the snapshot's best_tip directly (no passthrough)... + assert_eq!(tip, snapshot.get_nfs_snapshot().best_tip); + // ...which the harness has synced to the real chain tip. + assert_eq!(tip.height.0, mockchain.active_height()); } diff --git a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs index f4cd18337..e517aa4f0 100644 --- a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs +++ b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs @@ -114,7 +114,7 @@ fn passthrough_test( .await; let snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); assert_eq!(snapshot.max_serviceable_height().0 as usize, expected_max_serviceable_height); - assert!(matches!(snapshot, ChainIndexSnapshot::StillSyncingFinalizedState { .. })); + assert!(!snapshot.is_resolved()); test(&mockchain, index_reader, &snapshot).await; @@ -408,12 +408,12 @@ fn make_chain() { Duration::from_millis(25), || async { let snapshot = index_reader.snapshot_nonfinalized_state().await.ok()?; - (snapshot.get_nfs_snapshot()?.blocks.len() == expected_block_count) + (snapshot.resolved_nfs_snapshot()?.blocks.len() == expected_block_count) .then_some(snapshot) }, ) .await; - let non_finalized_snapshot = snapshot.get_nfs_snapshot().expect("not synced"); + let non_finalized_snapshot = snapshot.resolved_nfs_snapshot().expect("not synced"); let best_tip_hash = non_finalized_snapshot.best_tip.hash; let best_tip_block = non_finalized_snapshot .get_chainblock_by_hash(&best_tip_hash) diff --git a/packages/zaino-state/src/chain_index/tests/sync_loop.rs b/packages/zaino-state/src/chain_index/tests/sync_loop.rs index 7037f7e8e..5cfa5b2a7 100644 --- a/packages/zaino-state/src/chain_index/tests/sync_loop.rs +++ b/packages/zaino-state/src/chain_index/tests/sync_loop.rs @@ -120,7 +120,7 @@ async fn tip_converges_after_burst_mine() { .snapshot_nonfinalized_state() .await .ok()? - .get_nfs_snapshot()? + .resolved_nfs_snapshot()? .best_tip .height .0; @@ -133,7 +133,7 @@ async fn tip_converges_after_burst_mine() { .snapshot_nonfinalized_state() .await .unwrap() - .get_nfs_snapshot() + .resolved_nfs_snapshot() .unwrap() .best_tip .height From 469e70ccb2520e322a74299b411d6023c6dcd5b2 Mon Sep 17 00:00:00 2001 From: zancas Date: Wed, 27 May 2026 00:07:29 -0700 Subject: [PATCH 09/31] chain_index: make the non-finalized state lead, anchored at the finalization ceiling (#1096) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The non-finalized state (NFS) is now validator-sourced and always leads the finalized DB. Instead of being seeded from genesis and re-seeded from the finalized-DB tip, it anchors its window at the finalization ceiling (best_tip - NON_FINALIZED_DEPTH), fetching the seam block from the source, so it only ever walks the non-finalized window and never depends on finalized-DB progress. The sync loop runs nfs.sync before fs.sync_to_height; the finalized DB catches up to the ceiling, and a snapshot is Provisional until it does. Boundary helpers: - Replace the `max_serviceable_height` snapshot method and the `validator_finalized_height`-carrying `SnapshotAvailability::Provisional` field with two free functions: `finalization_ceiling(best_tip) -> Height` and `is_finalized(best_tip, height) -> bool`. Remove `finalized_height_floor` (subsumed by `finalization_ceiling`). Query surface: - Restore reorg-safe validator passthrough for the catch-up gap in get_block_height / get_block_hash / find_fork_point / get_transaction_status: serve NFS-own-data ∪ finalized DB first, then passthrough for heights at or below the finalization ceiling (is_finalized), never above it. update() / listener: - Trim the window to the seam (finalization_ceiling(best_tip)) rather than the lagging finalized-DB tip, so the floor advances with the chain (also fixes the #1126 eviction regression test). - Skip sub-seam blocks in handle_nfs_change_listener; they are finalized, and processing them recursed past genesis and errored with MissingBlockError. Tests: - Add a `BlockchainSource::finalized_sync_cap` hook (default: no cap) so the proptest mock holds the finalized DB below the ceiling deterministically, replacing the per-call sleep with a gate. The passthrough_* tests run with no artificial delays and assert every block is served (NFS window ∪ passthrough gap). The mock's get_commitment_tree_roots no longer rebuilds the commitment tree from genesis per call (the O(N²) hashing the routing tests don't need). Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/zaino-state/CHANGELOG.md | 1 - packages/zaino-state/src/backends/fetch.rs | 18 +- packages/zaino-state/src/backends/state.rs | 11 +- packages/zaino-state/src/chain_index.rs | 580 ++++++++---------- .../src/chain_index/finalised_state.rs | 8 + .../src/chain_index/non_finalised_state.rs | 134 ++-- .../zaino-state/src/chain_index/source.rs | 13 + packages/zaino-state/src/chain_index/tests.rs | 12 +- .../chain_index/tests/non_finalised_state.rs | 14 +- .../chain_index/tests/proptest_blockgen.rs | 286 ++++----- 10 files changed, 520 insertions(+), 557 deletions(-) diff --git a/packages/zaino-state/CHANGELOG.md b/packages/zaino-state/CHANGELOG.md index 50322b6d5..606fbd350 100644 --- a/packages/zaino-state/CHANGELOG.md +++ b/packages/zaino-state/CHANGELOG.md @@ -19,7 +19,6 @@ and this library adheres to Rust's notion of - `compact_vout` - `to_compact`: returns a compactTx from TxInCompact - new type: `non_finalized_state::ChainIndexSnapshot` - - `NonFinalizedSnapshot` trait has new method: `max_serviceable_height` - `::types` - new submodule `primitives` with type `BlockIndex { height, hash }` (re-exported as `chain_index::types::BlockIndex`) diff --git a/packages/zaino-state/src/backends/fetch.rs b/packages/zaino-state/src/backends/fetch.rs index c6047fb0c..843bdcac5 100644 --- a/packages/zaino-state/src/backends/fetch.rs +++ b/packages/zaino-state/src/backends/fetch.rs @@ -75,8 +75,8 @@ use crate::{ BackendType, }; use crate::{ - chain_index::{non_finalised_state::SnapshotAvailability, NonFinalizedSnapshot}, - ChainIndex, NodeBackedChainIndex, NodeBackedChainIndexSubscriber, + chain_index::non_finalised_state::SnapshotAvailability, ChainIndex, NodeBackedChainIndex, + NodeBackedChainIndexSubscriber, }; /// Chain fetch service backed by Zcashd's JsonRPC engine. @@ -742,7 +742,9 @@ impl ZcashIndexer for FetchServiceSubscriber { let (height, confirmations, block_hash, in_best_chain) = match best_chain_location { Some(types::BestChainLocation::Block(block_hash, height)) => { let confirmations = snapshot - .max_serviceable_height() + .get_nfs_snapshot() + .best_tip + .height .0 .saturating_sub(height.0) .saturating_add(1); @@ -799,7 +801,9 @@ impl ZcashIndexer for FetchServiceSubscriber { self.indexer .snapshot_nonfinalized_state() .await? - .max_serviceable_height() + .get_nfs_snapshot() + .best_tip + .height .0, )) } @@ -883,10 +887,8 @@ impl LightWalletIndexer for FetchServiceSubscriber { async fn get_latest_block(&self) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; match snapshot.availability() { - SnapshotAvailability::Resolved { .. } => { - Ok(snapshot.get_nfs_snapshot().best_tip.to_wire()) - } - SnapshotAvailability::Provisional { .. } => { + SnapshotAvailability::Resolved => Ok(snapshot.get_nfs_snapshot().best_tip.to_wire()), + SnapshotAvailability::Provisional => { // TODO: This probably shouldn't be an error. // this is an improvement over previous behaviour of reporting // the genesis block diff --git a/packages/zaino-state/src/backends/state.rs b/packages/zaino-state/src/backends/state.rs index d559eda23..7a9f819db 100644 --- a/packages/zaino-state/src/backends/state.rs +++ b/packages/zaino-state/src/backends/state.rs @@ -1,5 +1,6 @@ //! Zcash chain fetch and tx submission service backed by Zebras [`ReadStateService`]. +use crate::{chain_index::types::BestChainLocation, TransactionHash}; #[allow(deprecated)] use crate::{ chain_index::{ @@ -20,10 +21,6 @@ use crate::{ utils::{get_build_info, ServiceMetadata}, BackendType, NodeBackedChainIndex, NodeBackedChainIndexSubscriber, State, }; -use crate::{ - chain_index::{types::BestChainLocation, NonFinalizedSnapshot}, - TransactionHash, -}; use tokio_stream::StreamExt as _; use zaino_fetch::{ chain::{transaction::FullTransaction, utils::ParseFromSlice}, @@ -681,7 +678,7 @@ impl StateServiceSubscriber { height: u32, ) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let chain_height = snapshot.max_serviceable_height().0; + let chain_height = snapshot.get_nfs_snapshot().best_tip.height.0; Err(if height >= chain_height { StateServiceError::TonicStatusError(tonic::Status::out_of_range(format!( "Error: Height out of range [{height}]. Height requested \ @@ -1704,7 +1701,9 @@ impl ZcashIndexer for StateServiceSubscriber { let (height, confirmations, block_hash, in_best_chain) = match best_chain_location { Some(BestChainLocation::Block(block_hash, height)) => { let confirmations = snapshot - .max_serviceable_height() + .get_nfs_snapshot() + .best_tip + .height .0 .saturating_sub(height.0) .saturating_add(1); diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index 56efc9915..6429dd54d 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -70,16 +70,28 @@ mod tests; /// zingolabs/zaino#1130. pub(crate) const NON_FINALIZED_DEPTH: u32 = zebra_state::MAX_BLOCK_REORG_HEIGHT + 1; -/// Lower bound on zaino's finalized-DB tip, derived from the current -/// best-known chain tip. +/// The ceiling of the finalized (immutable) chain for a given chain tip: +/// `NON_FINALIZED_DEPTH` below `best_tip`. A block is finalized — and therefore +/// reorg-safe — exactly when its height is at or below this value. /// -/// After a chain-shortening reorg this floor can move backwards while -/// the on-disk `finalized_height` does not — finalized blocks are -/// never evicted. Callers comparing this floor against -/// `finalized_height` should account for the asymmetry -/// (see zingolabs/zaino#1128). -pub(crate) fn finalized_height_floor(chain_tip: u32) -> crate::Height { - crate::Height(chain_tip.saturating_sub(NON_FINALIZED_DEPTH)) +/// Reorg-safety and finality are the same notion: a reorg is bounded to the +/// non-finalized window and can never evict a finalized block, so the ceiling +/// monotonically increases and is unaffected by reorgs. +/// +/// This is the single boundary the passthrough path needs: a height at or below +/// it is reorg-safe to fetch from the backing validator by height (see +/// [`is_finalized`]). It also drives the finalized DB's sync target. +pub(crate) fn finalization_ceiling(best_tip: u32) -> crate::Height { + crate::Height(best_tip.saturating_sub(NON_FINALIZED_DEPTH)) +} + +/// Whether `height` is finalized given the chain `best_tip` — at or below the +/// [`finalization_ceiling`], in the immutable range, and therefore reorg-safe +/// to fetch from the backing validator by height. Heights above it are in the +/// (reorg-mutable) non-finalized window and must be served only from the NFS's +/// own validated view. +pub(crate) fn is_finalized(best_tip: types::Height, height: types::Height) -> bool { + height <= finalization_ceiling(best_tip.0) } /// Builds a zcashd-compatible `getchaintips` response from the local non-finalized snapshot. @@ -916,19 +928,21 @@ impl NodeBackedChainIndex { "node returned no best block height", )) })?; - let finalised_height = finalized_height_floor(chain_height.0); + // The NFS leads: validator-sourced and anchored at the + // finalization ceiling, it walks to the iter-committed + // `chain_height` without waiting for the finalized DB. + // Passing `chain_height` rather than letting the NFS extend + // until `get_block` returns None bounds the iter against + // mid-iter source advances (#1126). The finalized DB then + // catches up toward the ceiling; until it reaches the seam + // the published snapshot is Provisional. + nfs.sync(fs.clone(), chain_height.into()).await?; + let finalised_height = finalization_ceiling(chain_height.0); fs.sync_to_height(finalised_height, &source) .await .map_err(source_error)?; - // Sync the (always-present) nfs to the iter-committed - // `chain_height`, trimming blocks to the finalized tip. - // Passing `chain_height` rather than letting the NFS extend - // until `get_block` returns None bounds the iter against - // mid-iter source advances (#1126). - nfs.sync(fs.clone(), chain_height.into()).await?; - Ok(()) } => r, }; @@ -1173,46 +1187,6 @@ impl NodeBackedChainIndexSubscriber { .chain(non_finalized_blocks_containing_transaction)) } - async fn get_block_height_passthrough( - &self, - max_serviceable_height: &types::Height, - hash: types::BlockHash, - ) -> Result, ChainIndexError> { - //ChainIndex step 5: - match self - .source() - .get_block(HashOrHeight::Hash(hash.into())) - .await - { - Ok(Some(block)) => { - // At this point, we know that - // the block is in the VALIDATOR. - match block.coinbase_height() { - None => { - // the block is in the VALIDATOR. but doesnt have a height. That would imply a bug. - Err(ChainIndexError::validator_data_error_block_coinbase_height_missing()) - } - Some(height) => { - // The VALIDATOR returned a block with a height. - // However, there is as of yet no guaranteed the Block is FINALIZED - if height <= *max_serviceable_height { - Ok(Some(types::Height::from(height))) - } else { - // non-finalized block - // no passthrough - Ok(None) - } - } - } - } - Ok(None) => { - // the block is neither in the INDEXER nor VALIDATOR - Ok(None) - } - Err(e) => Err(ChainIndexError::backing_validator(e)), - } - } - // Get the height of the mempool fn get_mempool_height(&self, snapshot: &ChainIndexSnapshot) -> Option { // The mempool tip is a recent block; if it's in the always-present NFS @@ -1275,18 +1249,36 @@ impl ChainIndex for NodeBackedChainIndexSubscriber { - self.get_indexed_block_height(snapshot.get_nfs_snapshot(), hash) - .await - } - SnapshotAvailability::Provisional { - validator_finalized_height, - } => { - self.get_block_height_passthrough(&validator_finalized_height, hash) - .await - } // ChainIndex step 5 + // ChainIndex steps 2-4: serve the snapshot's own data first (NFS window + // ∪ finalised DB). + if let Some(height) = self + .get_indexed_block_height(snapshot.get_nfs_snapshot(), hash) + .await? + { + return Ok(Some(height)); + } + + // ChainIndex step 5: the block is in neither the NFS window nor the + // finalised DB. While the snapshot is provisional the finalised DB may + // not yet have ingested a finalized block the validator already has; + // that catch-up gap lies at or below the finalization ceiling and is + // reorg-safe to serve by passthrough. A block above the ceiling is in + // the reorg-mutable window and must not be served from the validator. + let best_tip = snapshot.get_nfs_snapshot().best_tip.height; + match self + .source() + .get_block(HashOrHeight::Hash(hash.into())) + .await + .map_err(ChainIndexError::backing_validator)? + { + Some(block) => match block.coinbase_height() { + None => Err(ChainIndexError::validator_data_error_block_coinbase_height_missing()), + Some(height) => { + let height = types::Height::from(height); + Ok(is_finalized(best_tip, height).then_some(height)) + } + }, + None => Ok(None), } } @@ -1298,45 +1290,40 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result, Self::Error> { - // First check non-finalised state. - match snapshot.availability() { - SnapshotAvailability::Resolved => match snapshot - .get_nfs_snapshot() - .heights_to_hashes - .get(&height) - .copied() - { - Some(block_hash) => Ok(Some(block_hash)), - // If not found check finalised state. - None => self - .finalized_state - .get_block_hash(height) - .await - .map_err(Into::into), - }, + // Serve the snapshot's own data first: NFS window, then finalised DB. + if let Some(block_hash) = snapshot + .get_nfs_snapshot() + .heights_to_hashes + .get(&height) + .copied() + { + return Ok(Some(block_hash)); + } + if let Some(block_hash) = self + .finalized_state + .get_block_hash(height) + .await + .map_err(ChainIndexError::from)? + { + return Ok(Some(block_hash)); + } - SnapshotAvailability::Provisional { - validator_finalized_height, - } => { - if height <= validator_finalized_height { - // If still syncing try to fetch from backing validator (*passthrough*). - // - // Note this requires fetching the full block from the backing node. - match self - .source() - .get_block(HashOrHeight::Height(height.into())) - .await - .map_err(ChainIndexError::backing_validator)? - { - Some(block) => Ok(Some(block.hash().into())), - None => Ok(None), - } - } else { - // The requested block is non-finalized - // We can't safely serve it via passthrough - Ok(None) - } - } + // Neither holds it. A finalized-range height (at or below the + // finalization ceiling) absent from the finalised DB is a provisional + // catch-up gap: it is reorg-safe to fetch by height from the validator. + // Heights above the ceiling are reorg-mutable — return None. + let best_tip = snapshot.get_nfs_snapshot().best_tip.height; + if !is_finalized(best_tip, height) { + return Ok(None); + } + match self + .source() + .get_block(HashOrHeight::Height(height.into())) + .await + .map_err(ChainIndexError::backing_validator)? + { + Some(block) => Ok(Some(block.hash().into())), + None => Ok(None), } } @@ -1388,9 +1375,8 @@ impl ChainIndex for NodeBackedChainIndexSubscriber ChainIndex for NodeBackedChainIndexSubscriber ChainIndex for NodeBackedChainIndexSubscriber self - // usually getting by height is not reorg-safe, but here, height is known to be below or equal to validator_finalized_height. + // Absent from both the finalized DB and the + // NFS window: a finalized-range catch-up gap. + // The height is at or below the finalization + // ceiling, so a by-height fetch is reorg-safe. .get_fullblock_bytes_from_node(HashOrHeight::Height( zebra_chain::block::Height(height), )) @@ -1770,113 +1766,113 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result<(Option, HashSet), ChainIndexError> { - match snapshot.availability() { - SnapshotAvailability::Resolved => { - let non_finalized_snapshot = snapshot.get_nfs_snapshot(); - let blocks_containing_transaction = self - .blocks_containing_transaction(non_finalized_snapshot, txid.0) - .await? - .collect::>(); - let Some(start_of_nonfinalized) = - non_finalized_snapshot.heights_to_hashes.keys().min() - else { - return Err(ChainIndexError::database_hole("no blocks", None)); - }; - let mut best_chain_block = blocks_containing_transaction - .iter() - .find(|block| { - non_finalized_snapshot - .heights_to_hashes - .get(&block.height()) - == Some(block.hash()) - || block.height() < *start_of_nonfinalized - // this block is either in the best chain ``heights_to_hashes`` or finalized. - }) - .map(|block| BestChainLocation::Block(*block.hash(), block.height())); - let mut non_best_chain_blocks: HashSet = - blocks_containing_transaction - .iter() - .filter(|block| { - non_finalized_snapshot - .heights_to_hashes - .get(&block.height()) - != Some(block.hash()) - && block.height() >= *start_of_nonfinalized - }) - .map(|block| NonBestChainLocation::Block(*block.hash(), block.height())) - .collect(); - let in_mempool = self - .mempool - .contains_txid(&mempool::MempoolKey { - txid: txid.to_rpc_hex(), - }) - .await; - if in_mempool { - let mempool_tip_hash = self.mempool.mempool_chain_tip(); - if mempool_tip_hash == non_finalized_snapshot.best_tip.hash { - if best_chain_block.is_some() { - return Err(ChainIndexError { + // The non-finalised snapshot serves its own data (own blocks ∪ + // finalised state) regardless of availability; it never passes through + // to the backing validator. + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); + let blocks_containing_transaction = self + .blocks_containing_transaction(non_finalized_snapshot, txid.0) + .await? + .collect::>(); + let Some(start_of_nonfinalized) = non_finalized_snapshot.heights_to_hashes.keys().min() + else { + return Err(ChainIndexError::database_hole("no blocks", None)); + }; + let mut best_chain_block = blocks_containing_transaction + .iter() + .find(|block| { + non_finalized_snapshot + .heights_to_hashes + .get(&block.height()) + == Some(block.hash()) + || block.height() < *start_of_nonfinalized + // this block is either in the best chain ``heights_to_hashes`` or finalized. + }) + .map(|block| BestChainLocation::Block(*block.hash(), block.height())); + let mut non_best_chain_blocks: HashSet = + blocks_containing_transaction + .iter() + .filter(|block| { + non_finalized_snapshot + .heights_to_hashes + .get(&block.height()) + != Some(block.hash()) + && block.height() >= *start_of_nonfinalized + }) + .map(|block| NonBestChainLocation::Block(*block.hash(), block.height())) + .collect(); + // If the snapshot's own data (NFS window ∪ finalised DB) doesn't contain + // the transaction, it may sit in a finalized-range block the finalised + // DB hasn't ingested yet (a provisional catch-up gap). Such a block, at + // or below the finalization ceiling, is reorg-safe to resolve via the + // validator. (A gap block is finalized, so it can't also be in the + // mempool — this is mutually exclusive with the mempool logic below.) + if best_chain_block.is_none() && non_best_chain_blocks.is_empty() { + if let Some((_transaction, GetTransactionLocation::BestChain(height))) = self + .source() + .get_transaction(*txid) + .await + .map_err(ChainIndexError::backing_validator)? + { + let best_tip = snapshot.get_nfs_snapshot().best_tip.height; + if is_finalized(best_tip, types::Height::from(height)) { + if let Some(block) = self + .source() + .get_block(HashOrHeight::Height(height)) + .await + .map_err(ChainIndexError::backing_validator)? + { + best_chain_block = Some(BestChainLocation::Block( + block.hash().into(), + types::Height::from(height), + )); + } + } + } + } + let in_mempool = self + .mempool + .contains_txid(&mempool::MempoolKey { + txid: txid.to_rpc_hex(), + }) + .await; + if in_mempool { + let mempool_tip_hash = self.mempool.mempool_chain_tip(); + if mempool_tip_hash == non_finalized_snapshot.best_tip.hash { + if best_chain_block.is_some() { + return Err(ChainIndexError { kind: ChainIndexErrorKind::InvalidSnapshot, message: "Best chain and up-to-date mempool both contain the same transaction" .to_string(), source: None, }); - } else { - best_chain_block = Some(BestChainLocation::Mempool( - non_finalized_snapshot.best_tip.height + 1, - )); - } - } else { - // the best chain and the mempool have divergent tip hashes - // get a new snapshot and use it to find the height of the mempool - { - // The NFS always exists; take a fresh snapshot to - // locate the mempool's tip block. - let fresh_snapshot = self.snapshot_nonfinalized_state().await?; - let new_snapshot = fresh_snapshot.get_nfs_snapshot(); - let target_height = - new_snapshot.blocks.iter().find_map(|(hash, block)| { - if *hash == mempool_tip_hash { - Some(block.height() + 1) - // found the block that is the tip that the mempool is hanging on to - } else { - None - } - }); - non_best_chain_blocks - .insert(NonBestChainLocation::Mempool(target_height)); - } - } + } else { + best_chain_block = Some(BestChainLocation::Mempool( + non_finalized_snapshot.best_tip.height + 1, + )); } - Ok((best_chain_block, non_best_chain_blocks)) - } - SnapshotAvailability::Provisional { - validator_finalized_height, - } => { - if let Some((_transaction, GetTransactionLocation::BestChain(height))) = self - .source() - .get_transaction(*txid) - .await - .map_err(ChainIndexError::backing_validator)? + } else { + // the best chain and the mempool have divergent tip hashes + // get a new snapshot and use it to find the height of the mempool { - if height <= validator_finalized_height { - if let Some(block) = self - .source() - .get_block(HashOrHeight::Height(height)) - .await - .map_err(ChainIndexError::backing_validator)? - { - return Ok(( - Some(BestChainLocation::Block(block.hash().into(), height.into())), - HashSet::new(), - )); + // The NFS always exists; take a fresh snapshot to + // locate the mempool's tip block. + let fresh_snapshot = self.snapshot_nonfinalized_state().await?; + let new_snapshot = fresh_snapshot.get_nfs_snapshot(); + let target_height = new_snapshot.blocks.iter().find_map(|(hash, block)| { + if *hash == mempool_tip_hash { + Some(block.height() + 1) + // found the block that is the tip that the mempool is hanging on to + } else { + None } - } + }); + non_best_chain_blocks.insert(NonBestChainLocation::Mempool(target_height)); } - Ok((None, HashSet::new())) } } + Ok((best_chain_block, non_best_chain_blocks)) } /// Returns all txids currently in the mempool. @@ -1929,7 +1925,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Some(s.get_nfs_snapshot()), // While Provisional the chain tip is newer than the snapshot's // tip, so there's no settled tip to anchor the stream to. - SnapshotAvailability::Provisional { .. } => return None, + SnapshotAvailability::Provisional => return None, }, None => None, }; @@ -2005,89 +2001,69 @@ impl ChainIndex for NodeBackedChainIndexSubscriber { - let non_finalized_snapshot = snapshot.get_nfs_snapshot(); - match non_finalized_snapshot.get_chainblock_by_hash(hash) { - Some(block) => { - // At this point, we know that - // The block is non-FINALIZED in the INDEXER - // ChainIndex step 3: - if non_finalized_snapshot - .heights_to_hashes - .get(&block.height()) - == Some(block.hash()) - { - // The block is in the best chain. - Ok(Some((*block.hash(), block.height()))) - } else { - // Otherwise, it's non-best chain! Grab its parent, and recurse. - // NOTE: walks the (provisional-stage UNTRUSTED) prev-hash linkage; - // the Availability step should gate this trust on Resolved (#1096). - Box::pin(self.find_fork_point(snapshot, block.parent_hash())).await - // gotta pin recursive async functions to prevent infinite-sized - // Future-implementing types - } - } - None => { - // At this point, we know that - // the block is NOT non-FINALIZED in the INDEXER. - // as the non finalzed state is known to be populated, - // we now check the finalized state - match self.finalized_state.get_block_height(*hash).await { - Ok(Some(height)) => { - // the block is FINALIZED in the INDEXER - Ok(Some((*hash, height))) - } - Err(e) => Err(ChainIndexError::database_hole(hash, Some(Box::new(e)))), - Ok(None) => Ok(None), - } - } + // The non-finalised snapshot serves its own data (own blocks ∪ + // finalised state) regardless of availability; it never passes through + // to the backing validator. + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); + match non_finalized_snapshot.get_chainblock_by_hash(hash) { + Some(block) => { + // At this point, we know that + // The block is non-FINALIZED in the INDEXER + // ChainIndex step 3: + if non_finalized_snapshot + .heights_to_hashes + .get(&block.height()) + == Some(block.hash()) + { + // The block is in the best chain. + Ok(Some((*block.hash(), block.height()))) + } else { + // Otherwise, it's non-best chain! Grab its parent, and recurse. + // NOTE: walks the (provisional-stage UNTRUSTED) prev-hash linkage; + // the Availability step should gate this trust on Resolved (#1096). + Box::pin(self.find_fork_point(snapshot, block.parent_hash())).await + // gotta pin recursive async functions to prevent infinite-sized + // Future-implementing types } } - SnapshotAvailability::Provisional { - validator_finalized_height, - } => { - // We're not fully synced, so we pass through. - // Now, we ask the VALIDATOR. - // ChainIndex step 5 - match self - .source() - .get_block(HashOrHeight::Hash(zebra_chain::block::Hash::from(*hash))) - .await - { - Ok(Some(block)) => { - // At this point, we know that - // the block is in the VALIDATOR. - match block.coinbase_height() { - None => { - // the block is in the VALIDATOR. but doesnt have a height. That would imply a bug. - Err(ChainIndexError::validator_data_error_block_coinbase_height_missing()) - } - Some(height) => { - // The VALIDATOR returned a block with a height. - // However, there is as of yet no guaranteed the Block is FINALIZED - if height <= validator_finalized_height { - Ok(Some(( - types::BlockHash::from(block.hash()), - types::Height::from(height), - ))) - } else { - // non-finalized block - // no passthrough - Ok(None) - } - } - } + None => { + // At this point, we know that + // the block is NOT non-FINALIZED in the INDEXER. + // as the non finalzed state is known to be populated, + // we now check the finalized state + match self.finalized_state.get_block_height(*hash).await { + Ok(Some(height)) => { + // the block is FINALIZED in the INDEXER + Ok(Some((*hash, height))) } - + Err(e) => Err(ChainIndexError::database_hole(hash, Some(Box::new(e)))), Ok(None) => { - // At this point, we know that - // the block is NOT FINALIZED in the VALIDATOR. - // Return Ok(None) = no block found. - Ok(None) + // Neither the NFS window nor the finalised DB holds it. + // While provisional, a finalized-range block the + // finalised DB hasn't ingested yet (a catch-up gap, at + // or below the finalization ceiling) is reorg-safe to + // fetch by hash from the validator, and is its own fork + // point. Above the ceiling the block is reorg-mutable; + // we must not passthrough. + let best_tip = snapshot.get_nfs_snapshot().best_tip.height; + match self + .source() + .get_block(HashOrHeight::Hash(zebra_chain::block::Hash::from(*hash))) + .await + .map_err(ChainIndexError::backing_validator)? + { + Some(block) => match block.coinbase_height() { + None => Err( + ChainIndexError::validator_data_error_block_coinbase_height_missing(), + ), + Some(height) => { + let height = types::Height::from(height); + Ok(is_finalized(best_tip, height).then_some((*hash, height))) + } + }, + None => Ok(None), + } } - Err(e) => Err(ChainIndexError::backing_validator(e)), } } } @@ -2200,7 +2176,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber snapshot.get_nfs_snapshot(), - SnapshotAvailability::Provisional { .. } => { + SnapshotAvailability::Provisional => { // Accumulator invariants are not established until the finalised state catches // up. Match zcashd's "stats collection failed" empty-object shape. return Ok(GetTxOutSetInfoResponse::Empty(EmptyTxOutSetInfo {})); @@ -2408,10 +2384,6 @@ where fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock> { self.as_ref().get_chainblock_by_height(target_height) } - - fn max_serviceable_height(&self) -> &types::Height { - self.as_ref().max_serviceable_height() - } } /// A snapshot of the non-finalized state, for consistent queries @@ -2420,8 +2392,6 @@ pub(crate) trait NonFinalizedSnapshot { fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock>; /// Height -> block fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock>; - /// The maximum height that this snapshot can serve data for. - fn max_serviceable_height(&self) -> &types::Height; } impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { @@ -2443,10 +2413,6 @@ impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { } }) } - - fn max_serviceable_height(&self) -> &types::Height { - &self.best_tip.height - } } impl NonFinalizedSnapshot for ChainIndexSnapshot { @@ -2459,16 +2425,4 @@ impl NonFinalizedSnapshot for ChainIndexSnapshot { self.get_nfs_snapshot() .get_chainblock_by_height(target_height) } - - fn max_serviceable_height(&self) -> &types::Height { - let nfs = self.get_nfs_snapshot(); - match &nfs.availability { - // Resolved: serve up to the non-finalized tip. - SnapshotAvailability::Resolved => &nfs.best_tip.height, - // Provisional: only the finalized range is serviceable via passthrough. - SnapshotAvailability::Provisional { - validator_finalized_height, - } => validator_finalized_height, - } - } } diff --git a/packages/zaino-state/src/chain_index/finalised_state.rs b/packages/zaino-state/src/chain_index/finalised_state.rs index 41c7cdac7..2d874e9b0 100644 --- a/packages/zaino-state/src/chain_index/finalised_state.rs +++ b/packages/zaino-state/src/chain_index/finalised_state.rs @@ -509,6 +509,14 @@ impl ZainoDB { where T: BlockchainSource, { + // Honour a source-imposed sync cap (a test/backpressure hook): the + // finalized DB may be held below the chain's finalization ceiling, + // which — since the non-finalized state leads — holds the snapshot + // Provisional with a deterministic catch-up gap. + let height = match source.finalized_sync_cap() { + Some(cap) => height.min(cap), + None => height, + }; let network = self.cfg.network; let db_height_opt = self.db_height().await?; let mut db_height = db_height_opt.unwrap_or(GENESIS_HEIGHT); diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 3c116d25c..c9b16c32f 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -88,24 +88,22 @@ impl ChainIndexSnapshot { } } -/// Whether a published snapshot's non-finalized window is anchored to the -/// validated finalized chain yet. +/// Whether a published snapshot's non-finalized window has been validated +/// against the finalized chain yet. /// -/// The snapshot always exists and always carries blocks; this says whether the -/// finalized DB has caught up to the seam. Flipped to `Resolved` inside -/// `update`, atomically with the `compare_and_swap` that publishes the -/// snapshot (the value rides in the snapshot, so the flip is not separable -/// from the block contents). +/// This is purely about *absolute cumulative chainwork*: the snapshot always +/// exists and always serves its own block data regardless. While `Provisional` +/// the window carries only relative work; once the finalized DB reaches the +/// seam it is `Resolved` and absolute work is recoverable. Flipped to +/// `Resolved` inside `update`, atomically with the `compare_and_swap` that +/// publishes the snapshot (the value rides in the snapshot, so the flip is not +/// separable from the block contents). It carries no validator/passthrough +/// height — the NFS never passes through; it serves its own data. #[derive(Debug, Clone, Copy)] pub(crate) enum SnapshotAvailability { - /// The finalized DB has not reached the seam. The window's prev-hash - /// linkage is unvalidated and its blocks have no absolute chainwork; reads - /// that need finalized data fall back to the validator, serving up to - /// `validator_finalized_height`. - Provisional { - /// The finalized-tip height passthrough may serve up to. - validator_finalized_height: Height, - }, + /// The finalized DB has not reached the seam: the window's prev-hash + /// linkage is unvalidated and its blocks carry only relative chainwork. + Provisional, /// The finalized DB has reached the seam: the window is validated against /// the finalized chain. (The seam's absolute-chainwork base — needed to /// resolve window blocks' *absolute* chainwork = `base + relative` — is @@ -398,9 +396,7 @@ impl NonfinalizedBlockCacheSnapshot { best_tip, // Newly seeded from the seam block: the finalized DB has not yet // caught up to it. `update` flips this to `Resolved` once it has. - availability: SnapshotAvailability::Provisional { - validator_finalized_height: height, - }, + availability: SnapshotAvailability::Provisional, } } @@ -555,24 +551,31 @@ impl NonFinalizedState { finalized_db: Arc, chain_height: Height, ) -> Result<(), SyncError> { + // The NFS is validator-sourced and *leads* the finalized DB: it holds + // only the non-finalized window `[ceiling, tip]`, anchored at the + // finalization ceiling (`chain_height - NON_FINALIZED_DEPTH`). Whenever + // the current floor sits below the ceiling — at cold start (seeded from + // genesis) or after a deep rollback — re-anchor at the ceiling so the + // NFS never walks below it (in particular, never from genesis). + // + // The seam (ceiling) block comes from the finalized DB if it has already + // reached the ceiling, otherwise straight from the source: the NFS does + // not wait for the finalized DB to catch up. + let anchor_height = super::finalization_ceiling(chain_height.0); let mut initial_state = self.get_snapshot(); - let local_finalized_tip = finalized_db.to_reader().db_height().await?; - if Some(initial_state.best_tip.height) < local_finalized_tip { + if initial_state.best_tip.height < anchor_height { + let seam = match finalized_db + .to_reader() + .get_chain_block_by_height(anchor_height) + .await? + { + Some(indexed) => indexed, + None => self.fetch_seam_indexed_block(anchor_height).await?, + }; self.current.swap(Arc::new( - NonfinalizedBlockCacheSnapshot::from_initial_block( - finalized_db - .to_reader() - .get_chain_block_by_height( - local_finalized_tip.expect("known to be some due to above if"), - ) - .await? - .ok_or(FinalisedStateError::DataUnavailable(format!( - "Missing block {}", - local_finalized_tip.unwrap().0 - )))?, - ), + NonfinalizedBlockCacheSnapshot::from_initial_block(seam), )); - initial_state = self.get_snapshot() + initial_state = self.get_snapshot(); } let mut working_snapshot = initial_state.as_ref().clone(); @@ -722,9 +725,18 @@ impl NonFinalizedState { return Err(SyncError::CompetingSyncProcess); }; + // The NFS holds only the non-finalized window `[seam, tip]`. The seam is + // the lowest height it tracks; listener blocks below it are finalized + // and not the NFS's concern. Processing one would walk its ancestry off + // the bottom of the window — recursing past genesis in `add_nonbest_block` + // and erroring with `MissingBlockError`. + let seam = super::finalization_ceiling(working_snapshot.best_tip.height.0); loop { match listener.try_recv() { Ok((hash, block)) => { + if block.coinbase_height().is_some_and(|h| Height(h.0) < seam) { + continue; + } if !self .current .load() @@ -759,7 +771,13 @@ impl NonFinalizedState { .map_err(|_e| UpdateError::FinalizedStateCorruption)? .unwrap_or(Height(0)); - new_snapshot.remove_finalized_blocks(finalized_height); + // The NFS holds only the non-finalized window. Trim to the seam (the + // finalization ceiling of the current tip), derived from the NFS's own + // best tip — *not* the finalized-DB tip, which lags since the NFS leads. + // This advances the window floor with the chain and evicts blocks that + // have fallen below the seam, independent of finalized-DB progress. + let seam = super::finalization_ceiling(new_snapshot.best_tip.height.0); + new_snapshot.remove_finalized_blocks(seam); let best_block = &new_snapshot .blocks .values() @@ -785,9 +803,7 @@ impl NonFinalizedState { // promotion, #1096, when its reader exists.) SnapshotAvailability::Resolved } else { - SnapshotAvailability::Provisional { - validator_finalized_height: finalized_height, - } + SnapshotAvailability::Provisional }; // Need to get best hash at some point in this process @@ -930,6 +946,50 @@ impl NonFinalizedState { }) } + /// Fetch the seam (finalization-ceiling) block from the source and build it + /// as an [`IndexedBlock`] to anchor the NFS window while the finalized DB is + /// still catching up. The absolute chainwork is a zero placeholder: + /// [`ProvisionalBlock::from_indexed_seam`] drops it and tracks the window's + /// work relative to this seam. + async fn fetch_seam_indexed_block(&self, height: Height) -> Result { + let block = self + .source + .get_block(HashOrHeight::Height(zebra_chain::block::Height(height.0))) + .await + .map_err(|e| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( + Box::new(e), + )) + })? + .ok_or_else(|| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( + Box::new(InvalidData(format!( + "source missing seam block at height {}", + height.0 + ))), + )) + })?; + let tree_roots = self + .get_tree_roots_from_source(block.hash().into()) + .await + .map_err(|e| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( + Box::new(InvalidData(format!("{e}"))), + )) + })?; + Self::create_indexed_block_with_optional_roots( + block.as_ref(), + &tree_roots, + ChainWork::from_u256(U256::zero()), + self.network.clone(), + ) + .map_err(|e| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( + InvalidData(e), + ))) + }) + } + /// Create IndexedBlock with optional tree roots (for genesis/sync cases) /// /// TODO: Issue #604 - This uses `unwrap_or_default()` uniformly for both Sapling and Orchard, diff --git a/packages/zaino-state/src/chain_index/source.rs b/packages/zaino-state/src/chain_index/source.rs index 4775f6b69..6335ad176 100644 --- a/packages/zaino-state/src/chain_index/source.rs +++ b/packages/zaino-state/src/chain_index/source.rs @@ -223,6 +223,19 @@ pub trait BlockchainSource: Clone + Send + Sync + 'static { fn subscribe_to_blocks_received(&self) -> Option> { None } + + /// An optional upper bound on the height the finalized DB may sync to this + /// iteration, independent of the chain tip. + /// + /// Default `None` imposes no cap — real validators let the finalized DB + /// climb to the finalization ceiling. Test mockchains override it to hold + /// the finalized DB at a known height below the ceiling: since the + /// non-finalized state is validator-sourced and *leads*, capping the + /// finalized DB deterministically holds the snapshot Provisional with a + /// known catch-up gap — without slowing any source call. + fn finalized_sync_cap(&self) -> Option { + None + } } /// Sleep up to `duration`, but return early if `change_rx` resolves first. diff --git a/packages/zaino-state/src/chain_index/tests.rs b/packages/zaino-state/src/chain_index/tests.rs index 6e9661e83..5b8d0804a 100644 --- a/packages/zaino-state/src/chain_index/tests.rs +++ b/packages/zaino-state/src/chain_index/tests.rs @@ -31,7 +31,7 @@ use zaino_common::{network::ActivationHeights, DatabaseConfig, Network, StorageC use crate::{ chain_index::{ finalised_state::ZainoDB, - finalized_height_floor, + finalization_ceiling, source::mockchain_source::MockchainSource, tests::vectors::{ build_active_mockchain_source, build_mockchain_source, copy_dir_recursive, @@ -48,12 +48,12 @@ use crate::{ /// /// - `Active` → `build_active_mockchain_source(150, blocks)`: source has a /// separately-tracked `active_height = 150` that tests can advance via -/// `mockchain.mine_blocks(N)`. Indexer's finalised sync target is -/// `finalized_height_floor(150) = 50`. +/// `mockchain.mine_blocks(N)`. Indexer's finalised sync target is the +/// finalization ceiling for tip 150 = 50. /// - `Static` → `build_mockchain_source(blocks)`: every loaded block is /// immediately active (`active_height = tip_height = 200` for the 201-block /// vector); the tip doesn't move during the test. Indexer's finalised sync -/// target is `finalized_height_floor(200) = 100`. +/// target is the finalization ceiling for tip 200 = 100. #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum MockchainMode { Active, @@ -143,7 +143,7 @@ async fn load_with_settings( // Wait until the indexer's non-finalised state has been built and its // best tip matches the source. The previous form checked only - // `finalized_state.db_height() == finalized_height_floor(active_height)`, + // `finalized_state.db_height() == finalization ceiling for active_height`, // which the seed copy makes true *before* the sync loop has had a chance // to initialise NFS. Tests that read the NFS immediately after the // helper returns (`nfs_lowest_block_matches_finalized_db_tip`, @@ -190,7 +190,7 @@ async fn v1_finalised_seed_dir(mode: MockchainMode) -> &'static Path { MockchainMode::Active => build_active_mockchain_source(150, blocks.clone()), MockchainMode::Static => build_mockchain_source(blocks.clone()), }; - let target = finalized_height_floor(source.active_height()).0; + let target = finalization_ceiling(source.active_height()).0; let temp_dir: TempDir = tempfile::tempdir().unwrap(); let config = BlockCacheConfig { diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index cbe1f6b5c..935e1b976 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -23,7 +23,7 @@ //! `best_chaintip` reading the snapshot's `best_tip` directly. use super::{load_test_vectors_and_sync_chain_index, poll::poll_until, MockchainMode}; -use crate::chain_index::{finalized_height_floor, ChainIndex}; +use crate::chain_index::{finalization_ceiling, ChainIndex}; use std::time::Duration; use tokio::time::sleep; @@ -41,7 +41,7 @@ async fn nfs_lowest_block_matches_finalized_db_tip() { .resolved_nfs_snapshot() .expect("NFS exists after harness completes finalized sync"); - let seam_height = finalized_height_floor(mockchain.active_height()); + let seam_height = finalization_ceiling(mockchain.active_height()); let nfs_seam_hash = nfs .heights_to_hashes .get(&seam_height) @@ -72,7 +72,7 @@ async fn block_is_evicted_from_nfs_when_finalized_advances_past_it() { let (_blocks, _indexer, index_reader, mockchain) = load_test_vectors_and_sync_chain_index(MockchainMode::Active).await; - let initial_seam_height = finalized_height_floor(mockchain.active_height()); + let initial_seam_height = finalization_ceiling(mockchain.active_height()); let initial_snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); let initial_nfs = initial_snapshot @@ -212,13 +212,13 @@ async fn shutdown_terminates_sync_loop_cleanly() { /// [`MockchainSource::arm_one_shot_get_block_hook`]. The hook fires the /// *first* time the worker requests `get_block(Height(_))`, which is the /// first call inside iter N's NFS-sync while loop *after* iter N has already -/// committed to `chain_height = initial_active` and called -/// `fs.sync_to_height(finalized_height_floor(initial_active))` as a no-op. +/// committed to `chain_height = initial_active` and synced the finalized DB to +/// `finalization_ceiling(initial_active)` as a no-op. /// From inside the hook the test mines 20 blocks; the same `get_block` call /// then reads the *new* `active_chain_height = initial_active + 20` and /// returns block `initial_active + 1`, which the worker's loop happily /// extends past the iter's commitment all the way to `initial_active + 20`. -/// The iter's `update` step uses `finalized_height_floor(initial_active)` +/// The iter's `update` step uses the finalization ceiling for `initial_active` /// for the trim and publishes a snapshot whose lowest height is *below* the /// post-mine seam. /// @@ -236,7 +236,7 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi load_test_vectors_and_sync_chain_index(MockchainMode::Active).await; let initial_active = mockchain.active_height(); - let pre_mine_finalized_height = finalized_height_floor(initial_active); + let pre_mine_finalized_height = finalization_ceiling(initial_active); let initial_snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); let initial_nfs = initial_snapshot diff --git a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs index e517aa4f0..2c5cc805c 100644 --- a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs +++ b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs @@ -23,11 +23,11 @@ use zebra_rpc::{ client::{GetAddressBalanceRequest, GetAddressTxIdsRequest}, methods::{AddressBalance, GetAddressUtxos}, }; -use zebra_state::{FromDisk, HashOrHeight, IntoDisk as _}; +use zebra_state::HashOrHeight; use crate::{ chain_index::{ - finalized_height_floor, + finalization_ceiling, non_finalised_state::ChainIndexSnapshot, source::{BlockchainSourceResult, GetTransactionLocation}, tests::{init_tracing, poll::poll_until, proptest_blockgen::proptest_helpers::add_segment}, @@ -38,6 +38,13 @@ use crate::{ NodeBackedChainIndexSubscriber, TransactionHash, }; +/// The finalization ceiling for a snapshot's own best tip: the highest height +/// it serves from its own data, and the boundary below which the validator may +/// be consulted by passthrough. +fn snapshot_finalization_ceiling(snapshot: &ChainIndexSnapshot) -> crate::Height { + finalization_ceiling(snapshot.get_nfs_snapshot().best_tip.height.0) +} + /// Handle all the boilerplate for a passthrough fn passthrough_test( // The actual assertions. Takes as args: @@ -66,10 +73,12 @@ fn passthrough_test( let mockchain = ProptestMockchain { genesis_segment, branching_segments, - // This number can be played with. We want to slow down - // sync enough to trigger passthrough without - // slowing down passthrough more than we need to - delay: Some(Duration::from_millis(100)), + // Hold the finalized DB at genesis so the snapshot stays + // Provisional: the always-leading NFS reaches the tip while the + // finalized DB lags, so every block below the finalization + // ceiling is served through the validator-passthrough gap. No + // artificial per-call delay — the gap is deterministic. + finalized_sync_cap: Arc::new(std::sync::atomic::AtomicU32::new(0)), best_branch_cache: Arc::new(std::sync::OnceLock::new()), tx_index: Arc::new(std::sync::OnceLock::new()), }; @@ -94,26 +103,29 @@ fn passthrough_test( .unwrap(); let index_reader = indexer.subscriber(); // 101 instead of 100 as heights are 0-indexed - let expected_max_serviceable_height = (2 * segment_length) - 101; - // Poll rather than sleeping a fixed 5 s: the indexer discovers the - // chain topology as soon as the sync task has walked enough of the - // source to identify the finalized-state cutoff. With a 1 s - // per-block source delay (above) that's well under 5 s in practice, - // but can be longer under parallel-suite scheduler pressure. + let expected_finalization_ceiling = (2 * segment_length) - 101; + // Poll rather than sleeping: the always-leading NFS reaches the tip + // (and so the expected finalization ceiling) as soon as the sync + // task has walked the non-finalized window from the source. With no + // artificial per-call delay this is fast; the budget only guards + // against parallel-suite scheduler pressure. poll_until( - "indexer to reach expected max_serviceable_height", + "indexer to reach the expected finalization ceiling", Duration::from_secs(30), Duration::from_millis(50), || async { let snapshot = index_reader.snapshot_nonfinalized_state().await.ok()?; - (snapshot.max_serviceable_height().0 as usize - == expected_max_serviceable_height) + (snapshot_finalization_ceiling(&snapshot).0 as usize + == expected_finalization_ceiling) .then_some(()) }, ) .await; let snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); - assert_eq!(snapshot.max_serviceable_height().0 as usize, expected_max_serviceable_height); + assert_eq!( + snapshot_finalization_ceiling(&snapshot).0 as usize, + expected_finalization_ceiling + ); assert!(!snapshot.is_resolved()); test(&mockchain, index_reader, &snapshot).await; @@ -135,7 +147,7 @@ fn passthrough_find_fork_point() { // as this lets us call all the get_raw_transaction requests // at the same time and wait for them in parallel // - // This allows the artificial delays to happen in parallel + // This lets the per-block passthrough source calls run concurrently. let mut parallel = FuturesUnordered::new(); // As we only have one branch, arbitrary branch order is fine for (height, hash) in mockchain @@ -150,14 +162,11 @@ fn passthrough_find_fork_point() { .await .unwrap(); - if height <= *snapshot.max_serviceable_height() { - // passthrough fork point can only ever be the requested block - // as we don't passthrough to nonfinalized state - assert_eq!(hash, fork_point.unwrap().0); - assert_eq!(height, fork_point.unwrap().1); - } else { - assert!(fork_point.is_none()); - } + // Single branch: every block is on the best chain, so its fork + // point is itself — served from the NFS window, or (below the + // ceiling) via the validator-passthrough gap. Never None. + assert_eq!(hash, fork_point.unwrap().0); + assert_eq!(height, fork_point.unwrap().1); }) } while let Some(_success) = parallel.next().await {} @@ -171,7 +180,7 @@ fn passthrough_get_transaction_status() { // as this lets us call all the get_raw_transaction requests // at the same time and wait for them in parallel // - // This allows the artificial delays to happen in parallel + // This lets the per-block passthrough source calls run concurrently. let mut parallel = FuturesUnordered::new(); // As we only have one branch, arbitrary branch order is fine for (height, txid) in mockchain.all_blocks_arb_branch_order().flat_map(|block| { @@ -189,18 +198,14 @@ fn passthrough_get_transaction_status() { .await .unwrap(); - if height <= *snapshot.max_serviceable_height() { - // passthrough transaction status can only ever be on the best - // chain as we don't passthrough to nonfinalized state - let Some(BestChainLocation::Block(_block_hash, transaction_height)) = - transaction_status.0 - else { - panic!("expected best chain location") - }; - assert_eq!(height, transaction_height); - } else { - assert!(transaction_status.0.is_none()); - } + // Single branch: every transaction is on the best chain, served + // from the NFS window or (below the ceiling) the passthrough gap. + let Some(BestChainLocation::Block(_block_hash, transaction_height)) = + transaction_status.0 + else { + panic!("expected best chain location") + }; + assert_eq!(height, transaction_height); assert!(transaction_status.1.is_empty()); }) } @@ -215,7 +220,7 @@ fn passthrough_get_raw_transaction() { // as this lets us call all the get_raw_transaction requests // at the same time and wait for them in parallel // - // This allows the artificial delays to happen in parallel + // This lets the per-block passthrough source calls run concurrently. let mut parallel = FuturesUnordered::new(); // As we only have one branch, arbitrary branch order is fine for (expected_transaction, height) in @@ -254,6 +259,8 @@ fn passthrough_get_raw_transaction() { fn passthrough_best_chaintip() { passthrough_test(async |mockchain, index_reader, snapshot| { let tip = index_reader.best_chaintip(snapshot).await.unwrap(); + // best_chaintip derives from the always-leading NFS, which sits at the + // full chain tip even while the finalized DB lags behind. assert_eq!( tip.height.0, mockchain @@ -261,8 +268,8 @@ fn passthrough_best_chaintip() { .last() .unwrap() .coinbase_height() - .map(|h| finalized_height_floor(h.0).0) .unwrap() + .0 ); }) } @@ -274,7 +281,7 @@ fn passthrough_get_block_height() { // as this lets us call all the get_raw_transaction requests // at the same time and wait for them in parallel // - // This allows the artificial delays to happen in parallel + // This lets the per-block passthrough source calls run concurrently. let mut parallel = FuturesUnordered::new(); for (expected_height, hash) in mockchain @@ -288,11 +295,9 @@ fn passthrough_get_block_height() { .get_block_height(&snapshot, hash.into()) .await .unwrap(); - if expected_height <= *snapshot.max_serviceable_height() { - assert_eq!(height, Some(expected_height.into())); - } else { - assert_eq!(height, None); - } + // Every block is served: the NFS window, or (below the ceiling) + // the validator-passthrough gap. + assert_eq!(height, Some(expected_height.into())); }); } while let Some(_success) = parallel.next().await {} @@ -306,7 +311,7 @@ fn passthrough_get_block_range() { // as this lets us call all the get_raw_transaction requests // at the same time and wait for them in parallel // - // This allows the artificial delays to happen in parallel + // This lets the per-block passthrough source calls run concurrently. let mut parallel = FuturesUnordered::new(); for expected_start_height in mockchain @@ -323,36 +328,33 @@ fn passthrough_get_block_range() { expected_start_height.into(), Some(expected_end_height.into()), ); - if expected_start_height <= *snapshot.max_serviceable_height() { - let mut block_range_stream = Box::pin(block_range_stream.unwrap()); - let mut num_blocks_in_stream = 0; - while let Some(block) = block_range_stream.next().await { - let expected_block = mockchain - .all_blocks_arb_branch_order() - .nth(expected_start_height.0 as usize + num_blocks_in_stream) - .unwrap() - .zcash_serialize_to_vec() - .unwrap(); - assert_eq!(block.unwrap(), expected_block); - num_blocks_in_stream += 1; - } - assert_eq!( - num_blocks_in_stream, - // expect 10 blocks - 10.min( - // unless the provided range overlaps the finalized boundary. - // in that case, expect all blocks between start height - // and finalized height, (+1 for inclusive range) - snapshot - .max_serviceable_height() - .0 - .saturating_sub(expected_start_height.0) - + 1 - ) as usize - ); - } else { - assert!(block_range_stream.is_none()) + // Every height up to the tip is served (NFS window ∪ + // passthrough gap), so the range is always servable. + let mut block_range_stream = Box::pin(block_range_stream.unwrap()); + let mut num_blocks_in_stream = 0; + while let Some(block) = block_range_stream.next().await { + let expected_block = mockchain + .all_blocks_arb_branch_order() + .nth(expected_start_height.0 as usize + num_blocks_in_stream) + .unwrap() + .zcash_serialize_to_vec() + .unwrap(); + assert_eq!(block.unwrap(), expected_block); + num_blocks_in_stream += 1; } + assert_eq!( + num_blocks_in_stream, + // 10 blocks, unless the range runs past the best tip. + 10.min( + snapshot + .get_nfs_snapshot() + .best_tip + .height + .0 + .saturating_sub(expected_start_height.0) + + 1 + ) as usize + ); }); } } @@ -377,7 +379,9 @@ fn make_chain() { let mockchain = ProptestMockchain { genesis_segment, branching_segments, - delay: None, + // No cap: the finalized DB syncs all the way to the ceiling, so + // the snapshot resolves. + finalized_sync_cap: Arc::new(std::sync::atomic::AtomicU32::new(u32::MAX)), best_branch_cache: Arc::new(std::sync::OnceLock::new()), tx_index: Arc::new(std::sync::OnceLock::new()), }; @@ -444,7 +448,13 @@ fn make_chain() { struct ProptestMockchain { genesis_segment: ChainSegment, branching_segments: Vec, - delay: Option, + /// Caps the height the finalized DB may sync to (`u32::MAX` = no cap), + /// surfaced via [`BlockchainSource::finalized_sync_cap`]. Holding it below + /// the finalization ceiling keeps the snapshot deterministically Provisional + /// — the always-leading NFS reaches the tip while the finalized DB stays + /// behind — so the catch-up gap (served by passthrough) is exercised without + /// any artificial per-call delay. + finalized_sync_cap: Arc, /// Cached result of `best_branch()`. The best branch is pure function of /// the other fields (which are never mutated after construction), so it's /// safe to memoize. Shared via `Arc` so `mockchain.clone()` — which @@ -533,33 +543,6 @@ impl ProptestMockchain { .flat_map(|branch| branch.iter()), ) } - - fn get_block_and_all_preceeding( - &self, - // This probably doesn't need to allow FnMut closures (Fn should suffice) - // but there's no cost to allowing it - mut block_identifier: impl FnMut(&zebra_chain::block::Block) -> bool, - ) -> std::option::Option>> { - let mut blocks = Vec::new(); - for block in self.genesis_segment.iter() { - blocks.push(block); - if block_identifier(block) { - return Some(blocks); - } - } - for branch in self.branching_segments.iter() { - let mut branch_blocks = Vec::new(); - for block in branch.iter() { - branch_blocks.push(block); - if block_identifier(block) { - blocks.extend_from_slice(&branch_blocks); - return Some(blocks); - } - } - } - - None - } } #[async_trait] @@ -569,9 +552,6 @@ impl BlockchainSource for ProptestMockchain { &self, id: HashOrHeight, ) -> BlockchainSourceResult>> { - if let Some(delay) = self.delay { - tokio::time::sleep(delay).await; - } match id { HashOrHeight::Hash(hash) => { let matches_hash = |block: &&Arc| block.hash() == hash; @@ -608,69 +588,19 @@ impl BlockchainSource for ProptestMockchain { /// Returns the block commitment tree data by hash async fn get_commitment_tree_roots( &self, - id: BlockHash, + _id: BlockHash, ) -> BlockchainSourceResult<( Option<(zebra_chain::sapling::tree::Root, u64)>, Option<(zebra_chain::orchard::tree::Root, u64)>, )> { - if let Some(delay) = self.delay { - tokio::time::sleep(delay).await; - } - let Some(chain_up_to_block) = - self.get_block_and_all_preceeding(|block| block.hash().0 == id.0) - else { - return Ok((None, None)); - }; - - let (sapling, orchard) = - chain_up_to_block - .iter() - .fold((None, None), |(mut sapling, mut orchard), block| { - for transaction in &block.transactions { - for sap_commitment in transaction.sapling_note_commitments() { - let sap_commitment = - sapling_crypto::Node::from_bytes(sap_commitment.to_bytes()) - .unwrap(); - - sapling = Some(sapling.unwrap_or_else(|| { - incrementalmerkletree::frontier::Frontier::<_, 32>::empty() - })); - - sapling = sapling.map(|mut tree| { - tree.append(sap_commitment); - tree - }); - } - for orc_commitment in transaction.orchard_note_commitments() { - let orc_commitment = - zebra_chain::orchard::tree::Node::from(*orc_commitment); - - orchard = Some(orchard.unwrap_or_else(|| { - incrementalmerkletree::frontier::Frontier::<_, 32>::empty() - })); - - orchard = orchard.map(|mut tree| { - tree.append(orc_commitment); - tree - }); - } - } - (sapling, orchard) - }); - Ok(( - sapling.map(|sap_front| { - ( - zebra_chain::sapling::tree::Root::from_bytes(sap_front.root().to_bytes()), - sap_front.tree_size(), - ) - }), - orchard.map(|orc_front| { - ( - zebra_chain::orchard::tree::Root::from_bytes(orc_front.root().as_bytes()), - orc_front.tree_size(), - ) - }), - )) + // These proptests exercise block/transaction *routing* (NFS window, + // finalized DB, validator passthrough), never commitment-tree data. + // Rebuilding the Sapling/Orchard frontier from genesis on every call is + // O(N) cryptographic hashing per block — O(N²) across the NFS window + // walk, which was the dominant (tens-of-seconds) cost of these tests. + // Serve no roots: blocks are built with default (empty) tree data, + // which these tests don't inspect. + Ok((None, None)) } /// Returns the sapling and orchard treestate by hash @@ -686,9 +616,6 @@ impl BlockchainSource for ProptestMockchain { async fn get_mempool_txids( &self, ) -> BlockchainSourceResult>> { - if let Some(delay) = self.delay { - tokio::time::sleep(delay).await; - } Ok(Some(Vec::new())) } @@ -702,9 +629,6 @@ impl BlockchainSource for ProptestMockchain { GetTransactionLocation, )>, > { - if let Some(delay) = self.delay { - tokio::time::sleep(delay).await; - } Ok(self.tx_index().get(&txid.into()).cloned()) } @@ -712,9 +636,6 @@ impl BlockchainSource for ProptestMockchain { async fn get_best_block_hash( &self, ) -> BlockchainSourceResult> { - if let Some(delay) = self.delay { - tokio::time::sleep(delay).await; - } Ok(Some(self.best_branch().last().unwrap().hash())) } @@ -722,9 +643,6 @@ impl BlockchainSource for ProptestMockchain { async fn get_best_block_height( &self, ) -> BlockchainSourceResult> { - if let Some(delay) = self.delay { - tokio::time::sleep(delay).await; - } Ok(Some( self.best_branch() .last() @@ -734,6 +652,16 @@ impl BlockchainSource for ProptestMockchain { )) } + fn finalized_sync_cap(&self) -> Option { + match self + .finalized_sync_cap + .load(std::sync::atomic::Ordering::Relaxed) + { + u32::MAX => None, + cap => Some(crate::Height(cap)), + } + } + /// Get a listener for new nonfinalized blocks, /// if supported async fn nonfinalized_listener( From cc6d49fbcd6e8670f62021745a1e30e253870c97 Mon Sep 17 00:00:00 2001 From: zancas Date: Wed, 27 May 2026 00:38:31 -0700 Subject: [PATCH 10/31] chain_index tests: add Resolved full-coverage test; cache proptest commitment roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `resolved_snapshot_serves_every_block` (non_finalised_state.rs), the Resolved counterpart to the Provisional `passthrough_*` tests. On the real-vector harness (Active mode → synced to 150, seam 50) the snapshot resolves with a genuine finalized-DB ∪ NFS-window split, and the test asserts every height 0..=150 round-trips through get_block_hash / get_block_height / find_fork_point with no gap. It lives on the real-vector source, not the proptest mock: the proptest mock generates arbitrary blocks that aren't a valid UTXO chain, so a real finalized sync over them fails the finalized DB's txout-set accumulator (and its commitment-tree checks). The proptest `passthrough_*` tests only work because they keep the finalized DB empty. Cache the proptest mock's `get_commitment_tree_roots`: the NFS sync calls it once per block while walking the window, and folding the Sapling/Orchard frontier from genesis on every call was O(N) hashing per call — O(N²) across the walk, the dominant cost of these tests. Precompute every block's roots in one incremental pass, keyed by hash, for O(1) lookups. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../chain_index/tests/non_finalised_state.rs | 41 ++++++++ .../chain_index/tests/proptest_blockgen.rs | 96 ++++++++++++++++--- 2 files changed, 122 insertions(+), 15 deletions(-) diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index 935e1b976..1d8b9d4b5 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -63,6 +63,47 @@ async fn nfs_lowest_block_matches_finalized_db_tip() { ); } +/// Converged full-coverage check, the Resolved counterpart to the Provisional +/// `passthrough_*` tests: once the indexer has resolved over the real-vector +/// chain, every height from genesis to the best tip — across the FS/NFS seam — +/// is served from the snapshot's own data (finalized DB below the seam, NFS +/// window above it), with no catch-up gap, and height ↔ hash round-trips +/// through all three query methods. +#[tokio::test(flavor = "multi_thread")] +async fn resolved_snapshot_serves_every_block() { + let (_blocks, _indexer, index_reader, mockchain) = + load_test_vectors_and_sync_chain_index(MockchainMode::Active).await; + + let snapshot = index_reader.snapshot_nonfinalized_state().await.unwrap(); + assert!( + snapshot.is_resolved(), + "harness syncs the finalized DB to the seam, so the snapshot is Resolved", + ); + + for height in 0..=mockchain.active_height() { + let height = crate::Height(height); + let hash = index_reader + .get_block_hash(&snapshot, height) + .await + .unwrap() + .unwrap_or_else(|| panic!("no block served at height {}", height.0)); + + assert_eq!( + index_reader.get_block_height(&snapshot, hash).await.unwrap(), + Some(height), + "get_block_height round-trip at height {}", + height.0, + ); + + let fork_point = index_reader + .find_fork_point(&snapshot, &hash) + .await + .unwrap() + .unwrap_or_else(|| panic!("no fork point served at height {}", height.0)); + assert_eq!(fork_point, (hash, height)); + } +} + /// **D**: A block in the NFS is evicted once the finalized DB advances /// past its height. Pins the trim step inside `update` /// (`non_finalised_state.rs:remove_finalized_blocks`, which retains diff --git a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs index 2c5cc805c..5e5e01fe5 100644 --- a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs +++ b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs @@ -23,7 +23,7 @@ use zebra_rpc::{ client::{GetAddressBalanceRequest, GetAddressTxIdsRequest}, methods::{AddressBalance, GetAddressUtxos}, }; -use zebra_state::HashOrHeight; +use zebra_state::{FromDisk, HashOrHeight, IntoDisk as _}; use crate::{ chain_index::{ @@ -81,6 +81,7 @@ fn passthrough_test( finalized_sync_cap: Arc::new(std::sync::atomic::AtomicU32::new(0)), best_branch_cache: Arc::new(std::sync::OnceLock::new()), tx_index: Arc::new(std::sync::OnceLock::new()), + commitment_roots_cache: Arc::new(std::sync::OnceLock::new()), }; let temp_dir: tempfile::TempDir = tempfile::tempdir().unwrap(); let db_path: std::path::PathBuf = temp_dir.path().to_path_buf(); @@ -384,6 +385,7 @@ fn make_chain() { finalized_sync_cap: Arc::new(std::sync::atomic::AtomicU32::new(u32::MAX)), best_branch_cache: Arc::new(std::sync::OnceLock::new()), tx_index: Arc::new(std::sync::OnceLock::new()), + commitment_roots_cache: Arc::new(std::sync::OnceLock::new()), }; let temp_dir: tempfile::TempDir = tempfile::tempdir().unwrap(); let db_path: std::path::PathBuf = temp_dir.path().to_path_buf(); @@ -444,6 +446,58 @@ fn make_chain() { }); } +/// Sapling and Orchard commitment-tree `(root, tree_size)` for a block, as +/// returned by [`BlockchainSource::get_commitment_tree_roots`]. +type CommitmentRoots = ( + Option<(zebra_chain::sapling::tree::Root, u64)>, + Option<(zebra_chain::orchard::tree::Root, u64)>, +); + +type SaplingFrontier = incrementalmerkletree::frontier::Frontier; +type OrchardFrontier = + incrementalmerkletree::frontier::Frontier; + +/// Append each block's note commitments to the running frontiers, recording the +/// resulting `(root, tree_size)` per block hash into `map`. Returns the final +/// frontiers so a branch can continue from where the genesis segment left off. +fn fold_commitment_roots<'a>( + blocks: impl Iterator>, + mut sapling: Option, + mut orchard: Option, + map: &mut std::collections::HashMap, +) -> (Option, Option) { + for block in blocks { + for transaction in &block.transactions { + for sap in transaction.sapling_note_commitments() { + let node = sapling_crypto::Node::from_bytes(sap.to_bytes()).unwrap(); + sapling.get_or_insert_with(SaplingFrontier::empty).append(node); + } + for orc in transaction.orchard_note_commitments() { + let node = zebra_chain::orchard::tree::Node::from(*orc); + orchard.get_or_insert_with(OrchardFrontier::empty).append(node); + } + } + map.insert( + BlockHash::from(block.hash()), + ( + sapling.as_ref().map(|f| { + ( + zebra_chain::sapling::tree::Root::from_bytes(f.root().to_bytes()), + f.tree_size(), + ) + }), + orchard.as_ref().map(|f| { + ( + zebra_chain::orchard::tree::Root::from_bytes(f.root().as_bytes()), + f.tree_size(), + ) + }), + ), + ); + } + (sapling, orchard) +} + #[derive(Clone)] struct ProptestMockchain { genesis_segment: ChainSegment, @@ -476,6 +530,11 @@ struct ProptestMockchain { >, >, >, + /// Cached commitment-tree roots keyed by block hash, built lazily in one + /// incremental pass. The finalized DB requires real roots (it rejects + /// `None`), and recomputing them by folding from genesis on every call was + /// O(N) crypto per call → O(N²) across a sync; this makes lookups O(1). + commitment_roots_cache: Arc>>, } impl ProptestMockchain { @@ -585,22 +644,29 @@ impl BlockchainSource for ProptestMockchain { } } - /// Returns the block commitment tree data by hash + /// Returns the block commitment tree data by hash. + /// + /// The NFS sync calls this once per block as it walks the window, so roots + /// are precomputed for every block in one incremental pass and cached by + /// hash (O(1) lookups). The previous implementation folded the frontier + /// from genesis on every call — O(N) cryptographic hashing per call, O(N²) + /// across the window walk, which dominated these tests' runtime. async fn get_commitment_tree_roots( &self, - _id: BlockHash, - ) -> BlockchainSourceResult<( - Option<(zebra_chain::sapling::tree::Root, u64)>, - Option<(zebra_chain::orchard::tree::Root, u64)>, - )> { - // These proptests exercise block/transaction *routing* (NFS window, - // finalized DB, validator passthrough), never commitment-tree data. - // Rebuilding the Sapling/Orchard frontier from genesis on every call is - // O(N) cryptographic hashing per block — O(N²) across the NFS window - // walk, which was the dominant (tens-of-seconds) cost of these tests. - // Serve no roots: blocks are built with default (empty) tree data, - // which these tests don't inspect. - Ok((None, None)) + id: BlockHash, + ) -> BlockchainSourceResult { + let roots = self.commitment_roots_cache.get_or_init(|| { + let mut map = std::collections::HashMap::new(); + // Genesis segment, then each branch continuing from the genesis-end + // frontier — one incremental fold over the whole tree. + let (sapling, orchard) = + fold_commitment_roots(self.genesis_segment.iter(), None, None, &mut map); + for branch in self.branching_segments.iter() { + fold_commitment_roots(branch.iter(), sapling.clone(), orchard.clone(), &mut map); + } + map + }); + Ok(roots.get(&id).cloned().unwrap_or((None, None))) } /// Returns the sapling and orchard treestate by hash From b5ecb5446ee400cf9e83ba1c2677b4603fd38f7d Mon Sep 17 00:00:00 2001 From: zancas Date: Wed, 27 May 2026 00:39:41 -0700 Subject: [PATCH 11/31] chain_index tests: add Resolved full-coverage test; cache proptest commitment roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `resolved_snapshot_serves_every_block` (non_finalised_state.rs), the Resolved counterpart to the Provisional `passthrough_*` tests. On the real-vector harness (Active mode → synced to 150, seam 50) the snapshot resolves with a genuine finalized-DB ∪ NFS-window split, and the test asserts every height 0..=150 round-trips through get_block_hash / get_block_height / find_fork_point with no gap. It lives on the real-vector source, not the proptest mock: the proptest mock generates arbitrary blocks that aren't a valid UTXO chain, so a real finalized sync over them fails the finalized DB's txout-set accumulator (and its commitment-tree checks). The proptest `passthrough_*` tests only work because they keep the finalized DB empty. Cache the proptest mock's `get_commitment_tree_roots`: the NFS sync calls it once per block while walking the window, and folding the Sapling/Orchard frontier from genesis on every call was O(N) hashing per call — O(N²) across the walk, the dominant cost of these tests. Precompute every block's roots in one incremental pass, keyed by hash, for O(1) lookups. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/chain_index/tests/non_finalised_state.rs | 5 ++++- .../src/chain_index/tests/proptest_blockgen.rs | 11 ++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index 1d8b9d4b5..29dc6e8b4 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -89,7 +89,10 @@ async fn resolved_snapshot_serves_every_block() { .unwrap_or_else(|| panic!("no block served at height {}", height.0)); assert_eq!( - index_reader.get_block_height(&snapshot, hash).await.unwrap(), + index_reader + .get_block_height(&snapshot, hash) + .await + .unwrap(), Some(height), "get_block_height round-trip at height {}", height.0, diff --git a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs index 5e5e01fe5..407f344ee 100644 --- a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs +++ b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs @@ -470,11 +470,15 @@ fn fold_commitment_roots<'a>( for transaction in &block.transactions { for sap in transaction.sapling_note_commitments() { let node = sapling_crypto::Node::from_bytes(sap.to_bytes()).unwrap(); - sapling.get_or_insert_with(SaplingFrontier::empty).append(node); + sapling + .get_or_insert_with(SaplingFrontier::empty) + .append(node); } for orc in transaction.orchard_note_commitments() { let node = zebra_chain::orchard::tree::Node::from(*orc); - orchard.get_or_insert_with(OrchardFrontier::empty).append(node); + orchard + .get_or_insert_with(OrchardFrontier::empty) + .append(node); } } map.insert( @@ -534,7 +538,8 @@ struct ProptestMockchain { /// incremental pass. The finalized DB requires real roots (it rejects /// `None`), and recomputing them by folding from genesis on every call was /// O(N) crypto per call → O(N²) across a sync; this makes lookups O(1). - commitment_roots_cache: Arc>>, + commitment_roots_cache: + Arc>>, } impl ProptestMockchain { From e871653439ea8d09f4431ce9245b2eee9000bb41 Mon Sep 17 00:00:00 2001 From: zancas Date: Wed, 27 May 2026 01:10:31 -0700 Subject: [PATCH 12/31] zaino-state backends: serve reads from the always-present NFS, not only when Resolved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LightWallet (fetch.rs) and State (state.rs) backends guarded chain-height reads on `resolved_nfs_snapshot()`, returning `UnavailableNotSyncedEnough` (or, in get_latest_block, matching `SnapshotAvailability`) while the snapshot was Provisional. The NFS is always present and validator-sourced (#1096), so read its `best_tip` directly via `get_nfs_snapshot()`: - state.rs: get_block_range, get_block_count, get_latest_block, get_block (Ok(None) and Err branches), and get_mempool_stream no longer error during Provisional. - fetch.rs: get_latest_block drops its `SnapshotAvailability` match; the six `resolved_nfs_snapshot()` guards become `get_nfs_snapshot()`; the now-unused `SnapshotAvailability` import is removed. - get_chain_tips (both backends) keeps `resolved_nfs_snapshot()` — it has a legitimate fallback to the RPC client when Provisional. zaino-testutils: the three `PollableTip::tip_height` impls return 0 on error instead of `.expect()`-panicking, so startup polling tolerates a not-yet-ready backend rather than crashing. Co-Authored-By: Claude Opus 4.7 (1M context) --- integration-tests/zaino-testutils/src/lib.rs | 27 ++++--- packages/zaino-state/src/backends/fetch.rs | 82 ++------------------ packages/zaino-state/src/backends/state.rs | 50 +++--------- 3 files changed, 33 insertions(+), 126 deletions(-) diff --git a/integration-tests/zaino-testutils/src/lib.rs b/integration-tests/zaino-testutils/src/lib.rs index 81bd57bc8..365204e99 100644 --- a/integration-tests/zaino-testutils/src/lib.rs +++ b/integration-tests/zaino-testutils/src/lib.rs @@ -117,10 +117,12 @@ pub trait PollableTip: Status + Sync { impl PollableTip for FetchServiceSubscriber { async fn tip_height(&self) -> u64 { + // A not-yet-ready backend reports tip 0 (poll keeps waiting) rather + // than panicking. self.get_latest_block() .await - .expect("PollableTip: FetchServiceSubscriber::get_latest_block failed") - .height + .map(|block| block.height) + .unwrap_or(0) } } @@ -128,23 +130,20 @@ impl PollableTip for StateServiceSubscriber { async fn tip_height(&self) -> u64 { self.get_latest_block() .await - .expect("PollableTip: StateServiceSubscriber::get_latest_block failed") - .height + .map(|block| block.height) + .unwrap_or(0) } } impl PollableTip for NodeBackedChainIndexSubscriber { async fn tip_height(&self) -> u64 { - let snapshot = self - .snapshot_nonfinalized_state() - .await - .expect("PollableTip: chain-index snapshot_nonfinalized_state failed"); - u64::from(u32::from( - self.best_chaintip(&snapshot) - .await - .expect("PollableTip: chain-index best_chaintip failed") - .height, - )) + let Ok(snapshot) = self.snapshot_nonfinalized_state().await else { + return 0; + }; + match self.best_chaintip(&snapshot).await { + Ok(tip) => u64::from(u32::from(tip.height)), + Err(_) => 0, + } } } diff --git a/packages/zaino-state/src/backends/fetch.rs b/packages/zaino-state/src/backends/fetch.rs index 843bdcac5..69545248e 100644 --- a/packages/zaino-state/src/backends/fetch.rs +++ b/packages/zaino-state/src/backends/fetch.rs @@ -74,10 +74,7 @@ use crate::{ utils::{get_build_info, ServiceMetadata}, BackendType, }; -use crate::{ - chain_index::non_finalised_state::SnapshotAvailability, ChainIndex, NodeBackedChainIndex, - NodeBackedChainIndexSubscriber, -}; +use crate::{ChainIndex, NodeBackedChainIndex, NodeBackedChainIndexSubscriber}; /// Chain fetch service backed by Zcashd's JsonRPC engine. /// @@ -886,16 +883,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { /// Return the height of the tip of the best chain async fn get_latest_block(&self) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - match snapshot.availability() { - SnapshotAvailability::Resolved => Ok(snapshot.get_nfs_snapshot().best_tip.to_wire()), - SnapshotAvailability::Provisional => { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of reporting - // the genesis block - Err(FetchServiceError::UnavailableNotSyncedEnough) - } - } - // dbg!(&tip); + Ok(snapshot.get_nfs_snapshot().best_tip.to_wire()) } /// Return the compact block corresponding to the given block identifier @@ -926,12 +914,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { } }; - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - return Err(FetchServiceError::UnavailableNotSyncedEnough); - }; + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); match self .indexer @@ -1002,12 +985,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { } } }; - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - return Err(FetchServiceError::UnavailableNotSyncedEnough); - }; + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); match self .indexer .get_compact_block(&snapshot, types::Height(height), PoolTypeFilter::default()) @@ -1099,20 +1077,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { let timeout_result = timeout( time::Duration::from_secs((service_timeout * 4) as u64), async { - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - if let Err(e) = channel_tx - .send(Err(tonic::Status::failed_precondition( - "zaino not yet synced".to_string(), - ))) - .await - { - warn!("GetBlockRange channel closed unexpectedly: {}", e); - }; - return; - }; + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); // Use the snapshot tip directly, as this function doesn't support passthrough let chain_height = non_finalized_snapshot.best_tip.height.0; @@ -1232,20 +1197,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { let timeout_result = timeout( time::Duration::from_secs((service_timeout * 4) as u64), async { - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - if let Err(e) = channel_tx - .send(Err(tonic::Status::failed_precondition( - "zaino not yet synced".to_string(), - ))) - .await - { - warn!("GetBlockRangeNullifiers channel closed unexpectedly: {}", e); - }; - return; - }; + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); // Use the snapshot tip directly, as this function doesn't support passthrough let chain_height = non_finalized_snapshot.best_tip.height.0; @@ -1369,12 +1321,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { // Zebra returns None for mempool transactions, convert to `Mempool Height`. None => { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - return Err(FetchServiceError::UnavailableNotSyncedEnough); - }; + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); non_finalized_snapshot.best_tip.height.0 as u64 } }; @@ -1725,20 +1672,7 @@ impl LightWalletIndexer for FetchServiceSubscriber { let timeout = timeout( time::Duration::from_secs((service_timeout * 6) as u64), async { - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - if let Err(e) = channel_tx - .send(Err(tonic::Status::failed_precondition( - "zaino not yet synced".to_string(), - ))) - .await - { - warn!("GetMempoolStream channel closed unexpectedly: {}", e); - }; - return; - }; + let non_finalized_snapshot = snapshot.get_nfs_snapshot(); let mempool_height = non_finalized_snapshot.best_tip.height.0; match indexer.get_mempool_stream(None) { Some(mut mempool_stream) => { diff --git a/packages/zaino-state/src/backends/state.rs b/packages/zaino-state/src/backends/state.rs index 7a9f819db..ac282f942 100644 --- a/packages/zaino-state/src/backends/state.rs +++ b/packages/zaino-state/src/backends/state.rs @@ -568,9 +568,8 @@ impl StateServiceSubscriber { let timeout_result = timeout( time::Duration::from_secs((service_timeout * 4) as u64), async { - // This method does not support passthrough. Just return. - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else {return}; - let chain_height = non_finalized_snapshot.best_tip.height.0; + let nfs_snapshot = snapshot.get_nfs_snapshot(); + let chain_height = nfs_snapshot.best_tip.height.0; match state_service_clone .indexer @@ -1478,13 +1477,8 @@ impl ZcashIndexer for StateServiceSubscriber { /// tags: blockchain async fn get_block_count(&self) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - return Err(StateServiceError::UnavailableNotSyncedEnough); - }; - let h = non_finalized_snapshot.best_tip.height; + let nfs_snapshot = snapshot.get_nfs_snapshot(); + let h = nfs_snapshot.best_tip.height; Ok(h.into()) } @@ -1877,13 +1871,8 @@ impl LightWalletIndexer for StateServiceSubscriber { /// Return the height of the tip of the best chain async fn get_latest_block(&self) -> Result { let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - return Err(StateServiceError::UnavailableNotSyncedEnough); - }; - Ok(non_finalized_snapshot.best_tip.to_wire()) + let nfs_snapshot = snapshot.get_nfs_snapshot(); + Ok(nfs_snapshot.best_tip.to_wire()) } /// Return the compact block corresponding to the given block identifier @@ -1918,13 +1907,8 @@ impl LightWalletIndexer for StateServiceSubscriber { { Ok(Some(block)) => Ok(block), Ok(None) => { - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - return Err(StateServiceError::UnavailableNotSyncedEnough); - }; - let chain_height = non_finalized_snapshot.best_tip.height.0; + let nfs_snapshot = snapshot.get_nfs_snapshot(); + let chain_height = nfs_snapshot.best_tip.height.0; match hash_or_height { HashOrHeight::Height(Height(height)) if height >= chain_height => Err( StateServiceError::TonicStatusError(tonic::Status::out_of_range(format!( @@ -1938,13 +1922,8 @@ impl LightWalletIndexer for StateServiceSubscriber { } } Err(e) => { - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - return Err(StateServiceError::UnavailableNotSyncedEnough); - }; - let chain_height = non_finalized_snapshot.best_tip.height.0; + let nfs_snapshot = snapshot.get_nfs_snapshot(); + let chain_height = nfs_snapshot.best_tip.height.0; match hash_or_height { HashOrHeight::Height(Height(height)) if height >= chain_height => Err( StateServiceError::TonicStatusError(tonic::Status::out_of_range(format!( @@ -2372,13 +2351,8 @@ impl LightWalletIndexer for StateServiceSubscriber { let (channel_tx, channel_rx) = mpsc::channel(self.config.common.service.channel_size as usize); let snapshot = self.indexer.snapshot_nonfinalized_state().await?; - let Some(non_finalized_snapshot) = snapshot.resolved_nfs_snapshot() else { - // TODO: This probably shouldn't be an error. - // this is an improvement over previous behaviour of - // acting as if we are only synced to the genesis block - return Err(StateServiceError::UnavailableNotSyncedEnough); - }; - let mempool_height = non_finalized_snapshot.best_tip.height.0; + let nfs_snapshot = snapshot.get_nfs_snapshot(); + let mempool_height = nfs_snapshot.best_tip.height.0; tokio::spawn(async move { let timeout = timeout( time::Duration::from_secs((service_timeout * 6) as u64), From 6721178fecfcd49bc7b077e2c7c9acb3fa0394ce Mon Sep 17 00:00:00 2001 From: zancas Date: Wed, 27 May 2026 09:11:53 -0700 Subject: [PATCH 13/31] make test should_panic --- .../chain_index/tests/non_finalised_state.rs | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index 339920e52..5820e1af5 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -20,13 +20,14 @@ //! omitted: that variant is being eliminated, and pinning its shape //! would create immediate test churn at the refactor PR. //! -//! The one exception is the trailing **red driver** for #1096 -//! (`best_chaintip_derives_tip_from_nfs_snapshot_not_validator_passthrough`). +//! The one exception is the trailing **bug-characterization tripwire** for +//! #1096 (`best_chaintip_derives_tip_from_nfs_snapshot_not_validator_passthrough`). //! Unlike the characterization tests above — which pin behavior that must -//! survive the refactor unchanged — that test is *failing on purpose* and is -//! expected to be rewritten when the still-syncing variant is removed. It -//! pins cold-start shape precisely because it is driving that variant's -//! elimination, so the churn it incurs is the point, not an accident. +//! survive the refactor unchanged — that test is `#[should_panic]`: it asserts +//! the *target* invariant and documents that the assertion does not hold today, +//! because the cold-start variant passes through to the validator. It passes +//! now and goes red the moment #1096 removes the still-syncing variant, which +//! is the signal to rewrite it against `snapshot_nonfinalized_state()`. use super::{load_test_vectors_and_sync_chain_index, poll::poll_until, MockchainMode}; use crate::chain_index::non_finalised_state::ChainIndexSnapshot; @@ -296,14 +297,22 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi ); } -/// **Red driver for #1096** (NOT a surviving characterization test — see the -/// module-level doc; this one is *failing on purpose* and is expected to be -/// rewritten when the still-syncing variant is removed). +/// **Bug-characterization tripwire for #1096** (NOT a surviving +/// characterization test — see the module-level doc; this one is +/// `#[should_panic]` and is expected to be rewritten when the still-syncing +/// variant is removed). /// /// Target invariant: `best_chaintip` must derive the chain tip from the /// non-finalized snapshot in *every* availability state — it must never fall /// back to a validator passthrough. /// +/// The body asserts that invariant directly. It does not hold today, so the +/// assertion panics — hence `#[should_panic]`: the test is green now precisely +/// *because* the cold-start variant is still broken. When #1096 removes the +/// still-syncing variant, `best_chaintip` reports the real tip, the assertion +/// passes, the expected panic never fires, and this test goes red — the signal +/// to rewrite it against a `snapshot_nonfinalized_state()` result. +/// /// Today the lazy design hands out /// [`ChainIndexSnapshot::StillSyncingFinalizedState`] during the cold-start /// window, before the NFS slot is populated. In that variant `best_chaintip` @@ -321,11 +330,7 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi /// multi_thread: depends on the harness's background sync loop advancing the /// NFS concurrently with the setup's poll-until-ready loop. #[tokio::test(flavor = "multi_thread")] -#[ignore = "red driver for #1096: fails on purpose against today's still-syncing \ - variant, which passes through to the validator and reports the \ - finalized floor instead of the tip. Pins the target invariant to \ - drive that variant's elimination; rewritten against \ - snapshot_nonfinalized_state() once #1096 lands."] +#[should_panic(expected = "passes through to the validator and reports the finalized floor")] async fn best_chaintip_derives_tip_from_nfs_snapshot_not_validator_passthrough() { let (_blocks, _indexer, index_reader, mockchain) = load_test_vectors_and_sync_chain_index(MockchainMode::Active).await; From c3a2f95ba6bee8e1bd730110b63ee0bc169e2464 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Wed, 10 Jun 2026 14:41:07 -0300 Subject: [PATCH 14/31] publicize new types now used in public API --- packages/zaino-state/src/chain_index.rs | 40 ++++++++++--------- .../src/chain_index/non_finalised_state.rs | 2 +- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index d45a8123e..2490c5fe4 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -175,30 +175,36 @@ fn branch_len_to_active_chain( /// Interim shape: once the Availability/Resolved work lands, a resolved NFS /// block promotes to `IndexedBlock` (via `into_indexed`), and these methods /// return `IndexedBlock` directly — see issue #1096. -pub(crate) enum ChainBlock { - Finalized(IndexedBlock), - NonFinalized(ProvisionalBlock), +pub enum ChainBlock { + /// We have indexed all blocks from this block down to the genesis block. + /// We know the chain's entire cumulative chainwork as of this block. + Reified(IndexedBlock), + /// We have not indexed some blocks below this block. + /// We know the sum of chainwork from this block down to the finalized tip, + /// which is sufficient for determining the best chain, but do not know + /// the absolute cumulative chainwork. + Provisional(ProvisionalBlock), } impl ChainBlock { pub(crate) fn hash(&self) -> &types::BlockHash { match self { - ChainBlock::Finalized(block) => block.hash(), - ChainBlock::NonFinalized(block) => block.hash(), + ChainBlock::Reified(block) => block.hash(), + ChainBlock::Provisional(block) => block.hash(), } } pub(crate) fn height(&self) -> types::Height { match self { - ChainBlock::Finalized(block) => block.height(), - ChainBlock::NonFinalized(block) => block.height(), + ChainBlock::Reified(block) => block.height(), + ChainBlock::Provisional(block) => block.height(), } } pub(crate) fn data(&self) -> &types::db::legacy::BlockData { match self { - ChainBlock::Finalized(block) => block.data(), - ChainBlock::NonFinalized(block) => block.data(), + ChainBlock::Reified(block) => block.data(), + ChainBlock::Provisional(block) => block.data(), } } } @@ -370,7 +376,6 @@ pub trait ChainIndex { // Interim return type while the NFS holds provisional blocks: a finalized // hit is an `IndexedBlock`, a non-finalized hit a `ProvisionalBlock`. Once // the FS is fully synced (Resolved), these resolve to `IndexedBlock` (#1096). - #[allow(private_interfaces)] fn get_indexed_block_by_hash( &self, snapshot: &Self::Snapshot, @@ -383,7 +388,6 @@ pub trait ChainIndex { /// /// **NOTE: This Method is currently not "passthrough aware", cumulative /// chain work must be made optional to enable this.** - #[allow(private_interfaces)] fn get_indexed_block_by_height( &self, snapshot: &Self::Snapshot, @@ -1259,12 +1263,12 @@ impl NodeBackedChainIndexSubscriber { None => None, } .into_iter() - .map(ChainBlock::Finalized); + .map(ChainBlock::Reified); let non_finalized_blocks_containing_transaction = snapshot.blocks.values().filter_map(move |block| { block.transactions().iter().find_map(|transaction| { if transaction.txid().0 == txid { - Some(ChainBlock::NonFinalized(block.clone())) + Some(ChainBlock::Provisional(block.clone())) } else { None } @@ -1472,13 +1476,13 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result, Self::Error> { match snapshot.get_chainblock_by_hash(target_hash) { - Some(block) => Ok(Some(ChainBlock::NonFinalized(block.clone()))), + Some(block) => Ok(Some(ChainBlock::Provisional(block.clone()))), None => match self.get_block_height(snapshot, *target_hash).await { Ok(Some(height)) => Ok(self .finalized_state .get_chain_block_by_height(height) .await? - .map(ChainBlock::Finalized)), + .map(ChainBlock::Reified)), Ok(None) => Ok(None), Err(e) => Err(e), }, @@ -1497,12 +1501,12 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Result, Self::Error> { match snapshot.get_chainblock_by_height(target_height) { - Some(block) => Ok(Some(ChainBlock::NonFinalized(block.clone()))), + Some(block) => Ok(Some(ChainBlock::Provisional(block.clone()))), None => Ok(self .finalized_state .get_chain_block_by_height(*target_height) .await? - .map(ChainBlock::Finalized)), + .map(ChainBlock::Reified)), } } @@ -2626,7 +2630,7 @@ where } /// A snapshot of the non-finalized state, for consistent queries -pub(crate) trait NonFinalizedSnapshot { +pub trait NonFinalizedSnapshot { /// Hash -> block fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock>; /// Height -> block diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index d141d8bfb..d367e5331 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -157,7 +157,7 @@ impl ProvisionalCumulativeWork { /// [`IndexedBlock`]'s absolute `chainwork` field, so the two cannot be /// misattributed. #[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct ProvisionalBlock { +pub struct ProvisionalBlock { /// Height + hash of this block. index: BlockIndex, /// Parent block hash as *claimed by this block's header*. From fad9f7b4d4623fda2c4f71ab72786a31e8241dec Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Wed, 10 Jun 2026 16:43:19 -0300 Subject: [PATCH 15/31] publicize some fields of ProvisionalBlock, update changelog --- packages/zaino-state/CHANGELOG.md | 13 +++++++++++++ .../src/chain_index/non_finalised_state.rs | 12 ++++++------ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/zaino-state/CHANGELOG.md b/packages/zaino-state/CHANGELOG.md index 50322b6d5..243071665 100644 --- a/packages/zaino-state/CHANGELOG.md +++ b/packages/zaino-state/CHANGELOG.md @@ -5,6 +5,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this library adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased not landing in 0.4.0] +### Added +- `chain_index::ChainBlock` +- `chain_index::non_finalized_state::ProvisionalBlock`, + +### Changed +- `chain_index::ChainIndex` methods now return `ChainBlock`s in + place of `IndexedBlock`s. ChainBlock enum may not contain ChainWork. +- `chain_index::NonFinalizedSnapshot` trait methods now return +- `ProvisionalBlock`s instead of `ChainBlocks`. TODO: Before release, + this will return `ChainBlock`s, where they are reified as `IndexedBlocks` + after sync completes + ## [Unreleased] ### Added diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index d367e5331..7112dead2 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -159,23 +159,23 @@ impl ProvisionalCumulativeWork { #[derive(Clone, Debug, PartialEq, Eq)] pub struct ProvisionalBlock { /// Height + hash of this block. - index: BlockIndex, + pub index: BlockIndex, /// Parent block hash as *claimed by this block's header*. /// /// UNTRUSTED while the snapshot is provisional: the linkage it asserts is /// not validated against the finalized chain until the seam connects, so /// it must not be used for trusted chain-walks (e.g. fork-point recursion) /// during the provisional stage. - parent_hash: BlockHash, + pub parent_hash: BlockHash, /// Cumulative work relative to the seam, header-derived. Never absolute — /// the type enforces that (see [`ProvisionalCumulativeWork`]). - provisional_cumulative_work: ProvisionalCumulativeWork, + pub(crate) provisional_cumulative_work: ProvisionalCumulativeWork, /// Header and auxiliary block data. - data: BlockData, + pub data: BlockData, /// Compact representations of the block's transactions. - transactions: Vec, + pub transactions: Vec, /// Commitment tree data for the chain after this block is applied. - commitment_tree_data: CommitmentTreeData, + pub commitment_tree_data: CommitmentTreeData, } impl ProvisionalBlock { From cc2c9e3e402c470703f4e18450a35a624fafacc1 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Thu, 11 Jun 2026 14:00:43 -0300 Subject: [PATCH 16/31] remove unneeded attributes --- packages/zaino-state/src/chain_index.rs | 1 - packages/zaino-state/src/chain_index/non_finalised_state.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index ab65d3fac..185365496 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -715,7 +715,6 @@ pub trait ChainIndex { /// - Snapshot-based consistency for queries #[derive(Debug)] pub struct NodeBackedChainIndex { - #[allow(dead_code)] mempool: std::sync::Arc>, non_finalized_state: Arc>, finalized_db: std::sync::Arc, diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 80a5e1112..6f2168a49 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -49,7 +49,6 @@ pub struct NonFinalizedState { /// finalized DB has reached its seam) is its [`SnapshotAvailability`]: while /// `Provisional`, reads that need finalized data pass through to the validator. pub struct ChainIndexSnapshot { - #[allow(private_interfaces)] non_finalized_snapshot: Arc, } From abf29d1b2bf0173ff5b80f466f99c7187422e776 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Thu, 11 Jun 2026 14:23:42 -0300 Subject: [PATCH 17/31] remove old attribute --- packages/zaino-state/src/chain_index.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index 185365496..a90ae3836 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -341,12 +341,6 @@ impl ChainBlock { /// /// When a call asks for info (e.g. a block), Zaino selects sources in this order: #[doc = simple_mermaid::mermaid!("chain_index_passthrough.mmd")] -// Interim (#1096): this pub trait's surface references in-crate types — the -// `Snapshot: NonFinalizedSnapshot` bound and the `ChainBlock` returned by -// `get_indexed_block_by_*` are `pub(crate)`. Keeping those types in-crate is -// the intended minimum visibility; the lint resolves when the resolution -// promotion turns `ChainBlock` into `IndexedBlock`. -#[allow(private_interfaces, private_bounds)] pub trait ChainIndex { /// A snapshot of the nonfinalized state, needed for atomic access type Snapshot: NonFinalizedSnapshot; From 55fd703f1c4014f80c56456dfc43b970e49d0c2a Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Thu, 11 Jun 2026 14:32:34 -0300 Subject: [PATCH 18/31] add cfg test --- packages/zaino-state/src/chain_index/finalised_state.rs | 1 + packages/zaino-state/src/chain_index/source.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/zaino-state/src/chain_index/finalised_state.rs b/packages/zaino-state/src/chain_index/finalised_state.rs index 2d874e9b0..ad4ec6b05 100644 --- a/packages/zaino-state/src/chain_index/finalised_state.rs +++ b/packages/zaino-state/src/chain_index/finalised_state.rs @@ -513,6 +513,7 @@ impl ZainoDB { // finalized DB may be held below the chain's finalization ceiling, // which — since the non-finalized state leads — holds the snapshot // Provisional with a deterministic catch-up gap. + #[cfg(test)] let height = match source.finalized_sync_cap() { Some(cap) => height.min(cap), None => height, diff --git a/packages/zaino-state/src/chain_index/source.rs b/packages/zaino-state/src/chain_index/source.rs index 6335ad176..3c52075f4 100644 --- a/packages/zaino-state/src/chain_index/source.rs +++ b/packages/zaino-state/src/chain_index/source.rs @@ -233,6 +233,7 @@ pub trait BlockchainSource: Clone + Send + Sync + 'static { /// non-finalized state is validator-sourced and *leads*, capping the /// finalized DB deterministically holds the snapshot Provisional with a /// known catch-up gap — without slowing any source call. + #[cfg(test)] fn finalized_sync_cap(&self) -> Option { None } From fffbff23868c167649db03acdbb17cd5eeddc0a1 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Thu, 11 Jun 2026 15:38:11 -0300 Subject: [PATCH 19/31] calculate provisional chainwork on demand --- .../src/chain_index/non_finalised_state.rs | 89 ++++++++----------- .../chain_index/tests/proptest_blockgen.rs | 4 +- 2 files changed, 37 insertions(+), 56 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 6f2168a49..b3bd81a86 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -1,7 +1,10 @@ use super::{finalised_state::ZainoDB, source::BlockchainSource, NON_FINALIZED_DEPTH}; -use crate::chain_index::types::db::{ - legacy::{compact_block_from_parts, BlockData, CompactTxData}, - CommitmentTreeData, +use crate::chain_index::{ + types::db::{ + legacy::{compact_block_from_parts, BlockData, CompactTxData}, + CommitmentTreeData, + }, + NonFinalizedSnapshot, }; use crate::{ chain_index::types::{ @@ -193,9 +196,8 @@ pub struct ProvisionalBlock { /// it must not be used for trusted chain-walks (e.g. fork-point recursion) /// during the provisional stage. pub parent_hash: BlockHash, - /// Cumulative work relative to the seam, header-derived. Never absolute — - /// the type enforces that (see [`ProvisionalCumulativeWork`]). - pub(crate) provisional_cumulative_work: ProvisionalCumulativeWork, + /// This block's individual chainwork + chainwork: ChainWork, /// Header and auxiliary block data. pub data: BlockData, /// Compact representations of the block's transactions. @@ -223,8 +225,17 @@ impl ProvisionalBlock { /// Cumulative work relative to the seam. Use this — never an absolute /// chainwork — for best-tip selection within the non-finalized window. - pub(crate) fn provisional_cumulative_work(&self) -> &ProvisionalCumulativeWork { - &self.provisional_cumulative_work + pub(crate) fn provisional_cumulative_work( + &self, + snapshot: &NonfinalizedBlockCacheSnapshot, + ) -> ProvisionalCumulativeWork { + let mut work = ProvisionalCumulativeWork::seam().add_block_work(&self.chainwork); + let mut parent_hash = self.parent_hash; + while let Some(prev_block) = snapshot.get_chainblock_by_hash(&parent_hash) { + work = work.add_block_work(&prev_block.chainwork); + parent_hash = prev_block.parent_hash; + } + work } /// The compact transactions in this block. @@ -265,7 +276,7 @@ impl ProvisionalBlock { Self { index: indexed.context.index, parent_hash: indexed.context.parent_hash, - provisional_cumulative_work: ProvisionalCumulativeWork::seam(), + chainwork: ChainWork::from_u256(0.into()), data: indexed.data, transactions: indexed.transactions, commitment_tree_data: indexed.commitment_tree_data, @@ -609,26 +620,7 @@ impl NonFinalizedState { let parent_hash = BlockHash::from(block.header.previous_block_hash); if parent_hash == working_snapshot.best_tip.hash { // Normal chain progression - let parent_work = { - let prev_block = working_snapshot - .blocks - .get(&working_snapshot.best_tip.hash) - .ok_or_else(|| { - SyncError::ReorgFailure(format!( - "found blocks {:?}, expected block {:?}", - working_snapshot - .blocks - .values() - .map(|block| (*block.hash(), block.height())) - .collect::>(), - working_snapshot.best_tip - )) - })?; - *prev_block.provisional_cumulative_work() - }; - let chainblock = self - .block_to_provisional_block(&parent_work, &block) - .await?; + let chainblock = self.block_to_provisional_block(&block).await?; info!( height = (working_snapshot.best_tip.height + 1).0, hash = %chainblock.hash(), @@ -667,7 +659,7 @@ impl NonFinalizedState { working_snapshot: &mut NonfinalizedBlockCacheSnapshot, block: &impl Block, ) -> Result { - let prev_block = match working_snapshot + match working_snapshot .get_block_by_hash_bytes_in_serialized_order(block.prev_hash_bytes_serialized_order()) .cloned() { @@ -704,8 +696,7 @@ impl NonFinalizedState { Box::pin(self.handle_reorg(working_snapshot, &*prev_block)).await? } }; - let parent_work = *prev_block.provisional_cumulative_work(); - let provisional_block = block.to_provisional_block(&parent_work, self).await?; + let provisional_block = block.to_provisional_block(self).await?; working_snapshot.add_block_new_chaintip(provisional_block.clone()); Ok(provisional_block) } @@ -780,7 +771,7 @@ impl NonFinalizedState { let best_block = &new_snapshot .blocks .values() - .max_by_key(|block| block.provisional_cumulative_work()) + .max_by_key(|block| block.provisional_cumulative_work(&new_snapshot)) .cloned() .expect("empty snapshot impossible"); self.handle_reorg(&mut new_snapshot, best_block) @@ -860,7 +851,6 @@ impl NonFinalizedState { /// block: no absolute chainwork is computed or required. async fn block_to_provisional_block( &self, - parent_work: &ProvisionalCumulativeWork, block: &zebra_chain::block::Block, ) -> Result { let tree_roots = self @@ -872,12 +862,11 @@ impl NonFinalizedState { )) })?; - Self::provisional_block_from_parts(block, &tree_roots, parent_work, self.network.clone()) - .map_err(|e| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( - Box::new(InvalidData(e)), - )) - }) + Self::provisional_block_from_parts(block, &tree_roots, self.network.clone()).map_err(|e| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( + InvalidData(e), + ))) + }) } /// Assemble a [`ProvisionalBlock`] from already-fetched parts. Reuses the @@ -888,7 +877,6 @@ impl NonFinalizedState { fn provisional_block_from_parts( block: &zebra_chain::block::Block, tree_roots: &TreeRootData, - parent_work: &ProvisionalCumulativeWork, network: Network, ) -> Result { let (sapling_root, sapling_size, orchard_root, orchard_size) = @@ -911,8 +899,7 @@ impl NonFinalizedState { let data = block_with_metadata.extract_block_data()?; let transactions = block_with_metadata.extract_transactions()?; let commitment_tree_data = block_with_metadata.create_commitment_tree_data(); - let provisional_cumulative_work = - parent_work.add_block_work(&block_with_metadata.block_work()?); + let chainwork = block_with_metadata.block_work()?; let hash = BlockHash::from(block.hash()); let parent_hash = BlockHash::from(block.header.previous_block_hash); @@ -924,7 +911,7 @@ impl NonFinalizedState { Ok(ProvisionalBlock { index: BlockIndex { height, hash }, parent_hash, - provisional_cumulative_work, + chainwork, data, transactions, commitment_tree_data, @@ -1021,7 +1008,7 @@ impl NonFinalizedState { working_snapshot: &mut NonfinalizedBlockCacheSnapshot, block: &impl Block, ) -> Result { - let prev_block = match working_snapshot + match working_snapshot .get_block_by_hash_bytes_in_serialized_order(block.prev_hash_bytes_serialized_order()) .cloned() { @@ -1048,8 +1035,7 @@ impl NonFinalizedState { Box::pin(self.add_nonbest_block(working_snapshot, &*prev_block)).await? } }; - let parent_work = *prev_block.provisional_cumulative_work(); - let provisional_block = block.to_provisional_block(&parent_work, self).await?; + let provisional_block = block.to_provisional_block(self).await?; working_snapshot .blocks .insert(*provisional_block.hash(), provisional_block.clone()); @@ -1079,12 +1065,9 @@ pub enum UpdateError { trait Block { fn hash_bytes_serialized_order(&self) -> [u8; 32]; fn prev_hash_bytes_serialized_order(&self) -> [u8; 32]; - /// Produce the [`ProvisionalBlock`] for this block, accumulating its work - /// onto `parent_work` (relative to the seam). Replaces the former - /// `to_indexed_block`: the NFS holds provisional, not indexed, blocks. + /// Produce the [`ProvisionalBlock`] for this block. async fn to_provisional_block( &self, - parent_work: &ProvisionalCumulativeWork, nfs: &NonFinalizedState, ) -> Result; } @@ -1100,7 +1083,6 @@ impl Block for ProvisionalBlock { async fn to_provisional_block( &self, - _parent_work: &ProvisionalCumulativeWork, _nfs: &NonFinalizedState, ) -> Result { // Identity: a block already in the snapshot keeps the relative work it @@ -1119,9 +1101,8 @@ impl Block for zebra_chain::block::Block { async fn to_provisional_block( &self, - parent_work: &ProvisionalCumulativeWork, nfs: &NonFinalizedState, ) -> Result { - nfs.block_to_provisional_block(parent_work, self).await + nfs.block_to_provisional_block(self).await } } diff --git a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs index 407f344ee..1b10e557b 100644 --- a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs +++ b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs @@ -427,8 +427,8 @@ fn make_chain() { for (hash, block) in &non_finalized_snapshot.blocks { if hash != &best_tip_hash { assert!( - block.provisional_cumulative_work() - <= best_tip_block.provisional_cumulative_work() + block.provisional_cumulative_work(non_finalized_snapshot) + <= best_tip_block.provisional_cumulative_work(non_finalized_snapshot) ); if non_finalized_snapshot.heights_to_hashes.get(&block.height()) == Some(block.hash()) { assert_eq!(index_reader.find_fork_point(&snapshot, hash).await.unwrap().unwrap().0, *hash); From 76661fe1c4d41db3f8c805d8657f790caa37884e Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Thu, 11 Jun 2026 16:35:19 -0300 Subject: [PATCH 20/31] initialize nfs from source instead of indexedblock, not yet error handled --- packages/zaino-state/src/chain_index.rs | 5 +- .../src/chain_index/non_finalised_state.rs | 263 +++++++++--------- 2 files changed, 136 insertions(+), 132 deletions(-) diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index a90ae3836..cf8abdbb5 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -808,9 +808,8 @@ impl NodeBackedChainIndex { // The non-finalized state is built eagerly so it always exists. It // seeds Provisional from the validator (genesis); the sync loop extends // it, and the finalized DB catching up to the seam flips it to Resolved. - let non_finalized_state = Arc::new( - crate::NonFinalizedState::initialize(source.clone(), network.clone(), None).await?, - ); + let non_finalized_state = + Arc::new(crate::NonFinalizedState::initialize(source.clone(), network.clone()).await?); let mut chain_index = Self { mempool: std::sync::Arc::new(mempool_state), diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index b3bd81a86..ec7b8ff83 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -1,5 +1,7 @@ use super::{finalised_state::ZainoDB, source::BlockchainSource, NON_FINALIZED_DEPTH}; use crate::chain_index::{ + finalization_ceiling, + source::{BlockchainSourceError, BlockchainSourceResult}, types::db::{ legacy::{compact_block_from_parts, BlockData, CompactTxData}, CommitmentTreeData, @@ -266,22 +268,6 @@ impl ProvisionalBlock { self.commitment_tree_data(), ) } - - /// Build the seam anchor from a finalized [`IndexedBlock`]. The anchor's - /// relative work is zero by definition ([`ProvisionalCumulativeWork::seam`]); - /// the block's absolute chainwork is intentionally dropped — the NFS tracks - /// work relative to this seam, and the absolute base is reattached only at - /// resolution. - pub(crate) fn from_indexed_seam(indexed: IndexedBlock) -> Self { - Self { - index: indexed.context.index, - parent_hash: indexed.context.parent_hash, - chainwork: ChainWork::from_u256(0.into()), - data: indexed.data, - transactions: indexed.transactions, - commitment_tree_data: indexed.commitment_tree_data, - } - } } #[derive(Debug)] @@ -388,26 +374,48 @@ impl BlockIndex { } impl NonfinalizedBlockCacheSnapshot { - /// Create initial snapshot from a single block - fn from_initial_block(block: IndexedBlock) -> Self { - let best_tip = BlockIndex::from_block(&block); - let hash = *block.hash(); - let height = best_tip.height; + async fn init_from_blockchain_source( + source: &impl BlockchainSource, + network: Network, + ) -> BlockchainSourceResult { + let tip_height = + source + .get_best_block_height() + .await? + .ok_or(BlockchainSourceError::Unrecoverable(String::from( + "source has no blocks", + )))?; + let start_height = finalization_ceiling(tip_height.0); + let start_block = source + .get_block(HashOrHeight::Height(start_height.into())) + .await? + .ok_or(BlockchainSourceError::Unrecoverable(String::from( + "source missing block 100 below tio", + )))?; + + let provisional_block = block_to_provisional_block(&start_block, source, network) + .await + .map_err(|e| todo!())?; let mut blocks = HashMap::new(); let mut heights_to_hashes = HashMap::new(); - blocks.insert(hash, ProvisionalBlock::from_indexed_seam(block)); - heights_to_hashes.insert(height, hash); + let block_index = BlockIndex { + height: provisional_block.height(), + hash: *provisional_block.hash(), + }; - Self { + blocks.insert(block_index.hash, provisional_block); + heights_to_hashes.insert(block_index.height, block_index.hash); + + Ok(Self { blocks, heights_to_hashes, - best_tip, + best_tip: block_index, // Newly seeded from the seam block: the finalized DB has not yet // caught up to it. `update` flips this to `Resolved` once it has. availability: SnapshotAvailability::Provisional, - } + }) } fn add_block_new_chaintip(&mut self, block: ProvisionalBlock) { @@ -448,19 +456,14 @@ impl NonFinalizedState { /// TODO: Currently, we can't initate without an snapshot, we need to create a cache /// of at least one block. Should this be tied to the instantiation of the data structure /// itself? - #[instrument(name = "NonFinalizedState::initialize", skip(source, start_block), fields(network = %network))] - pub async fn initialize( - source: Source, - network: Network, - start_block: Option, - ) -> Result { + #[instrument(name = "NonFinalizedState::initialize", skip(source), fields(network = %network))] + pub async fn initialize(source: Source, network: Network) -> Result { info!(network = %network, "Initializing non-finalized state"); - // Resolve the initial block (provided or genesis) - let initial_block = Self::resolve_initial_block(&source, &network, start_block).await?; - - // Create initial snapshot from the block - let snapshot = NonfinalizedBlockCacheSnapshot::from_initial_block(initial_block); + let snapshot = + NonfinalizedBlockCacheSnapshot::init_from_blockchain_source(&source, network.clone()) + .await + .map_err(|e| -> InitError { todo!() })?; // Set up optional listener let nfs_change_listener = Self::setup_listener(&source).await; @@ -583,7 +586,12 @@ impl NonFinalizedState { None => self.fetch_seam_indexed_block(anchor_height).await?, }; self.current.swap(Arc::new( - NonfinalizedBlockCacheSnapshot::from_initial_block(seam), + NonfinalizedBlockCacheSnapshot::init_from_blockchain_source( + &self.source, + self.network.clone(), + ) + .await + .expect("todo error handling"), )); initial_state = self.get_snapshot(); } @@ -620,7 +628,8 @@ impl NonFinalizedState { let parent_hash = BlockHash::from(block.header.previous_block_hash); if parent_hash == working_snapshot.best_tip.hash { // Normal chain progression - let chainblock = self.block_to_provisional_block(&block).await?; + let chainblock = + block_to_provisional_block(&block, &self.source, self.network.clone()).await?; info!( height = (working_snapshot.best_tip.height + 1).0, hash = %chainblock.hash(), @@ -845,93 +854,6 @@ impl NonFinalizedState { self.current.load_full() } - /// Build a [`ProvisionalBlock`] from a source block, accumulating its - /// header work onto the parent's relative total. Mirrors - /// [`Self::block_to_chainblock`] but produces a provisional (not indexed) - /// block: no absolute chainwork is computed or required. - async fn block_to_provisional_block( - &self, - block: &zebra_chain::block::Block, - ) -> Result { - let tree_roots = self - .get_tree_roots_from_source(block.hash().into()) - .await - .map_err(|e| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( - Box::new(InvalidData(format!("{}", e))), - )) - })?; - - Self::provisional_block_from_parts(block, &tree_roots, self.network.clone()).map_err(|e| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( - InvalidData(e), - ))) - }) - } - - /// Assemble a [`ProvisionalBlock`] from already-fetched parts. Reuses the - /// shared `BlockWithMetadata` extractors (`extract_block_data`, - /// `extract_transactions`, `create_commitment_tree_data`, `block_work`); - /// the only divergence from the indexed path is that work is accumulated - /// *relative to the seam* instead of into an absolute `BlockContext`. - fn provisional_block_from_parts( - block: &zebra_chain::block::Block, - tree_roots: &TreeRootData, - network: Network, - ) -> Result { - let (sapling_root, sapling_size, orchard_root, orchard_size) = - tree_roots.clone().extract_with_defaults(); - - // `parent_chainwork` is unused for a provisional block (it feeds only - // `create_block_context`, which we never call here); a zero placeholder - // keeps the throwaway `BlockMetadata` shape without touching absolute - // work. The relative total is tracked separately, below. - let metadata = BlockMetadata::new( - sapling_root, - sapling_size as u32, - orchard_root, - orchard_size as u32, - ChainWork::from_u256(U256::zero()), - network, - ); - let block_with_metadata = BlockWithMetadata::new(block, metadata); - - let data = block_with_metadata.extract_block_data()?; - let transactions = block_with_metadata.extract_transactions()?; - let commitment_tree_data = block_with_metadata.create_commitment_tree_data(); - let chainwork = block_with_metadata.block_work()?; - - let hash = BlockHash::from(block.hash()); - let parent_hash = BlockHash::from(block.header.previous_block_hash); - let height = block - .coinbase_height() - .map(|height| Height(height.0)) - .ok_or_else(|| String::from("Any valid block has a coinbase height"))?; - - Ok(ProvisionalBlock { - index: BlockIndex { height, hash }, - parent_hash, - chainwork, - data, - transactions, - commitment_tree_data, - }) - } - - /// Get commitment tree roots from the blockchain source - async fn get_tree_roots_from_source( - &self, - block_hash: BlockHash, - ) -> Result { - let (sapling_root_and_len, orchard_root_and_len) = - self.source.get_commitment_tree_roots(block_hash).await?; - - Ok(TreeRootData { - sapling: sapling_root_and_len, - orchard: orchard_root_and_len, - }) - } - /// Fetch the seam (finalization-ceiling) block from the source and build it /// as an [`IndexedBlock`] to anchor the NFS window while the finalized DB is /// still catching up. The absolute chainwork is a zero placeholder: @@ -955,8 +877,7 @@ impl NonFinalizedState { ))), )) })?; - let tree_roots = self - .get_tree_roots_from_source(block.hash().into()) + let tree_roots = get_tree_roots_from_source(block.hash().into(), &self.source) .await .map_err(|e| { SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( @@ -1043,6 +964,90 @@ impl NonFinalizedState { } } +/// Build a [`ProvisionalBlock`] from a source block, accumulating its +/// header work onto the parent's relative total. Mirrors +/// [`Self::block_to_chainblock`] but produces a provisional (not indexed) +/// block: no absolute chainwork is computed or required. +async fn block_to_provisional_block( + block: &zebra_chain::block::Block, + source: &impl BlockchainSource, + network: Network, +) -> Result { + let tree_roots = get_tree_roots_from_source(block.hash().into(), source) + .await + .map_err(|e| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( + InvalidData(format!("{}", e)), + ))) + })?; + + provisional_block_from_parts(block, &tree_roots, network).map_err(|e| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( + InvalidData(e), + ))) + }) +} +/// Assemble a [`ProvisionalBlock`] from already-fetched parts. Reuses the +/// shared `BlockWithMetadata` extractors (`extract_block_data`, +/// `extract_transactions`, `create_commitment_tree_data`, `block_work`); +/// the only divergence from the indexed path is that work is accumulated +/// *relative to the seam* instead of into an absolute `BlockContext`. +fn provisional_block_from_parts( + block: &zebra_chain::block::Block, + tree_roots: &TreeRootData, + network: Network, +) -> Result { + let (sapling_root, sapling_size, orchard_root, orchard_size) = + tree_roots.clone().extract_with_defaults(); + + // `parent_chainwork` is unused for a provisional block (it feeds only + // `create_block_context`, which we never call here); a zero placeholder + // keeps the throwaway `BlockMetadata` shape without touching absolute + // work. The relative total is tracked separately, below. + let metadata = BlockMetadata::new( + sapling_root, + sapling_size as u32, + orchard_root, + orchard_size as u32, + ChainWork::from_u256(U256::zero()), + network, + ); + let block_with_metadata = BlockWithMetadata::new(block, metadata); + + let data = block_with_metadata.extract_block_data()?; + let transactions = block_with_metadata.extract_transactions()?; + let commitment_tree_data = block_with_metadata.create_commitment_tree_data(); + let chainwork = block_with_metadata.block_work()?; + + let hash = BlockHash::from(block.hash()); + let parent_hash = BlockHash::from(block.header.previous_block_hash); + let height = block + .coinbase_height() + .map(|height| Height(height.0)) + .ok_or_else(|| String::from("Any valid block has a coinbase height"))?; + + Ok(ProvisionalBlock { + index: BlockIndex { height, hash }, + parent_hash, + chainwork, + data, + transactions, + commitment_tree_data, + }) +} +/// Get commitment tree roots from the blockchain source +async fn get_tree_roots_from_source( + block_hash: BlockHash, + source: &impl BlockchainSource, +) -> Result { + let (sapling_root_and_len, orchard_root_and_len) = + source.get_commitment_tree_roots(block_hash).await?; + + Ok(TreeRootData { + sapling: sapling_root_and_len, + orchard: orchard_root_and_len, + }) +} /// Errors that occur during a snapshot update pub enum UpdateError { /// The block reciever disconnected. This should only happen during shutdown. @@ -1103,6 +1108,6 @@ impl Block for zebra_chain::block::Block { &self, nfs: &NonFinalizedState, ) -> Result { - nfs.block_to_provisional_block(self).await + block_to_provisional_block(self, &nfs.source, nfs.network.clone()).await } } From 7335b4f51e95c21029d66bb9243a0405614a6f08 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Thu, 11 Jun 2026 17:21:20 -0300 Subject: [PATCH 21/31] remove now-unused code --- .../src/chain_index/non_finalised_state.rs | 70 ------------------- 1 file changed, 70 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index ec7b8ff83..7898cd22c 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -338,10 +338,6 @@ impl From for SyncError { } } -#[derive(thiserror::Error, Debug)] -#[error("Genesis block missing in validator")] -struct MissingGenesisBlock; - #[derive(thiserror::Error, Debug)] #[error("data from validator invalid: {0}")] struct InvalidData(String); @@ -363,16 +359,6 @@ pub enum InitError { InitalBlockMissingHeight, } -/// This is the core of the concurrent block cache. -impl BlockIndex { - /// Create a BlockID from an IndexedBlock - fn from_block(block: &IndexedBlock) -> Self { - let height = block.height(); - let hash = *block.hash(); - Self { height, hash } - } -} - impl NonfinalizedBlockCacheSnapshot { async fn init_from_blockchain_source( source: &impl BlockchainSource, @@ -476,62 +462,6 @@ impl NonFinalizedState { }) } - /// Fetch the genesis block and convert it to IndexedBlock - async fn get_genesis_indexed_block( - source: &Source, - network: &Network, - ) -> Result { - let genesis_block = source - .get_block(HashOrHeight::Height(zebra_chain::block::Height(0))) - .await - .map_err(|e| InitError::InvalidNodeData(Box::new(e)))? - .ok_or_else(|| InitError::InvalidNodeData(Box::new(MissingGenesisBlock)))?; - - let (sapling_root_and_len, orchard_root_and_len) = source - .get_commitment_tree_roots(genesis_block.hash().into()) - .await - .map_err(|e| InitError::InvalidNodeData(Box::new(e)))?; - - let tree_roots = TreeRootData { - sapling: sapling_root_and_len, - orchard: orchard_root_and_len, - }; - - // For genesis block, chainwork is just the block's own work (no previous blocks) - let genesis_work = ChainWork::from(U256::from( - genesis_block - .header - .difficulty_threshold - .to_work() - .ok_or_else(|| { - InitError::InvalidNodeData(Box::new(InvalidData( - "Invalid work field of genesis block".to_string(), - ))) - })? - .as_u128(), - )); - - Self::create_indexed_block_with_optional_roots( - genesis_block.as_ref(), - &tree_roots, - genesis_work, - network.clone(), - ) - .map_err(|e| InitError::InvalidNodeData(Box::new(InvalidData(e)))) - } - - /// Resolve the initial block - either use provided block or fetch genesis - async fn resolve_initial_block( - source: &Source, - network: &Network, - start_block: Option, - ) -> Result { - match start_block { - Some(block) => Ok(block), - None => Self::get_genesis_indexed_block(source, network).await, - } - } - /// Set up the optional non-finalized change listener async fn setup_listener( source: &Source, From b171a4ca07d7437bd0d523e79c85f6cf8df53e60 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Fri, 12 Jun 2026 14:40:33 -0300 Subject: [PATCH 22/31] some error handling plus start on reify fn --- packages/zaino-state/src/chain_index.rs | 4 +- .../src/chain_index/non_finalised_state.rs | 176 +++++++----------- 2 files changed, 71 insertions(+), 109 deletions(-) diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index cf8abdbb5..0f5206347 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -2056,7 +2056,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber Option, Self::Error>>> { let non_finalized_snapshot = match snapshot { Some(s) => match s.availability() { - SnapshotAvailability::Resolved => Some(s.get_nfs_snapshot()), + SnapshotAvailability::Reified => Some(s.get_nfs_snapshot()), // While Provisional the chain tip is newer than the snapshot's // tip, so there's no settled tip to anchor the stream to. SnapshotAvailability::Provisional => return None, @@ -2309,7 +2309,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber snapshot.get_nfs_snapshot(), + SnapshotAvailability::Reified => snapshot.get_nfs_snapshot(), SnapshotAvailability::Provisional => { // Accumulator invariants are not established until the finalised state catches // up. Match zcashd's "stats collection failed" empty-object shape. diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 7898cd22c..e01a000f2 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -1,7 +1,7 @@ use super::{finalised_state::ZainoDB, source::BlockchainSource, NON_FINALIZED_DEPTH}; use crate::chain_index::{ + finalised_state::capability::{CapabilityRequest, IndexedBlockExt}, finalization_ceiling, - source::{BlockchainSourceError, BlockchainSourceResult}, types::db::{ legacy::{compact_block_from_parts, BlockData, CompactTxData}, CommitmentTreeData, @@ -13,12 +13,15 @@ use crate::{ self, BlockHash, BlockIndex, BlockMetadata, BlockWithMetadata, Height, TreeRootData, }, error::FinalisedStateError, - ChainWork, IndexedBlock, + ChainWork, }; use arc_swap::ArcSwap; use futures::lock::Mutex; use primitive_types::U256; -use std::{collections::HashMap, sync::Arc}; +use std::{ + collections::{HashMap, HashSet}, + sync::Arc, +}; use tokio::sync::mpsc; use tracing::{info, instrument, warn}; use zebra_chain::{parameters::Network, serialization::BytesInDisplayOrder}; @@ -80,7 +83,7 @@ impl ChainIndexSnapshot { /// chain (the finalized DB reached the seam). While false, reads needing /// finalized data must pass through to the validator. pub(crate) fn is_resolved(&self) -> bool { - matches!(self.availability(), SnapshotAvailability::Resolved) + matches!(self.availability(), SnapshotAvailability::Reified) } /// The non-finalized snapshot, but only once it is `Resolved` (validated @@ -113,7 +116,7 @@ pub(crate) enum SnapshotAvailability { /// resolve window blocks' *absolute* chainwork = `base + relative` — is /// reattached by the resolution-promotion step, #1096, alongside its /// reader; it isn't carried yet.) - Resolved, + Reified, } #[derive(Debug, Clone)] @@ -354,34 +357,38 @@ pub enum InitError { #[error(transparent)] /// The finalized state failed to initialize FinalisedStateInitialzationError(#[from] FinalisedStateError), - /// the initial block provided was not on the best chain + /// The non-finalized state failed to initialize #[error("initial block not on best chain")] - InitalBlockMissingHeight, + NonFinalizedStateInitError(#[from] SyncError), } impl NonfinalizedBlockCacheSnapshot { async fn init_from_blockchain_source( source: &impl BlockchainSource, network: Network, - ) -> BlockchainSourceResult { - let tip_height = - source - .get_best_block_height() - .await? - .ok_or(BlockchainSourceError::Unrecoverable(String::from( - "source has no blocks", - )))?; + ) -> Result { + let missing_block_error = |target_block| { + SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( + MissingBlockError(format!("source missing block: {target_block}")), + ))) + }; + let tip_height = source + .get_best_block_height() + .await + .map_err(|e| SyncError::ErrorFromSource(Box::new(e)))? + .ok_or(missing_block_error("no chaintip"))?; let start_height = finalization_ceiling(tip_height.0); let start_block = source .get_block(HashOrHeight::Height(start_height.into())) - .await? - .ok_or(BlockchainSourceError::Unrecoverable(String::from( - "source missing block 100 below tio", - )))?; + .await + .map_err(|e| SyncError::ErrorFromSource(Box::new(e)))? + .ok_or(missing_block_error( + "100 blocks below self-reported chaintip height", + ))?; let provisional_block = block_to_provisional_block(&start_block, source, network) .await - .map_err(|e| todo!())?; + .map_err(|e| SyncError::ErrorFromSource(Box::new(e)))?; let mut blocks = HashMap::new(); let mut heights_to_hashes = HashMap::new(); @@ -448,8 +455,7 @@ impl NonFinalizedState { let snapshot = NonfinalizedBlockCacheSnapshot::init_from_blockchain_source(&source, network.clone()) - .await - .map_err(|e| -> InitError { todo!() })?; + .await?; // Set up optional listener let nfs_change_listener = Self::setup_listener(&source).await; @@ -507,14 +513,6 @@ impl NonFinalizedState { let anchor_height = super::finalization_ceiling(chain_height.0); let mut initial_state = self.get_snapshot(); if initial_state.best_tip.height < anchor_height { - let seam = match finalized_db - .to_reader() - .get_chain_block_by_height(anchor_height) - .await? - { - Some(indexed) => indexed, - None => self.fetch_seam_indexed_block(anchor_height).await?, - }; self.current.swap(Arc::new( NonfinalizedBlockCacheSnapshot::init_from_blockchain_source( &self.source, @@ -700,12 +698,23 @@ impl NonFinalizedState { .map_err(|_e| UpdateError::FinalizedStateCorruption)? .unwrap_or(Height(0)); - // The NFS holds only the non-finalized window. Trim to the seam (the - // finalization ceiling of the current tip), derived from the NFS's own - // best tip — *not* the finalized-DB tip, which lags since the NFS leads. - // This advances the window floor with the chain and evicts blocks that - // have fallen below the seam, independent of finalized-DB progress. - let seam = super::finalization_ceiling(new_snapshot.best_tip.height.0); + if new_snapshot + .heights_to_hashes + .contains_key(&finalized_height) + { + self.reify(&mut new_snapshot, finalized_db.as_ref()); + } + + let seam = match new_snapshot.availability { + SnapshotAvailability::Provisional => { + // While provisional, we keep the blocks we know to be finalized + super::finalization_ceiling(new_snapshot.best_tip.height.0) + } + // Once we're reified, we keep blocks until the FS syncs them, + // to ensure we never allow a gap + SnapshotAvailability::Reified => finalized_height, + }; + new_snapshot.remove_finalized_blocks(seam); let best_block = &new_snapshot .blocks @@ -730,7 +739,7 @@ impl NonFinalizedState { // Seam overlap: the finalized DB has reached the window's floor. // (The seam's absolute-chainwork base is fetched by the resolution // promotion, #1096, when its reader exists.) - SnapshotAvailability::Resolved + SnapshotAvailability::Reified } else { SnapshotAvailability::Provisional }; @@ -784,76 +793,6 @@ impl NonFinalizedState { self.current.load_full() } - /// Fetch the seam (finalization-ceiling) block from the source and build it - /// as an [`IndexedBlock`] to anchor the NFS window while the finalized DB is - /// still catching up. The absolute chainwork is a zero placeholder: - /// [`ProvisionalBlock::from_indexed_seam`] drops it and tracks the window's - /// work relative to this seam. - async fn fetch_seam_indexed_block(&self, height: Height) -> Result { - let block = self - .source - .get_block(HashOrHeight::Height(zebra_chain::block::Height(height.0))) - .await - .map_err(|e| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( - Box::new(e), - )) - })? - .ok_or_else(|| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( - Box::new(InvalidData(format!( - "source missing seam block at height {}", - height.0 - ))), - )) - })?; - let tree_roots = get_tree_roots_from_source(block.hash().into(), &self.source) - .await - .map_err(|e| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError( - Box::new(InvalidData(format!("{e}"))), - )) - })?; - Self::create_indexed_block_with_optional_roots( - block.as_ref(), - &tree_roots, - ChainWork::from_u256(U256::zero()), - self.network.clone(), - ) - .map_err(|e| { - SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( - InvalidData(e), - ))) - }) - } - - /// Create IndexedBlock with optional tree roots (for genesis/sync cases) - /// - /// TODO: Issue #604 - This uses `unwrap_or_default()` uniformly for both Sapling and Orchard, - /// but they have different activation heights. This masks potential bugs and prevents proper - /// validation based on network upgrade activation. - fn create_indexed_block_with_optional_roots( - block: &zebra_chain::block::Block, - tree_roots: &TreeRootData, - parent_chainwork: ChainWork, - network: Network, - ) -> Result { - let (sapling_root, sapling_size, orchard_root, orchard_size) = - tree_roots.clone().extract_with_defaults(); - - let metadata = BlockMetadata::new( - sapling_root, - sapling_size as u32, - orchard_root, - orchard_size as u32, - parent_chainwork, - network, - ); - - let block_with_metadata = BlockWithMetadata::new(block, metadata); - IndexedBlock::try_from(block_with_metadata) - } - async fn add_nonbest_block( &self, working_snapshot: &mut NonfinalizedBlockCacheSnapshot, @@ -892,6 +831,29 @@ impl NonFinalizedState { .insert(*provisional_block.hash(), provisional_block.clone()); Ok(provisional_block) } + + async fn reify( + &self, + new_snapshot: &mut NonfinalizedBlockCacheSnapshot, + + finalized_db: &ZainoDB, + ) -> Result<(), String> { + let mut reified_blocks = HashSet::new(); + let Some((seam_height, seam_hash)) = new_snapshot + .heights_to_hashes + .iter() + .min_by_key(|(height, _hash)| height) + else { + return Err("tried to reify empty snapshot".to_string()); + }; + let seam_block = finalized_db + .backend_for_cap(CapabilityRequest::IndexedBlockExt) + .map_err(|e| format!("backend can't serve indexed blocks"))? + .get_chain_block(*seam_height) + .await + .map_err(|e| format!("backend error: {e}"))?; + todo!() + } } /// Build a [`ProvisionalBlock`] from a source block, accumulating its From e286afaf7d0d0517aa95c11b963e9a1b7f38aea6 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Tue, 16 Jun 2026 14:09:17 -0300 Subject: [PATCH 23/31] seam_block error when missing --- .../zaino-state/src/chain_index/non_finalised_state.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index e01a000f2..899aa9641 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -846,12 +846,15 @@ impl NonFinalizedState { else { return Err("tried to reify empty snapshot".to_string()); }; - let seam_block = finalized_db + let Some(seam_block) = finalized_db .backend_for_cap(CapabilityRequest::IndexedBlockExt) .map_err(|e| format!("backend can't serve indexed blocks"))? .get_chain_block(*seam_height) .await - .map_err(|e| format!("backend error: {e}"))?; + .map_err(|e| format!("backend error: {e}"))? + else { + return Err("backend missing block below known block".to_string()); + }; todo!() } } From 6bac83eaf898f446fb8b67c4d447d7b2f08eded2 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Tue, 16 Jun 2026 15:53:54 -0300 Subject: [PATCH 24/31] impl Block for IndexedBlock, more reify fn work --- .../src/chain_index/non_finalised_state.rs | 61 +++++++++++++++---- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 899aa9641..7cc410aea 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -1,13 +1,4 @@ use super::{finalised_state::ZainoDB, source::BlockchainSource, NON_FINALIZED_DEPTH}; -use crate::chain_index::{ - finalised_state::capability::{CapabilityRequest, IndexedBlockExt}, - finalization_ceiling, - types::db::{ - legacy::{compact_block_from_parts, BlockData, CompactTxData}, - CommitmentTreeData, - }, - NonFinalizedSnapshot, -}; use crate::{ chain_index::types::{ self, BlockHash, BlockIndex, BlockMetadata, BlockWithMetadata, Height, TreeRootData, @@ -15,6 +6,18 @@ use crate::{ error::FinalisedStateError, ChainWork, }; +use crate::{ + chain_index::{ + finalised_state::capability::{CapabilityRequest, IndexedBlockExt}, + finalization_ceiling, + types::db::{ + legacy::{compact_block_from_parts, BlockData, CompactTxData}, + CommitmentTreeData, + }, + NonFinalizedSnapshot, + }, + IndexedBlock, +}; use arc_swap::ArcSwap; use futures::lock::Mutex; use primitive_types::U256; @@ -838,11 +841,11 @@ impl NonFinalizedState { finalized_db: &ZainoDB, ) -> Result<(), String> { - let mut reified_blocks = HashSet::new(); + let mut reified_blocks: HashSet = HashSet::new(); let Some((seam_height, seam_hash)) = new_snapshot .heights_to_hashes .iter() - .min_by_key(|(height, _hash)| height) + .min_by_key(|(height, _hash)| **height) else { return Err("tried to reify empty snapshot".to_string()); }; @@ -855,6 +858,14 @@ impl NonFinalizedState { else { return Err("backend missing block below known block".to_string()); }; + + let mut reify_children_of = HashSet::new(); + *new_snapshot.blocks.get_mut(seam_hash).expect("todo") = + seam_block.to_provisional_block(&self).await.expect("todo"); + reify_children_of.insert(seam_block.hash()); + + while reified_blocks.len() != new_snapshot.blocks.len() {} + todo!() } } @@ -990,6 +1001,34 @@ impl Block for ProvisionalBlock { Ok(self.clone()) } } + +impl Block for IndexedBlock { + fn hash_bytes_serialized_order(&self) -> [u8; 32] { + self.hash().0 + } + + fn prev_hash_bytes_serialized_order(&self) -> [u8; 32] { + self.context.parent_hash().0 + } + + async fn to_provisional_block( + &self, + _nfs: &NonFinalizedState, + ) -> Result { + Ok(ProvisionalBlock { + index: BlockIndex { + height: self.height(), + hash: *self.hash(), + }, + parent_hash: self.context.parent_hash, + chainwork: *self.chainwork(), + data: self.data, + transactions: self.transactions.clone(), + commitment_tree_data: self.commitment_tree_data, + }) + } +} + impl Block for zebra_chain::block::Block { fn hash_bytes_serialized_order(&self) -> [u8; 32] { self.hash().bytes_in_serialized_order() From d1038c95213ecd6dfa67574ed16ac83dc4f18a51 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Tue, 16 Jun 2026 16:11:11 -0300 Subject: [PATCH 25/31] finish (non_error_handled) reify fn --- .../src/chain_index/non_finalised_state.rs | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 7cc410aea..3125a3c01 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -705,7 +705,9 @@ impl NonFinalizedState { .heights_to_hashes .contains_key(&finalized_height) { - self.reify(&mut new_snapshot, finalized_db.as_ref()); + self.reify(&mut new_snapshot, finalized_db.as_ref()) + .await + .map_err(|e| todo!(""))?; } let seam = match new_snapshot.availability { @@ -841,7 +843,6 @@ impl NonFinalizedState { finalized_db: &ZainoDB, ) -> Result<(), String> { - let mut reified_blocks: HashSet = HashSet::new(); let Some((seam_height, seam_hash)) = new_snapshot .heights_to_hashes .iter() @@ -851,7 +852,7 @@ impl NonFinalizedState { }; let Some(seam_block) = finalized_db .backend_for_cap(CapabilityRequest::IndexedBlockExt) - .map_err(|e| format!("backend can't serve indexed blocks"))? + .map_err(|e| format!("backend can't serve indexed blocks: {e}"))? .get_chain_block(*seam_height) .await .map_err(|e| format!("backend error: {e}"))? @@ -862,11 +863,31 @@ impl NonFinalizedState { let mut reify_children_of = HashSet::new(); *new_snapshot.blocks.get_mut(seam_hash).expect("todo") = seam_block.to_provisional_block(&self).await.expect("todo"); - reify_children_of.insert(seam_block.hash()); - - while reified_blocks.len() != new_snapshot.blocks.len() {} + reify_children_of.insert(*seam_block.hash()); + + while !reify_children_of.is_empty() { + let to_reify: HashMap = new_snapshot + .blocks + .iter() + .filter(|(_hash, block)| reify_children_of.contains(block.parent_hash())) + .map(|(hash, block)| (*hash, block.clone())) + .collect(); + for (hash, block) in to_reify.iter() { + let mut block = block.clone(); + block.chainwork = block.chainwork.add( + &new_snapshot + .blocks + .get(block.parent_hash()) + .expect("todo") + .chainwork, + ); + new_snapshot.blocks.insert(*hash, block); + } + reify_children_of = to_reify.into_keys().collect(); + } - todo!() + new_snapshot.availability = SnapshotAvailability::Reified; + Ok(()) } } From c9761486946f7ed2db4c490b2481ec7bda2385e8 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Fri, 19 Jun 2026 12:38:57 -0300 Subject: [PATCH 26/31] provisional_cumulative_work now aggregates each block's work at calltime --- packages/zaino-state/src/chain_index/non_finalised_state.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 3125a3c01..2db62d4e5 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -237,10 +237,11 @@ impl ProvisionalBlock { &self, snapshot: &NonfinalizedBlockCacheSnapshot, ) -> ProvisionalCumulativeWork { - let mut work = ProvisionalCumulativeWork::seam().add_block_work(&self.chainwork); + let mut work = + ProvisionalCumulativeWork::seam().add_block_work(&ChainWork::from(self.data.work())); let mut parent_hash = self.parent_hash; while let Some(prev_block) = snapshot.get_chainblock_by_hash(&parent_hash) { - work = work.add_block_work(&prev_block.chainwork); + work = work.add_block_work(&ChainWork::from(prev_block.data.work())); parent_hash = prev_block.parent_hash; } work From 27cc7c16d2d68a8fa93b87e44348c1bf458cc005 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Fri, 19 Jun 2026 14:36:43 -0300 Subject: [PATCH 27/31] wip removal of Provisional block in favor of Provisional chainwork enum variant --- packages/zaino-state/src/chain_index.rs | 26 +- .../src/chain_index/non_finalised_state.rs | 376 ++++++------------ .../src/chain_index/types/db/legacy.rs | 75 +++- 3 files changed, 194 insertions(+), 283 deletions(-) diff --git a/packages/zaino-state/src/chain_index.rs b/packages/zaino-state/src/chain_index.rs index 0f5206347..96fea4030 100644 --- a/packages/zaino-state/src/chain_index.rs +++ b/packages/zaino-state/src/chain_index.rs @@ -28,7 +28,7 @@ use std::{sync::Arc, time::Duration}; use futures::{FutureExt, Stream}; use hex::FromHex as _; -use non_finalised_state::{NonfinalizedBlockCacheSnapshot, ProvisionalBlock}; +use non_finalised_state::NonfinalizedBlockCacheSnapshot; use source::{BlockchainSource, ValidatorConnector}; use tokio_stream::StreamExt; use tokio_util::sync::CancellationToken; @@ -156,7 +156,7 @@ pub(crate) fn chain_tips_from_nonfinalized_snapshot( fn branch_len_to_active_chain( snapshot: &NonfinalizedBlockCacheSnapshot, - block: &ProvisionalBlock, + block: &IndexedBlock, ) -> u32 { let mut branch_len = 0; let mut current = block; @@ -178,7 +178,7 @@ fn branch_len_to_active_chain( /// A block from either chain layer: the finalized state (an [`IndexedBlock`], /// carrying absolute chainwork) or the non-finalized state (a -/// [`ProvisionalBlock`], carrying relative work). Unifies the two for query +/// [`IndexedBlock`], carrying relative work). Unifies the two for query /// methods that may resolve a block from either layer while the NFS holds /// provisional blocks. /// @@ -193,7 +193,7 @@ pub enum ChainBlock { /// We know the sum of chainwork from this block down to the finalized tip, /// which is sufficient for determining the best chain, but do not know /// the absolute cumulative chainwork. - Provisional(ProvisionalBlock), + Provisional(IndexedBlock), } impl ChainBlock { @@ -2155,7 +2155,7 @@ impl ChainIndex for NodeBackedChainIndexSubscriber NonFinalizedSnapshot for Arc where T: NonFinalizedSnapshot, { - fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock> { + fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&IndexedBlock> { self.as_ref().get_chainblock_by_hash(target_hash) } - fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock> { + fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&IndexedBlock> { self.as_ref().get_chainblock_by_height(target_height) } } @@ -2523,13 +2523,13 @@ where /// A snapshot of the non-finalized state, for consistent queries pub trait NonFinalizedSnapshot { /// Hash -> block - fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock>; + fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&IndexedBlock>; /// Height -> block - fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock>; + fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&IndexedBlock>; } impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { - fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock> { + fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&IndexedBlock> { self.blocks.iter().find_map(|(hash, chainblock)| { if hash == target_hash { Some(chainblock) @@ -2538,7 +2538,7 @@ impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { } }) } - fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock> { + fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&IndexedBlock> { self.heights_to_hashes.iter().find_map(|(height, hash)| { if height == target_height { self.get_chainblock_by_hash(hash) @@ -2550,12 +2550,12 @@ impl NonFinalizedSnapshot for NonfinalizedBlockCacheSnapshot { } impl NonFinalizedSnapshot for ChainIndexSnapshot { - fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&ProvisionalBlock> { + fn get_chainblock_by_hash(&self, target_hash: &types::BlockHash) -> Option<&IndexedBlock> { // The NFS always exists; block lookup needs no absolute chainwork. self.get_nfs_snapshot().get_chainblock_by_hash(target_hash) } - fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&ProvisionalBlock> { + fn get_chainblock_by_height(&self, target_height: &types::Height) -> Option<&IndexedBlock> { self.get_nfs_snapshot() .get_chainblock_by_height(target_height) } diff --git a/packages/zaino-state/src/chain_index/non_finalised_state.rs b/packages/zaino-state/src/chain_index/non_finalised_state.rs index 2db62d4e5..cdfd234d7 100644 --- a/packages/zaino-state/src/chain_index/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/non_finalised_state.rs @@ -1,30 +1,16 @@ use super::{finalised_state::ZainoDB, source::BlockchainSource, NON_FINALIZED_DEPTH}; +use crate::{chain_index::finalization_ceiling, IndexedBlock}; use crate::{ chain_index::types::{ self, BlockHash, BlockIndex, BlockMetadata, BlockWithMetadata, Height, TreeRootData, }, error::FinalisedStateError, - ChainWork, -}; -use crate::{ - chain_index::{ - finalised_state::capability::{CapabilityRequest, IndexedBlockExt}, - finalization_ceiling, - types::db::{ - legacy::{compact_block_from_parts, BlockData, CompactTxData}, - CommitmentTreeData, - }, - NonFinalizedSnapshot, - }, - IndexedBlock, + BlockContext, ChainWork, }; use arc_swap::ArcSwap; use futures::lock::Mutex; use primitive_types::U256; -use std::{ - collections::{HashMap, HashSet}, - sync::Arc, -}; +use std::{collections::HashMap, sync::Arc}; use tokio::sync::mpsc; use tracing::{info, instrument, warn}; use zebra_chain::{parameters::Network, serialization::BytesInDisplayOrder}; @@ -129,7 +115,7 @@ pub(crate) struct NonfinalizedBlockCacheSnapshot { /// this includes all blocks on-chain, as well as /// all blocks known to have been on-chain before being /// removed by a reorg. Blocks reorged away have no height. - pub blocks: HashMap, + pub blocks: HashMap, /// hashes indexed by height /// Hashes in this map are part of the best chain. pub heights_to_hashes: HashMap, @@ -167,113 +153,13 @@ impl ProvisionalCumulativeWork { /// Accumulate one block's own (header-derived) work onto the running /// relative total. + /// Override the normal Chainwork behaviour of never + /// adding 0 pub(crate) fn add_block_work(&self, block_work: &ChainWork) -> Self { - Self(self.0.add(block_work)) - } -} - -/// A non-finalized block as held by the NFS. -/// -/// It is deliberately **not** an [`IndexedBlock`]. "Indexed" means the block -/// has been placed in the validated chain with its *absolute* cumulative -/// chainwork — and that absolute value is unknowable while the finalized -/// state has not yet caught up to the seam (the validator does not expose -/// chainwork, and the finalized DB has not reached the floor). A provisional -/// block instead carries [`Self::provisional_cumulative_work`]: cumulative -/// work measured *relative to the seam*, derived from block headers alone. -/// -/// Relative work is sufficient to choose the best tip within the -/// non-finalized window: under the assumption that no reorg exceeds -/// [`NON_FINALIZED_DEPTH`], the seam is a stable common ancestor of every -/// competing window chain, so ordering by relative work matches ordering by -/// absolute work. -/// -/// A provisional block is promoted to an [`IndexedBlock`] only at the -/// resolution boundary, when the seam's absolute work becomes known -/// (`absolute = seam_base + provisional_cumulative_work`). The relative value -/// lives only here and is never written into [`IndexedBlock`]'s absolute -/// `chainwork` field, so the two cannot be misattributed. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct ProvisionalBlock { - /// Height + hash of this block. - pub index: BlockIndex, - /// Parent block hash as *claimed by this block's header*. - /// - /// UNTRUSTED while the snapshot is provisional: the linkage it asserts is - /// not validated against the finalized chain until the seam connects, so - /// it must not be used for trusted chain-walks (e.g. fork-point recursion) - /// during the provisional stage. - pub parent_hash: BlockHash, - /// This block's individual chainwork - chainwork: ChainWork, - /// Header and auxiliary block data. - pub data: BlockData, - /// Compact representations of the block's transactions. - pub transactions: Vec, - /// Commitment tree data for the chain after this block is applied. - pub commitment_tree_data: CommitmentTreeData, -} - -impl ProvisionalBlock { - /// The block hash. - pub(crate) fn hash(&self) -> &BlockHash { - &self.index.hash - } - - /// The block height. - pub(crate) fn height(&self) -> Height { - self.index.height - } - - /// The parent block hash as claimed by the header. UNTRUSTED while - /// provisional — see the field doc; do not use for validated chain-walks. - pub(crate) fn parent_hash(&self) -> &BlockHash { - &self.parent_hash - } - - /// Cumulative work relative to the seam. Use this — never an absolute - /// chainwork — for best-tip selection within the non-finalized window. - pub(crate) fn provisional_cumulative_work( - &self, - snapshot: &NonfinalizedBlockCacheSnapshot, - ) -> ProvisionalCumulativeWork { - let mut work = - ProvisionalCumulativeWork::seam().add_block_work(&ChainWork::from(self.data.work())); - let mut parent_hash = self.parent_hash; - while let Some(prev_block) = snapshot.get_chainblock_by_hash(&parent_hash) { - work = work.add_block_work(&ChainWork::from(prev_block.data.work())); - parent_hash = prev_block.parent_hash; + match self.0 { + ChainWork::Indexed(_) => Self(self.0.add(block_work)), + ChainWork::Provisional => Self(*block_work), } - work - } - - /// The compact transactions in this block. - pub(crate) fn transactions(&self) -> &[CompactTxData] { - &self.transactions - } - - /// Header and auxiliary block data. - pub(crate) fn data(&self) -> &BlockData { - &self.data - } - - /// Commitment tree data after this block. - pub(crate) fn commitment_tree_data(&self) -> &CommitmentTreeData { - &self.commitment_tree_data - } - - /// The compact-block representation. Compact blocks carry no cumulative - /// chainwork, so this is identical to an indexed block's; both delegate to - /// [`compact_block_from_parts`]. - pub(crate) fn to_compact_block(&self) -> zaino_proto::proto::compact_formats::CompactBlock { - compact_block_from_parts( - self.height(), - self.hash(), - self.parent_hash(), - self.data().time(), - self.transactions(), - self.commitment_tree_data(), - ) } } @@ -390,19 +276,19 @@ impl NonfinalizedBlockCacheSnapshot { "100 blocks below self-reported chaintip height", ))?; - let provisional_block = block_to_provisional_block(&start_block, source, network) - .await - .map_err(|e| SyncError::ErrorFromSource(Box::new(e)))?; + let indexed_block = start_block + .to_indexed_block(source, network.clone()) + .await?; let mut blocks = HashMap::new(); let mut heights_to_hashes = HashMap::new(); let block_index = BlockIndex { - height: provisional_block.height(), - hash: *provisional_block.hash(), + height: indexed_block.height(), + hash: *indexed_block.hash(), }; - blocks.insert(block_index.hash, provisional_block); + blocks.insert(block_index.hash, indexed_block); heights_to_hashes.insert(block_index.height, block_index.hash); Ok(Self { @@ -415,7 +301,7 @@ impl NonfinalizedBlockCacheSnapshot { }) } - fn add_block_new_chaintip(&mut self, block: ProvisionalBlock) { + fn add_block_new_chaintip(&mut self, block: IndexedBlock) { self.best_tip = BlockIndex { height: block.height(), hash: *block.hash(), @@ -423,10 +309,7 @@ impl NonfinalizedBlockCacheSnapshot { self.add_block(block) } - fn get_block_by_hash_bytes_in_serialized_order( - &self, - hash: [u8; 32], - ) -> Option<&ProvisionalBlock> { + fn get_block_by_hash_bytes_in_serialized_order(&self, hash: [u8; 32]) -> Option<&IndexedBlock> { self.blocks .values() .find(|block| block.hash_bytes_serialized_order() == hash) @@ -441,7 +324,7 @@ impl NonfinalizedBlockCacheSnapshot { .retain(|height, _hash| height >= &finalized_height); } - fn add_block(&mut self, block: ProvisionalBlock) { + fn add_block(&mut self, block: IndexedBlock) { self.heights_to_hashes.insert(block.height(), *block.hash()); self.blocks.insert(*block.hash(), block); } @@ -561,7 +444,7 @@ impl NonFinalizedState { if parent_hash == working_snapshot.best_tip.hash { // Normal chain progression let chainblock = - block_to_provisional_block(&block, &self.source, self.network.clone()).await?; + block_to_indexed_block(&block, &self.source, self.network.clone()).await?; info!( height = (working_snapshot.best_tip.height + 1).0, hash = %chainblock.hash(), @@ -599,8 +482,8 @@ impl NonFinalizedState { &self, working_snapshot: &mut NonfinalizedBlockCacheSnapshot, block: &impl Block, - ) -> Result { - match working_snapshot + ) -> Result { + let prev_block = match working_snapshot .get_block_by_hash_bytes_in_serialized_order(block.prev_hash_bytes_serialized_order()) .cloned() { @@ -637,9 +520,11 @@ impl NonFinalizedState { Box::pin(self.handle_reorg(working_snapshot, &*prev_block)).await? } }; - let provisional_block = block.to_provisional_block(self).await?; - working_snapshot.add_block_new_chaintip(provisional_block.clone()); - Ok(provisional_block) + let indexed_block = block + .to_indexed_block(&self.source, self.network.clone()) + .await?; + working_snapshot.add_block_new_chaintip(indexed_block.clone()); + Ok(indexed_block) } /// Handle non-finalized change listener events @@ -706,9 +591,7 @@ impl NonFinalizedState { .heights_to_hashes .contains_key(&finalized_height) { - self.reify(&mut new_snapshot, finalized_db.as_ref()) - .await - .map_err(|e| todo!(""))?; + new_snapshot.availability = SnapshotAvailability::Reified; } let seam = match new_snapshot.availability { @@ -803,7 +686,7 @@ impl NonFinalizedState { &self, working_snapshot: &mut NonfinalizedBlockCacheSnapshot, block: &impl Block, - ) -> Result { + ) -> Result { match working_snapshot .get_block_by_hash_bytes_in_serialized_order(block.prev_hash_bytes_serialized_order()) .cloned() @@ -831,76 +714,76 @@ impl NonFinalizedState { Box::pin(self.add_nonbest_block(working_snapshot, &*prev_block)).await? } }; - let provisional_block = block.to_provisional_block(self).await?; + let provisional_block = block + .to_indexed_block(&self.source, self.network.clone()) + .await?; working_snapshot .blocks .insert(*provisional_block.hash(), provisional_block.clone()); Ok(provisional_block) } - async fn reify( - &self, - new_snapshot: &mut NonfinalizedBlockCacheSnapshot, - - finalized_db: &ZainoDB, - ) -> Result<(), String> { - let Some((seam_height, seam_hash)) = new_snapshot - .heights_to_hashes - .iter() - .min_by_key(|(height, _hash)| **height) - else { - return Err("tried to reify empty snapshot".to_string()); - }; - let Some(seam_block) = finalized_db - .backend_for_cap(CapabilityRequest::IndexedBlockExt) - .map_err(|e| format!("backend can't serve indexed blocks: {e}"))? - .get_chain_block(*seam_height) - .await - .map_err(|e| format!("backend error: {e}"))? - else { - return Err("backend missing block below known block".to_string()); - }; - - let mut reify_children_of = HashSet::new(); - *new_snapshot.blocks.get_mut(seam_hash).expect("todo") = - seam_block.to_provisional_block(&self).await.expect("todo"); - reify_children_of.insert(*seam_block.hash()); - - while !reify_children_of.is_empty() { - let to_reify: HashMap = new_snapshot - .blocks - .iter() - .filter(|(_hash, block)| reify_children_of.contains(block.parent_hash())) - .map(|(hash, block)| (*hash, block.clone())) - .collect(); - for (hash, block) in to_reify.iter() { - let mut block = block.clone(); - block.chainwork = block.chainwork.add( - &new_snapshot - .blocks - .get(block.parent_hash()) - .expect("todo") - .chainwork, - ); - new_snapshot.blocks.insert(*hash, block); - } - reify_children_of = to_reify.into_keys().collect(); - } - - new_snapshot.availability = SnapshotAvailability::Reified; - Ok(()) - } + // async fn reify( + // &self, + // new_snapshot: &mut NonfinalizedBlockCacheSnapshot, + + // finalized_db: &ZainoDB, + // ) -> Result<(), String> { + // let Some((seam_height, seam_hash)) = new_snapshot + // .heights_to_hashes + // .iter() + // .min_by_key(|(height, _hash)| **height) + // else { + // return Err("tried to reify empty snapshot".to_string()); + // }; + // let Some(seam_block) = finalized_db + // .backend_for_cap(CapabilityRequest::IndexedBlockExt) + // .map_err(|e| format!("backend can't serve indexed blocks: {e}"))? + // .get_chain_block(*seam_height) + // .await + // .map_err(|e| format!("backend error: {e}"))? + // else { + // return Err("backend missing block below known block".to_string()); + // }; + + // let mut reify_children_of = HashSet::new(); + // *new_snapshot.blocks.get_mut(seam_hash).expect("todo") = + // seam_block.to_provisional_block(&self).await.expect("todo"); + // reify_children_of.insert(*seam_block.hash()); + + // while !reify_children_of.is_empty() { + // let to_reify: HashMap = new_snapshot + // .blocks + // .iter() + // .filter(|(_hash, block)| reify_children_of.contains(block.parent_hash())) + // .map(|(hash, block)| (*hash, block.clone())) + // .collect(); + // for (hash, block) in to_reify.iter() { + // let mut block = block.clone(); + // block.chainwork = block.chainwork.add( + // &new_snapshot + // .blocks + // .get(block.parent_hash()) + // .expect("todo") + // .chainwork, + // ); + // new_snapshot.blocks.insert(*hash, block); + // } + // reify_children_of = to_reify.into_keys().collect(); + // } + + // new_snapshot.availability = SnapshotAvailability::Reified; + // Ok(()) + // } } -/// Build a [`ProvisionalBlock`] from a source block, accumulating its -/// header work onto the parent's relative total. Mirrors -/// [`Self::block_to_chainblock`] but produces a provisional (not indexed) -/// block: no absolute chainwork is computed or required. -async fn block_to_provisional_block( +/// Build an [`IndexedBlock`] from a source block +/// Chainwork will always be provisional +async fn block_to_indexed_block( block: &zebra_chain::block::Block, source: &impl BlockchainSource, network: Network, -) -> Result { +) -> Result { let tree_roots = get_tree_roots_from_source(block.hash().into(), source) .await .map_err(|e| { @@ -909,29 +792,23 @@ async fn block_to_provisional_block( ))) })?; - provisional_block_from_parts(block, &tree_roots, network).map_err(|e| { + indexed_block_from_parts(block, &tree_roots, network).map_err(|e| { SyncError::ValidatorConnectionError(NodeConnectionError::UnrecoverableError(Box::new( InvalidData(e), ))) }) } -/// Assemble a [`ProvisionalBlock`] from already-fetched parts. Reuses the +/// Assemble an [`IndexedBlock`] from already-fetched parts. Reuses the /// shared `BlockWithMetadata` extractors (`extract_block_data`, /// `extract_transactions`, `create_commitment_tree_data`, `block_work`); -/// the only divergence from the indexed path is that work is accumulated -/// *relative to the seam* instead of into an absolute `BlockContext`. -fn provisional_block_from_parts( +fn indexed_block_from_parts( block: &zebra_chain::block::Block, tree_roots: &TreeRootData, network: Network, -) -> Result { +) -> Result { let (sapling_root, sapling_size, orchard_root, orchard_size) = tree_roots.clone().extract_with_defaults(); - // `parent_chainwork` is unused for a provisional block (it feeds only - // `create_block_context`, which we never call here); a zero placeholder - // keeps the throwaway `BlockMetadata` shape without touching absolute - // work. The relative total is tracked separately, below. let metadata = BlockMetadata::new( sapling_root, sapling_size as u32, @@ -945,7 +822,7 @@ fn provisional_block_from_parts( let data = block_with_metadata.extract_block_data()?; let transactions = block_with_metadata.extract_transactions()?; let commitment_tree_data = block_with_metadata.create_commitment_tree_data(); - let chainwork = block_with_metadata.block_work()?; + let chainwork = ChainwChainWork::Provisional; let hash = BlockHash::from(block.hash()); let parent_hash = BlockHash::from(block.header.previous_block_hash); @@ -954,15 +831,18 @@ fn provisional_block_from_parts( .map(|height| Height(height.0)) .ok_or_else(|| String::from("Any valid block has a coinbase height"))?; - Ok(ProvisionalBlock { - index: BlockIndex { height, hash }, - parent_hash, - chainwork, + Ok(IndexedBlock { + context: BlockContext { + index: BlockIndex { height, hash }, + parent_hash, + chainwork, + }, data, transactions, commitment_tree_data, }) } + /// Get commitment tree roots from the blockchain source async fn get_tree_roots_from_source( block_hash: BlockHash, @@ -997,31 +877,12 @@ pub enum UpdateError { trait Block { fn hash_bytes_serialized_order(&self) -> [u8; 32]; - fn prev_hash_bytes_serialized_order(&self) -> [u8; 32]; - /// Produce the [`ProvisionalBlock`] for this block. - async fn to_provisional_block( + async fn to_indexed_block( &self, - nfs: &NonFinalizedState, - ) -> Result; -} - -impl Block for ProvisionalBlock { - fn hash_bytes_serialized_order(&self) -> [u8; 32] { - self.hash().0 - } - - fn prev_hash_bytes_serialized_order(&self) -> [u8; 32] { - self.parent_hash().0 - } - - async fn to_provisional_block( - &self, - _nfs: &NonFinalizedState, - ) -> Result { - // Identity: a block already in the snapshot keeps the relative work it - // was assigned when first added; `parent_work` is irrelevant here. - Ok(self.clone()) - } + source: &Source, + network: Network, + ) -> Result; + fn prev_hash_bytes_serialized_order(&self) -> [u8; 32]; } impl Block for IndexedBlock { @@ -1033,21 +894,12 @@ impl Block for IndexedBlock { self.context.parent_hash().0 } - async fn to_provisional_block( + async fn to_indexed_block( &self, - _nfs: &NonFinalizedState, - ) -> Result { - Ok(ProvisionalBlock { - index: BlockIndex { - height: self.height(), - hash: *self.hash(), - }, - parent_hash: self.context.parent_hash, - chainwork: *self.chainwork(), - data: self.data, - transactions: self.transactions.clone(), - commitment_tree_data: self.commitment_tree_data, - }) + _source: &Source, + _network: Network, + ) -> Result { + Ok(self.clone()) } } @@ -1059,11 +911,11 @@ impl Block for zebra_chain::block::Block { fn prev_hash_bytes_serialized_order(&self) -> [u8; 32] { self.header.previous_block_hash.bytes_in_serialized_order() } - - async fn to_provisional_block( + async fn to_indexed_block( &self, - nfs: &NonFinalizedState, - ) -> Result { - block_to_provisional_block(self, &nfs.source, nfs.network.clone()).await + source: &Source, + network: Network, + ) -> Result { + block_to_indexed_block(self, source, network).await } } diff --git a/packages/zaino-state/src/chain_index/types/db/legacy.rs b/packages/zaino-state/src/chain_index/types/db/legacy.rs index aa8745347..6295efa4d 100644 --- a/packages/zaino-state/src/chain_index/types/db/legacy.rs +++ b/packages/zaino-state/src/chain_index/types/db/legacy.rs @@ -41,7 +41,11 @@ use crate::chain_index::encoding::{ read_vec, version, write_fixed_le, write_i64_le, write_option, write_u16_be, write_u32_be, write_u32_le, write_u64_le, write_vec, FixedEncodedLen, ZainoVersionedSerde, }; +use crate::chain_index::non_finalised_state::{ + NonfinalizedBlockCacheSnapshot, ProvisionalCumulativeWork, +}; use crate::chain_index::types::BlockContext; +use crate::chain_index::NonFinalizedSnapshot as _; use super::commitment::{CommitmentTreeData, CommitmentTreeRoots, CommitmentTreeSizes}; @@ -725,33 +729,68 @@ impl FixedEncodedLen for Outpoint { /// stored as a **big-endian** 256-bit unsigned integer. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] #[cfg_attr(test, derive(serde::Serialize, serde::Deserialize))] -pub struct ChainWork([u8; 32]); +pub enum ChainWork { + /// Known to be non-zero + Indexed([u8; 32]), + /// Represented in some contexts as zero, which + /// is otherwise an impossible value + Provisional, +} impl ChainWork { ///Returns ChainWork as a U256. pub fn to_u256(&self) -> U256 { - U256::from_big_endian(&self.0) + match self { + ChainWork::Indexed(work) => U256::from_big_endian(work), + ChainWork::Provisional => U256::zero(), + } } /// Builds a ChainWork from a U256. pub fn from_u256(value: U256) -> Self { let buf: [u8; 32] = value.to_big_endian(); - ChainWork(buf) + if buf == [0; 32] { + ChainWork::Provisional + } else { + ChainWork::Indexed(buf) + } } /// Adds 2 ChainWorks. + /// Provisional chainwork is like NaN: if added to + /// a number it just gives Provisional pub fn add(&self, other: &Self) -> Self { - Self::from_u256(self.to_u256() + other.to_u256()) + if self.is_provisional() || other.is_provisional() { + ChainWork::Provisional + } else { + Self::from_u256(self.to_u256() + other.to_u256()) + } } /// Subtract one ChainWork from another. + /// Provisional chainwork is like NaN: if added to + /// a number it just gives Provisional pub fn sub(&self, other: &Self) -> Self { - Self::from_u256(self.to_u256() - other.to_u256()) + if self.is_provisional() || other.is_provisional() { + ChainWork::Provisional + } else { + Self::from_u256(self.to_u256() - other.to_u256()) + } } /// Returns ChainWork bytes. pub fn as_bytes(&self) -> &[u8; 32] { - &self.0 + match self { + ChainWork::Indexed(work) => work, + ChainWork::Provisional => &[0; 32], + } + } + + fn is_provisional(&self) -> bool { + match self { + ChainWork::Indexed(_) => false, + ChainWork::Provisional => true, + } } } @@ -785,12 +824,16 @@ impl ZainoVersionedSerde for ChainWork { } fn encode_v1(&self, w: &mut W) -> io::Result<()> { - write_fixed_le::<32, _>(w, &self.0) + write_fixed_le::<32, _>(w, &self.as_bytes()) } fn decode_v1(r: &mut R) -> io::Result { let bytes = read_fixed_le::<32, _>(r)?; - Ok(ChainWork(bytes)) + Ok(if bytes == [0; 32] { + ChainWork::Provisional + } else { + ChainWork::Indexed(bytes) + }) } } @@ -1170,6 +1213,22 @@ impl IndexedBlock { self.data.work() } + /// Cumulative work relative to the seam. Use this — never an absolute + /// chainwork — for best-tip selection within the non-finalized window. + pub(crate) fn provisional_cumulative_work( + &self, + snapshot: &NonfinalizedBlockCacheSnapshot, + ) -> ProvisionalCumulativeWork { + let mut work = + ProvisionalCumulativeWork::seam().add_block_work(&ChainWork::from(self.data.work())); + let mut parent_hash = self.context.parent_hash; + while let Some(prev_block) = snapshot.get_chainblock_by_hash(&parent_hash) { + work = work.add_block_work(&ChainWork::from(prev_block.data.work())); + parent_hash = prev_block.parent_hash; + } + work + } + /// Converts this `IndexedBlock` into a CompactBlock protobuf message using proto v4 format. /// /// NOTE: This method currently includes transparent tx data in the compact block produced, From dc843708e7b49de6a06aea9fd3106c3889f861b5 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Wed, 1 Jul 2026 14:28:43 -0300 Subject: [PATCH 28/31] wait for FS to catch up in test --- .../zaino-state/src/chain_index/tests/non_finalised_state.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index 98947cb1f..7893642b5 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -132,7 +132,6 @@ async fn block_is_evicted_from_nfs_when_finalized_advances_past_it() { ); mockchain.mine_blocks(20); - let post_mine_active_height = mockchain.active_height(); // Poll the *NFS tip*, not `finalized_state.db_height()`: // `fs.sync_to_height` advances the finalized DB BEFORE @@ -141,13 +140,13 @@ async fn block_is_evicted_from_nfs_when_finalized_advances_past_it() { // NFS reaching the post-mine chain tip is only observable after // `update` has published the trimmed snapshot. poll_until( - "NFS tip to catch up to the mined chain (post-trim state)", + "NFS tip to catch up to the mined chain (post-trim state) and FS to sync to finalized tip also", Duration::from_secs(10), Duration::from_millis(25), || async { let snapshot = index_reader.snapshot_nonfinalized_state().await.ok()?; let nfs = snapshot.resolved_nfs_snapshot()?; - (nfs.best_tip.height.0 == post_mine_active_height).then_some(()) + (!nfs.blocks.contains_key(&target_hash)).then_some(()) }, ) .await; From 7f82670820f37ccf70f8f4db44594ec603f24d11 Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Wed, 1 Jul 2026 17:35:21 -0300 Subject: [PATCH 29/31] fix missing roots in proptests --- packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs index 5a4d50381..56e81f269 100644 --- a/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs +++ b/packages/zaino-state/src/chain_index/tests/proptest_blockgen.rs @@ -480,6 +480,8 @@ fn fold_commitment_roots<'a>( mut orchard: Option, map: &mut std::collections::HashMap, ) -> (Option, Option) { + sapling.get_or_insert_with(SaplingFrontier::empty); + orchard.get_or_insert_with(OrchardFrontier::empty); for block in blocks { for transaction in &block.transactions { for sap in transaction.sapling_note_commitments() { From ccb3c13c9b9faa5f2dfd48767105b7902be2341a Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Wed, 1 Jul 2026 17:36:34 -0300 Subject: [PATCH 30/31] fix race condition test --- .../zaino-state/src/chain_index/tests/non_finalised_state.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs index 7893642b5..04889613a 100644 --- a/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs +++ b/packages/zaino-state/src/chain_index/tests/non_finalised_state.rs @@ -305,7 +305,6 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi let mc = mockchain.clone(); mockchain.arm_one_shot_get_block_hook(Box::new(move || mc.mine_blocks(advance))); - let post_mine_active = initial_active + advance; poll_until( "NFS tip to reach post-mine height (race window forced)", Duration::from_secs(10), @@ -313,7 +312,7 @@ async fn race_pre_mine_finalized_height_block_is_evicted_when_source_advances_mi || async { let snapshot = index_reader.snapshot_nonfinalized_state().await.ok()?; let nfs = snapshot.resolved_nfs_snapshot()?; - (nfs.best_tip.height.0 == post_mine_active).then_some(()) + (!nfs.blocks.contains_key(&target_hash)).then_some(()) }, ) .await; From 8684e98c2096cf6642297c5c02bfd9346eef12ee Mon Sep 17 00:00:00 2001 From: Hazel OHearn Date: Wed, 1 Jul 2026 17:42:37 -0300 Subject: [PATCH 31/31] update changelog --- packages/zaino-state/CHANGELOG.md | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/zaino-state/CHANGELOG.md b/packages/zaino-state/CHANGELOG.md index bb0f9b955..5ad4324af 100644 --- a/packages/zaino-state/CHANGELOG.md +++ b/packages/zaino-state/CHANGELOG.md @@ -7,16 +7,13 @@ and this library adheres to Rust's notion of ## [Unreleased not landing in 0.4.0] ### Added -- `chain_index::ChainBlock` -- `chain_index::non_finalized_state::ProvisionalBlock`, ### Changed -- `chain_index::ChainIndex` methods now return `ChainBlock`s in - place of `IndexedBlock`s. ChainBlock enum may not contain ChainWork. -- `chain_index::NonFinalizedSnapshot` trait methods now return -- `ProvisionalBlock`s instead of `ChainBlocks`. TODO: Before release, - this will return `ChainBlock`s, where they are reified as `IndexedBlocks` - after sync completes +- `chain_index::types::BlockContext`'s chainwork field is now optional +- `chain_index::ChainIndexSnapshot` has been completely overhauled, is now + a struct instead of an enum, with completely private internals. Callers + must now interface with it only by passing it to a ChainIndex implementor, + as intended. ## [Unreleased]