fix(p2p): treat unverifiable tx proofs as unknown, not invalid by spalladino · Pull Request #383 · aztec-labs-eng/aztec-node

spalladino · 2026-10-02T20:16:45Z

Stacked on #359 (which is stacked on #354). Merge those first.

Problem

A client proof that the node could not check (bb crashed, OOM, timed out, its FIFO broke, the verifier was stopped) has been reported as an invalid proof. Every consumer then acts on that verdict:

Peer disconnects. Gossip rejects the tx with a LowToleranceError (enough on its own to disconnect), and the reqresp batch requester penalises every peer that serves it. A node whose bb is down penalises its way off the network.
Slash votes. An embedded proposal tx that "fails" becomes invalid_embedded_txs, which is slashable. The node votes to slash the proposer and marks the slot invalid, so the attesters of that slot get ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL votes. One node alone can't reach quorum, but a correlated failure (load-induced OOM, a bad bb release) can.
Cache poisoning. TxValidationCache kept the result, rejections included, so one transient failure stuck to the tx until LRU eviction, for both reqresp and proposal validation.

#354/#359 make the verifiers throw ProofVerifierUnavailableError instead. Throwing alone doesn't fix this, because gossip and reqresp treat a thrown validation as a failure too.

The rule here: a proof is invalid only when a verification ran to completion and answered false. Anything else is unverifiable.

Changes

stdlib: TxValidationResult (and its zod schema) gains { result: 'unverifiable'; reason }, with the error text TX_ERROR_PROOF_UNVERIFIABLE. Adds the non-slashable UnverifiableBlockProposalTxsError.
Verifiers (bb-prover):
- BBCircuitVerifier returns valid: false only for a completed verified === false. Any thrown error from verifyChonkProof is now ProofVerifierUnavailableError, not just retry: true ones. The TS side already rejects malformed proofs before bb sees them: ChonkProof deserialization enforces the field count. The one gap is the empty placeholder proof, which both verifiers now reject as invalid up front, without asking bb.
- BatchChonkVerifier takes the same approach. Every rejection path (stopped, dead or rebuilding verifier, send failure, per-request timeout, missing VK index) surfaces as ProofVerifierUnavailableError. A verdict only comes from a FIFO result. A FAILED result whose message says bb threw is also unavailable.
- Adds an aztec.ivc_verifier.unavailable_count metric, recorded by QueuedIVCVerifier and BatchChonkVerifier.
- TestCircuitVerifier gets an outcome (valid, invalid, unavailable). The default is still valid.
TxProofValidator: valid: false → invalid. Any thrown error → unverifiable.
AggregateTxValidator: stops at the first invalid and returns it. An unverifiable result does not stop the run, so an invalid from any validator wins regardless of order. The first unverifiable is returned only when no validator found the tx invalid.
TxValidationCache: stores one wrapped promise and evicts it when it resolves unverifiable or rejects, but only while the map still holds that exact promise. Callers already waiting on it still get its result. Genuine invalid verdicts are still cached. This covers both the proof key and the aggregate integrity-check key.
Gossip: an unverifiable stage result is Ignore with no penalty. fix(bb-prover): rebuild the batch verifier after its bb dies #359's catch for ProofVerifierUnavailableError is removed: TxProofValidator already turns every verifier throw into unverifiable, so the catch could never be reached. Within a stage, an invalid from another validator still wins.
Reqresp batch requester: an unverifiable tx (or a thrown validation) is not penalised, not marked fetched, and does not count as the peer redeeming itself. It stays missing and is re-requested. To bound rework against inputs that deterministically break bb, a tx that fails to verify MAX_UNVERIFIABLE_ATTEMPTS_PER_TX (3) times in one run stops being requested in that run, still without penalty.
Block proposals: validateTxsReceivedInBlockProposal throws InvalidBlockProposalTxsError when any tx is invalid. Otherwise, if any tx is unverifiable, it throws UnverifiableBlockProposalTxsError. Unverifiable txs are never added to the pool as protected. ProposalHandler maps the new error to txs_unverifiable. That reason is classified non-slashable and counted as a node-issue metric, so it never reaches slashInvalidBlock or markInvalidProposalSlot. It fails fast with no retry until the deadline, for three reasons: that deadline is the attestation deadline, a single batch attempt can take up to 5 minutes, and the tx provider's Promise.all keeps sibling work running.
RPC: isValidTx returns the unverifiable variant as its contract. sendTx refuses such a tx with a retry-later error, not Invalid tx.
Other consumers handle the variant explicitly:
- The tx collection sources (node RPC, file store) report unverifiable hashes separately from invalid ones.
- The public processor skips such a tx rather than failing it, so it isn't dropped from the pool.
- The tx pool ignores it on add (rather than rejecting it) and neither restores nor deletes it on revalidation.
- None of these production validators check proofs today.
foundation: LruMap.peek, a lookup that doesn't refresh recency, used by the cache eviction.

