-
Notifications
You must be signed in to change notification settings - Fork 1.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
Add, use and test VerifyWeakSubjectivityRoot
#7344
Conversation
// Reference design: https://github.com/ethereum/eth2.0-specs/blob/master/specs/phase0/weak-subjectivity.md#weak-subjectivity-sync-procedure | ||
func (s *Service) VerifyWeakSubjectivityRoot(ctx context.Context) error { | ||
// TODO(7342): Remove the following to fully use weak subjectivity in production. | ||
if len(s.wsRoot) == 0 && s.wsEpoch == 0 { |
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.
should this be an || ?
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.
Either way is fine. This is not going to be production code. I changed it to | |
Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Codecov Report
@@ Coverage Diff @@
## master #7344 +/- ##
==========================================
- Coverage 60.42% 60.09% -0.34%
==========================================
Files 418 417 -1
Lines 30316 29961 -355
==========================================
- Hits 18319 18004 -315
+ Misses 9029 9016 -13
+ Partials 2968 2941 -27 |
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 great!
What type of PR is this?
What does this PR do? Why is it needed?
This PR adds in the weak subjectivity check as outlined in:
https://github.com/ethereum/eth2.0-specs/blob/master/specs/phase0/weak-subjectivity.md#weak-subjectivity-sync-procedure
This weak subjectivity check is currently opt-out. It'll switch to opt-in with the closure of issue #7342
Other notes for review
Tested run time under various conditions
1.) ws block not in db:
2.) ws block in db but incorrect epoch:
3.) ws check working during initial syncing
4.) ws check working for an almost caught up node