From e73441045e80c73e0c421ade0f5df267c9e54947 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Tue, 28 Jan 2025 13:48:31 +0100 Subject: [PATCH 1/2] fix(tests): raise `InvalidParams` for requests shorter than 1 byte --- .../eip7685_general_purpose_el_requests/test_request_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/prague/eip7685_general_purpose_el_requests/test_request_types.py b/tests/prague/eip7685_general_purpose_el_requests/test_request_types.py index e151868ccfd..5c66db111b2 100644 --- a/tests/prague/eip7685_general_purpose_el_requests/test_request_types.py +++ b/tests/prague/eip7685_general_purpose_el_requests/test_request_types.py @@ -28,7 +28,7 @@ @pytest.fixture -def block_body_extra_requests(fork: Fork, invalid_request_data: bytes) -> List[bytes]: +def block_body_override_requests(fork: Fork, invalid_request_data: bytes) -> List[bytes]: """ Create a request with an invalid type for the fork. From e37a2fe57dcfe6d25f95a4d3d9eb650813d0cf54 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 29 Jan 2025 11:20:58 -0600 Subject: [PATCH 2/2] Refactor to #1138 (#1141) * refactor(tests): EIP-7685: Add invalid cases * refactor(tests): EIP-7685: Remove redundant `test_request_types.py`, remove unused fixtures * small comment * Update tests/prague/eip7685_general_purpose_el_requests/test_deposits_withdrawals_consolidations.py Co-authored-by: danceratopz --------- Co-authored-by: danceratopz --- .../conftest.py | 12 -- ...est_deposits_withdrawals_consolidations.py | 32 ++++- .../test_request_types.py | 115 ------------------ 3 files changed, 31 insertions(+), 128 deletions(-) delete mode 100644 tests/prague/eip7685_general_purpose_el_requests/test_request_types.py diff --git a/tests/prague/eip7685_general_purpose_el_requests/conftest.py b/tests/prague/eip7685_general_purpose_el_requests/conftest.py index 0e969beabbd..1d347199a70 100644 --- a/tests/prague/eip7685_general_purpose_el_requests/conftest.py +++ b/tests/prague/eip7685_general_purpose_el_requests/conftest.py @@ -4,7 +4,6 @@ import pytest -from ethereum_test_forks import Fork from ethereum_test_tools import ( Alloc, Block, @@ -36,12 +35,6 @@ def block_body_override_requests( return None -@pytest.fixture -def block_body_extra_requests() -> List[SupportsBytes]: - """List of requests that overwrite the requests in the header. None by default.""" - return [] - - @pytest.fixture def correct_requests_hash_in_header() -> bool: """ @@ -80,7 +73,6 @@ def is_monotonically_increasing(requests: List[bytes]) -> bool: @pytest.fixture def blocks( - fork: Fork, pre: Alloc, requests: List[ DepositInteractionBase @@ -88,7 +80,6 @@ def blocks( | ConsolidationRequestInteractionBase ], block_body_override_requests: List[Bytes | SupportsBytes] | None, - block_body_extra_requests: List[SupportsBytes], correct_requests_hash_in_header: bool, exception: BlockException | None, engine_api_error_code: EngineAPIError | None, @@ -109,9 +100,6 @@ def blocks( valid_requests = Requests(*valid_requests_list) - if block_body_override_requests is None and block_body_extra_requests is not None: - block_body_override_requests = valid_requests.requests_list + block_body_extra_requests - rlp_modifier: Header | None = None if correct_requests_hash_in_header: rlp_modifier = Header( diff --git a/tests/prague/eip7685_general_purpose_el_requests/test_deposits_withdrawals_consolidations.py b/tests/prague/eip7685_general_purpose_el_requests/test_deposits_withdrawals_consolidations.py index bd32cecb5d4..99dc98c707b 100644 --- a/tests/prague/eip7685_general_purpose_el_requests/test_deposits_withdrawals_consolidations.py +++ b/tests/prague/eip7685_general_purpose_el_requests/test_deposits_withdrawals_consolidations.py @@ -465,7 +465,31 @@ def invalid_requests_block_combinations(fork: Fork) -> List[Any]: correct_order_transactions, correct_order + [bytes([fork.max_request_type() + 1])], BlockException.INVALID_REQUESTS, - id="extra_invalid_type_request", + id="extra_invalid_type_request_with_no_data", + ), + ) + combinations.append( + pytest.param( + correct_order_transactions, + correct_order + [bytes([fork.max_request_type() + 1, 0x00])], + BlockException.INVALID_REQUESTS, + id="extra_invalid_type_request_with_data_0x00", + ), + ) + combinations.append( + pytest.param( + correct_order_transactions, + correct_order + [bytes([fork.max_request_type() + 1, 0x01])], + BlockException.INVALID_REQUESTS, + id="extra_invalid_type_request_with_data_0x01", + ), + ) + combinations.append( + pytest.param( + correct_order_transactions, + correct_order + [bytes([fork.max_request_type() + 1, 0xFF])], + BlockException.INVALID_REQUESTS, + id="extra_invalid_type_request_with_data_0xff", ), ) @@ -516,6 +540,12 @@ def test_invalid_deposit_withdrawal_consolidation_requests_engine( so the block might execute properly if the client ignores the requests in the new payload parameters. + Note that the only difference between the engine version produced by this test and + the ones produced by `test_invalid_deposit_withdrawal_consolidation_requests` is the + `blockHash` value in the new payloads, which is calculated using different request hashes + for each test, but since the request hash is not a value that is included in the payload, + it might not be immediately apparent. + Also these tests would not fail if the block is imported via RLP (syncing from a peer), so we only generate the BlockchainTestEngine for them. """ diff --git a/tests/prague/eip7685_general_purpose_el_requests/test_request_types.py b/tests/prague/eip7685_general_purpose_el_requests/test_request_types.py deleted file mode 100644 index 5c66db111b2..00000000000 --- a/tests/prague/eip7685_general_purpose_el_requests/test_request_types.py +++ /dev/null @@ -1,115 +0,0 @@ -"""Test the request types that can be included in a block by the given fork.""" - -from typing import List - -import pytest - -from ethereum_test_exceptions import BlockException -from ethereum_test_forks import Fork -from ethereum_test_tools import Alloc, Block, BlockchainTestFiller, Environment - -from ..eip6110_deposits.helpers import DepositInteractionBase, DepositRequest, DepositTransaction -from ..eip7002_el_triggerable_withdrawals.helpers import ( - WithdrawalRequest, - WithdrawalRequestInteractionBase, - WithdrawalRequestTransaction, -) -from ..eip7251_consolidations.helpers import ( - ConsolidationRequest, - ConsolidationRequestInteractionBase, - ConsolidationRequestTransaction, -) -from .spec import ref_spec_7685 - -REFERENCE_SPEC_GIT_PATH = ref_spec_7685.git_path -REFERENCE_SPEC_VERSION = ref_spec_7685.version - -pytestmark = pytest.mark.valid_from("Prague") - - -@pytest.fixture -def block_body_override_requests(fork: Fork, invalid_request_data: bytes) -> List[bytes]: - """ - Create a request with an invalid type for the fork. - - This overrides the default fixture and its behavior defined in conftest.py. - """ - invalid_request_type = fork.max_request_type() + 1 - return [bytes([invalid_request_type]) + invalid_request_data] - - -@pytest.fixture -def requests( - fork: Fork, - include_valid_requests: bool, -) -> List[ - DepositInteractionBase | WithdrawalRequestInteractionBase | ConsolidationRequestInteractionBase -]: - """List of valid requests that are added along with the invalid request.""" - if not include_valid_requests: - return [] - if fork.max_request_type() == 2: - return [ - DepositTransaction( - requests=[ - DepositRequest( - pubkey=1, - withdrawal_credentials=2, - amount=1_000_000_000, - signature=3, - index=0, - ) - ] - ), - WithdrawalRequestTransaction( - requests=[ - WithdrawalRequest( - validator_pubkey=1, - amount=0, - fee=1, - ) - ] - ), - ConsolidationRequestTransaction( - requests=[ - ConsolidationRequest( - source_pubkey=2, - target_pubkey=5, - fee=1, - ) - ] - ), - ] - raise NotImplementedError(f"Unsupported fork: {fork}") - - -@pytest.mark.parametrize( - "include_valid_requests", - [False, True], -) -@pytest.mark.parametrize( - "invalid_request_data", - [ - pytest.param(b"", id="no_data"), - pytest.param(b"\0", id="single_byte"), - pytest.param(b"\0" * 32, id="32_bytes"), - ], -) -@pytest.mark.parametrize( - "exception", - [ - pytest.param(BlockException.INVALID_REQUESTS, id=""), - ], -) -def test_invalid_request_type( - blockchain_test: BlockchainTestFiller, - pre: Alloc, - blocks: List[Block], -): - """Test sending a block with an invalid request type.""" - blockchain_test( - genesis_environment=Environment(), - pre=pre, - post={}, - blocks=blocks, - )