From 9004bcf1a51ce11dfacfb044c4351a12165edfc7 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 25 Nov 2019 15:06:33 -0700 Subject: [PATCH 1/7] WIP filter block tree --- specs/core/0_fork-choice.md | 49 +++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index 099843067..86afdb7d5 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -136,17 +136,62 @@ def get_latest_attesting_balance(store: Store, root: Root) -> Gwei: )) ``` +#### `filter_block_tree` + +```python +def filter_block_tree(store, block_root, blocks): + block = store.blocks[block_root] + children = [ + root for root in store.blocks.keys() + if store.blocks[root].parent_root == block + ] + + # If any children branches contain expected finalized/justified checkpoints, + # add to filtered block-tree and signal viability to parent. + if any(children): + if True in [filter_block_tree(child, blocks) for child in children]: + blocks[block_root] = block + return True + return False + + # If leaf block, check finalized/justified checkpoints as matching latest. + # If matching, add to viable block-tree and signal viability to parent. + head_state = store.block_states[block_root] + is_viable_branch = ( + head_state.finalized_checkpoint == store.finalized_checkpoint + and head_state.current_justified_checkpoint == store.justified_checkpoint + ) + if is_viable_branch: + blocks[block_root] = block + return True + + # Otherwise, not viable + return False +``` + +#### `get_filtered_block_tree` + +```python +def get_filtered_block_tree(store: Store) -> Dict[Root, BeaconBlock]: + head = store.justified_checkpoint.root + blocks = {} + filter_block_tree(head, blocks) + return blocks +``` + #### `get_head` ```python def get_head(store: Store) -> Root: + # Get filtered block tree that includes viable branches + blocks = get_filtered_block_tree(store) # Execute the LMD-GHOST fork choice head = store.justified_checkpoint.root justified_slot = compute_start_slot_at_epoch(store.justified_checkpoint.epoch) while True: children = [ - root for root in store.blocks.keys() - if store.blocks[root].parent_root == head and store.blocks[root].slot > justified_slot + root for root in blocks.keys() + if blocks[root].parent_root == head and blocks[root].slot > justified_slot ] if len(children) == 0: return head From 8021f34c0682a639830f4a60a5f39c6b347e1280 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 25 Nov 2019 15:44:22 -0700 Subject: [PATCH 2/7] fix existing fork choce tests with new filter --- specs/core/0_fork-choice.md | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index 86afdb7d5..d62b04169 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -139,17 +139,17 @@ def get_latest_attesting_balance(store: Store, root: Root) -> Gwei: #### `filter_block_tree` ```python -def filter_block_tree(store, block_root, blocks): +def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconBlock]) -> bool: block = store.blocks[block_root] children = [ root for root in store.blocks.keys() - if store.blocks[root].parent_root == block + if store.blocks[root].parent_root == block_root ] # If any children branches contain expected finalized/justified checkpoints, # add to filtered block-tree and signal viability to parent. if any(children): - if True in [filter_block_tree(child, blocks) for child in children]: + if True in [filter_block_tree(store, child, blocks) for child in children]: blocks[block_root] = block return True return False @@ -157,10 +157,15 @@ def filter_block_tree(store, block_root, blocks): # If leaf block, check finalized/justified checkpoints as matching latest. # If matching, add to viable block-tree and signal viability to parent. head_state = store.block_states[block_root] - is_viable_branch = ( - head_state.finalized_checkpoint == store.finalized_checkpoint - and head_state.current_justified_checkpoint == store.justified_checkpoint - ) + + # Handle base case where justified hasn't updated yet + if head_state.current_justified_checkpoint.epoch == GENESIS_EPOCH: + is_viable_branch = True + else: + is_viable_branch = ( + head_state.finalized_checkpoint == store.finalized_checkpoint + and head_state.current_justified_checkpoint == store.justified_checkpoint + ) if is_viable_branch: blocks[block_root] = block return True @@ -174,8 +179,8 @@ def filter_block_tree(store, block_root, blocks): ```python def get_filtered_block_tree(store: Store) -> Dict[Root, BeaconBlock]: head = store.justified_checkpoint.root - blocks = {} - filter_block_tree(head, blocks) + blocks: Dict[Root, BeaconBlock] = {} + filter_block_tree(store, head, blocks) return blocks ``` @@ -185,6 +190,8 @@ def get_filtered_block_tree(store: Store) -> Dict[Root, BeaconBlock]: def get_head(store: Store) -> Root: # Get filtered block tree that includes viable branches blocks = get_filtered_block_tree(store) + print(len(blocks)) + print(len(store.blocks)) # Execute the LMD-GHOST fork choice head = store.justified_checkpoint.root justified_slot = compute_start_slot_at_epoch(store.justified_checkpoint.epoch) From 5aa9f9655871b2f86ac06bed676b0bf752ad851b Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 27 Nov 2019 13:11:30 -0700 Subject: [PATCH 3/7] Update specs/core/0_fork-choice.md Co-Authored-By: Diederik Loerakker --- specs/core/0_fork-choice.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index d62b04169..c9a5c90d6 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -149,7 +149,7 @@ def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconB # If any children branches contain expected finalized/justified checkpoints, # add to filtered block-tree and signal viability to parent. if any(children): - if True in [filter_block_tree(store, child, blocks) for child in children]: + if any(filter_block_tree(store, child, blocks) for child in children): blocks[block_root] = block return True return False From 2275cdfeb8e8d823fcf1af2f67884695c475e4a7 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 27 Nov 2019 13:26:44 -0700 Subject: [PATCH 4/7] fix child filter in get_head --- specs/core/0_fork-choice.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index c9a5c90d6..d62b04169 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -149,7 +149,7 @@ def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconB # If any children branches contain expected finalized/justified checkpoints, # add to filtered block-tree and signal viability to parent. if any(children): - if any(filter_block_tree(store, child, blocks) for child in children): + if True in [filter_block_tree(store, child, blocks) for child in children]: blocks[block_root] = block return True return False From dfcd6f6402be7d5d660f9aa91ebf72ebff0f01a6 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 4 Dec 2019 16:50:48 -0700 Subject: [PATCH 5/7] add tests for block filter in get_head --- specs/core/0_fork-choice.md | 38 ++++----- .../test/fork_choice/test_get_head.py | 80 ++++++++++++++++++- 2 files changed, 99 insertions(+), 19 deletions(-) diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index d62b04169..3cdbcbc5a 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -155,22 +155,22 @@ def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconB return False # If leaf block, check finalized/justified checkpoints as matching latest. - # If matching, add to viable block-tree and signal viability to parent. head_state = store.block_states[block_root] - # Handle base case where justified hasn't updated yet - if head_state.current_justified_checkpoint.epoch == GENESIS_EPOCH: - is_viable_branch = True - else: - is_viable_branch = ( - head_state.finalized_checkpoint == store.finalized_checkpoint - and head_state.current_justified_checkpoint == store.justified_checkpoint - ) - if is_viable_branch: + correct_justified = ( + store.justified_checkpoint.epoch == GENESIS_EPOCH + or head_state.current_justified_checkpoint == store.justified_checkpoint + ) + correct_finalized = ( + store.finalized_checkpoint.epoch == GENESIS_EPOCH + or head_state.finalized_checkpoint == store.finalized_checkpoint + ) + # If expected finalized/justified, add to viable block-tree and signal viability to parent. + if correct_justified and correct_finalized: blocks[block_root] = block return True - # Otherwise, not viable + # Otherwise, branch not viable return False ``` @@ -178,9 +178,13 @@ def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconB ```python def get_filtered_block_tree(store: Store) -> Dict[Root, BeaconBlock]: - head = store.justified_checkpoint.root + """ + Retrieve a filtered block true from ``store``, only returning branches + whose leaf state's justified/finalized info agrees with that in ``store``. + """ + base = store.justified_checkpoint.root blocks: Dict[Root, BeaconBlock] = {} - filter_block_tree(store, head, blocks) + filter_block_tree(store, base, blocks) return blocks ``` @@ -188,10 +192,8 @@ def get_filtered_block_tree(store: Store) -> Dict[Root, BeaconBlock]: ```python def get_head(store: Store) -> Root: - # Get filtered block tree that includes viable branches + # Get filtered block tree that only includes viable branches blocks = get_filtered_block_tree(store) - print(len(blocks)) - print(len(store.blocks)) # Execute the LMD-GHOST fork choice head = store.justified_checkpoint.root justified_slot = compute_start_slot_at_epoch(store.justified_checkpoint.epoch) @@ -224,8 +226,8 @@ def should_update_justified_checkpoint(store: Store, new_justified_checkpoint: C 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 + get_ancestor(store, new_justified_checkpoint.root, store.blocks[store.justified_checkpoint.root].slot) + == store.justified_checkpoint.root ): return False diff --git a/test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py b/test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py index ff5a822fb..f0098bb98 100644 --- a/test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py +++ b/test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py @@ -1,7 +1,12 @@ from eth2spec.test.context import with_all_phases, spec_state_test from eth2spec.test.helpers.attestations import get_valid_attestation from eth2spec.test.helpers.block import build_empty_block_for_next_slot -from eth2spec.test.helpers.state import state_transition_and_sign_block +from eth2spec.test.helpers.state import ( + next_epoch, + next_epoch_with_attestations, + state_transition_and_sign_block, +) +from eth2spec.utils.ssz.ssz_impl import signing_root def add_block_to_store(spec, store, block): @@ -112,3 +117,76 @@ def test_shorter_chain_but_heavier_weight(spec, state): add_attestation_to_store(spec, store, short_attestation) assert spec.get_head(store) == spec.signing_root(short_block) + + +@with_all_phases +@spec_state_test +def test_filtered_block_tree(spec, state): + genesis_state = state.copy() + + # transition state past initial couple of epochs + next_epoch(spec, state) + next_epoch(spec, state) + + # Initialization + store = spec.get_genesis_store(genesis_state) + genesis_block = spec.BeaconBlock(state_root=genesis_state.hash_tree_root()) + assert spec.get_head(store) == spec.signing_root(genesis_block) + + # fill in attestations for entire epoch, justifying the recent epoch + prev_state, blocks, state = next_epoch_with_attestations(spec, state, True, False) + attestations = [attestation for block in blocks for attestation in block.body.attestations] + assert state.current_justified_checkpoint.epoch > prev_state.current_justified_checkpoint.epoch + + # tick time forward and add blocks and attestations to store + current_time = state.slot * spec.SECONDS_PER_SLOT + store.genesis_time + spec.on_tick(store, current_time) + for block in blocks: + spec.on_block(store, block) + for attestation in attestations: + spec.on_attestation(store, attestation) + + assert store.justified_checkpoint == state.current_justified_checkpoint + + # the last block in the branch should be the head + expected_head_root = signing_root(blocks[-1]) + assert spec.get_head(store) == expected_head_root + + # + # create branch containing the justified block but not containing enough on + # chain votes to justify that block + # + + # build a chain without attestations off of previous justified block + non_viable_state = store.block_states[store.justified_checkpoint.root].copy() + + # ensure that next wave of votes are for future epoch + next_epoch(spec, non_viable_state) + next_epoch(spec, non_viable_state) + next_epoch(spec, non_viable_state) + assert spec.get_current_epoch(non_viable_state) > store.justified_checkpoint.epoch + + # create rogue block that will be attested to in this non-viable branch + rogue_block = build_empty_block_for_next_slot(spec, non_viable_state, True) + state_transition_and_sign_block(spec, non_viable_state, rogue_block) + + # create an epoch's worth of attestations for the rogue block + next_epoch(spec, non_viable_state) + attestations = [] + for i in range(spec.SLOTS_PER_EPOCH): + slot = rogue_block.slot + i + for index in range(spec.get_committee_count_at_slot(non_viable_state, slot)): + attestation = get_valid_attestation(spec, non_viable_state, rogue_block.slot + i, index) + attestations.append(attestation) + + # tick time forward to be able to include up to the latest attestation + current_time = (attestations[-1].data.slot + 1) * spec.SECONDS_PER_SLOT + store.genesis_time + spec.on_tick(store, current_time) + + # include rogue block and associated attestations in the store + spec.on_block(store, rogue_block) + for attestation in attestations: + spec.on_attestation(store, attestation) + + # ensure that get_head still returns the head from the previous branch + assert spec.get_head(store) == expected_head_root From 7baf81e4c2991d31982bf252562d4cfe3d7428e9 Mon Sep 17 00:00:00 2001 From: protolambda Date: Thu, 5 Dec 2019 18:47:41 +0100 Subject: [PATCH 6/7] minor test style change, avoid state copy --- .../pyspec/eth2spec/test/fork_choice/test_get_head.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py b/test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py index f0098bb98..c40864c81 100644 --- a/test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py +++ b/test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py @@ -122,15 +122,15 @@ def test_shorter_chain_but_heavier_weight(spec, state): @with_all_phases @spec_state_test def test_filtered_block_tree(spec, state): - genesis_state = state.copy() + # Initialization + genesis_state_root = state.hash_tree_root() + store = spec.get_genesis_store(state) + genesis_block = spec.BeaconBlock(state_root=genesis_state_root) # transition state past initial couple of epochs next_epoch(spec, state) next_epoch(spec, state) - # Initialization - store = spec.get_genesis_store(genesis_state) - genesis_block = spec.BeaconBlock(state_root=genesis_state.hash_tree_root()) assert spec.get_head(store) == spec.signing_root(genesis_block) # fill in attestations for entire epoch, justifying the recent epoch From e53a6daecc179f50cf9d3877483d568127a98d20 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 8 Dec 2019 11:50:04 -0700 Subject: [PATCH 7/7] clarify fliter block tree through two line usage Co-Authored-By: Hsiao-Wei Wang --- specs/core/0_fork-choice.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index 3cdbcbc5a..95e8ee7a2 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -149,7 +149,8 @@ def filter_block_tree(store: Store, block_root: Root, blocks: Dict[Root, BeaconB # If any children branches contain expected finalized/justified checkpoints, # add to filtered block-tree and signal viability to parent. if any(children): - if True in [filter_block_tree(store, child, blocks) for child in children]: + filter_block_tree_result = [filter_block_tree(store, child, blocks) for child in children] + if any(filter_block_tree_result): blocks[block_root] = block return True return False