Skip to content

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

Open
spalladino wants to merge 8 commits into
cl/batch-verifier-selfhealfrom
spl/unverifiable-tx-proofs
Open

spalladino wants to merge 8 commits into
cl/batch-verifier-selfhealfrom
spl/unverifiable-tx-proofs

Conversation

@spalladino

@spalladino spalladino commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

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 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.
@spalladino
spalladino added this pull request to stack #384 October 2, 2026 20:19
Comment thread yarn-project/p2p/src/msg_validators/tx_validator/aggregate_tx_validator.ts Outdated
Comment thread yarn-project/p2p/src/services/libp2p/libp2p_service.ts Outdated
@spalladino
spalladino marked this pull request as ready for review October 2, 2026 20:28

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant