fix(sync): stop Zakura sync drivers before legacy ChainSync fallback

[p0mvn]

Motivation

When network.v2_p2p is enabled, the legacy syncer runs in watchdog mode (ChainSync::bootstrap_genesis_then_pause) while the Zakura block-sync driver drives body sync. On a Zakura body-sync stall the watchdog called self.sync(), restarting the legacy ChainSync body downloader — but the Zakura header- and block-sync drivers kept running. Two committers then fed the state-commit pipeline at once, breaking its accounting and deadlocking the node.

Solution

Keep the recovery path, but make the switch safe and scope it correctly (per review feedback from @evan-forbes):

• Stop Zakura before legacy. When the watchdog falls back, it first cancels the Zakura endpoint shutdown token (header_sync_shutdown()) via the stop_zakura_sync helper, stopping the Zakura header- and block-sync drivers, then calls self.sync(). Only one body-sync committer is ever active, so the deadlock can't happen.
• Only dual-stack nodes fall back. legacy_fallback = config.network.v2_p2p && config.network.legacy_p2p, computed in start.rs and passed into bootstrap_genesis_then_pause. A Zakura-only node (legacy_p2p = false) has no legacy peers to fall back to, so it never switches: it warns once per stall window and keeps waiting for Zakura (and skips the FindBlocks cross-check probe).
• Both fallback triggers are preserved for dual-stack nodes: the gap-based stall rule and the legacy-informed FindBlocks cross-check (the fleet-restart blind spot).

No new config surface — the gate is derived from the existing v2_p2p/legacy_p2p flags, so zebrad generate output and the stored-config fixtures are unchanged.

Also bumps anyhow 1.0.102 → 1.0.103 (separate commit) to clear RUSTSEC-2026-0190, which the cargo deny advisories CI check started flagging on ironwood-main.

Tests

• New unit tests in sync/tests/fallback.rs:
  • fallback_cancels_the_zakura_shutdown_token — asserts the hand-off cancels the drivers' shutdown token (the deadlock-prevention invariant), verified via a child token standing in for the drivers' observed shutdown.
  • stop_zakura_sync_is_a_noop_without_a_token — a Zakura-only node has no token; the helper must not panic.
  • The existing gap-rule and legacy-probe decision tests are retained.
• cargo fmt --all -- --check — clean
• cargo build -p zebrad --locked — clean
• cargo clippy -p zebrad -p zebra-network --all-targets -- -D warnings — clean
• cargo test -p zebrad -- components::sync → all passing

AI Disclosure

• AI tools were used: Claude Code for the code changes, tests, changelog, and this PR description.

[v12-auditor]

Note

Complete: Audit complete. V12 found two issues worth reviewing.

Open the full results here.

Finding	Severity	Details
F-94112	🟠 High	Fallback does not drain commits The Zakura-to-legacy fallback cancellation path is not a shutdown barrier. When either fallback trigger fires, bootstrap_genesis_then_pause() calls stop_zakura_block_sync() and immediately enters self.sync().await, while stop_zakura_block_sync() only calls CancellationToken::cancel(). The Zakura block-sync driver observes that cancellation only at its idle receive/select point; ready actions, deferred actions, and already-started block-apply futures can continue long enough to submit or finish zebra_consensus::Request::Commit work. Legacy ChainSync then starts using the same verifier and state commit path, so a peer-timed Zakura apply can overlap with legacy commits despite the new comments saying concurrent committers break state-commit accounting and can deadlock the node.
F-94114	🟡 Medium	Unvalidated probe forces fallback The legacy-informed fallback probe trusts unvalidated FindBlocks responses to decide that Zakura is behind. legacy_peers_blocks_ahead() fans out requests and increments its ahead counter for every peer-supplied hash that is not already in local state. It does not require the hashes to connect to the locator, correspond to real blocks, or be corroborated by multiple peers before returning once the 64-hash threshold is reached. A single malicious selected legacy peer can return 64 arbitrary unknown hashes, causing the watchdog to cancel Zakura block sync and enter legacy ChainSync even when the legacy network is not actually ahead.

And two more auto-invalidated findings.

Analyzed three files, diff 963deb9...fa3aa7e.

[p0mvn]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit bf95d60. Configure here.}

[evan-forbes]

Instead of removing the fallback globally, maybe we could disable it only when v2_p2p = true and legacy_p2p = false?

For dual-stack nodes, fallback is still useful, but when it triggers we should first cancel the Zakura header/block sync shutdown token, then start legacy ChainSync.

That keeps the recovery path while ensuring only one body-sync committer is active.

  let legacy_fallback = config.network.v2_p2p && config.network.legacy_p2p;

  syncer
      .bootstrap_genesis_then_pause(
          read_only_state_service.clone(),
          legacy_fallback,
          zakura_endpoint
              .as_ref()
              .and_then(|endpoint| endpoint.header_sync_shutdown()),
      )
...
  if zakura_block_sync_stalled(...) {
      if !legacy_fallback {
          warn!(
              verified_tip = ?verified_height,
              header_tip = ?header_tip_height,
              stall = ?ZAKURA_BODY_SYNC_STALL_TIMEOUT,
              "Zakura body sync is stalled; legacy fallback disabled because legacy_p2p is off"
          );
          continue;
      }

      // dual-stack fallback path
  }

