Remove use of `engine_forkchoiceUpdatedV1`

[lightclient]

Currently engine_forkchoiceUpdatedV1 is used to communicate to relays the feeRecipient that a validator will be using. As a side effect, a lot of unnecessary information is communicated to the relays.

I propose two potential alternatives:

1. builder_getPayloadHeader(hash, feeRecipient) -> PayloadHeader
2. builder_preparePayload(hash, feeRecipient) -> uint64 + builder_getPayloadHeader(payloadId) -> PayloadHeader

I don't know if I have enough of an understanding of the latency of 1) to choose it over 2), but it is naively my preference.

[mkalinin]

I think that entire PayloadAttributes object is worth being communicated to relays. Ofc, this information is available in the network, but it probably would be easier for builders to receive it from the outside as in this case builders will not have to query CL to obtain this information. Also, querying CL isn't that easy to do in general case, suppose, there are two competing forks, a builder will have to understand which fork a validator is proposing on and make the corresponding query to CL -- and all information the builder has is hash, so, it will query CL by execution block hash which may be inconvenient.

I also think that it might worth communicating the attributes in advance as per (2). In this case feeRecipient and other payload attributes that may affect the result of transaction execution will be known to builders in advance aiding creation of a better block.

[come-maiz]

From @djrtwo in https://notes.ethereum.org/WX1hwcMRRzy5oh-zHyzqOA?view

use of forkchoiceUpdated

reuse of the engine_forkchoiceUpdatedV1 semantics seems off
I understand that some of the same info is to be communicated (e.g. payloadBuildAttributes) but to use the same namespace engine_ leads to a less clear separation of concerns and could lead to misunderstandings or accidental bugs
the description of the namespaces (under header API) conflicts with actual usage (i.e. engine is one of the methods called to the sidecar). suggest again for there to be a new builder_ endpoint that initiates the mev-build process rather than reusing engine_forkchoiceUpdated
I would suggest builder_beginPayloadBuildProcessV1 for BN->mev_boost and relay_begin_payloadBuildProcessV1 for mev_boost->relay

builder_beginPayloadBuildProcessV1 {
    headBlockHash,
    payloadAttributes
}

relay_beginPayloadBuildProcessV1 {
    headBlockHash,
    payloadAttributes
}

# if relay cannot build on requested headBlockHash, then can return null
# or if when requesting the payloadHeader via the payloadId, it can return null if it couldn't build anything

I don’t really think the beacon node is actually updating the forkchoice of these external builders (by default). they are sending a request to perform a build process. Such a builder could update it’s local node forkchoice to meet the response but it’s not really a dictate to update the forkchoice and smashing the semantics together here is likely to lead to confusion or unintended consequences

[metachris]

I'd very much agree that the semantics are off, both in using the engine_ prefix as well as using the forkchoiceUpdated in general. In practice, mev-boost wouldn't actually update the forkchoice of the relays.

I think the cleanest way would be to use the proposal of builder_beginPayloadBuildProcessV1 and relay_beginPayloadBuildProcessV1 instead of forkchoiceUpdated.

If that's too big of a change, we should at least change engine_forkchoiceUpdatedV1 to builder_forkchoiceUpdatedV1 and relay_forkchoiceUpdatedV1, although my preference would be to use builder_beginPayloadBuildProcessV1.

[lightclient]

IIUC, one goal is to remove any kind of synchronous communication from the critical path of block building. For that reason, it feels like we should avoid flows that require an "initialization" call. All parameters other than feeRecipient can be derived trivially from the beacon chain and feeRecipient can be bound to a validator asynchronously during the epoch prior to their proposal slot. Builders can store that information and retrieve it when they are ready to build a block for a certain slot. In which case, the only method we'd need is builder_getPayloadHeader(hash). Thoughts?

[metachris]

I'm trying to think through if there's any issue with option 1, removing the init call and having the attributes be part of getPayloadHeader...

[mkalinin]

If we suppose a builder will read all other values from the beacon state, then sending parentBeaconBlockRoot as a parameter to builder_getPayloadHeader might be more convenient for a builder, for instance, parentBlockHash may also be read from the beacon state.

If we don't have a prepare phase then how builder will know which fork a validator is proposing on in case of competing forks? Or is it supposed for a builder to prepare blocks for all forks to be ready to serve a payload on whatever fork a validator chooses?

[lightclient]

Or is it supposed for a builder to prepare blocks for all forks to be ready to serve a payload on whatever fork a validator chooses?

I think we should push the burden to the builder to monitor all potential forks and build on top of all of them. In milestone one, it may make sense to just tell the builder what fork you're on, but as we move to a p2p approach to maintain privacy, I think the latency will cause the builder to lose much of the advantage of building with confidence that the validator is on the fork.

[metachris]

Got some feedback from @jparyani on this too -- it seems feasible for builders to prepare blocks for all forks, and serve a payload for whatever fork the validator wants immediately. Seems like we could actually go with option 1.

The remaining question would be whether to include the full PayloadAttributesV1 object or only feeRecipient, besides the headBlockHash.

builder_getPayloadHeader(headBlockHash, feeRecipient) -> PayloadHeader

or

builder_getPayloadHeader(headBlockHash, payloadAttributes) -> PayloadHeader

[terencechain]

If the builder needs access to the parent beacon state or beacon block, it may be more accessible to use headBeaconBlockRoot than headBlockHash as the first argument

builder_getPayloadHeader(headBeaconBlockRoot, feeRecipient or payloadAttributes) -> PayloadHeader

From the consensus client, at least in Prysm, it's easier to look up beacon state, beacon block, and payload using the block root than using payload hash

[jparyani]

If the builder needs access to the parent beacon state or beacon block, it may be more accessible to use headBeaconBlockRoot than headBlockHash as the first argument

I’m not seeing a case where the builder would need access to the beacon chain state, but I might be missing something, @terencechain do you have any examples?

[terencechain]

I also don't see it, but I think it was mentioned a few times above, if the builder doesn't need to access the beacon chain state then the payload hash shall work just fine

[thegostep]

If going with the builder_getPayloadHeader approach, would mev-boost still performing polling or do a single call to all relays? If a single call, how long would it wait for a response before returning a payload to the BN?

[metachris]

Assuming the relays already have the headers prepared before the call, the responses should arrive fairly quickly (a few ms + network latency). What would be an acceptable upper limit?

Does waiting ~2 seconds for the getPayloadHeaderV1 responses from the relays feel too much? I guess that should suffice for a little computing overhead + network latency.

[mkalinin]

I’m not seeing a case where the builder would need access to the beacon chain state, but I might be missing something, @terencechain do you have any examples?

This would only matter if we have prepare_ call which is not what ppl are leaning to.

Does waiting ~2 seconds for the getPayloadHeaderV1 responses from the relays feel too much? I guess that should suffice for a little computing overhead + network latency.

Ideally, mev-boost responds immediately to builder_getPayloadHeaderV1 call in the same fashion as EL client do with engine_getPayloadV1 call. This is because CL is supposed to make the latter call right at the slot boundary while constructing a block and expects to get the built payload immediately. So, mev-boost should probably do polling and be ready to respond quickly to CL's request as it's mentioned by @thegostep