-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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: General purpose execution layer requests #8432
Conversation
✅ All reviewers have approved. |
The commit d8dca5c (as a parent of d7acb6a) contains errors. |
|
||
## Test Cases | ||
|
||
TODO |
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.
If you use HTML-style comments, the linter will make sure you replace them before going to review.
TODO | |
<!-- TODO --> |
|
||
## Security Considerations | ||
|
||
Needs discussion. |
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.
Needs discussion. | |
Needs discussion. <!-- TODO --> |
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.
All Reviewers Have Approved; Performing Automatic Merge...
Request = RequestType ++ RequestData | ||
``` | ||
|
||
Let `Requests` be the list of all `Request` objects in the block in ascending order by type. |
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.
ascending meaning by execution order?
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's ascending order by type. intra type ordering is not defined in this eip
|
||
### Consensus Layer | ||
|
||
Each proposal may choose how to extend `ExecutionPayload` to include the new EL request. |
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.
why wouldn't CL format just mirror the EL format?
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 don't really have a preference, it just seems more flexible for the CL to decide how to store the data instead of being forced to put all requests in a requests union type
|
||
Each proposal may choose how to extend `ExecutionPayload` to include the new EL request. | ||
|
||
A additional processing step is be added to `process_execution_payload` to iterate over and process all requests. |
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.
some things we will want to process before process_execution_payload
and some things we will want to process after
iirc none of the current 'requests' are actually addressed inside process_execution_payload
given the validity semantics of the spec, this isn't really material to the structure introduced by this EIP
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.
okay i will remove this
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.
|
||
#### Header | ||
|
||
Extend the header with a new 32 byte value `RequestsRoot`. |
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.
Can this be expanded to provide the complete structure of the RLP encoding of the header, along the lines of 4788
On a related note, given we're adding this new field, does this imply that 4844 and 4788 are required EIPs for this in the sense that they have both added new header fields that are prior to requestsRoot
in the header field order?
This EIP introduces the generic notion of an EL triggered requests. This is intended to encapsulate many of the request types which are being considered in Prague (and beyond). For example, instead of having individual types in the header, body, execution payload, etc. we can have a single generic
Request
that encompasses several different types.