Only return viable payload-status variants from get_head

specs/gloas/fork-choice.md

@@ -34,6 +34,7 @@
   - [New `get_payload_status_tiebreaker`](#new-get_payload_status_tiebreaker)
   - [New `should_apply_proposer_boost`](#new-should_apply_proposer_boost)
   - [Modified `get_weight`](#modified-get_weight)
+  - [Modified `get_filtered_node_tree`](#modified-get_filtered_node_tree)
   - [Modified `get_node_children`](#modified-get_node_children)
   - [Modified `get_head`](#modified-get_head)
   - [Modified `get_latest_message_epoch`](#modified-get_latest_message_epoch)
@@ -123,6 +124,12 @@ class ForkChoiceNode:
     root: Root
     # [New in Gloas:EIP7732]
     payload_status: PayloadStatus  # One of PAYLOAD_STATUS_* values
+
+    # The dataclass-generated ``__hash__`` would call the SHA256 ``hash``
+    # function defined in this module instead of the builtin ``hash``, so
+    # it is defined explicitly.
+    def __hash__(self) -> int:
+        return int.from_bytes(self.root, "little") * 31 + int(self.payload_status)
 ```
 
 ### Modified `PayloadAttributes`
@@ -543,15 +550,32 @@ def get_weight(store: Store, node: ForkChoiceNode) -> Gwei:
     return attestation_score + proposer_score
 ```
 
+### Modified `get_filtered_node_tree`
+
+```python
+def get_filtered_node_tree(store: Store) -> Set[ForkChoiceNode]:
+    """
+    Retrieve a filtered node tree from ``store``, only returning branches
+    whose leaf state's justified/finalized info agrees with that in ``store``.
+    """
+    # [Modified in Gloas:EIP7732]
+    base = ForkChoiceNode(
+        root=store.justified_checkpoint.root,
+        payload_status=PAYLOAD_STATUS_PENDING,
+    )
+    # [Modified in Gloas:EIP7732]
+    viable_nodes: Set[ForkChoiceNode] = set()
+    filter_node_tree(store, base, viable_nodes)
+    return viable_nodes
+```
+
 ### Modified `get_node_children`
 
 *Note*: This function is modified to introduce new type of children nodes
 representing *full* and *empty* blocks.
 
 ```python
-def get_node_children(
-    store: Store, blocks: Dict[Root, BeaconBlock], node: ForkChoiceNode
-) -> Sequence[ForkChoiceNode]:
+def get_node_children(store: Store, node: ForkChoiceNode) -> Sequence[ForkChoiceNode]:
     if node.payload_status == PAYLOAD_STATUS_PENDING:
         children = [ForkChoiceNode(root=node.root, payload_status=PAYLOAD_STATUS_EMPTY)]
         if is_payload_verified(store, node.root):
@@ -560,10 +584,10 @@ def get_node_children(
     else:
         return [
             ForkChoiceNode(root=root, payload_status=PAYLOAD_STATUS_PENDING)
-            for root in blocks
+            for root in store.blocks
             if (
-                blocks[root].parent_root == node.root
-                and node.payload_status == get_parent_payload_status(store, blocks[root])
+                store.blocks[root].parent_root == node.root
+                and node.payload_status == get_parent_payload_status(store, store.blocks[root])
             )
         ]
 ```
@@ -575,8 +599,8 @@ between *full* and *empty* nodes.
 
 ```python
 def get_head(store: Store) -> ForkChoiceNode:
-    # Get filtered block tree that only includes viable branches
-    blocks = get_filtered_block_tree(store)
+    # Get filtered node tree that only includes viable branches
+    filtered_node_tree = get_filtered_node_tree(store)
     # Execute the LMD-GHOST fork-choice
     head = ForkChoiceNode(
         root=store.justified_checkpoint.root,
@@ -585,7 +609,9 @@ def get_head(store: Store) -> ForkChoiceNode:
     )
 
     while True:
-        children = get_node_children(store, blocks, head)
+        children = [
+            child for child in get_node_children(store, head) if child in filtered_node_tree
+        ]
         if len(children) == 0:
             return head
         # Sort by latest attesting balance with ties broken lexicographically

specs/phase0/fork-choice.md

@@ -26,9 +26,9 @@
     - [`get_proposer_score`](#get_proposer_score)
     - [`get_weight`](#get_weight)
     - [`get_voting_source`](#get_voting_source)
-    - [`filter_block_tree`](#filter_block_tree)
-    - [`get_filtered_block_tree`](#get_filtered_block_tree)
     - [`get_node_children`](#get_node_children)
+    - [`filter_node_tree`](#filter_node_tree)
+    - [`get_filtered_node_tree`](#get_filtered_node_tree)
     - [`get_head`](#get_head)
     - [`update_checkpoints`](#update_checkpoints)
     - [`update_unrealized_checkpoints`](#update_unrealized_checkpoints)
@@ -152,6 +152,12 @@ This abstraction is introduced to support upgradability.
 @dataclass(eq=True, frozen=True)
 class ForkChoiceNode:
     root: Root
+
+    # The dataclass-generated ``__hash__`` would call the SHA256 ``hash``
+    # function defined in this module instead of the builtin ``hash``, so
+    # it is defined explicitly.
+    def __hash__(self) -> int:
+        return int.from_bytes(self.root, "little")
 ```
 
 #### `LatestMessage`
@@ -394,28 +400,43 @@ def get_voting_source(store: Store, block_root: Root) -> Checkpoint:
         return head_state.current_justified_checkpoint
 ```
 
-#### `filter_block_tree`
+#### `get_node_children`
+
+```python
+def get_node_children(
+    store: Store,
+    node: ForkChoiceNode,
+) -> Sequence[ForkChoiceNode]:
+    return [
+        ForkChoiceNode(root=root)
+        for root in store.blocks
+        if store.blocks[root].parent_root == node.root
+    ]
+```
+
+#### `filter_node_tree`
 
-*Note*: External calls to `filter_block_tree` (i.e., any calls that are not made
-by the recursive logic in this function) MUST set `block_root` to
-`store.justified_checkpoint.root`.
+*Note*: External calls to `filter_node_tree` (i.e., any calls that are not made
+by the recursive logic in this function) MUST set `node` to a `ForkChoiceNode`
+with root `store.justified_checkpoint.root`.
 
 ```python
-def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconBlock]) -> bool:
-    block = store.blocks[block_root]
-    children = [root for root in store.blocks if store.blocks[root].parent_root == block_root]
+def filter_node_tree(store: Store, node: ForkChoiceNode, viable_nodes: Set[ForkChoiceNode]) -> bool:
+    children = get_node_children(store, node)
 
     # If any children branches contain expected finalized/justified checkpoints,
-    # add to filtered block-tree and signal viability to parent.
+    # add to filtered node tree and signal viability to parent.
     if any(children):
-        filter_block_tree_result = [filter_block_tree(store, child, blocks) for child in children]
-        if any(filter_block_tree_result):
-            blocks[block_root] = block
+        filter_node_tree_result = [
+            filter_node_tree(store, child, viable_nodes) for child in children
+        ]
+        if any(filter_node_tree_result):
+            viable_nodes.add(node)
             return True
         return False
 
     current_epoch = get_current_store_epoch(store)
-    voting_source = get_voting_source(store, block_root)
+    voting_source = get_voting_source(store, node.root)
 
     # The voting source should be either at the same height as the store's justified checkpoint or
     # not more than two epochs ago
@@ -427,7 +448,7 @@ def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconB
 
     finalized_checkpoint_block = get_checkpoint_block(
         store,
-        block_root,
+        node.root,
         store.finalized_checkpoint.epoch,
     )
 
@@ -436,50 +457,41 @@ def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconB
         or store.finalized_checkpoint.root == finalized_checkpoint_block
     )
 
-    # If expected finalized/justified, add to viable block-tree and signal viability to parent.
+    # If expected finalized/justified, add to viable node tree and signal viability to parent.
     if correct_justified and correct_finalized:
-        blocks[block_root] = block
+        viable_nodes.add(node)
         return True
 
     # Otherwise, branch not viable
     return False
 ```
 
-#### `get_filtered_block_tree`
+#### `get_filtered_node_tree`
 
 ```python
-def get_filtered_block_tree(store: Store) -> Dict[Root, BeaconBlock]:
+def get_filtered_node_tree(store: Store) -> Set[ForkChoiceNode]:
     """
-    Retrieve a filtered block tree from ``store``, only returning branches
+    Retrieve a filtered node tree from ``store``, only returning branches
     whose leaf state's justified/finalized info agrees with that in ``store``.
     """
-    base = store.justified_checkpoint.root
-    blocks: Dict[Root, BeaconBlock] = {}
-    filter_block_tree(store, base, blocks)
-    return blocks
-```
-
-#### `get_node_children`
-
-```python
-def get_node_children(
-    store: Store,  # noqa: ARG001
-    blocks: Dict[Root, BeaconBlock],
-    node: ForkChoiceNode,
-) -> Sequence[ForkChoiceNode]:
-    return [ForkChoiceNode(root=root) for root in blocks if blocks[root].parent_root == node.root]
+    base = ForkChoiceNode(root=store.justified_checkpoint.root)
+    viable_nodes: Set[ForkChoiceNode] = set()
+    filter_node_tree(store, base, viable_nodes)
+    return viable_nodes
 ```
 
 #### `get_head`
 
 ```python
 def get_head(store: Store) -> ForkChoiceNode:
-    # Get filtered block tree that only includes viable branches
-    blocks = get_filtered_block_tree(store)
+    # Get filtered node tree that only includes viable branches
+    filtered_node_tree = get_filtered_node_tree(store)
     # Execute the LMD-GHOST fork choice
     head = ForkChoiceNode(root=store.justified_checkpoint.root)
     while True:
-        children = get_node_children(store, blocks, head)
+        children = [
+            child for child in get_node_children(store, head) if child in filtered_node_tree
+        ]
         if len(children) == 0:
             return head
         # Sort by latest attesting balance with ties broken lexicographically

sync/optimistic.md

@@ -388,7 +388,7 @@ verified or the block is older than `SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY`.
 These restraints are applied in order to mitigate an attack where a block which
 enables execution (a *transition block*) can reference a junk parent hash. This
 makes it impossible for honest nodes to build atop that block. If an attacker
-exploits a nuance in fork choice `filter_block_tree`, they can, in some rare
+exploits a nuance in fork choice `filter_node_tree`, they can, in some rare
 cases, produce a junk block that out-competes all locally produced blocks for
 the head. This prevents a node from producing a chain of blocks, therefore
 breaking liveness.

tests/core/pyspec/eth_consensus_specs/test/gloas/fork_choice/test_filter_node_tree_variants.py

@@ -0,0 +1,135 @@
+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
+from eth_consensus_specs.test.helpers.block import build_empty_block_for_next_slot
+from eth_consensus_specs.test.helpers.constants import MINIMAL
+from eth_consensus_specs.test.helpers.execution_payload import (
+    build_signed_execution_payload_envelope,
+)
+from eth_consensus_specs.test.helpers.fork_choice import (
+    add_execution_payload,
+    on_tick_and_append_step,
+    output_head_check,
+    setup_finalized_store,
+    tick_and_add_block,
+    tick_and_run_on_attestation,
+)
+from eth_consensus_specs.test.helpers.state import (
+    next_slots,
+    state_transition_and_sign_block,
+)
+
+
+@with_gloas_and_later
+@with_presets([MINIMAL], reason="too slow")
+@spec_state_test
+def test_get_head_prunes_childless_unviable_full_variant(spec, state):
+    """
+    Reproduces issue #5496: a childless FULL payload-status variant of a block
+    that fails the FFG test must not be returned by get_head.
+
+    Block B is built on the justified checkpoint from a fork of its state, so
+    its voting source is stale and B fails the FFG test. K builds on EMPTY(B)
+    and pulls up justification to the store's justified checkpoint (set below,
+    as in the issue's example), so EMPTY(B) is viable. FULL(B) exists because
+    B's envelope is delivered, but it is childless and never passes the FFG
+    test, so get_head must not return it.
+    """
+    store, _, test_steps = yield from setup_finalized_store(spec, state)
+
+    justified_epoch = store.justified_checkpoint.epoch
+    assert store.finalized_checkpoint.epoch == justified_epoch - 1
+
+    justified_state = store.block_states[store.justified_checkpoint.root]
+
+    # B is the first block of epoch `justified_epoch + 1` on a fork of the
+    # justified state. The fork has no on-chain votes, so B's voting source is
+    # stale and B fails the FFG test.
+    b_slot = spec.compute_start_slot_at_epoch(justified_epoch + 1)
+    fork_state = justified_state.copy()
+    next_slots(spec, fork_state, b_slot - fork_state.slot - 1)
+    b_block = build_empty_block_for_next_slot(spec, fork_state)
+    assert b_block.slot == b_slot
+    signed_b = state_transition_and_sign_block(spec, fork_state, b_block)
+    b_root = signed_b.message.hash_tree_root()
+    yield from tick_and_add_block(spec, store, signed_b, test_steps)
+
+    # Deliver B's envelope so the FULL(B) variant exists
+    b_state = store.block_states[b_root]
+    envelope = build_signed_execution_payload_envelope(spec, b_state, b_root, signed_b)
+    yield from add_execution_payload(spec, store, envelope, test_steps)
+    assert spec.is_payload_verified(store, b_root)
+
+    # Tick to the start of the next epoch so attestations for B's epoch can be
+    # processed
+    k_slot = spec.compute_start_slot_at_epoch(justified_epoch + 2)
+    on_tick_and_append_step(
+        spec, store, store.genesis_time + k_slot * spec.config.SLOT_DURATION_MS // 1000, test_steps
+    )
+
+    # Attest B's FULL variant with all committees of B's epoch, so the B branch
+    # outweighs the main chain in the LMD-GHOST walk
+    committees_per_slot = spec.get_committee_count_per_slot(b_state, justified_epoch + 1)
+    for slot in range(b_slot + 1, k_slot):
+        att_state = b_state.copy()
+        next_slots(spec, att_state, slot - att_state.slot)
+        for index in range(committees_per_slot):
+            attestation = get_valid_attestation(
+                spec,
+                att_state,
+                slot=slot,
+                index=index,
+                payload_index=1,
+                beacon_block_root=b_root,
+                signed=True,
+            )
+            yield from tick_and_run_on_attestation(spec, store, attestation, test_steps)
+
+    # K builds on EMPTY(B) at the start of the next epoch
+    k_state = b_state.copy()
+    next_slots(spec, k_state, k_slot - k_state.slot - 1)
+    k_block = build_empty_block_for_next_slot(spec, k_state)
+    assert k_block.slot == k_slot
+    signed_k = state_transition_and_sign_block(spec, k_state, k_block)
+    k_root = signed_k.message.hash_tree_root()
+    yield from tick_and_add_block(spec, store, signed_k, test_steps)
+    # K must claim an EMPTY parent for the B branch to stay viable
+    assert spec.get_parent_payload_status(store, signed_k.message) == spec.PAYLOAD_STATUS_EMPTY
+
+    # Simulate K's chain having pulled up justification to the store's justified
+    # checkpoint, while B's justification remains stale
+    store.unrealized_justifications[k_root] = store.justified_checkpoint
+
+    # Advance to the next epoch so that B is more than two epochs behind the
+    # voting source required by the FFG test
+    next_epoch_slot = spec.compute_start_slot_at_epoch(justified_epoch + 3)
+    on_tick_and_append_step(
+        spec,
+        store,
+        store.genesis_time + next_epoch_slot * spec.config.SLOT_DURATION_MS // 1000,
+        test_steps,
+    )
+
+    full_b_node = spec.ForkChoiceNode(root=b_root, payload_status=spec.PAYLOAD_STATUS_FULL)
+    empty_b_node = spec.ForkChoiceNode(root=b_root, payload_status=spec.PAYLOAD_STATUS_EMPTY)
+
+    # B fails the FFG test while K passes it
+    assert spec.get_voting_source(store, b_root).epoch + 2 < spec.get_current_store_epoch(store)
+    assert spec.get_voting_source(store, b_root).epoch != store.justified_checkpoint.epoch
+    assert spec.get_voting_source(store, k_root).epoch == store.justified_checkpoint.epoch
+
+    # The childless FULL(B) variant is not viable and must not be the head
+    head = spec.get_head(store)
+    assert head != full_b_node
+    assert head.root == k_root
+    assert head.payload_status == spec.PAYLOAD_STATUS_EMPTY
+
+    filtered_tree = spec.get_filtered_node_tree(store)
+    assert full_b_node not in filtered_tree
+    assert empty_b_node in filtered_tree
+
+    output_head_check(spec, store, test_steps)
+    yield "steps", test_steps