Skip to content

fix(zakura): only advance and gossip NewBlocks that land on the best chain#478

Closed
p0mvn wants to merge 1 commit into
zakura/header-sync-fork-recoveryfrom
zakura/newblock-best-chain-gate
Closed

fix(zakura): only advance and gossip NewBlocks that land on the best chain#478
p0mvn wants to merge 1 commit into
zakura/header-sync-fork-recoveryfrom
zakura/newblock-best-chain-gate

Conversation

@p0mvn

@p0mvn p0mvn commented Jul 6, 2026

Copy link
Copy Markdown

Stacked on #476 (zakura/header-sync-fork-recovery); please merge that first and this PR will retarget/rebase to ironwood-main cleanly.

Motivation

An inbound Zakura NewBlock that the block pipeline accepts can have committed to a side chain (contextually valid, lower work — e.g. a testnet min-difficulty branch). handle_new_block_accepted still advanced the node's header and verified frontiers and forwarded the block to every peer with a lower advertised tip, without checking which chain the block landed on. The result, observed live on the zakura testnet fleet: all 5 dual-stack fleet nodes advertised and re-forwarded a min-difficulty branch over Zakura (statuses at 4145079 on the losing branch) while their own chains stayed honest via legacy sync (4145205) — and the zakura-only node that trusted the gossip followed the losing branch end-to-end and stranded (recovered by #476, but the gossip layer should not propagate losing branches in the first place). This was discovered while validating #476 and is recorded as the follow-up bug there.

Solution

After the verifier accepts a NewBlock, the zebrad driver checks the committed hash against the best chain with ReadRequest::Depth (which answers Some only for best-chain blocks, so it also covers a reorg the block just won):

  • Best chain → unchanged behavior: NewBlockAccepted advances the frontiers and forwards to eligible peers.
  • Side chain → new HeaderSyncEvent::NewBlockAcceptedSideChain: the reactor remembers the hash for dedup only — no frontier advance, no height-covered marking, no forwarding. Counted in sync.header.tip.new_block.side_chain and traced with reason side_chain.

Read failures are conservatively treated as side-chain: the node's own frontier still advances through the chain-tip mirror, so a false negative only skips one gossip forward, while a false positive would gossip a possibly losing branch.

Tests

  • Integration (driver + real reactor): new_block_side_chain_commit_does_not_advance_header_frontier spawns a real Zakura endpoint with the header-sync driver, feeds NewBlockReceived actions through the real driver with a mocked verifier/state, and asserts that a Depth = None accept leaves the header frontier untouched while a Depth = Some accept advances it.
  • Reactor unit: accepted_side_chain_new_block_is_deduped_without_advancing_or_forwarding — a side-chain accept forwards nothing, advances nothing, publishes no tip, and its hash still dedups a later wire NewBlock without scoring the sender.
  • cargo fmt, cargo clippy --all-targets, cargo test -p zebra-network --lib (897 passed), -p zebrad --lib zakura (53 passed) all pass.
  • Fleet: deployed to all 6 zakura testnet nodes (binary d57970e2…), all healthy and hash-equal at the tip; a soak monitor is armed on the fleet for the next naturally occurring min-difficulty branch, watching the side_chain counter, header-vs-body frontier divergence, and strand recurrence on the zakura-only node (these branches occurred twice in the prior 4 hours, stranding the zakura-only node both times; with the gate the fleet should not follow or forward the next one). Will report the soak outcome on this PR.

Follow-up Work

  • Zakura tip/body propagation performance (zakura-only nodes fall behind at tip).
  • The duplicate-race service-session wedge (service peer map vs conn registry winner disagreement) reproduced on the block-sync service during this deploy's mass restart; it is diagnosable now via the park logs added in fix(zakura): keepalive status refresh and park visibility #472 and needs its own fix.

AI Disclosure

  • AI tools were used: Claude Code (Fable 5) for the implementation, tests, fleet validation, and this PR description, directed and reviewed by the author.

PR Checklist

  • The PR title follows conventional commits format: type(scope): description
  • The PR follows the contribution guidelines.
  • This change was discussed in an issue or with the team beforehand.
  • The solution is tested.
  • The documentation and changelogs are up to date.

@p0mvn p0mvn added the C-bug Bug fix label Jul 6, 2026
@v12-auditor

v12-auditor Bot commented Jul 6, 2026

Copy link
Copy Markdown

Note

Complete: Audit complete. V12 did not find any issues that need review.

Open the full results here.

Analyzed five files, diff fff1e47...7fe1a93.

@p0mvn
p0mvn marked this pull request as draft July 6, 2026 04:29
@p0mvn

p0mvn commented Jul 6, 2026

Copy link
Copy Markdown
Author

Fleet soak interim report: a natural one-block min-difficulty fork hit at 4145550 (0062c117…) ~15 min after deploy. The gate correctly stayed silent (side_chain=0 on all 6 nodes): the min-diff block extended every node's best chain at accept time, so it was not a side-chain commit — the gate's target scenario (a NewBlock committing to a side chain) has not occurred yet and the soak continues.

The event did surface two adjacent issues (both pre-existing, now documented for follow-up): (1) with every zakura peer's header frontier pinned on the same abandoned block, no peer advertises a higher tip, so the #476 walk-back gets no non-linking evidence — recovery needs at least one honest-frontier peer; (2) the #474 dual-stack watchdog then correctly fell all 5 dual-stack nodes back to legacy, which (being one-way) killed the fleet's zakura layer until staggered restarts, starving the zakura-only node. Reversible fallback and reanchoring the header frontier onto the verified body chain on divergence are the proposed next fixes.

…chain

An accepted NewBlock could have committed to a side chain; advancing the
header/verified frontiers and forwarding it made the whole Zakura layer
follow and propagate losing branches (e.g. testnet min-difficulty forks)
while each node's own chain stayed honest, stranding zakura-only peers.
The driver now checks the committed hash against the best chain
(ReadRequest::Depth) and routes side-chain accepts to a dedup-only event.
@p0mvn
p0mvn force-pushed the zakura/header-sync-fork-recovery branch from fff1e47 to d5f0456 Compare July 6, 2026 07:04
@p0mvn
p0mvn force-pushed the zakura/newblock-best-chain-gate branch from 7fe1a93 to 1aa8ad6 Compare July 6, 2026 07:04
@p0mvn

p0mvn commented Jul 7, 2026

Copy link
Copy Markdown
Author

Re-PR'd onto the main review stack as #500 (base zakura-linkage-verified-reads, #493) so the gate can land through ironwood-main without waiting on #476. Only the gate commit was ported — no #476 fork-recovery content rides along.

@p0mvn p0mvn closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant