Skip to content

feat(prover-node): make epoch proving robust to prune-induced fork faults#24678

Open
PhilWindle wants to merge 16 commits into
merge-train/spartan-v5from
phil/a-1418-prover-node-make-epoch-proving-robust-to-prune-induced-fork
Open

feat(prover-node): make epoch proving robust to prune-induced fork faults#24678
PhilWindle wants to merge 16 commits into
merge-train/spartan-v5from
phil/a-1418-prover-node-make-epoch-proving-robust-to-prune-induced-fork

Conversation

@PhilWindle

@PhilWindle PhilWindle commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #24436 (A-1290). That PR fixed the immediate prune-induced failure but left a class of race conditions around L1-reorg prunes: every "was this failure a prune or a genuine failure?" predicate samples control-plane state that lags the data-plane world-state unwind, so it is racy by construction.

The fix pushes the fact down to where it is unambiguous — decouple "a checkpoint prover failed" (a fact about the prover) from "the epoch failed" (a decision), and never treat a cancel (prune / reap / shutdown) as a failure. A genuine failure is then told apart from a prune with no control-plane sampling.

What changes

  • CheckpointProver — gains isFailed(), set when its block proofs reject for a non-cancel reason (a sub-tree fault or a prune-induced fork fault). On failure it fires an onFailed callback so the owner can upload that single checkpoint's post-mortem. A cancel leaves isFailed() false.
  • EpochSession — a fault now settles in one of two terminal states, decided by the provers:
    • stopped — a CheckpointProver under it failed. May be a prune, so it is not uploaded and the session is dropped; the epoch recovers when a prune/re-add installs a fresh prover.
    • failed (hasFailed()) — the session's own top-tree/submit work failed while every prover was healthy. Healthy provers rule out a prune, so this is a genuine, race-free failure.
  • SessionManager — removed lastTickEpoch. Two mechanisms stop a doomed epoch being re-proved every tick: openFullSessionIfReady refuses to build over a set containing a failed prover, and recreateInvalidSessions retains a genuinely-failed full session as a do-not-re-prove marker, replacing it only when its canonical content changes (a re-add). A failed session fires onSessionFailed (session-level upload). Recovery from prune/reorg flows through the ungated checkpoint/prune triggers, reusing already-completed sub-proofs from the content-addressed broker.
  • ProverNode — uploads post-mortems at two levels: per checkpoint (tryUploadCheckpointFailure, wired to CheckpointProver.onFailed — fires for prune-induced faults too, on purpose) and per session (tryUploadEpochFailure, wired to onSessionFailed). expireEpoch only releases the chonk cache and reaps — it does not upload, since a missed-window epoch's provers may already be pruned by then.
  • Re-proving — add rerunCheckpointProvingJob to re-prove a single downloaded checkpoint in isolation (sub-tree only; no epoch top-tree or L1 submit), sharing offline setup with rerunEpochProvingJob. downloadEpochProvingJob is unchanged (same serialized format).

Tests

  • epoch-session.test.ts — a fault ends stopped (a prover failed) vs failed (top-tree/submit with healthy provers).
  • checkpoint-prover.test.ts — a mid-proof fork fault sets isFailed() and fires onFailed once; a cancel does neither.
  • session-manager.test.ts — skips building over a failed prover; retains + uploads a failed session and replaces it on content change; does not upload on stopped; data-plane fault + re-add (identical and different content) recovers and completes.
  • prover-node.test.ts — a registered checkpoint prover failure routes to tryUploadCheckpointFailure; expireEpoch reaps without uploading.
  • upload_failed_proof.test.ts (e2e) — the existing epoch upload/rerun test, plus a new checkpoint test: force a sub-tree failure via the checkpointProveOverride hook, capture the per-checkpoint upload, and re-prove it via rerunCheckpointProvingJob.
  • prover-node README rewritten to the failed-prover / two-level-upload model, with the "how a genuine failure is told apart from a prune" rationale.

prover-node unit suite green; full yarn build; format + lint clean.

Resolves A-1418.

…bmission-window expiry

Decouple "a checkpoint prover failed" (a fact) from "the epoch failed" (a
decision). A proving or L1-submission fault now settles the EpochSession in the
non-declaring terminal 'stopped' instead of 'failed'; the reconciler rebuilds the
epoch over current canonical content each tick (retry-to-converge), cheap because
the broker reuses already-completed sub-proofs. An epoch is declared terminally
failed — with its post-mortem upload — only when its L1 proof-submission window
closes with the proven tip settled and the epoch still unproven, from
ProverNode.expireEpoch. This removes the racy, lagging-replica "was this a prune?"
classification entirely.

