From eb00f8f735a1d6e5d3fe614119d0cbcbceeeef77 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 17 Nov 2021 18:15:12 -0700 Subject: [PATCH] cleanup forkchoice tests --- specs/phase0/fork-choice.md | 11 ++++++---- .../eth2spec/test/helpers/fork_choice.py | 6 ++---- .../test/phase0/fork_choice/test_on_block.py | 20 ++++++++----------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 6331f496f..723102e4d 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -22,7 +22,7 @@ - [`get_head`](#get_head) - [`should_update_justified_checkpoint`](#should_update_justified_checkpoint) - [`on_attestation` helpers](#on_attestation-helpers) - - [`validate_target_epoch_scope`](#validate_target_epoch_scope) + - [`validate_target_epoch_against_current_time`](#validate_target_epoch_against_current_time) - [`validate_on_attestation`](#validate_on_attestation) - [`store_target_checkpoint_state`](#store_target_checkpoint_state) - [`update_latest_messages`](#update_latest_messages) @@ -259,10 +259,10 @@ def should_update_justified_checkpoint(store: Store, new_justified_checkpoint: C #### `on_attestation` helpers -##### `validate_target_epoch_scope` +##### `validate_target_epoch_against_current_time` ```python -def validate_target_epoch_scope(store: Store, attestation: Attestation) -> None: +def validate_target_epoch_against_current_time(store: Store, attestation: Attestation) -> None: target = attestation.data.target # Attestations must be from the current or previous epoch @@ -281,7 +281,7 @@ def validate_on_attestation(store: Store, attestation: Attestation, is_from_bloc # If the given attestation is not from a beacon block message, we have to check the target epoch scope. if not is_from_block: - validate_target_epoch_scope(store, attestation) + validate_target_epoch_against_current_time(store, attestation) # Check that the epoch number and slot number are matching assert target.epoch == compute_epoch_at_slot(attestation.data.slot) @@ -386,6 +386,9 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Update finalized checkpoint if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: + print("made it") + print(state.finalized_checkpoint) + print(state.current_justified_checkpoint) store.finalized_checkpoint = state.finalized_checkpoint store.justified_checkpoint = state.current_justified_checkpoint ``` diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index 1c49c102c..098c05bf9 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -60,13 +60,13 @@ def tick_and_run_on_attestation(spec, store, attestation, test_steps, is_from_bl def run_on_attestation(spec, store, attestation, is_from_block=False, valid=True): if not valid: try: - spec.on_attestation(store, attestation, is_from_block) + spec.on_attestation(store, attestation, is_from_block=is_from_block) except AssertionError: return else: assert False - spec.on_attestation(store, attestation, is_from_block) + spec.on_attestation(store, attestation, is_from_block=is_from_block) def get_genesis_forkchoice_store(spec, genesis_state): @@ -139,7 +139,6 @@ def add_block(spec, run_on_attestation(spec, store, attestation, is_from_block=True, valid=True) block_root = signed_block.message.hash_tree_root() - print(encode_hex(block_root)) assert store.blocks[block_root] == signed_block.message assert store.block_states[block_root].hash_tree_root() == signed_block.message.state_root test_steps.append({ @@ -160,7 +159,6 @@ def add_block(spec, }, } }) - print(test_steps[-1]) return store.block_states[signed_block.message.hash_tree_root()] diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index 1f2f45352..3670a883a 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -551,11 +551,9 @@ def test_new_justified_is_later_than_store_justified(spec, state): yield from add_block(spec, store, block, test_steps) assert store.finalized_checkpoint == fork_3_state.finalized_checkpoint - assert (store.justified_checkpoint - == fork_3_state.current_justified_checkpoint - != store.best_justified_checkpoint) - assert (store.best_justified_checkpoint - == fork_2_state.current_justified_checkpoint) + assert store.justified_checkpoint == fork_3_state.current_justified_checkpoint + assert store.justified_checkpoint != store.best_justified_checkpoint + assert store.best_justified_checkpoint == fork_2_state.current_justified_checkpoint yield 'steps', test_steps @@ -656,7 +654,6 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): """ test_steps = [] # Initialization - print("INIT") store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state) yield 'anchor_state', state yield 'anchor_block', anchor_block @@ -678,14 +675,12 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): state, store, _ = yield from apply_next_epoch_with_attestations( spec, state, store, False, True, test_steps=test_steps) - print("INIT FIN") assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 4 assert store.justified_checkpoint == state.current_justified_checkpoint # Create another chain # Forking from epoch 3 - print("FORK") all_blocks = [] slot = spec.compute_start_slot_at_epoch(3) block_root = spec.get_block_root_at_slot(state, slot) @@ -697,7 +692,6 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): assert another_state.finalized_checkpoint.epoch == 3 assert another_state.current_justified_checkpoint.epoch == 4 - print("APPLY FORK TO FORK CHOICE") pre_store_justified_checkpoint_root = store.justified_checkpoint.root for block in all_blocks: # FIXME: Once `on_block` and `on_attestation` logic is fixed, @@ -708,11 +702,13 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): ancestor_at_finalized_slot = spec.get_ancestor(store, pre_store_justified_checkpoint_root, finalized_slot) assert ancestor_at_finalized_slot == store.finalized_checkpoint.root - assert store.finalized_checkpoint == another_state.finalized_checkpoint - # Thus should fail with the fix. Once show fail, swap to == - # assert store.justified_checkpoint != another_state.current_justified_checkpoint print(store.finalized_checkpoint) print(store.justified_checkpoint) + print(another_state.current_justified_checkpoint) print('spec.get_head(store)', spec.get_head(store)) + assert store.finalized_checkpoint == another_state.finalized_checkpoint + # Thus should fail with the fix. Once show fail, swap to == + assert store.justified_checkpoint == another_state.current_justified_checkpoint + yield 'steps', test_steps