ethereum / ethereum/consensus-specs
Inconsistent dedup vs KZG ordering in sidecar gossip validation
- Dominant language
- Python
- Stars
- 4k
- Forks
- 1.3k
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 58
Description
Hi there,
I think I found a bug that the blob and data-column sidecar gossip validators order the dedup and the KZG check differently:
- Deneb/Electra `validate_blob_sidecar_gossip`: verify KZG first, then dedup.
- Fulu `validate_data_column_sidecar_gossip`: dedup first, then verify KZG.
So for the same input which
- a sidecar whose `(slot, proposer_index, index)` tuple is already seen and
- whose KZG proof is invalid
The two return different verdicts
- Deneb `REJECT`s (KZG runs first);
- Fulu `IGNORE`s (dedup runs first).
`REJECT` descores the peer, `IGNORE` doesn't. This is a peer score policy inconsistency.
Testing results
```
PROOF Deneb (deneb): tuple-in-seen + invalid KZG -> REJECT ('invalid blob kzg proof')
PROOF Deneb (electra): tuple-in-seen + invalid KZG -> REJECT ('invalid blob kzg proof')
PROOF Fulu (fulu): tuple-in-seen + invalid KZG -> IGNORE ('already seen sidecar ...')
```
Fulu originally matched Deneb; the order flipped in #5246.
I think the right order is verify-then-dedup, since before the flip:
https://github.com/ethereum/consensus-specs/blob/6454e6c730b5c834f8031246256de54a45bd44bf/specs/fulu/p2p-interface.md?plain=1#L267-L269
This only makes sense if verification runs first. Gloas is also correct here.
Happy to send a fix PR if it should be aligned.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.