feat: merge-train/spartan-v5 - #25156
Merged
Merged
Conversation
Gossip tx validation waits on the tx pool serial queue (canAddPendingTx / addPendingTxs). handleFinalizedBlock occupied that queue for the entire finalization of an epoch's worth of mined txs (hydrate + deserialize + archive + delete), stalling validation for 10-40s on mainnet nodes. - Split finalization into chunk-sized serial-queue items so gossip pool operations interleave with finalization. - Archive raw proof-stripped buffers instead of deserializing and re-serializing every tx. - Add per-stage timing instrumentation to gossiped tx validation, queue wait/execution metrics to the tx pool serial queue, and IVC metrics to the peer BatchChonkVerifier. - Scrape and log the new metrics in the spartan TPS benchmarks.
… cursor leak Finalization now runs as chunk-sized serial-queue items (prepare, archive, delete, complete) so gossip-driven pool operations interleave instead of waiting for an entire epoch's worth of mined txs. The archive copies raw proof-stripped buffers instead of deserializing and re-serializing each tx. Also fixes a latent deadlock in TxArchive: getHeadIndex/getTailIndex called .next() once on entriesAsync and abandoned the generator. Inside a write transaction the committed-state iterator is unbounded, so the abandoned generator never sent CLOSE_CURSOR, permanently leaking one of the store's cursor semaphore slots per call and deadlocking the store after 8 archive transactions.
…6 repro Revert this commit before merging.
…hive - deleteFinalizedTxs re-checks each tx is still mined at or before the cutoff before deleting, since other pool operations may interleave between the finalization plan being computed and the delete chunk. - archiveTxBuffers skips txs already archived, so a retried or crashed finalization cannot append a duplicate FIFO index entry whose eviction would delete the stored value out from under the newer entry. - handleFinalizedBlock chains concurrent finalizations so their chunked queue items never interleave with each other.
PR #25032 added a top-level import of standard-contracts/artifacts-historical/HandshakeRegistry-5.0.1.json, but the release image dockerignore only whitelists artifacts/, so every node started from a v5-line release image crash-loops at ESM link time with ERR_MODULE_NOT_FOUND. This broke all spartan network deploys from v5-next since 2026-07-29.
…or A-1656 repro" This reverts commit 4815488.
…#25155) Since #25032, `@aztec/standard-contracts` has a top-level import of historical artifact JSONs from `standard-contracts/artifacts-historical/`. The release image dockerignore ignores everything and then whitelists specific paths, and it only whitelisted `standard-contracts/artifacts/` — so the historical artifacts never made it into the image. - Every node built from a v5-next release image crash-loops on startup at ESM link time with `ERR_MODULE_NOT_FOUND` for `HandshakeRegistry-5.0.1.json`. - This has broken all spartan deploys from this line since 2026-07-29. - The fix is a single whitelist entry: `!/yarn-project/standard-contracts/artifacts-historical/`. Split out from #25148 so the deploy fix can land independently of that PR's review. Once this merges, #25148 will be rebased and its duplicate commit will drop out.
AztecBot
enabled auto-merge
August 11, 2026 18:16
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 failed status checks
Aug 12, 2026
Port of #25195 to `merge-train/spartan-v5`. Same two commits, cherry-picked clean (no conflicts); the lockfile hunks applied by context onto v5's own `yarn.lock`, which also already resolved `snappy@7.2.2`. 1. **Pin `snappy` to an exact `7.2.2`** in `@aztec/p2p`, `@aztec/blob-client` and `@aztec/end-to-end` (was `^7.2.2`), with the matching descriptor update in `yarn-project/yarn.lock`. No resolved version changes. 2. **Bound the local-network startup wait** in the `aztec-up` release tests. ## Why (1) `snappy@7.4.0` was published upstream on 2026-08-12 at 13:36:40 UTC and is broken on **every** platform: - `main.js` (the `.` export) does `require('./index.js')` expecting the NAPI-RS-generated native binding loader; - the published `index.js` is instead the **wasm/browser** entry — `import * as s from '@napi-rs/snappy-wasm32-wasi'`; - and `7.4.0` **dropped** `@napi-rs/snappy-wasm32-wasi` from its `optionalDependencies`, so that package is never installed. So any `require('snappy')` throws: ``` Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@napi-rs/snappy-wasm32-wasi' imported from .../node_modules/snappy/index.js ``` Reproduced on x86_64 with the correct native `@napi-rs/snappy-linux-x64-gnu` installed; `7.3.3` and `7.2.2` are fine. Not an architecture or missing-binary problem — all 17 platform packages for 7.4.0 were published. `yarn-project/yarn.lock` already resolved `7.2.2`, so the monorepo's own builds and tests were never exposed. But the **published** manifests carried the floating `^7.2.2`, and `aztec-up/bin/0.0.1/install` does a fresh, unlocked ```bash npm install @aztec/aztec@$VERSION @aztec/cli-wallet@$VERSION @aztec/bb.js@$VERSION --prefix "$version_path" ``` which resolved to `7.4.0` as soon as a CI box re-primed its offline Verdaccio cache after the publish. Pinning the dependency range (rather than adding a `resolutions` entry) is the lever that matters, because it is the *published* manifest that the fresh install reads. Revisit the pin via [A-1709](https://linear.app/aztec-labs/issue/A-1709/revisit-snappy-version-pin). ## Why (2) When the CLI died on startup, `amm_flow`, `basic_install` and `bridge_and_claim` each sat in ```bash while ! curl -fs localhost:8080/status &>/dev/null; do sleep 1; done ``` with no bound, until the 15-minute per-test timeout killed them. That is what turned a one-line dependency breakage into a ~49-minute CI leg that blew its SSM budget (exit 124) and knocked PRs out of the merge queue — reporting "timeout" rather than the actual error. New `aztec-up/test/utils.sh::wait_for_local_network <pid> [timeout=300]`: - returns non-zero **immediately** if the backgrounded `aztec start --local-network` process is gone, pointing at the real error above it in the log; - otherwise caps the wait at 300s, comfortably above any healthy startup, so a hung-but-alive node fails in 5 minutes instead of 15. ## Testing - (1) is a no-op at the resolved-version level: `7.2.2` is what the lockfile already pinned and what every build already used. CI here exercises the `aztec-up` suite, which is the path that was failing. - (2) `bash -n` on all four scripts, plus a harness over the helper covering the three paths: process exits before ready → fails in ~2s with the diagnostic; alive but never ready → honours the cap; service comes up → returns 0 immediately. --- *Created by [claudebox](https://claudebox.work/v2/sessions/547965cc2e7d127d/jobs/6) · group: `slackbot` · requested by Santiago Palladino · [Slack thread](https://aztecprotocol.slack.com/archives/C0BFRKFSLMP/p1786545464281799?thread_ts=1786545464.281799&cid=C0BFRKFSLMP)*
When the p2p client starts while there are blocks to sync, the libp2p service start is deferred to `startServiceIfSynched`, which runs inside the block stream event handler. That handler catches and only logs errors, so a TCP bind failure (e.g. `ERR_NO_VALID_ADDRESSES` after a restart) was swallowed. On top of that, the sync promise awaited by node creation was resolved before the service was started, so startup succeeded regardless and the node kept running with a dead p2p stack and zero peers. The service is now started before the sync promise settles, and a failure logs at fatal level and rejects the stored sync promise (rather than throwing, which the block stream would eat) so node creation fails and the process exits, letting the orchestrator restart it. Part of A-1701. --- Related PRs from the same incident (all independent, all targeting `merge-train/spartan-v5`): #25177 (fail startup when the p2p service fails to start), #25185 (connectivity signal + slasher/proposer/health/sendTx gates), #25183 (periodic zero-peers warning).
) A validator could run for hours with a dead libp2p stack - zero peers and zero gossip - without anything in its logs flagging the condition. The failure was only reconstructable after the fact from the absence of activity, rather than from any explicit signal. The peer manager heartbeat now tracks consecutive zero-peer heartbeats. After 3 of them (so startup and transient dips stay quiet) it logs a warning that the node has no connected peers and can neither gossip nor propagate txs, with the zero-peer heartbeat count, the heartbeat interval, time since a peer was last connected, and the cached peer count. The warning then repeats about once a minute, derived from peerCheckIntervalMS, instead of on every heartbeat. When peers come back, a single info log reports connectivity restored and the new peer count. Connected peers are counted via libp2p getPeers(), matching what getP2PConnectivity reports. A peer count gauge already existed (PEER_MANAGER_PEER_COUNT, recorded in discover()), so no metric was added - this change is logs only. Part of A-1701. --- Related PRs from the same incident (all independent, all targeting `merge-train/spartan-v5`): #25177 (fail startup when the p2p service fails to start), #25185 (connectivity signal + slasher/proposer/health/sendTx gates), #25183 (periodic zero-peers warning).
…25202) Validators were aborting checkpoint proposals on eth-mainnet with a generic `L1RpcError: L1 RPC request failed` during pre-broadcast header validation, e.g. `insufficient funds for gas * price + value: have 169461140054989709 want 432138191814787072`. With this PR we now skip any balance checks during simulate calls for header validation by removing fee-per-gas settings, and also just in case we fake a lot of ETH as balance. Created A-1712 to gate block-building on publisher balance. ## Context `L1TxUtils.simulate` attached production fee fields (competitive P75 priority fee, ~26 gwei during the incident) together with the worst-case `MAX_L1_TX_LIMIT` gas cap (16.7M) to every `eth_simulateV1` call, and `validateBlockHeader` overrode the multicall3 sender's balance with the validator's real balance. Supplying fee fields makes the node enforce the EIP-1559 upfront funds check (balance >= gasLimit x maxFeePerGas, ~0.43 ETH at those numbers) before executing anything, so any validator holding less than that failed header validation and skipped its proposal — even though the actual publish tx costs a small fraction of that. With fee fields omitted, the node defaults them to zero, so the check only fired because we supplied them. ## Approach - Omit `maxFeePerGas`/`maxPriorityFeePerGas` from simulated calls entirely, making the upfront check vacuous for all `simulate()` callers. - `validateBlockHeader` now always uses the ample 10 ETH multicall3 balance override (previously fisherman-mode only) instead of the real sender balance, as compatibility with providers that still apply an upfront check. - While at it, the third commit renames `SequencerPublisher.validateBlockHeader` to `validateCheckpointHeader`: it takes a `CheckpointHeader` and simulates the rollup's `validateHeaderWithAttestations`, so it never validated a *block* header. Mechanical rename of the method, its `trackSpan` label, and all call sites and doc references; contract-side names, the `header-validation-failed` event key, and metric labels are untouched. - An insufficient-funds rejection of the simulation request (code -38014, with a message-match fallback for clients and gateways that report it differently) now surfaces as a descriptive error carrying the sender and the provider's have/want message, instead of a generic RPC error. - `_simulate` is restructured so only the `simulateBlocks` transport call sits inside the classifying try/catch; decoding of failed calls and the success path happen after it. ## What to review - **The core semantic claim**, verified against client and library sources: viem 2.38.2's `simulateBlocks` forwards call objects verbatim (no fee filling, no type inference — unlike `simulateCalls`/`sendTransaction`, so re-verify on a viem bump), and geth, reth, anvil (>= v1.1.0), and nethermind all default omitted fee fields to a zero gas price and zero the block base fee under `validation: false`. The upfront funds check still runs in all four but degenerates to `balance >= value`; every call we simulate is zero-value, so it passes regardless of sender balance. Nothing in the simulated paths reads `tx.gasprice`. All four production `simulate()` callers were audited (`validateCheckpointHeader`, `simulateInvalidateCheckpoint`, the multicall aggregate helper, L1 contract deploy simulations): they consume only `gasUsed`/return data, both unaffected by gas price. - **The insufficient-funds classifier keeps a message fallback deliberately**: geth reports the rejection as spec code -38014, but reth uses -32003 (TransactionRejected) and anvil did too before its spec-conformance fixes, so matching on the code alone would miss those clients. - **The catch boundary in `readonly_l1_tx_utils.ts`** (second commit): only the RPC request is inside the try, so an execution revert whose message mentions "insufficient funds" (e.g. a plain `Error("insufficient funds")` revert string) can no longer be misclassified as an RPC rejection — this was a real bug in the first commit, caught in review and pinned by the new regression test. Also check the `MethodNotFound`/`fallbackGasEstimate` early-return path survived the restructure unchanged. - **Loss of signal**: dropping the real-balance override removes an (unintentional) affordability check. It never carried a real signal — wrong sender (multicall3), worst-case gas cap, inflated fees — and no other affordability check exists today: publisher selection only requires balance > 0, with the optional funding loop and balance metrics covering the rest operationally. An explicit post-simulation cost-vs-balance warning is a candidate follow-up. - **Tests are correct by inspection only** — this branch has not been built or run locally; CI is the first real execution. A port to `next` will follow; it needs the `getGasPrice` -> `getFeesPerGas` rename accounted for. Fixes A-1706
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 failed status checks
Aug 13, 2026
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
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.
BEGIN_COMMIT_OVERRIDE
fix: include standard-contracts historical artifacts in release image (#25155)
END_COMMIT_OVERRIDE