Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

eip7685: Pass execution_requests_hash to is_valid_block_hash #3950

Closed
wants to merge 10 commits into from
+46 −19
Closed

eip7685: Pass execution_requests_hash to is_valid_block_hash #3950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8f2cb7c
Pass execution_requests_hash to notify_new_payload
mkalinin Sep 30, 2024
a00d1fd
Apply suggestions by @ralexstokes
mkalinin Oct 1, 2024
69b7be5
Fix expression order in verify_and_notify_new_payload
mkalinin Oct 1, 2024
21fdd85
Pass execution_requests_hash only to is_valid_block_hash
mkalinin Oct 1, 2024
601631f
Send execution_requests to EE as a list of byte sequences
mkalinin Oct 3, 2024
6f4cc2b
Revert "Send execution_requests to EE as a list of byte sequences"
jtraglia Oct 7, 2024
ca62ad1
Fix hash computation
jtraglia Oct 7, 2024
3e3eec6
Delete trailing whitespace
jtraglia Oct 7, 2024
e02c2ee
Merge remote-tracking branch 'upstream/dev' into el-requests
jtraglia Oct 7, 2024
04b2a55
Compute requests hash in verify_and_notify_new_payload
jtraglia Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pysetup/spec_builders/electra.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class ElectraSpecBuilder(BaseSpecBuilder):
def imports(cls, preset_name: str):
return f'''
from eth2spec.deneb import {preset_name} as deneb
from eth2spec.utils.ssz.ssz_impl import serialize
'''

@classmethod
Expand All @@ -28,7 +29,6 @@ class NoopExecutionEngine(ExecutionEngine):

def notify_new_payload(self: ExecutionEngine,
execution_payload: ExecutionPayload,
execution_requests: ExecutionRequests,
parent_beacon_block_root: Root) -> bool:
return True

Expand All @@ -45,7 +45,8 @@ def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> GetPayloadRespo

def is_valid_block_hash(self: ExecutionEngine,
execution_payload: ExecutionPayload,
parent_beacon_block_root: Root) -> bool:
parent_beacon_block_root: Root,
execution_requests_hash: Hash32) -> bool:
return True

