engine: Make execution requests a sidecar, take 2

[mkalinin]

Alternative to:

• engine: unify request objects #565
• engine: unified list of opaque requests  #577

Proposal

• Drop requests from EL block and only keep requestsRoot commitment in the EL block header, originates from engine: Return and accept EL triggered requests as a sidecar #551 and engine: Return and accept EL triggered requests as a sidecar #551 (comment)
• Obtain and pass requests from getPayload and newPayload as a sidecar using byte sequence representation similar to engine: unify request objects #565
• Make EL validate requestsHash as a part of blockHash validation, note there is no need to keep requests_hash in the beacon block. CLs that run blockHash validation on their own will still be able to do that by implementing the algo EL uses to compute requestsHash
• Make EL validate requestsHash in the EL block header against requests obtained from transaction execution
• Make system smart contracts prepending a type byte to each encoded request record, this results in a smart contract returning a ready to be served request list, i.e. requests_01 = withdrawalContract.sysCall(), requests_02 = consolidationContract.sysCall(); deposit contract will always be an exception
• Use concatenation to compute the resulting list of all requests instead of RLP, i.e. do executionRequests = requests_00 || requests_01 || requests_02 instead of requests = RLP([requests_00, requests_01, requests_02]). And use the executionRequests in Engine API and in requestsHash computation, the latter can be as simple as requestsHash = executionRequests(requests)

Properties

• Adding a new request type on EL becomes as simple as specifying a new request smart contract address and that’s it, everything else is by generic computations
• CL has to decode/encode requests from/to byte sequence representation defined by the EIP-7685, but this is reasonable as the CL is the consumer of the requests and EL just surfaces them
• Optimistic sync is safe as the blockHash validation ensures that the beacon block contains requests that EL payload is committed to and then, upon executing a block, EL ensures that the same commitment corresponds to the requests obtained from transaction execution. These two steps ensure that either requests in the beacon block are the same as those produced by the transaction execution or the payload is INVALID

ToDo

• Update EIP-7685 by:
  • removing requests from the block body
  • switching requests list encoding from RLP to concatenation and updating requestsHash computation
• Update system smart contracts to prepend the type byte to each request

[lucassaldanha]

I love the idea of removing the requests from EL block. We should keep iterating on that.

However, when it comes to the Engine API definition, I think we should consider an approach where we send a list of opaque representations of the requests (like an array of byte arrays), similar to what was proposed here. This approach has the following benefits:

1. EL does not need to know about the structure of each request type;
2. We don't need to update the Engine API every time a new request type is added;

The requirements for that approach are:

1. Contracts that create the requests need to return the requests in the same structure the CL is expecting them (maybe as an encoded object?)
2. CL has to decode the data and respect the order they appear on the array.

[mkalinin]

After a fruitful discussion with @lucassaldanha I have updated the PR and its description with the design we came up to. The final design we propose stems from removing requests from EL block and passing them as a sidecar to the payload and leverages on the requests encoding proposed in #565

[mkalinin]

A thought after the ACDE call: we can use executionRequests representation as in #577, i.e. make it a list of byte arrays, this should still allow for a perfect generalization on the EL side and should make parsing requests on CL side more convenient

[fjl]

I think it's better to use the flat DATA encoding because then it matches the input for the requestsHash commitment exactly.

[fjl]

Here is the update to the system contracts (WIP): lightclient/sys-asm#22

[marioevz]

A thought after the ACDE call: we can use executionRequests representation as in #577, i.e. make it a list of byte arrays, this should still allow for a perfect generalization on the EL side and should make parsing requests on CL side more convenient

Just for the sake of my own understanding, we expect to always (in Prague that is) see a list here with three elements (one for each request type) every block and independent of whether there are requests of a given type or not, is that correct?

E.g.

[
  "0x00... 00... 00...",  # Three deposits here
  "0x",  # No withdrawals
  "0x02..."  # One consolidation
]

(So not exactly as #577 because to separate each request into a new array element we would need to parse the contract response)

Edit: I just saw fjl comment about how single flat data is better, I'll wait until this is settled before updating the tests.

[mkalinin]

(So not exactly as #577 because to separate each request into a new array element we would need to parse the contract response)

EL would not have to parse data to serve request lists separately. Each list is obtained from the corresponding smart contract and then concatenated into a single flat list to compute the requestsHash. We have two options in Engine API: a) serve the concatenated list with all requests b) serve three lists separately, as like in your example.

Obviously, (a) is better from EL perspective, while option (b) isn’t something complicated to do either. So, I would like to hear from CL client devs on that

[tersec]

