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

Clarify get_randao_mix accessor from generate_seed #1221

Merged
merged 5 commits into from
Jun 29, 2019
Merged

Conversation

ralexstokes
Copy link
Member

@ralexstokes ralexstokes commented Jun 26, 2019

We avoid a genesis underflow by taking the randao epoch in generate_seed to be + EPOCHS_PER_HISTORICAL_VECTOR.
This conflicts with the expected epoch bounds noted in get_randao_mix. This exceptional call to get_randao_mix is noted with a brief comment.

We avoid a genesis underflow by taking the randao epoch in `generate_seed` to be `+ EPOCHS_PER_HISTORICAL_VECTOR`.

This conflicts with the expected epoch bounds noted  in `get_randao_mix` and this PR attempts to clarify the situation by leaving a note.
@JustinDrake
Copy link
Contributor

IMO both comments (the old one, and the new one) are confusing. Less is more.

@ralexstokes
Copy link
Member Author

I agree less is more :)

I can say that we had a validation condition in Trinity based on an earlier instantiation of this comment and it did help catch a bug. So providing the guidance is helpful...

I guess it is a matter of style as to where this knowledge lives, if anywhere... Feel free to close the PR if there is consensus this isn't a helpful change

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

went for a minimal note at the calling site for the exceptional behavior

@djrtwo djrtwo merged commit aec9719 into dev Jun 29, 2019
@djrtwo djrtwo deleted the ralexstokes-patch-3 branch June 29, 2019 21:57
@djrtwo djrtwo changed the title Clarify get_randao_mix accessor Clarify get_randao_mix accessor from generate_seed Jun 29, 2019
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