feat: merge-train/spartan-v5#24524
Open
AztecBot wants to merge 17 commits into
Open
Conversation
Sequencer defines a set of fallback actions (voting and pruning) which are meant to be executed if it fails to propose a checkpoint, due to sync errors. However, those actions were being triggered as soon as the sequencer sync check failed, even if in the next sequencer iteration the check succeeded and the sequencer managed to produce a block. Now, the sequencer waits until it's past its build deadline (too late within the build slot to actually build anything), and only then triggers the fallback actions. Fixes A-1362 Supersedes #24488
Otherwise very few peers can exhaust the global quota of a node. Fixes A-1335
## Summary Experiment to validate [A-1326](https://linear.app/aztec-labs/issue/A-1326/proposal-accepted-when-expectedproposer-is-undefined-consensus-bypass): when `getProposerAttesterAddressInSlot` resolves to `undefined`, `ProposalValidator` currently skips the proposer-identity check entirely and accepts the proposal. This applies the audit's suggested fix — reject instead of skip — mirroring `CheckpointAttestationValidator`, which already rejects in the equivalent case. Note: `expectedProposer` resolves to `undefined` specifically when the committee is configured empty (`targetCommitteeSize == 0`, i.e. permissionless/open-committee mode where "anyone may propose") — `sequencer.ts`'s `checkCanPropose` treats that same case as `canPropose = true` for every node. This PR is intended to surface, via CI (in particular any multi-node/permissionless e2e coverage), whether rejecting proposals in that mode breaks anything that currently relies on it. ## Test plan - Updated `proposal_validator.test.ts`'s "open committee" cases to expect rejection instead of acceptance - `yarn workspace @aztec/p2p test src/msg_validators/proposal_validator/proposal_validator.test.ts` passes locally (49/49) - `yarn build` passes - Relying on CI (multi-node/e2e) to reveal any behavior that depends on the previous accept-when-undefined path
## Context Reconciling checkpoints from L1 could permanently wedge the archiver. `BlockStore.addCheckpoints` accepts an already-stored checkpoint that an L1 reorg re-included in a different L1 block (same archive root, new L1 metadata), but it filtered the already-stored prefix only locally: `ArchiverDataStoreUpdater.addCheckpoints` still re-ran log and contract-data extraction for every block in the original batch. Re-presented blocks carrying a `ContractClassPublished` or `ContractInstancePublished` event then threw `already exists`, aborting the whole write transaction — the L1 sync point never advanced, so every retry re-fetched the same range and failed deterministically until manual DB cleanup. ## Approach - `BlockStore.addCheckpoints` now returns the checkpoints it actually inserted (excluding the already-stored prefix handled by `skipOrUpdateAlreadyStoredCheckpoints`) instead of a boolean, and the data store updater derives the blocks for log/contract-data extraction from that suffix. - As defense in depth, duplicate contract class/instance inserts are now a no-op when the stored insertion block number matches the incoming one; a duplicate at a different block number still throws, since that signals genuine double-processing. - Regression tests cover re-including an already-stored checkpoint alone and batched with a new one, plus store-level tests for the same-block/different-block duplicate behavior. Reported in AztecProtocol/aztec-claude#1288. Fixes A-1350
#24515) ## Context A committee member — or any peer mutating the recovery byte in flight — can emit an attestation signature in yParity form (`v = 0/1`) that still recovers to the same signer, since coordination-signature recovery allows yParity as `v`. This is enough to brick an honest proposer. On the proposer side, `CommitteeAttestationsAndSigners.packAttestations` treated `v === 0` as an empty (address-only) slot, while `getSigners` treats a signature as empty only when `r`, `s`, and `v` are all zero. The two disagree for a real signature with `v = 0`: the packed bitmap popcount excludes it but the signers array keeps it, so an honest proposer's `propose()` reverts on L1 with `AttestationLib__SignersSizeMismatch`. A `v = 1` signature instead passes `propose()` but reverts epoch proving, where `verifyAttestations` runs `ECDSA.recover` and rejects `v ∉ {27,28}`. Either variant lets a single actor repeatably halt block production or proving for slots they attest to, with no slashing signal (they did attest). ## Approach Canonicalize the attester signature's recovery byte to `v ∈ {27,28}` (which keeps `(r, s)` and the recovery bit, so the recovered address and the signer's vote are preserved), at three layers: - `orderAttestations` — the honest path that assembles the L1 bundle, via the existing `normalizeSignature`. It runs before the adversarial-injection manipulation used by the invalid-attestation e2e tests, so those deliberately-malformed signatures are left untouched. - Attestation pool ingress (`tryAddCheckpointAttestation` and `addOwnCheckpointAttestations`) — defense in depth, so stored and re-gossiped bytes are always canonical. - `CommitteeAttestationsAndSigners.packAttestations` itself — its empty-slot check now matches `getSigners()` (`r`, `s`, `v` all zero, not just `v === 0`), and it canonicalizes a yParity recovery byte (`0/1 → 27/28`) when packing. This rewrites only the `v` byte — it does not touch `(r, s)` and leaves any other `v` value as-is — so the `injectHighSValueAttestation` / `injectUnrecoverableSignatureAttestation` / `MaliciousCommitteeAttestationsAndSigners` paths (which inject malformed signatures after `orderAttestations` to verify L1 *rejection*) still behave as before. A full `normalizeSignature` inside the class would break them; the `v`-byte-only fix does not. Regression tests cover both `v = 0` and `v = 1` at the `orderAttestations` layer, the pool ingress layer, and directly on `packAttestations` (each asserted red first, e.g. bitmap popcount 3/4 vs signers 4, then green). Fixes A-1351
Adds an EthCheatCodes helper that disables automine, mines a fresh block, syncs the TestDateProvider, and starts interval mining. Used when e2e setup hands off from automined L1 deployment to interval mining before sequencer startup. Should prevent a flake where anvil was left with a different date as the `TestDateProvider` after the automines triggered by the L1 deployment, and on the first L1 block mined the date provider automatically synced to the anvil date, warping time under the running node and causing a prune.
Part of the e2e round-2 consolidation (PR 1 of 9). Merges the automine
`token/` suites into fewer, parameterized files, with no loss of
asserted behavior. Failure cases are written as explicit per-case tests
(no shared dispatch DSL), and the two token harnesses share member names
so the merged suites can reuse code.
## Merges (old files → new)
- **Transfer quintet → two files.** `transfer.test.ts` (happy paths +
the private `Transfer` event assertion, grouped by entrypoint) and new
`transfer_failures.test.ts` (explicit per-case failure tests, grouped by
entrypoint). Deletes `transfer_in_private`, `transfer_in_public`,
`transfer_to_private`, `transfer_to_public`.
- **`burn.test.ts` + `blacklist_burn.test.ts` → `burn.test.ts`** via
`describe.each` over the two harnesses. Each scenario's setup returns
its harness directly; only the differing private-burn method
(`burn_private` vs `burn`) is mapped per scenario (`burn_public` is
identical on both). The blacklist-only "sender is blacklisted" cases are
an extra block. Deletes `blacklist_burn.test.ts`.
- **`blacklist_transfer_private.test.ts` +
`blacklist_transfer_public.test.ts` → `blacklist_transfer.test.ts`**,
parameterized by authwit mechanism (private-proxy vs public) over a
single shared harness (private transfers only touch private balances,
public only public, so neither disturbs the other's mint). Failure cases
are explicit per-case tests inside the mechanism loop. Deletes both.
- **`minting.test.ts` → `describe.each(['Public','Private'])`** for the
mirrored role/overflow cases; the private-only ABI-encoding overflow
case stays explicit.
- **`reading_constants.parallel.test.ts` →
`reading_constants.test.ts`**: dropped the `.parallel` suffix, converted
the 6 its to a single `it.each`, and deleted the empty no-op
`beforeEach`.
## Shared helpers (`token_test_helpers.ts`)
- `balanceOf` / `halfBalanceOf` / `amountAboveBalance` — the "read
balance, take half or balance±1, assert > 0" idiom.
- `assertPublicAuthwitReplayRejected` /
`assertAuthwitProxyReplayRejected` — the grant/consume-then-replay
dances for public and private-proxy authwits.
- `deployAmmWithTokens` — the AMM + 3-token + authwit-minter deployment
boilerplate previously duplicated in `token/amm.test.ts` and
`simulation/kernelless_simulation.test.ts`; both now call it.
Adopted the balance helpers (and the proxy-replay helper in unshielding)
in `blacklist_shielding.test.ts` and `blacklist_unshielding.test.ts`.
## Harness name unification
`TokenContractTest` and `BlacklistTokenContractTest` now share member
names so the merged burn suite reads members off the harness directly:
- Both expose a public `applyMint()` called after `setup()`; the Token
harness's pre-setup `applyMintSnapshot()` flag is gone.
- The Token harness's secondary account is renamed `account1Address` →
`otherAddress` to match the blacklist harness's owner/other vocabulary.
Consumers updated accordingly (`transfer`, `minting`,
`access_control.parallel`, `private_transfer_recursion.parallel`).
## Quick wins
- Deleted the byte-identical duplicate `it('transfer on behalf of other,
wrong designated caller')` in the old `transfer_in_public`.
- Collapsed the triplicate `it.skip('transfer into account to
overflow')` (GH #1259) to a single copy in `transfer_failures.test.ts`.
- Resolved the `new TokenContractTest('transfer_private')` harness-name
collision (the two files that shared it were merged; the survivors use
distinct names).
- Removed the redundant inline `tokenSim.check()` calls from the old
`transfer_to_private` (the harness `afterEach` already checks).
## Preserved-assertion mapping (summary)
Every asserted behavior survives: each failure mode × entrypoint pair is
an explicit test; happy paths and event/balance/simulator assertions
stay explicit. The `CHECK_ALERTS`/Grafana `publishing_mana_per_second`
QoS guard from the old `transfer_in_public` moved to `transfer.test.ts`
(which now owns the public-transfer execution).
## Intentional changes (no assertion lost)
- Balance-unchanged checks: where an original asserted only the owner's
balance unchanged (private wrong-caller cases), the failure tests now
assert both the owner's and other's balances unchanged — a
strengthening, since `simulate` cannot move balances.
- Dropped one redundant `createAuthWit`+`authWitnesses` in the old
`transfer_in_public` over-balance-via-authwit case: the public authwit
alone is the authorization mechanism (the private witness was inert on a
public call); the U128-underflow revert + balances-unchanged assertions
are unchanged.
- For `transfer_to_public` over-balance / invalid-nonce, the recipient
is now `other` rather than self; the asserted error (Balance too low /
invalid nonce) is independent of the recipient.
## Verification
`yarn build`, `yarn format`, `yarn lint` clean. CI is the oracle for the
full matrix.
Round-2 e2e consolidation (PR 2 of 9), scoped to
`automine/contracts/**`, `automine/effects/**`,
`automine/accounts/scope_isolation.test.ts`, and
`automine/mempool_limit.test.ts`. All 8 touched files pass locally.
## Merges (old → new)
- `effects/{event_only.test.ts, custom_message.parallel.test.ts,
large_public_event.test.ts}` → `effects/events.test.ts` — one
`beforeAll` node, per-contract `describe` blocks. custom_message loses
its `.parallel` suffix (bodies are tiny; one container is a net win). 3
files → 1.
- `contracts/nested/{manual_private_call.test.ts,
manual_public.parallel.test.ts,
manual_private_enqueue.parallel.test.ts}` →
`contracts/nested/manual_calls.test.ts` — single `beforeAll` deploying a
shared Parent/Child; the public and enqueued-call suites redeploy a
fresh Child per test because each asserts an absolute post-increment
storage value that only holds from a zero start. 3 files → 1.
## Table / de-parallel conversions
- `accounts/scope_isolation.test.ts` — the `external private` /
`external utility` describes (structural duplicates differing only by a
`_utility` method suffix) collapsed via `describe.each`.
- `contracts/option_params.parallel.test.ts` →
`contracts/option_params.test.ts` (plain) + `it.each` over the
public/utility/private variants.
- `contracts/nested_utility_calls.parallel.test.ts` →
`contracts/nested_utility_calls.test.ts` (plain). See decision below.
## `nested_utility_calls` decision
The file has two describes with genuinely different setups — the first
uses a plain PXE, the second registers a custom
`pxeCreationOptions.hooks.authorizeUtilityCall`. Per-container isolation
was not the point (all bodies are fast `simulate()` calls with no
cross-test shared state; the hook describe already resets its state in
`beforeEach`), so I converted it to a single plain file with both
describes, each keeping its own `beforeAll` (two sequential node
bootstraps in one container — a CPU win over the previous 14 per-it
containers). Within the hook describe the deny/allow ×
utility/private/view sextet is now an `it.each`; the two default-deny
cases in the first describe are also tableized. The `msg_sender` and
note-sync tests stay explicit. Runs in well under the container timeout
locally.
## Added assertions for previously zero-assertion its
- `manual_calls` "performs a nested private call": now simulates
`parent.entry_point → child.value(0)` and asserts it returns the same
preimage as a direct `child.value(0)` call, in addition to sending the
tx for on-chain inclusion.
- `manual_calls` "performs public nested calls": now routes
`pub_entry_point → pub_inc_value(42)` on a fresh child and asserts child
storage is 42 (was assertion-free).
- `importer.parallel` (stays `.parallel`, titles unchanged):
`call_no_args` asserts the imported call returns the Test contract
address; `call_public_fn` and `pub_call_public_fn` assert the nullifier
landed by checking that re-emitting it on the Test contract is rejected
as a duplicate.
## Other
- `contracts/state_vars.test.ts`: extracted a file-local
`expectInitialized(interaction, expected)` helper replacing ~15 repeated
`is_*_initialized` simulate+expect blocks.
- `mempool_limit.test.ts`: removed the explicit no-op
`proverTestVerificationDelayMs: undefined` option.
## Dropped assertions
None. Every asserted behavior is preserved; the zero-assertion its
gained real assertions rather than losing any. Test discovery and the
compat suite are glob-based, so no `bootstrap.sh` / `.test_patterns.yml`
edits were needed.
Part of the e2e round-2 consolidation (PR 3 of 9): fewer CI containers
for `single-node/cross-chain/` with every asserted behavior preserved.
## Merges (old → new)
- `token_bridge_private.test.ts` +
`token_bridge_public.parallel.test.ts` +
`token_bridge_failure_cases.test.ts` → **`token_bridge.test.ts`**: one
`CrossChainMessagingTest` setup in `beforeAll` (with `startProverNode:
true`, covering all three), with `describe('private')`,
`describe('public')`, `describe('failure cases')`. The public file
previously re-ran the full harness setup per it via `beforeEach`.
- `l1_to_l2.parallel.test.ts` → **`l1_to_l2.test.ts`** +
**`l1_to_l2_inbox_drift.test.ts`**: `beforeEach` full setup converted to
a single `beforeAll` per file; the scenarios converted to
`it.each(['private','public'])` tables. Split by scenario (see timings
below): `l1_to_l2.test.ts` keeps the
duplicate-message-from-non-registered-portal scenario,
`l1_to_l2_inbox_drift.test.ts` holds the inbox-drift scenario (which
resets the `markProvenEnabled` gate between its via `beforeEach`).
- `l2_to_l1.parallel.test.ts` → **`l2_to_l1.test.ts`**: dropped the
`.parallel` suffix; the two message-count-shape its (`[3,4]` and
`[3,1,2]`) tableized via `it.each` with per-row consume tuples; the
reorg-and-remine, mixed private/public, no-message-tx, and
multi-block-checkpoint its kept explicit and behaviorally unchanged.
## Harness construction
- The `new CrossChainMessagingTest(...)` for every suite now lives as
the first statement of `beforeAll` rather than at `describe` scope, so
nothing is constructed at test-registration time. Registration-time data
(the `it.each` scope tables) is static, and test bodies read off `t`
lazily.
- The shared L1→L2 message helpers (`sendMessageToL2`, `advanceBlock`,
`waitForMessageFetched`, `waitForMessageReady`) used by both `l1_to_l2`
files are extracted to **`message_test_helpers.ts`**. Each suite plugs
in its own proving policy via a `markAsProven` dependency (unconditional
for the duplicate-message suite; gated for the inbox-drift suite).
## Container count
- Before: 14 CI containers (2 plain files + 12 `.parallel` per-it
containers), each paying a full node bootstrap.
- After: 4 containers / 4 setups. Same 17 its in total.
## Timings (why l1_to_l2 was split)
Measured from the last full CI run on this branch (commit `ba5e29b`):
- `token_bridge.test.ts`: 463s (~7.7 min)
- `l2_to_l1.test.ts`: 384s (~6.4 min)
- `l1_to_l2.test.ts`: 738s (~12.3 min) — over the ~10 min target, so
split
`l1_to_l2`'s four its broke down as: duplicate-message private 149s /
public 162s; inbox-drift private 194s / public 172s (setup + teardown
~59s). Splitting by scenario yields `l1_to_l2` (~6 min) and
`l1_to_l2_inbox_drift` (~7 min), both comfortably under 10 min, without
resorting to the `.parallel` suffix.
## Assertion mapping
- No assertions dropped. All 17 its and their asserted properties
survive; every `it` moved intact to exactly one file.
- Because the token-bridge its now share one node, absolute balance
assertions became delta-from-initial assertions (each it snapshots L1/L2
balances at its start). Same properties: deposit debits L1 by the bridge
amount, claim credits L2, withdraw restores L1, wrong claims revert with
the same error strings.
- `.test_patterns.yml` flake-pattern regexes repointed from the three
old bridge file names to `token_bridge`; the `l1_to_l2` prefix regex
already covers both new `l1_to_l2*` files.
- `bootstrap.sh`: 25m per-container timeout overrides for the four
cross-chain files (bodies run serially in one container each). The glob
picks up the new file names unchanged.
## Notes
- No docs `#include_code` references any of the renamed files, so no
docs changes.
- `single-node/README.md` still lists the old cross-chain file names (it
was already stale); left untouched to avoid conflicting with the sibling
consolidation PRs — follow-up sweep will refresh it.
#24492) Round-2 e2e consolidation, PR 4 of 9 (single-node merges). All merges preserve every asserted behavior; no assertions dropped. ## Merges (old → new) - `single-node/fees/public_payments.test.ts` + `single-node/fees/sponsored_payments.test.ts` → folded into `single-node/fees/private_payments.parallel.test.ts` as two unrolled its (plain string titles — the target is `.parallel`). All three ran the same FeesTest+FPC fixture; the shared `beforeAll` gains `applySponsoredFPCSetup()`, which is a PXE contract registration (the SponsoredFPC is already funded at genesis via `fundSponsoredFPC`), not an extra tx. The folded its compute their padded-max-fee gas settings locally, matching the originals. - `single-node/proving/empty_blocks.test.ts` + `single-node/proving/world_state_pruning.test.ts` + `single-node/misc/node_block_api.test.ts` → `single-node/proving/default_node.test.ts`: one context-default `setupWithProver({})` node, one describe per former file. Ordered so `world_state_pruning` (needs minTxsPerBlock:0 empty checkpoints) runs before `empty_blocks` (raises minTxsPerBlock). `node_block_api` queries the genesis block, which is setup-agnostic, so it drops its former bespoke PIPELINING setup. - `single-node/sequencer/slasher_config.test.ts` + `single-node/sequencer/sequencer_config.test.ts` → `single-node/sequencer/runtime_config.test.ts`: one `setupBlockProducer` carrying both suites' knobs (slasher inactivity config + maxL2BlockGas/manaTarget/bot account). The sequencer half keeps its `syncChainTip: 'checkpointed'` via explicit `pxeOpts` since `setupBlockProducer` defaults to 'proposed'. - `single-node/block-building/block_building.test.ts`: the 8-it double-spend suite is now an `it.each` table over `{firstKind, secondKind, sameBlock, expectedError}` (plain file, tables safe). Filename kept so the bootstrap.sh TIMEOUT=25m override and the `.test_patterns.yml` entry keep applying. All other its unchanged. ## Container count - Plain-file containers: −5 (public_payments, sponsored_payments, empty_blocks, world_state_pruning, node_block_api, slasher_config, sequencer_config deleted; default_node, runtime_config added). - `.parallel` per-it containers: +2 (the folded fee its), each far cheaper than the full plain-file fixture they replace. - its: 8 double-spend its → 1 `it.each` × 8 rows (same coverage, one title pattern). ## Folds skipped - `single-node/proving/cross_chain_public_message.test.ts` stays put: it is not default-compatible (`sequencerPublisherAllowInvalidStates: true`, `minTxsPerBlock: 1`, `numberOfAccounts: 1`), unlike the plan's assumption. ## Docs - `docs/docs-developers/docs/aztec-js/how_to_pay_fees.md`: `#include_code sponsored_fpc_simple` path repointed to `private_payments.parallel.test.ts`; the `docs:start/end:sponsored_fpc_simple` block moved byte-identical. ## Local runs - `runtime_config.test.ts`: 3/3 in 140s - `default_node.test.ts`: 3/3 in 183s - `block_building.test.ts`: 18 passed / 1 skipped (pre-existing manual-only skip) in 564s - `private_payments.parallel.test.ts`: 8/8 in 333s ## Deferred - `end-to-end/README.md` example command and `single-node/README.md` per-file blurbs still name the old files (README sweep is deferred per the round-2 plan; PR 5+ owns it).
…onstants (#24494) Round-2 e2e consolidation, PR 5 of 9 (workstream B: timing/config constants). Introduces two named timing constants in `single-node/setup.ts` and sweeps single-node/multi-node files onto them. ## Constants added - `NO_REORG_SUBMISSION_EPOCHS = 1024` — proof-submission window so large the chain never prunes/reorgs in the test's lifetime. Re-exported via `multi_node_test_context.ts` and the per-directory setup re-exports. - `PROVING_SLOT_TIMING = { ethereumSlotDuration: 4, aztecSlotDurationInL1Slots: 3 }` — the "12s floor" cadence previously hand-spelled (with near-verbatim derivation comments) in 5 proving/partial-proofs files. One shared doc comment now carries the derivation; the per-file copies are deleted. ## Pure renames (same effective value) - `aztecProofSubmissionEpochs: 1024` → constant: `setupBlockProducer`, `MOCK_GOSSIP_MULTI_VALIDATOR_OPTS`, `missed_l1_slot`, `multi_validator_node.parallel`. - `PROVING_SLOT_TIMING` adoption with unchanged values: `single_root`, `multi_proof`, `long_proving_time`, `upload_failed_proof`. ## Nominal value changes, same "never reorg" semantic (large window → 1024) - `640` → constant: `bot`, `fees/failures`, `fees/fee_settings`, `block-building/debug_trace`. - `1000` → constant: `proving/long_proving_time`, `proving/optimistic.parallel` (6 sites). - `128` → constant: `sequencer/gov_proposal.parallel`. These windows only exist to pin blocks against pruning; none of the tests run anywhere near the window, so 1024 is inert. Verified by build/lint plus sample runs (below). ## Effective timing changes (each run locally twice, both green) - 16s-cluster collapse onto `PROVING_SLOT_TIMING` (16s → 12s aztec slot): `proving/proof_fails.parallel` (218s/218s), `recovery/prune_when_cannot_build` (83s/83s). - Epoch one-off audit: `partial-proofs/multi_root` epoch 1000 → 32 (232s/233s), `recovery/manual_rollback` epoch 100 → 32 + `PROVING_SLOT_TIMING` + never-reorg window (58s/58s). No fallbacks were needed; all collapses held over both runs. Rename-only sample runs: `single_root` 66s, `long_proving_time` 191s, `upload_failed_proof` 51s — all green. ## Deliberately left as literals - Small windows that are production-like or test the window itself: `1` (`prune_when_cannot_build`, `l1-reorgs/setup`, `sync/synching`, `proof_boundary.parallel`), `2` (`cross-chain/l1_to_l2.parallel`), `15` (`escape_hatch_vote_only`, needed for a valid EscapeHatch config — already commented). - `single-node/l1-reorgs/**` values untouched per round-1 finding that their proof windows are sensitive to `ethereumSlotDuration`. ## Skipped (sibling-PR file ownership; literal sweeps deferred to a follow-up) - `single-node/cross-chain/**` (PR 3). - `single-node/fees/private_payments.parallel` (640), `single-node/block-building/block_building`, `single-node/proving/default_node`, `single-node/sequencer/runtime_config` (PR 4). - `multi-node/slashing/**` (PR 6): ~10 files with `1024` literals, incl. `slashing/setup.ts` and `inactivity_setup.ts`. - `multi-node/governance/setup.ts` (640), `multi-node/block-production/{setup.ts,blob_promotion}`, `multi-node/recovery/pipeline_prune`, `multi-node/invalid-attestations/**` (PR 7). - `p2p/**` (PR 8, incl. `SHORTENED_BLOCK_TIME_CONFIG_NO_PRUNES`'s 640), `composed/**` (PR 9), `automine/**` (PRs 1-2).
…24495) Round-2 e2e consolidation, PR 6 of 9 — scope: `multi-node/slashing/**`. Every asserted behavior is preserved; no assertions were dropped. ## Timing profiles (B3) The directory now uses exactly two named profiles in `slashing/setup.ts`, with zero free-floating local slot/epoch timing constants in the test files: - **`baseSlashingOpts`** (existing, eth 8s / aztec 24s) — the equivocation suites. - **`SENTINEL_TIMING`** (new: `{anvilSlotsInAnEpoch:4, listenAddress, ethereumSlotDuration:4, aztecSlotDuration:8, aztecEpochDuration:2, sentinelEnabled:true}`) — the fast 8s-slot timing. Every adoption below is a pure re-expression: the effective config values passed to `setup` are unchanged. | File | Profile | Effective value change? | |---|---|---| | `equivocation_offenses` (merged) | `baseSlashingOpts` | no (both source files already used it) | | `sentinel_status_slash.parallel` | `SENTINEL_TIMING` | no | | `validators_sentinel.parallel` | `SENTINEL_TIMING` | no | | `multiple_validators_sentinel.parallel` | `SENTINEL_TIMING` | no | | `slash_veto_demo` | `SENTINEL_TIMING` | no | | `broadcasted_invalid_block_proposal_slash` | `SENTINEL_TIMING` + `sentinelEnabled:false` | no | | `broadcasted_invalid_checkpoint_proposal_slash.parallel` | `SENTINEL_TIMING` + `sentinelEnabled:false` | no | | `data_withholding_slash` | `SENTINEL_TIMING` + `sentinelEnabled:false`, `aztecSlotDuration:12` override | no | | `attested_invalid_proposal.parallel` | `SENTINEL_TIMING` + `sentinelEnabled:false`, `aztecSlotDuration:36` + `committee:3` override | no | **Documented overrides:** `data_withholding` (12s slots for the tolerance window) and `attested_invalid_proposal` (36s slots so an AVM-heavy 3-block checkpoint fits one slot) keep their bespoke `aztecSlotDuration` as a documented one-knob override on `SENTINEL_TIMING`; their eth/epoch come from the profile. Non-sentinel offense files spread `SENTINEL_TIMING` and set `sentinelEnabled:false` (= the prior default), reusing only the fast timing. **Kept bespoke:** `inactivity_setup.ts` retains its CI-conditional timing (`eth = CI ? 8 : 4`, `aztec = eth*2`) — it is a shared fixture with a deliberate CI slot-doubling that `SENTINEL_TIMING`'s fixed 4s cannot express; the two inactivity test files themselves declare no timing constants. ## Merges - `duplicate_attestation.test.ts` + `duplicate_proposal.test.ts` → **`equivocation_offenses.test.ts`**: a shared `runEquivocationScenario` runner plus two describes (the two setups differ in `slashDuplicateAttestationPenalty` / `attestToEquivocatedProposals`, so each keeps its own `beforeEach`). - `inactivity_slash.test.ts` and `inactivity_slash_with_consecutive_epochs.test.ts` stay as separate files. They share the `InactivityTest` fixture (`inactivity_setup.ts`), but the fixture is stateful across epochs, so each file still needs its own `beforeAll` — folding them into one file with two describes would have grown the file without sharing any setup. ## Helpers / cleanups - Centralized `findSlashOffense` (and the `SlashOffense` type) in `setup.ts`, adopted by `attested_invalid_proposal` and `broadcasted_invalid_checkpoint_proposal_slash` (the latter's local offense-lookup wrappers now delegate to it, switching validator comparison from string to `EthAddress.equals`). - `slash_veto_demo`: **deleted** the dead `shouldVeto:false` branch (`it.each([[true]])` → plain `it`). No running assertion was dropped — the false branch's balance-decrease assertions were never exercised. - `sentinel_status_slash` its 1-2: hoisted onto a shared `runProposerFaultScenario` helper as thin unrolled its (stays `.parallel`, plain string titles, no `it.each`). ## Verification `yarn build`, `yarn format end-to-end`, `yarn lint end-to-end` all pass. Ran locally (green): `equivocation_offenses` (both its ×2), `slash_veto_demo`, `sentinel_status_slash` (3), `attested_invalid_proposal` (2), `broadcasted_invalid_checkpoint_proposal_slash` (3), `broadcasted_invalid_block_proposal_slash`, `data_withholding_slash`, `validators_sentinel` (4), `multiple_validators_sentinel`. `inactivity_slash` and `inactivity_slash_with_consecutive_epochs` cannot run in this local env — the `InactivityTest` fixture derives 0 blocks/checkpoint at the local `aztec=8` (default 3s blocks); confirmed the **unmodified base files fail identically**, and CI's `aztec=16` derivation is fine. Relying on CI for those files.
…ups (#24498) Round-2 e2e consolidation, PR 7 of 9. Pure dedup/refactor of multi-node test setups — no timing or config values change; every asserted behavior is preserved. ## What changed - **block-production/blob_promotion + recovery/pipeline_prune → shared `setupBlockProductionWithProver`.** Both files reimplemented the same 4-validator + prover + `WIDE_SLOT_TIMING` + `mockGossipSubNetworkLatency: 500` cluster locally (`setupBlobPromotion` / `setupTest`). Those local helpers are deleted and both now call the shared helper, which gained four opt-in options: `mockGossipSubNetworkLatency`, `maxTxsPerCheckpoint`, `clearInheritedCoinbase` (per-attester coinbase), and `disableCheckpointPromotionOnFirstNode`. `buildValidatorCluster` now accepts a per-index `nodeOpts` function so node 0 can be configured distinctly. **The resolved setup options and per-node config are identical to the originals** (same keys/values; existing callers `cross_chain_messages`/`deploy_and_call_ordering`/`proposed_chain` are unaffected because both new booleans default to false). One benign side effect: pipeline_prune now also installs `watchNodeSequencerEvents` via the helper — instrumentation only, no assertions consume it. (recovery → block-production cross-directory import, as permitted by the plan.) - **governance mechanics dedup.** Extracted `createGovernanceTestDriver` + `driveGovernanceRound` into `governance/setup.ts`, wrapping the `govInfo` / round-boundary warp / quorum wait / submit-winner / vote-through-delays mechanics duplicated by `add_rollup` and `upgrade_governance_proposer`. Both files adopt it; their scenario-specific parts (payload construction, node signaling, node migration + bridging in add_rollup, pre/post-execute assertions) stay in place. Each caller passes its own quorum timeout (`upgrade`: `quorumSize * aztecSlotDuration * 3`; `add_rollup`: `600`) so no timing value changes. The vote-success assertion now lives in the driver and covers both callers. ## Dropped assertions None. The `upgrade_governance_proposer` vote-success assertion is preserved (moved into the driver's `voteToExecutable`, where it now also covers `add_rollup`). ## Local test runs (all pass) - `upgrade_governance_proposer` — 1 passed (116s) - `block-production/blob_promotion` — 1 passed (293s) - `recovery/pipeline_prune` — 1 passed (367s) - `governance/add_rollup` — 1 passed (327s locally); also green in CI.
…ME (#24500) PR 8 of 9 of the e2e round-2 consolidation. Scope: `end-to-end/src/p2p/**` only. Dedup + docs; no timing/config value changes. ## DATA_DIR centralization - Every p2p file previously repeated `DATA_DIR = fs.mkdtempSync(...)` at module scope plus a per-node `fs.rmSync` cleanup loop in `afterEach`. That boilerplate now lives on `P2PNetworkTest`: `setup()` creates one root temp dir (`this.dataDir`), `dataDirFor(label)` hands out a nested per-role path (multi-node `createNodes` appends `-<index>`; single-node factories use it verbatim), and `teardown()` removes the whole tree in one recursive delete. - Adopted in all files (the 6 test files + `reqresp/utils.ts`); deleted the per-file `fs`/`os`/`path` imports, `mkdtempSync`, and cleanup loops. This also fixes a small pre-existing leak: the old `mkdtempSync` root dir was never removed (node data went into sibling `-N` dirs), whereas nodes now live under the root that teardown deletes. ## Gossip scenario skeleton (`shared.ts`) - `runGossipScenario(opts): Promise<AztecNodeService[]>` — the shared bootstrap→createNodes→mesh→account→(submit)→verify flow. Varying parts are options/callbacks: `numValidators`, `bootNodePort`, `txsPerNode` (0 skips submission), `submitSequentially`, `mesh` (topic/peer-count overrides), `checkpointSource` (`'first-tx'` | `'first-published'`), `beforeCreateNodes`, `createExtraNodes`, `beforeSubmit`, `afterVerify`. Returns the validator nodes for teardown tracking. - Extracted the repeated building blocks it uses: `verifyAttestationSigners(t, nodes, checkpoint)` (recovers signers, asserts each ∈ validator set, returns signers), `getPublishedCheckpointForTx(node, txHash)`, `waitForFirstPublishedCheckpoint(t, nodes)`, `waitForNodesToSync(t, nodes)`, and `maybeCheckQosAlerts(logger)` (the `CHECK_ALERTS` Grafana block that was copy-pasted in 4 files). ## Merge: `gossip_network_no_cheat.test.ts` → `gossip_network.test.ts` The two files shared the skeleton but genuinely differ in network bootstrap (cheat MultiAdder registration vs. on-chain `addL1Validator` CLI flow) and config, so they became two `describe`s in one file, each with its own `beforeEach` — the mesh is not re-spun per `it`. - `describe('cheat-registered validators')` — was `gossip_network.test.ts`. Effective node config unchanged: 4 validators, `startProverNode:false`, `SHORTENED_BLOCK_TIME_CONFIG_NO_PRUNES` + `aztecSlotDuration:24, aztecEpochDuration:4, blockDurationMs:10000, slashingRoundSizeInEpochs:2, slashingQuorum:5, listenAddress:127.0.0.1, inboxLag:2`; plus a p2p-only prover node and a re-execution monitoring node; proven-block assertion preserved. - `describe('on-chain-registered validators (no cheats)')` — was `gossip_network_no_cheat.test.ts`. Effective node config unchanged: 4 validators, `SHORTENED_BLOCK_TIME_CONFIG_NO_PRUNES` + `aztecSlotDuration:24, blockDurationMs:10000, minTxsPerBlock:0, listenAddress:127.0.0.1, inboxLag:2`; full-mesh wait on the proposal/checkpoint topics; all on-chain registration assertions preserved. `BOOT_NODE_UDP_PORT` default stays 4500 (now shared with the cheat describe, which honored the `BOOT_NODE_UDP_PORT` env override — the default is unchanged). Both describes run sequentially in one CI container reusing port 4500 (teardown fully frees ports before the next `beforeEach`). ## `fee_asset_price_oracle_gossip.test.ts` Kept as its own file, now on the shared skeleton with `txsPerNode: 0` (it submits no txs — empty blocks carry the price) and `checkpointSource: 'first-published'`. Effective node config unchanged: 4 validators, `startProverNode:false`, `SHORTENED_BLOCK_TIME_CONFIG_NO_PRUNES` + `aztecSlotDuration:12, aztecEpochDuration:4, slashingRoundSizeInEpochs:2, slashingQuorum:5, listenAddress, inboxLag:2`, plus a prover node (`minTxsPerBlock:0`). Both oracle-convergence rounds and final assertions preserved. ## Other files - `preferred_gossip_network.test.ts` adopted `verifyAttestationSigners` + `maybeCheckQosAlerts` (its signer-check block was identical, keyed off the `validators` array), plus the DATA_DIR change. Node config and the `expect(signers.length).toEqual(validators.length)` assertion unchanged. - `late_prover_tx_collection.test.ts`, `rediscovery.test.ts`, `reqresp/reqresp.test.ts` + `reqresp/utils.ts`: DATA_DIR change only; node config unchanged. - Added `p2p/README.md` (the missing category README) matching the `multi-node`/`single-node` structure: what belongs here (real-libp2p subjects — discovery, req/resp, mesh, peer auth, transport), what does not (consensus/slashing/sentinel → `multi-node`), the `P2PNetworkTest` entrypoint, the shared skeleton/helpers, and the post-merge file inventory. ## Behavior notes - No dropped assertions. - Two benign, non-asserting implementation changes, documented for reviewers: (1) in the no-cheat describe `setupAccount()` now runs before the first-checkpoint wait instead of after — both still precede tx submission, and `setupAccount()` only registers the account in the PXE (no on-chain effect), so the "checkpoint published before submit" invariant is preserved; (2) the merged submit path uses `waitForTxs(...)` for both describes rather than the no-cheat file's per-tx `Promise.all(waitForTx)` — same asserted property (all txs mined). ## Local runs (one file at a time; real-libp2p tests bind fixed ports) All green: | File / describe | wall | |---|---| | gossip_network — cheat-registered | 121s | | gossip_network — on-chain (no cheats) | 172s | | fee_asset_price_oracle_gossip | 120s | | rediscovery | 69s | | late_prover_tx_collection | 47s | | reqresp/reqresp | 121s | | preferred_gossip_network | 141s | CI selection is glob/regex-based (`src/p2p/**/*.test.ts`, `src/p2p/*.test.ts`, `.test_patterns.yml`'s `src/p2p/.*`), so deleting the merged-away file needs no CI-config edits.
#24503) Round-2 e2e consolidation, PR 9/9 — owns `composed/**`. ## Clock-skew suite stays on the composed HA Postgres cluster The four `Clock Skew and Timezone Safety` its in `composed/ha/e2e_ha_full.parallel.test.ts` exercise `PostgresSlashingProtectionDatabase` directly against the running 5-node HA cluster's **real dockerized PostgreSQL** slashing-protection DB: - TZ-independent duty timestamps (absolute-time storage) - `cleanupOldDuties` keeps recent duties when the node clock is 2h ahead - `cleanupOldDuties` deletes old duties by DB time even when the node clock is 1h behind - `cleanupOwnStuckDuties` keeps recent stuck duties when the node clock is 3h ahead The property under test is that these DB-clock semantics (`CURRENT_TIMESTAMP`, `timestamptz`) are immune to node clock/timezone skew — the node and its database must be able to genuinely diverge in clock and timezone environment for the assertion to mean anything. That only holds against a real, separate Postgres process. An earlier revision moved these into an in-process PGlite file; since PGlite runs inside the Node process, the DB and node can never actually diverge, so that variant is dropped. The tests keep riding the composed cluster's docker Postgres via the shared `HaFullTestContext` (`t.mainPool` / `t.dateProvider`), exactly as before; `dateProvider` simulates the skewed node clock while the DB uses its own clock. The `@electric-sql/pglite` dependency added for the PGlite variant is removed from end-to-end (other workspaces that use pglite for unit tests are unaffected). ## Order-dependent HA test → own file The `should distribute work across multiple HA nodes` test was annotated `must run last` because it permanently kills every node. Extracted verbatim into `composed/ha/e2e_ha_distribute_work.test.ts`, which shares the cluster setup with the remaining suite via a new non-test `ha_full_setup.ts` module (the ~380-line `beforeAll`/`afterAll`/helpers moved verbatim into an `HaFullTestContext` class). Result: - The killer test gets its own cluster; the ordering contract is gone (no hidden reliance on definition order). - It has a single `it`, so it is a plain `.test.ts` (not `.parallel`) and runs as one whole-file CI container. - The remaining `e2e_ha_full.parallel.test.ts` keeps its 3 non-destructive its (block production, governance voting, keystore reload) plus the clock-skew describe, all order-independent. - Zero CI runtime change: setup was moved, not rewritten, so behavior is unchanged. No assertions dropped. Added a `.test_patterns.yml` flaky entry for the new file mirroring the existing `e2e_ha_full` one (owner unchanged). ## Stranded composed tests — left in place, still excluded `composed/e2e_persistence.test.ts` and `composed/integration_proof_verification.test.ts` are excluded from every CI list and have run nowhere since April 2025. I moved each into a category dir, ran it, and reverted the move because both are broken on the current branch — root causes outside this diff's editable scope: | File | Disposition | Why | |---|---|---| | `integration_proof_verification.test.ts` | left in `composed/`, excluded, documented | Committed `fixtures/dumps/epoch_proof_result.json` is stale (last regenerated Feb 2026; circuits/VK changed since). bb and the on-chain HonkVerifier both reject the proof (`Failed to verify RootRollupArtifact proof!`). Needs the fixture regenerated; better relocated to the bb-prover circuit tests. | | `e2e_persistence.test.ts` | left in `composed/`, excluded, documented | `beforeAll` no longer completes: the single-node sequencer stalls in checkpoint proposal (`waitForAttestationsAndEnqueueSubmissionAsync`) and the 600s hook times out. The root cause is in shared setup/sequencer, not this file. | Each file's header comment and the `bootstrap.sh` exclusion now document the real reason (previously "excluded for unknown reasons"). **Decision items for follow-up:** regenerate the epoch-proof fixture and relocate `integration_proof_verification` to bb-prover; fix the single-node checkpoint stall and refile `e2e_persistence` under `single-node/`. ## Verified locally - `yarn build` (full), `yarn format end-to-end`, `yarn lint end-to-end` — all clean. - Confirmed both stranded tests fail on this branch (evidence above), which is why they stay excluded. - The compose/ha suites need docker (Postgres/Web3Signer) and were not run locally; the `e2e_ha_full` / `e2e_ha_distribute_work` split and the clock-skew describe rely on CI. Setup is moved verbatim and each `.parallel` it is already isolated per container, so runtime behavior is unchanged.
Supersedes #24449 and #24450, reimplementing both on a simpler design. Single PR since the lifecycle work is only exercised by the e2e primitive. ## Context Several multi-node e2e tests spend minutes of wall-clock waiting for the L1 clock to roll while live sequencers sit idle. Warping the shared clock under a running sequencer interrupts whatever iteration is mid-build, producing the reorg / "Sequencer was interrupted" failures behind earlier revert attempts. Pausing sequencers around the warp requires a lifecycle that can actually stop and resume, which it couldn't: a second `start()` orphaned the previous poll loop (two loops racing), and a restarted sequencer never cleared the publishers' `interrupted` flag, so it would build and propose blocks but silently never publish to L1. ## Approach **Idempotent, restartable lifecycle.** `start()`/`stop()` are idempotent across `SequencerClient`, `Sequencer`, and `PublisherManager`, and `start()` while `STOPPING` is refused so a mid-stop start cannot orphan a fresh loop. `PublisherManager.start()` clears the interrupted flag via the previously-unused `restart()` methods, and `SequencerClient.start()` starts the publisher manager before the sequencer loop so publishers are un-interrupted before the first post-restart publish. The funding loop is created once in the constructor and restarted across cycles, and a start that failed to load publisher state can be retried. **`pause()` for restarts.** Restartability is funneled through a dedicated `Sequencer.pause()` / `SequencerClient.pause()`: it halts the poll loop and waits for the in-flight `work()` iteration and every pending L1 submission / fallback send to finish *without interrupting them* — no interrupt lands mid-build, so no spurious `checkpoint-error` is emitted and no enqueued checkpoint is dropped. It deliberately does **not** stop inner services: the validator/HA signer and publishers keep running, so the slashing-protection store stays open and a later `start()` cleanly resumes. Draining happens without entering `STOPPING`, since in that state the iteration's own `setState` calls throw `SequencerInterruptedError`, which would fail the very iteration being drained. This replaces the wait-for-IDLE heuristic from #24450, which raced: sequencers reach IDLE at different times, and any of them could re-enter `work()` before the stop landed, flaking any test that asserts no sequencer failure events. **`stop()` stays a full teardown.** `stop()` keeps the fast interrupting shutdown used for production teardown, and stopping the validator client closes its slashing-protection database (LMDB single-node / Postgres HA), as it did before this line of work. Because restarts now go through `pause()` — which never closes the store — `stop()` no longer needs to leave the store open, so there is no DB-ownership bookkeeping to distinguish "owned" from "shared" databases. **Single shared request tracker.** The checkpoint proposal jobs' backgrounded L1 submissions and the sequencer's own fire-and-forget fallback sends (vote-and-prune when we cannot build, escape-hatch votes) are tracked in one `RequestsTracker` owned by the sequencer and handed to each job it creates. `stop()` interrupts and drains that single tracker in one place; `pause()` awaits it untouched. A sender sleeping until its target slot therefore cannot survive a `stop()` and publish a stale-slot tx after a restart clears the pooled publishers' interrupted flag: interrupting the wrapper publisher is permanent, since wrappers are never restarted. **e2e primitive + pilot.** `warpWithSequencersPaused(nodes, cheatCodes, target, opts)` on `SingleNodeTestContext` (inherited by `MultiNodeTestContext`) pauses every sequencer, runs the warp with nobody building, and resumes them (`restart: false` leaves them paused). Archivers, provers, and the chain monitor keep running, so clock-driven effects such as an orphan-block prune still fire. Applied to `pipeline_prune` to collapse its ~2-minute dead wait for the orphan slot's prune deadline: warp two L1 slots into the slot after the orphaned one, and resume the sequencers only once the prune is confirmed. `TX_COUNT` is unchanged, so `assertMultipleBlocksPerSlot` and `assertProposerPipelining` still hold. Lifecycle calls are assumed to be serialized by the caller (all current callers are); this does not add a mutex for truly concurrent start/stop/pause. The `pipeline_prune` speedup is validated by this PR's CI run — the multi-node suite can't be run locally.
PhilWindle
approved these changes
Jul 5, 2026
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
3 participants
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.
BEGIN_COMMIT_OVERRIDE
fix(sequencer): do not run fallback actions within build window (#24504)
fix(proposal-handler): ensure we default to pushing blocks to archiver (#24506)
fix(p2p): bump global reqresp limits (#24507)
fix(p2p): reject proposal when expected proposer is undefined (#24509)
fix(archiver): tolerate re-included already-stored checkpoints (#24513)
fix(p2p): canonicalize yParity attestation signatures before L1 bundle (#24515)
test(e2e): sync anvil clock before sequencer start (#24474)
test(e2e): consolidate automine token suites (#24489)
test(e2e): consolidate automine contracts and effects suites (#24490)
test(e2e): consolidate cross-chain bridge and messaging suites (#24491)
test(e2e): merge single-node fee, proving, and sequencer config suites (#24492)
test(e2e): name shared timing presets and collapse proof-submission constants (#24494)
test(e2e): unify slashing timing profiles and merge duplicate suites (#24495)
test(e2e): dedupe multi-node block-production and governance test setups (#24498)
test(e2e): extract p2p gossip scenario skeleton and add category README (#24500)
test(e2e): split node-killing HA test into its own composed suite file (#24503)
feat(sequencer): make sequencer pausable (#24475)
END_COMMIT_OVERRIDE