-
Notifications
You must be signed in to change notification settings - Fork 745
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
Electra: Add Preset, Constants, & Config #5606
Electra: Add Preset, Constants, & Config #5606
Conversation
unset_deposit_receipts_start_index: u64::MAX, | ||
full_exit_request_amount: 0, | ||
min_activation_balance: option_wrapper(|| { | ||
u64::checked_pow(2, 5)?.checked_mul(u64::checked_pow(10, 9)?) |
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 not just write 32000000000
here? What value does it add to write it in a factored formula?
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.
mostly consistency with the way the other values are written.. the spec also provides them factored this way so that you can do quick calculations in your head if you want
14ae684
to
787e818
Compare
pub unset_deposit_receipts_start_index: u64, | ||
pub full_exit_request_amount: u64, |
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.
These two aren't configurable, so I'm not sure we should add them here. We aren't totally consistent with ChainSpec
, because the signature domains and prefixes are on this struct. But it should be config + preset from the spec. We have a consensus/types/consts.rs
file we could move them to.
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.
moved to issue #5631
787e818
to
9fe2e52
Compare
@mergify queue |
@mergify refresh |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 05fbbdd |
✅ Pull request refreshed |
Issue Addressed
Proposed Changes
This PR adds the Constants, Preset & Configuration changes for Electra