From dd9cb93b9d5d161ebffd6ab4966f6540a5a8794d Mon Sep 17 00:00:00 2001 From: twoeths Date: Thu, 20 Aug 2026 13:33:51 +0700 Subject: [PATCH 1/3] fix: recompute head after pulling up checkpoints --- .../test/spec/utils/specTestIterator.ts | 5 ----- .../fork-choice/src/forkChoice/forkChoice.ts | 16 ++++++++++++++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/beacon-node/test/spec/utils/specTestIterator.ts b/packages/beacon-node/test/spec/utils/specTestIterator.ts index cb96d7f87818..1cb0a77c79ca 100644 --- a/packages/beacon-node/test/spec/utils/specTestIterator.ts +++ b/packages/beacon-node/test/spec/utils/specTestIterator.ts @@ -102,11 +102,6 @@ export const defaultSkipOpts: SkipOpts = { // TODO-GLOAS: re-enable after gloas light client is implemented /\/gloas_fork$/, /\/heze_fork$/, - // TODO GLOAS: Proposer-boost dependent-root gate uses stale cached head across epoch-boundary ticks; - // boost wrongly denied. Fails identically on every pre-gloas fork. - // Enable this after https://github.com/ChainSafe/lodestar/issues/9666 is resolved - // The case name embeds the generation seed, so it changes whenever comptests are regenerated. - /fork_choice_compliance\/block_tree_test\/pyspec_tests\/block_tree_test_17_381675768_1$/, // TODO GLOAS: gloas/heze take ~23-24s on the mainnet preset (~7.5x pre-gloas) because every // post-gloas slot writes into the SLOTS_PER_HISTORICAL_ROOT-wide executionPayloadAvailability // bitvector, and this suite steps 8192 slots. That is 76-81% of the 30s sanity/slots timeout, diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 0169bd9d716e..415f046e8e99 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -1737,18 +1737,24 @@ export class ForkChoice implements IForkChoice { justifiedCheckpoint: CheckpointWithHex, finalizedCheckpoint: CheckpointWithHex, getJustifiedBalances: () => JustifiedBalances - ): void { + ): boolean { + let updated = false; + // Update justified checkpoint. if (justifiedCheckpoint.epoch > this.fcStore.justified.checkpoint.epoch) { this.fcStore.justified = {checkpoint: justifiedCheckpoint, balances: getJustifiedBalances()}; this.justifiedProposerBoostScore = null; + updated = true; } // Update finalized checkpoint. if (finalizedCheckpoint.epoch > this.fcStore.finalizedCheckpoint.epoch) { this.fcStore.finalizedCheckpoint = finalizedCheckpoint; this.justifiedProposerBoostScore = null; + updated = true; } + + return updated; } /** @@ -2060,11 +2066,17 @@ export class ForkChoice implements IForkChoice { } // If a new epoch, pull-up justification and finalization from previous epoch - this.updateCheckpoints( + const didUpdateCheckpoints = this.updateCheckpoints( this.fcStore.unrealizedJustified.checkpoint, this.fcStore.unrealizedFinalizedCheckpoint, () => this.fcStore.unrealizedJustified.balances ); + + // recompute head if we pull-up checkpoints because the head's dependent root could be changed + // this is to make sure isProposerBoostSameDependentRoot() correct on block 0 of the next epoch + if (didUpdateCheckpoints) { + this.updateHead(); + } } /** From 5c340278538dab30e74d0767a13163840e5b1f1c Mon Sep 17 00:00:00 2001 From: twoeths Date: Thu, 20 Aug 2026 15:12:07 +0700 Subject: [PATCH 2/3] fix: coordinate with FCR --- .../fork-choice/src/forkChoice/forkChoice.ts | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 415f046e8e99..545d58883a58 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -1081,11 +1081,20 @@ export class ForkChoice implements IForkChoice { while (this.fcStore.currentSlot < currentSlot) { const previousSlot = this.fcStore.currentSlot; // Note: we are relying upon `onTick` to update `fcStore.time` to ensure we don't get stuck in a loop. - this.onTick(previousSlot + 1); + const didUpdateCheckpoints = this.onTick(previousSlot + 1); this.queuedAttestationsPreviousSlot = 0; // Process any attestations that might now be eligible before running FCR for this slot. this.processAttestationQueue(); - this.runFastConfirmation(); + const didRecomputeHead = this.runFastConfirmation(); + + // An epoch-boundary checkpoint pull-up can move the head's dependent root and stale the cached + // head before block 0 of the new epoch is imported, making isProposerBoostSameDependentRoot() + // wrong for that block. Recompute the head so it reflects the new checkpoint and the queued + // votes — unless fast confirmation already did, to avoid a redundant head calculation. + if (didUpdateCheckpoints && !didRecomputeHead) { + this.updateHead(); + } + this.validatedAttestationDatas = new Set(); } } @@ -2041,7 +2050,7 @@ export class ForkChoice implements IForkChoice { * * https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/phase0/fork-choice.md#on_tick */ - private onTick(time: Slot): void { + private onTick(time: Slot): boolean { const previousSlot = this.fcStore.currentSlot; if (time > previousSlot + 1) { @@ -2062,21 +2071,16 @@ export class ForkChoice implements IForkChoice { // Not a new epoch, return. if (computeSlotsSinceEpochStart(time) !== 0) { - return; + return false; } - // If a new epoch, pull-up justification and finalization from previous epoch - const didUpdateCheckpoints = this.updateCheckpoints( + // If a new epoch, pull-up justification and finalization from previous epoch. Returns whether a + // checkpoint moved, so the caller can decide to recompute the head (see `updateTime`). + return this.updateCheckpoints( this.fcStore.unrealizedJustified.checkpoint, this.fcStore.unrealizedFinalizedCheckpoint, () => this.fcStore.unrealizedJustified.balances ); - - // recompute head if we pull-up checkpoints because the head's dependent root could be changed - // this is to make sure isProposerBoostSameDependentRoot() correct on block 0 of the next epoch - if (didUpdateCheckpoints) { - this.updateHead(); - } } /** @@ -2131,10 +2135,11 @@ export class ForkChoice implements IForkChoice { return {prelimProposerHead}; } - private runFastConfirmation(): void { + /** Returns whether it recomputed the head, so the caller can avoid a redundant `updateHead()`. */ + private runFastConfirmation(): boolean { const fastConfirmationRule = this.fastConfirmationRule; const fastConfirmationContext = this.fastConfirmationContext; - if (!fastConfirmationRule || !fastConfirmationContext) return; + if (!fastConfirmationRule || !fastConfirmationContext) return false; if (this.fastConfirmationPaused) { // Keep consumers on a safe, available root while the rule is paused @@ -2145,7 +2150,7 @@ export class ForkChoice implements IForkChoice { // Runs outside the timed try/catch below; a throw would escape to the clock listener this.logger?.debug("Fast confirmation notify failed", {slot: this.fcStore.currentSlot}, err as Error); } - return; + return false; } withObservedDuration(this.metrics?.fastConfirmation.totalDuration.startTimer(), () => { @@ -2168,6 +2173,8 @@ export class ForkChoice implements IForkChoice { ); } }); + + return true; } private createFastConfirmationContext(): FastConfirmationContext { From 5d0cee976a994fed7da6f7c48bbc7579b44970b3 Mon Sep 17 00:00:00 2001 From: lodekeeper-z <258924193+lodekeeper-z@users.noreply.github.com> Date: Mon, 24 Aug 2026 12:08:23 -0400 Subject: [PATCH 3/3] docs(fork-choice): document checkpoint return values (#9908) ## Summary - document the boolean return values of `updateCheckpoints()` and `onTick()` - remove the now-redundant inline return-value explanation ## Verification - `pnpm exec biome check packages/fork-choice/src/forkChoice/forkChoice.ts` --- packages/fork-choice/src/forkChoice/forkChoice.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 545d58883a58..36d77b7a5f76 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -1741,6 +1741,8 @@ export class ForkChoice implements IForkChoice { * May need the justified balances of: * - unrealizedJustified: Already available in `CheckpointWithBalance` * Since this balances are already available the getter is just `() => balances`, without cache interaction + * + * @returns Whether either checkpoint was updated. */ private updateCheckpoints( justifiedCheckpoint: CheckpointWithHex, @@ -2049,6 +2051,8 @@ export class ForkChoice implements IForkChoice { * Equivalent to: * * https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/phase0/fork-choice.md#on_tick + * + * @returns Whether an epoch-boundary checkpoint was updated. */ private onTick(time: Slot): boolean { const previousSlot = this.fcStore.currentSlot; @@ -2074,8 +2078,7 @@ export class ForkChoice implements IForkChoice { return false; } - // If a new epoch, pull-up justification and finalization from previous epoch. Returns whether a - // checkpoint moved, so the caller can decide to recompute the head (see `updateTime`). + // If a new epoch, pull-up justification and finalization from previous epoch. return this.updateCheckpoints( this.fcStore.unrealizedJustified.checkpoint, this.fcStore.unrealizedFinalizedCheckpoint,