then actually shutdown headersync and blocksync

  if let Some(shutdown) = zakura_shutdown.as_ref() {
      shutdown.cancel();
  }

  warn!(
      verified_tip = ?verified_height,
      header_tip = ?header_tip_height,
      "Zakura body sync stalled; stopped Zakura sync drivers and switching to legacy ChainSync"
  );

  return self.sync().await;

provided there are some other nodes running both stack, only running zakura should be fine

[p0mvn]

Thanks @evan-forbes — adopted this approach in 474ca09.

• Fallback is now gated on legacy_fallback = config.network.v2_p2p && config.network.legacy_p2p, computed in start.rs and passed into bootstrap_genesis_then_pause along with the endpoint shutdown token (zakura_endpoint.as_ref().and_then(|e| e.header_sync_shutdown())).
• Dual-stack nodes keep the recovery path (both the gap-based and the legacy-informed/FindBlocks triggers). When either fires, the watchdog now stop_zakura_sync()s — cancelling the shared endpoint shutdown token so the Zakura header- and block-sync drivers stop — before calling self.sync(), so only one body-sync committer is ever active.
• Zakura-only nodes (legacy_p2p = false) never switch: they warn once per stall window and keep waiting for Zakura, and skip the FindBlocks cross-check entirely.

No new config surface and no stored-config change (the gate is derived from existing flags). Local checks: fmt, clippy --all-targets -D warnings, build --locked, and cargo test -p zebrad -- components::sync (42 passed) all green.

[p0mvn]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 8a5f531. Configure here.}

[p0mvn]

@v12-auditor review

[v12-auditor]

Note

Complete: Audit complete. V12 found four issues worth reviewing.

Open the full results here.

Finding	Severity	Details
F-94205	🟠 High	Fabricated Hashes Downgrade Zakura A dual-stack node can be forced off Zakura by unauthenticated legacy FindBlocks inventory. When the node is caught up to its Zakura header frontier, zakura_block_sync_stalled() treats the small gap as healthy, but the refactored watchdog still arms ProbeLegacyPeers after the verified tip remains frozen for three polls. legacy_peers_blocks_ahead() then counts any locally unknown hashes in a legacy peer's BlockHashes response as evidence that the peer is at least 64 blocks ahead, without validating that the hashes form a real chain or that headers/blocks exist. Once that threshold is met, the new fallback path calls stop_zakura_sync() and returns into legacy ChainSync::sync(), cancelling the shared Zakura driver token. A malicious legacy peer selected by the probe fanout can return 64 fabricated block inventory hashes and trigger a persistent protocol downgrade until restart.
F-94206	🟠 High	Fallback Races Residual Commits The fallback handoff cancels Zakura but does not wait for Zakura tasks or their already-submitted commits to finish. stop_zakura_sync() only calls CancellationToken::cancel(), and bootstrap_genesis_then_pause() immediately returns into self.sync().await on both fallback branches. The block-sync driver may already have apply_block_sync_body() futures in its FuturesUnordered; those futures submit zebra_consensus::Request::Commit through the same verifier that legacy ChainSync uses. When the driver observes shutdown, it returns and drops the in-flight futures, but the consensus router documents that dropped requests are cancelled only best-effort and may continue processing, and checkpoint verification explicitly spawns commit work that continues even if response futures are dropped. A remote peer that triggers fallback while Zakura commit work is in flight can therefore create the concurrent committer condition the diff says must be avoided.
F-94207	🟠 High	Zakura-Only Recovery Disabled Zakura-only nodes now continue waiting forever when the watchdog detects a body-sync stall. Startup computes legacy_fallback = config.network.v2_p2p && config.network.legacy_p2p, so a v2_p2p=true, legacy_p2p=false node passes false into bootstrap_genesis_then_pause(). In that mode, zakura_watchdog_action() returns only WarnOnly or ContinueWaiting on a stall and explicitly skips the legacy-probe/fallback path. This is not equivalent to having no recovery transport: disabling legacy_p2p stops the TCP legacy listener, but the service handed to the syncer is still wrapped in ZakuraDualStackService, whose documented routing sends ChainSync inventory requests straight to Zakura when legacy is disabled. A hostile or degraded Zakura peer set can therefore keep native body sync from closing the header gap while the available ChainSync-over-Zakura recovery path remains unreachable.
F-94208	🟡 Medium	Oversized Inventory Probe Amplification The new legacy watchdog probe can be turned into large repeated state-query amplification. On ProbeLegacyPeers, legacy_peers_blocks_ahead() fans out FindBlocks to three peers, accepts each BlockHashes response, and calls state_contains() for every returned hash until it sees 64 unknown hashes. A malicious peer can avoid the early exit by returning a large inventory of already-known or duplicate-known block hashes, keeping the ahead count at zero while forcing every hash through the state lookup path. The connection layer converts all block inv entries into Response::BlockHashes, and the inventory deserializer permits up to 50,000 inventory items in a received message. Because the probe returns below threshold in this case, the watchdog continues and can repeat the expensive scan on later frozen-tip polls.

Analyzed two files, diff 963deb9...8a5f531.

[p0mvn]

F-94112 has been addressed adc1c5a

[p0mvn]

F-94114 is addressed here: #338

[p0mvn]

Discussed F-94205 and agreed as non-issue

[p0mvn]

F-94206 has been addressed adc1c5a

[p0mvn]

On F-94207, we intentionally do not want to fall back to legacy sync.

But I think we could add Zakura-specific recovery later, based on traces with the new congestion protocol #337.