Skip to content

test(zakura): restore the dropped test suite and cover the reorg stack end-to-end#498

Draft
p0mvn wants to merge 27 commits into
zakura/writeguards-canary-v3from
zakura-fleet-test-restore
Draft

test(zakura): restore the dropped test suite and cover the reorg stack end-to-end#498
p0mvn wants to merge 27 commits into
zakura/writeguards-canary-v3from
zakura-fleet-test-restore

Conversation

@p0mvn

@p0mvn p0mvn commented Jul 7, 2026

Copy link
Copy Markdown

Motivation

The reorg fix stack (#490#496) shipped with strong store-level coverage but three integration gaps, one of which the Pillar-1a crash table explicitly commits to: no automated kill-between-batches test, no end-to-end reorging CommitHeaderRange through the write worker, and no reactor-layer reorg scenarios. Worse, the canary cherry-picks dropped every test file, so the entire header_store_coherence harness did not exist on the fleet lineage where the 1a switch orchestration lives.

Stacks on #496 (zakura/writeguards-canary-v3).

Solution

Three commits, reviewable in order:

  1. Harness restore — cherry-picks the test content of test(state): add zakura header-store coherence harness with corruption repros #490/fix(state): verify zakura header-store invariants in consensus reads #493/fix(state): audit and self-repair the zakura header store at startup #494/fix(state): own zakura header-store mutations with a single suffix-replacement primitive #495 onto the fleet lineage (~57 tests: coherence suite, shared block-test helpers, proptest regression seeds). The only adaptation is the fleet range-writer API: helpers unwrap .tip_hash from HeaderRangeCommitOutcome.

  2. Switch orchestration end-to-end (service/write/switch_orchestration_tests.rs) — drives reorging header-range commits through the real worker function against a real header store and non-finalized state:

    • reorging commit: store switches, stranded body suffix rolls back to the fork, truncated chain published, response reports the reorg; post-reorg audit asserted clean, and a planted-stray-row variant asserts the audit hook repairs and emits the metric;
    • kill-between-batches (the crash-table commitment): body rollback ran, prepared batch dropped unwritten — the intermediate is coherent, a reopen passes the startup audit with no repair, and re-running the same commit converges (recovery is the ordinary flow);
    • whole-suffix switch: a reorg at the nf root empties the chain set without panicking and publishes the emptied state;
    • losing-then-winning branch: lower-work conflict rejected with byte-identical stores, higher-work extension switches, the original branch switches back, and its rolled-back bodies recommit through the full validation path — rollback-not-ban end to end.

    Fixture design (spike results recorded in the test module docs): fabricated bodies need V4 coinbases, and the tests run on a network whose upgrades activate above the test heights, so nf commits take the pre-Heartwood regime like the existing fake-mainnet fixtures.

  3. Reactor and driver scenarios — following the fix(zakura): recover header sync from a frontier stranded on an abandoned branch #476 walk-back test patterns:

    • lower-work conflict at the peer layer: the serving peer is never scored or disconnected, keeps receiving requests, and a later successful commit completes the switch (plus the zebrad classification test pinning LowerWorkConflict → Local);
    • the feat(state): sequence branch switches as evaluate, body rollback, then header rewrite #496 flow at the driver: a CommittedHeaderRange outcome with reorged_at set must not drive any follow-up state request — the mock state panics on anything but the commit itself;
    • store-incoherence walk-back to convergence: ContextMismatch failures from independent peers start a walk-back with no one scored, the frontier re-anchors from the store's reanchor target, and the re-delivered range re-commits through the fork point.

Tests

This PR is tests only (no production code changes beyond pub(crate) visibility on test-module paths and a doc comment). All gates green locally:

  • cargo test -p zebra-state --lib (352 passed), cargo test -p zebra-network --lib (901 passed), cargo test -p zebrad --lib -- zakura_header_sync_driver_tests (45 passed)
  • cargo fmt --all -- --check, cargo clippy -p zebra-state -p zebra-network -p zebrad --all-targets -- -D warnings
  • PROPTEST_CASES=1024 cargo test -p zebra-state --release --lib prop_random_sequences_uphold_invariants (discovery sweep)

Follow-up Work

AI Disclosure

  • AI tools were used: Claude Code wrote the tests and fixtures under review; the author reviewed every line and ran all gates locally.

PR Checklist

  • The PR title follows conventional commits format: type(scope): description
  • The solution is tested.

p0mvn added 27 commits July 6, 2026 00:57
…oned branch

On repeated first_header_does_not_link rejections from independent peers,
walk the request anchor back below the verified block tip (exponentially
deeper per round, floored at the finalized height) until responses link,
commit the higher-work range through the fork point, and invalidate the
stranded committed body suffix so block sync re-downloads the new branch.
Non-linking forward responses no longer score peers.
…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.
… back to legacy sync

Killing the reactors turned every fallback into a fleet-wide Zakura outage:
the fallback node is often the only peer with working block ingest, and with
its reactors dead it stops advertising, serving, and forwarding, pinning
every Zakura peer's frontier. Legacy ChainSync now resumes as the body-sync
driver while Zakura quiesces into a serving/advertising bridge. Also makes
inbound block-gossip ingest failures and queue drops visible at info with
counters, so tip-ingest freezes are attributable from logs.
…truncated chain tip

Invalidating a chain suffix (Zakura fork recovery) can leave last_fork_height
above the new tip; recent_fork_length deliberately returns None for that, but
the progress-bar metrics caller expected Some and panicked the block write
task, crash-looping the node. Metrics display code must never panic.
…ecovery

If the node exits between a durable header reorg and its body invalidation,
later header commits find the rows already replaced and report no conflict,
so the event-based invalidation never fires again and the body chain stays
stranded on the abandoned branch forever. Committed ranges overlapping the
body tip are now compared against the best body chain, and the first
stranded body block is invalidated (one extra state read per commit only
when the range overlaps committed bodies).
…rives commits

Two sync engines submitting bulk commits concurrently race in the applying
queue, so the fallback is now a commit barrier again — without killing the
reactors: an apply gate stops new Zakura block applies once fallback engages
and the watchdog waits for in-flight applies to finish (bounded; each apply
has its own driver timeout) before legacy ChainSync takes the pipeline.
Serving, statuses, header commits, and NewBlock accepts continue unaffected.
…h-up

Fallback was permanent per process lifetime: bootstrap_genesis_then_pause
exited into the legacy sync loop and nothing ever handed the driver role
back, so every min-difficulty burst demoted one more node's Zakura sync to
a serve-only bridge until restart. The watchdog is now a cycle: on fallback
it yields+drains the apply gate, legacy drives until caught up and stable
(three consecutive exhausted sync rounds advancing the tip by at most two
blocks), then the gate is returned to Zakura and the watchdog resumes.
Hand-back happens between legacy rounds, so no legacy bulk work is in
flight when Zakura resumes driving.
…dlines

When the download floor passed a request (other peers delivered its heights),
the GC removed it but only disarmed the liveness deadline for peers whose
last delivery was newer than their last request. A healthy peer with an older
delivery kept a deadline it could no longer answer, was disconnected for 'no
accepted block progress' with zero outstanding, and parked in the 180s
no-progress cooldown — at tip this serially exiled peers until the block-sync
peer set collapsed (reproduced live 5→0 after a fleet-wide restart, wedging
body sync at the next gap). Heights satisfied below the floor now always
clear an idle peer's deadline; unresponsive peers are still caught by request
timeouts, which deliberately keep the deadline armed.
The mirror maxed a TipAction::Reset tip against the latest_chain_tip watch,
which may not have observed the reset yet; the stale higher tip turned the
downstream VerifiedReset into a no-op, pinning the block-sync sequencer one
block above the real tip after a fork-recovery invalidation — the missing
parent was never requested and every body commit timed out until restart.
On Reset the action's tip is authoritative; anti-regression maxing now
applies only to Grow.
A ~45-minute uniform commit-stall (every Zakura block-sync commit timing
out while the state write task, runtime workers, and parent-wait queue were
all idle) could not be attributed from logs: the driver's timeout says only
that the verifier did not answer. Log the two stages that can silently hold
a block — the transaction-verification drain (UTXO awaits on out-of-order
parents serialize here, bounded by the 6-minute UTXO_LOOKUP_TIMEOUT) and
the state commit await (parent-waiting queue / write-task backlog) — at
warn when they exceed 20s, and log timed-out and slow (>20s) transparent
UTXO lookups with their outpoint.
…t stalls

'No accepted block progress' cannot be blamed on a peer while the node's own
apply pipeline is not landing commits: during a ~45-minute verifier-layer
commit stall the liveness deadline exiled healthy peers one by one into the
180s no-progress cooldown (observed live: parks +4 on t2, t4 down to 2
peers). When the sequencer view shows bodies in the applying queue, extend
the deadline instead of disconnecting; a genuinely dead peer is caught as
soon as the pipeline drains, and its timed-out requests were already
requeued by expire_due_timeouts.
…lures

InvalidPeerRange rejections score the peer, but the CommitHeaderRangeError
variant behind them was only visible at debug level: a stranded-context
wedge (every peer's response for the same range rejected identically for
hours, observed live at height 4148005) was indistinguishable from real
peer misbehavior at default log levels.
…of scoring peers

A fork-recovery re-commit can leave stale rows in the stored difficulty
window; every extension range is then rejected with
InvalidDifficultyThreshold, each rejection scored and disconnected the
honest serving peer, and the wedge survived restarts (on-disk rows).
No link failure ever fires in this state, so the existing walk-back
trigger never engages. Classify ValidateContextError commit failures as
ContextMismatch: never score the peer, and count them as stale-anchor
evidence — the quorum (3 failures, 2 distinct peers) starts the same
exponential walk-back, and each deepening re-commits a longer range that
rewrites the poisoned window.
…Mismatch

UnknownAnchor cannot be the peer's fault: the anchor is this node's own
request bookkeeping, and the error fires when the reactor's in-memory
frontier references a hash the header store does not have (observed live
at 4148376 after the frontier advanced over diverged store rows). It was
classified InvalidPeerRange, so honest peers were scored and disconnected
and no recovery trigger ever fired. As ContextMismatch it feeds the
stale-anchor quorum; the walk-back re-anchors from the store, which is
exactly the self-correction this state needs.
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.
…e canary picks

Cherry-picks the test content of #490/#493/#494/#495 onto the fleet
lineage: the header_store_coherence suite (fabricate/oracle/ops/audit/
scenarios/prop/reads/startup_audit/primitive), the shared block-test
helpers, and the proptest regression seeds. The only adaptation is the
fleet range-writer API: prepare_header_range_batch returns
HeaderRangeCommitOutcome here, so the two helpers that returned the tip
hash now unwrap .tip_hash.
…he switch orchestration

Adds the T1 suite from INTEGRATION_TEST_PLAN.md: request → body rollback
→ header rewrite → response through the real write-worker function,
against a real header store and non-finalized state.

- T1.1 reorging commit: store switches, stranded suffix rolls back to
  the fork, truncated chain published, response reports the reorg, and
  the post-reorg audit runs (clean run asserts no repair metric; a
  planted stray row asserts the hook repairs and emits it).
- T1.2 kill-between-batches (the REORG_PLAN §2 crash-table commitment):
  rollback ran, prepared batch dropped unwritten — the intermediate is
  coherent, a reopen passes the startup audit with no repair, and
  re-running the same commit converges (recovery is the ordinary flow).
- T1.3 whole-suffix switch: a reorg at the nf root empties the chain
  set without panicking and publishes the emptied state.
- T1.4 losing-then-winning branch: lower-work conflict rejected with
  byte-identical stores, higher-work extension switches, the original
  branch switches back, and its rolled-back bodies recommit through the
  full validation path — rollback-not-ban end to end.

Fixture design per the spike recorded in the plan: fabricated bodies
need V4 coinbases (fabricate_v4_body) and a network whose upgrades
activate above the test heights (pre_heartwood_test_network), so nf
commits run in the pre-Heartwood regime like the existing fake-mainnet
fixtures. CounterCapture moves to the shared test helpers; the block
test modules become pub(crate) so the write tests can reuse the
coherence fixtures.
Adds the T3 suite from INTEGRATION_TEST_PLAN.md (Phase-5 Layer 2),
following the walk-back reactor test patterns:

- T3.1 lower-work conflict at the peer layer: a served range rejected
  as a lower-work fork reports the non-scoring Local kind — the peer is
  never scored or disconnected, keeps receiving requests, and a later
  successful commit completes the switch. Plus the zebrad
  classification test pinning LowerWorkConflict → Local.
- T3.2 the #496 flow at the driver: a CommittedHeaderRange outcome
  with reorged_at set must not drive any follow-up state request (the
  state's switch orchestration already rolled the stranded suffix
  back) — the mock state panics on anything but the commit itself, and
  the frontier still advances through the ordinary success path.
- T3.3 store-incoherence walk-back to convergence: ContextMismatch
  failures from independent peers on every commit start a walk-back
  with no one scored, the frontier re-anchors below the verified tip
  from the store's reanchor target, and the re-delivered range
  re-commits through the fork point. (StoreIncoherent → ContextMismatch
  classification and the store-side repair are covered by the existing
  zebrad and startup-audit tests.)
@v12-auditor

v12-auditor Bot commented Jul 7, 2026

Copy link
Copy Markdown

Note

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

Open the full results here.

FindingSeverityDetails
F-96658 🟠 High
Root rollback leaves stale forks

A higher-work Zakura header range can reorg the stored header chain at the non-finalized root while the non-finalized state contains multiple forks sharing that old root. commit_header_range() invokes invalidate_stranded_body_suffix() before writing the header suffix, but that rollback is keyed only by the best-chain hash at reorged_at. When the hash is the non-finalized root, remove_block_and_descendants() removes only the selected chain whose tip matches chain_tip_hash; other chains that contain the same old root hash remain in chain_set. The rollback publisher then accepts any remaining best chain, and the header batch is written to the new branch. This leaves the durable header store naming reorged_to_hash at the reorg height while a surviving non-finalized body chain can still contain old_hash at that same height.

F-96659 🟠 High
Empty rollback preserves stale tip

When a header reorg rolls back the entire non-finalized body suffix, invalidate_stranded_body_suffix() sends the emptied NonFinalizedState watch value but deliberately does not update ChainTipSender. ChainTipSender permanently switches to non-finalized tips after the first non-None tip and ignores later None values. The LatestChainTip mirror can therefore remain on the pre-reorg body tip even though the non-finalized state is empty. Zakura block-sync frontier discovery merges the read-state tip with this stale latest tip and selects the higher height. Missing-body discovery then begins after that stale height, skipping the new branch bodies at and below it.

F-96661 🟠 High
Context errors force reanchors

A peer-supplied Headers range receives only context-free checks in the reactor before it is sent to state. State contextual checks can reject the same range for consensus-context failures such as median-time or expected-difficulty mismatch. The production driver maps every CommitHeaderRangeError::ValidateContextError to HeaderSyncCommitFailureKind::ContextMismatch. The reactor handles ContextMismatch by recording stale-anchor evidence and never reporting peer misbehavior. The stale-anchor counter is global and is not keyed by range, anchor, response hash, or specific validation error, so Sybil peers can turn invalid contextual data into unscored reanchor or walk-back signals.

F-96662 🟠 High
Lower-work retries duplicate ranges

The production driver classifies CommitHeaderRangeError::LowerWorkConflict as HeaderSyncCommitFailureKind::Local. The reactor reports misbehavior only for InvalidPeerRange; for Local, it clears the range assignment and retries. RangeScheduler::retry blindly pushes the same RangeRequest to the front of the queue without deduplication. Because pop_assignable leaves a queued range in place until the fanout threshold is filled, sub-fanout connectivity lets the original queued item remain while each Local failure adds another copy. The same lower-work peer becomes eligible again after assignment clearing, so it can repeatedly receive and fail the same range.

And one more auto-invalidated finding.

Analyzed seven files, diff c4345ff...65d0f41.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant