-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Updated light client sync for newer committees #1316
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Some minor things. Can probably merge relatively soon and clean up/add tests over time.
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
Heads up: I've started revamping this document and will push a new version tomorrow. Edit: Done :) |
* Lots of cosmetic changes * A few substantive changes including: * Use a more conservative threshold of 2/3 (as opposed to 1/2) * Implement optimisation to use `shard_block_root` (as opposed to `shard_block`) * Add `fork_version` as part of data to verify aggregate signature * Various typos/bugs fixed (a few typos/bugs also likely introduced)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple of small comments
@GregTheGreek brought up the fact that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anything else to do before merge @vbuterin @JustinDrake ?
I'm OK merging :) |
No description provided.