From 189a9d4ae993ce29ec1088ba2f50610819e14ca8 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 31 Aug 2021 12:10:37 +0800 Subject: [PATCH 1/5] Add the missed on_tick output and remove the useless on_tick call --- .../eth2spec/test/phase0/fork_choice/test_on_block.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 634610fe7..457eef7ff 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 @@ -483,7 +483,8 @@ def test_new_justified_is_later_than_store_justified(spec, state): assert fork_2_state.finalized_checkpoint.epoch == 0 assert fork_2_state.current_justified_checkpoint.epoch == 5 # Check SAFE_SLOTS_TO_UPDATE_JUSTIFIED - spec.on_tick(store, store.genesis_time + fork_2_state.slot * spec.config.SECONDS_PER_SLOT) + time = store.genesis_time + fork_2_state.slot * spec.config.SECONDS_PER_SLOT + on_tick_and_append_step(spec, store, time, test_steps) assert spec.compute_slots_since_epoch_start(spec.get_current_slot(store)) >= spec.SAFE_SLOTS_TO_UPDATE_JUSTIFIED # Run on_block yield from add_block(spec, store, signed_block, test_steps) @@ -526,7 +527,8 @@ def test_new_justified_is_later_than_store_justified(spec, state): # # Apply blocks of `fork_3_state` to `store` # for block in all_blocks: # if store.time < spec.compute_time_at_slot(fork_2_state, block.message.slot): - # spec.on_tick(store, store.genesis_time + block.message.slot * spec.config.SECONDS_PER_SLOT) + # time = store.genesis_time + block.message.slot * spec.config.SECONDS_PER_SLOT + # on_tick_and_append_step(spec, store, time, test_steps) # # valid_attestations=False because the attestations are outdated (older than previous epoch) # yield from add_block(spec, store, block, test_steps, allow_invalid_attestations=False) @@ -643,7 +645,6 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): # Process state next_epoch(spec, state) - spec.on_tick(store, store.genesis_time + state.slot * spec.config.SECONDS_PER_SLOT) state, store, _ = yield from apply_next_epoch_with_attestations( spec, state, store, False, True, test_steps=test_steps) From da8d22c7541d78774b2a1179dca225d63b8e24e2 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 31 Aug 2021 13:16:19 +0800 Subject: [PATCH 2/5] Update `checks` Checkpoint fields --- .../eth2spec/test/helpers/fork_choice.py | 15 ++++++-- tests/formats/fork_choice/README.md | 36 +++++++++++-------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index ec5793af5..65d6975f2 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -156,9 +156,18 @@ def add_block(spec, store, signed_block, test_steps, valid=True, allow_invalid_a 'checks': { 'time': int(store.time), 'head': get_formatted_head_output(spec, store), - 'justified_checkpoint_root': encode_hex(store.justified_checkpoint.root), - 'finalized_checkpoint_root': encode_hex(store.finalized_checkpoint.root), - 'best_justified_checkpoint': encode_hex(store.best_justified_checkpoint.root), + 'justified_checkpoint': { + 'epoch': int(store.justified_checkpoint.epoch), + 'root': encode_hex(store.justified_checkpoint.root), + }, + 'finalized_checkpoint': { + 'epoch': int(store.finalized_checkpoint.epoch), + 'root': encode_hex(store.finalized_checkpoint.root), + }, + 'best_justified_checkpoint': { + 'epoch': int(store.best_justified_checkpoint.epoch), + 'root': encode_hex(store.best_justified_checkpoint.root), + }, } }) diff --git a/tests/formats/fork_choice/README.md b/tests/formats/fork_choice/README.md index 90c0aafc7..cfc86776d 100644 --- a/tests/formats/fork_choice/README.md +++ b/tests/formats/fork_choice/README.md @@ -80,26 +80,34 @@ checks: {: value} -- the assertions. `` is the field member or property of [`Store`](../../../specs/phase0/fork-choice.md#store) object that maintained by client implementation. Currently, the possible fields included: ```yaml -head: { -- Encoded 32-byte value from get_head(store) - slot: slot, - root: string, +head: { + slot: int, + root: string, -- Encoded 32-byte value from get_head(store) +} +time: int -- store.time +genesis_time: int -- store.genesis_time +justified_checkpoint: { + epoch: int, -- Integer value from store.justified_checkpoint.epoch + root: string, -- Encoded 32-byte value from store.justified_checkpoint.root +} +finalized_checkpoint: { + epoch: int, -- Integer value from store.finalized_checkpoint.epoch + root: string, -- Encoded 32-byte value from store.finalized_checkpoint.root +} +best_justified_checkpoint: { + epoch: int, -- Integer value from store.best_justified_checkpoint.epoch + root: string, -- Encoded 32-byte value from store.best_justified_checkpoint.root } -time: int -- store.time -genesis_time: int -- store.genesis_time -justified_checkpoint_root: string -- Encoded 32-byte value from store.justified_checkpoint.root -finalized_checkpoint_root: string -- Encoded 32-byte value from store.finalized_checkpoint.root -best_justified_checkpoint_root: string -- Encoded 32-byte value from store.best_justified_checkpoint.root ``` For example: ```yaml - checks: - time: 144 - genesis_time: 0 - head: {slot: 17, root: '0xd2724c86002f7e1f8656ab44a341a409ad80e6e70a5225fd94835566deebb66f'} - justified_checkpoint_root: '0xcea6ecd3d3188e32ebf611f960eebd45b6c6f477a7cff242fa567a42653bfc7c' - finalized_checkpoint_root: '0xcea6ecd3d3188e32ebf611f960eebd45b6c6f477a7cff242fa567a42653bfc7c' - best_justified_checkpoint: '0xcea6ecd3d3188e32ebf611f960eebd45b6c6f477a7cff242fa567a42653bfc7c' + time: 192 + head: {slot: 32, root: '0xdaa1d49d57594ced0c35688a6da133abb086d191a2ebdfd736fad95299325aeb'} + justified_checkpoint: {epoch: 3, root: '0xc25faab4acab38d3560864ca01e4d5cc4dc2cd473da053fbc03c2669143a2de4'} + finalized_checkpoint: {epoch: 2, root: '0x40d32d6283ec11c53317a46808bc88f55657d93b95a1af920403187accf48f4f'} + best_justified_checkpoint: {epoch: 3, root: '0xc25faab4acab38d3560864ca01e4d5cc4dc2cd473da053fbc03c2669143a2de4'} ``` *Note*: Each `checks` step may include one or multiple items. Each item has to be checked against the current store. From 9b065c78167d1288e58913b56868e742207cd85e Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 31 Aug 2021 17:42:10 +0800 Subject: [PATCH 3/5] To avoid using non-genesis anchor state, rewrite `test_on_block_finalized_skip_slots_not_in_skip_chain` --- .../test/phase0/fork_choice/test_on_block.py | 83 +++++++++++-------- 1 file changed, 50 insertions(+), 33 deletions(-) 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 457eef7ff..0a3802338 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 @@ -26,7 +26,6 @@ from eth2spec.test.helpers.state import ( next_epoch, next_slots, state_transition_and_sign_block, - transition_to, ) @@ -191,6 +190,10 @@ def test_on_block_before_finalized(spec, state): @spec_state_test @with_presets([MINIMAL], reason="too slow") def test_on_block_finalized_skip_slots(spec, state): + """ + Test case was originally from https://github.com/ethereum/consensus-specs/pull/1579 + And then rewrote largely. + """ test_steps = [] # Initialization store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state) @@ -200,21 +203,28 @@ def test_on_block_finalized_skip_slots(spec, state): on_tick_and_append_step(spec, store, current_time, test_steps) assert store.time == current_time - # Create a finalized chain - for _ in range(4): + # Fill epoch 0 and the first slot of epoch 1 + state, store, _ = yield from apply_next_slots_with_attestations( + spec, state, store, spec.SLOTS_PER_EPOCH, True, False, test_steps) + + # Skip the rest slots of epoch 1 and the first slot of epoch 2 + next_slots(spec, state, spec.SLOTS_PER_EPOCH) + + # Fill epoch 3 and 4 + for _ in range(2): state, store, _ = yield from apply_next_epoch_with_attestations( - spec, state, store, True, False, test_steps=test_steps) - assert store.finalized_checkpoint.epoch == 2 + spec, state, store, True, True, test_steps=test_steps) - # Another chain - another_state = store.block_states[store.finalized_checkpoint.root].copy() - # Build block that includes the skipped slots up to finality in chain - block = build_empty_block(spec, - another_state, - spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) + 2) - block.body.graffiti = b'\x12' * 32 - signed_block = state_transition_and_sign_block(spec, another_state, block) + # Now we get finalized epoch 1, where compute_start_slot_at_epoch(1) is a skipped slot + assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 + assert store.finalized_checkpoint.root == spec.get_block_root(state, 1) == spec.get_block_root(state, 2) + assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 3 + assert store.justified_checkpoint == state.current_justified_checkpoint + # Now build a block at later slot than finalized *epoch* + # Includes finalized block in chain and the skipped slots + block = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) yield from tick_and_add_block(spec, store, signed_block, test_steps) yield 'steps', test_steps @@ -224,36 +234,43 @@ def test_on_block_finalized_skip_slots(spec, state): @spec_state_test @with_presets([MINIMAL], reason="too slow") def test_on_block_finalized_skip_slots_not_in_skip_chain(spec, state): + """ + Test case was originally from https://github.com/ethereum/consensus-specs/pull/1579 + And then rewrote largely. + """ test_steps = [] # Initialization - 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, anchor_block = get_genesis_forkchoice_store_and_block(spec, state) yield 'anchor_state', state - yield 'anchor_block', block - + yield 'anchor_block', anchor_block current_time = state.slot * spec.config.SECONDS_PER_SLOT + store.genesis_time on_tick_and_append_step(spec, store, current_time, test_steps) assert store.time == current_time - pre_finalized_checkpoint_epoch = store.finalized_checkpoint.epoch + # Fill epoch 0 and the first slot of epoch 1 + state, store, _ = yield from apply_next_slots_with_attestations( + spec, state, store, spec.SLOTS_PER_EPOCH, True, False, test_steps) - # Finalized - for _ in range(3): + # Skip the rest slots of epoch 1 and the first slot of epoch 2 + next_slots(spec, state, spec.SLOTS_PER_EPOCH) + + # Fill epoch 3 and 4 + for _ in range(2): state, store, _ = yield from apply_next_epoch_with_attestations( - spec, state, store, True, False, test_steps=test_steps) - assert store.finalized_checkpoint.epoch == pre_finalized_checkpoint_epoch + 1 + spec, state, store, True, True, test_steps=test_steps) - # Now build a block at later slot than finalized epoch - # Includes finalized block in chain, but not at appropriate skip slot - pre_state = store.block_states[block.hash_tree_root()].copy() - block = build_empty_block(spec, - state=pre_state, - slot=spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) + 2) - block.body.graffiti = b'\x12' * 32 - signed_block = sign_block(spec, pre_state, block) + # Now we get finalized epoch 1, where compute_start_slot_at_epoch(1) is a skipped slot + assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 + assert store.finalized_checkpoint.root == spec.get_block_root(state, 1) == spec.get_block_root(state, 2) + assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 3 + assert store.justified_checkpoint == state.current_justified_checkpoint + + # Now build a block after the block of the finalized **root** + # Includes finalized block in chain, but does not include finalized skipped slots + another_state = store.block_states[store.finalized_checkpoint.root].copy() + assert another_state.slot == spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch - 1) + block = build_empty_block_for_next_slot(spec, another_state) + signed_block = state_transition_and_sign_block(spec, another_state, block) yield from tick_and_add_block(spec, store, signed_block, test_steps, valid=False) yield 'steps', test_steps From b23ed05eee5f499d3ff41f8356c19bd07b01e0ca Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 31 Aug 2021 18:40:26 +0800 Subject: [PATCH 4/5] [`test_on_block_finalized_skip_slots`] Make target state right after skipped slots --- .../eth2spec/test/phase0/fork_choice/test_on_block.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 0a3802338..8227c4cfb 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 @@ -210,6 +210,9 @@ def test_on_block_finalized_skip_slots(spec, state): # Skip the rest slots of epoch 1 and the first slot of epoch 2 next_slots(spec, state, spec.SLOTS_PER_EPOCH) + # The state after the skipped slots + target_state = state.copy() + # Fill epoch 3 and 4 for _ in range(2): state, store, _ = yield from apply_next_epoch_with_attestations( @@ -223,8 +226,8 @@ def test_on_block_finalized_skip_slots(spec, state): # Now build a block at later slot than finalized *epoch* # Includes finalized block in chain and the skipped slots - block = build_empty_block_for_next_slot(spec, state) - signed_block = state_transition_and_sign_block(spec, state, block) + block = build_empty_block_for_next_slot(spec, target_state) + signed_block = state_transition_and_sign_block(spec, target_state, block) yield from tick_and_add_block(spec, store, signed_block, test_steps) yield 'steps', test_steps From 5bc59d8aabcee8930900466dee9fa9af2257622f Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 8 Sep 2021 21:22:48 +0800 Subject: [PATCH 5/5] Fix the comments --- .../pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 8227c4cfb..c00a91b28 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 @@ -218,7 +218,7 @@ def test_on_block_finalized_skip_slots(spec, state): state, store, _ = yield from apply_next_epoch_with_attestations( spec, state, store, True, True, test_steps=test_steps) - # Now we get finalized epoch 1, where compute_start_slot_at_epoch(1) is a skipped slot + # Now we get finalized epoch 2, where `compute_start_slot_at_epoch(2)` is a skipped slot assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 assert store.finalized_checkpoint.root == spec.get_block_root(state, 1) == spec.get_block_root(state, 2) assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 3 @@ -262,7 +262,7 @@ def test_on_block_finalized_skip_slots_not_in_skip_chain(spec, state): state, store, _ = yield from apply_next_epoch_with_attestations( spec, state, store, True, True, test_steps=test_steps) - # Now we get finalized epoch 1, where compute_start_slot_at_epoch(1) is a skipped slot + # Now we get finalized epoch 2, where `compute_start_slot_at_epoch(2)` is a skipped slot assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 assert store.finalized_checkpoint.root == spec.get_block_root(state, 1) == spec.get_block_root(state, 2) assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 3