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

Consolodating blinded/unblinded flow #309

Closed
realbigsean opened this issue Mar 20, 2023 · 13 comments
Closed

Consolodating blinded/unblinded flow #309

realbigsean opened this issue Mar 20, 2023 · 13 comments

Comments

@realbigsean
Copy link
Contributor

realbigsean commented Mar 20, 2023

Discussed here: #302

General suggested approach so far:

add a new header similar to the Eth-Consensus-Version to denote blinded vs unblinded

We could have the VC request a blinded/unblinded block preference via header and the beacon should respond with the correct header but isn't strictly required to, and in the BN response we add a header to denote what the actual message type is (blinded/unblinded). With this model you could simply ignore anything unexpected in the VC (i.e. blinded request with unblinded response). The VC is still able to in parallel make one blinded and one unblinded request and include fallback logic

Currently there isn't clarity in the spec around whether the VC or BN should handle things like fallback-to-local behavior. Allowing the BN to choose the return type makes it clear that fallback in the BN is supported, but it seems that some prefer this to be disable-able. We will need to add blockValue to the beacon API to make sure fallback-to-local is fully supported in the VC. And similarly this shouldOverrideBuilder flag will need to be made available to the VC eventually ethereum/execution-apis#388. Would it make sense to move towards consensus on the question of "who picks the payload at the end of the day, the BN or VC"? Being in-between on this makes the APIs more complicated and has created a lack of clarity around what the beacon node will do during the proposal flow. It'd be nice to iron this out in the spec.

So far the APIs seem to suggest it's the beacon node that should pick the payload (it has the blockValue information) but should this be how it works, or should validators choose? If validators should choose, should we think about separating the blinded API from the beacon node generally and instead move towards VC directly connecting to the builder API?

@rolfyone
Copy link
Collaborator

I'm not sure there's an easy answer.
Teku has supported blinded api usage for a while, and there's really no reason the VC needs all the block data. Having said that, by supporting 2 sets of api, it means they're free to choose, so in instances like vouch where full blocks are required, they're not prevented from having that available. There's also SSZ vs non SSZ which enters into this whole complicated set of scenarios...

Being too prescriptive at the api level would force clients to do very specific things, and I'm not sure we need to be too closely prescribing how a client has to act in this regard...

Is there a concrete example of an actual issue we're trying to resolve?

If we really did want to lock this down, the VC doesn't actually need the entire new block at all, so i'd probably be going a lot further with it, not just forcing the use of blinded or similar...

Moving onto deneb, and maybe past, it may become more important to be considering all the data moving about, but it'll be a very non trivial change and we'd want to take a step back and articulate what our end goal is etc IMO...

@james-prysm
Copy link
Contributor

but it'll be a very non trivial change and we'd want to take a step back and articulate what our end goal is etc IMO...
agreed, we should carefully evaluate the end goal and benefits. in the mean time we definitely don't have the capacity to do this.

@g11tech
Copy link
Contributor

g11tech commented Mar 30, 2023

So far the APIs seem to suggest it's the beacon node that should pick the payload (it has the blockValue information) but should this be how it works, or should validators choose? If validators should choose, should we think about separating the blinded API from the beacon node generally and instead move towards VC directly connecting to the builder API?

Although we can leave it to client implementations, but we can make sure the data is available to VCs to make an appropriate decision.

Further to the idea (of vc connecting directly with builder) that beacon is just an interface to gossip/req-resp is very tempting and whereever a normal model of http req/resp is available can be directly plugged into the validator (think external blob data availability services).

However the beacon still has the role of fully packing the block (and providing the head info to getheader) and generating post state even in the builder flow still would place the builder api flow (unless validator gets into fetching/providing these things to beacon, so still possible). And this can definitely be well positioned for High Availability solutions, where validator can hook into multiple beacons/builder sources

@mcdee
Copy link
Contributor

mcdee commented Apr 2, 2023

Would it make sense to move towards consensus on the question of "who picks the payload at the end of the day, the BN or VC"?

