fix(zakura): only advance and gossip NewBlocks that land on the best chain#500
Merged
evan-forbes merged 18 commits intoJul 7, 2026
Merged
Conversation
Close the three write-path corruption bugs proven by the Phase-1 header-store coherence suite (PR #490): 1. prepare_header_range_batch_with_roots now rejects ranges whose first header does not link to the anchor or whose headers are not internally contiguous (new CommitHeaderRangeError::UnlinkedRange, classified as a local non-scoring failure in zebrad). 2. The range insert loop skips heights that already have a committed block, so re-deliveries can no longer strand provisional zakura rows below the body tip. 3. prepare_zakura_header_from_committed_block refuses seeds that do not link to the stored row below them as silent no-ops; header-range sync converges the store instead (scenario s11). Flip the four *_upholds_invariants twins to permanent regression gates, delete the corruption_repro_* tests, remove the discovery masking, and un-ignore the random invariant sweep (clean at PROPTEST_CASES=4096).
Pillar 2 of the REORG_PLAN: a corrupted header store can no longer poison difficulty validation — readers surface the storage fault explicitly. - recent_header_context verifies, at every step of its walk, that the stored header is the block its hash row names (HeaderHashMismatch), that it links to the row below (BrokenLinkage), and that no row is missing below a stored one (Gap), returning the new StoreIncoherentError instead of a poisoned or silently shortened difficulty window. - The range writer's anchor round-trip distinguishes a bijection violation in our own indexes (StoreIncoherent::BijectionMismatch) from a genuinely unknown anchor (UnknownAnchor). - CommitHeaderRangeError::StoreIncoherent is classified as a local, non-peer-scoring commit failure in zebrad: the range was rejected because our store cannot supply trustworthy context, not because the peer's range was shown invalid. New reads.rs suite in header_store_coherence hand-corrupts the column families and pins the reader/writer behavior for all four fault shapes, including side-effect freedom of the rejections.
…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.
|
Note Complete: Audit complete. V12 did not find any issues that need review. Open the full results here. Analyzed five files, diff |
6 tasks
…t-chain-gate-ironwood
p0mvn
marked this pull request as ready for review
July 7, 2026 06:09
# Conflicts: # CHANGELOG.md # zebra-state/src/service/finalized_state/zebra_db/block/tests/header_store_coherence/README.md # zebrad/src/commands/start.rs # zebrad/src/commands/start/zakura/header_sync_driver.rs
evan-forbes
approved these changes
Jul 7, 2026
evan-forbes
left a comment
evan-forbes
left a comment
There was a problem hiding this comment.
nice!! makes a lot sense to do what block prop does here and not gossip
side node: I've never liked having the "drivers" for headersync or blocksync just due keeping important logic or failures in different spots. would love cleaning separation there. naively seems like we could do so with better api's to state/storage
we might be able to have a header_chain api for best_tip etc like we do for blocks as well
evan-forbes
changed the base branch from
zakura-linkage-verified-reads
to
ironwood-main
July 7, 2026 11:27
evan-forbes
force-pushed
the
zakura/newblock-best-chain-gate-ironwood
branch
from
July 7, 2026 11:33
a24b630 to
c31fc4f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Original motivation (see #478 for the live incident detail): an inbound Zakura
NewBlockaccepted by the block pipeline can land outside the best chain (contextually valid, lower work). The driver still advanced the header/verified frontiers and forwarded the block to peers without checking which chain it landed on — observed live as the whole fleet advertising and re-forwarding a min-difficulty losing branch over Zakura while each node's own chain stayed honest, stranding the zakura-only node that trusted the gossip.Solution
After the verifier accepts a
NewBlock, the driver checks the committed hash against the best chain withReadRequest::Depth(Someonly for best-chain blocks, so it also covers a reorg the block just won):Best chain → unchanged:
NewBlockAcceptedadvances the frontiers and forwards to eligible peers.Non-best chain →
HeaderSyncEvent::NewBlockAcceptedNonBestChain: dedup-only — no frontier advance, no height-covered marking, no forwarding. Counted insync.header.tip.new_block.non_best_chain.Read failures are conservatively treated as non-best-chain: the node's own frontier still advances through the chain-tip mirror, so a false negative only skips one gossip forward.