ethereum / ethereum/consensus-specs

Inconsistent dedup vs KZG ordering in sidecar gossip validation

Open
#5,451 1 comment 0 reactions 1 assignee Claimed by @jtraglia View on GitHub
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.