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.

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

[RFC] Engine API: Add block value in response to engine_getPayload #307

Closed
allboxes opened this issue Sep 16, 2022 · 18 comments
Closed

[RFC] Engine API: Add block value in response to engine_getPayload #307

allboxes opened this issue Sep 16, 2022 · 18 comments

Comments

@allboxes
Copy link
Contributor

Currently the CL is blind to the value of the locally built block and therefore, if configured with a builder, will always propose the remotely built block.

In order to allow the CL to intelligently decide between using the local or remote block, the EL could provide the value of its block in its response to engine_getPayload. The EL can be free to determine the value of its block as it sees fit (summing tx fees, checking feeRecipient balance pre/post execution, etc..).

The change will help to ensure validators propose the highest paying block in general, but more importantly will increase the cost of censorship by builders.

@mkalinin
Copy link
Collaborator

Suggested approach is to add engine_getPayloadV2 with the new response type including feeRecipientBalance

@allboxes
Copy link
Contributor Author

feeRecipientBalance meaning the new balance of the feeRecipient or the change in balance? It seems like the CL would need the change in the balance ("feeRecipientBalanceDelta")

@mkalinin
Copy link
Collaborator

feeRecipientBalance meaning the new balance of the feeRecipient or the change in balance? It seems like the CL would need the change in the balance ("feeRecipientBalanceDelta")

Can't we simply compare balances? Balance value with respect to the diff has lower proving complexity. For instance, to prove the diff we would have to have merkle proofs linked to pre and post block state roots. Proving the value does require only the latter.

@lightclient
Copy link
Member

A bit of additional context: #150

@allboxes
Copy link
Contributor Author

allboxes commented Sep 21, 2022

Can't we simply compare balances? Balance value with respect to the diff has lower proving complexity. For instance, to prove the diff we would have to have merkle proofs linked to pre and post block state roots. Proving the value does require only the latter.

I see there is a parallel discussion on proving external builder payments here: ethereum/builder-specs#51

Proving external builder's bids seems useful, but do we need to be concerned with proving in the execution API or can we assume honesty there?

If we can assume honesty from the execution API then it seems simpler to provide the balance delta ("bid") directly to the CL so it can compare it against any builders bid's (which may or may not have been proved through a separate builder flow). I can see it working either way though.

@mkalinin
Copy link
Collaborator

I see there is a parallel discussion on proving external builder payments here: ethereum/builder-specs#51

From the proposed change:

Merkle proof of post balance of the fee_recipient account

So, we actually need the post balance to compare local payload with external one

@g11tech
Copy link
Contributor

g11tech commented Sep 25, 2022

So, we actually need the post balance to compare local payload with external one

Correct ✔️ It would be good to bundle post balance of fee_recipient

As the EL<>CL connection is already trusted, so no merkle proof required for engine response BUT... it would be good to have the merkle proof since this is yet an unexecuted block (not in the strickest sense though since EL would have executed the block after construction) and hence getProof/getBalance for post balance can't be done.
(but still prudent to include getProof/getBalance on the engine api methods for pre balance fetchers/verification)

The post balance from here can directly be compared with the builder's balance (+ extra merkle proof verification for builder balance) and the greater one can be simply picked (since one will have both builder and engine responses while making the decision)

@mkalinin
Copy link
Collaborator

Interesting thought. Could a builder software leverage the proof obtained from getPayload?

@allboxes
Copy link
Contributor Author

Suggested approach is to add engine_getPayloadV2 with the new response type including feeRecipientBalance

Thinking about starting to work on PR for this. @potuz mentioned an idea which I think could make implementation a lot cheaper... We could leave the existing methods/data structures alone and add a new method which just retrieves the value/post balance of a given payloadId.

We can use the value cached at the time of the last engine_getPayloadV1 call to avoid synchrony issues.

This way all the existing tests etc don't have to be touched. Any thoughts on this?

@g11tech
Copy link
Contributor

g11tech commented Oct 12, 2022

Interesting thought. Could a builder software leverage the proof obtained from getPayload?

yes they can directly pass it in the BuilderBid

@g11tech
Copy link
Contributor

g11tech commented Oct 12, 2022

Suggested approach is to add engine_getPayloadV2 with the new response type including feeRecipientBalance

Thinking about starting to work on PR for this. @potuz mentioned an idea which I think could make implementation a lot cheaper... We could leave the existing methods/data structures alone and add a new method which just retrieves the value/post balance of a given payloadId.

