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

Upgrade getBlindedBlock API endpoint to Capella #11687

Merged
merged 8 commits into from
Nov 28, 2022

Conversation

rkapka
Copy link
Contributor

@rkapka rkapka commented Nov 24, 2022

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Upgrades https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/getBlindedBlock to handle Capella blocks.

(cherry picked from commit 7101910e0fab5a5572795115679fd6f8d8c8379b)
(cherry picked from commit e5c269ddf7b0c9e04f72ed28982a82de56fcac55)
(cherry picked from commit 1719ce5967b0f74786c596cc921f7256e6b224f3)
@rkapka rkapka requested a review from a team as a code owner November 24, 2022 16:09
@rkapka rkapka added Ready For Review API Api related tasks labels Nov 24, 2022
assert.Equal(t, "root", beaconBlock.ParentRoot)
assert.Equal(t, "root", beaconBlock.StateRoot)
assert.NotNil(t, beaconBlock.Body)
assert.Equal(t, true, resp.ExecutionOptimistic)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to check the execution payload header here?

@@ -813,6 +813,77 @@ func (bs *Server) GetBlindedBlock(ctx context.Context, req *ethpbv1.BlockRequest
return nil, status.Errorf(codes.Internal, "Could not get signed beacon block: %v", err)
}

capellaBlk, err := blk.PbCapellaBlock()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is unreadably long, can we talk about ways of avoiding all this code duplication? or even if we are going to duplicate it, perhaps having each case as a helper?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored the code and put it into its own file. Deduplicating is hard because there are lot of differences in terms of block types and function calls.

@rkapka rkapka force-pushed the capella-api-getblindedblock branch from 7608e78 to 617b1c4 Compare November 25, 2022 15:32
@rkapka rkapka force-pushed the capella-api-getblindedblock branch from 617b1c4 to 134b218 Compare November 25, 2022 15:44
Co-authored-by: Potuz <potuz@prysmaticlabs.com>
potuz
potuz previously approved these changes Nov 28, 2022
@prylabs-bulldozer prylabs-bulldozer bot merged commit 6c3b75f into develop Nov 28, 2022
@delete-merged-branch delete-merged-branch bot deleted the capella-api-getblindedblock branch November 28, 2022 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Api related tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants