From f463c328798e936a900c9ca53c40afbf4ef05b4a Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Sat, 6 Nov 2021 10:21:56 +0600 Subject: [PATCH] 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)