-
Notifications
You must be signed in to change notification settings - Fork 918
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
Use WaitForSynced in validator client for startup #5465
Use WaitForSynced in validator client for startup #5465
Conversation
…simplify-validator
…simplify-validator
…simplify-validator
Tests fail. Please wait to merge new features until after Topaz starts |
Is this unblocked now @0xKiwi ? |
Yes @nisdas |
…simplify-val-client
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.
Cant you split the client and server changes into 2 PRs ?
I did! This is the validator half but its waiting on #5366 to be merged first so it has changes for both :) |
Ah #5366 is meant to be merged first, just saw it. Lets merge that then |
…simplify-val-client
…into simplify-val-client
@@ -178,6 +178,7 @@ func (s *Service) Syncing() bool { | |||
func (s *Service) Resync() error { | |||
// set it to false since we are syncing again | |||
s.synced = false | |||
// How does the validator handle this? |
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 there be anything special done here? Missed it in the beacon-chain PR.
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.
Don't think you can, you would have to create new methods in the validator client to handle this.
…simplify-val-client
Codecov Report
@@ Coverage Diff @@
## master #5465 +/- ##
==========================================
+ Coverage 14.88% 19.00% +4.11%
==========================================
Files 8 238 +230
Lines 430 20326 +19896
==========================================
+ Hits 64 3862 +3798
- Misses 351 15869 +15518
- Partials 15 595 +580 |
…simplify-val-client
…simplify-val-client
if err != nil { | ||
return errors.Wrap(err, "could not setup beacon chain Synced streaming client") | ||
} | ||
for { |
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.
is there a need for this for loop. seem to happen once
Part of #5345, meant to be merged after #5366 , this PR changes the validator client to use
WaitForSynced
to listen for what the validator is ready, rather than pinging if the node is synced every 6 seconds.This feature is gated by the
--wait-for-synced
flag on the validator client.Confirmed in runtime:
Old Startup:
New Startup:
Resolves #5452