Deletes lastTickEpoch (the epoch-keyed anti-retry gate), the checkpointsMatch
upload-suppression in SessionManager.runSession, and the onSessionFailed callback.
The post-mortem upload moves to tryUploadEpochFailure(epoch, checkpoints), built
from the store's last-known canonical provers.
@PhilWindle PhilWindle added the ci-draft Run CI on draft PRs. label Jul 14, 2026
…fault path directly

Address review feedback on the retry-to-converge change:

- checkEpochExpiry was called from both handleBlockStreamEvent and the periodic
  ticker, so two sweeps could interleave and both upload a post-mortem for the same
  epoch before either advanced lastExpiredEpoch. Drop the inline block-stream call:
  the ticker (a RunningPromise, which never overlaps its own runs) is now the sole
  driver, so the high-water mark advances — and each epoch uploads — exactly once.
  Expiry is a background sweep keyed off the archiver's synced slot; it never needed
  to be on the event path. The A-1041 tips-unadvanced guard now covers only the
  registration/prune handling that genuinely needs it.

- Add a checkpoint-prover test for the actual data-plane race: dbProvider.fork
  rejecting mid-proof rejects whenBlockProofsReady(), which the EpochSession maps to
  'stopped'. Point the expiry unit tests at checkEpochExpiry directly rather than
  through a block-stream event.
The expiry sweep no longer runs from handleBlockStreamEvent. Rename "Per-event
expiry sweep" to "Periodic expiry sweep", redraw the diagram around the expiryTicker
(RunningPromise) as the sole driver, and fix the prose: the high-water mark advances
per sweep (not per event) and is seeded from resolveLastFullyProvenEpoch. Drop the
stale getCheckpointsData and computeStartupState references (expireEpoch uses
getBlocks; there is no computeStartupState).
… re-proved every tick

Retry-to-converge was naively per-tick: for an epoch that keeps failing, every tick
cleared the stopped session and re-created a fresh one, re-running proving work until
the deadline for no benefit. Key the retry off content instead, per the original
design: record the content key of a full session that ends in 'stopped', and have the
tick skip an epoch whose current canonical content matches an already-failed attempt.

Recovery is unaffected — it flows through the ungated checkpoint/prune triggers, which
fire on a genuine change (a re-add or reorg, including an identical-content re-add whose
world-state has resettled) and reopen the epoch regardless. The gate resets when the
epoch is proven, expires, or the proven frontier passes it.

Only full sessions are affected: partials are opened solely by an explicit startProof
and are never reopened by the tick or by events, so they never entered the re-spin loop.
…er mark

Replace the content-keyed retry gate (a per-epoch content-key map plus two helpers and
record/clear bookkeeping) with the monotonic lastTickEpoch high-water mark: the tick
opens an epoch once and does not re-create a session for it every tick. Recovery from a
genuine change still flows through the ungated checkpoint/prune triggers, so a
pruned-then-re-added epoch recovers exactly as before. The tradeoff — a transient failure
on an already-complete epoch waits for the deadline rather than being auto-retried by the
tick — is unchanged from the content-keyed version, at a fraction of the machinery.
…nt prover

Replace the lastTickEpoch high-water mark with a check at the point of construction:
a CheckpointProver whose block proofs rejected for a non-cancel reason (a sub-tree
fault or a prune-induced fork fault) now records isFailed(), and the SessionManager
refuses to open (or rebuild) an EpochSession over any set that contains a failed
prover. A stuck epoch is therefore skipped cheaply each tick — no session, no
re-proving — rather than being gated by per-epoch bookkeeping.

This keeps the resiliency and drops the tick gate: a pruned/re-added epoch recovers
because the re-add installs a fresh (non-failed) prover, and a session that stops with
healthy provers (a transient top-tree/submit error) is still retried by the next tick.
The failure lives on the prover, where it happened, with the store as the single source
of truth.
…upload eagerly, not at expiry

The expiry-time post-mortem upload could never fire for a persistently-failing epoch:
by the time its window closes, its checkpoint provers have been pruned, so there was
nothing to upload (the upload_failed_proof e2e test hung as a result).

