Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 max epoch activation churn limit (EIP-7514) to Deneb #3499
Add max epoch activation churn limit (EIP-7514) to Deneb #3499
Changes from 12 commits
e6f7c99
fd37ffc
cc3ced5
298a630
417b95c
8878a31
28286e7
19bf51d
a56c4d0
f165d39
0efd778
e5e50e3
26d3fa3
e804174
264dfad
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
just a nit: since this new param is only applied from
Deneb
, should it be calledDENEB_MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT
?There might be confusion in testnets with a
Bellatrix
genesis and aMAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT
set to1
and you may think that it is immediately applied but you actually have to wait untilDeneb
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.
Hmm, but it would break the current code conventions for the new constants/presets/configurations. 🤔
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.
🤔 ... It actually should be
MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT_DENEB
, similar toMAX_REQUEST_BLOCKS_DENEB
, even if the phase0 counterpartMAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT
won't exist.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 see your point, but we do have
MAX_REQUEST_BLOCKS
in phase0! Adding a suffix to a new preset breaks the current code conventions.