-
Notifications
You must be signed in to change notification settings - Fork 406
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
Add EIP-7594 (PeerDAS) related changes #630
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: fradamt <104826920+fradamt@users.noreply.github.com>
Co-authored-by: fradamt <104826920+fradamt@users.noreply.github.com>
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’ve added a few suggestions to slightly improve readability and clarity—mostly minor points for precision. None of them are strictly necessary, so please feel free to accept or ignore them based on your preference!
Co-authored-by: Tei Im <40449056+ImTei@users.noreply.github.com>
Co-authored-by: Tei Im <40449056+ImTei@users.noreply.github.com>
fix camel case and other small changes
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.
have a question / request for clarification on the eth_sendRawTransaction
semantics
|
||
## Methods | ||
|
||
### engine_getPayloadV5 |
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.
How much effort is it to make these endpoints return in SSZ encoding? I'd imagine JSON serialisation and deserialisation would take much longer with much higher blob count.
Given that engine has SSZ encoding capability (blob is already SSZ encoded), is it possible to support SSZ encoding the entire endpoint responses?
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.
Oh never mind - i think SSZ encoding the blobs already provide most of the savings on blobs.
actually, no, this is still hex encoded for JSON wrapped in a string, so it's still slow and expensive.
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.
Peter mentioned it before here:
https://x.com/peter_szilagyi/status/1841424117733478650
We should seriously consider doing this to support high blob count.
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.
We should seriously consider doing this to support high blob count.
the way the current engine-api is designed makes it almost impossible to add ssz as a wire format, yesterday the geth team mentioned they are working on a rest api following the beacon-api design, I think this is the right step forward, not only for blobs but also if we wanna bump up the gas limit
Summary
Introduces updates to rpc and engine API definitions for EIP-7594, changes defined in ethereum/EIPs#9378
Notes for reviewers
This PR introduces backward incompatible changes to
engine_getPayloadV4
, that returns newBlobsBundleV2
with cell proofs instead of blob proofs