Add validatior option to specify builder block selection strategy

packages/beacon-node/test/sim/mergemock.test.ts

@@ -5,7 +5,7 @@ import {LogLevel, sleep, TimestampFormatCode} from "@lodestar/utils";
 import {SLOTS_PER_EPOCH} from "@lodestar/params";
 import {IChainConfig} from "@lodestar/config";
 import {Epoch} from "@lodestar/types";
-import {ValidatorProposerConfig} from "@lodestar/validator";
+import {ValidatorProposerConfig, BuilderSelection} from "@lodestar/validator";
 
 import {ChainEvent} from "../../src/chain/index.js";
 import {testLogger, TestLoggerOpts} from "../utils/logger.js";
@@ -163,6 +163,7 @@ describe("executionEngine / ExecutionEngineHttp", function () {
         builder: {
           enabled: true,
           gasLimit: 30000000,
+          selection: BuilderSelection.BuilderAlways,
         },
       },
     } as ValidatorProposerConfig;

packages/cli/src/cmds/validator/handler.ts

@@ -1,7 +1,13 @@
 import path from "node:path";
 import {setMaxListeners} from "node:events";
 import {LevelDbController} from "@lodestar/db";
-import {ProcessShutdownCallback, SlashingProtection, Validator, ValidatorProposerConfig} from "@lodestar/validator";
+import {
+  ProcessShutdownCallback,
+  SlashingProtection,
+  Validator,
+  ValidatorProposerConfig,
+  BuilderSelection,
+} from "@lodestar/validator";
 import {getMetrics, MetricsRegister} from "@lodestar/validator";
 import {RegistryMetricCreator, collectNodeJSMetrics, HttpMetricsServer} from "@lodestar/beacon-node";
 import {getBeaconConfigFromArgs} from "../../config/index.js";
@@ -179,7 +185,11 @@ function getProposerConfigFromArgs(
     graffiti: args.graffiti || getDefaultGraffiti(),
     strictFeeRecipientCheck: args.strictFeeRecipientCheck,
     feeRecipient: args.suggestedFeeRecipient ? parseFeeRecipient(args.suggestedFeeRecipient) : undefined,
-    builder: {enabled: args.builder, gasLimit: args.defaultGasLimit},
+    builder: {
+      enabled: args.builder,
+      gasLimit: args.defaultGasLimit,
+      selection: parseBuilderSelection(args["builder.selection"]),
+    },
   };
 
   let valProposerConfig: ValidatorProposerConfig;
@@ -204,3 +214,17 @@ function getProposerConfigFromArgs(
   }
   return valProposerConfig;
 }
+
+function parseBuilderSelection(builderSelection?: string): BuilderSelection | undefined {
+  if (builderSelection) {
+    switch (builderSelection) {
+      case "maxprofit":
+        break;
+      case "builderalways":
+        break;
+      default:
+        throw Error("Invalid input for builder selection, check help.");
+    }
+  }
+  return builderSelection as BuilderSelection;
+}

packages/cli/src/cmds/validator/options.ts

@@ -34,7 +34,9 @@ export type IValidatorCliArgs = AccountValidatorArgs &
     strictFeeRecipientCheck?: boolean;
     doppelgangerProtectionEnabled?: boolean;
     defaultGasLimit?: number;
+
     builder?: boolean;
+    "builder.selection"?: string;
 
     importKeystores?: string[];
     importKeystoresPassword?: string;
@@ -204,6 +206,13 @@ export const validatorOptions: ICliCommandOptions<IValidatorCliArgs> = {
     group: "builder",
   },
 
+  "builder.selection": {
+    type: "string",
+    description: "Default builder block selection strategy: maxprofit or builderalways",
+    defaultDescription: `${defaultOptions.builderSelection}`,
+    group: "builder",
+  },
+
   importKeystores: {
     alias: ["keystore"], // Backwards compatibility with old `validator import` cmdx
     description: "Path(s) to a directory or single filepath to validator keystores, i.e. Launchpad validators",

packages/cli/src/util/proposerConfig.ts

@@ -2,7 +2,7 @@
 
 import fs from "node:fs";
 import path from "node:path";
-import {ValidatorProposerConfig} from "@lodestar/validator";
+import {ValidatorProposerConfig, BuilderSelection} from "@lodestar/validator";
 import {parseFeeRecipient} from "./feeRecipient.js";
 
 import {readFile} from "./file.js";
@@ -18,6 +18,7 @@ type ProposerConfigFileSection = {
     // for js-yaml
     enabled?: string;
     gas_limit?: number;
+    selection?: BuilderSelection;
   };
 };
 
@@ -55,7 +56,7 @@ function parseProposerConfigSection(
   overrideConfig?: ProposerConfig
 ): ProposerConfig {
   const {graffiti, strict_fee_recipient_check, fee_recipient, builder} = proposerFileSection;
-  const {enabled, gas_limit} = builder || {};
+  const {enabled, gas_limit, selection: builderSelection} = builder || {};
 
   if (graffiti !== undefined && typeof graffiti !== "string") {
     throw Error("graffiti is not 'string");
@@ -90,6 +91,7 @@ function parseProposerConfigSection(
     builder: {
       enabled: overrideConfig?.builder?.enabled ?? (enabled ? stringtoBool(enabled) : undefined),
       gasLimit: overrideConfig?.builder?.gasLimit ?? (gas_limit !== undefined ? Number(gas_limit) : undefined),
+      selection: overrideConfig?.builder?.selection ?? builderSelection,
     },
   };
 }

packages/cli/test/unit/validator/parseProposerConfig.test.ts

@@ -2,6 +2,8 @@
 import path from "node:path";
 import {fileURLToPath} from "node:url";
 import {expect} from "chai";
+import {BuilderSelection} from "@lodestar/validator";
+
 import {parseProposerConfig} from "../../../src/util/index.js";
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
@@ -15,6 +17,7 @@ const testValue = {
       builder: {
         enabled: true,
         gasLimit: 30000000,
+        selection: undefined,
       },
     },
     "0xa4855c83d868f772a579133d9f23818008417b743e8447e235d8eb78b1d8f8a9f63f98c551beb7de254400f89592314d": {
@@ -24,6 +27,7 @@ const testValue = {
       builder: {
         enabled: true,
         gasLimit: 35000000,
+        selection: BuilderSelection.MaxProfit,
       },
     },
   },
@@ -34,6 +38,7 @@ const testValue = {
     builder: {
       enabled: true,
       gasLimit: 30000000,
+      selection: BuilderSelection.BuilderAlways,
     },
   },
 };

packages/cli/test/unit/validator/proposerConfigs/validData.yaml

@@ -11,10 +11,12 @@ proposer_config:
     builder:
       enabled: "true"
       gas_limit: "35000000"
+      selection: "maxprofit"
 default_config:
   graffiti: 'default graffiti'
   strict_fee_recipient_check: "true"
   fee_recipient: '0xcccccccccccccccccccccccccccccccccccccccc'
   builder:
     enabled: true
     gas_limit: "30000000"
+    selection: "builderalways"

packages/validator/src/index.ts

@@ -8,6 +8,7 @@ export {
   ValidatorProposerConfig,
   defaultOptions,
   ProposerConfig,
+  BuilderSelection,
 } from "./services/validatorStore.js";
 export {waitForGenesis} from "./genesis.js";
 export {getMetrics, Metrics, MetricsRegister} from "./metrics.js";

packages/validator/src/services/block.ts

@@ -7,9 +7,17 @@ import {Api, ApiError, ServerApi} from "@lodestar/api";
 import {IClock, ILoggerVc} from "../util/index.js";
 import {PubkeyHex} from "../types.js";
 import {Metrics} from "../metrics.js";
-import {ValidatorStore} from "./validatorStore.js";
+import {ValidatorStore, BuilderSelection} from "./validatorStore.js";
 import {BlockDutiesService, GENESIS_SLOT} from "./blockDuties.js";
 
+const ETH_TO_WEI = BigInt("1000000000000000000");
+
+type ProduceBlockOpts = {
+  expectedFeeRecipient: string;
+  strictFeeRecipientCheck: boolean;
+  isBuilderEnabled: boolean;
+  builderSelection: BuilderSelection;
+};
 /**
  * Service that sets up and handles validator block proposal duties.
  */
@@ -74,12 +82,14 @@ export class BlockProposingService {
 
       const strictFeeRecipientCheck = this.validatorStore.strictFeeRecipientCheck(pubkeyHex);
       const isBuilderEnabled = this.validatorStore.isBuilderEnabled(pubkeyHex);
+      const builderSelection = this.validatorStore.getBuilderSelection(pubkeyHex);
       const expectedFeeRecipient = this.validatorStore.getFeeRecipient(pubkeyHex);
 
       const block = await this.produceBlockWrapper(slot, randaoReveal, graffiti, {
         expectedFeeRecipient,
         strictFeeRecipientCheck,
         isBuilderEnabled,
+        builderSelection,
       }).catch((e: Error) => {
         this.metrics?.blockProposingErrors.inc({error: "produce"});
         throw extendError(e, "Failed to produce block");
@@ -115,14 +125,10 @@ export class BlockProposingService {
     slot: Slot,
     randaoReveal: BLSSignature,
     graffiti: string,
-    {
-      expectedFeeRecipient,
-      strictFeeRecipientCheck,
-      isBuilderEnabled,
-    }: {expectedFeeRecipient: string; strictFeeRecipientCheck: boolean; isBuilderEnabled: boolean}
+    {expectedFeeRecipient, strictFeeRecipientCheck, isBuilderEnabled, builderSelection}: ProduceBlockOpts
   ): Promise<{data: allForks.FullOrBlindedBeaconBlock} & {debugLogCtx: Record<string, string>}> => {
     const blindedBlockPromise = isBuilderEnabled
-      ? this.api.validator.produceBlindedBlock(slot, randaoReveal, graffiti).catch((e: Error) => {
+      ? this.produceBlindedBlock(slot, randaoReveal, graffiti).catch((e: Error) => {
           this.logger.error("Failed to produce builder block", {}, e as Error);
           return null;
         })
@@ -136,51 +142,88 @@ export class BlockProposingService {
     await Promise.all([blindedBlockPromise, fullBlockPromise]);
 
     const blindedBlock = await blindedBlockPromise;
+    const builderBlockValue = blindedBlock?.blockValue ?? BigInt(0);
+
     const fullBlock = await fullBlockPromise;
+    const engineBlockValue = fullBlock?.blockValue ?? BigInt(0);
+
+    const feeRecipientCheck = {expectedFeeRecipient, strictFeeRecipientCheck};
+
+    if (fullBlock && blindedBlock) {
+      let selectedSource: string;
+      let selectedBlock;
+      switch (builderSelection) {
+        case BuilderSelection.MaxProfit: {
+          if (engineBlockValue >= builderBlockValue) {
+            selectedSource = "engine";
+            selectedBlock = fullBlock;
+          } else {
+            selectedSource = "builder";
+            selectedBlock = blindedBlock;
+          }
+          break;
+        }
 
-    // A metric on the choice between blindedBlock and normal block can be applied
-    // TODO: compare the blockValue that has been obtained in each of full or blinded block
-    if (blindedBlock && blindedBlock.ok) {
-      const debugLogCtx = {source: "builder", blockValue: blindedBlock.response.blockValue.toString()};
-      return {...blindedBlock.response, debugLogCtx};
-    } else {
-      if (!fullBlock) {
-        throw Error("Failed to produce engine or builder block");
+        case BuilderSelection.BuilderAlways:
+        default: {
+          selectedSource = "builder";
+          selectedBlock = blindedBlock;
+        }
       }
-      const debugLogCtx = {source: "engine", blockValue: fullBlock.blockValue.toString()};
-      const blockFeeRecipient = (fullBlock.data as bellatrix.BeaconBlock).body.executionPayload?.feeRecipient;
-      const feeRecipient = blockFeeRecipient !== undefined ? toHexString(blockFeeRecipient) : undefined;
+      this.logger.debug(`Selected ${selectedSource} block`, {
+        builderSelection,
+        // winston logger doesn't like bigint
+        engineBlockValue: `${engineBlockValue}`,
+        builderBlockValue: `${builderBlockValue}`,
+      });
+      return this.getBlockWithDebugLog(selectedBlock, selectedSource, feeRecipientCheck);
+    } else if (fullBlock && !blindedBlock) {
+      this.logger.debug("Selected engine block: no builder block produced", {
+        // winston logger doesn't like bigint
+        engineBlockValue: `${engineBlockValue}`,
+      });
+      return this.getBlockWithDebugLog(fullBlock, "engine", feeRecipientCheck);
+    } else if (blindedBlock && !fullBlock) {
+      this.logger.debug("Selected builder block: no engine block produced", {
+        // winston logger doesn't like bigint
+        builderBlockValue: `${builderBlockValue}`,
+      });
+      return this.getBlockWithDebugLog(blindedBlock, "builder", feeRecipientCheck);
+    } else {
+      throw Error("Failed to produce engine or builder block");
+    }
+  };
+
+  private getBlockWithDebugLog(
+    fullOrBlindedBlock: {data: allForks.FullOrBlindedBeaconBlock; blockValue: Wei},
+    source: string,
+    {expectedFeeRecipient, strictFeeRecipientCheck}: {expectedFeeRecipient: string; strictFeeRecipientCheck: boolean}
+  ): {data: allForks.FullOrBlindedBeaconBlock} & {debugLogCtx: Record<string, string>} {
+    const debugLogCtx = {
+      source: source,
+      // winston logger doesn't like bigint
+      "blockValue(eth)": `${fullOrBlindedBlock.blockValue / ETH_TO_WEI}`,
+    };
+    const blockFeeRecipient = (fullOrBlindedBlock.data as bellatrix.BeaconBlock).body.executionPayload?.feeRecipient;
+    const feeRecipient = blockFeeRecipient !== undefined ? toHexString(blockFeeRecipient) : undefined;
+
+    if (source === "engine") {
       if (feeRecipient !== undefined) {
-        // In Mev Builder, the feeRecipient could differ and rewards to the feeRecipient
-        // might be included in the block transactions as indicated by the BuilderBid
-        // Address this appropriately in the Mev boost PR
-        //
-        // Even for engine, there could be divergence of feeRecipient the argument being
-        // that the bn <> engine setup has implied trust and are user-agents of the same entity.
-        // A better approach would be to have engine also provide something akin to BuilderBid
-        //
-        // The following conversation in the interop R&D channel can provide some context
-        // https://discord.com/channels/595666850260713488/892088344438255616/978374892678426695
-        //
-        // For now providing a strick check flag to enable disable this
         if (feeRecipient !== expectedFeeRecipient && strictFeeRecipientCheck) {
           throw Error(`Invalid feeRecipient=${feeRecipient}, expected=${expectedFeeRecipient}`);
         }
-        const transactions = (fullBlock.data as bellatrix.BeaconBlock).body.executionPayload?.transactions.length;
-        const withdrawals = (fullBlock.data as capella.BeaconBlock).body.executionPayload?.withdrawals?.length;
-        Object.assign(debugLogCtx, {feeRecipient, transactions}, withdrawals !== undefined ? {withdrawals} : {});
       }
-      return {...fullBlock, debugLogCtx};
-      // throw Error("random")
     }
-  };
+
+    const transactions = (fullOrBlindedBlock.data as bellatrix.BeaconBlock).body.executionPayload?.transactions.length;
+    const withdrawals = (fullOrBlindedBlock.data as capella.BeaconBlock).body.executionPayload?.withdrawals?.length;
+    Object.assign(debugLogCtx, {feeRecipient, transactions}, withdrawals !== undefined ? {withdrawals} : {});
+
+    return {...fullOrBlindedBlock, debugLogCtx};
+  }
 
   /** Wrapper around the API's different methods for producing blocks across forks */
-  private produceBlock: ServerApi<Api["validator"]>["produceBlock"] = async (
-    slot,
-    randaoReveal,
-    graffiti
-  ): Promise<{data: allForks.BeaconBlock; blockValue: Wei}> => {
+  private produceBlock: ServerApi<Api["validator"]>["produceBlock"] = async (slot, randaoReveal, graffiti) => {
     switch (this.config.getForkName(slot)) {
       case ForkName.phase0: {
         const res = await this.api.validator.produceBlock(slot, randaoReveal, graffiti);
@@ -196,4 +239,14 @@ export class BlockProposingService {
       }
     }
   };
+
+  private produceBlindedBlock: ServerApi<Api["validator"]>["produceBlindedBlock"] = async (
+    slot,
+    randaoReveal,
+    graffiti
+  ) => {
+    const res = await this.api.validator.produceBlindedBlock(slot, randaoReveal, graffiti);
+    ApiError.assert(res, "Failed to produce block: validator.produceBlindedBlock");
+    return res.response;
+  };
 }