Skip to content

fix(p2p): gossip tx validation stalls behind tx pool finalization (A-1656) - #25148

Open
spalladino wants to merge 6 commits into
merge-train/spartan-v5from
spl/a-1656-p2p-gossip-validation-stall
Open

fix(p2p): gossip tx validation stalls behind tx pool finalization (A-1656)#25148
spalladino wants to merge 6 commits into
merge-train/spartan-v5from
spl/a-1656-p2p-gossip-validation-stall

Conversation

@spalladino

@spalladino spalladino commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Fixes A-1656 (mainnet nodes logging `Gossip validation for tx took 20000+ms, approaching mcache eviction window of 8400ms`).

Root cause

Gossip tx validation waits on the tx pool's single serial queue (`canAddPendingTx` / `addPendingTxs`). `handleFinalizedBlock` occupied that queue as one item for the entire finalization of an epoch's worth of mined txs — hydrating each tx from LMDB, deserializing it, re-serializing it into the archive, and deleting it. Mainnet log analysis shows the slow-validation spikes cluster exactly on the once-per-epoch finalization tick (every 38.4 min, ~100–200 txs per tick); the worst spike (43s) coincides with the snapshot pause of archiver/world-state sync.

Reproduced in a unit test (tx_pool_v2_finalization.test.ts): finalizing 128 mined txs of ~1.1MB each stalls a concurrent canAddPendingTx for 3.9s on a local NVMe machine with archiving enabled — on 0.5–2 vCPU cloud nodes with network disks the observed 10–40s is consistent.

Chonk verification is ruled out: the batch verifier shows no pile-up (queue wait avg 6ms, max 172ms on next-net), and a prove-n-tps-real bench run from this PR pushed 891 real-proven txs at ~1 TPS through real node-side chonk verification with zero slow-validation warnings (tx validation ~170–440ms throughout).

Fixes

  • Chunked finalization: handleFinalizedBlock now runs as chunk-sized serial-queue items (prepare → archive per chunk → delete per chunk → complete), so gossip pool operations interleave with finalization. Post-fix, the repro scenario stalls canAddPendingTx for 1ms (was 3.9s). Hardened per review: delete chunks re-check each tx is still mined at or before the cutoff, archiving is idempotent (a retried finalization can no longer create a duplicate FIFO index entry whose eviction corrupts the newer one), and concurrent finalizations are chained.
  • Raw-buffer archiving: the pool already stores txs proof-stripped, which is exactly the archive format — the archive now copies buffers as-is instead of Tx.fromBufferwithoutProof()toBuffer() per tx. Total finalization time drops 3.9s → 395ms in the repro.
  • Latent store deadlock: TxArchive.getHeadIndex/getTailIndex called .next() once on entriesAsync and abandoned the generator. Inside a write transaction the committed-state iterator is unbounded (WriteTransaction#iterate does not pass the limit down), so the abandoned generator never sent CLOSE_CURSOR, permanently leaking one of the store's maxReaders - 1 cursor-semaphore slots per call — deadlocking the whole store (and with it the pool serial queue) after 8 archive transactions. Now consumed via for await.
  • Release image packaging (v5 line): standard-contracts/artifacts-historical/ was missing from the release image dockerignore whitelist, so every node started from a v5-line release image since fix(pxe): support contracts built with the v5.0.1 handshake registry #25032 (Jul 29) crash-looped with ERR_MODULE_NOT_FOUND — this broke all spartan network deploys from v5-next.

Instrumentation

  • Per-stage timings for gossiped tx validation (deserialize, stage1_setup, stage1, pool_precheck, proof_verify, pool_add), attached to the slow-validation warn log and exported as aztec.p2p.gossip.tx_validation_stage_duration.
  • Tx pool serial queue wait/execution histograms per operation plus queue length gauge (aztec.mempool.tx_pool_v2.queue_*).
  • The peer BatchChonkVerifier now records the standard IVC verifier metrics (it previously recorded none).
  • The spartan TPS benchmarks (n_tps, n_tps_prove) scrape and log all of the above at the end of a run.

Network-level validation (ci-network-bench runs on this PR)

With the fix, finalization queue items execute in ~10ms and no slow validations cluster on finalize ticks; canAddPendingTx/addPendingTxs wait ~10–19ms P95 across all TPS scenarios. The remaining serializer surfaced by the new metrics is handlePrunedBlocks (~57s on the queue during a reorg, 24 slow validations) — tracked separately as A-1668.

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.
@spalladino
spalladino requested a review from charlielye as a code owner August 7, 2026 23:59
@spalladino spalladino added the ci-network-bench Run Spartan network benchmarks (TPS + proving) on this PR label Aug 7, 2026
@AztecBot AztecBot added the port-to-next Forward-port this merged PR into next label Aug 7, 2026
…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.
@AztecBot AztecBot removed the ci-network-bench Run Spartan network benchmarks (TPS + proving) on this PR label Aug 8, 2026
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.
@spalladino spalladino added the ci-network-bench Run Spartan network benchmarks (TPS + proving) on this PR label Aug 8, 2026
@AztecBot AztecBot removed the ci-network-bench Run Spartan network benchmarks (TPS + proving) on this PR label Aug 8, 2026
spalladino added a commit that referenced this pull request Aug 10, 2026
…#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

port-to-next Forward-port this merged PR into next

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants