Skip to content
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

Reduce GENESIS_SLOT to 2**32 #655

Merged
merged 1 commit into from
Feb 21, 2019
Merged

Reduce GENESIS_SLOT to 2**32 #655

merged 1 commit into from
Feb 21, 2019

Conversation

JustinDrake
Copy link
Collaborator

@JustinDrake JustinDrake commented Feb 19, 2019

* Address the slot/epoch underflow problem, even for Java implementers! 🎉
* Squash a bug with `get_previous_epoch`
* Fix #642
* Address #626 (Vitalik, Danny, myself agree that avoiding signed integers is probably best)
@Nashatyrev
Copy link
Member

I would vote for 2**20. It still addresses underflows, but is easier to remember and perform decimal calculations in mind.

@JustinDrake
Copy link
Collaborator Author

We will reach 2**20 slots ~70 days after genesis, so - 2**20 cannot be done by zeroing a single bit. On the other hand it will take 800+ years to reach slot 2**32 so there's no possibility of overflowing past the 32nd bit.

@Nashatyrev
Copy link
Member

@JustinDrake thanks for clarification!
Sorry, I probably missed the rationale for zeroing a single bit instead of doing - GENESIS_SLOT, can you please recall?

@JustinDrake
Copy link
Collaborator Author

JustinDrake commented Feb 21, 2019

It's friendlier to bit operations. For example, we can cast the slot to uint32 (stripping off the top 32 bits), or equivalently (slot << 32) >> 32, or equivalently bitwise XOR slot ^ 2**32.

Unfortunately there isn't a single canonical way forward so it's liable to bikeshedding. Vitalik independently suggested 2**32 on a call recently 😂 🤷‍♂️I think we all don't care too much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants