fix(e2e): pause the sequencer before the prover suites' epoch warps - #261
spalladino wants to merge 5 commits into
Conversation
FullProverTest extends SingleNodeTestContext but overrides `setup` and calls the raw
`setup(0, {...})` with no PXE options argument, so it never picked up the base's
`{ syncChainTip: 'checkpointed' }` default and both of its PXEs ran on the proposed tip.
The fixture calls `advanceToNextEpoch()`, which warps L1 by a full epoch while the
pipelining sequencer has an uncheckpointed block in flight. The archiver then prunes that
block as an orphaned proposal, which is correct: its slot closed without a checkpoint. But
the PXE had already anchored an operation to it, and when the prune lands between that
operation's pre-sync and its last anchored read the node no longer serves the hash, so the
operation fails with "Reference block not found ... possibly a reorg has occurred".
Anchoring to the checkpointed tip is not what detects a stalled checkpointer in these
suites: `send().wait()` already defaults to `TxStatus.CHECKPOINTED`, and the suites assert
on `waitForProven` and the L1 proven checkpoint number. Those instruments are unaffected.
Also makes the prover node stop optional during teardown, so a setup failure before the
node exists surfaces as itself rather than as a TypeError from the teardown cascade.
`FullProverTest` warps a full epoch between phases. Under pipelining there is normally a built-but-unpublished block in flight, and the warp moves its target slot into the past before its submission lands, so the archiver prunes it as an orphaned proposal. Anchoring the PXEs on the checkpointed tip stops that from breaking a PXE operation, but it does not stop the warp from throwing the block away. The four warps (the fixture's and three in `full.test.ts`) now go through `advanceToNextEpochWithSequencersPaused`, which pauses the sequencer, lets its current iteration and pending L1 submissions finish, proves the work landed, and only then moves the clock. `pause()` drains with `Promise.allSettled`, so its return says the submissions settled, not that they succeeded: every block the sequencer proposed — the tip as of entry plus everything proposed while draining — must be shown to sit in the checkpointed chain under the same hash first. Comparing proposed and checkpointed tips would not do, since a prune makes them equal by deleting the blocks in question. The prover-only node keeps running; it has no sequencer and has to keep tracking L1 across the warp. Checkpoint health is now asserted outright in the transfer test rather than left to surface incidentally: `watchSequencerEvents` over the whole test, drain included, and `assertNoFailuresFromSequencers` at the end. The reward and proven-checkpoint baselines move ahead of the warp, since draining takes at least a slot and a proof landing during it would fold the expected increase into the "before" values. `watchSequencerEvents` gains a `stop()` so a watch can be scoped to an interval, and its failure-event list moves to a shared constant (dropping a duplicate `checkpoint-publish-failed` that registered two listeners). New `single-node/sequencer/safe_epoch_warp` covers both directions: an in-flight proposal survives the advance with its hash intact, and dropping the sequencer's next L1 tx makes the advance fail while the checkpointed-tip PXE keeps syncing happily — which is exactly why the health check has to be explicit.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| `Refusing to warp: the sequencers drained without every proposed block reaching the checkpointed chain, so the warp would orphan them.`, | ||
| outstanding.length > 0 ? `Outstanding: ${outstanding.join('; ')}.` : undefined, | ||
| `Cause: ${err}.`, | ||
| `Sequencer failures while draining: ${failEvents.length === 0 ? 'none' : JSON.stringify(failEvents)}.`, |
There was a problem hiding this comment.
A captured checkpoint-publish-failed event can contain a viem TransactionReceipt with bigint fields. If a proposal remains uncheckpointed, JSON.stringify(failEvents) then throws while building this error, replacing the intended Refusing to warp message and its outstanding-proposal details with a serialization TypeError. Use a bigint-safe representation for these events.
|
|
||
| // Sampled before the pause: a block proposed before this call started listening is still in flight | ||
| // and must survive the warp just the same. | ||
| const { proposed } = await node.getChainTips(); |
There was a problem hiding this comment.
The helper collects new proposals from every sequencer in nodes, but samples pre-existing proposals from only the single node argument. Because this public helper accepts multiple sequencer-bearing nodes, another node can already have an unpublished proposal that is never added to proposals. The validation can then pass before the warp moves that proposal's target slot into the past. Seed the proposal set from every node or restrict the API to its supported single-node topology.
|
|
||
| // Anchoring the PXEs on the checkpointed tip removes the incidental way a stalled sequencer used | ||
| // to surface here, so checkpoint health is asserted outright over the whole test, drain included. | ||
| const watch = t.watchSequencerEvents(t.getSequencers(t.nodes)); |
There was a problem hiding this comment.
This describe-scoped watcher is stopped only after every preceding await and assertion succeeds. If the test exits earlier, its listeners remain attached to the sequencer reused by later tests, where they continue collecting and logging unrelated events. The helper has a similar gap because it installs listeners before the awaited getChainTips() call but enters its cleanup finally afterward. Put both watcher lifetimes behind guaranteed cleanup such as a disposable scope or finally.
| * started rather than before. | ||
| */ | ||
| function isBenignSequencerFailure(eventName: keyof SequencerEvents, args: unknown): boolean { | ||
| return eventName === 'block-build-failed' && (args as { reason?: string }).reason === 'Insufficient valid txs'; |
There was a problem hiding this comment.
The changed event handling uses unchecked as Type assertions both when reading the failure payload here and when unregistering the listener. The repository TypeScript directive requires type guards instead of as Type casts. Narrow the block-build payload with a guard and preserve the listener's concrete event type without asserting it; this repository requirement must be satisfied before merging.
Context Used: yarn-project/CLAUDE.md (source)
|
|
||
| let advanced = false; | ||
| try { | ||
| this.logger.warn(`Pausing ${sequencers.length} sequencers before advancing to the next epoch`); |
There was a problem hiding this comment.
This log interpolates the sequencer count without a structured second argument. The same pattern appears in the new resume log and in the safe-warp test's proposal log. The repository logging directive requires dynamic values in structured context objects so they remain filterable. Update these new logs accordingly before merging.
Context Used: yarn-project/CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…e drain Review follow-ups on the safe epoch advance. The helper only used the failure events it recorded to decorate an error message, so a `checkpoint-error` or publication failure during the drain passed silently whenever the tracked blocks happened to checkpoint (or none were recorded at all). Only the transfer test had an outer assertion, and none of the other warp sites did. Non-benign failures now block the warp outright, before and after the checkpointed-history check — warping on top of an unhealthy chain buries the cause under the prune that follows. `block-proposed` fires before the proposal reaches the archiver, so a block built in the instant before the helper subscribed appeared in neither the event stream nor the pre-pause tip sample. The proposed tip is now sampled on both sides of the drain. A block already pruned by then is still uncovered; the doc comment says so. `opts.timeout` only bounded the verification, leaving the drain itself to the enclosing jest timeout with no useful diagnostic. It now bounds `pause()` too, and a drain that does not finish leaves the sequencers down rather than racing a restart against submissions that are still in flight. The failure-injection case armed `cancelNextTx()` and advanced immediately, which is racy: the in-flight proposal's tx may already have been broadcast, and the pause could then halt the loop before another was sent, leaving the delayer armed and nothing dropped. It now waits until a publication has actually been dropped. Also: both watches move under `afterEach` so the listeners are detached when a test throws before its assertion, and the transfer test reads its three baselines in one batch so a proof cannot land between them.
Two review follow-ups, both wording. The drain-failure check makes this helper the wrong tool for a recovery scenario, where `proposer-rollup-check-failed` (transient archiver mismatch) and `pipelined-checkpoint-discarded` (an unexpected parent arriving) are part of the behavior under test rather than a fault. Say so before someone reuses it there. The "leaving sequencers paused" log also fired when the pause had never started (a failed tip query), which reads as a drain that hung. It now says the pause did not complete, which covers both.
eslint's no-unsafe-finally rejects a throw inside finally, since it would discard an in-flight exception. Capture the resume failure and rethrow it after the block: when the advance itself failed, its error propagates out of the finally first, so the resume failure still cannot mask it.
1 participant
Stacked on #253.
#253 stopped the prover fixture's PXEs from breaking when a warp orphans the block they anchored on. It did not stop the warp from orphaning the block. This does.
The remaining problem
FullProverTestruns its setup underPIPELINING_SETUP_OPTS(minTxsPerBlock: 0), so the sequencer pipelines: it builds the next slot's block during the current one and publishes at the target slot. There is normally a built-but-unpublished block in flight.advanceToNextEpoch()then warps 26 slots, moving that block's target slot into the past before its submission completes, and the archiver prunes it as an orphaned proposal:Healthy work, thrown away by the test's own clock.
What this does
The four warps — the fixture's
Move to a clean epochand three infull.test.ts— go through a newSingleNodeTestContext.advanceToNextEpochWithSequencersPaused. It pauses the sequencer (the existingpause(), which halts the poll loop and lets the in-flight iteration and its pending submissions finish untouched), verifies the work landed, warps, and resumes. Only the sequencer-bearing node is paused; the prover-only node keeps running, since it has no sequencer and has to keep tracking L1 across the warp.pause()returning is not proof of publication.CheckpointProposalJob.finish()drains withPromise.allSettled, so its return means the submissions settled, not that they succeeded. Before the clock moves, every block the sequencer proposed — the proposed tip sampled at entry, plus everyblock-proposedseen while draining — must be shown to sit in the checkpointed chain under the same hash, with a bounded wait and an error naming what is outstanding and which sequencer failures fired during the drain. Comparing the proposed and checkpointed tips would not do: a prune makes them equal by deleting the very blocks in question.Any non-benign sequencer failure recorded while draining also blocks the warp, checked both when the pause returns and after the history check. Warping on top of an unhealthy chain buries the cause under the prune that follows, and only one of the four warp sites sits inside a test-level
assertNoFailuresFromSequencers.opts.timeout(default 120s) bounds the drain and the verification separately. On failure the sequencer is resumed anyway, but a resume error is logged rather than thrown so it cannot replace the reason the advance was unsafe; a drain that never finished leaves the sequencers down instead, rather than racing a restart against submissions still in flight.block-proposedfires before the proposal reaches the archiver, so the proposed tip is sampled on both sides of the drain. A block that emits in the instant before the helper subscribes and is pruned before the post-drain sample is still uncovered; the doc comment says so.Checkpoint health is now asserted, not incidental
#253's tradeoff was that checkpointed anchoring removes the incidental way a stalled sequencer used to surface in these suites. The transfer test now says it outright:
watchSequencerEvents()over the whole test, drain included, andassertNoFailuresFromSequencers()at the end. Publish failures, checkpoint errors, header-validation failures and discarded pipelined checkpoints fail the test; "insufficient transactions" stays allowed, and there is no exception carved out around the warp. The existing checkpointed receipt waits and L1 proof assertions are untouched, and no new checkpoint-progress waits were needed — the transfers already wait toTxStatus.CHECKPOINTED.The reward and proven-checkpoint baselines move ahead of the warp. Draining takes at least a slot, and a proof landing during it would fold the very increase
expect(newProvenCheckpointNumber).toBeGreaterThan(oldProvenCheckpointNumber)looks for into the "before" value.epochis still read before the advance, so the reward assertions stay pinned to the epoch the txs landed in even if the drain crosses a boundary.Also in this PR
watchSequencerEventsreturns astop()so a watch can be scoped to an interval instead of leaking listeners into later phases, and its failure-event list moves to a shared module constant — which drops a duplicatedcheckpoint-publish-failedthat had been registering two listeners for the same event.Testing
New
single-node/sequencer/safe_epoch_warp.test.ts, two cases, both on a checkpointed-tip PXE:maxSpeedUpAttempts: 0/cancelTxOnTimeout: falseso a dropped tx stays dropped, thensequencerDelayer.cancelNextTx()and a wait until a publication has actually been dropped — arming alone is racy, since the in-flight tx may already be broadcast and the pause could halt the loop before another is sent. The advance rejects — and the wallet's PXE goes on syncing happily to a block that predates the dropped publication, which is precisely why the health check has to be explicit rather than inferred from anchoring.Red/green on the first case, by swapping the helper for the plain
cheatCodes.rollup.advanceToNextEpoch(). With a 60s window to publish, the in-flight block never comes back — the height is taken by a different block:With the helper, both cases pass (28s and 36s).
Run locally with fake proofs:
single-node/prover/server/full4/4,single-node/prover/client/client1/1,bench/tx_stats_bench3/3 (1 skipped),single-node/sequencer/safe_epoch_warp2/2. Typecheck and prettier clean.Not run locally: real proofs (
FAKE_PROOFS=0) — no bb toolchain in this worktree — sofull.test.ts's invalid-proof and ddos cases (which include a fourth warp) only ran as no-ops. CI is the verification for those.Out of scope
PXE re-anchoring during an operation is F-895 and is not touched here.