Fix payload availability lookup using the parent block's slot (#5473)

In `get_attestation_participation_flag_indices`, the payload
availability bit was looked up at `data.slot`, which is not the slot of
the attested block when there are skipped slots. As a result, an
attestation to a skipped slot with `data.index == 1` always failed
payload matching and missed out on the timely head reward.

Since the timely head flag requires both the minimum inclusion delay and
a head root that matches the block root at the attestation slot, an
attestation can only receive the flag if it attests to the parent block
of the block that includes it. The availability bit for the attested
block therefore lives at the parent block's slot, which
`process_execution_payload_bid` now returns (read from the bid in the
state before it is overwritten) and `process_block` passes down to
`process_attestation`.

This is a loop-free alternative to #5442, as suggested in
#5442 (comment).
Unlike the walk back through the historical block roots, there is no
special case when the attested block is more than
`SLOTS_PER_HISTORICAL_ROOT` slots old. The availability bit at the
parent block's slot is always up to date, because the including block's
parent payload processing is what last wrote it.

Fixes #5399.

Author: jtraglia

specs/gloas/beacon-chain.md

@@ -872,11 +872,21 @@ def get_next_sync_committee_indices(state: BeaconState) -> Sequence[ValidatorInd
 #### Modified `get_attestation_participation_flag_indices`
 
 *Note*: The function `get_attestation_participation_flag_indices` is modified to
-include a new payload matching constraint to `is_matching_head`.
+include a new payload matching constraint to `is_matching_head`. The new
+`parent_slot` parameter is the slot of the parent block of the block being
+processed. Since the timely head flag requires both the minimum inclusion delay
+and a head root that matches the block root at the attestation slot, an
+attestation can only receive the flag if it attests to the parent block. The
+payload availability of the attested block is therefore tracked at
+`parent_slot`, even when `data.slot` is a skipped slot.
 
 ```python
 def get_attestation_participation_flag_indices(
-    state: BeaconState, data: AttestationData, inclusion_delay: Uint64
+    state: BeaconState,
+    data: AttestationData,
+    inclusion_delay: Uint64,
+    # [New in Gloas:EIP7732]
+    parent_slot: Slot,
 ) -> Sequence[int]:
     """
     Return the flag indices that are satisfied by an attestation.
@@ -898,7 +908,7 @@ def get_attestation_participation_flag_indices(
         assert data.index == 0
         payload_matches = True
     else:
-        slot_index = data.slot % SLOTS_PER_HISTORICAL_ROOT
+        slot_index = parent_slot % SLOTS_PER_HISTORICAL_ROOT
         payload_index = state.execution_payload_availability[slot_index]
         payload_matches = data.index == payload_index
 
@@ -1251,11 +1261,11 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None:
     # [Modified in Gloas:EIP7732]
     # Removed `process_execution_payload`
     # [New in Gloas:EIP7732]
-    process_execution_payload_bid(state, block.body.signed_execution_payload_bid)
+    parent_slot = process_execution_payload_bid(state, block.body.signed_execution_payload_bid)
     process_randao(state, block.body)
     process_eth1_data(state, block.body)
     # [Modified in Gloas:EIP7732]
-    process_operations(state, block.body)
+    process_operations(state, block.body, parent_slot)
     process_sync_aggregate(state, block.body.sync_aggregate)
 ```
 
@@ -1607,10 +1617,14 @@ def verify_execution_payload_bid_signature(
 
 ##### New `process_execution_payload_bid`
 
+*Note*: This function returns the slot of the parent block, read from the bid in
+the state before it is overwritten by the new bid. The slot is later given to
+`process_attestation` to look up the payload availability of the attested block.
+
 ```python
 def process_execution_payload_bid(
     state: BeaconState, signed_bid: SignedExecutionPayloadBid
-) -> None:
+) -> Slot:
     bid = signed_bid.message
     builder_index = bid.builder_index
     amount = bid.value
@@ -1658,25 +1672,37 @@ def process_execution_payload_bid(
             pending_payment
         )
 
+    # Cache the parent block's slot before overwriting the bid
+    parent_slot = state.latest_execution_payload_bid.slot
+
     # Cache the signed execution payload bid
     state.latest_execution_payload_bid = bid
+
+    return parent_slot
 ```
 
 #### Operations
 
 ##### Modified `process_operations`
 
-*Note*: `process_operations` is modified to process PTC attestations and removes
-calls to `process_deposit_request`, `process_withdrawal_request`, and
+*Note*: `process_operations` is modified to process PTC attestations, to pass
+the parent block's slot to `process_attestation`, and removes calls to
+`process_deposit_request`, `process_withdrawal_request`, and
 `process_consolidation_request`.
 
 ```python
-def process_operations(state: BeaconState, body: BeaconBlockBody) -> None:
+def process_operations(
+    state: BeaconState,
+    body: BeaconBlockBody,
+    # [New in Gloas:EIP7732]
+    parent_slot: Slot,
+) -> None:
     assert len(body.deposits) == 0
 
-    def for_ops(operations: Sequence[Any], fn: Callable[[BeaconState, Any], None]) -> None:
+    # [Modified in Gloas:EIP7732]
+    def for_ops(operations: Sequence[Any], fn: Callable[..., None], *args: Any) -> None:
         for operation in operations:
-            fn(state, operation)
+            fn(state, operation, *args)
 
     # [New in Gloas:EIP7688]
     assert len(body.proposer_slashings) <= MAX_PROPOSER_SLASHINGS
@@ -1690,7 +1716,7 @@ def process_operations(state: BeaconState, body: BeaconBlockBody) -> None:
     for_ops(body.proposer_slashings, process_proposer_slashing)
     for_ops(body.attester_slashings, process_attester_slashing)
     # [Modified in Gloas:EIP7732]
-    for_ops(body.attestations, process_attestation)
+    for_ops(body.attestations, process_attestation, parent_slot)
     for_ops(body.voluntary_exits, process_voluntary_exit)
     for_ops(body.bls_to_execution_changes, process_bls_to_execution_change)
     # [Modified in Gloas:EIP7732]
@@ -1827,10 +1853,16 @@ def process_builder_exit_request(state: BeaconState, request: BuilderExitRequest
 
 *Note*: The function is modified to track the weight for pending builder
 payments and to use the `index` field in the `AttestationData` to signal the
-payload availability.
+payload availability. The new `parent_slot` parameter, the slot of the parent
+block, is where the payload availability of the attested block is looked up.
 
 ```python
-def process_attestation(state: BeaconState, attestation: Attestation) -> None:
+def process_attestation(
+    state: BeaconState,
+    attestation: Attestation,
+    # [New in Gloas:EIP7732]
+    parent_slot: Slot,
+) -> None:
     data = attestation.data
     assert data.target.epoch in (get_previous_epoch(state), get_current_epoch(state))
     assert data.target.epoch == compute_epoch_at_slot(data.slot)
@@ -1855,8 +1887,9 @@ def process_attestation(state: BeaconState, attestation: Attestation) -> None:
     assert len(attestation.aggregation_bits) == committee_offset
 
     # Participation flag indices
+    # [Modified in Gloas:EIP7732]
     participation_flag_indices = get_attestation_participation_flag_indices(
-        state, data, state.slot - data.slot
+        state, data, state.slot - data.slot, parent_slot
     )
 
     # Verify signature

tests/core/pyspec/eth_consensus_specs/test/gloas/block_processing/test_process_attestation.py

@@ -1,13 +1,15 @@
 from eth_consensus_specs.test.context import (
     spec_state_test,
     with_gloas_and_later,
+    with_presets,
 )
 from eth_consensus_specs.test.helpers.attestations import (
     get_valid_attestation,
     run_attestation_processing,
     sign_attestation,
 )
 from eth_consensus_specs.test.helpers.block import apply_empty_block
+from eth_consensus_specs.test.helpers.constants import MINIMAL
 from eth_consensus_specs.test.helpers.state import (
     next_slots,
     transition_to_slot_via_block,
@@ -348,15 +350,19 @@ def test_same_slot_attestation_ignores_payload_availability(spec, state):
 @spec_state_test
 def test_matching_payload_gets_head_flag(spec, state):
     """
-    Test get_attestation_participation_flag_indices for historical slots where
-    is_matching_payload = data.index == state.execution_payload_availability[data.slot % SLOTS_PER_HISTORICAL_ROOT]
-    and is_matching_head = is_matching_blockroot and is_matching_payload.
+    Test that ``get_attestation_participation_flag_indices`` compares
+    ``data.index`` against the availability bit of the attested block's slot
+    (the parent block's slot), not ``data.slot``, when the attestation is for
+    a skipped slot.
     """
     # Use missed slot scenario: blocks for slots 1, 2, 3 but skip slot 4
     slot_4_block_root = _setup_missed_slot_scenario(spec, state)
 
-    # Set payload availability bit to 1 for slot 4
-    state.execution_payload_availability[4] = 1
+    # The attested block is at slot 3, so its availability bit is the one
+    # that matters. Set it to 1 to indicate the payload was revealed. Unset
+    # the bit at data.slot to show that it is ignored.
+    state.execution_payload_availability[3] = 1
+    state.execution_payload_availability[4] = 0
 
     # Create attestation with index = 1 to match the availability bit
     attestation = get_valid_attestation(spec, state, slot=4, beacon_block_root=slot_4_block_root)
@@ -388,8 +394,11 @@ def test_mismatched_payload_no_head_flag(spec, state):
     # Use missed slot scenario: blocks for slots 1, 2, 3 but skip slot 4
     slot_4_block_root = _setup_missed_slot_scenario(spec, state)
 
-    # Set payload availability bit to 0 for slot 4
-    state.execution_payload_availability[4] = 0
+    # The attested block is at slot 3, so its availability bit is the one
+    # that matters. Set it to 0 to indicate the payload was withheld. Set
+    # the bit at data.slot to show that it is ignored.
+    state.execution_payload_availability[3] = 0
+    state.execution_payload_availability[4] = 1
 
     # Create attestation with index = 1 which does NOT match availability bit = 0
     attestation = get_valid_attestation(spec, state, slot=4, beacon_block_root=slot_4_block_root)
@@ -411,6 +420,44 @@ def test_mismatched_payload_no_head_flag(spec, state):
     assert not final_head_flag, "Should not get head flag when payload doesn't match"
 
 
+@with_gloas_and_later
+@with_presets([MINIMAL], reason="too slow")
+@spec_state_test
+def test_old_attested_block_gets_head_flag(spec, state):
+    """
+    Test that an attestation still receives the timely head flag when the
+    attested block is ``SLOTS_PER_HISTORICAL_ROOT`` slots old. The availability
+    bit at the parent block's slot is up to date no matter how old the parent
+    block is, because the parent payload processing of the including block is
+    what last wrote it.
+    """
+    # Skip every slot since genesis, so the attested block (genesis) is
+    # SLOTS_PER_HISTORICAL_ROOT slots before the attestation slot
+    attestation_slot = spec.Slot(spec.SLOTS_PER_HISTORICAL_ROOT)
+    next_slots(spec, state, attestation_slot + spec.MIN_ATTESTATION_INCLUSION_DELAY)
+
+    attestation = get_valid_attestation(spec, state, slot=attestation_slot, signed=True)
+
+    assert attestation.data.index == 0
+    assert spec.is_attestation_same_slot(state, attestation.data) is False
+    # The attested block (genesis) still matches the historical block roots
+    assert attestation.data.beacon_block_root == spec.get_block_root_at_slot(
+        state, attestation_slot
+    )
+    # The genesis payload was never revealed, which matches data.index == 0
+    parent_slot = state.latest_execution_payload_bid.slot
+    assert parent_slot == 0
+    assert not state.execution_payload_availability[parent_slot % spec.SLOTS_PER_HISTORICAL_ROOT]
+
+    attesting_indices = spec.get_attesting_indices(state, attestation)
+    validator_index = next(iter(attesting_indices))
+
+    yield from run_attestation_processing(spec, state, attestation, valid=True)
+
+    final_participation = state.current_epoch_participation[validator_index]
+    assert spec.has_flag(final_participation, spec.TIMELY_HEAD_FLAG_INDEX)
+
+
 @with_gloas_and_later
 @spec_state_test
 def test_builder_payment_weight_tracking_previous_epoch(spec, state):
@@ -438,7 +485,10 @@ def test_builder_payment_weight_tracking_previous_epoch(spec, state):
 
     attester = spec.get_beacon_committee(state, attestation_slot, 0)[0]
     expected_flag_indices = spec.get_attestation_participation_flag_indices(
-        state, attestation.data, state.slot - attestation.data.slot
+        state,
+        attestation.data,
+        state.slot - attestation.data.slot,
+        state.latest_execution_payload_bid.slot,
     )
     pre_prev_flags = state.previous_epoch_participation[attester]
     pre_curr_flags = state.current_epoch_participation[attester]

tests/core/pyspec/eth_consensus_specs/test/helpers/attestations.py

@@ -18,6 +18,15 @@
 from eth_consensus_specs.utils.ssz.ssz_typing import Bitlist
 
 
+def process_attestation(spec, state, attestation):
+    if is_post_gloas(spec):
+        # Outside of block processing, the bid in the state is still the
+        # parent block's bid, so its slot is the parent block's slot.
+        spec.process_attestation(state, attestation, state.latest_execution_payload_bid.slot)
+    else:
+        spec.process_attestation(state, attestation)
+
+
 def run_attestation_processing(spec, state, attestation, valid=True):
     """
     Run ``process_attestation``, yielding:
@@ -33,7 +42,7 @@ def run_attestation_processing(spec, state, attestation, valid=True):
 
     # If the attestation is invalid, processing is aborted, and there is no post-state.
     if not valid:
-        expect_assertion_error(lambda: spec.process_attestation(state, attestation))
+        expect_assertion_error(lambda: process_attestation(spec, state, attestation))
         yield "post", None
         return
 
@@ -42,7 +51,7 @@ def run_attestation_processing(spec, state, attestation, valid=True):
         previous_epoch_count = len(state.previous_epoch_attestations)
 
     # process attestation
-    spec.process_attestation(state, attestation)
+    process_attestation(spec, state, attestation)
 
     # Make sure the attestation has been processed
     if not is_post_altair(spec):
@@ -253,7 +262,7 @@ def add_attestations_to_state(spec, state, attestations, slot):
     if state.slot < slot:
         spec.process_slots(state, slot)
     for attestation in attestations:
-        spec.process_attestation(state, attestation)
+        process_attestation(spec, state, attestation)
 
 
 def get_valid_attestations_at_slot(

tests/core/pyspec/eth_consensus_specs/test/helpers/block_processing.py

@@ -1,3 +1,6 @@
+from eth_consensus_specs.test.helpers.attestations import process_attestation
+
+
 def for_ops(state, operations, fn) -> None:
     for operation in operations:
         fn(state, operation)
@@ -19,7 +22,9 @@ def get_process_calls(spec):
             state, block.body.shard_headers, spec.process_shard_header
         ),
         "process_attestation": lambda state, block: for_ops(
-            state, block.body.attestations, spec.process_attestation
+            state,
+            block.body.attestations,
+            lambda state, attestation: process_attestation(spec, state, attestation),
         ),
         "process_deposit": lambda state, block: for_ops(
             state, block.body.deposits, spec.process_deposit

tests/core/pyspec/eth_consensus_specs/test/phase0/sanity/test_blocks.py

@@ -185,7 +185,12 @@ def process_and_sign_block_without_header_validations(spec, state, block):
     # Perform rest of process_block transitions
     spec.process_randao(state, block.body)
     spec.process_eth1_data(state, block.body)
-    spec.process_operations(state, block.body)
+    if is_post_gloas(spec):
+        # The bid is not processed here, so the bid in the state is still
+        # the parent block's bid and its slot is the parent block's slot.
+        spec.process_operations(state, block.body, state.latest_execution_payload_bid.slot)
+    else:
+        spec.process_operations(state, block.body)
     if is_post_altair(spec):
         spec.process_sync_aggregate(state, block.body.sync_aggregate)