-
Notifications
You must be signed in to change notification settings - Fork 974
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
epoch trans at end of epoch and fix finality conditions #492
Conversation
Here's a PR to your PR 🤣 The logic that you actually changes all looks good, it's more about a couple other things that still need to be changed. |
A few more state.slot -> current/previous/next epoch start slot changes
Thanks! Merged the PR into my PR |
Looks good to me at the moment. Maybe after merging this one and the light client one we can do a final pass on the epoch transition logic. I think for cleanliness reasons it may be a good idea to remove use of |
Did not yet handle the I'm starting to think that a lot of these vars and state attributes should just reference epochs. We are doing a lot of error prone math everywhere to get the start of epochs and convert slots to epochs. And we inconsistently use epochs vs slots |
I'm working on a PR to this PR that makes epochs a first class citizen. Seeing if it cleans things up. edit: PR here #499 |
make epochs first class citizens
Adds a minimum slot number large enough that integer underflows involving epochs and slots will not happen; simplifies some logic that was more complex to handle them.
Minimum slot number, simplify excessive anti-underflow logic
Genesis slot changes look good to me @vbuterin |
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.
Good!
@@ -189,24 +190,36 @@ Code snippets appearing in `this style` are to be interpreted as Python code. Be | |||
| Name | Value | | |||
| - | - | | |||
| `GENESIS_FORK_VERSION` | `0` | | |||
| `GENESIS_SLOT` | `0` | | |||
| `GENESIS_SLOT` | `2**19` | | |||
| `GENESIS_EPOCH` | `slot_to_epoch(GENESIS_SLOT)` | |
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.
Note that slot_to_epoch
use EPOCH_LENGTH
that is defined in the next section. So in the chain config, this will look like:
genesis_slot = SlotNumber(2**19)
epoch_length = 2**6
SERENITY_CONFIG = BeaconConfig(
...
GENESIS_SLOT=genesis_slot,
GENESIS_EPOCH=slot_to_epoch(genesis_slot, epoch_length),
...
EPOCH_LENGTH=epoch_length
...
)
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 the prettiest. I think we want to move to #504 which will use chain start time to seed the initial state rather than config.
Co-Authored-By: djrtwo <[email protected]>
shard_committees_at_slots
missing when processing previous epoch attestations #352 and get_shard_committees_at_slot out of range #409 (shard committees for prev epoch still available when processing attestations)