From 2f9275ec13d12b37ed54884642bc628592b6ec78 Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Wed, 31 Jul 2024 01:04:16 +0900 Subject: [PATCH] feat: include EL client info in graffiti (#6753) * Define ClientCode and engine_getClientVersionV1 * Default graffiti in beacon node * Update packages/beacon-node/src/api/impl/validator/index.ts Co-authored-by: Nico Flaig * Fix rebase * Make graffiti optional in validator store * Fix merge * Fix lint * Update packages/beacon-node/src/execution/engine/types.ts Co-authored-by: Cayman * Add fallback graffiti * Address comment * Address comment * Cache client version in ExecutionEngine * Hide graffiti if private flag is set * Improve readability * Partially address comment * Partially address comment * Partially address comment * Refactor * Update packages/beacon-node/src/execution/engine/http.ts Co-authored-by: Nico Flaig * Partial address comment * Add unit test * Fix unit test * Review PR, mostly cosmetic * Fix graffiti tests * Add workaround to test code instead of src * Set client version to null if not supported by EL * Log failed client version updates as debug * Throw error if EL client returns empty client versions array * Update engine mock * Set client version to null initially to avoid fetching multiple times * Reorder statements --------- Co-authored-by: Nico Flaig Co-authored-by: Cayman --- packages/api/src/beacon/routes/validator.ts | 12 ++--- packages/api/src/utils/serdes.ts | 11 +++- packages/beacon-node/src/api/impl/api.ts | 2 +- .../src/api/impl/validator/index.ts | 34 +++++++------ packages/beacon-node/src/api/options.ts | 3 ++ .../beacon-node/src/execution/engine/http.ts | 51 ++++++++++++++++++- .../beacon-node/src/execution/engine/index.ts | 2 +- .../src/execution/engine/interface.ts | 29 +++++++++++ .../beacon-node/src/execution/engine/mock.ts | 9 +++- .../beacon-node/src/execution/engine/types.ts | 18 +++++++ packages/beacon-node/src/util/graffiti.ts | 21 ++++++++ packages/beacon-node/src/util/metadata.ts | 10 ++++ .../test/mocks/mockedBeaconChain.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 +- .../test/unit/executionEngine/http.test.ts | 5 ++ .../test/unit/util/graffiti.test.ts | 28 +++++++++- .../test/unit/util/metadata.test.ts | 17 +++++++ packages/cli/src/cmds/beacon/handler.ts | 6 ++- packages/cli/src/cmds/validator/handler.ts | 4 +- .../cli/src/cmds/validator/keymanager/impl.ts | 6 ++- packages/cli/src/cmds/validator/options.ts | 1 - packages/cli/src/util/graffiti.ts | 18 ------- packages/cli/src/util/index.ts | 1 - packages/validator/src/services/block.ts | 4 +- .../validator/src/services/validatorStore.ts | 6 +-- 28 files changed, 249 insertions(+), 62 deletions(-) create mode 100644 packages/beacon-node/src/util/metadata.ts create mode 100644 packages/beacon-node/test/unit/util/metadata.test.ts 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 32a76a536d8..33161ec789e 100644 --- a/packages/api/src/beacon/routes/validator.ts +++ b/packages/api/src/beacon/routes/validator.ts @@ -304,13 +304,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; @@ -333,7 +333,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, @@ -341,7 +341,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; @@ -359,9 +359,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}}, BlindedBeaconBlock, VersionMeta >; diff --git a/packages/api/src/utils/serdes.ts b/packages/api/src/utils/serdes.ts index 73196c917a6..233d7db9e7f 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,7 +97,10 @@ export function toGraffitiHex(utf8: string): string { return hex; } -export function fromGraffitiHex(hex: string): string { +export function fromGraffitiHex(hex?: string): string | undefined { + if (hex === undefined) { + return undefined; + } try { return new TextDecoder("utf8").decode(fromHexString(hex)); } catch { diff --git a/packages/beacon-node/src/api/impl/api.ts b/packages/beacon-node/src/api/impl/api.ts index 6ec7180cf0f..3647feed2d7 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 486fc6e8062..b2a0b8575f5 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 {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"; @@ -63,6 +63,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 {ApiOptions} from "../../options.js"; +import {getLodestarClientVersion} from "../../../util/metadata.js"; import {computeSubnetForCommitteesAtSlot, getPubkeysForIndices, selectBlockProductionSource} from "./utils.js"; /** @@ -110,14 +112,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; /** @@ -348,7 +346,7 @@ export function getValidatorApi({ 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, @@ -406,7 +404,9 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti || ""), + graffiti: toGraffitiBuffer( + graffiti ?? getDefaultGraffiti(getLodestarClientVersion(opts), chain.executionEngine.clientVersion, opts) + ), commonBlockBody, }); @@ -432,7 +432,7 @@ export function getValidatorApi({ async function produceEngineFullBlockOrContents( slot: Slot, randaoReveal: BLSSignature, - graffiti: string, + graffiti?: string, { feeRecipient, strictFeeRecipientCheck, @@ -474,7 +474,9 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti || ""), + graffiti: toGraffitiBuffer( + graffiti ?? getDefaultGraffiti(getLodestarClientVersion(opts), chain.executionEngine.clientVersion, opts) + ), feeRecipient, commonBlockBody, }); @@ -522,7 +524,7 @@ export function getValidatorApi({ async function produceEngineOrBuilderBlock( slot: Slot, randaoReveal: BLSSignature, - graffiti: string, + graffiti?: string, // TODO deneb: skip randao verification _skipRandaoVerification?: boolean, builderBoostFactor?: bigint, @@ -585,7 +587,9 @@ export function getValidatorApi({ slot, parentBlockRoot, randaoReveal, - graffiti: toGraffitiBuffer(graffiti || ""), + 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 4b9aed7e0d9..811621ba97b 100644 --- a/packages/beacon-node/src/api/options.ts +++ b/packages/beacon-node/src/api/options.ts @@ -3,11 +3,14 @@ import {beaconRestApiServerOpts, BeaconRestApiServerOpts} from "./rest/index.js" export type ApiOptions = { maxGindicesInProof?: number; rest: BeaconRestApiServerOpts; + commit?: string; version?: string; + private?: boolean; }; 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 c57d5f9b2db..c64a9715589 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/metadata.js"; import { ExecutionPayloadStatus, ExecutePayloadResponse, @@ -21,6 +22,8 @@ import { BlobsBundle, VersionedHashes, ExecutionEngineState, + ClientVersion, + ClientCode, } from "./interface.js"; import {PayloadIdCache} from "./payloadIdCache.js"; import { @@ -63,6 +66,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 = { @@ -105,6 +116,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 state: ExecutionEngineState = ExecutionEngineState.ONLINE; + /** Cached EL client version from the latest getClientVersion call */ + clientVersion?: ClientVersion | null; + readonly payloadIdCache = new PayloadIdCache(); /** * A queue to serialize the fcUs and newPayloads calls: @@ -126,7 +140,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, @@ -140,6 +155,13 @@ 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.updateEngineState(getExecutionEngineState({targetState: ExecutionEngineState.ONLINE, oldState: this.state})); }); } @@ -417,6 +439,29 @@ export class ExecutionEngineHttp implements IExecutionEngine { return response.map(deserializeExecutionPayloadBody); } + private async getClientVersion(clientVersion: ClientVersion): Promise { + const method = "engine_getClientVersionV1"; + + const response = await this.rpc.fetchWithRetries< + EngineApiRpcReturnTypes[typeof method], + EngineApiRpcParamTypes[typeof method] + >({method, params: [clientVersion]}); + + 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}; + }); + + 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; + } + private updateEngineState(newState: ExecutionEngineState): void { const oldState = this.state; @@ -425,6 +470,10 @@ 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.debug("Unable to get execution client version", {}, e); + this.clientVersion = null; + }); 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 581933d2429..a8b878502af 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(toPrintableUrl).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 13575f01bce..fa1da210cd1 100644 --- a/packages/beacon-node/src/execution/engine/interface.ts +++ b/packages/beacon-node/src/execution/engine/interface.ts @@ -38,6 +38,26 @@ 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 = "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 = | { status: ExecutionPayloadStatus.SYNCING | ExecutionPayloadStatus.ACCEPTED; @@ -80,6 +100,13 @@ export type BlobsBundle = { proofs: KZGProof[]; }; +export type ClientVersion = { + code: ClientCode; + name: string; + version: string; + commit: string; +}; + export type VersionedHashes = Uint8Array[]; /** @@ -91,6 +118,8 @@ export type VersionedHashes = Uint8Array[]; export interface IExecutionEngine { readonly state: ExecutionEngineState; + readonly clientVersion?: ClientVersion | null; + payloadIdCache: PayloadIdCache; /** * A state transition function which applies changes to the self.execution_state. diff --git a/packages/beacon-node/src/execution/engine/mock.ts b/packages/beacon-node/src/execution/engine/mock.ts index 5779713435a..a99a76508df 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,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: this.getClientVersionV1.bind(this), }; } @@ -386,6 +387,12 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend { return payload.executionPayload; } + private getClientVersionV1( + _clientVersion: 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; diff --git a/packages/beacon-node/src/execution/engine/types.ts b/packages/beacon-node/src/execution/engine/types.ts index 9fe9a990f76..85f514c953b 100644 --- a/packages/beacon-node/src/execution/engine/types.ts +++ b/packages/beacon-node/src/execution/engine/types.ts @@ -62,6 +62,11 @@ export type EngineApiRpcParamTypes = { * 2. count: QUANTITY, 64 bits - Number of blocks to return */ engine_getPayloadBodiesByRangeV1: [start: QUANTITY, count: QUANTITY]; + + /** + * Object - Instance of ClientVersion + */ + 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,17 @@ export type PayloadAttributesRpc = { parentBeaconBlockRoot?: DATA; }; +export type ClientVersionRpc = { + /** 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 build */ + commit: DATA; +}; + export interface BlobsBundleRpc { commitments: DATA[]; // each 48 bytes blobs: DATA[]; // each 4096 * 32 = 131072 bytes diff --git a/packages/beacon-node/src/util/graffiti.ts b/packages/beacon-node/src/util/graffiti.ts index 514be4aa5bd..9a4bc3d9689 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 {ClientVersion} from "../execution/index.js"; /** * Parses a graffiti UTF8 string and returns a 32 bytes buffer right padded with zeros @@ -6,3 +7,23 @@ 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 getDefaultGraffiti( + consensusClientVersion: ClientVersion, + executionClientVersion: ClientVersion | null | undefined, + opts: {private?: boolean} +): string { + if (opts.private) { + return ""; + } + + 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 + 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.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 00000000000..9e6d2071010 --- /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/mocks/mockedBeaconChain.ts b/packages/beacon-node/test/mocks/mockedBeaconChain.ts index b63eb11435c..22a48ebd050 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/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 d4f7705fb25..b101382e01a 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 256d772d5fc..84872ca6045 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 0868834e619..a23373938f6 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 851bc9d0f33..a7299aa3b95 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/beacon-node/test/unit/executionEngine/http.test.ts b/packages/beacon-node/test/unit/executionEngine/http.test.ts index 27e9b86887e..aa33c7dbbc4 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; diff --git a/packages/beacon-node/test/unit/util/graffiti.test.ts b/packages/beacon-node/test/unit/util/graffiti.test.ts index b1338181b32..0197e39c5e5 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(consensusClientVersion, executionClientVersion, {private: true}); + expect(result).toBe(""); + }); + + it("should return CL only info if EL client version is missing", () => { + const result = getDefaultGraffiti(consensusClientVersion, undefined, {private: false}); + expect(result).toBe("LS80c2"); + }); + + it("should return combined version codes and commits if executionClientVersion is provided", () => { + 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 new file mode 100644 index 00000000000..4e732d93e5d --- /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 42128b45e4a..5e132c89eb4 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( @@ -199,7 +199,7 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { } 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]; @@ -209,6 +209,8 @@ export async function beaconHandlerInit(args: BeaconArgs & GlobalArgs) { beaconNodeOptions.set({executionBuilder: {userAgent: versionStr}}); // Set jwt version with version string beaconNodeOptions.set({executionEngine: {jwtVersion: versionStr}, eth1: {jwtVersion: versionStr}}); + // Set commit and version for ClientVersion + beaconNodeOptions.set({executionEngine: {commit, version}}); } // Render final options diff --git a/packages/cli/src/cmds/validator/handler.ts b/packages/cli/src/cmds/validator/handler.ts index fdd93f1ebfc..01c15126b79 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 ?? getDefaultGraffiti(), + 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 b3b7c9f458b..2915c125eac 100644 --- a/packages/cli/src/cmds/validator/keymanager/impl.ts +++ b/packages/cli/src/cmds/validator/keymanager/impl.ts @@ -60,7 +60,11 @@ 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(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 fc7cb197c5a..aaa0e96d25a 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/packages/cli/src/cmds/validator/options.ts @@ -200,7 +200,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", }, diff --git a/packages/cli/src/util/graffiti.ts b/packages/cli/src/util/graffiti.ts deleted file mode 100644 index 1d859ea8d9b..00000000000 --- 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/cli/src/util/index.ts b/packages/cli/src/util/index.ts index 3d94977f5fb..579587317e0 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"; diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index 661ff47440d..a9dd7654a8f 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -207,7 +207,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 @@ -245,7 +245,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 b1564d6e7d2..53299463ad2 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -66,7 +66,7 @@ export type SignerRemote = { }; type DefaultProposerConfig = { - graffiti: string; + graffiti?: string; strictFeeRecipientCheck: boolean; feeRecipient: Eth1Address; builder: { @@ -167,7 +167,7 @@ export class ValidatorStore { } this.defaultProposerConfig = { - graffiti: defaultConfig.graffiti ?? "", + graffiti: defaultConfig.graffiti, strictFeeRecipientCheck: defaultConfig.strictFeeRecipientCheck ?? false, feeRecipient: defaultConfig.feeRecipient ?? defaultOptions.suggestedFeeRecipient, builder: { @@ -244,7 +244,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; }