fix(state): keep evicted forks recoverable

[czarcas7ic]

Motivation

At the fork limit, an accepted block can be discarded before a child extends it. Stale duplicate and header records can then prevent an evicted block from being downloaded again. Removing that block can also delete cached transaction outputs another queued block still needs, leaving its child waiting for an output that should be available.

Solution

Keep the newly accepted fork within the existing limit and clear evicted blocks from duplicate tracking and header verification records. Apply the same cleanup during reconsideration. Count the distinct blocks sharing each cached output, so eviction, rejection, and pruning preserve it until its last owner leaves. Batch eviction cleanup scans the tracking buffers once. Eviction preserves the selected full-state chain and clears obsolete ancestor failures for discarded blocks. Commit-time waiter notifications and the broader lifecycle refactor remain separate follow-up work. This PR keeps the current hash-based equal-work ordering. #1084 builds on it for first-received mining.

Testing

Regression tests cover eviction and redelivery, restart with missing backed-up bodies, reconsideration, shared outputs, and failed parents that later commit and get evicted. Both new regressions failed before their fixes. All 687 state and 289 header-chain library tests pass on Rust 1.97.1, with six ignored. All-target Clippy, formatting, and changelog validation pass. Used Codex for implementation and tests.

Follow-up Work

• Partially addresses Preserve shared cached UTXOs until all providers are removed #493. This PR fixes the SentHashes half. The QueuedBlocks half and waking AwaitUtxo waiters at commit time stay there.
• Requeue a non-finalized block whose parent disappears before it commits #1123: requeue a block whose parent disappears before it commits. This PR's eviction policy can evict an older fork while that fork's child is already waiting to be written. The child fails as Retryable and the legacy syncer restarts. On main, that child committed.
• Demote verified body markers for bodies missing from the restored backup #1124: startup keeps Verified markers for bodies lost from the backup, so operator invalidate or reconsider fails after that restart. Pre-existing.
• Keep sibling fork bodies when invalidating a shared non-finalized ancestor #1125: invalidating a shared ancestor loses sibling fork bodies. Pre-existing.
• Report non-finalized block removals from one place, in step with publication #1126: removal bookkeeping. Covers the KnownBlock race between publishing and Evicted, which is new here but still better than main, and forks dropped by finalization, which is pre-existing.

[v12-auditor]

Note

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

Open the full results here.

Finding	Severity	Details
F-283868	🟡 Medium	Shared invalidation strands sibling fork bodies Invalidating a block shared by multiple forks records removed blocks from only one selected chain, but removes every chain containing the target. Unique suffixes from the other matching forks are therefore discarded without being retained in invalidated_blocks. The new eviction-authority gate covers acceptance, reconsideration, and grow/reset events, but excludes OperatorInvalidate, so these discarded suffixes are not reported to either header-body demotion or sent-cache cleanup. A later reconsideration starts from an already-pruned state and cannot rediscover the lost hashes. Their sent hashes can continue blocking redelivery while their durable headers remain BodyValidationState::Verified, and subsequent operator changes can select a bodyless stale fork and fail the combined transition's verified-frontier check.
F-283869	🟡 Medium	Eviction corrupts shared in-flight UTXO cache NonFinalizedWriteUpdate::Evicted retires each block through SentHashes::remove, but the UTXO cache is a single map keyed by outpoint with no ownership count. Distinct sibling blocks can contain the same transaction and therefore expose the same outpoint. If a later sibling is already added to SentHashes but remains in the writer queue when an earlier sibling is evicted, removing the evicted hash unconditionally deletes the shared outpoint still needed by the in-flight block. A child of the in-flight sibling can then miss both the sent cache and committed state and register an AwaitUtxo waiter. The parent commit does not recheck pending UTXOs, so the valid child remains blocked until the six-minute UTXO lookup timeout.
F-283870	🟡 Medium	Eviction reactivates stale ancestor failures A block hash can first fail a non-finalized write and later commit successfully on retry. The failure update records that hash in non_finalized_failed_ancestors, but the later success clears only the writer-local rejected_ancestor_map. If reconsideration or another fork insertion subsequently evicts the committed block, the new Evicted handler removes its sent hash and returns without clearing the stale service-level failure record. A valid child delivered before the evicted parent is replayed then finds the obsolete verdict. Because the parent is no longer forkable after eviction, the child is synchronously rejected instead of being queued for the recoverable missing parent.
F-283871	🟡 Medium	Published eviction races duplicate-cache cleanup Successful commits and reconsiderations publish the new body-less NonFinalizedState before sending the separate NonFinalizedWriteUpdate::Evicted message. A concurrent KnownBlock request can therefore observe that the body is absent from the published state while the old hash is still present in SentHashes. Its nonblocking drain sees no update yet, captures KnownBlock::WriteChannel, and returns that stale value after the state lookup misses. The later eviction update cannot amend the already-created response. Chain discovery treats every non-None result as present, so it can skip downloading the evicted parent and request only descendants that cannot commit without it.
F-283872	🟡 Medium	Startup preserves verified markers for missing bodies Startup reconciliation treats restored full state as authority for the verified projection but does not demote durable BodyValidationState::Verified nodes whose bodies are absent from that state. This state is reachable both from a pre-upgrade database containing the original stale markers and from a crash after the combined header write but before the asynchronous non-finalized backup is updated. reconcile_verified_path resets only the projection and marks supplied path members verified; its authority returns no evicted-body list, so omitted nodes remain verified. The subsequent verify_full_state_headers check is one-way and validates only that restored bodies have matching headers. A later operator invalidation or other dirty-selection event can reselect the strongest eligible stale node, after which apply_combined_expected rejects the transition because that bodyless verified frontier differs from staged full state.

