Skip to content
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-7840 #1828

Merged
merged 7 commits into from
Dec 23, 2024
Merged

feat: EIP-7840 #1828

merged 7 commits into from
Dec 23, 2024

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Dec 23, 2024

Motivation

Implements EIP-7840 ethereum/EIPs#9129

Solution

alloy_eips::eip7840 has 2 structs: BlobScheduleItem which is the one present in genesis config as per EIP, and BlobParams which is a new aggregated configuration for blob constants, including an update fraction which is going to change in Prague as well with EIP-7691. All methods of header which need to perform blobfee-related calculations are now accepting BlobParams similarly to methods performing eip1559 calculations.

ChainConfig is extended with blob_schedule which is a mapping from String to BlobScheduleItem. It is not great because this means that we'd need to perform parsing of strings into the actual hardforks names on demand and won't have type-safety during parsing.

One way to improve this could be to make Genesis and ChainConfig generic over hardforks enum but this might be quite invasive, wdyt @mattsse

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits

crates/eips/src/eip7840.rs Outdated Show resolved Hide resolved
crates/genesis/src/lib.rs Show resolved Hide resolved
@mattsse
Copy link
Member

mattsse commented Dec 23, 2024

It is not great because this means that we'd need to perform parsing of strings into the actual hardforks names on demand and won't have type-safety during parsing.

imo this is fine because genesis -> chainspec is an additional step anyway

@klkvr
Copy link
Member Author

klkvr commented Dec 23, 2024

It is not great because this means that we'd need to perform parsing of strings into the actual hardforks names on demand and won't have type-safety during parsing.

imo this is fine because genesis -> chainspec is an additional step anyway

rn we're keeping Genesis directly on chainspec so we would need to update this to ensure correct hardfork names https://github.com/paradigmxyz/reth/blob/dd4715f5d1b33c695783541fe1b32d07baff8bf5/crates/chainspec/src/spec.rs#L35-L36

@mattsse
Copy link
Member

mattsse commented Dec 23, 2024

right, I think for this we should introduce a new chainspec field that we then set for the known networks accordingly and update the from genesis -> chainspec conversion as well

@klkvr klkvr merged commit 4cd7313 into main Dec 23, 2024
26 checks passed
@klkvr klkvr deleted the klkvr/eip-7840 branch December 23, 2024 18:50
MBerguer added a commit to MBerguer/alloy-zksync that referenced this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants