Repository navigation
feat(validator): voluntary exits, as an ethlambda voluntary-exit command and the keymanager /voluntary_exit route - #779
Conversation
…exit`
sub-command that signs one keystore's exit, asks for a confirmation phrase,
publishes it to a beacon node and waits for the chain to accept it, plus the
keymanager API's POST /eth/v1/validator/{pubkey}/voluntary_exit, which signs
without publishing. Exits sign under the capella fork version from deneb on
(EIP-7044), keyed on the fork at the exit's epoch as Lighthouse does, so a
pre-signed exit stays valid across later forks.
The beacon node API gains `validator` (one registry entry with its activation,
exit and withdrawable epochs) and `submit_voluntary_exit`, implemented for the
HTTP client, failover and the test mock. The keymanager context carries the
beacon node, signing context and clock the new route needs.
The client's three required flags become `Option`s clap still marks required,
because `subcommand_negates_reqs` only relaxes the parser: the derived
`from_arg_matches` refused `validator exit` on a plain `PathBuf` field.
🤖 Codex Code Review
Tests could not run because rustup attempted to write to a read-only directory. Automated review by OpenAI Codex · gpt-6.1-sol · custom prompt |
🤖 Claude Code ReviewReview:
|
🤖 Gemini Code ReviewThis PR is well-structured and thoroughly tested, including EIP-7044 ( A few minor edge cases in
Automated review by Gemini · gemini-4-argon · custom prompt |
🤖 Grok Code ReviewThe voluntary-exit flow is careful in the right places (syncing refusal, pubkey/index binding, confirmation before publish, EIP-7044 called out, e2e coverage). A few consensus and CLI correctness gaps should be fixed before this is relied on for real exits.
The pubkey check in Automated review by Grok (xAI) · grok-4.7 · custom prompt |
🤖 DeepSeek Code ReviewOverall the PR is well structured and the EIP-7044 signing-domain logic looks correct. The CLI split, the response-pubkey check in
Automated review by DeepSeek · deepseek-v4-pro · custom prompt |
🤖 Kimi Code Review
The normal current/future-epoch path correctly implements EIP-7044. The confirmation flow, presign/no-publication separation, node-response pubkey check, and keymanager authentication coverage look good. Automated review by Kimi (Moonshot AI) · kimi-k3.1-pro · custom prompt |
Conflicts were all two sides adding to the same place, and both are kept: - bin/ethlambda/src/main.rs: the `validator exit` dispatch beside the new `slashing-protection` command. - bin/ethlambda/src/validator.rs: `--init-slashing-protection` is read before `ValidatorOptions::required` consumes the options. - crates/validator: `KeymanagerContext` carries the exit route's `beacon_node`, `signing` and `clock` beside main's `definitions_unflushed` and `protection`, at its definition and at every site that builds one, the exit route's own test harness included.
…e rest of the review The signing domain was keyed on the fork at the exit's own epoch. A node verifies against the state that processes the exit: any deneb-or-later state uses the capella version whatever epoch the exit names (EIP-7044), so `--epoch 0`, or `?epoch=0` on the keymanager route, produced an exit every node on a chain past deneb rejects. The domain now follows the fork at max(current_epoch, exit.epoch), the earliest epoch a state can process the exit in, and before deneb mirrors `get_domain` (the processing state's previous version for an epoch before its fork). `sign_voluntary_exit` and `voluntary_exit_signing_root` take the current epoch for that. `ethlambda validator exit`: - The shard committee period is measured against max(current_epoch, exit.epoch), as the chain measures it, so an exit naming an old epoch is no longer refused for a validator that is eligible now. A pre-signed exit is still checked against the epoch it is signed for. - A future `--epoch` without `--presign` is refused before the prompt rather than by the node's 400 after it. - The wait for the chain is bounded at two epochs, ends on any status that carries an exit epoch (`active_slashed` and `withdrawal_*` included), and no longer turns a node error after the publish into a failed run: the exit went out, so the node is asked again next slot. - An aborted confirmation prompt, an empty stdin included, exits with a failing status. - The slots-to-seconds product no longer truncates through `as u32`, a JSON encoding failure is no longer reported as a malformed response, and the password is zeroized on drop like the secret it decrypts. The keymanager route's comment no longer claims the read lock is held across the beacon node round trip; a delete landing in between is the same 404. docs/cli.md follows all of the above and notes electra's pending-withdrawal refusal, which only the node can check. Tests: the domain for a pre-deneb epoch on a chain past deneb and for an epoch two forks back before it; the old-epoch, future-epoch and pre-signed eligibility cases; every terminal status of the wait, its bound, and an outage after the publish, with the mock answering as a chain that processed the exit instead of a racing flip; the keymanager route at `?epoch=0`; and an exit naming epoch 0 accepted by this node's Beacon API end to end.
|
/review |
MegaRedHand
left a comment
MegaRedHand
left a comment
There was a problem hiding this comment.
No correctness issues found in the exit logic or the EIP-7044 domain handling. The tests against the STF's process_voluntary_exit domain and the end-to-end run against this node's Beacon API are good to have.
One change I want before this merges: the validator client's required flags must not become Options that are still marked required (see the comment on bin/ethlambda/src/validator.rs). Please keep them as plain required fields and give exit its own command instead.
The rest are nits, plus a heads-up on how this collides with #776.
…nd validator lookups Review follow-ups on the voluntary exit: - `ethlambda validator exit` becomes `ethlambda voluntary-exit`, a command of its own like `slashing-protection`. Living inside `validator` had turned the client's three required flags into `Option`s clap still marked required, re-checked at runtime by `ValidatorOptions::required`, with `subcommand_negates_reqs` and `args_conflicts_with_subcommands` on the variant to let the exit omit them. All three workarounds are gone: `bin/ethlambda/src/validator.rs` is as on main, and a client run without its flags is clap's usage error again rather than a runtime one. The command's options and entry point move to `voluntary_exit.rs`. - `ValidatorStore::insert_keystore_file` is the one place a keystore and its password file are read from disk, used by `ValidatorStore::load` and by the exit command, which had its own copy of the same steps. The password is zeroized on drop in both now. - The keymanager route resolves the validator through `validator()`, the lookup the command makes, instead of `validator_indices`, so the two ways to sign an exit cannot disagree about a validator's index. - `HttpBeaconNode::get_optional` takes the status that means "nothing", a 204 or a 404, so `validator()` no longer hand-rolls its GET and error mapping. docs/cli.md gives `voluntary-exit` its own section beside `slashing-protection`.
pablodeymo
changed the title
ethlambda validator exit sub-command and the keymanager /voluntary_exit routeethlambda voluntary-exit command and the keymanager /voluntary_exit route
MegaRedHand
left a comment
MegaRedHand
left a comment
There was a problem hiding this comment.
Re-reviewed at 92a9d6b. All the earlier threads are addressed. The EIP-7044 domain now keys on max(current_epoch, exit.epoch), which matches what the processing state verifies (capella from deneb on, the previous fork's version for an older epoch before deneb), and the node's own gossip check verifies under the same domain.
One thing I'd fix before merging (the SHARD_COMMITTEE_PERIOD comment on exit.rs); the other two are nits.
… on a refused exit Review follow-ups on the voluntary exit: - `config_from_spec_response` starts from `Config::mainnet()` and never read `SHARD_COMMITTEE_PERIOD`, so the exit command measured every validator against mainnet's 256 epochs whatever the node reported. On a mainnet-preset devnet that shortens the period, the usual way to try an exit without waiting about 27 hours, it refused an eligible validator and no flag got past it. The key is read now, and the function's doc names the keys it reads and says every other `Config` field stays mainnet's, so the next field this crate comes to read is added there first. - `FallbackBeaconNode::submit_voluntary_exit` went through `try_each`, which walks past every `Err`: a 400 from the first node was resent to each node behind it and came back as `AllBeaconNodesFailed`, logged as a missed duty, where the trait promises `BeaconNodeStatus` with the node's reason. A 400 now ends the walk and is returned as it is. Any other failure still fails over, since it is about the node and not the exit. - `FallbackBeaconNode::validator` is documented as it behaves: the first node's answer is final, a 404 included, and with every node down `try_each` logs a missed duty for what is only the keymanager's request. The keymanager route's call site points at it. docs/cli.md says where the period comes from, and no longer lists voluntary exits among what the validator client does not implement. Tests: the period read, defaulted and rejected when malformed; the whole command run against this node serving a two-epoch period at epoch 2, which mainnet's 256 would refuse; a refused exit ending the walk with the node's reason and reaching no second node; an exit failing over from a node that is down; every node down still being `AllBeaconNodesFailed`.
…through the async signing path. The two collide in `http_api/mod.rs`, `http_api/proposer.rs`, `lib.rs`, `signing.rs` and `docs/cli.md`, and #779 signs through the synchronous `sign_root` this branch replaces. Both sides are kept: `KeymanagerContext` carries the remote signer default beside the exit route's node, signing context and clock. `sign_voluntary_exit` is async over `&RwLock<ValidatorStore>` like every other signer, so the keymanager route passes `&context.store` and holds no guard across what is a round trip for a remote key, and the `voluntary-exit` command puts its one decrypted keystore behind the lock. The `SigningContext { .. }` literals become `SigningContext::new`. A remote key's exit is a `VOLUNTARY_EXIT` request. Web3Signer derives the root itself from the fork it is told and refuses a `signingRoot` that disagrees (`Eth2SignForIdentifierHandler`, via Teku's `signingRootForSignVoluntaryExit`), and it has picked the rule by the exit's epoch in older releases and by the told fork's epoch in current ones. So the signer is told the one version the exit is signed under as both `previous_version` and `current_version`, with the exit's epoch, which lands on this client's root under either reading; `sign` gains `sign_under` to take that fork in place of the schedule's.
🗒️ Description / Motivation
voluntary_exitroute, and this client had neither. The beacon node already accepts, validates, pools and gossips aSignedVoluntaryExit(POST /eth/v1/beacon/pool/voluntary_exits), but nothing on the validator side could produce one.ethlambda voluntary-exit, which signs one keystore's exit and hands it to a beacon node, andPOST /eth/v1/validator/{pubkey}/voluntary_exiton the keymanager API, which signs without publishing.What Changed
crates/validator/src/signing.rs:voluntary_exit_signing_rootandsign_voluntary_exit. The domain follows the fork of the state that processes the exit, the one atmax(current_epoch, exit.epoch), which both functions take the current epoch for. EIP-7044: from deneb on that isCAPELLA_FORK_VERSIONwhatever epoch the exit names, an epoch from before deneb included; before deneb it mirrorsget_domainon that state (its previous fork version for an epoch before its fork). Lighthouse keys on the exit's epoch alone, which signs--epoch 0under the genesis version on a chain past deneb, an exit no node accepts. Local keystores only: it goes through the privatesign_root, which feat(validator): Web3Signer remote signing and the keymanager /eth/v1/remotekeys route #776 replaces (see Related).crates/validator/src/beacon_node/: two newBeaconNodeApimethods.validator(pubkey)isGET /eth/v1/beacon/states/head/validators/{pubkey}with the activation, exit and withdrawable epochs (404 isNone; an answer naming another key isInconsistentResponse).submit_voluntary_exitisPOST /eth/v1/beacon/pool/voluntary_exits; a 400 surfaces asBeaconNodeStatuswith the node's reason. Implemented for the HTTP client,FallbackBeaconNodeand the test mock.FallbackBeaconNode::submit_voluntary_exitfails over from a node that is down but ends the walk on a 400, so a refused exit is not resent to every node and reported as all of them failing;FallbackBeaconNode::validatortakes the first node's answer as final, a 404 included.config_from_spec_response(dto.rs) now readsSHARD_COMMITTEE_PERIOD, which it had left at mainnet's 256 whatever the node reported.crates/validator/src/exit.rs(new): the command. Refuses a syncing or optimistic node, then a future--epochwithout--presign, then a validator the chain would refuse (notactive_ongoing, or not yetSHARD_COMMITTEE_PERIODpast activation, naming the eligible epoch; the period is the node's own, from/eth/v1/config/spec, so a devnet that shortens it is followed, and it is measured againstmax(current_epoch, exit.epoch), as the chain measures it), signs, and unless--no-confirmationasks for the phraseExit my validatoron stdin before publishing.--presignstops after signing and returns theSignedVoluntaryExit. Unless--no-wait, it then polls the validator once a slot, for up to two epochs, until the chain has given it an exit epoch:active_exiting(printing the current, exit and withdrawable epochs and the time to the exit epoch), any later status, oractive_slashed. A node error during the wait is retried at the next slot, and a wait that runs out returns successfully saying so, since the exit was published.run_withis the testable core over anyBeaconNodeApiand anyBufRead. The keystore and its password file are read byValidatorStore::insert_keystore_file, the one helperValidatorStore::loadnow uses too, so the command and the client cannot come to read a password differently.crates/validator/src/http_api/voluntary_exit.rs(new): the keymanager route.epochquery parameter (the spec'sUint64string) or the current epoch by the client's clock; 404 for a key not held and for a key the beacon node has no index for yet, through the samevalidator()lookup the command makes; 400 for a malformed key or epoch; the read lock is not held across the beacon node round trip, so a keystore delete landing in between is the same 404.KeymanagerContextgainsbeacon_node,signingandclock;parse_uint64inproposer.rsnow names the field it rejects.bin/ethlambda/src/voluntary_exit.rs(new),command.rs,main.rs:ethlambda voluntary-exit --keystore <path> --password-file <path> [--beacon-node URL] [--epoch N] [--presign] [--no-confirmation] [--no-wait], a top-level command of its own likeslashing-protection. Logs go to stderr so a pre-signed exit on stdout pipes cleanly.ethlambda validatorandbin/ethlambda/src/validator.rsare untouched.crates/validator/src/error.rs:ExitRefused, deterministic likeProposalRefused.slot_clock.rs:slot_duration().docs/cli.md: avoluntary-exitsection (flags, the phrase, what the wait prints, the EIP-7044 note, where the shard committee period comes from) and the keymanager route. Thevalidatorsection no longer lists voluntary exits as not implemented.Correctness / Behavior Guarantees
voluntary_exit_signing_root, andone_object_root_signs_differently_under_each_domainstill covers the rest.process_voluntary_exitwould refuse, so an operator sees the reason rather than a node's 400; the node's gossip rules remain the authority and a refusal from them is reported verbatim.ethlambda validatoris exactly as onmain: its options, its required flags and its tests are not touched by this PR.--presignwith a future--epochproduces an exit a node would refuse today and accept once that epoch comes, which is the point of it. Without--presigna future epoch is refused before the prompt.Tests Added / Run
signing.rs: an exit verifies under its own root; at and after deneb it signs under the capella version and not the schedule's; an exit naming a pre-deneb epoch signs under capella on a chain past deneb; before deneb under what the processing state verifies, an epoch two forks back included.exit.rs: a syncing node, an unknown validator, an inactive one and one inside its shard committee period are refused with nothing published; an exit naming a past epoch publishes for an eligible validator; a future epoch without presign is refused; presign verifies and submits nothing; a wrong phrase submits nothing; the phrase publishes;--no-confirmationskips the prompt; the epoch defaults to the current one; the wait loop ends on every status that carries an exit epoch, gives up after its bound, and rides out a node outage after the publish (understart_paused).beacon_node/dto.rs:SHARD_COMMITTEE_PERIODis read from the spec response, keeps mainnet's value when absent, and is rejected when malformed.beacon_node/fallback.rs: a refused exit ends the walk with the node's 400 and reason and reaches no second node; an exit fails over from a node that is down; every node down is stillAllBeaconNodesFailed.http_api/voluntary_exit.rs: the happy path returns a verifying signature with quoted integers;?epoch=is honoured;?epoch=0on a chain past deneb verifies under the capella domain; malformed epoch and pubkey are 400; a key not held and a key with no index are 404; no bearer token is 401.crates/net/rpc/src/beacon/validator_client_tests.rs: the whole command (minus the prompt) run against this node's Beacon API over a real socket: the lookup, the signature, the pool endpoint, the gossip publish, a 404 for an unknown key, a forged exit answered with the node's 400, and an exit naming epoch 0 accepted by the node. A second test runs the command against this node serving a two-epochSHARD_COMMITTEE_PERIODat epoch 2, which mainnet's 256 would refuse.bin/ethlambda/src/command.rs:voluntary-exitparses as its own sub-command with its own flags, and is not given thenodedefault.cargo test -p ethlambda-validator --profile release-fast: 405 passed.cargo test -p ethlambda --profile release-fast: 266 passed.cargo test -p ethlambda-rpc --profile release-fast: 337 passed, the two integration tests among them.make lint: clean.cargo fmt --check: clean.Related Issues / PRs
sign_*async over&RwLock<ValidatorStore>, replacessign_rootwithsign(store, pubkey, root, epoch, SigningBody)and givesSigningContexta private field behindSigningContext::new.sign_voluntary_exit, theSigningContext { .. }literals here and the keymanager route's read guard all have to follow. Remote keys cannot sign exits untilSigningBodygains a voluntary-exit variant whosefork_infoyields the EIP-7044 capella domain rather than the exit epoch's scheduled fork; what Web3Signer does with it after deneb is still to be confirmed.--presignfor a validator that is not active yet (the keymanager route already signs for any held key with an index), a password prompt when--password-fileis absent (needs anrpassword-style dependency), exiting several validators in one call, and exiting through a running client's keymanager from the CLI (Lighthouse'svalidator-manager exit).process_voluntary_exitdomain.✅ Verification Checklist
make fmt— cleanmake lint(clippy with-D warnings) — cleanmake test— the validator crate, the binary and the rpc integration test were run atrelease-fast; CI runs the rest