From c6c5ffbbaf7952ae993d2ce3053e9328b9710885 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Thu, 9 May 2024 17:54:56 +0300 Subject: [PATCH 01/31] Define ClientCode and engine_getClientVersionV1 --- .../src/execution/engine/disabled.ts | 6 ++++- .../beacon-node/src/execution/engine/http.ts | 20 ++++++++++++++- .../src/execution/engine/interface.ts | 25 +++++++++++++++++++ .../beacon-node/src/execution/engine/types.ts | 17 ++++++++++++- 4 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/disabled.ts b/packages/beacon-node/src/execution/engine/disabled.ts index 82bf84c37d33..5de713c37788 100644 --- a/packages/beacon-node/src/execution/engine/disabled.ts +++ b/packages/beacon-node/src/execution/engine/disabled.ts @@ -1,4 +1,4 @@ -import {ExecutionEngineState, IExecutionEngine, PayloadIdCache} from "./interface.js"; +import {ClientVersion, ExecutionEngineState, IExecutionEngine, PayloadIdCache} from "./interface.js"; export class ExecutionEngineDisabled implements IExecutionEngine { readonly payloadIdCache = new PayloadIdCache(); @@ -30,4 +30,8 @@ export class ExecutionEngineDisabled implements IExecutionEngine { getState(): ExecutionEngineState { throw Error("Execution engine disabled"); } + + getClientVersion(): Promise { + throw Error("Execution engine disabled"); + } } diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index f5ec03f41626..4634557e658c 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -11,7 +11,7 @@ import { import {Metrics} from "../../metrics/index.js"; import {JobItemQueue} from "../../util/queue/index.js"; import {EPOCHS_PER_BATCH} from "../../sync/constants.js"; -import {numToQuantity} from "../../eth1/provider/utils.js"; +import {bytesToData, dataToBytes, numToQuantity} from "../../eth1/provider/utils.js"; import { ExecutionPayloadStatus, ExecutePayloadResponse, @@ -21,6 +21,8 @@ import { BlobsBundle, VersionedHashes, ExecutionEngineState, + ClientVersion, + ClientCode, } from "./interface.js"; import {PayloadIdCache} from "./payloadIdCache.js"; import { @@ -421,6 +423,22 @@ export class ExecutionEngineHttp implements IExecutionEngine { return this.state; } + async getClientVersion(clientVersion: ClientVersion): Promise { + const method = "engine_getClientVersionV1"; + const serializedClientVersion = {...clientVersion, commit: bytesToData(clientVersion.commit).slice(0, 4)} + + const response = await this.rpc.fetchWithRetries< + EngineApiRpcReturnTypes[typeof method], + EngineApiRpcParamTypes[typeof method] + >({method, params: [serializedClientVersion]}); + + return response.map((cv) => { + const commit = dataToBytes(cv.commit, 4); + const code = (cv.code in ClientCode) ? ClientCode[cv.code as keyof typeof ClientCode] : ClientCode.XX; + return {code, name: cv.name, version: cv.version, commit}; + }); + } + private updateEngineState(newState: ExecutionEngineState): void { const oldState = this.state; diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index e5f612fc0965..e39bd1a5b10c 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -38,6 +38,22 @@ export enum ExecutionEngineState { AUTH_FAILED = "AUTH_FAILED", } +export enum ClientCode { + BU = "besu", + EJ = "ethereumJS", + EG = "erigon", + GE = "go-ethereum", + GR = "grandine", + LH = "lighthouse", + LS = "lodestar", + NM = "nethermind", + NB = "nimbus", + TK = "teku", + PM = "prysm", + RH = "reth", + XX = "unknown", +} + export type ExecutePayloadResponse = | { status: ExecutionPayloadStatus.SYNCING | ExecutionPayloadStatus.ACCEPTED; @@ -80,6 +96,13 @@ export type BlobsBundle = { proofs: KZGProof[]; }; +export type ClientVersion = { + code: ClientCode, + name: string, + version: string, + commit: Uint8Array, +} + export type VersionedHashes = Uint8Array[]; /** @@ -148,4 +171,6 @@ export interface IExecutionEngine { getPayloadBodiesByRange(start: number, count: number): Promise<(ExecutionPayloadBody | null)[]>; getState(): ExecutionEngineState; + + getClientVersion(clientVersion: ClientVersion): Promise; } diff --git a/packages/beacon-node/src/execution/engine/types.ts b/packages/beacon-node/src/execution/engine/types.ts index 72a0100f7a51..c2005967ef6f 100644 --- a/packages/beacon-node/src/execution/engine/types.ts +++ b/packages/beacon-node/src/execution/engine/types.ts @@ -16,7 +16,7 @@ import { QUANTITY, quantityToBigint, } from "../../eth1/provider/utils.js"; -import {ExecutionPayloadStatus, BlobsBundle, PayloadAttributes, VersionedHashes} from "./interface.js"; +import {ExecutionPayloadStatus, BlobsBundle, PayloadAttributes, VersionedHashes, ClientVersion} from "./interface.js"; import {WithdrawalV1} from "./payloadIdCache.js"; /* eslint-disable @typescript-eslint/naming-convention */ @@ -62,6 +62,11 @@ export type EngineApiRpcParamTypes = { * 2. count: QUANTITY, 64 bits - Number of blocks to return */ engine_getPayloadBodiesByRangeV1: [start: QUANTITY, count: QUANTITY]; + + /** + * TODO Electra: add comment + */ + engine_getClientVersionV1: [ClientVersionRpc]; }; export type PayloadStatus = { @@ -100,6 +105,8 @@ export type EngineApiRpcReturnTypes = { engine_getPayloadBodiesByHashV1: (ExecutionPayloadBodyRpc | null)[]; engine_getPayloadBodiesByRangeV1: (ExecutionPayloadBodyRpc | null)[]; + + engine_getClientVersionV1: ClientVersionRpc[]; }; type ExecutionPayloadRpcWithValue = { @@ -157,6 +164,14 @@ export type PayloadAttributesRpc = { parentBeaconBlockRoot?: DATA; }; +// TODO Electra: add comment +export type ClientVersionRpc = { + code: DATA; + name: DATA; + version: DATA; + commit: DATA; +} + export interface BlobsBundleRpc { commitments: DATA[]; // each 48 bytes blobs: DATA[]; // each 4096 * 32 = 131072 bytes From e51c61824e9fac064da85c128b9797262289a093 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Sun, 2 Jun 2024 23:00:43 +0200 Subject: [PATCH 02/31] Default graffiti in beacon node --- packages/beacon-node/src/api/impl/api.ts | 2 +- .../src/api/impl/validator/index.ts | 74 ++++++++++++++++--- packages/beacon-node/src/api/options.ts | 2 + .../beacon-node/src/execution/engine/http.ts | 10 +-- .../src/execution/engine/interface.ts | 10 +-- .../beacon-node/src/execution/engine/mock.ts | 1 + .../beacon-node/src/execution/engine/types.ts | 17 +++-- packages/cli/src/cmds/beacon/handler.ts | 2 +- packages/cli/src/cmds/validator/handler.ts | 2 +- 9 files changed, 89 insertions(+), 31 deletions(-) diff --git a/packages/beacon-node/src/api/impl/api.ts b/packages/beacon-node/src/api/impl/api.ts index 6ec7180cf0f4..3647feed2d7c 100644 --- a/packages/beacon-node/src/api/impl/api.ts +++ b/packages/beacon-node/src/api/impl/api.ts @@ -21,6 +21,6 @@ export function getApi(opts: ApiOptions, modules: ApiModules): BeaconApiMethods lodestar: getLodestarApi(modules), node: getNodeApi(opts, modules), proof: getProofApi(opts, modules), - validator: getValidatorApi(modules), + validator: getValidatorApi(opts, modules), }; } diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 2c0958de5d5a..bc5420c839a5 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -61,6 +61,8 @@ import {getValidatorStatus} from "../beacon/state/utils.js"; import {validateGossipFnRetryUnknownRoot} from "../../../network/processor/gossipHandlers.js"; import {SCHEDULER_LOOKAHEAD_FACTOR} from "../../../chain/prepareNextSlot.js"; import {ChainEvent, CheckpointHex, CommonBlockBody} from "../../../chain/index.js"; +import {ClientCode} from "../../../execution/index.js"; +import {ApiOptions} from "../../options.js"; import {computeSubnetForCommitteesAtSlot, getPubkeysForIndices, selectBlockProductionSource} from "./utils.js"; /** @@ -127,6 +129,8 @@ export function getValidatorApi({ const MAX_API_CLOCK_DISPARITY_SEC = Math.min(0.5, config.SECONDS_PER_SLOT / 2); const MAX_API_CLOCK_DISPARITY_MS = MAX_API_CLOCK_DISPARITY_SEC * 1000; + const defaultGraffiti = getDefaultGraffiti(); + /** Compute and cache the genesis block root */ async function getGenesisBlockRoot(state: CachedBeaconStateAllForks): Promise { if (!genesisBlockRoot) { @@ -333,6 +337,56 @@ export function getValidatorApi({ ); } + async function getDefaultGraffiti(): Promise { + const consensusCode = ClientCode.LS; + const consensusCommit = opts.version ?? ""; + + const lodestarClientVersion = { + code: consensusCode, + name: "Lodestar", + version: opts.version ?? "", + commit: consensusCommit, + }; + + const executionClientVersions = await chain.executionEngine + .getClientVersion(lodestarClientVersion) + .catch((_) => []); + + if (executionClientVersions.length !== 0) { + const executionCode = executionClientVersions[0].code; + const executionCommit = executionClientVersions[0].commit; + + return `${executionCode}|${executionCommit.slice(0, 2)}|${consensusCode}|${consensusCommit.slice(0, 2)}`; + } + + return ""; + } + + async function getDefaultGraffiti(): Promise { + const consensusCode = ClientCode.LS; + const consensusCommit = opts.version ?? ""; + + const lodestarClientVersion = { + code: consensusCode, + name: "Lodestar", + version: opts.version ?? "", + commit: consensusCommit, + }; + + const executionClientVersions = await chain.executionEngine + .getClientVersion(lodestarClientVersion) + .catch((_) => []); + + if (executionClientVersions.length !== 0) { + const executionCode = executionClientVersions[0].code; + const executionCommit = executionClientVersions[0].commit; + + return `${executionCode}|${executionCommit.slice(0, 2)}|${consensusCode}|${consensusCommit.slice(0, 2)}`; + } + + return ""; + } + function notOnOutOfRangeData(beaconBlockRoot: Root): void { const protoBeaconBlock = chain.forkChoice.getBlock(beaconBlockRoot); if (!protoBeaconBlock) { @@ -404,7 +458,7 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti || ""), + graffiti: toGraffitiBuffer(graffiti || (await defaultGraffiti)), commonBlockBody, }); @@ -472,7 +526,7 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti || ""), + graffiti: toGraffitiBuffer(graffiti || (await defaultGraffiti)), feeRecipient, commonBlockBody, }); @@ -578,14 +632,14 @@ export function getValidatorApi({ builderBoostFactor: `${builderBoostFactor}`, }; - logger.verbose("Assembling block with produceEngineOrBuilderBlock", loggerContext); - const commonBlockBody = await chain.produceCommonBlockBody({ - slot, - parentBlockRoot, - randaoReveal, - graffiti: toGraffitiBuffer(graffiti || ""), - }); - logger.debug("Produced common block body", loggerContext); + logger.verbose("Assembling block with produceEngineOrBuilderBlock", loggerContext); + const commonBlockBody = await chain.produceCommonBlockBody({ + slot, + parentBlockRoot, + randaoReveal, + graffiti: toGraffitiBuffer(graffiti || (await defaultGraffiti)), + }); + logger.debug("Produced common block body", loggerContext); logger.verbose("Block production race (builder vs execution) starting", { ...loggerContext, diff --git a/packages/beacon-node/src/api/options.ts b/packages/beacon-node/src/api/options.ts index 4b9aed7e0d94..f794dd6c4386 100644 --- a/packages/beacon-node/src/api/options.ts +++ b/packages/beacon-node/src/api/options.ts @@ -4,10 +4,12 @@ export type ApiOptions = { maxGindicesInProof?: number; rest: BeaconRestApiServerOpts; version?: string; + commit?: string; }; export const defaultApiOptions: ApiOptions = { maxGindicesInProof: 512, rest: beaconRestApiServerOpts, version: "dev", + commit: "", }; diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 4634557e658c..c23f47349642 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -11,7 +11,7 @@ import { import {Metrics} from "../../metrics/index.js"; import {JobItemQueue} from "../../util/queue/index.js"; import {EPOCHS_PER_BATCH} from "../../sync/constants.js"; -import {bytesToData, dataToBytes, numToQuantity} from "../../eth1/provider/utils.js"; +import {numToQuantity} from "../../eth1/provider/utils.js"; import { ExecutionPayloadStatus, ExecutePayloadResponse, @@ -425,17 +425,15 @@ export class ExecutionEngineHttp implements IExecutionEngine { async getClientVersion(clientVersion: ClientVersion): Promise { const method = "engine_getClientVersionV1"; - const serializedClientVersion = {...clientVersion, commit: bytesToData(clientVersion.commit).slice(0, 4)} const response = await this.rpc.fetchWithRetries< EngineApiRpcReturnTypes[typeof method], EngineApiRpcParamTypes[typeof method] - >({method, params: [serializedClientVersion]}); + >({method, params: [clientVersion]}); return response.map((cv) => { - const commit = dataToBytes(cv.commit, 4); - const code = (cv.code in ClientCode) ? ClientCode[cv.code as keyof typeof ClientCode] : ClientCode.XX; - return {code, name: cv.name, version: cv.version, commit}; + const code = cv.code in ClientCode ? ClientCode[cv.code as keyof typeof ClientCode] : ClientCode.XX; + return {code, name: cv.name, version: cv.version, commit: cv.commit}; }); } diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index e39bd1a5b10c..b7daf6552dc2 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -97,11 +97,11 @@ export type BlobsBundle = { }; export type ClientVersion = { - code: ClientCode, - name: string, - version: string, - commit: Uint8Array, -} + code: ClientCode; + name: string; + version: string; + commit: string; +}; export type VersionedHashes = Uint8Array[]; diff --git a/packages/beacon-node/src/execution/engine/mock.ts b/packages/beacon-node/src/execution/engine/mock.ts index 5779713435a5..11d47735a7d0 100644 --- a/packages/beacon-node/src/execution/engine/mock.ts +++ b/packages/beacon-node/src/execution/engine/mock.ts @@ -96,6 +96,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend { engine_getPayloadV3: this.getPayload.bind(this), engine_getPayloadBodiesByHashV1: this.getPayloadBodiesByHash.bind(this), engine_getPayloadBodiesByRangeV1: this.getPayloadBodiesByRange.bind(this), + engine_getClientVersionV1: () => [], }; } diff --git a/packages/beacon-node/src/execution/engine/types.ts b/packages/beacon-node/src/execution/engine/types.ts index c2005967ef6f..94bf9d207fbd 100644 --- a/packages/beacon-node/src/execution/engine/types.ts +++ b/packages/beacon-node/src/execution/engine/types.ts @@ -16,7 +16,7 @@ import { QUANTITY, quantityToBigint, } from "../../eth1/provider/utils.js"; -import {ExecutionPayloadStatus, BlobsBundle, PayloadAttributes, VersionedHashes, ClientVersion} from "./interface.js"; +import {ExecutionPayloadStatus, BlobsBundle, PayloadAttributes, VersionedHashes} from "./interface.js"; import {WithdrawalV1} from "./payloadIdCache.js"; /* eslint-disable @typescript-eslint/naming-convention */ @@ -64,7 +64,7 @@ export type EngineApiRpcParamTypes = { engine_getPayloadBodiesByRangeV1: [start: QUANTITY, count: QUANTITY]; /** - * TODO Electra: add comment + * Object - Instance of ClientVersion */ engine_getClientVersionV1: [ClientVersionRpc]; }; @@ -164,13 +164,16 @@ export type PayloadAttributesRpc = { parentBeaconBlockRoot?: DATA; }; -// TODO Electra: add comment export type ClientVersionRpc = { - code: DATA; - name: DATA; - version: DATA; + /** ClientCode */ + code: string; + /** string, Human-readable name of the client */ + name: string; + /** string, the version string of the current implementation */ + version: string; + /** DATA, 4 bytes - first four bytes of the latest commit hash of this Lodestar build */ commit: DATA; -} +}; export interface BlobsBundleRpc { commitments: DATA[]; // each 48 bytes diff --git a/packages/cli/src/cmds/beacon/handler.ts b/packages/cli/src/cmds/beacon/handler.ts index e24089194185..6c19bb250f5d 100644 --- a/packages/cli/src/cmds/beacon/handler.ts +++ b/packages/cli/src/cmds/beacon/handler.ts @@ -182,7 +182,7 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { beaconNodeOptions.set({metrics: {metadata: {version, commit, network}}}); beaconNodeOptions.set({metrics: {validatorMonitorLogs: args.validatorMonitorLogs}}); // Add detailed version string for API node/version endpoint - beaconNodeOptions.set({api: {version}}); + beaconNodeOptions.set({api: {version, commit}}); // Combine bootnodes from different sources const bootnodes = (beaconNodeOptions.get().network?.discv5?.bootEnrs ?? []).concat( diff --git a/packages/cli/src/cmds/validator/handler.ts b/packages/cli/src/cmds/validator/handler.ts index fdd93f1ebfc9..17e2b056f105 100644 --- a/packages/cli/src/cmds/validator/handler.ts +++ b/packages/cli/src/cmds/validator/handler.ts @@ -226,7 +226,7 @@ function getProposerConfigFromArgs( }: {persistedKeysBackend: IPersistedKeysBackend; accountPaths: {proposerDir: string}} ): ValidatorProposerConfig { const defaultConfig = { - graffiti: args.graffiti ?? getDefaultGraffiti(), + graffiti: args.graffiti ?? "", strictFeeRecipientCheck: args.strictFeeRecipientCheck, feeRecipient: args.suggestedFeeRecipient ? parseFeeRecipient(args.suggestedFeeRecipient) : undefined, builder: { From 59fc851d2e86d2dfa25196faae89993ec95df3db Mon Sep 17 00:00:00 2001 From: NC Date: Mon, 10 Jun 2024 15:50:18 +0300 Subject: [PATCH 03/31] Update packages/beacon-node/src/api/impl/validator/index.ts 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 bc5420c839a5..55819af781a2 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -458,7 +458,7 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti || (await defaultGraffiti)), + graffiti: toGraffitiBuffer(graffiti ?? (await defaultGraffiti)), commonBlockBody, }); From 1780cf441489e338eece45cb3c208d7b7e87edff Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Fri, 21 Jun 2024 18:25:35 +0300 Subject: [PATCH 04/31] Fix rebase --- .../src/api/impl/validator/index.ts | 31 ++++--------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 55819af781a2..c49bb4c198a2 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -110,7 +110,7 @@ type ProduceFullOrBlindedBlockOrContentsRes = {executionPayloadSource: ProducedB * Server implementation for handling validator duties. * See `@lodestar/validator/src/api` for the client implementation). */ -export function getValidatorApi({ +export function getValidatorApi(opts: ApiOptions, { chain, config, logger, @@ -129,7 +129,9 @@ export function getValidatorApi({ const MAX_API_CLOCK_DISPARITY_SEC = Math.min(0.5, config.SECONDS_PER_SLOT / 2); const MAX_API_CLOCK_DISPARITY_MS = MAX_API_CLOCK_DISPARITY_SEC * 1000; - const defaultGraffiti = getDefaultGraffiti(); + let defaultGraffiti: string; + updateDefaultGraffiti(); + chain.emitter.on(routes.events.EventType.payloadAttributes, updateDefaultGraffiti); /** Compute and cache the genesis block root */ async function getGenesisBlockRoot(state: CachedBeaconStateAllForks): Promise { @@ -337,29 +339,8 @@ export function getValidatorApi({ ); } - async function getDefaultGraffiti(): Promise { - const consensusCode = ClientCode.LS; - const consensusCommit = opts.version ?? ""; - - const lodestarClientVersion = { - code: consensusCode, - name: "Lodestar", - version: opts.version ?? "", - commit: consensusCommit, - }; - - const executionClientVersions = await chain.executionEngine - .getClientVersion(lodestarClientVersion) - .catch((_) => []); - - if (executionClientVersions.length !== 0) { - const executionCode = executionClientVersions[0].code; - const executionCommit = executionClientVersions[0].commit; - - return `${executionCode}|${executionCommit.slice(0, 2)}|${consensusCode}|${consensusCommit.slice(0, 2)}`; - } - - return ""; + async function updateDefaultGraffiti(): Promise { + defaultGraffiti = await getDefaultGraffiti(); } async function getDefaultGraffiti(): Promise { From 5b3f39877ba39b955ad919f48864ed5b85e5de45 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Mon, 24 Jun 2024 15:32:19 +0300 Subject: [PATCH 05/31] Make graffiti optional in validator store --- packages/api/src/beacon/routes/validator.ts | 16 +++---- packages/api/src/utils/serdes.ts | 11 ++++- .../src/api/impl/validator/index.ts | 45 +++++++++---------- .../impl/validator/duties/proposer.test.ts | 3 +- .../validator/produceAttestationData.test.ts | 3 +- .../api/impl/validator/produceBlockV2.test.ts | 3 +- .../api/impl/validator/produceBlockV3.test.ts | 3 +- packages/cli/src/cmds/validator/handler.ts | 4 +- .../cli/src/cmds/validator/keymanager/impl.ts | 7 ++- packages/cli/src/util/graffiti.ts | 18 -------- .../test/unit/webEsmBundle.browser.test.ts | 2 +- packages/validator/src/services/block.ts | 4 +- .../validator/src/services/validatorStore.ts | 6 +-- 13 files changed, 60 insertions(+), 65 deletions(-) delete mode 100644 packages/cli/src/util/graffiti.ts diff --git a/packages/api/src/beacon/routes/validator.ts b/packages/api/src/beacon/routes/validator.ts index 7f704edd542a..3d9cfbb2ba51 100644 --- a/packages/api/src/beacon/routes/validator.ts +++ b/packages/api/src/beacon/routes/validator.ts @@ -302,9 +302,9 @@ export type Endpoints = { /** The validator's randao reveal value */ randaoReveal: BLSSignature; /** Arbitrary data validator wants to include in block */ - graffiti: string; + graffiti?: string; }, - {params: {slot: number}; query: {randao_reveal: string; graffiti: string}}, + {params: {slot: number}; query: {randao_reveal: string; graffiti?: string}}, allForks.BeaconBlock, VersionMeta >; @@ -322,13 +322,13 @@ export type Endpoints = { /** The validator's randao reveal value */ randaoReveal: BLSSignature; /** Arbitrary data validator wants to include in block */ - graffiti: string; + graffiti?: string; } & Omit, { params: {slot: number}; query: { randao_reveal: string; - graffiti: string; + graffiti?: string; fee_recipient?: string; builder_selection?: string; strict_fee_recipient_check?: boolean; @@ -351,7 +351,7 @@ export type Endpoints = { /** The validator's randao reveal value */ randaoReveal: BLSSignature; /** Arbitrary data validator wants to include in block */ - graffiti: string; + graffiti?: string; skipRandaoVerification?: boolean; builderBoostFactor?: UintBn64; } & ExtraProduceBlockOpts, @@ -359,7 +359,7 @@ export type Endpoints = { params: {slot: number}; query: { randao_reveal: string; - graffiti: string; + graffiti?: string; skip_randao_verification?: string; fee_recipient?: string; builder_selection?: string; @@ -377,9 +377,9 @@ export type Endpoints = { { slot: Slot; randaoReveal: BLSSignature; - graffiti: string; + graffiti?: string; }, - {params: {slot: number}; query: {randao_reveal: string; graffiti: string}}, + {params: {slot: number}; query: {randao_reveal: string; graffiti?: string}}, allForks.BlindedBeaconBlock, VersionMeta >; diff --git a/packages/api/src/utils/serdes.ts b/packages/api/src/utils/serdes.ts index 73196c917a66..468b4a468d55 100644 --- a/packages/api/src/utils/serdes.ts +++ b/packages/api/src/utils/serdes.ts @@ -77,7 +77,11 @@ export function fromValidatorIdsStr(ids?: string[]): (string | number)[] | undef const GRAFFITI_HEX_LENGTH = 66; -export function toGraffitiHex(utf8: string): string { +export function toGraffitiHex(utf8?: string): string | undefined { + if (utf8 === undefined) { + return undefined; + } + const hex = toHexString(new TextEncoder().encode(utf8)); if (hex.length > GRAFFITI_HEX_LENGTH) { @@ -93,8 +97,11 @@ export function toGraffitiHex(utf8: string): string { return hex; } -export function fromGraffitiHex(hex: string): string { +export function fromGraffitiHex(hex?: string): string | undefined { try { + if (hex === undefined) { + return undefined; + } return new TextDecoder("utf8").decode(fromHexString(hex)); } catch { // allow malformed graffiti hex string diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index c49bb4c198a2..b88f6eb71751 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -63,6 +63,7 @@ import {SCHEDULER_LOOKAHEAD_FACTOR} from "../../../chain/prepareNextSlot.js"; import {ChainEvent, CheckpointHex, CommonBlockBody} from "../../../chain/index.js"; import {ClientCode} from "../../../execution/index.js"; import {ApiOptions} from "../../options.js"; +import {ClockEvent} from "../../../util/clock.js"; import {computeSubnetForCommitteesAtSlot, getPubkeysForIndices, selectBlockProductionSource} from "./utils.js"; /** @@ -110,14 +111,10 @@ type ProduceFullOrBlindedBlockOrContentsRes = {executionPayloadSource: ProducedB * Server implementation for handling validator duties. * See `@lodestar/validator/src/api` for the client implementation). */ -export function getValidatorApi(opts: ApiOptions, { - chain, - config, - logger, - metrics, - network, - sync, -}: ApiModules): ApplicationMethods { +export function getValidatorApi( + opts: ApiOptions, + {chain, config, logger, metrics, network, sync}: ApiModules +): ApplicationMethods { let genesisBlockRoot: Root | null = null; /** @@ -129,9 +126,9 @@ export function getValidatorApi(opts: ApiOptions, { const MAX_API_CLOCK_DISPARITY_SEC = Math.min(0.5, config.SECONDS_PER_SLOT / 2); const MAX_API_CLOCK_DISPARITY_MS = MAX_API_CLOCK_DISPARITY_SEC * 1000; - let defaultGraffiti: string; - updateDefaultGraffiti(); - chain.emitter.on(routes.events.EventType.payloadAttributes, updateDefaultGraffiti); + let defaultGraffiti: string = ""; + void updateDefaultGraffiti(); + chain.clock.on(ClockEvent.epoch, updateDefaultGraffiti); /** Compute and cache the genesis block root */ async function getGenesisBlockRoot(state: CachedBeaconStateAllForks): Promise { @@ -381,7 +378,7 @@ export function getValidatorApi(opts: ApiOptions, { async function produceBuilderBlindedBlock( slot: Slot, randaoReveal: BLSSignature, - graffiti: string, + graffiti?: string, // as of now fee recipient checks can not be performed because builder does not return bid recipient { skipHeadChecksAndUpdate, @@ -439,7 +436,7 @@ export function getValidatorApi(opts: ApiOptions, { slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? (await defaultGraffiti)), + graffiti: toGraffitiBuffer(graffiti ?? defaultGraffiti), commonBlockBody, }); @@ -465,7 +462,7 @@ export function getValidatorApi(opts: ApiOptions, { async function produceEngineFullBlockOrContents( slot: Slot, randaoReveal: BLSSignature, - graffiti: string, + graffiti?: string, { feeRecipient, strictFeeRecipientCheck, @@ -507,7 +504,7 @@ export function getValidatorApi(opts: ApiOptions, { slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti || (await defaultGraffiti)), + graffiti: toGraffitiBuffer(graffiti ?? defaultGraffiti), feeRecipient, commonBlockBody, }); @@ -555,7 +552,7 @@ export function getValidatorApi(opts: ApiOptions, { async function produceEngineOrBuilderBlock( slot: Slot, randaoReveal: BLSSignature, - graffiti: string, + graffiti?: string, // TODO deneb: skip randao verification _skipRandaoVerification?: boolean, builderBoostFactor?: bigint, @@ -613,14 +610,14 @@ export function getValidatorApi(opts: ApiOptions, { builderBoostFactor: `${builderBoostFactor}`, }; - logger.verbose("Assembling block with produceEngineOrBuilderBlock", loggerContext); - const commonBlockBody = await chain.produceCommonBlockBody({ - slot, - parentBlockRoot, - randaoReveal, - graffiti: toGraffitiBuffer(graffiti || (await defaultGraffiti)), - }); - logger.debug("Produced common block body", loggerContext); + logger.verbose("Assembling block with produceEngineOrBuilderBlock", loggerContext); + const commonBlockBody = await chain.produceCommonBlockBody({ + slot, + parentBlockRoot, + randaoReveal, + graffiti: toGraffitiBuffer(graffiti ?? defaultGraffiti), + }); + logger.debug("Produced common block body", loggerContext); logger.verbose("Block production race (builder vs execution) starting", { ...loggerContext, diff --git a/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts b/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts index d4f7705fb25b..b101382e01a0 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts @@ -10,6 +10,7 @@ import {generateState, zeroProtoBlock} from "../../../../../utils/state.js"; import {generateValidators} from "../../../../../utils/validator.js"; import {createCachedBeaconStateTest} from "../../../../../utils/cachedBeaconState.js"; import {SyncState} from "../../../../../../src/sync/interface.js"; +import {defaultApiOptions} from "../../../../../../src/api/options.js"; describe("get proposers api impl", function () { let api: ReturnType; @@ -20,7 +21,7 @@ describe("get proposers api impl", function () { beforeEach(function () { vi.useFakeTimers({now: 0}); modules = getApiTestModules({clock: "real"}); - api = getValidatorApi(modules); + api = getValidatorApi(defaultApiOptions, modules); state = generateState( { diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts index 256d772d5fc0..84872ca6045c 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts @@ -3,6 +3,7 @@ import {ProtoBlock} from "@lodestar/fork-choice"; import {SyncState} from "../../../../../src/sync/interface.js"; import {ApiTestModules, getApiTestModules} from "../../../../utils/api.js"; import {getValidatorApi} from "../../../../../src/api/impl/validator/index.js"; +import {defaultApiOptions} from "../../../../../src/api/options.js"; describe("api - validator - produceAttestationData", function () { let modules: ApiTestModules; @@ -10,7 +11,7 @@ describe("api - validator - produceAttestationData", function () { beforeEach(function () { modules = getApiTestModules(); - api = getValidatorApi(modules); + api = getValidatorApi(defaultApiOptions, modules); }); it("Should throw when node is not synced", async function () { diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts index 0868834e619e..a23373938f64 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts @@ -14,6 +14,7 @@ import {toGraffitiBuffer} from "../../../../../src/util/graffiti.js"; import {BlockType, produceBlockBody} from "../../../../../src/chain/produceBlock/produceBlockBody.js"; import {generateProtoBlock} from "../../../../utils/typeGenerator.js"; import {ZERO_HASH_HEX} from "../../../../../src/constants/index.js"; +import {defaultApiOptions} from "../../../../../src/api/options.js"; describe("api/validator - produceBlockV2", function () { let api: ReturnType; @@ -22,7 +23,7 @@ describe("api/validator - produceBlockV2", function () { beforeEach(() => { modules = getApiTestModules(); - api = getValidatorApi(modules); + api = getValidatorApi(defaultApiOptions, modules); state = generateCachedBellatrixState(); }); diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts index 851bc9d0f33b..a7299aa3b956 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts @@ -10,6 +10,7 @@ import {SyncState} from "../../../../../src/sync/interface.js"; import {getValidatorApi} from "../../../../../src/api/impl/validator/index.js"; import {CommonBlockBody} from "../../../../../src/chain/interface.js"; import {zeroProtoBlock} from "../../../../utils/state.js"; +import {defaultApiOptions} from "../../../../../src/api/options.js"; /* eslint-disable @typescript-eslint/naming-convention */ describe("api/validator - produceBlockV3", function () { @@ -26,7 +27,7 @@ describe("api/validator - produceBlockV3", function () { beforeEach(() => { modules = getApiTestModules(); - api = getValidatorApi({...modules, config}); + api = getValidatorApi(defaultApiOptions, {...modules, config}); modules.chain.executionBuilder.status = true; }); diff --git a/packages/cli/src/cmds/validator/handler.ts b/packages/cli/src/cmds/validator/handler.ts index 17e2b056f105..01c15126b796 100644 --- a/packages/cli/src/cmds/validator/handler.ts +++ b/packages/cli/src/cmds/validator/handler.ts @@ -19,7 +19,7 @@ import { import {getNodeLogger} from "@lodestar/logger/node"; import {getBeaconConfigFromArgs} from "../../config/index.js"; import {GlobalArgs} from "../../options/index.js"; -import {YargsError, cleanOldLogFiles, getDefaultGraffiti, mkdir, parseLoggerArgs} from "../../util/index.js"; +import {YargsError, cleanOldLogFiles, mkdir, parseLoggerArgs} from "../../util/index.js"; import {onGracefulShutdown, parseFeeRecipient, parseProposerConfig} from "../../util/index.js"; import {getVersionData} from "../../util/version.js"; import {parseBuilderSelection, parseBuilderBoostFactor} from "../../util/proposerConfig.js"; @@ -226,7 +226,7 @@ function getProposerConfigFromArgs( }: {persistedKeysBackend: IPersistedKeysBackend; accountPaths: {proposerDir: string}} ): ValidatorProposerConfig { const defaultConfig = { - graffiti: args.graffiti ?? "", + graffiti: args.graffiti, strictFeeRecipientCheck: args.strictFeeRecipientCheck, feeRecipient: args.suggestedFeeRecipient ? parseFeeRecipient(args.suggestedFeeRecipient) : undefined, builder: { diff --git a/packages/cli/src/cmds/validator/keymanager/impl.ts b/packages/cli/src/cmds/validator/keymanager/impl.ts index 36ded66976b1..b9aca929e1c3 100644 --- a/packages/cli/src/cmds/validator/keymanager/impl.ts +++ b/packages/cli/src/cmds/validator/keymanager/impl.ts @@ -60,7 +60,12 @@ export class KeymanagerApi implements Api { } async getGraffiti({pubkey}: {pubkey: PubkeyHex}): ReturnType { - return {data: {pubkey, graffiti: this.validator.validatorStore.getGraffiti(pubkey)}}; + const graffiti = this.validator.validatorStore.getGraffiti(pubkey); + if (graffiti === undefined) { + throw new ApiError(500, `No graffiti for pubkey ${pubkey}`); + } else { + return {data: {pubkey, graffiti}}; + } } async setGraffiti({pubkey, graffiti}: GraffitiData): ReturnType { diff --git a/packages/cli/src/util/graffiti.ts b/packages/cli/src/util/graffiti.ts deleted file mode 100644 index 1d859ea8d9b8..000000000000 --- a/packages/cli/src/util/graffiti.ts +++ /dev/null @@ -1,18 +0,0 @@ -import {getVersionData} from "./version.js"; - -const lodestarPackageName = "Lodestar"; - -/** - * Computes a default graffiti fetching dynamically the package info. - * @returns a string containing package name and version. - */ -export function getDefaultGraffiti(): string { - try { - const {version} = getVersionData(); - return `${lodestarPackageName}-${version}`; - } catch (e) { - // eslint-disable-next-line no-console - console.error("Error guessing lodestar version", e as Error); - return lodestarPackageName; - } -} diff --git a/packages/light-client/test/unit/webEsmBundle.browser.test.ts b/packages/light-client/test/unit/webEsmBundle.browser.test.ts index defc421d7071..8dc3f452c79d 100644 --- a/packages/light-client/test/unit/webEsmBundle.browser.test.ts +++ b/packages/light-client/test/unit/webEsmBundle.browser.test.ts @@ -1,4 +1,4 @@ -/* eslint-disable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-call */ +/* eslint-disable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access */ import {expect, describe, it, vi, beforeAll} from "vitest"; import {sleep} from "@lodestar/utils"; import {Lightclient, LightclientEvent, utils, transport} from "../../dist/lightclient.min.mjs"; diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index ab855e264447..d6cd385ad610 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -198,7 +198,7 @@ export class BlockProposingService { _config: ChainForkConfig, slot: Slot, randaoReveal: BLSSignature, - graffiti: string, + graffiti: string | undefined, builderBoostFactor: bigint, {feeRecipient, strictFeeRecipientCheck, blindedLocal}: routes.validator.ExtraProduceBlockOpts, builderSelection: routes.validator.BuilderSelection @@ -236,7 +236,7 @@ export class BlockProposingService { config: ChainForkConfig, slot: Slot, randaoReveal: BLSSignature, - graffiti: string, + graffiti: string | undefined, _builderBoostFactor: bigint, _opts: routes.validator.ExtraProduceBlockOpts, builderSelection: routes.validator.BuilderSelection diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 6cd9ed8dc065..5f063138b851 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -63,7 +63,7 @@ export type SignerRemote = { }; type DefaultProposerConfig = { - graffiti: string; + graffiti?: string; strictFeeRecipientCheck: boolean; feeRecipient: Eth1Address; builder: { @@ -164,7 +164,7 @@ export class ValidatorStore { } this.defaultProposerConfig = { - graffiti: defaultConfig.graffiti ?? "", + graffiti: defaultConfig.graffiti, strictFeeRecipientCheck: defaultConfig.strictFeeRecipientCheck ?? false, feeRecipient: defaultConfig.feeRecipient ?? defaultOptions.suggestedFeeRecipient, builder: { @@ -241,7 +241,7 @@ export class ValidatorStore { delete validatorData["feeRecipient"]; } - getGraffiti(pubkeyHex: PubkeyHex): string { + getGraffiti(pubkeyHex: PubkeyHex): string | undefined { return this.validators.get(pubkeyHex)?.graffiti ?? this.defaultProposerConfig.graffiti; } From 1cb3c0c0f9a4211cf2fb87058f54692dd64a9b50 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Mon, 24 Jun 2024 15:52:09 +0300 Subject: [PATCH 06/31] Fix merge --- packages/cli/src/util/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/util/index.ts b/packages/cli/src/util/index.ts index 3d94977f5fb7..579587317e02 100644 --- a/packages/cli/src/util/index.ts +++ b/packages/cli/src/util/index.ts @@ -4,7 +4,6 @@ export * from "./file.js"; export * from "./format.js"; export * from "./fs.js"; export * from "./gitData/index.js"; -export * from "./graffiti.js"; export * from "./logger.js"; export * from "./object.js"; export * from "./passphrase.js"; From 65385b90ff9ee5968443214152fb9f80b6709bfb Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Mon, 24 Jun 2024 16:28:50 +0300 Subject: [PATCH 07/31] Fix lint --- packages/light-client/test/unit/webEsmBundle.browser.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/light-client/test/unit/webEsmBundle.browser.test.ts b/packages/light-client/test/unit/webEsmBundle.browser.test.ts index 8dc3f452c79d..defc421d7071 100644 --- a/packages/light-client/test/unit/webEsmBundle.browser.test.ts +++ b/packages/light-client/test/unit/webEsmBundle.browser.test.ts @@ -1,4 +1,4 @@ -/* eslint-disable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access */ +/* eslint-disable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-call */ import {expect, describe, it, vi, beforeAll} from "vitest"; import {sleep} from "@lodestar/utils"; import {Lightclient, LightclientEvent, utils, transport} from "../../dist/lightclient.min.mjs"; From 2f0706060fbe5818b1b62921d11b3da03d97ea26 Mon Sep 17 00:00:00 2001 From: NC Date: Mon, 24 Jun 2024 17:17:32 +0300 Subject: [PATCH 08/31] Update packages/beacon-node/src/execution/engine/types.ts Co-authored-by: Cayman --- packages/beacon-node/src/execution/engine/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/execution/engine/types.ts b/packages/beacon-node/src/execution/engine/types.ts index 56a21c2522be..85f514c953b0 100644 --- a/packages/beacon-node/src/execution/engine/types.ts +++ b/packages/beacon-node/src/execution/engine/types.ts @@ -171,7 +171,7 @@ export type ClientVersionRpc = { name: string; /** string, the version string of the current implementation */ version: string; - /** DATA, 4 bytes - first four bytes of the latest commit hash of this Lodestar build */ + /** DATA, 4 bytes - first four bytes of the latest commit hash of this build */ commit: DATA; }; From 0da00204f2fe6284a0876442a3f0d45c6b2bf100 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Mon, 24 Jun 2024 17:28:49 +0300 Subject: [PATCH 09/31] Add fallback graffiti --- packages/beacon-node/src/api/impl/validator/index.ts | 3 ++- packages/beacon-node/src/execution/engine/interface.ts | 4 ++++ 2 files changed, 6 insertions(+), 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 09cd07c99a40..394b355ecbff 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -364,7 +364,8 @@ export function getValidatorApi( return `${executionCode}|${executionCommit.slice(0, 2)}|${consensusCode}|${consensusCommit.slice(0, 2)}`; } - return ""; + // No EL client info available. We still want to include CL info albeit not spec compliant + return `${consensusCode}|${consensusCommit.slice(0, 2)}`; } function notOnOutOfRangeData(beaconBlockRoot: Root): void { diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index 273527c04c0c..1558e4d1b675 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -38,6 +38,10 @@ export enum ExecutionEngineState { AUTH_FAILED = "AUTH_FAILED", } +/** + * Client code as defined in https://github.com/ethereum/execution-apis/blob/v1.0.0-beta.4/src/engine/identification.md#clientcode + * ClientCode.XX is dedicated to other clients which do not have their own code + */ export enum ClientCode { BU = "besu", EJ = "ethereumJS", From 8cebfae194713f7fa04d64781c53064a726b8ca4 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Mon, 24 Jun 2024 20:48:25 +0300 Subject: [PATCH 10/31] Address comment --- packages/beacon-node/src/api/impl/validator/index.ts | 3 +++ packages/beacon-node/test/mocks/mockedBeaconChain.ts | 1 + packages/cli/src/cmds/validator/keymanager/impl.ts | 5 ++--- packages/cli/src/cmds/validator/options.ts | 1 - 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 394b355ecbff..34380350104f 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -340,6 +340,7 @@ export function getValidatorApi( async function updateDefaultGraffiti(): Promise { defaultGraffiti = await getDefaultGraffiti(); + logger.debug(`default graffiti is set to ${defaultGraffiti}`); } async function getDefaultGraffiti(): Promise { @@ -357,6 +358,8 @@ export function getValidatorApi( .getClientVersion(lodestarClientVersion) .catch((_) => []); + logger.debug(`Receive executionClientVersions ${executionClientVersions}`); + if (executionClientVersions.length !== 0) { const executionCode = executionClientVersions[0].code; const executionCommit = executionClientVersions[0].commit; diff --git a/packages/beacon-node/test/mocks/mockedBeaconChain.ts b/packages/beacon-node/test/mocks/mockedBeaconChain.ts index b63eb11435ca..22a48ebd050a 100644 --- a/packages/beacon-node/test/mocks/mockedBeaconChain.ts +++ b/packages/beacon-node/test/mocks/mockedBeaconChain.ts @@ -117,6 +117,7 @@ vi.mock("../../src/chain/chain.js", async (importActual) => { executionEngine: { notifyForkchoiceUpdate: vi.fn(), getPayload: vi.fn(), + getClientVersion: vi.fn(), }, executionBuilder: {}, // eslint-disable-next-line @typescript-eslint/ban-ts-comment diff --git a/packages/cli/src/cmds/validator/keymanager/impl.ts b/packages/cli/src/cmds/validator/keymanager/impl.ts index b9aca929e1c3..113b3a1a67f0 100644 --- a/packages/cli/src/cmds/validator/keymanager/impl.ts +++ b/packages/cli/src/cmds/validator/keymanager/impl.ts @@ -62,10 +62,9 @@ export class KeymanagerApi implements Api { async getGraffiti({pubkey}: {pubkey: PubkeyHex}): ReturnType { const graffiti = this.validator.validatorStore.getGraffiti(pubkey); if (graffiti === undefined) { - throw new ApiError(500, `No graffiti for pubkey ${pubkey}`); - } else { - return {data: {pubkey, graffiti}}; + throw new ApiError(404, `No graffiti for pubkey ${pubkey}`); } + return {data: {pubkey, graffiti}}; } async setGraffiti({pubkey, graffiti}: GraffitiData): ReturnType { diff --git a/packages/cli/src/cmds/validator/options.ts b/packages/cli/src/cmds/validator/options.ts index 08548edb1072..7a92929fbd3a 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/packages/cli/src/cmds/validator/options.ts @@ -199,7 +199,6 @@ export const validatorOptions: CliCommandOptions = { graffiti: { description: "Specify your custom graffiti to be included in blocks (plain UTF8 text, 32 characters max)", - // Don't use a default here since it should be computed only if necessary by getDefaultGraffiti() type: "string", }, From 5afa36ebc5af3afd72c525e5bb19ee293173af30 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Mon, 24 Jun 2024 21:30:07 +0300 Subject: [PATCH 11/31] Address comment --- packages/api/src/utils/serdes.ts | 6 +++--- packages/beacon-node/src/api/impl/validator/index.ts | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/api/src/utils/serdes.ts b/packages/api/src/utils/serdes.ts index 468b4a468d55..233d7db9e7f8 100644 --- a/packages/api/src/utils/serdes.ts +++ b/packages/api/src/utils/serdes.ts @@ -98,10 +98,10 @@ export function toGraffitiHex(utf8?: string): string | undefined { } export function fromGraffitiHex(hex?: string): string | undefined { + if (hex === undefined) { + return undefined; + } try { - if (hex === undefined) { - return undefined; - } return new TextDecoder("utf8").decode(fromHexString(hex)); } catch { // allow malformed graffiti hex string diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 34380350104f..8274828bba02 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -364,7 +364,8 @@ export function getValidatorApi( const executionCode = executionClientVersions[0].code; const executionCommit = executionClientVersions[0].commit; - return `${executionCode}|${executionCommit.slice(0, 2)}|${consensusCode}|${consensusCommit.slice(0, 2)}`; + // Follow the 2-byte commit format in https://github.com/ethereum/execution-apis/pull/517#issuecomment-1918512560 + return `${executionCode}${executionCommit.slice(0, 2)}${consensusCode}${consensusCommit.slice(0, 2)}`; } // No EL client info available. We still want to include CL info albeit not spec compliant From 467de59596cb7af75b285b652bc92ea57f52fd70 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Wed, 26 Jun 2024 23:07:20 +0300 Subject: [PATCH 12/31] Cache client version in ExecutionEngine --- packages/beacon-node/src/api/impl/api.ts | 2 +- .../src/api/impl/validator/index.ts | 59 ++++++------------- packages/beacon-node/src/api/options.ts | 2 - .../beacon-node/src/chain/prepareNextSlot.ts | 4 ++ .../src/execution/engine/disabled.ts | 10 +++- .../beacon-node/src/execution/engine/http.ts | 40 +++++++++++-- .../beacon-node/src/execution/engine/index.ts | 2 +- .../src/execution/engine/interface.ts | 10 +++- .../impl/validator/duties/proposer.test.ts | 3 +- .../validator/produceAttestationData.test.ts | 3 +- .../api/impl/validator/produceBlockV2.test.ts | 3 +- .../api/impl/validator/produceBlockV3.test.ts | 3 +- packages/cli/src/cmds/beacon/handler.ts | 4 ++ 13 files changed, 86 insertions(+), 59 deletions(-) diff --git a/packages/beacon-node/src/api/impl/api.ts b/packages/beacon-node/src/api/impl/api.ts index 3647feed2d7c..6ec7180cf0f4 100644 --- a/packages/beacon-node/src/api/impl/api.ts +++ b/packages/beacon-node/src/api/impl/api.ts @@ -21,6 +21,6 @@ export function getApi(opts: ApiOptions, modules: ApiModules): BeaconApiMethods lodestar: getLodestarApi(modules), node: getNodeApi(opts, modules), proof: getProofApi(opts, modules), - validator: getValidatorApi(opts, modules), + validator: getValidatorApi(modules), }; } diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 8274828bba02..23d8d03f8c71 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -63,9 +63,6 @@ import {getValidatorStatus} from "../beacon/state/utils.js"; import {validateGossipFnRetryUnknownRoot} from "../../../network/processor/gossipHandlers.js"; import {SCHEDULER_LOOKAHEAD_FACTOR} from "../../../chain/prepareNextSlot.js"; import {ChainEvent, CheckpointHex, CommonBlockBody} from "../../../chain/index.js"; -import {ClientCode} from "../../../execution/index.js"; -import {ApiOptions} from "../../options.js"; -import {ClockEvent} from "../../../util/clock.js"; import {computeSubnetForCommitteesAtSlot, getPubkeysForIndices, selectBlockProductionSource} from "./utils.js"; /** @@ -113,10 +110,14 @@ type ProduceFullOrBlindedBlockOrContentsRes = {executionPayloadSource: ProducedB * Server implementation for handling validator duties. * See `@lodestar/validator/src/api` for the client implementation). */ -export function getValidatorApi( - opts: ApiOptions, - {chain, config, logger, metrics, network, sync}: ApiModules -): ApplicationMethods { +export function getValidatorApi({ + chain, + config, + logger, + metrics, + network, + sync, +}: ApiModules): ApplicationMethods { let genesisBlockRoot: Root | null = null; /** @@ -128,10 +129,6 @@ export function getValidatorApi( const MAX_API_CLOCK_DISPARITY_SEC = Math.min(0.5, config.SECONDS_PER_SLOT / 2); const MAX_API_CLOCK_DISPARITY_MS = MAX_API_CLOCK_DISPARITY_SEC * 1000; - let defaultGraffiti: string = ""; - void updateDefaultGraffiti(); - chain.clock.on(ClockEvent.epoch, updateDefaultGraffiti); - /** Compute and cache the genesis block root */ async function getGenesisBlockRoot(state: CachedBeaconStateAllForks): Promise { if (!genesisBlockRoot) { @@ -338,38 +335,20 @@ export function getValidatorApi( ); } - async function updateDefaultGraffiti(): Promise { - defaultGraffiti = await getDefaultGraffiti(); - logger.debug(`default graffiti is set to ${defaultGraffiti}`); - } - - async function getDefaultGraffiti(): Promise { - const consensusCode = ClientCode.LS; - const consensusCommit = opts.version ?? ""; - - const lodestarClientVersion = { - code: consensusCode, - name: "Lodestar", - version: opts.version ?? "", - commit: consensusCommit, - }; - - const executionClientVersions = await chain.executionEngine - .getClientVersion(lodestarClientVersion) - .catch((_) => []); - - logger.debug(`Receive executionClientVersions ${executionClientVersions}`); + function getDefaultGraffiti(): string { + const executionClientVersion = chain.executionEngine.getExecutionClientVersion(); + const consensusClientVersion = chain.executionEngine.getConsensusClientVersion(); - if (executionClientVersions.length !== 0) { - const executionCode = executionClientVersions[0].code; - const executionCommit = executionClientVersions[0].commit; + if (executionClientVersion.length > 0) { + const executionCode = executionClientVersion[0].code; + const executionCommit = executionClientVersion[0].commit; // Follow the 2-byte commit format in https://github.com/ethereum/execution-apis/pull/517#issuecomment-1918512560 - return `${executionCode}${executionCommit.slice(0, 2)}${consensusCode}${consensusCommit.slice(0, 2)}`; + return `${executionCode}${executionCommit.slice(0, 2)}${consensusClientVersion.code}${consensusClientVersion.commit}`; } // No EL client info available. We still want to include CL info albeit not spec compliant - return `${consensusCode}|${consensusCommit.slice(0, 2)}`; + return `${consensusClientVersion.code}${consensusClientVersion.commit}`; } function notOnOutOfRangeData(beaconBlockRoot: Root): void { @@ -443,7 +422,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? defaultGraffiti), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), commonBlockBody, }); @@ -511,7 +490,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? defaultGraffiti), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), feeRecipient, commonBlockBody, }); @@ -622,7 +601,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? defaultGraffiti), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), }); logger.debug("Produced common block body", loggerContext); diff --git a/packages/beacon-node/src/api/options.ts b/packages/beacon-node/src/api/options.ts index f794dd6c4386..4b9aed7e0d94 100644 --- a/packages/beacon-node/src/api/options.ts +++ b/packages/beacon-node/src/api/options.ts @@ -4,12 +4,10 @@ export type ApiOptions = { maxGindicesInProof?: number; rest: BeaconRestApiServerOpts; version?: string; - commit?: string; }; export const defaultApiOptions: ApiOptions = { maxGindicesInProof: 512, rest: beaconRestApiServerOpts, version: "dev", - commit: "", }; diff --git a/packages/beacon-node/src/chain/prepareNextSlot.ts b/packages/beacon-node/src/chain/prepareNextSlot.ts index 3f730df3bf1d..85b729bcdf1c 100644 --- a/packages/beacon-node/src/chain/prepareNextSlot.ts +++ b/packages/beacon-node/src/chain/prepareNextSlot.ts @@ -196,6 +196,10 @@ export class PrepareNextSlotScheduler { proposerIndex, feeRecipient, }); + + this.chain.executionEngine.getClientVersion().catch((e) => { + this.logger.error("Unable to get client version", {caller: "prepareNextSlot"}, e); + }); } this.computeStateHashTreeRoot(updatedPrepareState); diff --git a/packages/beacon-node/src/execution/engine/disabled.ts b/packages/beacon-node/src/execution/engine/disabled.ts index 5de713c37788..9e07bd093919 100644 --- a/packages/beacon-node/src/execution/engine/disabled.ts +++ b/packages/beacon-node/src/execution/engine/disabled.ts @@ -27,11 +27,19 @@ export class ExecutionEngineDisabled implements IExecutionEngine { throw Error("Execution engine disabled"); } + getClientVersion(): Promise { + throw Error("Execution engine disabled"); + } + getState(): ExecutionEngineState { throw Error("Execution engine disabled"); } - getClientVersion(): Promise { + getExecutionClientVersion(): ClientVersion[] { + throw Error("Execution engine disabled"); + } + + getConsensusClientVersion(): ClientVersion { throw Error("Execution engine disabled"); } } diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 529fe6618123..246e12a5fb38 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -65,6 +65,14 @@ export type ExecutionEngineHttpOpts = { * A version string that will be set in `clv` field of jwt claims */ jwtVersion?: string; + /** + * Lodestar version to be used for `ClientVersion` + */ + version?: string; + /** + * Lodestar commit to be used for `ClientVersion` + */ + commit?: string; }; export const defaultExecutionEngineHttpOpts: ExecutionEngineHttpOpts = { @@ -107,6 +115,9 @@ export class ExecutionEngineHttp implements IExecutionEngine { // It's safer to to avoid false positives and assume that the EL is syncing until we receive the first payload private state: ExecutionEngineState = ExecutionEngineState.ONLINE; + // Cache EL client version from the latest getClientVersion call + private executionClientVersion: ClientVersion[] = []; + readonly payloadIdCache = new PayloadIdCache(); /** * A queue to serialize the fcUs and newPayloads calls: @@ -128,7 +139,8 @@ export class ExecutionEngineHttp implements IExecutionEngine { constructor( private readonly rpc: IJsonRpcHttpClient, - {metrics, signal, logger}: ExecutionEngineModules + {metrics, signal, logger}: ExecutionEngineModules, + private readonly opts?: ExecutionEngineHttpOpts ) { this.rpcFetchQueue = new JobItemQueue<[EngineRequest], EngineResponse>( this.jobQueueProcessor, @@ -423,18 +435,35 @@ export class ExecutionEngineHttp implements IExecutionEngine { return this.state; } - async getClientVersion(clientVersion: ClientVersion): Promise { + async getClientVersion(clientVersion?: ClientVersion): Promise { const method = "engine_getClientVersionV1"; const response = await this.rpc.fetchWithRetries< EngineApiRpcReturnTypes[typeof method], EngineApiRpcParamTypes[typeof method] - >({method, params: [clientVersion]}); + >({method, params: [clientVersion ?? this.getConsensusClientVersion()]}); - return response.map((cv) => { + this.executionClientVersion = response.map((cv) => { const code = cv.code in ClientCode ? ClientCode[cv.code as keyof typeof ClientCode] : ClientCode.XX; return {code, name: cv.name, version: cv.version, commit: cv.commit}; }); + + this.logger.debug(`executionClientVersion is set to ${JSON.stringify(this.executionClientVersion)}`); + + return this.executionClientVersion; + } + + getExecutionClientVersion(): ClientVersion[] { + return this.executionClientVersion; + } + + getConsensusClientVersion(): ClientVersion { + return { + code: ClientCode.LS, + name: "Lodestar", + version: this.opts?.version ?? "", + commit: this.opts?.commit?.slice(0, 2) ?? "", + }; } private updateEngineState(newState: ExecutionEngineState): void { @@ -445,6 +474,9 @@ export class ExecutionEngineHttp implements IExecutionEngine { switch (newState) { case ExecutionEngineState.ONLINE: this.logger.info("Execution client became online", {oldState, newState}); + this.getClientVersion().catch((e) => { + this.logger.error("Unable to get client version", {caller: "updateEngineState"}, e); + }); break; case ExecutionEngineState.OFFLINE: this.logger.error("Execution client went offline", {oldState, newState}); diff --git a/packages/beacon-node/src/execution/engine/index.ts b/packages/beacon-node/src/execution/engine/index.ts index 2d92a439c86d..f06e920af76a 100644 --- a/packages/beacon-node/src/execution/engine/index.ts +++ b/packages/beacon-node/src/execution/engine/index.ts @@ -40,7 +40,7 @@ export function getExecutionEngineHttp( jwtVersion: opts.jwtVersion, }); modules.logger.info("Execution client", {urls: opts.urls.map(toSafePrintableUrl).toString()}); - return new ExecutionEngineHttp(rpc, modules); + return new ExecutionEngineHttp(rpc, modules, opts); } export function initializeExecutionEngine( diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index 1558e4d1b675..7f959e692ca7 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -174,7 +174,13 @@ export interface IExecutionEngine { getPayloadBodiesByRange(start: number, count: number): Promise<(ExecutionPayloadBody | null)[]>; - getState(): ExecutionEngineState; + getClientVersion(clientVersion?: ClientVersion): Promise; - getClientVersion(clientVersion: ClientVersion): Promise; + getState(): ExecutionEngineState; + /** + * Not to be confused with `getClientVersion`. This method returns cached client version + * from `getClientVersion` which is a rpc call to EL client + */ + getExecutionClientVersion(): ClientVersion[]; + getConsensusClientVersion(): ClientVersion; } diff --git a/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts b/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts index b101382e01a0..d4f7705fb25b 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts @@ -10,7 +10,6 @@ import {generateState, zeroProtoBlock} from "../../../../../utils/state.js"; import {generateValidators} from "../../../../../utils/validator.js"; import {createCachedBeaconStateTest} from "../../../../../utils/cachedBeaconState.js"; import {SyncState} from "../../../../../../src/sync/interface.js"; -import {defaultApiOptions} from "../../../../../../src/api/options.js"; describe("get proposers api impl", function () { let api: ReturnType; @@ -21,7 +20,7 @@ describe("get proposers api impl", function () { beforeEach(function () { vi.useFakeTimers({now: 0}); modules = getApiTestModules({clock: "real"}); - api = getValidatorApi(defaultApiOptions, modules); + api = getValidatorApi(modules); state = generateState( { diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts index 84872ca6045c..256d772d5fc0 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts @@ -3,7 +3,6 @@ import {ProtoBlock} from "@lodestar/fork-choice"; import {SyncState} from "../../../../../src/sync/interface.js"; import {ApiTestModules, getApiTestModules} from "../../../../utils/api.js"; import {getValidatorApi} from "../../../../../src/api/impl/validator/index.js"; -import {defaultApiOptions} from "../../../../../src/api/options.js"; describe("api - validator - produceAttestationData", function () { let modules: ApiTestModules; @@ -11,7 +10,7 @@ describe("api - validator - produceAttestationData", function () { beforeEach(function () { modules = getApiTestModules(); - api = getValidatorApi(defaultApiOptions, modules); + api = getValidatorApi(modules); }); it("Should throw when node is not synced", async function () { diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts index a23373938f64..0868834e619e 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts @@ -14,7 +14,6 @@ import {toGraffitiBuffer} from "../../../../../src/util/graffiti.js"; import {BlockType, produceBlockBody} from "../../../../../src/chain/produceBlock/produceBlockBody.js"; import {generateProtoBlock} from "../../../../utils/typeGenerator.js"; import {ZERO_HASH_HEX} from "../../../../../src/constants/index.js"; -import {defaultApiOptions} from "../../../../../src/api/options.js"; describe("api/validator - produceBlockV2", function () { let api: ReturnType; @@ -23,7 +22,7 @@ describe("api/validator - produceBlockV2", function () { beforeEach(() => { modules = getApiTestModules(); - api = getValidatorApi(defaultApiOptions, modules); + api = getValidatorApi(modules); state = generateCachedBellatrixState(); }); diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts index a7299aa3b956..851bc9d0f33b 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts @@ -10,7 +10,6 @@ import {SyncState} from "../../../../../src/sync/interface.js"; import {getValidatorApi} from "../../../../../src/api/impl/validator/index.js"; import {CommonBlockBody} from "../../../../../src/chain/interface.js"; import {zeroProtoBlock} from "../../../../utils/state.js"; -import {defaultApiOptions} from "../../../../../src/api/options.js"; /* eslint-disable @typescript-eslint/naming-convention */ describe("api/validator - produceBlockV3", function () { @@ -27,7 +26,7 @@ describe("api/validator - produceBlockV3", function () { beforeEach(() => { modules = getApiTestModules(); - api = getValidatorApi(defaultApiOptions, {...modules, config}); + api = getValidatorApi({...modules, config}); modules.chain.executionBuilder.status = true; }); diff --git a/packages/cli/src/cmds/beacon/handler.ts b/packages/cli/src/cmds/beacon/handler.ts index 6c19bb250f5d..6e41f4700bf2 100644 --- a/packages/cli/src/cmds/beacon/handler.ts +++ b/packages/cli/src/cmds/beacon/handler.ts @@ -218,6 +218,10 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { // Render final options const options = beaconNodeOptions.getWithDefaults(); + const executionEngineMode = options.executionEngine.mode; + if (executionEngineMode === undefined || executionEngineMode === "http") { + options.executionEngine = {...options.executionEngine, version, commit}; + } return {config, options, beaconPaths, network, version, commit, peerId, logger}; } From 42bafc88c61e442dde24081426fa2365ce308055 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Wed, 26 Jun 2024 23:32:06 +0300 Subject: [PATCH 13/31] Hide graffiti if private flag is set --- packages/beacon-node/src/api/impl/api.ts | 2 +- .../src/api/impl/validator/index.ts | 19 ++++++++----------- packages/beacon-node/src/api/options.ts | 1 + .../impl/validator/duties/proposer.test.ts | 3 ++- .../validator/produceAttestationData.test.ts | 3 ++- .../api/impl/validator/produceBlockV2.test.ts | 3 ++- .../api/impl/validator/produceBlockV3.test.ts | 3 ++- packages/cli/src/cmds/beacon/handler.ts | 10 +++------- 8 files changed, 21 insertions(+), 23 deletions(-) diff --git a/packages/beacon-node/src/api/impl/api.ts b/packages/beacon-node/src/api/impl/api.ts index 6ec7180cf0f4..3647feed2d7c 100644 --- a/packages/beacon-node/src/api/impl/api.ts +++ b/packages/beacon-node/src/api/impl/api.ts @@ -21,6 +21,6 @@ export function getApi(opts: ApiOptions, modules: ApiModules): BeaconApiMethods lodestar: getLodestarApi(modules), node: getNodeApi(opts, modules), proof: getProofApi(opts, modules), - validator: getValidatorApi(modules), + validator: getValidatorApi(opts, modules), }; } diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 23d8d03f8c71..3f292f0796d7 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -63,6 +63,7 @@ import {getValidatorStatus} from "../beacon/state/utils.js"; import {validateGossipFnRetryUnknownRoot} from "../../../network/processor/gossipHandlers.js"; import {SCHEDULER_LOOKAHEAD_FACTOR} from "../../../chain/prepareNextSlot.js"; import {ChainEvent, CheckpointHex, CommonBlockBody} from "../../../chain/index.js"; +import {ApiOptions} from "../../options.js"; import {computeSubnetForCommitteesAtSlot, getPubkeysForIndices, selectBlockProductionSource} from "./utils.js"; /** @@ -110,14 +111,10 @@ type ProduceFullOrBlindedBlockOrContentsRes = {executionPayloadSource: ProducedB * Server implementation for handling validator duties. * See `@lodestar/validator/src/api` for the client implementation). */ -export function getValidatorApi({ - chain, - config, - logger, - metrics, - network, - sync, -}: ApiModules): ApplicationMethods { +export function getValidatorApi( + opts: ApiOptions, + {chain, config, logger, metrics, network, sync}: ApiModules +): ApplicationMethods { let genesisBlockRoot: Root | null = null; /** @@ -422,7 +419,7 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(opts.private ? "" : graffiti ?? getDefaultGraffiti()), commonBlockBody, }); @@ -490,7 +487,7 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(opts.private ? "" : graffiti ?? getDefaultGraffiti()), feeRecipient, commonBlockBody, }); @@ -601,7 +598,7 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(opts.private ? "" : graffiti ?? getDefaultGraffiti()), }); logger.debug("Produced common block body", loggerContext); diff --git a/packages/beacon-node/src/api/options.ts b/packages/beacon-node/src/api/options.ts index 4b9aed7e0d94..8025aeb4a79e 100644 --- a/packages/beacon-node/src/api/options.ts +++ b/packages/beacon-node/src/api/options.ts @@ -4,6 +4,7 @@ export type ApiOptions = { maxGindicesInProof?: number; rest: BeaconRestApiServerOpts; version?: string; + private?: boolean; }; export const defaultApiOptions: ApiOptions = { diff --git a/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts b/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts index d4f7705fb25b..b101382e01a0 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/duties/proposer.test.ts @@ -10,6 +10,7 @@ import {generateState, zeroProtoBlock} from "../../../../../utils/state.js"; import {generateValidators} from "../../../../../utils/validator.js"; import {createCachedBeaconStateTest} from "../../../../../utils/cachedBeaconState.js"; import {SyncState} from "../../../../../../src/sync/interface.js"; +import {defaultApiOptions} from "../../../../../../src/api/options.js"; describe("get proposers api impl", function () { let api: ReturnType; @@ -20,7 +21,7 @@ describe("get proposers api impl", function () { beforeEach(function () { vi.useFakeTimers({now: 0}); modules = getApiTestModules({clock: "real"}); - api = getValidatorApi(modules); + api = getValidatorApi(defaultApiOptions, modules); state = generateState( { diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts index 256d772d5fc0..84872ca6045c 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceAttestationData.test.ts @@ -3,6 +3,7 @@ import {ProtoBlock} from "@lodestar/fork-choice"; import {SyncState} from "../../../../../src/sync/interface.js"; import {ApiTestModules, getApiTestModules} from "../../../../utils/api.js"; import {getValidatorApi} from "../../../../../src/api/impl/validator/index.js"; +import {defaultApiOptions} from "../../../../../src/api/options.js"; describe("api - validator - produceAttestationData", function () { let modules: ApiTestModules; @@ -10,7 +11,7 @@ describe("api - validator - produceAttestationData", function () { beforeEach(function () { modules = getApiTestModules(); - api = getValidatorApi(modules); + api = getValidatorApi(defaultApiOptions, modules); }); it("Should throw when node is not synced", async function () { diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts index 0868834e619e..a23373938f64 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts @@ -14,6 +14,7 @@ import {toGraffitiBuffer} from "../../../../../src/util/graffiti.js"; import {BlockType, produceBlockBody} from "../../../../../src/chain/produceBlock/produceBlockBody.js"; import {generateProtoBlock} from "../../../../utils/typeGenerator.js"; import {ZERO_HASH_HEX} from "../../../../../src/constants/index.js"; +import {defaultApiOptions} from "../../../../../src/api/options.js"; describe("api/validator - produceBlockV2", function () { let api: ReturnType; @@ -22,7 +23,7 @@ describe("api/validator - produceBlockV2", function () { beforeEach(() => { modules = getApiTestModules(); - api = getValidatorApi(modules); + api = getValidatorApi(defaultApiOptions, modules); state = generateCachedBellatrixState(); }); diff --git a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts index 851bc9d0f33b..a7299aa3b956 100644 --- a/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts +++ b/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts @@ -10,6 +10,7 @@ import {SyncState} from "../../../../../src/sync/interface.js"; import {getValidatorApi} from "../../../../../src/api/impl/validator/index.js"; import {CommonBlockBody} from "../../../../../src/chain/interface.js"; import {zeroProtoBlock} from "../../../../utils/state.js"; +import {defaultApiOptions} from "../../../../../src/api/options.js"; /* eslint-disable @typescript-eslint/naming-convention */ describe("api/validator - produceBlockV3", function () { @@ -26,7 +27,7 @@ describe("api/validator - produceBlockV3", function () { beforeEach(() => { modules = getApiTestModules(); - api = getValidatorApi({...modules, config}); + api = getValidatorApi(defaultApiOptions, {...modules, config}); modules.chain.executionBuilder.status = true; }); diff --git a/packages/cli/src/cmds/beacon/handler.ts b/packages/cli/src/cmds/beacon/handler.ts index 6e41f4700bf2..1fa209793cbd 100644 --- a/packages/cli/src/cmds/beacon/handler.ts +++ b/packages/cli/src/cmds/beacon/handler.ts @@ -182,7 +182,7 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { beaconNodeOptions.set({metrics: {metadata: {version, commit, network}}}); beaconNodeOptions.set({metrics: {validatorMonitorLogs: args.validatorMonitorLogs}}); // Add detailed version string for API node/version endpoint - beaconNodeOptions.set({api: {version, commit}}); + beaconNodeOptions.set({api: {version}}); // Combine bootnodes from different sources const bootnodes = (beaconNodeOptions.get().network?.discv5?.bootEnrs ?? []).concat( @@ -204,7 +204,7 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { beaconNodeOptions.set({network: {discv5: {enr: enr.encodeTxt(), config: {enrUpdate: !enr.ip && !enr.ip6}}}}); if (args.private) { - beaconNodeOptions.set({network: {private: true}}); + beaconNodeOptions.set({network: {private: true}, api: {private: true}}); } else { const versionStr = `Lodestar/${version}`; const simpleVersionStr = version.split("/")[0]; @@ -213,15 +213,11 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { // Add User-Agent header to all builder requests beaconNodeOptions.set({executionBuilder: {userAgent: versionStr}}); // Set jwt version with version string - beaconNodeOptions.set({executionEngine: {jwtVersion: versionStr}, eth1: {jwtVersion: versionStr}}); + beaconNodeOptions.set({executionEngine: {jwtVersion: versionStr, commit, version}, eth1: {jwtVersion: versionStr}}); } // Render final options const options = beaconNodeOptions.getWithDefaults(); - const executionEngineMode = options.executionEngine.mode; - if (executionEngineMode === undefined || executionEngineMode === "http") { - options.executionEngine = {...options.executionEngine, version, commit}; - } return {config, options, beaconPaths, network, version, commit, peerId, logger}; } From 70358e9aaa5377cef74316ab3184f737d920fd62 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Wed, 26 Jun 2024 23:41:08 +0300 Subject: [PATCH 14/31] Improve readability --- packages/cli/src/cmds/beacon/handler.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/cli/src/cmds/beacon/handler.ts b/packages/cli/src/cmds/beacon/handler.ts index 1fa209793cbd..8a9552f85351 100644 --- a/packages/cli/src/cmds/beacon/handler.ts +++ b/packages/cli/src/cmds/beacon/handler.ts @@ -214,6 +214,8 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { beaconNodeOptions.set({executionBuilder: {userAgent: versionStr}}); // Set jwt version with version string beaconNodeOptions.set({executionEngine: {jwtVersion: versionStr, commit, version}, eth1: {jwtVersion: versionStr}}); + // Set commit and version for ClientVersion + beaconNodeOptions.set({executionEngine: {commit, version}}); } // Render final options From fc70f033043e01e3e1d52c9660ccd78acc76d4fa Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Thu, 27 Jun 2024 16:00:45 +0300 Subject: [PATCH 15/31] Partially address comment --- .../beacon-node/src/api/impl/validator/index.ts | 16 ++++++++++------ .../beacon-node/src/chain/prepareNextSlot.ts | 4 ---- .../beacon-node/src/execution/engine/disabled.ts | 2 +- .../beacon-node/src/execution/engine/http.ts | 15 +++++++++------ .../src/execution/engine/interface.ts | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 3f292f0796d7..28e160d22db6 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -333,12 +333,16 @@ export function getValidatorApi( } function getDefaultGraffiti(): string { + + if (opts.private) { + return ""; + } + const executionClientVersion = chain.executionEngine.getExecutionClientVersion(); const consensusClientVersion = chain.executionEngine.getConsensusClientVersion(); - if (executionClientVersion.length > 0) { - const executionCode = executionClientVersion[0].code; - const executionCommit = executionClientVersion[0].commit; + if (executionClientVersion != undefined) { + const {code: executionCode, commit: executionCommit} = executionClientVersion; // Follow the 2-byte commit format in https://github.com/ethereum/execution-apis/pull/517#issuecomment-1918512560 return `${executionCode}${executionCommit.slice(0, 2)}${consensusClientVersion.code}${consensusClientVersion.commit}`; @@ -419,7 +423,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(opts.private ? "" : graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), commonBlockBody, }); @@ -487,7 +491,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(opts.private ? "" : graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), feeRecipient, commonBlockBody, }); @@ -598,7 +602,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(opts.private ? "" : graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), }); logger.debug("Produced common block body", loggerContext); diff --git a/packages/beacon-node/src/chain/prepareNextSlot.ts b/packages/beacon-node/src/chain/prepareNextSlot.ts index 85b729bcdf1c..3f730df3bf1d 100644 --- a/packages/beacon-node/src/chain/prepareNextSlot.ts +++ b/packages/beacon-node/src/chain/prepareNextSlot.ts @@ -196,10 +196,6 @@ export class PrepareNextSlotScheduler { proposerIndex, feeRecipient, }); - - this.chain.executionEngine.getClientVersion().catch((e) => { - this.logger.error("Unable to get client version", {caller: "prepareNextSlot"}, e); - }); } this.computeStateHashTreeRoot(updatedPrepareState); diff --git a/packages/beacon-node/src/execution/engine/disabled.ts b/packages/beacon-node/src/execution/engine/disabled.ts index 9e07bd093919..99270d5c44b7 100644 --- a/packages/beacon-node/src/execution/engine/disabled.ts +++ b/packages/beacon-node/src/execution/engine/disabled.ts @@ -35,7 +35,7 @@ export class ExecutionEngineDisabled implements IExecutionEngine { throw Error("Execution engine disabled"); } - getExecutionClientVersion(): ClientVersion[] { + getExecutionClientVersion(): ClientVersion { throw Error("Execution engine disabled"); } diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 246e12a5fb38..0f41a549f333 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -116,7 +116,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { private state: ExecutionEngineState = ExecutionEngineState.ONLINE; // Cache EL client version from the latest getClientVersion call - private executionClientVersion: ClientVersion[] = []; + private clientVersion?: ClientVersion; readonly payloadIdCache = new PayloadIdCache(); /** @@ -443,18 +443,21 @@ export class ExecutionEngineHttp implements IExecutionEngine { EngineApiRpcParamTypes[typeof method] >({method, params: [clientVersion ?? this.getConsensusClientVersion()]}); - this.executionClientVersion = response.map((cv) => { + const clientVersions = response.map((cv) => { const code = cv.code in ClientCode ? ClientCode[cv.code as keyof typeof ClientCode] : ClientCode.XX; return {code, name: cv.name, version: cv.version, commit: cv.commit}; }); - this.logger.debug(`executionClientVersion is set to ${JSON.stringify(this.executionClientVersion)}`); + if (clientVersions.length > 0) { + this.clientVersion = clientVersions[0]; + this.logger.info(`executionClientVersion is set to ${JSON.stringify(this.clientVersion)}`); + } - return this.executionClientVersion; + return clientVersions; } - getExecutionClientVersion(): ClientVersion[] { - return this.executionClientVersion; + get executionClientVersion(): ClientVersion | undefined { + return this.clientVersion; } getConsensusClientVersion(): ClientVersion { diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index 7f959e692ca7..24262880b2bd 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -181,6 +181,6 @@ export interface IExecutionEngine { * Not to be confused with `getClientVersion`. This method returns cached client version * from `getClientVersion` which is a rpc call to EL client */ - getExecutionClientVersion(): ClientVersion[]; + getExecutionClientVersion(): ClientVersion; getConsensusClientVersion(): ClientVersion; } From 455bddd05ede2e4a89190517e66447d8fdb6fff2 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Thu, 18 Jul 2024 18:07:41 +0900 Subject: [PATCH 16/31] Partially address comment --- packages/beacon-node/src/api/impl/validator/index.ts | 4 ++-- .../beacon-node/src/execution/engine/disabled.ts | 4 ---- packages/beacon-node/src/execution/engine/http.ts | 12 ++---------- .../beacon-node/src/execution/engine/interface.ts | 1 + packages/beacon-node/src/util/graffiti.ts | 10 ++++++++++ 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 28e160d22db6..57d679348224 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -53,7 +53,7 @@ import {validateApiAggregateAndProof} from "../../../chain/validation/index.js"; import {ZERO_HASH} from "../../../constants/index.js"; import {SyncState} from "../../../sync/index.js"; import {isOptimisticBlock} from "../../../util/forkChoice.js"; -import {toGraffitiBuffer} from "../../../util/graffiti.js"; +import {getLodestarClientVersion, toGraffitiBuffer} from "../../../util/graffiti.js"; import {ApiError, NodeIsSyncing, OnlySupportedByDVT} from "../errors.js"; import {validateSyncCommitteeGossipContributionAndProof} from "../../../chain/validation/syncCommitteeContributionAndProof.js"; import {CommitteeSubscription} from "../../../network/subnets/index.js"; @@ -339,7 +339,7 @@ export function getValidatorApi( } const executionClientVersion = chain.executionEngine.getExecutionClientVersion(); - const consensusClientVersion = chain.executionEngine.getConsensusClientVersion(); + const consensusClientVersion = getLodestarClientVersion(); if (executionClientVersion != undefined) { const {code: executionCode, commit: executionCommit} = executionClientVersion; diff --git a/packages/beacon-node/src/execution/engine/disabled.ts b/packages/beacon-node/src/execution/engine/disabled.ts index 49bd3d8c68bf..87b43840d0af 100644 --- a/packages/beacon-node/src/execution/engine/disabled.ts +++ b/packages/beacon-node/src/execution/engine/disabled.ts @@ -40,8 +40,4 @@ export class ExecutionEngineDisabled implements IExecutionEngine { getExecutionClientVersion(): ClientVersion { throw Error("Execution engine disabled"); } - - getConsensusClientVersion(): ClientVersion { - throw Error("Execution engine disabled"); - } } diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 4585bc3667e8..0d1e0e296f83 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -38,6 +38,7 @@ import { deserializeExecutionPayloadBody, } from "./types.js"; import {getExecutionEngineState} from "./utils.js"; +import { getLodestarClientVersion } from "../../util/graffiti.js"; export type ExecutionEngineModules = { signal: AbortSignal; @@ -441,7 +442,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { const response = await this.rpc.fetchWithRetries< EngineApiRpcReturnTypes[typeof method], EngineApiRpcParamTypes[typeof method] - >({method, params: [clientVersion ?? this.getConsensusClientVersion()]}); + >({method, params: [clientVersion ?? getLodestarClientVersion()]}); const clientVersions = response.map((cv) => { const code = cv.code in ClientCode ? ClientCode[cv.code as keyof typeof ClientCode] : ClientCode.XX; @@ -460,15 +461,6 @@ export class ExecutionEngineHttp implements IExecutionEngine { return this.clientVersion; } - getConsensusClientVersion(): ClientVersion { - return { - code: ClientCode.LS, - name: "Lodestar", - version: this.opts?.version ?? "", - commit: this.opts?.commit?.slice(0, 2) ?? "", - }; - } - private updateEngineState(newState: ExecutionEngineState): void { const oldState = this.state; diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index fb4a97061fef..ce443e7b80a9 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -118,6 +118,7 @@ export type VersionedHashes = Uint8Array[]; export interface IExecutionEngine { readonly state: ExecutionEngineState; + readonly clientVersion?: ClientVersion; payloadIdCache: PayloadIdCache; /** * A state transition function which applies changes to the self.execution_state. diff --git a/packages/beacon-node/src/util/graffiti.ts b/packages/beacon-node/src/util/graffiti.ts index 514be4aa5bdd..27cb892540c3 100644 --- a/packages/beacon-node/src/util/graffiti.ts +++ b/packages/beacon-node/src/util/graffiti.ts @@ -1,4 +1,5 @@ import {GRAFFITI_SIZE} from "../constants/index.js"; +import { ClientCode, ClientVersion } from "../execution/index.js"; /** * Parses a graffiti UTF8 string and returns a 32 bytes buffer right padded with zeros @@ -6,3 +7,12 @@ import {GRAFFITI_SIZE} from "../constants/index.js"; export function toGraffitiBuffer(graffiti: string): Buffer { return Buffer.concat([Buffer.from(graffiti, "utf8"), Buffer.alloc(GRAFFITI_SIZE, 0)], GRAFFITI_SIZE); } + +export function getLodestarClientVersion(info?: {version: string, commit: string}): ClientVersion { + return { + code: ClientCode.LS, + name: "Lodestar", + version: info?.version ?? "", + commit: info?.commit?.slice(0, 2) ?? "", + }; +} \ No newline at end of file From 1c91dc4c3d2592f241bb3bceb55a1a534ecacd50 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Thu, 18 Jul 2024 19:55:02 +0900 Subject: [PATCH 17/31] Partially address comment --- .../src/api/impl/validator/index.ts | 28 +++---------------- .../beacon-node/src/execution/engine/http.ts | 9 ++++-- .../src/execution/engine/interface.ts | 6 ---- packages/beacon-node/src/util/graffiti.ts | 21 +++++++++++++- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 57d679348224..2b4e112a58d6 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -53,7 +53,7 @@ import {validateApiAggregateAndProof} from "../../../chain/validation/index.js"; import {ZERO_HASH} from "../../../constants/index.js"; import {SyncState} from "../../../sync/index.js"; import {isOptimisticBlock} from "../../../util/forkChoice.js"; -import {getLodestarClientVersion, toGraffitiBuffer} from "../../../util/graffiti.js"; +import {getDefaultGraffiti, toGraffitiBuffer} from "../../../util/graffiti.js"; import {ApiError, NodeIsSyncing, OnlySupportedByDVT} from "../errors.js"; import {validateSyncCommitteeGossipContributionAndProof} from "../../../chain/validation/syncCommitteeContributionAndProof.js"; import {CommitteeSubscription} from "../../../network/subnets/index.js"; @@ -332,26 +332,6 @@ export function getValidatorApi( ); } - function getDefaultGraffiti(): string { - - if (opts.private) { - return ""; - } - - const executionClientVersion = chain.executionEngine.getExecutionClientVersion(); - const consensusClientVersion = getLodestarClientVersion(); - - if (executionClientVersion != undefined) { - const {code: executionCode, commit: executionCommit} = executionClientVersion; - - // Follow the 2-byte commit format in https://github.com/ethereum/execution-apis/pull/517#issuecomment-1918512560 - return `${executionCode}${executionCommit.slice(0, 2)}${consensusClientVersion.code}${consensusClientVersion.commit}`; - } - - // No EL client info available. We still want to include CL info albeit not spec compliant - return `${consensusClientVersion.code}${consensusClientVersion.commit}`; - } - function notOnOutOfRangeData(beaconBlockRoot: Root): void { const protoBeaconBlock = chain.forkChoice.getBlock(beaconBlockRoot); if (!protoBeaconBlock) { @@ -423,7 +403,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti(opts, chain.executionEngine.clientVersion)), commonBlockBody, }); @@ -491,7 +471,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti(opts, chain.executionEngine.clientVersion)), feeRecipient, commonBlockBody, }); @@ -602,7 +582,7 @@ export function getValidatorApi( slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti()), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti(opts, chain.executionEngine.clientVersion)), }); logger.debug("Produced common block body", loggerContext); diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 0d1e0e296f83..cbfd5b0ae939 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -117,7 +117,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { state: ExecutionEngineState = ExecutionEngineState.ONLINE; // Cache EL client version from the latest getClientVersion call - private clientVersion?: ClientVersion; + clientVersion?: ClientVersion; readonly payloadIdCache = new PayloadIdCache(); /** @@ -157,6 +157,11 @@ export class ExecutionEngineHttp implements IExecutionEngine { this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); }); + + // Initial state engine state is ONLINE. Need to explicitly call `getClientVersion` once on startup + this.getClientVersion().catch((e) => { + this.logger.error("Unable to get client version", {caller: "ExecutionEngineHttp constructor"}, e); + }); } /** @@ -451,7 +456,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (clientVersions.length > 0) { this.clientVersion = clientVersions[0]; - this.logger.info(`executionClientVersion is set to ${JSON.stringify(this.clientVersion)}`); + this.logger.info("Execution client version is updated", this.clientVersion); } return clientVersions; diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index ce443e7b80a9..7623fd6df335 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -180,10 +180,4 @@ export interface IExecutionEngine { getClientVersion(clientVersion?: ClientVersion): Promise; getState(): ExecutionEngineState; - /** - * Not to be confused with `getClientVersion`. This method returns cached client version - * from `getClientVersion` which is a rpc call to EL client - */ - getExecutionClientVersion(): ClientVersion; - getConsensusClientVersion(): ClientVersion; } diff --git a/packages/beacon-node/src/util/graffiti.ts b/packages/beacon-node/src/util/graffiti.ts index 27cb892540c3..7d0374e294e9 100644 --- a/packages/beacon-node/src/util/graffiti.ts +++ b/packages/beacon-node/src/util/graffiti.ts @@ -15,4 +15,23 @@ export function getLodestarClientVersion(info?: {version: string, commit: string version: info?.version ?? "", commit: info?.commit?.slice(0, 2) ?? "", }; -} \ No newline at end of file +} + +export function getDefaultGraffiti(opts: {private?: boolean}, executionClientVersion?: ClientVersion): string { + + if (opts.private) { + return ""; + } + + const consensusClientVersion = getLodestarClientVersion(); + + if (executionClientVersion != undefined) { + const {code: executionCode, commit: executionCommit} = executionClientVersion; + + // Follow the 2-byte commit format in https://github.com/ethereum/execution-apis/pull/517#issuecomment-1918512560 + return `${executionCode}${executionCommit.slice(0, 2)}${consensusClientVersion.code}${consensusClientVersion.commit}`; + } + + // No EL client info available. We still want to include CL info albeit not spec compliant + return `${consensusClientVersion.code}${consensusClientVersion.commit}`; +} From e666c97e9c12b9d60036db1fd8ac6b5b35589ede Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Thu, 18 Jul 2024 20:29:43 +0900 Subject: [PATCH 18/31] Refactor --- packages/beacon-node/src/execution/engine/http.ts | 12 ++++++------ .../beacon-node/src/execution/engine/interface.ts | 2 +- packages/beacon-node/src/util/graffiti.ts | 8 ++++---- packages/cli/src/cmds/beacon/handler.ts | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index cbfd5b0ae939..985efe4ceef1 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -12,6 +12,7 @@ import {Metrics} from "../../metrics/index.js"; import {JobItemQueue} from "../../util/queue/index.js"; import {EPOCHS_PER_BATCH} from "../../sync/constants.js"; import {numToQuantity} from "../../eth1/provider/utils.js"; +import {getLodestarClientVersion} from "../../util/graffiti.js"; import { ExecutionPayloadStatus, ExecutePayloadResponse, @@ -38,7 +39,6 @@ import { deserializeExecutionPayloadBody, } from "./types.js"; import {getExecutionEngineState} from "./utils.js"; -import { getLodestarClientVersion } from "../../util/graffiti.js"; export type ExecutionEngineModules = { signal: AbortSignal; @@ -158,8 +158,8 @@ export class ExecutionEngineHttp implements IExecutionEngine { this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); }); - // Initial state engine state is ONLINE. Need to explicitly call `getClientVersion` once on startup - this.getClientVersion().catch((e) => { + // Initial engine state is ONLINE. Need to explicitly call `getClientVersion` once on startup + this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { this.logger.error("Unable to get client version", {caller: "ExecutionEngineHttp constructor"}, e); }); } @@ -441,13 +441,13 @@ export class ExecutionEngineHttp implements IExecutionEngine { return this.state; } - async getClientVersion(clientVersion?: ClientVersion): Promise { + async getClientVersion(clientVersion: ClientVersion): Promise { const method = "engine_getClientVersionV1"; const response = await this.rpc.fetchWithRetries< EngineApiRpcReturnTypes[typeof method], EngineApiRpcParamTypes[typeof method] - >({method, params: [clientVersion ?? getLodestarClientVersion()]}); + >({method, params: [clientVersion]}); const clientVersions = response.map((cv) => { const code = cv.code in ClientCode ? ClientCode[cv.code as keyof typeof ClientCode] : ClientCode.XX; @@ -474,7 +474,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { switch (newState) { case ExecutionEngineState.ONLINE: this.logger.info("Execution client became online", {oldState, newState}); - this.getClientVersion().catch((e) => { + this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { this.logger.error("Unable to get client version", {caller: "updateEngineState"}, e); }); break; diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index 7623fd6df335..dd480e5f2258 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -177,7 +177,7 @@ export interface IExecutionEngine { getPayloadBodiesByRange(start: number, count: number): Promise<(ExecutionPayloadBody | null)[]>; - getClientVersion(clientVersion?: ClientVersion): Promise; + getClientVersion(clientVersion: ClientVersion): Promise; getState(): ExecutionEngineState; } diff --git a/packages/beacon-node/src/util/graffiti.ts b/packages/beacon-node/src/util/graffiti.ts index 7d0374e294e9..af3d4ccff254 100644 --- a/packages/beacon-node/src/util/graffiti.ts +++ b/packages/beacon-node/src/util/graffiti.ts @@ -1,5 +1,6 @@ import {GRAFFITI_SIZE} from "../constants/index.js"; -import { ClientCode, ClientVersion } from "../execution/index.js"; +import {ExecutionEngineHttpOpts} from "../execution/engine/http.js"; +import {ClientCode, ClientVersion} from "../execution/index.js"; /** * Parses a graffiti UTF8 string and returns a 32 bytes buffer right padded with zeros @@ -8,17 +9,16 @@ export function toGraffitiBuffer(graffiti: string): Buffer { return Buffer.concat([Buffer.from(graffiti, "utf8"), Buffer.alloc(GRAFFITI_SIZE, 0)], GRAFFITI_SIZE); } -export function getLodestarClientVersion(info?: {version: string, commit: string}): ClientVersion { +export function getLodestarClientVersion(info?: ExecutionEngineHttpOpts): ClientVersion { return { code: ClientCode.LS, name: "Lodestar", version: info?.version ?? "", - commit: info?.commit?.slice(0, 2) ?? "", + commit: info?.commit?.slice(0, 4) ?? "", }; } export function getDefaultGraffiti(opts: {private?: boolean}, executionClientVersion?: ClientVersion): string { - if (opts.private) { return ""; } diff --git a/packages/cli/src/cmds/beacon/handler.ts b/packages/cli/src/cmds/beacon/handler.ts index 02592c50b89a..bf9eb6cc96e0 100644 --- a/packages/cli/src/cmds/beacon/handler.ts +++ b/packages/cli/src/cmds/beacon/handler.ts @@ -204,7 +204,7 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { // Add User-Agent header to all builder requests beaconNodeOptions.set({executionBuilder: {userAgent: versionStr}}); // Set jwt version with version string - beaconNodeOptions.set({executionEngine: {jwtVersion: versionStr, commit, version}, eth1: {jwtVersion: versionStr}}); + beaconNodeOptions.set({executionEngine: {jwtVersion: versionStr}, eth1: {jwtVersion: versionStr}}); // Set commit and version for ClientVersion beaconNodeOptions.set({executionEngine: {commit, version}}); } From 6257dcf690e0126ee7a8b69d8ffe8ff44bbb8a0d Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Wed, 24 Jul 2024 12:08:11 +0900 Subject: [PATCH 19/31] Update packages/beacon-node/src/execution/engine/http.ts Co-authored-by: Nico Flaig --- packages/beacon-node/src/execution/engine/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 985efe4ceef1..c7973c9fae49 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -116,7 +116,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { // It's safer to to avoid false positives and assume that the EL is syncing until we receive the first payload state: ExecutionEngineState = ExecutionEngineState.ONLINE; - // Cache EL client version from the latest getClientVersion call + // Cached EL client version from the latest getClientVersion call clientVersion?: ClientVersion; readonly payloadIdCache = new PayloadIdCache(); From fc4eaa024b3419477f8e6ca3e1830d33192bad0b Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Wed, 24 Jul 2024 13:04:04 +0900 Subject: [PATCH 20/31] Partial address comment --- .../src/execution/engine/disabled.ts | 14 +------------ .../beacon-node/src/execution/engine/http.ts | 21 +++++++------------ .../src/execution/engine/interface.ts | 4 ---- packages/beacon-node/src/util/graffiti.ts | 3 +-- 4 files changed, 9 insertions(+), 33 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/disabled.ts b/packages/beacon-node/src/execution/engine/disabled.ts index 87b43840d0af..68c72dc02ac6 100644 --- a/packages/beacon-node/src/execution/engine/disabled.ts +++ b/packages/beacon-node/src/execution/engine/disabled.ts @@ -1,4 +1,4 @@ -import {ClientVersion, ExecutionEngineState, IExecutionEngine, PayloadIdCache} from "./interface.js"; +import {ExecutionEngineState, IExecutionEngine, PayloadIdCache} from "./interface.js"; export class ExecutionEngineDisabled implements IExecutionEngine { state = ExecutionEngineState.OFFLINE; @@ -28,16 +28,4 @@ export class ExecutionEngineDisabled implements IExecutionEngine { getPayloadBodiesByRange(): Promise { throw Error("Execution engine disabled"); } - - getClientVersion(): Promise { - throw Error("Execution engine disabled"); - } - - getState(): ExecutionEngineState { - throw Error("Execution engine disabled"); - } - - getExecutionClientVersion(): ClientVersion { - throw Error("Execution engine disabled"); - } } diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index c7973c9fae49..1bca0e1d8b42 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -155,13 +155,14 @@ export class ExecutionEngineHttp implements IExecutionEngine { }); this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { + if (this.clientVersion === undefined) { + // This statement should only be called first time receiving response after start up + this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { + this.logger.error("Unable to get client version", {caller: "ExecutionEngineHttp constructor"}, e); + }); + } this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); }); - - // Initial engine state is ONLINE. Need to explicitly call `getClientVersion` once on startup - this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get client version", {caller: "ExecutionEngineHttp constructor"}, e); - }); } /** @@ -437,11 +438,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { return response.map(deserializeExecutionPayloadBody); } - getState(): ExecutionEngineState { - return this.state; - } - - async getClientVersion(clientVersion: ClientVersion): Promise { + private async getClientVersion(clientVersion: ClientVersion): Promise { const method = "engine_getClientVersionV1"; const response = await this.rpc.fetchWithRetries< @@ -462,10 +459,6 @@ export class ExecutionEngineHttp implements IExecutionEngine { return clientVersions; } - get executionClientVersion(): ClientVersion | undefined { - return this.clientVersion; - } - private updateEngineState(newState: ExecutionEngineState): void { const oldState = this.state; diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index dd480e5f2258..7368b1a1d697 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -176,8 +176,4 @@ export interface IExecutionEngine { getPayloadBodiesByHash(blockHash: DATA[]): Promise<(ExecutionPayloadBody | null)[]>; getPayloadBodiesByRange(start: number, count: number): Promise<(ExecutionPayloadBody | null)[]>; - - getClientVersion(clientVersion: ClientVersion): Promise; - - getState(): ExecutionEngineState; } diff --git a/packages/beacon-node/src/util/graffiti.ts b/packages/beacon-node/src/util/graffiti.ts index af3d4ccff254..d8192b99fde3 100644 --- a/packages/beacon-node/src/util/graffiti.ts +++ b/packages/beacon-node/src/util/graffiti.ts @@ -1,5 +1,4 @@ import {GRAFFITI_SIZE} from "../constants/index.js"; -import {ExecutionEngineHttpOpts} from "../execution/engine/http.js"; import {ClientCode, ClientVersion} from "../execution/index.js"; /** @@ -9,7 +8,7 @@ export function toGraffitiBuffer(graffiti: string): Buffer { return Buffer.concat([Buffer.from(graffiti, "utf8"), Buffer.alloc(GRAFFITI_SIZE, 0)], GRAFFITI_SIZE); } -export function getLodestarClientVersion(info?: ExecutionEngineHttpOpts): ClientVersion { +export function getLodestarClientVersion(info?: {version?: string; commit?: string}): ClientVersion { return { code: ClientCode.LS, name: "Lodestar", From 5aefdb9b60517d1e7924401841e72ce0c9df339a Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Thu, 25 Jul 2024 20:20:41 +0900 Subject: [PATCH 21/31] Add unit test --- .../src/api/impl/validator/index.ts | 17 +++++++++-- packages/beacon-node/src/api/options.ts | 1 + .../beacon-node/src/execution/engine/http.ts | 8 +++--- .../src/execution/engine/interface.ts | 26 ++++++++--------- packages/beacon-node/src/util/graffiti.ts | 27 ++++++++---------- packages/beacon-node/src/util/metadata.ts | 10 +++++++ .../test/unit/util/graffiti.test.ts | 28 ++++++++++++++++++- .../test/unit/util/metadata.test.ts | 17 +++++++++++ packages/cli/src/cmds/beacon/handler.ts | 2 +- 9 files changed, 99 insertions(+), 37 deletions(-) create mode 100644 packages/beacon-node/src/util/metadata.ts create mode 100644 packages/beacon-node/test/unit/util/metadata.test.ts diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 2b4e112a58d6..999f2907c7f2 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -64,6 +64,7 @@ import {validateGossipFnRetryUnknownRoot} from "../../../network/processor/gossi import {SCHEDULER_LOOKAHEAD_FACTOR} from "../../../chain/prepareNextSlot.js"; import {ChainEvent, CheckpointHex, CommonBlockBody} from "../../../chain/index.js"; import {ApiOptions} from "../../options.js"; +import {getLodestarClientVersion} from "../../../util/metadata.js"; import {computeSubnetForCommitteesAtSlot, getPubkeysForIndices, selectBlockProductionSource} from "./utils.js"; /** @@ -399,11 +400,15 @@ export function getValidatorApi( let timer; try { timer = metrics?.blockProductionTime.startTimer(); + const consensusClientVersion = getLodestarClientVersion(opts); + const executionClientVersion = chain.executionEngine.clientVersion; const {block, executionPayloadValue, consensusBlockValue} = await chain.produceBlindedBlock({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti(opts, chain.executionEngine.clientVersion)), + graffiti: toGraffitiBuffer( + graffiti ?? getDefaultGraffiti(opts, consensusClientVersion, executionClientVersion) + ), commonBlockBody, }); @@ -467,11 +472,15 @@ export function getValidatorApi( let timer; try { timer = metrics?.blockProductionTime.startTimer(); + const consensusClientVersion = getLodestarClientVersion(opts); + const executionClientVersion = chain.executionEngine.clientVersion; const {block, executionPayloadValue, consensusBlockValue, shouldOverrideBuilder} = await chain.produceBlock({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti(opts, chain.executionEngine.clientVersion)), + graffiti: toGraffitiBuffer( + graffiti ?? getDefaultGraffiti(opts, consensusClientVersion, executionClientVersion) + ), feeRecipient, commonBlockBody, }); @@ -578,11 +587,13 @@ export function getValidatorApi( }; logger.verbose("Assembling block with produceEngineOrBuilderBlock", loggerContext); + const consensusClientVersion = getLodestarClientVersion(opts); + const executionClientVersion = chain.executionEngine.clientVersion; const commonBlockBody = await chain.produceCommonBlockBody({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti(opts, chain.executionEngine.clientVersion)), + graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti(opts, consensusClientVersion, executionClientVersion)), }); logger.debug("Produced common block body", loggerContext); diff --git a/packages/beacon-node/src/api/options.ts b/packages/beacon-node/src/api/options.ts index 8025aeb4a79e..b7434c4ed82f 100644 --- a/packages/beacon-node/src/api/options.ts +++ b/packages/beacon-node/src/api/options.ts @@ -3,6 +3,7 @@ import {beaconRestApiServerOpts, BeaconRestApiServerOpts} from "./rest/index.js" export type ApiOptions = { maxGindicesInProof?: number; rest: BeaconRestApiServerOpts; + commit?: string; version?: string; private?: boolean; }; diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 1bca0e1d8b42..2eb8bb9511c2 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -12,7 +12,7 @@ import {Metrics} from "../../metrics/index.js"; import {JobItemQueue} from "../../util/queue/index.js"; import {EPOCHS_PER_BATCH} from "../../sync/constants.js"; import {numToQuantity} from "../../eth1/provider/utils.js"; -import {getLodestarClientVersion} from "../../util/graffiti.js"; +import {getLodestarClientVersion} from "../../util/metadata.js"; import { ExecutionPayloadStatus, ExecutePayloadResponse, @@ -158,7 +158,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (this.clientVersion === undefined) { // This statement should only be called first time receiving response after start up this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get client version", {caller: "ExecutionEngineHttp constructor"}, e); + this.logger.error("Unable to get client version", e); }); } this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); @@ -453,7 +453,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (clientVersions.length > 0) { this.clientVersion = clientVersions[0]; - this.logger.info("Execution client version is updated", this.clientVersion); + this.logger.debug("Execution client version is updated", this.clientVersion); } return clientVersions; @@ -468,7 +468,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { case ExecutionEngineState.ONLINE: this.logger.info("Execution client became online", {oldState, newState}); this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get client version", {caller: "updateEngineState"}, e); + this.logger.error("Unable to get client version", e); }); break; case ExecutionEngineState.OFFLINE: diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index 7368b1a1d697..1df2b7a9e3f4 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -43,19 +43,19 @@ export enum ExecutionEngineState { * ClientCode.XX is dedicated to other clients which do not have their own code */ export enum ClientCode { - BU = "besu", - EJ = "ethereumJS", - EG = "erigon", - GE = "go-ethereum", - GR = "grandine", - LH = "lighthouse", - LS = "lodestar", - NM = "nethermind", - NB = "nimbus", - TK = "teku", - PM = "prysm", - RH = "reth", - XX = "unknown", + BU = "BU", // besu + EJ = "EJ", // ethereumJS + EG = "EG", // erigon + GE = "GE", // go-ethereum + GR = "GR", // grandine + LH = "LH", // lighthouse + LS = "LS", // lodestar + NM = "NM", // nethermind + NB = "NB", // nimbus + TK = "TK", // teku + PM = "PM", // prysm + RH = "RH", // reth + XX = "XX", // unknown } export type ExecutePayloadResponse = diff --git a/packages/beacon-node/src/util/graffiti.ts b/packages/beacon-node/src/util/graffiti.ts index d8192b99fde3..17820b5cc2d5 100644 --- a/packages/beacon-node/src/util/graffiti.ts +++ b/packages/beacon-node/src/util/graffiti.ts @@ -1,5 +1,5 @@ import {GRAFFITI_SIZE} from "../constants/index.js"; -import {ClientCode, ClientVersion} from "../execution/index.js"; +import {ClientVersion} from "../execution/index.js"; /** * Parses a graffiti UTF8 string and returns a 32 bytes buffer right padded with zeros @@ -8,29 +8,26 @@ export function toGraffitiBuffer(graffiti: string): Buffer { return Buffer.concat([Buffer.from(graffiti, "utf8"), Buffer.alloc(GRAFFITI_SIZE, 0)], GRAFFITI_SIZE); } -export function getLodestarClientVersion(info?: {version?: string; commit?: string}): ClientVersion { - return { - code: ClientCode.LS, - name: "Lodestar", - version: info?.version ?? "", - commit: info?.commit?.slice(0, 4) ?? "", - }; -} - -export function getDefaultGraffiti(opts: {private?: boolean}, executionClientVersion?: ClientVersion): string { +export function getDefaultGraffiti( + opts: {private?: boolean}, + consensusClientVersion?: ClientVersion, + executionClientVersion?: ClientVersion +): string { if (opts.private) { return ""; } - const consensusClientVersion = getLodestarClientVersion(); + if (consensusClientVersion === undefined) { + throw new Error("Consensus client version must be provided if opts.private is set to false"); + } - if (executionClientVersion != undefined) { + if (executionClientVersion !== undefined) { const {code: executionCode, commit: executionCommit} = executionClientVersion; // Follow the 2-byte commit format in https://github.com/ethereum/execution-apis/pull/517#issuecomment-1918512560 - return `${executionCode}${executionCommit.slice(0, 2)}${consensusClientVersion.code}${consensusClientVersion.commit}`; + return `${executionCode}${executionCommit.slice(0, 4)}${consensusClientVersion.code}${consensusClientVersion.commit.slice(0, 4)}`; } // No EL client info available. We still want to include CL info albeit not spec compliant - return `${consensusClientVersion.code}${consensusClientVersion.commit}`; + return `${consensusClientVersion.code}${consensusClientVersion.commit.slice(0, 4)}`; } diff --git a/packages/beacon-node/src/util/metadata.ts b/packages/beacon-node/src/util/metadata.ts new file mode 100644 index 000000000000..9e6d20710105 --- /dev/null +++ b/packages/beacon-node/src/util/metadata.ts @@ -0,0 +1,10 @@ +import {ClientCode, ClientVersion} from "../execution/index.js"; + +export function getLodestarClientVersion(info?: {version?: string; commit?: string}): ClientVersion { + return { + code: ClientCode.LS, + name: "Lodestar", + version: info?.version ?? "", + commit: info?.commit?.slice(0, 8) ?? "", + }; +} diff --git a/packages/beacon-node/test/unit/util/graffiti.test.ts b/packages/beacon-node/test/unit/util/graffiti.test.ts index b1338181b324..7675032e9d34 100644 --- a/packages/beacon-node/test/unit/util/graffiti.test.ts +++ b/packages/beacon-node/test/unit/util/graffiti.test.ts @@ -1,5 +1,6 @@ import {describe, it, expect} from "vitest"; -import {toGraffitiBuffer} from "../../../src/util/graffiti.js"; +import {getDefaultGraffiti, toGraffitiBuffer} from "../../../src/util/graffiti.js"; +import {ClientCode} from "../../../src/execution/index.js"; describe("Graffiti helper", () => { describe("toGraffitiBuffer", () => { @@ -26,4 +27,29 @@ describe("Graffiti helper", () => { }); } }); + + describe("getDefaultGraffiti", () => { + const executionClientVersion = {code: ClientCode.BU, name: "Besu", version: "24.1.1", commit: "9b0e38fa"}; + const consensusClientVersion = { + code: ClientCode.LS, + name: "Lodestar", + version: "v0.36.0/80c248b", + commit: "80c248bb", + }; // Sample output of getLodestarClientVersion() + + it("should return empty if private option is set", () => { + const result = getDefaultGraffiti({private: true}); + expect(result).toBe(""); + }); + + it("should return CL only info if EL client version is missing", () => { + const result = getDefaultGraffiti({private: false}, consensusClientVersion); + expect(result).toBe("LS80c2"); + }); + + it("should return combined version codes and commits if executionClientVersion is provided", () => { + const result = getDefaultGraffiti({private: false}, consensusClientVersion, executionClientVersion); + expect(result).toBe("BU9b0eLS80c2"); + }); + }); }); diff --git a/packages/beacon-node/test/unit/util/metadata.test.ts b/packages/beacon-node/test/unit/util/metadata.test.ts new file mode 100644 index 000000000000..7f8b8e44380a --- /dev/null +++ b/packages/beacon-node/test/unit/util/metadata.test.ts @@ -0,0 +1,17 @@ +import {describe, it, expect} from "vitest"; +import {getLodestarClientVersion} from "../../../src/util/metadata.js"; +import {ClientCode} from "../../../src/execution/index.js"; + +describe("util / metadata ", () => { + describe("getLodestarClientVersion", () => { + it("should return empty version and commit", () => { + const expected = {code: ClientCode.LS, name: "Lodestar", version: "", commit: ""}; + expect(getLodestarClientVersion()).toEqual(expected); + }); + it("should return full client info", () => { + const info = {version: "v0.36.0/80c248b", commit: "80c248bb392f512cc115d95059e22239a17bbd7d"}; // Version and long commit from readAndGetGitData() + const expected = {code: ClientCode.LS, name: "Lodestar", version: "v0.36.0/80c248b", commit: "80c248bb"}; + expect(getLodestarClientVersion(info)).toEqual(expected); + }); + }); +}); diff --git a/packages/cli/src/cmds/beacon/handler.ts b/packages/cli/src/cmds/beacon/handler.ts index bf9eb6cc96e0..4c30506915c9 100644 --- a/packages/cli/src/cmds/beacon/handler.ts +++ b/packages/cli/src/cmds/beacon/handler.ts @@ -173,7 +173,7 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { beaconNodeOptions.set({metrics: {metadata: {version, commit, network}}}); beaconNodeOptions.set({metrics: {validatorMonitorLogs: args.validatorMonitorLogs}}); // Add detailed version string for API node/version endpoint - beaconNodeOptions.set({api: {version}}); + beaconNodeOptions.set({api: {commit, version}}); // Combine bootnodes from different sources const bootnodes = (beaconNodeOptions.get().network?.discv5?.bootEnrs ?? []).concat( From 0734b9ec9fa840cb60e08d22b107142dbf653da9 Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Thu, 25 Jul 2024 20:57:53 +0900 Subject: [PATCH 22/31] Fix unit test --- .../beacon-node/src/execution/engine/http.ts | 34 ++++++++++++------- .../test/unit/executionEngine/http.test.ts | 1 + 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 2eb8bb9511c2..5515758ae043 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -74,6 +74,10 @@ export type ExecutionEngineHttpOpts = { * Lodestar commit to be used for `ClientVersion` */ commit?: string; + /* + * Disable client version fetch and update. Use in testing only + */ + disableClientVersionFetch?: boolean; }; export const defaultExecutionEngineHttpOpts: ExecutionEngineHttpOpts = { @@ -154,15 +158,19 @@ export class ExecutionEngineHttp implements IExecutionEngine { this.updateEngineState(getExecutionEngineState({payloadError: error, oldState: this.state})); }); - this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { - if (this.clientVersion === undefined) { - // This statement should only be called first time receiving response after start up - this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get client version", e); - }); - } - this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); - }); + if (!opts?.disableClientVersionFetch) { + this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { + if (this.clientVersion === undefined) { + // This statement should only be called first time receiving response after start up + this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { + this.logger.error("Unable to get client version", e); + }); + } + this.updateEngineState( + getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state}) + ); + }); + } } /** @@ -467,9 +475,11 @@ export class ExecutionEngineHttp implements IExecutionEngine { switch (newState) { case ExecutionEngineState.ONLINE: this.logger.info("Execution client became online", {oldState, newState}); - this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get client version", e); - }); + if (!this.opts?.disableClientVersionFetch) { + this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { + this.logger.error("Unable to get client version", e); + }); + } break; case ExecutionEngineState.OFFLINE: this.logger.error("Execution client went offline", {oldState, newState}); diff --git a/packages/beacon-node/test/unit/executionEngine/http.test.ts b/packages/beacon-node/test/unit/executionEngine/http.test.ts index 27e9b86887ee..f1de226ad4c0 100644 --- a/packages/beacon-node/test/unit/executionEngine/http.test.ts +++ b/packages/beacon-node/test/unit/executionEngine/http.test.ts @@ -47,6 +47,7 @@ describe("ExecutionEngine / http", () => { urls: [baseUrl], retries: defaultExecutionEngineHttpOpts.retries, retryDelay: defaultExecutionEngineHttpOpts.retryDelay, + disableClientVersionFetch: true, }, {signal: controller.signal, logger: console as unknown as Logger} ); From 44a3c8b3eee0734965d0d631e8a87bc6ff3d84bd Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 15:44:58 +0100 Subject: [PATCH 23/31] Review PR, mostly cosmetic --- .../beacon-node/src/api/impl/validator/index.ts | 14 +++++--------- packages/beacon-node/src/api/options.ts | 1 + packages/beacon-node/src/execution/engine/http.ts | 8 ++++---- .../beacon-node/src/execution/engine/interface.ts | 1 + packages/beacon-node/src/util/graffiti.ts | 10 +++------- .../beacon-node/test/unit/util/graffiti.test.ts | 6 +++--- .../beacon-node/test/unit/util/metadata.test.ts | 2 +- 7 files changed, 18 insertions(+), 24 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 999f2907c7f2..b2a0b8575f5c 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -400,14 +400,12 @@ export function getValidatorApi( let timer; try { timer = metrics?.blockProductionTime.startTimer(); - const consensusClientVersion = getLodestarClientVersion(opts); - const executionClientVersion = chain.executionEngine.clientVersion; const {block, executionPayloadValue, consensusBlockValue} = await chain.produceBlindedBlock({ slot, parentBlockRoot, randaoReveal, graffiti: toGraffitiBuffer( - graffiti ?? getDefaultGraffiti(opts, consensusClientVersion, executionClientVersion) + graffiti ?? getDefaultGraffiti(getLodestarClientVersion(opts), chain.executionEngine.clientVersion, opts) ), commonBlockBody, }); @@ -472,14 +470,12 @@ export function getValidatorApi( let timer; try { timer = metrics?.blockProductionTime.startTimer(); - const consensusClientVersion = getLodestarClientVersion(opts); - const executionClientVersion = chain.executionEngine.clientVersion; const {block, executionPayloadValue, consensusBlockValue, shouldOverrideBuilder} = await chain.produceBlock({ slot, parentBlockRoot, randaoReveal, graffiti: toGraffitiBuffer( - graffiti ?? getDefaultGraffiti(opts, consensusClientVersion, executionClientVersion) + graffiti ?? getDefaultGraffiti(getLodestarClientVersion(opts), chain.executionEngine.clientVersion, opts) ), feeRecipient, commonBlockBody, @@ -587,13 +583,13 @@ export function getValidatorApi( }; logger.verbose("Assembling block with produceEngineOrBuilderBlock", loggerContext); - const consensusClientVersion = getLodestarClientVersion(opts); - const executionClientVersion = chain.executionEngine.clientVersion; const commonBlockBody = await chain.produceCommonBlockBody({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti ?? getDefaultGraffiti(opts, consensusClientVersion, executionClientVersion)), + graffiti: toGraffitiBuffer( + graffiti ?? getDefaultGraffiti(getLodestarClientVersion(opts), chain.executionEngine.clientVersion, opts) + ), }); logger.debug("Produced common block body", loggerContext); diff --git a/packages/beacon-node/src/api/options.ts b/packages/beacon-node/src/api/options.ts index b7434c4ed82f..811621ba97bf 100644 --- a/packages/beacon-node/src/api/options.ts +++ b/packages/beacon-node/src/api/options.ts @@ -12,4 +12,5 @@ export const defaultApiOptions: ApiOptions = { maxGindicesInProof: 512, rest: beaconRestApiServerOpts, version: "dev", + private: false, }; diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 5515758ae043..c7c91138f42b 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -120,7 +120,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { // It's safer to to avoid false positives and assume that the EL is syncing until we receive the first payload state: ExecutionEngineState = ExecutionEngineState.ONLINE; - // Cached EL client version from the latest getClientVersion call + /** Cached EL client version from the latest getClientVersion call */ clientVersion?: ClientVersion; readonly payloadIdCache = new PayloadIdCache(); @@ -163,7 +163,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (this.clientVersion === undefined) { // This statement should only be called first time receiving response after start up this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get client version", e); + this.logger.error("Unable to get execution client version", {}, e); }); } this.updateEngineState( @@ -461,7 +461,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (clientVersions.length > 0) { this.clientVersion = clientVersions[0]; - this.logger.debug("Execution client version is updated", this.clientVersion); + this.logger.debug("Execution client version updated", this.clientVersion); } return clientVersions; @@ -477,7 +477,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { this.logger.info("Execution client became online", {oldState, newState}); if (!this.opts?.disableClientVersionFetch) { this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get client version", e); + this.logger.error("Unable to get execution client version", {}, e); }); } break; diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index 1df2b7a9e3f4..3a4df41cbe8b 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -119,6 +119,7 @@ export interface IExecutionEngine { readonly state: ExecutionEngineState; readonly clientVersion?: ClientVersion; + payloadIdCache: PayloadIdCache; /** * A state transition function which applies changes to the self.execution_state. diff --git a/packages/beacon-node/src/util/graffiti.ts b/packages/beacon-node/src/util/graffiti.ts index 17820b5cc2d5..fe78d993a408 100644 --- a/packages/beacon-node/src/util/graffiti.ts +++ b/packages/beacon-node/src/util/graffiti.ts @@ -9,18 +9,14 @@ export function toGraffitiBuffer(graffiti: string): Buffer { } export function getDefaultGraffiti( - opts: {private?: boolean}, - consensusClientVersion?: ClientVersion, - executionClientVersion?: ClientVersion + consensusClientVersion: ClientVersion, + executionClientVersion: ClientVersion | undefined, + opts: {private?: boolean} ): string { if (opts.private) { return ""; } - if (consensusClientVersion === undefined) { - throw new Error("Consensus client version must be provided if opts.private is set to false"); - } - if (executionClientVersion !== undefined) { const {code: executionCode, commit: executionCommit} = executionClientVersion; diff --git a/packages/beacon-node/test/unit/util/graffiti.test.ts b/packages/beacon-node/test/unit/util/graffiti.test.ts index 7675032e9d34..afc2fe18dc4d 100644 --- a/packages/beacon-node/test/unit/util/graffiti.test.ts +++ b/packages/beacon-node/test/unit/util/graffiti.test.ts @@ -38,17 +38,17 @@ describe("Graffiti helper", () => { }; // Sample output of getLodestarClientVersion() it("should return empty if private option is set", () => { - const result = getDefaultGraffiti({private: true}); + const result = getDefaultGraffiti(consensusClientVersion, executionClientVersion, {private: true}); expect(result).toBe(""); }); it("should return CL only info if EL client version is missing", () => { - const result = getDefaultGraffiti({private: false}, consensusClientVersion); + const result = getDefaultGraffiti(consensusClientVersion, executionClientVersion, {private: false}); expect(result).toBe("LS80c2"); }); it("should return combined version codes and commits if executionClientVersion is provided", () => { - const result = getDefaultGraffiti({private: false}, consensusClientVersion, executionClientVersion); + const result = getDefaultGraffiti(consensusClientVersion, executionClientVersion, {private: false}); expect(result).toBe("BU9b0eLS80c2"); }); }); diff --git a/packages/beacon-node/test/unit/util/metadata.test.ts b/packages/beacon-node/test/unit/util/metadata.test.ts index 7f8b8e44380a..4e732d93e5d0 100644 --- a/packages/beacon-node/test/unit/util/metadata.test.ts +++ b/packages/beacon-node/test/unit/util/metadata.test.ts @@ -2,7 +2,7 @@ import {describe, it, expect} from "vitest"; import {getLodestarClientVersion} from "../../../src/util/metadata.js"; import {ClientCode} from "../../../src/execution/index.js"; -describe("util / metadata ", () => { +describe("util / metadata", () => { describe("getLodestarClientVersion", () => { it("should return empty version and commit", () => { const expected = {code: ClientCode.LS, name: "Lodestar", version: "", commit: ""}; From 52edf2bac7bebf3674136af5d1ccb5c7cd730eba Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 16:01:36 +0100 Subject: [PATCH 24/31] Fix graffiti tests --- packages/beacon-node/test/unit/util/graffiti.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/test/unit/util/graffiti.test.ts b/packages/beacon-node/test/unit/util/graffiti.test.ts index afc2fe18dc4d..0197e39c5e52 100644 --- a/packages/beacon-node/test/unit/util/graffiti.test.ts +++ b/packages/beacon-node/test/unit/util/graffiti.test.ts @@ -43,7 +43,7 @@ describe("Graffiti helper", () => { }); it("should return CL only info if EL client version is missing", () => { - const result = getDefaultGraffiti(consensusClientVersion, executionClientVersion, {private: false}); + const result = getDefaultGraffiti(consensusClientVersion, undefined, {private: false}); expect(result).toBe("LS80c2"); }); From 35b8e95d85fbb8f4556b060c48a6b85565af5bb9 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 18:06:21 +0100 Subject: [PATCH 25/31] Add workaround to test code instead of src --- .../beacon-node/src/execution/engine/http.ts | 34 +++++++------------ .../test/unit/executionEngine/http.test.ts | 6 +++- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index c7c91138f42b..1a5322fff00d 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -74,10 +74,6 @@ export type ExecutionEngineHttpOpts = { * Lodestar commit to be used for `ClientVersion` */ commit?: string; - /* - * Disable client version fetch and update. Use in testing only - */ - disableClientVersionFetch?: boolean; }; export const defaultExecutionEngineHttpOpts: ExecutionEngineHttpOpts = { @@ -158,19 +154,15 @@ export class ExecutionEngineHttp implements IExecutionEngine { this.updateEngineState(getExecutionEngineState({payloadError: error, oldState: this.state})); }); - if (!opts?.disableClientVersionFetch) { - this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { - if (this.clientVersion === undefined) { - // This statement should only be called first time receiving response after start up - this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get execution client version", {}, e); - }); - } - this.updateEngineState( - getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state}) - ); - }); - } + this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { + if (this.clientVersion === undefined) { + // This statement should only be called first time receiving response after start up + this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { + this.logger.error("Unable to get execution client version", {}, e); + }); + } + this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); + }); } /** @@ -475,11 +467,9 @@ export class ExecutionEngineHttp implements IExecutionEngine { switch (newState) { case ExecutionEngineState.ONLINE: this.logger.info("Execution client became online", {oldState, newState}); - if (!this.opts?.disableClientVersionFetch) { - this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get execution client version", {}, e); - }); - } + this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { + this.logger.error("Unable to get execution client version", {}, e); + }); break; case ExecutionEngineState.OFFLINE: this.logger.error("Execution client went offline", {oldState, newState}); diff --git a/packages/beacon-node/test/unit/executionEngine/http.test.ts b/packages/beacon-node/test/unit/executionEngine/http.test.ts index f1de226ad4c0..aa33c7dbbc40 100644 --- a/packages/beacon-node/test/unit/executionEngine/http.test.ts +++ b/packages/beacon-node/test/unit/executionEngine/http.test.ts @@ -9,6 +9,7 @@ import { serializeExecutionPayload, serializeExecutionPayloadBody, } from "../../../src/execution/engine/types.js"; +import {RpcPayload} from "../../../src/eth1/interface.js"; import {numToQuantity} from "../../../src/eth1/provider/utils.js"; describe("ExecutionEngine / http", () => { @@ -29,6 +30,10 @@ describe("ExecutionEngine / http", () => { const server = fastify({logger: false}); server.post("/", async (req) => { + if ((req.body as RpcPayload).method === "engine_getClientVersionV1") { + // Ignore client version requests + return []; + } reqJsonRpcPayload = req.body; delete (reqJsonRpcPayload as {id?: number}).id; return returnValue; @@ -47,7 +52,6 @@ describe("ExecutionEngine / http", () => { urls: [baseUrl], retries: defaultExecutionEngineHttpOpts.retries, retryDelay: defaultExecutionEngineHttpOpts.retryDelay, - disableClientVersionFetch: true, }, {signal: controller.signal, logger: console as unknown as Logger} ); From 5fd6b4fef4badc085150765b23c2c7fa207f7810 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 18:51:46 +0100 Subject: [PATCH 26/31] Set client version to null if not supported by EL --- packages/beacon-node/src/execution/engine/http.ts | 8 ++++++-- packages/beacon-node/src/execution/engine/interface.ts | 2 +- packages/beacon-node/src/util/graffiti.ts | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 1a5322fff00d..0b226ef30ea2 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -117,7 +117,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { state: ExecutionEngineState = ExecutionEngineState.ONLINE; /** Cached EL client version from the latest getClientVersion call */ - clientVersion?: ClientVersion; + clientVersion?: ClientVersion | null; readonly payloadIdCache = new PayloadIdCache(); /** @@ -159,6 +159,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { // This statement should only be called first time receiving response after start up this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { this.logger.error("Unable to get execution client version", {}, e); + this.clientVersion = null; }); } this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); @@ -454,6 +455,8 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (clientVersions.length > 0) { this.clientVersion = clientVersions[0]; this.logger.debug("Execution client version updated", this.clientVersion); + } else { + this.clientVersion = null; } return clientVersions; @@ -468,7 +471,8 @@ export class ExecutionEngineHttp implements IExecutionEngine { case ExecutionEngineState.ONLINE: this.logger.info("Execution client became online", {oldState, newState}); this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get execution client version", {}, e); + this.logger.debug("Unable to get execution client version", {}, e); + this.clientVersion = null; }); break; case ExecutionEngineState.OFFLINE: diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts index 3a4df41cbe8b..fa1da210cd12 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -118,7 +118,7 @@ export type VersionedHashes = Uint8Array[]; export interface IExecutionEngine { readonly state: ExecutionEngineState; - readonly clientVersion?: ClientVersion; + readonly clientVersion?: ClientVersion | null; payloadIdCache: PayloadIdCache; /** diff --git a/packages/beacon-node/src/util/graffiti.ts b/packages/beacon-node/src/util/graffiti.ts index fe78d993a408..9a4bc3d9689b 100644 --- a/packages/beacon-node/src/util/graffiti.ts +++ b/packages/beacon-node/src/util/graffiti.ts @@ -10,14 +10,14 @@ export function toGraffitiBuffer(graffiti: string): Buffer { export function getDefaultGraffiti( consensusClientVersion: ClientVersion, - executionClientVersion: ClientVersion | undefined, + executionClientVersion: ClientVersion | null | undefined, opts: {private?: boolean} ): string { if (opts.private) { return ""; } - if (executionClientVersion !== undefined) { + if (executionClientVersion != null) { const {code: executionCode, commit: executionCommit} = executionClientVersion; // Follow the 2-byte commit format in https://github.com/ethereum/execution-apis/pull/517#issuecomment-1918512560 From 85389884e0ecea1fed444873b5a3a6a6f141c67b Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 18:52:59 +0100 Subject: [PATCH 27/31] Log failed client version updates as debug --- packages/beacon-node/src/execution/engine/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 0b226ef30ea2..eb9f32da48f7 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -158,7 +158,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { if (this.clientVersion === undefined) { // This statement should only be called first time receiving response after start up this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { - this.logger.error("Unable to get execution client version", {}, e); + this.logger.debug("Unable to get execution client version", {}, e); this.clientVersion = null; }); } From 1c8352ee372685df01180e711a2ed8f18a78578c Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 19:10:33 +0100 Subject: [PATCH 28/31] Throw error if EL client returns empty client versions array --- packages/beacon-node/src/execution/engine/http.ts | 12 ++++++------ packages/beacon-node/src/execution/engine/mock.ts | 10 ++++++++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index eb9f32da48f7..218d9eed707b 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -156,7 +156,7 @@ export class ExecutionEngineHttp implements IExecutionEngine { this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { if (this.clientVersion === undefined) { - // This statement should only be called first time receiving response after start up + // This statement should only be called first time receiving response after startup this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { this.logger.debug("Unable to get execution client version", {}, e); this.clientVersion = null; @@ -452,13 +452,13 @@ export class ExecutionEngineHttp implements IExecutionEngine { return {code, name: cv.name, version: cv.version, commit: cv.commit}; }); - if (clientVersions.length > 0) { - this.clientVersion = clientVersions[0]; - this.logger.debug("Execution client version updated", this.clientVersion); - } else { - this.clientVersion = null; + if (clientVersions.length === 0) { + throw Error("Received empty client versions array"); } + this.clientVersion = clientVersions[0]; + this.logger.debug("Execution client version updated", this.clientVersion); + return clientVersions; } diff --git a/packages/beacon-node/src/execution/engine/mock.ts b/packages/beacon-node/src/execution/engine/mock.ts index 11d47735a7d0..096f45569b6a 100644 --- a/packages/beacon-node/src/execution/engine/mock.ts +++ b/packages/beacon-node/src/execution/engine/mock.ts @@ -24,7 +24,7 @@ import { BlobsBundleRpc, ExecutionPayloadBodyRpc, } from "./types.js"; -import {ExecutionPayloadStatus, PayloadIdCache} from "./interface.js"; +import {ClientCode, ExecutionPayloadStatus, PayloadIdCache} from "./interface.js"; import {JsonRpcBackend} from "./utils.js"; const INTEROP_GAS_LIMIT = 30e6; @@ -96,7 +96,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend { engine_getPayloadV3: this.getPayload.bind(this), engine_getPayloadBodiesByHashV1: this.getPayloadBodiesByHash.bind(this), engine_getPayloadBodiesByRangeV1: this.getPayloadBodiesByRange.bind(this), - engine_getClientVersionV1: () => [], + engine_getClientVersionV1: this.getClientVersionV1.bind(this), }; } @@ -387,6 +387,12 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend { return payload.executionPayload; } + private getClientVersionV1( + _blockHex: EngineApiRpcParamTypes["engine_getClientVersionV1"][0] + ): EngineApiRpcReturnTypes["engine_getClientVersionV1"] { + return [{code: ClientCode.XX, name: "mock", version: "", commit: ""}]; + } + private timestampToFork(timestamp: number): ForkExecution { if (timestamp > (this.opts.denebForkTimestamp ?? Infinity)) return ForkName.deneb; if (timestamp > (this.opts.capellaForkTimestamp ?? Infinity)) return ForkName.capella; From c78179796364f1e6b0834596f6fc395eedcf950c Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 19:19:23 +0100 Subject: [PATCH 29/31] Update engine mock --- packages/beacon-node/src/execution/engine/mock.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/execution/engine/mock.ts b/packages/beacon-node/src/execution/engine/mock.ts index 096f45569b6a..a99a76508df8 100644 --- a/packages/beacon-node/src/execution/engine/mock.ts +++ b/packages/beacon-node/src/execution/engine/mock.ts @@ -388,7 +388,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend { } private getClientVersionV1( - _blockHex: EngineApiRpcParamTypes["engine_getClientVersionV1"][0] + _clientVersion: EngineApiRpcParamTypes["engine_getClientVersionV1"][0] ): EngineApiRpcReturnTypes["engine_getClientVersionV1"] { return [{code: ClientCode.XX, name: "mock", version: "", commit: ""}]; } From 038e9802af0d8d45a1f81108757c0730c8cb6429 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 23:08:49 +0100 Subject: [PATCH 30/31] Set client version to null initially to avoid fetching multiple times --- packages/beacon-node/src/execution/engine/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index 218d9eed707b..bb868572c28d 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -159,8 +159,8 @@ export class ExecutionEngineHttp implements IExecutionEngine { // This statement should only be called first time receiving response after startup this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { this.logger.debug("Unable to get execution client version", {}, e); - this.clientVersion = null; }); + this.clientVersion = null; } this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); }); From f2736320252ff3a957fbcd79118a5dd8a73f9c33 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 29 Jul 2024 23:10:51 +0100 Subject: [PATCH 31/31] Reorder statements --- packages/beacon-node/src/execution/engine/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts index bb868572c28d..c64a9715589f 100644 --- a/packages/beacon-node/src/execution/engine/http.ts +++ b/packages/beacon-node/src/execution/engine/http.ts @@ -156,11 +156,11 @@ export class ExecutionEngineHttp implements IExecutionEngine { this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => { if (this.clientVersion === undefined) { + this.clientVersion = null; // This statement should only be called first time receiving response after startup this.getClientVersion(getLodestarClientVersion(this.opts)).catch((e) => { this.logger.debug("Unable to get execution client version", {}, e); }); - this.clientVersion = null; } this.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); });