Give EpochSession a genuine-failure state, told apart by the checkpoint provers'
isFailed() flag:
  - a fault while a prover under it failed → 'stopped' (maybe a prune): not uploaded,
    not retried over the failed prover, recovered on re-add.
  - the session's own top-tree/submit work failed while every prover was healthy →
    'failed' (hasFailed()): definitively not a prune, so it is race-free. The reconciler
    retains such a full session (so the tick doesn't re-prove a deterministic failure)
    and uploads a post-mortem once, eagerly, from the session's checkpoints.

Removes the fail-at-expiry upload (expireEpoch is back to chonk-release + reap only);
reinstates the onSessionFailed → tryUploadEpochFailure wiring on the genuine-failure
path. Reverts the e2e test to warp-to-epoch-1 + the eager upload trigger.
…st per failed session

A checkpoint prover that fails to produce its block proofs (a sub-tree fault or a
prune-induced fork fault) now fires an onFailed callback, and ProverNode uploads a
snapshot for that single checkpoint via tryUploadCheckpointFailure. This captures a
genuine checkpoint proving failure that ends its session in 'stopped' — which the
session-level upload (only on a session's own 'failed') deliberately does not cover.

The checkpoint upload fires for prune-induced faults too, on purpose: a prune-caused
checkpoint snapshot is harmless, and not trying to tell prune from genuine failure is
what keeps it race-free. A cancelled prover (control-plane prune / shutdown) is not a
failure and does not upload.
…kpoint upload

Add rerunCheckpointProvingJob: reuses the epoch rerun's offline setup (world state +
archiver snapshot, local broker/prover, replaying tx provider) but rebuilds just the one
checkpoint's sub-tree prover and awaits its block proofs — no epoch top-tree or L1
submit. Extract the shared setup into createRerunContext / buildCheckpointProver.

Add a test-only checkpointProveOverride hook (CheckpointProverDeps → CheckpointStore
setTestHooks → ProverNode.setCheckpointHooks) so a test can force a sub-tree failure,
mirroring the existing session topTreeProveOverride hook.

Extend upload_failed_proof.e2e with a second test: force a checkpoint sub-tree failure,
capture the eager per-checkpoint upload URL via tryUploadCheckpointFailure, download, and
re-prove that single checkpoint with rerunCheckpointProvingJob.
…cle flags; drop public isCompleted

completed/failed/cancelled are three orthogonal facts, not a single status — a prover can
be completed+cancelled (routine teardown) or completed+failed (enqueued then the sub-tree
faulted); only failed+cancelled is excluded. Add a comment explaining why they aren't one
enum, with per-field docs.

isCompleted() had no callers outside tests (internally the `completed` field is used
directly), so remove the public getter and the two secondary test assertions that used it.
@PhilWindle
PhilWindle marked this pull request as ready for review July 16, 2026 13:21
…ke-epoch-proving-robust-to-prune-induced-fork
@PhilWindle PhilWindle changed the title feat(prover-node): retry-to-converge, declare epoch failed only at submission-window expiry feat(prover-node): make epoch proving robust to prune-induced fork faults Jul 16, 2026
…l/a-1418-prover-node-make-epoch-proving-robust-to-prune-induced-fork

@spalladino spalladino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good to me, though agents pointed out a possible issue. I think it's correct in the analysis, but I'm not completely sure.

Comment on lines +254 to +260
// A full session that failed on its own account is retained as a "do not re-prove" marker while
// its content is unchanged — this is what stops the tick re-proving a deterministically-failing
// epoch. When the content changes (a re-add), it is replaced so the epoch retries over the new
// provers. Any other terminal full session is simply dropped.
if (session.hasFailed() && !contentChanged) {
continue;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there non-determinstic errors we could hit during proving, that could require a retry? Like is it possible that bb prove just happens to fail and a re-run would fix it? Or is that retry handled at the broker level?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The broker is configured to retry failed jobs a configurable number of time. Failure only reaches the prover node once all retries have been exhausted.

Comment on lines +262 to +266
if (contentChanged && this.canBuildOver(canonical)) {
const newSession = this.constructSession(session.getSpec(), canonical);
this.fullSessions.set(key, newSession);
void this.runSession(newSession);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see canBuildOver checks if any of the checkpoints failed. Does it make sense to collect an subarray of checkpoints that didn't fail, and build from there? Or is that reuse already handled somewhere else?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure what you mean here. The session can't build a root proof if any of the checkpoints have failed, it would be a waste to continue building the tree from non-failed checkpoints.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering whether there was work that could be reused from the checkpoints that didn't fail, re-triggering the ones that did fail (assuming it was retryable). Nevermind though, it seems like it gets retried anyway, and proofs are cached by the broker, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already the case. Checkpoints are proven up to but not including the checkpoint root. As long as this completes successfully, this work can be (re-)used by as many sessions as required. If the proving of a checkpoint fails, then it is of no use to any session. Either it is a genuine failure and the epoch will ultimately be pruned, or it's a transient error, most likely due to a re-org'd checkpoint that will be corrected as and when the blockstream events all arrive.

return await uploadEpochProofFailure(
this.config.proverNodeFailedEpochStore,
session.getId(),
`failed-epoch-${epoch}`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this change, since uploadEpochProofFailure already adds the epoch number to the path where it gets uploaded.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Resolved

Comment on lines +172 to +173
// A checkpoint prover that fails (a sub-tree fault or a prune-induced fork fault) uploads a
// post-mortem for its own checkpoint, independently of any session. Fire-and-forget.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth uploading failure data on a prune? Note that the upload includes the entire world-state and archive. I'd save it to actual proving failures.

// race-free post-mortem.
if (!this.isTerminal()) {
this.state = 'failed';
this.state = this.checkpoints.some(c => c.isFailed()) ? 'stopped' : 'failed';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A prune can still be misclassified here as a genuine failed.

A prune reaches a running full session on two channels, not one: a data-plane fork fault (isFailed()===true) and the control-plane cancelAndRemoveAboveBlock in handlePruneEvent, which cancel()s the affected provers — leaving isCancelled()===true but isFailed()===false. That cancel rejects the provers' blockProofs synchronously; handlePruneEvent then awaits getL1Constants() before onPrune schedules the reconcile, so the rejection can propagate into start()'s catch before the reconcile's session.cancel() sets 'cancelled'. With only cancelled (not failed) provers, some(c => c.isFailed()) is false → the session is classified failed: a spurious tryUploadEpochFailure (a full world-state snapshot) for what is actually a prune, plus a briefly-retained do-not-re-prove marker. This is the prune-vs-failure race the PR aims to remove.

Suggested fix — treat a cancelled prover as prune-ambiguous too, so only an all-healthy, un-cancelled set yields failed:

Suggested change
this.state = this.checkpoints.some(c => c.isFailed()) ? 'stopped' : 'failed';
this.state = this.checkpoints.some(c => c.isFailed() || c.isCancelled()) ? 'stopped' : 'failed';

This is conservative (a genuine failure coinciding with an unrelated cancel gets suppressed to stopped), but that coincidence is itself prune-ambiguous, so stopped is the safe call. The fully causal fix would propagate typed failure provenance up from the checkpoint/orchestrator rather than sampling mutable flags after the fact. Not covered by the current tests — their failing stub reports isFailed()===true, never the cancelled-only case.

Written by Claude (Opus 4.8), reviewed by Codex (gpt-5.6).

Comment on lines +304 to 309
// A session present here already covers the epoch: either live, or a retained genuinely-failed
// session kept by `recreateInvalidSessions` as a "do not re-prove" marker. Either way, don't open
// another — the retained-failed one is replaced only when its canonical content changes.
if (this.fullSessions.has(epoch)) {
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this is what protects against epochsForTrigger returning always the nextUnprovenEpoch now that lastTickEpoch was removed, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

… jobId

uploadEpochProofFailure already prefixes the upload path with the epoch number, so
the epoch in the jobId was redundant — and the epoch-only string dropped the
per-upload uniqueness the original session.getId() UUID gave. Use each entity's own
id instead: the session's id for a session (epoch) failure, and the prover's
content-addressed id for a checkpoint failure. Drop the now-unused epoch param from
tryUploadEpochFailure.
Two prune-vs-failure gaps remained after decoupling checkpoint failure from
epoch failure:

- Session classification only checked isFailed(), missing a control-plane
  cancel that reaches start()'s catch before the reconcile marks the session
  'cancelled'. Such a cancelled-but-not-failed prover was misclassified as the
  session's own 'failed', triggering a spurious full-snapshot upload for a
  prune. Treat a cancelled prover as prune-ambiguous too.

- A data-plane fork fault reaches the checkpoint-level onFailed upload
  indistinguishable from a genuine sub-tree failure (no cancel has landed
  yet). Gate the upload on the archiver: a pruned checkpoint's last block is no
  longer canonical there, so skip the expensive world-state + archiver snapshot
  when it has been pruned out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-draft Run CI on draft PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants