Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Properly handle skip slots in fork choice #1579

Merged
merged 3 commits into from
Jan 22, 2020
Merged

Properly handle skip slots in fork choice #1579

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1697d0
fix corner case to properly handle skipped slots in get_ancestor
djrtwo Jan 21, 2020
e98c1b4
don't consider blocks with slots earlier than finalized in on_block f…
djrtwo Jan 21, 2020
b357e43
clarifying comment on call to get_ancestor in on_block
djrtwo Jan 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 6 additions & 14 deletions specs/phase0/fork-choice.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ def get_ancestor(store: Store, root: Root, slot: Slot) -> Root:
elif block.slot == slot:
return root
else:
return Bytes32() # root is older than queried slot: no results.
# root is older than queried slot, thus a skip slot. Return earliest root prior to slot
return root
```

#### `get_latest_attesting_balance`
Expand Down Expand Up @@ -239,13 +240,8 @@ def should_update_justified_checkpoint(store: Store, new_justified_checkpoint: C
if compute_slots_since_epoch_start(get_current_slot(store)) < SAFE_SLOTS_TO_UPDATE_JUSTIFIED:
return True

new_justified_block = store.blocks[new_justified_checkpoint.root]
if new_justified_block.slot <= compute_start_slot_at_epoch(store.justified_checkpoint.epoch):
return False
if not (
get_ancestor(store, new_justified_checkpoint.root, store.blocks[store.justified_checkpoint.root].slot)
== store.justified_checkpoint.root
):
justified_slot = compute_start_slot_at_epoch(store.justified_checkpoint.epoch)
if not get_ancestor(store, new_justified_checkpoint.root, justified_slot) == store.justified_checkpoint.root:
return False

return True
Expand Down Expand Up @@ -284,12 +280,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None:
# Add new block to the store
store.blocks[hash_tree_root(block)] = block
# Check block is a descendant of the finalized block
assert (
get_ancestor(store, hash_tree_root(block), store.blocks[store.finalized_checkpoint.root].slot) ==
store.finalized_checkpoint.root
)
# Check that block is later than the finalized epoch slot
assert block.slot > compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)
djrtwo marked this conversation as resolved.
Show resolved Hide resolved
finalized_slot = compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)
assert get_ancestor(store, hash_tree_root(block), finalized_slot) == store.finalized_checkpoint.root
# Check the block is valid and compute the post-state
state = state_transition(pre_state, signed_block, True)
# Add new state for this block to the store
Expand Down
42 changes: 42 additions & 0 deletions tests/core/pyspec/eth2spec/test/fork_choice/test_on_block.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,48 @@ def test_on_block_before_finalized(spec, state):
run_on_block(spec, store, signed_block, False)


@with_all_phases
@spec_state_test
def test_on_block_finalized_skip_slots(spec, state):
# Initialization
store = spec.get_genesis_store(state)
time = 100
spec.on_tick(store, time)

store.finalized_checkpoint = spec.Checkpoint(
epoch=store.finalized_checkpoint.epoch + 2,
root=store.finalized_checkpoint.root
)

# Build block that includes the skipped slots up to finality in chain
block = build_empty_block(spec, state, spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) + 2)
signed_block = state_transition_and_sign_block(spec, state, block)
spec.on_tick(store, store.time + state.slot * spec.SECONDS_PER_SLOT)
run_on_block(spec, store, signed_block)


@with_all_phases
@spec_state_test
def test_on_block_finalized_skip_slots_not_in_skip_chain(spec, state):
# Initialization
store = spec.get_genesis_store(state)

store.finalized_checkpoint = spec.Checkpoint(
epoch=store.finalized_checkpoint.epoch + 2,
root=store.finalized_checkpoint.root
)

# First transition through the epoch to ensure no skipped slots
state, store, last_signed_block = apply_next_epoch_with_attestations(spec, state, store)

# Now build a block at later slot than finalized epoch
# Includes finalized block in chain, but not at appropriate skip slot
block = build_empty_block(spec, state, spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) + 2)
signed_block = state_transition_and_sign_block(spec, state, block)
spec.on_tick(store, store.time + state.slot * spec.SECONDS_PER_SLOT)
run_on_block(spec, store, signed_block, False)


@with_all_phases
@spec_state_test
def test_on_block_update_justified_checkpoint_within_safe_slots(spec, state):
Expand Down