[fc-tip-tracking-pull-up] Fix on_tick catch-up logic (#9)

* Fix `on_tick` catch-up logic * Also test `on_tick` catch-up in `test_new_justified_is_later_than_store_justified`
ethereum · Aug 16, 2022 · 717c2bd · 717c2bd
1 parent 07068d4
commit 717c2bd
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 16 deletions.
diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md
@@ -446,8 +446,8 @@ def update_latest_messages(store: Store, attesting_indices: Sequence[ValidatorIn
 def on_tick(store: Store, time: uint64) -> None:
     # If the ``store.time`` falls behind, catch up slot by slot to
     # ensure that every previous slot will be processed with ``on_tick_per_slot``.
-    tick_slot = (time - store.genesis_time) % SECONDS_PER_SLOT
-    while get_current_slot(store) <= tick_slot:
+    tick_slot = (time - store.genesis_time) // SECONDS_PER_SLOT
+    while get_current_slot(store) < tick_slot:
         previous_time = store.genesis_time + (get_current_slot(store) + 1) * SECONDS_PER_SLOT
         on_tick_per_slot(store, previous_time)
     on_tick_per_slot(store, time)

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
@@ -426,8 +426,8 @@ def test_new_justified_is_later_than_store_justified(spec, state):
 
     fork_2_state (forked from fork_1_state's epoch 2):
         epoch
-                       └──── [3] <- [4] <- [5] <- [6]
-         F                           J
+                       └──── [3] <- [4] <- [5] <- [6] <- [7]
+         F                                  J
 
     fork_3_state (forked from genesis):
         [0] <- [1] <- [2] <- [3] <- [4] <- [5]
@@ -493,21 +493,15 @@ def test_new_justified_is_later_than_store_justified(spec, state):
         yield from tick_and_add_block(spec, store, signed_block, test_steps)
         assert fork_2_state.current_justified_checkpoint.epoch == 0
 
-    # Propose a block at epoch 6, SAFE_SLOTS_TO_UPDATE_JUSTIFIED + 2 slot
+    # Test `on_tick` catch-up
+    # Skip epoch 6
     next_epoch(spec, fork_2_state)
-    next_slots(spec, fork_2_state, spec.SAFE_SLOTS_TO_UPDATE_JUSTIFIED + 2)
-    signed_block = state_transition_with_full_attestations_block(spec, fork_2_state, True, True)
-    assert fork_2_state.finalized_checkpoint.epoch == 0
-    assert fork_2_state.current_justified_checkpoint.epoch == 5
-    # Check SAFE_SLOTS_TO_UPDATE_JUSTIFIED
+    # At the middle of epoch 7
+    next_slots(spec, fork_2_state, spec.SLOTS_PER_EPOCH // 2)
+    # Due to tip pull-up, `store.current_justified_checkpoint` would be updated.
     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)
-    assert store.finalized_checkpoint.epoch == 0
-    assert store.justified_checkpoint.epoch == 3
-    assert store.best_justified_checkpoint.epoch == 5
+    assert store.justified_checkpoint.epoch == 5
 
     # ------ fork_3_state: Create another chain to test the
     # "Update justified if new justified is later than store justified" case
@@ -548,9 +542,12 @@ 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.finalized_checkpoint.epoch == 3
     assert store.justified_checkpoint == fork_3_state.current_justified_checkpoint
+    assert store.justified_checkpoint.epoch == 4
     assert store.justified_checkpoint != store.best_justified_checkpoint
     assert store.best_justified_checkpoint == fork_2_state.current_justified_checkpoint
+    assert store.best_justified_checkpoint.epoch == 5
 
     yield 'steps', test_steps