From cd5cf60deb83b654e847d00a60be31b5617154c2 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 18 Oct 2021 13:32:29 +0800 Subject: [PATCH 1/5] Clarify `get_pow_block` block-not-found case --- setup.py | 2 +- specs/merge/fork-choice.md | 12 +++++++++++- specs/merge/validator.md | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 3dcaaa44f..0611e5567 100644 --- a/setup.py +++ b/setup.py @@ -510,7 +510,7 @@ from eth2spec.utils.ssz.ssz_typing import Bytes20, ByteList, ByteVector, uint256 ExecutionState = Any -def get_pow_block(hash: Bytes32) -> PowBlock: +def get_pow_block(hash: Bytes32) -> Optional[PowBlock]: return PowBlock(block_hash=hash, parent_hash=Bytes32(), total_difficulty=uint256(0), difficulty=uint256(0)) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index ea4a5588a..eb99cce64 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -64,7 +64,8 @@ class PowBlock(Container): ### `get_pow_block` -Let `get_pow_block(block_hash: Hash32) -> PowBlock` be the function that given the hash of the PoW block returns its data. +Let `get_pow_block(block_hash: Hash32) -> Optional[PowBlock]` be the function that given the hash of the PoW block returns its data. +It may result in `None` if the requested block is not found if execution engine is still syncing. *Note*: The `eth_getBlockByHash` JSON-RPC method may be used to pull this information from an execution client. @@ -90,6 +91,12 @@ def is_valid_terminal_pow_block(block: PowBlock, parent: PowBlock) -> bool: ```python def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: + """ + Run ``on_block`` upon receiving a new block. + + An block that is asserted as invalid due to unavailable PoW block may be valid at a later time, + consider scheduling it for later processing in such case. + """ block = signed_block.message # Parent block must be known assert block.parent_root in store.block_states @@ -110,8 +117,11 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # [New in Merge] if is_merge_block(pre_state, block.body): + # Note: the unavailable PoW block(s) may be available later pow_block = get_pow_block(block.body.execution_payload.parent_hash) + assert pow_block is not None pow_parent = get_pow_block(pow_block.parent_hash) + assert pow_parent is not None assert is_valid_terminal_pow_block(pow_block, pow_parent) # Add new block to the store diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 2de4ef6b1..9fd1a24f7 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -103,6 +103,7 @@ def get_pow_block_at_terminal_total_difficulty(pow_chain: Sequence[PowBlock]) -> # `pow_chain` abstractly represents all blocks in the PoW chain for block in pow_chain: parent = get_pow_block(block.parent_hash) + assert parent is not None block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY if block_reached_ttd and not parent_reached_ttd: From e787da395403f732df46ba60a44cf117abd18e4c Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 19 Oct 2021 14:35:30 -0600 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Hsiao-Wei Wang --- specs/merge/fork-choice.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index eb99cce64..18f3d7d7f 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -65,7 +65,7 @@ class PowBlock(Container): ### `get_pow_block` Let `get_pow_block(block_hash: Hash32) -> Optional[PowBlock]` be the function that given the hash of the PoW block returns its data. -It may result in `None` if the requested block is not found if execution engine is still syncing. +It may result in `None` if the requested block is not yet available. *Note*: The `eth_getBlockByHash` JSON-RPC method may be used to pull this information from an execution client. @@ -117,7 +117,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # [New in Merge] if is_merge_block(pre_state, block.body): - # Note: the unavailable PoW block(s) may be available later + # Note: unavailable PoW block(s) may later become available. + # Nodes should queue such beacon blocks for later processing. pow_block = get_pow_block(block.body.execution_payload.parent_hash) assert pow_block is not None pow_parent = get_pow_block(pow_block.parent_hash) From 71d315950fa12b1cf27616854b37eab10db19e28 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 26 Oct 2021 21:38:22 +0800 Subject: [PATCH 3/5] Fix typo Co-authored-by: Mikhail Kalinin --- specs/merge/fork-choice.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 7b066c801..42c61b83d 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -114,7 +114,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: """ Run ``on_block`` upon receiving a new block. - An block that is asserted as invalid due to unavailable PoW block may be valid at a later time, + A block that is asserted as invalid due to unavailable PoW block may be valid at a later time, consider scheduling it for later processing in such case. """ block = signed_block.message From 60163475d719dcfa48cb6a669bc60299cd93dccd Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 28 Oct 2021 16:00:01 +0800 Subject: [PATCH 4/5] PR feedback from @mkalinin --- specs/merge/fork-choice.md | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 42c61b83d..eec084f62 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -16,6 +16,7 @@ - [`PowBlock`](#powblock) - [`get_pow_block`](#get_pow_block) - [`is_valid_terminal_pow_block`](#is_valid_terminal_pow_block) + - [`validate_merge_block`](#validate_merge_block) - [Updated fork-choice handlers](#updated-fork-choice-handlers) - [`on_block`](#on_block) @@ -103,6 +104,30 @@ def is_valid_terminal_pow_block(block: PowBlock, parent: PowBlock) -> bool: return is_total_difficulty_reached and is_parent_total_difficulty_valid ``` +### `validate_merge_block` + +```python +def validate_merge_block(block: BeaconBlock) -> None: + """ + Check the parent PoW block of execution payload is a valid terminal PoW block. + + Note: Unavailable PoW block(s) may later become available and a client software MAY delay + a call to ``validate_merge_block`` until the PoW block(s) become available. + """ + pow_block = get_pow_block(block.body.execution_payload.parent_hash) + # Check if `pow_block` is unavailable + assert pow_block is not None + pow_parent = get_pow_block(pow_block.parent_hash) + # Check if `pow_parent` is unavailable + assert pow_parent is not None + # Check if `pow_block` is a valid terminal PoW block + assert is_valid_terminal_pow_block(pow_block, pow_parent) + + # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. + if TERMINAL_BLOCK_HASH != Hash32(): + assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH +``` + ## Updated fork-choice handlers ### `on_block` @@ -137,17 +162,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # [New in Merge] if is_merge_block(pre_state, block.body): - # Check the parent PoW block of execution payload is a valid terminal PoW block. - # Note: unavailable PoW block(s) may later become available. - # Nodes should queue such beacon blocks for later processing. - pow_block = get_pow_block(block.body.execution_payload.parent_hash) - assert pow_block is not None - pow_parent = get_pow_block(pow_block.parent_hash) - assert pow_parent is not None - assert is_valid_terminal_pow_block(pow_block, pow_parent) - # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. - if TERMINAL_BLOCK_HASH != Hash32(): - assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH + validate_merge_block(block) # Add new block to the store store.blocks[hash_tree_root(block)] = block From 8ecbffa821818ecd89fa5029dd003662407c6b65 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 28 Oct 2021 16:06:58 +0800 Subject: [PATCH 5/5] Minor comment fix --- specs/merge/fork-choice.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index eec084f62..e1e12ad7a 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -111,14 +111,15 @@ def validate_merge_block(block: BeaconBlock) -> None: """ Check the parent PoW block of execution payload is a valid terminal PoW block. - Note: Unavailable PoW block(s) may later become available and a client software MAY delay - a call to ``validate_merge_block`` until the PoW block(s) become available. + Note: Unavailable PoW block(s) may later become available, + and a client software MAY delay a call to ``validate_merge_block`` + until the PoW block(s) become available. """ pow_block = get_pow_block(block.body.execution_payload.parent_hash) - # Check if `pow_block` is unavailable + # Check if `pow_block` is available assert pow_block is not None pow_parent = get_pow_block(pow_block.parent_hash) - # Check if `pow_parent` is unavailable + # Check if `pow_parent` is available assert pow_parent is not None # Check if `pow_block` is a valid terminal PoW block assert is_valid_terminal_pow_block(pow_block, pow_parent)