From fb7cbb7b9b64d1d9cd14e1b0f0d1c1c815037c21 Mon Sep 17 00:00:00 2001 From: Mitchell Catoen Date: Mon, 23 May 2022 11:19:19 -0400 Subject: [PATCH 1/8] Adding test case --- eth/_utils/headers.py | 3 ++- tests/core/vm/test_vm.py | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/eth/_utils/headers.py b/eth/_utils/headers.py index 98b8cb94bb..f5dbc8a64b 100644 --- a/eth/_utils/headers.py +++ b/eth/_utils/headers.py @@ -57,6 +57,7 @@ def fill_header_params_from_parent( gas_limit: int, difficulty: int, timestamp: int, + block_number: int = GENESIS_BLOCK_NUMBER, coinbase: Address = ZERO_ADDRESS, nonce: bytes = None, extra_data: bytes = None, @@ -67,7 +68,7 @@ def fill_header_params_from_parent( if parent is None: parent_hash = GENESIS_PARENT_HASH - block_number = GENESIS_BLOCK_NUMBER + block_number = block_number if state_root is None: state_root = BLANK_ROOT_HASH else: diff --git a/tests/core/vm/test_vm.py b/tests/core/vm/test_vm.py index 951c557fc6..216e3ec04d 100644 --- a/tests/core/vm/test_vm.py +++ b/tests/core/vm/test_vm.py @@ -15,6 +15,8 @@ from eth.tools.factories.transaction import ( new_transaction ) +from eth_typing import BlockNumber +from eth._utils.headers import fill_header_params_from_parent @pytest.fixture(params=MAINNET_VMS) @@ -191,3 +193,23 @@ def test_validate_gas_limit_too_high(noproof_consensus_chain): with pytest.raises(ValidationError, match="[Gg]as limit"): vm.validate_header(invalid_header, block1.header) + + +def test_fill_header_params_from_parent(noproof_consensus_chain): + block = noproof_consensus_chain.mine_block() + new_fields = { + 'gas_limit': 1, + 'difficulty': 10, + 'timestamp': 100, + 'block_number': 1000 + } + + new_header_params = fill_header_params_from_parent(block.header, + **new_fields) + # Make sure default is set to genesis block number + if block.header is None: + new_fields['block_number'] = constants.GENESIS_BLOCK_NUMBER + else: + new_fields['block_number'] = BlockNumber(block.header.block_number + 1) + for field in new_fields: + assert new_header_params[field] == new_fields[field] From cbf02f8066e8b102837e6098c7b73340e01bc5c5 Mon Sep 17 00:00:00 2001 From: Mitchell Catoen Date: Mon, 23 May 2022 11:41:10 -0400 Subject: [PATCH 2/8] Proper unit test --- eth/_utils/headers.py | 27 ++++++++++++---------- tests/core/vm/test_vm.py | 48 ++++++++++++++++++++++++++-------------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/eth/_utils/headers.py b/eth/_utils/headers.py index f5dbc8a64b..83d7e61ebb 100644 --- a/eth/_utils/headers.py +++ b/eth/_utils/headers.py @@ -6,6 +6,7 @@ from eth_typing import ( Address, + Hash32, ) from eth.abc import BlockHeaderAPI @@ -57,7 +58,8 @@ def fill_header_params_from_parent( gas_limit: int, difficulty: int, timestamp: int, - block_number: int = GENESIS_BLOCK_NUMBER, + parent_hash: Hash32 = None, + block_number: int = None, coinbase: Address = ZERO_ADDRESS, nonce: bytes = None, extra_data: bytes = None, @@ -66,17 +68,18 @@ def fill_header_params_from_parent( mix_hash: bytes = None, receipt_root: bytes = None) -> Dict[str, HeaderParams]: - if parent is None: - parent_hash = GENESIS_PARENT_HASH - block_number = block_number - if state_root is None: - state_root = BLANK_ROOT_HASH - else: - parent_hash = parent.hash - block_number = BlockNumber(parent.block_number + 1) - - if state_root is None: - state_root = parent.state_root + if block_number is None: + if parent is None: + parent_hash = GENESIS_PARENT_HASH + block_number = GENESIS_BLOCK_NUMBER + if state_root is None: + state_root = BLANK_ROOT_HASH + else: + parent_hash = parent.hash + block_number = BlockNumber(parent.block_number + 1) + + if state_root is None: + state_root = parent.state_root header_kwargs: Dict[str, HeaderParams] = { 'parent_hash': parent_hash, diff --git a/tests/core/vm/test_vm.py b/tests/core/vm/test_vm.py index 216e3ec04d..b20af3f4c8 100644 --- a/tests/core/vm/test_vm.py +++ b/tests/core/vm/test_vm.py @@ -15,7 +15,7 @@ from eth.tools.factories.transaction import ( new_transaction ) -from eth_typing import BlockNumber +from eth_typing import BlockNumber, Hash32 from eth._utils.headers import fill_header_params_from_parent @@ -195,21 +195,35 @@ def test_validate_gas_limit_too_high(noproof_consensus_chain): vm.validate_header(invalid_header, block1.header) -def test_fill_header_params_from_parent(noproof_consensus_chain): +@pytest.mark.parametrize( + "custom_header_params", + ( + { + 'gas_limit': 1, + 'difficulty': 10, + 'timestamp': 100, + }, + { + 'gas_limit': 1, + 'difficulty': 10, + 'timestamp': 100, + 'block_number': 1000, + 'parent_hash': Hash32(100) + } + ) +) +def test_fill_header_params_from_parent(custom_header_params, + noproof_consensus_chain): block = noproof_consensus_chain.mine_block() - new_fields = { - 'gas_limit': 1, - 'difficulty': 10, - 'timestamp': 100, - 'block_number': 1000 - } - new_header_params = fill_header_params_from_parent(block.header, - **new_fields) - # Make sure default is set to genesis block number - if block.header is None: - new_fields['block_number'] = constants.GENESIS_BLOCK_NUMBER - else: - new_fields['block_number'] = BlockNumber(block.header.block_number + 1) - for field in new_fields: - assert new_header_params[field] == new_fields[field] + **custom_header_params) + for field in custom_header_params: + assert new_header_params[field] == custom_header_params[field] + + if 'block_number' not in new_header_params: + if block.header is not None: + assert new_header_params['block_number'] == block.header.block_number + assert new_header_params['parent_hash'] == block.header.hash + else: + assert new_header_params['block_number'] == constants.GENESIS_BLOCK_NUMBER + assert new_header_params['parent_hash'] == constants.GENESIS_PARENT_HASH From 796c4f2790f617ab12d5b39c7f48c5ad01d0ab37 Mon Sep 17 00:00:00 2001 From: Mitchell Catoen Date: Mon, 23 May 2022 11:42:19 -0400 Subject: [PATCH 3/8] Reordering params --- eth/_utils/headers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/_utils/headers.py b/eth/_utils/headers.py index 83d7e61ebb..96437549b4 100644 --- a/eth/_utils/headers.py +++ b/eth/_utils/headers.py @@ -58,8 +58,8 @@ def fill_header_params_from_parent( gas_limit: int, difficulty: int, timestamp: int, - parent_hash: Hash32 = None, block_number: int = None, + parent_hash: Hash32 = None, coinbase: Address = ZERO_ADDRESS, nonce: bytes = None, extra_data: bytes = None, From cf786c96675369105f7cd81f0f520e92558ad8eb Mon Sep 17 00:00:00 2001 From: Mitchell Catoen Date: Mon, 23 May 2022 11:53:03 -0400 Subject: [PATCH 4/8] Linting --- tests/core/vm/test_vm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/vm/test_vm.py b/tests/core/vm/test_vm.py index b20af3f4c8..720db16623 100644 --- a/tests/core/vm/test_vm.py +++ b/tests/core/vm/test_vm.py @@ -15,7 +15,7 @@ from eth.tools.factories.transaction import ( new_transaction ) -from eth_typing import BlockNumber, Hash32 +from eth_typing import Hash32 from eth._utils.headers import fill_header_params_from_parent From 0ca5f68fc6446f321f8e615e0896320004a17969 Mon Sep 17 00:00:00 2001 From: Mitchell Catoen Date: Mon, 23 May 2022 17:43:36 -0400 Subject: [PATCH 5/8] Updating according to issue comments --- eth/_utils/headers.py | 27 +++++++++++++------------ tests/core/vm/test_vm.py | 43 +++++++++++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/eth/_utils/headers.py b/eth/_utils/headers.py index 96437549b4..2e8799fffb 100644 --- a/eth/_utils/headers.py +++ b/eth/_utils/headers.py @@ -22,6 +22,7 @@ GAS_LIMIT_USAGE_ADJUSTMENT_DENOMINATOR, ZERO_ADDRESS, ) +from eth.exceptions import PyEVMError from eth.typing import ( BlockNumber, HeaderParams, @@ -59,7 +60,6 @@ def fill_header_params_from_parent( difficulty: int, timestamp: int, block_number: int = None, - parent_hash: Hash32 = None, coinbase: Address = ZERO_ADDRESS, nonce: bytes = None, extra_data: bytes = None, @@ -68,18 +68,19 @@ def fill_header_params_from_parent( mix_hash: bytes = None, receipt_root: bytes = None) -> Dict[str, HeaderParams]: - if block_number is None: - if parent is None: - parent_hash = GENESIS_PARENT_HASH - block_number = GENESIS_BLOCK_NUMBER - if state_root is None: - state_root = BLANK_ROOT_HASH - else: - parent_hash = parent.hash - block_number = BlockNumber(parent.block_number + 1) - - if state_root is None: - state_root = parent.state_root + if parent is None: + parent_hash = GENESIS_PARENT_HASH + block_number = block_number if block_number else GENESIS_BLOCK_NUMBER + if state_root is None: + state_root = BLANK_ROOT_HASH + else: + parent_hash = parent.hash + if block_number: + raise PyEVMError("block_number cannot be configured if a parent header exists.") + block_number = BlockNumber(parent.block_number + 1) + + if state_root is None: + state_root = parent.state_root header_kwargs: Dict[str, HeaderParams] = { 'parent_hash': parent_hash, diff --git a/tests/core/vm/test_vm.py b/tests/core/vm/test_vm.py index 720db16623..9dfdcfa8e7 100644 --- a/tests/core/vm/test_vm.py +++ b/tests/core/vm/test_vm.py @@ -11,11 +11,12 @@ MiningChain, ) from eth.chains.mainnet import MAINNET_VMS +from eth.exceptions import PyEVMError from eth.tools.builder.chain import api from eth.tools.factories.transaction import ( new_transaction ) -from eth_typing import Hash32 +from eth_typing import BlockNumber, Hash32 from eth._utils.headers import fill_header_params_from_parent @@ -208,22 +209,40 @@ def test_validate_gas_limit_too_high(noproof_consensus_chain): 'difficulty': 10, 'timestamp': 100, 'block_number': 1000, - 'parent_hash': Hash32(100) } ) ) def test_fill_header_params_from_parent(custom_header_params, noproof_consensus_chain): block = noproof_consensus_chain.mine_block() - new_header_params = fill_header_params_from_parent(block.header, - **custom_header_params) - for field in custom_header_params: + + # Cannot specify block number and a parent. + if 'block_number' in custom_header_params and block.header is not None: + with pytest.raises( + PyEVMError, + match="block_number cannot be configured if a parent header exists."): + new_header_params = fill_header_params_from_parent( + block.header, + **custom_header_params) + return + + new_header_params = fill_header_params_from_parent( + block.header, + **custom_header_params) + + # Compare fields which are copied no matter what. + trivial_fields = ['gas_limit', 'difficulty', 'timestamp'] + for field in trivial_fields: assert new_header_params[field] == custom_header_params[field] - if 'block_number' not in new_header_params: - if block.header is not None: - assert new_header_params['block_number'] == block.header.block_number - assert new_header_params['parent_hash'] == block.header.hash - else: - assert new_header_params['block_number'] == constants.GENESIS_BLOCK_NUMBER - assert new_header_params['parent_hash'] == constants.GENESIS_PARENT_HASH + # Check `block_number` and `parent_hash` cases. + if 'block_number' in custom_header_params: + assert new_header_params['block_number'] == \ + custom_header_params['block_number'] + assert new_header_params['block_number'] == \ + constants.GENESIS_BLOCK_NUMBER + assert new_header_params['parent_hash'] == \ + constants.GENESIS_PARENT_HASH + else: + assert new_header_params['block_number'] == \ + BlockNumber(block.header.block_number + 1) From 67ac21205b5e8d793fd4826abdc15fa43b162db7 Mon Sep 17 00:00:00 2001 From: Mitchell Catoen Date: Mon, 23 May 2022 19:31:16 -0400 Subject: [PATCH 6/8] Fixing unit testing --- tests/core/vm/test_vm.py | 50 ++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/core/vm/test_vm.py b/tests/core/vm/test_vm.py index 9dfdcfa8e7..d1d0ecf413 100644 --- a/tests/core/vm/test_vm.py +++ b/tests/core/vm/test_vm.py @@ -197,38 +197,39 @@ def test_validate_gas_limit_too_high(noproof_consensus_chain): @pytest.mark.parametrize( - "custom_header_params", + "custom_header_params,null_parent", ( - { + ({ 'gas_limit': 1, 'difficulty': 10, 'timestamp': 100, - }, - { + 'block_number': 1000 + }, True), + ({ + 'gas_limit': 1, + 'difficulty': 10, + 'timestamp': 100, + }, False), + ({ 'gas_limit': 1, 'difficulty': 10, 'timestamp': 100, 'block_number': 1000, - } + }, False) ) ) -def test_fill_header_params_from_parent(custom_header_params, +def test_fill_header_params_from_parent(custom_header_params, null_parent, noproof_consensus_chain): + # Handle cases which want to specify a null parent. block = noproof_consensus_chain.mine_block() + header = block.header if null_parent is False else None # Cannot specify block number and a parent. - if 'block_number' in custom_header_params and block.header is not None: - with pytest.raises( - PyEVMError, - match="block_number cannot be configured if a parent header exists."): - new_header_params = fill_header_params_from_parent( - block.header, - **custom_header_params) + if 'block_number' in custom_header_params and header is not None: + with pytest.raises(PyEVMError, match="block_number cannot be configured if a parent header exists."): + new_header_params = fill_header_params_from_parent(header, **custom_header_params) return - - new_header_params = fill_header_params_from_parent( - block.header, - **custom_header_params) + new_header_params = fill_header_params_from_parent(header, **custom_header_params) # Compare fields which are copied no matter what. trivial_fields = ['gas_limit', 'difficulty', 'timestamp'] @@ -237,12 +238,11 @@ def test_fill_header_params_from_parent(custom_header_params, # Check `block_number` and `parent_hash` cases. if 'block_number' in custom_header_params: - assert new_header_params['block_number'] == \ - custom_header_params['block_number'] - assert new_header_params['block_number'] == \ - constants.GENESIS_BLOCK_NUMBER - assert new_header_params['parent_hash'] == \ - constants.GENESIS_PARENT_HASH + assert new_header_params['block_number'] == custom_header_params['block_number'] + assert new_header_params['parent_hash'] == constants.GENESIS_PARENT_HASH + elif header is None: + assert new_header_params['block_number'] == constants.GENESIS_BLOCK_NUMBER + assert new_header_params['parent_hash'] == constants.GENESIS_PARENT_HASH else: - assert new_header_params['block_number'] == \ - BlockNumber(block.header.block_number + 1) + assert new_header_params['block_number'] == BlockNumber(header['block_number'] + 1) + assert new_header_params['parent_hash'] == header.hash From 47074a4e440cc6517c40b453cfd88c93d9eab93d Mon Sep 17 00:00:00 2001 From: Mitchell Catoen Date: Mon, 23 May 2022 19:33:58 -0400 Subject: [PATCH 7/8] Linting --- tests/core/vm/test_vm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/vm/test_vm.py b/tests/core/vm/test_vm.py index d1d0ecf413..efee5becf8 100644 --- a/tests/core/vm/test_vm.py +++ b/tests/core/vm/test_vm.py @@ -16,7 +16,7 @@ from eth.tools.factories.transaction import ( new_transaction ) -from eth_typing import BlockNumber, Hash32 +from eth_typing import BlockNumber from eth._utils.headers import fill_header_params_from_parent From afadd3f51573cdee0e4292eb2d3a20b0527dbcfb Mon Sep 17 00:00:00 2001 From: Mitchell Catoen Date: Mon, 23 May 2022 22:38:40 -0400 Subject: [PATCH 8/8] Fixing another small linting problem --- eth/_utils/headers.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/eth/_utils/headers.py b/eth/_utils/headers.py index 2e8799fffb..c9bfa21bc9 100644 --- a/eth/_utils/headers.py +++ b/eth/_utils/headers.py @@ -4,11 +4,7 @@ Tuple, ) -from eth_typing import ( - Address, - Hash32, -) - +from eth_typing import Address from eth.abc import BlockHeaderAPI from eth.constants import ( BLANK_ROOT_HASH,