chore: Accumulated ports to next - #25063
Merged
Merged
Conversation
…ures (#25031) ## Context A selected proposer can attach signed txs to a block proposal that pass the proposal-level checks (each tx hash is listed in `txHashes` and each tx recomputes to its own hash) but fail minimum tx integrity validation — bad proof, metadata, size, data, or contract instances. Tx collection threw a bare `Error('Invalid tx detected')` out of the validator's block-received callback before `handleBlockProposal` could return a typed result, so `validateBlockProposal` never reached the branch that emits `BROADCASTED_INVALID_BLOCK_PROPOSAL` and marks the slot invalid. The proposer avoided invalid-block accountability, and observers missed the invalid slot that the delayed attested-to-invalid-proposal watcher relies on. The same callback rejection also leaked tx-pool protections: `processValidBlockProposal` protects the proposal's txs before awaiting the callback but only unprotected them when the callback *returned* false. ## Approach - `validateTxsReceivedInBlockProposal` now throws a typed `InvalidBlockProposalTxsError` carrying each offending tx hash and its validation reasons, instead of a bare error. - The proposal handler catches that error around tx collection (in a `collectProposalTxs` helper) and returns a new `invalid_embedded_txs` failure reason, which is in `SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT`, so both the validator and non-validator handlers emit the offense and mark the slot invalid — still gated on the escape hatch being closed. Any other collection error keeps propagating: only proposer misbehavior gets a typed, slashable reason, so a local pool or network failure is not mistaken for an invalid proposal. The error crosses a package boundary, so it is matched with `isErrorClass` rather than `instanceof`, and sets its `name` accordingly. - `processValidBlockProposal` releases the protections it created when the callback rejects, not just when it returns false. A `tryBlockReceivedCallback` helper reports a throwing callback as a rejection — the proposal is no more usable than one explicitly rejected, and the error would otherwise escape with the protections still held — leaving one release path for both outcomes. The outer gossip handler only logged such errors anyway; one visible consequence is that an unexpected error while processing a checkpoint's embedded block no longer skips the checkpoint proposal itself. - A proposal that lists the same tx hash twice is rejected with a new slashable `duplicate_txs` reason. The repeated tx makes the block unbuildable (it would emit nullifiers the first copy already emitted), and tx collection reconciles a deduplicated hash set against the full list, so a duplicate that is not available locally threw the invariant error `Error collecting txs for proposal with N txs: found X and flagged Y as missing` out of `getTxsForBlockProposal` — the same unattributable-exception shape this PR removes. The check lives in the proposal handler rather than in gossip validation: a duplicate hash is proposer misbehavior, not a relaying-peer fault, so rejecting it at ingress would penalize the forwarding peer and short-circuit before the block-received callback, leaving the offense unrecorded. This mirrors how an over-limit `indexWithinCheckpoint` is handled. The handler runs the check before tx collection, so the invariant error is unreachable, and the terminal block carried in a checkpoint proposal goes through the same handler. Slashing here is attributable to the proposer. The proposal is validated for the right signing domain, slot window, and expected-proposer signature before any tx is looked at, and `validateTxs` requires every embedded tx to be listed in the signed `txHashes` with a self-consistent hash. `BlockProposal.getSender()` additionally returns undefined unless the embedded `SignedTxs` bundle shares the outer proposal's signature context and recovers to the same signer. A relaying peer can strip txs from the body, which surfaces as the non-slashable `txs_not_available`, but cannot graft foreign txs into an honest proposer's proposal. Two limitations worth noting, both pre-existing and left as-is: - `TxProvider.extractFromProposal` only considers hashes missing from the local pool, so a tx carried in the proposal whose hash is already pooled is never integrity-checked. Detection is therefore mempool-dependent. Closing that gap means proof-verifying every carried tx on the attestation path, which is a separate performance call. - `SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT`, `badProposalReasons` in `validator.ts`, and the metric label seeds in `metrics.ts` are three hand-maintained lists of the same taxonomy and have already drifted (`global_variables_mismatch` is slashable but counted as a node issue). The new reason is added to all three; consolidating them into one exhaustive record would be a good follow-up. Fixes A-1525
AztecBot
added this pull request to the merge queue
Jul 30, 2026
Collaborator
Author
|
🤖 Auto-merge enabled after 8 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
Jul 30, 2026
This happened due to a Noir version bump.
AztecBot
added this pull request to the merge queue
Aug 4, 2026
Collaborator
Author
|
🤖 Auto-merge enabled after 8 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 4, 2026
AztecBot
added this pull request to the merge queue
Aug 6, 2026
Collaborator
Author
|
🤖 Auto-merge enabled after 8 hours of inactivity. This PR will be merged automatically once all checks pass. |
nchamo
removed this pull request from the merge queue due to a manual request
Aug 6, 2026
AztecBot
added this pull request to the merge queue
Aug 7, 2026
Collaborator
Author
|
🤖 Auto-merge enabled after 8 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
4 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
fix(validator): classify invalid proposal txs as slashable block failures (#25031)
docs: note visibility requirement for in-contract note types (#25071)
END_COMMIT_OVERRIDE