Known limitation

bb exposes only OK/FAILED per proof. Its batch verifier catches a batch-check exception and returns false, and bisection then reports it as a per-proof failure ("batch check failed (bisected to individual)"). So some internal bb errors still look like invalid proofs. This PR classifies the threw messages visible in the bb binary (reduce_to_triple_ipa_opening threw: …, ChonkBatchVerifier: result callback threw: …). The rest need an upstream VerifyStatus::ERROR in barretenberg.

Testing

The whole stack builds locally against the pinned bb.js (6.0.0-nightly.20261001): make yarn-project, then yarn build. yarn lint is clean on the touched packages.

New and updated unit tests, all passing:

bb-prover bb_verifier, batch_chonk_verifier (17)
p2p tx validators, cache, libp2p_service, batch tx requester, tx collection, tx provider, tx pool v2 (923)
validator-client proposal_handler and validator (183)
aztec-node server (115)
simulator public_processor (17)
foundation lru_map (15)

The new tests cover:

each verifier outcome in TxProofValidator
cache eviction: concurrent waiters, a replacement before settlement, and both the proof key and the aggregate key
gossip Ignore with no penalty, against Reject with a penalty
the requester: no penalty, a later re-collection, and giving up at the cap
txs_unverifiable being non-slashable, with no slash event and the slot not marked invalid
sendTx and isValidTx

Red/green: the new p2p, validator-client and bb-prover tests fail against the code from before this PR (11 p2p, 2 validator-client, 2 bb-prover). The gossip "verifier throws" case already passed thanks to #359.

ivc-integration batch_verifier_queue.test.ts (26 tests) passes with real bb against the changed BatchChonkVerifier. Corrupted proofs come back as reduction failed, so they stay invalid and are not classified as bb errors.

Not run: e2e and multi-node tests, and a live run where bb is killed mid-verification.

Fixes A-2276

A pooled bb instance whose process died was returned to the pool and handed out again, so every later borrow of it failed. The pool now checks BBJsApi.isAlive(): a dead instance is destroyed when it is returned or found idle, and a replacement is spawned when a borrower finds no idle instance while the pool is below its size. A failed spawn fails that borrow and is retried by the next one, and a failed pool initialization is retried too. Requires a bb.js release with Barretenberg.isAlive().

…nvalid proof BBCircuitVerifier.verifyProof turned every error into { valid: false }, so a bb process that died was reported as an invalid transaction proof and counted as a verification failure. A call that fails because its bb died is now retried once on another pooled instance; if that bb dies too, or no instance can be started, verifyProof throws ProofVerifierUnavailableError. RPC admission then fails with that error instead of rejecting the tx as invalid, and QueuedIVCVerifier does not record it as a failed verification. A call that fails while bb is alive still means bb rejected the proof.

…p a partly started pool A borrower waiting on a full pool was stranded when a borrowed instance died, because the eviction freed room without spawning anything. Returning a dead instance now spawns its replacement, which goes straight to a waiting borrower. A borrow whose replacement spawn fails waits for a borrowed instance instead of failing, and only throws when the pool holds none. Pool initialization keeps the instances that started and spawns the rest on demand, rather than destroying them all when one fails.

…e checked ClientProtocolCircuitVerifier.verifyProof documents that it rejects, rather than reporting the proof invalid, when the proof could not be checked. BBCircuitVerifier's docs no longer claim that every failure while bb is alive is a rejection of the proof, and its failure log is structured.

…not strand them A borrower waiting on an empty pool counted on an in-flight spawn or a borrowed instance to come back, and was never woken when the spawn failed or the instance died without a replacement. Waiting borrowers now re-check every second: they spawn an instance themselves when the pool is below its size, and throw when that fails while no instance exists or is being spawned. A dead instance's replacement is spawned in the background, so returning it does not hold up the verification that found it dead.

…royed A borrower re-checking after destroy(), or an eviction just before it, spawned a bb only to destroy it. The pool test that waits for a borrowed instance also plans enough failed spawns for re-checks that land before that instance returns.