And one more auto-invalidated finding.

Analyzed eight files, diff 207d416...5783931.

[czarcas7ic]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 57839311e8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-22T12:33:02.048217Z	f39f8d8	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[evan-forbes]

We likely want to fix that v12 before merging. would be rare I think, but still. evicting a fork can delete UTXOs that an in-flight sibling block still needs.

SentHashes::known_utxos is a single map keyed by outpoint, with no owner count. Sibling blocks can include the same transaction, so they share outpoints. The new NonFinalizedWriteUpdate::Evicted path calls SentHashes::remove for each evicted hash, and remove deletes every outpoint listed under that hash, including outpoints that a later, still-queued sibling also lists.

Sequence:

1. The writer commits sibling A on a fork. The service then sends sibling B, which shares a transaction with A.
2. A later commit pushes the fork count past MAX_NON_FINALIZED_CHAIN_FORKS and evicts A. The service drains Evicted([A]) and deletes the shared outpoints.
3. B's child calls AwaitUtxo before B reaches the read snapshot. The lookup misses the sent cache and AnyChainUtxo, so the child registers a waiter.
4. Nothing re-checks pending_utxos when the writer commits B. The child waits until UTXO_LOOKUP_TIMEOUT (6 minutes).

Before this PR, evicted blocks stayed in SentHashes until prune_by_height, so eviction could not trigger this. The existing Failed path has the same weakness.

This unit test fails on 5783931:

/// Removing one sent block must keep outpoints that another sent block still exposes.
#[test]
fn sent_hashes_remove_keeps_outpoints_shared_with_sent_sibling() -> Result<()> {
    let _init_guard = zakura_test::init();

    let block: Arc<Block> =
        zakura_test::vectors::BLOCK_MAINNET_419201_BYTES.zcash_deserialize_into()?;
    let siblings = block.make_fake_siblings(2);
    let evicted = siblings[0].clone().prepare();
    let in_flight = siblings[1].clone().prepare();
    assert_ne!(evicted.hash, in_flight.hash);

    let mut sent = SentHashes::default();
    sent.add(&evicted);
    sent.add(&in_flight);

    sent.remove(&evicted.hash);

    assert!(sent.contains(&in_flight.hash));
    for outpoint in in_flight.new_outputs.keys() {
        assert!(
            sent.utxo(outpoint).is_some(),
            "removing a sibling must not drop outpoint {outpoint:?} of a sent block"
        );
    }

    Ok(())
}

Message:  removing a sibling must not drop outpoint OutPoint { hash: transaction::Hash("94cec31b…"), index: 60 } of a sent block
Location: crates/zakura-state/src/service/queued_blocks/tests/vectors.rs:266

we might be able to get by with a hackier solution, but this suggestion seems reasonable, although not all of it needs to be handled here

Step 1: Split SentHashes by concern.
- SentBlocks tracks membership by hash. Eviction and rejection remove hashes from it. This is all #1104 actually needs, so that evicted blocks can be downloaded again.
- InFlightUtxos maps each outpoint to its UTXO and the set of hashes that own it. An outpoint leaves the map only when its last owner leaves.

Eviction then no longer needs to touch UTXOs at all. Keeping an evicted block's outputs a little longer is harmless: before this PR they stayed until pruning, and contextual validation already rejects spends that aren't on the chain.

Step 2: Make waiter wakeup follow data availability, not submission. The writer already reports back over the channel this PR adds. Extend it into one lifecycle stream:

enum BlockLifecycle {
    Committed { hash, new_outputs },
    Rejected(NonFinalizedWriteFailure),
    Evicted(Vec<block::Hash>),
    Finalized(block::Height),
}

When the service handles Committed, it calls pending_utxos.check_against_ordered(&new_outputs). A waiter that missed the cache then wakes as soon as its UTXO becomes readable. The cache goes back to being an optimization, and the 6-minute stall is impossible whatever the cache does.

Step 3: Put one owner behind the lifecycle. An InFlightBlocks component consumes BlockLifecycle and owns membership, the UTXO cache and waiter wakeups. The service handlers then call in_flight.sent(block) and in_flight.apply(event) instead of updating three structures. Adding a new writer outcome means handling one event in one place.

[evan-forbes]

pre-approving pending either fixes here or flups

[czarcas7ic]

