added an executor for Gossip beacon attestation reftests#10546
added an executor for Gossip beacon attestation reftests#10546rolfyone wants to merge 1 commit intoConsensys:masterfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| testDefinition, | ||
| message.getMessage() + ".ssz_snappy", | ||
| spec.getGenesisSchemaDefinitions().getAttestationSchema()); | ||
| final Bytes32 attestationRoot = attestation.hashTreeRoot(); |
There was a problem hiding this comment.
Wrong attestation schema used to deserialize SSZ
High Severity
The test loads gossip attestation SSZ files using spec.getGenesisSchemaDefinitions().getAttestationSchema(), which returns AttestationPhase0Schema. However, in Electra/Fulu the beacon_attestation_{subnet_id} gossip topic was changed to carry SingleAttestation objects, whose SSZ layout (committee_index: uint64, attester_index: uint64, data, signature) is entirely different from the phase0 layout (aggregation_bits: bitlist, data, signature). Deserializing a SingleAttestation blob with the phase0 schema will produce corrupted data or a deserialization error, causing every Fulu gossip attestation reftest to fail or silently produce wrong results. The correct schema to use is getSingleAttestationSchema() from SchemaDefinitionsElectra (as already used in SszTestExecutor).
| new GossipValidationHelper(spec, recentChainData, metricsSystem)); | ||
|
|
||
| // Track seen attestations (by hash tree root) for "already seen" duplicate detection | ||
| final Set<Bytes32> seenAttestationRoots = new HashSet<>(); |
There was a problem hiding this comment.
so this is required to simulate the fact that, at low level in libp2p, we ignore already seen messages (done via sha256). Am I correct?
There was a problem hiding this comment.
if true, we should add them to the seenCache regardless of its validity
There was a problem hiding this comment.
im actually not sure... :)
There was a problem hiding this comment.
it currently passes the test cases so probably academic i guess
tbenr
left a comment
tbenr
left a comment
There was a problem hiding this comment.
I'd just move the seenAttestationRoots.add regardless the validation result
2 participants
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog
Note
Low Risk
Test-only change that adds a new reference-test executor and wires it into the test-type registry; no production logic is modified.
Overview
Adds a new reference-test executor,
GossipBeaconAttestationTestExecutor, to runnetworking/gossip_beacon_attestationvectors by building an in-memory chain (genesis + optional imported blocks) and validating gossiped attestations against expectedaccept/reject/ignoreoutcomes.Registers the new executor in
Eth2ReferenceTestCaseso the test runner can dispatch this new networking test type.Written by Cursor Bugbot for commit 04ad22c. This will update automatically on new commits. Configure here.