…tenance run The pool no longer spawns from the borrow path. Every second a maintenance run destroys the pooled instances whose bb died and spawns instances until the pool is back at poolSize, retrying failed spawns on the next run. A borrower skips dead instances and waits for a live one however long that takes, as it does while every instance is busy. destroy() does not wait for a spawn in flight; the spawn destroys its instance when it completes. QueuedIVCVerifier stops its verifier before draining its queue, so a verification waiting for a bb instance fails instead of blocking shutdown.

…n the factory is destroyed destroy() retires every dead member as well as the idle ones, so a dead instance a borrow dropped between maintenance runs is destroyed. Dispose returns an instance to the pool as before; the borrow path skips it if it is dead. QueuedIVCVerifier.stop() drains its queue even if stopping the verifier fails.

…covers

…r instead of a member list The idle queue is the only record of idle instances. Pool maintenance sweeps it, destroying dead instances and putting the live ones back, and spawns until idle, borrowed and spawning instances reach poolSize. A borrow destroys a dead instance it takes. Each instance is idle, borrowed or destroyed, so destroy() tears down the idle queue as on main.

The count covers idle, borrowed and spawning instances: a spawn adds one, and a failed spawn or a destroyed dead instance removes one. Pool maintenance spawns up to poolSize from it, and the dispose code is main's again.

… the pool Keeps the verifier half of this branch and replaces the pool half with one option. A pooled instance now spawns with bb.js's `respawn`, so an instance whose bb process dies replaces it and the next borrower gets a working one. The pool needs no liveness check, no eviction and no maintenance loop: returning an instance unconditionally is correct again, exactly as it already is for the AVM simulator pool. The verification retry keys off the failure rather than a liveness query. bb.js marks an environmental failure with `retry: true`, feature-detected rather than imported, so the same check works across the bb.js and ipc-runtime package boundaries. Asking an instance whether it is still alive can only ever be a guess, since the process can die between the answer and the next call, and it would not distinguish a dead helper from a bad proof, which is what makes the node report one as the other. The double models the same thing: a death fails the call in flight, retryably, and leaves the instance usable, because a replacement process serves the next call. Needs a bb.js release carrying aztec-packages#25548, which adds `respawn` and the `retry` property. Until the pin moves this does not build, as the branch it adjusts did not. The bb.js side is tested there, including against a real bb. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4

`ProvingError` was the only retryable failure the prover recognised, so a bb process that died mid-call surfaced as permanent and the job was not retried. Both the prover and the verifier now ask the same question of the error, and the classifier lives next to the backend that raises it rather than once per caller. `ProvingError.retry` still answers true, since the check is on the property rather than the class. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4

Reverting the pool to main's brought back main's permanent wedge, which #351 had also been fixing: initPromise caches its rejection, so one failed spawn made every later borrow fail for the life of the process. It is cleared on failure now, as the bb.js singleton's is, so the next borrow starts the pool again. A borrow also raced destruction, so a pool whose bb never comes up no longer holds up shutdown: the wait ends when the factory is destroyed and the borrow says so. The verifier spends its attempt budget on both halves. A bb that could not be started is worth another go, on the same terms a bb that died under the call gets; anything else, such as the factory being destroyed, will not improve by asking again. Either way no proof was checked, so neither is ever reported as the proof's fault. The double now fails a spawn retryably, as BBJsInstance.create does, rather than showing a test a permanent failure where production has a transient one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4

… test A failed spawn is now retried, so failing it once let the second attempt succeed and the proof verify. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4

…into respawn The bb-prover and AVM pool copies of the retry check become one type guard in foundation, without a cast. Respawn is now an explicit BBJsFactory option the verifier sets, rather than implied by pooling, so a future pooled owner of stateful sessions does not get it by accident. The retry wording says attempts, since with respawn a retry usually lands on the same instance's new process. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4

A dead batch verifier latched a fatal error and stayed dead for the life of the node. Every gossiped transaction then failed proof verification, and because verifyProof turned any failure into `valid: false`, and that result feeds gossip validation at LowToleranceError, the node worked through its peer set penalising honest nodes for its own dead helper process. Two changes, and the second matters even if the first never fires. The verifier rebuilds itself on the next verification after a fatal error. Everything the session holds comes from configuration — the protocol circuit verification keys, the batch size, the core count — so nothing is lost by replacing the bb process and starting a fresh session on it; only the proofs already in flight were lost, and those were rejected when it failed. Concurrent callers share one rebuild, and attempts are paced at one a second, indefinitely rather than a bounded number of times and flat rather than backing off, which is the convention the AVM simulator pool settled on so a node recovers as soon as bb is healthy rather than waiting out a backoff it has already outlived. A verifier that could not check a proof no longer reports it as invalid. It has not judged the proof, and saying otherwise is what turns a dead helper into a punished peer. That is the same distinction the RPC verifier draws. Not run: the labs tests need stdlib, which needs constants generated from the Noir sources in aztec-packages. The behaviour wants an integration test that kills the bb the verifier spawned and verifies again, which needs the pid exposed; happy to add it if that is wanted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4

