Repository navigation
feat: merge-train/spartan-v5 - #25062
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 4 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
…targets (#25058) Closes A-1443. Reworked version of #24923 targeting the v5 line, addressing the review feedback there. Operators currently get no signal when the network starts voting to slash their validators — during the v5 inactivity-slashing incident, operators found out only after stake was lost. This PR makes the node detect when its own validators are targeted by onchain slashing, at two points in the lifecycle: - **Vote time (early warning)**: a new `OwnValidatorSlashMonitor` (following the `SlashRoundMonitor`/`SlashOffensesCollector` decomposition) subscribes to `VoteCast` events — only when the node runs validators — and warns on every vote that names one of them, with the running tally against the quorum: `Own validator 0x… targeted by slashing vote (7 of 65 votes needed to slash)`. Quorum needs a majority of a round's slots, so warnings start well inside the window an operator has to react. - **Round execution**: `handleRoundExecuted` filters the already-fetched `Slashed` events to own validators and emits a WARN with the exact slashed amount. No additional L1 reads. Five node-level metrics (no per-validator labels, so one series per node regardless of attester count): `aztec.slasher.own_validator.targeted_count`, `.current_round_votes_max` (highest tally against any committee position held by an own validator, reset each round), `aztec.slasher.quorum_size` to compare it against, and `.slashed_count` / `.slashed_amount` for executed slashes. The alert is `current_round_votes_max` approaching `quorum_size`; the WARN identifies which validator via a structured log field. Design notes: - **Votes are read by index, events are only triggers.** `VoteCast` carries no vote index, so the monitor keeps a per-round cursor: on each event it reads the round's `voteCount` and processes votes `[cursor, voteCount)` via a new `getVoteAt(round, index)`. Duplicate or batched deliveries are no-ops, backlogs are caught up by the next event, and a failed read is retried because the cursor only advances after a vote is successfully processed. - **All processing is serialized** through a single queue, with the round revalidated after every await, so concurrent event handlers cannot double-count, drop votes into the wrong round, or emit out-of-order tallies. `stop()` drains the queue and suppresses any late warns/metrics. - **Startup baseline instead of replay.** On start the monitor reads the current `voteCount` (before subscribing) and skips everything before it: replaying old votes would double-count cumulative counters across restarts. Votes cast while the node is offline or starting up are not counted, and L1 reorgs can cause small drift within a round — both documented on the metrics. - **The tally is per flattened committee position** because that is the unit the contract tallies quorum by: a validator sitting in several of the round's committees is named once per position by a single vote, each position racing quorum independently. Warnings and `targeted_count` remain per (vote, validator), reporting the validator at its highest position tally. - Own validator addresses were already passed to `createSlasherFacade` for `slashValidatorsNever`; they are now also threaded as `ownValidators` into `SlasherClient`, independent of `slashSelfAllowed`. `getSlashingAmounts` is memoized (the amounts are Solidity `immutable`), and `SlashVote` is renamed to the singular `SlashVoteTarget`. Testing: 22 unit tests for the monitor (per-vote warning with pinned message/context, per-position tallying, multi-validator nodes, round rollover in both directions and quiet-round reset, closed-round and foreign-validator votes, cursor semantics under duplicate/batched/missed events, failed-read retry, drain serialization, mid-drain rollover, startup baseline and its failure fallback, stop/restart lifecycle, subscription gating), 3 client wiring tests, and an anvil-backed `getVoteAt` decoding test in the ethereum package.
spalladino
added this pull request to the merge queue
Aug 1, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 1, 2026
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
AztecBot
added this pull request to the merge queue
Aug 3, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 3, 2026
spalladino
added this pull request to the merge queue
Aug 4, 2026
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
3 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)
feat(slasher): warn and expose metrics when own validators are slash targets (#25058)
END_COMMIT_OVERRIDE