From 5b33d75d904f0621d21fbe7b9e3ba0a721294876 Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 25 Dec 2023 15:29:10 +0530 Subject: [PATCH 01/12] feat: add and support builder_boost_factor query param to produceBlockV3 api lint --- packages/api/src/beacon/routes/validator.ts | 8 +++ .../src/api/impl/validator/index.ts | 14 ++++- packages/cli/src/cmds/validator/handler.ts | 1 + packages/cli/src/cmds/validator/options.ts | 9 +++ packages/validator/src/services/block.ts | 60 +++++++++++++------ .../src/services/prepareBeaconProposer.ts | 3 +- .../validator/src/services/validatorStore.ts | 29 ++++++++- 7 files changed, 102 insertions(+), 22 deletions(-) diff --git a/packages/api/src/beacon/routes/validator.ts b/packages/api/src/beacon/routes/validator.ts index 78fc1a67d32c..a7d4c377851a 100644 --- a/packages/api/src/beacon/routes/validator.ts +++ b/packages/api/src/beacon/routes/validator.ts @@ -53,6 +53,10 @@ export enum BuilderSelection { export type ExtraProduceBlockOps = { feeRecipient?: string; builderSelection?: BuilderSelection; + // precise value isn't required because super high values will be treated as always builder prefered + // and hence UintNum64 is sufficient. If this param is present, builderSelection will be infered to + // be of maxprofit (unless explicity provided) with this %age boost factor applied to the builder values + builderBoostFactor?: UintNum64; strictFeeRecipientCheck?: boolean; blindedLocal?: boolean; }; @@ -487,6 +491,7 @@ export type ReqTypes = { skip_randao_verification?: boolean; fee_recipient?: string; builder_selection?: string; + builder_boost_factor?: UintNum64; strict_fee_recipient_check?: boolean; blinded_local?: boolean; }; @@ -555,6 +560,7 @@ export function getReqSerializers(): ReqSerializers { fee_recipient: opts?.feeRecipient, skip_randao_verification: skipRandaoVerification, builder_selection: opts?.builderSelection, + builder_boost_factor: opts?.builderBoostFactor, strict_fee_recipient_check: opts?.strictFeeRecipientCheck, blinded_local: opts?.blindedLocal, }, @@ -567,6 +573,7 @@ export function getReqSerializers(): ReqSerializers { { feeRecipient: query.fee_recipient, builderSelection: query.builder_selection as BuilderSelection, + builderBoostFactor: query.builder_boost_factor, strictFeeRecipientCheck: query.strict_fee_recipient_check, blindedLocal: query.blinded_local, }, @@ -579,6 +586,7 @@ export function getReqSerializers(): ReqSerializers { fee_recipient: Schema.String, skip_randao_verification: Schema.Boolean, builder_selection: Schema.String, + builder_boost_factor: Schema.Uint, strict_fee_recipient_check: Schema.Boolean, blinded_local: Schema.Boolean, }, diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 395684960c7f..2f144f42904c 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -423,7 +423,12 @@ export function getValidatorApi({ graffiti, // TODO deneb: skip randao verification _skipRandaoVerification?: boolean, - {feeRecipient, builderSelection, strictFeeRecipientCheck}: routes.validator.ExtraProduceBlockOps = {} + { + feeRecipient, + builderSelection, + builderBoostFactor, + strictFeeRecipientCheck, + }: routes.validator.ExtraProduceBlockOps = {} ) { notWhileSyncing(); await waitForSlot(slot); // Must never request for a future slot > currentSlot @@ -436,7 +441,10 @@ export function getValidatorApi({ const fork = config.getForkName(slot); // set some sensible opts + // builderSelection will be deprecated and will run in mode MaxProfit if builder is enabled + // and the actual selection will be determined using builderBoostFactor passed by the validator builderSelection = builderSelection ?? routes.validator.BuilderSelection.MaxProfit; + builderBoostFactor = builderBoostFactor ?? 100; const isBuilderEnabled = ForkSeq[fork] >= ForkSeq.bellatrix && chain.executionBuilder !== undefined && @@ -448,6 +456,7 @@ export function getValidatorApi({ slot, isBuilderEnabled, strictFeeRecipientCheck, + builderBoostFactor, }); // Start calls for building execution and builder blocks const blindedBlockPromise = isBuilderEnabled @@ -541,7 +550,7 @@ export function getValidatorApi({ if (fullBlock && blindedBlock) { switch (builderSelection) { case routes.validator.BuilderSelection.MaxProfit: { - if (blockValueEngine >= blockValueBuilder) { + if (blockValueEngine >= (blockValueBuilder * BigInt(builderBoostFactor)) / BigInt(100)) { executionPayloadSource = ProducedBlockSource.engine; } else { executionPayloadSource = ProducedBlockSource.builder; @@ -561,6 +570,7 @@ export function getValidatorApi({ } logger.verbose(`Selected executionPayloadSource=${executionPayloadSource} block`, { builderSelection, + builderBoostFactor, // winston logger doesn't like bigint enginePayloadValue: `${enginePayloadValue}`, builderPayloadValue: `${builderPayloadValue}`, diff --git a/packages/cli/src/cmds/validator/handler.ts b/packages/cli/src/cmds/validator/handler.ts index 69e4610bff0e..57b00d78c706 100644 --- a/packages/cli/src/cmds/validator/handler.ts +++ b/packages/cli/src/cmds/validator/handler.ts @@ -227,6 +227,7 @@ function getProposerConfigFromArgs( selection: parseBuilderSelection( args["builder.selection"] ?? (args["builder"] ? defaultOptions.builderAliasSelection : undefined) ), + boostFactor: args["builder.boostFactor"], }, }; diff --git a/packages/cli/src/cmds/validator/options.ts b/packages/cli/src/cmds/validator/options.ts index 41069cfbdd34..2c8862400516 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/packages/cli/src/cmds/validator/options.ts @@ -45,6 +45,7 @@ export type IValidatorCliArgs = AccountValidatorArgs & builder?: boolean; "builder.selection"?: string; + "builder.boostFactor"?: number; useProduceBlockV3?: boolean; broadcastValidation?: string; @@ -246,6 +247,14 @@ export const validatorOptions: CliCommandOptions = { group: "builder", }, + "builder.boostFactor": { + type: "number", + description: + "A factor in percentage requested to block producing beacon to boost (>100) or dampen(<100) builder block value for selection against engine, is overriden when `--builder.selection` set to anything other than `maxprofit`", + defaultDescription: `${defaultOptions.builderBoostFactor}`, + group: "builder", + }, + useProduceBlockV3: { type: "boolean", description: "Enable/disable usage of produceBlockV3 that might not be supported by all beacon clients yet", diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index b59902870c92..c11af904b34d 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -121,13 +121,15 @@ export class BlockProposingService { const debugLogCtx = {...logCtx, validator: pubkeyHex}; const strictFeeRecipientCheck = this.validatorStore.strictFeeRecipientCheck(pubkeyHex); - const builderSelection = this.validatorStore.getBuilderSelection(pubkeyHex); + const {selection: builderSelection, boostFactor: builderBoostFactor} = + this.validatorStore.getBuilderSelectionParams(pubkeyHex); const feeRecipient = this.validatorStore.getFeeRecipient(pubkeyHex); const blindedLocal = this.opts.blindedLocal; this.logger.debug("Producing block", { ...debugLogCtx, builderSelection, + builderBoostFactor, feeRecipient, strictFeeRecipientCheck, useProduceBlockV3: this.opts.useProduceBlockV3, @@ -139,15 +141,20 @@ export class BlockProposingService { const produceOpts = { feeRecipient, strictFeeRecipientCheck, - builderSelection, + builderBoostFactor, blindedLocal, }; - const blockContents = await produceBlockFn(this.config, slot, randaoReveal, graffiti, produceOpts).catch( - (e: Error) => { - this.metrics?.blockProposingErrors.inc({error: "produce"}); - throw extendError(e, "Failed to produce block"); - } - ); + const blockContents = await produceBlockFn( + this.config, + slot, + randaoReveal, + graffiti, + produceOpts, + builderSelection + ).catch((e: Error) => { + this.metrics?.blockProposingErrors.inc({error: "produce"}); + throw extendError(e, "Failed to produce block"); + }); this.logger.debug("Produced block", {...debugLogCtx, ...blockContents.debugLogCtx}); this.metrics?.blocksProduced.inc(); @@ -195,13 +202,15 @@ export class BlockProposingService { slot: Slot, randaoReveal: BLSSignature, graffiti: string, - {feeRecipient, strictFeeRecipientCheck, builderSelection, blindedLocal}: routes.validator.ExtraProduceBlockOps + {feeRecipient, strictFeeRecipientCheck, builderBoostFactor, blindedLocal}: routes.validator.ExtraProduceBlockOps, + builderSelection: routes.validator.BuilderSelection ): Promise => { const res = await this.api.validator.produceBlockV3(slot, randaoReveal, graffiti, false, { feeRecipient, builderSelection, strictFeeRecipientCheck, blindedLocal, + builderBoostFactor, }); ApiError.assert(res, "Failed to produce block: validator.produceBlockV2"); const {response} = res; @@ -223,7 +232,7 @@ export class BlockProposingService { api: "produceBlockV3", }; - return parseProduceBlockResponse(response, debugLogCtx); + return parseProduceBlockResponse(response, debugLogCtx, builderSelection); }; /** a wrapper function used for backward compatibility with the clients who don't have v3 implemented yet */ @@ -232,7 +241,8 @@ export class BlockProposingService { slot: Slot, randaoReveal: BLSSignature, graffiti: string, - {builderSelection}: routes.validator.ExtraProduceBlockOps + _opts: routes.validator.ExtraProduceBlockOps, + builderSelection: routes.validator.BuilderSelection ): Promise => { // other clients have always implemented builder vs execution race in produce blinded block // so if builderSelection is executiononly then only we call produceBlockV2 else produceBlockV3 always @@ -248,7 +258,8 @@ export class BlockProposingService { return parseProduceBlockResponse( {executionPayloadBlinded: false, executionPayloadSource, ...response}, - debugLogCtx + debugLogCtx, + builderSelection ); } else { Object.assign(debugLogCtx, {api: "produceBlindedBlock"}); @@ -259,7 +270,8 @@ export class BlockProposingService { return parseProduceBlockResponse( {executionPayloadBlinded: true, executionPayloadSource, ...response}, - debugLogCtx + debugLogCtx, + builderSelection ); } }; @@ -267,15 +279,29 @@ export class BlockProposingService { function parseProduceBlockResponse( response: routes.validator.ProduceFullOrBlindedBlockOrContentsRes, - debugLogCtx: Record + debugLogCtx: Record, + builderSelection: routes.validator.BuilderSelection ): FullOrBlindedBlockWithContents & DebugLogCtx { + const executionPayloadSource = response.executionPayloadSource; + + if ( + (builderSelection === routes.validator.BuilderSelection.BuilderOnly && + executionPayloadSource === ProducedBlockSource.engine) || + (builderSelection === routes.validator.BuilderSelection.ExecutionOnly && + executionPayloadSource === ProducedBlockSource.builder) + ) { + throw Error( + `Block not produced as per desired builderSelection=${builderSelection} executionPayloadSource=${executionPayloadSource}` + ); + } + if (response.executionPayloadBlinded) { return { block: response.data, contents: null, version: response.version, executionPayloadBlinded: true, - executionPayloadSource: response.executionPayloadSource, + executionPayloadSource, debugLogCtx, } as FullOrBlindedBlockWithContents & DebugLogCtx; } else { @@ -285,7 +311,7 @@ function parseProduceBlockResponse( contents: {blobs: response.data.blobs, kzgProofs: response.data.kzgProofs}, version: response.version, executionPayloadBlinded: false, - executionPayloadSource: response.executionPayloadSource, + executionPayloadSource, debugLogCtx, } as FullOrBlindedBlockWithContents & DebugLogCtx; } else { @@ -294,7 +320,7 @@ function parseProduceBlockResponse( contents: null, version: response.version, executionPayloadBlinded: false, - executionPayloadSource: response.executionPayloadSource, + executionPayloadSource, debugLogCtx, } as FullOrBlindedBlockWithContents & DebugLogCtx; } diff --git a/packages/validator/src/services/prepareBeaconProposer.ts b/packages/validator/src/services/prepareBeaconProposer.ts index 7ca939fb0c41..7d7907a4592d 100644 --- a/packages/validator/src/services/prepareBeaconProposer.ts +++ b/packages/validator/src/services/prepareBeaconProposer.ts @@ -86,7 +86,8 @@ export function pollBuilderValidatorRegistration( .filter( (pubkeyHex): pubkeyHex is string => pubkeyHex !== undefined && - validatorStore.getBuilderSelection(pubkeyHex) !== routes.validator.BuilderSelection.ExecutionOnly + validatorStore.getBuilderSelectionParams(pubkeyHex).selection !== + routes.validator.BuilderSelection.ExecutionOnly ); if (pubkeyHexes.length > 0) { diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 8cafaa5b14b6..61c794414303 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -69,6 +69,7 @@ type DefaultProposerConfig = { builder: { gasLimit: number; selection: routes.validator.BuilderSelection; + boostFactor: number; }; }; @@ -79,6 +80,7 @@ export type ProposerConfig = { builder?: { gasLimit?: number; selection?: routes.validator.BuilderSelection; + boostFactor?: number; }; }; @@ -123,6 +125,7 @@ export const defaultOptions = { defaultGasLimit: 30_000_000, builderSelection: routes.validator.BuilderSelection.ExecutionOnly, builderAliasSelection: routes.validator.BuilderSelection.MaxProfit, + builderBoostFactor: 100, // turn it off by default, turn it back on once other clients support v3 api useProduceBlockV3: false, // spec asks for gossip validation by default @@ -131,6 +134,8 @@ export const defaultOptions = { blindedLocal: false, }; +const MAX_BUILDER_BOOST_FACTOR = 2 ** 64 - 1; + /** * Service that sets up and handles validator attester duties. */ @@ -162,6 +167,7 @@ export class ValidatorStore { builder: { gasLimit: defaultConfig.builder?.gasLimit ?? defaultOptions.defaultGasLimit, selection: defaultConfig.builder?.selection ?? defaultOptions.builderSelection, + boostFactor: defaultConfig.builder?.boostFactor ?? defaultOptions.builderBoostFactor, }, }; @@ -252,8 +258,27 @@ export class ValidatorStore { delete validatorData["graffiti"]; } - getBuilderSelection(pubkeyHex: PubkeyHex): routes.validator.BuilderSelection { - return (this.validators.get(pubkeyHex)?.builder || {}).selection ?? this.defaultProposerConfig.builder.selection; + getBuilderSelectionParams(pubkeyHex: PubkeyHex): {selection: routes.validator.BuilderSelection; boostFactor: number} { + const selection = + (this.validators.get(pubkeyHex)?.builder || {}).selection ?? this.defaultProposerConfig.builder.selection; + + let boostFactor; + switch (selection) { + case routes.validator.BuilderSelection.MaxProfit: + boostFactor = + (this.validators.get(pubkeyHex)?.builder || {}).boostFactor ?? this.defaultProposerConfig.builder.boostFactor; + break; + + case routes.validator.BuilderSelection.BuilderAlways: + case routes.validator.BuilderSelection.BuilderOnly: + boostFactor = MAX_BUILDER_BOOST_FACTOR; + break; + + case routes.validator.BuilderSelection.ExecutionOnly: + boostFactor = 0; + } + + return {selection, boostFactor}; } strictFeeRecipientCheck(pubkeyHex: PubkeyHex): boolean { From f8960af449029114a52ae1b659033e8859dcd2df Mon Sep 17 00:00:00 2001 From: harkamal Date: Thu, 4 Jan 2024 17:27:27 +0530 Subject: [PATCH 02/12] add keymanager endpoint and update the test --- packages/api/src/keymanager/routes.ts | 40 +++++++++++++++++++ .../cli/src/cmds/validator/keymanager/impl.ts | 9 +++++ .../validator/src/services/validatorStore.ts | 8 ++++ .../test/unit/services/block.test.ts | 26 ++++++++++++ 4 files changed, 83 insertions(+) diff --git a/packages/api/src/keymanager/routes.ts b/packages/api/src/keymanager/routes.ts index 09f5e7610604..c9a16a52e343 100644 --- a/packages/api/src/keymanager/routes.ts +++ b/packages/api/src/keymanager/routes.ts @@ -247,6 +247,16 @@ export type Api = { > >; + updateBuilderBoostFactor( + pubkey: string, + builderBoostFactor: number + ): Promise< + ApiClientResponse< + {[HttpStatusCode.OK]: void; [HttpStatusCode.NO_CONTENT]: void}, + HttpStatusCode.UNAUTHORIZED | HttpStatusCode.FORBIDDEN | HttpStatusCode.NOT_FOUND + > + >; + /** * Create a signed voluntary exit message for an active validator, identified by a public key known to the validator * client. This endpoint returns a `SignedVoluntaryExit` object, which can be used to initiate voluntary exit via the @@ -290,6 +300,8 @@ export const routesData: RoutesData = { setGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "POST", statusOk: 202}, deleteGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "DELETE", statusOk: 204}, + updateBuilderBoostFactor: {url: "/eth/v1/validator/{pubkey}/builder_boost_factor", method: "POST", statusOk: 202}, + signVoluntaryExit: {url: "/eth/v1/validator/{pubkey}/voluntary_exit", method: "POST"}, }; @@ -326,6 +338,8 @@ export type ReqTypes = { setGasLimit: {params: {pubkey: string}; body: {gas_limit: string}}; deleteGasLimit: {params: {pubkey: string}}; + updateBuilderBoostFactor: {params: {pubkey: string}; body: {builder_boost_factor: string}}; + signVoluntaryExit: {params: {pubkey: string}; query: {epoch?: number}}; }; @@ -423,6 +437,17 @@ export function getReqSerializers(): ReqSerializers { params: {pubkey: Schema.StringRequired}, }, }, + updateBuilderBoostFactor: { + writeReq: (pubkey, builderBoostFactor) => ({ + params: {pubkey}, + body: {builder_boost_factor: builderBoostFactor.toString(10)}, + }), + parseReq: ({params: {pubkey}, body: {builder_boost_factor}}) => [pubkey, parseBoostFactor(builder_boost_factor)], + schema: { + params: {pubkey: Schema.StringRequired}, + body: Schema.Object, + }, + }, signVoluntaryExit: { writeReq: (pubkey, epoch) => ({params: {pubkey}, query: epoch !== undefined ? {epoch} : {}}), parseReq: ({params: {pubkey}, query: {epoch}}) => [pubkey, epoch], @@ -469,3 +494,18 @@ function parseGasLimit(gasLimitInput: string | number): number { } return gasLimit; } + +function parseBoostFactor(builderBoostFactorInput: string | number): number { + if ( + (typeof builderBoostFactorInput !== "string" && typeof builderBoostFactorInput !== "number") || + `${builderBoostFactorInput}`.trim() === "" + ) { + throw Error("Not valid Builder Boost Factor"); + } + + const builderBoostFactor = Number(builderBoostFactorInput); + if (Number.isNaN(builderBoostFactor)) { + throw Error(`Builder Boost Factor is not valid builderBoostFactor=${builderBoostFactor}`); + } + return builderBoostFactor; +} diff --git a/packages/cli/src/cmds/validator/keymanager/impl.ts b/packages/cli/src/cmds/validator/keymanager/impl.ts index 2abda3c9642e..fd57b624f50d 100644 --- a/packages/cli/src/cmds/validator/keymanager/impl.ts +++ b/packages/cli/src/cmds/validator/keymanager/impl.ts @@ -390,6 +390,15 @@ export class KeymanagerApi implements Api { }; } + async updateBuilderBoostFactor(pubkeyHex: string, builderBoostFactor: number): Promise { + this.checkIfProposerWriteEnabled(); + this.validator.validatorStore.updateBuilderBoostFactor(pubkeyHex, builderBoostFactor); + this.persistedKeysBackend.writeProposerConfig( + pubkeyHex, + this.validator.validatorStore.getProposerConfig(pubkeyHex) + ); + } + /** * Create and sign a voluntary exit message for an active validator */ diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 61c794414303..18f9dba28529 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -311,6 +311,14 @@ export class ValidatorStore { delete validatorData.builder?.gasLimit; } + updateBuilderBoostFactor(pubkeyHex: PubkeyHex, boostFactor: number): void { + const validatorData = this.validators.get(pubkeyHex); + if (validatorData === undefined) { + throw Error(`Validator pubkey ${pubkeyHex} not known`); + } + validatorData.builder = {...validatorData.builder, boostFactor}; + } + /** Return true if `index` is active part of this validator client */ hasValidatorIndex(index: ValidatorIndex): boolean { return this.indicesService.index2pubkey.has(index); diff --git a/packages/validator/test/unit/services/block.test.ts b/packages/validator/test/unit/services/block.test.ts index f879017c95f1..7825208ee5a5 100644 --- a/packages/validator/test/unit/services/block.test.ts +++ b/packages/validator/test/unit/services/block.test.ts @@ -59,6 +59,14 @@ describe("BlockDutiesService", function () { const signedBlock = ssz.phase0.SignedBeaconBlock.defaultValue(); validatorStore.signRandao.resolves(signedBlock.message.body.randaoReveal); validatorStore.signBlock.callsFake(async (_, block) => ({message: block, signature: signedBlock.signature})); + validatorStore.getBuilderSelectionParams.returns({ + selection: routes.validator.BuilderSelection.MaxProfit, + boostFactor: 100, + }); + validatorStore.getGraffiti.returns("aaaa"); + validatorStore.getFeeRecipient.returns("0x00"); + validatorStore.strictFeeRecipientCheck.returns(false); + api.validator.produceBlockV3.resolves({ response: { data: signedBlock.message, @@ -86,6 +94,24 @@ describe("BlockDutiesService", function () { [signedBlock, {broadcastValidation: routes.beacon.BroadcastValidation.consensus}], "wrong publishBlock() args" ); + + // ProduceBlockV3 is called with all correct arguments + expect(api.validator.produceBlockV3.getCall(0).args).to.deep.equal( + [ + 1, + signedBlock.message.body.randaoReveal, + "aaaa", + false, + { + feeRecipient: "0x00", + builderSelection: routes.validator.BuilderSelection.MaxProfit, + strictFeeRecipientCheck: false, + blindedLocal: false, + builderBoostFactor: 100, + }, + ], + "wrong produceBlockV3() args" + ); }); it("Should produce, sign, and publish a blinded block", async function () { From 4eefc31d8a615649ebe021cbb91cd5050b4a219d Mon Sep 17 00:00:00 2001 From: harkamal Date: Sun, 7 Jan 2024 14:39:25 +0530 Subject: [PATCH 03/12] update builder boost factor to bigint --- packages/api/src/beacon/routes/validator.ts | 15 ++++++--- packages/api/src/keymanager/routes.ts | 19 ++--------- .../test/unit/beacon/testData/validator.ts | 32 ++++++++++++++++--- packages/api/test/unit/keymanager/testData.ts | 5 +++ .../src/api/impl/validator/index.ts | 9 +++--- .../cli/src/cmds/validator/keymanager/impl.ts | 2 +- packages/cli/src/cmds/validator/options.ts | 2 +- .../validator/src/services/validatorStore.ts | 14 ++++---- .../test/unit/services/block.test.ts | 4 +-- 9 files changed, 61 insertions(+), 41 deletions(-) diff --git a/packages/api/src/beacon/routes/validator.ts b/packages/api/src/beacon/routes/validator.ts index a7d4c377851a..0fa58f7e9032 100644 --- a/packages/api/src/beacon/routes/validator.ts +++ b/packages/api/src/beacon/routes/validator.ts @@ -13,6 +13,7 @@ import { Slot, ssz, UintNum64, + UintBn64, ValidatorIndex, RootHex, StringType, @@ -56,7 +57,7 @@ export type ExtraProduceBlockOps = { // precise value isn't required because super high values will be treated as always builder prefered // and hence UintNum64 is sufficient. If this param is present, builderSelection will be infered to // be of maxprofit (unless explicity provided) with this %age boost factor applied to the builder values - builderBoostFactor?: UintNum64; + builderBoostFactor?: UintBn64; strictFeeRecipientCheck?: boolean; blindedLocal?: boolean; }; @@ -491,7 +492,7 @@ export type ReqTypes = { skip_randao_verification?: boolean; fee_recipient?: string; builder_selection?: string; - builder_boost_factor?: UintNum64; + builder_boost_factor?: string; strict_fee_recipient_check?: boolean; blinded_local?: boolean; }; @@ -560,7 +561,7 @@ export function getReqSerializers(): ReqSerializers { fee_recipient: opts?.feeRecipient, skip_randao_verification: skipRandaoVerification, builder_selection: opts?.builderSelection, - builder_boost_factor: opts?.builderBoostFactor, + builder_boost_factor: opts?.builderBoostFactor?.toString(), strict_fee_recipient_check: opts?.strictFeeRecipientCheck, blinded_local: opts?.blindedLocal, }, @@ -573,7 +574,7 @@ export function getReqSerializers(): ReqSerializers { { feeRecipient: query.fee_recipient, builderSelection: query.builder_selection as BuilderSelection, - builderBoostFactor: query.builder_boost_factor, + builderBoostFactor: parseBuilderBoostFactor(query.builder_boost_factor), strictFeeRecipientCheck: query.strict_fee_recipient_check, blindedLocal: query.blinded_local, }, @@ -586,7 +587,7 @@ export function getReqSerializers(): ReqSerializers { fee_recipient: Schema.String, skip_randao_verification: Schema.Boolean, builder_selection: Schema.String, - builder_boost_factor: Schema.Uint, + builder_boost_factor: Schema.String, strict_fee_recipient_check: Schema.Boolean, blinded_local: Schema.Boolean, }, @@ -793,3 +794,7 @@ export function getReturnTypes(): ReturnTypes { getLiveness: jsonType("snake"), }; } + +function parseBuilderBoostFactor(builderBoostFactorInput?: string | number | bigint): bigint | undefined { + return builderBoostFactorInput !== undefined ? BigInt(builderBoostFactorInput) : undefined; +} diff --git a/packages/api/src/keymanager/routes.ts b/packages/api/src/keymanager/routes.ts index c9a16a52e343..e43f3b6ffee7 100644 --- a/packages/api/src/keymanager/routes.ts +++ b/packages/api/src/keymanager/routes.ts @@ -249,7 +249,7 @@ export type Api = { updateBuilderBoostFactor( pubkey: string, - builderBoostFactor: number + builderBoostFactor: bigint ): Promise< ApiClientResponse< {[HttpStatusCode.OK]: void; [HttpStatusCode.NO_CONTENT]: void}, @@ -442,7 +442,7 @@ export function getReqSerializers(): ReqSerializers { params: {pubkey}, body: {builder_boost_factor: builderBoostFactor.toString(10)}, }), - parseReq: ({params: {pubkey}, body: {builder_boost_factor}}) => [pubkey, parseBoostFactor(builder_boost_factor)], + parseReq: ({params: {pubkey}, body: {builder_boost_factor}}) => [pubkey, BigInt(builder_boost_factor)], schema: { params: {pubkey: Schema.StringRequired}, body: Schema.Object, @@ -494,18 +494,3 @@ function parseGasLimit(gasLimitInput: string | number): number { } return gasLimit; } - -function parseBoostFactor(builderBoostFactorInput: string | number): number { - if ( - (typeof builderBoostFactorInput !== "string" && typeof builderBoostFactorInput !== "number") || - `${builderBoostFactorInput}`.trim() === "" - ) { - throw Error("Not valid Builder Boost Factor"); - } - - const builderBoostFactor = Number(builderBoostFactorInput); - if (Number.isNaN(builderBoostFactor)) { - throw Error(`Builder Boost Factor is not valid builderBoostFactor=${builderBoostFactor}`); - } - return builderBoostFactor; -} diff --git a/packages/api/test/unit/beacon/testData/validator.ts b/packages/api/test/unit/beacon/testData/validator.ts index c10f67fa4095..2688f2080eba 100644 --- a/packages/api/test/unit/beacon/testData/validator.ts +++ b/packages/api/test/unit/beacon/testData/validator.ts @@ -50,7 +50,13 @@ export const testData: GenericServerTestCases = { randaoReveal, graffiti, undefined, - {feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined}, + { + feeRecipient, + builderSelection: undefined, + strictFeeRecipientCheck: undefined, + blindedLocal: undefined, + builderBoostFactor: 100n, + }, ] as unknown as GenericServerTestCases["produceBlock"]["args"], res: {data: ssz.phase0.BeaconBlock.defaultValue()}, }, @@ -60,7 +66,13 @@ export const testData: GenericServerTestCases = { randaoReveal, graffiti, undefined, - {feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined}, + { + feeRecipient, + builderSelection: undefined, + strictFeeRecipientCheck: undefined, + blindedLocal: undefined, + builderBoostFactor: 100n, + }, ] as unknown as GenericServerTestCases["produceBlockV2"]["args"], res: { data: ssz.altair.BeaconBlock.defaultValue(), @@ -75,7 +87,13 @@ export const testData: GenericServerTestCases = { randaoReveal, graffiti, true, - {feeRecipient, builderSelection: undefined, strictFeeRecipientCheck: undefined}, + { + feeRecipient, + builderSelection: undefined, + strictFeeRecipientCheck: undefined, + blindedLocal: undefined, + builderBoostFactor: 100n, + }, ], res: { data: ssz.altair.BeaconBlock.defaultValue(), @@ -92,7 +110,13 @@ export const testData: GenericServerTestCases = { randaoReveal, graffiti, undefined, - {feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined}, + { + feeRecipient, + builderSelection: undefined, + strictFeeRecipientCheck: undefined, + blindedLocal: undefined, + builderBoostFactor: 100n, + }, ] as unknown as GenericServerTestCases["produceBlindedBlock"]["args"], res: { data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(), diff --git a/packages/api/test/unit/keymanager/testData.ts b/packages/api/test/unit/keymanager/testData.ts index a4fc72fc8e2d..c63d974ab5c4 100644 --- a/packages/api/test/unit/keymanager/testData.ts +++ b/packages/api/test/unit/keymanager/testData.ts @@ -13,6 +13,7 @@ const pubkeyRand = "0x84105a985058fc8740a48bf1ede9d223ef09e8c6b1735ba0a55cf4a9ff const ethaddressRand = "0xabcf8e0d4e9587369b2301d0790347320302cc09"; const graffitiRandUtf8 = "636861696e736166652f6c6f64657374"; const gasLimitRand = 30_000_000; +const builderBoostFactor = BigInt(100); export const testData: GenericServerTestCases = { listKeys: { @@ -99,4 +100,8 @@ export const testData: GenericServerTestCases = { args: [pubkeyRand, 1], res: {data: ssz.phase0.SignedVoluntaryExit.defaultValue()}, }, + updateBuilderBoostFactor: { + args: [pubkeyRand, builderBoostFactor], + res: undefined, + }, }; diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 2f144f42904c..04b1c862d0f8 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -444,7 +444,7 @@ export function getValidatorApi({ // builderSelection will be deprecated and will run in mode MaxProfit if builder is enabled // and the actual selection will be determined using builderBoostFactor passed by the validator builderSelection = builderSelection ?? routes.validator.BuilderSelection.MaxProfit; - builderBoostFactor = builderBoostFactor ?? 100; + builderBoostFactor = builderBoostFactor ?? BigInt(100); const isBuilderEnabled = ForkSeq[fork] >= ForkSeq.bellatrix && chain.executionBuilder !== undefined && @@ -456,7 +456,8 @@ export function getValidatorApi({ slot, isBuilderEnabled, strictFeeRecipientCheck, - builderBoostFactor, + // winston logger doesn't like bigint + builderBoostFactor: `${builderBoostFactor}`, }); // Start calls for building execution and builder blocks const blindedBlockPromise = isBuilderEnabled @@ -550,7 +551,7 @@ export function getValidatorApi({ if (fullBlock && blindedBlock) { switch (builderSelection) { case routes.validator.BuilderSelection.MaxProfit: { - if (blockValueEngine >= (blockValueBuilder * BigInt(builderBoostFactor)) / BigInt(100)) { + if (blockValueEngine >= (blockValueBuilder * builderBoostFactor) / BigInt(100)) { executionPayloadSource = ProducedBlockSource.engine; } else { executionPayloadSource = ProducedBlockSource.builder; @@ -570,8 +571,8 @@ export function getValidatorApi({ } logger.verbose(`Selected executionPayloadSource=${executionPayloadSource} block`, { builderSelection, - builderBoostFactor, // winston logger doesn't like bigint + builderBoostFactor: `${builderBoostFactor}`, enginePayloadValue: `${enginePayloadValue}`, builderPayloadValue: `${builderPayloadValue}`, consensusBlockValueEngine: `${consensusBlockValueEngine}`, diff --git a/packages/cli/src/cmds/validator/keymanager/impl.ts b/packages/cli/src/cmds/validator/keymanager/impl.ts index fd57b624f50d..0f94943a706c 100644 --- a/packages/cli/src/cmds/validator/keymanager/impl.ts +++ b/packages/cli/src/cmds/validator/keymanager/impl.ts @@ -390,7 +390,7 @@ export class KeymanagerApi implements Api { }; } - async updateBuilderBoostFactor(pubkeyHex: string, builderBoostFactor: number): Promise { + async updateBuilderBoostFactor(pubkeyHex: string, builderBoostFactor: bigint): Promise { this.checkIfProposerWriteEnabled(); this.validator.validatorStore.updateBuilderBoostFactor(pubkeyHex, builderBoostFactor); this.persistedKeysBackend.writeProposerConfig( diff --git a/packages/cli/src/cmds/validator/options.ts b/packages/cli/src/cmds/validator/options.ts index 2c8862400516..8de2a6002686 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/packages/cli/src/cmds/validator/options.ts @@ -45,7 +45,7 @@ export type IValidatorCliArgs = AccountValidatorArgs & builder?: boolean; "builder.selection"?: string; - "builder.boostFactor"?: number; + "builder.boostFactor"?: bigint; useProduceBlockV3?: boolean; broadcastValidation?: string; diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 18f9dba28529..13fd1543f6ff 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -69,7 +69,7 @@ type DefaultProposerConfig = { builder: { gasLimit: number; selection: routes.validator.BuilderSelection; - boostFactor: number; + boostFactor: bigint; }; }; @@ -80,7 +80,7 @@ export type ProposerConfig = { builder?: { gasLimit?: number; selection?: routes.validator.BuilderSelection; - boostFactor?: number; + boostFactor?: bigint; }; }; @@ -125,7 +125,7 @@ export const defaultOptions = { defaultGasLimit: 30_000_000, builderSelection: routes.validator.BuilderSelection.ExecutionOnly, builderAliasSelection: routes.validator.BuilderSelection.MaxProfit, - builderBoostFactor: 100, + builderBoostFactor: BigInt(100), // turn it off by default, turn it back on once other clients support v3 api useProduceBlockV3: false, // spec asks for gossip validation by default @@ -134,7 +134,7 @@ export const defaultOptions = { blindedLocal: false, }; -const MAX_BUILDER_BOOST_FACTOR = 2 ** 64 - 1; +const MAX_BUILDER_BOOST_FACTOR = BigInt(2 ** 64 - 1); /** * Service that sets up and handles validator attester duties. @@ -258,7 +258,7 @@ export class ValidatorStore { delete validatorData["graffiti"]; } - getBuilderSelectionParams(pubkeyHex: PubkeyHex): {selection: routes.validator.BuilderSelection; boostFactor: number} { + getBuilderSelectionParams(pubkeyHex: PubkeyHex): {selection: routes.validator.BuilderSelection; boostFactor: bigint} { const selection = (this.validators.get(pubkeyHex)?.builder || {}).selection ?? this.defaultProposerConfig.builder.selection; @@ -275,7 +275,7 @@ export class ValidatorStore { break; case routes.validator.BuilderSelection.ExecutionOnly: - boostFactor = 0; + boostFactor = BigInt(0); } return {selection, boostFactor}; @@ -311,7 +311,7 @@ export class ValidatorStore { delete validatorData.builder?.gasLimit; } - updateBuilderBoostFactor(pubkeyHex: PubkeyHex, boostFactor: number): void { + updateBuilderBoostFactor(pubkeyHex: PubkeyHex, boostFactor: bigint): void { const validatorData = this.validators.get(pubkeyHex); if (validatorData === undefined) { throw Error(`Validator pubkey ${pubkeyHex} not known`); diff --git a/packages/validator/test/unit/services/block.test.ts b/packages/validator/test/unit/services/block.test.ts index 7825208ee5a5..3677cdac3a7a 100644 --- a/packages/validator/test/unit/services/block.test.ts +++ b/packages/validator/test/unit/services/block.test.ts @@ -61,7 +61,7 @@ describe("BlockDutiesService", function () { validatorStore.signBlock.callsFake(async (_, block) => ({message: block, signature: signedBlock.signature})); validatorStore.getBuilderSelectionParams.returns({ selection: routes.validator.BuilderSelection.MaxProfit, - boostFactor: 100, + boostFactor: BigInt(100), }); validatorStore.getGraffiti.returns("aaaa"); validatorStore.getFeeRecipient.returns("0x00"); @@ -107,7 +107,7 @@ describe("BlockDutiesService", function () { builderSelection: routes.validator.BuilderSelection.MaxProfit, strictFeeRecipientCheck: false, blindedLocal: false, - builderBoostFactor: 100, + builderBoostFactor: BigInt(100), }, ], "wrong produceBlockV3() args" From 41d111267f3dc34918f9a52a9676a03b5891451e Mon Sep 17 00:00:00 2001 From: g11tech Date: Sun, 7 Jan 2024 21:29:06 +0530 Subject: [PATCH 04/12] update the help Co-authored-by: Nico Flaig --- packages/cli/src/cmds/validator/options.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/cmds/validator/options.ts b/packages/cli/src/cmds/validator/options.ts index 8de2a6002686..e68e04a4b884 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/packages/cli/src/cmds/validator/options.ts @@ -250,7 +250,7 @@ export const validatorOptions: CliCommandOptions = { "builder.boostFactor": { type: "number", description: - "A factor in percentage requested to block producing beacon to boost (>100) or dampen(<100) builder block value for selection against engine, is overriden when `--builder.selection` set to anything other than `maxprofit`", + "Percentage multiplier the block producing beacon node must apply to boost (>100) or dampen (<100) builder block value for selection against execution block. The multiplier is ignored if `--builder.selection` is set to anything other than `maxprofit`", defaultDescription: `${defaultOptions.builderBoostFactor}`, group: "builder", }, From 3e73ac82d68821d6df99b3f211feafb161f89657 Mon Sep 17 00:00:00 2001 From: harkamal Date: Sun, 7 Jan 2024 21:33:17 +0530 Subject: [PATCH 05/12] remove comment --- packages/api/src/beacon/routes/validator.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/api/src/beacon/routes/validator.ts b/packages/api/src/beacon/routes/validator.ts index 0fa58f7e9032..0746797cbf0e 100644 --- a/packages/api/src/beacon/routes/validator.ts +++ b/packages/api/src/beacon/routes/validator.ts @@ -54,9 +54,6 @@ export enum BuilderSelection { export type ExtraProduceBlockOps = { feeRecipient?: string; builderSelection?: BuilderSelection; - // precise value isn't required because super high values will be treated as always builder prefered - // and hence UintNum64 is sufficient. If this param is present, builderSelection will be infered to - // be of maxprofit (unless explicity provided) with this %age boost factor applied to the builder values builderBoostFactor?: UintBn64; strictFeeRecipientCheck?: boolean; blindedLocal?: boolean; From 0d135729a2c183284c47fc3c49dad7d5de1608ee Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 8 Jan 2024 00:24:06 +0530 Subject: [PATCH 06/12] validate and use boostfactor ranges as per spec --- .../beacon-node/src/api/impl/validator/index.ts | 13 +++++++++++-- packages/validator/src/index.ts | 2 +- .../validator/src/services/validatorStore.ts | 17 +++++++++++++++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 04b1c862d0f8..897b65a5fd83 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -19,6 +19,7 @@ import { isForkExecution, ForkSeq, } from "@lodestar/params"; +import {MAX_BUILDER_BOOST_FACTOR} from "@lodestar/validator"; import { Root, Slot, @@ -445,10 +446,15 @@ export function getValidatorApi({ // and the actual selection will be determined using builderBoostFactor passed by the validator builderSelection = builderSelection ?? routes.validator.BuilderSelection.MaxProfit; builderBoostFactor = builderBoostFactor ?? BigInt(100); + if (builderBoostFactor > MAX_BUILDER_BOOST_FACTOR) { + throw Error(`Invalid builderBoostFactor=${builderBoostFactor} > MAX_BUILDER_BOOST_FACTOR`); + } + const isBuilderEnabled = ForkSeq[fork] >= ForkSeq.bellatrix && chain.executionBuilder !== undefined && - builderSelection !== routes.validator.BuilderSelection.ExecutionOnly; + builderSelection !== routes.validator.BuilderSelection.ExecutionOnly && + builderBoostFactor !== BigInt(0); logger.verbose("Assembling block with produceEngineOrBuilderBlock ", { fork, @@ -551,7 +557,10 @@ export function getValidatorApi({ if (fullBlock && blindedBlock) { switch (builderSelection) { case routes.validator.BuilderSelection.MaxProfit: { - if (blockValueEngine >= (blockValueBuilder * builderBoostFactor) / BigInt(100)) { + if ( + builderBoostFactor === MAX_BUILDER_BOOST_FACTOR || + blockValueEngine >= (blockValueBuilder * builderBoostFactor) / BigInt(100) + ) { executionPayloadSource = ProducedBlockSource.engine; } else { executionPayloadSource = ProducedBlockSource.builder; diff --git a/packages/validator/src/index.ts b/packages/validator/src/index.ts index 381db59b7c85..4619b924ef63 100644 --- a/packages/validator/src/index.ts +++ b/packages/validator/src/index.ts @@ -1,5 +1,5 @@ export {Validator, type ValidatorOptions} from "./validator.js"; -export {ValidatorStore, SignerType, defaultOptions} from "./services/validatorStore.js"; +export {ValidatorStore, SignerType, defaultOptions, MAX_BUILDER_BOOST_FACTOR} from "./services/validatorStore.js"; export type { Signer, SignerLocal, diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 13fd1543f6ff..1987af8ee719 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -134,7 +134,7 @@ export const defaultOptions = { blindedLocal: false, }; -const MAX_BUILDER_BOOST_FACTOR = BigInt(2 ** 64 - 1); +export const MAX_BUILDER_BOOST_FACTOR = BigInt(2 ** 64 - 1); /** * Service that sets up and handles validator attester duties. @@ -160,6 +160,11 @@ export class ValidatorStore { this.metrics = metrics; const defaultConfig = valProposerConfig.defaultConfig; + const builderBoostFactor = defaultConfig.builder?.boostFactor ?? defaultOptions.builderBoostFactor; + if (builderBoostFactor > MAX_BUILDER_BOOST_FACTOR) { + throw Error(`Invalid builderBoostFactor=${builderBoostFactor} > MAX_BUILDER_BOOST_FACTOR for defaultConfigc`); + } + this.defaultProposerConfig = { graffiti: defaultConfig.graffiti ?? "", strictFeeRecipientCheck: defaultConfig.strictFeeRecipientCheck ?? false, @@ -167,7 +172,7 @@ export class ValidatorStore { builder: { gasLimit: defaultConfig.builder?.gasLimit ?? defaultOptions.defaultGasLimit, selection: defaultConfig.builder?.selection ?? defaultOptions.builderSelection, - boostFactor: defaultConfig.builder?.boostFactor ?? defaultOptions.builderBoostFactor, + boostFactor: builderBoostFactor, }, }; @@ -312,6 +317,10 @@ export class ValidatorStore { } updateBuilderBoostFactor(pubkeyHex: PubkeyHex, boostFactor: bigint): void { + if (boostFactor > MAX_BUILDER_BOOST_FACTOR) { + throw Error(`Invalid builderBoostFactor=${boostFactor} > MAX_BUILDER_BOOST_FACTOR`); + } + const validatorData = this.validators.get(pubkeyHex); if (validatorData === undefined) { throw Error(`Validator pubkey ${pubkeyHex} not known`); @@ -348,6 +357,10 @@ export class ValidatorStore { async addSigner(signer: Signer, valProposerConfig?: ValidatorProposerConfig): Promise { const pubkey = getSignerPubkeyHex(signer); const proposerConfig = (valProposerConfig?.proposerConfig ?? {})[pubkey]; + const builderBoostFactor = proposerConfig.builder?.boostFactor; + if (builderBoostFactor !== undefined && builderBoostFactor > MAX_BUILDER_BOOST_FACTOR) { + throw Error(`Invalid builderBoostFactor=${builderBoostFactor} > MAX_BUILDER_BOOST_FACTOR for pubkey=${pubkey}`); + } if (!this.validators.has(pubkey)) { // Doppelganger registration must be done before adding validator to signers From 24925f2118faf08b8ab67d57cbcd25eb13b43015 Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 8 Jan 2024 00:29:12 +0530 Subject: [PATCH 07/12] fix test --- packages/validator/src/services/validatorStore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 1987af8ee719..89835a62e132 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -357,7 +357,7 @@ export class ValidatorStore { async addSigner(signer: Signer, valProposerConfig?: ValidatorProposerConfig): Promise { const pubkey = getSignerPubkeyHex(signer); const proposerConfig = (valProposerConfig?.proposerConfig ?? {})[pubkey]; - const builderBoostFactor = proposerConfig.builder?.boostFactor; + const builderBoostFactor = proposerConfig?.builder?.boostFactor; if (builderBoostFactor !== undefined && builderBoostFactor > MAX_BUILDER_BOOST_FACTOR) { throw Error(`Invalid builderBoostFactor=${builderBoostFactor} > MAX_BUILDER_BOOST_FACTOR for pubkey=${pubkey}`); } From deaaa1bd4f1ee6ef9af72d8e32ee4e32aab94df4 Mon Sep 17 00:00:00 2001 From: g11tech Date: Mon, 8 Jan 2024 14:33:34 +0530 Subject: [PATCH 08/12] correct typo Co-authored-by: NC --- packages/validator/src/services/validatorStore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 89835a62e132..b00ff562bf6d 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -162,7 +162,7 @@ export class ValidatorStore { const defaultConfig = valProposerConfig.defaultConfig; const builderBoostFactor = defaultConfig.builder?.boostFactor ?? defaultOptions.builderBoostFactor; if (builderBoostFactor > MAX_BUILDER_BOOST_FACTOR) { - throw Error(`Invalid builderBoostFactor=${builderBoostFactor} > MAX_BUILDER_BOOST_FACTOR for defaultConfigc`); + throw Error(`Invalid builderBoostFactor=${builderBoostFactor} > MAX_BUILDER_BOOST_FACTOR for defaultConfig`); } this.defaultProposerConfig = { From 15948e8c7e2baa423c1545098f40a80d977a3c37 Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 8 Jan 2024 17:04:13 +0530 Subject: [PATCH 09/12] fix the block selection condition --- packages/beacon-node/src/api/impl/validator/index.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 897b65a5fd83..2479fda5ea62 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -558,8 +558,10 @@ export function getValidatorApi({ switch (builderSelection) { case routes.validator.BuilderSelection.MaxProfit: { if ( - builderBoostFactor === MAX_BUILDER_BOOST_FACTOR || - blockValueEngine >= (blockValueBuilder * builderBoostFactor) / BigInt(100) + // explicitly handle the two special values mentioned in spec for builder preferred/ engine preffered + builderBoostFactor !== MAX_BUILDER_BOOST_FACTOR && + (builderBoostFactor === BigInt(0) || + blockValueEngine >= (blockValueBuilder * builderBoostFactor) / BigInt(100)) ) { executionPayloadSource = ProducedBlockSource.engine; } else { From 0119cd927f087a53178a3acba8ebbc1ca4848ec0 Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 8 Jan 2024 17:08:04 +0530 Subject: [PATCH 10/12] fixes for spec complaince --- packages/beacon-node/src/api/impl/validator/index.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 2479fda5ea62..e8f736e0b3c5 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -447,14 +447,13 @@ export function getValidatorApi({ builderSelection = builderSelection ?? routes.validator.BuilderSelection.MaxProfit; builderBoostFactor = builderBoostFactor ?? BigInt(100); if (builderBoostFactor > MAX_BUILDER_BOOST_FACTOR) { - throw Error(`Invalid builderBoostFactor=${builderBoostFactor} > MAX_BUILDER_BOOST_FACTOR`); + throw new ApiError(400, `Invalid builderBoostFactor=${builderBoostFactor} > MAX_BUILDER_BOOST_FACTOR`); } const isBuilderEnabled = ForkSeq[fork] >= ForkSeq.bellatrix && chain.executionBuilder !== undefined && - builderSelection !== routes.validator.BuilderSelection.ExecutionOnly && - builderBoostFactor !== BigInt(0); + builderSelection !== routes.validator.BuilderSelection.ExecutionOnly; logger.verbose("Assembling block with produceEngineOrBuilderBlock ", { fork, From bca1d6376537535c2eeff0443a8c92ec0307b11a Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 8 Jan 2024 17:34:15 +0530 Subject: [PATCH 11/12] fix the keymanager routes --- packages/api/src/keymanager/routes.ts | 52 +++++++++++++++++-- packages/api/test/unit/keymanager/testData.ts | 14 +++-- .../cli/src/cmds/validator/keymanager/impl.ts | 18 ++++++- .../validator/src/services/validatorStore.ts | 18 ++++++- 4 files changed, 92 insertions(+), 10 deletions(-) diff --git a/packages/api/src/keymanager/routes.ts b/packages/api/src/keymanager/routes.ts index e43f3b6ffee7..48f928e86100 100644 --- a/packages/api/src/keymanager/routes.ts +++ b/packages/api/src/keymanager/routes.ts @@ -72,6 +72,10 @@ export type GasLimitData = { pubkey: string; gasLimit: number; }; +export type BuilderBoostFactorData = { + pubkey: string; + builderBoostFactor: bigint; +}; export type SignerDefinition = { pubkey: PubkeyHex; @@ -247,7 +251,10 @@ export type Api = { > >; - updateBuilderBoostFactor( + getBuilderBoostFactor( + pubkey: string + ): Promise>; + setBuilderBoostFactor( pubkey: string, builderBoostFactor: bigint ): Promise< @@ -256,6 +263,14 @@ export type Api = { HttpStatusCode.UNAUTHORIZED | HttpStatusCode.FORBIDDEN | HttpStatusCode.NOT_FOUND > >; + deleteBuilderBoostFactor( + pubkey: string + ): Promise< + ApiClientResponse< + {[HttpStatusCode.OK]: void; [HttpStatusCode.NO_CONTENT]: void}, + HttpStatusCode.UNAUTHORIZED | HttpStatusCode.FORBIDDEN | HttpStatusCode.NOT_FOUND + > + >; /** * Create a signed voluntary exit message for an active validator, identified by a public key known to the validator @@ -300,7 +315,9 @@ export const routesData: RoutesData = { setGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "POST", statusOk: 202}, deleteGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "DELETE", statusOk: 204}, - updateBuilderBoostFactor: {url: "/eth/v1/validator/{pubkey}/builder_boost_factor", method: "POST", statusOk: 202}, + getBuilderBoostFactor: {url: "/eth/v1/validator/{pubkey}/builder_boost_factor", method: "GET"}, + setBuilderBoostFactor: {url: "/eth/v1/validator/{pubkey}/builder_boost_factor", method: "POST", statusOk: 202}, + deleteBuilderBoostFactor: {url: "/eth/v1/validator/{pubkey}/builder_boost_factor", method: "DELETE", statusOk: 204}, signVoluntaryExit: {url: "/eth/v1/validator/{pubkey}/voluntary_exit", method: "POST"}, }; @@ -338,7 +355,9 @@ export type ReqTypes = { setGasLimit: {params: {pubkey: string}; body: {gas_limit: string}}; deleteGasLimit: {params: {pubkey: string}}; - updateBuilderBoostFactor: {params: {pubkey: string}; body: {builder_boost_factor: string}}; + getBuilderBoostFactor: {params: {pubkey: string}}; + setBuilderBoostFactor: {params: {pubkey: string}; body: {builder_boost_factor: string}}; + deleteBuilderBoostFactor: {params: {pubkey: string}}; signVoluntaryExit: {params: {pubkey: string}; query: {epoch?: number}}; }; @@ -437,7 +456,15 @@ export function getReqSerializers(): ReqSerializers { params: {pubkey: Schema.StringRequired}, }, }, - updateBuilderBoostFactor: { + + getBuilderBoostFactor: { + writeReq: (pubkey) => ({params: {pubkey}}), + parseReq: ({params: {pubkey}}) => [pubkey], + schema: { + params: {pubkey: Schema.StringRequired}, + }, + }, + setBuilderBoostFactor: { writeReq: (pubkey, builderBoostFactor) => ({ params: {pubkey}, body: {builder_boost_factor: builderBoostFactor.toString(10)}, @@ -448,6 +475,14 @@ export function getReqSerializers(): ReqSerializers { body: Schema.Object, }, }, + deleteBuilderBoostFactor: { + writeReq: (pubkey) => ({params: {pubkey}}), + parseReq: ({params: {pubkey}}) => [pubkey], + schema: { + params: {pubkey: Schema.StringRequired}, + }, + }, + signVoluntaryExit: { writeReq: (pubkey, epoch) => ({params: {pubkey}, query: epoch !== undefined ? {epoch} : {}}), parseReq: ({params: {pubkey}, query: {epoch}}) => [pubkey, epoch], @@ -480,6 +515,15 @@ export function getReturnTypes(): ReturnTypes { {jsonCase: "eth2"} ) ), + getBuilderBoostFactor: ContainerData( + new ContainerType( + { + pubkey: stringType, + builderBoostFactor: ssz.UintBn64, + }, + {jsonCase: "eth2"} + ) + ), signVoluntaryExit: ContainerData(ssz.phase0.SignedVoluntaryExit), }; } diff --git a/packages/api/test/unit/keymanager/testData.ts b/packages/api/test/unit/keymanager/testData.ts index c63d974ab5c4..2c66610c8733 100644 --- a/packages/api/test/unit/keymanager/testData.ts +++ b/packages/api/test/unit/keymanager/testData.ts @@ -13,7 +13,7 @@ const pubkeyRand = "0x84105a985058fc8740a48bf1ede9d223ef09e8c6b1735ba0a55cf4a9ff const ethaddressRand = "0xabcf8e0d4e9587369b2301d0790347320302cc09"; const graffitiRandUtf8 = "636861696e736166652f6c6f64657374"; const gasLimitRand = 30_000_000; -const builderBoostFactor = BigInt(100); +const builderBoostFactorRand = BigInt(100); export const testData: GenericServerTestCases = { listKeys: { @@ -100,8 +100,16 @@ export const testData: GenericServerTestCases = { args: [pubkeyRand, 1], res: {data: ssz.phase0.SignedVoluntaryExit.defaultValue()}, }, - updateBuilderBoostFactor: { - args: [pubkeyRand, builderBoostFactor], + getBuilderBoostFactor: { + args: [pubkeyRand], + res: {data: {pubkey: pubkeyRand, builderBoostFactor: builderBoostFactorRand}}, + }, + setBuilderBoostFactor: { + args: [pubkeyRand, builderBoostFactorRand], + res: undefined, + }, + deleteBuilderBoostFactor: { + args: [pubkeyRand], res: undefined, }, }; diff --git a/packages/cli/src/cmds/validator/keymanager/impl.ts b/packages/cli/src/cmds/validator/keymanager/impl.ts index 0f94943a706c..4628c96285df 100644 --- a/packages/cli/src/cmds/validator/keymanager/impl.ts +++ b/packages/cli/src/cmds/validator/keymanager/impl.ts @@ -390,9 +390,23 @@ export class KeymanagerApi implements Api { }; } - async updateBuilderBoostFactor(pubkeyHex: string, builderBoostFactor: bigint): Promise { + async getBuilderBoostFactor(pubkeyHex: string): ReturnType { + const builderBoostFactor = this.validator.validatorStore.getBuilderBoostFactor(pubkeyHex); + return {data: {pubkey: pubkeyHex, builderBoostFactor}}; + } + + async setBuilderBoostFactor(pubkeyHex: string, builderBoostFactor: bigint): Promise { + this.checkIfProposerWriteEnabled(); + this.validator.validatorStore.setBuilderBoostFactor(pubkeyHex, builderBoostFactor); + this.persistedKeysBackend.writeProposerConfig( + pubkeyHex, + this.validator.validatorStore.getProposerConfig(pubkeyHex) + ); + } + + async deleteBuilderBoostFactor(pubkeyHex: string): Promise { this.checkIfProposerWriteEnabled(); - this.validator.validatorStore.updateBuilderBoostFactor(pubkeyHex, builderBoostFactor); + this.validator.validatorStore.deleteBuilderBoostFactor(pubkeyHex); this.persistedKeysBackend.writeProposerConfig( pubkeyHex, this.validator.validatorStore.getProposerConfig(pubkeyHex) diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index b00ff562bf6d..809ca0c8a7c6 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -316,7 +316,15 @@ export class ValidatorStore { delete validatorData.builder?.gasLimit; } - updateBuilderBoostFactor(pubkeyHex: PubkeyHex, boostFactor: bigint): void { + getBuilderBoostFactor(pubkeyHex: PubkeyHex): bigint { + const validatorData = this.validators.get(pubkeyHex); + if (validatorData === undefined) { + throw Error(`Validator pubkey ${pubkeyHex} not known`); + } + return validatorData?.builder?.boostFactor ?? this.defaultProposerConfig.builder.boostFactor; + } + + setBuilderBoostFactor(pubkeyHex: PubkeyHex, boostFactor: bigint): void { if (boostFactor > MAX_BUILDER_BOOST_FACTOR) { throw Error(`Invalid builderBoostFactor=${boostFactor} > MAX_BUILDER_BOOST_FACTOR`); } @@ -328,6 +336,14 @@ export class ValidatorStore { validatorData.builder = {...validatorData.builder, boostFactor}; } + deleteBuilderBoostFactor(pubkeyHex: PubkeyHex): void { + const validatorData = this.validators.get(pubkeyHex); + if (validatorData === undefined) { + throw Error(`Validator pubkey ${pubkeyHex} not known`); + } + delete validatorData.builder?.boostFactor; + } + /** Return true if `index` is active part of this validator client */ hasValidatorIndex(index: ValidatorIndex): boolean { return this.indicesService.index2pubkey.has(index); From a543e3c0b1c9a18bd69ca4ee03c1d9322900d6c2 Mon Sep 17 00:00:00 2001 From: g11tech Date: Mon, 8 Jan 2024 19:47:23 +0530 Subject: [PATCH 12/12] comment typo fix Co-authored-by: Nico Flaig --- packages/beacon-node/src/api/impl/validator/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index e8f736e0b3c5..9f98c606d525 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -557,7 +557,7 @@ export function getValidatorApi({ switch (builderSelection) { case routes.validator.BuilderSelection.MaxProfit: { if ( - // explicitly handle the two special values mentioned in spec for builder preferred/ engine preffered + // explicitly handle the two special values mentioned in spec for builder preferred / engine preferred builderBoostFactor !== MAX_BUILDER_BOOST_FACTOR && (builderBoostFactor === BigInt(0) || blockValueEngine >= (blockValueBuilder * builderBoostFactor) / BigInt(100))