fix(network): block-sync stack review follow-ups

CHANGELOG.md

@@ -112,8 +112,9 @@ and this project adheres to [Semantic Versioning](https://semver.org).
   no untrusted data can influence consensus state. This is the default whenever
   `consensus.checkpoint_sync = true` on a network with an embedded handoff frontier
   (Mainnet), for both Archive and Pruned storage modes. The new
-  `consensus.disable_vct_fast_sync` flag (default `false`) keeps checkpoint sync
-  enabled while forcing the legacy per-block recompute. Bumps the state database
+  `consensus.vct_fast_sync` flag (default `true`) selects this fast path; set it to
+  `false` to keep checkpoint sync enabled while forcing the legacy per-block
+  recompute. Bumps the state database
   format to 27.3.0 (new column families only; no data migration).
 - Include the `zebra-rollback-state` and `zebra-prune-state` utilities alongside
   `zebrad` in release Docker images and Docker CI builds.

docs/design/verified-commitment-trees.md

@@ -67,7 +67,7 @@ the direct below-Heartwood/below-NU5 checks); fold it in; freeze the frontier (
 | **Fail closed** | In the frozen window, refuse the commit (retryable) rather than recompute or guess (§8). |
 | **Provisional roots** | Peer-supplied roots carried in the header-sync `Headers` message and persisted to `zakura_header_commitment_roots_by_height` ahead of body commit. Advisory until verify-before-commit authenticates them (§4.2, §6). |
 | **All-or-nothing** | A `Headers` message carries roots for _every_ header in the range or none; a partial root set is rejected on the wire and never served (§5.4). |
-| **Kill switch** | `consensus.disable_vct_fast_sync = true`: keep checkpoint sync but force the legacy committer (§4.4). |
+| **Kill switch** | `consensus.vct_fast_sync = false`: keep checkpoint sync but force the legacy committer (§4.4). |
 
 For where each piece lives in the tree, see the file map (§15).
 
@@ -97,10 +97,10 @@ or verify a root falls back to the legacy recompute, bit-identical to today.
   fast-synced database format.
 - **Not a consensus change.** There are exactly two enduring code paths: the standard local
   tree rebuild (legacy) and the fast verified path. Which one runs is config-driven by
-  `consensus.checkpoint_sync` plus the rollout force-disable knob
-  (`consensus.disable_vct_fast_sync`; §4.4); the `state.storage_mode` axis (Archive vs. Pruned)
+  `consensus.checkpoint_sync` plus the rollout fast-sync knob
+  (`consensus.vct_fast_sync`; §4.4); the `state.storage_mode` axis (Archive vs. Pruned)
   is orthogonal — it controls raw-tx/index pruning, not the tree path, so both storage modes
-  use the fast path under checkpoint sync unless force-disabled. The network `PeerSource` and
+  use the fast path under checkpoint sync unless fast sync is disabled. The network `PeerSource` and
   crate-local test fixtures are _sources_ behind one seam (§5.3) — not modes.
 - **No new cryptography.** Verification reuses the existing consensus checks
   (`block_commitment_is_valid_for_chain_history`, `HistoryTree::push`); see §6.
@@ -178,15 +178,15 @@ precedence once the corresponding body is durable.
 ### 4.4 Mode selection: fast under checkpoint sync
 
 The fast-vs-legacy choice is driven by user-facing config, not by env vars. The axes are
-`consensus.checkpoint_sync` (full checkpoint trust), `consensus.disable_vct_fast_sync` (initial
-rollout force-disable for VCT fast sync), and `state.storage_mode` (Archive vs. Pruned, an
+`consensus.checkpoint_sync` (full checkpoint trust), `consensus.vct_fast_sync` (initial
+rollout fast-sync knob for VCT fast sync), and `state.storage_mode` (Archive vs. Pruned, an
 orthogonal pruning axis). The resulting modes:
 
 | Mode | Config | Tree behavior |
 | --- | --- | --- |
-| **Archive** (default) | `consensus.checkpoint_sync = true`, `consensus.disable_vct_fast_sync = false`, `storage_mode = archive` | Fast — verified roots folded in, recompute skipped. Unpruned (raw tx + indexes kept). No per-height tree history below the last checkpoint height _for now_ (§7, §10). |
-| **Pruning** | `consensus.checkpoint_sync = true`, `consensus.disable_vct_fast_sync = false`, `storage_mode.pruned` | Fast — same as Archive, **plus** raw-tx/index pruning outside the retention window. |
-| **Force-disabled VCT** | `consensus.checkpoint_sync = true`, `consensus.disable_vct_fast_sync = true` (any storage mode) | Legacy — keeps checkpoint sync enabled but fully reconstructs the Sapling/Orchard trees per block. |
+| **Archive** (default) | `consensus.checkpoint_sync = true`, `consensus.vct_fast_sync = true`, `storage_mode = archive` | Fast — verified roots folded in, recompute skipped. Unpruned (raw tx + indexes kept). No per-height tree history below the last checkpoint height _for now_ (§7, §10). |
+| **Pruning** | `consensus.checkpoint_sync = true`, `consensus.vct_fast_sync = true`, `storage_mode.pruned` | Fast — same as Archive, **plus** raw-tx/index pruning outside the retention window. |
+| **Force-disabled VCT** | `consensus.checkpoint_sync = true`, `consensus.vct_fast_sync = false` (any storage mode) | Legacy — keeps checkpoint sync enabled but fully reconstructs the Sapling/Orchard trees per block. |
 | **Checkpoint sync disabled** | `consensus.checkpoint_sync = false` (any storage mode) | Legacy — fully reconstructs the Sapling/Orchard trees per block, using only mandatory checkpoints. |
 
 Gating fast on `checkpoint_sync` is also a correctness precondition: the embedded last checkpoint height
@@ -198,17 +198,17 @@ Canopy mandatory checkpoint, so there is no valid last checkpoint height to resu
 `zebra-consensus`.
 
 Precedence is resolved by a pure, unit-tested `select_source_mode` (no process env, no embedded
-files in the decision — `consensus.checkpoint_sync`, `consensus.disable_vct_fast_sync`, and the
+files in the decision — `consensus.checkpoint_sync`, `consensus.vct_fast_sync`, and the
 embedded-frontier presence are passed in as plain inputs):
 
-1. `consensus.checkpoint_sync = false`, `consensus.disable_vct_fast_sync = true`, or a network
+1. `consensus.checkpoint_sync = false`, `consensus.vct_fast_sync = false`, or a network
    with **no embedded frontier** → **legacy** (no VCT state, zero overhead);
 2. else → **peer** (the default under checkpoint sync where embedded frontiers exist).
 
 The earlier file-backed checkpoint/fixture root source (`VCT_FAST`/`VCT_FIXTURE`) and capture
 mode (`VCT_CAPTURE`) were transient integration scaffolding before peer delivery existed and
 have been removed. `VCT_REGTEST_FRONTIER` remains as a Regtest final-frontier test hook.
-`consensus.disable_vct_fast_sync = true` is the supported user-facing way to force the legacy
+`consensus.vct_fast_sync = false` is the supported user-facing way to force the legacy
 committer without disabling checkpoint sync (the deliberate opt-out for the default-on path; see
 the status note at the top of this document).
 
@@ -427,16 +427,16 @@ occurred — the needed `H+1` witness is merely not buffered yet.
 to `pruning_metadata`, not a reuse — pruning drops tx bytes and keeps trees, fast-sync drops the
 per-height trees; a DB can be both. Because fast sync deletes nothing, a **completed** fast-synced
 DB (tip at/above the last checkpoint height) **reopens in any storage mode** — a reopen loses no servable data,
-and `consensus.disable_vct_fast_sync = true` or `consensus.checkpoint_sync = false` simply resumes
+and `consensus.vct_fast_sync = false` or `consensus.checkpoint_sync = false` simply resumes
 the legacy recompute from the real tip frontier.
 
 The one reopen that _is_ refused is an **interrupted** fast sync (frozen frontier, tip below the
 last checkpoint height) reopened with the fast path disabled (legacy mode —
-`consensus.disable_vct_fast_sync = true`, `consensus.checkpoint_sync = false`, or no embedded
+`consensus.vct_fast_sync = false`, `consensus.checkpoint_sync = false`, or no embedded
 frontier). The on-disk frontier is stale and no source can supply the verified roots, so the
 fail-closed policy (§8) would refuse every below-last checkpoint height block forever. The open guard refuses
 with a clear recovery path (finish the fast sync under `consensus.checkpoint_sync = true` and
-`consensus.disable_vct_fast_sync = false`, or re-sync from genesis) instead of stalling silently.
+`consensus.vct_fast_sync = true`, or re-sync from genesis) instead of stalling silently.
 Guards: per-height tree reads return `None` below the last checkpoint height (before the backward search, so no
 stale tree and no panic); `z_gettreestate` returns a typed archive-mode error below the last checkpoint height;
 genesis-root and subtree format-validity checks skip fast-synced DBs.
@@ -623,7 +623,7 @@ Live commit-path counters distinguish the fast and legacy paths and the failure
 | Metric | Meaning |
 | --- | --- |
 | `state.vct.fast.block.count` | block folded supplied roots, skipped the recompute |
-| `state.vct.legacy.block.count` | block recomputed the frontier (`consensus.disable_vct_fast_sync = true`, `consensus.checkpoint_sync = false`, or fell back outside the frozen window) |
+| `state.vct.legacy.block.count` | block recomputed the frontier (`consensus.vct_fast_sync = false`, `consensus.checkpoint_sync = false`, or fell back outside the frozen window) |
 | `state.vct.prevalidated.block.count` | dedup sub-case: the previous fast block's look-ahead already validated this header |
 | `state.vct.root.rejected.count` | supplied root failed verification and was deleted for re-delivery |
 | `state.vct.root.unavailable.count` | frozen-frontier height with no valid root; commit refused (retryable) |
@@ -644,7 +644,7 @@ asserts to prove roots actually came over the wire rather than a silent legacy s
   invalid-marker / unrequested-roots rejections
   (`decode_rejects_tree_aux_roots_when_not_requested`,
   `non_finalized_response_carrying_tree_aux_roots_is_malformed`) and the byte-budget clamp with
-  roots requested; `select_source_mode` precedence (`consensus.disable_vct_fast_sync = true` or
+  roots requested; `select_source_mode` precedence (`consensus.vct_fast_sync = false` or
   `consensus.checkpoint_sync = false` ⇒ legacy regardless of storage mode or embedded frontier;
   checkpoint sync + enabled VCT + embedded frontier ⇒ peer); a completed fast-synced DB reopens
   in archive mode (`reopening_fast_synced_database_in_archive_mode_succeeds`) while an interrupted

docs/plans/headersync_roots_review.md

@@ -33,7 +33,7 @@ mandatory-roots rule on _ranged_ requests does not starve the tip.
 1. **`zebra-state` proptest `service::finalized_state::tests::prop::vct_frozen_frontier_survives_reopen`.**
    Panics at `finalized_state.rs:551`: "database was previously synced in verified
    commitment tree mode ... fast path ... is disabled. Set `consensus.checkpoint_sync = true`
-   and `consensus.disable_vct_fast_sync = false`...". This is #254's VCT fast-sync resume
+   and `consensus.vct_fast_sync = true`...". This is #254's VCT fast-sync resume
    gate; the proptest reopen config doesn't satisfy the resume preconditions. Verified to
    fail identically on the base branch. Relies on later VCT-resume wiring → flup.
 

zebra-consensus/src/config.rs

@@ -26,8 +26,8 @@ pub struct Config {
     ///
     /// Disabling this option makes Zebra start full validation earlier.
     /// It is slower and less secure.
-    /// To keep checkpoint sync enabled but force-disable the initial VCT fast-sync rollout, use
-    /// [`disable_vct_fast_sync`](Self::disable_vct_fast_sync) instead.
+    /// To keep checkpoint sync enabled but opt out of the initial VCT fast-sync rollout, set
+    /// [`vct_fast_sync`](Self::vct_fast_sync) to `false`.
     ///
     /// Zebra requires some checkpoints to simplify validation of legacy network upgrades.
     /// Required checkpoints are always active, even when this option is `false`.
@@ -38,26 +38,26 @@ pub struct Config {
     /// release.
     pub checkpoint_sync: bool,
 
-    /// Force-disable the verified-commitment-trees fast sync path during its initial rollout.
+    /// Use the verified-commitment-trees fast sync path during its initial rollout.
     ///
-    /// This keeps [`checkpoint_sync`](Self::checkpoint_sync) enabled while forcing the legacy
-    /// per-block Sapling/Orchard tree recompute in both Archive and Pruned storage modes. Set to
-    /// `false` by default: checkpoint sync uses VCT fast sync on networks with embedded handoff
-    /// frontiers.
-    pub disable_vct_fast_sync: bool,
+    /// `true` by default: checkpoint sync folds in verified Sapling/Orchard roots and skips the
+    /// per-block tree recompute on networks with embedded handoff frontiers. Set to `false` to
+    /// keep [`checkpoint_sync`](Self::checkpoint_sync) enabled while forcing the legacy per-block
+    /// recompute in both Archive and Pruned storage modes.
+    pub vct_fast_sync: bool,
 }
 
 impl From<InnerConfig> for Config {
     fn from(
         InnerConfig {
             checkpoint_sync,
-            disable_vct_fast_sync,
+            vct_fast_sync,
             ..
         }: InnerConfig,
     ) -> Self {
         Self {
             checkpoint_sync,
-            disable_vct_fast_sync,
+            vct_fast_sync,
         }
     }
 }
@@ -66,12 +66,12 @@ impl From<Config> for InnerConfig {
     fn from(
         Config {
             checkpoint_sync,
-            disable_vct_fast_sync,
+            vct_fast_sync,
         }: Config,
     ) -> Self {
         Self {
             checkpoint_sync,
-            disable_vct_fast_sync,
+            vct_fast_sync,
             _debug_skip_parameter_preload: false,
         }
     }
@@ -88,7 +88,7 @@ pub struct InnerConfig {
     pub checkpoint_sync: bool,
 
     /// See [`Config`] for more details.
-    pub disable_vct_fast_sync: bool,
+    pub vct_fast_sync: bool,
 
     #[serde(skip_serializing, rename = "debug_skip_parameter_preload")]
     /// Unused config field for backwards compatibility.
@@ -102,7 +102,7 @@ impl Default for Config {
     fn default() -> Self {
         Self {
             checkpoint_sync: true,
-            disable_vct_fast_sync: false,
+            vct_fast_sync: true,
         }
     }
 }
@@ -111,7 +111,7 @@ impl Default for InnerConfig {
     fn default() -> Self {
         Self {
             checkpoint_sync: Config::default().checkpoint_sync,
-            disable_vct_fast_sync: Config::default().disable_vct_fast_sync,
+            vct_fast_sync: Config::default().vct_fast_sync,
             _debug_skip_parameter_preload: false,
         }
     }
@@ -122,19 +122,19 @@ mod tests {
     use super::*;
 
     #[test]
-    fn disable_vct_fast_sync_defaults_false_and_converts_through_inner_config() {
-        assert!(!Config::default().disable_vct_fast_sync);
+    fn vct_fast_sync_defaults_true_and_converts_through_inner_config() {
+        assert!(Config::default().vct_fast_sync);
 
         let force_disabled = Config::from(InnerConfig {
             checkpoint_sync: true,
-            disable_vct_fast_sync: true,
+            vct_fast_sync: false,
             _debug_skip_parameter_preload: false,
         });
 
         assert!(force_disabled.checkpoint_sync);
-        assert!(force_disabled.disable_vct_fast_sync);
+        assert!(!force_disabled.vct_fast_sync);
 
         let inner = InnerConfig::from(force_disabled);
-        assert!(inner.disable_vct_fast_sync);
+        assert!(!inner.vct_fast_sync);
     }
 }

zebra-network/src/zakura/block_sync/config.rs

@@ -76,8 +76,6 @@ pub const BS_CHECKPOINT_RANGE_BYTE_FLOOR: u64 =
     MIN_BS_CHECKPOINT_SUBMITTED_BLOCK_APPLIES as u64 * BS_PER_BLOCK_WORST_CASE_BYTES;
 /// Default block-sync request timeout.
 pub const DEFAULT_BS_REQUEST_TIMEOUT: Duration = Duration::from_secs(8);
-/// Default central floor-watchdog cadence.
-pub const DEFAULT_BS_FLOOR_WATCHDOG_TICK: Duration = Duration::from_secs(1);
 /// Default hard floor-peer avoid cooldown after a watchdog cancellation.
 pub const DEFAULT_BS_FLOOR_PEER_AVOID_COOLDOWN: Duration = DEFAULT_BS_REQUEST_TIMEOUT;
 /// Default block-sync status refresh interval reserved for later advertisement.
@@ -176,9 +174,6 @@ pub struct ZakuraBlockSyncConfig {
     pub max_reorder_lookahead_bytes: u64,
     /// Maximum speculative body heights tracked above the download floor.
     pub max_reorder_lookahead_blocks: u32,
-    /// Cadence for the central floor watchdog that rescues expired floor claims.
-    #[serde(with = "humantime_serde")]
-    pub floor_watchdog_tick: Duration,
     /// How long to avoid reassigning an expired floor height to the same peer.
     #[serde(with = "humantime_serde")]
     pub floor_peer_avoid_cooldown: Duration,
@@ -218,7 +213,6 @@ impl Default for ZakuraBlockSyncConfig {
             max_inflight_block_bytes: DEFAULT_BS_MAX_INFLIGHT_BLOCK_BYTES,
             max_reorder_lookahead_bytes: DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BYTES,
             max_reorder_lookahead_blocks: DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BLOCKS,
-            floor_watchdog_tick: DEFAULT_BS_FLOOR_WATCHDOG_TICK,
             floor_peer_avoid_cooldown: DEFAULT_BS_FLOOR_PEER_AVOID_COOLDOWN,
             max_submitted_block_applies: DEFAULT_BS_MAX_SUBMITTED_BLOCK_APPLIES,
             request_timeout: DEFAULT_BS_REQUEST_TIMEOUT,
@@ -258,13 +252,6 @@ impl ZakuraBlockSyncConfig {
             .min(self.max_inflight_block_bytes)
     }
 
-    /// Return the watchdog tick clamped to a positive duration no larger than the request timeout.
-    pub fn effective_floor_watchdog_tick(&self) -> Duration {
-        self.floor_watchdog_tick
-            .min(self.request_timeout)
-            .max(Duration::from_millis(1))
-    }
-
     /// Return the floor avoid cooldown clamped to a positive duration.
     pub fn effective_floor_peer_avoid_cooldown(&self) -> Duration {
         self.floor_peer_avoid_cooldown.max(Duration::from_millis(1))

zebra-network/src/zakura/block_sync/events.rs

@@ -169,8 +169,8 @@ pub enum BlockSyncMisbehavior {
 pub(super) enum RoutineToReactor {
     /// A routine received a `Status` and updated its own servable/caps + the
     /// registry. The reactor advertises our `Status` reply and republishes the
-    /// candidate set. `send_reply` is the routine's rate-meter decision (the
-    /// previous `unsolicited.try_take`) for whether a reply is due this time.
+    /// candidate set. `send_reply` is the routine's rate-meter decision for whether
+    /// a reply is due this time.
     StatusReceived {
         /// Peer whose status was applied.
         peer: ZakuraPeerId,

zebra-network/src/zakura/block_sync/peer_registry.rs

@@ -51,9 +51,8 @@ pub(super) struct Entry {
     /// reject-rollback window.
     pub(super) outstanding: BTreeMap<block::Height, OutstandingMeta>,
     /// Routine-published slot diagnostics (trace only): the per-peer download
-    /// window state the reactor used to read off `PeerBlockState` for the periodic
-    /// `BLOCK_SYNC_STATE` row. Updated whenever the routine issues/finishes/times
-    /// out a request.
+    /// window state the reactor reads for the periodic `BLOCK_SYNC_STATE` row.
+    /// Updated whenever the routine issues/finishes/times out a request.
     pub(super) slots: SlotDiagnostics,
     /// Heights this peer may not re-take until the given instant.
     pub(super) retry_avoid: BTreeMap<block::Height, Instant>,
@@ -426,6 +425,20 @@ impl PeerRegistry {
         (servable, outstanding)
     }
 
+    /// The soonest deadline among all peer claims for one height, if any. Lets the
+    /// reactor arm its floor watchdog to the exact expiry without allocating a
+    /// claim snapshot on every loop iteration.
+    pub(super) fn earliest_outstanding_deadline_at(
+        &self,
+        height: block::Height,
+    ) -> Option<Instant> {
+        let peers = self.lock();
+        peers
+            .values()
+            .filter_map(|entry| entry.outstanding.get(&height).map(|meta| meta.deadline))
+            .min()
+    }
+
     /// Snapshot all peer claims for one height.
     pub(super) fn outstanding_claims_at(&self, height: block::Height) -> Vec<OutstandingClaim> {
         let peers = self.lock();
@@ -502,7 +515,7 @@ pub(super) struct DirectionStatusCounts {
 }
 
 /// Hard outbound concurrency ceiling for a peer with the given advertised
-/// in-flight cap (the routine's slot bound; mirrors `PeerBlockState`).
+/// in-flight cap (the routine's slot bound).
 pub(super) fn hard_outbound_capacity(max_inflight_requests: u32) -> usize {
     usize::try_from(max_inflight_requests)
         .expect("u32 max inflight requests fits in usize on supported targets")