Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add and support builder_boost_factor query param to produceBlockV3 api #6236

Merged
merged 12 commits into from
Jan 8, 2024
10 changes: 10 additions & 0 deletions packages/api/src/beacon/routes/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
Slot,
ssz,
UintNum64,
UintBn64,
ValidatorIndex,
RootHex,
StringType,
Expand Down Expand Up @@ -53,6 +54,7 @@ export enum BuilderSelection {
export type ExtraProduceBlockOps = {
feeRecipient?: string;
builderSelection?: BuilderSelection;
builderBoostFactor?: UintBn64;
g11tech marked this conversation as resolved.
Show resolved Hide resolved
strictFeeRecipientCheck?: boolean;
blindedLocal?: boolean;
};
Expand Down Expand Up @@ -487,6 +489,7 @@ export type ReqTypes = {
skip_randao_verification?: boolean;
fee_recipient?: string;
builder_selection?: string;
builder_boost_factor?: string;
strict_fee_recipient_check?: boolean;
blinded_local?: boolean;
};
Expand Down Expand Up @@ -555,6 +558,7 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
fee_recipient: opts?.feeRecipient,
skip_randao_verification: skipRandaoVerification,
builder_selection: opts?.builderSelection,
builder_boost_factor: opts?.builderBoostFactor?.toString(),
strict_fee_recipient_check: opts?.strictFeeRecipientCheck,
blinded_local: opts?.blindedLocal,
},
Expand All @@ -567,6 +571,7 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
{
feeRecipient: query.fee_recipient,
builderSelection: query.builder_selection as BuilderSelection,
builderBoostFactor: parseBuilderBoostFactor(query.builder_boost_factor),
strictFeeRecipientCheck: query.strict_fee_recipient_check,
blindedLocal: query.blinded_local,
},
Expand All @@ -579,6 +584,7 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
fee_recipient: Schema.String,
skip_randao_verification: Schema.Boolean,
builder_selection: Schema.String,
builder_boost_factor: Schema.String,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to use uint here, the schema validation ensures there can't be negative values

case Schema.Uint:
case Schema.UintRequired:
return {type: "integer", minimum: 0};

This should address issues mentioned by @ensi321 in #6236 (comment) and #6236 (comment).

But I think the problem is that this only supports 32-bit integers but 2**64 - 1 needs to be allowed, based on openapi docs, we could add another uint type which support 64-bit integers.

Copy link
Contributor Author

@g11tech g11tech Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we can't for the exact reason you mentioned, if you want, could you pick that up as a followup (openapi one)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we can improve the validation in a separate PR

strict_fee_recipient_check: Schema.Boolean,
blinded_local: Schema.Boolean,
},
Expand Down Expand Up @@ -785,3 +791,7 @@ export function getReturnTypes(): ReturnTypes<Api> {
getLiveness: jsonType("snake"),
};
}

function parseBuilderBoostFactor(builderBoostFactorInput?: string | number | bigint): bigint | undefined {
return builderBoostFactorInput !== undefined ? BigInt(builderBoostFactorInput) : undefined;
g11tech marked this conversation as resolved.
Show resolved Hide resolved
g11tech marked this conversation as resolved.
Show resolved Hide resolved
}
69 changes: 69 additions & 0 deletions packages/api/src/keymanager/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ export type GasLimitData = {
pubkey: string;
gasLimit: number;
};
export type BuilderBoostFactorData = {
pubkey: string;
builderBoostFactor: bigint;
};

export type SignerDefinition = {
pubkey: PubkeyHex;
Expand Down Expand Up @@ -247,6 +251,27 @@ export type Api = {
>
>;

getBuilderBoostFactor(
pubkey: string
): Promise<ApiClientResponse<{[HttpStatusCode.OK]: {data: BuilderBoostFactorData}}>>;
setBuilderBoostFactor(
pubkey: string,
builderBoostFactor: bigint
): Promise<
ApiClientResponse<
{[HttpStatusCode.OK]: void; [HttpStatusCode.NO_CONTENT]: void},
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
* client. This endpoint returns a `SignedVoluntaryExit` object, which can be used to initiate voluntary exit via the
Expand Down Expand Up @@ -290,6 +315,10 @@ export const routesData: RoutesData<Api> = {
setGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "POST", statusOk: 202},
deleteGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "DELETE", statusOk: 204},

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"},
};

