-
Notifications
You must be signed in to change notification settings - Fork 801
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
[Merged by Bors] - Fix voluntary exit to work with latest beacon api #2257
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.
Nice, thanks for following up on this. There's a TODO in here, which I've commented. Perhaps we could also add some sort of message telling the user that we're going to wait until the exit is accepted into the chain and it might take several minutes. Perhaps adding a message when we poll but don't break
would be handy too?
loop { | ||
// Sleep for 1 slot duration and then check if voluntary exit was processed | ||
// by checking the validator status. | ||
// TODO: should we wait for longer to account for forks? |
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.
TODO
Perhaps we just update the "Voluntary exit has been accepted..." text to say something along the lines of:
"Voluntary exit has been accepted into the beacon chain, but not yet finalized. Finalization may take several minutes or longer. Before finalization there is a low probability that the exit may be reverted."
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.
Yep, this sounds perfect. Will make the changes.
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.
Nice one!
bors r+
## Issue Addressed Also fixes #1932 ## Proposed Changes Use `ValidatorStatus::ActiveOngoing` instead of `ValidatorStatus::Active` to filter active validators. Prints extra information regarding successful voluntary exit.
I'll batch this bors r- |
Canceled. |
bors r+ |
## Issue Addressed Also fixes #1932 ## Proposed Changes Use `ValidatorStatus::ActiveOngoing` instead of `ValidatorStatus::Active` to filter active validators. Prints extra information regarding successful voluntary exit.
Pull request successfully merged into unstable. Build succeeded: |
Issue Addressed
Also fixes #1932
Proposed Changes
Use
ValidatorStatus::ActiveOngoing
instead ofValidatorStatus::Active
to filter active validators.Prints extra information regarding successful voluntary exit.