-
Notifications
You must be signed in to change notification settings - Fork 134
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
docs: add ADR for epochs #1550
docs: add ADR for epochs #1550
Conversation
|
||
- Add a param that sets the number of blocks in an epoch, i.e., `BlocksPerEpoch`. | ||
We can use `BlockHeight() % BlocksPerEpoch == 0` to decide when an epoch is over. | ||
Note that `BlocksPerEpoch` can also be a hardcoded constant as it's unlikely that it will change often. |
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 think hardcoded constant isn't really a great option, since we don't want to force people to fork if they just need a different epoch, right?
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.
Sure. A parameter would be cleaner, but it's not a must. Especially as this will be code run on the Hub.
Co-authored-by: Philip Offtermatt <[email protected]>
Co-authored-by: Philip Offtermatt <[email protected]>
|
||
The implementation of epochs requires the following changes: | ||
|
||
- Add a param that sets the number of blocks in an epoch, i.e., `BlocksPerEpoch`. |
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.
is there a max value for BlocksPerEpoch
which should not be exeeded?
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.
Not really. Of course reasonable values are needed to not increase too much the delay of propagating validator updates, see #1550 (comment).
Description
Closes: #1514
Add ADR for ICS Epochs.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
docs:
prefix in the PR titleReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
docs:
prefix in the PR titlemake build-docs
)