Skip to content

Add fork choice test for split LMD votes across payload variants#5345

Draft
barnabasbusa wants to merge 5 commits into
ethereum:masterfrom
barnabasbusa:bbusa/gloas-forkchoice-split-payload-vote
Draft

Add fork choice test for split LMD votes across payload variants#5345
barnabasbusa wants to merge 5 commits into
ethereum:masterfrom
barnabasbusa:bbusa/gloas-forkchoice-split-payload-vote

Conversation

@barnabasbusa

Copy link
Copy Markdown
Member

Summary

Adds a Gloas fork_choice trace in which two disjoint beacon committees attest to the same beacon_block_root with opposite payload_index values, so the same root ends up with non-zero LMD weight on both PAYLOAD_STATUS_FULL and PAYLOAD_STATUS_EMPTY viable fork-choice nodes at the same time.

  • File: tests/core/pyspec/eth_consensus_specs/test/gloas/fork_choice/test_split_payload_status_lmd_votes.py
  • Test: test_lmd_vote_split_across_payload_status_variants
  • Generated under reftests/{minimal,mainnet}/gloas/fork_choice/split_payload_status_lmd_votes/.

Why

Under EIP-7732 the same LatestMessage.root resolves through get_supported_node to two distinct ForkChoiceNode values depending on LatestMessage.payload_present:

  • ForkChoiceNode(root=R, payload_status=PAYLOAD_STATUS_FULL)
  • ForkChoiceNode(root=R, payload_status=PAYLOAD_STATUS_EMPTY)

get_weight / get_attestation_score in the spec recompute the score from scratch each call by summing effective_balance over store.latest_messages, so the spec never adds or subtracts a running weight and stays correct by construction.

Production clients (e.g. Teku's ProtoArray, Grandine's fork_choice_store) maintain running per-node weights and must independently track two totals for the same root once Gloas activates. There is currently no Gloas fork_choice test that places non-zero weight on both payload variants of the same root in a single store, so the test suite cannot distinguish an implementation that correctly maintains the two running totals from one that conflates or double-counts them.

On a recent Glamsterdam devnet (glamsterdam-devnet-5), the absence of such coverage coincided with two independent fork-choice ERROR-level signatures around exactly this surface:

ERROR fork_choice_store::store:5042: attesting balance should never go below zero
  (block: 0x109cfb…b090 with payload None, balance kind: empty,
   current value: 0, add value: -32000000000)
ERROR - PLEASE FIX OR REPORT ProtoArray adjustWeight bug:
  Delta to be subtracted causes uint64 underflow for block 0x27cd17…25c7 (27366).
  Attempting to subtract 32000000000 from 0

This PR does not claim to reproduce either client bug exactly. It adds a minimal, spec-faithful scenario that emits the per-variant weights an incremental implementation must match.

What the test does

Trace, on top of the Gloas anchor (minimal + mainnet presets):

  1. Apply block_1 at slot 1 and reveal its execution payload envelope, so (block_1, FULL) is a viable fork-choice node.
  2. Tick to slot 2; the slot-2 beacon committee attests beacon_block_root = block_1 with payload_index = 1 (payload_present = True).
  3. Tick to slot 3; the slot-3 beacon committee attests beacon_block_root = block_1 with payload_index = 0 (payload_present = False).

The test then asserts:

  • Each FULL attester's LatestMessage.payload_present is True (slot 2 stamp).
  • Each EMPTY attester's LatestMessage.payload_present is False (slot 3 stamp).
  • Any overlap between the two committees correctly moves to payload_present = False (the later slot > store.latest_messages[i].slot rule from update_latest_messages).
  • get_attestation_score is > 0 for (block_1, EMPTY), and > 0 for (block_1, FULL) whenever at least one slot-2 voter did not re-attest at slot 3.
  • full_score + empty_score <= sum(effective_balance for v in full ∪ empty) — i.e. no validator's balance is double-counted across the two variants.
  • A final output_store_checks(..., with_viable_for_head_weights=True) emits viable_for_head_roots_and_weights in steps.yaml, which lists the two (block_1, FULL) and (block_1, EMPTY) leaves with their weights — the exact values incremental implementations must reproduce.

Generated steps.yaml tail (minimal preset):

- checks:
    time: 24
    head: { slot: 1, root: '0xe4f0…419b', payload_status: 1 }
    ...
    viable_for_head_roots_and_weights:
    - { root: '0xe4f0…419b', weight: 128000000000, payload_status: 1 }
    - { root: '0xe4f0…419b', weight: 128000000000, payload_status: 0 }

Test plan

  • make test k="lmd_vote_split_across_payload_status_variants" fork=gloas — passes (2 passed across minimal+mainnet, 1 skipped on general preset).
  • make test k="lmd_vote_split_across_payload_status_variants" fork=gloas reftests=true — fixtures generated under reftests/minimal/gloas/fork_choice/split_payload_status_lmd_votes/ and reftests/mainnet/gloas/fork_choice/split_payload_status_lmd_votes/.
  • make lint — clean.
  • Confirm trace replays cleanly on at least one client's fork-choice consumer (out of scope for this PR; flagged for follow-up by Teku/Grandine teams).

Notes for reviewers

Adds a Gloas fork_choice trace in which two disjoint beacon committees
attest to the same beacon_block_root with opposite `payload_index`
values, so the same root receives non-zero LMD weight on both
`PAYLOAD_STATUS_FULL` and `PAYLOAD_STATUS_EMPTY` viable fork-choice
nodes simultaneously.

The pyspec passes by construction because `get_attestation_score`
recomputes per-node weight as a stateless sum over
`store.latest_messages` (via `get_supported_node`). Incremental
fork-choice implementations that maintain running per-node weights
must produce the same `viable_for_head_roots_and_weights` values
emitted in the generated trace; this is the surface where weight
accounting bugs (e.g. uint64 underflow, "attesting balance should
never go below zero") for the new FULL/EMPTY axis surface.
@github-actions github-actions Bot added the testing CI, actions, tests, testing infra label Jun 8, 2026
@jtraglia jtraglia changed the title Gloas: add fork_choice test for split LMD votes across payload variants Add fork choice test for split LMD votes across payload variants Jun 8, 2026
@jtraglia jtraglia requested a review from brech1 June 8, 2026 15:41
Address review feedback: rephrase docstring to describe the incremental
weight optimisation abstractly without naming specific client implementations.

@brech1 brech1 left a comment

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.

Some small comments but LGTM!

Address review feedback on the split-payload LMD test: replace the local
init block with setup_one_block_store, drop the _advance_store_to_slot
wrapper in favour of tick_store_to_slot, and assert get_attestation_score
returns the pre-calculated weight per payload variant rather than just
checking sign.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants