-
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: Add blob schedule to EL config files #9129
Conversation
✅ All reviewers have approved. |
The commit 16a6427 (as a parent of 6586687) 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.
<-- TODO --> tag for tracking via bot
|
||
## 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.
<-- TODO --> tag
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.
Nethermind team is okay with this change. Signaling support 👍
Besu is ok with this change to get us through Prague and have a prototype implementation ready hyperledger/besu#8042 |
I prototyped the test changes required in this PR: ethereum/execution-spec-tests#1040 Feedback is appreciated. |
Should we configure update fraction in this object as well? Given that EIP-7691 defines different update fractions for Cancun and Prague it seems reasonable to handle it in the same way as target/max blob count? |
i think in interop call was discussed that upgrade fraction should also be provided with the config butt it only matters w.r.t. to handle the max swing on the asymmetry schedule (drop is bigger on empty blobs compared to full blobs block) 7691 already modifies that for 6/9 so not sure if we need to provide it here. although there could be later EIPs to deal with assymetry (like normalization) so we don't really need it. |
However, without the update fraction being specified in the genesis files - we can't properly test future blob increases on testnets (atleast not with proper gas markets). E.g if we set 10/20 as the blob target/max on pectra (in order to do some load testing), we will have a situation where the wrong update fraction is applied for this blob limit. Allowing us to specify everything in the genesis file will fix this. |
this EIP itself is baffling,
|
For 1. I don't imagine node runners using any signaling as the canonical files for mainnet are coded into the client implementations. This is mostly just for testnets and testing purposes that we care about specifying the values instead of just hardcoding it. |
Co-authored-by: g11tech <develop@g11tech.io>
@g11tech can we please merge this now that the track has been changed, as we've already accepted to include it in the fork. Thanks! |
@timbeiko Side-topic, it would be new (?) for informational EIPs to become part of a hardfork spec? (Because this is not part of consensus and thus does not require a fork) |
@jochem-brouwer we agreed to put non-Core EIPs in hardfork spec for clarity (e.g. PeerDAS isn't a Core EIP), but I haven't formalized that anywhere. Need to open a PR to EIP-7723 to mention it 😄 |
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...
Adds a new object to the EL genesis chain config:
of the shape: