Skip to content

fix(validator): remove duplicate time-sensitive validation of block proposals - #25207

Merged
alexghr merged 2 commits into
merge-train/spartan-v5from
spl/a-1703-remove-duplicate-block-proposal-validation
Aug 14, 2026
Merged

fix(validator): remove duplicate time-sensitive validation of block proposals#25207
alexghr merged 2 commits into
merge-train/spartan-v5from
spl/a-1703-remove-duplicate-block-proposal-validation

Conversation

@spalladino

Copy link
Copy Markdown
Contributor

Removes the duplicate block-proposal validation that ran again downstream of p2p ingress, whose repeated wall-clock check could turn an honest, on-time proposal into a false slashable offense.

Context

On mainnet a validator node classified a canonical block proposal as a slashable invalid_proposal offense. Block proposals are validated at p2p ingress, where the receive-window check decides whether they arrived in time; ProposalHandler.handleBlockProposal then re-ran that same validation "out of caution" before re-execution. When node-local processing stalled (~50s in the incident), the repeated wall-clock check failed an on-time proposal, and invalid_proposal is in SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT — so local latency was converted into slashing evidence against an honest proposer.

Approach

Delete the downstream blockProposalValidator.validate(proposal) call and, with it, the now-unused BlockProposalValidator dependency of ProposalHandler (and its construction in ValidatorClient.new and createProposalHandler).

This is safe because every path into handleBlockProposal is post-ingress — gossiped block proposals and checkpoint-embedded blocks both pass ingress validation first, and there is no replay path — so the second pass added no coverage. The checks that remain downstream are deterministic properties of the signed payload rather than of the clock: the signature via getSender(), duplicate tx hashes, checkpoint/index consistency, in-hash agreement, embedded-tx integrity via tx collection, and full re-execution. The checkpoint-proposal path already operates without such a re-check. invalid_proposal stays in the slashable list, since it is still produced by the structural computeCheckpointNumber checks. The p2p package is untouched: ingress keeps its own validator.

Note that dropping the whole call also stops re-deriving the expected proposer downstream; that identity check likewise belongs to ingress, and the two validator.test.ts cases that pinned the old repeated-gate behavior were rewritten to assert the new contract (validates, no WANT_TO_SLASH_EVENT, slot not marked invalid).

This is the simpler alternative to #25201, which instead splits the validator into validate() / validateStableFields() and adds a non-slashable reason; the two are open in parallel for comparison.

Fixes A-1703

@github-actions github-actions Bot added the port-to-next Forward-port this merged PR into next label Aug 12, 2026
…roposals

Block proposals were validated twice: once at p2p ingress, and again at the
top of `handleBlockProposal` "out of caution". That second pass re-ran the
wall-clock receive-window check, whose outcome depends on when processing
starts rather than on anything the proposer did. On a node whose processing
stalled, an on-time canonical proposal was rejected as `invalid_proposal` —
a reason in the slashable list — manufacturing a false invalid-block offense
against an honest proposer.

Ingress is the arrival gate, and every path into the handler is post-ingress
(gossiped block proposals and checkpoint-embedded blocks alike), so the
duplicate pass added no coverage. What remains downstream are deterministic
properties of the signed payload: the signature via `getSender()`, duplicate
tx hashes, checkpoint/index consistency, in-hash agreement, embedded-tx
integrity from tx collection, and full re-execution.

Dropping the check makes the `BlockProposalValidator` dependency of
`ProposalHandler` unused, so it is removed along with its two construction
sites.
@alexghr
alexghr merged commit a5eeac4 into merge-train/spartan-v5 Aug 14, 2026
12 checks passed
@alexghr
alexghr deleted the spl/a-1703-remove-duplicate-block-proposal-validation branch August 14, 2026 09:47
AztecBot pushed a commit that referenced this pull request Aug 14, 2026
@AztecBot

Copy link
Copy Markdown
Collaborator

❌ Failed to cherry-pick to next due to conflicts. (🤖) View backport run.

spalladino added a commit that referenced this pull request Aug 14, 2026
…osal type (#25222)

Follow-up to #25207. That PR removed the duplicate downstream
re-validation of inbound block and checkpoint proposals, leaving p2p
ingress (gossipsub topic validation) as the single place proposals are
validated. The downstream handlers document that precondition in prose;
this PR enforces it with the type system.

- Adds branded `ValidatedBlockProposal` and
`ValidatedCheckpointProposalCore` types (plus their minting functions)
in `stdlib/src/p2p/validated_proposal.ts`, following the existing
`Branded<T, Brand>` convention used by `BlockNumber` and friends.
- The p2p received-proposal callbacks (`P2PBlockReceivedCallback`,
`P2PCheckpointReceivedCallback`) and the downstream consumers
(`ValidatorClient.validateBlockProposal` / `attestToCheckpointProposal`,
`ProposalHandler.handleBlockProposal` / `handleCheckpointProposal`, and
the `Validator` interface) now take the branded types, so a raw inbound
`BlockProposal` / `CheckpointProposalCore` cannot reach them.
- The brands are minted only in `libp2p_service.ts`, at the three points
where the topic validator has already returned `Accept`: the
block-proposal topic path, the checkpoint-embedded block path
(`processBlock` is only set after Accept), and the checkpoint path.
- Purely a compile-time marker: no runtime validation is added and there
is no behavior change. The only non-type edits are in tests, which mint
validated proposals from constructed ones.

Related to A-1703.
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.

3 participants