-
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: Uncouple blob count between CL and EL #8735
Conversation
✅ All reviewers have approved. |
The commit 965c047 (as a parent of e4a8338) contains errors. |
EIPS/eip-7733.md
Outdated
|--- |--- | | ||
| `FORK_TIMESTAMP` | TBD | | ||
|
||
### Background |
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.
IMHO, this should be moved to Motivation or partially to Rationale
EIPS/eip-7733.md
Outdated
|
||
Even if the EL keeps a fixed target value based on the max, removing the max implies the EL would not know what the target | ||
value should be. To address this lack of information, this EIP proposes the CL sends the current target value to the EL | ||
with each provided payload over the Engine API. The EL block header will also need to be extended with this target value |
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.
AFAICS, target_blob_gas_per_block
is used to compute excess_blob_gas
. Would it be possible to send the target as a part of PayloadAttributes
so the EL can compute the excess and make CL validate that the excess is computed correctly given the target and parent’s excess?
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.
the CL part of this change is here: ethereum/consensus-specs#3800 where the target is sent w/ the payload attributes
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.
Sorry, i wasn’t clear enough on the CL validation part, it seems to be possible to me to not include target_blob_gas_per_block
into EL block by doing the following:
- Send this parameter to payload build process so EL can correctly compute
excess_blob_gas
- Make CL validate that the computation was done correctly by utilising a simple math:
assert payload.excess_blob_gas == min(0, state.latest_execution_payload_header.excess_blob_gas + target_blob_gas_per_block - blob_gas_used)
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.
would you still commit the target_blob_gas
in the EL header?
it seems to break the ability for the EL to independently verify the excess_blob_gas
(and by extension the blob base fee) if it doesn't have access to the target value
if we keep the header extension then your change is just moving this check from the EL to the CL, which is fine in principle although I could argue violates the separation of concerns of "blob (max) limit check on the CL for DoS reasons" and "blob fee accounting on the EL"
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 seems to break the ability for the EL to independently verify the
excess_blob_gas
(and by extension the blob base fee) if it doesn't have access to the target value
I’d argue that EL doesn’t need to have this ability as long as CL is responsible for verifying the excess_blob_gas
computation. If we put it this way then we have no change to the EL block structure with a bit of cross-layer logic duplication: EL and CL will have to compute excess_blob_gas
. But I think this duplication is fine as the excess_blob_gas
formula is unlikely to change.
965c047
to
89a94f2
Compare
89a94f2
to
2a4b016
Compare
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...
ATTENTION: ERC-RELATED PULL REQUESTS NOW OCCUR IN ETHEREUM/ERCS
--
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: