Skip to content

Enable gossip blob sidecar ref-test - #10780

Merged
gfukushima merged 38 commits into
Consensys:masterfrom
gfukushima:reftest-gossip-blob
Jun 15, 2026
Merged

Enable gossip blob sidecar ref-test#10780
gfukushima merged 38 commits into
Consensys:masterfrom
gfukushima:reftest-gossip-blob

Conversation

@gfukushima

@gfukushima gfukushima commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

PR Description

Created on top of #10770, it add a GossipBlobSidecar executor and enables the suite

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

Note

Low Risk
Changes are confined to reference tests and test visibility on a gossip wrapper; production blob validation behavior is not altered.

Overview
Enables the networking/gossip_blob_sidecar consensus reference suite by replacing the ignored placeholder with a new GossipBlobSidecarTestExecutor.

The executor builds an in-memory chain (fork choice, block imports, optional finalized checkpoint), runs BlobSidecarGossipValidator through the same TopicSubnetIdAwareOperationProcessor wrapper used in production when tests specify a subnet_id, and asserts accept / reject / ignore (including SAVE_FOR_FUTURE) per fixture. It handles failed setup blocks, BLS ignored specs, raw-root finalized checkpoints via a test-only GossipValidationHelper override, and loads a real KZG trusted setup only for invalid KZG proof cases.

eth-reference-tests gains a networking:eth2 dependency; TopicSubnetIdAwareOperationProcessor in BlobSidecarGossipManager is public and @VisibleForTesting so ref tests can instantiate it.

Reviewed by Cursor Bugbot for commit b4f00a9. Bugbot is set up for automated code reviews on this repo. Configure here.

zilm13 and others added 27 commits May 28, 2026 14:01
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…Helper method isCurrentTimeWithinInclusiveSlotRange

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…createGossipValidationHelper (needs review)

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…finalized checkpoint

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
@gfukushima gfukushima changed the title Reftest gossip blob Enable gossip blob sidecar ref-test Jun 2, 2026
@rolfyone

rolfyone commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

looks ok to me, one codex observation to consider

Finding
[P2] GossipTests.java (line 28) enables gossip_beacon_attestation, but GossipBeaconAttestationTestExecutor.java (line 138) still treats any finalized_checkpoint by skipping block imports and using the default GossipValidationHelper. That means finalized-checkpoint attestation cases can pass as block-not-available SAVE_FOR_FUTURE/IGNORE, without exercising the new finalized-checkpoint ancestry hook added to AttestationValidator. I’d mirror the structured handling added to the aggregate executor: parse the checkpoint, set the store checkpoint when it names a block, and override the helper for fake roots.

finalizedCheckpoint ->
new GossipValidationHelper(spec, recentChainData, metricsSystem) {
@Override
public boolean currentFinalizedCheckpointIsAncestorOfBlock(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we overriding this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a test that validates against a non-canonical checkpoint, this test use a fake/non-existent finalized root to exercise the rejection rule, but Teku’s store cannot commit that because the finalized block is not present. So the override is not bypassing blob validation; it is replacing the fixture’s finalized checkpoint root while still using the live fork-choice ancestry data.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put it in comments somewhere around it because it's not like GossipBeaconAttestationTestExecutor override where you override noop logic to op, it looks like different logic to what is in prod and it should be clear, why it was done.

# Conflicts:
#	eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/gossip/GossipBeaconAggregateAndProofTestExecutor.java
#	eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/gossip/GossipTests.java
#	ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/validation/Phase0AttestationValidatorTest.java
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
zilm13
zilm13 previously approved these changes Jun 12, 2026

@zilm13 zilm13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one comment nit

…sAncestorOfBlock

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
@gfukushima
gfukushima enabled auto-merge (squash) June 15, 2026 01:25
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

@zilm13 zilm13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gfukushima
gfukushima merged commit 6f77864 into Consensys:master Jun 15, 2026
82 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants