small refac and test fixes and selection test extenstion for override…

… combinations

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

@@ -288,7 +288,7 @@ export function getValidatorApi({
     {
       skipHeadChecksAndUpdate,
     }: Omit<routes.validator.ExtraProduceBlockOps, "builderSelection"> & {skipHeadChecksAndUpdate?: boolean} = {}
-  ): Promise<routes.validator.ProduceBlindedBlockRes & {shouldOverrideBuilder: boolean}> {
+  ): Promise<routes.validator.ProduceBlindedBlockRes> {
     const version = config.getForkName(slot);
     if (!isForkExecution(version)) {
       throw Error(`Invalid fork=${version} for produceBuilderBlindedBlock`);
@@ -319,12 +319,11 @@ export function getValidatorApi({
     let timer;
     try {
       timer = metrics?.blockProductionTime.startTimer();
-      const {block, executionPayloadValue, consensusBlockValue, shouldOverrideBuilder} =
-        await chain.produceBlindedBlock({
-          slot,
-          randaoReveal,
-          graffiti: toGraffitiBuffer(graffiti || ""),
-        });
+      const {block, executionPayloadValue, consensusBlockValue} = await chain.produceBlindedBlock({
+        slot,
+        randaoReveal,
+        graffiti: toGraffitiBuffer(graffiti || ""),
+      });
 
       metrics?.blockProductionSuccess.inc({source});
       metrics?.blockProductionNumAggregated.observe({source}, block.body.attestations.length);
@@ -339,7 +338,7 @@ export function getValidatorApi({
         void chain.persistBlock(block, "produced_builder_block");
       }
 
-      return {data: block, version, executionPayloadValue, consensusBlockValue, shouldOverrideBuilder};
+      return {data: block, version, executionPayloadValue, consensusBlockValue};
     } finally {
       if (timer) timer({source});
     }
@@ -354,7 +353,7 @@ export function getValidatorApi({
       strictFeeRecipientCheck,
       skipHeadChecksAndUpdate,
     }: Omit<routes.validator.ExtraProduceBlockOps, "builderSelection"> & {skipHeadChecksAndUpdate?: boolean} = {}
-  ): Promise<routes.validator.ProduceBlockOrContentsRes & {shouldOverrideBuilder: boolean}> {
+  ): Promise<routes.validator.ProduceBlockOrContentsRes & {shouldOverrideBuilder?: boolean}> {
     const source = ProducedBlockSource.engine;
     metrics?.blockProductionRequests.inc({source});
 
@@ -507,7 +506,7 @@ export function getValidatorApi({
         const promisesOrder = [ProducedBlockSource.builder, ProducedBlockSource.engine];
         [blindedBlock, fullBlock] = await racePromisesWithCutoff<
           | ((routes.validator.ProduceBlockOrContentsRes | routes.validator.ProduceBlindedBlockRes) & {
-              shouldOverrideBuilder: boolean;
+              shouldOverrideBuilder?: boolean;
             })
           | null
         >(
@@ -562,7 +561,7 @@ export function getValidatorApi({
       // handle the builder override case separately
       if (shouldOverrideBuilder === true) {
         executionPayloadSource = ProducedBlockSource.engine;
-        logger.verbose("Selected engine block as censorship suspected in builder blocks", {
+        logger.info("Selected engine block as censorship suspected in builder blocks", {
           // winston logger doesn't like bigint
           enginePayloadValue: `${enginePayloadValue}`,
           consensusBlockValueEngine: `${consensusBlockValueEngine}`,
@@ -596,7 +595,7 @@ export function getValidatorApi({
             executionPayloadSource = ProducedBlockSource.builder;
           }
         }
-        logger.verbose(`Selected executionPayloadSource=${executionPayloadSource} block`, {
+        logger.info(`Selected executionPayloadSource=${executionPayloadSource} block`, {
           builderSelection,
           // winston logger doesn't like bigint
           builderBoostFactor: `${builderBoostFactor}`,
@@ -611,7 +610,7 @@ export function getValidatorApi({
         });
       } else if (fullBlock && !blindedBlock) {
         executionPayloadSource = ProducedBlockSource.engine;
-        logger.verbose("Selected engine block: no builder block produced", {
+        logger.info("Selected engine block: no builder block produced", {
           // winston logger doesn't like bigint
           enginePayloadValue: `${enginePayloadValue}`,
           consensusBlockValueEngine: `${consensusBlockValueEngine}`,
@@ -621,7 +620,7 @@ export function getValidatorApi({
         });
       } else if (blindedBlock && !fullBlock) {
         executionPayloadSource = ProducedBlockSource.builder;
-        logger.verbose("Selected builder block: no engine block produced", {
+        logger.info("Selected builder block: no engine block produced", {
           // winston logger doesn't like bigint
           builderPayloadValue: `${builderPayloadValue}`,
           consensusBlockValueBuilder: `${consensusBlockValueBuilder}`,

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

@@ -474,7 +474,7 @@ export class BeaconChain implements IBeaconChain {
     block: allForks.BeaconBlock;
     executionPayloadValue: Wei;
     consensusBlockValue: Gwei;
-    shouldOverrideBuilder: boolean;
+    shouldOverrideBuilder?: boolean;
   }> {
     return this.produceBlockWrapper<BlockType.Full>(BlockType.Full, blockAttributes);
   }
@@ -483,7 +483,7 @@ export class BeaconChain implements IBeaconChain {
     block: allForks.BlindedBeaconBlock;
     executionPayloadValue: Wei;
     consensusBlockValue: Gwei;
-    shouldOverrideBuilder: boolean;
+    shouldOverrideBuilder?: boolean;
   }> {
     return this.produceBlockWrapper<BlockType.Blinded>(BlockType.Blinded, blockAttributes);
   }
@@ -495,7 +495,7 @@ export class BeaconChain implements IBeaconChain {
     block: AssembledBlockType<T>;
     executionPayloadValue: Wei;
     consensusBlockValue: Gwei;
-    shouldOverrideBuilder: boolean;
+    shouldOverrideBuilder?: boolean;
   }> {
     const head = this.forkChoice.getHead();
     const state = await this.regen.getBlockSlotState(

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

@@ -158,13 +158,13 @@ export interface IBeaconChain {
     block: allForks.BeaconBlock;
     executionPayloadValue: Wei;
     consensusBlockValue: Gwei;
-    shouldOverrideBuilder: boolean;
+    shouldOverrideBuilder?: boolean;
   }>;
   produceBlindedBlock(blockAttributes: BlockAttributes): Promise<{
     block: allForks.BlindedBeaconBlock;
     executionPayloadValue: Wei;
     consensusBlockValue: Gwei;
-    shouldOverrideBuilder: boolean;
+    shouldOverrideBuilder?: boolean;
   }>;
 
   /** Process a block until complete */

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

@@ -113,7 +113,7 @@ export async function produceBlockBody<T extends BlockType>(
   body: AssembledBodyType<T>;
   blobs: BlobsResult;
   executionPayloadValue: Wei;
-  shouldOverrideBuilder: boolean;
+  shouldOverrideBuilder?: boolean;
 }> {
   // Type-safe for blobs variable. Translate 'null' value into 'preDeneb' enum
   // TODO: Not ideal, but better than just using null.
@@ -122,7 +122,7 @@ export async function produceBlockBody<T extends BlockType>(
   let executionPayloadValue: Wei;
   // even though shouldOverrideBuilder is relevant for the engine response, for simplicity of typing
   // we can also return is just as false for builder which will anyway be ignored by the caller
-  let shouldOverrideBuilder: boolean;
+  let shouldOverrideBuilder: boolean | undefined;
   const fork = currentState.config.getForkName(blockSlot);
 
   const logMeta: Record<string, string | number | bigint> = {
@@ -212,7 +212,6 @@ export async function produceBlockBody<T extends BlockType>(
 
     if (blockType === BlockType.Blinded) {
       if (!this.executionBuilder) throw Error("Execution Builder not available");
-      shouldOverrideBuilder = false;
 
       // This path will not be used in the production, but is here just for merge mock
       // tests because merge-mock requires an fcU to be issued prior to fetch payload
@@ -289,7 +288,6 @@ export async function produceBlockBody<T extends BlockType>(
             ssz.allForksExecution[fork].ExecutionPayload.defaultValue();
           blobsResult = {type: BlobsResultType.preDeneb};
           executionPayloadValue = BigInt(0);
-          shouldOverrideBuilder = false;
         } else {
           const {prepType, payloadId} = prepareRes;
           Object.assign(logMeta, {executionPayloadPrepType: prepType});
@@ -359,7 +357,6 @@ export async function produceBlockBody<T extends BlockType>(
             ssz.allForksExecution[fork].ExecutionPayload.defaultValue();
           blobsResult = {type: BlobsResultType.preDeneb};
           executionPayloadValue = BigInt(0);
-          shouldOverrideBuilder = false;
         } else {
           // since merge transition is complete, we need a valid payload even if with an
           // empty (transactions) one. defaultValue isn't gonna cut it!
@@ -370,7 +367,6 @@ export async function produceBlockBody<T extends BlockType>(
   } else {
     blobsResult = {type: BlobsResultType.preDeneb};
     executionPayloadValue = BigInt(0);
-    shouldOverrideBuilder = false;
   }
   endExecutionPayload?.({
     step: BlockProductionStep.executionPayload,

packages/beacon-node/src/execution/engine/http.ts

@@ -367,7 +367,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
     executionPayload: allForks.ExecutionPayload;
     executionPayloadValue: Wei;
     blobsBundle?: BlobsBundle;
-    shouldOverrideBuilder: boolean;
+    shouldOverrideBuilder?: boolean;
   }> {
     const method =
       ForkSeq[fork] >= ForkSeq.deneb

packages/beacon-node/src/execution/engine/interface.ts

@@ -140,7 +140,7 @@ export interface IExecutionEngine {
     executionPayload: allForks.ExecutionPayload;
     executionPayloadValue: Wei;
     blobsBundle?: BlobsBundle;
-    shouldOverrideBuilder: boolean;
+    shouldOverrideBuilder?: boolean;
   }>;
 
   getPayloadBodiesByHash(blockHash: DATA[]): Promise<(ExecutionPayloadBody | null)[]>;

packages/beacon-node/src/execution/engine/types.ts

@@ -212,7 +212,7 @@ export function parseExecutionPayload(
   executionPayload: allForks.ExecutionPayload;
   executionPayloadValue: Wei;
   blobsBundle?: BlobsBundle;
-  shouldOverrideBuilder: boolean;
+  shouldOverrideBuilder?: boolean;
 } {
   let data: ExecutionPayloadRpc;
   let executionPayloadValue: Wei;

packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts

@@ -89,7 +89,6 @@ describe("api/validator - produceBlockV2", function () {
       block: fullBlock,
       executionPayloadValue,
       consensusBlockValue,
-      shouldOverrideBuilder: false,
     });
 
     // check if expectedFeeRecipient is passed to produceBlock
@@ -134,7 +133,6 @@ describe("api/validator - produceBlockV2", function () {
     executionEngineStub.getPayload.mockResolvedValue({
       executionPayload: ssz.bellatrix.ExecutionPayload.defaultValue(),
       executionPayloadValue,
-      shouldOverrideBuilder: false,
     });
 
     // use fee recipient passed in produceBlockBody call for payload gen in engine notifyForkchoiceUpdate

packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts

@@ -41,23 +41,38 @@ describe("api/validator - produceBlockV3", function () {
     vi.clearAllMocks();
   });
 
-  const testCases: [routes.validator.BuilderSelection, number | null, number | null, number, string][] = [
-    [routes.validator.BuilderSelection.MaxProfit, 1, 0, 0, "builder"],
-    [routes.validator.BuilderSelection.MaxProfit, 1, 2, 1, "engine"],
-    [routes.validator.BuilderSelection.MaxProfit, null, 0, 0, "engine"],
-    [routes.validator.BuilderSelection.MaxProfit, 0, null, 1, "builder"],
-
-    [routes.validator.BuilderSelection.BuilderAlways, 1, 2, 0, "builder"],
-    [routes.validator.BuilderSelection.BuilderAlways, 1, 0, 1, "builder"],
-    [routes.validator.BuilderSelection.BuilderAlways, null, 0, 0, "engine"],
-    [routes.validator.BuilderSelection.BuilderAlways, 0, null, 1, "builder"],
-
-    [routes.validator.BuilderSelection.BuilderOnly, 0, 2, 0, "builder"],
-    [routes.validator.BuilderSelection.ExecutionOnly, 2, 0, 1, "execution"],
+  const testCases: [routes.validator.BuilderSelection, number | null, number | null, number, boolean, string][] = [
+    [routes.validator.BuilderSelection.MaxProfit, 1, 0, 0, false, "builder"],
+    [routes.validator.BuilderSelection.MaxProfit, 1, 2, 1, false, "engine"],
+    [routes.validator.BuilderSelection.MaxProfit, null, 0, 0, false, "engine"],
+    [routes.validator.BuilderSelection.MaxProfit, 0, null, 1, false, "builder"],
+    [routes.validator.BuilderSelection.MaxProfit, 0, null, 1, true, "builder"],
+    [routes.validator.BuilderSelection.MaxProfit, 1, 1, 1, true, "engine"],
+    [routes.validator.BuilderSelection.MaxProfit, 2, 1, 1, true, "engine"],
+
+    [routes.validator.BuilderSelection.BuilderAlways, 1, 2, 0, false, "builder"],
+    [routes.validator.BuilderSelection.BuilderAlways, 1, 0, 1, false, "builder"],
+    [routes.validator.BuilderSelection.BuilderAlways, null, 0, 0, false, "engine"],
+    [routes.validator.BuilderSelection.BuilderAlways, 0, null, 1, false, "builder"],
+    [routes.validator.BuilderSelection.BuilderAlways, 0, 1, 1, true, "engine"],
+    [routes.validator.BuilderSelection.BuilderAlways, 1, 1, 1, true, "engine"],
+    [routes.validator.BuilderSelection.BuilderAlways, 1, null, 1, true, "builder"],
+
+    [routes.validator.BuilderSelection.BuilderOnly, 0, 2, 0, false, "builder"],
+    [routes.validator.BuilderSelection.ExecutionOnly, 2, 0, 1, false, "engine"],
+    [routes.validator.BuilderSelection.BuilderOnly, 1, 1, 0, true, "builder"],
+    [routes.validator.BuilderSelection.ExecutionOnly, 1, 1, 1, true, "engine"],
   ];
 
   testCases.forEach(
-    ([builderSelection, builderPayloadValue, enginePayloadValue, consensusBlockValue, finalSelection]) => {
+    ([
+      builderSelection,
+      builderPayloadValue,
+      enginePayloadValue,
+      consensusBlockValue,
+      shouldOverrideBuilder,
+      finalSelection,
+    ]) => {
       it(`produceBlockV3  - ${finalSelection} produces block`, async () => {
         syncStub = server.syncStub;
         modules = {
@@ -89,7 +104,7 @@ describe("api/validator - produceBlockV3", function () {
             block: fullBlock,
             executionPayloadValue: BigInt(enginePayloadValue),
             consensusBlockValue: BigInt(consensusBlockValue),
-            shouldOverrideBuilder: false,
+            shouldOverrideBuilder,
           });
         } else {
           chainStub.produceBlock.mockRejectedValue(Error("not produced"));
@@ -100,7 +115,6 @@ describe("api/validator - produceBlockV3", function () {
             block: blindedBlock,
             executionPayloadValue: BigInt(builderPayloadValue),
             consensusBlockValue: BigInt(consensusBlockValue),
-            shouldOverrideBuilder: false,
           });
         } else {
           chainStub.produceBlindedBlock.mockRejectedValue(Error("not produced"));