Skip to content

Enable gossip attestation and aggregate ref-test - #10770

Merged
gfukushima merged 37 commits into
Consensys:masterfrom
gfukushima:reftests-att-and-aggregate
Jun 7, 2026
Merged

Enable gossip attestation and aggregate ref-test#10770
gfukushima merged 37 commits into
Consensys:masterfrom
gfukushima:reftests-att-and-aggregate

Conversation

@gfukushima

@gfukushima gfukushima commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

PR Description

This PR was built on top of #10762 and fixes the enables the attestation and aggregation gossip 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

Medium Risk
Changes live gossip attestation/aggregate validation (invalid blocks, finalized ancestry, Electra committee bits) and fork-choice store initialization from anchors; risk is moderate but bounded by new ref and unit tests.

Overview
This PR turns on the Ethereum consensus reference tests for gossip beacon attestation and gossip beacon aggregate and proof, replacing ignored placeholders with real executors that drive fork choice, store setup, and the production attestation/aggregate validators.

Gossip validation now tracks invalid block roots (like BlockManager) and rejects attestations/aggregates that vote for them; it also enforces that the current finalized checkpoint is an ancestor of the voted block (with a test-only GossipValidationHelper override for synthetic finalized roots). Electra aggregates/single attestations must have exactly one committee bit set. Deneb slot/epoch gossip timing is aligned with consensus-specs via MiscHelpers.isCurrentTimeWithinInclusiveSlotRange, with updated unit tests.

Reference-test harness improvements: richer anchor selection when state and block roots differ, optional finalized_checkpoint in YAML (commit to store or override ancestor check), and block import even when a custom finalized checkpoint is configured.

Store bootstrap uses the anchor block slot (not advanced state slot) when building fork choice from a transitioned anchor, with storage/client tests for that case.

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

zilm13 and others added 20 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>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 0fded54. Configure here.

Comment thread specrefs/functions.yml Outdated

@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.

I'd also copy following tests from commit af3d146:

  • MiscHelpersTest changes, because this PR doesn't test clock disparity changes
  • RecentChainDataTest changes, StorageBackedRecentChainData changes, StoreBuilderTest changes - all of these tests influence of switching from stateSlot to blockSlot in initialization on dependent classes

Comment thread storage/src/main/java/tech/pegasys/teku/storage/store/StoreBuilder.java Outdated
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
recentChainData.getForkChoiceStrategy().orElseThrow());
}

public boolean currentFinalizedCheckpointIsAncestorOfAttestationBlock(final Bytes32 blockRoot) {
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>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
@gfukushima gfukushima changed the title Enable some ref tests of the gossip suite Enable gossip attestation and aggregate ref-test Jun 2, 2026
@rolfyone

rolfyone commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

one bot question
[P2] eth-reference-tests/.../GossipTests.java:28 now enables gossip_beacon_attestation, but GossipBeaconAttestationTestExecutor still handles finalized_checkpoint by only skipping setup block imports (GossipBeaconAttestationTestExecutor.java:138). It then uses the default GossipValidationHelper, whose new attestation finalized-ancestor hook is a production no-op, and the harness accepts either IGNORE or SAVE_FOR_FUTURE for expected ignore results (GossipBeaconAttestationTestExecutor.java:222). So the concrete gossip_beacon_attestation__ignore_finalized_not_ancestor fixtures can pass via “block not available” instead of exercising the new finalized-ancestor validation. I’d mirror the aggregate executor’s parsed FinalizedCheckpoint plus helper override for the attestation executor before enabling this suite.

…ch teku doesn't explicitly implement since it relies on the protoarray only stores finalized blocks

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
…ows the patter used for blobs, data column and execution payload

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
@zilm13 zilm13 mentioned this pull request Jun 4, 2026
17 tasks

@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
Great work!

@gfukushima
gfukushima merged commit e6cec32 into Consensys:master Jun 7, 2026
83 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 7, 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