-
Notifications
You must be signed in to change notification settings - Fork 386
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.
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
engine: Return and accept EL triggered requests as a sidecar #551
Conversation
|
Requests become a part of the
For deposit requests it would be possible, but withdrawal requests aren’t emitting any events so they can’t be proved using the
It is not required as per this proposal requests aren’t included into the EL block. |
Use case for proving against
Using a single mechanism would simplify such consumers, so maybe emitting an event as a log should be considered, similar to how there is a log for deposits |
It sounds to me that emitting an event for each request is orthogonal to this proposal as currently there are no withdrawal request events, and the only way to prove them against the EL block structure is to use an MPT commitment to the list of requests in each EL block. I believe staking pools can build proofs linked to beacon block roots if this is needed. It is a good question if such information can be useful to JSON-RPC consumers as actually exits are happening today (CL triggered though) and JSON-RPC does not provide the access to this data. |
JSON-RPC provides withdrawals (as part of Agree that the proof itself does not have to be solved today. But JSON-RPC should expose the data in some form that is theoretically provable against on-chain commitments. A log serves that purpose. You are right that for purely CL triggered events, that they are not available directly via JSON-RPC. As they are CL originated, though, they can be checked via EIP-4788. However, here, user triggered withdrawals are EL originated, and are only available in the subsequent block via EIP-4788 (as that one points to the previous slot). So it's not ideal to have certain EL data that requires pulling from an entirely different API. |
Also interested in exploring other solutions besides logs. It just seems the most intuitive one, given how deposits work, and also how EIP-4788 works via a system contract so that EVM code can also access it. |
How do you ensure correctness of the deposit / withdrawal requests during optimistic sync? |
A node will rely on the chain it is being synced with is a valid chain with the same assumptions as with the existing design. |
As I understand, the deposit/withdrawal/consolidation requests will no longer be part of the EL block hash. With the old design, the CL can optimistically validate the EL block hash. Once the EL syncs up, it can use the 'last valid hash' mechanism to say what portion of the chain is invalid. With the new design, the BeaconBlockBody fields are no longer tied to the EL block hash, so it becomes possible to have bogus entries in the BeaconBlockBody that still have a VALID ExecutionPayload. |
Consider an invalid withdrawal request In both cases a syncing node can’t conclude that The above case stems from the following property. If a chain has an invalid state modification and being synced beyond the point of that modification via state sync, it won’t be possible to test whether any invalid modification has been made to that chain in the past, and new valid blocks extending this chain will be applied without any issue. |
Waiting to discuss with prysm teammates on this one. Could some notes on rationale be added to the description? seems very doable but I feel a bit naive on the benefits/tradeoffs ( I missed the original discussion I believe) |
@james-prysm added the motivation section in the PR description |
I’ve added an Optimistic sync attack opened by this proposal to the PR description. Thanks @etan-status and @ethDreamer for raising your concerns around the fact that this proposal can break the optimistic sync. The scenario described by the optimistic sync attack felt out of my consideration when I did the initial analysis 😬️️️️️️ @etan-status this is probably the case you had in mind behind #551 (comment) |
Yes. The CL has no chance to verify that the requests within the sidecar relate to the ExecutionPayload, so requires to loop in the EL on each individual block. In contrast, if the requests are part of the execution block hash, the CL can verify that the requests are linked to the ExecutionPayload, and because its a blockchain any EL verdict on a later block also confirms all prior data to be valid, while latestValidHash tells you exactly from where things went wrong.
That may not be viable if the data is too far in the past, due to pruning. It's also not too performant -- right now during sync Nimbus only sends a newPayload / fcu every couple 1000 blocks to avoid interrupting ongoing sync.
Other alternative could be to simply mirror the ExecutionPayload, namely making separate block header fields for each of the trees. That's how things were done in the past, and also how things would look if the EL block header becomes transitioned to SSZ. As I understand, EIP-7685 is primarily motivated by implementation choices that make extending EL block header with new field annoying, but we could also just go through that annoyance to have a simpler spec design that is consistent with CL and JSON-RPC, both of which have separate fields per request type. |
I think we can still avoid this edge case if we just keep the |
Yes, indeed
While with the existing design we have:
After weighing in the complexity of passing requests between the layers and having EL running the same checks as today, from my perspective this proposal has marginal benefits and might not worth the engineering complexity. |
Does it make sense that the beacon block body has Deposits and DepositRequests? I'm finding that a bit odd / strange. Edit: Discussed offline that it is necessary for the fork transition and eth1data voting period after the fork. The field is obsolete in electra, but not immediately so. Perhaps it can be removed in the following fork. |
One another argument raised on the ACDE#189 is that having data on CL layer with the commitment to these data on the EL will make ePBS design more complicated. Nonetheless, closing the proposal as it appears that the complexity outweighs the benefits. |
Drafts an idea by @fjl and @lightclient to surface EL triggered requests as a sidecar to the ExecutionPayload.
Motivation
Simplify the EL part of the EL triggered requests design by avoiding requests inclusion into the ExecutionPayload structure.
Consensus layer
CL obtains deposit, withdrawal and requests of any other type in the response to the
engine_getPayload
method call in a similar way as blobs are obtained. And then CL includes obtained requests into aBeaconBlockBody
instead of theExecutionPayload
:When a beacon block is being processed, CL applies the requests as it is done today and also sends them via separate parameters to the
engine_newPayload
call to get them verified by the EL.Execution layer
EL is responsible to surface each request type in a separate list in the response to the
engine_getPayload
and validate that expected requests passed from the CL viaengine_newPayload
matches actual requests obtained from the block execution.This design is an alternative to the EIP-7685.
Security and other considerations
Since EL triggered requests are a product of block execution they only can be validated when a block is being fully executed, thus this change doesn’t affect the security of neither syncing nor online nodes. For syncing nodes the validation will still be happening only when the state is fully available which is happening near to the head of the chain, for online nodes full validation will be happening on each block.
The shift takes place on the request data commitment and storage part. Instead of duplicating requests between both layers it is proposed to keep them only on the CL — the layer which this requests are targeted for.
This proposal affects mev-boost flow in a way that the requests should be sent to the CL alongside to the
ExecutionPayloadHeader
. But this would also be required if requests were a part of theExecutionPayload
as the CL would need to apply them to its state in order to build a blinded beacon block.Optimistic sync attack
There is an optimistic sync attack vector opened by this proposal.
Consider a fully synced node that has experienced an outage with the following impact on the block's availability:
A: (EL, CL) <- B: (CL) <- C: (new malicious block) <- D (new block)
The node starts up with the head
B
which wasn't stored by the EL and then attempts to import malicious blockC
(a child ofB
):newPayload(C)
is responded withSYNCING
as the EL does not have the blockB
C
with the requests that were never triggered by the ELD
andnewPayload(D)
is responded withVALID
As a result of this attack the node would be tricked into following the chain containing malicious block.
It is important to say that after such an outage the EL of an already synced node will use full block execution (instead of the state sync) to catch up with the head of the chain.
Thus, in the case when requests are a part of the EL block (the existing design) step (4) from the above sequence of events would instead be as follows:
D
andnewPayload(D)
is responded with{status: INVALID, latestValidHash: B.block_hash}
Which will make the CL to reject the malicious chain and revert to block
B
.If an adversary finds a way to induce an outage on the majority nodes in the network, it can be leveraged to finalize a chain with invalid EL requests.
The mitigation of this attack is for EL to keep requests sent via
newPayload
around and validate them after each executed block. Potential problem with this mitigation is that CL clients may not send each block vianewPayload
during the optimistic sync thus the requests data may not always be available for the EL.Todo