feat: defer voluntary exits with transient validation failures#9216
feat: defer voluntary exits with transient validation failures#9216markolazic01 wants to merge 15 commits intoChainSafe:unstablefrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a deferred voluntary exit pool to handle exits that are transiently invalid, such as those submitted too early. It includes a background process to re-validate and publish these exits every epoch. Feedback suggests integrating published exits into the local operation pool and event system, adding error handling to prevent log spam from persistent failures in the pool, and refactoring duplicated validation logic.
| export async function validateApiVoluntaryExit( | ||
| chain: IBeaconChain, | ||
| voluntaryExit: phase0.SignedVoluntaryExit | ||
| ): Promise<void> { | ||
| ): Promise<ApiVoluntaryExitResult> { | ||
| const prioritizeBls = true; | ||
| return validateVoluntaryExit(chain, voluntaryExit, prioritizeBls); | ||
|
|
||
| if (chain.opPool.hasSeenVoluntaryExit(voluntaryExit.message.validatorIndex)) { | ||
| throw new VoluntaryExitError(GossipAction.IGNORE, { | ||
| code: VoluntaryExitErrorCode.ALREADY_EXISTS, | ||
| }); | ||
| } | ||
|
|
||
| const state = await chain.getHeadStateAtCurrentEpoch(RegenCaller.validateApiVoluntaryExit); | ||
| const validity = state.getVoluntaryExitValidity(voluntaryExit, false); | ||
|
|
||
| if (validity !== VoluntaryExitValidity.valid && !isTransientExitValidity(validity)) { | ||
| throw new VoluntaryExitError(GossipAction.REJECT, { | ||
| code: voluntaryExitValidityToErrorCode(validity), | ||
| }); | ||
| } | ||
|
|
||
| const signatureSet = getVoluntaryExitSignatureSet(chain.config, state, voluntaryExit); | ||
| if (!(await chain.bls.verifySignatureSets([signatureSet], {batchable: true, priority: prioritizeBls}))) { | ||
| throw new VoluntaryExitError(GossipAction.REJECT, { | ||
| code: VoluntaryExitErrorCode.INVALID_SIGNATURE, | ||
| }); | ||
| } | ||
|
|
||
| if (validity !== VoluntaryExitValidity.valid) { | ||
| // Transient failure — signature is good, defer | ||
| return {status: "deferred", validity}; | ||
| } | ||
|
|
||
| return {status: "published"}; | ||
| } |
There was a problem hiding this comment.
The logic in validateApiVoluntaryExit significantly duplicates validateVoluntaryExit. Consider refactoring these into a shared helper function or making validateVoluntaryExit return a result object that includes the validity status, which would improve maintainability and ensure consistency in validation logic.
There was a problem hiding this comment.
I would prefer to keep these 2 paths distinct as the flow differs. Even though they seem similar it is probably cleanest to keep the things as they are. I am happy to reconsider the change if reviewers require so.
|
Manual e2e succeeded, details in the description. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08725109b0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const validity = state.getVoluntaryExitValidity(entry.exit, false); | ||
| if (validity === VoluntaryExitValidity.valid) { | ||
| validExits.push(entry.exit); |
There was a problem hiding this comment.
Deferred exits are promoted with state.getVoluntaryExitValidity(entry.exit, false), which skips signature checks, even though those signatures were verified only at initial submission time. On pre-Deneb networks, a deferred exit can cross a fork boundary where the voluntary-exit signing domain changes; the signature that was valid at enqueue time can become invalid later. Because this path then inserts the exit into opPool without re-verifying against the current state, the node can propagate/include an invalid voluntary exit and risk producing an invalid block.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This does not apply for the future forks as all post-Deneb forks have a fixed Capella domain for voluntary exits which means that the exit will always remain valid.
Source: fn getDomainForVoluntaryExit
| const signatureSet = getVoluntaryExitSignatureSet(chain.config, state, voluntaryExit); | ||
| if (!(await chain.bls.verifySignatureSets([signatureSet], {batchable: true, priority: prioritizeBls}))) { |
There was a problem hiding this comment.
This path now verifies signatures for transient validity results, but earlyEpoch is computed before checking whether the validator index exists. A request with a future epoch and a non-existent validator index therefore reaches signature verification, where pubkey lookup throws (getOrThrow) rather than returning a typed validation error. That exception is not converted to an ApiError, so the REST layer returns a 500 instead of a client-facing 4xx rejection for malformed input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added an explicit validator existence check before the signature verification for a cleaner fail, but this exception not being converted to 4xx seems like a preexisting issue. It is probably worth fixing but out of scope for this PR.
1 participant
Motivation
Feature that enables postponed execution of voluntary exits that are not initially processable (transiently invalid).
Description
Perception of voluntary exit validity is split into two categories: permanent and transient.
Transient (in)validity is the one that can become valid later on, and those exits we keep in the
deferredVoluntaryExitPool.They are kept until they become permanently invalid, valid or until the max defer window elapses.
Manual e2e test on local devnet suceeded with the following flow:
submitted a future-dated voluntary exit via the API, confirmed it was deferred (due to shortTimeActive), observed it publish automatically on the next eligible epoch, verified block inclusion and validator status transition to active_exiting.
Pool size metric added.
Closes #7431
AI Assistance Disclosure
Development of this feature was assisted by Claude, helped understanding the existing codebase patterns, reviewing code and consultations on ideas during the process.