From b82f69395d5e455532f396e761cde92de018d544 Mon Sep 17 00:00:00 2001 From: harkamal Date: Fri, 27 Jan 2023 12:26:53 +0530 Subject: [PATCH 1/8] Add validatior option to specify builder block selection strategy --- packages/cli/src/cmds/validator/handler.ts | 28 ++- packages/cli/src/cmds/validator/options.ts | 9 + packages/cli/src/util/proposerConfig.ts | 6 +- packages/validator/src/index.ts | 1 + packages/validator/src/services/block.ts | 94 ++++++---- .../validator/src/services/validatorStore.ts | 15 +- .../unit/services/produceBlockwrapper.test.ts | 170 ++++++++++++++++++ 7 files changed, 283 insertions(+), 40 deletions(-) create mode 100644 packages/validator/test/unit/services/produceBlockwrapper.test.ts diff --git a/packages/cli/src/cmds/validator/handler.ts b/packages/cli/src/cmds/validator/handler.ts index d59fdafec901..804a9462710b 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 "builderprefered": + 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..2c9581b01542 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 builderprefered", + 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/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..847cc762ccd5 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -7,9 +7,15 @@ 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"; +type ProduceBlockOpts = { + expectedFeeRecipient: string; + strictFeeRecipientCheck: boolean; + isBuilderEnabled: boolean; + builderSelection: BuilderSelection; +}; /** * Service that sets up and handles validator block proposal duties. */ @@ -74,12 +80,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,11 +123,7 @@ 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) => { @@ -138,42 +142,62 @@ export class BlockProposingService { const blindedBlock = await blindedBlockPromise; const fullBlock = await fullBlockPromise; - // 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"); + const feeRecipientCheck = {expectedFeeRecipient, strictFeeRecipientCheck}; + + if (fullBlock && blindedBlock) { + switch (builderSelection) { + case BuilderSelection.MaxProfit: { + if (fullBlock.blockValue > blindedBlock.blockValue) { + this.logger.debug( + "Returning Fullblock as fullblock.blockValue > blindedBlock.blockValue and BuilderSelection = MaxProfit " + ); + return this.getFullorBlindedBlockBundle(fullBlock, "engine", feeRecipientCheck); + } else { + this.logger.debug("Returning blindedBlock"); + return this.getFullorBlindedBlockBundle(blindedBlock, "builder", feeRecipientCheck); + } + break; + } + case BuilderSelection.BuilderAlways: + default: { + this.logger.debug("Returning blindedBlock"); + return this.getFullorBlindedBlockBundle(blindedBlock, "builder", feeRecipientCheck); + } } - 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; + } else if (fullBlock && !blindedBlock) { + this.logger.debug("Returning fullBlock - No builder block produced"); + return this.getFullorBlindedBlockBundle(fullBlock, "engine", feeRecipientCheck); + } else if (blindedBlock && !fullBlock) { + this.logger.debug("Returning blindedBlock - No execution block produced"); + return this.getFullorBlindedBlockBundle(blindedBlock, "builder", feeRecipientCheck); + } else { + throw Error("Failed to produce engine or builder block"); + } + }; + + private getFullorBlindedBlockBundle( + fullOrBlindedBlock: {data: allForks.FullOrBlindedBeaconBlock; blockValue: Wei}, + source: string, + {expectedFeeRecipient, strictFeeRecipientCheck}: {expectedFeeRecipient: string; strictFeeRecipientCheck: boolean} + ): {data: allForks.FullOrBlindedBeaconBlock} & {debugLogCtx: Record} { + const debugLogCtx = {source: source, blockValue: fullOrBlindedBlock.blockValue.toString()}; + 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 ( diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 84b59acbddc8..7ace1d62e6ac 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 = "builderprefered", + 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.MaxProfit, }; /** @@ -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..5c968ff0d037 --- /dev/null +++ b/packages/validator/test/unit/services/produceBlockwrapper.test.ts @@ -0,0 +1,170 @@ +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 {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"]; + + let controller: AbortController; // To stop clock + beforeEach(() => (controller = new AbortController())); + afterEach(() => controller.abort()); + + it("1. BuilderSelection = MaxProfit - BlindedBlock blockValue > fullBlock blockValue - return blindedBlock ", async function () { + const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + api.validator.produceBlindedBlock.resolves({ + data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), + version: ForkName.bellatrix, + blockValue: BigInt(1), + }); + api.validator.produceBlock.resolves({data: fullBlock, blockValue: ssz.Wei.defaultValue()}); + api.validator.produceBlockV2.resolves({ + data: fullBlock, + version: ForkName.bellatrix, + blockValue: ssz.Wei.defaultValue(), + }); + + const produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection: BuilderSelection.MaxProfit, + }; + const returnedBlock = await produceBlockWrapper(144897, signedBlock.body.randaoReveal, "", produceBlockOpts); + expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); + }); + it("2. BuilderSelection = MaxProfit - fullBlock blockValue > BlindedBlock blockValue - return FullBlock", async function () { + const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + api.validator.produceBlindedBlock.resolves({ + data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), + version: ForkName.bellatrix, + blockValue: ssz.Wei.defaultValue(), + }); + api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); + api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); + + const produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection: BuilderSelection.MaxProfit, + }; + const returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); + }); + it("3. BuilderSelection = BuilderAlways - return BlindedBlock", async function () { + const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + api.validator.produceBlindedBlock.resolves({ + data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), + version: ForkName.bellatrix, + blockValue: ssz.Wei.defaultValue(), + }); + api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); + api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); + + const produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection: BuilderSelection.BuilderAlways, + }; + const returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); + }); + it("4. fullBlock - null, BlindedBlock !=null return blindedBlock", async function () { + const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + //const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + api.validator.produceBlindedBlock.resolves({ + data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), + version: ForkName.bellatrix, + blockValue: ssz.Wei.defaultValue(), + }); + api.validator.produceBlock.resolves(); + api.validator.produceBlockV2.resolves(); + + let produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection: BuilderSelection.BuilderAlways, + }; + let returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); + produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection: BuilderSelection.MaxProfit, + }; + returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); + }); + it("5. fullBlock != null, BlindedBlock =null return fullBlock", async function () { + const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + api.validator.produceBlindedBlock.resolves(); + api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); + api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); + + let produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection: BuilderSelection.BuilderAlways, + }; + let returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); + produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection: BuilderSelection.MaxProfit, + }; + returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); + }); + it("6. !fullBlock, !BlindedBlock, throw error", async function () { + const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + + api.validator.produceBlindedBlock.resolves(); + api.validator.produceBlock.resolves(); + api.validator.produceBlockV2.resolves(); + + const produceBlockOpts = { + strictFeeRecipientCheck: false, + expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + isBuilderEnabled: true, + builderSelection: BuilderSelection.MaxProfit, + }; + + try { + await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + } catch (e) { + expect(e).to.be.instanceOf(Error); + } + }); +}); From 1834a3952bccfb87ca57333f4498218bd1f7f4bd Mon Sep 17 00:00:00 2001 From: harkamal Date: Fri, 27 Jan 2023 12:38:35 +0530 Subject: [PATCH 2/8] cleanup and improvements --- packages/beacon-node/test/sim/mergemock.test.ts | 3 ++- packages/cli/src/cmds/validator/handler.ts | 2 +- packages/cli/src/cmds/validator/options.ts | 2 +- packages/validator/src/services/block.ts | 12 ++++++------ packages/validator/src/services/validatorStore.ts | 4 ++-- 5 files changed, 12 insertions(+), 11 deletions(-) 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 804a9462710b..1949d5e4265e 100644 --- a/packages/cli/src/cmds/validator/handler.ts +++ b/packages/cli/src/cmds/validator/handler.ts @@ -220,7 +220,7 @@ function parseBuilderSelection(builderSelection?: string): BuilderSelection | un switch (builderSelection) { case "maxprofit": break; - case "builderprefered": + case "builderalways": break; default: throw Error("Invalid input for builder selection, check help."); diff --git a/packages/cli/src/cmds/validator/options.ts b/packages/cli/src/cmds/validator/options.ts index 2c9581b01542..e743c2cb545f 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/packages/cli/src/cmds/validator/options.ts @@ -208,7 +208,7 @@ export const validatorOptions: ICliCommandOptions = { "builder.selection": { type: "string", - description: "Default builder block selection strategy: maxprofit or builderprefered", + description: "Default builder block selection strategy: maxprofit or builderalways", defaultDescription: `${defaultOptions.builderSelection}`, group: "builder", }, diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index 847cc762ccd5..e39e903bf756 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -151,31 +151,31 @@ export class BlockProposingService { this.logger.debug( "Returning Fullblock as fullblock.blockValue > blindedBlock.blockValue and BuilderSelection = MaxProfit " ); - return this.getFullorBlindedBlockBundle(fullBlock, "engine", feeRecipientCheck); + return this.getBlockWithDebugLog(fullBlock, "engine", feeRecipientCheck); } else { this.logger.debug("Returning blindedBlock"); - return this.getFullorBlindedBlockBundle(blindedBlock, "builder", feeRecipientCheck); + return this.getBlockWithDebugLog(blindedBlock, "builder", feeRecipientCheck); } break; } case BuilderSelection.BuilderAlways: default: { this.logger.debug("Returning blindedBlock"); - return this.getFullorBlindedBlockBundle(blindedBlock, "builder", feeRecipientCheck); + return this.getBlockWithDebugLog(blindedBlock, "builder", feeRecipientCheck); } } } else if (fullBlock && !blindedBlock) { this.logger.debug("Returning fullBlock - No builder block produced"); - return this.getFullorBlindedBlockBundle(fullBlock, "engine", feeRecipientCheck); + return this.getBlockWithDebugLog(fullBlock, "engine", feeRecipientCheck); } else if (blindedBlock && !fullBlock) { this.logger.debug("Returning blindedBlock - No execution block produced"); - return this.getFullorBlindedBlockBundle(blindedBlock, "builder", feeRecipientCheck); + return this.getBlockWithDebugLog(blindedBlock, "builder", feeRecipientCheck); } else { throw Error("Failed to produce engine or builder block"); } }; - private getFullorBlindedBlockBundle( + private getBlockWithDebugLog( fullOrBlindedBlock: {data: allForks.FullOrBlindedBeaconBlock; blockValue: Wei}, source: string, {expectedFeeRecipient, strictFeeRecipientCheck}: {expectedFeeRecipient: string; strictFeeRecipientCheck: boolean} diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 7ace1d62e6ac..5a46ed40509b 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -63,7 +63,7 @@ export type SignerRemote = { }; export enum BuilderSelection { - BuilderAlways = "builderprefered", + BuilderAlways = "builderalways", MaxProfit = "maxprofit", } @@ -120,7 +120,7 @@ type ValidatorData = ProposerConfig & { export const defaultOptions = { suggestedFeeRecipient: "0x0000000000000000000000000000000000000000", defaultGasLimit: 30_000_000, - builderSelection: BuilderSelection.MaxProfit, + builderSelection: BuilderSelection.BuilderAlways, }; /** From f6eb10baca81bb766c986fa898d04e4ed3de9761 Mon Sep 17 00:00:00 2001 From: harkamal Date: Fri, 27 Jan 2023 13:02:25 +0530 Subject: [PATCH 3/8] server api fx --- packages/validator/src/services/block.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index e39e903bf756..a88b5ae8284d 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -126,7 +126,7 @@ export class BlockProposingService { {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; }) @@ -200,11 +200,7 @@ export class BlockProposingService { } /** 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); @@ -220,4 +216,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; + }; } From dbb8a59cd954be89664eb0616522218aa11cd604 Mon Sep 17 00:00:00 2001 From: harkamal Date: Fri, 27 Jan 2023 14:36:37 +0530 Subject: [PATCH 4/8] fix test --- .../unit/services/produceBlockwrapper.test.ts | 315 ++++++++++-------- 1 file changed, 176 insertions(+), 139 deletions(-) diff --git a/packages/validator/test/unit/services/produceBlockwrapper.test.ts b/packages/validator/test/unit/services/produceBlockwrapper.test.ts index 5c968ff0d037..bfb0f277ba80 100644 --- a/packages/validator/test/unit/services/produceBlockwrapper.test.ts +++ b/packages/validator/test/unit/services/produceBlockwrapper.test.ts @@ -4,6 +4,8 @@ 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"; @@ -22,149 +24,184 @@ describe("Produce Block with BuilderSelection", function () { 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()); - it("1. BuilderSelection = MaxProfit - BlindedBlock blockValue > fullBlock blockValue - return blindedBlock ", async function () { - const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - api.validator.produceBlindedBlock.resolves({ - data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), - version: ForkName.bellatrix, - blockValue: BigInt(1), - }); - api.validator.produceBlock.resolves({data: fullBlock, blockValue: ssz.Wei.defaultValue()}); - api.validator.produceBlockV2.resolves({ - data: fullBlock, - version: ForkName.bellatrix, - blockValue: ssz.Wei.defaultValue(), - }); - - const produceBlockOpts = { - strictFeeRecipientCheck: false, - expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - isBuilderEnabled: true, - builderSelection: BuilderSelection.MaxProfit, - }; - const returnedBlock = await produceBlockWrapper(144897, signedBlock.body.randaoReveal, "", produceBlockOpts); - expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); - }); - it("2. BuilderSelection = MaxProfit - fullBlock blockValue > BlindedBlock blockValue - return FullBlock", async function () { - const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - api.validator.produceBlindedBlock.resolves({ - data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), - version: ForkName.bellatrix, - blockValue: ssz.Wei.defaultValue(), + // Testcase: BuilderSelection, builderBlockValue, engineBlockValue, selection + // null blockValue means the block was not produced + const testCases: [BuilderSelection, number | null, number, string][] = [ + [BuilderSelection.MaxProfit, 1, 0, "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"); }); - api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); - api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); - - const produceBlockOpts = { - strictFeeRecipientCheck: false, - expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - isBuilderEnabled: true, - builderSelection: BuilderSelection.MaxProfit, - }; - const returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); - }); - it("3. BuilderSelection = BuilderAlways - return BlindedBlock", async function () { - const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - api.validator.produceBlindedBlock.resolves({ - data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), - version: ForkName.bellatrix, - blockValue: ssz.Wei.defaultValue(), - }); - api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); - api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); - - const produceBlockOpts = { - strictFeeRecipientCheck: false, - expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - isBuilderEnabled: true, - builderSelection: BuilderSelection.BuilderAlways, - }; - const returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); - }); - it("4. fullBlock - null, BlindedBlock !=null return blindedBlock", async function () { - const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - //const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - api.validator.produceBlindedBlock.resolves({ - data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), - version: ForkName.bellatrix, - blockValue: ssz.Wei.defaultValue(), - }); - api.validator.produceBlock.resolves(); - api.validator.produceBlockV2.resolves(); - - let produceBlockOpts = { - strictFeeRecipientCheck: false, - expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - isBuilderEnabled: true, - builderSelection: BuilderSelection.BuilderAlways, - }; - let returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); - produceBlockOpts = { - strictFeeRecipientCheck: false, - expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - isBuilderEnabled: true, - builderSelection: BuilderSelection.MaxProfit, - }; - returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); - }); - it("5. fullBlock != null, BlindedBlock =null return fullBlock", async function () { - const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - api.validator.produceBlindedBlock.resolves(); - api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); - api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); - - let produceBlockOpts = { - strictFeeRecipientCheck: false, - expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - isBuilderEnabled: true, - builderSelection: BuilderSelection.BuilderAlways, - }; - let returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); - produceBlockOpts = { - strictFeeRecipientCheck: false, - expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - isBuilderEnabled: true, - builderSelection: BuilderSelection.MaxProfit, - }; - returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); - }); - it("6. !fullBlock, !BlindedBlock, throw error", async function () { - const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - - api.validator.produceBlindedBlock.resolves(); - api.validator.produceBlock.resolves(); - api.validator.produceBlockV2.resolves(); - - const produceBlockOpts = { - strictFeeRecipientCheck: false, - expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - isBuilderEnabled: true, - builderSelection: BuilderSelection.MaxProfit, - }; - - try { - await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - } catch (e) { - expect(e).to.be.instanceOf(Error); - } }); + + // it("2. BuilderSelection = MaxProfit - fullBlock blockValue > BlindedBlock blockValue - return FullBlock", async function () { + // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + // const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + // api.validator.produceBlindedBlock.resolves({ + // data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), + // version: ForkName.bellatrix, + // blockValue: ssz.Wei.defaultValue(), + // }); + // api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); + // api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); + + // const produceBlockOpts = { + // strictFeeRecipientCheck: false, + // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + // isBuilderEnabled: true, + // builderSelection: BuilderSelection.MaxProfit, + // }; + // const returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + // expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); + // }); + // it("3. BuilderSelection = BuilderAlways - return BlindedBlock", async function () { + // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + // const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + // api.validator.produceBlindedBlock.resolves({ + // data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), + // version: ForkName.bellatrix, + // blockValue: ssz.Wei.defaultValue(), + // }); + // api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); + // api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); + + // const produceBlockOpts = { + // strictFeeRecipientCheck: false, + // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + // isBuilderEnabled: true, + // builderSelection: BuilderSelection.BuilderAlways, + // }; + // const returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + // expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); + // }); + // it("4. fullBlock - null, BlindedBlock !=null return blindedBlock", async function () { + // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + // //const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + // api.validator.produceBlindedBlock.resolves({ + // data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), + // version: ForkName.bellatrix, + // blockValue: ssz.Wei.defaultValue(), + // }); + // api.validator.produceBlock.resolves(); + // api.validator.produceBlockV2.resolves(); + + // let produceBlockOpts = { + // strictFeeRecipientCheck: false, + // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + // isBuilderEnabled: true, + // builderSelection: BuilderSelection.BuilderAlways, + // }; + // let returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + // expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); + // produceBlockOpts = { + // strictFeeRecipientCheck: false, + // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + // isBuilderEnabled: true, + // builderSelection: BuilderSelection.MaxProfit, + // }; + // returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + // expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); + // }); + // it("5. fullBlock != null, BlindedBlock =null return fullBlock", async function () { + // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + // const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); + + // api.validator.produceBlindedBlock.resolves(); + // api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); + // api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); + + // let produceBlockOpts = { + // strictFeeRecipientCheck: false, + // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + // isBuilderEnabled: true, + // builderSelection: BuilderSelection.BuilderAlways, + // }; + // let returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + // expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); + // produceBlockOpts = { + // strictFeeRecipientCheck: false, + // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + // isBuilderEnabled: true, + // builderSelection: BuilderSelection.MaxProfit, + // }; + // returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + // expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); + // }); + // it("6. !fullBlock, !BlindedBlock, throw error", async function () { + // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); + + // api.validator.produceBlindedBlock.resolves(); + // api.validator.produceBlock.resolves(); + // api.validator.produceBlockV2.resolves(); + + // const produceBlockOpts = { + // strictFeeRecipientCheck: false, + // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + // isBuilderEnabled: true, + // builderSelection: BuilderSelection.MaxProfit, + // }; + + // try { + // await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); + // } catch (e) { + // expect(e).to.be.instanceOf(Error); + // } + // }); }); From 3d72e8d72d53366a2eb10b5bff48ea9f02e10f93 Mon Sep 17 00:00:00 2001 From: harkamal Date: Fri, 27 Jan 2023 14:44:34 +0530 Subject: [PATCH 5/8] fix the testcase --- .../unit/services/produceBlockwrapper.test.ts | 127 ++---------------- 1 file changed, 9 insertions(+), 118 deletions(-) diff --git a/packages/validator/test/unit/services/produceBlockwrapper.test.ts b/packages/validator/test/unit/services/produceBlockwrapper.test.ts index bfb0f277ba80..b7c0ba9d1355 100644 --- a/packages/validator/test/unit/services/produceBlockwrapper.test.ts +++ b/packages/validator/test/unit/services/produceBlockwrapper.test.ts @@ -34,8 +34,16 @@ describe("Produce Block with BuilderSelection", function () { // Testcase: BuilderSelection, builderBlockValue, engineBlockValue, selection // null blockValue means the block was not produced - const testCases: [BuilderSelection, number | null, number, string][] = [ + 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 () { @@ -87,121 +95,4 @@ describe("Produce Block with BuilderSelection", function () { expect(source).to.equal(finalSelection, "blindedBlock must be returned"); }); }); - - // it("2. BuilderSelection = MaxProfit - fullBlock blockValue > BlindedBlock blockValue - return FullBlock", async function () { - // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - // const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - // api.validator.produceBlindedBlock.resolves({ - // data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), - // version: ForkName.bellatrix, - // blockValue: ssz.Wei.defaultValue(), - // }); - // api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); - // api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); - - // const produceBlockOpts = { - // strictFeeRecipientCheck: false, - // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - // isBuilderEnabled: true, - // builderSelection: BuilderSelection.MaxProfit, - // }; - // const returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - // expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); - // }); - // it("3. BuilderSelection = BuilderAlways - return BlindedBlock", async function () { - // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - // const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - // api.validator.produceBlindedBlock.resolves({ - // data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), - // version: ForkName.bellatrix, - // blockValue: ssz.Wei.defaultValue(), - // }); - // api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); - // api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); - - // const produceBlockOpts = { - // strictFeeRecipientCheck: false, - // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - // isBuilderEnabled: true, - // builderSelection: BuilderSelection.BuilderAlways, - // }; - // const returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - // expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); - // }); - // it("4. fullBlock - null, BlindedBlock !=null return blindedBlock", async function () { - // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - // //const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - // api.validator.produceBlindedBlock.resolves({ - // data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), - // version: ForkName.bellatrix, - // blockValue: ssz.Wei.defaultValue(), - // }); - // api.validator.produceBlock.resolves(); - // api.validator.produceBlockV2.resolves(); - - // let produceBlockOpts = { - // strictFeeRecipientCheck: false, - // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - // isBuilderEnabled: true, - // builderSelection: BuilderSelection.BuilderAlways, - // }; - // let returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - // expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); - // produceBlockOpts = { - // strictFeeRecipientCheck: false, - // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - // isBuilderEnabled: true, - // builderSelection: BuilderSelection.MaxProfit, - // }; - // returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - // expect(returnedBlock.data).to.deep.equal(signedBlock, "blindedBlock must be returned"); - // }); - // it("5. fullBlock != null, BlindedBlock =null return fullBlock", async function () { - // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - // const fullBlock = ssz.bellatrix.BeaconBlock.defaultValue(); - - // api.validator.produceBlindedBlock.resolves(); - // api.validator.produceBlock.resolves({data: fullBlock, blockValue: BigInt(1)}); - // api.validator.produceBlockV2.resolves({data: fullBlock, version: ForkName.bellatrix, blockValue: BigInt(1)}); - - // let produceBlockOpts = { - // strictFeeRecipientCheck: false, - // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - // isBuilderEnabled: true, - // builderSelection: BuilderSelection.BuilderAlways, - // }; - // let returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - // expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); - // produceBlockOpts = { - // strictFeeRecipientCheck: false, - // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - // isBuilderEnabled: true, - // builderSelection: BuilderSelection.MaxProfit, - // }; - // returnedBlock = await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - // expect(returnedBlock.data).to.deep.equal(fullBlock, "fullBlock must be returned"); - // }); - // it("6. !fullBlock, !BlindedBlock, throw error", async function () { - // const signedBlock = ssz.bellatrix.BlindedBeaconBlock.defaultValue(); - - // api.validator.produceBlindedBlock.resolves(); - // api.validator.produceBlock.resolves(); - // api.validator.produceBlockV2.resolves(); - - // const produceBlockOpts = { - // strictFeeRecipientCheck: false, - // expectedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - // isBuilderEnabled: true, - // builderSelection: BuilderSelection.MaxProfit, - // }; - - // try { - // await produceBlockWrapper(144900, signedBlock.body.randaoReveal, "", produceBlockOpts); - // } catch (e) { - // expect(e).to.be.instanceOf(Error); - // } - // }); }); From badfc5cbef6dbf9927ec3c6df00759ea40cdd7e6 Mon Sep 17 00:00:00 2001 From: harkamal Date: Fri, 27 Jan 2023 16:10:44 +0530 Subject: [PATCH 6/8] fix test --- packages/cli/test/unit/validator/parseProposerConfig.test.ts | 5 +++++ .../cli/test/unit/validator/proposerConfigs/validData.yaml | 2 ++ 2 files changed, 7 insertions(+) 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" From 3f4673bf2f91f39afcd62de98e40d3374c60bee4 Mon Sep 17 00:00:00 2001 From: harkamal Date: Sun, 29 Jan 2023 17:39:49 +0530 Subject: [PATCH 7/8] improv --- packages/validator/src/services/block.ts | 32 +++++++++++++++--------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index a88b5ae8284d..3f53889dcc49 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -10,6 +10,8 @@ import {Metrics} from "../metrics.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; @@ -140,35 +142,41 @@ 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 (fullBlock.blockValue > blindedBlock.blockValue) { - this.logger.debug( - "Returning Fullblock as fullblock.blockValue > blindedBlock.blockValue and BuilderSelection = MaxProfit " - ); - return this.getBlockWithDebugLog(fullBlock, "engine", feeRecipientCheck); + if (engineBlockValue >= builderBlockValue) { + selectedSource = "engine"; + selectedBlock = fullBlock; } else { - this.logger.debug("Returning blindedBlock"); - return this.getBlockWithDebugLog(blindedBlock, "builder", feeRecipientCheck); + selectedSource = "builder"; + selectedBlock = blindedBlock; } break; } + case BuilderSelection.BuilderAlways: default: { - this.logger.debug("Returning blindedBlock"); - return this.getBlockWithDebugLog(blindedBlock, "builder", feeRecipientCheck); + selectedSource = "builder"; + selectedBlock = blindedBlock; } } + this.logger.debug(`Selected ${selectedSource} block`, {builderSelection, engineBlockValue, builderBlockValue}); + return this.getBlockWithDebugLog(selectedBlock, selectedSource, feeRecipientCheck); } else if (fullBlock && !blindedBlock) { - this.logger.debug("Returning fullBlock - No builder block produced"); + this.logger.debug("Selected engine block: no builder block produced", {engineBlockValue}); return this.getBlockWithDebugLog(fullBlock, "engine", feeRecipientCheck); } else if (blindedBlock && !fullBlock) { - this.logger.debug("Returning blindedBlock - No execution block produced"); + this.logger.debug("Selected builder block: no engine block produced", {builderBlockValue}); return this.getBlockWithDebugLog(blindedBlock, "builder", feeRecipientCheck); } else { throw Error("Failed to produce engine or builder block"); @@ -180,7 +188,7 @@ export class BlockProposingService { source: string, {expectedFeeRecipient, strictFeeRecipientCheck}: {expectedFeeRecipient: string; strictFeeRecipientCheck: boolean} ): {data: allForks.FullOrBlindedBeaconBlock} & {debugLogCtx: Record} { - const debugLogCtx = {source: source, blockValue: fullOrBlindedBlock.blockValue.toString()}; + const debugLogCtx = {source: source, "blockValue(eth)": (fullOrBlindedBlock.blockValue / ETH_TO_WEI).toString()}; const blockFeeRecipient = (fullOrBlindedBlock.data as bellatrix.BeaconBlock).body.executionPayload?.feeRecipient; const feeRecipient = blockFeeRecipient !== undefined ? toHexString(blockFeeRecipient) : undefined; From 6d30049e1f3f2196fdb7b24dcfb80bd23ab64113 Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 30 Jan 2023 00:00:38 +0530 Subject: [PATCH 8/8] fix test --- packages/validator/src/services/block.ts | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index 3f53889dcc49..6e6088e2000b 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -170,13 +170,24 @@ export class BlockProposingService { selectedBlock = blindedBlock; } } - this.logger.debug(`Selected ${selectedSource} block`, {builderSelection, engineBlockValue, builderBlockValue}); + 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", {engineBlockValue}); + 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", {builderBlockValue}); + 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"); @@ -188,7 +199,11 @@ export class BlockProposingService { source: string, {expectedFeeRecipient, strictFeeRecipientCheck}: {expectedFeeRecipient: string; strictFeeRecipientCheck: boolean} ): {data: allForks.FullOrBlindedBeaconBlock} & {debugLogCtx: Record} { - const debugLogCtx = {source: source, "blockValue(eth)": (fullOrBlindedBlock.blockValue / ETH_TO_WEI).toString()}; + 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;