Expand Down Expand Up @@ -326,6 +355,10 @@ export type ReqTypes = {
setGasLimit: {params: {pubkey: string}; body: {gas_limit: string}};
deleteGasLimit: {params: {pubkey: 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}};
};

Expand Down Expand Up @@ -423,6 +456,33 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
params: {pubkey: Schema.StringRequired},
},
},

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)},
}),
parseReq: ({params: {pubkey}, body: {builder_boost_factor}}) => [pubkey, BigInt(builder_boost_factor)],
schema: {
params: {pubkey: Schema.StringRequired},
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],
Expand Down Expand Up @@ -455,6 +515,15 @@ export function getReturnTypes(): ReturnTypes<Api> {
{jsonCase: "eth2"}
)
),
getBuilderBoostFactor: ContainerData(
new ContainerType(
{
pubkey: stringType,
builderBoostFactor: ssz.UintBn64,
},
{jsonCase: "eth2"}
)
),
signVoluntaryExit: ContainerData(ssz.phase0.SignedVoluntaryExit),
};
}
Expand Down
32 changes: 28 additions & 4 deletions packages/api/test/unit/beacon/testData/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,13 @@ export const testData: GenericServerTestCases<Api> = {
randaoReveal,
graffiti,
undefined,
{feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined},
{
feeRecipient,
builderSelection: undefined,
strictFeeRecipientCheck: undefined,
blindedLocal: undefined,
builderBoostFactor: 100n,
},
] as unknown as GenericServerTestCases<Api>["produceBlock"]["args"],
res: {data: ssz.phase0.BeaconBlock.defaultValue()},
},
Expand All @@ -60,7 +66,13 @@ export const testData: GenericServerTestCases<Api> = {
randaoReveal,
graffiti,
undefined,
{feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined},
{
feeRecipient,
builderSelection: undefined,
strictFeeRecipientCheck: undefined,
blindedLocal: undefined,
builderBoostFactor: 100n,
},
] as unknown as GenericServerTestCases<Api>["produceBlockV2"]["args"],
res: {
data: ssz.altair.BeaconBlock.defaultValue(),
Expand All @@ -75,7 +87,13 @@ export const testData: GenericServerTestCases<Api> = {
randaoReveal,
graffiti,
true,
{feeRecipient, builderSelection: undefined, strictFeeRecipientCheck: undefined},
{
feeRecipient,
builderSelection: undefined,
strictFeeRecipientCheck: undefined,
blindedLocal: undefined,
builderBoostFactor: 100n,
},
],
res: {
data: ssz.altair.BeaconBlock.defaultValue(),
Expand All @@ -92,7 +110,13 @@ export const testData: GenericServerTestCases<Api> = {
randaoReveal,
graffiti,
undefined,
{feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined},
{
feeRecipient,
builderSelection: undefined,
strictFeeRecipientCheck: undefined,
blindedLocal: undefined,
builderBoostFactor: 100n,
},
] as unknown as GenericServerTestCases<Api>["produceBlindedBlock"]["args"],
res: {
data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(),
Expand Down
13 changes: 13 additions & 0 deletions packages/api/test/unit/keymanager/testData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const pubkeyRand = "0x84105a985058fc8740a48bf1ede9d223ef09e8c6b1735ba0a55cf4a9ff
const ethaddressRand = "0xabcf8e0d4e9587369b2301d0790347320302cc09";
const graffitiRandUtf8 = "636861696e736166652f6c6f64657374";
const gasLimitRand = 30_000_000;
const builderBoostFactorRand = BigInt(100);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bunch of new hardcoded 100, could a constant be used more globally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also added to todo cleanup list


export const testData: GenericServerTestCases<Api> = {
listKeys: {
Expand Down Expand Up @@ -99,4 +100,16 @@ export const testData: GenericServerTestCases<Api> = {
args: [pubkeyRand, 1],
res: {data: ssz.phase0.SignedVoluntaryExit.defaultValue()},
},
getBuilderBoostFactor: {
args: [pubkeyRand],
res: {data: {pubkey: pubkeyRand, builderBoostFactor: builderBoostFactorRand}},
},
setBuilderBoostFactor: {
args: [pubkeyRand, builderBoostFactorRand],
res: undefined,
},
deleteBuilderBoostFactor: {
args: [pubkeyRand],
res: undefined,
},
};
25 changes: 23 additions & 2 deletions packages/beacon-node/src/api/impl/validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
isForkExecution,
ForkSeq,
} from "@lodestar/params";
import {MAX_BUILDER_BOOST_FACTOR} from "@lodestar/validator";
import {
Root,
Slot,
Expand Down Expand Up @@ -423,7 +424,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
Expand All @@ -436,7 +442,14 @@ 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 ?? BigInt(100);
if (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 &&
Expand All @@ -448,6 +461,8 @@ export function getValidatorApi({
slot,
isBuilderEnabled,
strictFeeRecipientCheck,
// winston logger doesn't like bigint
builderBoostFactor: `${builderBoostFactor}`,
g11tech marked this conversation as resolved.
Show resolved Hide resolved
});
// Start calls for building execution and builder blocks
const blindedBlockPromise = isBuilderEnabled
Expand Down Expand Up @@ -541,7 +556,12 @@ export function getValidatorApi({
if (fullBlock && blindedBlock) {
switch (builderSelection) {
case routes.validator.BuilderSelection.MaxProfit: {
if (blockValueEngine >= blockValueBuilder) {
if (
// explicitly handle the two special values mentioned in spec for builder preferred/ engine preffered
g11tech marked this conversation as resolved.
Show resolved Hide resolved
builderBoostFactor !== MAX_BUILDER_BOOST_FACTOR &&
(builderBoostFactor === BigInt(0) ||
blockValueEngine >= (blockValueBuilder * builderBoostFactor) / BigInt(100))
g11tech marked this conversation as resolved.
Show resolved Hide resolved
) {
g11tech marked this conversation as resolved.
Show resolved Hide resolved
executionPayloadSource = ProducedBlockSource.engine;
} else {
executionPayloadSource = ProducedBlockSource.builder;
Expand All @@ -562,6 +582,7 @@ export function getValidatorApi({
logger.verbose(`Selected executionPayloadSource=${executionPayloadSource} block`, {
builderSelection,
// winston logger doesn't like bigint
builderBoostFactor: `${builderBoostFactor}`,
enginePayloadValue: `${enginePayloadValue}`,
builderPayloadValue: `${builderPayloadValue}`,
consensusBlockValueEngine: `${consensusBlockValueEngine}`,
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/cmds/validator/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ function getProposerConfigFromArgs(
selection: parseBuilderSelection(
args["builder.selection"] ?? (args["builder"] ? defaultOptions.builderAliasSelection : undefined)
),
boostFactor: args["builder.boostFactor"],
},
};

Expand Down
23 changes: 23 additions & 0 deletions packages/cli/src/cmds/validator/keymanager/impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,29 @@ export class KeymanagerApi implements Api {
};
}

async getBuilderBoostFactor(pubkeyHex: string): ReturnType<Api["getBuilderBoostFactor"]> {
const builderBoostFactor = this.validator.validatorStore.getBuilderBoostFactor(pubkeyHex);
return {data: {pubkey: pubkeyHex, builderBoostFactor}};
}

async setBuilderBoostFactor(pubkeyHex: string, builderBoostFactor: bigint): Promise<void> {
this.checkIfProposerWriteEnabled();
this.validator.validatorStore.setBuilderBoostFactor(pubkeyHex, builderBoostFactor);
this.persistedKeysBackend.writeProposerConfig(
pubkeyHex,
this.validator.validatorStore.getProposerConfig(pubkeyHex)
);
}

async deleteBuilderBoostFactor(pubkeyHex: string): Promise<void> {
this.checkIfProposerWriteEnabled();
this.validator.validatorStore.deleteBuilderBoostFactor(pubkeyHex);
this.persistedKeysBackend.writeProposerConfig(
pubkeyHex,
this.validator.validatorStore.getProposerConfig(pubkeyHex)
);
}

/**
* Create and sign a voluntary exit message for an active validator
*/
Expand Down
9 changes: 9 additions & 0 deletions packages/cli/src/cmds/validator/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export type IValidatorCliArgs = AccountValidatorArgs &

builder?: boolean;
"builder.selection"?: string;
"builder.boostFactor"?: bigint;

useProduceBlockV3?: boolean;
broadcastValidation?: string;
Expand Down Expand Up @@ -246,6 +247,14 @@ export const validatorOptions: CliCommandOptions<IValidatorCliArgs> = {
group: "builder",
},

"builder.boostFactor": {
type: "number",
description:
"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",
},

useProduceBlockV3: {
type: "boolean",
description: "Enable/disable usage of produceBlockV3 that might not be supported by all beacon clients yet",
Expand Down
2 changes: 1 addition & 1 deletion packages/validator/src/index.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
Loading
Loading