Bellatrix and blinded blocks

[realbigsean]

Resolves #179

I wanted to progress #179 because I think multiple CL teams are now working on MEV-boost compatibility implementations so it would be good to have a clear definition. I noticed this repo doesn't yet define API changes for Bellatrix, so I added those in this PR let me know if we want them in a separate PR though.

[mcdee]

Thanks for taking this on. A couple of comments.

[mcdee]

Considering the flow here for a validator client and two block-related endpoints. Given that the validator client does not know the configuration of the beacon node, and the beacon node could be able to return blocks, blocks or blinded blocks, or only blinded blocks, it seems that all validator client calls will be for blinded blocks as these are guaranteed to be returned regardless of the beacon node configuration.

As a practical upshot of the above, does it make sense to:

• deprecate /eth/v1/beacon/blocks and replace it with /eth/v2/beacon/blocks that takes a signed blinded block
• deprecate /eth/v2/validator/blocks/{slot} and replace it with /eth/v3/validator/blocks/{slot} that receives a blinded block

i.e. change communications between the VC and BN such that only blinded blocks are passed between them, and as a result make these the official block-related endpoints rather than having the separate blinded_blocks endpoints?

(Note that full blocks can still be obtained from the beacon node so this wouldn't cause problems for explorers and the like, this is just for the communications between VC and BN for block proposals.)

[paulhauner]

i.e. change communications between the VC and BN such that only blinded blocks are passed between them, and as a result make these the official block-related endpoints rather than having the separate blinded_blocks endpoints?

I generally like this idea, but perhaps we'd be prudent to add some other method to allow "revealing" of blinded payloads? Otherwise the BN API would be slightly odd in the way that it can't actually produce complete blocks.

[mcdee]

I generally like this idea, but perhaps we'd be prudent to add some other method to allow "revealing" of blinded payloads?

GET /eth/v2/beacon/blocks/{block_id} would still be available to provide full blocks.

(Sorry realized that the original proposal wasn't clear; the first endpoint I was talking about is a POST for sending signed blocks from the VC to the BN, not the GET which would remain with the current functionality.)

[mcdee]

Given that kiln is nearly upon us and there could be implications for deprecating the blocks API I suggest that we accept this PR as is and revisit the idea of deprecating the validator-related blocks endpoints at a later date.

As such, I'm happy with the PR as it stands.

[mpetrunic]

Given that kiln is nearly upon us and there could be implications for deprecating the blocks API I suggest that we accept this PR as is and revisit the idea of deprecating the validator-related blocks endpoints at a later date.

As such, I'm happy with the PR as it stands.

Should we release this under v2.2.0?

[rkapka]

I noticed that several references lead to Altair types. Should we move these types to a separate shared folder?

[mcdee]

Shared structs may still be change over time, though, so we'd need some sort of versioned shared and may as well just keep using the names of the forks in which they are introduced/updated.

[djrtwo]

Concur with deprecating the full block endpoint for blinded-block all around.
And definitely agree in releasing this asap (after minimum correctness review is complete)

[realbigsean]

Concur with deprecating the full block endpoint for blinded-block all around.
And definitely agree in releasing this asap (after minimum correctness review is complete)

@djrtwo do you mean deprecating the full block endpoints in this PR or in a future release as suggested by @mcdee here: #194 (comment) ?

[djrtwo]

Ah, I see @realbigsean. I think given time constraints that deprecating it in a subsequent release is the best path

[realbigsean]

I made an update here to handle pre-Belltrix blocks on blinded endpoints. It makes the merge transition logic easier for the VC because it doesn't have to worry about which endpoints to hit. And if we deprecate full blocks endpoints we will need to serve pre-Bellatrix blocks from blinded endpoints regardless.

[mpetrunic]

Merge branch 'bellatrix-and-blinded-blocks' of

Is there a reason why we need a new blinded blocks endpoint (POST /eth/v2/validator/blinded_blocks/{slot})?

Couldn't we just add another BlindedBlock request type to POST /eth/v2/validator/blocks/{slot} as it's not breaking change?

[rkapka]

I guess the same could be said about publishBlindedBlock (add a new type to /eth/v1/beacon/blocks).

The reason why I like this distinction as a client implementer is because currently I can easily discern posted block types by slot. If there are two possible types for the same fork, Bellatrix.BeaconBlock and Bellatrix.BlindedBeaconBlock, the logic will become more convoluted. I also personally think it's better to have several specialized, well-scoped endpoints, rather than one catch-all endpoint.