-
Notifications
You must be signed in to change notification settings - Fork 249
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: EIP-7742 #1600
feat: EIP-7742 #1600
Conversation
55ac6ae
to
b283c74
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.
cool, the integration in reth will be slightly more tricky because we now ran out of bits
/// Similar to [crate::eip4844::calc_excess_blob_gas], but derives the target blob gas from | ||
/// `parent_target_blob_count`. | ||
#[inline] | ||
pub const fn calc_excess_blob_gas( |
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'm a bit concerned that this will be easy to misuse if the functions have the same name, but since this takes an additional param this is okay imo
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 now completely identical :/ EIP-7742 just expects a normalized excess gas
should we rename the new one?
pushed latest changes as per ethereum/EIPs#9047 in db75ee4 |
/// Note: this function will return incorrect (unnormalized, lower) value at EIP-7742 activation | ||
/// block. If this is undesired, consider using [`eip7742::calc_excess_blob_gas_at_transition`]. |
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 think this should be OK as I'd expect all consumers of this to not know whether the next block activates 7742 unless it's an 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.
does this mean we need a special check for the transition block in reth?
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.
yep
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.
lgtm
/// Note: this function will return incorrect (unnormalized, lower) value at EIP-7742 activation | ||
/// block. If this is undesired, consider using [`eip7742::calc_excess_blob_gas_at_transition`]. |
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.
does this mean we need a special check for the transition block in reth?
Motivation
https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7742.md
ethereum/execution-apis#574
ethereum/EIPs#8994
PayloadAttributes
withtarget_blobs_per_block
andmax_blobs_per_block
.Some PRs are not merged and field names are different per different specs for now, but besides that this should be complete
Solution
PR Checklist