diff --git a/packages/beacon-node/test/sim/mergemock.test.ts b/packages/beacon-node/test/sim/mergemock.test.ts index d3601ff20d3f..ffe544d00655 100644 --- a/packages/beacon-node/test/sim/mergemock.test.ts +++ b/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; diff --git a/packages/cli/src/cmds/validator/handler.ts b/packages/cli/src/cmds/validator/handler.ts index d59fdafec901..1949d5e4265e 100644 --- a/packages/cli/src/cmds/validator/handler.ts +++ b/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; +} diff --git a/packages/cli/src/cmds/validator/options.ts b/packages/cli/src/cmds/validator/options.ts index 46ecdd6054bf..e743c2cb545f 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/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 = { 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", diff --git a/packages/cli/src/util/proposerConfig.ts b/packages/cli/src/util/proposerConfig.ts index 3da7307e05f0..29fcb25a48ea 100644 --- a/packages/cli/src/util/proposerConfig.ts +++ b/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, }, }; } diff --git a/packages/cli/test/unit/validator/parseProposerConfig.test.ts b/packages/cli/test/unit/validator/parseProposerConfig.test.ts index e90c8fa38c9b..ba4a7610f521 100644 --- a/packages/cli/test/unit/validator/parseProposerConfig.test.ts +++ b/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, }, }, }; diff --git a/packages/cli/test/unit/validator/proposerConfigs/validData.yaml b/packages/cli/test/unit/validator/proposerConfigs/validData.yaml index 377d9e7630e7..2d954e85d7b3 100644 --- a/packages/cli/test/unit/validator/proposerConfigs/validData.yaml +++ b/packages/cli/test/unit/validator/proposerConfigs/validData.yaml @@ -11,6 +11,7 @@ proposer_config: builder: enabled: "true" gas_limit: "35000000" + selection: "maxprofit" default_config: graffiti: 'default graffiti' strict_fee_recipient_check: "true" @@ -18,3 +19,4 @@ default_config: builder: enabled: true gas_limit: "30000000" + selection: "builderalways" diff --git a/packages/validator/src/index.ts b/packages/validator/src/index.ts index 0663b59aced0..03a124e7f593 100644 --- a/packages/validator/src/index.ts +++ b/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"; diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index 6e38d6033281..6e6088e2000b 100644 --- a/packages/validator/src/services/block.ts +++ b/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}> => { 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} { + 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["produceBlock"] = async ( - slot, - randaoReveal, - graffiti - ): Promise<{data: allForks.BeaconBlock; blockValue: Wei}> => { + private produceBlock: ServerApi["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["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; + }; } diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 84b59acbddc8..5a46ed40509b 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -62,6 +62,11 @@ export type SignerRemote = { pubkey: PubkeyHex; }; +export enum BuilderSelection { + BuilderAlways = "builderalways", + MaxProfit = "maxprofit", +} + type DefaultProposerConfig = { graffiti: string; strictFeeRecipientCheck: boolean; @@ -69,6 +74,7 @@ type DefaultProposerConfig = { builder: { enabled: boolean; gasLimit: number; + selection: BuilderSelection; }; }; @@ -79,6 +85,7 @@ export type ProposerConfig = { builder?: { enabled?: boolean; gasLimit?: number; + selection?: BuilderSelection; }; }; @@ -113,6 +120,7 @@ type ValidatorData = ProposerConfig & { export const defaultOptions = { suggestedFeeRecipient: "0x0000000000000000000000000000000000000000", defaultGasLimit: 30_000_000, + builderSelection: BuilderSelection.BuilderAlways, }; /** @@ -142,6 +150,7 @@ export class ValidatorStore { builder: { enabled: defaultConfig.builder?.enabled ?? false, gasLimit: defaultConfig.builder?.gasLimit ?? defaultOptions.defaultGasLimit, + selection: defaultConfig.builder?.selection ?? defaultOptions.builderSelection, }, }; @@ -206,7 +215,11 @@ export class ValidatorStore { } isBuilderEnabled(pubkeyHex: PubkeyHex): boolean { - return (this.validators.get(pubkeyHex)?.builder || {}).enabled ?? this.defaultProposerConfig?.builder.enabled; + return (this.validators.get(pubkeyHex)?.builder || {}).enabled ?? this.defaultProposerConfig.builder.enabled; + } + + getBuilderSelection(pubkeyHex: PubkeyHex): BuilderSelection { + return (this.validators.get(pubkeyHex)?.builder || {}).selection ?? this.defaultProposerConfig.builder.selection; } strictFeeRecipientCheck(pubkeyHex: PubkeyHex): boolean { diff --git a/packages/validator/test/unit/services/produceBlockwrapper.test.ts b/packages/validator/test/unit/services/produceBlockwrapper.test.ts new file mode 100644 index 000000000000..b7c0ba9d1355 --- /dev/null +++ b/packages/validator/test/unit/services/produceBlockwrapper.test.ts @@ -0,0 +1,98 @@ +import {expect} from "chai"; +import sinon from "sinon"; +import {createIChainForkConfig} from "@lodestar/config"; +import {config as mainnetConfig} from "@lodestar/config/default"; +import {ssz} from "@lodestar/types"; +import {ForkName} from "@lodestar/params"; +import {HttpStatusCode} from "@lodestar/api"; + +import {BlockProposingService} from "../../../src/services/block.js"; +import {ValidatorStore, BuilderSelection} from "../../../src/services/validatorStore.js"; +import {getApiClientStub} from "../../utils/apiStub.js"; +import {loggerVc} from "../../utils/logger.js"; +import {ClockMock} from "../../utils/clock.js"; + +describe("Produce Block with BuilderSelection", function () { + const sandbox = sinon.createSandbox(); + const api = getApiClientStub(sandbox); + const validatorStore = sinon.createStubInstance(ValidatorStore) as ValidatorStore & + sinon.SinonStubbedInstance; + + const config = createIChainForkConfig(mainnetConfig); + + const clock = new ClockMock(); + const blockService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, null); + const produceBlockWrapper = blockService["produceBlockWrapper"]; + + const blindedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + const randaoReveal = fullBlock.body.randaoReveal; + + let controller: AbortController; // To stop clock + beforeEach(() => (controller = new AbortController())); + afterEach(() => controller.abort()); + + // Testcase: BuilderSelection, builderBlockValue, engineBlockValue, selection + // null blockValue means the block was not produced + const testCases: [BuilderSelection, number | null, number | null, string][] = [ + [BuilderSelection.MaxProfit, 1, 0, "builder"], + [BuilderSelection.MaxProfit, 1, 2, "engine"], + [BuilderSelection.MaxProfit, null, 0, "engine"], + [BuilderSelection.MaxProfit, 0, null, "builder"], + + [BuilderSelection.BuilderAlways, 1, 2, "builder"], + [BuilderSelection.BuilderAlways, 1, 0, "builder"], + [BuilderSelection.BuilderAlways, null, 0, "engine"], + [BuilderSelection.BuilderAlways, 0, null, "builder"], + ]; + testCases.forEach(([builderSelection, builderBlockValue, engineBlockValue, finalSelection]) => { + it(`builder selection = ${builderSelection}, builder blockValue = ${builderBlockValue}, engine blockValue = ${engineBlockValue} - expected selection = ${finalSelection} `, async function () { + if (builderBlockValue !== null) { + api.validator.produceBlindedBlock.resolves({ + response: { + data: blindedBlock, + version: ForkName.bellatrix, + blockValue: BigInt(builderBlockValue), + }, + ok: true, + status: HttpStatusCode.OK, + }); + } else { + api.validator.produceBlindedBlock.throws(Error("not produced")); + } + + if (engineBlockValue !== null) { + api.validator.produceBlock.resolves({ + response: {data: fullBlock, blockValue: BigInt(engineBlockValue)}, + ok: true, + status: HttpStatusCode.OK, + }); + api.validator.produceBlockV2.resolves({ + response: { + data: fullBlock, + version: ForkName.bellatrix, + blockValue: BigInt(engineBlockValue), + }, + ok: true, + status: HttpStatusCode.OK, + }); + } else { + api.validator.produceBlock.throws(Error("not produced")); + api.validator.produceBlockV2.throws(Error("not produced")); + } + + const produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection, + }; + const { + debugLogCtx: {source}, + } = ((await produceBlockWrapper(144897, randaoReveal, "", produceBlockOpts)) as unknown) as { + debugLogCtx: {source: string}; + }; + expect(source).to.equal(finalSelection, "blindedBlock must be returned"); + }); + }); +});