fix: proposer boost reorg e2e test

packages/beacon-node/src/api/impl/validator/index.ts

@@ -323,10 +323,20 @@ export function getValidatorApi({
     {
       skipHeadChecksAndUpdate,
       commonBlockBody,
-    }: Omit<routes.validator.ExtraProduceBlockOps, "builderSelection"> & {
-      skipHeadChecksAndUpdate?: boolean;
-      commonBlockBody?: CommonBlockBody;
-    } = {}
+      parentBlockRoot: inParentBlockRoot,
+    }: Omit<routes.validator.ExtraProduceBlockOps, "builderSelection"> &
+      (
+        | {
+            skipHeadChecksAndUpdate: true;
+            commonBlockBody: CommonBlockBody;
+            parentBlockRoot: Root;
+          }
+        | {
+            skipHeadChecksAndUpdate?: false | undefined;
+            commonBlockBody?: undefined;
+            parentBlockRoot?: undefined;
+          }
+      ) = {}
   ): Promise<routes.validator.ProduceBlindedBlockRes> {
     const version = config.getForkName(slot);
     if (!isForkExecution(version)) {
@@ -344,6 +354,7 @@ export function getValidatorApi({
       throw Error("Execution builder disabled");
     }
 
+    let parentBlockRoot: Root;
     if (skipHeadChecksAndUpdate !== true) {
       notWhileSyncing();
       await waitForSlot(slot); // Must never request for a future slot > currentSlot
@@ -352,14 +363,17 @@ export function getValidatorApi({
       // forkChoice.updateTime() might have already been called by the onSlot clock
       // handler, in which case this should just return.
       chain.forkChoice.updateTime(slot);
-      chain.recomputeForkChoiceHead();
+      parentBlockRoot = fromHexString(chain.getProposerHead(slot).blockRoot);
+    } else {
+      parentBlockRoot = inParentBlockRoot;
     }
 
     let timer;
     try {
       timer = metrics?.blockProductionTime.startTimer();
       const {block, executionPayloadValue, consensusBlockValue} = await chain.produceBlindedBlock({
         slot,
+        parentBlockRoot,
         randaoReveal,
         graffiti: toGraffitiBuffer(graffiti || ""),
         commonBlockBody,
@@ -393,14 +407,21 @@ export function getValidatorApi({
       strictFeeRecipientCheck,
       skipHeadChecksAndUpdate,
       commonBlockBody,
-    }: Omit<routes.validator.ExtraProduceBlockOps, "builderSelection"> & {
-      skipHeadChecksAndUpdate?: boolean;
-      commonBlockBody?: CommonBlockBody;
-    } = {}
+      parentBlockRoot: inParentBlockRoot,
+    }: Omit<routes.validator.ExtraProduceBlockOps, "builderSelection"> &
+      (
+        | {
+            skipHeadChecksAndUpdate: true;
+            commonBlockBody: CommonBlockBody;
+            parentBlockRoot: Root;
+          }
+        | {skipHeadChecksAndUpdate?: false | undefined; commonBlockBody?: undefined; parentBlockRoot?: undefined}
+      ) = {}
   ): Promise<routes.validator.ProduceBlockOrContentsRes & {shouldOverrideBuilder?: boolean}> {
     const source = ProducedBlockSource.engine;
     metrics?.blockProductionRequests.inc({source});
 
+    let parentBlockRoot: Root;
     if (skipHeadChecksAndUpdate !== true) {
       notWhileSyncing();
       await waitForSlot(slot); // Must never request for a future slot > currentSlot
@@ -409,14 +430,17 @@ export function getValidatorApi({
       // forkChoice.updateTime() might have already been called by the onSlot clock
       // handler, in which case this should just return.
       chain.forkChoice.updateTime(slot);
-      chain.recomputeForkChoiceHead();
+      parentBlockRoot = fromHexString(chain.getProposerHead(slot).blockRoot);
+    } else {
+      parentBlockRoot = inParentBlockRoot;
     }
 
     let timer;
     try {
       timer = metrics?.blockProductionTime.startTimer();
       const {block, executionPayloadValue, consensusBlockValue, shouldOverrideBuilder} = await chain.produceBlock({
         slot,
+        parentBlockRoot,
         randaoReveal,
         graffiti: toGraffitiBuffer(graffiti || ""),
         feeRecipient,
@@ -528,13 +552,13 @@ export function getValidatorApi({
       };
 
       logger.verbose("Assembling block with produceEngineOrBuilderBlock", loggerContext);
-      const proposerHead = chain.getProposerHead(slot);
+      const parentBlockRoot = fromHexString(chain.getProposerHead(slot).blockRoot);
 
       const commonBlockBody = await chain.produceCommonBlockBody({
         slot,
+        parentBlockRoot,
         randaoReveal,
         graffiti: toGraffitiBuffer(graffiti || ""),
-        proposerHead,
       });
       logger.debug("Produced common block body", loggerContext);
 
@@ -557,6 +581,7 @@ export function getValidatorApi({
             // skip checking and recomputing head in these individual produce calls
             skipHeadChecksAndUpdate: true,
             commonBlockBody,
+            parentBlockRoot,
           })
         : Promise.reject(new Error("Builder disabled"));
 
@@ -567,6 +592,7 @@ export function getValidatorApi({
             // skip checking and recomputing head in these individual produce calls
             skipHeadChecksAndUpdate: true,
             commonBlockBody,
+            parentBlockRoot,
           }).then((engineBlock) => {
             // Once the engine returns a block, in the event of either:
             // - suspected builder censorship

packages/beacon-node/src/chain/chain.ts

@@ -483,22 +483,19 @@ export class BeaconChain implements IBeaconChain {
   }
 
   async produceCommonBlockBody(blockAttributes: BlockAttributes): Promise<CommonBlockBody> {
-    const {slot} = blockAttributes;
-    const head = blockAttributes.proposerHead ?? this.forkChoice.getHead();
+    const {slot, parentBlockRoot} = blockAttributes;
     const state = await this.regen.getBlockSlotState(
-      head.blockRoot,
+      toHexString(parentBlockRoot),
       slot,
       {dontTransferCache: true},
       RegenCaller.produceBlock
     );
-    const parentBlockRoot = fromHexString(head.blockRoot);
 
     // TODO: To avoid breaking changes for metric define this attribute
     const blockType = BlockType.Full;
 
     return produceCommonBlockBody.call(this, blockType, state, {
       ...blockAttributes,
-      parentBlockRoot,
       parentSlot: slot - 1,
     });
   }
@@ -522,21 +519,26 @@ export class BeaconChain implements IBeaconChain {
 
   async produceBlockWrapper<T extends BlockType>(
     blockType: T,
-    {randaoReveal, graffiti, slot, feeRecipient, commonBlockBody}: BlockAttributes & {commonBlockBody?: CommonBlockBody}
+    {
+      randaoReveal,
+      graffiti,
+      slot,
+      feeRecipient,
+      commonBlockBody,
+      parentBlockRoot,
+    }: BlockAttributes & {commonBlockBody?: CommonBlockBody}
   ): Promise<{
     block: AssembledBlockType<T>;
     executionPayloadValue: Wei;
     consensusBlockValue: Wei;
     shouldOverrideBuilder?: boolean;
   }> {
-    const head = this.forkChoice.getHead();
     const state = await this.regen.getBlockSlotState(
-      head.blockRoot,
+      toHexString(parentBlockRoot),
       slot,
       {dontTransferCache: true},
       RegenCaller.produceBlock
     );
-    const parentBlockRoot = fromHexString(head.blockRoot);
     const proposerIndex = state.epochCtx.getBeaconProposer(slot);
     const proposerPubKey = state.epochCtx.index2pubkey[proposerIndex].toBytes();
 

packages/beacon-node/src/chain/forkChoice/index.ts

@@ -7,7 +7,7 @@ import {
   ForkChoiceStore,
   ExecutionStatus,
   JustifiedBalancesGetter,
-  ForkChoiceOpts,
+  ForkChoiceOpts as RawForkChoiceOpts,
 } from "@lodestar/fork-choice";
 import {
   CachedBeaconStateAllForks,
@@ -22,7 +22,10 @@ import {ChainEventEmitter} from "../emitter.js";
 import {ChainEvent} from "../emitter.js";
 import {GENESIS_SLOT} from "../../constants/index.js";
 
-export type {ForkChoiceOpts};
+export type ForkChoiceOpts = RawForkChoiceOpts & {
+  // for testing only
+  forkchoiceConstructor?: typeof ForkChoice;
+};
 
 /**
  * Fork Choice extended with a ChainEventEmitter
@@ -49,7 +52,11 @@ export function initializeForkChoice(
 
   const justifiedBalances = getEffectiveBalanceIncrementsZeroInactive(state);
 
-  return new ForkChoice(
+  // forkchoiceConstructor is only used for some test cases
+  // production code use ForkChoice constructor directly
+  const forkchoiceConstructor = opts.forkchoiceConstructor ?? ForkChoice;
+
+  return new forkchoiceConstructor(
     config,
 
     new ForkChoiceStore(

packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts

@@ -28,7 +28,6 @@ import {ChainForkConfig} from "@lodestar/config";
 import {ForkSeq, ForkExecution, isForkExecution} from "@lodestar/params";
 import {toHex, sleep, Logger} from "@lodestar/utils";
 
-import {ProtoBlock} from "@lodestar/fork-choice";
 import type {BeaconChain} from "../chain.js";
 import {PayloadId, IExecutionEngine, IExecutionBuilder, PayloadAttributes} from "../../execution/index.js";
 import {ZERO_HASH, ZERO_HASH_HEX} from "../../constants/index.js";
@@ -65,8 +64,8 @@ export type BlockAttributes = {
   randaoReveal: BLSSignature;
   graffiti: Bytes32;
   slot: Slot;
+  parentBlockRoot: Root;
   feeRecipient?: string;
-  proposerHead?: ProtoBlock;
 };
 
 export enum BlockType {
@@ -97,7 +96,6 @@ export async function produceBlockBody<T extends BlockType>(
   currentState: CachedBeaconStateAllForks,
   blockAttr: BlockAttributes & {
     parentSlot: Slot;
-    parentBlockRoot: Root;
     proposerIndex: ValidatorIndex;
     proposerPubKey: BLSPubkey;
     commonBlockBody?: CommonBlockBody;
@@ -582,7 +580,6 @@ export async function produceCommonBlockBody<T extends BlockType>(
     parentBlockRoot,
   }: BlockAttributes & {
     parentSlot: Slot;
-    parentBlockRoot: Root;
   }
 ): Promise<CommonBlockBody> {
   const stepsMetrics =

packages/beacon-node/test/e2e/chain/proposerBoostReorg.test.ts

@@ -0,0 +1,140 @@
+import {describe, it, afterEach, expect} from "vitest";
+import {SLOTS_PER_EPOCH} from "@lodestar/params";
+import {TimestampFormatCode} from "@lodestar/logger";
+import {ChainConfig} from "@lodestar/config";
+import {RootHex, Slot} from "@lodestar/types";
+import {routes} from "@lodestar/api";
+import {toHexString} from "@lodestar/utils";
+import {LogLevel, TestLoggerOpts, testLogger} from "../../utils/logger.js";
+import {getDevBeaconNode} from "../../utils/node/beacon.js";
+import {TimelinessForkChoice} from "../../mocks/fork-choice/timeliness.js";
+import {getAndInitDevValidators} from "../../utils/node/validator.js";
+import {waitForEvent} from "../../utils/events/resolver.js";
+import {ReorgEventData} from "../../../src/chain/emitter.js";
+
+describe(
+  "proposer boost reorg",
+  function () {
+    const validatorCount = 8;
+    const testParams: Pick<ChainConfig, "SECONDS_PER_SLOT" | "REORG_PARENT_WEIGHT_THRESHOLD" | "PROPOSER_SCORE_BOOST"> =
+      {
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        SECONDS_PER_SLOT: 2,
+        // need this to make block `reorgSlot - 1` strong enough
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        REORG_PARENT_WEIGHT_THRESHOLD: 80,
+        // need this to make block `reorgSlot + 1` to become the head
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        PROPOSER_SCORE_BOOST: 120,
+      };
+
+    const afterEachCallbacks: (() => Promise<unknown> | void)[] = [];
+    afterEach(async () => {
+      while (afterEachCallbacks.length > 0) {
+        const callback = afterEachCallbacks.pop();
+        if (callback) await callback();
+      }
+    });
+
+    const reorgSlot = 10;
+    const proposerBoostReorgEnabled = true;
+    /**
+     *                reorgSlot
+     *              /
+     * reorgSlot - 1 ------------ reorgSlot + 1
+     *
+     * Note that in additional of being not timely, there are other criterion that
+     * the block needs to satisfied before being re-orged out. This test assumes
+     * other criterion are satisfied except timeliness.
+     * Note that in additional of being not timely, there are other criterion that 
+     * the block needs to satisfy before being re-orged out. This test assumes 
+     * other criterion are already satisfied
+     */
+    it(`should reorg a late block at slot ${reorgSlot}`, async () => {
+      // the node needs time to transpile/initialize bls worker threads
+      const genesisSlotsDelay = 7;
+      const genesisTime = Math.floor(Date.now() / 1000) + genesisSlotsDelay * testParams.SECONDS_PER_SLOT;
+      const testLoggerOpts: TestLoggerOpts = {
+        level: LogLevel.debug,
+        timestampFormat: {
+          format: TimestampFormatCode.EpochSlot,
+          genesisTime,
+          slotsPerEpoch: SLOTS_PER_EPOCH,
+          secondsPerSlot: testParams.SECONDS_PER_SLOT,
+        },
+      };
+      const logger = testLogger("BeaconNode", testLoggerOpts);
+      const bn = await getDevBeaconNode({
+        params: testParams,
+        options: {
+          sync: {isSingleNode: true},
+          network: {allowPublishToZeroPeers: true, mdns: true, useWorker: false},
+          // run the first bn with ReorgedForkChoice, no nHistoricalStates flag so it does not have to reload
+          chain: {
+            blsVerifyAllMainThread: true,
+            forkchoiceConstructor: TimelinessForkChoice,
+            proposerBoostEnabled: true,
+            proposerBoostReorgEnabled,
+          },
+        },
+        validatorCount,
+        genesisTime,
+        logger,
+      });
+
+      (bn.chain.forkChoice as TimelinessForkChoice).lateSlot = reorgSlot;
+      afterEachCallbacks.push(async () => bn.close());
+      const {validators} = await getAndInitDevValidators({
+        node: bn,
+        logPrefix: "vc-0",
+        validatorsPerClient: validatorCount,
+        validatorClientCount: 1,
+        startIndex: 0,
+        useRestApi: false,
+        testLoggerOpts,
+      });
+      afterEachCallbacks.push(() => Promise.all(validators.map((v) => v.close())));
+
+      const commonAncestor = await waitForEvent<{slot: Slot; block: RootHex}>(
+        bn.chain.emitter,
+        routes.events.EventType.head,
+        240000,
+        ({slot}) => slot === reorgSlot - 1
+      );
+      //               reorgSlot
+      //                /
+      // commonAncestor ------------ newBlock
+      const commonAncestorRoot = commonAncestor.block;
+      const reorgBlockEventData = await waitForEvent<{slot: Slot; block: RootHex}>(
+        bn.chain.emitter,
+        routes.events.EventType.head,
+        240000,
+        ({slot}) => slot === reorgSlot
+      );
+      const reorgBlockRoot = reorgBlockEventData.block;
+      const [newBlockEventData, reorgEventData] = await Promise.all([
+        waitForEvent<{slot: Slot; block: RootHex}>(
+          bn.chain.emitter,
+          routes.events.EventType.block,
+          240000,
+          ({slot}) => slot === reorgSlot + 1
+        ),
+        waitForEvent<ReorgEventData>(bn.chain.emitter, routes.events.EventType.chainReorg, 240000),
+      ]);
+      expect(reorgEventData.slot).toEqual(reorgSlot + 1);
+      const newBlock = await bn.chain.getBlockByRoot(newBlockEventData.block);
+      if (newBlock == null) {
+        throw Error(`Block ${reorgSlot + 1} not found`);
+      }
+      expect(reorgEventData.oldHeadBlock).toEqual(reorgBlockRoot);
+      expect(reorgEventData.newHeadBlock).toEqual(newBlockEventData.block);
+      expect(reorgEventData.depth).toEqual(2);
+      expect(toHexString(newBlock?.block.message.parentRoot)).toEqual(commonAncestorRoot);
+      logger.info("New block", {
+        slot: newBlock.block.message.slot,
+        parentRoot: toHexString(newBlock.block.message.parentRoot),
+      });
+    });
+  },
+  {timeout: 60000}
+);