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

Add sanity unit test for validator guide + update validator guide #1745

Merged
merged 7 commits into from
Apr 27, 2020

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Apr 23, 2020

  1. Add unit tests for validator guide to improve test coverage. (Fix Add tests for executable validator guide #1352)
    • phase 0 spec.py test coverage raises from 93% to 99%
    • note that these tests are not part of client test vectors
  2. Validator guide update:
    1. Avoid negative computation in is_candidate_block
    2. Fix get_block_signature: avoid extra casting; it's simpler to use BeaconBlock instead of BeaconHeader.
    3. Make compute_new_state_root a pure function.
  3. Update minimal.config: Increase EPOCHS_PER_ETH1_VOTING_PERIOD from 2 to 4 for testing eth1 votes consensus vote

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.

nice! finally, we won't introduce silly errors when updating v-guide and this should help in the phase 1 v-guide dev too :)

1. Avoid negative computation in `is_candidate_block`
2. Fix `get_block_signature`: avoid extra casting; it's simpler to use BeaconBlock instead of
BeaconHeader
1. "Becoming a validator"
2. "Validator assignments"
3. "Beacon chain responsibilities: Block proposal"
1. "Beacon chain responsibilities: Attesting"
2. "Beacon chain responsibilities: Attestation aggregation"
@hwwhww
Copy link
Contributor Author

hwwhww commented Apr 27, 2020

@djrtwo thanks for the review!

Changelog

  1. Apply PR feedback
  2. Update minimal.config: Increase EPOCHS_PER_ETH1_VOTING_PERIOD from 2 to 4 since we need more than 2 epochs for testing test_get_eth1_vote_consensus_vote.
  3. Make compute_new_state_root a pure function (Add sanity unit test for validator guide + update validator guide #1745 (comment))

@hwwhww hwwhww requested a review from djrtwo April 27, 2020 14:23
@djrtwo djrtwo merged commit f95a135 into v012x Apr 27, 2020
@djrtwo djrtwo deleted the hwwhww/validator-tests branch April 27, 2020 15:56
@hwwhww hwwhww changed the title Add sanity, unit test for validator guide Add sanity unit test for validator guide + update validator guide Apr 28, 2020
@hwwhww hwwhww added the general:enhancement New feature or request label Apr 28, 2020
@hwwhww hwwhww added this to the v0.12.0 milestone May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants