Update EIP-7685: Update based on latest discussions

[lucassaldanha]

Taking a stab at updating this EIP to capture the design decisions we made on ethereum/execution-apis#591.

[lucassaldanha]

cc @lightclient

[eth-bot]

File EIPS/eip-7685.md

Requires 1 more reviewers from @lightclient

[github-actions]

The commit 008a695 (as a parent of 33cdb50) contains errors.
Please inspect the Run Summary for details.

[mkalinin]

It might worth mentioning a special case for an empty list, where request_data = requests_type, to make the spec more explicit

[etan-status]

I don't see any benefit for using SSZ here. The CL consumes this data via JSON (engine API), and uses a different data structure internally (ExecutionRequests) with strong typing.

What I find useful is the grouping by type, as in, having separate lists per request type. However, they can be a regular RLP array.

0x00_requests = 0x00_request_0 + 0x00_request_1 + 0x00_request_2   # without request type prefixes
0x01_requests = 0x01_request_0 + 0x01_request_1 + 0x01_request_2   # without request type prefixes
0x02_requests = 0x02_request_0 + 0x02_request_1 + 0x02_request_2   # without request type prefixes
requests_hash = keccak256(rlp([0x00_requests, 0x01_requests, 0x02_requests])) 

Engine would have a list of the concatenated requests:

{
    requests: [
        "0x...... 0x00_requests",
        "0x...... 0x01_requests",
        "0x...... 0x02_requests"
    ]
}

Each of the individual requests items coincides with the encoding for List[XyzRequest, AnyLimit] in SSZ, as long as the request data per item is constant sized. This is regardless of what the list limit actually is (SSZ doesn't encode the list length but computes it from the data length). Therefore, the CL can efficiently parse each of the request lists, and enables the EL to treat the data as opaque, while avoiding the introduction of random SSZ List types that are EL-only.

If the goal is a requests_root SSZ commitment in the EL, the root should match the hash_tree_root(beacon_block.body.execution_requests) to have any advantage. The advantage would be that in newPayload, instead of sending the full requests, the CL could just send the requests_root (self-computed), reducing the amount of JSON encoding that's necessary for block validation, and reducing the number of hashes that the EL has to do to validate newPayload. In such a scenario, the EL cannot treat the request content as opaque, though.

[mkalinin]

Encoding of a new request list is supposed to be implemented as a part of the corresponding smart contract, like it is done today already for withdrawals and consolidations. So, SSZ is the encoding that we assume can be implemented in a smart contract even if a request data will be of a dynamic size in the future. Given that request encoding becomes opaque for EL clients which is nice as any new request doesn’t require any work on EL side besides adding a new requests smart contract.

The benefit of requestsHash = sha256(0x00_requests || 0x01_requests || 0x02_requests) is that CL can already send requestsHash to EL in the Engine API instead of sending request data, and this approach is easy to implement without RLP.

[etan-status]

sha256(0x00_requests || 0x01_requests || 0x02_requests) is not the same as hash_tree_root(beacon_block.body.execution_requests).

[lucassaldanha]

sha256(0x00_requests || 0x01_requests || 0x02_requests) is not the same as hash_tree_root(beacon_block.body.execution_requests).

It is not the same. But it is trivial for CL to calculate it. And we avoid sending back the whole execution requests object that the EL does not care about. Not saying it can't be done though. I think there was a suggestion about it in the Discord thread.

[lucassaldanha]

I don't see any benefit for using SSZ here. The CL consumes this data via JSON (engine API), and uses a different data structure internally (ExecutionRequests) with strong typing.

The benefit is that each request list can be decoded to a list of corresponding request types. Each one of the lists of requests inExecutionRequest has its container type defined, so we can directly decode this list into a ssz list with container elements of the corresponding request type (DepositRequest, ConsolidationRequest and WithdrawalRequest).

[etan-status]

it may be trivial for the CL given that CL already uses SHA256 for SSZ hashes, but it is a second hash over the request data, so if the request data is large it can be expensive; but if the data is small, the optimization may be premature? Realistically, the extra requests data tops out around 4k when JSON encoded?

on ETH R&D Discord we have also identified a hash collision problem. imagine that all request types consist of just a single uint8 for these examples:

example 1:

• type 0 -> 0x01, 0x02, 0x02, 0x02
• type 1 -> none
• type 2 -> none

example 2

• type 0 -> none
• type 1 -> 0x02
• type 2 -> 0x02, 0x01, 0x02

both hash as sha256(0x00, 0x01, 0x02, 0x02, 0x02, 0x01, 0x02). So, if one were to swap out example 1 and example 2 in a beacon block, the EL would deem both as VALID.

I'm not sure how easy it is to construct a collision with the initial 3 types and their limits, but as I understand the idea of this request bus is to be generic and extensible, and at the very least the security section of the EIP should address the hashing method, and should prove that collisions are not possible at least for the proposed request types.

in my opinion, the bandwith savings on newPayload are not large enough to warrant introducing an experimental hashing method. it feels off to me that we are fine with exchanging the full request data via libp2p among beacon nodes, which is an inefficient link where latency matters, but then focus on optimizing a local connection where it is deemd fine to waste 50% of data on encoding binary data as hex strings. I understand that it is ugly to send unnecessary data to the EL, but we can also address the optimization in a future fork with alternatives such as SSZ block headers.

if we want to go with a special purpose hash just for requests, from a CL perspective I think it should be SHA256 based, and exclude MPT, RLP, and keccak (so the current proposal would satisfy that).

[lucassaldanha]

PSA: more on this convo on https://discord.com/channels/595666850260713488/1288408367068610585

[lucassaldanha]

Closing in favour of #8924