From c4e1bb9bd49bbe414399e7af50953a33fa31c895 Mon Sep 17 00:00:00 2001 From: Roman Date: Sun, 5 Jul 2026 00:44:17 +0000 Subject: [PATCH 01/12] feat(network): verify Zakura header-sync commitment roots before persisting --- CHANGELOG.md | 14 + docs/design/verified-commitment-trees.md | 108 ++++- .../src/parallel/commitment_aux_verify.rs | 215 ++++++++- zebra-network/src/zakura/handler.rs | 17 + zebra-network/src/zakura/header_sync/error.rs | 17 + .../src/zakura/header_sync/events.rs | 64 ++- zebra-network/src/zakura/header_sync/mod.rs | 4 +- .../src/zakura/header_sync/reactor.rs | 281 ++++++++++- .../src/zakura/header_sync/service.rs | 2 +- zebra-network/src/zakura/header_sync/state.rs | 82 +++- zebra-network/src/zakura/header_sync/tests.rs | 445 +++++++++++++++++- .../src/zakura/header_sync/validation.rs | 35 ++ zebra-network/src/zakura/testkit/cluster.rs | 70 ++- zebra-state/src/request.rs | 29 +- zebra-state/src/response.rs | 13 + zebra-state/src/service.rs | 154 ++++++ .../service/finalized_state/zebra_db/block.rs | 75 ++- .../zebra_db/block/tests/vectors.rs | 77 ++- zebra-state/src/service/tests.rs | 409 +++++++++++++++- zebrad/src/commands/start.rs | 141 +++--- .../start/zakura/header_sync_driver.rs | 338 ++++++------- zebrad/src/commands/start/zakura/mod.rs | 5 +- 22 files changed, 2225 insertions(+), 370 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 808aa12cd37..c2b61d3ec2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,20 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Zebra now tags the coinbase input of every block it mines with a `🌸`. The `mining.extra_coinbase_data` option is now limited to 86 bytes (was 94); Zebra refuses to start if it is exceeded. +- Zakura header sync now verifies peer-supplied commitment (tree-aux) roots + against block header commitments before persisting them, writing only the + header-authenticated confirmed prefix (the range tip's root is confirmed by + the next overlapping range). Root verification, the one-block overlap, and the + in-memory ZIP-221 history tree are bounded to the verified-commitment-trees + fast-sync handoff β€” the last checkpoint, the only region where the roots are + consumed β€” and run through the last checkpoint inclusive (its root is needed by + the handoff block). Above that boundary header sync runs plainly (blocks are + fully re-verified). The header frontier keeps tracking the committed chain + (checkpoint or legacy sync); the tree grows only as header ranges commit and is + rebuilt lazily from durable state on the rare occasion a forward range finds it + behind a non-Zakura commit. Serving is unaffected: `BlockRoots` still returns + roots derived from real note-commitment trees at any height where the node has + them. ### Changed diff --git a/docs/design/verified-commitment-trees.md b/docs/design/verified-commitment-trees.md index 186f7f74cbc..eaed3fc7084 100644 --- a/docs/design/verified-commitment-trees.md +++ b/docs/design/verified-commitment-trees.md @@ -342,14 +342,17 @@ Wire and DoS bounds: the header count (`TreeAuxRootCountMismatch`); and the root vector is preallocated only with the already-bounded header count, never an independent untrusted length. - The reactor additionally checks each root's height is `start_height + offset` - (`TreeAuxRootHeightMismatch` / `validate_tree_aux_root_heights`) before the roots reach - state. State re-checks the count and alignment invariants in `CommitHeaderRange` - (`prepare_header_range_batch_with_roots`) as defense in depth, and never + (`TreeAuxRootHeightMismatch` / `validate_tree_aux_root_heights`) and authenticates the roots + against the header commitments (Β§6.4) before the range reaches state. State re-checks the + alignment invariants in `CommitHeaderRange` (`prepare_header_range_batch_with_roots`) as + defense in depth, persists only the header-authenticated confirmed prefix (one shorter than the + headers; the range tip's root is confirmed by the next overlapping range, Β§6.4), and never writes peer-supplied roots for a height whose body is already committed β€” a re-delivered header range over committed heights cannot overwrite the verified serving-index rows. `BlockCommitmentRoots` still carries no trust: a recipient re-verifies every root against its -own checkpoint-committed headers (Β§6) before folding it in, so a forwarding/serving node is -exactly as trustworthy as an originating one. +own checkpoint-committed headers β€” at the header-sync reactor before persisting (Β§6.4) and again +at block commit (Β§6) β€” before folding it in, so a forwarding/serving node is exactly as +trustworthy as an originating one. ## 6. Verification β€” verify-before-commit @@ -417,6 +420,66 @@ is now locked together: `auth_data_root` is `pub(crate)`, `CheckpointVerifiedBlo `set_deferred_pool_balance_change`, and the semantic verifier builds blocks through `from_semantic_data` (auth-data root left unset). Compile-time enforced (fix in commit #192). +### 6.4 Header-sync-layer authentication and startup reconstruction + +Β§6.1–§6.3 are the consensus-critical gate at block-commit time. Header sync adds an _earlier_ gate +in the network reactor, so the roots it persists to `commitment_roots_by_height` and serves to +other peers are already header-authenticated, and so a restart never trusts an unconfirmed root. + +- **Bounded to the last checkpoint (inclusive).** Root verification, the one-block overlap, and the + in-memory header-frontier history tree exist only through the last checkpoint β€” + `CheckpointList::max_height()`, exactly the VCT fast-sync handoff height (Β§7). That is the only + region a consumer reads the persisted roots: above the last checkpoint blocks go through full + semantic verification and recompute their own trees, so peer roots there have no reader. The regime + is **inclusive** of the last checkpoint, because the handoff block *at* `last_checkpoint` reads its + own persisted root from the roots CF (`vct_roots_at_height` is inclusive; an empty slot there wedges + the handoff on the frozen-frontier safety check, Β§8.1). A root at `H` is only confirmed by the header + at `H + 1`, so the root-carrying regime runs until the frontier reaches `last_checkpoint + 1` β€” that + successor header confirms and persists the `last_checkpoint` root; its own root is never persisted. + Above `last_checkpoint + 1` header sync runs plainly (no roots, no overlap, no tree). +- **The tree is a function of header-range commits; rebuilt lazily.** The frontier tree moves only as + header ranges commit (folding the confirmed prefix). `best_header_tip` itself keeps tracking the + committed main chain β€” including blocks committed by checkpoint sync or the legacy syncer (surfaced + via the full-block mirror) β€” so it can run ahead of the tree when a non-Zakura path commits below the + checkpoint. The tree is not eagerly kept in sync across those paths; instead the one operation that + reads it, the forward root-verifying request, detects a behind tree (`MissingHeaderHistoryTree`) and + rebuilds it from durable state (`ReadRequest::BestHeaderHistoryTree`) β€” a single, guarded reload that + never fires in the normal header-leading path. This replaces the reorg/gossip/catch-up eager-reload + machinery of earlier increments. +- **Verify before persisting.** When a below-checkpoint header range arrives, the reactor folds its + supplied roots into the running header-frontier history tree and checks each header's commitment + against it (`verify_supplied_roots_from_parts` in `zebra-chain/src/parallel/commitment_aux_verify.rs`, + reusing the same header-commitment check plus the Β§6.1 below-Heartwood/NU5/`Nu6_3` pins β€” no new + crypto). A well-formed but wrong root fails this check and the range is rejected and scored + through header sync's misbehavior path before any root reaches state. +- **Serving is not bounded.** The client-side gate above never restricts what the node will _serve_. + `BlockRoots` (`block_roots_by_height_range`) remains a general API: per height it derives roots + from the real finalized note-commitment trees, else the real non-finalized chain's trees, else the + provisional header-sync roots CF β€” so a node with committed bodies above the checkpoint serves + real-tree roots there; only header-only heights fall back to provisional roots. +- **Persist only the confirmed prefix.** A block's commitment binds the history tree as of its + parent, so a range `[start..=end]` authenticates the roots for `[start..=end-1]`; the tip's own + root is only confirmed once the next range delivers `end+1`. Forward ranges therefore overlap by + one block β€” the next request re-anchors at the tip's parent β€” and `CommitHeaderRange` persists + only the header-authenticated confirmed prefix; the range tip's root is never written. The state + writes exactly the roots it is handed (`prepare_header_range_batch_with_roots` accepts a prefix one + shorter than the headers, or none β€” see the checkpoint-backfill note below), so the "one root per + header" wire invariant (Β§5.4) and the persisted set are deliberately distinct. +- **Checkpoint backfill skips this gate.** Only _forward_ ranges carry a frontier tree that can be + folded and checked. _Backward_ checkpoint-backfill ranges (headers below the sync anchor) are + authenticated by the checkpoint hash and fold onto the previous checkpoint's tree, not the forward + frontier the reactor caches, so they are committed without header-commitment validation and + persist no provisional roots. `prepare_header_range_batch_with_roots` accepts an empty roots vector + for exactly this shape; a full-length (tip-included) vector is still rejected, so the trust + boundary is unchanged β€” nothing unauthenticated is ever written. +- **Reconstruct at startup.** The durable roots CF therefore holds a contiguous run of confirmed + roots above the verified body tip, but never the header tip's own root. On startup + `ReadRequest::BestHeaderHistoryTree` folds the durable confirmed roots onto the verified-tip + history tree up to the highest _contiguous_ frontier and returns that frontier `(height, hash)`. + Header sync resumes from `frontier + 1` with an overlapping range that re-validates the next root, + so a restart never folds an unauthenticated root into the header-frontier tree and never caps back + to the verified tip on a one-block gap in the persisted roots. + ## 7. The fast commit path and checkpoint last checkpoint height The commit-path hook lives in `finalized_state.rs`; everything about _where data comes from_ @@ -607,12 +670,16 @@ commitment before it influences the anchor set or the history MMR.** Consequence - The below-NU5 Orchard pin and below-Heartwood Sapling check (Β§6.1) close the only ranges the MMR cannot vouch for. Skipping either would let an untrusted source inject an anchor the legacy recompute never produces β€” a consensus-equivalence break, not just a slowdown. -- The frozen-frontier fail-closed policy (Β§8) means a hostile root never corrupts state: it is - deleted and refused. A malformed root set is rejected at the header-sync reactor before it - reaches state and is scored through header sync's misbehavior path; a well-formed wrong root - is evicted on verify-before-commit and the commit stays parked (Β§8.1). The trade-off is - availability, not integrity: a settled bad root stalls the fast sync at that height instead - of writing wrong state (Β§8.1). +- Peer-supplied roots are authenticated against header commitments at the header-sync reactor + **before they are persisted** (Β§6.4): a malformed root set is rejected on decode, and a + well-formed wrong root fails the header-commitment fold (confirmed by the next range, one-block + lag) and is rejected and scored through header sync's misbehavior path β€” so only the + header-authenticated confirmed prefix is ever written, and a restart rebuilds the header-frontier + tree from those durable confirmed roots (never an unconfirmed tip root). +- The frozen-frontier fail-closed policy (Β§8) is the second, consensus-critical gate at block + commit: a hostile root that reached state anyway is deleted and refused, and the commit stays + parked (Β§8.1). The trade-off is availability, not integrity: a settled bad root stalls the fast + sync at that height instead of writing wrong state (Β§8.1). - DoS bounds on the header-sync roots fields (Β§5.4) β€” the all-or-nothing count check, the per-height alignment check, the bounded preallocation, and the message byte budget β€” protect the serving and client paths from unbounded memory growth. @@ -630,14 +697,29 @@ commitment before it influences the anchor set or the history MMR.** Consequence network. - **Increment 6b β€” adversarial peer policy.** A `zebrad` driver recorded heightβ†’peer provenance and ran a roots-specific cooldown/demotion/disconnect policy over the `tree_aux` stream. -- **Increment 6c β€” fold roots into header sync (current).** The standalone `tree_aux` stream, +- **Increment 6c β€” fold roots into header sync.** The standalone `tree_aux` stream, its driver, in-memory cache writer, and bespoke peer policy are **removed**. Roots now ride the header-sync `Headers` message as all-or-nothing metadata (Β§4.2, Β§5.4), are - persisted provisionally to `commitment_roots_by_height` ahead of body commit, and + persisted to `commitment_roots_by_height` ahead of body commit, and are read back by a DB-backed `PeerSource`. Recovery from a bad/missing root is an in-place commit retry fed only by an in-flight fanout re-delivery of the same header range β€” roots are not individually re-requested, so a settled hole is a fail-closed stall (Β§8.1); peer accountability rides header sync's existing misbehavior scoring. +- **Increment 6d β€” authenticate roots before persisting; reconstruct on restart.** + Header sync now verifies supplied roots against the header commitments before writing them, + persists only the header-authenticated confirmed prefix (the range tip's root is confirmed by the + next overlapping range), and rebuilds the header-frontier history tree from the durable confirmed + roots on startup β€” resuming from the highest contiguous frontier (Β§6.4). This closes the gap + where roots were persisted ahead of authentication and a restart could fold an unconfirmed tip + root into the header-frontier tree. +- **Increment 6e β€” bound the frontier tree to below the last checkpoint (current).** Root + verification, the one-block overlap, and the in-memory header-frontier tree now run only while the + frontier is below the last checkpoint (the VCT handoff height, the only region the roots are + consumed); at/above it header sync runs plainly (Β§6.4). Because that region is checkpoint-final and + gossip-free, the tree advances only by fold-on-commit and never needs a live reload, so the + reorg/gossip/catch-up tree-reload machinery (a `QueryBestHeaderHistoryTree` action, its + `BestHeaderHistoryTreeLoaded` reply, and the reactor's reload dispatches) is **removed**. The + reanchor/follow-verified-tip scheduling is kept but gated to fire only at/above the checkpoint. - **Increment 7 β€” indexing follower lane (archive only).** Relocate `tx_by_loc` + address indexes and the per-height trees + subtree CFs onto an async follower, so archive mode regains historical RPC without re-adding the frontier recompute to the consensus path. diff --git a/zebra-chain/src/parallel/commitment_aux_verify.rs b/zebra-chain/src/parallel/commitment_aux_verify.rs index 309c7f9205e..0f5c863139a 100644 --- a/zebra-chain/src/parallel/commitment_aux_verify.rs +++ b/zebra-chain/src/parallel/commitment_aux_verify.rs @@ -16,13 +16,38 @@ use crate::{ sapling, }; -/// Result of verifying supplied header-sync commitment roots from header parts. -#[derive(Clone, Debug)] -pub struct SuppliedRootsVerification { +/// Header-sync roots that have been verified against header commitments. +/// +/// This type is the boundary between untrusted peer-supplied tree-aux data and state +/// persistence. Its fields are private so callers can only create it by successfully running +/// [`verify_supplied_roots_from_parts`]. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct VerifiedHeaderCommitmentRoots { + tree: HistoryTree, + confirmed_tip: Option, + confirmed_roots: Vec, +} + +impl VerifiedHeaderCommitmentRoots { /// The history tree after folding only roots confirmed by this delivery. - pub tree: HistoryTree, + pub fn tree(&self) -> &HistoryTree { + &self.tree + } + /// Last height whose supplied roots were confirmed and folded. - pub confirmed_tip: Option, + pub fn confirmed_tip(&self) -> Option { + self.confirmed_tip + } + + /// The confirmed prefix of this delivery's supplied roots. + pub fn confirmed_roots(&self) -> &[BlockCommitmentRoots] { + &self.confirmed_roots + } + + /// Consume this payload and return its verified history tree. + pub fn into_tree(self) -> HistoryTree { + self.tree + } } /// A supplied-root verification failure. @@ -54,12 +79,13 @@ pub fn verify_supplied_roots_from_parts<'a, I>( network: &Network, mut tree: HistoryTree, items: I, -) -> Result +) -> Result where I: IntoIterator, { let items = items.into_iter().collect::>(); let mut confirmed_tip = None; + let mut confirmed_roots = Vec::with_capacity(items.len().saturating_sub(1)); for (index, (header, roots)) in items.iter().enumerate() { let height = roots.height; @@ -107,16 +133,56 @@ where .map_err(SuppliedRootsError::from) .map_err(|error| (height, error))?; confirmed_tip = Some(height); + confirmed_roots.push((*roots).clone()); } - Ok(SuppliedRootsVerification { + Ok(VerifiedHeaderCommitmentRoots { tree, confirmed_tip, + confirmed_roots, }) } -/// Header-driven commitment check against `history_tree`, the history tree as -/// of the parent. +/// Append already-trusted per-block roots to `tree`, advancing it through every supplied height. +/// +/// Unlike [`verify_supplied_roots_from_parts`], this performs no header-commitment checks and has +/// no one-block confirmation lag: the roots are assumed to have been verified when they were +/// persisted, so the returned tree is positioned at the last supplied height. +/// +/// `items` are `(header, roots)` in ascending, contiguous height order, each one height above +/// `tree`'s current tip. Used at startup and after a re-anchor to rebuild the header-frontier +/// history tree from the roots durably stored between the verified block tip and the header tip. +pub fn append_confirmed_roots<'a, I>( + network: &Network, + mut tree: HistoryTree, + items: I, +) -> Result)> +where + I: IntoIterator, +{ + for (header, roots) in items { + let height = roots.height; + tree.push_from_parts( + network, + HistoryTreeBlockParts { + header, + height, + sapling_root: &roots.sapling_root, + orchard_root: &roots.orchard_root, + ironwood_root: &roots.ironwood_root, + sapling_tx: roots.sapling_tx, + orchard_tx: roots.orchard_tx, + ironwood_tx: roots.ironwood_tx, + }, + ) + .map_err(Arc::new) + .map_err(|error| (height, error))?; + } + + Ok(tree) +} + +/// Header-driven commitment check against `history_tree`, the history tree as of the parent. pub fn header_commitment_is_valid_for_chain_history( header: &block::Header, height: block::Height, @@ -544,6 +610,137 @@ mod tests { ); } + #[test] + fn fold_confirmed_roots_advances_through_the_last_supplied_height_without_lag() { + let activation = NetworkUpgrade::Heartwood + .activation_height(&Mainnet) + .expect("mainnet has Heartwood") + .0; + + let act_block = mainnet_block_at(activation); + let next_block = mainnet_block_at(activation + 1); + let act_root = mainnet_sapling_root_at(activation); + let next_root = mainnet_sapling_root_at(activation + 1); + let empty_orchard_root = orchard::tree::NoteCommitmentTree::default().root(); + + let act_roots = roots_from_block(&act_block, act_root, empty_orchard_root); + let next_roots = roots_from_block(&next_block, next_root, empty_orchard_root); + let items = vec![ + (act_block.header.as_ref(), &act_roots), + (next_block.header.as_ref(), &next_roots), + ]; + + let tree = append_confirmed_roots(&Mainnet, empty_history_tree(), items) + .expect("already-verified roots fold without error"); + + // Unlike `verify_supplied_roots_from_parts`, which lags one block (leaving the tree at the + // activation height), a direct fold of trusted roots advances the tree through the final + // supplied height. This is what lets startup reconstruction land exactly at the header tip. + assert_eq!( + tree.as_ref() + .expect("post-Heartwood tree is non-empty") + .current_height(), + Height(activation + 1), + "the fold positions the tree at the last supplied height, not one behind", + ); + + // The folded tree matches one built by pushing both blocks in order. + let mut expected = HistoryTree::from_block( + &Mainnet, + act_block, + &act_root, + &empty_orchard_root, + &empty_ironwood_root(), + ) + .expect("activation block builds a history tree"); + expected + .push( + &Mainnet, + next_block, + &next_root, + &empty_orchard_root, + &empty_ironwood_root(), + ) + .expect("successor block extends the history tree"); + assert_eq!( + tree.hash(), + expected.hash(), + "the fold reproduces the block-driven history tree", + ); + } + + /// Startup/re-anchor reconstruction resumes the fold from the *non-empty* durable tree at a + /// post-Heartwood verified tip, not from an empty tree. This mirrors that: fold one confirmed + /// height onto a base tree already positioned at Heartwood activation and assert the result is + /// the block-driven tree β€” the non-empty-base case the state reconstruction relies on. + #[test] + fn append_confirmed_roots_resumes_from_a_non_empty_base_tree() { + let activation = NetworkUpgrade::Heartwood + .activation_height(&Mainnet) + .expect("mainnet has Heartwood") + .0; + + let act_block = mainnet_block_at(activation); + let next_block = mainnet_block_at(activation + 1); + let act_root = mainnet_sapling_root_at(activation); + let next_root = mainnet_sapling_root_at(activation + 1); + let empty_orchard_root = orchard::tree::NoteCommitmentTree::default().root(); + let next_roots = roots_from_block(&next_block, next_root, empty_orchard_root); + + // Base: the non-empty durable tree positioned at the Heartwood-activation verified tip. + let base = HistoryTree::from_block( + &Mainnet, + act_block.clone(), + &act_root, + &empty_orchard_root, + &empty_ironwood_root(), + ) + .expect("activation block builds a non-empty history tree"); + assert!( + base.as_ref().is_some(), + "the base tree must be non-empty at Heartwood activation", + ); + + // Reconstruction folds only the next confirmed height onto that non-empty base. + let tree = append_confirmed_roots( + &Mainnet, + base, + std::iter::once((next_block.header.as_ref(), &next_roots)), + ) + .expect("folding onto a non-empty base succeeds"); + + assert_eq!( + tree.as_ref() + .expect("post-Heartwood tree is non-empty") + .current_height(), + Height(activation + 1), + "the fold advances the non-empty base to the last supplied height", + ); + + let mut expected = HistoryTree::from_block( + &Mainnet, + act_block, + &act_root, + &empty_orchard_root, + &empty_ironwood_root(), + ) + .expect("activation block builds a history tree"); + expected + .push( + &Mainnet, + next_block, + &next_root, + &empty_orchard_root, + &empty_ironwood_root(), + ) + .expect("successor block extends the history tree"); + assert_eq!( + tree.hash(), + expected.hash(), + "resuming the fold from a non-empty base reproduces the block-driven tree", + ); + } + #[test] fn rejects_wrong_root_at_successor_height() { let activation = NetworkUpgrade::Heartwood diff --git a/zebra-network/src/zakura/handler.rs b/zebra-network/src/zakura/handler.rs index 405b10456e2..c5a68b24138 100644 --- a/zebra-network/src/zakura/handler.rs +++ b/zebra-network/src/zakura/handler.rs @@ -32,6 +32,7 @@ use tokio::{ use tokio_util::sync::CancellationToken; use zebra_chain::{ block::{self, Block, CountedHeader}, + history_tree::HistoryTree, parameters::{Network, NetworkKind}, serialization::{CompactSizeMessage, ZcashDeserialize, MAX_HEADERS_PER_MESSAGE}, transaction::Transaction, @@ -543,6 +544,13 @@ pub struct ZakuraHeaderSyncDriverStartup { pub frontiers: HeaderSyncFrontiers, /// Durable best header tip loaded from state. pub best_header_tip: Option<(block::Height, block::Hash)>, + /// Hash of the durable best header tip's parent, if available. + pub best_header_parent_hash: Option, + /// History tree positioned at the durable best header tip. + /// + /// Reconstructed from durable state so post-Heartwood header-sync root verification can start + /// immediately; the default (empty) tree is the pre-Heartwood/empty-state value. + pub best_header_history_tree: Arc, /// Hash of `frontiers.verified_block_tip`. pub verified_block_tip_hash: block::Hash, } @@ -2776,6 +2784,13 @@ pub async fn spawn_zakura_endpoint_with_header_sync_driver( let best_header_tip = header_sync_driver_startup .as_ref() .map_or(Some(anchor), |startup| startup.best_header_tip); + let best_header_parent_hash = header_sync_driver_startup + .as_ref() + .and_then(|startup| startup.best_header_parent_hash); + let best_header_history_tree = header_sync_driver_startup + .as_ref() + .map(|startup| startup.best_header_history_tree.clone()) + .unwrap_or_else(|| Arc::new(HistoryTree::default())); let sync_frontier = header_sync_driver_startup.as_ref().map(|driver_startup| { let best_header_tip = driver_startup.best_header_tip.unwrap_or(anchor); let initial = FrontierUpdate { @@ -2799,6 +2814,8 @@ pub async fn spawn_zakura_endpoint_with_header_sync_driver( config.zakura.header_sync.clone(), limits.max_frame_bytes, ); + startup.best_header_parent_hash = best_header_parent_hash; + startup.best_header_history_tree = best_header_history_tree; startup.status_refresh_interval = config.zakura.header_sync.status_refresh_interval; startup.trace = trace.clone(); startup.frontier_updates = sync_frontier diff --git a/zebra-network/src/zakura/header_sync/error.rs b/zebra-network/src/zakura/header_sync/error.rs index dbaabf5461e..a6c25cb4758 100644 --- a/zebra-network/src/zakura/header_sync/error.rs +++ b/zebra-network/src/zakura/header_sync/error.rs @@ -121,6 +121,23 @@ pub enum HeaderSyncWireError { #[error("first Zakura header-sync range header does not link to anchor")] FirstHeaderDoesNotLink, + /// A header commitment did not match its supplied auxiliary data. + #[error("invalid Zakura header-sync header commitment: {0}")] + InvalidHeaderCommitment(#[from] block::CommitmentError), + + /// The supplied auxiliary roots could not extend the history tree. + #[error("invalid Zakura header-sync history tree update: {0}")] + HistoryTree(#[from] Arc), + + /// The parent history tree needed to validate a header range is unavailable. + #[error("missing Zakura header-sync parent history tree at height {height:?}, hash {hash:?}")] + MissingHeaderHistoryTree { + /// Height whose history tree is required. + height: block::Height, + /// Hash of the required parent header. + hash: block::Hash, + }, + /// Equihash solution size did not match the active network. #[error("Zakura header-sync Equihash solution size does not match the active network")] WrongEquihashSolutionSize, diff --git a/zebra-network/src/zakura/header_sync/events.rs b/zebra-network/src/zakura/header_sync/events.rs index ffaa75fcc55..7b04ed3844f 100644 --- a/zebra-network/src/zakura/header_sync/events.rs +++ b/zebra-network/src/zakura/header_sync/events.rs @@ -1,3 +1,9 @@ +use std::sync::Arc; + +use zebra_chain::{ + history_tree::HistoryTree, parallel::commitment_aux_verify::VerifiedHeaderCommitmentRoots, +}; + use super::{config::*, error::*, validation::*, wire::*, *}; use crate::zakura::{ FrontierUpdate, HeaderSyncPeerSession, HeaderSyncServiceSummary, ServicePeerSnapshot, @@ -26,6 +32,20 @@ pub struct HeaderSyncStartup { pub frontiers: HeaderSyncFrontiers, /// Durable best header tip loaded from storage at startup. pub best_header_tip: Option<(block::Height, block::Hash)>, + /// Hash of the durable best header tip's parent, if the tip is above genesis. + pub best_header_parent_hash: Option, + /// The last checkpoint height β€” the VCT fast-sync handoff boundary. + /// + /// Header sync requests, verifies, persists, and tree-tracks peer-supplied commitment roots only + /// while the header frontier is below this height (the only region a VCT consumer reads them). + /// At/above it the frontier tree is dropped and header sync runs plain. Defaults to + /// `network.checkpoint_list().max_height()` (the height the embedded VCT frontier is pinned to). + pub last_checkpoint_height: block::Height, + /// History tree positioned at the durable best header tip. + /// + /// Pre-Heartwood or empty state uses the default (empty) tree; post-Heartwood this is the tree + /// reconstructed from durable roots so header-sync root verification can start immediately. + pub best_header_history_tree: Arc, /// Shared sync exchange frontier stream. pub frontier_updates: Option>, /// Local stream-5 advertisement. @@ -56,11 +76,15 @@ impl HeaderSyncStartup { config: ZakuraHeaderSyncConfig, max_frame_bytes: u32, ) -> Self { + let last_checkpoint_height = network.checkpoint_list().max_height(); Self { network, anchor, frontiers, best_header_tip, + best_header_parent_hash: None, + last_checkpoint_height, + best_header_history_tree: Arc::new(HistoryTree::default()), frontier_updates: None, config, max_frame_bytes, @@ -217,6 +241,21 @@ pub enum HeaderSyncEvent { }, /// State finalized or verified-body frontiers changed. StateFrontiersChanged(HeaderSyncFrontiers), + /// State returned the history tree rebuilt for the current best header tip. + /// + /// Answers a [`HeaderSyncAction::QueryBestHeaderHistoryTree`] β€” the single lazy rebuild triggered + /// when a below-checkpoint forward range finds the header-frontier tree behind the committed tip + /// (a non-Zakura path committed ahead). The tree is repositioned from durable state. + /// + /// Always sent once per query β€” including on a state read error β€” so the reactor's in-flight + /// rebuild guard is always cleared. `history_tree` is `None` when the rebuild failed (the guard is + /// cleared and the next forward range re-triggers it), `Some` on success. + BestHeaderHistoryTreeLoaded { + /// Best header tip the tree is positioned at when this reload was requested. + best_header_tip: block::Height, + /// History tree reconstructed by state, positioned at `best_header_tip`; `None` on failure. + history_tree: Option>, + }, /// State successfully committed a header range. HeaderRangeCommitted { /// First committed height. @@ -225,6 +264,8 @@ pub enum HeaderSyncEvent { tip_height: block::Height, /// New best header tip hash. tip_hash: block::Hash, + /// Hash of the new best header tip's parent, if known. + tip_parent_hash: Option, }, /// State rejected a previously requested range. HeaderRangeCommitFailed { @@ -313,13 +354,28 @@ pub enum HeaderSyncAction { headers: Vec>, /// Advisory serialized body sizes, parallel to `headers`. body_sizes: Vec, - /// Per-height commitment roots, parallel to `headers`. - tree_aux_roots: Vec, + /// Header-layer verified commitment roots for the confirmed prefix of `headers`. + /// + /// `Some` only for below-checkpoint forward ranges (the confirmed prefix is persisted). + /// `None` for checkpoint-authenticated backward backfill ranges (they fold onto the previous + /// checkpoint's tree, not the forward frontier) and for above-checkpoint forward ranges (past + /// the VCT handoff boundary, where roots are neither requested nor needed). Both persist no + /// roots. + verified_roots: Option>, /// Whether the range is expected to be finalized by checkpoint policy. finalized: bool, }, - /// Ask state for the durable best header tip. - QueryBestHeaderTip, + /// Ask state to rebuild the header-frontier history tree at the current best header tip. + /// + /// The single reload trigger: dispatched when a below-checkpoint forward range finds the tree + /// behind the committed tip. State reconstructs from durable roots; `verified_block_tip` is the + /// reconstruction base. + QueryBestHeaderHistoryTree { + /// Verified block tip that is the reconstruction base. + verified_block_tip: block::Height, + /// Header tip the rebuilt tree must be positioned at. + best_header_tip: block::Height, + }, /// Ask state for a bounded contiguous range of headers. QueryHeadersByHeightRange { /// Peer that requested the range. diff --git a/zebra-network/src/zakura/header_sync/mod.rs b/zebra-network/src/zakura/header_sync/mod.rs index 556b1496f3d..a875d919448 100644 --- a/zebra-network/src/zakura/header_sync/mod.rs +++ b/zebra-network/src/zakura/header_sync/mod.rs @@ -61,8 +61,8 @@ pub(crate) use service::{ drive_header_sync_actions, HeaderSyncPassthroughService, HeaderSyncService, }; pub use validation::{ - validate_header_range_links, validate_headers_stateless, validate_new_block_stateless, - HeaderSyncDecodeContext, HeaderSyncValidationContext, + validate_header_aux_commitments, validate_header_range_links, validate_headers_stateless, + validate_new_block_stateless, HeaderSyncDecodeContext, HeaderSyncValidationContext, }; pub use wire::{ HeaderSyncMessage, DEFAULT_HS_MAX_INFLIGHT, DEFAULT_HS_RANGE, MAX_HS_MESSAGE_BYTES, diff --git a/zebra-network/src/zakura/header_sync/reactor.rs b/zebra-network/src/zakura/header_sync/reactor.rs index cfd22679ca0..4680b198803 100644 --- a/zebra-network/src/zakura/header_sync/reactor.rs +++ b/zebra-network/src/zakura/header_sync/reactor.rs @@ -70,7 +70,6 @@ impl HeaderSyncReactor { let mut frontier_updates = self.startup.frontier_updates.clone(); let mut frontier_updates_open = frontier_updates.is_some(); if self.startup.range_state_actions_enabled { - let _ = self.dispatch_action(HeaderSyncAction::QueryBestHeaderTip); let _ = self.dispatch_action(HeaderSyncAction::QueryMissingBlockBodies { from: next_height(self.state.verified_block_tip) .unwrap_or(self.state.verified_block_tip), @@ -190,13 +189,26 @@ impl HeaderSyncReactor { HeaderSyncEvent::StateFrontiersChanged(frontiers) => { self.handle_state_frontiers_changed(frontiers).await; } + HeaderSyncEvent::BestHeaderHistoryTreeLoaded { + best_header_tip, + history_tree, + } => { + self.handle_best_header_history_tree_loaded(best_header_tip, history_tree) + .await; + } HeaderSyncEvent::HeaderRangeCommitted { start_height, tip_height, tip_hash, + tip_parent_hash, } => { - self.handle_header_range_committed(start_height, tip_height, tip_hash) - .await + self.handle_header_range_committed( + start_height, + tip_height, + tip_hash, + tip_parent_hash, + ) + .await } HeaderSyncEvent::HeaderRangeCommitFailed { peer, @@ -466,8 +478,13 @@ impl HeaderSyncReactor { self.update_verified_block_tip(height, hash); self.state.schedule.mark_height_covered(height); self.cancel_covered_outstanding(); + // `best_header_tip` is the header frontier and must keep tracking the committed main chain, + // including blocks committed by checkpoint sync or the legacy syncer (surfaced here via the + // full-block mirror). Advance it whenever the committed tip runs ahead; the in-memory tree is + // left untouched β€” a below-checkpoint forward range that then finds the tree behind rebuilds it + // lazily (the single `MissingHeaderHistoryTree` reload path). if height > self.state.best_header_tip { - self.publish_best_tip(height, hash).await; + self.publish_best_tip(height, hash, None).await; } self.schedule().await; } @@ -491,7 +508,7 @@ impl HeaderSyncReactor { self.state.schedule.mark_height_covered(height); self.cancel_covered_outstanding(); if height > self.state.best_header_tip { - self.publish_best_tip(height, hash).await; + self.publish_best_tip(height, hash, None).await; } let destinations = self.eligible_tip_destinations(&peer, height); @@ -603,11 +620,32 @@ impl HeaderSyncReactor { self.schedule().await; } + /// Installs a lazily-rebuilt header-frontier history tree (the answer to + /// `QueryBestHeaderHistoryTree`). Always clears the in-flight guard so a failed rebuild + /// (`history_tree == None`) does not wedge future rebuilds; installs only on success and only if + /// the best header tip has not moved since the query. Reschedules so the forward range that + /// triggered the rebuild is re-tried (against the repositioned tree, or re-triggering the rebuild + /// if it failed). + async fn handle_best_header_history_tree_loaded( + &mut self, + best_header_tip: block::Height, + history_tree: Option>, + ) { + self.state.rebuild_in_flight = false; + if let Some(history_tree) = history_tree { + if best_header_tip == self.state.best_header_tip { + self.state.best_header_history_tree = history_tree; + } + } + self.schedule().await; + } + async fn handle_header_range_committed( &mut self, start_height: block::Height, tip_height: block::Height, tip_hash: block::Hash, + tip_parent_hash: Option, ) { metrics::counter!("sync.header.range.committed").increment(1); self.trace_range_event( @@ -617,9 +655,10 @@ impl HeaderSyncReactor { None, None, ); + let committed_history_tree = self.pending_header_history_tree(start_height, tip_height); self.state .pending_commits - .retain(|_, range| !range.is_within(start_height, tip_height)); + .retain(|_, commit| !commit.range.is_within(start_height, tip_height)); self.state .schedule .mark_range_covered(start_height, tip_height); @@ -627,7 +666,13 @@ impl HeaderSyncReactor { // startup. In that path start==tip, so covered-range side effects are bounded. self.cancel_covered_outstanding(); if tip_height > self.state.best_header_tip { - self.publish_best_tip(tip_height, tip_hash).await; + // A genuine range commit always has a matching pending commit; keep the existing tree + // if none is found (e.g. the startup best-header-tip reload, where tip does not advance). + if let Some(committed_history_tree) = committed_history_tree { + self.state.best_header_history_tree = committed_history_tree; + } + self.publish_best_tip(tip_height, tip_hash, tip_parent_hash) + .await; } self.notify_body_gaps().await; self.schedule().await; @@ -657,11 +702,11 @@ impl HeaderSyncReactor { start_height, count, }; - if let Some(range) = self.state.pending_commits.remove(&key) { + if let Some(commit) = self.state.pending_commits.remove(&key) { if kind == HeaderSyncCommitFailureKind::Local { - self.state.schedule.clear_assignment(range); + self.state.schedule.clear_assignment(commit.range); } - self.state.schedule.retry(range); + self.state.schedule.retry(commit.range); } self.schedule().await; } @@ -1024,8 +1069,12 @@ impl HeaderSyncReactor { peer_max_headers_per_response: u32, in_flight_count: usize, ) { + // A root-carrying request must be answered with one root per header; a plain (above-checkpoint) + // request must carry no roots (checked separately just below). Only enforce the one-per-header + // count when roots were actually requested. if validate_body_sizes_len(headers.len(), body_sizes.len()).is_err() - || validate_tree_aux_roots_len(headers.len(), tree_aux_roots.len()).is_err() + || (outstanding.range.want_tree_aux_roots + && validate_tree_aux_roots_len(headers.len(), tree_aux_roots.len()).is_err()) { self.report_misbehavior(peer, HeaderSyncMisbehavior::MalformedMessage) .await; @@ -1188,13 +1237,94 @@ impl HeaderSyncReactor { } } + // Only below-checkpoint forward ranges carry a verifiable frontier tree and confirmed roots. + // Backward (checkpoint-backfill) ranges are authenticated by the checkpoint hash, not ZIP-221 + // header commitments (they fold onto the previous checkpoint's tree, not the forward frontier + // this reactor caches). Above-checkpoint forward ranges request no roots (`want_tree_aux_roots` + // is false past the VCT handoff boundary) and are fully re-verified at block commit. Both + // commit their headers with no provisional roots. + let verified_roots = if outstanding.range.priority == RangePriority::Forward + && outstanding.range.want_tree_aux_roots + { + match self.validate_forward_header_aux_commitments( + &peer, + outstanding.range, + &headers, + &tree_aux_roots, + ) { + Ok(verified_roots) => Some(verified_roots), + Err(error) + if matches!(error, HeaderSyncWireError::MissingHeaderHistoryTree { .. }) => + { + debug!( + ?peer, + ?error, + start_height = ?outstanding.range.start_height, + count = ?header_count, + "Zakura header-sync skipped header auxiliary validation" + ); + self.trace_range_validation_rejected( + &peer, + outstanding.range, + header_count, + "header_aux", + header_sync_wire_error_kind(&error), + ); + // The tree is behind this range's parent β€” a non-Zakura path (checkpoint/legacy + // sync) committed ahead of the header-commit frontier, so `best_header_tip` was + // bumped past where the tree is folded. This is the *only* reload trigger: rebuild + // the tree at the current frontier from durable state, then retry the range. In the + // normal Zakura path (header sync leading) this never fires. Guard so a run of + // forward ranges arriving before the rebuild lands dispatches only one reload. + if !self.state.rebuild_in_flight { + self.state.rebuild_in_flight = true; + metrics::counter!("sync.header.history_tree.rebuild").increment(1); + let _ = self.dispatch_action(HeaderSyncAction::QueryBestHeaderHistoryTree { + verified_block_tip: self.state.verified_block_tip, + best_header_tip: self.state.best_header_tip, + }); + } + self.state.schedule.clear_assignment(outstanding.range); + self.state.schedule.retry(outstanding.range); + self.schedule().await; + return; + } + Err(error) => { + debug!( + ?peer, + ?error, + start_height = ?outstanding.range.start_height, + count = ?header_count, + "Zakura header-sync rejected header auxiliary data" + ); + self.trace_range_validation_rejected( + &peer, + outstanding.range, + header_count, + "header_aux", + header_sync_wire_error_kind(&error), + ); + self.report_misbehavior(peer.clone(), HeaderSyncMisbehavior::InvalidRange) + .await; + self.state.schedule.retry(outstanding.range); + self.schedule().await; + return; + } + } + } else { + None + }; + self.state.pending_commits.insert( PendingCommitKey { peer: peer.clone(), start_height: outstanding.range.start_height, count: header_count, }, - outstanding.range, + PendingHeaderCommit { + range: outstanding.range, + verified_roots: verified_roots.clone(), + }, ); let _ = self.dispatch_action(HeaderSyncAction::CommitHeaderRange { peer, @@ -1202,11 +1332,90 @@ impl HeaderSyncReactor { start_height: outstanding.range.start_height, headers, body_sizes, - tree_aux_roots, + verified_roots: verified_roots.map(Box::new), finalized: outstanding.range.finalized, }); } + // Validates the header auxiliary commitments for the given range. + // Returns the verified root payload if it is valid, otherwise returns an error. + fn validate_forward_header_aux_commitments( + &self, + peer: &ZakuraPeerId, + range: RangeRequest, + headers: &[Arc], + tree_aux_roots: &[BlockCommitmentRoots], + ) -> Result< + zebra_chain::parallel::commitment_aux_verify::VerifiedHeaderCommitmentRoots, + HeaderSyncWireError, + > { + if !range.want_tree_aux_roots || range.priority != RangePriority::Forward { + return Err(HeaderSyncWireError::MissingHeaderHistoryTree { + height: range.start_height, + hash: range.anchor_hash, + }); + } + + let parent_height = previous_height(range.start_height) + .ok_or(HeaderSyncWireError::HeightOutOfRange(range.start_height.0))?; + if !self.cached_header_history_tree_matches(parent_height, range.anchor_hash) { + debug!( + ?peer, + start_height = ?range.start_height, + anchor_hash = ?range.anchor_hash, + "Zakura header-sync cannot validate header auxiliary data without parent history tree" + ); + return Err(HeaderSyncWireError::MissingHeaderHistoryTree { + height: parent_height, + hash: range.anchor_hash, + }); + } + + validate_header_aux_commitments( + &self.startup.network, + &self.state.best_header_history_tree, + headers, + tree_aux_roots, + ) + } + + // Returns true if the cached history tree matches the given parent height and hash. + fn cached_header_history_tree_matches( + &self, + parent_height: block::Height, + parent_hash: block::Hash, + ) -> bool { + let height_matches = header_history_tree_is_at_height( + &self.state.best_header_history_tree, + parent_height, + &self.startup.network, + ); + let hash_matches = (parent_height == self.state.best_header_tip + && parent_hash == self.state.best_header_hash) + || (previous_height(self.state.best_header_tip) == Some(parent_height) + && self.state.best_header_parent_hash == Some(parent_hash)); + + height_matches && hash_matches + } + + // Returns the history tree for the pending commit that matches the given start and tip heights. + // Backward (checkpoint-authenticated) commits carry no verified roots, so they never install a + // frontier tree; the caller keeps its existing tree in that case. + fn pending_header_history_tree( + &self, + start_height: block::Height, + tip_height: block::Height, + ) -> Option> { + self.state + .pending_commits + .values() + .find(|commit| { + commit.range.start_height == start_height && commit.range.end_height() == tip_height + }) + .and_then(|commit| commit.verified_roots.as_ref()) + .map(|verified_roots| Arc::new(verified_roots.tree().clone())) + } + async fn handle_possible_stale_anchor_link_failure( &mut self, peer: &ZakuraPeerId, @@ -1244,7 +1453,7 @@ impl HeaderSyncReactor { self.state.schedule.clear_forward(); self.state .pending_commits - .retain(|_, range| range.priority != RangePriority::Forward); + .retain(|_, commit| commit.range.priority != RangePriority::Forward); self.cancel_forward_outstanding(); self.publish_best_tip_reanchored(height, hash).await; } @@ -1451,9 +1660,15 @@ impl HeaderSyncReactor { true } - async fn publish_best_tip(&mut self, height: block::Height, hash: block::Hash) { + async fn publish_best_tip( + &mut self, + height: block::Height, + hash: block::Hash, + parent_hash: Option, + ) { self.state.best_header_tip = height; self.state.best_header_hash = hash; + self.state.best_header_parent_hash = parent_hash; metrics::gauge!("sync.header.best_tip.height").set(height.0 as f64); self.trace_frontier_advanced(height, hash); let _ = self.tip.send((height, hash)); @@ -1466,6 +1681,7 @@ impl HeaderSyncReactor { let old = (self.state.best_header_tip, self.state.best_header_hash); self.state.best_header_tip = height; self.state.best_header_hash = hash; + self.state.best_header_parent_hash = None; metrics::gauge!("sync.header.best_tip.height").set(height.0 as f64); self.trace_frontier_reanchored(height, hash); let _ = self.tip.send((height, hash)); @@ -1676,10 +1892,17 @@ impl HeaderSyncReactor { insert_height(row, "finalized_height", frontiers.finalized_height); insert_height(row, "verified_block_tip", frontiers.verified_block_tip); } + HeaderSyncEvent::BestHeaderHistoryTreeLoaded { + best_header_tip, .. + } => { + insert_optional_str(row, hs_trace::KIND, Some("best_header_history_tree_loaded")); + insert_height(row, hs_trace::BEST_HEADER_TIP, *best_header_tip); + } HeaderSyncEvent::HeaderRangeCommitted { start_height, tip_height, tip_hash, + tip_parent_hash: _, } => { insert_optional_str(row, hs_trace::KIND, Some("header_range_committed")); insert_height(row, hs_trace::RANGE_START, *start_height); @@ -1796,8 +2019,13 @@ impl HeaderSyncReactor { insert_height(row, hs_trace::RANGE_START, *start_height); insert_u64(row, hs_trace::RANGE_COUNT, headers.len() as u64); } - HeaderSyncAction::QueryBestHeaderTip => { - insert_optional_str(row, hs_trace::KIND, Some("query_best_header_tip")); + HeaderSyncAction::QueryBestHeaderHistoryTree { + verified_block_tip, + best_header_tip, + } => { + insert_optional_str(row, hs_trace::KIND, Some("query_best_header_history_tree")); + insert_height(row, hs_trace::VERIFIED_BLOCK_TIP, *verified_block_tip); + insert_height(row, hs_trace::BEST_HEADER_TIP, *best_header_tip); } HeaderSyncAction::QueryMissingBlockBodies { from, limit } => { insert_optional_str(row, hs_trace::KIND, Some("query_missing_block_bodies")); @@ -2173,6 +2401,22 @@ impl HeaderSyncReactor { } } +/// Returns true if the history tree is at the given height. +fn header_history_tree_is_at_height( + tree: &zebra_chain::history_tree::HistoryTree, + height: block::Height, + network: &Network, +) -> bool { + match tree.as_ref().map(|tree| tree.current_height()) { + Some(current_height) => current_height == height, + None => match zebra_chain::parameters::NetworkUpgrade::Heartwood.activation_height(network) + { + Some(heartwood) => height < heartwood, + None => true, + }, + } +} + fn header_sync_wire_error_kind(error: &HeaderSyncWireError) -> &'static str { match error { HeaderSyncWireError::OversizedPayload { .. } => "oversized_payload", @@ -2192,6 +2436,9 @@ fn header_sync_wire_error_kind(error: &HeaderSyncWireError) -> &'static str { HeaderSyncWireError::TrailingBytes => "trailing_bytes", HeaderSyncWireError::NonContiguousHeaders => "non_contiguous_headers", HeaderSyncWireError::FirstHeaderDoesNotLink => "first_header_does_not_link", + HeaderSyncWireError::InvalidHeaderCommitment(_) => "invalid_header_commitment", + HeaderSyncWireError::HistoryTree(_) => "history_tree", + HeaderSyncWireError::MissingHeaderHistoryTree { .. } => "missing_header_history_tree", HeaderSyncWireError::WrongEquihashSolutionSize => "wrong_equihash_solution_size", HeaderSyncWireError::InvalidDifficultyThreshold => "invalid_difficulty_threshold", HeaderSyncWireError::DifficultyFilter { .. } => "difficulty_filter", diff --git a/zebra-network/src/zakura/header_sync/service.rs b/zebra-network/src/zakura/header_sync/service.rs index 385ce9aaeee..f8375a08646 100644 --- a/zebra-network/src/zakura/header_sync/service.rs +++ b/zebra-network/src/zakura/header_sync/service.rs @@ -283,7 +283,7 @@ pub(crate) async fn drive_header_sync_actions( "suppressing Zakura header range commit until state driver is wired" ); } - HeaderSyncAction::QueryBestHeaderTip + HeaderSyncAction::QueryBestHeaderHistoryTree { .. } | HeaderSyncAction::QueryMissingBlockBodies { .. } | HeaderSyncAction::BodyGaps { .. } | HeaderSyncAction::HeaderAdvanced { .. } diff --git a/zebra-network/src/zakura/header_sync/state.rs b/zebra-network/src/zakura/header_sync/state.rs index 7b7826a1c37..3c6a86c651a 100644 --- a/zebra-network/src/zakura/header_sync/state.rs +++ b/zebra-network/src/zakura/header_sync/state.rs @@ -1,3 +1,7 @@ +use std::sync::Arc; + +use zebra_chain::history_tree::HistoryTree; + use super::{error::*, events::*, scheduler::*, validation::*, wire::*, *}; use crate::zakura::{ HeaderSyncServiceSummary, ServicePeerDirection, DEFAULT_LIVE_SERVICE_SUMMARY_TTL, @@ -17,20 +21,31 @@ pub(super) struct HeaderSyncCore { pub(super) verified_block_hash: block::Hash, pub(super) best_header_tip: block::Height, pub(super) best_header_hash: block::Hash, + pub(super) best_header_parent_hash: Option, + /// History tree positioned at the parent of the next forward range. + /// + /// Seeded at startup from durable state and repositioned as ranges commit, so peer-supplied + /// roots can be folded and authenticated against header commitments. The empty tree is the + /// natural pre-Heartwood value. + pub(super) best_header_history_tree: Arc, pub(super) peers: HashMap, pub(super) parked_peers: HashSet, pub(super) seen: HeaderHashDedup, pub(super) pending_new_blocks: HashSet, pub(super) schedule: RangeScheduler, - pub(super) pending_commits: HashMap, + pub(super) pending_commits: HashMap, pub(super) advisory: HashMap, pub(super) stale_anchor: StaleAnchorFailures, + /// True while a `QueryBestHeaderHistoryTree` rebuild is outstanding, so a run of forward ranges + /// that all find the tree stale dispatches only one reload. + pub(super) rebuild_in_flight: bool, } impl HeaderSyncCore { pub(super) fn new(startup: &HeaderSyncStartup) -> Result { validate_anchor(&startup.network, startup.anchor)?; let (best_header_tip, best_header_hash) = startup.best_header_tip.unwrap_or(startup.anchor); + let best_header_history_tree = startup.best_header_history_tree.clone(); Ok(Self { anchor: startup.anchor, @@ -39,6 +54,8 @@ impl HeaderSyncCore { verified_block_hash: startup.frontiers.verified_block_hash, best_header_tip, best_header_hash, + best_header_parent_hash: startup.best_header_parent_hash, + best_header_history_tree, peers: HashMap::new(), parked_peers: HashSet::new(), seen: HeaderHashDedup::default(), @@ -47,6 +64,7 @@ impl HeaderSyncCore { pending_commits: HashMap::new(), advisory: HashMap::new(), stale_anchor: StaleAnchorFailures::default(), + rebuild_in_flight: false, }) } @@ -63,7 +81,42 @@ impl HeaderSyncCore { } let checkpoints = startup.network.checkpoint_list(); - let Some(start) = next_height(self.best_header_tip) else { + // Commitment-root verification, the one-block overlap, and the in-memory header-frontier + // history tree only exist up to the last checkpoint β€” the VCT fast-sync handoff boundary, the + // only region where a consumer reads the persisted roots. Crucially the handoff block *at* + // `last_checkpoint` reads its own persisted root from the roots CF (`vct_roots_at_height` is + // inclusive of `last_checkpoint`); an empty slot there wedges the handoff on the frozen-frontier + // safety check. A root at `H` is only confirmed by the header at `H + 1`, so to persist the + // confirmed root at `last_checkpoint` the root-carrying regime must run until the frontier + // reaches `last_checkpoint + 1`. Above that, header sync runs plain: no roots, no overlap. + let last_checkpoint = startup.last_checkpoint_height; + // The frontier height at which root work stops: one above the last checkpoint (so the range + // that reaches `last_checkpoint` still confirms its root via the `last_checkpoint + 1` header). + let root_regime_end = next_height(last_checkpoint).unwrap_or(last_checkpoint); + // Header sync persists confirmed roots for `(max(anchor, finalized_height) .. last_checkpoint]` + // β€” the below-checkpoint heights this node forward-syncs (above its anchor) that are not yet + // committed. Below the last checkpoint, blocks are checkpoint-verified straight into the + // finalized state, so `finalized_height` is the committed floor VCT consumes roots up to. Root + // work runs while that region is non-empty and the frontier has not passed the confirming + // height (`last_checkpoint + 1`). A node anchored at / already synced through the last + // checkpoint (handoff done via the embedded frontier), and a genesis-only checkpoint list + // (`max_height == 0`), both have an empty region and run plain. + let root_region_floor = self.anchor.0.max(self.finalized_height); + let below_root_boundary = + root_region_floor < last_checkpoint && self.best_header_tip < root_regime_end; + let want_tree_aux_roots = below_root_boundary; + + // Root-carrying ranges leave the best tip's roots unconfirmed, so the next request + // redelivers the tip header anchored at its parent. Overlap only applies in the root regime. + let overlap_forward_range = below_root_boundary + && self + .best_header_parent_hash + .is_some_and(|_| self.best_header_tip > block::Height(0)); + let Some(start) = (if overlap_forward_range { + Some(self.best_header_tip) + } else { + next_height(self.best_header_tip) + }) else { return; }; let mut end = best_peer_tip; @@ -77,6 +130,13 @@ impl HeaderSyncCore { finalized = true; } } + // Cap the root-carrying range at `last_checkpoint + 1`: the range that reaches the checkpoint + // confirms and persists the root at `last_checkpoint` (via the `last_checkpoint + 1` header), + // and the following (above-boundary) ranges run plain. The tip root at `last_checkpoint + 1` is + // itself left unconfirmed and unpersisted β€” VCT never reads it. + if below_root_boundary { + end = end.min(root_regime_end); + } let count = count_between(start, end); if count == 0 { @@ -85,9 +145,14 @@ impl HeaderSyncCore { self.schedule.ensure_forward(RangeRequest { start_height: start, count, - anchor_hash: self.best_header_hash, + anchor_hash: if overlap_forward_range { + self.best_header_parent_hash + .expect("overlapped ranges have a parent hash") + } else { + self.best_header_hash + }, finalized, - want_tree_aux_roots: true, + want_tree_aux_roots, priority: RangePriority::Forward, }); } @@ -123,6 +188,15 @@ impl HeaderSyncCore { } } +#[derive(Clone, Debug)] +pub(super) struct PendingHeaderCommit { + pub(super) range: RangeRequest, + /// `Some` for aux-validated forward ranges; `None` for checkpoint-authenticated backward + /// backfill ranges, which persist no provisional roots and never install a frontier tree. + pub(super) verified_roots: + Option, +} + #[derive(Clone, Debug, Default)] pub(super) struct StaleAnchorFailures { pub(super) count: u32, diff --git a/zebra-network/src/zakura/header_sync/tests.rs b/zebra-network/src/zakura/header_sync/tests.rs index bfe6257b667..85a6a916dc1 100644 --- a/zebra-network/src/zakura/header_sync/tests.rs +++ b/zebra-network/src/zakura/header_sync/tests.rs @@ -16,6 +16,7 @@ use std::{ sync::{Mutex, OnceLock}, }; use zebra_chain::{ + history_tree::HistoryTree, orchard, parallel::commitment_aux::BlockCommitmentRoots, parameters::{ @@ -30,7 +31,7 @@ use zebra_chain::{ }; use zebra_test::vectors::{ BLOCK_MAINNET_1_BYTES, BLOCK_MAINNET_2_BYTES, BLOCK_MAINNET_3_BYTES, BLOCK_MAINNET_4_BYTES, - BLOCK_MAINNET_GENESIS_BYTES, BLOCK_TESTNET_GENESIS_BYTES, + BLOCK_MAINNET_5_BYTES, BLOCK_MAINNET_GENESIS_BYTES, BLOCK_TESTNET_GENESIS_BYTES, }; #[derive(Default)] @@ -245,6 +246,48 @@ fn roots_from_height(start_height: block::Height, count: usize) -> Vec], +) -> Vec { + headers + .iter() + .enumerate() + .map(|(offset, header)| { + let offset = u32::try_from(offset).expect("test root count fits in u32"); + let height = block::Height(start_height.0 + offset); + let sapling_root = match header + .commitment(network, height) + .expect("test header commitment parses") + { + zebra_chain::block::Commitment::FinalSaplingRoot(root) => root, + _ => sapling::tree::NoteCommitmentTree::default().root(), + }; + BlockCommitmentRoots { + height, + sapling_root, + ..root_at(height) + } + }) + .collect() +} + +fn roots_message_from( + _start_height: block::Height, + headers: Vec>, + tree_aux_roots: Vec, +) -> HeaderSyncMessage { + HeaderSyncMessage::Headers { + body_sizes: vec![0; headers.len()], + headers, + tree_aux_roots, + } +} + async fn validate_headers_stateless_after_equihash_acceptance( headers: Vec>, context: HeaderSyncValidationContext<'_>, @@ -322,6 +365,30 @@ fn regtest_network() -> Network { Network::new_regtest(Default::default()) } +fn pre_sapling_checkpoint_regtest( + checkpoint_height: block::Height, + checkpoint_hash: block::Hash, +) -> Network { + let default_regtest = regtest_network(); + Network::new_regtest(RegtestParameters { + activation_heights: ConfiguredActivationHeights { + before_overwinter: Some(10), + ..Default::default() + }, + checkpoints: Some(ConfiguredCheckpoints::HeightsAndHashes(vec![ + (block::Height(0), default_regtest.genesis_hash()), + (checkpoint_height, checkpoint_hash), + ])), + ..Default::default() + }) +} + +fn pre_sapling_test_header(bytes: &[u8], previous_hash: block::Hash) -> Arc { + let mut header = *mainnet_header(bytes); + header.previous_block_hash = previous_hash; + Arc::new(header) +} + fn checkpoint_testnet_with_hash( checkpoint_height: block::Height, checkpoint_hash: block::Hash, @@ -393,6 +460,12 @@ fn startup_for( ); startup.range_state_actions_enabled = true; startup.inbound_new_block_acceptance_enabled = true; + // Test networks (regtest/custom) usually carry only a genesis checkpoint, so their real + // `max_height()` is 0 and every height would count as above the VCT handoff boundary. These + // fixtures exercise the below-checkpoint fast-sync machinery (root verify/persist/overlap), so + // push the boundary above all test heights. Tests that specifically exercise the at/above- + // checkpoint behavior override `last_checkpoint_height` back down. + startup.last_checkpoint_height = block::Height::MAX; startup } @@ -774,8 +847,7 @@ async fn next_non_query_action(actions: &mut mpsc::Receiver) - let action = next_action(actions).await; if !matches!( action, - HeaderSyncAction::QueryBestHeaderTip - | HeaderSyncAction::QueryMissingBlockBodies { .. } + HeaderSyncAction::QueryMissingBlockBodies { .. } | HeaderSyncAction::QueryHeadersByHeightRange { .. } | HeaderSyncAction::HeaderAdvanced { .. } ) { @@ -806,7 +878,9 @@ async fn next_outbound_get_headers( HeaderSyncMessage::GetHeaders { start_height, count, - want_tree_aux_roots: true, + // Accept both root-carrying (below-checkpoint) and plain (at/above-checkpoint) + // forward requests; callers that care about the flag assert it separately. + want_tree_aux_roots: _, }, } => return (peer, start_height, count), HeaderSyncAction::Misbehavior { peer, reason } => { @@ -817,6 +891,25 @@ async fn next_outbound_get_headers( } } +async fn next_forward_get_headers( + actions: &mut mpsc::Receiver, +) -> (block::Height, u32, bool) { + loop { + if let HeaderSyncAction::SendMessage { + msg: + HeaderSyncMessage::GetHeaders { + start_height, + count, + want_tree_aux_roots, + }, + .. + } = next_non_query_action(actions).await + { + return (start_height, count, want_tree_aux_roots); + } + } +} + async fn assert_no_commit_or_misbehavior(actions: &mut mpsc::Receiver) { while let Ok(Some(action)) = tokio::time::timeout(std::time::Duration::from_millis(50), actions.recv()).await @@ -1138,6 +1231,30 @@ fn headers_codec_rejects_body_size_mismatch_truncation_and_trailing_bytes() { )); } +#[test] +fn header_aux_validation_rejects_bad_roots_before_state_commit() { + let header = mainnet_header(&BLOCK_MAINNET_1_BYTES); + let mut roots = root_at(block::Height(1)); + roots.orchard_root = + orchard::tree::Root::try_from([0u8; 32]).expect("zero is a valid Pallas base field"); + + assert_ne!( + roots.orchard_root, + orchard::tree::NoteCommitmentTree::default().root(), + "the bad root must differ from the pre-NU5 empty Orchard root" + ); + + assert!(matches!( + validate_header_aux_commitments( + &Network::Mainnet, + &HistoryTree::default(), + &[header], + &[roots] + ), + Err(HeaderSyncWireError::InvalidHeaderCommitment(_)) + )); +} + #[test] fn decode_rejects_non_empty_headers_without_tree_aux_roots() { let headers = vec![mainnet_header(&BLOCK_MAINNET_1_BYTES)]; @@ -1626,6 +1743,7 @@ async fn scheduler_narrows_large_ranges_before_tracking_fanout() { start_height: start, tip_height: chunk_tip, tip_hash: block::Hash([4; 32]), + tip_parent_hash: Some(block::Hash([3; 32])), }) .await .unwrap(); @@ -1641,10 +1759,7 @@ async fn scheduler_narrows_large_ranges_before_tracking_fanout() { .. } = next_non_query_action(&mut fixture.actions).await { - assert_eq!( - start_height, - next_height(chunk_tip).expect("committed chunk tip has successor") - ); + assert_eq!(start_height, chunk_tip,); assert_eq!(count, clamped_count); break; } @@ -2243,6 +2358,7 @@ async fn covered_hedged_outstanding_ranges_do_not_commit_twice() { start_height: block::Height(1), tip_height: block::Height(2), tip_hash: block::Hash([2; 32]), + tip_parent_hash: None, }) .await .unwrap(); @@ -2300,6 +2416,7 @@ async fn late_covered_response_does_not_reanchor_newer_outstanding_range() { start_height: block::Height(1), tip_height: block::Height(1), tip_hash: committed_hash, + tip_parent_hash: None, }) .await .unwrap(); @@ -2472,6 +2589,7 @@ async fn material_tip_advance_sends_rate_limited_unsolicited_status() { tip_hash: block::Hash( [u8::try_from(height.0).expect("test heights fit in u8"); 32], ), + tip_parent_hash: None, }) .await .unwrap(); @@ -2713,6 +2831,9 @@ async fn reconnect_clears_session_bound_outstanding_ranges() { #[tokio::test(flavor = "current_thread")] async fn full_block_committed_covers_outstanding_height() { let network = regtest_network(); + // A full-block commit covers the outstanding height and advances the header frontier (which tracks + // the committed chain). The in-memory tree is untouched; a later forward range would rebuild it + // lazily if it needed it. let mut fixture = spawn_test_reactor(startup_for( network.clone(), (block::Height(0), network.genesis_hash()), @@ -3904,6 +4025,7 @@ async fn header_sync_metrics_record_status_range_new_block_dedup_and_violation() start_height: next_height(first_checkpoint).expect("checkpoint has a successor"), tip_height: next_height(first_checkpoint).expect("checkpoint has a successor"), tip_hash: committed_hash, + tip_parent_hash: None, }) .await .unwrap(); @@ -4032,6 +4154,7 @@ async fn committed_range_updates_best_tip_watch_and_does_not_advance_finality() start_height: block::Height(1), tip_height: block::Height(1), tip_hash, + tip_parent_hash: None, }) .await .unwrap(); @@ -4041,6 +4164,290 @@ async fn committed_range_updates_best_tip_watch_and_does_not_advance_finality() assert_ne!(fixture.handle.best_header_tip().0, block::Height(0)); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn invalid_header_aux_roots_score_before_commit() { + let default_regtest = regtest_network(); + let block1 = pre_sapling_test_header(&BLOCK_MAINNET_1_BYTES, default_regtest.genesis_hash()); + let block1_hash = block::Hash::from(block1.as_ref()); + let network = pre_sapling_checkpoint_regtest(block::Height(1), block1_hash); + let mut fixture = spawn_test_reactor(startup_for( + network.clone(), + (block::Height(0), network.genesis_hash()), + None, + )); + let peer_id = peer(81); + + connect_peer(&fixture, peer_id.clone()).await; + advertise_tip( + &fixture, + peer_id.clone(), + block::Height(0), + block::Height(1), + DEFAULT_HS_RANGE, + 1, + ) + .await; + + let (served_peer, start_height, count) = next_outbound_get_headers(&mut fixture.actions).await; + assert_eq!(served_peer, peer_id); + assert_eq!(start_height, block::Height(1)); + assert_eq!(count, 1); + + let mut roots = roots_from_height(start_height, 1); + roots[0].sapling_root = + sapling::tree::Root::try_from([1u8; 32]).expect("test Sapling root is valid"); + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id.clone(), + msg: roots_message_from(start_height, vec![block1], roots), + }) + .await + .unwrap(); + + loop { + match next_non_query_action(&mut fixture.actions).await { + HeaderSyncAction::Misbehavior { peer, reason } => { + assert_eq!(peer, peer_id); + assert_eq!(reason, HeaderSyncMisbehavior::InvalidRange); + break; + } + HeaderSyncAction::CommitHeaderRange { .. } => { + panic!("invalid header aux roots must not reach state commit") + } + _ => {} + } + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn valid_header_aux_roots_reach_commit_before_advancing_tip() { + let default_regtest = regtest_network(); + let block1 = pre_sapling_test_header(&BLOCK_MAINNET_1_BYTES, default_regtest.genesis_hash()); + let block1_hash = block::Hash::from(block1.as_ref()); + let network = pre_sapling_checkpoint_regtest(block::Height(1), block1_hash); + let mut fixture = spawn_test_reactor(startup_for( + network.clone(), + (block::Height(0), network.genesis_hash()), + None, + )); + let peer_id = peer(82); + + connect_peer(&fixture, peer_id.clone()).await; + advertise_tip( + &fixture, + peer_id.clone(), + block::Height(0), + block::Height(1), + DEFAULT_HS_RANGE, + 1, + ) + .await; + + let (served_peer, start_height, count) = next_outbound_get_headers(&mut fixture.actions).await; + assert_eq!(served_peer, peer_id); + assert_eq!(start_height, block::Height(1)); + assert_eq!(count, 1); + + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id.clone(), + msg: finalized_headers_message_from(start_height, vec![block1]), + }) + .await + .unwrap(); + + loop { + match next_non_query_action(&mut fixture.actions).await { + HeaderSyncAction::CommitHeaderRange { + peer, + start_height, + headers, + verified_roots, + .. + } => { + assert_eq!(peer, peer_id); + assert_eq!(start_height, block::Height(1)); + assert_eq!(headers.len(), 1); + let verified_roots = + verified_roots.expect("forward header range carries verified roots"); + assert!( + verified_roots.confirmed_roots().is_empty(), + "a one-header range verifies but has no confirmed prefix yet" + ); + assert_eq!(fixture.handle.best_header_tip().0, block::Height(0)); + break; + } + HeaderSyncAction::Misbehavior { peer, reason } => { + panic!("unexpected misbehavior from {peer:?}: {reason:?}"); + } + _ => {} + } + } +} + +/// When the in-memory tree is behind the header frontier β€” here a startup with a durable best header +/// tip (4) but an empty tree, standing in for a non-Zakura commit that ran the frontier ahead of the +/// tree β€” the forward range that needs the tree triggers exactly one `QueryBestHeaderHistoryTree` +/// lazy rebuild (guarded), with no commit and no misbehavior. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn missing_header_aux_tree_triggers_single_lazy_rebuild() { + let block4 = mainnet_header(&BLOCK_MAINNET_4_BYTES); + let block4_hash = block::Hash::from(block4.as_ref()); + let block5 = mainnet_header(&BLOCK_MAINNET_5_BYTES); + let block5_hash = block::Hash::from(block5.as_ref()); + let (network, _) = checkpoint_testnet_with_hash(block::Height(5), block5_hash); + let mut fixture = spawn_test_reactor(startup_for( + network.clone(), + (block::Height(0), network.genesis_hash()), + Some((block::Height(4), block4_hash)), + )); + let peer_id = peer(83); + + connect_peer(&fixture, peer_id.clone()).await; + advertise_tip( + &fixture, + peer_id.clone(), + block::Height(0), + block::Height(5), + DEFAULT_HS_RANGE, + 1, + ) + .await; + + let (served_peer, start_height, count) = next_outbound_get_headers(&mut fixture.actions).await; + assert_eq!(served_peer, peer_id); + assert_eq!(start_height, block::Height(5)); + assert_eq!(count, 1); + + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id, + msg: finalized_headers_message_from(start_height, vec![block5]), + }) + .await + .unwrap(); + + // The stale tree triggers exactly one rebuild at the current frontier; never a commit or a score. + let mut rebuilds = 0; + for _ in 0..16 { + match tokio::time::timeout( + std::time::Duration::from_millis(200), + fixture.actions.recv(), + ) + .await + { + Ok(Some(HeaderSyncAction::QueryBestHeaderHistoryTree { + best_header_tip, .. + })) => { + assert_eq!(best_header_tip, block::Height(4)); + rebuilds += 1; + } + Ok(Some(HeaderSyncAction::CommitHeaderRange { .. })) => { + panic!("a stale tree must not commit the range"); + } + Ok(Some(HeaderSyncAction::Misbehavior { .. })) => { + panic!("a stale tree must not score the peer"); + } + Ok(Some(_)) => {} + _ => break, + } + } + assert_eq!(rebuilds, 1, "a stale tree triggers exactly one lazy rebuild"); +} + +/// A gossiped `NewBlock` accepted ahead of the header frontier advances the header tip β€” the frontier +/// tracks the committed chain. It does not touch the in-memory tree; if this were below the checkpoint +/// and a later forward range needed the tree, the lazy rebuild would reposition it. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn gossip_tip_advance_advances_tip() { + let network = Network::Mainnet; + let block = mainnet_block(&BLOCK_MAINNET_1_BYTES); + let hash = block.hash(); + let height = block.coinbase_height().expect("test block has height"); + let fixture = spawn_test_reactor(startup_for( + network.clone(), + (block::Height(0), network.genesis_hash()), + None, + )); + + fixture + .handle + .send(HeaderSyncEvent::NewBlockAccepted { + peer: peer(84), + height, + hash, + block, + }) + .await + .unwrap(); + + // The tip advances to the gossiped height (the tip watch is updated synchronously by the bump). + let mut tip = fixture.handle.subscribe_tip(); + for _ in 0..16 { + if fixture.handle.best_header_tip().0 == height { + break; + } + let _ = tokio::time::timeout(std::time::Duration::from_millis(100), tip.changed()).await; + } + assert_eq!( + fixture.handle.best_header_tip().0, + height, + "a gossip accepted ahead of the frontier must advance the header tip", + ); +} + +/// The root-carrying regime runs *through* the last checkpoint inclusive β€” the VCT handoff block at +/// `last_checkpoint` reads its own persisted root β€” and stops one block above. Confirming the root at +/// `last_checkpoint` needs the `last_checkpoint + 1` header, so the range is capped there. +#[tokio::test(flavor = "current_thread")] +async fn forward_ranges_request_roots_through_the_last_checkpoint() { + let network = regtest_network(); + let genesis = (block::Height(0), network.genesis_hash()); + let boundary = block::Height(10); + + // Well below the checkpoint: request roots; capped at `last_checkpoint + 1` (heights 1..=11) so the + // range reaching the checkpoint can confirm the root at `last_checkpoint`. + let mut below = startup_for(network.clone(), genesis, None); + below.last_checkpoint_height = boundary; + let mut fixture = spawn_test_reactor(below); + connect_peer(&fixture, peer(70)).await; + advertise_tip(&fixture, peer(70), genesis.0, block::Height(20), DEFAULT_HS_RANGE, 1).await; + let (start, count, want) = next_forward_get_headers(&mut fixture.actions).await; + assert_eq!(start, block::Height(1)); + assert!(want, "a below-checkpoint forward range must request roots"); + assert_eq!(count, 11, "the range is capped one block above the last checkpoint"); + + // Frontier at the last checkpoint: still in the root regime (this range confirms the root at + // `last_checkpoint` via the successor header). + let mut at = startup_for(network.clone(), genesis, Some((boundary, block::Hash([10; 32])))); + at.last_checkpoint_height = boundary; + let mut fixture = spawn_test_reactor(at); + connect_peer(&fixture, peer(71)).await; + advertise_tip(&fixture, peer(71), boundary, block::Height(20), DEFAULT_HS_RANGE, 1).await; + let (_start, _count, want) = next_forward_get_headers(&mut fixture.actions).await; + assert!( + want, + "the range reaching the last checkpoint still requests roots (to confirm its own root)", + ); + + // Frontier one block above the last checkpoint: plain, no roots. + let mut above = startup_for( + network.clone(), + genesis, + Some((block::Height(11), block::Hash([11; 32]))), + ); + above.last_checkpoint_height = boundary; + let mut fixture = spawn_test_reactor(above); + connect_peer(&fixture, peer(72)).await; + advertise_tip(&fixture, peer(72), block::Height(11), block::Height(20), DEFAULT_HS_RANGE, 1).await; + let (start, _count, want) = next_forward_get_headers(&mut fixture.actions).await; + assert_eq!(start, block::Height(12)); + assert!(!want, "above the last checkpoint the forward range is plain"); +} + #[tokio::test(flavor = "current_thread")] async fn forward_link_wedge_reanchors_to_verified_tip_without_banning() { let network = regtest_network(); @@ -4085,9 +4492,12 @@ async fn forward_link_wedge_reanchors_to_verified_tip_without_banning() { .handle .send(HeaderSyncEvent::WireMessage { peer: served_peer, - msg: headers_message_from( + // Above the checkpoint the range carries no roots, so the (non-linking) response must + // be rootless too β€” else it is rejected as malformed before the link check. + msg: roots_message_from( start_height, vec![mainnet_header(&BLOCK_MAINNET_1_BYTES)], + Vec::new(), ), }) .await @@ -4111,8 +4521,10 @@ async fn forward_link_wedge_reanchors_to_verified_tip_without_banning() { msg: HeaderSyncMessage::GetHeaders { start_height, - count: _, - want_tree_aux_roots: true, + // This test exercises reanchor-and-resume, not the root regime; accept either + // flag (regtest's genesis-only `max_height` makes the resumed range's root + // flag a degenerate edge). + .. }, .. } if saw_reanchor_action && start_height == expected_start => { @@ -4170,9 +4582,12 @@ async fn single_peer_forward_link_failures_do_not_reanchor_globally() { .handle .send(HeaderSyncEvent::WireMessage { peer: served_peer, - msg: headers_message_from( + // Above the checkpoint the range carries no roots, so the (non-linking) response must + // be rootless too β€” else it is rejected as malformed before the link check. + msg: roots_message_from( start_height, vec![mainnet_header(&BLOCK_MAINNET_1_BYTES)], + Vec::new(), ), }) .await @@ -4232,11 +4647,15 @@ async fn forward_genesis_backfill_reaches_checkpoint_before_finalized_commit() { } } + // Supply roots matching each header's own Sapling commitment: this multi-block forward range is + // aux-validated (Sapling activates at height 2 on this fixture network), so empty roots would be + // correctly rejected. The commit path is exercised with roots that pass validation. + let matching_roots = header_matching_roots(&network, block::Height(1), &headers); fixture .handle .send(HeaderSyncEvent::WireMessage { peer: peer_id.clone(), - msg: finalized_headers_message(headers.to_vec()), + msg: roots_message_from(block::Height(1), headers.to_vec(), matching_roots), }) .await .unwrap(); diff --git a/zebra-network/src/zakura/header_sync/validation.rs b/zebra-network/src/zakura/header_sync/validation.rs index 6e16519e27a..b1b83aa477a 100644 --- a/zebra-network/src/zakura/header_sync/validation.rs +++ b/zebra-network/src/zakura/header_sync/validation.rs @@ -1,4 +1,8 @@ use super::{error::*, events::*, wire::*, *}; +use zebra_chain::{ + history_tree::HistoryTree, + parallel::commitment_aux_verify::{self, SuppliedRootsError, VerifiedHeaderCommitmentRoots}, +}; pub(super) fn validate_anchor( network: &Network, @@ -193,6 +197,37 @@ pub fn validate_header_range_links( validate_internal_continuity(headers) } +/// Verify supplied roots against the header commitments using `parent_history_tree`. +pub fn validate_header_aux_commitments( + network: &Network, + parent_history_tree: &HistoryTree, + headers: &[Arc], + tree_aux_roots: &[BlockCommitmentRoots], +) -> Result { + validate_tree_aux_roots_len(headers.len(), tree_aux_roots.len())?; + + let items = headers + .iter() + .zip(tree_aux_roots.iter()) + .map(|(header, roots)| (header.as_ref(), roots)); + + commitment_aux_verify::verify_supplied_roots_from_parts( + network, + parent_history_tree.clone(), + items, + ) + .map_err(|(_height, error)| supplied_roots_error_into_wire(error)) +} + +fn supplied_roots_error_into_wire(error: SuppliedRootsError) -> HeaderSyncWireError { + match error { + SuppliedRootsError::InvalidHeaderCommitment(error) => { + HeaderSyncWireError::InvalidHeaderCommitment(error) + } + SuppliedRootsError::HistoryTree(error) => HeaderSyncWireError::HistoryTree(error), + } +} + /// Run all context-free validation checks for an inbound full-block tip flood. #[tracing::instrument(skip(block, network))] pub async fn validate_new_block_stateless( diff --git a/zebra-network/src/zakura/testkit/cluster.rs b/zebra-network/src/zakura/testkit/cluster.rs index ab8539787ad..cc2a3fe8af0 100644 --- a/zebra-network/src/zakura/testkit/cluster.rs +++ b/zebra-network/src/zakura/testkit/cluster.rs @@ -203,6 +203,19 @@ mod tests { } } + /// A `Headers` message carrying no tree-aux roots β€” the correct shape of a response to an + /// above-checkpoint (plain) range. Used by hostile-peer tests whose victims are past the VCT + /// handoff boundary, so a root-carrying response would be rejected as malformed before the + /// intended violation (out-of-range, too-long, bad-PoW, …) is even checked. + fn rootless_headers_message(headers: Vec>) -> HeaderSyncMessage { + let body_sizes = vec![0; headers.len()]; + HeaderSyncMessage::Headers { + headers, + body_sizes, + tree_aux_roots: Vec::new(), + } + } + fn root_at(height: block::Height) -> BlockCommitmentRoots { BlockCommitmentRoots { height, @@ -908,7 +921,10 @@ mod tests { } } HeaderSyncAction::QueryHeadersByHeightRange { - peer, start, count, .. + peer, + start, + count, + want_tree_aux_roots, } => { let headers = local .store @@ -917,11 +933,22 @@ mod tests { .headers_by_range(start, count); let returned_count = u32::try_from(headers.len()).unwrap_or(u32::MAX); if let Some(target) = peer_to_index.get(&peer) { + // Respect the request: an above-checkpoint (plain) range must be served + // without roots, or the requester rejects the response as malformed. + let msg = if want_tree_aux_roots { + headers_message(headers) + } else { + HeaderSyncMessage::Headers { + body_sizes: vec![0; headers.len()], + headers, + tree_aux_roots: Vec::new(), + } + }; let _ = nodes[*target] .handle .send(HeaderSyncEvent::WireMessage { peer: local.peer_id.clone(), - msg: headers_message(headers), + msg, }) .await; let _ = local @@ -962,6 +989,7 @@ mod tests { start_height, tip_height, tip_hash, + tip_parent_hash: None, }) .await; let _ = local @@ -982,18 +1010,18 @@ mod tests { } } } - HeaderSyncAction::QueryBestHeaderTip => { - let (tip_height, tip_hash) = local - .store - .lock() - .expect("test store mutex is not poisoned") - .best_header_tip(); + HeaderSyncAction::QueryBestHeaderHistoryTree { + best_header_tip, .. + } => { + // The e2e header store uses pre-Heartwood vectors, so the empty tree is the + // correct reconstruction; a post-Heartwood tree would be rejected on height. let _ = local .handle - .send(HeaderSyncEvent::HeaderRangeCommitted { - start_height: tip_height, - tip_height, - tip_hash, + .send(HeaderSyncEvent::BestHeaderHistoryTreeLoaded { + best_header_tip, + history_tree: Some(Arc::new( + zebra_chain::history_tree::HistoryTree::default(), + )), }) .await; } @@ -1351,7 +1379,7 @@ mod tests { .send(HeaderSyncEvent::NewBlockDuplicate { peer, height, hash }) .await; } - HeaderSyncAction::QueryBestHeaderTip + HeaderSyncAction::QueryBestHeaderHistoryTree { .. } | HeaderSyncAction::QueryMissingBlockBodies { .. } | HeaderSyncAction::BodyGaps { .. } | HeaderSyncAction::HeaderAdvanced { .. } @@ -3034,7 +3062,9 @@ mod tests { .inject( victim, unsolicited, - headers_message(vec![mainnet_block(&BLOCK_MAINNET_1_BYTES).header.clone()]), + rootless_headers_message(vec![mainnet_block(&BLOCK_MAINNET_1_BYTES) + .header + .clone()]), ) .await; await_until( @@ -3070,7 +3100,9 @@ mod tests { .inject( victim, out_of_range, - headers_message(vec![mainnet_block(&BLOCK_MAINNET_2_BYTES).header.clone()]), + rootless_headers_message(vec![mainnet_block(&BLOCK_MAINNET_2_BYTES) + .header + .clone()]), ) .await; cluster @@ -3113,7 +3145,7 @@ mod tests { .inject( victim, response_too_long, - headers_message(vec![ + rootless_headers_message(vec![ mainnet_block(&BLOCK_MAINNET_1_BYTES).header.clone(), mainnet_block(&BLOCK_MAINNET_2_BYTES).header.clone(), ]), @@ -3151,7 +3183,7 @@ mod tests { .inject( bad_continuity_victim, bad_continuity, - headers_message(vec![ + rootless_headers_message(vec![ mainnet_block(&BLOCK_MAINNET_1_BYTES).header.clone(), Arc::new(non_contiguous), ]), @@ -3183,7 +3215,7 @@ mod tests { .inject( bad_pow_victim, bad_pow, - headers_message(vec![Arc::new(bad_pow_header)]), + rootless_headers_message(vec![Arc::new(bad_pow_header)]), ) .await; cluster @@ -3216,7 +3248,7 @@ mod tests { .inject( bad_daa_victim, bad_daa, - headers_message(vec![ + rootless_headers_message(vec![ mainnet_block(&BLOCK_MAINNET_1_BYTES).header.clone(), mainnet_block(&BLOCK_MAINNET_2_BYTES).header.clone(), mainnet_block(&BLOCK_MAINNET_3_BYTES).header.clone(), diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index b834aa935cd..b7d044b9f69 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -932,10 +932,15 @@ pub enum Request { /// /// A `0` value means unknown. These hints are not consensus data. body_sizes: Vec, - /// Tree-aux roots, parallel to `headers`. + /// The header-authenticated confirmed prefix of the range's tree-aux roots, aligned from the + /// first header's height. /// - /// Every non-empty Zakura header range must provide one root per header. - /// Roots are advisory until verified during block commit. + /// For a semantically-validated forward range this is exactly one shorter than `headers`: + /// the range tip's root is only authenticated once the next range delivers its successor + /// header (the one-block confirmation lag), so the caller omits it and the state persists + /// exactly the roots it is given. Checkpoint-authenticated backfill ranges carry no roots, + /// so an empty vector is also accepted (nothing is persisted). Roots remain advisory + /// (superseded by the verified row) until the block body is committed. tree_aux_roots: Vec, }, @@ -1407,6 +1412,23 @@ pub enum ReadRequest { /// Returns the highest header held on disk. BestHeaderTip, + /// Reconstructs the ZIP-221 history tree positioned at the confirmed header frontier. + /// + /// The durable history tree at `verified_block_tip` is the reconstruction base; the per-height + /// roots stored for `(verified_block_tip, best_header_tip)` β€” exclusive of the header tip, whose + /// root is never persisted β€” are folded onto it. These roots were verified when they were + /// committed, so the fold is direct (no header re-check), and the returned tree is positioned at + /// the confirmed frontier one block behind the header tip. + /// + /// Returns [`ReadResponse::BestHeaderHistoryTree`](ReadResponse::BestHeaderHistoryTree), which + /// also carries the `(height, hash)` the tree is positioned at when a header lead exists. + BestHeaderHistoryTree { + /// Verified block tip whose durable history tree is the reconstruction base. + verified_block_tip: block::Height, + /// Header tip; roots are folded up to the block below it (its own root is unpersisted). + best_header_tip: block::Height, + }, + /// Returns header-known, body-missing heights in `(verified_block_tip, best_header_tip]`. MissingBlockBodies { /// First height to consider. @@ -1635,6 +1657,7 @@ impl ReadRequest { ReadRequest::HeadersByHeightRange { .. } => "headers_by_height_range", ReadRequest::BlockRoots { .. } => "block_roots", ReadRequest::BestHeaderTip => "best_header_tip", + ReadRequest::BestHeaderHistoryTree { .. } => "best_header_history_tree", ReadRequest::MissingBlockBodies { .. } => "missing_block_bodies", ReadRequest::MissingBlockBodyMetadata { .. } => "missing_block_body_metadata", ReadRequest::BlockSizeHints { .. } => "block_size_hints", diff --git a/zebra-state/src/response.rs b/zebra-state/src/response.rs index 80991e95d54..e4b678b1cfe 100644 --- a/zebra-state/src/response.rs +++ b/zebra-state/src/response.rs @@ -395,6 +395,18 @@ pub enum ReadResponse { /// Response to [`ReadRequest::BestHeaderTip`]. BestHeaderTip(Option<(block::Height, block::Hash)>), + /// Response to [`ReadRequest::BestHeaderHistoryTree`]. + /// + /// `tree` is positioned at `frontier`, the highest contiguous confirmed header-root height the + /// fold could reach. Callers use `frontier` as the overlap anchor and resume header sync from the + /// next height. + BestHeaderHistoryTree { + /// The reconstructed history tree, positioned at `frontier`. + tree: Arc, + /// The `(height, hash)` the tree is positioned at (the confirmed contiguous frontier). + frontier: (block::Height, block::Hash), + }, + /// Response to [`ReadRequest::MissingBlockBodies`]. MissingBlockBodies(Vec), @@ -594,6 +606,7 @@ impl TryFrom for Response { | ReadResponse::ChainInfo(_) | ReadResponse::Headers(_) | ReadResponse::BestHeaderTip(_) + | ReadResponse::BestHeaderHistoryTree { .. } | ReadResponse::MissingBlockBodies(_) | ReadResponse::MissingBlockBodyMetadata(_) | ReadResponse::BlockSizeHints(_) diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 4641208fc79..1473086d30b 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -1611,6 +1611,146 @@ where roots } +/// Reconstructs the ZIP-221 history tree positioned at the *confirmed* header frontier. +/// +/// The durable history tree at `verified_block_tip` is the base. The per-height roots stored for +/// `(verified_block_tip, best_header_tip)` are folded onto it in bounded windows β€” note the range +/// is **exclusive** of `best_header_tip`. A committed forward range never persists the root of its +/// own tip block: that root is only authenticated by the *next* range's first header (the one-block +/// confirmation lag), so the highest durable root is at `best_header_tip - 1`. The returned tree is +/// therefore positioned at the confirmed frontier (`best_header_tip - 1` when the header tip leads +/// the verified body tip, otherwise the base). +/// +/// This mirrors the running reactor, which keeps its in-memory tree one block behind the header tip +/// and re-validates the tip's root through the overlapping next forward range. A restart that folded +/// the tip's root here would instead *trust* an unauthenticated peer-supplied root, so we stop one +/// block short and let the overlap re-validate it. +/// +/// Errors only if the base history tree at `verified_block_tip` is missing at a real verified tip, +/// or a fold fails. A missing header/root *above* the base is not an error but the contiguous +/// frontier, where folding stops. +/// +/// Alongside the tree, returns the frontier `(height, hash)` the tree is positioned at: the highest +/// contiguous confirmed header-root height reached (the verified base when nothing folds). The +/// caller seeds `best_header_parent_hash` and resumes header sync from `frontier + 1` off this one +/// authoritative value, so a one-block gap in the persisted roots resumes from the gap rather than +/// capping all the way back to the verified tip. +fn best_header_history_tree( + chain: Option, + db: &ZebraDb, + network: &Network, + verified_block_tip: block::Height, + best_header_tip: block::Height, +) -> Result< + ( + Arc, + (block::Height, block::Hash), + ), + BoxError, +> +where + C: AsRef + Clone, +{ + use zebra_chain::{history_tree::HistoryTree, parallel::commitment_aux_verify}; + + // Base tree as of the verified block tip. A missing tree is only legitimate for the empty or + // genesis state (height 0); at any real verified tip the state always holds a tree (empty + // pre-Heartwood, non-empty after), so a `None` there is an inconsistency we must not paper over + // with an empty base β€” that would silently stall (or, once folding starts, panic) verification. + let base_tree = match read::tree::history_tree(chain.clone(), db, verified_block_tip.into()) { + Some(tree) => tree, + None if verified_block_tip == block::Height(0) => Arc::new(HistoryTree::default()), + None => { + return Err(format!( + "cannot rebuild header-tip history tree: no history tree at verified block tip \ + {verified_block_tip:?}" + ) + .into()) + } + }; + + // Construct the MMR tree up to + // this height. + let mmr_tree_height_target = if best_header_tip > verified_block_tip { + block::Height(best_header_tip.0 - 1) + } else { + verified_block_tip + }; + + let mut tree = (*base_tree).clone(); + let mut next = verified_block_tip + .next() + .map_err(|_| BoxError::from("verified block tip height overflow"))?; + + // The (height, hash) of the last folded header β€” the contiguous frontier the tree ends at. + // Stays `None` while the tree is still at the base. + let mut reconstructed_frontier: Option<(block::Height, block::Hash)> = None; + + while next <= mmr_tree_height_target { + // `+ 1` because the range is inclusive of `mmr_tree_height_target`. + let remaining = mmr_tree_height_target.0 - next.0 + 1; + let count = remaining.min(MAX_HEADER_SYNC_HEIGHT_RANGE); + + let headers = headers_by_height_range(chain.clone(), db, next, count); + let roots = block_roots_by_height_range(chain.clone(), db, next, count); + + // Both reads return a contiguous prefix from `next`, so the number of aligned `(header, + // root)` pairs is the shorter length. A short read means the persisted roots stop here (a + // gap, or simply the frontier): fold what is contiguous and stop β€” this is not an error. + let contiguous = headers.len().min(roots.len()); + if contiguous > 0 { + reconstructed_frontier = headers + .get(contiguous - 1) + .map(|(height, hash, _header)| (*height, *hash)); + + let roots_by_height = headers + .iter() + .zip(roots.iter()) + .take(contiguous) + .map(|((_height, _hash, header), root)| (header.as_ref(), root)); + + tree = commitment_aux_verify::append_confirmed_roots(network, tree, roots_by_height) + .map_err(|(height, error)| { + BoxError::from(format!( + "failed to fold header-tip history tree at {height:?}: {error}" + )) + })?; + } + + // `count` is capped at `MAX_HEADER_SYNC_HEIGHT_RANGE`. + if contiguous < count as usize { + break; + } + + let Some(advanced_blocks) = next.0.checked_add(count) else { + break; + }; + next = block::Height(advanced_blocks); + } + + // The frontier is the last folded height, or the verified base when nothing folded. The base + // always has a durable header hash (the verified tip is committed); genesis is the sole + // empty-state case. + let frontier = match reconstructed_frontier { + Some(frontier) => frontier, + None => { + let base_hash = read::hash_by_height(chain.clone(), db, verified_block_tip) + .or_else(|| { + (verified_block_tip == block::Height(0)).then(|| network.genesis_hash()) + }) + .ok_or_else(|| { + BoxError::from(format!( + "cannot rebuild header-tip history tree: no header hash at verified block \ + tip {verified_block_tip:?}" + )) + })?; + (verified_block_tip, base_hash) + } + }; + + Ok((Arc::new(tree), frontier)) +} + impl Service for ReadStateService { type Response = ReadResponse; type Error = BoxError; @@ -1877,6 +2017,20 @@ impl Service for ReadStateService { )) } + ReadRequest::BestHeaderHistoryTree { + verified_block_tip, + best_header_tip, + } => { + let (tree, frontier) = best_header_history_tree( + state.latest_best_chain(), + &state.db, + &state.db.network(), + verified_block_tip, + best_header_tip, + )?; + Ok(ReadResponse::BestHeaderHistoryTree { tree, frontier }) + } + ReadRequest::MissingBlockBodies { from, limit } => { let verified_block_tip = read::tip_height(state.latest_best_chain(), &state.db); let best_header_tip = state diff --git a/zebra-state/src/service/finalized_state/zebra_db/block.rs b/zebra-state/src/service/finalized_state/zebra_db/block.rs index 8feead51841..dc64a28448d 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/block.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/block.rs @@ -1957,12 +1957,22 @@ impl DiskWriteBatch { headers: &[Arc], body_sizes: &[u32], ) -> Result { - let roots = inferred_header_range_roots(zebra_db, anchor, headers.len())?; + // The commit path only accepts the confirmed prefix (one root shorter than the headers, and + // none for a single-header range), so infer exactly that many placeholder roots. + let roots = inferred_header_range_roots(zebra_db, anchor, headers.len().saturating_sub(1))?; self.prepare_header_range_batch_with_roots(zebra_db, anchor, headers, body_sizes, &roots) } /// Prepare a database batch containing a contextually validated header range - /// and one provisional tree-aux root per header. + /// and the tree-aux roots supplied for it. + /// + /// `tree_aux_roots` is the caller's confirmed prefix, aligned from the range start. For a + /// semantically-validated forward range it is exactly one shorter than `headers` (zero roots + /// for a single-header range): the range tip's root is only authenticated by the next range's + /// successor header, so it is never confirmed here. Checkpoint-authenticated backfill ranges + /// pass an empty vector and persist no roots. Both shapes are enforced, not merely expected, so + /// it is structurally impossible to persist the unconfirmed tip root and expose peer-supplied + /// data to startup history-tree reconstruction. #[allow(clippy::unwrap_in_result)] pub fn prepare_header_range_batch_with_roots( &mut self, @@ -1983,7 +1993,17 @@ impl DiskWriteBatch { }); } - if headers.len() != tree_aux_roots.len() { + // A semantically-validated header range carries its *confirmed prefix* of roots: header + // `H + 1` authenticates the root for `H`, so over `[start..=tip]` the roots confirmed are + // `[start..=tip - 1]` and the tip's own root stays unconfirmed until the next overlapping + // range delivers its successor header. That prefix is always exactly one shorter than the + // headers (zero roots for a single-header range). + // + // Checkpoint-authenticated backfill ranges (backward ranges) carry no provisional roots at + // all, so an empty vector is also accepted. Both accepted shapes keep the trust boundary a + // state invariant: the unconfirmed tip root can never be persisted, because a full-length + // (or otherwise-longer) vector is still rejected, and an empty vector persists nothing. + if !(tree_aux_roots.is_empty() || tree_aux_roots.len() + 1 == headers.len()) { return Err(CommitHeaderRangeError::TreeAuxRootCountMismatch { headers: headers.len(), roots: tree_aux_roots.len(), @@ -2038,12 +2058,15 @@ impl DiskWriteBatch { .ok_or(CommitHeaderRangeError::HeightOverflow)?; let hash = block::Hash::from(&**header); let body_size = body_sizes[index]; - let roots = &tree_aux_roots[index]; - if roots.height != height { - return Err(CommitHeaderRangeError::TreeAuxRootHeightMismatch { - expected_height: height, - root_height: roots.height, - }); + // The tip's root is omitted from the confirmed prefix, so the last header may have no + // matching root. Present roots must align with their header height. + if let Some(roots) = tree_aux_roots.get(index) { + if roots.height != height { + return Err(CommitHeaderRangeError::TreeAuxRootHeightMismatch { + expected_height: height, + root_height: roots.height, + }); + } } if let Some(expected) = checkpoints.hash(height) { @@ -2186,22 +2209,24 @@ impl DiskWriteBatch { // only ever runs above the body tip β€” so a header range re-delivered // over committed heights (a header store behind the body store, or a // late range response racing body sync) must never overwrite the - // verified row: committed roots win on any overlap (design Β§9). - if !zebra_db.contains_body_at_height(height) { - let roots = &tree_aux_roots[index]; - self.zs_insert( - &roots_by_height, - height, - CommitmentRootsByHeight { - sapling: roots.sapling_root, - orchard: roots.orchard_root, - ironwood: roots.ironwood_root, - sapling_tx: roots.sapling_tx, - orchard_tx: roots.orchard_tx, - ironwood_tx: roots.ironwood_tx, - auth_data_root: roots.auth_data_root, - }, - ); + // verified row: committed roots win on any overlap (design Β§9). The tip's root is absent + // from the confirmed prefix (`tree_aux_roots.get` is `None`), so it is never persisted. + if let Some(roots) = tree_aux_roots.get(index) { + if !zebra_db.contains_body_at_height(height) { + self.zs_insert( + &roots_by_height, + height, + CommitmentRootsByHeight { + sapling: roots.sapling_root, + orchard: roots.orchard_root, + ironwood: roots.ironwood_root, + sapling_tx: roots.sapling_tx, + orchard_tx: roots.orchard_tx, + ironwood_tx: roots.ironwood_tx, + auth_data_root: roots.auth_data_root, + }, + ); + } } } diff --git a/zebra-state/src/service/finalized_state/zebra_db/block/tests/vectors.rs b/zebra-state/src/service/finalized_state/zebra_db/block/tests/vectors.rs index 92230f2e8d1..7231137302b 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/block/tests/vectors.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/block/tests/vectors.rs @@ -625,6 +625,10 @@ fn header_range_roots_do_not_overwrite_committed_serving_index_rows() { false, ); + let block2 = zebra_test::vectors::BLOCK_MAINNET_2_BYTES + .zcash_deserialize_into::>() + .expect("block 2 deserializes"); + write_full_block(&mut state, genesis.clone()); write_full_block(&mut state, block1.clone()); @@ -637,7 +641,9 @@ fn header_range_roots_do_not_overwrite_committed_serving_index_rows() { // Re-deliver the real header for the committed height, but with garbage roots β€” // exactly what a malicious serving peer can put in a `Headers` response, since - // root bytes are unauthenticated at header-commit time. + // root bytes are unauthenticated at header-commit time. The committed height must be a confirmed + // (non-tip) root of the range, so re-deliver it as the first header of a two-header range whose + // confirmed prefix is exactly the poisoned height-1 root. let mut poisoned = root_at(Height(1)); poisoned.sapling_tx = 99; poisoned.auth_data_root = zebra_chain::block::merkle::AuthDataRoot::from([0xAA; 32]); @@ -647,8 +653,8 @@ fn header_range_roots_do_not_overwrite_committed_serving_index_rows() { .prepare_header_range_batch_with_roots( &state, genesis.hash(), - std::slice::from_ref(&block1.header), - &[0], + &[block1.header.clone(), block2.header.clone()], + &[0, 0], &[poisoned], ) .expect("re-delivering the same header over a committed height is accepted"); @@ -663,6 +669,71 @@ fn header_range_roots_do_not_overwrite_committed_serving_index_rows() { ); } +/// A header-range commit persists exactly the confirmed prefix of roots it is given: the range +/// tip's root is unauthenticated (only confirmed once the next range delivers its successor header), +/// so the caller omits it and the state must not fabricate or persist one. A tip root left on disk +/// would be folded, unverified, into the startup history-tree reconstruction. +#[test] +fn header_range_commit_persists_only_the_confirmed_root_prefix() { + let _init_guard = zebra_test::init(); + let (state, genesis, block1) = mainnet_state_with_genesis(); + let block2 = mainnet_block(2); + + let headers = vec![block1.header.clone(), block2.header.clone()]; + // Confirmed prefix: one root for the two-header range. The tip (height 2) root is omitted. + let confirmed_roots = vec![root_at(Height(1))]; + + let mut batch = DiskWriteBatch::new(); + batch + .prepare_header_range_batch_with_roots( + &state, + genesis.hash(), + &headers, + &[0, 0], + &confirmed_roots, + ) + .expect("a confirmed prefix one shorter than the headers is accepted"); + state + .write_batch(batch) + .expect("header range batch writes successfully"); + + // Both headers are durable and the header tip advances to the range tip... + assert_eq!(state.best_header_tip(), Some((Height(2), block2.hash()))); + // ...but only the confirmed height has a persisted root; the tip root is absent. + assert_eq!( + state.zakura_header_commitment_roots_by_height_range(Height(1)..=Height(1)), + vec![root_at(Height(1))], + "the confirmed height keeps its provisional root", + ); + assert!( + state + .zakura_header_commitment_roots_by_height_range(Height(2)..=Height(2)) + .is_empty(), + "the unconfirmed range tip must not have a persisted root", + ); + + // An empty vector is the checkpoint-authenticated backfill shape: it is accepted and persists no + // provisional roots, so the trust boundary is never crossed. + let mut batch = DiskWriteBatch::new(); + batch + .prepare_header_range_batch_with_roots(&state, genesis.hash(), &headers, &[0, 0], &[]) + .expect("an empty root vector is accepted for checkpoint-authenticated backfill"); + + // A full-length vector would include the unauthenticated tip root, so it is rejected. + let mut batch = DiskWriteBatch::new(); + let full_roots = vec![root_at(Height(1)), root_at(Height(2))]; + assert!(matches!( + batch.prepare_header_range_batch_with_roots( + &state, + genesis.hash(), + &headers, + &[0, 0], + &full_roots, + ), + Err(CommitHeaderRangeError::TreeAuxRootCountMismatch { .. }), + )); +} + /// Pruning-readiness guard: a committed height whose body is removed (as online /// pruning deletes `tx_by_loc` rows) keeps its header readable from the retained /// consensus `block_header_by_height`, because the header readers are not gated diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index 8f363c28cdb..f7a94cb9819 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -13,6 +13,7 @@ use zebra_chain::{ block::{self, Block, CountedHeader, Height}, chain_tip::ChainTip, fmt::SummaryDebug, + history_tree::HistoryTree, orchard, parallel::commitment_aux::BlockCommitmentRoots, parameters::{Network, NetworkUpgrade}, @@ -26,10 +27,17 @@ use zebra_test::{prelude::*, transcript::Transcript}; use crate::{ arbitrary::Prepare, + constants::{state_database_format_version_in_code, STATE_DATABASE_KIND}, init_test, + request::{FinalizedBlock, Treestate}, service::{ - arbitrary::populated_state, chain_tip::TipAction, headers_by_height_range, - non_finalized_state::Chain, read, StateService, + arbitrary::populated_state, + best_header_history_tree, + chain_tip::TipAction, + finalized_state::{DiskWriteBatch, WriteDisk, ZebraDb, STATE_COLUMN_FAMILIES_IN_CODE}, + headers_by_height_range, + non_finalized_state::Chain, + read, StateService, }, tests::{ setup::{partial_nu5_chain_strategy, transaction_v4_from_coinbase}, @@ -56,6 +64,94 @@ fn roots_from_height(start: Height, count: u32) -> Vec { .collect() } +fn root_at(height: Height) -> BlockCommitmentRoots { + roots_from_height(height, 1) + .into_iter() + .next() + .expect("one root was requested") +} + +fn mainnet_block(height: u32) -> Arc { + zebra_test::vectors::MAINNET_BLOCKS + .get(&height) + .expect("mainnet test vector exists") + .zcash_deserialize_into::>() + .expect("mainnet test vector deserializes") +} + +fn mainnet_state_with_genesis() -> (ZebraDb, Arc, Arc) { + let genesis = mainnet_block(0); + let block1 = mainnet_block(1); + let state = new_ephemeral_db(&Network::Mainnet); + + write_full_block_header_and_transactions(&state, genesis.clone()); + + (state, genesis, block1) +} + +fn new_ephemeral_db(network: &Network) -> ZebraDb { + ZebraDb::new( + &Config::ephemeral(), + STATE_DATABASE_KIND, + &state_database_format_version_in_code(), + network, + true, + STATE_COLUMN_FAMILIES_IN_CODE + .iter() + .map(ToString::to_string), + false, + ) +} + +fn write_full_block_header_and_transactions(state: &ZebraDb, block: Arc) { + let checkpoint_verified = CheckpointVerifiedBlock::from(block); + let finalized = + FinalizedBlock::from_checkpoint_verified(checkpoint_verified, Treestate::default()); + + let mut batch = DiskWriteBatch::new(); + batch + .prepare_block_header_and_transaction_data_batch(state, &finalized, true, None) + .expect("full block header and transaction batch is valid"); + state.write_batch(batch).expect("full block batch writes"); +} + +fn commit_header_range_with_roots( + state: &ZebraDb, + anchor: block::Hash, + headers: &[Arc], + roots: &[BlockCommitmentRoots], +) -> block::Hash { + let mut batch = DiskWriteBatch::new(); + let body_sizes = vec![0; headers.len()]; + let committed_hash = batch + .prepare_header_range_batch_with_roots(state, anchor, headers, &body_sizes, roots) + .expect("header range with roots is valid"); + state + .write_batch(batch) + .expect("header range batch writes successfully"); + + committed_hash +} + +fn insert_zakura_header(state: &ZebraDb, height: Height, header: Arc) { + let header_by_height = state.db().cf_handle("zakura_header_by_height").unwrap(); + let hash_by_height = state + .db() + .cf_handle("zakura_header_hash_by_height") + .unwrap(); + let height_by_hash = state + .db() + .cf_handle("zakura_header_height_by_hash") + .unwrap(); + let hash = block::Hash::from(header.as_ref()); + let mut batch = DiskWriteBatch::new(); + + batch.zs_insert(&header_by_height, height, &header); + batch.zs_insert(&hash_by_height, height, hash); + batch.zs_insert(&height_by_hash, hash, height); + state.write_batch(batch).expect("test header rows write"); +} + async fn test_populated_state_responds_correctly( mut state: Buffer, Request>, ) -> Result<()> { @@ -535,7 +631,7 @@ async fn header_only_service_requests_preserve_body_boundary() -> std::result::R anchor: genesis.hash(), headers: vec![block1.header.clone(), block2.header.clone()], body_sizes: vec![999_999, 0], - tree_aux_roots: roots_from_height(Height(1), 2), + tree_aux_roots: roots_from_height(Height(1), 1), }) .await?, Response::Committed(block2_hash), @@ -787,7 +883,7 @@ async fn commit_header_range_completes_while_in_finalized_write_phase( anchor: genesis.hash(), headers: vec![block1.header.clone(), block2.header.clone()], body_sizes: vec![999_999, 0], - tree_aux_roots: roots_from_height(Height(1), 2), + tree_aux_roots: roots_from_height(Height(1), 1), }), ) .await @@ -861,6 +957,311 @@ async fn header_range_reads_include_non_finalized_best_chain_blocks() -> Result< Ok(()) } +#[test] +fn best_header_history_tree_uses_genesis_base_when_no_header_lead() { + let _init_guard = zebra_test::init(); + let (state, genesis, _block1) = mainnet_state_with_genesis(); + + let (tree, frontier) = best_header_history_tree( + None::>, + &state, + &Network::Mainnet, + Height(0), + Height(0), + ) + .expect("genesis-only state has a valid base history tree"); + + assert_eq!(frontier, (Height(0), genesis.hash())); + assert_eq!(tree.as_ref(), &HistoryTree::default()); +} + +#[test] +fn best_header_history_tree_stops_one_block_before_header_tip() { + let _init_guard = zebra_test::init(); + let (state, genesis, block1) = mainnet_state_with_genesis(); + let block2 = mainnet_block(2); + + commit_header_range_with_roots( + &state, + genesis.hash(), + &[block1.header.clone(), block2.header.clone()], + &[root_at(Height(1))], + ); + + let (tree, frontier) = best_header_history_tree( + None::>, + &state, + &Network::Mainnet, + Height(0), + Height(2), + ) + .expect("contiguous confirmed root reconstructs from genesis"); + + assert_eq!(frontier, (Height(1), block1.hash())); + assert_ne!(frontier, (Height(2), block2.hash())); + assert_eq!( + tree.hash(), + None, + "pre-Heartwood mainnet roots should not create a history tree", + ); +} + +#[test] +fn best_header_history_tree_stops_at_first_missing_root_or_header() { + let _init_guard = zebra_test::init(); + let (state, genesis, block1) = mainnet_state_with_genesis(); + let block2 = mainnet_block(2); + let block3 = mainnet_block(3); + + commit_header_range_with_roots( + &state, + genesis.hash(), + &[ + block1.header.clone(), + block2.header.clone(), + block3.header.clone(), + ], + &[root_at(Height(1)), root_at(Height(2))], + ); + state + .delete_zakura_header_commitment_roots([Height(2)]) + .expect("test root deletion succeeds"); + + let (_tree, frontier) = best_header_history_tree( + None::>, + &state, + &Network::Mainnet, + Height(0), + Height(3), + ) + .expect("reconstruction succeeds up to the contiguous prefix"); + + assert_eq!( + frontier, + (Height(1), block1.hash()), + "reconstruction stops at the last contiguous root before the gap", + ); +} + +#[test] +fn best_header_history_tree_returns_base_when_no_contiguous_fold_exists() { + let _init_guard = zebra_test::init(); + let (state, genesis, block1) = mainnet_state_with_genesis(); + let block2 = mainnet_block(2); + + commit_header_range_with_roots( + &state, + genesis.hash(), + &[block1.header.clone(), block2.header.clone()], + &[root_at(Height(1))], + ); + state + .delete_zakura_header_commitment_roots([Height(1)]) + .expect("test root deletion succeeds"); + + let (tree, frontier) = best_header_history_tree( + None::>, + &state, + &Network::Mainnet, + Height(0), + Height(2), + ) + .expect("missing roots above genesis leave the base frontier"); + + assert_eq!(frontier, (Height(0), genesis.hash())); + assert_eq!(tree.as_ref(), &HistoryTree::default()); +} + +#[test] +fn best_header_history_tree_errors_on_missing_real_base_tree() { + let _init_guard = zebra_test::init(); + let (state, _genesis, block1) = mainnet_state_with_genesis(); + insert_zakura_header(&state, Height(1), block1.header.clone()); + + let error = best_header_history_tree( + None::>, + &state, + &Network::Mainnet, + Height(1), + Height(1), + ) + .expect_err("a real verified tip without a history tree is inconsistent"); + + assert!( + error + .to_string() + .contains("no history tree at verified block tip"), + "unexpected error: {error}", + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn best_header_history_tree_read_request_returns_reconstructed_frontier() { + let _init_guard = zebra_test::init(); + let network = Network::Mainnet; + let (mut state_service, read_state, _, _) = + StateService::new(Config::ephemeral(), &network, Height::MAX, 0).await; + let block1 = mainnet_block(1); + let block2 = mainnet_block(2); + let genesis = mainnet_block(0); + + state_service + .ready() + .await + .expect("state service is ready") + .call(Request::CommitCheckpointVerifiedBlock( + CheckpointVerifiedBlock::from(genesis.clone()), + )) + .await + .expect("genesis block commits"); + + state_service + .ready() + .await + .expect("state service is ready") + .call(Request::CommitHeaderRange { + anchor: genesis.hash(), + headers: vec![block1.header.clone(), block2.header.clone()], + body_sizes: vec![0, 0], + tree_aux_roots: roots_from_height(Height(1), 1), + }) + .await + .expect("header range commits"); + + let response = read_state + .oneshot(ReadRequest::BestHeaderHistoryTree { + verified_block_tip: Height(0), + best_header_tip: Height(2), + }) + .await + .expect("best header history tree read succeeds"); + + let ReadResponse::BestHeaderHistoryTree { tree, frontier } = response else { + panic!("unexpected response: {response:?}"); + }; + + assert_eq!( + frontier, + (Height(1), block1.hash()), + "the read service returns the same confirmed frontier as the helper", + ); + assert_eq!( + tree.hash(), + None, + "pre-Heartwood mainnet roots should not create a history tree", + ); +} + +/// A block-level reorg below the header frontier: when a full block commits a header that conflicts +/// with the provisional header-sync chain, the conflicting provisional headers *and* their roots are +/// dropped, so no stale peer-supplied root survives to be folded by history-tree reconstruction. +#[test] +fn block_reorg_drops_conflicting_provisional_roots_before_reconstruction() { + let _init_guard = zebra_test::init(); + let (state, genesis, block1) = mainnet_state_with_genesis(); + let block2 = mainnet_block(2); + let block3 = mainnet_block(3); + + // Header sync races ahead on chain A: provisional headers 1..=3 with the confirmed root prefix + // (heights 1..=2; the tip's root is never persisted). + commit_header_range_with_roots( + &state, + genesis.hash(), + &[ + block1.header.clone(), + block2.header.clone(), + block3.header.clone(), + ], + &[root_at(Height(1)), root_at(Height(2))], + ); + assert_eq!( + state + .zakura_header_commitment_roots_by_height_range(Height(1)..=Height(2)) + .len(), + 2, + "chain A provisional roots are persisted before the reorg", + ); + + // A full block commits at height 1 with a header that conflicts with chain A (a reorg). The body + // commit path must drop chain A's provisional descendants and their roots β€” nothing carries a + // committed body yet, so the whole provisional suffix is reconciled away. + let mut conflicting_header = *block1.header; + conflicting_header.previous_block_hash = block::Hash([0x5a; 32]); + let conflicting_block = Arc::new(Block { + header: Arc::new(conflicting_header), + transactions: block1.transactions.clone(), + }); + assert_ne!(conflicting_block.header, block1.header); + + state + .seed_zakura_header_from_committed_block(Height(1), &conflicting_block) + .expect("conflicting body commit reconciles the provisional header store"); + + // Chain A's provisional roots at every height are gone: nothing stale survives to be folded. + assert!( + state + .zakura_header_commitment_roots_by_height_range(Height(1)..=Height(3)) + .is_empty(), + "a conflicting commit must drop all chain A provisional roots", + ); + + // Reconstruction over the reorged state folds nothing stale and returns the verified base. + let (tree, frontier) = best_header_history_tree( + None::>, + &state, + &Network::Mainnet, + Height(0), + Height(1), + ) + .expect("reconstruction succeeds after the reorg"); + assert_eq!(frontier, (Height(0), genesis.hash())); + assert_eq!(tree.as_ref(), &HistoryTree::default()); +} + +/// The tightened state invariant is enforced through the real request path: a `CommitHeaderRange` +/// carrying a full-length roots vector (one per header, *including* the unconfirmed tip) is rejected, +/// so a future or refactored caller cannot persist the unconfirmed tip root by mistake. +#[tokio::test(flavor = "multi_thread")] +async fn commit_header_range_rejects_full_length_roots_through_the_service() { + let _init_guard = zebra_test::init(); + let network = Network::Mainnet; + let (mut state_service, _read_state, _, _) = + StateService::new(Config::ephemeral(), &network, Height::MAX, 0).await; + let genesis = mainnet_block(0); + let block1 = mainnet_block(1); + let block2 = mainnet_block(2); + + state_service + .ready() + .await + .expect("state service is ready") + .call(Request::CommitCheckpointVerifiedBlock( + CheckpointVerifiedBlock::from(genesis.clone()), + )) + .await + .expect("genesis block commits"); + + // Full-length roots: one per header, including the range tip (height 2). Only the confirmed + // prefix (height 1) may be persisted, so the boundary must reject this shape outright. + let error = state_service + .ready() + .await + .expect("state service is ready") + .call(Request::CommitHeaderRange { + anchor: genesis.hash(), + headers: vec![block1.header.clone(), block2.header.clone()], + body_sizes: vec![0, 0], + tree_aux_roots: roots_from_height(Height(1), 2), + }) + .await + .expect_err("a full-length roots vector must be rejected by the tightened boundary"); + + assert!( + error.to_string().contains("does not match header count"), + "unexpected error: {error}", + ); +} + #[test] fn state_behaves_when_blocks_are_committed_in_order() -> Result<()> { let _init_guard = zebra_test::init(); diff --git a/zebrad/src/commands/start.rs b/zebrad/src/commands/start.rs index d3a6799870d..c099816cc05 100644 --- a/zebrad/src/commands/start.rs +++ b/zebrad/src/commands/start.rs @@ -2109,7 +2109,7 @@ mod zakura_header_sync_driver_tests { use futures::stream::{FuturesUnordered, StreamExt}; use tokio::sync::mpsc; - use tower::{service_fn, util::BoxService, ServiceExt}; + use tower::{service_fn, util::BoxService, Service, ServiceExt}; use zebra_chain::serialization::ZcashDeserializeInto; use zebra_chain::{block, orchard, parallel::commitment_aux::BlockCommitmentRoots, sapling}; use zebra_network::zakura::testkit::{TraceCapture, TraceValue}; @@ -2120,17 +2120,18 @@ mod zakura_header_sync_driver_tests { Service as ZakuraService, Stream as ZakuraStream, ZakuraHeaderSyncDriverStartup, BLOCK_SYNC_TABLE, COMMIT_STATE_TABLE, DEFAULT_HS_RANGE, }; - use zebra_test::vectors::{BLOCK_MAINNET_1_BYTES, BLOCK_MAINNET_2_BYTES}; + use zebra_test::vectors::{ + BLOCK_MAINNET_1_BYTES, BLOCK_MAINNET_2_BYTES, BLOCK_MAINNET_GENESIS_BYTES, + }; use super::zakura::{ - apply_block_sync_body, block_apply_class, block_roots_cover_range, - block_sync_chain_tip_event, block_sync_missing_body_window, - block_sync_needed_blocks_from_state, block_verify_error_is_duplicate, - body_sizes_for_served_header_range, chain_tip_mirror_frontier_change, - coalesce_ready_needed_block_queries, coalesce_stale_needed_block_queries, - commit_block_sync_body, drive_block_sync_actions, drive_zakura_header_sync_actions, - header_range_commit_failure_kind, notify_block_sync_header_tip, query_block_sync_frontiers, - query_block_sync_needed_blocks, root_covered_query_best_header_tip, + apply_block_sync_body, block_apply_class, block_sync_chain_tip_event, + block_sync_missing_body_window, block_sync_needed_blocks_from_state, + block_verify_error_is_duplicate, body_sizes_for_served_header_range, + chain_tip_mirror_frontier_change, coalesce_ready_needed_block_queries, + coalesce_stale_needed_block_queries, commit_block_sync_body, drive_block_sync_actions, + drive_zakura_header_sync_actions, header_range_commit_failure_kind, + notify_block_sync_header_tip, query_block_sync_frontiers, query_block_sync_needed_blocks, tree_aux_roots_for_served_header_range, verified_block_tip_from_state, BlockApplyClass, BlocksyncThroughputProbe, ZakuraHeaderSyncDriverHandles, ZAKURA_BLOCK_SYNC_CHECKPOINT_FRONTIER_REFRESH_INTERVAL, ZAKURA_BLOCK_SYNC_DRIVER_TIMEOUT, @@ -2290,54 +2291,6 @@ mod zakura_header_sync_driver_tests { ); } - #[test] - fn startup_root_backfill_gate_requires_complete_root_coverage() { - let start = block::Height(10); - let complete_roots = [ - root_at(block::Height(10)), - root_at(block::Height(11)), - root_at(block::Height(12)), - ]; - assert!(block_roots_cover_range(start, 3, &complete_roots)); - assert!(!block_roots_cover_range(start, 3, &complete_roots[..2])); - - let roots_with_gap = [ - root_at(block::Height(10)), - root_at(block::Height(12)), - root_at(block::Height(13)), - ]; - assert!(!block_roots_cover_range(start, 3, &roots_with_gap)); - } - - #[tokio::test] - async fn query_best_header_tip_is_capped_when_roots_are_missing() { - let verified_tip = (block::Height(0), block::Hash([0; 32])); - let durable_header_tip = (block::Height(2), block::Hash([2; 32])); - let read_state = service_fn(move |request: zebra_state::ReadRequest| async move { - match request { - zebra_state::ReadRequest::Tip => Ok::<_, zebra_state::BoxError>( - zebra_state::ReadResponse::Tip(Some(verified_tip)), - ), - zebra_state::ReadRequest::BlockRoots { - start_height, - count, - } => { - assert_eq!(start_height, block::Height(1)); - assert_eq!(count, 2); - Ok(zebra_state::ReadResponse::BlockRoots(Vec::new())) - } - request => panic!("unexpected read request: {request:?}"), - } - }); - - assert_eq!( - root_covered_query_best_header_tip(read_state, durable_header_tip) - .await - .expect("capped query succeeds"), - verified_tip - ); - } - #[test] fn block_verify_error_duplicate_classifier_detects_router_and_block_errors() { let hash = block::Hash([1; 32]); @@ -2670,6 +2623,74 @@ mod zakura_header_sync_driver_tests { reactor_task.abort(); } + /// End-to-end restart-resume: with a persisted header lead (bodies still at genesis), + /// `zakura_header_sync_driver_startup` reconstructs the history tree at the confirmed frontier + /// (`header_tip - 1`), seeds `best_header_parent_hash` with the frontier hash, and resumes header + /// sync from `frontier + 1` so the first forward range overlaps and re-validates the still- + /// unconfirmed tip root β€” the exact posture the running reactor keeps. + #[tokio::test(flavor = "multi_thread")] + async fn header_sync_driver_startup_resumes_at_reconstructed_frontier() { + let network = zebra_chain::parameters::Network::Mainnet; + let (mut state_service, read_state, _, _) = zebra_state::init_test_services(&network).await; + + let genesis = mainnet_block(&BLOCK_MAINNET_GENESIS_BYTES); + let block1 = mainnet_block(&BLOCK_MAINNET_1_BYTES); + let block2 = mainnet_block(&BLOCK_MAINNET_2_BYTES); + + state_service + .ready() + .await + .expect("state service is ready") + .call(zebra_state::Request::CommitCheckpointVerifiedBlock( + zebra_state::CheckpointVerifiedBlock::from(genesis.clone()), + )) + .await + .expect("genesis block commits"); + + // Persist a header lead: headers 1..=2 with the confirmed prefix (height 1 only; the tip's + // root is never persisted). Durable header tip is 2, so the confirmed frontier is 1. + state_service + .ready() + .await + .expect("state service is ready") + .call(zebra_state::Request::CommitHeaderRange { + anchor: genesis.hash(), + headers: vec![block1.header.clone(), block2.header.clone()], + body_sizes: vec![0, 0], + tree_aux_roots: vec![root_at(block::Height(1))], + }) + .await + .expect("header range commits"); + + let startup = super::zakura::zakura_header_sync_driver_startup(read_state, &network) + .await + .expect("driver startup reconstructs from durable state"); + + // Bodies are still at genesis; the header lead advanced to height 2. + assert_eq!(startup.frontiers.verified_block_tip, block::Height(0)); + + // Resume at frontier + 1 == 2 (the durable header tip), anchored at the frontier hash + // (height 1) so the first forward range overlaps and re-validates the tip's root. + assert_eq!( + startup.best_header_tip, + Some((block::Height(2), block2.hash())), + "resume height is frontier + 1, anchored at the durable tip", + ); + assert_eq!( + startup.best_header_parent_hash, + Some(block1.hash()), + "overlap anchor is the confirmed frontier hash (height 1)", + ); + + // The reconstructed tree sits one block behind the header tip, at the frontier. These are + // pre-Heartwood mainnet blocks, so that frontier tree is the empty tree. + assert_eq!( + startup.best_header_history_tree.as_ref(), + &zebra_chain::history_tree::HistoryTree::default(), + "reconstructed frontier tree is the pre-Heartwood empty tree", + ); + } + #[tokio::test] async fn header_sync_driver_header_advanced_updates_exchange_header_only() { let network = zebra_chain::parameters::Network::Mainnet; @@ -2689,6 +2710,10 @@ mod zakura_header_sync_driver_tests { verified_block_hash: genesis_hash, }, best_header_tip: Some((block::Height(0), genesis_hash)), + best_header_parent_hash: None, + best_header_history_tree: Arc::new( + zebra_chain::history_tree::HistoryTree::default(), + ), verified_block_tip_hash: genesis_hash, }), ) diff --git a/zebrad/src/commands/start/zakura/header_sync_driver.rs b/zebrad/src/commands/start/zakura/header_sync_driver.rs index a1d9ca0bec7..e13753d71b2 100644 --- a/zebrad/src/commands/start/zakura/header_sync_driver.rs +++ b/zebrad/src/commands/start/zakura/header_sync_driver.rs @@ -3,7 +3,7 @@ use std::{future::Future, time::Instant}; use color_eyre::eyre::{eyre, Report}; use tokio::{pin, select, sync::mpsc}; use tower::{Service, ServiceExt}; -use tracing::{debug, warn}; +use tracing::{debug, info, warn}; use zebra_chain::{ block::{self}, @@ -62,115 +62,85 @@ pub(crate) async fn zakura_header_sync_driver_startup( let finalized_height = finalized_tip.map_or(block::Height(0), |(height, _)| height); let verified_block_tip = verified_block_tip_from_state(finalized_tip, verified_block_tip, empty_state_tip); - let best_header_tip = root_covered_best_header_tip_or_verified( - read_state, - best_header_tip.unwrap_or(empty_state_tip), - verified_block_tip, - ) - .await?; + let durable_best_header_tip = best_header_tip.unwrap_or(empty_state_tip); - Ok(ZakuraHeaderSyncDriverStartup { - frontiers: HeaderSyncFrontiers { - finalized_height, + // Rebuild the ZIP-221 history tree so post-Heartwood header-sync root verification can resume + // immediately after a restart. The read returns the tree positioned at the highest *contiguous* + // confirmed header-root frontier, and that frontier's `(height, hash)` β€” the single authoritative + // value we use both to anchor overlap and to pick the resume height. + let (best_header_history_tree, (frontier_height, frontier_hash)) = match read_state + .clone() + .oneshot(zebra_state::ReadRequest::BestHeaderHistoryTree { verified_block_tip: verified_block_tip.0, - verified_block_hash: verified_block_tip.1, - }, - best_header_tip: Some(best_header_tip), - verified_block_tip_hash: verified_block_tip.1, - }) -} - -async fn root_covered_best_header_tip_or_verified( - read_state: ReadState, - best_header_tip: (block::Height, block::Hash), - verified_block_tip: (block::Height, block::Hash), -) -> Result<(block::Height, block::Hash), Report> -where - ReadState: Service< - zebra_state::ReadRequest, - Response = zebra_state::ReadResponse, - Error = zebra_state::BoxError, - > + Send - + 'static, - ReadState::Future: Send + 'static, -{ - if best_header_tip.0 <= verified_block_tip.0 { - return Ok(best_header_tip); - } - - let Ok(start_height) = verified_block_tip.0.next() else { - return Ok(verified_block_tip); - }; - let best_header_height = best_header_tip.0; - let verified_block_height = verified_block_tip.0; - let count = best_header_height - .0 - .checked_sub(verified_block_height.0) - .ok_or_else(|| eyre!("best header tip is unexpectedly below verified block tip"))?; - let roots = match read_state - .oneshot(zebra_state::ReadRequest::BlockRoots { - start_height, - count, + best_header_tip: durable_best_header_tip.0, }) .await .map_err(|error| eyre!("{error}"))? { - zebra_state::ReadResponse::BlockRoots(roots) => roots, - response => Err(eyre!("unexpected BlockRoots response: {response:?}"))?, - }; - - if block_roots_cover_range(start_height, count, &roots) { - Ok(best_header_tip) - } else { - Ok(verified_block_tip) - } -} - -pub(crate) async fn root_covered_query_best_header_tip( - read_state: ReadState, - best_header_tip: (block::Height, block::Hash), -) -> Result<(block::Height, block::Hash), Report> -where - ReadState: Service< - zebra_state::ReadRequest, - Response = zebra_state::ReadResponse, - Error = zebra_state::BoxError, - > + Clone - + Send - + 'static, - ReadState::Future: Send + 'static, -{ - let verified_block_tip = match read_state - .clone() - .oneshot(zebra_state::ReadRequest::Tip) - .await - .map_err(|error| eyre!("{error}"))? - { - zebra_state::ReadResponse::Tip(Some(tip)) => tip, - zebra_state::ReadResponse::Tip(None) => return Ok(best_header_tip), - response => Err(eyre!("unexpected Tip response: {response:?}"))?, + zebra_state::ReadResponse::BestHeaderHistoryTree { tree, frontier } => (tree, frontier), + response => Err(eyre!( + "unexpected BestHeaderHistoryTree response: {response:?}" + ))?, }; - root_covered_best_header_tip_or_verified(read_state, best_header_tip, verified_block_tip).await -} + // Resume one block above the contiguous frontier (where the reconstructed tree sits), anchored at + // it, so the first forward range re-validates from there. If the persisted roots have a one-block + // gap (a header-tip advance that never overlapped), this resumes from the gap instead of capping + // all the way back to the verified tip. With no header lead there is nothing to resume, so the + // durable tip is kept and no overlap anchor is set. + let (best_header_tip, best_header_parent_hash) = + if durable_best_header_tip.0 > verified_block_tip.0 { + let resume_height = frontier_height + .next() + .map_err(|_| eyre!("header frontier height overflow"))?; + // The common (no-gap) frontier is one below the durable tip, so its successor is the durable + // tip and we already hold that hash; only a gap needs a lookup of the durable resume header. + let resume_hash = if resume_height == durable_best_header_tip.0 { + durable_best_header_tip.1 + } else { + match read_state + .oneshot(zebra_state::ReadRequest::HeadersByHeightRange { + start: resume_height, + count: 1, + }) + .await + .map_err(|error| eyre!("{error}"))? + { + zebra_state::ReadResponse::Headers(headers) => headers + .first() + .map(|(_height, hash, _header)| *hash) + .ok_or_else(|| { + eyre!("missing durable header at resume height {resume_height:?}") + })?, + response => Err(eyre!( + "unexpected HeadersByHeightRange response: {response:?}" + ))?, + } + }; + ((resume_height, resume_hash), Some(frontier_hash)) + } else { + (durable_best_header_tip, None) + }; -pub(crate) fn block_roots_cover_range( - start_height: block::Height, - count: u32, - roots: &[BlockCommitmentRoots], -) -> bool { - if roots.len() != usize::try_from(count).unwrap_or(usize::MAX) { - return false; - } + info!( + verified = ?verified_block_tip.0, + durable_header_tip = ?durable_best_header_tip.0, + frontier = ?frontier_height, + resume_tip = ?best_header_tip.0, + overlap = best_header_parent_hash.is_some(), + "Zakura header-sync startup: reconstructed history tree at frontier, resuming header sync" + ); - roots.iter().enumerate().all(|(offset, roots)| { - let Ok(offset) = u32::try_from(offset) else { - return false; - }; - start_height - .0 - .checked_add(offset) - .is_some_and(|height| roots.height == block::Height(height)) + Ok(ZakuraHeaderSyncDriverStartup { + frontiers: HeaderSyncFrontiers { + finalized_height, + verified_block_tip: verified_block_tip.0, + verified_block_hash: verified_block_tip.1, + }, + best_header_tip: Some(best_header_tip), + best_header_parent_hash, + best_header_history_tree, + verified_block_tip_hash: verified_block_tip.1, }) } @@ -390,7 +360,7 @@ pub(crate) async fn drive_zakura_header_sync_actions { + Ok(zebra_state::ReadResponse::Headers(mut headers)) => { emit_commit_state( &trace, cs_trace::STATE_READ_SUCCESS, @@ -505,6 +475,16 @@ pub(crate) async fn drive_zakura_header_sync_actions = headers.iter().map(|(height, _, _)| *height).collect(); let tree_aux_roots = if want_tree_aux_roots { @@ -613,11 +593,20 @@ pub(crate) async fn drive_zakura_header_sync_actions { let count = u32::try_from(headers.len()).unwrap_or(u32::MAX); - let tree_aux_roots_len = u32::try_from(tree_aux_roots.len()).unwrap_or(u32::MAX); + // Persist only the header-authenticated confirmed prefix. The range tip's root is + // unconfirmed until the next overlapping range delivers its successor header, so it + // is deliberately excluded here; the state writes exactly what it is given. + let committed_roots = verified_roots + .as_deref() + .map_or_else(Vec::new, |verified_roots| { + verified_roots.confirmed_roots().to_vec() + }); + let tree_aux_roots_len = u32::try_from(committed_roots.len()).unwrap_or(u32::MAX); + let tip_parent_hash = header_range_tip_parent_hash(anchor, start_height, &headers); emit_commit_state( &trace, cs_trace::COMMIT_START, @@ -642,7 +631,7 @@ pub(crate) async fn drive_zakura_header_sync_actions { - emit_commit_state( - &trace, - cs_trace::STATE_READ_START, - "header_sync_driver", - |row| { - insert_cs_str(row, cs_trace::ACTION, "query_best_header_tip"); - }, - ); - let started = Instant::now(); - match read_state + HeaderSyncAction::QueryBestHeaderHistoryTree { + verified_block_tip, + best_header_tip, + } => { + // Reconstruct the header-frontier tree at the current frontier from durable roots. + // Always report completion back (`Some` on success, `None` on failure) so the reactor + // clears its in-flight rebuild guard even when the read errors β€” otherwise the guard + // would wedge and suppress all future rebuilds, stranding the stale tree. + let history_tree = match read_state .clone() - .oneshot(zebra_state::ReadRequest::BestHeaderTip) + .oneshot(zebra_state::ReadRequest::BestHeaderHistoryTree { + verified_block_tip, + best_header_tip, + }) .await { - Ok(zebra_state::ReadResponse::BestHeaderTip(Some(best_header_tip))) => { - let (tip_height, tip_hash) = match root_covered_query_best_header_tip( - read_state.clone(), - best_header_tip, - ) - .await - { - Ok(tip) => tip, - Err(error) => { - trace_state_read_error( - &trace, - "query_best_header_tip_roots", - None, - best_header_tip.0, - 1, - &format!("{error}"), - started, - ); - warn!( - ?error, - "failed to apply Zakura root coverage to best header tip" - ); - continue; - } - }; - emit_commit_state( - &trace, - cs_trace::STATE_READ_SUCCESS, - "header_sync_driver", - |row| { - insert_cs_str(row, cs_trace::ACTION, "query_best_header_tip"); - insert_cs_height(row, cs_trace::BEST_HEADER_TIP, tip_height); - insert_cs_hash(row, cs_trace::HASH, tip_hash); - }, - ); - let _ = handles - .header_sync - .send(HeaderSyncEvent::HeaderRangeCommitted { - start_height: tip_height, - tip_height, - tip_hash, - }) - .await; - publish_header_frontier( - &handles.endpoint, - tip_height, - tip_hash, - FrontierChange::HeaderAdvanced, - &trace, - ); - } - Ok(zebra_state::ReadResponse::BestHeaderTip(None)) => {} + Ok(zebra_state::ReadResponse::BestHeaderHistoryTree { tree, .. }) => Some(tree), Ok(response) => { - trace_state_read_error( - &trace, - "query_best_header_tip", - None, - block::Height(0), - 0, - "unexpected_response", - Instant::now(), - ); - warn!(?response, "unexpected BestHeaderTip response") + warn!(?response, "unexpected BestHeaderHistoryTree response"); + None } Err(error) => { - trace_state_read_error( - &trace, - "query_best_header_tip", - None, - block::Height(0), - 0, - &format!("{error}"), - Instant::now(), - ); - warn!(?error, "failed to query Zakura best header tip") + warn!(?error, "failed to rebuild Zakura best header history tree"); + None } - } + }; + let _ = handles + .header_sync + .send(HeaderSyncEvent::BestHeaderHistoryTreeLoaded { + best_header_tip, + history_tree, + }) + .await; } HeaderSyncAction::QueryMissingBlockBodies { from, limit } => { log_missing_block_bodies(read_state.clone(), from, limit, &trace).await; @@ -1382,9 +1313,6 @@ fn trace_header_driver_action(trace: &ZakuraTrace, action: &HeaderSyncAction) { insert_cs_height(row, cs_trace::RANGE_START, *start_height); insert_cs_u64(row, cs_trace::RANGE_COUNT, headers.len() as u64); } - HeaderSyncAction::QueryBestHeaderTip => { - insert_cs_str(row, cs_trace::ACTION, "query_best_header_tip"); - } HeaderSyncAction::QueryHeadersByHeightRange { peer, start, count, .. } => { @@ -1393,6 +1321,14 @@ fn trace_header_driver_action(trace: &ZakuraTrace, action: &HeaderSyncAction) { insert_cs_height(row, cs_trace::RANGE_START, *start); insert_cs_u64(row, cs_trace::RANGE_COUNT, u64::from(*count)); } + HeaderSyncAction::QueryBestHeaderHistoryTree { + verified_block_tip, + best_header_tip, + } => { + insert_cs_str(row, cs_trace::ACTION, "query_best_header_history_tree"); + insert_cs_height(row, cs_trace::VERIFIED_BLOCK_TIP, *verified_block_tip); + insert_cs_height(row, cs_trace::BEST_HEADER_TIP, *best_header_tip); + } HeaderSyncAction::QueryMissingBlockBodies { from, limit } => { insert_cs_str(row, cs_trace::ACTION, "query_missing_block_bodies"); insert_cs_height(row, cs_trace::RANGE_START, *from); @@ -1459,6 +1395,24 @@ fn trace_header_commit_finish( ); } +fn header_range_tip_parent_hash( + anchor: block::Hash, + start_height: block::Height, + headers: &[std::sync::Arc], +) -> Option { + if headers.is_empty() { + return None; + } + + if headers.len() == 1 { + return (start_height > block::Height(0)).then_some(anchor); + } + + headers + .get(headers.len().saturating_sub(2)) + .map(|header| block::Hash::from(header.as_ref())) +} + fn trace_header_reactor_event( trace: &ZakuraTrace, action: &'static str, diff --git a/zebrad/src/commands/start/zakura/mod.rs b/zebrad/src/commands/start/zakura/mod.rs index bff8f791fd8..67051364a35 100644 --- a/zebrad/src/commands/start/zakura/mod.rs +++ b/zebrad/src/commands/start/zakura/mod.rs @@ -24,10 +24,9 @@ pub(crate) use block_sync_driver::{ pub(crate) use frontier::{query_block_sync_frontiers, verified_block_tip_from_state}; #[cfg(test)] pub(crate) use header_sync_driver::{ - block_roots_cover_range, block_sync_chain_tip_event, body_sizes_for_served_header_range, + block_sync_chain_tip_event, body_sizes_for_served_header_range, chain_tip_mirror_frontier_change, header_range_commit_failure_kind, - notify_block_sync_header_tip, root_covered_query_best_header_tip, - tree_aux_roots_for_served_header_range, + notify_block_sync_header_tip, tree_aux_roots_for_served_header_range, }; pub(crate) use header_sync_driver::{ drive_zakura_header_sync_actions, mirror_zakura_full_block_commits, From 0fef0c4e2fda47bf672e0714f55af5f1db8885f3 Mon Sep 17 00:00:00 2001 From: Roman Date: Sun, 5 Jul 2026 06:46:39 +0000 Subject: [PATCH 02/12] fix(network): harden Zakura header-sync history-tree lazy rebuild Robustness fixes to the below-checkpoint header-sync history-tree reconstruction/rebuild path, from a self-audit: - Arm the rebuild in-flight guard only when the QueryBestHeaderHistoryTree action is actually queued. dispatch_action is a non-blocking try_send that drops on a full/closed channel; arming the guard on a dropped send would suppress every future rebuild permanently (the guard clears only when the reload completes). A dropped send now leaves the guard clear so the next range retry re-dispatches. - Re-base the reconstruction at the current snapshot tip when the caller's cached verified_block_tip has no stored tree. The finalized history tree is tip-only, so a concurrent checkpoint/legacy commit advancing the finalized tip during the round-trip made read::tree::history_tree return None, then Err, then a network-paced no-progress rebuild loop. Reading the tip from the same snapshot is atomic and is the correct committed base below the checkpoint. - Clamp finalized_height with max() in handle_state_frontiers_changed so a stale or out-of-order frontier update cannot move the root-regime floor backward. verified_block_tip is left free (it can reorg above the checkpoint). Adds failed_rebuild_clears_guard_and_retriggers, and updates docs/design/verified-commitment-trees.md (6.4 robustness details, 13 the sync.header.history_tree.rebuild counter). --- docs/design/verified-commitment-trees.md | 12 ++- .../src/zakura/header_sync/reactor.rs | 24 +++-- zebra-network/src/zakura/header_sync/tests.rs | 81 +++++++++++++++ zebra-state/src/service.rs | 99 ++++++++++++++++--- 4 files changed, 192 insertions(+), 24 deletions(-) diff --git a/docs/design/verified-commitment-trees.md b/docs/design/verified-commitment-trees.md index eaed3fc7084..d163edbe84f 100644 --- a/docs/design/verified-commitment-trees.md +++ b/docs/design/verified-commitment-trees.md @@ -445,7 +445,16 @@ other peers are already header-authenticated, and so a restart never trusts an u reads it, the forward root-verifying request, detects a behind tree (`MissingHeaderHistoryTree`) and rebuilds it from durable state (`ReadRequest::BestHeaderHistoryTree`) β€” a single, guarded reload that never fires in the normal header-leading path. This replaces the reorg/gossip/catch-up eager-reload - machinery of earlier increments. + machinery of earlier increments. Two robustness details: the in-flight guard is armed only once the + reload action is actually queued β€” the reactorβ†’driver action channel is a non-blocking `try_send`, + and arming the guard on a dropped send (full/closed channel) would suppress every future rebuild + permanently, since the guard clears only when a reload completes; a dropped send instead leaves it + clear so the next range retry re-dispatches. And the reconstruction re-bases at the *current* + committed tip read from the same state snapshot: the finalized history tree is stored only at the tip + (there is no per-height finalized history tree), and a concurrent checkpoint/legacy commit can + advance the finalized tip past the reactor's cached `verified_block_tip` during the round-trip, so + folding must start from the tip actually present rather than the caller's (possibly lagging) value β€” + otherwise the base read returns `None` and the rebuild degrades to a network-paced no-progress loop. - **Verify before persisting.** When a below-checkpoint header range arrives, the reactor folds its supplied roots into the running header-frontier history tree and checks each header's commitment against it (`verify_supplied_roots_from_parts` in `zebra-chain/src/parallel/commitment_aux_verify.rs`, @@ -753,6 +762,7 @@ Live commit-path counters distinguish the fast and legacy paths and the failure | `state.vct.fast_path.hit` | a finalized commit consumed header-carried roots to skip the recompute | | `state.vct.fast_path.miss` | a finalized commit did not take the fast path | | `state.vct.root.stalled.height` (gauge) | a height stuck on a retryable stall past the warn threshold | +| `sync.header.history_tree.rebuild` | header-frontier history tree lazily rebuilt (Β§6.4) because a non-Zakura commit ran ahead of it below the checkpoint; **stays 0 in the normal header-leading path**, so a nonzero value flags fallback/catch-up | The header-sync `headers_received` / `headers_served` / commit-state trace rows also carry `want_tree_aux_roots` and `tree_aux_roots_len`, so root delivery is visible per range. The diff --git a/zebra-network/src/zakura/header_sync/reactor.rs b/zebra-network/src/zakura/header_sync/reactor.rs index 4680b198803..fa23495776e 100644 --- a/zebra-network/src/zakura/header_sync/reactor.rs +++ b/zebra-network/src/zakura/header_sync/reactor.rs @@ -611,7 +611,10 @@ impl HeaderSyncReactor { } async fn handle_state_frontiers_changed(&mut self, frontiers: HeaderSyncFrontiers) { - self.state.finalized_height = frontiers.finalized_height; + // The finalized height is append-only; clamp against an out-of-order/stale frontier update so + // the root-regime floor (which keys off it) can never move backward. The verified block tip is + // left free β€” it can legitimately reorg backward above the checkpoint. + self.state.finalized_height = self.state.finalized_height.max(frontiers.finalized_height); self.state.verified_block_tip = frontiers.verified_block_tip; self.state.verified_block_hash = frontiers.verified_block_hash; if self.state.best_header_tip <= self.state.verified_block_tip { @@ -1274,15 +1277,20 @@ impl HeaderSyncReactor { // sync) committed ahead of the header-commit frontier, so `best_header_tip` was // bumped past where the tree is folded. This is the *only* reload trigger: rebuild // the tree at the current frontier from durable state, then retry the range. In the - // normal Zakura path (header sync leading) this never fires. Guard so a run of - // forward ranges arriving before the rebuild lands dispatches only one reload. - if !self.state.rebuild_in_flight { - self.state.rebuild_in_flight = true; - metrics::counter!("sync.header.history_tree.rebuild").increment(1); - let _ = self.dispatch_action(HeaderSyncAction::QueryBestHeaderHistoryTree { + // normal Zakura path (header sync leading) this never fires. Only arm the in-flight + // guard once the reload action is actually queued β€” `dispatch_action` is a + // non-blocking `try_send` that drops on a full/closed channel, and arming the guard + // on a dropped send would suppress every future rebuild permanently (the guard is + // only cleared when the reload completes). A dropped send just leaves the guard + // clear so the next retry re-dispatches. + if !self.state.rebuild_in_flight + && self.dispatch_action(HeaderSyncAction::QueryBestHeaderHistoryTree { verified_block_tip: self.state.verified_block_tip, best_header_tip: self.state.best_header_tip, - }); + }) + { + self.state.rebuild_in_flight = true; + metrics::counter!("sync.header.history_tree.rebuild").increment(1); } self.state.schedule.clear_assignment(outstanding.range); self.state.schedule.retry(outstanding.range); diff --git a/zebra-network/src/zakura/header_sync/tests.rs b/zebra-network/src/zakura/header_sync/tests.rs index 85a6a916dc1..a0681d9e9e9 100644 --- a/zebra-network/src/zakura/header_sync/tests.rs +++ b/zebra-network/src/zakura/header_sync/tests.rs @@ -4358,6 +4358,87 @@ async fn missing_header_aux_tree_triggers_single_lazy_rebuild() { assert_eq!(rebuilds, 1, "a stale tree triggers exactly one lazy rebuild"); } +/// A failed rebuild (`BestHeaderHistoryTreeLoaded { history_tree: None }`, e.g. a state read error) +/// must clear the in-flight guard so a subsequent stale forward range can re-trigger the rebuild β€” the +/// guard must never wedge future rebuilds. Regression test for the lazy-rebuild robustness path. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn failed_rebuild_clears_guard_and_retriggers() { + let block4 = mainnet_header(&BLOCK_MAINNET_4_BYTES); + let block4_hash = block::Hash::from(block4.as_ref()); + let block5 = mainnet_header(&BLOCK_MAINNET_5_BYTES); + let block5_hash = block::Hash::from(block5.as_ref()); + let (network, _) = checkpoint_testnet_with_hash(block::Height(5), block5_hash); + let mut fixture = spawn_test_reactor(startup_for( + network.clone(), + (block::Height(0), network.genesis_hash()), + Some((block::Height(4), block4_hash)), + )); + let peer_id = peer(88); + + connect_peer(&fixture, peer_id.clone()).await; + advertise_tip( + &fixture, + peer_id.clone(), + block::Height(0), + block::Height(5), + DEFAULT_HS_RANGE, + 1, + ) + .await; + + // Drive one stale-tree round, feed a *failed* rebuild, then drive a second stale-tree round. + // A second `QueryBestHeaderHistoryTree` must be dispatched β€” proving the guard was cleared by the + // `None` completion rather than wedged. + let mut rebuilds = 0; + for round in 0..2 { + let (_p, start_height, _c) = next_outbound_get_headers(&mut fixture.actions).await; + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id.clone(), + msg: finalized_headers_message_from(start_height, vec![block5.clone()]), + }) + .await + .unwrap(); + // Wait for this round's rebuild dispatch. + let mut saw = false; + for _ in 0..16 { + match tokio::time::timeout( + std::time::Duration::from_millis(200), + fixture.actions.recv(), + ) + .await + { + Ok(Some(HeaderSyncAction::QueryBestHeaderHistoryTree { .. })) => { + rebuilds += 1; + saw = true; + break; + } + Ok(Some(HeaderSyncAction::SendMessage { + msg: HeaderSyncMessage::GetHeaders { .. }, + .. + })) => { /* the retry re-request; ignore until the rebuild dispatch */ } + Ok(Some(_)) => {} + _ => break, + } + } + assert!(saw, "round {round}: expected a rebuild dispatch"); + // Fail the rebuild β€” the guard must clear so the next round can re-trigger. + fixture + .handle + .send(HeaderSyncEvent::BestHeaderHistoryTreeLoaded { + best_header_tip: block::Height(4), + history_tree: None, + }) + .await + .unwrap(); + } + assert_eq!( + rebuilds, 2, + "a failed rebuild must clear the guard so the next stale range re-triggers it", + ); +} + /// A gossiped `NewBlock` accepted ahead of the header frontier advances the header tip β€” the frontier /// tracks the committed chain. It does not touch the in-memory tree; if this were below the checkpoint /// and a later forward range needed the tree, the lazy rebuild would reposition it. diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 1473086d30b..3b6c6160d49 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -1535,6 +1535,8 @@ where C: AsRef, { let capped_count = count.min(MAX_HEADER_SYNC_HEIGHT_RANGE); + let finalized_tip_height = db.finalized_tip_height(); + let best_header_tip_height = db.best_header_tip().map(|(height, _hash)| height); let mut roots = Vec::with_capacity(usize::try_from(capped_count).expect("capped root count fits in usize")); @@ -1543,18 +1545,23 @@ where break; }; - let root = if db - .finalized_tip_height() - .is_some_and(|finalized_tip| height <= finalized_tip) - { + let chain_contains_height = chain + .as_ref() + .map(|chain| chain.as_ref().contains_block_height(height)) + .unwrap_or(false); + + let mut root_source = "zakura_header"; + let root = if finalized_tip_height.is_some_and(|finalized_tip| height <= finalized_tip) { + root_source = "finalized"; finalized_state::serve_block_roots(db, height..=height) .into_iter() .next() - } else if let Some(chain) = chain - .as_ref() - .map(|chain| chain.as_ref()) - .filter(|chain| chain.contains_block_height(height)) - { + } else if chain_contains_height { + root_source = "non_finalized"; + let chain = chain + .as_ref() + .map(|chain| chain.as_ref()) + .expect("chain exists because it contains this block height"); match ( chain.sapling_tree(height.into()), chain.orchard_tree(height.into()), @@ -1598,10 +1605,36 @@ where }; let Some(root) = root else { + tracing::warn!( + ?start, + count, + capped_count, + ?height, + offset, + root_source, + ?finalized_tip_height, + ?best_header_tip_height, + chain_contains_height, + has_body = db.contains_body_at_height(height), + has_header = !db.headers_by_height_range(height, 1).is_empty(), + returned_count = roots.len(), + "Zakura BlockRoots read stopped at missing commitment root" + ); break; }; if root.height != height { + tracing::warn!( + ?start, + count, + capped_count, + ?height, + returned_height = ?root.height, + offset, + root_source, + returned_count = roots.len(), + "Zakura BlockRoots read stopped at non-contiguous commitment root" + ); break; } @@ -1639,7 +1672,7 @@ fn best_header_history_tree( chain: Option, db: &ZebraDb, network: &Network, - verified_block_tip: block::Height, + mut verified_block_tip: block::Height, best_header_tip: block::Height, ) -> Result< ( @@ -1661,11 +1694,24 @@ where Some(tree) => tree, None if verified_block_tip == block::Height(0) => Arc::new(HistoryTree::default()), None => { - return Err(format!( - "cannot rebuild header-tip history tree: no history tree at verified block tip \ - {verified_block_tip:?}" - ) - .into()) + // The caller's `verified_block_tip` lagged the committed tip: a concurrent checkpoint or + // legacy commit advanced the finalized tip during the round-trip (the runtime lazy-rebuild + // path), and below the checkpoint the finalized history tree is stored only at the current + // tip β€” there is no per-height finalized history tree. Re-base at this snapshot's tip, + // read atomically from the same `chain`/`db`, which is the real committed base; the header + // frontier tracks it, so any header lead folds on top from here. + let tip = read::tip_height(chain.clone(), db).ok_or_else(|| { + BoxError::from("cannot rebuild header-tip history tree: no state tip") + })?; + verified_block_tip = tip; + match read::tree::history_tree(chain.clone(), db, tip.into()) { + Some(tree) => tree, + None if tip == block::Height(0) => Arc::new(HistoryTree::default()), + None => return Err(format!( + "cannot rebuild header-tip history tree: no history tree at state tip {tip:?}" + ) + .into()), + } } }; @@ -1999,6 +2045,29 @@ impl Service for ReadStateService { ) }; + let capped_count = count.min(MAX_HEADER_SYNC_HEIGHT_RANGE); + let capped_count_usize = + usize::try_from(capped_count).expect("capped root count fits in usize"); + if count > 0 && roots.len() < capped_count_usize { + tracing::warn!( + ?start_height, + count, + capped_count, + returned_count = roots.len(), + last_returned_height = ?roots.last().map(|root| root.height), + "Zakura BlockRoots read returned a short contiguous prefix" + ); + } else { + tracing::debug!( + ?start_height, + count, + capped_count, + returned_count = roots.len(), + last_returned_height = ?roots.last().map(|root| root.height), + "Zakura BlockRoots read completed" + ); + } + Ok(ReadResponse::BlockRoots(roots)) } From 2f4d3b3ee151219230f1be9dfb2054007b9a7865 Mon Sep 17 00:00:00 2001 From: Roman Date: Sun, 5 Jul 2026 07:31:08 +0000 Subject: [PATCH 03/12] simpler comment --- zebra-network/src/zakura/header_sync/state.rs | 31 +++++-------------- 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/zebra-network/src/zakura/header_sync/state.rs b/zebra-network/src/zakura/header_sync/state.rs index 3c6a86c651a..b97e44b9d64 100644 --- a/zebra-network/src/zakura/header_sync/state.rs +++ b/zebra-network/src/zakura/header_sync/state.rs @@ -81,33 +81,19 @@ impl HeaderSyncCore { } let checkpoints = startup.network.checkpoint_list(); - // Commitment-root verification, the one-block overlap, and the in-memory header-frontier - // history tree only exist up to the last checkpoint β€” the VCT fast-sync handoff boundary, the - // only region where a consumer reads the persisted roots. Crucially the handoff block *at* - // `last_checkpoint` reads its own persisted root from the roots CF (`vct_roots_at_height` is - // inclusive of `last_checkpoint`); an empty slot there wedges the handoff on the frozen-frontier - // safety check. A root at `H` is only confirmed by the header at `H + 1`, so to persist the - // confirmed root at `last_checkpoint` the root-carrying regime must run until the frontier - // reaches `last_checkpoint + 1`. Above that, header sync runs plain: no roots, no overlap. + // Commitment-root work only runs through the VCT fast-sync handoff boundary. + // The root at `last_checkpoint` is confirmed by the next header, so root-carrying + // ranges stop once the frontier reaches `last_checkpoint + 1`. let last_checkpoint = startup.last_checkpoint_height; - // The frontier height at which root work stops: one above the last checkpoint (so the range - // that reaches `last_checkpoint` still confirms its root via the `last_checkpoint + 1` header). let root_regime_end = next_height(last_checkpoint).unwrap_or(last_checkpoint); - // Header sync persists confirmed roots for `(max(anchor, finalized_height) .. last_checkpoint]` - // β€” the below-checkpoint heights this node forward-syncs (above its anchor) that are not yet - // committed. Below the last checkpoint, blocks are checkpoint-verified straight into the - // finalized state, so `finalized_height` is the committed floor VCT consumes roots up to. Root - // work runs while that region is non-empty and the frontier has not passed the confirming - // height (`last_checkpoint + 1`). A node anchored at / already synced through the last - // checkpoint (handoff done via the embedded frontier), and a genesis-only checkpoint list - // (`max_height == 0`), both have an empty region and run plain. + // Only persist roots for below-checkpoint heights this node forward-syncs but has + // not committed yet. let root_region_floor = self.anchor.0.max(self.finalized_height); let below_root_boundary = root_region_floor < last_checkpoint && self.best_header_tip < root_regime_end; let want_tree_aux_roots = below_root_boundary; - // Root-carrying ranges leave the best tip's roots unconfirmed, so the next request - // redelivers the tip header anchored at its parent. Overlap only applies in the root regime. + // Root-carrying ranges redeliver the tip header so its root can be confirmed. let overlap_forward_range = below_root_boundary && self .best_header_parent_hash @@ -130,10 +116,7 @@ impl HeaderSyncCore { finalized = true; } } - // Cap the root-carrying range at `last_checkpoint + 1`: the range that reaches the checkpoint - // confirms and persists the root at `last_checkpoint` (via the `last_checkpoint + 1` header), - // and the following (above-boundary) ranges run plain. The tip root at `last_checkpoint + 1` is - // itself left unconfirmed and unpersisted β€” VCT never reads it. + // Keep root-carrying ranges at or below the confirming header for the last checkpoint. if below_root_boundary { end = end.min(root_regime_end); } From 8a93f78233469244da1a1d5cfe83c4df2b5ba29a Mon Sep 17 00:00:00 2001 From: Roman Date: Sun, 5 Jul 2026 07:32:59 +0000 Subject: [PATCH 04/12] fix tests and lint --- docs/design/verified-commitment-trees.md | 4 +- zebra-network/src/zakura/header_sync/tests.rs | 51 ++++++++++++++++--- zebra-state/src/service.rs | 6 ++- zebra-state/src/service/tests.rs | 20 ++++---- 4 files changed, 60 insertions(+), 21 deletions(-) diff --git a/docs/design/verified-commitment-trees.md b/docs/design/verified-commitment-trees.md index d163edbe84f..0a439c65236 100644 --- a/docs/design/verified-commitment-trees.md +++ b/docs/design/verified-commitment-trees.md @@ -431,7 +431,7 @@ other peers are already header-authenticated, and so a restart never trusts an u `CheckpointList::max_height()`, exactly the VCT fast-sync handoff height (Β§7). That is the only region a consumer reads the persisted roots: above the last checkpoint blocks go through full semantic verification and recompute their own trees, so peer roots there have no reader. The regime - is **inclusive** of the last checkpoint, because the handoff block *at* `last_checkpoint` reads its + is **inclusive** of the last checkpoint, because the handoff block _at_ `last_checkpoint` reads its own persisted root from the roots CF (`vct_roots_at_height` is inclusive; an empty slot there wedges the handoff on the frozen-frontier safety check, Β§8.1). A root at `H` is only confirmed by the header at `H + 1`, so the root-carrying regime runs until the frontier reaches `last_checkpoint + 1` β€” that @@ -449,7 +449,7 @@ other peers are already header-authenticated, and so a restart never trusts an u reload action is actually queued β€” the reactorβ†’driver action channel is a non-blocking `try_send`, and arming the guard on a dropped send (full/closed channel) would suppress every future rebuild permanently, since the guard clears only when a reload completes; a dropped send instead leaves it - clear so the next range retry re-dispatches. And the reconstruction re-bases at the *current* + clear so the next range retry re-dispatches. And the reconstruction re-bases at the _current_ committed tip read from the same state snapshot: the finalized history tree is stored only at the tip (there is no per-height finalized history tree), and a concurrent checkpoint/legacy commit can advance the finalized tip past the reactor's cached `verified_block_tip` during the round-trip, so diff --git a/zebra-network/src/zakura/header_sync/tests.rs b/zebra-network/src/zakura/header_sync/tests.rs index a0681d9e9e9..432f4dda7d5 100644 --- a/zebra-network/src/zakura/header_sync/tests.rs +++ b/zebra-network/src/zakura/header_sync/tests.rs @@ -4355,7 +4355,10 @@ async fn missing_header_aux_tree_triggers_single_lazy_rebuild() { _ => break, } } - assert_eq!(rebuilds, 1, "a stale tree triggers exactly one lazy rebuild"); + assert_eq!( + rebuilds, 1, + "a stale tree triggers exactly one lazy rebuild" + ); } /// A failed rebuild (`BestHeaderHistoryTreeLoaded { history_tree: None }`, e.g. a state read error) @@ -4495,19 +4498,42 @@ async fn forward_ranges_request_roots_through_the_last_checkpoint() { below.last_checkpoint_height = boundary; let mut fixture = spawn_test_reactor(below); connect_peer(&fixture, peer(70)).await; - advertise_tip(&fixture, peer(70), genesis.0, block::Height(20), DEFAULT_HS_RANGE, 1).await; + advertise_tip( + &fixture, + peer(70), + genesis.0, + block::Height(20), + DEFAULT_HS_RANGE, + 1, + ) + .await; let (start, count, want) = next_forward_get_headers(&mut fixture.actions).await; assert_eq!(start, block::Height(1)); assert!(want, "a below-checkpoint forward range must request roots"); - assert_eq!(count, 11, "the range is capped one block above the last checkpoint"); + assert_eq!( + count, 11, + "the range is capped one block above the last checkpoint" + ); // Frontier at the last checkpoint: still in the root regime (this range confirms the root at // `last_checkpoint` via the successor header). - let mut at = startup_for(network.clone(), genesis, Some((boundary, block::Hash([10; 32])))); + let mut at = startup_for( + network.clone(), + genesis, + Some((boundary, block::Hash([10; 32]))), + ); at.last_checkpoint_height = boundary; let mut fixture = spawn_test_reactor(at); connect_peer(&fixture, peer(71)).await; - advertise_tip(&fixture, peer(71), boundary, block::Height(20), DEFAULT_HS_RANGE, 1).await; + advertise_tip( + &fixture, + peer(71), + boundary, + block::Height(20), + DEFAULT_HS_RANGE, + 1, + ) + .await; let (_start, _count, want) = next_forward_get_headers(&mut fixture.actions).await; assert!( want, @@ -4523,10 +4549,21 @@ async fn forward_ranges_request_roots_through_the_last_checkpoint() { above.last_checkpoint_height = boundary; let mut fixture = spawn_test_reactor(above); connect_peer(&fixture, peer(72)).await; - advertise_tip(&fixture, peer(72), block::Height(11), block::Height(20), DEFAULT_HS_RANGE, 1).await; + advertise_tip( + &fixture, + peer(72), + block::Height(11), + block::Height(20), + DEFAULT_HS_RANGE, + 1, + ) + .await; let (start, _count, want) = next_forward_get_headers(&mut fixture.actions).await; assert_eq!(start, block::Height(12)); - assert!(!want, "above the last checkpoint the forward range is plain"); + assert!( + !want, + "above the last checkpoint the forward range is plain" + ); } #[tokio::test(flavor = "current_thread")] diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 3b6c6160d49..faec13d41c3 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -1707,10 +1707,12 @@ where match read::tree::history_tree(chain.clone(), db, tip.into()) { Some(tree) => tree, None if tip == block::Height(0) => Arc::new(HistoryTree::default()), - None => return Err(format!( + None => { + return Err(format!( "cannot rebuild header-tip history tree: no history tree at state tip {tip:?}" ) - .into()), + .into()) + } } } }; diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index f7a94cb9819..cd644dc79b4 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -1073,26 +1073,26 @@ fn best_header_history_tree_returns_base_when_no_contiguous_fold_exists() { } #[test] -fn best_header_history_tree_errors_on_missing_real_base_tree() { +fn best_header_history_tree_rebases_to_committed_tip_when_verified_tip_has_no_tree() { let _init_guard = zebra_test::init(); - let (state, _genesis, block1) = mainnet_state_with_genesis(); + let (state, genesis, block1) = mainnet_state_with_genesis(); insert_zakura_header(&state, Height(1), block1.header.clone()); - let error = best_header_history_tree( + // `verified_block_tip` is ahead of the real committed tip (only genesis is committed) and has no + // stored history tree. The lazy rebuild re-bases at the committed tip read from the same snapshot + // instead of erroring, so it returns the committed base frontier (genesis) rather than papering + // over a real tip with an empty tree. + let (tree, frontier) = best_header_history_tree( None::>, &state, &Network::Mainnet, Height(1), Height(1), ) - .expect_err("a real verified tip without a history tree is inconsistent"); + .expect("rebuild re-bases to the committed tip when the verified tip has no stored tree"); - assert!( - error - .to_string() - .contains("no history tree at verified block tip"), - "unexpected error: {error}", - ); + assert_eq!(frontier, (Height(0), genesis.hash())); + assert_eq!(tree.as_ref(), &HistoryTree::default()); } #[tokio::test(flavor = "multi_thread")] From c2e21aa50896b694754041592778c3ccffe7e818 Mon Sep 17 00:00:00 2001 From: roman Date: Sun, 5 Jul 2026 16:35:56 -0600 Subject: [PATCH 05/12] repro tests & smaller decouple requested from delivered ranges --- zebra-network/src/zakura/header_sync/pipe.rs | 72 ++++ .../src/zakura/header_sync/reactor.rs | 30 +- .../src/zakura/header_sync/scheduler.rs | 8 + zebra-network/src/zakura/header_sync/state.rs | 14 +- zebra-network/src/zakura/header_sync/tests.rs | 398 +++++++++++++++++- 5 files changed, 512 insertions(+), 10 deletions(-) diff --git a/zebra-network/src/zakura/header_sync/pipe.rs b/zebra-network/src/zakura/header_sync/pipe.rs index c528cc1c4de..504ba9321fa 100644 --- a/zebra-network/src/zakura/header_sync/pipe.rs +++ b/zebra-network/src/zakura/header_sync/pipe.rs @@ -480,6 +480,78 @@ mod tests { } } + /// Regression test (review feedback): tree-aux roots are an optional serving capability β€” + /// a peer legitimately may not have roots for the requested range (it can never serve the + /// root for its own header tip). A solicited, otherwise well-formed `Headers` response that + /// carries no roots must therefore not be a *fatal protocol reject* that disconnects the + /// peer; it should be forwarded (or at worst dropped non-punitively) so the reactor can + /// retry the range elsewhere while keeping the session. + #[test] + fn deliver_rootless_solicited_headers_does_not_disconnect_the_peer() { + use zebra_chain::{orchard, sapling, serialization::ZcashDeserializeInto}; + use zebra_test::vectors::BLOCK_MAINNET_1_BYTES; + + let (handle, mut events) = test_handle(); + let expected = + ExpectedHeadersResponse::new(block::Height(1), 1, true).expect("count is valid"); + + let block_one: Arc = Arc::new( + BLOCK_MAINNET_1_BYTES + .zcash_deserialize_into() + .expect("block 1 vector parses"), + ); + // `encode()` enforces the all-or-nothing root invariant, so build a rooted frame and + // strip the roots on the wire: flip `has_roots` to 0 and drop the trailing root bytes. + let mut frame = HeaderSyncMessage::Headers { + headers: vec![block_one.header.clone()], + body_sizes: vec![0], + tree_aux_roots: vec![BlockCommitmentRoots { + height: block::Height(1), + sapling_root: sapling::tree::NoteCommitmentTree::default().root(), + orchard_root: orchard::tree::NoteCommitmentTree::default().root(), + ironwood_root: zebra_chain::ironwood::tree::NoteCommitmentTree::default().root(), + sapling_tx: 0, + orchard_tx: 0, + ironwood_tx: 0, + auth_data_root: block::merkle::AuthDataRoot::from([0u8; 32]), + }], + } + .encode_frame() + .expect("headers frame encodes"); + frame.payload[HEADER_SYNC_MESSAGE_TYPE_BYTES + HEADER_SYNC_COUNT_BYTES] = 0; + frame + .payload + .truncate(frame.payload.len() - HEADER_SYNC_BLOCK_COMMITMENT_ROOTS_BYTES); + + let flow = deliver(&handle, Some(expected), peer(), frame); + + assert!( + !matches!(flow, Flow::Reject(SinkReject::Protocol(_))), + "a rootless solicited Headers response must not be a fatal protocol reject \ + (it disconnects an honest peer that simply has no roots to serve)" + ); + match events.try_recv() { + Ok(HeaderSyncEvent::WireMessage { + msg: + HeaderSyncMessage::Headers { + headers, + tree_aux_roots, + .. + }, + .. + }) => { + assert_eq!(headers.len(), 1); + assert!( + tree_aux_roots.is_empty(), + "the rootless response is forwarded as-is for non-punitive handling" + ); + } + other => panic!( + "expected the rootless solicited response to reach the reactor, got {other:?}" + ), + } + } + /// The peer-local correlation queue is FIFO and is filled by draining ready /// commands. This is the invariant `run_peer` relies on: an expectation /// recorded by a `RecordExpectedHeaders` command is drained and available to diff --git a/zebra-network/src/zakura/header_sync/reactor.rs b/zebra-network/src/zakura/header_sync/reactor.rs index fa23495776e..4b4a1d5d9b7 100644 --- a/zebra-network/src/zakura/header_sync/reactor.rs +++ b/zebra-network/src/zakura/header_sync/reactor.rs @@ -659,9 +659,17 @@ impl HeaderSyncReactor { None, ); let committed_history_tree = self.pending_header_history_tree(start_height, tip_height); - self.state - .pending_commits - .retain(|_, commit| !commit.range.is_within(start_height, tip_height)); + let mut clear_assignments = Vec::new(); + self.state.pending_commits.retain(|_, commit| { + let delivered = commit.delivered_range.is_within(start_height, tip_height); + if delivered { + clear_assignments.push(commit.requested_range); + } + !delivered + }); + for range in clear_assignments { + self.state.schedule.retire_request(range); + } self.state .schedule .mark_range_covered(start_height, tip_height); @@ -707,9 +715,9 @@ impl HeaderSyncReactor { }; if let Some(commit) = self.state.pending_commits.remove(&key) { if kind == HeaderSyncCommitFailureKind::Local { - self.state.schedule.clear_assignment(commit.range); + self.state.schedule.clear_assignment(commit.requested_range); } - self.state.schedule.retry(commit.range); + self.state.schedule.retry(commit.requested_range); } self.schedule().await; } @@ -1323,6 +1331,10 @@ impl HeaderSyncReactor { None }; + let delivered_range = RangeRequest { + count: header_count, + ..outstanding.range + }; self.state.pending_commits.insert( PendingCommitKey { peer: peer.clone(), @@ -1330,7 +1342,8 @@ impl HeaderSyncReactor { count: header_count, }, PendingHeaderCommit { - range: outstanding.range, + requested_range: outstanding.range, + delivered_range, verified_roots: verified_roots.clone(), }, ); @@ -1418,7 +1431,8 @@ impl HeaderSyncReactor { .pending_commits .values() .find(|commit| { - commit.range.start_height == start_height && commit.range.end_height() == tip_height + commit.delivered_range.start_height == start_height + && commit.delivered_range.end_height() == tip_height }) .and_then(|commit| commit.verified_roots.as_ref()) .map(|verified_roots| Arc::new(verified_roots.tree().clone())) @@ -1461,7 +1475,7 @@ impl HeaderSyncReactor { self.state.schedule.clear_forward(); self.state .pending_commits - .retain(|_, commit| commit.range.priority != RangePriority::Forward); + .retain(|_, commit| commit.requested_range.priority != RangePriority::Forward); self.cancel_forward_outstanding(); self.publish_best_tip_reanchored(height, hash).await; } diff --git a/zebra-network/src/zakura/header_sync/scheduler.rs b/zebra-network/src/zakura/header_sync/scheduler.rs index 583d829e30a..c3dd9080249 100644 --- a/zebra-network/src/zakura/header_sync/scheduler.rs +++ b/zebra-network/src/zakura/header_sync/scheduler.rs @@ -162,6 +162,14 @@ impl RangeScheduler { self.assigned.remove(&range); } + pub(super) fn retire_request(&mut self, range: RangeRequest) { + match range.priority { + RangePriority::Forward => self.forward.retain(|queued| *queued != range), + RangePriority::Backward => self.backward.retain(|queued| *queued != range), + } + self.clear_assignment(range); + } + pub(super) fn clear_forward(&mut self) { self.forward.clear(); self.assigned diff --git a/zebra-network/src/zakura/header_sync/state.rs b/zebra-network/src/zakura/header_sync/state.rs index b97e44b9d64..e07e67378f8 100644 --- a/zebra-network/src/zakura/header_sync/state.rs +++ b/zebra-network/src/zakura/header_sync/state.rs @@ -173,7 +173,19 @@ impl HeaderSyncCore { #[derive(Clone, Debug)] pub(super) struct PendingHeaderCommit { - pub(super) range: RangeRequest, + /// The full range requested from the peer. + /// + /// A peer may legally return a short prefix of the requested range. Keep the + /// requested range so success, local commit failure, and reanchor cleanup can + /// clear or retry the scheduler assignment that was created for the original + /// `GetHeaders` request. + pub(super) requested_range: RangeRequest, + /// The range actually delivered by the peer and handed to the commit path. + /// + /// Successful commit events are reported for this delivered range, so coverage + /// and verified frontier-tree lookup must use this range rather than the + /// original request. + pub(super) delivered_range: RangeRequest, /// `Some` for aux-validated forward ranges; `None` for checkpoint-authenticated backward /// backfill ranges, which persist no provisional roots and never install a frontier tree. pub(super) verified_roots: diff --git a/zebra-network/src/zakura/header_sync/tests.rs b/zebra-network/src/zakura/header_sync/tests.rs index 432f4dda7d5..d9a9354c265 100644 --- a/zebra-network/src/zakura/header_sync/tests.rs +++ b/zebra-network/src/zakura/header_sync/tests.rs @@ -16,7 +16,7 @@ use std::{ sync::{Mutex, OnceLock}, }; use zebra_chain::{ - history_tree::HistoryTree, + history_tree::{HistoryTree, HistoryTreeBlockParts}, orchard, parallel::commitment_aux::BlockCommitmentRoots, parameters::{ @@ -5413,3 +5413,399 @@ async fn misbehavior_is_recorded_without_disconnecting_the_peer() { "misbehavior is record-only: an InvalidStatus peer must NOT be disconnected", ); } + +// --------------------------------------------------------------------------- +// Regression tests for requested-vs-delivered range identity and backfill. +// +// A server cannot serve the tree-aux root for its own header tip (the root is +// only confirmed by the next header), so the zebrad driver truncates +// root-carrying responses to root coverage. A successful-but-short delivery is +// therefore an expected, honest response β€” exactly the two-node bootstrap +// shape where a follower requests up to a header-leading peer's tip. The tests +// below assert the correct requester-side bookkeeping for that shape. +// --------------------------------------------------------------------------- + +/// A Heartwood-at-1 regtest (PoW checks are skipped on regtest) with checkpoints at genesis and +/// `checkpoint_height`, so synthetic post-Heartwood header chains carrying real ZIP-221 +/// commitments can drive the root-verifying forward path. +fn heartwood_regtest_with_checkpoint( + checkpoint_height: block::Height, + checkpoint_hash: block::Hash, +) -> Network { + let default_regtest = regtest_network(); + Network::new_regtest(RegtestParameters { + activation_heights: ConfiguredActivationHeights { + overwinter: Some(1), + sapling: Some(1), + blossom: Some(1), + heartwood: Some(1), + canopy: Some(1), + ..Default::default() + }, + checkpoints: Some(ConfiguredCheckpoints::HeightsAndHashes(vec![ + (block::Height(0), default_regtest.genesis_hash()), + (checkpoint_height, checkpoint_hash), + ])), + ..Default::default() + }) +} + +/// Builds a synthetic post-Heartwood header: real solution bytes from a mainnet vector, relinked +/// to `previous_hash`, with `commitment_bytes` set to the ZIP-221 chain history root of the +/// parent tree (the all-zero reserved value at Heartwood activation, where the tree is empty). +fn chain_history_test_header( + bytes: &[u8], + previous_hash: block::Hash, + parent_tree: &HistoryTree, +) -> Arc { + let mut header = *mainnet_header(bytes); + header.previous_block_hash = previous_hash; + header.commitment_bytes = parent_tree + .hash() + .map(<[u8; 32]>::from) + .unwrap_or(block::CHAIN_HISTORY_ACTIVATION_RESERVED) + .into(); + Arc::new(header) +} + +/// Folds the empty-root parts for `header` at `height` into `tree` β€” the same ZIP-221 leaf the +/// header-sync verifier folds when the peer supplies `root_at(height)` alongside the header. +fn fold_empty_roots( + network: &Network, + tree: &mut HistoryTree, + header: &block::Header, + height: block::Height, +) { + let roots = root_at(height); + tree.push_from_parts( + network, + HistoryTreeBlockParts { + header, + height, + sapling_root: &roots.sapling_root, + orchard_root: &roots.orchard_root, + ironwood_root: &roots.ironwood_root, + sapling_tx: roots.sapling_tx, + orchard_tx: roots.orchard_tx, + ironwood_tx: roots.ironwood_tx, + }, + ) + .expect("synthetic test chain extends the history tree"); +} + +/// Regression test: a legally-short served forward range must not +/// permanently wedge the scheduler. +/// +/// The short prefix commits, but the assignment for the *requested* range is only cleared on +/// disconnect, local commit failure, or timeout β€” never on the success path β€” and `prune_covered` +/// never fires for it because the requested end was never covered. The stale assignment then +/// blocks `RangeScheduler::ensure`'s start-height dedupe from ever queueing the follow-up +/// overlapping range (which shares the requested range's start height), so the follower wedges +/// below the checkpoint and never re-requests the missing height. +#[tokio::test(flavor = "current_thread")] +async fn short_served_forward_range_must_not_wedge_the_scheduler() { + let block3 = mainnet_header(&BLOCK_MAINNET_3_BYTES); + let block3_hash = block::Hash::from(block3.as_ref()); + let block4 = mainnet_header(&BLOCK_MAINNET_4_BYTES); + let block4_hash = block::Hash::from(block4.as_ref()); + let (network, _) = checkpoint_testnet_with_hash(block::Height(3), block3_hash); + let mut fixture = spawn_test_reactor(startup_for( + network.clone(), + (block::Height(0), network.genesis_hash()), + Some((block::Height(3), block3_hash)), + )); + let peer_id = peer(90); + + connect_peer(&fixture, peer_id.clone()).await; + advertise_tip( + &fixture, + peer_id.clone(), + block::Height(0), + block::Height(5), + DEFAULT_HS_RANGE, + 1, + ) + .await; + + // The follower requests up to the peer's tip: heights 4..=5, with roots. + let (served_peer, start_height, count) = next_outbound_get_headers(&mut fixture.actions).await; + assert_eq!(served_peer, peer_id); + assert_eq!(start_height, block::Height(4)); + assert_eq!(count, 2); + + // The peer serves a legally-short prefix: header 4 only, with its matching root (a server + // can never serve the root for its own tip, so it truncates the response to root coverage). + let matching_roots = + header_matching_roots(&network, block::Height(4), std::slice::from_ref(&block4)); + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id.clone(), + msg: roots_message_from(block::Height(4), vec![block4], matching_roots), + }) + .await + .unwrap(); + + // The short prefix is accepted and committed. + loop { + match next_non_query_action(&mut fixture.actions).await { + HeaderSyncAction::CommitHeaderRange { + peer, + start_height, + headers, + .. + } => { + assert_eq!(peer, peer_id); + assert_eq!(start_height, block::Height(4)); + assert_eq!(headers.len(), 1, "the short prefix is a legal response"); + break; + } + HeaderSyncAction::Misbehavior { peer, reason } => { + panic!("unexpected misbehavior from {peer:?}: {reason:?}"); + } + _ => {} + } + } + fixture + .handle + .send(HeaderSyncEvent::HeaderRangeCommitted { + start_height: block::Height(4), + tip_height: block::Height(4), + tip_hash: block4_hash, + tip_parent_hash: Some(block3_hash), + }) + .await + .unwrap(); + + // The uncovered remainder (height 5) must be re-requested from the peer. If the stale + // requested-range assignment is never cleared, the scheduler dedupes every follow-up range + // sharing its start height and no further `GetHeaders` is ever sent. + let requested_remainder = async { + loop { + match fixture.actions.recv().await { + Some(HeaderSyncAction::SendMessage { + msg: + HeaderSyncMessage::GetHeaders { + start_height, + count, + .. + }, + .. + }) => { + let end = start_height.0.saturating_add(count.saturating_sub(1)); + if start_height.0 <= 5 && end >= 5 { + break; + } + } + Some(HeaderSyncAction::Misbehavior { peer, reason }) => { + panic!("unexpected misbehavior from {peer:?}: {reason:?}"); + } + Some(_) => {} + None => panic!("reactor action channel closed"), + } + } + }; + tokio::time::timeout(std::time::Duration::from_secs(3), requested_remainder) + .await + .expect("scheduler wedged: height 5 was never re-requested after a legally-short delivery"); +} + +/// Regression test: a legally-short delivery must install the frontier +/// history tree it just verified, keyed by the *delivered* range β€” not discard it because the +/// pending-commit lookup matches on the requested range end. +/// +/// `pending_header_history_tree` matches pending commits on the requested `range.end_height()`, +/// so a short (but successful) delivery commits its headers while the verified tree for the +/// delivered prefix is dropped. The next overlapping forward range then finds the cached tree +/// behind its parent and dispatches a `QueryBestHeaderHistoryTree` rebuild β€” an O(frontier) +/// durable-state fold per short range β€” instead of committing directly. +#[tokio::test(flavor = "current_thread")] +async fn short_served_forward_range_must_install_the_delivered_frontier_tree() { + let genesis_hash = regtest_network().genesis_hash(); + + // A synthetic post-Heartwood chain 1..=4 with real ZIP-221 commitments: header H commits to + // the history tree of its parent, so the trees are folded alongside the headers. + let mut tree = HistoryTree::default(); + let h1 = chain_history_test_header(&BLOCK_MAINNET_1_BYTES, genesis_hash, &tree); + let h1_hash = block::Hash::from(h1.as_ref()); + let network = heartwood_regtest_with_checkpoint(block::Height(1), h1_hash); + + fold_empty_roots(&network, &mut tree, &h1, block::Height(1)); + let tree_at_1 = tree.clone(); + let h2 = chain_history_test_header(&BLOCK_MAINNET_2_BYTES, h1_hash, &tree); + let h2_hash = block::Hash::from(h2.as_ref()); + fold_empty_roots(&network, &mut tree, &h2, block::Height(2)); + let h3 = chain_history_test_header(&BLOCK_MAINNET_3_BYTES, h2_hash, &tree); + let h3_hash = block::Hash::from(h3.as_ref()); + fold_empty_roots(&network, &mut tree, &h3, block::Height(3)); + let h4 = chain_history_test_header(&BLOCK_MAINNET_4_BYTES, h3_hash, &tree); + + let mut startup = startup_for( + network.clone(), + (block::Height(0), genesis_hash), + Some((block::Height(1), h1_hash)), + ); + startup.best_header_history_tree = Arc::new(tree_at_1); + let mut fixture = spawn_test_reactor(startup); + let peer_id = peer(91); + + connect_peer(&fixture, peer_id.clone()).await; + advertise_tip( + &fixture, + peer_id.clone(), + block::Height(0), + block::Height(4), + DEFAULT_HS_RANGE, + 1, + ) + .await; + + // Heights 2..=4 are requested with roots. + let (served_peer, start_height, count) = next_outbound_get_headers(&mut fixture.actions).await; + assert_eq!(served_peer, peer_id); + assert_eq!(start_height, block::Height(2)); + assert_eq!(count, 3); + + // The peer legally serves a short prefix (2..=3); verification confirms and folds height 2, + // positioning the just-verified frontier tree at height 2. + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id.clone(), + msg: headers_message_from(block::Height(2), vec![h2.clone(), h3.clone()]), + }) + .await + .unwrap(); + loop { + match next_non_query_action(&mut fixture.actions).await { + HeaderSyncAction::CommitHeaderRange { + start_height, + headers, + verified_roots, + .. + } => { + assert_eq!(start_height, block::Height(2)); + assert_eq!(headers.len(), 2); + let verified_roots = verified_roots.expect("root-carrying range verifies"); + assert_eq!( + verified_roots.confirmed_tip(), + Some(block::Height(2)), + "the delivered prefix confirms height 2 and folds its root", + ); + break; + } + HeaderSyncAction::Misbehavior { peer, reason } => { + panic!("unexpected misbehavior from {peer:?}: {reason:?}"); + } + _ => {} + } + } + fixture + .handle + .send(HeaderSyncEvent::HeaderRangeCommitted { + start_height: block::Height(2), + tip_height: block::Height(3), + tip_hash: h3_hash, + tip_parent_hash: Some(h2_hash), + }) + .await + .unwrap(); + + // The next forward range overlaps the committed tip: heights 3..=4 anchored on header 2. + let (served_peer, start_height, count) = next_outbound_get_headers(&mut fixture.actions).await; + assert_eq!(served_peer, peer_id); + assert_eq!(start_height, block::Height(3)); + assert_eq!(count, 2); + + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id.clone(), + msg: headers_message_from(block::Height(3), vec![h3, h4]), + }) + .await + .unwrap(); + + // With the delivered tree installed at height 2, this range verifies against it and commits + // directly. Discarding the just-verified tree instead forces a `QueryBestHeaderHistoryTree` + // rebuild from durable state β€” the recurring-cost path this test guards against. + loop { + match next_non_query_action(&mut fixture.actions).await { + HeaderSyncAction::CommitHeaderRange { + start_height, + headers, + .. + } => { + assert_eq!(start_height, block::Height(3)); + assert_eq!(headers.len(), 2); + break; + } + HeaderSyncAction::QueryBestHeaderHistoryTree { .. } => { + panic!( + "the verified frontier tree for the delivered prefix was discarded: the \ + overlapping follow-up range should verify against the installed tree, not \ + trigger a durable-state rebuild" + ); + } + HeaderSyncAction::Misbehavior { peer, reason } => { + panic!("unexpected misbehavior from {peer:?}: {reason:?}"); + } + _ => {} + } + } +} + +/// Regression test: backward (below-anchor) checkpoint backfill is unused by the current sync +/// wiring and does not work with root verification (backward ranges fold onto the previous +/// checkpoint's tree, which this reactor never tracks), so it must be explicitly disabled rather +/// than left scheduling silently. +/// +/// A node anchored at a checkpoint above genesis whose forward frontier already matches the +/// peer's tip has no forward work; any `GetHeaders` it emits is the backward backfill bracket. +/// It must emit none. +#[tokio::test(flavor = "current_thread")] +async fn backward_checkpoint_backfill_is_explicitly_disabled() { + let checkpoint_hash = block::Hash::from(mainnet_header(&BLOCK_MAINNET_3_BYTES).as_ref()); + let (network, _) = checkpoint_testnet_with_hash(block::Height(3), checkpoint_hash); + let mut fixture = spawn_test_reactor(startup_for( + network, + (block::Height(3), checkpoint_hash), + Some((block::Height(3), checkpoint_hash)), + )); + let peer_id = peer(92); + + connect_peer(&fixture, peer_id.clone()).await; + advertise_tip( + &fixture, + peer_id.clone(), + block::Height(0), + block::Height(3), + DEFAULT_HS_RANGE, + 1, + ) + .await; + + // Drain actions for a bounded window: no backward `GetHeaders` may be scheduled. + while let Ok(Some(action)) = tokio::time::timeout( + std::time::Duration::from_millis(250), + fixture.actions.recv(), + ) + .await + { + if let HeaderSyncAction::SendMessage { + msg: + HeaderSyncMessage::GetHeaders { + start_height, + count, + .. + }, + .. + } = action + { + panic!( + "backward checkpoint backfill must be explicitly disabled, but a below-anchor \ + range was requested: start {start_height:?} count {count}" + ); + } + } +} From d1713aee7dd2a185cb980c7e574c21da898c57e9 Mon Sep 17 00:00:00 2001 From: roman Date: Sun, 5 Jul 2026 16:45:44 -0600 Subject: [PATCH 06/12] address decode layer serving and make backwards fill disabled --- .../src/zakura/header_sync/reactor.rs | 44 ++++- zebra-network/src/zakura/header_sync/state.rs | 13 ++ zebra-network/src/zakura/header_sync/tests.rs | 166 +++++------------- zebra-network/src/zakura/header_sync/wire.rs | 8 +- zebra-network/src/zakura/testkit/cluster.rs | 116 ++++-------- 5 files changed, 140 insertions(+), 207 deletions(-) diff --git a/zebra-network/src/zakura/header_sync/reactor.rs b/zebra-network/src/zakura/header_sync/reactor.rs index 4b4a1d5d9b7..812238d2745 100644 --- a/zebra-network/src/zakura/header_sync/reactor.rs +++ b/zebra-network/src/zakura/header_sync/reactor.rs @@ -1080,12 +1080,44 @@ impl HeaderSyncReactor { peer_max_headers_per_response: u32, in_flight_count: usize, ) { - // A root-carrying request must be answered with one root per header; a plain (above-checkpoint) - // request must carry no roots (checked separately just below). Only enforce the one-per-header - // count when roots were actually requested. - if validate_body_sizes_len(headers.len(), body_sizes.len()).is_err() - || (outstanding.range.want_tree_aux_roots - && validate_tree_aux_roots_len(headers.len(), tree_aux_roots.len()).is_err()) + if validate_body_sizes_len(headers.len(), body_sizes.len()).is_err() { + self.report_misbehavior(peer, HeaderSyncMisbehavior::MalformedMessage) + .await; + self.state.schedule.retry(outstanding.range); + self.schedule().await; + return; + } + + // Tree-aux roots are an optional serving capability: a peer may legitimately have no + // roots for a root-carrying request (it can never serve the root for its own tip, and + // may not persist roots at all). A completely rootless non-empty response is therefore + // not misbehavior β€” the headers are unusable for this root-carrying range, so drop them, + // release the request, and retry the range without scoring the peer. + if outstanding.range.want_tree_aux_roots && !headers.is_empty() && tree_aux_roots.is_empty() + { + metrics::counter!("sync.header.response.rootless").increment(1); + self.record_advisory_unconfirmed(&peer); + self.trace_headers_received( + &peer, + outstanding.range.start_height, + u32::try_from(headers.len()).unwrap_or(u32::MAX), + outstanding.expected_max_count, + peer_max_headers_per_response, + in_flight_count, + outstanding.range.want_tree_aux_roots, + 0, + ); + self.state.schedule.clear_assignment(outstanding.range); + self.state.schedule.retry(outstanding.range); + self.schedule().await; + return; + } + + // A root-carrying request must otherwise be answered with one root per header; a plain + // (above-checkpoint) request must carry no roots (checked separately just below). Only + // enforce the one-per-header count when roots were actually requested. + if outstanding.range.want_tree_aux_roots + && validate_tree_aux_roots_len(headers.len(), tree_aux_roots.len()).is_err() { self.report_misbehavior(peer, HeaderSyncMisbehavior::MalformedMessage) .await; diff --git a/zebra-network/src/zakura/header_sync/state.rs b/zebra-network/src/zakura/header_sync/state.rs index e07e67378f8..7e0b1b38fe2 100644 --- a/zebra-network/src/zakura/header_sync/state.rs +++ b/zebra-network/src/zakura/header_sync/state.rs @@ -12,6 +12,10 @@ pub(super) const HEADER_SYNC_ADVISORY_BACKOFF: Duration = Duration::from_secs(60 pub(super) const HEADER_SYNC_ADVISORY_TTL: Duration = DEFAULT_LIVE_SERVICE_SUMMARY_TTL; pub(super) const HEADER_SYNC_STALE_ANCHOR_LINK_FAILURES: u32 = 3; pub(super) const HEADER_SYNC_STALE_ANCHOR_DISTINCT_PEERS: usize = 2; +/// Below-anchor checkpoint backfill is unused by the current sync wiring and cannot verify +/// roots (backward ranges fold onto the previous checkpoint's tree, which this reactor never +/// tracks), so it stays disabled. See [`HeaderSyncCore::refresh_backward_range`]. +pub(super) const BACKWARD_CHECKPOINT_BACKFILL_ENABLED: bool = false; #[derive(Clone, Debug)] pub(super) struct HeaderSyncCore { @@ -141,6 +145,15 @@ impl HeaderSyncCore { } pub(super) fn refresh_backward_range(&mut self, startup: &HeaderSyncStartup) { + // Below-anchor checkpoint backfill is explicitly disabled: backward ranges fold onto the + // previous checkpoint's tree, which this reactor never tracks, so their roots cannot be + // verified, and the current node wiring never consumes backfilled headers. Leaving the + // scheduling live would silently emit unsupported below-anchor `GetHeaders` requests. + // Re-enable once backfill has a consumer and checkpoint-bracket tree tracking. + if !BACKWARD_CHECKPOINT_BACKFILL_ENABLED { + return; + } + if self.anchor.0 == block::Height(0) { return; } diff --git a/zebra-network/src/zakura/header_sync/tests.rs b/zebra-network/src/zakura/header_sync/tests.rs index d9a9354c265..1603ba342ab 100644 --- a/zebra-network/src/zakura/header_sync/tests.rs +++ b/zebra-network/src/zakura/header_sync/tests.rs @@ -1255,19 +1255,31 @@ fn header_aux_validation_rejects_bad_roots_before_state_commit() { )); } +/// Tree-aux roots are an optional serving capability: a solicited non-empty `Headers` +/// response without roots must decode (the reactor handles it non-punitively), while the +/// same rootless frame with no correlated request is still rejected as unsolicited. #[test] -fn decode_rejects_non_empty_headers_without_tree_aux_roots() { +fn decode_accepts_rootless_non_empty_solicited_headers() { let headers = vec![mainnet_header(&BLOCK_MAINNET_1_BYTES)]; let mut encoded = headers_message(headers).encode().unwrap(); encoded[HEADER_SYNC_MESSAGE_TYPE_BYTES + HEADER_SYNC_COUNT_BYTES] = 0; encoded.truncate(encoded.len() - HEADER_SYNC_BLOCK_COMMITMENT_ROOTS_BYTES); + match HeaderSyncMessage::decode(&encoded, finalized_headers_context(1, 1)) { + Ok(HeaderSyncMessage::Headers { + headers, + tree_aux_roots, + .. + }) => { + assert_eq!(headers.len(), 1); + assert!(tree_aux_roots.is_empty()); + } + other => panic!("rootless solicited Headers must decode, got {other:?}"), + } + assert!(matches!( - HeaderSyncMessage::decode(&encoded, finalized_headers_context(1, 1)), - Err(HeaderSyncWireError::TreeAuxRootCountMismatch { - headers: 1, - roots: 0, - }) + HeaderSyncMessage::decode(&encoded, HeaderSyncDecodeContext::control()), + Err(HeaderSyncWireError::UnsolicitedHeaders) )); } @@ -1805,45 +1817,6 @@ async fn scheduler_creates_checkpoint_forward_before_backward_ranges() { } } -#[tokio::test(flavor = "current_thread")] -async fn scheduler_creates_backward_checkpoint_terminating_ranges() { - let (network, checkpoint_hash) = checkpoint_regtest(block::Height(3)); - let mut fixture = spawn_test_reactor(startup_for( - network, - (block::Height(3), checkpoint_hash), - Some((block::Height(3), checkpoint_hash)), - )); - let peer_id = peer(7); - - connect_peer(&fixture, peer_id.clone()).await; - advertise_tip( - &fixture, - peer_id, - block::Height(0), - block::Height(3), - DEFAULT_HS_RANGE, - 10, - ) - .await; - - loop { - if let HeaderSyncAction::SendMessage { - msg: - HeaderSyncMessage::GetHeaders { - start_height, - count, - want_tree_aux_roots: true, - }, - .. - } = next_non_query_action(&mut fixture.actions).await - { - assert_eq!(start_height, block::Height(1)); - assert_eq!(count, 3); - break; - } - } -} - #[tokio::test(flavor = "current_thread")] async fn forward_ranges_below_checkpoint_handoff_request_tree_aux_roots() { let network = Parameters::build() @@ -1985,8 +1958,11 @@ async fn incoming_headers_match_outstanding_before_commit() { } } +/// Tree-aux roots are an optional serving capability, so a rootless non-empty response to a +/// root-carrying request is not misbehavior: the headers are unusable for the root regime and +/// must not commit, but the peer keeps its session and the range is retried. #[tokio::test(flavor = "current_thread")] -async fn rootless_non_empty_response_is_malformed() { +async fn rootless_non_empty_response_retries_without_misbehavior() { let checkpoint_hash = block::Hash::from(mainnet_header(&BLOCK_MAINNET_3_BYTES).as_ref()); let (network, _) = checkpoint_testnet_with_hash(block::Height(3), checkpoint_hash); let first_checkpoint = block::Height(3); @@ -2024,12 +2000,21 @@ async fn rootless_non_empty_response_is_malformed() { .await .unwrap(); + // The range must be retried (a fresh `GetHeaders` for the same start height), with no + // commit and no misbehavior along the way. loop { match next_non_query_action(&mut fixture.actions).await { + HeaderSyncAction::SendMessage { + msg: + HeaderSyncMessage::GetHeaders { + start_height, + want_tree_aux_roots: true, + .. + }, + .. + } if start_height == start => break, HeaderSyncAction::Misbehavior { peer, reason } => { - assert_eq!(peer, peer_id); - assert_eq!(reason, HeaderSyncMisbehavior::MalformedMessage); - break; + panic!("rootless response must not be misbehavior, got {reason:?} from {peer:?}"); } HeaderSyncAction::CommitHeaderRange { .. } => { panic!("a rootless non-empty response must not commit") @@ -2037,6 +2022,7 @@ async fn rootless_non_empty_response_is_malformed() { _ => {} } } + assert_no_commit_or_misbehavior(&mut fixture.actions).await; } #[tokio::test(flavor = "current_thread")] @@ -4852,77 +4838,17 @@ async fn truncated_finalized_backfill_is_rejected_before_commit() { assert_no_commit_or_misbehavior(&mut fixture.actions).await; } -#[tokio::test(flavor = "current_thread")] -async fn backward_checkpoint_backfill_accepts_linking_run_as_finalized() { - let headers = [ - mainnet_header(&BLOCK_MAINNET_1_BYTES), - mainnet_header(&BLOCK_MAINNET_2_BYTES), - mainnet_header(&BLOCK_MAINNET_3_BYTES), - ]; - let checkpoint_hash = block::Hash::from(headers[2].as_ref()); - let (network, _) = checkpoint_testnet_with_hash(block::Height(3), checkpoint_hash); - let mut fixture = spawn_test_reactor(startup_for( - network, - (block::Height(3), checkpoint_hash), - Some((block::Height(3), checkpoint_hash)), - )); - let peer_id = peer(45); - - connect_peer(&fixture, peer_id.clone()).await; - advertise_tip( - &fixture, - peer_id.clone(), - block::Height(0), - block::Height(3), - DEFAULT_HS_RANGE, - 1, - ) - .await; - loop { - if matches!( - next_non_query_action(&mut fixture.actions).await, - HeaderSyncAction::SendMessage { - msg: HeaderSyncMessage::GetHeaders { .. }, - .. - } - ) { - break; - } - } - - fixture - .handle - .send(HeaderSyncEvent::WireMessage { - peer: peer_id.clone(), - msg: headers_message(headers.to_vec()), - }) - .await - .unwrap(); - - match next_non_query_action(&mut fixture.actions).await { - HeaderSyncAction::CommitHeaderRange { - peer, - start_height, - headers, - finalized, - .. - } => { - assert_eq!(peer, peer_id); - assert_eq!(start_height, block::Height(1)); - assert_eq!(headers.len(), 3); - assert!(finalized); - } - action => panic!("unexpected action: {action:?}"), - } -} +// Backward (below-anchor) checkpoint backfill is explicitly disabled (see +// `backward_checkpoint_backfill_is_explicitly_disabled`), so finalized checkpoint-range +// validation is exercised through the forward genesis-backfill path below. #[tokio::test(flavor = "current_thread")] async fn checkpoint_backfill_rejects_non_contiguous_run_before_commit() { - let (network, checkpoint_hash) = checkpoint_regtest(block::Height(3)); + let (network, _checkpoint_hash) = checkpoint_regtest(block::Height(3)); let mut fixture = spawn_test_reactor(startup_for( - network, - (block::Height(3), checkpoint_hash), - Some((block::Height(3), checkpoint_hash)), + network.clone(), + (block::Height(0), network.genesis_hash()), + None, )); let peer_id = peer(10); @@ -5034,10 +4960,12 @@ async fn checkpoint_backfill_rejects_checkpoint_hash_mismatch_before_commit() { ]; let divergent_checkpoint_hash = block::Hash::from(headers[0].as_ref()); let (network, _) = checkpoint_testnet_with_hash(block::Height(3), divergent_checkpoint_hash); + // Forward genesis backfill toward the (divergent) first checkpoint: the linking run's last + // header must hash-match the checkpoint or the whole finalized range is rejected. let mut fixture = spawn_test_reactor(startup_for( - network, - (block::Height(3), divergent_checkpoint_hash), - Some((block::Height(3), divergent_checkpoint_hash)), + network.clone(), + (block::Height(0), network.genesis_hash()), + None, )); let peer_id = peer(46); diff --git a/zebra-network/src/zakura/header_sync/wire.rs b/zebra-network/src/zakura/header_sync/wire.rs index 04a42ed8563..af097075e3d 100644 --- a/zebra-network/src/zakura/header_sync/wire.rs +++ b/zebra-network/src/zakura/header_sync/wire.rs @@ -207,8 +207,14 @@ impl HeaderSyncMessage { for _ in 0..count { tree_aux_roots.push(BlockCommitmentRoots::zcash_deserialize(&mut reader)?); } + validate_tree_aux_roots_len(count, tree_aux_roots.len())?; } - validate_tree_aux_roots_len(count, tree_aux_roots.len())?; + // `has_roots == false` with non-empty headers is legal on the requester side: + // roots are an optional serving capability, and a solicited response reaching + // this decode is always correlated to an in-flight request (an uncorrelated + // `Headers` frame was already rejected as `UnsolicitedHeaders` above). The + // reactor decides how to handle a rootless response to a root-carrying + // request; it must not be a fatal wire reject that disconnects the peer. if let Some(requested) = context.requested { validate_tree_aux_root_heights(requested.start_height, &tree_aux_roots)?; } diff --git a/zebra-network/src/zakura/testkit/cluster.rs b/zebra-network/src/zakura/testkit/cluster.rs index cc2a3fe8af0..3ea8f108fb3 100644 --- a/zebra-network/src/zakura/testkit/cluster.rs +++ b/zebra-network/src/zakura/testkit/cluster.rs @@ -1256,39 +1256,6 @@ mod tests { .expect("e2e network has enough checkpoint coverage") } - fn e2e_network_with_checkpoint_hash(height: u32, hash: block::Hash) -> Network { - let checkpoints = vec![ - (block::Height(0), mainnet_genesis_hash()), - (block::Height(height), hash), - ]; - - TestnetParameters::build() - .with_genesis_hash(mainnet_genesis_hash()) - .expect("mainnet genesis vector hash parses") - .with_activation_heights(ConfiguredActivationHeights { - before_overwinter: None, - overwinter: Some(1), - sapling: Some(1), - blossom: Some(1), - heartwood: Some(1), - canopy: Some(1), - nu5: None, - nu6: None, - nu6_1: None, - nu6_2: None, - nu6_3: None, - nu7: None, - #[cfg(zcash_unstable = "zfuture")] - zfuture: None, - }) - .expect("height-1 activation set is valid") - .with_funding_streams(Vec::new()) - .with_checkpoints(ConfiguredCheckpoints::HeightsAndHashes(checkpoints)) - .expect("e2e checkpoints use valid header hashes") - .to_network() - .expect("e2e network has enough checkpoint coverage") - } - fn checkpoint_network(checkpoint_height: u32) -> (Network, block::Hash) { let checkpoint_hash = mainnet_block(block_bytes(checkpoint_height)).hash(); (e2e_network([checkpoint_height]), checkpoint_hash) @@ -2839,12 +2806,16 @@ mod tests { Ok(()) } + /// A checkpoint-anchored node syncs forward past its anchor, and below-anchor backward + /// backfill stays explicitly disabled: no `GetHeaders` for the bracket below the anchor is + /// ever sent, and the below-anchor headers stay absent (see + /// `backward_checkpoint_backfill_is_explicitly_disabled` for the reactor-level regression). #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn header_sync_e2e_checkpoint_forward_then_backward_finalizes_backfill( + async fn header_sync_e2e_checkpoint_forward_syncs_without_backward_backfill( ) -> Result<(), BoxError> { let _guard = zebra_test::init(); let mut capture = TraceCapture::for_test_with_keep_override( - "header_sync_e2e_checkpoint_forward_then_backward_finalizes_backfill", + "header_sync_e2e_checkpoint_forward_syncs_without_backward_backfill", false, )?; let (network, checkpoint_hash) = checkpoint_network(3); @@ -2887,18 +2858,35 @@ mod tests { .await; cluster.wait_for_tip(checkpointed, block::Height(4)).await?; - await_until( - "checkpoint backfill headers committed", - Duration::from_secs(5), - || cluster.has_headers(checkpointed, 1..=3), - ) - .await?; + + // Give the scheduler time to (incorrectly) emit a backward bracket if it were still + // enabled, then assert nothing below the anchor was requested or stored. + tokio::time::sleep(Duration::from_millis(500)).await; + assert!( + !cluster.has_headers(checkpointed, 1..=3), + "below-anchor headers must not be backfilled while backward backfill is disabled" + ); capture.flush().await; let reader = capture.reader()?; let target_trace = reader.node("02").table("header_sync"); target_trace.assert_header_range_request(4, 1); - target_trace.assert_header_range_request(1, 3); + let backward_requests = target_trace + .rows() + .iter() + .filter(|row| { + row.get("event").and_then(serde_json::Value::as_str) + == Some(hs_trace::HEADER_GET_HEADERS_SENT) + && row + .get(hs_trace::RANGE_START) + .and_then(serde_json::Value::as_u64) + == Some(1) + }) + .count(); + assert_eq!( + backward_requests, 0, + "backward checkpoint backfill is disabled: no below-anchor GetHeaders may be sent" + ); assert_eq!( cluster.finalized_height(checkpointed).await, block::Height(3) @@ -3260,44 +3248,10 @@ mod tests { .wait_for_misbehavior_reason(bad_daa_victim, HeaderSyncMisbehavior::InvalidRange) .await?; - let bad_checkpoint_backfill = e2e_peer(101); - let checkpoint_hash = mainnet_block(&BLOCK_MAINNET_1_BYTES).hash(); - let checkpoint_network = e2e_network_with_checkpoint_hash(3, checkpoint_hash); - let checkpointed = cluster.spawn_node( - 6, - checkpoint_network, - (block::Height(3), checkpoint_hash), - E2eHeaderStore::with_checkpoint_anchor(3), - ZakuraTrace::new(capture.tracer_for_node(6), "06"), - )?; - cluster.start_drivers(); - cluster - .connect_peer(checkpointed, bad_checkpoint_backfill.clone()) - .await; - cluster - .inject( - checkpointed, - bad_checkpoint_backfill.clone(), - status_for_tip(3, 4, 1), - ) - .await; - cluster - .wait_for_get_headers(checkpointed, &bad_checkpoint_backfill, block::Height(1), 3) - .await?; - cluster - .inject( - checkpointed, - bad_checkpoint_backfill, - headers_message(vec![ - mainnet_block(&BLOCK_MAINNET_1_BYTES).header.clone(), - mainnet_block(&BLOCK_MAINNET_2_BYTES).header.clone(), - mainnet_block(&BLOCK_MAINNET_3_BYTES).header.clone(), - ]), - ) - .await; - cluster - .wait_for_misbehavior_reason(checkpointed, HeaderSyncMisbehavior::InvalidRange) - .await?; + // Checkpoint-hash-mismatch backfill responses are covered at the reactor level by + // `checkpoint_backfill_rejects_checkpoint_hash_mismatch_before_commit` (via the forward + // genesis-backfill path); the backward below-anchor bracket that used to drive it here + // is explicitly disabled. let over_cap = e2e_peer(91); cluster.connect_peer(victim, over_cap.clone()).await; @@ -3389,7 +3343,7 @@ mod tests { trace.assert_header_violation("status_spam"); trace.assert_header_violation("new_block_spam"); trace.assert_header_violation("malformed_message"); - for node in ["03", "04", "05", "06"] { + for node in ["03", "04", "05"] { reader .node(node) .table("header_sync") From 593d63b4f87a4e1823ede69fd8928d7f8b507281 Mon Sep 17 00:00:00 2001 From: roman Date: Sun, 5 Jul 2026 17:04:45 -0600 Subject: [PATCH 07/12] fix test --- zebra-network/src/zakura/testkit/cluster.rs | 32 +++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/zebra-network/src/zakura/testkit/cluster.rs b/zebra-network/src/zakura/testkit/cluster.rs index 3ea8f108fb3..bc7f920c7a1 100644 --- a/zebra-network/src/zakura/testkit/cluster.rs +++ b/zebra-network/src/zakura/testkit/cluster.rs @@ -1220,6 +1220,21 @@ mod tests { } fn e2e_network(checkpoints: impl IntoIterator) -> Network { + e2e_network_with_shielded_activation(checkpoints, 1) + } + + /// Builds an e2e testnet whose Sapling..Canopy upgrades activate at `shielded_activation`. + /// + /// Overwinter stays at height 1 and NU5+ stay inactive. Setting `shielded_activation` above the + /// synced range keeps those heights in the pre-Sapling regime, where headers present unverified + /// `PreSaplingReserved` commitments β€” the only regime the empty placeholder tree-aux roots + /// served by the test peer can satisfy. `Canopy` must be present and + /// `max_checkpoint >= Canopy - 1` for the checkpoint-coverage check, so the highest usable value + /// is `max_checkpoint + 1`. + fn e2e_network_with_shielded_activation( + checkpoints: impl IntoIterator, + shielded_activation: u32, + ) -> Network { let checkpoints = std::iter::once((block::Height(0), mainnet_genesis_hash())) .chain(checkpoints.into_iter().map(|height| { ( @@ -1235,10 +1250,10 @@ mod tests { .with_activation_heights(ConfiguredActivationHeights { before_overwinter: None, overwinter: Some(1), - sapling: Some(1), - blossom: Some(1), - heartwood: Some(1), - canopy: Some(1), + sapling: Some(shielded_activation), + blossom: Some(shielded_activation), + heartwood: Some(shielded_activation), + canopy: Some(shielded_activation), nu5: None, nu6: None, nu6_1: None, @@ -1248,7 +1263,7 @@ mod tests { #[cfg(zcash_unstable = "zfuture")] zfuture: None, }) - .expect("height-1 activation set is valid") + .expect("e2e activation set is valid") .with_funding_streams(Vec::new()) .with_checkpoints(ConfiguredCheckpoints::HeightsAndHashes(checkpoints)) .expect("e2e checkpoints use valid header hashes") @@ -2713,7 +2728,12 @@ mod tests { false, )?; let mut cluster = HeaderSyncE2eCluster::new(); - let network = e2e_network([4]); + // Sapling..Canopy activate at height 5, just above the synced 1..=4 range and exactly at the + // checkpoint-coverage bound (Canopy - 1 == the height-4 checkpoint). This keeps heights 1..=4 + // pre-Sapling, so their `PreSaplingReserved` commitments verify against the empty placeholder + // tree-aux roots the test peer serves β€” otherwise a post-Heartwood schedule reads these real + // mainnet vectors as `ChainHistoryRoot` and rejects the placeholder roots. + let network = e2e_network_with_shielded_activation([4], 5); let anchor = (block::Height(0), mainnet_genesis_hash()); let source = cluster.spawn_node( 1, From 2f94ba6bc9459cd23b0cdf151d19979d1838dadf Mon Sep 17 00:00:00 2001 From: roman Date: Sun, 5 Jul 2026 18:21:14 -0600 Subject: [PATCH 08/12] fix(network): reanchor Zakura header tip when lazy rebuild folds below it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime header-frontier history-tree lazy rebuild discarded the frontier that `ReadResponse::BestHeaderHistoryTree` returns, keeping only the tree (`{ tree, .. }`) and echoing the requested `best_header_tip` back to the reactor. When durable roots have a gap β€” a non-Zakura path (checkpoint/legacy sync) committed below the checkpoint without persisting header-commit roots β€” the fold stops below `best_header_tip - 1`, so the rebuilt tree never reaches the current tip's parent. The reactor installed the low tree but left the tip high, so every forward range re-detected the behind tree and re-triggered the identical deterministic rebuild forever (a network-paced no-progress loop that also re-ran the O(gap) durable-root fold each retry). The startup path already honors the frontier contract (resume from `frontier + 1`, anchor on the frontier hash); the runtime path did not. This threads the frontier from where it is known (the zebrad driver, which owns the state read) to where it is actionable (the reactor, which owns the tip): - Add `HeaderFrontierReanchor` and carry it on `BestHeaderHistoryTreeLoaded`. - Driver `header_frontier_reanchor` resolves the resume anchor on a gap (mirroring the startup resume), `None` in the common no-gap case. - Reactor `reanchor_header_frontier` drops the tip onto the rebuilt tree, clearing the stranded forward schedule and repositioning tip/parent-hash, then reschedules so the resumed range verifies against the reinstalled tree. - Generalize `publish_best_tip_reanchored` to take the overlap parent hash. Adds `lazy_rebuild_reanchors_tip_onto_lower_frontier_tree` (verified to fail without the reanchor) and the `sync.header.history_tree.reanchor` counter; documents both in docs/design/verified-commitment-trees.md (6.4, 13). --- docs/design/verified-commitment-trees.md | 8 + .../src/zakura/header_sync/events.rs | 28 +++- zebra-network/src/zakura/header_sync/mod.rs | 5 +- .../src/zakura/header_sync/reactor.rs | 57 ++++++- zebra-network/src/zakura/header_sync/tests.rs | 150 ++++++++++++++++++ zebra-network/src/zakura/testkit/cluster.rs | 1 + .../start/zakura/header_sync_driver.rs | 88 +++++++++- 7 files changed, 318 insertions(+), 19 deletions(-) diff --git a/docs/design/verified-commitment-trees.md b/docs/design/verified-commitment-trees.md index 0a439c65236..1783c94b1d3 100644 --- a/docs/design/verified-commitment-trees.md +++ b/docs/design/verified-commitment-trees.md @@ -455,6 +455,13 @@ other peers are already header-authenticated, and so a restart never trusts an u advance the finalized tip past the reactor's cached `verified_block_tip` during the round-trip, so folding must start from the tip actually present rather than the caller's (possibly lagging) value β€” otherwise the base read returns `None` and the rebuild degrades to a network-paced no-progress loop. + And the reload carries the confirmed frontier `(height, hash)` the fold actually reached, not just the + tree: when durable roots have a gap the fold stops _below_ `best_header_tip - 1`, so the rebuilt tree + never reaches the current tip's parent. The reactor then reanchors its header tip down onto that + frontier β€” resuming one block above it, anchored on the frontier hash, exactly as the startup + reconstruction does (`sync.header.history_tree.reanchor`) β€” instead of keeping the stale higher tip, + where every forward range would re-detect the behind tree and re-trigger the identical deterministic + rebuild forever. - **Verify before persisting.** When a below-checkpoint header range arrives, the reactor folds its supplied roots into the running header-frontier history tree and checks each header's commitment against it (`verify_supplied_roots_from_parts` in `zebra-chain/src/parallel/commitment_aux_verify.rs`, @@ -763,6 +770,7 @@ Live commit-path counters distinguish the fast and legacy paths and the failure | `state.vct.fast_path.miss` | a finalized commit did not take the fast path | | `state.vct.root.stalled.height` (gauge) | a height stuck on a retryable stall past the warn threshold | | `sync.header.history_tree.rebuild` | header-frontier history tree lazily rebuilt (Β§6.4) because a non-Zakura commit ran ahead of it below the checkpoint; **stays 0 in the normal header-leading path**, so a nonzero value flags fallback/catch-up | +| `sync.header.history_tree.reanchor` | a lazy rebuild (Β§6.4) folded to a frontier _below_ `best_header_tip - 1` (durable roots had a gap), so the reactor reanchored its header tip down onto the rebuilt tree; a subset of `rebuild`, and likewise 0 in the normal path | The header-sync `headers_received` / `headers_served` / commit-state trace rows also carry `want_tree_aux_roots` and `tree_aux_roots_len`, so root delivery is visible per range. The diff --git a/zebra-network/src/zakura/header_sync/events.rs b/zebra-network/src/zakura/header_sync/events.rs index 7b04ed3844f..33d624c39b2 100644 --- a/zebra-network/src/zakura/header_sync/events.rs +++ b/zebra-network/src/zakura/header_sync/events.rs @@ -21,6 +21,24 @@ pub struct HeaderSyncFrontiers { pub verified_block_hash: block::Hash, } +/// Where to reanchor the header frontier after a lazy history-tree rebuild whose durable header-root +/// frontier folded *below* `best_header_tip - 1` (a gap left by a non-Zakura commit racing ahead). +/// +/// Mirrors the startup resume: the rebuilt tree sits at `parent_hash` (the confirmed frontier), and +/// header sync resumes one block above it at `tip`, re-fetching that block so its root re-confirms. +/// Without this the header tip would stay above the tree and every forward range would re-trigger the +/// identical rebuild forever. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub struct HeaderFrontierReanchor { + /// Height to resume the header frontier at β€” one above the confirmed frontier (`frontier + 1`). + pub tip: block::Height, + /// Hash of the resume header at `tip`. + pub tip_hash: block::Hash, + /// Hash of the confirmed header-root frontier at `tip - 1`; the rebuilt tree sits here, and it is + /// the overlap anchor for the resumed forward range. + pub parent_hash: block::Hash, +} + /// Startup inputs for the dependency-neutral header-sync reactor. #[derive(Clone, Debug)] pub struct HeaderSyncStartup { @@ -251,9 +269,15 @@ pub enum HeaderSyncEvent { /// rebuild guard is always cleared. `history_tree` is `None` when the rebuild failed (the guard is /// cleared and the next forward range re-triggers it), `Some` on success. BestHeaderHistoryTreeLoaded { - /// Best header tip the tree is positioned at when this reload was requested. + /// Best header tip the reload was requested against, used as a staleness guard: the tree is + /// only installed if the reactor tip has not moved since the query was dispatched. best_header_tip: block::Height, - /// History tree reconstructed by state, positioned at `best_header_tip`; `None` on failure. + /// Where to reanchor the header frontier, or `None` when no reanchor is needed (the tree + /// folded to `best_header_tip - 1`, the common no-gap case) or the rebuild failed. `Some` when + /// the durable header-root frontier folded *below* `best_header_tip - 1` β€” a gap left by a + /// non-Zakura commit β€” so the tip must drop onto the rebuilt tree to make progress. + reanchor: Option, + /// History tree reconstructed by state; `None` on failure. history_tree: Option>, }, /// State successfully committed a header range. diff --git a/zebra-network/src/zakura/header_sync/mod.rs b/zebra-network/src/zakura/header_sync/mod.rs index a875d919448..75a0d066654 100644 --- a/zebra-network/src/zakura/header_sync/mod.rs +++ b/zebra-network/src/zakura/header_sync/mod.rs @@ -52,8 +52,9 @@ pub use config::{ }; pub use error::{HeaderSyncStartError, HeaderSyncWireError}; pub use events::{ - ExpectedHeadersResponse, HeaderSyncAction, HeaderSyncCommitFailureKind, HeaderSyncEvent, - HeaderSyncFrontiers, HeaderSyncHandle, HeaderSyncMisbehavior, HeaderSyncStartup, + ExpectedHeadersResponse, HeaderFrontierReanchor, HeaderSyncAction, HeaderSyncCommitFailureKind, + HeaderSyncEvent, HeaderSyncFrontiers, HeaderSyncHandle, HeaderSyncMisbehavior, + HeaderSyncStartup, }; pub use reactor::spawn_header_sync_reactor; pub use service::HeaderSyncPeerSession; diff --git a/zebra-network/src/zakura/header_sync/reactor.rs b/zebra-network/src/zakura/header_sync/reactor.rs index 812238d2745..c5d2b7ed8fa 100644 --- a/zebra-network/src/zakura/header_sync/reactor.rs +++ b/zebra-network/src/zakura/header_sync/reactor.rs @@ -191,10 +191,15 @@ impl HeaderSyncReactor { } HeaderSyncEvent::BestHeaderHistoryTreeLoaded { best_header_tip, + reanchor, history_tree, } => { - self.handle_best_header_history_tree_loaded(best_header_tip, history_tree) - .await; + self.handle_best_header_history_tree_loaded( + best_header_tip, + reanchor, + history_tree, + ) + .await; } HeaderSyncEvent::HeaderRangeCommitted { start_height, @@ -626,23 +631,54 @@ impl HeaderSyncReactor { /// Installs a lazily-rebuilt header-frontier history tree (the answer to /// `QueryBestHeaderHistoryTree`). Always clears the in-flight guard so a failed rebuild /// (`history_tree == None`) does not wedge future rebuilds; installs only on success and only if - /// the best header tip has not moved since the query. Reschedules so the forward range that - /// triggered the rebuild is re-tried (against the repositioned tree, or re-triggering the rebuild - /// if it failed). + /// the best header tip has not moved since the query. + /// + /// When `reanchor` is set the durable header-root frontier folded *below* `best_header_tip - 1` + /// (a gap left by a non-Zakura commit racing ahead), so the tree does not reach the current tip's + /// parent. Drop the header frontier onto the rebuilt tree β€” mirroring the startup resume β€” so the + /// next forward range's parent matches the tree; otherwise the range would re-trigger the identical + /// deterministic rebuild forever. Reschedules so the triggering range is re-tried (against the + /// repositioned tree, or re-triggering the rebuild if it failed). async fn handle_best_header_history_tree_loaded( &mut self, best_header_tip: block::Height, + reanchor: Option, history_tree: Option>, ) { self.state.rebuild_in_flight = false; if let Some(history_tree) = history_tree { if best_header_tip == self.state.best_header_tip { self.state.best_header_history_tree = history_tree; + if let Some(reanchor) = reanchor { + self.reanchor_header_frontier(reanchor).await; + } } } self.schedule().await; } + /// Reanchors the header frontier down onto a just-rebuilt tree that folded below the current tip. + /// + /// Discards the forward schedule/commit state stranded above the frontier and repositions the tip + /// at `reanchor.tip`, anchored on `reanchor.parent_hash` (where the tree sits), so `schedule` + /// re-derives a fresh overlap forward range from the reanchored tip. Analogous to + /// [`Self::reanchor_to_verified_block_tip`], but lands on the confirmed header-root frontier rather + /// than the verified block tip, keeping the header lead already re-validated below the frontier. + async fn reanchor_header_frontier(&mut self, reanchor: HeaderFrontierReanchor) { + metrics::counter!("sync.header.history_tree.reanchor").increment(1); + self.state.schedule.clear_forward(); + self.state + .pending_commits + .retain(|_, commit| commit.requested_range.priority != RangePriority::Forward); + self.cancel_forward_outstanding(); + self.publish_best_tip_reanchored( + reanchor.tip, + reanchor.tip_hash, + Some(reanchor.parent_hash), + ) + .await; + } + async fn handle_header_range_committed( &mut self, start_height: block::Height, @@ -1509,7 +1545,7 @@ impl HeaderSyncReactor { .pending_commits .retain(|_, commit| commit.requested_range.priority != RangePriority::Forward); self.cancel_forward_outstanding(); - self.publish_best_tip_reanchored(height, hash).await; + self.publish_best_tip_reanchored(height, hash, None).await; } async fn handle_timeouts(&mut self) { @@ -1731,11 +1767,16 @@ impl HeaderSyncReactor { self.broadcast_status_refresh().await; } - async fn publish_best_tip_reanchored(&mut self, height: block::Height, hash: block::Hash) { + async fn publish_best_tip_reanchored( + &mut self, + height: block::Height, + hash: block::Hash, + parent_hash: Option, + ) { let old = (self.state.best_header_tip, self.state.best_header_hash); self.state.best_header_tip = height; self.state.best_header_hash = hash; - self.state.best_header_parent_hash = None; + self.state.best_header_parent_hash = parent_hash; metrics::gauge!("sync.header.best_tip.height").set(height.0 as f64); self.trace_frontier_reanchored(height, hash); let _ = self.tip.send((height, hash)); diff --git a/zebra-network/src/zakura/header_sync/tests.rs b/zebra-network/src/zakura/header_sync/tests.rs index 1603ba342ab..b8697a1aeb9 100644 --- a/zebra-network/src/zakura/header_sync/tests.rs +++ b/zebra-network/src/zakura/header_sync/tests.rs @@ -4417,6 +4417,7 @@ async fn failed_rebuild_clears_guard_and_retriggers() { .handle .send(HeaderSyncEvent::BestHeaderHistoryTreeLoaded { best_header_tip: block::Height(4), + reanchor: None, history_tree: None, }) .await @@ -5683,6 +5684,155 @@ async fn short_served_forward_range_must_install_the_delivered_frontier_tree() { } } +/// Regression test for the runtime lazy-rebuild reanchor. A non-Zakura path (checkpoint/legacy sync) +/// runs the header tip ahead of the folded tree, so the forward range that needs the tree triggers a +/// `QueryBestHeaderHistoryTree` rebuild. When durable roots have a gap the rebuilt tree lands *below* +/// `best_header_tip - 1`, so the answer carries a [`HeaderFrontierReanchor`]. The reactor must drop its +/// tip onto the rebuilt tree (emitting `HeaderReanchored`) and then verify the resumed forward range +/// against it β€” instead of keeping the stale higher tip and re-triggering the identical deterministic +/// rebuild forever (the pre-fix loop, where the frontier was discarded before reaching the reactor). +#[tokio::test(flavor = "current_thread")] +async fn lazy_rebuild_reanchors_tip_onto_lower_frontier_tree() { + let genesis_hash = regtest_network().genesis_hash(); + + // A synthetic post-Heartwood chain 1..=4 with real ZIP-221 commitments, so the resumed range can + // fold and verify against the reinstalled tree. + let mut tree = HistoryTree::default(); + let h1 = chain_history_test_header(&BLOCK_MAINNET_1_BYTES, genesis_hash, &tree); + let h1_hash = block::Hash::from(h1.as_ref()); + let network = heartwood_regtest_with_checkpoint(block::Height(1), h1_hash); + + fold_empty_roots(&network, &mut tree, &h1, block::Height(1)); + let tree_at_1 = tree.clone(); + let h2 = chain_history_test_header(&BLOCK_MAINNET_2_BYTES, h1_hash, &tree); + let h2_hash = block::Hash::from(h2.as_ref()); + fold_empty_roots(&network, &mut tree, &h2, block::Height(2)); + let h3 = chain_history_test_header(&BLOCK_MAINNET_3_BYTES, h2_hash, &tree); + let h3_hash = block::Hash::from(h3.as_ref()); + fold_empty_roots(&network, &mut tree, &h3, block::Height(3)); + let h4 = chain_history_test_header(&BLOCK_MAINNET_4_BYTES, h3_hash, &tree); + + // The header tip sits at 3 (bumped by a non-Zakura commit) but the header-frontier tree is still + // the empty startup default β€” the frontier folded nothing above the verified tip at 1. + let mut fixture = spawn_test_reactor(startup_for( + network.clone(), + (block::Height(1), h1_hash), + Some((block::Height(3), h3_hash)), + )); + let peer_id = peer(94); + + connect_peer(&fixture, peer_id.clone()).await; + advertise_tip( + &fixture, + peer_id.clone(), + block::Height(0), + block::Height(4), + DEFAULT_HS_RANGE, + 1, + ) + .await; + + // The forward range above the stale tip is requested and served; aux validation finds the tree + // behind the range's parent and dispatches exactly one rebuild against the current tip (3). + let (served_peer, start_height, _count) = next_outbound_get_headers(&mut fixture.actions).await; + assert_eq!(served_peer, peer_id); + assert_eq!(start_height, block::Height(4)); + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id.clone(), + msg: headers_message_from(block::Height(4), vec![h4.clone()]), + }) + .await + .unwrap(); + loop { + match next_non_query_action(&mut fixture.actions).await { + HeaderSyncAction::QueryBestHeaderHistoryTree { + best_header_tip, .. + } => { + assert_eq!(best_header_tip, block::Height(3)); + break; + } + HeaderSyncAction::CommitHeaderRange { .. } => { + panic!("a stale tree must not commit the range") + } + HeaderSyncAction::Misbehavior { peer, reason } => { + panic!("unexpected misbehavior from {peer:?}: {reason:?}") + } + _ => {} + } + } + + // Answer the rebuild with a tree that folded only to the confirmed frontier at height 1 β€” below + // `best_header_tip - 1` (2), a durable-root gap β€” plus the reanchor onto it. + fixture + .handle + .send(HeaderSyncEvent::BestHeaderHistoryTreeLoaded { + best_header_tip: block::Height(3), + reanchor: Some(HeaderFrontierReanchor { + tip: block::Height(2), + tip_hash: h2_hash, + parent_hash: h1_hash, + }), + history_tree: Some(Arc::new(tree_at_1)), + }) + .await + .unwrap(); + + // The reactor drops the tip onto the rebuilt tree and resumes the forward range from the reanchor, + // verifying it against the reinstalled tree β€” never a second rebuild. + let mut saw_reanchor = false; + loop { + match next_non_query_action(&mut fixture.actions).await { + HeaderSyncAction::HeaderReanchored { new, .. } => { + assert_eq!(new.0, block::Height(2), "tip reanchored onto the frontier"); + saw_reanchor = true; + } + HeaderSyncAction::SendMessage { + msg: + HeaderSyncMessage::GetHeaders { + start_height, + count, + .. + }, + .. + } => { + // Ignore the pre-reanchor retry of the stale [4..] range; only serve the range the + // reactor resumes from the reanchor at height 2. + if start_height != block::Height(2) { + continue; + } + let headers = [h2.clone(), h3.clone(), h4.clone()]; + let take = (count as usize).min(headers.len()); + fixture + .handle + .send(HeaderSyncEvent::WireMessage { + peer: peer_id.clone(), + msg: headers_message_from(block::Height(2), headers[..take].to_vec()), + }) + .await + .unwrap(); + } + HeaderSyncAction::CommitHeaderRange { start_height, .. } => { + assert_eq!(start_height, block::Height(2)); + break; + } + HeaderSyncAction::QueryBestHeaderHistoryTree { .. } => panic!( + "the reanchored tree was ignored: the resumed range re-triggered a durable-state \ + rebuild instead of verifying against the reinstalled tree" + ), + HeaderSyncAction::Misbehavior { peer, reason } => { + panic!("unexpected misbehavior from {peer:?}: {reason:?}") + } + _ => {} + } + } + assert!( + saw_reanchor, + "the tip must be reanchored down onto the rebuilt frontier tree" + ); +} + /// Regression test: backward (below-anchor) checkpoint backfill is unused by the current sync /// wiring and does not work with root verification (backward ranges fold onto the previous /// checkpoint's tree, which this reactor never tracks), so it must be explicitly disabled rather diff --git a/zebra-network/src/zakura/testkit/cluster.rs b/zebra-network/src/zakura/testkit/cluster.rs index bc7f920c7a1..c572636381d 100644 --- a/zebra-network/src/zakura/testkit/cluster.rs +++ b/zebra-network/src/zakura/testkit/cluster.rs @@ -1019,6 +1019,7 @@ mod tests { .handle .send(HeaderSyncEvent::BestHeaderHistoryTreeLoaded { best_header_tip, + reanchor: None, history_tree: Some(Arc::new( zebra_chain::history_tree::HistoryTree::default(), )), diff --git a/zebrad/src/commands/start/zakura/header_sync_driver.rs b/zebrad/src/commands/start/zakura/header_sync_driver.rs index e13753d71b2..b063d6830dd 100644 --- a/zebrad/src/commands/start/zakura/header_sync_driver.rs +++ b/zebrad/src/commands/start/zakura/header_sync_driver.rs @@ -11,9 +11,10 @@ use zebra_chain::{ parallel::commitment_aux::BlockCommitmentRoots, }; use zebra_network::zakura::{ - commit_state_trace as cs_trace, BlockSyncFrontiers, Frontier, FrontierChange, HeaderSyncAction, - HeaderSyncCommitFailureKind, HeaderSyncEvent, HeaderSyncFrontiers, ZakuraEndpoint, - ZakuraHeaderSyncDriverStartup, ZakuraTrace, DEFAULT_HS_RANGE, + commit_state_trace as cs_trace, BlockSyncFrontiers, Frontier, FrontierChange, + HeaderFrontierReanchor, HeaderSyncAction, HeaderSyncCommitFailureKind, HeaderSyncEvent, + HeaderSyncFrontiers, ZakuraEndpoint, ZakuraHeaderSyncDriverStartup, ZakuraTrace, + DEFAULT_HS_RANGE, }; #[cfg(test)] @@ -144,6 +145,58 @@ pub(crate) async fn zakura_header_sync_driver_startup( }) } +/// Resolves the header-frontier reanchor a runtime lazy rebuild needs when the durable header-root +/// `frontier` folded *below* `best_header_tip - 1` (a gap left by a non-Zakura commit racing ahead). +/// +/// Mirrors the startup resume in [`zakura_header_sync_driver_startup`]: resume one block above the +/// confirmed frontier, anchored on the frontier hash, looking up the resume header's hash. Returns +/// `Ok(None)` when the frontier already reached `best_header_tip - 1` (the common no-gap rebuild, no +/// reanchor needed). +async fn header_frontier_reanchor( + read_state: ReadState, + best_header_tip: block::Height, + frontier: (block::Height, block::Hash), +) -> Result, Report> +where + ReadState: Service< + zebra_state::ReadRequest, + Response = zebra_state::ReadResponse, + Error = zebra_state::BoxError, + > + Send, + ReadState::Future: Send, +{ + let (frontier_height, frontier_hash) = frontier; + let resume_height = frontier_height + .next() + .map_err(|_| eyre!("header frontier height overflow"))?; + // No gap: the tree folded to `best_header_tip - 1`, so its position already matches the parent of + // the next forward range. Nothing to reanchor β€” the plain tree install suffices. + if resume_height >= best_header_tip { + return Ok(None); + } + let tip_hash = match read_state + .oneshot(zebra_state::ReadRequest::HeadersByHeightRange { + start: resume_height, + count: 1, + }) + .await + .map_err(|error| eyre!("{error}"))? + { + zebra_state::ReadResponse::Headers(headers) => headers + .first() + .map(|(_height, hash, _header)| *hash) + .ok_or_else(|| eyre!("missing durable header at resume height {resume_height:?}"))?, + response => Err(eyre!( + "unexpected HeadersByHeightRange response: {response:?}" + ))?, + }; + Ok(Some(HeaderFrontierReanchor { + tip: resume_height, + tip_hash, + parent_hash: frontier_hash, + })) +} + #[derive(Clone)] pub(crate) struct ZakuraHeaderSyncDriverHandles { pub(crate) endpoint: ZakuraEndpoint, @@ -769,7 +822,7 @@ pub(crate) async fn drive_zakura_header_sync_actions Some(tree), + Ok(zebra_state::ReadResponse::BestHeaderHistoryTree { tree, frontier }) => { + // The fold can stop below `best_header_tip - 1` when durable roots have a gap + // (a non-Zakura commit raced ahead). In that case the reactor must drop its tip + // onto the rebuilt tree; resolve that reanchor here, where the read access + // lives, mirroring the startup resume. A resolution failure is reported as a + // failed rebuild (both `None`) so the guard clears and the range re-triggers, + // rather than installing a lower tree the stale tip could never match. + match header_frontier_reanchor( + read_state.clone(), + best_header_tip, + frontier, + ) + .await + { + Ok(reanchor) => (Some(tree), reanchor), + Err(error) => { + warn!(?error, "failed to resolve Zakura header frontier reanchor"); + (None, None) + } + } + } Ok(response) => { warn!(?response, "unexpected BestHeaderHistoryTree response"); - None + (None, None) } Err(error) => { warn!(?error, "failed to rebuild Zakura best header history tree"); - None + (None, None) } }; let _ = handles .header_sync .send(HeaderSyncEvent::BestHeaderHistoryTreeLoaded { best_header_tip, + reanchor, history_tree, }) .await; From 9b86d526cb4541b5e49ac54f6c53d0d1134e5be2 Mon Sep 17 00:00:00 2001 From: roman Date: Sun, 5 Jul 2026 22:01:38 -0600 Subject: [PATCH 09/12] handle duplicate peer connection disconnect bug --- zebra-network/src/zakura/handler.rs | 331 +++++++++++++++--- .../src/zakura/header_sync/events.rs | 16 +- zebra-network/src/zakura/header_sync/pipe.rs | 5 +- .../src/zakura/header_sync/reactor.rs | 26 +- .../src/zakura/header_sync/service.rs | 47 ++- zebra-network/src/zakura/header_sync/tests.rs | 162 ++++++++- .../src/zakura/transport/registry.rs | 95 +++-- zebra-network/src/zakura/transport/service.rs | 14 + 8 files changed, 612 insertions(+), 84 deletions(-) diff --git a/zebra-network/src/zakura/handler.rs b/zebra-network/src/zakura/handler.rs index c5a68b24138..03bb4729498 100644 --- a/zebra-network/src/zakura/handler.rs +++ b/zebra-network/src/zakura/handler.rs @@ -910,17 +910,6 @@ struct ZakuraSupervisorState { next_registration_id: ZakuraConnId, } -#[derive(Debug)] -struct ZakuraPeerConnectionEntry { - /// Monotonic supervisor registration generation used by services to ignore - /// stale add/remove work from a superseded connection. - conn_id: ZakuraConnId, - outbound_handle: ZakuraPeerHandle, - disconnect_token: CancellationToken, - registered_at: Instant, - remote_ip: Option, -} - impl ZakuraSupervisorState { fn increment_ip(&mut self, remote_ip: Option) { if let Some(remote_ip) = remote_ip { @@ -956,6 +945,27 @@ impl ZakuraSupervisorState { fn debug_assert_accounting(&self) {} } +/// One authenticated peer's currently registered connection. +/// +/// The entry is bound to a per-supervisor monotonic connection generation, so +/// teardown ([`ZakuraSupervisorHandle::deregister`]) only removes state when the +/// exiting connection is still the registered one. Without this guard, a +/// displaced duplicate's late teardown would deregister the live winner. +#[derive(Debug)] +struct ZakuraPeerConnectionEntry { + /// Monotonic supervisor generation used by services to ignore stale work + /// from a superseded connection. + conn_id: ZakuraConnId, + outbound_handle: ZakuraPeerHandle, + disconnect_token: CancellationToken, + /// The remote IP this entry incremented in `active_by_ip`, released when + /// this exact entry is removed. + remote_ip: Option, + /// When this connection registered, used to decide whether a duplicate may + /// evict a stale incumbent (see [`ZAKURA_DUPLICATE_EVICT_MIN_AGE`]). + registered_at: Instant, +} + /// Queue-backed outbound send handle for one authenticated Zakura peer. #[derive(Clone, Debug)] pub struct ZakuraPeerHandle { @@ -1140,32 +1150,31 @@ impl ZakuraSupervisorHandle { ZakuraUpgradeOutcome::Upgraded { .. } => { let conn_id = state.next_registration_id; state.next_registration_id += 1; - let entry = ZakuraPeerConnectionEntry { - conn_id, - outbound_handle, - disconnect_token, - registered_at: Instant::now(), - remote_ip, - }; - if let Some(old_entry) = state.active_by_peer.insert(peer_id.clone(), entry) { + if let Some(old_entry) = state.active_by_peer.remove(&peer_id) { state.decrement_ip(old_entry.remote_ip); old_entry.disconnect_token.cancel(); - metrics::counter!("zakura.p2p.conn.duplicate.evicted_upgraded").increment(1); + metrics::counter!("zakura.p2p.conn.duplicate.displaced_incumbent").increment(1); } state.increment_ip(remote_ip); + state.active_by_peer.insert( + peer_id.clone(), + ZakuraPeerConnectionEntry { + conn_id, + outbound_handle, + disconnect_token: disconnect_token.clone(), + registered_at: Instant::now(), + remote_ip, + }, + ); state.debug_assert_accounting(); let registered_ids: Vec<_> = state.active_by_peer.keys().cloned().collect(); set_active_connection_gauge(registered_ids.len()); self.peer_set_tx.send_replace(registered_ids); - let disconnect_token = state - .active_by_peer - .get(&peer_id) - .map(|entry| entry.disconnect_token.clone()) - .expect("disconnect token exists because this peer was just registered"); ZakuraRegistration::Registered { conn_id, peer_id, remote_ip, + registration_id: conn_id, disconnect_token, } } @@ -1206,15 +1215,21 @@ impl ZakuraSupervisorHandle { } } - async fn deregister(&self, peer_id: &ZakuraPeerId, conn_id: ZakuraConnId) { + /// Removes a connection's registration, but only when `conn_id` still + /// identifies the currently registered connection for this peer. + /// + /// Returns true when the registration was removed. A displaced connection's + /// late teardown returns false and leaves the winning registration (and its + /// per-IP slot accounting) untouched. + async fn deregister(&self, peer_id: &ZakuraPeerId, conn_id: ZakuraConnId) -> bool { let mut state = self.inner.lock().await; let Some(entry) = state.active_by_peer.get(peer_id) else { state.debug_assert_accounting(); - return; + return false; }; if entry.conn_id != conn_id { state.debug_assert_accounting(); - return; + return false; } let entry = state .active_by_peer @@ -1226,6 +1241,7 @@ impl ZakuraSupervisorHandle { let registered_ids: Vec<_> = state.active_by_peer.keys().cloned().collect(); set_active_connection_gauge(registered_ids.len()); self.peer_set_tx.send_replace(registered_ids); + true } fn shutdown(&self) { @@ -1263,6 +1279,7 @@ enum ZakuraRegistration { conn_id: ZakuraConnId, peer_id: ZakuraPeerId, remote_ip: Option, + registration_id: u64, disconnect_token: CancellationToken, }, Duplicate { @@ -1427,6 +1444,10 @@ struct RegisteredConnectionServeContext { connection_token: CancellationToken, close_cause: CloseCause, accepted_capabilities: u64, + /// Supervisor registration id for this connection; teardown only + /// deregisters and fans out service removal while this id is still the + /// peer's registered connection. + registration_id: u64, /// Whether this side dialed the connection. The dialer (initiator) opens all /// of its demanded ordered streams as before; the responder additionally /// opens only block-sync (the sole symmetric service), keeping the legacy @@ -1966,8 +1987,8 @@ impl ZakuraProtocolHandler { should_run_freshness_reaper(queue_split_stream_count, request_response_stream_count); if negotiated_ordered_streams.is_empty() { - self.registry - .add_peer(Peer::new_with_conn_id_and_direction_and_close_cause( + self.registry.add_peer( + Peer::new_with_conn_id_and_direction_and_close_cause( conn_id, peer_id.clone(), remote_ip, @@ -1976,7 +1997,9 @@ impl ZakuraProtocolHandler { HashMap::new(), connection_token.clone(), close_cause.clone(), - )); + ) + .with_registration_id(context.registration_id), + ); cleanup_guard.add_admitted_capabilities(accepted_capabilities); } else if !connection_token.is_cancelled() { let mut opened_capabilities = 0; @@ -2025,18 +2048,19 @@ impl ZakuraProtocolHandler { // Escalation is already narrowed to opened ordered services. // Disconnect fanout still uses the registry's returned admitted // mask, not this peer context. - let admitted_capabilities = - self.registry - .add_escalated_peer(Peer::new_with_service_streams( - conn_id, - peer_id.clone(), - remote_ip, - opened_capabilities, - context.direction, - std::mem::take(&mut service_streams), - connection_token.clone(), - close_cause.clone(), - )); + let admitted_capabilities = self.registry.add_escalated_peer( + Peer::new_with_service_streams( + conn_id, + peer_id.clone(), + remote_ip, + opened_capabilities, + context.direction, + std::mem::take(&mut service_streams), + connection_token.clone(), + close_cause.clone(), + ) + .with_registration_id(context.registration_id), + ); cleanup_guard.add_admitted_capabilities(admitted_capabilities); } } @@ -2197,7 +2221,8 @@ impl ZakuraProtocolHandler { service_streams, connection_token.clone(), close_cause.clone(), - ), + ) + .with_registration_id(context.registration_id), ); cleanup_guard.add_admitted_capabilities(admitted_capabilities); } @@ -2611,6 +2636,7 @@ impl ZakuraProtocolHandler { conn_id, peer_id, remote_ip, + registration_id, disconnect_token, } => { metrics::counter!("zakura.p2p.conn.accepted", "role" => context.role).increment(1); @@ -2641,6 +2667,7 @@ impl ZakuraProtocolHandler { connection_token: disconnect_token, close_cause, accepted_capabilities: context.accepted_capabilities, + registration_id, is_initiator: context.role == "initiator", i_open_collision_winner: context.i_open_collision_winner, direction: context.direction, @@ -6450,6 +6477,222 @@ mod tests { Ok(()) } + async fn register_with_transcript( + supervisor: &ZakuraSupervisorHandle, + peer: &ZakuraPeerId, + ip: IpAddr, + transcript_hash: [u8; TRANSCRIPT_HASH_BYTES], + token: CancellationToken, + ) -> ZakuraRegistration { + let (outbound_tx, _outbound_rx) = mpsc::channel(1); + let outbound_handle = ZakuraPeerHandle::new_for_tests(peer.clone(), outbound_tx); + supervisor + .register( + test_conn_id(), + peer.clone(), + Some(ip), + transcript_hash, + outbound_handle, + token, + ZAKURA_CAP_LEGACY_GOSSIP | ZAKURA_CAP_HEADER_SYNC, + ) + .await + } + + // A duplicate connection whose transcript hash wins arbitration (`Upgraded`) + // replaces the incumbent registration. The displaced incumbent must be + // cancelled and release its per-IP slot immediately, and its later teardown + // (bound to the old registration id) must not deregister the winner. + #[tokio::test] + async fn winning_duplicate_displaces_incumbent_and_stale_teardown_is_ignored( + ) -> Result<(), BoxError> { + let supervisor = ZakuraSupervisorHandle::new(1); + let ip: IpAddr = "203.0.113.21".parse().expect("test ip parses"); + let peer = test_peer(21); + + // The incumbent registers with the larger transcript hash, so a + // cross-direction duplicate can win arbitration against it. + let incumbent_token = CancellationToken::new(); + let ZakuraRegistration::Registered { + registration_id: incumbent_id, + .. + } = register_with_transcript(&supervisor, &peer, ip, [2; 32], incumbent_token.clone()) + .await + else { + panic!("the incumbent registers"); + }; + + // The duplicate with the strictly smaller transcript hash upgrades and + // must displace the incumbent: cancel it and take over its per-IP slot. + let winner_token = CancellationToken::new(); + let ZakuraRegistration::Registered { + registration_id: winner_id, + .. + } = register_with_transcript(&supervisor, &peer, ip, [1; 32], winner_token.clone()).await + else { + panic!("the winning duplicate registers"); + }; + assert_ne!(incumbent_id, winner_id); + assert!( + incumbent_token.is_cancelled(), + "the displaced incumbent's connection must be cancelled so it stops serving", + ); + assert!(!winner_token.is_cancelled()); + assert_eq!(supervisor.registered_ids().await, vec![peer.clone()]); + assert!( + !supervisor.can_accept_remote_ip_with_in_flight(ip, 0).await, + "the winner holds exactly the one per-IP slot at cap 1", + ); + + // The displaced incumbent's late teardown carries the old registration + // id and must leave the winner's registration and per-IP slot intact. + assert!( + !supervisor.deregister(&peer, incumbent_id).await, + "a displaced connection's teardown must not deregister the winner", + ); + assert_eq!(supervisor.registered_ids().await, vec![peer.clone()]); + assert!( + !supervisor.can_accept_remote_ip_with_in_flight(ip, 0).await, + "a stale teardown must not release the winner's per-IP slot", + ); + + // The winner's own teardown deregisters normally and frees the slot. + assert!(supervisor.deregister(&peer, winner_id).await); + assert!(supervisor.registered_ids().await.is_empty()); + assert!( + supervisor.can_accept_remote_ip_with_in_flight(ip, 0).await, + "the per-IP slot is free once the current registration deregisters", + ); + + Ok(()) + } + + // A winning duplicate can arrive from a different IP than the incumbent + // (e.g. a redial after the peer's address changed). Displacement must move + // the per-IP slot: the incumbent's IP frees immediately, the winner's IP is + // charged, and the incumbent's stale teardown must not touch either count. + #[tokio::test] + async fn winning_duplicate_across_ips_moves_per_ip_slot() -> Result<(), BoxError> { + let supervisor = ZakuraSupervisorHandle::new(1); + let old_ip: IpAddr = "203.0.113.31".parse().expect("test ip parses"); + let new_ip: IpAddr = "203.0.113.32".parse().expect("test ip parses"); + let peer = test_peer(31); + + let incumbent_token = CancellationToken::new(); + let ZakuraRegistration::Registered { + registration_id: incumbent_id, + .. + } = register_with_transcript(&supervisor, &peer, old_ip, [2; 32], incumbent_token.clone()) + .await + else { + panic!("the incumbent registers"); + }; + assert!( + !supervisor + .can_accept_remote_ip_with_in_flight(old_ip, 0) + .await + ); + + let winner_token = CancellationToken::new(); + let ZakuraRegistration::Registered { + registration_id: winner_id, + .. + } = register_with_transcript(&supervisor, &peer, new_ip, [1; 32], winner_token.clone()) + .await + else { + panic!("the winning duplicate registers"); + }; + assert!( + supervisor + .can_accept_remote_ip_with_in_flight(old_ip, 0) + .await, + "displacement must free the incumbent's per-IP slot immediately", + ); + assert!( + !supervisor + .can_accept_remote_ip_with_in_flight(new_ip, 0) + .await, + "the winner is charged against its own IP", + ); + + // The stale teardown carries the incumbent's old IP implicitly through + // its registration id; it must not decrement the winner's IP count. + assert!(!supervisor.deregister(&peer, incumbent_id).await); + assert!( + supervisor + .can_accept_remote_ip_with_in_flight(old_ip, 0) + .await + ); + assert!( + !supervisor + .can_accept_remote_ip_with_in_flight(new_ip, 0) + .await, + "a stale teardown must not release the winner's per-IP slot", + ); + + assert!(supervisor.deregister(&peer, winner_id).await); + assert!( + supervisor + .can_accept_remote_ip_with_in_flight(new_ip, 0) + .await + ); + + Ok(()) + } + + // After displacement, supervisor-facing APIs must operate on the winner's + // entry: `disconnect_peer` cancels the winner's token, and the peer-set + // watch publishes no change for a stale teardown. + #[tokio::test] + async fn supervisor_apis_track_winner_after_displacement() -> Result<(), BoxError> { + let supervisor = ZakuraSupervisorHandle::new(1); + let ip: IpAddr = "203.0.113.41".parse().expect("test ip parses"); + let peer = test_peer(41); + let mut peer_set = supervisor.subscribe(); + + let incumbent_token = CancellationToken::new(); + let ZakuraRegistration::Registered { + registration_id: incumbent_id, + .. + } = register_with_transcript(&supervisor, &peer, ip, [2; 32], incumbent_token.clone()) + .await + else { + panic!("the incumbent registers"); + }; + let winner_token = CancellationToken::new(); + let ZakuraRegistration::Registered { + registration_id: winner_id, + .. + } = register_with_transcript(&supervisor, &peer, ip, [1; 32], winner_token.clone()).await + else { + panic!("the winning duplicate registers"); + }; + + // Mark the registration-time peer-set updates as seen, then verify a + // stale teardown publishes nothing: the registered set did not change. + peer_set.borrow_and_update(); + assert!(!supervisor.deregister(&peer, incumbent_id).await); + assert!( + !peer_set.has_changed()?, + "a stale teardown must not publish a peer-set change", + ); + + // `disconnect_peer` must target the winner's (current) connection. + assert!(!winner_token.is_cancelled()); + assert!(supervisor.disconnect_peer(&peer).await); + assert!( + winner_token.is_cancelled(), + "disconnect_peer cancels the currently registered connection", + ); + + // The winner's teardown publishes the now-empty peer set. + assert!(supervisor.deregister(&peer, winner_id).await); + assert!(peer_set.has_changed()?); + assert!(peer_set.borrow_and_update().is_empty()); + + Ok(()) + } + #[tokio::test] async fn header_sync_misbehavior_action_does_not_disconnect_peer() -> Result<(), BoxError> { let _guard = zebra_test::init(); diff --git a/zebra-network/src/zakura/header_sync/events.rs b/zebra-network/src/zakura/header_sync/events.rs index 33d624c39b2..49a5da785ad 100644 --- a/zebra-network/src/zakura/header_sync/events.rs +++ b/zebra-network/src/zakura/header_sync/events.rs @@ -190,7 +190,18 @@ pub enum HeaderSyncEvent { /// A peer became available for stream-5 header sync. PeerConnected(HeaderSyncPeerSession), /// A peer disconnected; all of its outstanding work is dropped. - PeerDisconnected(ZakuraPeerId), + PeerDisconnected { + /// The disconnected peer. + peer: ZakuraPeerId, + /// Registration id of the connection whose session is tearing down, or + /// `None` to disconnect unconditionally. + /// + /// A `Some` id is ignored when the reactor's admitted session rides on + /// a different (newer) connection: duplicate arbitration replaced the + /// sender's connection, and its late teardown must not remove the + /// replacement session. + registration_id: Option, + }, /// First-party header-sync summary observed over the authenticated discovery stream. AdvisoryHeaderSummary { /// Peer that supplied its own summary. @@ -337,7 +348,7 @@ impl HeaderSyncEvent { pub(super) fn metrics_label(&self) -> &'static str { match self { Self::PeerConnected(_) => "peer_connected", - Self::PeerDisconnected(_) => "peer_disconnected", + Self::PeerDisconnected { .. } => "peer_disconnected", Self::AdvisoryHeaderSummary { .. } => "advisory_header_summary", Self::FullBlockCommitted { .. } => "full_block_committed", Self::NewBlockAccepted { .. } => "new_block_accepted", @@ -347,6 +358,7 @@ impl HeaderSyncEvent { Self::WireDecodeFailed { .. } => "wire_decode_failed", Self::WireProtocolFailure { .. } => "wire_protocol_failure", Self::StateFrontiersChanged(_) => "state_frontiers_changed", + Self::BestHeaderHistoryTreeLoaded { .. } => "best_header_history_tree_loaded", Self::HeaderRangeCommitted { .. } => "header_range_committed", Self::HeaderRangeCommitFailed { .. } => "header_range_commit_failed", Self::HeaderRangeResponseFinished { .. } => "header_range_response_finished", diff --git a/zebra-network/src/zakura/header_sync/pipe.rs b/zebra-network/src/zakura/header_sync/pipe.rs index 504ba9321fa..6192827a034 100644 --- a/zebra-network/src/zakura/header_sync/pipe.rs +++ b/zebra-network/src/zakura/header_sync/pipe.rs @@ -416,7 +416,10 @@ mod tests { fn saturated_events_handle() -> (HeaderSyncHandle, mpsc::Receiver) { let (events, events_rx) = mpsc::channel(1); events - .try_send(HeaderSyncEvent::PeerDisconnected(peer())) + .try_send(HeaderSyncEvent::PeerDisconnected { + peer: peer(), + registration_id: None, + }) .expect("the single events slot is free"); let (lifecycle, _lifecycle_rx) = mpsc::unbounded_channel(); let (_tip_tx, tip) = watch::channel((block::Height(0), block::Hash([0; 32]))); diff --git a/zebra-network/src/zakura/header_sync/reactor.rs b/zebra-network/src/zakura/header_sync/reactor.rs index c5d2b7ed8fa..c728b8946b0 100644 --- a/zebra-network/src/zakura/header_sync/reactor.rs +++ b/zebra-network/src/zakura/header_sync/reactor.rs @@ -149,7 +149,10 @@ impl HeaderSyncReactor { async fn handle_event_inner(&mut self, event: HeaderSyncEvent) { match event { HeaderSyncEvent::PeerConnected(session) => self.handle_peer_connected(session).await, - HeaderSyncEvent::PeerDisconnected(peer) => self.handle_peer_disconnected(peer), + HeaderSyncEvent::PeerDisconnected { + peer, + registration_id, + } => self.handle_peer_disconnected(peer, registration_id), HeaderSyncEvent::AdvisoryHeaderSummary { peer, summary } => { self.handle_advisory_header_summary(peer, summary) } @@ -468,7 +471,24 @@ impl HeaderSyncReactor { self.schedule().await; } - fn handle_peer_disconnected(&mut self, peer: ZakuraPeerId) { + fn handle_peer_disconnected(&mut self, peer: ZakuraPeerId, registration_id: Option) { + // A session-scoped disconnect from a connection that duplicate + // arbitration already displaced must not remove the winning + // connection's admitted session. + if let (Some(event_registration_id), Some(peer_state)) = + (registration_id, self.state.peers.get(&peer)) + { + if peer_state.session.registration_id() != event_registration_id { + tracing::debug!( + ?peer, + event_registration_id, + current_registration_id = peer_state.session.registration_id(), + "ignoring stale header-sync disconnect from a displaced connection" + ); + return; + } + } + self.state.peers.remove(&peer); self.state.parked_peers.remove(&peer); self.state.advisory.remove(&peer); @@ -1930,7 +1950,7 @@ impl HeaderSyncReactor { insert_optional_str(row, hs_trace::KIND, Some("peer_connected")); insert_peer(row, hs_trace::PEER, session.peer_id()); } - HeaderSyncEvent::PeerDisconnected(peer) => { + HeaderSyncEvent::PeerDisconnected { peer, .. } => { insert_optional_str(row, hs_trace::KIND, Some("peer_disconnected")); insert_peer(row, hs_trace::PEER, peer); } diff --git a/zebra-network/src/zakura/header_sync/service.rs b/zebra-network/src/zakura/header_sync/service.rs index f8375a08646..504ac770be3 100644 --- a/zebra-network/src/zakura/header_sync/service.rs +++ b/zebra-network/src/zakura/header_sync/service.rs @@ -36,6 +36,12 @@ pub(crate) fn header_sync_streams() -> &'static [Stream] { pub struct HeaderSyncPeerSession { peer_id: ZakuraPeerId, direction: ServicePeerDirection, + /// Supervisor registration id of the connection this session rides on. + /// + /// Used to drop stale `PeerDisconnected` teardown events from a session + /// whose connection was displaced by a winning duplicate. Zero for test + /// sessions built without a supervisor registration. + registration_id: u64, inner: Arc, } @@ -50,15 +56,25 @@ impl HeaderSyncPeerSession { fn new_with_commands( session: &PeerStreamSession, direction: ServicePeerDirection, + registration_id: u64, commands: mpsc::UnboundedSender, ) -> Self { - Self::from_parts_with_direction_and_commands( + let mut this = Self::from_parts_with_direction_and_commands( session.peer_id().clone(), direction, session.sender(), session.cancel_token(), Some(commands), - ) + ); + this.registration_id = registration_id; + this + } + + /// Set the supervisor registration id of this session's connection. + #[cfg(test)] + pub(crate) fn with_registration_id(mut self, registration_id: u64) -> Self { + self.registration_id = registration_id; + self } #[cfg(test)] @@ -91,6 +107,7 @@ impl HeaderSyncPeerSession { Self { peer_id, direction, + registration_id: 0, inner: Arc::new(HeaderSyncPeerSessionInner { send, cancel_token, @@ -110,6 +127,7 @@ impl HeaderSyncPeerSession { Self { peer_id, direction, + registration_id: 0, inner: Arc::new(HeaderSyncPeerSessionInner { send, cancel_token, @@ -128,6 +146,11 @@ impl HeaderSyncPeerSession { self.direction } + /// Supervisor registration id of the connection this session rides on. + pub(crate) fn registration_id(&self) -> u64 { + self.registration_id + } + /// Peer disconnect/local shutdown cancellation token. pub fn cancel_token(&self) -> CancellationToken { self.inner.cancel_token.clone() @@ -344,6 +367,7 @@ impl Service for HeaderSyncService { }; let peer_id = peer.id.clone(); + let registration_id = peer.registration_id; let session = PeerStreamSession::new( peer_id.clone(), ZAKURA_STREAM_HEADER_SYNC, @@ -361,8 +385,12 @@ impl Service for HeaderSyncService { let close_cause = peer.close_cause(); let conn_id = peer.conn_id; let (commands_tx, commands_rx) = mpsc::unbounded_channel(); - let header_sync_session = - HeaderSyncPeerSession::new_with_commands(&session, peer.direction, commands_tx); + let header_sync_session = HeaderSyncPeerSession::new_with_commands( + &session, + peer.direction, + registration_id, + commands_tx, + ); { let mut peers = self @@ -447,8 +475,10 @@ impl Service for HeaderSyncService { } }; if should_notify { - let _ = teardown_handle - .send_lifecycle(HeaderSyncEvent::PeerDisconnected(teardown_peer)); + let _ = teardown_handle.send_lifecycle(HeaderSyncEvent::PeerDisconnected { + peer: teardown_peer, + registration_id: Some(registration_id), + }); } }; let panic_connection_cancel_token = connection_cancel_token.clone(); @@ -482,7 +512,10 @@ impl Service for HeaderSyncService { record.cancel_token.cancel(); let _ = self .header_sync - .send_lifecycle(HeaderSyncEvent::PeerDisconnected(peer.clone())); + .send_lifecycle(HeaderSyncEvent::PeerDisconnected { + peer: peer.clone(), + registration_id: None, + }); } } diff --git a/zebra-network/src/zakura/header_sync/tests.rs b/zebra-network/src/zakura/header_sync/tests.rs index b8697a1aeb9..db38fece496 100644 --- a/zebra-network/src/zakura/header_sync/tests.rs +++ b/zebra-network/src/zakura/header_sync/tests.rs @@ -578,7 +578,10 @@ async fn peer_caps_reject_full_without_status_or_misbehavior_and_free_on_remove( fixture .handle - .send(HeaderSyncEvent::PeerDisconnected(admitted)) + .send(HeaderSyncEvent::PeerDisconnected { + peer: admitted, + registration_id: None, + }) .await .unwrap(); tokio::time::sleep(std::time::Duration::from_millis(10)).await; @@ -586,6 +589,139 @@ async fn peer_caps_reject_full_without_status_or_misbehavior_and_free_on_remove( assert_eq!(fixture.handle.peer_snapshot().inbound_slots_free, 1); } +/// Duplicate arbitration can replace a peer's registered connection while the +/// displaced connection's header-sync session task is still tearing down. The +/// displaced session's `PeerDisconnected` (scoped to its registration id) must +/// not remove the winning connection's admitted session; the winner's own +/// teardown still must. +#[tokio::test] +async fn stale_disconnect_from_displaced_connection_keeps_live_session() { + let network = Network::Mainnet; + let anchor = (block::Height(0), network.genesis_hash()); + let mut startup = startup_for(network, anchor, Some(anchor)); + startup.range_state_actions_enabled = false; + let fixture = spawn_test_reactor(startup); + let peer_id = peer(33); + + // Session riding the peer's first registered connection. + connect_peer_with_registration(&fixture, peer_id.clone(), ServicePeerDirection::Inbound, 1) + .await; + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!(fixture.handle.peer_snapshot().inbound_peers, 1); + + // A winning duplicate connection replaces the registration and its session + // supersedes the first one in the reactor. + connect_peer_with_registration(&fixture, peer_id.clone(), ServicePeerDirection::Inbound, 2) + .await; + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!(fixture.handle.peer_snapshot().inbound_peers, 1); + + // The displaced connection's teardown arrives late and must be ignored. + fixture + .handle + .send(HeaderSyncEvent::PeerDisconnected { + peer: peer_id.clone(), + registration_id: Some(1), + }) + .await + .unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!( + fixture.handle.peer_snapshot().inbound_peers, + 1, + "a stale disconnect from a displaced connection must not remove the live session", + ); + + // The winning connection's own teardown still removes the peer. + fixture + .handle + .send(HeaderSyncEvent::PeerDisconnected { + peer: peer_id.clone(), + registration_id: Some(2), + }) + .await + .unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!(fixture.handle.peer_snapshot().inbound_peers, 0); +} + +/// The opposite event ordering to +/// [`stale_disconnect_from_displaced_connection_keeps_live_session`]: the +/// displaced session's scoped disconnect arrives while its own session is still +/// admitted (before the winner's `PeerConnected`). The disconnect matches and +/// removes the old session, and the winner's connect then re-admits the peer, +/// so the reactor converges to one live session in either ordering. +#[tokio::test] +async fn stale_disconnect_before_winner_connect_still_converges() { + let network = Network::Mainnet; + let anchor = (block::Height(0), network.genesis_hash()); + let mut startup = startup_for(network, anchor, Some(anchor)); + startup.range_state_actions_enabled = false; + let fixture = spawn_test_reactor(startup); + let peer_id = peer(34); + + connect_peer_with_registration(&fixture, peer_id.clone(), ServicePeerDirection::Inbound, 1) + .await; + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!(fixture.handle.peer_snapshot().inbound_peers, 1); + + // The displaced connection's teardown fires while its own session is still + // the admitted one, so the id matches and the session is removed. + fixture + .handle + .send(HeaderSyncEvent::PeerDisconnected { + peer: peer_id.clone(), + registration_id: Some(1), + }) + .await + .unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!(fixture.handle.peer_snapshot().inbound_peers, 0); + + // The winning connection's session arrives afterwards and is admitted. + connect_peer_with_registration(&fixture, peer_id.clone(), ServicePeerDirection::Inbound, 2) + .await; + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!( + fixture.handle.peer_snapshot().inbound_peers, + 1, + "the winner's session is admitted after the old session's removal", + ); +} + +/// An unscoped disconnect (`registration_id: None`, the registry fanout path) +/// removes the session regardless of the registration id it rides on, because +/// the fanout only runs for the currently registered connection. +#[tokio::test] +async fn unscoped_disconnect_removes_session_with_any_registration_id() { + let network = Network::Mainnet; + let anchor = (block::Height(0), network.genesis_hash()); + let mut startup = startup_for(network, anchor, Some(anchor)); + startup.range_state_actions_enabled = false; + let fixture = spawn_test_reactor(startup); + let peer_id = peer(35); + + connect_peer_with_registration(&fixture, peer_id.clone(), ServicePeerDirection::Inbound, 5) + .await; + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!(fixture.handle.peer_snapshot().inbound_peers, 1); + + fixture + .handle + .send(HeaderSyncEvent::PeerDisconnected { + peer: peer_id, + registration_id: None, + }) + .await + .unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + assert_eq!( + fixture.handle.peer_snapshot().inbound_peers, + 0, + "an unconditional disconnect removes the session whatever its registration id", + ); +} + #[tokio::test(flavor = "current_thread")] async fn advisory_summary_status_mismatch_uses_status_without_misbehavior_and_backs_off() { let network = regtest_network(); @@ -736,7 +872,10 @@ async fn advisory_backoff_is_pruned_on_peer_disconnected() { fixture .handle - .send(HeaderSyncEvent::PeerDisconnected(peer_id.clone())) + .send(HeaderSyncEvent::PeerDisconnected { + peer: peer_id.clone(), + registration_id: None, + }) .await .unwrap(); tokio::time::sleep(std::time::Duration::from_millis(10)).await; @@ -932,6 +1071,15 @@ async fn connect_peer_with_direction( fixture: &ReactorFixture, peer_id: ZakuraPeerId, direction: ServicePeerDirection, +) -> CancellationToken { + connect_peer_with_registration(fixture, peer_id, direction, 0).await +} + +async fn connect_peer_with_registration( + fixture: &ReactorFixture, + peer_id: ZakuraPeerId, + direction: ServicePeerDirection, + registration_id: u64, ) -> CancellationToken { let (send, recv) = crate::zakura::framed_channel(32); fixture @@ -941,7 +1089,8 @@ async fn connect_peer_with_direction( .push(recv); let cancel = CancellationToken::new(); let session = - HeaderSyncPeerSession::from_parts_with_direction(peer_id, direction, send, cancel.clone()); + HeaderSyncPeerSession::from_parts_with_direction(peer_id, direction, send, cancel.clone()) + .with_registration_id(registration_id); fixture .handle .send(HeaderSyncEvent::PeerConnected(session)) @@ -1030,7 +1179,7 @@ async fn stale_header_sync_teardown_keeps_replacement_session() { service.remove_peer(&peer_id, new_conn_id); assert!(matches!( lifecycle.recv().await, - Some(HeaderSyncEvent::PeerDisconnected(disconnected)) if disconnected == peer_id + Some(HeaderSyncEvent::PeerDisconnected { peer, registration_id: None }) if peer == peer_id )); } @@ -2233,7 +2382,10 @@ async fn peer_disconnect_removes_outstanding_requests_for_that_peer() { fixture .handle - .send(HeaderSyncEvent::PeerDisconnected(peer_id.clone())) + .send(HeaderSyncEvent::PeerDisconnected { + peer: peer_id.clone(), + registration_id: None, + }) .await .unwrap(); fixture diff --git a/zebra-network/src/zakura/transport/registry.rs b/zebra-network/src/zakura/transport/registry.rs index d40505aa6b6..8143d1706b1 100644 --- a/zebra-network/src/zakura/transport/registry.rs +++ b/zebra-network/src/zakura/transport/registry.rs @@ -304,6 +304,7 @@ impl ServiceRegistry { /// Fan a newly connected peer out to every service enabled by its negotiated capabilities. pub fn add_peer(&self, peer: Peer) { + let registration_id = peer.registration_id; let ( peer_id, conn_id, @@ -331,17 +332,20 @@ impl ServiceRegistry { .map(|stream| stream.cancel_token.clone()) .unwrap_or_else(|| cancel_token.child_token()); - service.add_peer(Peer::new_with_service_cancel_token( - conn_id, - peer_id.clone(), - remote_ip, - negotiated, - direction, - service_streams, - cancel_token.clone(), - service_cancel_token, - close_cause.clone(), - )); + service.add_peer( + Peer::new_with_service_cancel_token( + conn_id, + peer_id.clone(), + remote_ip, + negotiated, + direction, + service_streams, + cancel_token.clone(), + service_cancel_token, + close_cause.clone(), + ) + .with_registration_id(registration_id), + ); } } @@ -350,6 +354,7 @@ impl ServiceRegistry { /// Returns the capability mask for services that received a peer session, so /// disconnect fanout can be limited to reactors that were actually reached. pub fn add_escalated_peer(&self, peer: Peer) -> u64 { + let registration_id = peer.registration_id; let ( peer_id, conn_id, @@ -384,17 +389,20 @@ impl ServiceRegistry { .map(|stream| stream.cancel_token.clone()) .unwrap_or_else(|| cancel_token.child_token()); - service.add_peer(Peer::new_with_service_cancel_token( - conn_id, - peer_id.clone(), - remote_ip, - negotiated, - direction, - service_streams, - cancel_token.clone(), - service_cancel_token, - close_cause.clone(), - )); + service.add_peer( + Peer::new_with_service_cancel_token( + conn_id, + peer_id.clone(), + remote_ip, + negotiated, + direction, + service_streams, + cancel_token.clone(), + service_cancel_token, + close_cause.clone(), + ) + .with_registration_id(registration_id), + ); } admitted_capabilities @@ -424,6 +432,7 @@ mod tests { wants: Mutex, added: Mutex>, added_streams: Mutex>>, + added_registration_ids: Mutex>, removed: Mutex>, } @@ -435,6 +444,7 @@ mod tests { wants: Mutex::new(true), added: Mutex::new(Vec::new()), added_streams: Mutex::new(Vec::new()), + added_registration_ids: Mutex::new(Vec::new()), removed: Mutex::new(Vec::new()), }) } @@ -469,6 +479,10 @@ mod tests { } fn add_peer(&self, peer: Peer) { + self.added_registration_ids + .lock() + .expect("test service registration id list should not be poisoned") + .push(peer.registration_id); let ( peer_id, _conn_id, @@ -800,4 +814,41 @@ mod tests { &[peer] ); } + + // Duplicate arbitration relies on every service seeing the supervisor + // registration id of the connection a session rides on, so both fanout + // paths must copy it onto the per-service `Peer`. + #[test] + fn add_peer_and_escalated_peer_propagate_registration_id() { + let header = TestService::new("header", vec![stream(5, 0b0001)]); + let registry = ServiceRegistry::new(vec![header.clone()]).expect("stream kinds are unique"); + let peer = ZakuraPeerId::new(vec![13; 32]).expect("32-byte test peer id is valid"); + + registry.add_peer( + Peer::new( + peer.clone(), + None, + 0b0001, + HashMap::new(), + CancellationToken::new(), + ) + .with_registration_id(7), + ); + + let (send_5, recv_5) = framed_channel(1); + let streams = HashMap::from([(5, (recv_5, send_5))]); + registry.add_escalated_peer( + Peer::new(peer, None, 0b0001, streams, CancellationToken::new()) + .with_registration_id(8), + ); + + assert_eq!( + header + .added_registration_ids + .lock() + .expect("test mutex should not be poisoned") + .as_slice(), + &[7, 8], + ); + } } diff --git a/zebra-network/src/zakura/transport/service.rs b/zebra-network/src/zakura/transport/service.rs index 943662adb6d..2ab394c638b 100644 --- a/zebra-network/src/zakura/transport/service.rs +++ b/zebra-network/src/zakura/transport/service.rs @@ -71,6 +71,13 @@ pub struct Peer { pub negotiated: u64, /// Direction of the underlying authenticated connection. pub direction: ServicePeerDirection, + /// Supervisor registration id of the connection this peer session rides on. + /// + /// Duplicate arbitration can replace a peer's registered connection while + /// the displaced one is still tearing down; services use this id to ignore + /// lifecycle events from sessions that no longer belong to the registered + /// connection. Zero for test peers built without a supervisor registration. + pub registration_id: u64, streams: HashMap, cancel_token: CancellationToken, service_cancel_token: CancellationToken, @@ -218,6 +225,7 @@ impl Peer { remote_ip, negotiated, direction, + registration_id: 0, streams, cancel_token, service_cancel_token, @@ -225,6 +233,12 @@ impl Peer { } } + /// Set the supervisor registration id of this peer's connection. + pub(crate) fn with_registration_id(mut self, registration_id: u64) -> Self { + self.registration_id = registration_id; + self + } + /// Take ownership of a stream pair for `kind`. pub fn take_stream(&mut self, kind: u16) -> Option<(FramedRecv, FramedSend)> { self.streams From 758da8403f24c7d3142409b0a9cadff1a6867906 Mon Sep 17 00:00:00 2001 From: roman Date: Sun, 5 Jul 2026 22:14:42 -0600 Subject: [PATCH 10/12] nits --- docs/design/verified-commitment-trees.md | 8 ++- .../src/parallel/commitment_aux_verify.rs | 4 +- .../src/zakura/header_sync/reactor.rs | 32 ++++++----- zebra-network/src/zakura/header_sync/tests.rs | 18 +++++- zebra-state/src/service.rs | 55 +++++++++++++++++++ 5 files changed, 96 insertions(+), 21 deletions(-) diff --git a/docs/design/verified-commitment-trees.md b/docs/design/verified-commitment-trees.md index 1783c94b1d3..75b1333bdd5 100644 --- a/docs/design/verified-commitment-trees.md +++ b/docs/design/verified-commitment-trees.md @@ -732,9 +732,11 @@ commitment before it influences the anchor set or the history MMR.** Consequence verification, the one-block overlap, and the in-memory header-frontier tree now run only while the frontier is below the last checkpoint (the VCT handoff height, the only region the roots are consumed); at/above it header sync runs plainly (Β§6.4). Because that region is checkpoint-final and - gossip-free, the tree advances only by fold-on-commit and never needs a live reload, so the - reorg/gossip/catch-up tree-reload machinery (a `QueryBestHeaderHistoryTree` action, its - `BestHeaderHistoryTreeLoaded` reply, and the reactor's reload dispatches) is **removed**. The + gossip-free, the normal Zakura path advances the tree by fold-on-commit. The + `QueryBestHeaderHistoryTree` / `BestHeaderHistoryTreeLoaded` reload path remains as a guarded + fallback when checkpoint, legacy, or gossip-driven commits move the durable body frontier ahead of + the header-frontier tree below the checkpoint; it rebuilds from durable roots, reanchors if the + rebuilt frontier stops at a gap, and stays idle during ordinary header-leading sync. The reanchor/follow-verified-tip scheduling is kept but gated to fire only at/above the checkpoint. - **Increment 7 β€” indexing follower lane (archive only).** Relocate `tx_by_loc` + address indexes and the per-height trees + subtree CFs onto an async follower, so archive mode regains diff --git a/zebra-chain/src/parallel/commitment_aux_verify.rs b/zebra-chain/src/parallel/commitment_aux_verify.rs index 0f5c863139a..1d8cd537b97 100644 --- a/zebra-chain/src/parallel/commitment_aux_verify.rs +++ b/zebra-chain/src/parallel/commitment_aux_verify.rs @@ -203,7 +203,7 @@ pub fn header_commitment_is_valid_for_chain_history( | Commitment::FinalSaplingRoot(_) | Commitment::ChainHistoryActivationReserved => Ok(()), Commitment::ChainHistoryRoot(actual_history_tree_root) => { - // Heartwood through NU6_2 commits directly to the parent history + // Heartwood through Canopy commits directly to the parent history // tree root, so the current header confirms the roots that were // folded into `history_tree` before this block. let history_tree_root = history_tree @@ -220,7 +220,7 @@ pub fn header_commitment_is_valid_for_chain_history( } } Commitment::ChainHistoryBlockTxAuthCommitment(actual_hash_block_commitments) => { - // NU6_3 onward commits to both the parent history tree root and + // NU5 onward commits to both the parent history tree root and // this block's auth data root. That still preserves the one-block // lag for supplied note commitment roots: `history_tree` is the // parent tree, while `auth_data_root` belongs to the current diff --git a/zebra-network/src/zakura/header_sync/reactor.rs b/zebra-network/src/zakura/header_sync/reactor.rs index c728b8946b0..bd5de4fd23c 100644 --- a/zebra-network/src/zakura/header_sync/reactor.rs +++ b/zebra-network/src/zakura/header_sync/reactor.rs @@ -1147,8 +1147,8 @@ impl HeaderSyncReactor { // Tree-aux roots are an optional serving capability: a peer may legitimately have no // roots for a root-carrying request (it can never serve the root for its own tip, and // may not persist roots at all). A completely rootless non-empty response is therefore - // not misbehavior β€” the headers are unusable for this root-carrying range, so drop them, - // release the request, and retry the range without scoring the peer. + // not misbehavior β€” the headers are unusable for this root-carrying range, so drop them + // and retry the range after the same short backoff as an empty response. if outstanding.range.want_tree_aux_roots && !headers.is_empty() && tree_aux_roots.is_empty() { metrics::counter!("sync.header.response.rootless").increment(1); @@ -1163,9 +1163,7 @@ impl HeaderSyncReactor { outstanding.range.want_tree_aux_roots, 0, ); - self.state.schedule.clear_assignment(outstanding.range); - self.state.schedule.retry(outstanding.range); - self.schedule().await; + self.delay_unusable_response_retry(&peer, outstanding); return; } @@ -1191,7 +1189,6 @@ impl HeaderSyncReactor { if headers.is_empty() { self.record_advisory_unconfirmed(&peer); - let deadline = Instant::now() + self.empty_headers_retry_delay(); self.trace_headers_received( &peer, outstanding.range.start_height, @@ -1202,13 +1199,7 @@ impl HeaderSyncReactor { outstanding.range.want_tree_aux_roots, u32::try_from(tree_aux_roots.len()).unwrap_or(u32::MAX), ); - if let Some(peer_state) = self.state.peers.get_mut(&peer) { - peer_state.outstanding.push(OutstandingRange { - deadline, - clear_assignment_on_timeout: true, - ..outstanding - }); - } + self.delay_unusable_response_retry(&peer, outstanding); return; } @@ -1595,6 +1586,21 @@ impl HeaderSyncReactor { self.startup.request_timeout.min(EMPTY_HEADERS_RETRY_DELAY) } + fn delay_unusable_response_retry( + &mut self, + peer: &ZakuraPeerId, + outstanding: OutstandingRange, + ) { + let deadline = Instant::now() + self.empty_headers_retry_delay(); + if let Some(peer_state) = self.state.peers.get_mut(peer) { + peer_state.outstanding.push(OutstandingRange { + deadline, + clear_assignment_on_timeout: true, + ..outstanding + }); + } + } + async fn schedule(&mut self) { if !self.startup.range_state_actions_enabled { return; diff --git a/zebra-network/src/zakura/header_sync/tests.rs b/zebra-network/src/zakura/header_sync/tests.rs index db38fece496..cbb08c97bf4 100644 --- a/zebra-network/src/zakura/header_sync/tests.rs +++ b/zebra-network/src/zakura/header_sync/tests.rs @@ -2116,11 +2116,13 @@ async fn rootless_non_empty_response_retries_without_misbehavior() { let (network, _) = checkpoint_testnet_with_hash(block::Height(3), checkpoint_hash); let first_checkpoint = block::Height(3); let start = block::Height(4); - let mut fixture = spawn_test_reactor(startup_for( + let mut startup = startup_for( network.clone(), (block::Height(0), network.genesis_hash()), Some((first_checkpoint, checkpoint_hash)), - )); + ); + startup.request_timeout = std::time::Duration::from_millis(20); + let mut fixture = spawn_test_reactor(startup); let peer_id = peer(8); connect_peer(&fixture, peer_id.clone()).await; @@ -2149,7 +2151,17 @@ async fn rootless_non_empty_response_retries_without_misbehavior() { .await .unwrap(); - // The range must be retried (a fresh `GetHeaders` for the same start height), with no + assert!( + tokio::time::timeout( + std::time::Duration::from_millis(5), + next_non_query_action(&mut fixture.actions) + ) + .await + .is_err(), + "rootless unusable responses must not retry immediately" + ); + + // The range must be retried after the short unusable-response backoff, with no // commit and no misbehavior along the way. loop { match next_non_query_action(&mut fixture.actions).await { diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index faec13d41c3..f2b455ac047 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -66,6 +66,8 @@ pub mod block_iter; pub mod chain_tip; pub mod watch_receiver; +const BEST_HEADER_HISTORY_TREE_REBUILD_LOG_INTERVAL: u32 = 100_000; + pub mod check; pub(crate) mod finalized_state; @@ -1724,6 +1726,23 @@ where } else { verified_block_tip }; + let total_to_fold = mmr_tree_height_target + .0 + .saturating_sub(verified_block_tip.0); + let started_at = Instant::now(); + let mut next_progress_log = verified_block_tip + .0 + .saturating_add(BEST_HEADER_HISTORY_TREE_REBUILD_LOG_INTERVAL); + + if total_to_fold > 0 { + tracing::info!( + ?verified_block_tip, + ?best_header_tip, + target = ?mmr_tree_height_target, + total_to_fold, + "rebuilding Zakura best header history tree from durable roots" + ); + } let mut tree = (*base_tree).clone(); let mut next = verified_block_tip @@ -1763,6 +1782,28 @@ where "failed to fold header-tip history tree at {height:?}: {error}" )) })?; + + if total_to_fold >= BEST_HEADER_HISTORY_TREE_REBUILD_LOG_INTERVAL { + let (frontier_height, _frontier_hash) = reconstructed_frontier + .expect("frontier exists because at least one header was folded"); + if frontier_height.0 >= next_progress_log + || frontier_height == mmr_tree_height_target + { + tracing::info!( + ?verified_block_tip, + ?best_header_tip, + frontier = ?frontier_height, + target = ?mmr_tree_height_target, + folded = frontier_height.0.saturating_sub(verified_block_tip.0), + total_to_fold, + elapsed = ?started_at.elapsed(), + "rebuilding Zakura best header history tree" + ); + next_progress_log = frontier_height + .0 + .saturating_add(BEST_HEADER_HISTORY_TREE_REBUILD_LOG_INTERVAL); + } + } } // `count` is capped at `MAX_HEADER_SYNC_HEIGHT_RANGE`. @@ -1796,6 +1837,20 @@ where } }; + if total_to_fold > 0 { + tracing::info!( + ?verified_block_tip, + ?best_header_tip, + frontier = ?frontier.0, + target = ?mmr_tree_height_target, + folded = frontier.0.0.saturating_sub(verified_block_tip.0), + total_to_fold, + elapsed = ?started_at.elapsed(), + complete = frontier.0 == mmr_tree_height_target, + "rebuilt Zakura best header history tree from durable roots" + ); + } + Ok((Arc::new(tree), frontier)) } From 46f41c6312fa3c2c124f6fdd1d560f3722894516 Mon Sep 17 00:00:00 2001 From: roman Date: Sun, 5 Jul 2026 22:35:56 -0600 Subject: [PATCH 11/12] test and merge conflicts --- zebra-test/src/lib.rs | 10 +- .../src/components/zcashd_compat/datadir.rs | 131 +++++++++++++----- 2 files changed, 103 insertions(+), 38 deletions(-) diff --git a/zebra-test/src/lib.rs b/zebra-test/src/lib.rs index a02d268d7cf..6a461ad0cfb 100644 --- a/zebra-test/src/lib.rs +++ b/zebra-test/src/lib.rs @@ -110,7 +110,7 @@ pub fn init() -> impl Drop { .with(ErrorLayer::default()) .init(); - color_eyre::config::HookBuilder::default() + if let Err(error) = color_eyre::config::HookBuilder::default() .add_frame_filter(Box::new(|frames| { let mut displayed = std::collections::HashSet::new(); let filters = &[ @@ -151,7 +151,13 @@ pub fn init() -> impl Drop { })) .panic_message(SkipTestReturnedErrPanicMessages) .install() - .unwrap(); + { + let message = error.to_string(); + assert!( + message.contains("hook has already been installed"), + "failed to install color-eyre test hook: {error:?}", + ); + } }); drop_guard diff --git a/zebrad/src/components/zcashd_compat/datadir.rs b/zebrad/src/components/zcashd_compat/datadir.rs index eb781ea2862..db2e51b6714 100644 --- a/zebrad/src/components/zcashd_compat/datadir.rs +++ b/zebrad/src/components/zcashd_compat/datadir.rs @@ -347,9 +347,16 @@ fn is_truthy(value: &str) -> bool { #[cfg(test)] mod tests { - use std::{fs, path::PathBuf}; + use std::{ + fs, + io::{self, Write}, + path::PathBuf, + sync::{Arc, Mutex}, + }; use tempfile::TempDir; + use tracing::Dispatch; + use tracing_subscriber::fmt::{self, writer::MakeWriter}; use super::{ audit_zcash_conf, ensure_zcashd_datadir, resolve_zcashd_conf_path, BOOTSTRAP_ZCASH_CONF, @@ -418,46 +425,96 @@ mod tests { ); } + #[derive(Clone, Debug)] + struct TestLogWriter { + output: Arc>>, + } + + impl<'writer> MakeWriter<'writer> for TestLogWriter { + type Writer = Self; + + fn make_writer(&'writer self) -> Self::Writer { + self.clone() + } + } + + impl Write for TestLogWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.output + .lock() + .expect("test log buffer should not be poisoned") + .extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + fn captured_logs(action: impl FnOnce()) -> String { + let output = Arc::new(Mutex::new(Vec::new())); + let writer = TestLogWriter { + output: Arc::clone(&output), + }; + let subscriber = fmt::Subscriber::builder() + .with_writer(writer) + .with_ansi(false) + .without_time() + .finish(); + + tracing::dispatcher::with_default(&Dispatch::new(subscriber), action); + + let logs = output + .lock() + .expect("test log buffer should not be poisoned") + .clone(); + String::from_utf8(logs).expect("tracing logs should be valid UTF-8") + } + #[test] - #[tracing_test::traced_test] fn resolves_first_conf_arg_and_warns_on_duplicate() { let datadir = PathBuf::from("/zcashd-datadir"); let extra_args = vec!["-conf=old.conf".to_string(), "--conf=new.conf".to_string()]; - assert_eq!( - resolve_zcashd_conf_path(&datadir, &extra_args), - datadir.join("old.conf") - ); + let logs = captured_logs(|| { + assert_eq!( + resolve_zcashd_conf_path(&datadir, &extra_args), + datadir.join("old.conf") + ); + }); - assert!(logs_contain("multiple path values")); + assert!(logs.contains("multiple path values")); } #[test] - #[tracing_test::traced_test] fn ignores_paired_conf_arg_and_warns() { let datadir = PathBuf::from("/zcashd-datadir"); let extra_args = vec!["-conf".to_string(), "custom.conf".to_string()]; - assert_eq!( - resolve_zcashd_conf_path(&datadir, &extra_args), - datadir.join("zcash.conf") - ); + let logs = captured_logs(|| { + assert_eq!( + resolve_zcashd_conf_path(&datadir, &extra_args), + datadir.join("zcash.conf") + ); + }); - assert!(logs_contain("paired zcashd_extra_args")); + assert!(logs.contains("paired zcashd_extra_args")); } #[test] - #[tracing_test::traced_test] fn ignores_empty_conf_arg_and_warns() { let datadir = PathBuf::from("/zcashd-datadir"); let extra_args = vec!["-conf=".to_string()]; - assert_eq!( - resolve_zcashd_conf_path(&datadir, &extra_args), - datadir.join("zcash.conf") - ); + let logs = captured_logs(|| { + assert_eq!( + resolve_zcashd_conf_path(&datadir, &extra_args), + datadir.join("zcash.conf") + ); + }); - assert!(logs_contain("empty zcashd_extra_args value")); + assert!(logs.contains("empty zcashd_extra_args value")); } #[test] @@ -481,7 +538,6 @@ mod tests { } #[test] - #[tracing_test::traced_test] fn ignores_paired_datadir_extra_arg_override_and_warns() { let temp_dir = TempDir::new().expect("tempdir should be created"); let datadir = temp_dir.path().join("zcashd-datadir"); @@ -491,8 +547,10 @@ mod tests { override_datadir.to_string_lossy().to_string(), ]; - ensure_zcashd_datadir(&datadir, &extra_args) - .expect("paired datadir override warning should be non-fatal"); + let logs = captured_logs(|| { + ensure_zcashd_datadir(&datadir, &extra_args) + .expect("paired datadir override warning should be non-fatal"); + }); assert_eq!( fs::read_to_string(datadir.join("zcash.conf")) @@ -503,7 +561,7 @@ mod tests { !override_datadir.exists(), "paired datadir should not be used for bootstrap inference" ); - assert!(logs_contain("paired zcashd_extra_args")); + assert!(logs.contains("paired zcashd_extra_args")); } #[test] @@ -530,7 +588,6 @@ mod tests { } #[test] - #[tracing_test::traced_test] fn warns_on_listen_but_continues() { let temp_dir = TempDir::new().expect("tempdir should be created"); let conf_path = temp_dir.path().join("zcash.conf"); @@ -540,14 +597,15 @@ mod tests { ) .expect("config should be written"); - audit_zcash_conf(&conf_path).expect("config audit should continue"); + let logs = captured_logs(|| { + audit_zcash_conf(&conf_path).expect("config audit should continue"); + }); - assert!(logs_contain("listen")); - assert!(logs_contain("overridden by supervisor CLI arguments")); + assert!(logs.contains("listen")); + assert!(logs.contains("overridden by supervisor CLI arguments")); } #[test] - #[tracing_test::traced_test] fn warns_on_bind() { let temp_dir = TempDir::new().expect("tempdir should be created"); let conf_path = temp_dir.path().join("zcash.conf"); @@ -557,23 +615,24 @@ mod tests { ) .expect("config should be written"); - audit_zcash_conf(&conf_path).expect("config audit should continue"); + let logs = captured_logs(|| { + audit_zcash_conf(&conf_path).expect("config audit should continue"); + }); - assert!(logs_contain("bind")); - assert!(logs_contain("incompatible with -zebra-compat")); + assert!(logs.contains("bind")); + assert!(logs.contains("incompatible with -zebra-compat")); } #[test] - #[tracing_test::traced_test] fn warns_missing_deprecation_ack() { let temp_dir = TempDir::new().expect("tempdir should be created"); let conf_path = temp_dir.path().join("zcash.conf"); fs::write(&conf_path, "\n").expect("config should be written"); - audit_zcash_conf(&conf_path).expect("config audit should continue"); + let logs = captured_logs(|| { + audit_zcash_conf(&conf_path).expect("config audit should continue"); + }); - assert!(logs_contain( - "missing the zcashd deprecation acknowledgement" - )); + assert!(logs.contains("missing the zcashd deprecation acknowledgement")); } } From 13b08b2c1964bbdbca9bcda0f5ea40c1fd86ab69 Mon Sep 17 00:00:00 2001 From: roman Date: Sun, 5 Jul 2026 22:40:09 -0600 Subject: [PATCH 12/12] lint --- zebrad/src/components/zcashd_compat/datadir.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/zebrad/src/components/zcashd_compat/datadir.rs b/zebrad/src/components/zcashd_compat/datadir.rs index db2e51b6714..8d474eb5d4d 100644 --- a/zebrad/src/components/zcashd_compat/datadir.rs +++ b/zebrad/src/components/zcashd_compat/datadir.rs @@ -440,10 +440,11 @@ mod tests { impl Write for TestLogWriter { fn write(&mut self, buf: &[u8]) -> io::Result { - self.output + let mut output = self + .output .lock() - .expect("test log buffer should not be poisoned") - .extend_from_slice(buf); + .map_err(|error| io::Error::other(format!("test log buffer poisoned: {error}")))?; + output.extend_from_slice(buf); Ok(buf.len()) }