I don't think that we will reach consensus on this. On the one hand, the VC is the entity in charge of the proposal process so should be able to tell the beacon node what it wants. On the other, the BN has the wider view of the chain (as opposed to a single proposal) and so items such as circuit breakers naturally live there. But I don't think we really need consensus. I think that both pieces are parts of the whole, so we need to allow both entities to have their say. The flow could be:

  • VC asks BN for a proposal
  • BN will decide which payload it uses, based on its configuration and current circuit breaker status
    • if the circuit breaker is tripped or no MEV relays are configured then return a proposal with local payload
    • if the circuit breaker is not tripped and MEV relays are configured then return a proposal with the highest-value payload from the MEV relays and the local payload
  • BN returns as much data as possible (unblinded block where it has access to the payload) along with metadata to help the VC make a decision as to if it wants to select the proposal

The metadata sent from the BN to the VC should include the version of the payload (capella, deneb, _etc.), the style of the payload (blinded/unblinded), the provenance of the payload (local or MEV relay, and if the latter which relay), and its value. This gives the VC all the information it needs to decide if it wants to progress with the proposal or not.

If validators should choose, should we think about separating the blinded API from the beacon node generally and instead move towards VC directly connecting to the builder API?

Would this solve much by itself? If the VC picked a blinded payload from a relay and then sent it to a BN as part of the proposal request, but the BN's circuit breaker was tripped, wouldn't the BN want to return a local payload anyway? From Vouch's point of view this would be slightly cleaner in terms of code, but I don't think that it gains enough for it to be worth changing all of the existing BNs. That said, if we want this as an option then allowing the VC to pass an execution payload header as a body to the proposal request is something that could be made available. If this ends up becoming the default option then over time we could deprecate and ultimately remove the relevant code from the BNs.

As to the additional complexity: I think that a lot of this process is already present in the code paths to fetch blinded blocks. Indeed this could reduce complexity, by not requiring BNs to keep track of payloads they have received from local execution clients and set as part of a returned (blinded) proposal. And the long-term benefits of having a single endpoint for fetching a proposal does seem to simplify things rather than complicate them.

One question that I've been wondering: is there any value in the VC being able to ask the BN to return blinded blocks only? It would reduce the amount of data being moved between VC and BN during the proposal process, but does create a dependency on the BN's state and that makes life harder for advanced scenarios (load balancing across BNs, proxies, Vouch, etc.) and so flies in the face of some of the reasons that we chose a REST API in the first place. The increase in resilience of the proposal process by having full data on the VC where possible seems to be a good reason not to have this ability, although there is an argument that this is a benefit to larger staking operations running multiple BNs and a detriment to home stakers and the like. I suspect that it stops being so much of an issue if we ensure that SSZ is available for this endpoint, but it's up for debate.

@arnetheduck
Copy link
Contributor

The metadata sent from the BN to the VC should include the version of the payload (capella, deneb, _etc.), the style of the payload (blinded/unblinded), the provenance of the payload (local or MEV relay, and if the latter which relay), and its value. This gives the VC all the information it needs to decide if it wants to progress with the proposal or not.

this is not quite the case: the fact that the BN chooses between relays, local blocks and so on discards the blocks that it did not choose - a "complete information API" would return all possible block sources / options to the VC - a possible architecture thus is to do exactly that (return a list instead of a single item).

circuit-breaker

I think one of the most important properties to maintain for Deneb in the block production API is a fully stateless option which preserves blocks and blobs without blinding anything along the full block production chain as an option to the VC: anything else from there, ie any form of blinding and reliance of statefulness increases complexity significantly by adding various forms of constraints. As long as the state-minimising option exists in the API (either to be selected via a parameter or not), we have a reasonably implementable plan B for any complex failures that might happen during blob introduction.

Future versions might require us to maintain more state somewhere in the chain, but as has happened recently, some of the "statefulness" is being fixed at the source for example by the unblinders publishing the unblinded block themselves instead of handing it back to the "signer" - structural improvements like this keep things simple (and should maybe be enshrined in the blinded api flow in general) and more resilient against failure in general.

@mcdee
Copy link
Contributor

mcdee commented Apr 16, 2023

