test: deneb and electra gossip validation spec tests#9372
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements and refines gossip validation for blob sidecars to align with the Deneb specification, including tracking seen sidecar tuples, validating proposer indices, and checking finalized ancestors. It also updates attestation slot range verification for EIP-7045 and adds pre-Capella checks for BLS-to-execution changes. Feedback suggests updating a comment to correctly describe the sidecar tracking tuple and reordering validation steps to perform the expensive KZG proof verification after the proposer check to improve efficiency.
| // [REJECT] The sidecar is proposed by the expected proposer_index | ||
| // (if shuffling is not available, IGNORE instead and MAY be queued for later) | ||
| const blockState = await chain.regen | ||
| .getBlockSlotState(parentBlock, blobSlot, {dontTransferCache: true}, RegenCaller.validateGossipBlock) | ||
| .catch(() => { | ||
| throw new BlobSidecarGossipError(GossipAction.IGNORE, { | ||
| code: BlobSidecarErrorCode.PARENT_UNKNOWN, | ||
| parentRoot, | ||
| blockRoot: blockHex, | ||
| slot: blobSlot, | ||
| }); | ||
| }); | ||
| if (blockState.getBeaconProposer(blobSlot) !== proposerIndex) { | ||
| throw new BlobSidecarGossipError(GossipAction.REJECT, { | ||
| code: BlobSidecarErrorCode.INCORRECT_PROPOSER, | ||
| proposerIndex, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The proposer index validation (which may involve an expensive state regeneration via regen.getBlockSlotState) is currently performed after the expensive KZG proof verification (line 171).
Following the spec's recommended validation order, the proposer check should be performed before the blob (KZG) validation. If the proposer is incorrect, the sidecar should be rejected immediately without wasting CPU cycles on KZG verification.
There was a problem hiding this comment.
our check is as per spec, this is also dead code, generally we should follow spec order unless spec has a order that doesn't make sense in terms of running cheap vs. expensive checks first
There was a problem hiding this comment.
but this needs more review, ideally we wanna run cheap checks first
Performance Report✔️ no performance regression detected Full benchmark results
|
Adds runner support for the fulu `gossip_data_column_sidecar` networking spec tests from consensus-specs PR ChainSafe#5246. Mirrors the existing `gossip_blob_sidecar` arm: SSZ-decodes `fulu.DataColumnSidecar` and invokes `validateGossipFuluDataColumnSidecar` with the fixture's `subnet_id`. Builds on top of ChainSafe#9372. Does not address `gossip_partial_data_column_sidecar` (optional feature) nor the open `gossip_beacon_block__valid_at_blob_parameters_limit` zero-parent harness gap. 🤖 Generated with AI assistance
See ethereum/consensus-specs#5146 and ethereum/consensus-specs#5238