Merge pull request #2061 from ethereum/anchor_block_header_3

Pass `anchor_block` to `get_forkchoice_store`

specs/phase0/fork-choice.md

@@ -98,14 +98,10 @@ This should be the genesis state for a full client.
 
 *Note* With regards to fork choice, block headers are interchangeable with blocks. The spec is likely to move to headers for reduced overhead in test vectors and better encapsulation. Full implementations store blocks as part of their database and will often use full blocks when dealing with production fork choice.
 
-_The block for `anchor_root` is incorrectly initialized to the block header, rather than the full block. This does not affect functionality but will be cleaned up in subsequent releases._
-
 ```python
-def get_forkchoice_store(anchor_state: BeaconState) -> Store:
+def get_forkchoice_store(anchor_state: BeaconState, anchor_block: BeaconBlock) -> Store:
-    anchor_block_header = copy(anchor_state.latest_block_header)
+    assert anchor_block.state_root == hash_tree_root(anchor_state)
-    if anchor_block_header.state_root == Bytes32():
+    anchor_root = hash_tree_root(anchor_block)
-        anchor_block_header.state_root = hash_tree_root(anchor_state)
-    anchor_root = hash_tree_root(anchor_block_header)
     anchor_epoch = get_current_epoch(anchor_state)
     justified_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root)
     finalized_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root)
@@ -115,7 +111,7 @@ def get_forkchoice_store(anchor_state: BeaconState) -> Store:
         justified_checkpoint=justified_checkpoint,
         finalized_checkpoint=finalized_checkpoint,
         best_justified_checkpoint=justified_checkpoint,
-        blocks={anchor_root: anchor_block_header},
+        blocks={anchor_root: copy(anchor_block)},
         block_states={anchor_root: copy(anchor_state)},
         checkpoint_states={justified_checkpoint: copy(anchor_state)},
     )

specs/phase1/fork-choice.md

@@ -71,11 +71,9 @@ class ShardStore:
 #### Updated `get_forkchoice_store`
 
 ```python
-def get_forkchoice_store(anchor_state: BeaconState) -> Store:
+def get_forkchoice_store(anchor_state: BeaconState, anchor_block: BeaconBlock) -> Store:
-    anchor_block_header = anchor_state.latest_block_header.copy()
+    assert anchor_block.state_root == hash_tree_root(anchor_state)
-    if anchor_block_header.state_root == Bytes32():
+    anchor_root = hash_tree_root(anchor_block)
-        anchor_block_header.state_root = hash_tree_root(anchor_state)
-    anchor_root = hash_tree_root(anchor_block_header)
     anchor_epoch = get_current_epoch(anchor_state)
     justified_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root)
     finalized_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root)
@@ -85,7 +83,7 @@ def get_forkchoice_store(anchor_state: BeaconState) -> Store:
         justified_checkpoint=justified_checkpoint,
         finalized_checkpoint=finalized_checkpoint,
         best_justified_checkpoint=justified_checkpoint,
-        blocks={anchor_root: anchor_block_header},
+        blocks={anchor_root: copy(anchor_block)},
         block_states={anchor_root: anchor_state.copy()},
         checkpoint_states={justified_checkpoint: anchor_state.copy()},
         shard_stores={

tests/core/pyspec/eth2spec/test/helpers/fork_choice.py

@@ -1,3 +1,6 @@
+from eth2spec.phase0 import spec as phase0_spec
+
+
 def get_anchor_root(spec, state):
     anchor_block_header = state.latest_block_header.copy()
     if anchor_block_header.state_root == spec.Bytes32():
@@ -25,3 +28,10 @@ def add_attestation_to_store(spec, store, attestation):
         spec.on_tick(store, next_epoch_time)
 
     spec.on_attestation(store, attestation)
+
+
+def get_genesis_forkchoice_store(spec, genesis_state):
+    assert genesis_state.slot == spec.GENESIS_SLOT
+    # The genesis block must be a Phase 0 `BeaconBlock`
+    genesis_block = phase0_spec.BeaconBlock(state_root=genesis_state.hash_tree_root())
+    return spec.get_forkchoice_store(genesis_state, genesis_block)

tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_get_head.py

@@ -1,7 +1,11 @@
 from eth2spec.test.context import with_all_phases, spec_state_test
 from eth2spec.test.helpers.attestations import get_valid_attestation, next_epoch_with_attestations
 from eth2spec.test.helpers.block import build_empty_block_for_next_slot
-from eth2spec.test.helpers.fork_choice import add_attestation_to_store, add_block_to_store, get_anchor_root
+from eth2spec.test.helpers.fork_choice import (
+    add_attestation_to_store,
+    add_block_to_store, get_anchor_root,
+    get_genesis_forkchoice_store,
+)
 from eth2spec.test.helpers.state import (
     next_epoch,
     state_transition_and_sign_block,
@@ -12,7 +16,7 @@ from eth2spec.test.helpers.state import (
 @spec_state_test
 def test_genesis(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     anchor_root = get_anchor_root(spec, state)
     assert spec.get_head(store) == anchor_root
 
@@ -21,7 +25,7 @@ def test_genesis(spec, state):
 @spec_state_test
 def test_chain_no_attestations(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     anchor_root = get_anchor_root(spec, state)
     assert spec.get_head(store) == anchor_root
 
@@ -44,7 +48,7 @@ def test_split_tie_breaker_no_attestations(spec, state):
     genesis_state = state.copy()
 
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     anchor_root = get_anchor_root(spec, state)
     assert spec.get_head(store) == anchor_root
 
@@ -72,13 +76,13 @@ def test_shorter_chain_but_heavier_weight(spec, state):
     genesis_state = state.copy()
 
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     anchor_root = get_anchor_root(spec, state)
     assert spec.get_head(store) == anchor_root
 
     # build longer tree
     long_state = genesis_state.copy()
-    for i in range(3):
+    for _ in range(3):
         long_block = build_empty_block_for_next_slot(spec, long_state)
         signed_long_block = state_transition_and_sign_block(spec, long_state, long_block)
         add_block_to_store(spec, store, signed_long_block)
@@ -100,7 +104,7 @@ def test_shorter_chain_but_heavier_weight(spec, state):
 @spec_state_test
 def test_filtered_block_tree(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     anchor_root = get_anchor_root(spec, state)
 
     # transition state past initial couple of epochs

tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_attestation.py

@@ -2,6 +2,7 @@ from eth2spec.test.context import PHASE0, with_all_phases, spec_state_test
 from eth2spec.test.helpers.block import build_empty_block_for_next_slot
 from eth2spec.test.helpers.attestations import get_valid_attestation, sign_attestation
 from eth2spec.test.helpers.state import transition_to, state_transition_and_sign_block, next_epoch, next_slot
+from eth2spec.test.helpers.fork_choice import get_genesis_forkchoice_store
 
 
 def run_on_attestation(spec, state, store, attestation, valid=True):
@@ -41,7 +42,7 @@ def run_on_attestation(spec, state, store, attestation, valid=True):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_current_epoch(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     spec.on_tick(store, store.time + spec.SECONDS_PER_SLOT * 2)
 
     block = build_empty_block_for_next_slot(spec, state)
@@ -60,7 +61,7 @@ def test_on_attestation_current_epoch(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_previous_epoch(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     spec.on_tick(store, store.time + spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH)
 
     block = build_empty_block_for_next_slot(spec, state)
@@ -79,7 +80,7 @@ def test_on_attestation_previous_epoch(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_past_epoch(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
 
     # move time forward 2 epochs
     time = store.time + 2 * spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH
@@ -101,7 +102,7 @@ def test_on_attestation_past_epoch(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_mismatched_target_and_slot(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     spec.on_tick(store, store.time + spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH)
 
     block = build_empty_block_for_next_slot(spec, state)
@@ -124,7 +125,7 @@ def test_on_attestation_mismatched_target_and_slot(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_inconsistent_target_and_head(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     spec.on_tick(store, store.time + 2 * spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH)
 
     # Create chain 1 as empty chain between genesis and start of 1st epoch
@@ -162,7 +163,7 @@ def test_on_attestation_inconsistent_target_and_head(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_target_block_not_in_store(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = store.time + spec.SECONDS_PER_SLOT * (spec.SLOTS_PER_EPOCH + 1)
     spec.on_tick(store, time)
 
@@ -184,7 +185,7 @@ def test_on_attestation_target_block_not_in_store(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_target_checkpoint_not_in_store(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = store.time + spec.SECONDS_PER_SLOT * (spec.SLOTS_PER_EPOCH + 1)
     spec.on_tick(store, time)
 
@@ -209,7 +210,7 @@ def test_on_attestation_target_checkpoint_not_in_store(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_target_checkpoint_not_in_store_diff_slot(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = store.time + spec.SECONDS_PER_SLOT * (spec.SLOTS_PER_EPOCH + 1)
     spec.on_tick(store, time)
 
@@ -236,7 +237,7 @@ def test_on_attestation_target_checkpoint_not_in_store_diff_slot(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_beacon_block_not_in_store(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = store.time + spec.SECONDS_PER_SLOT * (spec.SLOTS_PER_EPOCH + 1)
     spec.on_tick(store, time)
 
@@ -265,7 +266,7 @@ def test_on_attestation_beacon_block_not_in_store(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_future_epoch(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = store.time + 3 * spec.SECONDS_PER_SLOT
     spec.on_tick(store, time)
 
@@ -285,7 +286,7 @@ def test_on_attestation_future_epoch(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_future_block(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = store.time + spec.SECONDS_PER_SLOT * 5
     spec.on_tick(store, time)
 
@@ -305,7 +306,7 @@ def test_on_attestation_future_block(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_same_slot(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = store.time + spec.SECONDS_PER_SLOT
     spec.on_tick(store, time)
 
@@ -321,7 +322,7 @@ def test_on_attestation_same_slot(spec, state):
 @with_all_phases
 @spec_state_test
 def test_on_attestation_invalid_attestation(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = store.time + 3 * spec.SECONDS_PER_SLOT
     spec.on_tick(store, time)
 

tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_block.py

@@ -2,10 +2,11 @@ from copy import deepcopy
 from eth2spec.utils.ssz.ssz_impl import hash_tree_root
 
 from eth2spec.test.context import with_all_phases, spec_state_test
+from eth2spec.test.helpers.attestations import next_epoch_with_attestations
 from eth2spec.test.helpers.block import build_empty_block_for_next_slot, sign_block, transition_unsigned_block, \
     build_empty_block
-from eth2spec.test.helpers.attestations import next_epoch_with_attestations
+from eth2spec.test.helpers.fork_choice import get_genesis_forkchoice_store
-from eth2spec.test.helpers.state import next_epoch, state_transition_and_sign_block
+from eth2spec.test.helpers.state import next_epoch, state_transition_and_sign_block, transition_to
 
 
 def run_on_block(spec, store, signed_block, valid=True):
@@ -37,7 +38,7 @@ def apply_next_epoch_with_attestations(spec, state, store):
 @spec_state_test
 def test_basic(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = 100
     spec.on_tick(store, time)
     assert store.time == time
@@ -61,7 +62,7 @@ def test_basic(spec, state):
 @spec_state_test
 def test_on_block_checkpoints(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = 100
     spec.on_tick(store, time)
 
@@ -87,7 +88,7 @@ def test_on_block_checkpoints(spec, state):
 @spec_state_test
 def test_on_block_future_block(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
 
     # do not tick time
 
@@ -101,7 +102,7 @@ def test_on_block_future_block(spec, state):
 @spec_state_test
 def test_on_block_bad_parent_root(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = 100
     spec.on_tick(store, time)
 
@@ -121,7 +122,7 @@ def test_on_block_bad_parent_root(spec, state):
 @spec_state_test
 def test_on_block_before_finalized(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = 100
     spec.on_tick(store, time)
 
@@ -140,7 +141,7 @@ def test_on_block_before_finalized(spec, state):
 @spec_state_test
 def test_on_block_finalized_skip_slots(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = 100
     spec.on_tick(store, time)
 
@@ -160,16 +161,18 @@ def test_on_block_finalized_skip_slots(spec, state):
 @spec_state_test
 def test_on_block_finalized_skip_slots_not_in_skip_chain(spec, state):
     # Initialization
-    next_epoch(spec, state)
+    transition_to(spec, state, state.slot + spec.SLOTS_PER_EPOCH - 1)
-    store = spec.get_forkchoice_store(state)
+    block = build_empty_block_for_next_slot(spec, state)
-
+    transition_unsigned_block(spec, state, block)
+    block.state_root = state.hash_tree_root()
+    store = spec.get_forkchoice_store(state, block)
     store.finalized_checkpoint = spec.Checkpoint(
         epoch=store.finalized_checkpoint.epoch + 2,
         root=store.finalized_checkpoint.root
     )
 
     # First transition through the epoch to ensure no skipped slots
-    state, store, last_signed_block = apply_next_epoch_with_attestations(spec, state, store)
+    state, store, _ = apply_next_epoch_with_attestations(spec, state, store)
 
     # Now build a block at later slot than finalized epoch
     # Includes finalized block in chain, but not at appropriate skip slot
@@ -183,7 +186,7 @@ def test_on_block_finalized_skip_slots_not_in_skip_chain(spec, state):
 @spec_state_test
 def test_on_block_update_justified_checkpoint_within_safe_slots(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = 0
     spec.on_tick(store, time)
 
@@ -214,7 +217,7 @@ def test_on_block_update_justified_checkpoint_within_safe_slots(spec, state):
 @spec_state_test
 def test_on_block_outside_safe_slots_and_multiple_better_justified(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = 0
     spec.on_tick(store, time)
 
@@ -264,7 +267,7 @@ def test_on_block_outside_safe_slots_and_multiple_better_justified(spec, state):
 @spec_state_test
 def test_on_block_outside_safe_slots_but_finality(spec, state):
     # Initialization
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     time = 100
     spec.on_tick(store, time)
 

tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_tick.py

@@ -1,4 +1,5 @@
 from eth2spec.test.context import with_all_phases, spec_state_test
+from eth2spec.test.helpers.fork_choice import get_genesis_forkchoice_store
 
 
 def run_on_tick(spec, store, time, new_justified_checkpoint=False):
@@ -19,14 +20,14 @@ def run_on_tick(spec, store, time, new_justified_checkpoint=False):
 @with_all_phases
 @spec_state_test
 def test_basic(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     run_on_tick(spec, store, store.time + 1)
 
 
 @with_all_phases
 @spec_state_test
 def test_update_justified_single(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     next_epoch = spec.get_current_epoch(state) + 1
     next_epoch_start_slot = spec.compute_start_slot_at_epoch(next_epoch)
     seconds_until_next_epoch = next_epoch_start_slot * spec.SECONDS_PER_SLOT - store.time
@@ -42,7 +43,7 @@ def test_update_justified_single(spec, state):
 @with_all_phases
 @spec_state_test
 def test_no_update_same_slot_at_epoch_boundary(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     seconds_per_epoch = spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH
 
     store.best_justified_checkpoint = spec.Checkpoint(
@@ -59,7 +60,7 @@ def test_no_update_same_slot_at_epoch_boundary(spec, state):
 @with_all_phases
 @spec_state_test
 def test_no_update_not_epoch_boundary(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
 
     store.best_justified_checkpoint = spec.Checkpoint(
         epoch=store.justified_checkpoint.epoch + 1,
@@ -72,7 +73,7 @@ def test_no_update_not_epoch_boundary(spec, state):
 @with_all_phases
 @spec_state_test
 def test_no_update_new_justified_equal_epoch(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     seconds_per_epoch = spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH
 
     store.best_justified_checkpoint = spec.Checkpoint(
@@ -91,7 +92,7 @@ def test_no_update_new_justified_equal_epoch(spec, state):
 @with_all_phases
 @spec_state_test
 def test_no_update_new_justified_later_epoch(spec, state):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     seconds_per_epoch = spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH
 
     store.best_justified_checkpoint = spec.Checkpoint(

tests/core/pyspec/eth2spec/test/phase1/unittests/fork_choice/test_on_shard_block.py

@@ -7,7 +7,7 @@ from eth2spec.test.helpers.shard_block import (
     get_shard_transitions,
     get_committee_index_of_shard,
 )
-from eth2spec.test.helpers.fork_choice import add_block_to_store, get_anchor_root
+from eth2spec.test.helpers.fork_choice import add_block_to_store, get_anchor_root, get_genesis_forkchoice_store
 from eth2spec.test.helpers.shard_transitions import is_full_crosslink
 from eth2spec.test.helpers.state import state_transition_and_sign_block
 from eth2spec.test.helpers.block import build_empty_block
@@ -29,7 +29,7 @@ def run_on_shard_block(spec, store, signed_block, valid=True):
 
 
 def initialize_store(spec, state, shards):
-    store = spec.get_forkchoice_store(state)
+    store = get_genesis_forkchoice_store(spec, state)
     anchor_root = get_anchor_root(spec, state)
     assert spec.get_head(store) == anchor_root