We can use the value cached at the time of the last engine_getPayloadV1 call to avoid synchrony issues.

This way all the existing tests etc don't have to be touched. Any thoughts on this?

tbh a proof would be so much better and would like to have it bundled in the same response to save round trip time and random api error changes, and as @mkalinin observed, builders can use it directly

@mkalinin
Copy link
Collaborator

Suggested approach is to add engine_getPayloadV2 with the new response type including feeRecipientBalance

Thinking about starting to work on PR for this. @potuz mentioned an idea which I think could make implementation a lot cheaper... We could leave the existing methods/data structures alone and add a new method which just retrieves the value/post balance of a given payloadId.

We can use the value cached at the time of the last engine_getPayloadV1 call to avoid synchrony issues.

This way all the existing tests etc don't have to be touched. Any thoughts on this?

With introduction of engine_getPayloadV2 all existing tests won't be touched as well, we will have to create new ones. I feel like having reading from the same instance of payload build process via different methods may be a complexity vs returning everything at once and destroying the process thereafter. Although, ELs are using timeout to get rid of the result of the recent build process, I feel that a new method returning all at once would be cleaner, CLs won't have to handle the case when the payload is requested successfully but the balance request failed as @g11tech mentioned

@allboxes
Copy link
Contributor Author

allboxes commented Oct 14, 2022

Fair enough, I can see the value in both approaches and don't have a good enough view into all the CL/EL codebases to feel strongly about creating a standalone value retrieval method, so I'll draft up a engine_getPayloadV2 PR.

@g11tech can you elaborate a bit on why a proof is valuable here? If the only value is for builders, I'd lean towards putting that complexity on the builders, who are likely already heavily modifying the block building code to generate their blocks, and keeping as much complexity out of the core EL implementations as possible.

ETA: Just re-read the comments on the builder API adding payment proof and it sounds like relay is already validating the scores of the builder blocks and they can just generate the proof there, so builders won't even necessarily need to implement a proof.

@g11tech
Copy link
Contributor

g11tech commented Oct 15, 2022

Fair enough, I can see the value in both approaches and don't have a good enough view into all the CL/EL codebases to feel strongly about creating a standalone value retrieval method, so I'll draft up a engine_getPayloadV2 PR.

@g11tech can you elaborate a bit on why a proof is valuable here? If the only value is for builders, I'd lean towards putting that complexity on the builders, who are likely already heavily modifying the block building code to generate their blocks, and keeping as much complexity out of the core EL implementations as possible.

ETA: Just re-read the comments on the builder API adding payment proof and it sounds like relay is already validating the scores of the builder blocks and they can just generate the proof there, so builders won't even necessarily need to implement a proof.

if we assume the el<>cl link to be 100% trusted (which we already do as per the architecture of el/cl coupling), yes we do not require a proof, if thats too much of a burden. as per my understanding most ELs have getProof implemented so should not really be difficult to add this step (for the mev builders to add to their implementaions)

So i am ok with this just being a value that can be directly compared against other engine/builder candidates 👍

@allboxes
Copy link
Contributor Author

ethereum/go-ethereum#25998 Geth has started implementing this with the total of the transaction fees (i.e. feeRecipient balance diff) as the value returned with getPayload. Additionally it sounds like MEV-Boost will also lean towards proving the final payment amount to the feeRecipient as opposed to the final balance (ethereum/builder-specs#51 (comment)), so is everyone ok if we use the balance diff a.k.a. block value as the new value returned with ExecutionPayloadV2?

@g11tech
Copy link
Contributor

g11tech commented Oct 17, 2022

ethereum/go-ethereum#25998 Geth has started implementing this with the total of the transaction fees (i.e. feeRecipient balance diff) as the value returned with getPayload. Additionally it sounds like MEV-Boost will also lean towards proving the final payment amount to the feeRecipient as opposed to the final balance (ethereum/builder-specs#51 (comment)), so is everyone ok if we use the balance diff a.k.a. block value as the new value returned with ExecutionPayloadV2?

Yes I think the fees paid out, i.e. balance diff of ONLY fees make sense as well and will be in line with the "payment transaction scoring" of the builder blocks

@LukaszRozmej
Copy link

feeReceipient balance changes can come from two things: fees, direct transfer to that address. Do we want to include both or just the former here? I see Geth did the former only, in pre-merge MEV in flashbots spec it was the sum that was used to compare blocks.

@allboxes
Copy link
Contributor Author

Block value (and withdrawals) added to spec with engine_getPayloadV2 #314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants