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 INITIAL_SLOT_NUMBER to avoid underflow checks #229

Closed
wants to merge 1 commit into from

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented Dec 4, 2018

Changed INITIAL_SLOT_NUMBER to be define as 3 * EPOCH_LENGTH to avoid a host of underflow checks/errors that needed to be fixed in state recalculations.

addresses #224

@djrtwo djrtwo requested review from JustinDrake and hwwhww and removed request for JustinDrake December 4, 2018 16:45
@JustinDrake
Copy link
Contributor

Paging @paulhauner and @arnetheduck

@hwwhww
Copy link
Contributor

hwwhww commented Dec 4, 2018

Looked the spec again, it seems we didn't check if block.slot >= INITIAL_SLOT_NUMBER?

I think it enhances the correctness at a certain level; but in practice, the block, attestations are downloaded/received from the internet, so the client side still has to implement the type check upon receiving the data. That said, it seems this spec implies a validity check of that "slot number should be equal to or larger than INITIAL_SLOT_NUMBER" upon receiving block or attestation.

@djrtwo
Copy link
Contributor Author

djrtwo commented Dec 4, 2018

The genesis block at INITIAL_SLOT_NUMBER is finalized. A client can disregard any blocks that come prior to or are in forks that do not include the last finalized block.

Because of this, I don't think we need to include that extra validity condition

@hwwhww
Copy link
Contributor

hwwhww commented Dec 4, 2018

A client can disregard any blocks that come prior to or are in forks that do not include the last finalized block.

Yeah, that works (covered block.slot >= INITIAL_SLOT_NUMBER condition).

I think "a client can disregard any blocks that come prior to or are in forks that do not include the last finalized block" is similar to what I mean by "the client side still has to implement the type check upon receiving the data"? It's implicit here.

@djrtwo
Copy link
Contributor Author

djrtwo commented Dec 5, 2018

Closing in favor of shifting in general to additions in conditionals rather than the current subtractions.

@djrtwo djrtwo closed this Dec 5, 2018
@JustinDrake JustinDrake deleted the initial_slot_number branch February 14, 2019 22:54
@JustinDrake JustinDrake restored the initial_slot_number branch February 14, 2019 22:54
@djrtwo djrtwo deleted the initial_slot_number branch May 20, 2020 23:10
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