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

Change if statements in get_crosslink_committes #642

Closed
wants to merge 1 commit into from

Conversation

paulhauner
Copy link
Contributor

Fixes #639.

@JustinDrake
Copy link
Collaborator

Why doesn't get_previous_epoch(state) always return get_current_epoch(state) - 1?

@djrtwo
Copy link
Contributor

djrtwo commented Feb 18, 2019

^ remnant from when we started epoch at 0 to prevent underflow.
We should resolve #626 before we modify get_previous_epoch. Seems like people want to see epochs/slots to start at 0

@JustinDrake
Copy link
Collaborator

I see three ways to prevent underflows:

  1. Add a bunch of subtle and crufty if special cases across the codebase. We already tried this and In hindsight it feels like the wrong way forward.
  2. As suggested in The integer debate #626, use int64 (as opposed to uint64) for slot and epoch and start with GENESIS_SLOT = 0. Definitely better than 1) but maybe not quite as clean as 2).
  3. Have GENESIS_SLOT be large enough. It was my mistake to not realise that 2**63 would cause complications with Java and it's unfortunate there's no obvious choice. My latest proposal is 2**n * SECONDS_PER_DAY/SECONDS_PER_SLOT where n is the smallest sufficiently large exponent.

JustinDrake added a commit that referenced this pull request 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)
@djrtwo djrtwo closed this in #655 Feb 21, 2019
@djrtwo
Copy link
Contributor

djrtwo commented Feb 21, 2019

@paulhauner #655 addresses this immediate issue. I'm combing the spec to make sure that the initial previous epoch being before genesis_epoch doesn't open up any unexpected new bugs

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