(So not exactly as #577 because to separate each request into a new array element we would need to parse the contract response)

EL would not have to parse data to serve request lists separately. Each list is obtained from the corresponding smart contract and then concatenated into a single flat list to compute the requestsHash. We have two options in Engine API: a) serve the concatenated list with all requests b) serve three lists separately, as like in your example.

Obviously, (a) is better from EL perspective, while option (b) isn’t something complicated to do either. So, I would like to hear from CL client devs on that

I significantly prefer (b).

The only specific utility of the flat list is for hashing, and that can be done by init(), then update() on each sublist in turn, then finalize. The flat is on its own isn't useful to either the EL or CL.

[marioevz]

(So not exactly as #577 because to separate each request into a new array element we would need to parse the contract response)

EL would not have to parse data to serve request lists separately. Each list is obtained from the corresponding smart contract and then concatenated into a single flat list to compute the requestsHash. We have two options in Engine API: a) serve the concatenated list with all requests b) serve three lists separately, as like in your example.
Obviously, (a) is better from EL perspective, while option (b) isn’t something complicated to do either. So, I would like to hear from CL client devs on that

I significantly prefer (b).

The only specific utility of the flat list is for hashing, and that can be done by init(), then update() on each sublist in turn, then finalize. The flat is on its own isn't useful to either the EL or CL.

I agree, I'm even less inclined to a single flat byte string now.

Another advantage of three-element list (or N element for future forks) is that you can modulo the length of each element in the list against the length of a single request for the given type and immediately validate it has the correct size.

We could even skip the request-type byte at the beginning of every request this way.

Wdyt @fjl @mkalinin ?

[fjl]

I think it's better to serve the requests as a single flat byte array because that is also how they will be hashed to create the requestsHash in the block header. For example, if a block has one deposit (D₁), two withdrawal requests (W₁, W₂), and one consolidation request (C₁), the requestsHash commitment can be sha256(D₁ || W₁ || W₂ || C₁).

So I think we should relay the requests to the CL as the byte array D₁ || W₁ || W₂ || C₁ because this makes it trivial to check the hash with no extra encoding step. In order to process the requests, the CL just has to parse them from the byte array, which is trivial because the size of each request type is fixed and known to the CL.

[marioevz]

I think it's better to serve the requests as a single flat byte array because that is also how they will be hashed to create the requestsHash in the block header. For example, if a block has one deposit (D₁), two withdrawal requests (W₁, W₂), and one consolidation request (C₁), the requestsHash commitment can be sha256(D₁ || W₁ || W₂ || C₁).

So I think we should relay the requests to the CL as the byte array D₁ || W₁ || W₂ || C₁ because this makes it trivial to check the hash with no extra encoding step. In order to process the requests, the CL just has to parse them from the byte array, which is trivial because the size of each request type is fixed and known to the CL.

I agree that it is a bit simpler to hash on a flat array but having a list of byte arrays you only need to do two byte array concatenations (or N-1 on future forks where N is the number of request types) and you end up with the same information before hashing, because given D = D₁, W = W₁ || W₂ and C = C₁, D || W || C = D₁ || W₁ || W₂ || C₁.

[fjl]

Yes I understand. But there is another benefit to the flat array, and it has to do with abstraction. At the moment, we are focused on the three request types coming out of three distinct contract invocations. However, this may not always be the case in the future.

We could later decide that the withdrawal request queue logic should be changed and have a more complex behavior. We can't change the contract, but we could just add another system contract then and make it output the same withdrawal events. Or we could have a future system contract that outputs multiple kinds of events.

For this reason, I strongly believe we should go with the flat byte array returning the contracts return data as-is. It removes the need for the EL to know anything about the output, and it allows the CL to be ignorant about the source of the requests.

We should probably also remove the type-ordering requirement in CL validation. The only thing we really need to specify is the system contract call order for the EL.

[pawanjay176]

Also prefer (b), but I kinda understand the motivation for a flat list.

If we are going with (b), the spec should be clear on the validity of ordering within the flat list. Should we expect the EL to return an ordered by type flat list?

For e.g. if we receive [D₁ , W₁ ,D₂, C₁ , W₂ ], should we

1. set the execution_requests object in the beacon block body as
   deposits: [D₁ , D₂ ]
   withdrawals: [W₁ ,W₂]
   consolidations: [C₁ ]
2. return a deserialisation error
3. Ignore the out of order types and set it as
   deposits: [D₁ ]
   withdrawals: [W₁ ]
   consolidations: [C₁ ]

(3) doesn't make sense to me, I'm unclear if we should do (1) or (2)

[lightclient]

I updated the PR to send exeucitonRequestsHash instead of full requests, per the decision on the testing call today. PTAL.

[marioevz]

LGTM.