Awaiting the rebuild check before queueing let a stop() issued after the proof was submitted close the queue first, so the proof was rejected instead of drained. Only defer when a rebuild is actually needed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4

…thout penalty Propagating ProofVerifierUnavailableError was not enough: gossip validation turns any thrown error into a rejection with a penalty, so a dead local bb still had the node penalising honest peers. Stage 2 now ignores the tx instead. The error moves to stdlib so p2p can recognise it. In the batch verifier, proofs in flight when the session fails, and proofs bb fails to take, now fail as unavailable rather than invalid. stop() waits out a rebuild in progress so it cannot leave a bb process running after teardown, and each session gets its own FIFO reader, so a replaced reader's late 'end' cannot fail the session that replaced it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4

A tx validation that could not run (for example, because the proof verifier is unavailable) now has its own result, distinct from a verdict that the tx is invalid. Adds the non-slashable UnverifiableBlockProposalTxsError for block proposal txs that could not be checked, and LruMap.peek for lookups that must not refresh recency.

…ted it BBCircuitVerifier and BatchChonkVerifier now return valid: false only for a completed verification that answered false, or for the empty placeholder proof, which they reject up front. Every other failure, including any error bb throws while alive and a batch result whose message says bb threw, rejects with ProofVerifierUnavailableError. Adds an unavailable-verification metric, and lets TestCircuitVerifier answer invalid or unavailable.

…ache it TxProofValidator maps a rejected proof to invalid and any verifier failure to unverifiable. AggregateTxValidator stops at, and returns, the first result that is not valid, so a cheap deterministic invalid still wins. TxValidationCache evicts a validation that ends unverifiable or rejects, once it settles and only if it is still the cached entry, so one transient failure no longer sticks to the tx; invalid verdicts stay cached.

Gossip ignores an unverifiable tx without a penalty. The batch tx requester neither penalises nor redeems a peer for one, and leaves the tx missing so it is requested again, giving up on it for the run after a bounded number of attempts. Block proposal txs that could not be checked, with none invalid, throw UnverifiableBlockProposalTxsError and are not added to the pool. The tx collection sources report unverifiable txs apart from invalid ones, and the tx pool ignores rather than rejects them.

…be verified A block proposal whose carried txs could not be checked now fails with the non-slashable txs_unverifiable, counted as a node issue, instead of escaping as an error or being classified as invalid_embedded_txs. It never reaches slashInvalidBlock or markInvalidProposalSlot.

sendTx refuses a tx whose proof could not be checked with a retry-later error rather than as an invalid tx; isValidTx returns the unverifiable result. The public processor skips such a tx instead of failing it, so it is not dropped from the pool.

…ggregate validator

… validation

fcarreiro and others added 25 commits October 1, 2026 12:58

test(bb-prover): drop a pool test that the failed-spawn waiting test …

c205681

…covers

spalladino added this pull request to stack #384 October 2, 2026 20:19

spalladino commented Oct 2, 2026

View reviewed changes

Comment thread yarn-project/p2p/src/msg_validators/tx_validator/aggregate_tx_validator.ts Outdated

spalladino commented Oct 2, 2026

View reviewed changes

Comment thread yarn-project/p2p/src/services/libp2p/libp2p_service.ts Outdated

spalladino added 2 commits October 2, 2026 17:28

fix(p2p): let an invalid result win over an unverifiable one in the a…

0417b13

…ggregate validator

refactor(p2p): drop the unreachable verifier-error catch in gossip tx…

a3a6960

… validation

spalladino marked this pull request as ready for review October 2, 2026 20:28

spalladino requested review from IlyasRidhuan, alexghr and fcarreiro as code owners October 2, 2026 20:28

Navigation Menu

fix(p2p): treat unverifiable tx proofs as unknown, not invalid - #383

fix(p2p): treat unverifiable tx proofs as unknown, not invalid#383
spalladino wants to merge 27 commits into
cl/batch-verifier-selfhealfrom
spl/unverifiable-tx-proofs

spalladino commented Oct 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

spalladino commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Known limitation

Testing

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

spalladino commented Oct 2, 2026 •

edited

Loading