From 0c5a1362c983c7e49519b9a603e62f13c0b45963 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 29 Oct 2024 14:42:13 -0500 Subject: [PATCH 01/15] Exclude empty requests in requests list --- specs/electra/beacon-chain.md | 24 +++- specs/electra/validator.md | 43 +++++-- .../unittests/test_execution_requests.py | 115 ++++++++++++++++++ 3 files changed, 169 insertions(+), 13 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py diff --git a/specs/electra/beacon-chain.md b/specs/electra/beacon-chain.md index 0c56491907..5f0f77a2b8 100644 --- a/specs/electra/beacon-chain.md +++ b/specs/electra/beacon-chain.md @@ -137,6 +137,14 @@ The following values are (non-configurable) constants used throughout the specif | - | - | | `COMPOUNDING_WITHDRAWAL_PREFIX` | `Bytes1('0x02')` | +### Execution layer triggered requests + +| Name | Value | +| - | - | +| `DEPOSIT_REQUEST_TYPE` | `Bytes1('0x00')` | +| `WITHDRAWAL_REQUEST_TYPE` | `Bytes1('0x01')` | +| `CONSOLIDATION_REQUEST_TYPE` | `Bytes1('0x02')` | + ## Preset ### Gwei values @@ -1146,11 +1154,17 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: ```python def get_execution_requests_list(execution_requests: ExecutionRequests) -> Sequence[bytes]: - deposit_bytes = ssz_serialize(execution_requests.deposits) - withdrawal_bytes = ssz_serialize(execution_requests.withdrawals) - consolidation_bytes = ssz_serialize(execution_requests.consolidations) - - return [deposit_bytes, withdrawal_bytes, consolidation_bytes] + requests = [ + (DEPOSIT_REQUEST_TYPE, execution_requests.deposits), + (WITHDRAWAL_REQUEST_TYPE, execution_requests.withdrawals), + (CONSOLIDATION_REQUEST_TYPE, execution_requests.consolidations), + ] + + return [ + request_type + ssz_serialize(request_data) + for request_type, request_data in requests + if len(request_data) != 0 + ] ``` ##### Modified `process_execution_payload` diff --git a/specs/electra/validator.md b/specs/electra/validator.md index 553eeaa702..c896efe11b 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -189,18 +189,45 @@ def prepare_execution_payload(state: BeaconState, *[New in Electra]* -1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `execution_requests` entry containing a list of bytes. Each element on the list corresponds to one SSZ list of requests as defined -in [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685). The index of each element in the array determines the type of request. +1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `execution_requests` entry containing a list of bytes. Each element on the list corresponds to one SSZ list of requests as defined in [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685). The first byte of each request is used to determine the request type. There can only be one instance of each request type per execution requests. 2. Set `block.body.execution_requests = get_execution_requests(execution_requests)`, where: ```python def get_execution_requests(execution_requests: Sequence[bytes]) -> ExecutionRequests: - deposits = ssz_deserialize(List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD], execution_requests[0]) - withdrawals = ssz_deserialize(List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD], execution_requests[1]) - consolidations = ssz_deserialize(List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD], - execution_requests[2]) - - return ExecutionRequests(deposits, withdrawals, consolidations) + deposits = None + withdrawals = None + consolidations = None + + for request in execution_requests: + request_type, request_data = request[0:1], request[1:] + assert len(request_data) != 0, "empty request data" + + if request_type == DEPOSIT_REQUEST_TYPE: + assert deposits is None, "duplicate deposit request" + deposits = ssz_deserialize( + List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD], + request_data + ) + elif request_type == WITHDRAWAL_REQUEST_TYPE: + assert withdrawals is None, "duplicate withdrawal request" + withdrawals = ssz_deserialize( + List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD], + request_data + ) + elif request_type == CONSOLIDATION_REQUEST_TYPE: + assert consolidations is None, "duplicate consolidation request" + consolidations = ssz_deserialize( + List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD], + request_data + ) + else: + raise ValueError(f"unexpected request type: {repr(request_type)}") + + return ExecutionRequests( + deposits=deposits or [], + withdrawals=withdrawals or [], + consolidations=consolidations or [] + ) ``` ## Attesting diff --git a/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py b/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py new file mode 100644 index 0000000000..581b1a54a8 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py @@ -0,0 +1,115 @@ +from eth2spec.test.context import ( + single_phase, + spec_test, + with_electra_and_later, +) + + +@with_electra_and_later +@spec_test +@single_phase +def test_requests_serialization_round_trip__empty(spec): + execution_requests = spec.ExecutionRequests() + serialized_execution_requests = spec.get_execution_requests_list(execution_requests) + deserialized_execution_requests = spec.get_execution_requests(serialized_execution_requests) + assert deserialized_execution_requests == execution_requests + + +@with_electra_and_later +@spec_test +@single_phase +def test_requests_serialization_round_trip__one_request(spec): + execution_requests = spec.ExecutionRequests( + deposits=[spec.DepositRequest()], + ) + serialized_execution_requests = spec.get_execution_requests_list(execution_requests) + deserialized_execution_requests = spec.get_execution_requests(serialized_execution_requests) + assert deserialized_execution_requests == execution_requests + + +@with_electra_and_later +@spec_test +@single_phase +def test_requests_serialization_round_trip__multiple_requests(spec): + execution_requests = spec.ExecutionRequests( + deposits=[spec.DepositRequest()], + withdrawals=[spec.WithdrawalRequest()], + consolidations=[spec.ConsolidationRequest()], + ) + serialized_execution_requests = spec.get_execution_requests_list(execution_requests) + deserialized_execution_requests = spec.get_execution_requests(serialized_execution_requests) + assert deserialized_execution_requests == execution_requests + + +@with_electra_and_later +@spec_test +@single_phase +def test_requests_serialization_round_trip__one_request_with_real_data(spec): + execution_requests = spec.ExecutionRequests( + deposits=[ + spec.DepositRequest( + pubkey=spec.BLSPubkey(48 * "aa"), + withdrawal_credentials=spec.Bytes32(32 * "bb"), + amount=spec.Gwei(11111111), + signature=spec.BLSSignature(96 * "cc"), + index=spec.uint64(22222222), + ), + ] + ) + serialized_execution_requests = spec.get_execution_requests_list(execution_requests) + deserialized_execution_requests = spec.get_execution_requests(serialized_execution_requests) + assert deserialized_execution_requests == execution_requests + + +@with_electra_and_later +@spec_test +@single_phase +def test_requests_deserialize__allow_out_of_order_requests(spec): + serialized_execution_requests = [ + spec.WITHDRAWAL_REQUEST_TYPE + 76 * b"\x0a", + spec.DEPOSIT_REQUEST_TYPE + 192 * b"\x0b", + ] + assert int(serialized_execution_requests[0][0]) > int(serialized_execution_requests[1][0]) + spec.get_execution_requests(serialized_execution_requests) + + +@with_electra_and_later +@spec_test +@single_phase +def test_requests_deserialize__reject_empty_request(spec): + serialized_execution_requests = [b"\x01"] + try: + spec.get_execution_requests(serialized_execution_requests) + assert False, "expected exception" + except Exception as e: + assert "empty request data" in str(e) + + +@with_electra_and_later +@spec_test +@single_phase +def test_requests_deserialize__reject_duplicate_request(spec): + serialized_withdrawal = 76 * b"\x0a" + serialized_execution_requests = [ + spec.WITHDRAWAL_REQUEST_TYPE + serialized_withdrawal, + spec.WITHDRAWAL_REQUEST_TYPE + serialized_withdrawal, + ] + try: + spec.get_execution_requests(serialized_execution_requests) + assert False, "expected exception" + except Exception as e: + assert "duplicate withdrawal request" in str(e) + + +@with_electra_and_later +@spec_test +@single_phase +def test_requests_deserialize__reject_unexpected_request_type(spec): + serialized_execution_requests = [ + b"\x03\xff\xff\xff", + ] + try: + spec.get_execution_requests(serialized_execution_requests) + assert False, "expected exception" + except Exception as e: + assert "unexpected request type" in str(e) From 369b7e578072aa3c6640351db39fd696a4fd484c Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 29 Oct 2024 14:47:16 -0500 Subject: [PATCH 02/15] Fix table of contents --- specs/electra/beacon-chain.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/electra/beacon-chain.md b/specs/electra/beacon-chain.md index 5f0f77a2b8..940bba7b8b 100644 --- a/specs/electra/beacon-chain.md +++ b/specs/electra/beacon-chain.md @@ -12,6 +12,7 @@ - [Constants](#constants) - [Misc](#misc) - [Withdrawal prefixes](#withdrawal-prefixes) + - [Execution layer triggered requests](#execution-layer-triggered-requests) - [Preset](#preset) - [Gwei values](#gwei-values) - [Rewards and penalties](#rewards-and-penalties) From 2bd570c6be385d372ad877713e9d0c366b07ca2f Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 29 Oct 2024 14:58:21 -0500 Subject: [PATCH 03/15] Use assert instead of raise --- specs/electra/validator.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/specs/electra/validator.md b/specs/electra/validator.md index c896efe11b..6b544f56e4 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -199,7 +199,13 @@ def get_execution_requests(execution_requests: Sequence[bytes]) -> ExecutionRequ consolidations = None for request in execution_requests: + assert len(request) >= 1 request_type, request_data = request[0:1], request[1:] + assert request_type in [ + DEPOSIT_REQUEST_TYPE, + WITHDRAWAL_REQUEST_TYPE, + CONSOLIDATION_REQUEST_TYPE, + ], f"unexpected request type: {repr(request_type)}" assert len(request_data) != 0, "empty request data" if request_type == DEPOSIT_REQUEST_TYPE: @@ -220,8 +226,6 @@ def get_execution_requests(execution_requests: Sequence[bytes]) -> ExecutionRequ List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD], request_data ) - else: - raise ValueError(f"unexpected request type: {repr(request_type)}") return ExecutionRequests( deposits=deposits or [], From 8a8a3214cdaaedd6622329de1753d7484df46b4b Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 29 Oct 2024 15:10:22 -0500 Subject: [PATCH 04/15] Fix indentation --- specs/electra/validator.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/specs/electra/validator.md b/specs/electra/validator.md index 6b544f56e4..028d4b659d 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -202,9 +202,9 @@ def get_execution_requests(execution_requests: Sequence[bytes]) -> ExecutionRequ assert len(request) >= 1 request_type, request_data = request[0:1], request[1:] assert request_type in [ - DEPOSIT_REQUEST_TYPE, - WITHDRAWAL_REQUEST_TYPE, - CONSOLIDATION_REQUEST_TYPE, + DEPOSIT_REQUEST_TYPE, + WITHDRAWAL_REQUEST_TYPE, + CONSOLIDATION_REQUEST_TYPE, ], f"unexpected request type: {repr(request_type)}" assert len(request_data) != 0, "empty request data" From 3ce29297ce1f954e006d60f926d9c16b008cce0f Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 29 Oct 2024 16:04:03 -0500 Subject: [PATCH 05/15] Simplify get_execution_requests --- specs/electra/validator.md | 63 ++++++++----------- .../unittests/test_execution_requests.py | 2 +- 2 files changed, 27 insertions(+), 38 deletions(-) diff --git a/specs/electra/validator.md b/specs/electra/validator.md index 028d4b659d..4348b0a320 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -193,45 +193,34 @@ def prepare_execution_payload(state: BeaconState, 2. Set `block.body.execution_requests = get_execution_requests(execution_requests)`, where: ```python -def get_execution_requests(execution_requests: Sequence[bytes]) -> ExecutionRequests: - deposits = None - withdrawals = None - consolidations = None - - for request in execution_requests: - assert len(request) >= 1 +def get_execution_requests(execution_requests_list: Sequence[bytes]) -> ExecutionRequests: + requests = { + DEPOSIT_REQUEST_TYPE: { + "value": None, + "field": "deposits", + "type": List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD], + }, + WITHDRAWAL_REQUEST_TYPE: { + "value": None, + "field": "withdrawals", + "type": List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD], + }, + CONSOLIDATION_REQUEST_TYPE: { + "value": None, + "field": "consolidations", + "type": List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD], + }, + } + + execution_requests = ExecutionRequests() + for request in execution_requests_list: request_type, request_data = request[0:1], request[1:] - assert request_type in [ - DEPOSIT_REQUEST_TYPE, - WITHDRAWAL_REQUEST_TYPE, - CONSOLIDATION_REQUEST_TYPE, - ], f"unexpected request type: {repr(request_type)}" + assert request_type in requests, "unexpected request type" assert len(request_data) != 0, "empty request data" - - if request_type == DEPOSIT_REQUEST_TYPE: - assert deposits is None, "duplicate deposit request" - deposits = ssz_deserialize( - List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD], - request_data - ) - elif request_type == WITHDRAWAL_REQUEST_TYPE: - assert withdrawals is None, "duplicate withdrawal request" - withdrawals = ssz_deserialize( - List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD], - request_data - ) - elif request_type == CONSOLIDATION_REQUEST_TYPE: - assert consolidations is None, "duplicate consolidation request" - consolidations = ssz_deserialize( - List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD], - request_data - ) - - return ExecutionRequests( - deposits=deposits or [], - withdrawals=withdrawals or [], - consolidations=consolidations or [] - ) + assert requests[request_type]["value"] is None, "duplicate request" + requests[request_type]["value"] = ssz_deserialize(requests[request_type]["type"], request_data) + setattr(execution_requests, requests[request_type]["field"], requests[request_type]["value"]) + return execution_requests ``` ## Attesting diff --git a/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py b/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py index 581b1a54a8..1204ffce17 100644 --- a/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py +++ b/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py @@ -98,7 +98,7 @@ def test_requests_deserialize__reject_duplicate_request(spec): spec.get_execution_requests(serialized_execution_requests) assert False, "expected exception" except Exception as e: - assert "duplicate withdrawal request" in str(e) + assert "duplicate request" in str(e) @with_electra_and_later From 4438166552c25536ed6c7d90712dfb2e85fc7d22 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 29 Oct 2024 19:22:27 -0500 Subject: [PATCH 06/15] Check for ascending order --- specs/electra/validator.md | 13 ++++--- .../unittests/test_execution_requests.py | 34 +++++++++++-------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/specs/electra/validator.md b/specs/electra/validator.md index 4348b0a320..04e521ab84 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -189,37 +189,36 @@ def prepare_execution_payload(state: BeaconState, *[New in Electra]* -1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `execution_requests` entry containing a list of bytes. Each element on the list corresponds to one SSZ list of requests as defined in [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685). The first byte of each request is used to determine the request type. There can only be one instance of each request type per execution requests. +1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `execution_requests` entry containing a list of bytes. Each element on the list corresponds to one SSZ list of requests as defined in [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685). The first byte of each request is used to determine the request type. Requests must be order by request type in ascending order. As a result, there can only be one instance of each request type per execution requests. 2. Set `block.body.execution_requests = get_execution_requests(execution_requests)`, where: ```python def get_execution_requests(execution_requests_list: Sequence[bytes]) -> ExecutionRequests: requests = { DEPOSIT_REQUEST_TYPE: { - "value": None, "field": "deposits", "type": List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD], }, WITHDRAWAL_REQUEST_TYPE: { - "value": None, "field": "withdrawals", "type": List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD], }, CONSOLIDATION_REQUEST_TYPE: { - "value": None, "field": "consolidations", "type": List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD], }, } + prev_request_type = None execution_requests = ExecutionRequests() for request in execution_requests_list: request_type, request_data = request[0:1], request[1:] assert request_type in requests, "unexpected request type" assert len(request_data) != 0, "empty request data" - assert requests[request_type]["value"] is None, "duplicate request" - requests[request_type]["value"] = ssz_deserialize(requests[request_type]["type"], request_data) - setattr(execution_requests, requests[request_type]["field"], requests[request_type]["value"]) + assert prev_request_type is None or prev_request_type < request_type, "not ascending order" + prev_request_type = request_type + deserialized_request = ssz_deserialize(requests[request_type]["type"], request_data) + setattr(execution_requests, requests[request_type]["field"], deserialized_request) return execution_requests ``` diff --git a/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py b/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py index 1204ffce17..2a63c17a3f 100644 --- a/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py +++ b/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py @@ -64,41 +64,45 @@ def test_requests_serialization_round_trip__one_request_with_real_data(spec): @with_electra_and_later @spec_test @single_phase -def test_requests_deserialize__allow_out_of_order_requests(spec): +def test_requests_deserialize__reject_duplicate_request(spec): + serialized_withdrawal = 76 * b"\x0a" serialized_execution_requests = [ - spec.WITHDRAWAL_REQUEST_TYPE + 76 * b"\x0a", - spec.DEPOSIT_REQUEST_TYPE + 192 * b"\x0b", + spec.WITHDRAWAL_REQUEST_TYPE + serialized_withdrawal, + spec.WITHDRAWAL_REQUEST_TYPE + serialized_withdrawal, ] - assert int(serialized_execution_requests[0][0]) > int(serialized_execution_requests[1][0]) - spec.get_execution_requests(serialized_execution_requests) + try: + spec.get_execution_requests(serialized_execution_requests) + assert False, "expected exception" + except Exception as e: + assert "not ascending order" in str(e) @with_electra_and_later @spec_test @single_phase -def test_requests_deserialize__reject_empty_request(spec): - serialized_execution_requests = [b"\x01"] +def test_requests_deserialize__reject_out_of_order_requests(spec): + serialized_execution_requests = [ + spec.WITHDRAWAL_REQUEST_TYPE + 76 * b"\x0a", + spec.DEPOSIT_REQUEST_TYPE + 192 * b"\x0b", + ] + assert int(serialized_execution_requests[0][0]) > int(serialized_execution_requests[1][0]) try: spec.get_execution_requests(serialized_execution_requests) assert False, "expected exception" except Exception as e: - assert "empty request data" in str(e) + assert "not ascending order" in str(e) @with_electra_and_later @spec_test @single_phase -def test_requests_deserialize__reject_duplicate_request(spec): - serialized_withdrawal = 76 * b"\x0a" - serialized_execution_requests = [ - spec.WITHDRAWAL_REQUEST_TYPE + serialized_withdrawal, - spec.WITHDRAWAL_REQUEST_TYPE + serialized_withdrawal, - ] +def test_requests_deserialize__reject_empty_request(spec): + serialized_execution_requests = [b"\x01"] try: spec.get_execution_requests(serialized_execution_requests) assert False, "expected exception" except Exception as e: - assert "duplicate request" in str(e) + assert "empty request data" in str(e) @with_electra_and_later From e90c792c5cf4412f6885f48f6a639a53ac570522 Mon Sep 17 00:00:00 2001 From: Justin Traglia <95511699+jtraglia@users.noreply.github.com> Date: Wed, 30 Oct 2024 15:51:49 -0500 Subject: [PATCH 07/15] Fix typo Co-authored-by: Alex Stokes --- specs/electra/validator.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/electra/validator.md b/specs/electra/validator.md index 04e521ab84..b79c262507 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -189,7 +189,7 @@ def prepare_execution_payload(state: BeaconState, *[New in Electra]* -1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `execution_requests` entry containing a list of bytes. Each element on the list corresponds to one SSZ list of requests as defined in [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685). The first byte of each request is used to determine the request type. Requests must be order by request type in ascending order. As a result, there can only be one instance of each request type per execution requests. +1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `execution_requests` entry containing a list of bytes. Each element on the list corresponds to one SSZ list of requests as defined in [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685). The first byte of each request is used to determine the request type. Requests must be ordered by request type in ascending order. As a result, there can only be one instance of each request type per execution requests. 2. Set `block.body.execution_requests = get_execution_requests(execution_requests)`, where: ```python From b3e77d2f575d9a03a3afd13269cfb6ca421cef78 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 30 Oct 2024 15:55:41 -0500 Subject: [PATCH 08/15] Apply other stokes suggestion --- specs/electra/validator.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/electra/validator.md b/specs/electra/validator.md index b79c262507..55f9f10273 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -189,7 +189,7 @@ def prepare_execution_payload(state: BeaconState, *[New in Electra]* -1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `execution_requests` entry containing a list of bytes. Each element on the list corresponds to one SSZ list of requests as defined in [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685). The first byte of each request is used to determine the request type. Requests must be ordered by request type in ascending order. As a result, there can only be one instance of each request type per execution requests. +1. The execution payload is obtained from the execution engine as defined above using `payload_id`. The response also includes a `execution_requests` entry containing a list of bytes. Each element on the list corresponds to one SSZ list of requests as defined in [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685). The first byte of each request is used to determine the request type. Requests must be ordered by request type in ascending order. As a result, there can only be at most one instance of each request type. 2. Set `block.body.execution_requests = get_execution_requests(execution_requests)`, where: ```python From 276c56201a56d9e343f500db13d4b8dd4482a1f7 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 30 Oct 2024 16:47:53 -0500 Subject: [PATCH 09/15] Use less python sugar --- specs/electra/validator.md | 51 ++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/specs/electra/validator.md b/specs/electra/validator.md index 55f9f10273..1cd4b91eca 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -194,32 +194,45 @@ def prepare_execution_payload(state: BeaconState, ```python def get_execution_requests(execution_requests_list: Sequence[bytes]) -> ExecutionRequests: - requests = { - DEPOSIT_REQUEST_TYPE: { - "field": "deposits", - "type": List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD], - }, - WITHDRAWAL_REQUEST_TYPE: { - "field": "withdrawals", - "type": List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD], - }, - CONSOLIDATION_REQUEST_TYPE: { - "field": "consolidations", - "type": List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD], - }, - } + deposits = [] + withdrawals = [] + consolidations = [] + + request_types = [ + DEPOSIT_REQUEST_TYPE, + WITHDRAWAL_REQUEST_TYPE, + CONSOLIDATION_REQUEST_TYPE, + ] prev_request_type = None - execution_requests = ExecutionRequests() for request in execution_requests_list: request_type, request_data = request[0:1], request[1:] - assert request_type in requests, "unexpected request type" + assert request_type in request_types, "unexpected request type" assert len(request_data) != 0, "empty request data" assert prev_request_type is None or prev_request_type < request_type, "not ascending order" prev_request_type = request_type - deserialized_request = ssz_deserialize(requests[request_type]["type"], request_data) - setattr(execution_requests, requests[request_type]["field"], deserialized_request) - return execution_requests + + if request_type == DEPOSIT_REQUEST_TYPE: + deposits = ssz_deserialize( + List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD], + request_data + ) + elif request_type == WITHDRAWAL_REQUEST_TYPE: + withdrawals = ssz_deserialize( + List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD], + request_data + ) + elif request_type == CONSOLIDATION_REQUEST_TYPE: + consolidations = ssz_deserialize( + List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD], + request_data + ) + + return ExecutionRequests( + deposits=deposits, + withdrawals=withdrawals, + consolidations=consolidations, + ) ``` ## Attesting From ebbce03db569b81df9eefc2f7832252081b45690 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 6 Nov 2024 13:26:59 -0600 Subject: [PATCH 10/15] Replace assert messages with comments --- specs/electra/validator.md | 11 ++++++++--- .../electra/unittests/test_execution_requests.py | 16 ++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/specs/electra/validator.md b/specs/electra/validator.md index 1cd4b91eca..ca92a1d955 100644 --- a/specs/electra/validator.md +++ b/specs/electra/validator.md @@ -207,9 +207,14 @@ def get_execution_requests(execution_requests_list: Sequence[bytes]) -> Executio prev_request_type = None for request in execution_requests_list: request_type, request_data = request[0:1], request[1:] - assert request_type in request_types, "unexpected request type" - assert len(request_data) != 0, "empty request data" - assert prev_request_type is None or prev_request_type < request_type, "not ascending order" + + # Check that the request type is valid + assert request_type in request_types + # Check that the request data is not empty + assert len(request_data) != 0 + # Check that requests are in strictly ascending order + # Each successive type must be greater than the last with no duplicates + assert prev_request_type is None or prev_request_type < request_type prev_request_type = request_type if request_type == DEPOSIT_REQUEST_TYPE: diff --git a/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py b/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py index 2a63c17a3f..d57e724312 100644 --- a/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py +++ b/tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py @@ -73,8 +73,8 @@ def test_requests_deserialize__reject_duplicate_request(spec): try: spec.get_execution_requests(serialized_execution_requests) assert False, "expected exception" - except Exception as e: - assert "not ascending order" in str(e) + except Exception: + pass @with_electra_and_later @@ -89,8 +89,8 @@ def test_requests_deserialize__reject_out_of_order_requests(spec): try: spec.get_execution_requests(serialized_execution_requests) assert False, "expected exception" - except Exception as e: - assert "not ascending order" in str(e) + except Exception: + pass @with_electra_and_later @@ -101,8 +101,8 @@ def test_requests_deserialize__reject_empty_request(spec): try: spec.get_execution_requests(serialized_execution_requests) assert False, "expected exception" - except Exception as e: - assert "empty request data" in str(e) + except Exception: + pass @with_electra_and_later @@ -115,5 +115,5 @@ def test_requests_deserialize__reject_unexpected_request_type(spec): try: spec.get_execution_requests(serialized_execution_requests) assert False, "expected exception" - except Exception as e: - assert "unexpected request type" in str(e) + except Exception: + pass From e472afd14407169d685723706842130162277942 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 20 Nov 2024 10:32:35 -0600 Subject: [PATCH 11/15] Include requests_root in block_hash computation --- .../test/helpers/execution_payload.py | 42 +++++++++++++++---- .../pyspec/eth2spec/test/helpers/genesis.py | 5 +++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py index 0766008b84..fa24309320 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py @@ -1,4 +1,5 @@ from eth_hash.auto import keccak +from hashlib import sha256 from trie import HexaryTrie from rlp import encode from rlp.sedes import big_endian_int, Binary, List @@ -7,7 +8,12 @@ from eth2spec.utils.ssz.ssz_impl import hash_tree_root from eth2spec.debug.random_value import get_random_bytes_list from eth2spec.test.helpers.withdrawals import get_expected_withdrawals -from eth2spec.test.helpers.forks import is_post_capella, is_post_deneb, is_post_eip7732 +from eth2spec.test.helpers.forks import ( + is_post_capella, + is_post_deneb, + is_post_electra, + is_post_eip7732, +) def get_execution_payload_header(spec, execution_payload): @@ -59,13 +65,22 @@ def compute_trie_root_from_indexed_data(data): return t.root_hash +def compute_requests_hash(block_requests): + m = sha256() + for r in block_requests: + if len(r) > 1: + m.update(sha256(r)) + return m.digest() + + # https://eips.ethereum.org/EIPS/eip-4895 # https://eips.ethereum.org/EIPS/eip-4844 def compute_el_header_block_hash(spec, payload_header, transactions_trie_root, withdrawals_trie_root=None, - parent_beacon_block_root=None): + parent_beacon_block_root=None, + requests_root=None): """ Computes the RLP execution block hash described by an `ExecutionPayloadHeader`. """ @@ -116,6 +131,9 @@ def compute_el_header_block_hash(spec, execution_payload_header_rlp.append((big_endian_int, payload_header.excess_blob_gas)) # parent_beacon_root execution_payload_header_rlp.append((Binary(32, 32), parent_beacon_block_root)) + if is_post_electra(spec): + # requests_root + execution_payload_header_rlp.append((Binary(32, 32), requests_root)) sedes = List([schema for schema, _ in execution_payload_header_rlp]) values = [value for _, value in execution_payload_header_rlp] @@ -191,7 +209,7 @@ def get_consolidation_request_rlp_bytes(consolidation_request): return b"\x02" + encode(values, sedes) -def compute_el_block_hash_with_parent_root(spec, payload, parent_beacon_block_root): +def compute_el_block_hash_with_new_fields(spec, payload, parent_beacon_block_root, requests_root): if payload == spec.ExecutionPayload(): return spec.Hash32() @@ -213,25 +231,35 @@ def compute_el_block_hash_with_parent_root(spec, payload, parent_beacon_block_ro transactions_trie_root, withdrawals_trie_root, parent_beacon_block_root, + requests_root, ) def compute_el_block_hash(spec, payload, pre_state): parent_beacon_block_root = None + requests_root = None if is_post_deneb(spec): previous_block_header = pre_state.latest_block_header.copy() if previous_block_header.state_root == spec.Root(): previous_block_header.state_root = pre_state.hash_tree_root() parent_beacon_block_root = previous_block_header.hash_tree_root() + if is_post_electra(spec): + requests_root = compute_requests_hash([]) - return compute_el_block_hash_with_parent_root( - spec, payload, parent_beacon_block_root) + return compute_el_block_hash_with_new_fields( + spec, payload, parent_beacon_block_root, requests_root) def compute_el_block_hash_for_block(spec, block): - return compute_el_block_hash_with_parent_root( - spec, block.body.execution_payload, block.parent_root) + requests_root = None + + if is_post_electra(spec): + requests_list = spec.get_execution_requests_list(block.body.execution_requests) + requests_root = compute_requests_hash(requests_list) + + return compute_el_block_hash_with_new_fields( + spec, block.body.execution_payload, block.parent_root, requests_root) def build_empty_post_eip7732_execution_payload_header(spec, state): diff --git a/tests/core/pyspec/eth2spec/test/helpers/genesis.py b/tests/core/pyspec/eth2spec/test/helpers/genesis.py index bd4e5d3bf3..3213169145 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/genesis.py +++ b/tests/core/pyspec/eth2spec/test/helpers/genesis.py @@ -1,3 +1,4 @@ +from hashlib import sha256 from eth2spec.test.helpers.constants import ( PHASE0, PREVIOUS_FORK_OF, @@ -66,11 +67,14 @@ def get_sample_genesis_execution_payload_header(spec, transactions_trie_root = bytes.fromhex("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421") withdrawals_trie_root = None parent_beacon_block_root = None + requests_root = None if is_post_capella(spec): withdrawals_trie_root = bytes.fromhex("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421") if is_post_deneb(spec): parent_beacon_block_root = bytes.fromhex("0000000000000000000000000000000000000000000000000000000000000000") + if is_post_electra(spec): + requests_root = sha256(b"").digest() payload_header.block_hash = compute_el_header_block_hash( spec, @@ -78,6 +82,7 @@ def get_sample_genesis_execution_payload_header(spec, transactions_trie_root, withdrawals_trie_root, parent_beacon_block_root, + requests_root, ) return payload_header From b1c9d279bda21400caa9897c6552b6aa9ea9cec2 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 20 Nov 2024 10:34:36 -0600 Subject: [PATCH 12/15] Add comment to compute_requests_hash --- tests/core/pyspec/eth2spec/test/helpers/execution_payload.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py index fa24309320..e38a4c5045 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py @@ -65,6 +65,7 @@ def compute_trie_root_from_indexed_data(data): return t.root_hash +# https://eips.ethereum.org/EIPS/eip-7685 def compute_requests_hash(block_requests): m = sha256() for r in block_requests: From de52c76bd38a954c58270d402214ac716a9474a3 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Thu, 21 Nov 2024 23:58:31 +0100 Subject: [PATCH 13/15] Fix block hash computation for deposit transition tests Request hash is not considered in `compute_el_block_hash`, have to use one of the other overloads for this to work. --- .../test/electra/sanity/blocks/test_deposit_transition.py | 6 +++--- .../core/pyspec/eth2spec/test/helpers/execution_payload.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py index 9749c89ffd..a9c2c62814 100644 --- a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py +++ b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_deposit_transition.py @@ -12,7 +12,7 @@ prepare_deposit_request, ) from eth2spec.test.helpers.execution_payload import ( - compute_el_block_hash, + compute_el_block_hash_for_block, ) from eth2spec.test.helpers.keys import privkeys, pubkeys from eth2spec.test.helpers.state import ( @@ -134,7 +134,7 @@ def prepare_state_and_block(spec, # Assign deposits and deposit requests block.body.deposits = deposits block.body.execution_requests.deposits = deposit_requests - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload, state) + block.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block) return state, block @@ -251,7 +251,7 @@ def test_deposit_transition__deposit_and_top_up_same_block(spec, state): # Artificially assign deposit's pubkey to a deposit request of the same block top_up_keys = [block.body.deposits[0].data.pubkey] block.body.execution_requests.deposits[0].pubkey = top_up_keys[0] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload, state) + block.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block) pre_pending_deposits = len(state.pending_deposits) diff --git a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py index e38a4c5045..fe54b00b66 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py @@ -70,7 +70,7 @@ def compute_requests_hash(block_requests): m = sha256() for r in block_requests: if len(r) > 1: - m.update(sha256(r)) + m.update(sha256(r).digest()) return m.digest() From 15e3f151d7c7951a489404e20540b2049286d545 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Fri, 22 Nov 2024 09:03:02 +0100 Subject: [PATCH 14/15] Fix block hash computation for withdrawal sanity tests Request hash is not considered in `compute_el_block_hash`, have to use one of the other overloads for this to work. --- .../eth2spec/test/electra/sanity/blocks/test_blocks.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_blocks.py b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_blocks.py index c3d2284610..5a4b98c3c8 100644 --- a/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/electra/sanity/blocks/test_blocks.py @@ -9,7 +9,7 @@ get_signed_address_change, ) from eth2spec.test.helpers.execution_payload import ( - compute_el_block_hash, + compute_el_block_hash_for_block, ) from eth2spec.test.helpers.voluntary_exits import ( prepare_signed_exits, @@ -42,7 +42,7 @@ def test_basic_el_withdrawal_request(spec, state): ) block = build_empty_block_for_next_slot(spec, state) block.body.execution_requests.withdrawals = [withdrawal_request] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload, state) + block.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block) signed_block = state_transition_and_sign_block(spec, state, block) yield 'blocks', [signed_block] @@ -80,7 +80,7 @@ def test_basic_btec_and_el_withdrawal_request_in_same_block(spec, state): ) block.body.execution_requests.withdrawals = [withdrawal_request] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload, state) + block.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block) signed_block = state_transition_and_sign_block(spec, state, block) yield 'blocks', [signed_block] @@ -132,7 +132,7 @@ def test_basic_btec_before_el_withdrawal_request(spec, state): ) block_2 = build_empty_block_for_next_slot(spec, state) block_2.body.execution_requests.withdrawals = [withdrawal_request] - block_2.body.execution_payload.block_hash = compute_el_block_hash(spec, block_2.body.execution_payload, state) + block_2.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block_2) signed_block_2 = state_transition_and_sign_block(spec, state, block_2) yield 'blocks', [signed_block_1, signed_block_2] @@ -165,7 +165,7 @@ def test_cl_exit_and_el_withdrawal_request_in_same_block(spec, state): block = build_empty_block_for_next_slot(spec, state) block.body.voluntary_exits = signed_voluntary_exits block.body.execution_requests.withdrawals = [withdrawal_request] - block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload, state) + block.body.execution_payload.block_hash = compute_el_block_hash_for_block(spec, block) signed_block = state_transition_and_sign_block(spec, state, block) yield 'blocks', [signed_block] From 4aad8ebcdc1cb8030e1f6b2a8d05a8adfb6b45ef Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Fri, 22 Nov 2024 07:02:29 -0600 Subject: [PATCH 15/15] Rename requests_root to requests_hash --- .../test/helpers/execution_payload.py | 22 +++++++++---------- .../pyspec/eth2spec/test/helpers/genesis.py | 6 ++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py index fe54b00b66..80684b9e6e 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py @@ -81,7 +81,7 @@ def compute_el_header_block_hash(spec, transactions_trie_root, withdrawals_trie_root=None, parent_beacon_block_root=None, - requests_root=None): + requests_hash=None): """ Computes the RLP execution block hash described by an `ExecutionPayloadHeader`. """ @@ -133,8 +133,8 @@ def compute_el_header_block_hash(spec, # parent_beacon_root execution_payload_header_rlp.append((Binary(32, 32), parent_beacon_block_root)) if is_post_electra(spec): - # requests_root - execution_payload_header_rlp.append((Binary(32, 32), requests_root)) + # requests_hash + execution_payload_header_rlp.append((Binary(32, 32), requests_hash)) sedes = List([schema for schema, _ in execution_payload_header_rlp]) values = [value for _, value in execution_payload_header_rlp] @@ -210,7 +210,7 @@ def get_consolidation_request_rlp_bytes(consolidation_request): return b"\x02" + encode(values, sedes) -def compute_el_block_hash_with_new_fields(spec, payload, parent_beacon_block_root, requests_root): +def compute_el_block_hash_with_new_fields(spec, payload, parent_beacon_block_root, requests_hash): if payload == spec.ExecutionPayload(): return spec.Hash32() @@ -232,13 +232,13 @@ def compute_el_block_hash_with_new_fields(spec, payload, parent_beacon_block_roo transactions_trie_root, withdrawals_trie_root, parent_beacon_block_root, - requests_root, + requests_hash, ) def compute_el_block_hash(spec, payload, pre_state): parent_beacon_block_root = None - requests_root = None + requests_hash = None if is_post_deneb(spec): previous_block_header = pre_state.latest_block_header.copy() @@ -246,21 +246,21 @@ def compute_el_block_hash(spec, payload, pre_state): previous_block_header.state_root = pre_state.hash_tree_root() parent_beacon_block_root = previous_block_header.hash_tree_root() if is_post_electra(spec): - requests_root = compute_requests_hash([]) + requests_hash = compute_requests_hash([]) return compute_el_block_hash_with_new_fields( - spec, payload, parent_beacon_block_root, requests_root) + spec, payload, parent_beacon_block_root, requests_hash) def compute_el_block_hash_for_block(spec, block): - requests_root = None + requests_hash = None if is_post_electra(spec): requests_list = spec.get_execution_requests_list(block.body.execution_requests) - requests_root = compute_requests_hash(requests_list) + requests_hash = compute_requests_hash(requests_list) return compute_el_block_hash_with_new_fields( - spec, block.body.execution_payload, block.parent_root, requests_root) + spec, block.body.execution_payload, block.parent_root, requests_hash) def build_empty_post_eip7732_execution_payload_header(spec, state): diff --git a/tests/core/pyspec/eth2spec/test/helpers/genesis.py b/tests/core/pyspec/eth2spec/test/helpers/genesis.py index 3213169145..9c43676a41 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/genesis.py +++ b/tests/core/pyspec/eth2spec/test/helpers/genesis.py @@ -67,14 +67,14 @@ def get_sample_genesis_execution_payload_header(spec, transactions_trie_root = bytes.fromhex("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421") withdrawals_trie_root = None parent_beacon_block_root = None - requests_root = None + requests_hash = None if is_post_capella(spec): withdrawals_trie_root = bytes.fromhex("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421") if is_post_deneb(spec): parent_beacon_block_root = bytes.fromhex("0000000000000000000000000000000000000000000000000000000000000000") if is_post_electra(spec): - requests_root = sha256(b"").digest() + requests_hash = sha256(b"").digest() payload_header.block_hash = compute_el_header_block_hash( spec, @@ -82,7 +82,7 @@ def get_sample_genesis_execution_payload_header(spec, transactions_trie_root, withdrawals_trie_root, parent_beacon_block_root, - requests_root, + requests_hash, ) return payload_header