@v12-auditor review

[v12-auditor]

Note

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

Open the full results here.

Finding	Severity	Details
F-284974	🟡 Medium	Stale failure marker rejects valid descendants A retryable write failure records the block hash in non_finalized_failed_ancestors, but the service-facing NonFinalizedWriteUpdate channel has no successful-commit variant that can clear this attempt-level failure. A same-hash retry can commit because admission checks the failure map using the block's parent, while the successful writer branch clears only its private rejection map. If fork pressure later evicts the successfully retried block before a child arrives, the Evicted handler removes its sent-cache membership but leaves the old failure marker intact. A subsequently delivered valid child sees its parent as failed and non-forkable, so it is immediately rejected and recorded as another failed descendant. The fork becomes recoverable only if its parent is redelivered before the child, the bounded marker expires, or the service restarts.
F-284975	🟡 Medium	Shared queued output disappears across cache transition QueuedBlocks still stores one cache value per outpoint, so sibling blocks on different unavailable parents can overwrite the same shared output without recording ownership. When one sibling becomes ready, dequeue_children unconditionally deletes all of its outpoints from the queued cache even though the other sibling remains queued; the ready sibling temporarily becomes the only SentHashes owner. If that sent sibling is subsequently rejected or evicted, the new reference-counted sent cache releases the output because it does not count queued owners, leaving neither cache able to answer a lookup. A child of the still-queued sibling can continue into transaction verification, register AwaitUtxo, miss both caches and the chain snapshot, and then wait only on its pending channel. Releasing or committing the queued sibling later does not recheck that waiter, so the lookup expires after the six-minute verifier timeout.
F-284976	🟡 Medium	Finalization leaves dropped forks in sent cache The writer computes and reports fork-limit evictions immediately after a successful block commit, before entering the contextual-finalization loop. NonFinalizedState::finalize can then discard entire side chains whose root differs from the newly finalized best-chain root, but the writer republishes the reduced state without sending a second Evicted update. Descendants of the discarded side fork that are above the new finalized height remain in SentHashes, because the fallback height prune cannot remove them yet and runs only from a later block-submission path. Those stale hashes continue to authorize fork admission and their outputs continue to satisfy AwaitUtxo, even though no live chain contains them. Actual contextual validation eventually rejects dependent blocks, but an attacker can repeatedly drive semantic verification and serialized writer work against dead-fork parents until later finality or restart clears the cache.
F-284977	🔵 Low	Nonmonotonic batches defeat sent-cache pruning SentHashes::prune_by_height requires every batch to be in monotonically increasing height order and stops scanning a batch at the first entry above the bound. The service's supposed breadth-first queue drain actually uses a LIFO Vec, so two multi-level fork trees can be appended in an order such as h, h, h+1, h+2, h+1; startup likewise flattens complete chains into one batch, resetting height when iteration moves to another fork. In either case, an above-bound entry can hide later entries that are already finalized and prunable. Their hash membership and UTXO owner references remain live until the finalized bound reaches the preceding higher entry. The stale entries can satisfy AwaitUtxo and can_fork_chain_at, causing avoidable verification and writer work, although actual-chain checks prevent invalid state commitment.
F-284978	🟡 Medium	Recovery preserves verified markers for missing bodies Header body markers are committed synchronously, while the default non-finalized body backup may lag by up to five seconds. A crash after accepting a side branch or extending the winning branch can therefore leave durable headers marked Verified whose bodies are absent from the restored NonFinalizedState. Startup reconciliation derives its authoritative path and side paths only from that restored state, but its authority supplies no evicted_bodies; it either returns immediately when the winning projection already matches or performs a VerifiedChainChanged::Reset that changes the projection without demoting omitted nodes. The final coherence check verifies only that restored full-state headers exist in the DAG, not that every durable Verified node has a restored body. Missing bodies can consequently remain selectable as fully verified, and transient body-unavailable evidence is rejected while the stale marker remains Verified.
F-284979	🔵 Low	Invalidation leaves removed sibling headers verified Invalidating a shared non-finalized ancestor removes every live chain containing that ancestor, but stores invalidated bodies from only the single strongest matching chain. Unique bodies from the other removed siblings are therefore destroyed immediately. The combined header transition is OperatorInvalidate, yet PreparedFullStateTransition::commit populates evicted_bodies only for acceptance, reconsideration, and Grow/Reset chain changes, so the removed sibling headers remain marked Verified. Reconsideration restores only the one recorded branch and makes the stale sibling header path eligible again despite its missing bodies. A later selective invalidation that shortens the restored branch can then make the bodyless sibling the strongest verified header path, causing the staged full-state frontier and header frontier to mismatch and abort the operator transition. No inconsistent state is published because the mismatch check precedes the write, but the operator action remains uncommittable until the missing sibling body is restored or pruned.

And one more auto-invalidated finding.

Analyzed nine files, diff d4997d9...2c5ede5.

[p0mvn]

LGTM.

Some outstanding v12s might be good to address still but non-blocking