-
-
Notifications
You must be signed in to change notification settings - Fork 457
feat: defer voluntary exits with transient validation failures #9216
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
base: unstable
Are you sure you want to change the base?
Changes from all commits
e853959
d3881da
ae85846
56051d6
468ac5b
4f0ece2
0872510
2872fd5
977a9de
fe98ce0
132e11f
cbeffd3
91dfaae
443cc94
bb21fca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import {Logger} from "@lodestar/logger"; | ||
| import {IBeaconStateView, VoluntaryExitValidity, isTransientExitValidity} from "@lodestar/state-transition"; | ||
| import {Epoch, ValidatorIndex} from "@lodestar/types"; | ||
| import {SignedVoluntaryExit} from "@lodestar/types/phase0"; | ||
|
|
||
| type DeferredEntry = { | ||
| exit: SignedVoluntaryExit; | ||
| validity: VoluntaryExitValidity; | ||
| insertedAtEpoch: Epoch; | ||
| }; | ||
|
|
||
| export class DeferredVoluntaryExitPool { | ||
| private pool = new Map<ValidatorIndex, DeferredEntry>(); | ||
|
|
||
| constructor( | ||
| private readonly logger: Logger, | ||
| private readonly maxSize = 1024, | ||
| private readonly maxDeferEpochs = 256 | ||
| ) {} | ||
|
|
||
| insert(exit: SignedVoluntaryExit, validity: VoluntaryExitValidity, currentEpoch: Epoch): boolean { | ||
| if (!isTransientExitValidity(validity)) return false; | ||
| if (this.pool.size === this.maxSize) return false; | ||
| if (this.pool.has(exit.message.validatorIndex)) return false; | ||
|
|
||
| this.pool.set(exit.message.validatorIndex, {exit, validity, insertedAtEpoch: currentEpoch}); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| retrieveProcessableExits(state: IBeaconStateView): SignedVoluntaryExit[] { | ||
| const epoch = state.epoch; | ||
| const validExits: SignedVoluntaryExit[] = []; | ||
| for (const [validatorIndex, entry] of this.pool) { | ||
| try { | ||
| if (epoch - entry.insertedAtEpoch > this.maxDeferEpochs) { | ||
| this.pool.delete(validatorIndex); | ||
| continue; | ||
| } | ||
| const validity = state.getVoluntaryExitValidity(entry.exit, false); | ||
| if (validity === VoluntaryExitValidity.valid) { | ||
| validExits.push(entry.exit); | ||
| this.pool.delete(validatorIndex); | ||
| } else if (!isTransientExitValidity(validity)) { | ||
| this.pool.delete(validatorIndex); | ||
| } | ||
| // Else if still transient - keep | ||
| } catch (e) { | ||
| this.logger.debug("Processing deferred voluntary exit failed", {validatorIndex}, e as Error); | ||
| } | ||
|
markolazic01 marked this conversation as resolved.
|
||
| } | ||
| return validExits; | ||
| } | ||
|
|
||
| size(): number { | ||
| return this.pool.size; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import {VoluntaryExitValidity, getVoluntaryExitSignatureSet} from "@lodestar/state-transition"; | ||
| import {VoluntaryExitValidity, getVoluntaryExitSignatureSet, isTransientExitValidity} from "@lodestar/state-transition"; | ||
| import {phase0} from "@lodestar/types"; | ||
| import { | ||
| GossipAction, | ||
|
|
@@ -9,12 +9,50 @@ import { | |
| import {IBeaconChain} from "../index.js"; | ||
| import {RegenCaller} from "../regen/index.js"; | ||
|
|
||
| export type ApiVoluntaryExitResult = {status: "published"} | {status: "deferred"; validity: VoluntaryExitValidity}; | ||
|
|
||
| // Comments for each call are present inside `validateVoluntaryExit`. | ||
| 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); | ||
|
|
||
| if (voluntaryExit.message.validatorIndex >= state.validatorCount) { | ||
| throw new VoluntaryExitError(GossipAction.REJECT, { | ||
| code: VoluntaryExitErrorCode.INACTIVE, | ||
| }); | ||
| } | ||
|
|
||
| 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}))) { | ||
|
Comment on lines
+43
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This path now verifies signatures for transient validity results, but Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| 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"}; | ||
| } | ||
|
Comment on lines
15
to
56
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
| export async function validateGossipVoluntaryExit( | ||
|
|
||
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.
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 intoopPoolwithout 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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