a "complete information API" would return all possible block sources / options to the VC - a possible architecture thus is to do exactly that (return a list instead of a single item).

Returning multiple blocks would not only be very heavy data-wise, but it would remove the "BN will decide which payload it uses" piece. From my understanding, this is something that at least some of the client teams feel strongly that they should be able to do.

I think one of the most important properties to maintain for Deneb in the block production API is a fully stateless option...

Agree with this.

@rolfyone
Copy link
Collaborator

rolfyone commented Jul 4, 2023

I'm not against having a consolidated flow, but:

  • lets have a new api
  • lets make Eth-Consensus-version required and any other new header required if that's how we're passing in context
  • on the old api potentially you could have an optional api, but IMO Eth-Consensus-Version is close to useless on the existing api because we cant rely on it being there.
  • keep in mind the block api we recently defined from @michaelsproul - is this one that could be the consolidated flow or should we have a v3?

Potentially I'd be leaning towards having a v3 and the v2 /v1 being deprecated... Mostly in case the v2 is currently implemented and used...
Time line would also be the other, would we be aiming to use this in time for deneb?

@mcdee
Copy link
Contributor

mcdee commented Jul 4, 2023

Agree with the above points.

Can you link Michael's block API? It may be that this issue has already been addressed.

I would love to see this for deneb, but I think we need to spec it out first and then decide if client teams have the bandwidth to implement it.

@rolfyone
Copy link
Collaborator

rolfyone commented Jul 4, 2023

Can you link Michael's block API? It may be that this issue has already been addressed.

I was thinking #317

I would love to see this for deneb, but I think we need to spec it out first and then decide if client teams have the bandwidth to implement it.

100% agree, will be a good idea to get an idea of when we'd likely be able to see the changes, and have that discussion relatively early :)

@mcdee
Copy link
Contributor

mcdee commented Jul 4, 2023

Thanks; that looks like something that would need to be incorporated.

Let me have a consider and write something up so that we can get a feel for the scope.

@mcdee
Copy link
Contributor

mcdee commented Jul 5, 2023

Okay, here is an attempt to scope out a consolidated block request API endpoint. Thoughts and comments appreciated.

Path

/eth/v3/validator/blocks/{slot}

Parameters

{slot} is as per /eth/v2/validator/blocks/{slot}. Query parameters are:

  • randao_reveal: required, as per /eth/v2/validator/blocks/{slot}
  • graffiti: required, as per /eth/v2/validator/blocks/{slot}
  • skip_randao_verification: default false, as per /eth/v2/validator/blocks/{slot}

Operation

Requests a beacon node to produce a valid block, which can then be signed by a validator. The returned block may be blinded or unblinded, depending on the current state of the network as decided by the execution and beacon nodes.

The beacon node must return an unblinded block if it obtains the execution payload from its paired execution node. It must only return a blinded block if it obtains the execution payload header from an MEV relay.

Response

Header

The response must contain the following headers:

  • Eth-Consensus-Version: the version of the data returned, for example 'capella'
  • Eth-Blinded-Payload: true if the response is a blinded block, false if it is an unblinded block
  • Eth-Payload-Value: the stated value of the execution payload to the proposer, in Wei

Body

The response body can be in SSZ or JSON. For JSON the response body is:

{
  "data": {
    ...
  }
}
  • "data" is the blinded or unblinded beacon block, depending on the information in the "Eth-Consensus-Version" and "Eth-Blinded-Payload" headers.

For SSZ, the response body is the SSZ encoded blinded or unblinded beacon block, depending on the information in the "Eth-Consensus-Version" and "Eth-Blinded-Payload" headers.

@rolfyone
Copy link
Collaborator

rolfyone commented Jul 5, 2023

  • graffiti: required, as per /eth/v2/validator/blocks/{slot}

graffiti isn't required in v2...

other than that this sounds good.

@dapplion
Copy link
Collaborator

dapplion commented Dec 8, 2023

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

9 participants
@mcdee @arnetheduck @rolfyone @realbigsean @dapplion @g11tech @james-prysm and others