Merge pull request #2577 from ethereum/fix-fork-choice-tests

Fix fork choice `on_block` tests and update test format

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),
+            'justified_checkpoint': {
-            'finalized_checkpoint_root': encode_hex(store.finalized_checkpoint.root),
+                'epoch': int(store.justified_checkpoint.epoch),
-            'best_justified_checkpoint': encode_hex(store.best_justified_checkpoint.root),
+                '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),
+            },
         }
     })
 

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,31 @@ 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
+    # Fill epoch 0 and the first slot of epoch 1
-    for _ in range(4):
+    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)
+
+    # 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(
-            spec, state, store, True, False, test_steps=test_steps)
+            spec, state, store, True, True, test_steps=test_steps)
-    assert store.finalized_checkpoint.epoch == 2
 
-    # Another chain
+    # Now we get finalized epoch 2, where `compute_start_slot_at_epoch(2)` is a skipped slot
-    another_state = store.block_states[store.finalized_checkpoint.root].copy()
+    assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2
-    # Build block that includes the skipped slots up to finality in chain
+    assert store.finalized_checkpoint.root == spec.get_block_root(state, 1) == spec.get_block_root(state, 2)
-    block = build_empty_block(spec,
+    assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 3
-                              another_state,
+    assert store.justified_checkpoint == state.current_justified_checkpoint
-                              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 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, 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
@@ -224,36 +237,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)
+    store, anchor_block = get_genesis_forkchoice_store_and_block(spec, 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)
     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
+    # Skip the rest slots of epoch 1 and the first slot of epoch 2
-    for _ in range(3):
+    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)
+            spec, state, store, True, True, test_steps=test_steps)
-    assert store.finalized_checkpoint.epoch == pre_finalized_checkpoint_epoch + 1
 
-    # Now build a block at later slot than finalized epoch
+    # Now we get finalized epoch 2, where `compute_start_slot_at_epoch(2)` is a skipped slot
-    # Includes finalized block in chain, but not at appropriate skip slot
+    assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2
-    pre_state = store.block_states[block.hash_tree_root()].copy()
+    assert store.finalized_checkpoint.root == spec.get_block_root(state, 1) == spec.get_block_root(state, 2)
-    block = build_empty_block(spec,
+    assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 3
-                              state=pre_state,
+    assert store.justified_checkpoint == state.current_justified_checkpoint
-                              slot=spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) + 2)
+
-    block.body.graffiti = b'\x12' * 32
+    # Now build a block after the block of the finalized **root**
-    signed_block = sign_block(spec, pre_state, block)
+    # 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
@@ -483,7 +503,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 +547,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 +665,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)

tests/formats/fork_choice/README.md

@@ -80,26 +80,34 @@ checks: {<store_attibute>: value}  -- the assertions.
 `<store_attibute>` 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)
+head: {
-    slot: slot,
+    slot: int,
-    root: string,
+    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
+    time: 192
-    genesis_time: 0
+    head: {slot: 32, root: '0xdaa1d49d57594ced0c35688a6da133abb086d191a2ebdfd736fad95299325aeb'}
-    head: {slot: 17, root: '0xd2724c86002f7e1f8656ab44a341a409ad80e6e70a5225fd94835566deebb66f'}
+    justified_checkpoint: {epoch: 3, root: '0xc25faab4acab38d3560864ca01e4d5cc4dc2cd473da053fbc03c2669143a2de4'}
-    justified_checkpoint_root: '0xcea6ecd3d3188e32ebf611f960eebd45b6c6f477a7cff242fa567a42653bfc7c'
+    finalized_checkpoint: {epoch: 2, root: '0x40d32d6283ec11c53317a46808bc88f55657d93b95a1af920403187accf48f4f'}
-    finalized_checkpoint_root: '0xcea6ecd3d3188e32ebf611f960eebd45b6c6f477a7cff242fa567a42653bfc7c'
+    best_justified_checkpoint: {epoch: 3, root: '0xc25faab4acab38d3560864ca01e4d5cc4dc2cd473da053fbc03c2669143a2de4'}
-    best_justified_checkpoint: '0xcea6ecd3d3188e32ebf611f960eebd45b6c6f477a7cff242fa567a42653bfc7c'
 ```
 
 *Note*: Each `checks` step may include one or multiple items. Each item has to be checked against the current store.