From 22c8eff307825791df880c0e11335c0b2de53f34 Mon Sep 17 00:00:00 2001
From: NC <17676176+ensi321@users.noreply.github.com>
Date: Thu, 2 Jul 2026 20:35:04 -0700
Subject: [PATCH] refactor: defer fork-choice store mutations until after
 protoArray.onBlock
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Per review on #9565. `onBlock` mutated `proposerBoostRoot` and the justified/
finalized checkpoints before `protoArray.onBlock()` inserted the block. If the
insertion threw, the store was left pointing at / justified on a block that was
never added.

Compute the proposer-boost decision into a temp, pass the candidate root to
`protoArray.onBlock()` for its best-child weighting, and commit the store
mutations (proposerBoostRoot + updateCheckpoints + updateUnrealizedCheckpoints)
only after the insertion succeeds — mirroring the spec's `on_block` ordering.
Behavior-preserving on the happy path: ProtoArray holds its own justified/
finalized epochs and never reads fcStore, so checkpoint-update ordering does not
affect insertion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../fork-choice/src/forkChoice/forkChoice.ts  | 60 ++++++++++---------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts
index 51d358000c4d..3c71ffbaf9d6 100644
--- a/packages/fork-choice/src/forkChoice/forkChoice.ts
+++ b/packages/fork-choice/src/forkChoice/forkChoice.ts
@@ -698,32 +698,27 @@ export class ForkChoice implements IForkChoice {
     const blockRoot = this.config.getForkTypes(slot).BeaconBlock.hashTreeRoot(block);
     const blockRootHex = toRootHex(blockRoot);
 
-    // Assign proposer score boost if the block is timely
-    // before attesting interval = before 1st interval
+    // Decide whether this block should receive proposer boost, and whether the block is timely.
+    // The store field `this.proposerBoostRoot` and `updateCheckpoints()` are mutated only after
+    // `protoArray.onBlock()` succeeds
     const isTimely = this.isBlockTimely(block, blockDelaySec);
-    if (
-      this.opts?.proposerBoost &&
+    const isProposerBoostBlock =
+      this.opts?.proposerBoost === true &&
       isTimely &&
       // only boost the first block we see
       this.proposerBoostRoot === null &&
       expectedProposerIndex !== null &&
       block.proposerIndex === expectedProposerIndex &&
-      this.isProposerBoostSameDependentRoot(this.head.blockRoot, parentRootHex)
-    ) {
-      this.proposerBoostRoot = blockRootHex;
-    }
+      this.isProposerBoostSameDependentRoot(this.head.blockRoot, parentRootHex);
+    // Candidate boost root used for protoArray.onBlock's best-child weighting. Committed to the
+    // store only after the insertion succeeds.
+    const proposerBoostRoot = isProposerBoostBlock ? blockRootHex : this.proposerBoostRoot;
 
     const justifiedCheckpoint = toCheckpointWithHex(state.currentJustifiedCheckpoint);
     const stateJustifiedEpoch = justifiedCheckpoint.epoch;
 
     const finalizedCheckpoint = toCheckpointWithHex(state.finalizedCheckpoint);
 
-    // Justified balances for `justifiedCheckpoint` are new to the fork-choice. Compute them on demand only if
-    // the justified checkpoint changes
-    this.updateCheckpoints(justifiedCheckpoint, finalizedCheckpoint, () =>
-      this.fcStore.justifiedBalancesGetter(justifiedCheckpoint, state)
-    );
-
     const blockEpoch = computeEpochAtSlot(slot);
 
     // same logic to compute_pulled_up_tip in the spec, making it inline because of reusing variables
@@ -767,19 +762,6 @@ export class ForkChoice implements IForkChoice {
       unrealizedFinalizedCheckpoint = finalizedCheckpoint;
     }
 
-    // Un-realized checkpoints
-    // Update best known unrealized justified & finalized checkpoints
-    this.updateUnrealizedCheckpoints(unrealizedJustifiedCheckpoint, unrealizedFinalizedCheckpoint, () =>
-      this.fcStore.justifiedBalancesGetter(unrealizedJustifiedCheckpoint, state)
-    );
-
-    // If block is from past epochs, try to update store's justified & finalized checkpoints right away
-    if (blockEpoch < computeEpochAtSlot(currentSlot)) {
-      this.updateCheckpoints(unrealizedJustifiedCheckpoint, unrealizedFinalizedCheckpoint, () =>
-        this.fcStore.justifiedBalancesGetter(unrealizedJustifiedCheckpoint, state)
-      );
-    }
-
     const targetSlot = computeStartSlotAtEpoch(blockEpoch);
     const targetRoot = slot === targetSlot ? blockRoot : state.getBlockRootAtSlot(targetSlot);
 
@@ -850,7 +832,29 @@ export class ForkChoice implements IForkChoice {
       parentBlockHash: parentHashHex,
     };
 
-    this.protoArray.onBlock(protoBlock, currentSlot, this.proposerBoostRoot);
+    this.protoArray.onBlock(protoBlock, currentSlot, proposerBoostRoot);
+
+    if (isProposerBoostBlock) {
+      this.proposerBoostRoot = blockRootHex;
+    }
+
+    // Justified balances for `justifiedCheckpoint` are new to the fork-choice. Compute them on
+    // demand only if the justified checkpoint changes.
+    this.updateCheckpoints(justifiedCheckpoint, finalizedCheckpoint, () =>
+      this.fcStore.justifiedBalancesGetter(justifiedCheckpoint, state)
+    );
+
+    // Un-realized checkpoints. Update best known unrealized justified & finalized checkpoints
+    this.updateUnrealizedCheckpoints(unrealizedJustifiedCheckpoint, unrealizedFinalizedCheckpoint, () =>
+      this.fcStore.justifiedBalancesGetter(unrealizedJustifiedCheckpoint, state)
+    );
+
+    // If block is from past epochs, try to update store's justified & finalized checkpoints right away
+    if (blockEpoch < computeEpochAtSlot(currentSlot)) {
+      this.updateCheckpoints(unrealizedJustifiedCheckpoint, unrealizedFinalizedCheckpoint, () =>
+        this.fcStore.justifiedBalancesGetter(unrealizedJustifiedCheckpoint, state)
+      );
+    }
 
     return protoBlock;
   }