Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
REST API produceBlockV3 implementation #5474
REST API produceBlockV3 implementation #5474
Changes from 9 commits
3e6da9a
e445843
a019314
4f79df5
f088b69
ef21953
bbd1cfb
e7d2304
e4b0f08
3431640
2066b0a
528dd7e
3a23982
cffd096
d7eb510
39afe69
97b1360
fbc841d
c3db2b6
dbd0bf1
e7b55d4
4d8ed25
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It apparently needs
ForkedAndBlindedBeaconBlock
here primarily because it's storing one asmessage
, but then it immediately does two things with thismessage
:getHeaders
(defined above); andRestApiResponse.sszResponse
, not even on theForked
type per se, but on each specific case object field. See question there about whywriteValue
is defined onForkedAndBlindedBeaconBlock
, as a result.But neither really requires storage of a forked/"any fork" version of a
BlindedBeaconBlock
orBlindedBeaconBlockContents
, because what thisproduceBlockV3
endpoint returns is neither, but a bytestring of some sort (e.g., SSZ). So it's still unclear to me why have all this infrastructure for aForkedAndBlindedBeaconBlock
which seems not to be necessary to begin with.The result is hundreds of lines of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need some form of
ForkedBlindedBeaconBlock
, because v3 may return either forked or non-forked depending on which one is better. And that is not known at call-time.This, unless you want to do that logic inside the REST layer, which doesn't seem correct, because it makes it non-reusable, e.g., for the non-VC local block proposal flow.
Also having a
Forked
type instead of forkyMaybeBlindedBeaconBlock
makes it consistent with the existing v1/v2makeBeaconBlockForHeadAndSlot
which also returns aForkedBeaconBlock
.