Merge pull request #3998 from jtraglia/exclude-empty-requests

Exclude empty requests in requests list

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)
@@ -138,6 +139,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
@@ -1168,11 +1177,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`

specs/electra/validator.md

@@ -189,18 +189,55 @@ 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. 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
-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)
+def get_execution_requests(execution_requests_list: Sequence[bytes]) -> ExecutionRequests:
+    deposits = []
+    withdrawals = []
+    consolidations = []
+
+    request_types = [
+        DEPOSIT_REQUEST_TYPE,
+        WITHDRAWAL_REQUEST_TYPE,
+        CONSOLIDATION_REQUEST_TYPE,
+    ]
+
+    prev_request_type = None
+    for request in execution_requests_list:
+        request_type, request_data = request[0:1], request[1:]
+
+        # 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:
+            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

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]

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)
 

tests/core/pyspec/eth2spec/test/electra/unittests/test_execution_requests.py

@@ -0,0 +1,119 @@
+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__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:
+        pass
+
+
+@with_electra_and_later
+@spec_test
+@single_phase
+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:
+        pass
+
+
+@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:
+        pass
+
+
+@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:
+        pass