Skip to content

Merge getPayloadV3 and getBlobsBundleV1#402

Merged
mkalinin merged 4 commits into
ethereum:mainfrom
mkalinin:blobs-payload-merge
Apr 21, 2023
Merged

Merge getPayloadV3 and getBlobsBundleV1#402
mkalinin merged 4 commits into
ethereum:mainfrom
mkalinin:blobs-payload-merge

Conversation

@mkalinin

Copy link
Copy Markdown
Contributor

As per decision made on EIP-4844 Implementers' Call #20 adds blobsBundle field to the getPayloadV3 response getting rid of blockHash field in BlobsBundleV1 data type and statements enforcing consistency between getPayloadV3 and corresponding getBlobsBundleV1 calls.

@g11tech g11tech left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!

Comment thread src/engine/experimental/blob-extension.md Outdated
mkalinin and others added 2 commits April 19, 2023 16:40
Comment thread src/engine/experimental/blob-extension.md Outdated
@flcl42

flcl42 commented Apr 19, 2023

Copy link
Copy Markdown
Contributor

Another option is to encode the transactions in network form, which encodes blobs/proofs/commitments, so you do not need to find which transaction a blob belongs to. Looks a bit cleaner, but may be harder to handle by CLs. Wdyt?
And we do not need payloadv3 actually if so

@g11tech

g11tech commented Apr 19, 2023

Copy link
Copy Markdown
Contributor

Another option is to encode the transactions in network form, which encodes blobs/proofs/commitments, so you do not need to find which transaction a blob belongs to. Looks a bit cleaner, but may be harder to handle by CLs. Wdyt? And we do not need payloadv3 actually if so

yes right now CLs don't need to ssz decode any txs as of now, although since ssz is already part of CL and won't be hard to do this

But from CL side the current solution (getpayloadv3) is simple enough to not think twice.

@flcl42

flcl42 commented Apr 20, 2023

Copy link
Copy Markdown
Contributor

@g11tech yes, it definitely moves a bit of complexity of handling blobs from EL to CL. On EL side we have an option to optimize blobs handling by having them as just 3 byte arrays which can be passed to a KZG library for batch verification and to the CL then. But we need to split them and separate them from the transactions when we form blobsbundlev1.
I mean it seems zero sum (- for EL, + for CL) in terms of complexity, but removes need in v3 API in addition.
No strong push from me if CLs do not like it.

@marioevz

Copy link
Copy Markdown
Member

Can getPayloadV3 be used before Deneb? If so, maybe return null in blobsBundle?

@mkalinin

Copy link
Copy Markdown
Contributor Author

Can getPayloadV3 be used before Deneb? If so, maybe return null in blobsBundle?

Good point. There is a pushback from at least Geth team on making Engine API methods accept and return multiple datatypes and be used across multiple forks. I think this worth debating but until that time i would leave this method definition as is and follow the decision when we move this method from experimental to cancun spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants