Skip to content

Gloas set AttestationData.index #9100

Merged
mergify[bot] merged 16 commits intosigp:unstablefrom
eserilev:gloas-attestation-index
Apr 26, 2026
Merged

Gloas set AttestationData.index #9100
mergify[bot] merged 16 commits intosigp:unstablefrom
eserilev:gloas-attestation-index

Conversation

@eserilev
Copy link
Copy Markdown
Member

@eserilev eserilev commented Apr 6, 2026

Issue Addressed

For gloas attestation.data.index should be set to 1 if we are attesting to a block whose slot is not the attestation duty slot and slot payload_status is FULL

@eserilev eserilev added gloas ready-for-review The code is ready for review labels Apr 6, 2026
{
self.canonical_head
.fork_choice_read_lock()
.is_payload_received(&beacon_block_root)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should check canonicity not just existence, I think. Spec says:

Set data.index = 1 to signal that the payload is present in the canonical chain (payload status is FULL in the fork-choice).

https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/validator.md#attestation

Might need the canonicity code from:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh yeah, not sure how i mixed that up lol. thanks!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've got a feeling this will be a common Gloas bug haha

@eserilev eserilev added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Apr 8, 2026
@eserilev eserilev mentioned this pull request Apr 15, 2026
40 tasks
@eserilev eserilev added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Apr 25, 2026
Copy link
Copy Markdown
Collaborator

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

Logic is correct, good to go!

@dapplion dapplion added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 26, 2026
@mergify mergify Bot added the queued label Apr 26, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 26, 2026

Merge Queue Status

This pull request spent 27 minutes 57 seconds in the queue, including 26 minutes 39 seconds running CI.

Required conditions to merge

mergify Bot added a commit that referenced this pull request Apr 26, 2026
@mergify mergify Bot merged commit 276c4d5 into sigp:unstable Apr 26, 2026
39 checks passed
@mergify mergify Bot removed the queued label Apr 26, 2026
dapplion added a commit to dapplion/lighthouse that referenced this pull request Apr 28, 2026
PR sigp#9178 added this Gloas-only test using ApiTester::new() which
produces a phase0 chain even under FORK_NAME=gloas, so
head.beacon_state.get_ptc(...) errored with IncorrectStateVariant.
After switching to new_with_hard_forks() three further issues
surfaced:

1. The slot clock is left at head_slot + 1 by the harness setup, so
   a payload attestation for head_slot fails gossip propagation as a
   PastSlot. Rewind the clock to head_slot in
   make_valid_payload_attestation_message.

2. With VALIDATOR_COUNT = 32 and 32 slots/epoch, a slot's committees
   often hold only a single validator. The PTC for that slot then
   has one distinct validator regardless of PTCSize. The original
   test chained JSON and SSZ sub-tests on the same harness with
   ptc_offset 0 and 1 and asserted both gossip-published, but the
   second message is a duplicate (same slot/validator) and is
   silently dropped as PriorPayloadAttestationMessageKnown — so the
   second recv() hangs forever. Split the SSZ variant into its own
   test with its own harness so the two don't collide in the
   ObservedPayloadAttesters cache.

3. Switching the JSON test to new_with_hard_forks() so the chain
   actually reaches Gloas under FORK_NAME=gloas (same fix as the
   sibling tests added in sigp#8415 and sigp#9100).

Verified locally: full Gloas suite 197/197 passed (350s).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gloas ready-for-merge This PR is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants