From 1e29563242feb73d91b4d03d0dfd7feba7d1a7c1 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 15 Sep 2020 12:51:11 +0800 Subject: [PATCH] Pass `anchor_block` to `get_forkchoice_store` --- specs/phase0/fork-choice.md | 12 +++---- specs/phase1/fork-choice.md | 10 +++--- .../eth2spec/test/helpers/fork_choice.py | 10 ++++++ .../unittests/fork_choice/test_get_head.py | 18 ++++++---- .../fork_choice/test_on_attestation.py | 27 +++++++-------- .../unittests/fork_choice/test_on_block.py | 33 ++++++++++--------- .../unittests/fork_choice/test_on_tick.py | 13 ++++---- .../fork_choice/test_on_shard_block.py | 4 +-- 8 files changed, 70 insertions(+), 57 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 8e1f65121..018cbec4b 100644 --- a/specs/phase0/fork-choice.md +++ b/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: - anchor_block_header = copy(anchor_state.latest_block_header) - if anchor_block_header.state_root == Bytes32(): - anchor_block_header.state_root = hash_tree_root(anchor_state) - anchor_root = hash_tree_root(anchor_block_header) +def get_forkchoice_store(anchor_state: BeaconState, anchor_block: BeaconBlock) -> Store: + assert anchor_block.state_root == hash_tree_root(anchor_state) + anchor_root = hash_tree_root(anchor_block) 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)}, ) diff --git a/specs/phase1/fork-choice.md b/specs/phase1/fork-choice.md index d0b06ffa9..3845b2b74 100644 --- a/specs/phase1/fork-choice.md +++ b/specs/phase1/fork-choice.md @@ -71,11 +71,9 @@ class ShardStore: #### Updated `get_forkchoice_store` ```python -def get_forkchoice_store(anchor_state: BeaconState) -> Store: - anchor_block_header = anchor_state.latest_block_header.copy() - if anchor_block_header.state_root == Bytes32(): - anchor_block_header.state_root = hash_tree_root(anchor_state) - anchor_root = hash_tree_root(anchor_block_header) +def get_forkchoice_store(anchor_state: BeaconState, anchor_block: BeaconBlock) -> Store: + assert anchor_block.state_root == hash_tree_root(anchor_state) + anchor_root = hash_tree_root(anchor_block) 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={ diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index 04e36ea84..85437e98a 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/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) diff --git a/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_get_head.py b/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_get_head.py index 859fc797f..b470ab079 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_get_head.py +++ b/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 diff --git a/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_attestation.py b/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_attestation.py index 6fa842255..63b0572b1 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_attestation.py +++ b/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) diff --git a/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_block.py index 016326b30..7fa32b86c 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_block.py +++ b/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.state import next_epoch, state_transition_and_sign_block +from eth2spec.test.helpers.fork_choice import get_genesis_forkchoice_store +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) - store = spec.get_forkchoice_store(state) - + transition_to(spec, state, state.slot + spec.SLOTS_PER_EPOCH - 1) + 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) diff --git a/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_tick.py b/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_tick.py index 93f3bd9bb..9cf0e9e26 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_tick.py +++ b/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( diff --git a/tests/core/pyspec/eth2spec/test/phase1/unittests/fork_choice/test_on_shard_block.py b/tests/core/pyspec/eth2spec/test/phase1/unittests/fork_choice/test_on_shard_block.py index de47aaa39..ddf66b0f6 100644 --- a/tests/core/pyspec/eth2spec/test/phase1/unittests/fork_choice/test_on_shard_block.py +++ b/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