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 voluntary exit via validator manager #6612

Open
wants to merge 49 commits into
base: unstable
Choose a base branch
from

Conversation

chong-he
Copy link
Member

@chong-he chong-he commented Nov 26, 2024

Issue Addressed

Proposed Changes

-Add voluntary exit feature to the validator manager
-Add delete all validators by using the keyword "all"

Additional Info

I reuse some code from the lighthouse account validator exit file:
https://github.com/sigp/lighthouse/blob/stable/account_manager/src/validator/exit.rs

Thank you @michaelsproul for the help and guidance along the way

@chong-he chong-he added work-in-progress PR is a work-in-progress UX-and-logs labels Nov 26, 2024
}

// Check validator status after publishing voluntary exit
if exit_status {
Copy link
Member Author

@chong-he chong-he Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The --status is used to query the exit status of validators. The reason behind introducing it is:

In lighthouse account validator exit, the status was shown after a sleep of, e.g., 12s

sleep(Duration::from_secs(spec.seconds_per_slot)).await;

If we introduce a sleep here, then for each validator_to_exit, it will sleep for 12s, which is not really UX friendly. For practical consideration, the user's priority may just want to first publish the exit messages for all validators.

While there is a way to tweak it so that it only sleeps once and not for the rest of validator_to_exit (or shortening the sleep time), doing so will likely cause the exit status to be shown as:

Voluntary exit for Validator {} is waiting to be accepted into the beacon chain.

for the rest of validator_to_exit as the head block is not yet updated, which is not really useful either.

So I ended up introducing this --status flag to query the exit status separately. But I am not sure if this is a good idea.

Keen on hear the thoughts of reviewers on this.

@chong-he chong-he added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Dec 18, 2024
@michaelsproul michaelsproul added the v6.1.0 New release c. Q1 2025 label Dec 19, 2024
@michaelsproul michaelsproul self-requested a review December 19, 2024 04:13
}
}

if merge {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this --merge is useful? I tested pasting the generated JSON on beaconcha.in and it doesn't seem to work. Maybe it is an issue with the generated JSON, or is it because submitting voluntary exit can only be done one at a time?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review The code is ready for review UX-and-logs v6.1.0 New release c. Q1 2025
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants