feat(prover-node): make epoch proving robust to prune-induced fork faults#24678
Conversation
…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.
…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.
…ke-epoch-proving-robust-to-prune-induced-fork
…l/a-1418-prover-node-make-epoch-proving-robust-to-prune-induced-fork
spalladino
left a comment
There was a problem hiding this comment.
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.
| // 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; | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| if (contentChanged && this.canBuildOver(canonical)) { | ||
| const newSession = this.constructSession(session.getSpec(), canonical); | ||
| this.fullSessions.set(key, newSession); | ||
| void this.runSession(newSession); | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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}`, |
There was a problem hiding this comment.
Curious why this change, since uploadEpochProofFailure already adds the epoch number to the path where it gets uploaded.
There was a problem hiding this comment.
Good catch. Resolved
| // 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. |
There was a problem hiding this comment.
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'; |
There was a problem hiding this comment.
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:
| 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).
| // 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; | ||
| } |
There was a problem hiding this comment.
IIUC this is what protects against epochsForTrigger returning always the nextUnprovenEpoch now that lastTickEpoch was removed, right?
… 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.
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— gainsisFailed(), 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 anonFailedcallback so the owner can upload that single checkpoint's post-mortem. A cancel leavesisFailed()false.EpochSession— a fault now settles in one of two terminal states, decided by the provers:stopped— aCheckpointProverunder 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— removedlastTickEpoch. Two mechanisms stop a doomed epoch being re-proved every tick:openFullSessionIfReadyrefuses to build over a set containing a failed prover, andrecreateInvalidSessionsretains a genuinely-failedfull session as a do-not-re-prove marker, replacing it only when its canonical content changes (a re-add). Afailedsession firesonSessionFailed(session-level upload). Recovery from prune/reorg flows through the ungatedcheckpoint/prunetriggers, reusing already-completed sub-proofs from the content-addressed broker.ProverNode— uploads post-mortems at two levels: per checkpoint (tryUploadCheckpointFailure, wired toCheckpointProver.onFailed— fires for prune-induced faults too, on purpose) and per session (tryUploadEpochFailure, wired toonSessionFailed).expireEpochonly releases the chonk cache and reaps — it does not upload, since a missed-window epoch's provers may already be pruned by then.rerunCheckpointProvingJobto re-prove a single downloaded checkpoint in isolation (sub-tree only; no epoch top-tree or L1 submit), sharing offline setup withrerunEpochProvingJob.downloadEpochProvingJobis unchanged (same serialized format).Tests
epoch-session.test.ts— a fault endsstopped(a prover failed) vsfailed(top-tree/submit with healthy provers).checkpoint-prover.test.ts— a mid-proof fork fault setsisFailed()and firesonFailedonce; a cancel does neither.session-manager.test.ts— skips building over a failed prover; retains + uploads afailedsession and replaces it on content change; does not upload onstopped; data-plane fault + re-add (identical and different content) recovers and completes.prover-node.test.ts— a registered checkpoint prover failure routes totryUploadCheckpointFailure;expireEpochreaps 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 thecheckpointProveOverridehook, capture the per-checkpoint upload, and re-prove it viarerunCheckpointProvingJob.prover-node unit suite green; full
yarn build; format + lint clean.Resolves A-1418.