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

add feeRecipientDiff as return value #150

Closed
wants to merge 2 commits into from

Conversation

thegostep
Copy link

@thegostep thegostep commented Dec 20, 2021

In order to perform profit switching between payloads constructed from multiple sources, the profit switcher needs to be made aware of the value of a payload produced locally and be able to measure the value of a payload produced by a third party.

For this, we need to define a standard way to measure the value of a payload and include it in the engine API.

I propose we define a new value called feeRecipientDiff which represents a signed integer denominating the change in balance of the feeRecipient address defined in the ExecutionPayload object.

I avoided adding feeRecipientDiff directly to the ExecutionPayload object as it seemed to introduce unnecessary complexity with sending the value around the network and handling it as part of block validity.

Instead, feeRecipientDiff can be added as a return value to the engine_executePayloadV1 and engine_getPayloadV1 functions.

This is currently spec'ed as a mandatory return value as I expect making it optional will cause setup confusion when users configure their nodes.

@mkalinin
Copy link
Collaborator

I have two concerns regarding proposed changes:

  • Engine API design space is narrowed to the minimal set of methods required for communication between CL and EL clients. We're keeping it lean as any feature on top of this core functionality would increase implementation and testing complexity.
  • The value of feeRecipient field in ExecutionPayload may not match suggestedFeeRecipient parameter of the corresponding forkchoiceUpdated method call -- the one that is announced by validator as an address where transaction fees should go to. IIUC, profit switcher should work with suggestedFeeRecipient balance, and the change proposed to executePayload won't properly work for the scheme when builder seeds feeRecipient field with its own address and pays validator via separate transaction. Though, this shouldn't be the case for local calls to getPayload as in this case feeRecipient will match suggestedFeeRecipient.

It sounds reasonable to consider local EL client as a safe net for the case when relay can't get back with a payload in time or at all due to failure. But how bad do we want profit switcher to assess payloads that are produced locally alongside with those that are received from relays? Aren't external ones are by default at least as profitable as local because local ones are built from a mempool which is also accessible by builders?

@thegostep
Copy link
Author

To summarize the feedback:

  1. we could profit switch on ending fee recipient balance instead of balance diff to remove implementation complexity
  2. if we use a merkle proof of the ending balance provided with the payload header, we don’t need to have feeRecipientDiff in executePayload
  3. we exclude the local execution client from profit switching and only fallback to it if no relays have provided a payload which would avoid the need for changing getPayload

@thegostep thegostep closed this Jan 18, 2022
@thegostep thegostep deleted the patch-1 branch January 18, 2022 13:43
@realbigsean
Copy link

But how bad do we want profit switcher to assess payloads that are produced locally alongside with those that are received from relays? Aren't external ones are by default at least as profitable as local because local ones are built from a mempool which is also accessible by builders?

If relays are likely to censor and a local EE doesn't, a local EE will have transactions available relays won't. Who knows how high a tx fee would be on a transaction that is censored in other places. I think it's important to include the local EE for the sake of censorship resistance generally and there is a scenario where censorship on relays is extreme enough to make a local payload's uncensored transactions comparably valuable. We also have users requesting logic around "use an external builder if it is more profitable than X". Adding a way to get payload value from the local EE can enable this comparison. It seems there's a price at which people are willing to use a censored service and this would help discover this price, I don't think people should be forced into the binary option of using a builder always or not.

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

Successfully merging this pull request may close these issues.

3 participants