Merge pull request #1250 from ethereum/fork_choice_rule_test

Fix `justified_checkpoint` update

specs/core/0_fork-choice.md

@@ -171,8 +171,6 @@ def on_block(store: Store, block: BeaconBlock) -> None:
     # Update justified checkpoint
     if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch:
         store.justified_checkpoint = state.current_justified_checkpoint
-    elif state.previous_justified_checkpoint.epoch > store.justified_checkpoint.epoch:
-        store.justified_checkpoint = state.previous_justified_checkpoint
 
     # Update finalized checkpoint
     if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch:

test_libs/pyspec/eth2spec/test/fork_choice/test_on_block.py

@@ -2,6 +2,7 @@ from eth2spec.utils.ssz.ssz_impl import signing_root
 
 from eth2spec.test.context import with_all_phases, with_state, bls_switch
 from eth2spec.test.helpers.block import build_empty_block_for_next_slot
+from eth2spec.test.helpers.state import next_epoch, next_epoch_with_attestations
 
 
 def run_on_block(spec, state, store, block, valid=True):
@@ -17,6 +18,17 @@ def run_on_block(spec, state, store, block, valid=True):
     assert store.blocks[signing_root(block)] == block
 
 
+def apply_next_epoch_with_attestations(spec, state, store):
+    _, new_blocks, state = next_epoch_with_attestations(spec, state, True, False)
+    for block in new_blocks:
+        block_root = signing_root(block)
+        store.blocks[block_root] = block
+        store.block_states[block_root] = state
+        last_block = block
+    spec.on_tick(store, store.time + state.slot * spec.SECONDS_PER_SLOT)
+    return state, store, last_block
+
+
 @with_all_phases
 @with_state
 @bls_switch
@@ -41,6 +53,32 @@ def test_basic(spec, state):
     # TODO: add tests for justified_root and finalized_root
 
 
+@with_all_phases
+@with_state
+@bls_switch
+def test_on_block_checkpoints(spec, state):
+    # Initialization
+    store = spec.get_genesis_store(state)
+    time = 100
+    spec.on_tick(store, time)
+
+    next_epoch(spec, state)
+    spec.on_tick(store, store.time + state.slot * spec.SECONDS_PER_SLOT)
+    state, store, last_block = apply_next_epoch_with_attestations(spec, state, store)
+    next_epoch(spec, state)
+    spec.on_tick(store, store.time + state.slot * spec.SECONDS_PER_SLOT)
+    last_block_root = signing_root(last_block)
+
+    # Mock the finalized_checkpoint
+    store.block_states[last_block_root].finalized_checkpoint = (
+        store.block_states[last_block_root].current_justified_checkpoint
+    )
+
+    # On receiving a block of `GENESIS_SLOT + 1` slot
+    block = build_empty_block_for_next_slot(spec, state)
+    run_on_block(spec, state, store, block)
+
+
 @with_all_phases
 @with_state
 @bls_switch

test_libs/pyspec/eth2spec/test/helpers/state.py

@@ -1,4 +1,6 @@
-from eth2spec.test.helpers.block import sign_block
+from copy import deepcopy
+from eth2spec.test.helpers.attestations import get_valid_attestation
+from eth2spec.test.helpers.block import sign_block, build_empty_block_for_next_slot
 
 
 def get_balance(state, index):
@@ -36,3 +38,30 @@ def state_transition_and_sign_block(spec, state, block):
     spec.state_transition(state, block)
     block.state_root = state.hash_tree_root()
     sign_block(spec, state, block)
+
+
+def next_epoch_with_attestations(spec,
+                                 state,
+                                 fill_cur_epoch,
+                                 fill_prev_epoch):
+    assert state.slot % spec.SLOTS_PER_EPOCH == 0
+
+    post_state = deepcopy(state)
+    blocks = []
+    for _ in range(spec.SLOTS_PER_EPOCH):
+        block = build_empty_block_for_next_slot(spec, post_state)
+        if fill_cur_epoch and post_state.slot >= spec.MIN_ATTESTATION_INCLUSION_DELAY:
+            slot_to_attest = post_state.slot - spec.MIN_ATTESTATION_INCLUSION_DELAY + 1
+            if slot_to_attest >= spec.compute_start_slot_of_epoch(spec.get_current_epoch(post_state)):
+                cur_attestation = get_valid_attestation(spec, post_state, slot_to_attest)
+                block.body.attestations.append(cur_attestation)
+
+        if fill_prev_epoch:
+            slot_to_attest = post_state.slot - spec.SLOTS_PER_EPOCH + 1
+            prev_attestation = get_valid_attestation(spec, post_state, slot_to_attest)
+            block.body.attestations.append(prev_attestation)
+
+        state_transition_and_sign_block(spec, post_state, block)
+        blocks.append(block)
+
+    return state, blocks, post_state

test_libs/pyspec/eth2spec/test/test_finality.py

@@ -1,9 +1,6 @@
-from copy import deepcopy
-
 from eth2spec.test.context import spec_state_test, never_bls, with_all_phases
-from eth2spec.test.helpers.state import next_epoch, state_transition_and_sign_block
+from eth2spec.test.helpers.state import next_epoch, next_epoch_with_attestations
-from eth2spec.test.helpers.block import build_empty_block_for_next_slot, apply_empty_block
+from eth2spec.test.helpers.block import apply_empty_block
-from eth2spec.test.helpers.attestations import get_valid_attestation
 
 
 def check_finality(spec,
@@ -31,33 +28,6 @@ def check_finality(spec,
         assert state.finalized_checkpoint == prev_state.finalized_checkpoint
 
 
-def next_epoch_with_attestations(spec,
-                                 state,
-                                 fill_cur_epoch,
-                                 fill_prev_epoch):
-    assert state.slot % spec.SLOTS_PER_EPOCH == 0
-
-    post_state = deepcopy(state)
-    blocks = []
-    for _ in range(spec.SLOTS_PER_EPOCH):
-        block = build_empty_block_for_next_slot(spec, post_state)
-        if fill_cur_epoch and post_state.slot >= spec.MIN_ATTESTATION_INCLUSION_DELAY:
-            slot_to_attest = post_state.slot - spec.MIN_ATTESTATION_INCLUSION_DELAY + 1
-            if slot_to_attest >= spec.compute_start_slot_of_epoch(spec.get_current_epoch(post_state)):
-                cur_attestation = get_valid_attestation(spec, post_state, slot_to_attest)
-                block.body.attestations.append(cur_attestation)
-
-        if fill_prev_epoch:
-            slot_to_attest = post_state.slot - spec.SLOTS_PER_EPOCH + 1
-            prev_attestation = get_valid_attestation(spec, post_state, slot_to_attest)
-            block.body.attestations.append(prev_attestation)
-
-        state_transition_and_sign_block(spec, post_state, block)
-        blocks.append(block)
-
-    return state, blocks, post_state
-
-
 @with_all_phases
 @never_bls
 @spec_state_test