[Merged by Bors] - Block v3 endpoint

[eserilev]

Issue Addressed

#4582

Proposed Changes

Add a new v3 block fetching flow that can decide to return a Full OR Blinded payload

Additional Info

[eserilev]

Preface

Up until this point, i've sort of just "cloned" the existing block fetching flow and added the ability to return either a full or blinded payload. I've added several enums and structs to make this possible.

Note that there is some additional refactoring/consolidation that needs to happen here. Theres some code on both the old flow and new flow that can be extracted and shared between the two. The code at this point can be viewed as a starting point for some additional refactoring.

It's also important to mention that the old flow is technically deprecated post deneb fork. So a lot of the refactoring here may technically be a waste of time? I guess it depends how the Lighthouse team/Ethereum foundation treats deprecated code. If we expect the old block fetching flow to exist forever, then maybe its worth it to put significant effort here into really cleaning this up.

[eserilev]

Naming Convention

Enums

where it made sense I added enums with the suffix "Type" i.e. BlockProposalContentsType etc. The enum types in all cases are either Full or Blinded

New structs

Added the v3 suffix to new structs. Most of the new structs I created were clones of existing structs without the AbstractExecPayload generic type param

Function names

Where it made sense I affixed 'determine' to the function names. These functions are mostly copy/paste jobs from existing functions without the AbstractExecPayload generic type param. 'determine' affix was used to communicate that the function not only returns payload, but also determines if the payload should be blinded vs full. In some situations I added the v3 suffix instead.

[jimmygchen]

I'm not up to date with this endpoint, but should this be targeting deneb branch? Is there any discussion on when to switch over?
cc @realbigsean

[realbigsean]

I'm not up to date with this endpoint, but should this be targeting deneb branch? Is there any discussion on when to switch over?

I think targeting unstable is a bit nicer from a review POV. this can be released independent of a fork, we shouldn't need coordination across teams for this one. You should be able to participate in deneb using the v2 endpoints if need be because they also have the deneb types

[realbigsean]

Looks like you've got it so far!

It's also important to mention that the old flow is technically deprecated post deneb fork. So a lot of the refactoring here may technically be a waste of time? I guess it depends how the Lighthouse team/Ethereum foundation treats deprecated code. If we expect the old block fetching flow to exist forever, then maybe its worth it to put significant effort here into really cleaning this up.

We will need to support both the new and old endpoints for some amount of time so people have a chance to migrate. Some users update bn + vc separately. And other clients may not release these changes at the same time as us, so running a teku VC with a lighthouse BN might stop working for a period if we deprecate the old flow.

I think the concrete flow you have here could replace the abstract flow entirely without too many changes. We would need to pass into the block production flow something like this (feel free to improve naming):

enum BlockProductionVersion {
    V3,
    BlindedV2,
    FullV2
}

We could use information in this enum to replace the info from BlockType in this method, following the blinded flow if our variant is BlindedV2 or V3, otherwise following the full flow:
https://github.com/sigp/lighthouse/blob/f1ac12f23ae581ace17f1cabcff36a6fb3a3e13f/beacon_node/execution_layer/src/lib.rs

Then at this line we could return the requested block type if we have one of the v2 variants of otherwise, always return the full payload
https://github.com/sigp/lighthouse/blob/f1ac12f23ae581ace17f1cabcff36a6fb3a3e13f/beacon_node/execution_layer/src/lib.rs:1110

I'm not positive this will work as expected but might be a decent route to explore to reduce the code duplication

[eserilev]

@realbigsean thanks for the feedback and additional info. I think the BlockProductionVersion enum you suggested could be a nice way to consolidate the old and new block production flow. I'll begin exploring that option.

[eserilev]

I resolved the merge conflicts, however there still might be a few opportunities to refactor/clean up the code

[eserilev]

I pushed up a small refactor, this should be ready for another review

EDIT: Theres a few tests failing, will fix soon

[eserilev]

resolved the failing tests

[michaelsproul]

Looking great on the whole! I just had a few minor comments and suggestions

[eserilev]

Thanks for the feedback, I pushed up the relevant changes

[realbigsean]

Nice! Looks great just had a couple tiny comments but looks good to me

[michaelsproul]

Approved!

[michaelsproul]

bors r+

[bors]

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• beacon-chain-tests
• cargo-udeps
• check-code
• check-msrv
• compile-with-beta-compiler
• debug-tests-ubuntu
• dockerfile-ubuntu
• doppelganger-protection-test
• ef-tests-ubuntu
• eth1-simulator-ubuntu
• execution-engine-integration-ubuntu
• merge-transition-ubuntu
• no-eth1-simulator-ubuntu
• op-pool-tests
• release-tests-ubuntu
• release-tests-windows
• slasher-tests
• state-transition-vectors-ubuntu
• syncing-simulator-ubuntu