From 63ca480ea3f36f80e9f542a29262bc8e6ed3ef5d Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 14 Jul 2021 20:02:21 +0800 Subject: [PATCH 1/3] Add condition check in `on_tick` to ensure that `store.justified_checkpoint` is a descendant of `store.finalized_checkpoint` --- specs/phase0/fork-choice.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 181a874fb..d4a3a9b85 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -327,9 +327,13 @@ def on_tick(store: Store, time: uint64) -> None: # Not a new epoch, return if not (current_slot > previous_slot and compute_slots_since_epoch_start(current_slot) == 0): return - # Update store.justified_checkpoint if a better checkpoint is known + + # Update store.justified_checkpoint if a better checkpoint on the store.finalized_checkpoint chain if store.best_justified_checkpoint.epoch > store.justified_checkpoint.epoch: - store.justified_checkpoint = store.best_justified_checkpoint + finalized_slot = compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) + ancestor_at_finalized_slot = get_ancestor(store, store.best_justified_checkpoint.root, finalized_slot) + if ancestor_at_finalized_slot == store.finalized_checkpoint.root: + store.justified_checkpoint = store.best_justified_checkpoint ``` #### `on_block` From cc3690ce3899cfe22a4d44890d63739f3aed093f Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 14 Jul 2021 20:05:14 +0800 Subject: [PATCH 2/3] Add unit tests to test the new condition. --- .../unittests/fork_choice/test_on_tick.py | 98 +++++++++++++++++-- 1 file changed, 90 insertions(+), 8 deletions(-) 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 ef643fd6b..40606197e 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,5 +1,13 @@ from eth2spec.test.context import with_all_phases, spec_state_test from eth2spec.test.helpers.fork_choice import get_genesis_forkchoice_store +from eth2spec.test.helpers.block import ( + build_empty_block_for_next_slot, +) +from eth2spec.test.helpers.state import ( + next_epoch, + state_transition_and_sign_block, + transition_to, +) def run_on_tick(spec, store, time, new_justified_checkpoint=False): @@ -26,18 +34,92 @@ def test_basic(spec, state): @with_all_phases @spec_state_test -def test_update_justified_single(spec, state): +def test_update_justified_single_on_store_finalized_chain(spec, 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.config.SECONDS_PER_SLOT - store.time - store.best_justified_checkpoint = spec.Checkpoint( - epoch=store.justified_checkpoint.epoch + 1, - root=b'\x55' * 32, + # [Mock store.best_justified_checkpoint] + # Create a block at epoch 1 + next_epoch(spec, state) + block = build_empty_block_for_next_slot(spec, state) + state_transition_and_sign_block(spec, state, block) + store.blocks[block.hash_tree_root()] = block.copy() + store.block_states[block.hash_tree_root()] = state.copy() + parent_block = block.copy() + # To make compute_slots_since_epoch_start(current_slot) == 0, transition to the end of the epoch + slot = state.slot + spec.SLOTS_PER_EPOCH - (state.slot + spec.SLOTS_PER_EPOCH) % spec.SLOTS_PER_EPOCH - 1 + transition_to(spec, state, slot) + # Create a block at the start of epoch 2 + block = build_empty_block_for_next_slot(spec, state) + # Mock state + state.current_justified_checkpoint = spec.Checkpoint( + epoch=spec.compute_epoch_at_slot(parent_block.slot), + root=parent_block.hash_tree_root(), + ) + state_transition_and_sign_block(spec, state, block) + store.blocks[block.hash_tree_root()] = block + store.block_states[block.hash_tree_root()] = state + # Mock store.best_justified_checkpoint + store.best_justified_checkpoint = state.current_justified_checkpoint.copy() + + run_on_tick( + spec, + store, + store.genesis_time + state.slot * spec.config.SECONDS_PER_SLOT, + new_justified_checkpoint=True ) - run_on_tick(spec, store, store.time + seconds_until_next_epoch, True) + +@with_all_phases +@spec_state_test +def test_update_justified_single_not_on_store_finalized_chain(spec, state): + store = get_genesis_forkchoice_store(spec, state) + init_state = state.copy() + + # Chain grows + # Create a block at epoch 1 + next_epoch(spec, state) + block = build_empty_block_for_next_slot(spec, state) + block.body.graffiti = b'\x11' * 32 + state_transition_and_sign_block(spec, state, block) + store.blocks[block.hash_tree_root()] = block.copy() + store.block_states[block.hash_tree_root()] = state.copy() + # Mock store.finalized_checkpoint + store.finalized_checkpoint = spec.Checkpoint( + epoch=spec.compute_epoch_at_slot(block.slot), + root=block.hash_tree_root(), + ) + + # [Mock store.best_justified_checkpoint] + # Create a block at epoch 1 + state = init_state.copy() + next_epoch(spec, state) + block = build_empty_block_for_next_slot(spec, state) + block.body.graffiti = b'\x22' * 32 + state_transition_and_sign_block(spec, state, block) + store.blocks[block.hash_tree_root()] = block.copy() + store.block_states[block.hash_tree_root()] = state.copy() + parent_block = block.copy() + # To make compute_slots_since_epoch_start(current_slot) == 0, transition to the end of the epoch + slot = state.slot + spec.SLOTS_PER_EPOCH - (state.slot + spec.SLOTS_PER_EPOCH) % spec.SLOTS_PER_EPOCH - 1 + transition_to(spec, state, slot) + # Create a block at the start of epoch 2 + block = build_empty_block_for_next_slot(spec, state) + # Mock state + state.current_justified_checkpoint = spec.Checkpoint( + epoch=spec.compute_epoch_at_slot(parent_block.slot), + root=parent_block.hash_tree_root(), + ) + state_transition_and_sign_block(spec, state, block) + store.blocks[block.hash_tree_root()] = block.copy() + store.block_states[block.hash_tree_root()] = state.copy() + # Mock store.best_justified_checkpoint + store.best_justified_checkpoint = state.current_justified_checkpoint.copy() + + run_on_tick( + spec, + store, + store.genesis_time + state.slot * spec.config.SECONDS_PER_SLOT, + ) @with_all_phases From 838c263c4a82ffc7c43a426bfb91adc958e001c4 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 23 Aug 2021 23:21:15 +0800 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Aditya Asgaonkar --- .../test/phase0/unittests/fork_choice/test_on_tick.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 40606197e..0d9f6ddf5 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 @@ -46,7 +46,7 @@ def test_update_justified_single_on_store_finalized_chain(spec, state): store.block_states[block.hash_tree_root()] = state.copy() parent_block = block.copy() # To make compute_slots_since_epoch_start(current_slot) == 0, transition to the end of the epoch - slot = state.slot + spec.SLOTS_PER_EPOCH - (state.slot + spec.SLOTS_PER_EPOCH) % spec.SLOTS_PER_EPOCH - 1 + slot = state.slot + spec.SLOTS_PER_EPOCH - state.slot % spec.SLOTS_PER_EPOCH - 1 transition_to(spec, state, slot) # Create a block at the start of epoch 2 block = build_empty_block_for_next_slot(spec, state) @@ -100,7 +100,7 @@ def test_update_justified_single_not_on_store_finalized_chain(spec, state): store.block_states[block.hash_tree_root()] = state.copy() parent_block = block.copy() # To make compute_slots_since_epoch_start(current_slot) == 0, transition to the end of the epoch - slot = state.slot + spec.SLOTS_PER_EPOCH - (state.slot + spec.SLOTS_PER_EPOCH) % spec.SLOTS_PER_EPOCH - 1 + slot = state.slot + spec.SLOTS_PER_EPOCH - state.slot % spec.SLOTS_PER_EPOCH - 1 transition_to(spec, state, slot) # Create a block at the start of epoch 2 block = build_empty_block_for_next_slot(spec, state)