feat: merge-train/spartan-v5 - #25019
Merged
Merged
Conversation
Merges the current `v5-next` tree (`c11bc68`) back into `v5` and bumps the release-please manifest to `5.0.1` to cut the v5.0.1 patch release. - `.release-please-manifest.json`: `5.0.0` → `5.0.1` - Brings all commits on `v5-next` since `v5.0.0` onto `v5` (companion change: `v5-next` manifest bumped to `5.1.0` in `ee3716277a`). **Merge strategy:** this is a release-branch sync — merge so `v5` remains a true superset of the history (merge commit or fast-forward). A squash merge would collapse the whole tree into a single commit and rewrite the SHAs, so avoid it here. Tagging `v5.0.1` on the resulting release commit is a follow-up step.
…s executed A mainnet sequencer kept signalling a governance payload whose proposal had already been executed, wasting ~100k gas per slot on a signal the canonical rollup rejected. Three fixes: - Fix the `ProposalState` enum, which was missing `Droppable` (Solidity `IGovernance` has 9 states). The mismatch made Solidity `Expired` decode as out-of-range and throw in `asProposalState`, so the publisher failed open and signalled anyway. Add an explicit `LIVE_PROPOSAL_STATES` set for the sweep. - Replace `hasActiveProposalWithPayload` (boolean) with `getPayloadProposalStatus` returning `'live' | 'executed' | 'none'`, matching a proposal by its stored payload directly or via its GSEPayload wrapper. The publisher now stops signalling an executed payload (memoized in-process, live takes precedence over executed) unless `GOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE` is set, for payloads designed to be re-executed. - Add a canonicality guard: resolve the canonical rollup instance once and skip the signal when the configured rollup is not canonical, reusing that instance for round accounting and the EIP-712 digest so the read cannot race a canonical-rollup change.
…ser API Resolve the canonical rollup via getRollupAddress() instead of the getInstance() wrapper, dropping the now-unused instance parameter threaded through createSignalRequestWithSignature. Since the guard returns early when the configured rollup is not canonical, round accounting and the EIP-712 digest can just use the configured rollup address. Also drop the redundant sawExecuted flag from getPayloadProposalStatus (the executed verdict is already memoized in the set).
getL2ToL1MembershipWitness assembled its witness from several non-atomic archiver reads (getTxEffect, then the epoch blocks, target block and checkpoint metadata read inside computeL2ToL1MembershipWitness). A store commit landing between those reads could splice together two different chain states and surface a spurious "message does not exist" error even when the tx-effect index was healthy. Wrap the witness assembly in a single store.transactionAsync so every archiver read binds to one write transaction and observes a consistent snapshot (the store serializes writers, so no commit can interleave). The L1 Outbox roots fetch stays outside the transaction to avoid holding the archiver's writer lock across a network round-trip, and the tx effect is re-read inside the snapshot so the receipt's block number and tx index stay consistent with the block data. Add a kv-store test asserting reads inside a transaction see a consistent snapshot while a concurrent write is queued behind it.
…ed elsewhere BlockStore.removeBlocksAfter had two defects that corrupt the tx-effect index (txHash -> block position) when the same tx exists in two stored blocks, e.g. re-included after its original proposal expired: - deleteBlock removed #txEffects entries blindly by txHash, destroying the entry of a tx whose index already points at another stored block, which makes that block unreadable. - removeBlocksAfter skipped cleanup entirely for blocks it could not reconstruct, leaking their row, tx effects, and indices; a later insert at the same number then overwrites the row in place, leaving stale tx-effect entries pointing into the new chain. A stale entry makes getL2ToL1MembershipWitness resolve the tx at wrong coordinates and throw 'The L2ToL1Message you are trying to prove inclusion of does not exist' for a message in a proven block, and lets getTxReceipt report a proven position the chain no longer has. Cleanup now works from the raw storage row (so unreadable blocks are still fully released) and only deletes tx-effect entries still owned by the block being removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oval The ownership-checked delete silently skipped entries owned by another block. That state (two stored blocks sharing a tx) should be unreachable for honest chains, so skipping it silently hides direct evidence of an upstream bug. Warn on both anomalies (foreign-owned entry and missing entry) and correct the comment that attributed the duplication to routine proposal expiry, which cannot produce it.
The lazy KZG singleton (getKzg) builds its precomputation tables synchronously on first use, blocking the event loop for ~2s locally and 12-15s under production CPU limits. On an RPC node the first use is the archiver reconstructing blobs or the proposal handler uploading them, right after a checkpoint arrives, so the stalled loop overruns the gossipsub mcache window and attestation forwarding is skipped. Warm it in the node factory alongside the bb.js singleton, before any subsystem runs, keeping the cost off the gossip path.
Move the trusted-setup load timing and logging out of the node factory and into getKzg itself, gated on an optional logger argument, using elapsedSync for the measurement. Keeps the factory call site to a plain getKzg(log) and makes the timing available to any warm-up caller.
The socket backend gave bb a hard 5s budget shared between socket file creation and connect, so a loaded prover spawning many bb processes at once (epoch top-tree) could fail startup with 'Timeout connecting to bb socket: unknown' before a single connect attempt was made, and the resulting plain Error was reported retry=false, costing the epoch. The backend now waits as long as the bb process is alive, failing fast with the real cause if it dies, with a single generous 60s backstop for a wedged process. Startup failures are wrapped as retryable ProvingErrors and the bb_prover wrap sites preserve the retry flag.
Unpinned, typescript now resolves to 7.x, which no longer ships lib/_tsc.js and crashes Yarn 4's builtin compat/typescript patch during install, failing docs/examples/bootstrap.sh before any example runs. Matches the existing pin in docs/examples/ts/bootstrap.sh.
Promotes `v5-next` onto `v5` for the **v5.1.0** release. - Merges `v5-next` into `v5`; the resulting tree is **identical to `v5-next`** and `.release-please-manifest.json` is set to **5.1.0** (the only merge conflict was the manifest — v5 held 5.0.1, resolved to v5-next's 5.1.0). - Merge via **merge commit** (v5 ruleset allows `merge` only). - After this merges, `v5` HEAD is tagged `v5.1.0`. Does not include #24840 (json-rpc result key), which is still on `merge-train/spartan-v5` and not in `v5-next`.
…4939) ## Summary Retargeted to `merge-train/spartan-v5` per request. The matching `merge-train/spartan` port is [aztec-packages#24940](#24940). The merge-train merged Slack alert was reporting the PR head branch as the destination. For [aztec-packages#24851](#24851), the PR head was `merge-train/spartan-v5` but the actual base was `v5-next`, so the Slack message was misleading. ## Changes - Pass `github.event.pull_request.base.ref` to the merged notification step as `TARGET_BRANCH`. - Make `ci3/merge_train_failure_slack_notify --merged` prefer `TARGET_BRANCH`, with a `REF_NAME` fallback for direct/local use. ## Tests - `bash -n ci3/merge_train_failure_slack_notify` - `git diff --check` --- *Created by [claudebox](https://claudebox.work/v2/sessions/85cdf7ecc9727322/jobs/1) · group: `slackbot` · requested by Facundo Carreiro · [Slack thread](https://aztecprotocol.slack.com/archives/C0AU8BULZHC/p1784809391722469?thread_ts=1784809391.722469&cid=C0AU8BULZHC)*
…24802) Fixes the `Error: Timeout connecting to bb socket: unknown (retry=false)` failures reported by an operator since v5.0.0, which killed proving jobs during epoch top-tree and cost epochs. ## Root cause The NativeUnixSocket backend gave bb a hard 5s wall-clock budget that was **shared** between two phases: waiting for bb to create its socket file, and connecting to it. `connectWithRetry` reused the `startTime` captured before the file-wait poll loop, so when bb took close to 5s to create the socket (many bb processes spawning simultaneously during top-tree checkpoint/merge jobs, or the Node event loop starving the 50ms polls), the connect phase was entered with its budget already exhausted and threw without making a single connect attempt — that is what the `unknown` in the error message means (`lastErr` was never set). Two aggravating factors: - 5s is an arbitrary opinion about how fast a loaded machine should spawn a process. The pre-socket v4 CLI model had no such deadline — you waited on the child, and the only failures were real process events. - The resulting plain `Error` reached the proving agent, which only honours the retry flag on `ProvingError`s, so the failure was reported `retry=false` and the job failed permanently instead of being re-enqueued. ## Changes ### `barretenberg/ts` — socket backend restructure (`native_socket.ts`) - Replaced the constructor + deferred `connectionPromise` wiring with a `static async new()` factory, matching the shm and wasm backends. An instance can now only exist once connected, so `call()` no longer awaits a stashed connection promise (and is no longer `async` — its body has no awaits). - Startup is one flat wait-and-connect loop with the correct liveness condition: **retry for as long as the bb process is alive**. If bb dies, fail immediately with the real cause (exit code / signal); spawn failures are caught up front by awaiting the `spawn` event. Both 5s timers are deleted. - One generous 60s backstop remains for a bb that is alive but wedged before `listen()`. It kills the process (routing cleanup through the exit path) rather than leaving an orphan. It is a broken-process detector, not a performance expectation: the timed window ends at bb's `listen()`, which is reached after only exec + linking + minimal init (the expensive startup work comes after the socket is up), so firing it requires a machine degraded far beyond ordinary proving load. And if it ever does fire on a merely-distressed machine, the failure is retryable (see below), so the cost is a re-enqueue, not an epoch. - The four copy-pasted reject-pending-callbacks blocks (process error/exit, socket error/end) are consolidated into `failAllPending()`. Note one deliberate behavioural improvement: the socket `error`/`end` handlers now also destroy and null the socket, so subsequent `call()`s fail fast with `Socket not connected` instead of `write after destroy`. - New `native_socket.test.ts` covers: prompt startup, bb taking >5s to create its socket (the incident's failure mode — fails on the old code by construction, passes now), bb dying before the socket exists, and a nonexistent binary. ### `yarn-project/bb-prover` — make startup failures retryable - `BBJsInstance.create` wraps any `Barretenberg.new` failure as `ProvingError(..., retry: true)`: bb startup failures are environmental (machine load, wedged process), never a property of the proof inputs, so the job is always safe to retry. - The two catch sites in `bb_prover.ts` that re-wrap errors (`generateProof`, `verifyProof`) previously constructed a fresh `ProvingError` with the default `retry=false`, silently dropping the flag. They now propagate the inner error's retryability and attach it as `cause`. Errors that were non-retryable before remain non-retryable. The AVM paths don't wrap, so they needed no change. - New `bb_js_backend.test.ts` asserts a startup failure surfaces as a retryable `ProvingError`. ## Result for operators A loaded prover can take as long as it needs to spawn bb; a genuinely broken bb fails after 60s with a concrete, diagnosable cause instead of `Timeout ... unknown`; and if that ever happens the broker re-enqueues the job (up to its retry limit) instead of failing it permanently and costing the epoch. ## Testing - `barretenberg/ts`: new jest suite passes (including a 7s-delayed fake bb that the old implementation fails on). - `yarn-project`: full `yarn build`, bb-prover lint, and the new unit test pass. Root `./bootstrap.sh` green. ## Unrelated CI fix included `docs/examples/ts/aztecjs_runner/run.sh` installed `typescript` and `tsx` unpinned; the TypeScript 7.0.2 release (which drops `lib/_tsc.js`) crashes Yarn 4's builtin `compat/typescript` patch at install time, failing `docs/examples/bootstrap.sh execute` on every branch. Pinned to `typescript@^5.3.3` / `tsx@^4`, matching the existing pin in `docs/examples/ts/bootstrap.sh`. This was the only failure in this PR's first full CI run and needs forward-porting to the `next` line as well.
WorkerWallet.call unconditionally JSON.parse'd the transport response, but void-returning methods (registerContract, registerContractClass) come back as `undefined` from the worker. JSON.parse(undefined) throws `SyntaxError: "undefined" is not valid JSON`, which crashed registerSponsoredFPC during n_tps.test.ts setup and failed every nightly bench inclusion-sweep point (1/5/10 TPS) deterministically. Pass an undefined response straight to the method's schema, whose z.void() output accepts it, and widen callRaw's return type to string | undefined to match.
…kport to merge-train/spartan-v5] (#24961) ## Backport Backports the WorkerWallet fix from `next` ([#24843](#24843), commit `fc2d7444fc2`) onto `merge-train/spartan-v5`. Cherry-pick applied cleanly; the void-return fix hunks are byte-identical to the original, and the branch's own `registerAccount(signingKey)` signature was preserved untouched. ## Problem `WorkerWallet.call` unconditionally `JSON.parse`'d the worker's transport response. But void-returning methods — `registerContract`, `registerContractClass` (both `output: z.void()` in `WalletSchema`) — return nothing, and arrive at the client as the JS value `undefined`. `JSON.parse(undefined)` coerces to `JSON.parse("undefined")` and throws `SyntaxError: "undefined" is not valid JSON`. This crashed `registerSponsoredFPC` during `n_tps.test.ts` setup and failed every nightly bench inclusion-sweep point (1/5/10 TPS) deterministically, ~5ms into the test body. ## Fix Pass an `undefined` response straight to the method's schema (its `z.void()` output accepts `undefined`) instead of `JSON.parse`-ing it, and widen `callRaw`'s return type to `string | undefined`. Adds a `worker_wallet.test.ts` unit test covering the void-return path. Refs [#24843](#24843). --- *Created by [claudebox](https://claudebox.work/v2/sessions/29aeafd6e697c027/jobs/1) · group: `slackbot` · requested by Phil Windle · [Slack thread](https://aztecprotocol.slack.com/archives/C0AU8BULZHC/p1784891623684979?thread_ts=1784891623.684979&cid=C0AU8BULZHC)*
…24754) `getL2ToL1MembershipWitness` assembled its witness from several non-atomic archiver reads (`getTxEffect`, then the epoch blocks, target block and checkpoint metadata read inside `computeL2ToL1MembershipWitness`). A store commit landing between those reads could splice together two different chain states and surface a spurious "message does not exist" error even when the tx-effect index was healthy. Wrap the witness assembly in a single `store.transactionAsync` so every archiver read binds to one write transaction and observes a consistent snapshot (the store serializes writers, so no commit can interleave). The L1 Outbox roots fetch stays outside the transaction to avoid holding the archiver's writer lock across a network round-trip, and the tx effect is re-read inside the snapshot so the receipt's block number and tx index stay consistent with the block data. Cost: witness assembly briefly serializes with archiver writers (it holds the store's writer queue while it reads), but the work under the lock is pure store reads plus an in-memory tree build — no network I/O. Context: this came out of the A-1426 investigation. The incident symptom itself turned out to be a downstream client bug following a v5 behavior change in `L2BlockSynchronizer` — not this race, and not archiver index corruption. This PR is therefore hardening against a real but so-far-unobserved torn-read window, not the incident fix. See #24765 for the companion store hardening from the same investigation. Related to A-1426
…24764) ## Context A mainnet sequencer kept casting governance signals for a payload whose proposal had already been executed: the outer Multicall3 tx succeeds but the inner `signalWithSig` reverts (swallowed by `allowFailure`), wasting ~100k gas per slot. Two client-side bugs allowed this: executed proposals were treated like any other terminal state, so the payload was re-signalled every round; and the TS `ProposalState` enum was missing `Droppable` (present in `IGovernance.sol` since July 2025), so decoding an `Expired` proposal threw, and the publisher failed open and signalled anyway. Additionally, since the executed payload upgraded the canonical rollup, proposers of the old rollup kept signalling on a `GovernanceProposer` now keyed to a different instance, where their signals always revert. ## Approach - Fix the `ProposalState` enum to match `IGovernance.sol` (9 states including `Droppable`) and drive the sweep from an explicit `LIVE_PROPOSAL_STATES` set. - Replace the boolean `hasActiveProposalWithPayload` sweep with `getPayloadProposalStatus` returning `'live' | 'executed' | 'none'`. The publisher stops signalling a payload whose proposal was executed, unless the new `GOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE` config is set (L1 permits re-proposing an executed payload, and some payloads are designed to be re-executed). Live proposals take precedence over executed ones, executed verdicts are memoized in-process, and the sweep remains a bounded lookback capped by the protocol-wide proposal lifetime. - Add a canonicality guard to `enqueueCastSignalHelper`: resolve the canonical rollup via `getRollupAddress()` and skip the signal when the configured rollup is not canonical. Only the canonical rollup's current proposer can signal, so a non-canonical node would otherwise waste gas on a reverting signal. Because the guard returns early on a mismatch, round accounting and the EIP-712 digest use the configured (verified-canonical) rollup address. - The proposal-status guard fails open on RPC errors — at worst a duplicate signal the contract simply counts alongside others in the round — so a flaky L1 endpoint cannot silence governance participation. A failure to resolve the canonical rollup instead skips the signal for that slot and is retried on the next one. ## API changes `ReadOnlyGovernanceContract.hasActiveProposalWithPayload` and the `GovernanceProposerContract` wrapper are replaced by `getPayloadProposalStatus(payload)`. The redundant `GovernanceProposerContract.getInstance()` wrapper is removed in favour of the existing `getRollupAddress()`. New optional sequencer config `governanceProposerForcePayloadVote` (env `GOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE`, default `false`). To be forward-ported to `merge-train/spartan` after landing on the v5 line. Fixes A-1422
…eck tx-effect deletes (#24765) ## Problem Two defects in `BlockStore` block removal could corrupt the tx-effect index (txHash → owning block hash/position) if the store ever held two blocks sharing a tx: 1. **Blind delete** — `deleteBlock` removed `#txEffects` entries by txHash without checking the entry still pointed at the block being deleted. If another stored block contained the same tx, that block's entry was destroyed collaterally, making the block unreadable (`Could not find tx effect for tx…`). 2. **Skip-on-unreadable leak** — `removeBlocksAfter` did `getBlock(bn) === undefined → warn + continue`, so an unreadable block's row, tx effects, and indices were never released. A later `addCheckpoints` overwrites the leaked row in place ("L1 data is authoritative"), leaving stale tx-effect entries pointing at coordinates now occupied by different txs — `getTxReceipt` then reports positions the chain no longer has, and `getL2ToL1MembershipWitness` resolves the wrong tx and throws `The L2ToL1Message you are trying to prove inclusion of does not exist`. This is hardening, not an incident fix. No honest code path produces two stored blocks sharing a tx: duplicate nullifiers are rejected at block building (double-spend validation against the build fork), at validator re-execution, and at world-state sync (nullifier leaves are non-updateable). The incident that prompted the investigation turned out to be a downstream client bug following a v5 behavior change in `L2BlockSynchronizer`, unrelated to the store. The store should still not amplify a duplicated-tx state into silent corruption if it ever appears — e.g. via a future upstream bug, or byzantine tx effects that repeat a txHash with distinct nullifiers (invisible to every nullifier-based check). ## Fix - `removeBlocksAfter` cleans up from the raw storage row, so blocks whose bodies can no longer be fully loaded still release their row, tx effects (enumerated from `#blockTxs`), and hash/archive indices. - `deleteBlock` only deletes a tx-effect entry still owned by the block being removed (compares the entry's leading block hash). - Both anomalies now log a warning (an entry owned by another block, or a missing entry). These states should be unreachable, so a warning in production logs is direct evidence of an upstream bug worth investigating — previously the ownership skip was silent, hiding exactly that signal. ## Cost The added reads are confined to prune/reorg paths: one `#blockTxs` read plus one tx-effect point read per tx of each *removed* block (roughly doubling point reads on block removal). Block ingestion and query paths are untouched. ## Verification - Regression test from #24732: two proposed blocks sharing a tx effect; asserts removal releases both blocks fully with no residue (fails on the pre-fix implementation). - Full `block_store.test.ts` suite passes (160 tests). Recreates #24732 with the original commit cherry-picked and authorship preserved (thanks @benesjan). On top of it, this PR updates the description and code comments to match what the follow-up investigation established about how a duplicated-tx state can and cannot arise, and adds the anomaly warnings. Supersedes #24732. Related to A-1426
## Context An RPC node logged `Gossip validation for checkpoint_attestation took 15533ms, approaching mcache eviction window of 8400ms` shortly after startup. Investigation (see A-1425) showed the validations were not slow — the Node.js event loop was fully blocked for 12–15s, and the attestations were queued behind it. The blocking call is `getKzg()`, which lazily builds the `@crate-crypto/node-eth-kzg` precomputation tables synchronously (~2s locally, 12–15s under the pod's CPU limits) on first use. On a plain RPC node that first use is the archiver reconstructing blobs during sync, or the proposal handler uploading blobs right after a checkpoint arrives — both immediately adjacent to gossip traffic, so the stalled loop overruns the gossipsub mcache window (8.4s) and attestation forwarding is skipped. Sequencers already avoid this via `Sequencer.init()`, so only non-sequencer nodes were affected. ## Approach Warm the KZG singleton in `createAztecNodeService`, right after the existing bb.js WASM singleton init and before any subsystem runs. The cost is unchanged (every full node reconstructs blobs and pays it eventually) — it just moves off the gossip path to a point where blocking the loop is harmless. `getKzg()` is an idempotent per-process singleton, so this is a no-op for sequencers and for tests that pre-warm via `warmBlobKzg` (all e2e setups do), and adds no work for pure unit tests, which never construct a full node. Fixes A-1425
…l for failure upload (A-1216) (#24983) ## What Stops the prover-node from holding every checkpoint's `Tx` objects in memory for the whole proof-submission window. - Drops `CheckpointProver.txs` (an instance map that was populated at gather and never cleared). `executeCheckpoint` now consumes the gathered txs locally and lets them go out of scope. - The only reader of the cached txs *after* execution was failure upload. `SessionManager.buildProvingData` now re-fetches each checkpoint's txs from the tx pool (via a new `CheckpointProver.getTxsForUpload`, concurrently across checkpoints) instead of reading the cached map. It becomes `async`, and the two `prover-node` upload call sites `await` it. ## Why A live heap snapshot of a prover-node under load showed thousands of retained input `Tx` objects (client proofs + public inputs) — one set per registered `CheckpointProver`, held until the 100-epoch submission window expired. The tx pool already stores these txs durably on disk (kept until L1 finality, with A-1274's retention margin covering a lagging prover), so the in-memory copy is pure duplication of data the pool holds. Re-fetch is safe: at failure time the txs were used for proving moments ago, so they are almost always still resident in the local pool (reqresp covers the rest); the A-1274 margin guarantees the durable case. Re-fetch is best-effort — a tx the pool can no longer supply is logged and omitted rather than failing the post-mortem, since a partial snapshot is still useful. Note the rerun/replay path (`rerun-epoch-proving-job.ts`) is unaffected — it rebuilds from the already-uploaded `jobData.txs`, not from live prover state. ## Testing - `yarn build`, full `@aztec/prover-node` suite (184/184). - New tests: a `CheckpointProver` caches no txs and re-fetches from the pool on demand; failure upload rebuilds complete `EpochProvingJobData` by re-fetching. ## Depends on A-1274 (tx-pool retention margin behind finality) — guarantees the pool keeps txs long enough for a lagging prover's failure-upload re-fetch.
…ool (#25000) ## Problem When the block builder flags a tx as failed, it hard-deletes it from the local mempool via `handleFailedExecution`. That is terminal for the tx — it will never be retried by that node — but the **reason** is not visible at the log level nodes actually run. There are two ways a tx lands in `failed`: - **Execution threw** — already logged at `warn` (`Failed to process tx <hash>: <error>`). - **Pre-process validation rejected it** — logged at **`debug`** only (`Rejecting tx <hash> due to pre-process validation fail: <reason>`). The second is the common case: `createTxValidatorForBlockBuilding` runs timestamp, phases, block-header, double-spend and gas checks before execution, and a rejection there is fast and silent. The drop itself (`Dropping failed txs <hashes>`) is `verbose`, and the pool-side confirmation (`Deleted N failed txs`) is `info` but carries hashes with no reasons. Net effect on a node running at `info`: transactions disappear from the mempool with no recorded cause. This came up while investigating mainnet transactions that sat unincluded — the logs showed `Processed 0 successful txs and 6 failed txs` immediately followed by `Deleted 6 failed txs`, and the actual rejection reason for those txs was unrecoverable from production logs. ## Change - `public_processor.ts` — pre-process validation rejection moves from `debug` to `warn`, with `txHash`, `reason` and `blockNumber` as structured context. - `checkpoint_proposal_job.ts` — `dropFailedTxsFromP2P` moves from `verbose` to `warn` and now includes the per-tx reason (`fail.error.message`) alongside the hash, plus `slot` / `checkpointNumber` context. - `automine_sequencer.ts` — same change to its copy of `dropFailedTxsFromP2P`, so local/dev runs behave the same. After this, every tx removed from the mempool by the builder leaves a `warn` line naming the tx and why. ## Trade-offs This raises log volume during a period of mass tx invalidation — e.g. after a reorg, when many txs are rejected with `Block header not found` in a short window. That is deliberate: those are exactly the windows where the reason matters most, and the alternative is losing the information. The drop path already deletes in batches, so it is one line per batch, not one per tx. ## Not included `invalid_txs_after_reorg_rule.ts` logs `Evicting N txs from pool due to referencing pruned blocks` with no tx hashes and no structured context, so eviction victims are still unnamed. That is a separate path from the builder's failed-tx drop and is left alone here. ## Testing The repo in this container has no `node_modules` and the `noir` submodule is not checked out (`yarn install` fails resolving the `@aztec/noir-acvm_js` portal dependency), so a full `yarn build` was not possible locally — CI is the verification for compilation. The edits were checked statically: `this.globalVariables` is already used for the same `blockNumber` context at `public_processor.ts:410`, `this.targetSlot` / `this.checkpointNumber` are existing fields on `CheckpointProposalJob`, and `FailedTx.error` is typed `Error`. No behavior outside logging is touched. --- *Created by [claudebox](https://claudebox.work/v2/sessions/b14cb9eb6114c0bf/jobs/4) · group: `slackbot` · requested by Santiago Palladino · [Slack thread](https://aztecprotocol.slack.com/archives/C0AU8BULZHC/p1785158724697589?thread_ts=1785158724.697589&cid=C0AU8BULZHC)* --------- Co-authored-by: Santiago Palladino <santiago@aztec-labs.com>
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
8 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
chore(release): v5.0.1 (#24737)
chore(release): merge v5-next into v5 for v5.1.0 (#24897)
fix(ci): report merge train merged alerts against the base branch (#24939)
fix: don't time out bb socket startup while the bb process is alive (#24802)
fix(bench): handle void worker-wallet returns in inclusion sweep [backport to merge-train/spartan-v5] (#24961)
fix(archiver): resolve L2-to-L1 witness from a single store snapshot (#24754)
fix(sequencer): stop signalling already-executed governance payloads (#24764)
fix(archiver): clean up removed blocks from raw rows and ownership-check tx-effect deletes (#24765)
fix(node): warm KZG trusted setup at startup (#24775)
feat(prover-node): stop caching checkpoint txs; re-fetch from the pool for failure upload (A-1216) (#24983)
fix(sequencer): log tx failure reason at warn when dropping from mempool (#25000)
END_COMMIT_OVERRIDE