feat: merge-train/spartan-v5#24454
Merged
Merged
Conversation
…failures (#24426) ## Problem In the docs-examples CI job (`docs/examples/bootstrap.sh execute`), `example_swap` and `recursive_verification` exhausted all 5 retry attempts on the same signature during `.send()` fee estimation: ``` Error: An unknown error occurred while executing the contract function "getTimestampForSlot". args: (54) Details: L1 RPC request failed code: -32702 ``` Stack: `sendTx -> completeFeeOptions -> getMinFees -> aztecNode.getPredictedMinFees / getCurrentMinFees` (over JSON-RPC to the node). The failing reads are node-side, in `FeeProviderImpl.computeCurrentMinFees` (`getTimestampForSlot` / `getManaMinFeeAt`) and `FeePredictor.fetchState`. Failing run: http://ci.aztec-labs.com/66ef70e6044f13bd (branch `merge-train/spartan-v5`, commit `0e1827b945`). ## Root cause `FeeProviderImpl.getCurrentMinFees` and `FeePredictor.getState` cache a promise keyed on the L1 block number, refreshing only when the block advances. When the cached computation rejects (a transient `L1 RPC request failed` blip against the single 1-CPU anvil the docs-examples job runs), the rejected promise is replayed on every subsequent call at the same L1 block. `getCurrentMinFees` was worse: it chained the next computation via `currentMinFees.then(() => computeCurrentMinFees())`. Once `currentMinFees` is rejected, `.then(onFulfilled)` propagates the rejection and never invokes `computeCurrentMinFees` again — so the cache stays poisoned permanently, regardless of L1 block advancement, until the node restarts. Because fee estimation fails before any L1 tx is submitted, no new L1 block is mined by that path, so a single blip permanently wedges `.send()` on the node. The CI log confirms this: after one transient failure on `getManaMinFeeAt`, `getTimestampForSlot(54)` failed identically across all 5 retries of both examples — even though the node kept mining L1 blocks (148-152) for the examples' own contract deploys in the same second. This rules out a live node/anvil stall and pinpoints the poisoned promise cache. ## Fix - `getCurrentMinFees`: chain off the previous promise's settlement (via a swallowing `.catch`) rather than its fulfillment, so a prior rejection no longer short-circuits the new computation; and reset the cached L1 block number when the computation rejects so the next call recomputes at the same block. - `getState` (FeePredictor): reset the cached L1 block number on rejection for the same reason. Both are idempotent read paths, so recomputing on the next call is safe. ## Testing - Added unit tests to `fee_provider.test.ts` and `fee_predictor.test.ts` that fail (red) against the current code and pass (green) with the fix: a transient rejection followed by a call at the same L1 block must recompute rather than replay the cached rejection. - Red/green verified locally: both new tests fail on the unpatched source and the full `sequencer-client` fee suites pass (13 tests) with the fix. - A deterministic docs-examples repro was not feasible (it depends on a transient L1 RPC blip under CI contention), so the tests target the poisoning mechanism directly.
…24441) Fixes a genuine timing flake in `single-node/proving/proof_fails.parallel.test.ts` › `does not allow submitting proof after epoch end`, observed in CI run `99ed463db48b9cf2` on the v5 line (`Expected: 2, Received: 0` at the final epoch assertion). This test ran into a situation where the proposer `prune`d the unproven chain without `propose`ing a new checkpoint, so the check for "has the last checkpoint been mined in epoch 2" failed, since there was _no checkpoint at all_ in the entire chain. This fixes it so we wait until there's actually a checkpoint mined. In parallel, I'm looking into _why_ the proposer decided to prune without proposing.
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Jul 2, 2026
## Motivation Cleanup finishing #24404's work. That PR adopted shared wait helpers across the e2e suite but deliberately deferred 17 `// REFACTOR:` wait-code markers as harder or ambiguous cases. This resolves all of them, leaving zero `// REFACTOR:` markers under `end-to-end/src`. ## Approach - Adopt a shared helper where one genuinely fits. - Extract single-use / hard-to-adapt polls into descriptively-named local functions in the same file, then delete the marker. - Delete stale markers whose referenced code no longer exists. All changes are behavior-preserving refactors: comparator direction, timeouts (with seconds-vs-ms conversions where a helper's unit differs), the `retryUntil` falsy-means-keep-polling contract, and side effects (loops that send txs / advance blocks / rotate the oracle keep doing so every tick) are all retained. ## API changes New/changed test-only helpers (in `@aztec/ethereum` test utils and the e2e fees harness): - `ChainMonitor.waitForCheckpoint(match, opts)` gains an opt-in flag. When set, it takes a fresh `run()` snapshot and short-circuits if the current checkpoint already satisfies `match` before attaching the event listener. This closes an event-vs-poll already-satisfied race: a checkpoint could advance during a preceding wait and be missed by the purely event-driven path. The existing caller is unchanged. - New `RollupCheatCodes.waitForCheckpointBelow(checkpoint, opts)` — a poll-only waiter (mirroring `waitForEpoch`/`waitForSlot`) that reads the L1 rollup contract until the pending checkpoint drops below a baseline (rollback detection). No node dependency. - `snapshot_sync` now adopts the existing `ChainMonitor.waitUntilCheckpoint` (which has the already-satisfied guard) instead of a hand-rolled poll. - New `FeesTest.waitForEpochProven()` encapsulates the recurring `advanceToNextEpoch()` + `catchUpProvenChain()` pair (adopted in `failures` and `private_payments`).
…4448) Round of low-risk e2e test-suite speedups that touch only test fixtures and test-support code (no production behavior change). ## Approach - Parallelize the four independent setup boot steps (telemetry client, shared blob storage, ACVM config, BB config) in `setupInner` — they write disjoint config keys, so `Promise.all` is safe. - Parallelize the per-node `submitTransactions` loops in the gossip / preferred-gossip / rediscovery P2P tests (`Promise.all` preserves index order; `gossip_network_no_cheat` deliberately left serial). - Replace the per-tx full Schnorr account deployment in `submitTxsTo` with register-only `TestContract.emit_nullifier` throwaway txs (`#[noinitcheck]`, so no on-chain deploy) — same tx count, much lighter per-tx proving and DA across the P2P and slashing throwaway-tx paths. - Add an opt-out flag to `ChainMonitor` fee-data polling (default preserves current behavior) so slot-heavy tests can skip 5 rollup reads per L1 block.
…encer nonce race (#24386) ## Root cause `e2e_token_bridge_tutorial_test` flakes with `WaitForTransactionReceiptTimeoutError: Timed out while waiting for transaction … to be confirmed` ([example](http://ci.aztec-labs.com/1643e817d7242e0e)). It is **not** caused by #24378 — that PR only touches `forge_broadcast.js` (the L1 *contract deploy*), which succeeds in the failing run; the timeout is ~3.5 min later on one of the test's own L1 txs. The test runs against `aztec start --local-network`, whose `AutomineSequencer` continuously publishes checkpoint txs to L1. In `local-network.ts` the sequencer-publisher and validator keys are derived from `DefaultMnemonic` (`'test test … junk'`) at **address index 0** — `0xf39Fd6…` (visible as the publisher in the log). The test's L1 client was created with the **same** mnemonic and the **same** default index 0: ```ts const l1Client = createExtendedL1Client(ETHEREUM_HOSTS.split(','), MNEMONIC); // → account index 0 ``` So the test and the node's sequencer send L1 txs from one account, sharing a single nonce sequence. Against an automining anvil they race: a test tx can land with a nonce the sequencer just consumed (dropped / "already known"), or with a future-nonce gap that automine won't mine until the gap fills. Either way the tx gets a hash but never confirms, and viem's per-tx confirmation timeout fires. This is intermittent because it only bites when a test tx and a sequencer publish overlap. ## Fix Derive the test's L1 client from a **different** address index so its nonce space is independent of the sequencer's: ```ts const l1Client = createExtendedL1Client(ETHEREUM_HOSTS.split(','), mnemonicToAccount(MNEMONIC, { addressIndex: 1 })); ``` Index 1 (`0x70997970…`) is funded by anvil's default mnemonic and is unused by the local-network node (publisher/validator = index 0; index 2 is the prover and 3+ are attesters by the existing `setup.ts` / `setup_p2p_test.ts` convention, none of which run in local-network). The test is fully self-contained on its own L1 account — it deploys and owns the `TestERC20`, `FeeAssetHandler` and `TokenPortal`, and bridges to/from `l1Client.account.address` — so nothing requires it to be account 0. All of the test's L1 txs (including the `L1TokenManager` / `L1TokenPortalManager`, built from `l1Client`) now go through index 1. ## Verification This is an `compose` e2e test that needs docker + a full network; the container here is a source-only checkout (no `node_modules`, no docker), so I could not run the test or `./bootstrap.sh ci` locally — flagging that explicitly per the repo's red/green policy. The fix is verified by analysis: the timed-out tx is one of the test's own L1 txs, the shared-account nonce race is the established cause of this exact viem symptom, and account index 0 vs 1 are the well-known distinct anvil accounts (idx 0 = `0xf39Fd6…`, the publisher in the log; idx 1 = `0x70997970…`). ## Relationship to #24385 #24385 broadened this test's `.test_patterns.yml` flake pattern to keep CI from going red on the residual flake. This PR removes the underlying cause. The flake entry can stay as a safety net, or be narrowed back to the jest-timeout-only form once this has soaked — happy to follow up either way. --- *Created by [claudebox](https://claudebox.work/v2/sessions/c01cbe3dd422bf00) · group: `slackbot`*
## What Lands the **inclusion sweep** on the v5 line — a nightly benchmark that deploys a live network and drives sustained **1 / 5 / 10 TPS** mixed-priority load, measuring transaction inclusion under load. Supersedes the stacked PRs **#24260** and **#24265**. ## Changes **Benchmark harness & output** - Run JSON **schema v5** + scraper: proving-infra (hint-gen + proving-queue), per-role `saturation` (ELU / CPU / mem / event-loop delay), per-pod node resource sizing, RPC ingress duration, per-tx block-build decomposition, a pool-side pending→mined delay, and validator attestation-failure metrics (the signal upstream of quorum loss). `run.benchmarkType` lets the dashboard group a day's sweeps by kind. - **`inclusionTpsMean`** is a steady-state rate (warmup ramp + drain tail trimmed), so `headlineKpi ≈ 1.0` on healthy runs. - **`reorgCount`** counts genuine chain reorgs only — events where the whole validator set pruned; a single/subset divergence is reported separately as `nodeLocalPruneCount`. Adds `prune_count` `prune_type` attribution for cause breakdown. - `throughputTps` (gossip / rpc / mined funnel) and `p2pBandwidthBytesPerSec` series. **Node-side metrics (aztec image)** - Accurate pending→mined delay (`now − receivedAt` at the mined transition, no eviction conflation), widened mined-delay histogram buckets, and `prune_count` attributed by `prune_type` so pending-chain reorgs are countable from metrics instead of logs. **Deployment / CI** - `bench-inclusion-sweep.env` (12×4 validators, 10 RPC, 10 full nodes, 72s slots, fake proving) + guaranteed-QoS resource profiles so every node gets reserved cores on an adequately-sized instance. - `nightly-bench-inclusion-sweep.yml`: deploy once, run 1/5/10 sequentially (own namespace each, shared sweepId), scrape-on-failure, GCS upload. Retires the single-point `nightly-bench-10tps` and the obsolete `bench_10tps` scenario. ## Validation (smoke network, all fixes applied) | TPS | Mined / failed | Chain reorgs | Checkpoint publish | Client incl. P50 / P95 | |---|---|---|---|---| | 1 | 291 / 0 | 0 | 14/14 | 9.1s / 20.8s | | 5 | 1595 / 0 | 0 | 15/15 | 12.3s / 28.2s | | 10 | 3261 / 0 | 0 | 14/15 | 19.5s / 41.9s | Under-provisioned validators (bin-packed onto 2-core nodes) were the original reorg cause: attesters couldn't re-execute proposed blocks fast enough to hold quorum. The dedicated-core profiles fix it at the root — at 10 TPS, 13–15 reorgs → 0 and ~57% → ~93% checkpoint publish. The few single-node divergences seen under load self-healed with 0 tx loss and now count as `nodeLocalPruneCount`, not chain reorgs. ## Follow-ups (not in this PR) - **Dashboard** (`AztecProtocol/explorations`): renders the v5 schema (companion PR #22). - The **nightly image** must contain the node-side metric/telemetry fixes before the nightly build picks them up. - Prover-node hint-gen (single-threaded) and attester block re-execution are the next throughput ceilings above 10 TPS — observable now, not blockers here. --------- Co-authored-by: AztecBot <tech@aztec-labs.com>
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 3, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Jul 3, 2026
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
Collaborator
Author
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
spalladino
pushed a commit
that referenced
this pull request
Jul 6, 2026
## Problem `l1_tx_utils.test.ts` flaked in CI ([log](http://ci.aztec-labs.com/390c881d64e094b6), seen on [#24454](#24454 (comment)), which is unrelated to the failure). The failing test was `monitors all sent txs`: ``` ● L1TxUtils › L1TxUtils with blobs › monitors all sent txs TimeoutError: L1 transaction 0x82cf...aa3a timed out at TestL1TxUtils.monitorTransaction (l1_tx_utils/l1_tx_utils.ts:601:11) ``` ## Root cause The test creates the monitor promise and only attaches its rejection expectation after several intervening awaits: ```ts const monitorPromise = gasUtils.monitorTransaction(state); await sleep(100); await cheatCodes.mineEmptyBlock(); await expect(monitorPromise).rejects.toThrow('timed out'); // handler attached here ``` Anvil timestamps have 1s granularity, so when the wall clock crosses a second boundary between the send and the monitor's timeout check, the monitor (with `txTimeoutMs: 200`, `checkIntervalMs: 100`) can observe the L1 timestamp already 1s past `sentAt` and reject **before** `mineEmptyBlock()` completes. In the failing run the timeout fired at `16:41:20.355`, before the empty block was mined at `.356` and before the `.rejects` handler attached at `.357`. Node emitted `unhandledRejection` in that window and jest 30 reported it as the test failure — which is why the failure stack has no test-file frame and the test's own assertions all passed in the log. The same file already uses the safe idiom in a dozen newer tests: attach the handler synchronously with `.catch(err => err)` and assert via `resolves.toBeInstanceOf(TimeoutError)`. ## Fix Apply that established pattern to the three remaining tests that expect a monitor timeout but attach the handler only after intervening awaits: - `stops trying after timeout once block is mined` - `attempts to cancel timed out transactions` - `monitors all sent txs` (the observed flake) Test-only change; no product code touched. `TimeoutError` is exactly what `monitorTransaction` throws on timeout, so the assertions are equivalent-or-stricter than the previous message matching. Evidence: CI log http://ci.aztec-labs.com/390c881d64e094b6 (search for `monitors all sent txs`). --- *Created by [claudebox](https://claudebox.work/v2/sessions/0959caa9f03f59fa) · group: `slackbot`*
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
4 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): recover fee-estimation caches from transient L1 read failures (#24426)
test(e2e): fix epoch-2 tip race in proof_fails after-epoch-end test (#24441)
test(e2e): resolve remaining wait-code REFACTOR markers (#24428)
perf(e2e): parallelize setup & p2p work and lighten throwaway txs (#24448)
fix(e2e): give token_bridge_tutorial its own L1 account to avoid sequencer nonce race (#24386)
feat(bench): Updated tx inclusion benchmarking (#24266)
END_COMMIT_OVERRIDE