def is_valid_versioned_hashes(self: ExecutionEngine, new_payload_request: NewPayloadRequest) -> bool:
Expand Down
60 changes: 43 additions & 17 deletions specs/electra/beacon-chain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -77,13 +78,14 @@
- [Request data](#request-data)
- [Modified `NewPayloadRequest`](#modified-newpayloadrequest)
- [Engine APIs](#engine-apis)
- [Modified `notify_new_payload`](#modified-notify_new_payload)
- [Modified `is_valid_block_hash`](#modified-is_valid_block_hash)
- [Modified `verify_and_notify_new_payload`](#modified-verify_and_notify_new_payload)
- [Block processing](#block-processing)
- [Withdrawals](#withdrawals)
- [Modified `get_expected_withdrawals`](#modified-get_expected_withdrawals)
- [Modified `process_withdrawals`](#modified-process_withdrawals)
- [Execution payload](#execution-payload)
- [New `compute_execution_requests_hash`](#new-compute_execution_requests_hash)
- [Modified `process_execution_payload`](#modified-process_execution_payload)
- [Operations](#operations)
- [Modified `process_operations`](#modified-process_operations)
Expand Down Expand Up @@ -136,6 +138,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
Expand Down Expand Up @@ -978,23 +988,22 @@ class NewPayloadRequest(object):
execution_payload: ExecutionPayload
versioned_hashes: Sequence[VersionedHash]
parent_beacon_block_root: Root
execution_requests: ExecutionRequests # [New in Electra]
execution_requests: ExecutionRequests # [New in Electra:EIP7685]
```

#### Engine APIs

##### Modified `notify_new_payload`
##### Modified `is_valid_block_hash`

*Note*: The function `notify_new_payload` is modified to include the additional `execution_requests` parameter in Electra.
*Note*: The function `is_valid_block_hash` is modified to include the additional `execution_requests_hash` parameter for EIP-7685.

```python
def notify_new_payload(self: ExecutionEngine,
execution_payload: ExecutionPayload,
execution_requests: ExecutionRequests,
parent_beacon_block_root: Root) -> bool:
def is_valid_block_hash(self: ExecutionEngine,
execution_payload: ExecutionPayload,
parent_beacon_block_root: Root,
execution_requests_hash: Hash32) -> bool:
"""
Return ``True`` if and only if ``execution_payload`` and ``execution_requests``
are valid with respect to ``self.execution_state``.
Return ``True`` if and only if ``execution_payload.block_hash`` is computed correctly.
"""
...
```
Expand All @@ -1011,19 +1020,22 @@ def verify_and_notify_new_payload(self: ExecutionEngine,
Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``.
"""
execution_payload = new_payload_request.execution_payload
execution_requests = new_payload_request.execution_requests # [New in Electra]
parent_beacon_block_root = new_payload_request.parent_beacon_block_root
jtraglia marked this conversation as resolved.
Show resolved Hide resolved

if not self.is_valid_block_hash(execution_payload, parent_beacon_block_root):
# [New in Electra]
execution_requests_hash = compute_execution_requests_hash(new_payload_request.execution_requests)

# [Modified in Electra:EIP7685]
if not self.is_valid_block_hash(
execution_payload,
parent_beacon_block_root,
execution_requests_hash):
return False

if not self.is_valid_versioned_hashes(new_payload_request):
return False

# [Modified in Electra]
if not self.notify_new_payload(
execution_payload,
execution_requests,
execution_payload,
parent_beacon_block_root):
return False

Expand Down Expand Up @@ -1139,6 +1151,20 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None:

#### Execution payload

##### New `compute_execution_requests_hash`

*Note*: Computes commitment to the execution layer triggered requests data.
The computation algorithm is defined by [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685).

```python
def compute_execution_requests_hash(execution_requests: ExecutionRequests) -> Hash32:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we can do:

def compute_execution_requests_hash(execution_requests: ExecutionRequests) -> Hash32:
    request_list = [execution_requests.deposits, execution_requests.withdrawals, execution_requests.consolidations]
    hash_list = []
    for index, requests in enumerate(requests_list):
        request_type = uint_to_bytes(uint8(index))
        hash_list.append(hash(request_type + serialize(requests)))

    return hash(b''.join(hash_list))

This allows to get rid of explicit type bytes in the spec and easy to extend in the future by adding requests to the flat list used for commitment computation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we wanted to get rid of these custom one-off hashing schemes that are just a mess to maintain over time?

deposit_requests = DEPOSIT_REQUEST_TYPE + serialize(execution_requests.deposits)
withdrawal_requests = WITHDRAWAL_REQUEST_TYPE + serialize(execution_requests.withdrawals)
consolidation_requests = CONSOLIDATION_REQUEST_TYPE + serialize(execution_requests.consolidations)

return hash(hash(deposit_requests) + hash(withdrawal_requests) + hash(consolidation_requests))
```

##### Modified `process_execution_payload`

*Note*: The function `process_execution_payload` is modified to pass `execution_requests` into `execution_engine.verify_and_notify_new_payload` (via the updated `NewPayloadRequest`).
Expand All @@ -1160,9 +1186,9 @@ def process_execution_payload(state: BeaconState, body: BeaconBlockBody, executi
assert execution_engine.verify_and_notify_new_payload(
NewPayloadRequest(
execution_payload=payload,
execution_requests=body.execution_requests, # [New in Electra]
versioned_hashes=versioned_hashes,
parent_beacon_block_root=state.latest_block_header.parent_root,
execution_requests=body.execution_requests, # [New in Electra]
)
)
# Cache execution payload header
Expand Down