-
Notifications
You must be signed in to change notification settings - Fork 1k
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 ePBS fork schedule to config #14383
Add ePBS fork schedule to config #14383
Conversation
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.
This will conflict with Peer-DAS, but it should still be safe to add.
4b37588
to
5691fbc
Compare
5691fbc
to
355a199
Compare
e := fsched[i].Epoch | ||
ad, err := dc.forEpoch(e) | ||
n := fsched[i].Name | ||
ad, err := dc.forName(n) |
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 this change?
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.
Using epoch to query fork domain is based an assumption that no two updates have a same epoch. While it's a solid one, it doesn't hold when more than two updates are underway like current case — Electra and ePBS. I think assuming there will be no duplicate name is slightly better than epoch.
What type of PR is this?
This PR adds ePBS fork schedule to config. I guess it was omitted as Electra fork epoch is not defined yet, making both
cfg.ElectraForkEpoch
andcfg.EPBSForkEpoch
havemath.MaxUint64
inHoleskyConfig
andSepoliaConfig
after adding ePBS fork. If it ought to be added later, please feel free to close this PR.