diff --git a/specs/bellatrix/optimistic-sync.md b/specs/bellatrix/optimistic-sync.md index 567f482438f..c4d3dcb7fb4 100644 --- a/specs/bellatrix/optimistic-sync.md +++ b/specs/bellatrix/optimistic-sync.md @@ -371,7 +371,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. diff --git a/specs/gloas/fork-choice.md b/specs/gloas/fork-choice.md index e86225299ad..7802e1d4bdd 100644 --- a/specs/gloas/fork-choice.md +++ b/specs/gloas/fork-choice.md @@ -36,6 +36,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) @@ -585,15 +586,29 @@ 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) -> Sequence[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, + ) + return filter_node_tree(store, base) +``` + ### 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): @@ -602,10 +617,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]) ) ] ``` @@ -617,8 +632,18 @@ 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) + + # [New in Gloas:EIP7732] + if not any(filtered_node_tree): + # Return empty node if there are no viable nodes + # to ensure that head is never a pending node + return ForkChoiceNode( + root=store.justified_checkpoint.root, + payload_status=PAYLOAD_STATUS_EMPTY, + ) + # Execute the LMD-GHOST fork-choice head = ForkChoiceNode( root=store.justified_checkpoint.root, @@ -627,7 +652,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 diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 206c2bd7e5a..18e6b0f2575 100644 --- a/specs/phase0/fork-choice.md +++ b/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) @@ -394,28 +394,38 @@ 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 + ] +``` -*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`. +#### `filter_node_tree` ```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) -> Sequence[ForkChoiceNode]: + 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. + # include this node and those descendants in the filtered node tree. 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 - return True - return False + viable_nodes: list[ForkChoiceNode] = [] + for child in children: + viable_nodes.extend(filter_node_tree(store, child)) + if any(viable_nodes): + return viable_nodes + [node] + return [] 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 +437,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 +446,38 @@ 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 - return True + return [node] # Otherwise, branch not viable - return False + return [] ``` -#### `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) -> Sequence[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) + return filter_node_tree(store, base) ``` #### `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 diff --git a/tests/core/pyspec/eth_consensus_specs/test/gloas/fork_choice/test_filter_node_tree_variants.py b/tests/core/pyspec/eth_consensus_specs/test/gloas/fork_choice/test_filter_node_tree_variants.py new file mode 100644 index 00000000000..9bb5ae8ea6c --- /dev/null +++ b/tests/core/pyspec/eth_consensus_specs/test/gloas/fork_choice/test_filter_node_tree_variants.py @@ -0,0 +1,350 @@ +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_at_slot +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 one epoch older than the store's justified checkpoint + and B fails the FFG test once the store advances past epoch + `justified_epoch + 1`. K builds on EMPTY(B) + through a chain of blocks that carry attestations targeting the + (B, justified_epoch + 1) checkpoint, so K's branch naturally justifies it + and K's voting source is pulled up to the store's justified checkpoint, + making EMPTY(B) 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 + # the fork's greatest justified checkpoint, one epoch older than the + # store's justified checkpoint: `justified_epoch - 1`. B passes the FFG + # test while the store is at epoch `justified_epoch + 1` and fails from + # epoch `justified_epoch + 2` onwards. + 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 for every slot of B's epoch after the first + # (post-Electra, each slot has a single aggregate), so the B branch + # outweighs the main chain in the LMD-GHOST walk + att_state = b_state.copy() + for slot in range(b_slot + 1, k_slot): + next_slots(spec, att_state, slot - att_state.slot) + attestation = get_valid_attestation_at_slot( + att_state, spec, slot, beacon_block_root=b_root, payload_index=1 + ) + yield from tick_and_run_on_attestation(spec, store, attestation, test_steps) + + # K's branch justifies the (B, justified_epoch + 1) checkpoint by including + # attestations targeting B in its blocks, so that K's voting source is + # naturally pulled up to the store's justified checkpoint once K is + # processed. Attesting all but the first and last slots of B's epoch + # exceeds 2/3 of the total active balance, which justifies the checkpoint + # at the next epoch boundary. (The first slot is skipped since same-slot + # attestations must carry index 0, and the last slot is skipped since its + # attestations could only be included after the epoch boundary.) + att_state = b_state.copy() + slot_attestations = [] + for slot in range(b_slot + 1, k_slot - 1): + next_slots(spec, att_state, slot - att_state.slot) + slot_attestations.append( + get_valid_attestation_at_slot( + att_state, spec, slot, beacon_block_root=b_root, payload_index=1 + ) + ) + + # Intermediate blocks on the B fork carry the attestations, up to the + # block-level maximum per block + branch_state = b_state.copy() + branch_root = b_root + for att_start in range(0, len(slot_attestations), spec.MAX_ATTESTATIONS_ELECTRA): + attestations = slot_attestations[att_start : att_start + spec.MAX_ATTESTATIONS_ELECTRA] + # The block must be built after the latest attested slot so that the + # attestation inclusion delay is satisfied + branch_slot = attestations[-1].data.slot + 1 + next_slots(spec, branch_state, branch_slot - branch_state.slot - 1) + branch_block = build_empty_block_for_next_slot(spec, branch_state) + assert branch_block.slot == branch_slot + for attestation in attestations: + branch_block.body.attestations.append(attestation) + signed_branch_block = state_transition_and_sign_block(spec, branch_state, branch_block) + yield from tick_and_add_block(spec, store, signed_branch_block, test_steps) + branch_root = signed_branch_block.message.hash_tree_root() + + # K builds on the last branch block at the start of the next epoch + k_state = store.block_states[branch_root].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 + # K's branch justifies the (B, justified_epoch + 1) checkpoint, so K's + # voting source is pulled up to the store's justified checkpoint, which + # advances to (B, justified_epoch + 1) + assert store.justified_checkpoint.epoch == justified_epoch + 1 + assert store.justified_checkpoint.root == b_root + assert store.unrealized_justifications[k_root] == store.justified_checkpoint + + # The store is already at the start of epoch `justified_epoch + 2`, where + # K was built. With B's voting source at `justified_epoch - 1`, B is more + # than two epochs behind the store's current epoch, so it fails the FFG + # test right here: no further advancement is needed for B to be filtered + # out. + + 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 + + +@with_gloas_and_later +@with_presets([MINIMAL], reason="too slow") +@spec_state_test +def test_get_head_prunes_childless_unviable_empty_variant(spec, state): + """ + Mirror of test_get_head_prunes_childless_unviable_full_variant: + a childless EMPTY payload-status variant of a block that fails the FFG test + must not be returned by get_head or included in get_filtered_node_tree. + + Block B is built on the justified checkpoint from a fork of its state, so + its voting source is one epoch older than the store's justified checkpoint + and B fails the FFG test once the store advances past epoch + `justified_epoch + 1`. K builds on FULL(B) through a chain of blocks that + carry attestations targeting the (B, justified_epoch + 1) checkpoint, so K's + branch naturally justifies it and K's voting source is pulled up to the + store's justified checkpoint, making FULL(B) viable. + EMPTY(B) exists because B was processed, but it is childless (since K built + on FULL(B)) and never passes the FFG test, so get_head must not return it + and get_filtered_node_tree must prune 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 + # the fork's greatest justified checkpoint, one epoch older than the + # store's justified checkpoint: `justified_epoch - 1`. B passes the FFG + # test while the store is at epoch `justified_epoch + 1` and fails from + # epoch `justified_epoch + 2` onwards. + 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 for every slot of B's epoch after the first + # (post-Electra, each slot has a single aggregate), so the B branch + # outweighs the main chain in the LMD-GHOST walk + att_state = b_state.copy() + for slot in range(b_slot + 1, k_slot): + next_slots(spec, att_state, slot - att_state.slot) + attestation = get_valid_attestation_at_slot( + att_state, spec, slot, beacon_block_root=b_root, payload_index=1 + ) + yield from tick_and_run_on_attestation(spec, store, attestation, test_steps) + + # K's branch justifies the (B, justified_epoch + 1) checkpoint by including + # attestations targeting B in its blocks, so that K's voting source is + # naturally pulled up to the store's justified checkpoint once K is + # processed. + att_state = b_state.copy() + slot_attestations = [] + for slot in range(b_slot + 1, k_slot - 1): + next_slots(spec, att_state, slot - att_state.slot) + slot_attestations.append( + get_valid_attestation_at_slot( + att_state, spec, slot, beacon_block_root=b_root, payload_index=1 + ) + ) + + # Intermediate blocks on the B fork carry the attestations. + # The first block building directly on B explicitly claims FULL parent payload status + # by matching parent_block_hash to signed_b's bid block_hash. + branch_state = b_state.copy() + branch_root = b_root + first_branch_block = None + for att_start in range(0, len(slot_attestations), spec.MAX_ATTESTATIONS_ELECTRA): + attestations = slot_attestations[att_start : att_start + spec.MAX_ATTESTATIONS_ELECTRA] + branch_slot = attestations[-1].data.slot + 1 + next_slots(spec, branch_state, branch_slot - branch_state.slot - 1) + branch_block = build_empty_block_for_next_slot(spec, branch_state) + assert branch_block.slot == branch_slot + for attestation in attestations: + branch_block.body.attestations.append(attestation) + if branch_root == b_root: + branch_block.body.signed_execution_payload_bid.message.parent_block_hash = ( + signed_b.message.body.signed_execution_payload_bid.message.block_hash + ) + branch_block.body.signed_execution_payload_bid.message.block_hash = spec.Hash32( + b"\x02" + b"\x00" * 31 + ) + signed_branch_block = state_transition_and_sign_block(spec, branch_state, branch_block) + if first_branch_block is None: + first_branch_block = signed_branch_block.message + yield from tick_and_add_block(spec, store, signed_branch_block, test_steps) + branch_root = signed_branch_block.message.hash_tree_root() + + # Confirm the branch directly builds on FULL(B) + assert spec.get_parent_payload_status(store, first_branch_block) == spec.PAYLOAD_STATUS_FULL + + # K builds on the last branch block at the start of the next epoch + k_state = store.block_states[branch_root].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's branch justifies the (B, justified_epoch + 1) checkpoint, so K's + # voting source is pulled up to the store's justified checkpoint, which + # advances to (B, justified_epoch + 1) + assert store.justified_checkpoint.epoch == justified_epoch + 1 + assert store.justified_checkpoint.root == b_root + assert store.unrealized_justifications[k_root] == store.justified_checkpoint + + 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 EMPTY(B) variant is not viable and must not be the head + head = spec.get_head(store) + assert head != empty_b_node + assert head.root == k_root + + filtered_tree = spec.get_filtered_node_tree(store) + assert empty_b_node not in filtered_tree + assert full_b_node in filtered_tree + + output_head_check(spec, store, test_steps) + yield "steps", test_steps + + +@with_gloas_and_later +@with_presets([MINIMAL], reason="too slow") +@spec_state_test +def test_get_filtered_node_tree_contains_both_viable_childless_variants(spec, state): + """ + Verifies that when both FULL(B) and EMPTY(B) variants of a block pass the FFG test + and are both childless, get_filtered_node_tree contains both variants. + """ + store, state, test_steps = yield from setup_finalized_store(spec, state) + + # Build block B directly on the justified state (passes FFG test) + b_block = build_empty_block_for_next_slot(spec, state) + signed_b = state_transition_and_sign_block(spec, 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 both FULL(B) and EMPTY(B) variants exist in store + 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) + + 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) + + # Both childless variants pass the FFG test and must be present in get_filtered_node_tree + filtered_tree = spec.get_filtered_node_tree(store) + assert full_b_node in filtered_tree + assert empty_b_node in filtered_tree + + output_head_check(spec, store, test_steps) + yield "steps", test_steps diff --git a/tests/core/pyspec/eth_consensus_specs/test/helpers/fork_choice.py b/tests/core/pyspec/eth_consensus_specs/test/helpers/fork_choice.py index d6889cbf651..3d8e4a24d6a 100644 --- a/tests/core/pyspec/eth_consensus_specs/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth_consensus_specs/test/helpers/fork_choice.py @@ -586,14 +586,16 @@ def get_weighed_node_checks(spec, store, node): def get_viable_for_head_checks(spec, store): - filtered_blocks = spec.get_filtered_block_tree(store) + filtered_node_tree = spec.get_filtered_node_tree(store) root_node = get_fork_choice_node(spec, store.justified_checkpoint.root) pending_nodes = [root_node] leaves_viable_for_head = [] while len(pending_nodes) > 0: node = pending_nodes.pop() - children = spec.get_node_children(store, filtered_blocks, node) + children = [ + child for child in spec.get_node_children(store, node) if child in filtered_node_tree + ] if len(children) == 0: leaves_viable_for_head.append(node) else: diff --git a/tests/core/pyspec/eth_consensus_specs/test/helpers/optimistic_sync.py b/tests/core/pyspec/eth_consensus_specs/test/helpers/optimistic_sync.py index f2e9e968172..be7666e61dc 100644 --- a/tests/core/pyspec/eth_consensus_specs/test/helpers/optimistic_sync.py +++ b/tests/core/pyspec/eth_consensus_specs/test/helpers/optimistic_sync.py @@ -173,26 +173,26 @@ def get_opt_head_block_root(spec, mega_store): """ store = mega_store.fc_store - # Get filtered block tree that only includes viable branches - blocks = spec.get_filtered_block_tree(store) + # Get filtered node tree that only includes viable branches + filtered_node_tree = spec.get_filtered_node_tree(store) # Execute the LMD-GHOST fork choice - head = store.justified_checkpoint.root + head = spec.ForkChoiceNode(root=store.justified_checkpoint.root) while True: children = [ - root - for root in blocks + child + for child in spec.get_node_children(store, head) if ( - blocks[root].parent_root == head - and not is_invalidated(mega_store, root) # For optimistic sync + child in filtered_node_tree + and not is_invalidated(mega_store, child.root) # For optimistic sync ) ] if len(children) == 0: - return head + return head.root # Sort by latest attesting balance with ties broken lexicographically # Ties broken by favoring block with lexicographically higher root head = max( children, - key=lambda root: (spec.get_weight(store, spec.ForkChoiceNode(root=root)), root), + key=lambda node: (spec.get_weight(store, node), node.root), ) diff --git a/tests/core/pyspec/eth_consensus_specs/test/phase0/fork_choice/test_get_head.py b/tests/core/pyspec/eth_consensus_specs/test/phase0/fork_choice/test_get_head.py index c653412dd47..3e7f6330c10 100644 --- a/tests/core/pyspec/eth_consensus_specs/test/phase0/fork_choice/test_get_head.py +++ b/tests/core/pyspec/eth_consensus_specs/test/phase0/fork_choice/test_get_head.py @@ -707,7 +707,7 @@ def test_voting_source_beyond_two_epoch(spec, state): - is a descendant of store.justified_checkpoint.root The block being a descendant of store.justified_checkpoint.root is necessary because -filter_block_tree descends the tree starting at store.justified_checkpoint.root +filter_node_tree descends the tree starting at store.justified_checkpoint.root @with_altair_and_later @spec_state_test diff --git a/tests/formats/fork_choice/README.md b/tests/formats/fork_choice/README.md index 86068a3de8a..8e68529955c 100644 --- a/tests/formats/fork_choice/README.md +++ b/tests/formats/fork_choice/README.md @@ -252,7 +252,7 @@ finalized_checkpoint: { } proposer_boost_root: string -- Encoded 32-byte value from store.proposer_boost_root viable_for_head_roots_and_weights: [{ - root: string, -- Encoded 32-byte value of filtered_block_tree leaf blocks/nodes + root: string, -- Encoded 32-byte value of filtered_node_tree leaf blocks/nodes weight: int, -- Integer value of the weight of the block/node payload_status: int, -- Gloas and later, the payload_status of the node }] diff --git a/tests/generators/compliance_runners/fork_choice/instantiators/block_cover.py b/tests/generators/compliance_runners/fork_choice/instantiators/block_cover.py index 38145b99500..855a788f970 100644 --- a/tests/generators/compliance_runners/fork_choice/instantiators/block_cover.py +++ b/tests/generators/compliance_runners/fork_choice/instantiators/block_cover.py @@ -475,10 +475,12 @@ def run_sanity_checks(spec, store, model_params, target_block_root): else: assert voting_source.epoch + 2 < current_epoch, "block_vse_plus_two_ge_curr_e not satisfied" - # Ensure the target block is in filtered blocks if it is a leaf and eligible + # Ensure the target block is in the filtered node tree if it is a leaf and eligible if predicates["block_is_leaf"] and ( predicates["store_je_eq_zero"] or predicates["block_vse_eq_store_je"] or predicates["block_vse_plus_two_ge_curr_e"] ): - assert target_block_root in spec.get_filtered_block_tree(store) + filtered_tree = spec.get_filtered_node_tree(store) + filtered_roots = [node.root for node in filtered_tree] + assert target_block_root in filtered_roots