From c9d7b8533c05aded559acf2208ec74f864b4d76c Mon Sep 17 00:00:00 2001 From: terence tsao Date: Wed, 3 Nov 2021 20:13:18 -0700 Subject: [PATCH 01/10] Use correct payload block hash --- 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 bf1634de4..a013f426b 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -115,7 +115,7 @@ def validate_merge_block(block: BeaconBlock) -> None: if TERMINAL_BLOCK_HASH != Hash32(): # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH - return block.block_hash == TERMINAL_BLOCK_HASH + return block.body.execution_payload.parent_hash == TERMINAL_BLOCK_HASH pow_block = get_pow_block(block.body.execution_payload.parent_hash) # Check if `pow_block` is available From c144844279ce9d0065d00f0b234e6b321b1d48fb Mon Sep 17 00:00:00 2001 From: terence tsao Date: Wed, 3 Nov 2021 20:31:31 -0700 Subject: [PATCH 02/10] Lint fails, it should be assert? --- 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 a013f426b..11ddd5ac3 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -115,7 +115,7 @@ def validate_merge_block(block: BeaconBlock) -> None: if TERMINAL_BLOCK_HASH != Hash32(): # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH - return block.body.execution_payload.parent_hash == TERMINAL_BLOCK_HASH + assert block.body.execution_payload.parent_hash == TERMINAL_BLOCK_HASH pow_block = get_pow_block(block.body.execution_payload.parent_hash) # Check if `pow_block` is available From 0ad344bf7291d7f3528e1e78318788a48ae087da Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Thu, 4 Nov 2021 15:58:48 +0600 Subject: [PATCH 03/10] Make notify_forkchoice_updated return payload id value --- setup.py | 2 +- specs/merge/fork-choice.md | 10 ++++++++-- specs/merge/validator.md | 27 +-------------------------- 3 files changed, 10 insertions(+), 29 deletions(-) diff --git a/setup.py b/setup.py index 4bb26bccd..355f6e2c1 100644 --- a/setup.py +++ b/setup.py @@ -530,7 +530,7 @@ class NoopExecutionEngine(ExecutionEngine): def notify_forkchoice_updated(self: ExecutionEngine, head_block_hash: Hash32, finalized_block_hash: Hash32, - payload_attributes: Optional[PayloadAttributes]) -> None: + payload_attributes: Optional[PayloadAttributes]) -> Optional[PayloadId]: pass def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> ExecutionPayload: diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index bf1634de4..d4d437bf1 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -29,6 +29,12 @@ This is the modification of the fork choice according to the executable beacon c *Note*: It introduces the process of transition from the last PoW block to the first PoS block. +## Custom types + +| Name | SSZ equivalent | Description | +| - | - | - | +| `PayloadId` | `Bytes8` | Identifier of a payload building process | + ## Protocols ### `ExecutionEngine` @@ -46,13 +52,13 @@ This function performs two actions *atomically*: and corresponding state, up to and including `finalized_block_hash`. Additionally, if `payload_attributes` is provided, this function sets in motion a payload build process on top of -`head_block_hash` with the result to be gathered by a followup call to `get_payload`. +`head_block_hash` and returns an identifier of initiated process. ```python def notify_forkchoice_updated(self: ExecutionEngine, head_block_hash: Hash32, finalized_block_hash: Hash32, - payload_attributes: Optional[PayloadAttributes]) -> None: + payload_attributes: Optional[PayloadAttributes]) -> Optional[PayloadId]: ... ``` diff --git a/specs/merge/validator.md b/specs/merge/validator.md index f47ede4f7..61a60bcd8 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -38,12 +38,6 @@ All behaviors and definitions defined in this document, and documents it extends All terminology, constants, functions, and protocol mechanics defined in the updated Beacon Chain doc of [The Merge](./beacon-chain.md) are requisite for this document and used throughout. Please see related Beacon Chain doc before continuing and use them as a reference throughout. -## Custom types - -| Name | SSZ equivalent | Description | -| - | - | - | -| `PayloadId` | `Bytes8` | Identifier of a payload building process | - ## Helpers ### `get_pow_block_at_terminal_total_difficulty` @@ -75,24 +69,6 @@ def get_terminal_pow_block(pow_chain: Dict[Hash32, PowBlock]) -> Optional[PowBlo return get_pow_block_at_terminal_total_difficulty(pow_chain) ``` -### `get_payload_id` - -Given the `head_block_hash` and the `payload_attributes` that were used to -initiate the build process via `notify_forkchoice_updated`, `get_payload_id()` -returns the `payload_id` used to retrieve the payload via `get_payload`. - -```python -def get_payload_id(parent_hash: Hash32, payload_attributes: PayloadAttributes) -> PayloadId: - return PayloadId( - hash( - parent_hash - + uint_to_bytes(payload_attributes.timestamp) - + payload_attributes.random - + payload_attributes.fee_recipient - )[0:8] - ) -``` - *Note*: This function does *not* use simple serialize `hash_tree_root` as to avoid requiring simple serialize hashing capabilities in the Execution Layer. @@ -168,8 +144,7 @@ def prepare_execution_payload(state: BeaconState, random=get_randao_mix(state, get_current_epoch(state)), fee_recipient=fee_recipient, ) - execution_engine.notify_forkchoice_updated(parent_hash, finalized_block_hash, payload_attributes) - return get_payload_id(parent_hash, payload_attributes) + return execution_engine.notify_forkchoice_updated(parent_hash, finalized_block_hash, payload_attributes) ``` 2. Set `block.body.execution_payload = get_execution_payload(payload_id, execution_engine)`, where: From a3380db5976866f701abb83b4da89a359589de45 Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Thu, 4 Nov 2021 16:30:23 +0600 Subject: [PATCH 04/10] Fix toc --- specs/merge/fork-choice.md | 1 + specs/merge/validator.md | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index d4d437bf1..9d4f84d7f 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -8,6 +8,7 @@ - [Introduction](#introduction) +- [Custom types](#custom-types) - [Protocols](#protocols) - [`ExecutionEngine`](#executionengine) - [`notify_forkchoice_updated`](#notify_forkchoice_updated) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 61a60bcd8..1ff55ad1b 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -10,11 +10,9 @@ - [Introduction](#introduction) - [Prerequisites](#prerequisites) -- [Custom types](#custom-types) - [Helpers](#helpers) - [`get_pow_block_at_terminal_total_difficulty`](#get_pow_block_at_terminal_total_difficulty) - [`get_terminal_pow_block`](#get_terminal_pow_block) - - [`get_payload_id`](#get_payload_id) - [Protocols](#protocols) - [`ExecutionEngine`](#executionengine) - [`get_payload`](#get_payload) From 0ae9a10123560465823688d92223834b93d1ea6d Mon Sep 17 00:00:00 2001 From: terence tsao Date: Thu, 4 Nov 2021 06:59:13 -0700 Subject: [PATCH 05/10] Proper return after second assertion --- specs/merge/fork-choice.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 11ddd5ac3..9fd16089c 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -116,6 +116,7 @@ def validate_merge_block(block: BeaconBlock) -> None: # If `TERMINAL_BLOCK_HASH` is used as an override, the activation epoch must be reached. assert compute_epoch_at_slot(block.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH assert block.body.execution_payload.parent_hash == TERMINAL_BLOCK_HASH + return pow_block = get_pow_block(block.body.execution_payload.parent_hash) # Check if `pow_block` is available From c6381345e2c277b81708edace2fccf0087eac6ff Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Fri, 5 Nov 2021 15:00:39 +0600 Subject: [PATCH 06/10] Fixes TBH activation epoch check --- specs/merge/validator.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 1ff55ad1b..d36397353 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -121,8 +121,8 @@ def prepare_execution_payload(state: BeaconState, execution_engine: ExecutionEngine) -> Optional[PayloadId]: if not is_merge_complete(state): is_terminal_block_hash_set = TERMINAL_BLOCK_HASH != Hash32() - is_activation_epoch_reached = get_current_epoch(state.slot) < TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH - if is_terminal_block_hash_set and is_activation_epoch_reached: + is_activation_epoch_reached = get_current_epoch(state.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH + if is_terminal_block_hash_set and not is_activation_epoch_reached: # Terminal block hash is set but activation epoch is not yet reached, no prepare payload call is needed return None From 56bb393eb56449dd3c7b0115f1911d8ef5f0efc0 Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Fri, 5 Nov 2021 19:52:21 +0600 Subject: [PATCH 07/10] Refine validate_merge_block unit tests --- .../eth2spec/test/helpers/fork_choice.py | 11 -- .../pyspec/eth2spec/test/helpers/pow_block.py | 20 +++ .../merge/fork_choice/test_on_merge_block.py | 4 +- .../test_is_valid_terminal_pow_block.py | 44 +++++ .../merge/unittests/test_terminal_validity.py | 143 ---------------- .../unittests/test_validate_merge_block.py | 153 ++++++++++++++++++ 6 files changed, 220 insertions(+), 155 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/helpers/pow_block.py create mode 100644 tests/core/pyspec/eth2spec/test/merge/unittests/test_is_valid_terminal_pow_block.py delete mode 100644 tests/core/pyspec/eth2spec/test/merge/unittests/test_terminal_validity.py create mode 100644 tests/core/pyspec/eth2spec/test/merge/unittests/test_validate_merge_block.py diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index 04843078f..7c6ac89a5 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -1,7 +1,5 @@ -from random import Random from eth_utils import encode_hex from eth2spec.test.exceptions import BlockNotFoundException -from eth2spec.utils.ssz.ssz_typing import uint256 from eth2spec.test.helpers.attestations import ( next_epoch_with_attestations, next_slots_with_attestations, @@ -247,15 +245,6 @@ def apply_next_slots_with_attestations(spec, return post_state, store, last_signed_block -def prepare_empty_pow_block(spec, rng=Random(3131)): - return spec.PowBlock( - block_hash=spec.Hash32(spec.hash(bytearray(rng.getrandbits(8) for _ in range(32)))), - parent_hash=spec.Hash32(spec.hash(bytearray(rng.getrandbits(8) for _ in range(32)))), - total_difficulty=uint256(0), - difficulty=uint256(0) - ) - - def get_pow_block_file_name(pow_block): return f"pow_block_{encode_hex(pow_block.block_hash)}" diff --git a/tests/core/pyspec/eth2spec/test/helpers/pow_block.py b/tests/core/pyspec/eth2spec/test/helpers/pow_block.py new file mode 100644 index 000000000..ed71f531c --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/helpers/pow_block.py @@ -0,0 +1,20 @@ +from random import Random +from eth2spec.utils.ssz.ssz_typing import uint256 + + +def prepare_empty_pow_block(spec, rng=Random(3131)): + return spec.PowBlock( + block_hash=spec.Hash32(spec.hash(bytearray(rng.getrandbits(8) for _ in range(32)))), + parent_hash=spec.Hash32(spec.hash(bytearray(rng.getrandbits(8) for _ in range(32)))), + total_difficulty=uint256(0), + difficulty=uint256(0) + ) + + +def prepare_random_pow_chain(spec, length, rng=Random(3131)): + assert length > 0 + chain = [prepare_empty_pow_block(spec, rng)] + for i in range(1, length): + chain.append(prepare_empty_pow_block(spec, rng)) + chain[i].parent_hash = chain[i - 1].block_hash + return chain diff --git a/tests/core/pyspec/eth2spec/test/merge/fork_choice/test_on_merge_block.py b/tests/core/pyspec/eth2spec/test/merge/fork_choice/test_on_merge_block.py index 830f20f23..ab4cdbfa2 100644 --- a/tests/core/pyspec/eth2spec/test/merge/fork_choice/test_on_merge_block.py +++ b/tests/core/pyspec/eth2spec/test/merge/fork_choice/test_on_merge_block.py @@ -13,9 +13,11 @@ from eth2spec.test.helpers.state import ( state_transition_and_sign_block, ) from eth2spec.test.helpers.fork_choice import ( - prepare_empty_pow_block, add_pow_block, ) +from eth2spec.test.helpers.pow_block import ( + prepare_empty_pow_block, +) from eth2spec.test.helpers.execution_payload import ( build_state_with_incomplete_transition, ) diff --git a/tests/core/pyspec/eth2spec/test/merge/unittests/test_is_valid_terminal_pow_block.py b/tests/core/pyspec/eth2spec/test/merge/unittests/test_is_valid_terminal_pow_block.py new file mode 100644 index 000000000..984431ce4 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/merge/unittests/test_is_valid_terminal_pow_block.py @@ -0,0 +1,44 @@ +from eth2spec.utils.ssz.ssz_typing import uint256 +from eth2spec.test.helpers.pow_block import ( + prepare_empty_pow_block, +) +from eth2spec.test.context import ( + spec_state_test, + with_merge_and_later, +) + + +@with_merge_and_later +@spec_state_test +def test_is_valid_terminal_pow_block_success_valid(spec, state): + parent_block = prepare_empty_pow_block(spec) + parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + block = prepare_empty_pow_block(spec) + block.parent_hash = parent_block.block_hash + block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + + assert spec.is_valid_terminal_pow_block(block, parent_block) + + +@with_merge_and_later +@spec_state_test +def test_is_valid_terminal_pow_block_fail_before_terminal(spec, state): + parent_block = prepare_empty_pow_block(spec) + parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(2) + block = prepare_empty_pow_block(spec) + block.parent_hash = parent_block.block_hash + block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + + assert not spec.is_valid_terminal_pow_block(block, parent_block) + + +@with_merge_and_later +@spec_state_test +def test_is_valid_terminal_pow_block_fail_just_after_terminal(spec, state): + parent_block = prepare_empty_pow_block(spec) + parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + block = prepare_empty_pow_block(spec) + block.parent_hash = parent_block.block_hash + block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + uint256(1) + + assert not spec.is_valid_terminal_pow_block(block, parent_block) diff --git a/tests/core/pyspec/eth2spec/test/merge/unittests/test_terminal_validity.py b/tests/core/pyspec/eth2spec/test/merge/unittests/test_terminal_validity.py deleted file mode 100644 index cfd5ea091..000000000 --- a/tests/core/pyspec/eth2spec/test/merge/unittests/test_terminal_validity.py +++ /dev/null @@ -1,143 +0,0 @@ -from eth2spec.test.exceptions import BlockNotFoundException -from eth2spec.utils.ssz.ssz_typing import uint256 -from eth2spec.test.helpers.fork_choice import ( - prepare_empty_pow_block, -) -from eth2spec.test.context import spec_state_test, with_merge_and_later - - -# Copy of conditional merge part of `on_block(store: Store, signed_block: SignedBeaconBlock)` handler -def validate_transition_execution_payload(spec, execution_payload): - pow_block = spec.get_pow_block(execution_payload.parent_hash) - pow_parent = spec.get_pow_block(pow_block.parent_hash) - assert spec.is_valid_terminal_pow_block(pow_block, pow_parent) - - -def run_validate_transition_execution_payload(spec, block, parent_block, payload, - valid=True, block_lookup_success=True): - """ - Run ``validate_transition_execution_payload`` - If ``valid == False``, run expecting ``AssertionError`` - If ``block_lookup_success == False``, run expecting ``BlockNotFoundException`` - """ - - def get_pow_block(hash: spec.Bytes32) -> spec.PowBlock: - if hash == block.block_hash: - return block - elif hash == parent_block.block_hash: - return parent_block - else: - raise BlockNotFoundException() - save_pow_block = spec.get_pow_block - - # Guido authorized everyone to do this - spec.get_pow_block = get_pow_block - exception_caught = False - block_not_found_exception_caught = False - try: - validate_transition_execution_payload(spec, payload) - except BlockNotFoundException: - block_not_found_exception_caught = True - except AssertionError: - exception_caught = True - except Exception as e: - spec.get_pow_block = save_pow_block - raise e - spec.get_pow_block = save_pow_block - - if block_lookup_success: - assert not block_not_found_exception_caught - else: - assert block_not_found_exception_caught - if valid: - assert not exception_caught - else: - assert exception_caught - - -@with_merge_and_later -@spec_state_test -def test_valid_terminal_pow_block_success_valid(spec, state): - parent_block = prepare_empty_pow_block(spec) - parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - block = prepare_empty_pow_block(spec) - block.parent_hash = parent_block.block_hash - block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - - assert spec.is_valid_terminal_pow_block(block, parent_block) - - -@with_merge_and_later -@spec_state_test -def test_valid_terminal_pow_block_fail_before_terminal(spec, state): - parent_block = prepare_empty_pow_block(spec) - parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(2) - block = prepare_empty_pow_block(spec) - block.parent_hash = parent_block.block_hash - block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - - assert not spec.is_valid_terminal_pow_block(block, parent_block) - - -@with_merge_and_later -@spec_state_test -def test_valid_terminal_pow_block_fail_just_after_terminal(spec, state): - parent_block = prepare_empty_pow_block(spec) - parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - block = prepare_empty_pow_block(spec) - block.parent_hash = parent_block.block_hash - block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + uint256(1) - - assert not spec.is_valid_terminal_pow_block(block, parent_block) - - -@with_merge_and_later -@spec_state_test -def test_validate_transition_execution_payload_success(spec, state): - parent_block = prepare_empty_pow_block(spec) - parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - block = prepare_empty_pow_block(spec) - block.parent_hash = parent_block.block_hash - block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - payload = spec.ExecutionPayload() - payload.parent_hash = block.block_hash - run_validate_transition_execution_payload(spec, block, parent_block, payload) - - -@with_merge_and_later -@spec_state_test -def test_validate_transition_execution_payload_fail_block_lookup(spec, state): - parent_block = prepare_empty_pow_block(spec) - parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - block = prepare_empty_pow_block(spec) - block.parent_hash = parent_block.block_hash - block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - payload = spec.ExecutionPayload() - run_validate_transition_execution_payload(spec, block, parent_block, payload, - block_lookup_success=False) - - -@with_merge_and_later -@spec_state_test -def test_validate_transition_execution_payload_fail_parent_block_lookup(spec, state): - parent_block = prepare_empty_pow_block(spec) - parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - block = prepare_empty_pow_block(spec) - block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - payload = spec.ExecutionPayload() - payload.parent_hash = block.block_hash - run_validate_transition_execution_payload(spec, block, parent_block, payload, - block_lookup_success=False) - - -@with_merge_and_later -@spec_state_test -def test_validate_transition_execution_payload_fail_after_terminal(spec, state): - parent_block = prepare_empty_pow_block(spec) - parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - block = prepare_empty_pow_block(spec) - block.parent_hash = parent_block.block_hash - block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + 1 - payload = spec.ExecutionPayload() - payload.parent_hash = block.block_hash - run_validate_transition_execution_payload(spec, block, parent_block, payload, valid=False) diff --git a/tests/core/pyspec/eth2spec/test/merge/unittests/test_validate_merge_block.py b/tests/core/pyspec/eth2spec/test/merge/unittests/test_validate_merge_block.py new file mode 100644 index 000000000..a19b07bd7 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/merge/unittests/test_validate_merge_block.py @@ -0,0 +1,153 @@ +from typing import Optional +from eth2spec.utils.ssz.ssz_typing import uint256, Bytes32 +from eth2spec.test.helpers.block import ( + build_empty_block_for_next_slot, +) +from eth2spec.test.helpers.pow_block import ( + prepare_random_pow_chain, +) +from eth2spec.test.context import ( + spec_state_test, + with_merge_and_later, + spec_configured_state_test +) + + +TERMINAL_BLOCK_HASH_CONFIG_VAR = '0x0000000000000000000000000000000000000000000000000000000000000001' +TERMINAL_BLOCK_HASH = Bytes32(TERMINAL_BLOCK_HASH_CONFIG_VAR) + + +def run_validate_merge_block(spec, pow_chain, beacon_block, valid=True): + """ + Run ``validate_merge_block`` + If ``valid == False``, run expecting ``AssertionError`` + """ + + def get_pow_block(hash: spec.Bytes32) -> Optional[spec.PowBlock]: + for block in pow_chain: + if block.block_hash == hash: + return block + return None + + get_pow_block_backup = spec.get_pow_block + + # Guido authorized everyone to do this + spec.get_pow_block = get_pow_block + assertion_error_caught = False + try: + spec.validate_merge_block(beacon_block) + except AssertionError: + assertion_error_caught = True + except Exception as e: + spec.get_pow_block = get_pow_block_backup + raise e + spec.get_pow_block = get_pow_block_backup + + if valid: + assert not assertion_error_caught + else: + assert assertion_error_caught + + +@with_merge_and_later +@spec_state_test +def test_validate_merge_block_success(spec, state): + pow_chain = prepare_random_pow_chain(spec, 2) + pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + block = build_empty_block_for_next_slot(spec, state) + block.body.execution_payload.parent_hash = pow_chain[1].block_hash + run_validate_merge_block(spec, pow_chain, block) + + +@with_merge_and_later +@spec_state_test +def test_validate_merge_block_fail_block_lookup(spec, state): + pow_chain = prepare_random_pow_chain(spec, 2) + pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + block = build_empty_block_for_next_slot(spec, state) + run_validate_merge_block(spec, pow_chain, block, valid=False) + + +@with_merge_and_later +@spec_state_test +def test_validate_merge_block_fail_parent_block_lookup(spec, state): + pow_chain = prepare_random_pow_chain(spec, 1) + pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + block = build_empty_block_for_next_slot(spec, state) + block.body.execution_payload.parent_hash = pow_chain[0].block_hash + run_validate_merge_block(spec, pow_chain, block, valid=False) + + +@with_merge_and_later +@spec_state_test +def test_validate_merge_block_fail_after_terminal(spec, state): + pow_chain = prepare_random_pow_chain(spec, 2) + pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + uint256(1) + block = build_empty_block_for_next_slot(spec, state) + block.body.execution_payload.parent_hash = pow_chain[1].block_hash + run_validate_merge_block(spec, pow_chain, block, valid=False) + + +@with_merge_and_later +@spec_configured_state_test({ + 'TERMINAL_BLOCK_HASH': TERMINAL_BLOCK_HASH_CONFIG_VAR, + 'TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH': '0' +}) +def test_validate_merge_block_tbh_override_success(spec, state): + pow_chain = prepare_random_pow_chain(spec, 2) + # should fail if TTD check is reached + pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(2) + pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain[1].block_hash = TERMINAL_BLOCK_HASH + block = build_empty_block_for_next_slot(spec, state) + block.body.execution_payload.parent_hash = pow_chain[1].block_hash + run_validate_merge_block(spec, pow_chain, block) + + +@with_merge_and_later +@spec_configured_state_test({ + 'TERMINAL_BLOCK_HASH': TERMINAL_BLOCK_HASH_CONFIG_VAR, + 'TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH': '0' +}) +def test_validate_merge_block_fail_parent_hash_is_not_tbh(spec, state): + pow_chain = prepare_random_pow_chain(spec, 2) + # shouldn't fail if TTD check is reached + pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + block = build_empty_block_for_next_slot(spec, state) + block.body.execution_payload.parent_hash = pow_chain[1].block_hash + run_validate_merge_block(spec, pow_chain, block, valid=False) + + +@with_merge_and_later +@spec_configured_state_test({ + 'TERMINAL_BLOCK_HASH': TERMINAL_BLOCK_HASH_CONFIG_VAR, + 'TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH': '1' +}) +def test_validate_merge_block_terminal_block_hash_fail_activation_not_reached(spec, state): + pow_chain = prepare_random_pow_chain(spec, 2) + # shouldn't fail if TTD check is reached + pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain[1].block_hash = TERMINAL_BLOCK_HASH + block = build_empty_block_for_next_slot(spec, state) + block.body.execution_payload.parent_hash = pow_chain[1].block_hash + run_validate_merge_block(spec, pow_chain, block, valid=False) + + +@with_merge_and_later +@spec_configured_state_test({ + 'TERMINAL_BLOCK_HASH': TERMINAL_BLOCK_HASH_CONFIG_VAR, + 'TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH': '1' +}) +def test_validate_merge_block_fail_activation_not_reached_parent_hash_is_not_tbh(spec, state): + pow_chain = prepare_random_pow_chain(spec, 2) + # shouldn't fail if TTD check is reached + pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + block = build_empty_block_for_next_slot(spec, state) + block.body.execution_payload.parent_hash = pow_chain[1].block_hash + run_validate_merge_block(spec, pow_chain, block, valid=False) From f463c328798e936a900c9ca53c40afbf4ef05b4a Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Sat, 6 Nov 2021 10:21:56 +0600 Subject: [PATCH 08/10] Apply feedback from the review of test_validate_merge_block --- .../pyspec/eth2spec/test/helpers/pow_block.py | 24 ++++++++-- .../merge/fork_choice/test_on_merge_block.py | 16 +++---- .../test_is_valid_terminal_pow_block.py | 14 +++--- .../unittests/test_validate_merge_block.py | 48 +++++++++---------- 4 files changed, 58 insertions(+), 44 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/helpers/pow_block.py b/tests/core/pyspec/eth2spec/test/helpers/pow_block.py index ed71f531c..58989a420 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/pow_block.py +++ b/tests/core/pyspec/eth2spec/test/helpers/pow_block.py @@ -2,7 +2,21 @@ from random import Random from eth2spec.utils.ssz.ssz_typing import uint256 -def prepare_empty_pow_block(spec, rng=Random(3131)): +class PowChain: + blocks = [] + + def __init__(self, blocks): + self.blocks = blocks + + def __iter__(self): + return iter(self.blocks) + + def head(self, offset=0): + assert offset <= 0 + return self.blocks[offset - 1] + + +def prepare_random_pow_block(spec, rng=Random(3131)): return spec.PowBlock( block_hash=spec.Hash32(spec.hash(bytearray(rng.getrandbits(8) for _ in range(32)))), parent_hash=spec.Hash32(spec.hash(bytearray(rng.getrandbits(8) for _ in range(32)))), @@ -11,10 +25,10 @@ def prepare_empty_pow_block(spec, rng=Random(3131)): ) -def prepare_random_pow_chain(spec, length, rng=Random(3131)): +def prepare_random_pow_chain(spec, length, rng=Random(3131)) -> PowChain: assert length > 0 - chain = [prepare_empty_pow_block(spec, rng)] + chain = [prepare_random_pow_block(spec, rng)] for i in range(1, length): - chain.append(prepare_empty_pow_block(spec, rng)) + chain.append(prepare_random_pow_block(spec, rng)) chain[i].parent_hash = chain[i - 1].block_hash - return chain + return PowChain(chain) diff --git a/tests/core/pyspec/eth2spec/test/merge/fork_choice/test_on_merge_block.py b/tests/core/pyspec/eth2spec/test/merge/fork_choice/test_on_merge_block.py index ab4cdbfa2..e0703fdf7 100644 --- a/tests/core/pyspec/eth2spec/test/merge/fork_choice/test_on_merge_block.py +++ b/tests/core/pyspec/eth2spec/test/merge/fork_choice/test_on_merge_block.py @@ -16,7 +16,7 @@ from eth2spec.test.helpers.fork_choice import ( add_pow_block, ) from eth2spec.test.helpers.pow_block import ( - prepare_empty_pow_block, + prepare_random_pow_block, ) from eth2spec.test.helpers.execution_payload import ( build_state_with_incomplete_transition, @@ -60,9 +60,9 @@ def test_all_valid(spec, state): on_tick_and_append_step(spec, store, current_time, test_steps) assert store.time == current_time - pow_block_parent = prepare_empty_pow_block(spec) + pow_block_parent = prepare_random_pow_block(spec) pow_block_parent.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - pow_block = prepare_empty_pow_block(spec) + pow_block = prepare_random_pow_block(spec) pow_block.parent_hash = pow_block_parent.block_hash pow_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY pow_blocks = [pow_block, pow_block_parent] @@ -94,7 +94,7 @@ def test_block_lookup_failed(spec, state): on_tick_and_append_step(spec, store, current_time, test_steps) assert store.time == current_time - pow_block = prepare_empty_pow_block(spec) + pow_block = prepare_random_pow_block(spec) pow_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) pow_blocks = [pow_block] for pb in pow_blocks: @@ -124,9 +124,9 @@ def test_too_early_for_merge(spec, state): on_tick_and_append_step(spec, store, current_time, test_steps) assert store.time == current_time - pow_block_parent = prepare_empty_pow_block(spec) + pow_block_parent = prepare_random_pow_block(spec) pow_block_parent.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(2) - pow_block = prepare_empty_pow_block(spec) + pow_block = prepare_random_pow_block(spec) pow_block.parent_hash = pow_block_parent.block_hash pow_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) pow_blocks = [pow_block, pow_block_parent] @@ -156,9 +156,9 @@ def test_too_late_for_merge(spec, state): on_tick_and_append_step(spec, store, current_time, test_steps) assert store.time == current_time - pow_block_parent = prepare_empty_pow_block(spec) + pow_block_parent = prepare_random_pow_block(spec) pow_block_parent.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - pow_block = prepare_empty_pow_block(spec) + pow_block = prepare_random_pow_block(spec) pow_block.parent_hash = pow_block_parent.block_hash pow_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + uint256(1) pow_blocks = [pow_block, pow_block_parent] diff --git a/tests/core/pyspec/eth2spec/test/merge/unittests/test_is_valid_terminal_pow_block.py b/tests/core/pyspec/eth2spec/test/merge/unittests/test_is_valid_terminal_pow_block.py index 984431ce4..f20c15e35 100644 --- a/tests/core/pyspec/eth2spec/test/merge/unittests/test_is_valid_terminal_pow_block.py +++ b/tests/core/pyspec/eth2spec/test/merge/unittests/test_is_valid_terminal_pow_block.py @@ -1,6 +1,6 @@ from eth2spec.utils.ssz.ssz_typing import uint256 from eth2spec.test.helpers.pow_block import ( - prepare_empty_pow_block, + prepare_random_pow_block, ) from eth2spec.test.context import ( spec_state_test, @@ -11,9 +11,9 @@ from eth2spec.test.context import ( @with_merge_and_later @spec_state_test def test_is_valid_terminal_pow_block_success_valid(spec, state): - parent_block = prepare_empty_pow_block(spec) + parent_block = prepare_random_pow_block(spec) parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - block = prepare_empty_pow_block(spec) + block = prepare_random_pow_block(spec) block.parent_hash = parent_block.block_hash block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY @@ -23,9 +23,9 @@ def test_is_valid_terminal_pow_block_success_valid(spec, state): @with_merge_and_later @spec_state_test def test_is_valid_terminal_pow_block_fail_before_terminal(spec, state): - parent_block = prepare_empty_pow_block(spec) + parent_block = prepare_random_pow_block(spec) parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(2) - block = prepare_empty_pow_block(spec) + block = prepare_random_pow_block(spec) block.parent_hash = parent_block.block_hash block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) @@ -35,9 +35,9 @@ def test_is_valid_terminal_pow_block_fail_before_terminal(spec, state): @with_merge_and_later @spec_state_test def test_is_valid_terminal_pow_block_fail_just_after_terminal(spec, state): - parent_block = prepare_empty_pow_block(spec) + parent_block = prepare_random_pow_block(spec) parent_block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - block = prepare_empty_pow_block(spec) + block = prepare_random_pow_block(spec) block.parent_hash = parent_block.block_hash block.total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + uint256(1) diff --git a/tests/core/pyspec/eth2spec/test/merge/unittests/test_validate_merge_block.py b/tests/core/pyspec/eth2spec/test/merge/unittests/test_validate_merge_block.py index a19b07bd7..cf4c2234a 100644 --- a/tests/core/pyspec/eth2spec/test/merge/unittests/test_validate_merge_block.py +++ b/tests/core/pyspec/eth2spec/test/merge/unittests/test_validate_merge_block.py @@ -53,10 +53,10 @@ def run_validate_merge_block(spec, pow_chain, beacon_block, valid=True): @spec_state_test def test_validate_merge_block_success(spec, state): pow_chain = prepare_random_pow_chain(spec, 2) - pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY block = build_empty_block_for_next_slot(spec, state) - block.body.execution_payload.parent_hash = pow_chain[1].block_hash + block.body.execution_payload.parent_hash = pow_chain.head().block_hash run_validate_merge_block(spec, pow_chain, block) @@ -64,8 +64,8 @@ def test_validate_merge_block_success(spec, state): @spec_state_test def test_validate_merge_block_fail_block_lookup(spec, state): pow_chain = prepare_random_pow_chain(spec, 2) - pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY block = build_empty_block_for_next_slot(spec, state) run_validate_merge_block(spec, pow_chain, block, valid=False) @@ -74,9 +74,9 @@ def test_validate_merge_block_fail_block_lookup(spec, state): @spec_state_test def test_validate_merge_block_fail_parent_block_lookup(spec, state): pow_chain = prepare_random_pow_chain(spec, 1) - pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY block = build_empty_block_for_next_slot(spec, state) - block.body.execution_payload.parent_hash = pow_chain[0].block_hash + block.body.execution_payload.parent_hash = pow_chain.head().block_hash run_validate_merge_block(spec, pow_chain, block, valid=False) @@ -84,10 +84,10 @@ def test_validate_merge_block_fail_parent_block_lookup(spec, state): @spec_state_test def test_validate_merge_block_fail_after_terminal(spec, state): pow_chain = prepare_random_pow_chain(spec, 2) - pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + uint256(1) + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + uint256(1) block = build_empty_block_for_next_slot(spec, state) - block.body.execution_payload.parent_hash = pow_chain[1].block_hash + block.body.execution_payload.parent_hash = pow_chain.head().block_hash run_validate_merge_block(spec, pow_chain, block, valid=False) @@ -99,11 +99,11 @@ def test_validate_merge_block_fail_after_terminal(spec, state): def test_validate_merge_block_tbh_override_success(spec, state): pow_chain = prepare_random_pow_chain(spec, 2) # should fail if TTD check is reached - pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(2) - pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - pow_chain[1].block_hash = TERMINAL_BLOCK_HASH + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(2) + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain.head().block_hash = TERMINAL_BLOCK_HASH block = build_empty_block_for_next_slot(spec, state) - block.body.execution_payload.parent_hash = pow_chain[1].block_hash + block.body.execution_payload.parent_hash = pow_chain.head().block_hash run_validate_merge_block(spec, pow_chain, block) @@ -115,10 +115,10 @@ def test_validate_merge_block_tbh_override_success(spec, state): def test_validate_merge_block_fail_parent_hash_is_not_tbh(spec, state): pow_chain = prepare_random_pow_chain(spec, 2) # shouldn't fail if TTD check is reached - pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY block = build_empty_block_for_next_slot(spec, state) - block.body.execution_payload.parent_hash = pow_chain[1].block_hash + block.body.execution_payload.parent_hash = pow_chain.head().block_hash run_validate_merge_block(spec, pow_chain, block, valid=False) @@ -130,11 +130,11 @@ def test_validate_merge_block_fail_parent_hash_is_not_tbh(spec, state): def test_validate_merge_block_terminal_block_hash_fail_activation_not_reached(spec, state): pow_chain = prepare_random_pow_chain(spec, 2) # shouldn't fail if TTD check is reached - pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - pow_chain[1].block_hash = TERMINAL_BLOCK_HASH + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain.head().block_hash = TERMINAL_BLOCK_HASH block = build_empty_block_for_next_slot(spec, state) - block.body.execution_payload.parent_hash = pow_chain[1].block_hash + block.body.execution_payload.parent_hash = pow_chain.head().block_hash run_validate_merge_block(spec, pow_chain, block, valid=False) @@ -146,8 +146,8 @@ def test_validate_merge_block_terminal_block_hash_fail_activation_not_reached(sp def test_validate_merge_block_fail_activation_not_reached_parent_hash_is_not_tbh(spec, state): pow_chain = prepare_random_pow_chain(spec, 2) # shouldn't fail if TTD check is reached - pow_chain[0].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) - pow_chain[1].total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - uint256(1) + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY block = build_empty_block_for_next_slot(spec, state) - block.body.execution_payload.parent_hash = pow_chain[1].block_hash + block.body.execution_payload.parent_hash = pow_chain.head().block_hash run_validate_merge_block(spec, pow_chain, block, valid=False) From 46f7845c422554f3548532f0382de1e732ef386d Mon Sep 17 00:00:00 2001 From: Fredrik Svantes Date: Sat, 6 Nov 2021 16:28:13 +0100 Subject: [PATCH 09/10] Fixed link to Attesting in beacon chain responsibilities --- specs/phase0/validator.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/phase0/validator.md b/specs/phase0/validator.md index 8de16e805..cc5b7aeb5 100644 --- a/specs/phase0/validator.md +++ b/specs/phase0/validator.md @@ -274,7 +274,7 @@ Specifically a validator should: ## Beacon chain responsibilities -A validator has two primary responsibilities to the beacon chain: [proposing blocks](#block-proposal) and [creating attestations](#attestations-1). Proposals happen infrequently, whereas attestations should be created once per epoch. +A validator has two primary responsibilities to the beacon chain: [proposing blocks](#block-proposal) and [creating attestations](#attesting). Proposals happen infrequently, whereas attestations should be created once per epoch. ### Block proposal From 342171f4419827ab9a75f8ef4dfd1b5d571f613d Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 8 Nov 2021 07:50:54 -0700 Subject: [PATCH 10/10] version.txt bump to v1.1.5 --- tests/core/pyspec/eth2spec/VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/VERSION.txt b/tests/core/pyspec/eth2spec/VERSION.txt index 1b87bcd0b..314c3d717 100644 --- a/tests/core/pyspec/eth2spec/VERSION.txt +++ b/tests/core/pyspec/eth2spec/VERSION.txt @@ -1 +1 @@ -1.1.4 \ No newline at end of file +1.1.5 \ No newline at end of file