fix(sequencer): use targetSlot in tryVoteWhenEscapeHatchOpen under pipelining#23296
Merged
PhilWindle merged 2 commits intoMay 15, 2026
Merged
Conversation
…pelining tryVoteWhenEscapeHatchOpen constructed CheckpointVoter with the wall-clock slot and called publisher.sendRequestsAt(slot). Under proposer pipelining we are the elected proposer for slot + 1 (targetSlot), and the multicall is expected to mine in that slot. Signing for the wall-clock slot makes the L1 contract's EIP-712 digest mismatch and the require msg.sender == getCurrentProposer() check fail, because the wall-clock slot's proposer is someone else. The whole multicall reverts silently inside Multicall3 and every governance/slashing entry is dropped. Thread targetSlot through and use it for both CheckpointVoter (which binds the EIP-712 signature) and publisher.sendRequestsAt (which delays submission so the tx mines in targetSlot). Mirrors tryVoteWhenSyncFails and CheckpointProposalJob.execute. When pipelining is disabled targetSlot equals slot, so sendRequestsAt resolves with no extra sleep and the legacy behaviour is preserved. Unblocks e2e_sequencer/escape_hatch_vote_only and the "should vote even when unable to build blocks" case in e2e_sequencer/gov_proposal.parallel under pipelining (these were skipped in #23275 pending this fix). See PIPELINING_TEST_STATUS.md §6 B5.
Exercises the §6 B5 source-level fix (`tryVoteWhenEscapeHatchOpen` now signs the CheckpointVoter for `targetSlot` and submits via `sendRequestsAt(targetSlot)`). Without the fix, governance signals cast during the escape-hatch window would fail signature verification inside Multicall3 and the test's `finalStats.votes >= slotsPassed` assertion would fail. Test-side adjustments needed for the pipelined timing model: - Move event listener attachment to after the warp into the escape-hatch epoch. Checkpoint proposals in flight at warp time fail their L1 propose (their target slot is now in the past after the L1 timestamp jump) and we don't want to count those setup-warp artifacts as escape-hatch failures. Also filter remaining events to those at or after `initialStats.slot` for the same reason. - Snapshot `slotAtMeasurement` for the vote-count lower bound, then wait for the L1 slot to advance two more so the trailing vote (signed in build slot N for target slot N+1) has time to mine before we count. Without this drain, `finalStats.votes` consistently lags `slotsPassed` by 1-2 under pipelining. Inlines `enableProposerPipelining: true` and `inboxLag: 2` directly in the test rather than importing a shared `PIPELINING_SETUP_OPTS` preset, since this is the only pipelined test on this branch.
spalladino
added a commit
that referenced
this pull request
May 15, 2026
…is in Rebased onto palla/fix-b5-escape-hatch-slot-targeting (PR #23296), which forward-ports the B5 escape-hatch slot-targeting fix onto the modern buildCheckpointSimulationOverridesPlan + flat l1Contracts API. - sequencer.ts: restored to the B5 base state. The earlier cherry-pick in this branch reintroduced the pre-refactor API surface (buildPipelinedParentSimulationOverridesPlan, config.rollupAddress, setProposerAddressForSimulation, Date arg to sendRequestsAt). With the rebase onto PR #23296 the B5 fix is correctly applied against the current API. - e2e_sequencer/escape_hatch_vote_only.test.ts: re-opted-in to PIPELINING_SETUP_OPTS (was un-opted via the previous "B5 not on this PR" commit; B5 is on this PR now). Removed the redundant explicit enableProposerPipelining/inboxLag overrides that duplicated the preset. - e2e_sequencer/gov_proposal.parallel.test.ts: un-skipped 'should vote even when unable to build blocks' (was kept as it.skip pending the B5 fix). Restored the pipelining-aware timing tweaks: wait two slots before declaring the tx un-syncable, retryUntil the L1 checkpoint advance, and bump the round-end timeout by an extra slot. - end-to-end test files: re-applied the build/lint fixes that the earlier "unblock CI build/lint failures" commit had introduced -- BlockNumber typing in contract_class_registration.test.ts, { receipt } destructuring in e2e_ordering.test.ts, underscore-prefix unused vars in e2e_block_building.test.ts and e2e_crowdfunding_and_claim.test.ts.
spalladino
added a commit
that referenced
this pull request
May 15, 2026
…proposal "vote even when unable to build blocks" Under pipelining the proposer pushes its own built block straight into its local archiver via syncProposedBlockToArchiver (checkpoint_proposal_job.ts), bypassing the blob client. So disabling the blob client alone is no longer sufficient to recreate the legacy "node cannot sync the block" state — the tx resolves locally as `checkpointed` and the test's `expect(...).rejects .toThrow(TimeoutError)` fails. Also set `skipPushProposedBlocksToArchiver: true` via aztecNodeAdmin before disabling the blob client, forcing the node back onto the blob client for sync. This restores the legacy premise so the tryVoteWhenSyncFails fallback path the test is actually exercising still has a syncing-broken state to exercise. This is a separate issue from §6 B5 (escape-hatch signature targeting, fixed via the rebase onto PR #23296) — both fixes are needed for this test under pipelining.
PhilWindle
approved these changes
May 15, 2026
This was referenced May 15, 2026
PhilWindle
pushed a commit
that referenced
this pull request
May 15, 2026
## Summary > **Depends on PR #23296** -- this PR is rebased on top of `palla/fix-b5-escape-hatch-slot-targeting`, which forward-ports the §6 B5 escape-hatch slot-targeting fix onto the modern `buildCheckpointSimulationOverridesPlan` + flat `l1Contracts` API. With B5 in, `e2e_sequencer/escape_hatch_vote_only` and `e2e_sequencer/gov_proposal.parallel` "should vote even when unable to build blocks" are now re-enabled under pipelining on this PR. Extracts the tests known to pass under proposer pipelining from PR #23150, without flipping the global default. Tests opt into pipelining explicitly via a new `PIPELINING_SETUP_OPTS` helper. The global `enableProposerPipelining` default stays `false` on `merge-train/spartan`; this PR migrates tests file-by-file so each one is opted in by name. This PR is intentionally scoped: it only includes tests whose pipelining-ready status is reasonably well understood. Tests that depend on shared base-class fixtures (`FeesTest`, `BlacklistTokenContractTest`, `CrossChainMessagingTest`, `DeployTest`, `FullProverTest`, etc.) keep their branch changes but are not yet wired to pipelining via their base class -- those base classes are used by tests outside this batch and a blanket opt-in would over-migrate. They will be migrated in follow-up PRs. Two commits: 1. **`test(e2e): opt unchanged tests into proposer pipelining`** -- adds `PIPELINING_SETUP_OPTS` to `fixtures.ts`, the small deploy-phase `accountsDeployMinTxs` conditional to `setup.ts`, and the explicit opt-in to every §1 test that calls `setup()` directly. 2. **`test(e2e): migrate tests that needed fixes into proposer pipelining`** -- the §2 tests with their branch fixes plus the infrastructure they depend on (sequencer.ts B5 fix, dummy_service.ts loopback, sequencer-publisher.ts error logging, sequencer-client READMEs rewrite, bootstrap.sh / test_simple.sh timeout bumps). The global default flip and the migration of base-class-using tests are intentionally deferred. They will land separately once each batch can be verified independently. --- ## §1 -- Pipelining enabled and passing (no code changes) Tests that pick up `enableProposerPipelining=true` from the explicit opt-in and pass without any per-test fix. This is the majority of the suite -- too many to enumerate. Examples include the unmodified `e2e_authwit`, `e2e_nft`, `e2e_amm`, `e2e_partial_notes`, `e2e_token_contract/*` (non-overflow), `e2e_offchain_*`, `e2e_orderbook`, `e2e_event_*`, `e2e_keys`, `e2e_avm_simulator` (after the suite-level timeout bump only), `e2e_pending_note_hashes_contract`, etc. None of these required test-level pipelining adaptations. Pre-existing `it.skip`s in this bucket are unrelated to pipelining (they predate the branch) and were not touched: - `e2e_token_contract/{transfer,transfer_in_private,transfer_in_public}` "transfer into account to overflow" - `e2e_blacklist_token_contract/{transfer_private,transfer_public}` "transfer into account to overflow" - `e2e_synching` "replay history and then do a fresh sync" / "a wild prune appears" - `e2e_p2p/reex` "validators re-execute transactions before attesting" ## §2 -- Pipelining enabled and needed fixes Tests that needed test- or fixture-level changes to pass under pipelining. All currently passing under PR #23150. **Fixture-level (`src/fixtures/fixtures.ts` + `src/fixtures/setup.ts`)** - New `PIPELINING_SETUP_OPTS` preset exporting `inboxLag=2`, `minTxsPerBlock=0`, `aztecSlotDuration=12s`, `ethereumSlotDuration=4s`, `walletMinFeePadding=PIPELINED_FEE_PADDING` (30x), and `enableProposerPipelining=true`. - `setup.ts` gains a small conditional so the deploy-phase `minTxsPerBlock` override uses `0` instead of `1` under pipelining (otherwise the chain stalls on alternating slots). **Cheat-codes (`src/testing/cheat_codes.ts`)** -- already on `merge-train/spartan` via cherry-pick of #23213. **P2P (`src/services/dummy_service.ts`)** - `notifyOwnCheckpointProposal` now invokes the all-nodes callback synchronously, mirroring libp2p loopback. Without this the in-process e2e sequencer never sees its own proposal and the pipelined parent verification blocks indefinitely. **Sequencer-client** - `sequencer.ts::tryVoteWhenEscapeHatchOpen` -- §6 B5 fix: takes `targetSlot`, signs the voter for `targetSlot`, and delays submission via `sendRequestsAt(getTimestampForSlot(targetSlot))` when pipelining is enabled. Mirrors the existing `tryVoteWhenSyncFails` and `CheckpointProposalJob.execute` patterns. Plus a refactor of `canProposeAt` simulation overrides via `SimulationOverridesBuilder`. - `sequencer-publisher.ts` -- error log on publisher exhaustion now includes the underlying viem error and tried-addresses context. **Per-suite test fixes** - `e2e_lending_contract` -- predictable-time stub, longer hook windows. - `e2e_fees/private_payments` "pays fees for tx that dont run public app logic". - `e2e_blacklist_token_contract/{burn, minting, shielding, transfer_private, transfer_public, unshielding}` -- 6/7 suites re-enabled (`access_control` still skipped, see §5). - `e2e_contract_updates` -- all 4 tests re-enabled (covered by §1 opt-in in this PR). - `e2e_expiration_timestamp` invalidates tests -- L1-only `eth.warp(target, { resetBlockInterval: true })`, no publisher cascade. - `e2e_ordering` -- switched from "latest block" to receipt-block reads; helper renamed to `expectLogsFromBlockToBe(logMessages, fromBlock)`. - `e2e_fees/failures` -- snapshot `provenCheckpointBefore/After`, use `waitForProven` with extended timeout, account for newly-proven checkpoint deltas in reward math, read committed fee headers via `getCommittedProverFee` / `getCommittedBurn`. - `e2e_fees/gas_estimation` -- pad `maxFeesPerGas` via `getPaddedMaxFeesPerGas(aztecNode)` in `beforeEach` to absorb fee-asset price evolution between snapshot and submission. 3/3 passing. - `e2e_crowdfunding_and_claim` "cannot donate after a deadline" -- L1-only `cheatCodes.eth.warp(deadline+1, { resetBlockInterval: true })`. - `e2e_deploy_contract/contract_class_registration` private-ctor variants -- thread `receipt.blockNumber` through `deployFn`, read logs from that specific block instead of "latest". 21/21 passing. - `e2e_state_vars` DelayedPublicMutable -- root cause was slot-duration mismatch (`delay(4)` assumed `aztecSlotDuration=72s` from `DefaultL1ContractsConfig`; fixture forces `12s` under pipelining). Replaced `delay(4)` with a loop that pumps no-op txs until `timestamp >= timestamp_of_change`, and asserted exact equality against `tx.data.constants.anchorBlockHeader.globalVariables.timestamp + newDelay - 1n`. Tight `toEqual`, no widened bound. - `e2e_pending_note_hashes_contract` -- squash helpers use the latest *non-empty* block. - `e2e_expiration_timestamp` -- include-by computation bumped by 2x `aztecSlotDuration`. - `e2e_p2p/*` and `e2e_epochs/*` -- explicit `enableProposerPipelining: true` + `inboxLag: 2` on every test that builds its own config (so behavior is intentional rather than implicit). - `e2e_block_building` "processes txs until hitting timetable" -- replaced legacy `canStartNextBlock` mock + single-deadline timetable with the pipelined sub-slot budget (`blockDurationMs=2000`, `enforceTimeTable=true`, `fakeProcessingDelayPerTxMs=500`). 10 simultaneous txs must span at least 2 distinct blocks; would fail if the proposer reverted to single-block-per-slot or stopped enforcing sub-slot deadlines. - `e2e_block_building` "assembles a block with multiple txs" (x2) -- pre-publish the contract class once and pass `skipClassPublication: true` on each per-tx deploy so the deploys don't all share the same `ContractClassRegistry.publish` nullifier and get RBF-rejected against each other. Also reset `blockDurationMs` in `afterEach` so the multi-block-per-slot state from the previous test doesn't leak. - `e2e_block_building` "publishes two empty blocks" -- `buildCheckpointIfEmpty: true` so the proposer doesn't skip empty sub-slots; retry budget bumped from 10s -> 60s because empty checkpoints land every `aztecSlotDuration` (12s) rather than every legacy block. - `e2e_epochs/epochs_mbps.parallel` "builds multiple blocks per slot with L2 to L1 messages" -- pipelined timing loses one sub-slot to attestation propagation; expectation dropped from `EXPECTED_BLOCKS_PER_CHECKPOINT=3` to `>= 2`, mirroring the sibling MBPS tests. - `e2e_l1_with_wall_time` -- test was explicitly passing `ethereumSlotDuration` from env (=12s), defeating the fixture's pipelining override (=4s). With `aztec=eth=12s`, pipelined timing can't fit propose+attest+publish in one Aztec slot. Removed the explicit `ethereumSlotDuration`; also wrapped `teardown` in `afterEach` so setup failures surface their real error. - `e2e_p2p/add_rollup` re-enabled (entire describe; 1 test, passes in ~9:14 locally). AttestationTimeoutError still fires in some slots, but the bundled-multicall governance-signal preCheck is independent of the propose preCheck -- signals accumulate and reach quorum even when checkpoint proposes fail to attest. - `e2e_pruned_blocks` "can discover and use notes created in both pruned and available blocks" -- restored the explicit `markAsProven` call (as it had pre-#21156) + a 2-block buffer for Anvil's `finalized = latest - 2` heuristic; test re-enabled and passes. - `e2e_sequencer/escape_hatch_vote_only` re-enabled. Source fix at `sequencer.ts::tryVoteWhenEscapeHatchOpen` (see §B5 in PR #23150). Test-side: attach event listeners *after* the warp, explicitly drain trailing in-flight votes before counting. - `e2e_sequencer/gov_proposal.parallel` re-enabled (both tests). Two pipelining-aware adjustments: warp offset bumped to `nextRoundBeginsAtTimestamp - AZTEC_SLOT_DURATION - ETHEREUM_SLOT_DURATION`, and per-tx wait timeouts tuned for two slots of catch-up (proposer + L1 mine). **Bash-level timeout adjustments (`end-to-end/bootstrap.sh`)** -- pipelined sequential dependent txs run at ~2x legacy latency: - simple e2e default: 10m -> 20m - `e2e_block_building`: 25m - `e2e_avm_simulator`: 30m - compose/web3signer: 20m - HA: 30m - `scripts/test_simple.sh` Jest `--testTimeout` 5m -> 10m - ~21 test files: per-file `const TIMEOUT` raised from 100/120/150/180s -> 300s. --- ## Out of scope - **Global default flip**: PR #23150 flipped `enableProposerPipelining=true` everywhere. This PR keeps the default `false` and migrates per-test. The global flip will land in a follow-up. - **§3 opt-outs** (`e2e_l1_publisher` "with attestations" describe, `epoch_cache.test.ts` non-pipelined branch coverage, demo `docker-compose.yml`): no change required while the default is `false`. - **§5 still-skipped tests**: the tests in §5 of PR #23150's categorization (e.g. `e2e_blacklist_token_contract/access_control`, `e2e_publisher_funding_multi`, `e2e_fees/fee_settings`, etc.) remain at `merge-train/spartan` state. - **Base-class fixtures** (`FeesTest`, `BlacklistTokenContractTest`, `CrossChainMessagingTest`, `DeployTest`, `FullProverTest`, `EpochesTest`, P2P fixtures): test files using these get their branch-side changes preserved but are not wired to pipelining via the base class -- those base classes are shared with tests not in this batch and a blanket opt-in would over-migrate. Follow-up PRs will opt them in selectively. Reference: PR #23150 (`palla/kill-non-pipelined-flow`) for full context on the categorization, source-level bugs surfaced (§6 B1-B6), and per-suite investigation notes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Sequencer.tryVoteWhenEscapeHatchOpenconstructedCheckpointVoterwith the wall-clockslotand calledpublisher.sendRequestsAt(slot). Under proposer pipelining we are the elected proposer forslot + 1(targetSlot), and the multicall is expected to mine intargetSlot.EmpireBase.sol::_internalSignal:msg.sender == getCurrentProposer()for the mining slotBoth fail under pipelining because we're the proposer for
targetSlot, notslot. The multicall reverts silently inside Multicall3 and every governance/slashing entry is dropped.Fix
Thread
targetSlotthroughtryVoteWhenEscapeHatchOpenand use it for both:CheckpointVoter(binds the EIP-712 signature totargetSlot)publisher.sendRequestsAt(targetSlot)(delays submission so the tx mines intargetSlot)This mirrors
tryVoteWhenSyncFailsandCheckpointProposalJob.execute, which already usetargetSlotcorrectly. When pipelining is disabledtargetSlot == slot(fromepochCache.getTargetEpochAndSlotInNextL1Slot()), sosendRequestsAtresolves with no extra sleep and the legacy behaviour is preserved.Showcase
Re-enables
e2e_sequencer/escape_hatch_vote_only.test.tswithenableProposerPipelining: trueandinboxLag: 2. The test assertsfinalStats.votes >= slotsPassedover the escape-hatch window — this assertion fails without the fix because no votes ever land.Test-side adjustments for the pipelined timing model:
slotAtMeasurementfor the vote-count lower bound, then wait for the L1 slot to advance two more so the trailing vote (signed in build slot N for target slot N+1) has time to mine before counting.