-
Notifications
You must be signed in to change notification settings - Fork 63
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
feat(pectra): Use EIP-7865 encoding for execution_requests #107
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,12 @@ Electra: | |
maxItems: 4096 | ||
description: "the `KZGCommitment`s for the associated blobs for this `header`" | ||
execution_requests: | ||
$ref: "../../beacon-apis/types/electra/execution_requests.yaml#/Electra/ExecutionRequests" | ||
description: "`ExecutionRequests` to include in block proposal." | ||
type: array | ||
items: | ||
$ref: '../../beacon-apis/types/primitive.yaml#/Bitvector' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if the Bitvector is a good semantic representation for the opaque field, although technically, it could work. I would consider adding a Keen to have people weight in on this before committing to it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Funny when writing this up I originally used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe so. It looks like builder-specs is importing beacon-api as a git submodule and referring to the types defined there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok cool I just sent ethereum/beacon-APIs#484 I guess if this is accepted I can update the submodule in this PR and point to that. Thanks for the suggestion! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
you can't have unbounded ssz types |
||
minItems: 0 | ||
maxItems: 3 | ||
description: "EIP-7685 encoded `ExecutionRequests` to include in block proposal." | ||
- $ref: '../bellatrix/bid.yaml#/Bellatrix/BuilderBidCommon' | ||
|
||
SignedBuilderBid: | ||
|
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.
those are not really opaque, just ssz encoded and 1-byte prefixed, anyone can decode the requests and read the data
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.
They're "semantically" opaque, EIP-7865 stipulates that each request type is allowed to decide its own encoding, it's just a coincidence that the current three are all SSZ in the same way that all EIP-2718 txs are currently RLP.
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 guess if the same wording is used on the engine-api side this is fine