From 660356bbd8e9788cf961ee1d070d7e12db493c20 Mon Sep 17 00:00:00 2001 From: Derek Guenther Date: Fri, 25 Apr 2025 15:35:04 -0400 Subject: [PATCH 1/3] Return Fulu metadata from beacon API --- packages/api/src/beacon/routes/node.ts | 2 +- .../api/test/unit/beacon/testData/node.ts | 2 +- .../src/network/core/networkCore.ts | 2 +- packages/beacon-node/src/network/metadata.ts | 16 ++++++++-------- .../src/network/peers/peerManager.ts | 19 ++++++++++--------- .../onWorker/dataSerialization.test.ts | 2 +- .../e2e/network/peers/peerManager.test.ts | 6 +++--- .../test/unit/network/metadata.test.ts | 2 +- packages/types/src/fulu/sszTypes.ts | 2 +- 9 files changed, 27 insertions(+), 26 deletions(-) diff --git a/packages/api/src/beacon/routes/node.ts b/packages/api/src/beacon/routes/node.ts index 028afd25df83..f255773e39c3 100644 --- a/packages/api/src/beacon/routes/node.ts +++ b/packages/api/src/beacon/routes/node.ts @@ -25,7 +25,7 @@ export const NetworkIdentityType = new ContainerType( p2pAddresses: ArrayOf(stringType), discoveryAddresses: ArrayOf(stringType), /** Based on Ethereum Consensus [Metadata object](https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/phase0/p2p-interface.md#metadata) */ - metadata: ssz.altair.Metadata, + metadata: ssz.fulu.Metadata, }, {jsonCase: "eth2"} ); diff --git a/packages/api/test/unit/beacon/testData/node.ts b/packages/api/test/unit/beacon/testData/node.ts index e46aa3e28850..2b0b5effe059 100644 --- a/packages/api/test/unit/beacon/testData/node.ts +++ b/packages/api/test/unit/beacon/testData/node.ts @@ -20,7 +20,7 @@ export const testData: GenericServerTestCases = { enr: "enr", p2pAddresses: ["p2pAddresses"], discoveryAddresses: ["discoveryAddresses"], - metadata: ssz.altair.Metadata.defaultValue(), + metadata: ssz.fulu.Metadata.defaultValue(), }, }, }, diff --git a/packages/beacon-node/src/network/core/networkCore.ts b/packages/beacon-node/src/network/core/networkCore.ts index d9033f95327a..a0c2d94648bf 100644 --- a/packages/beacon-node/src/network/core/networkCore.ts +++ b/packages/beacon-node/src/network/core/networkCore.ts @@ -365,7 +365,7 @@ export class NetworkCore implements INetworkCore { async setAdvertisedGroupCount(count: number): Promise { this.networkConfig.setAdvertisedGroupCount(count); - this.metadata.cgc = count; + this.metadata.custodyGroupCount = count; } // REST API queries diff --git a/packages/beacon-node/src/network/metadata.ts b/packages/beacon-node/src/network/metadata.ts index 146c1f491ff3..1b3c72d6e4b2 100644 --- a/packages/beacon-node/src/network/metadata.ts +++ b/packages/beacon-node/src/network/metadata.ts @@ -44,7 +44,7 @@ export class MetadataController { this.onSetValue = modules.onSetValue; this._metadata = opts.metadata ?? { ...ssz.fulu.Metadata.defaultValue(), - cgc: modules.networkConfig.getCustodyConfig().advertisedCustodyGroupCount, + custodyGroupCount: modules.networkConfig.getCustodyConfig().advertisedCustodyGroupCount, }; } @@ -63,7 +63,7 @@ export class MetadataController { } // Set CGC regardless of fork. It may be useful to clients before Fulu, and will be ignored otherwise. - this.onSetValue(ENRKey.cgc, serializeCgc(this._metadata.cgc)); + this.onSetValue(ENRKey.cgc, serializeCgc(this._metadata.custodyGroupCount)); } get seqNumber(): bigint { @@ -89,16 +89,16 @@ export class MetadataController { this._metadata.attnets = attnets; } - get cgc(): number { - return this._metadata.cgc; + get custodyGroupCount(): number { + return this._metadata.custodyGroupCount; } - set cgc(cgc: number) { - if (cgc === this._metadata.cgc) { + set custodyGroupCount(custodyGroupCount: number) { + if (custodyGroupCount === this._metadata.custodyGroupCount) { return; } - this.onSetValue(ENRKey.cgc, serializeCgc(cgc)); - this._metadata.cgc = cgc; + this.onSetValue(ENRKey.cgc, serializeCgc(custodyGroupCount)); + this._metadata.custodyGroupCount = custodyGroupCount; } /** Consumers that need the phase0.Metadata type can just ignore the .syncnets property */ diff --git a/packages/beacon-node/src/network/peers/peerManager.ts b/packages/beacon-node/src/network/peers/peerManager.ts index 5bd9c34590c4..b6a859831df4 100644 --- a/packages/beacon-node/src/network/peers/peerManager.ts +++ b/packages/beacon-node/src/network/peers/peerManager.ts @@ -328,27 +328,28 @@ export class PeerManager { this.logger.warn("onMetadata", { peer: peer.toString(), peerData: peerData !== undefined, - cgc: (metadata as Partial).cgc, + custodyGroupCount: (metadata as Partial).custodyGroupCount, }); if (peerData) { const oldMetadata = peerData.metadata; - const cgc = (metadata as Partial).cgc ?? this.config.CUSTODY_REQUIREMENT; + const custodyGroupCount = + (metadata as Partial).custodyGroupCount ?? this.config.CUSTODY_REQUIREMENT; const nodeId = peerData?.nodeId ?? computeNodeId(peer); const custodyGroups = - oldMetadata == null || oldMetadata.custodyGroups == null || cgc !== oldMetadata.cgc - ? getCustodyGroups(nodeId, cgc) + oldMetadata == null || oldMetadata.custodyGroups == null || custodyGroupCount !== oldMetadata.custodyGroupCount + ? getCustodyGroups(nodeId, custodyGroupCount) : oldMetadata.custodyGroups; peerData.metadata = { seqNumber: metadata.seqNumber, attnets: metadata.attnets, syncnets: (metadata as Partial).syncnets ?? BitArray.fromBitLen(SYNC_COMMITTEE_SUBNET_COUNT), - cgc: - (metadata as Partial).cgc ?? + custodyGroupCount: + (metadata as Partial).custodyGroupCount ?? // TODO: spec says that Clients MAY reject peers with a value less than CUSTODY_REQUIREMENT this.config.CUSTODY_REQUIREMENT, custodyGroups, }; - if (oldMetadata === null || oldMetadata.cgc !== peerData.metadata.cgc) { + if (oldMetadata === null || oldMetadata.custodyGroupCount !== peerData.metadata.custodyGroupCount) { void this.requestStatus(peer, this.statusCache.get()); } } @@ -417,7 +418,7 @@ export class PeerManager { } if (getConnection(this.libp2p, peer.toString())) { const nodeId = peerData?.nodeId ?? computeNodeId(peer); - const custodyGroupCount = peerData?.metadata?.cgc; + const custodyGroupCount = peerData?.metadata?.custodyGroupCount; const peerCustodyGroupCount = custodyGroupCount ?? this.config.CUSTODY_REQUIREMENT; const dataColumns = getDataColumns(nodeId, peerCustodyGroupCount); @@ -805,7 +806,7 @@ export class PeerManager { // TODO: Consider optimizing by doing observe in batch metrics.peerLongLivedAttnets.observe(attnets ? attnets.getTrueBitIndexes().length : 0); - metrics.peerColumnGroupCount.observe(peerData?.metadata?.cgc ?? 0); + metrics.peerColumnGroupCount.observe(peerData?.metadata?.custodyGroupCount ?? 0); metrics.peerScoreByClient.observe({client}, this.peerRpcScores.getScore(peerId)); metrics.peerGossipScoreByClient.observe({client}, this.peerRpcScores.getGossipScore(peerId)); metrics.peerConnectionLength.observe((now - openCnx.timeline.open) / 1000); diff --git a/packages/beacon-node/test/e2e/network/onWorker/dataSerialization.test.ts b/packages/beacon-node/test/e2e/network/onWorker/dataSerialization.test.ts index 73422e8a346b..06e2f4f26aff 100644 --- a/packages/beacon-node/test/e2e/network/onWorker/dataSerialization.test.ts +++ b/packages/beacon-node/test/e2e/network/onWorker/dataSerialization.test.ts @@ -173,7 +173,7 @@ describe("data serialization through worker boundary", () => { enr: "test-enr", p2pAddresses: ["/ip4/1.2.3.4/tcp/0"], discoveryAddresses: ["/ip4/1.2.3.4/tcp/0"], - metadata: ssz.altair.Metadata.defaultValue(), + metadata: ssz.fulu.Metadata.defaultValue(), }, subscribeGossipCoreTopics: null, unsubscribeGossipCoreTopics: null, diff --git a/packages/beacon-node/test/e2e/network/peers/peerManager.test.ts b/packages/beacon-node/test/e2e/network/peers/peerManager.test.ts index 49acc07b4e8f..cf3c09cb77c3 100644 --- a/packages/beacon-node/test/e2e/network/peers/peerManager.test.ts +++ b/packages/beacon-node/test/e2e/network/peers/peerManager.test.ts @@ -188,13 +188,13 @@ describe("network / peers / PeerManager", () => { // Simulate peer1 returning a PING and STATUS message const remoteStatus = statusCache.get(); - const cgc = config.CUSTODY_REQUIREMENT; + const custodyGroupCount = config.CUSTODY_REQUIREMENT; const remoteMetadata: NonNullable>["metadata"] = { seqNumber: BigInt(1), attnets: getAttnets(), syncnets: getSyncnets(), - cgc, - custodyGroups: getCustodyGroups(computeNodeId(peerId1), cgc), + custodyGroupCount, + custodyGroups: getCustodyGroups(computeNodeId(peerId1), custodyGroupCount), }; reqResp.sendPing.mockResolvedValue(remoteMetadata.seqNumber); reqResp.sendStatus.mockResolvedValue(remoteStatus); diff --git a/packages/beacon-node/test/unit/network/metadata.test.ts b/packages/beacon-node/test/unit/network/metadata.test.ts index 204f22cb8c75..ff77095f149e 100644 --- a/packages/beacon-node/test/unit/network/metadata.test.ts +++ b/packages/beacon-node/test/unit/network/metadata.test.ts @@ -54,7 +54,7 @@ describe("network / metadata", () => { const onSetValue = vi.fn(); const networkConfig = new NetworkConfig(getValidPeerId(), config); const metadata = new MetadataController({}, {onSetValue, networkConfig}); - metadata.cgc = 128; + metadata.custodyGroupCount = 128; expect(onSetValue).toHaveBeenCalledWith(ENRKey.cgc, serializeCgc(128)); }); }); diff --git a/packages/types/src/fulu/sszTypes.ts b/packages/types/src/fulu/sszTypes.ts index dea952738b5b..088cb6e0cee5 100644 --- a/packages/types/src/fulu/sszTypes.ts +++ b/packages/types/src/fulu/sszTypes.ts @@ -22,7 +22,7 @@ export const Blob = denebSsz.Blob; export const Metadata = new ContainerType( { ...altariSsz.Metadata.fields, - cgc: UintNum64, + custodyGroupCount: UintNum64, }, {typeName: "Metadata", jsonCase: "eth2"} ); From cffe46263955794babc34f089492ded34fe3fd95 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sat, 26 Apr 2025 20:36:56 +0100 Subject: [PATCH 2/3] Add workaround to allow custody_group_count field to be omitted --- packages/api/src/beacon/routes/node.ts | 27 ++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/api/src/beacon/routes/node.ts b/packages/api/src/beacon/routes/node.ts index f255773e39c3..9ce947f45960 100644 --- a/packages/api/src/beacon/routes/node.ts +++ b/packages/api/src/beacon/routes/node.ts @@ -1,6 +1,6 @@ import {ContainerType, ValueOf} from "@chainsafe/ssz"; import {ChainForkConfig} from "@lodestar/config"; -import {ssz, stringType} from "@lodestar/types"; +import {ssz, stringType, fulu} from "@lodestar/types"; import { ArrayOf, EmptyArgs, @@ -24,8 +24,9 @@ export const NetworkIdentityType = new ContainerType( enr: stringType, p2pAddresses: ArrayOf(stringType), discoveryAddresses: ArrayOf(stringType), + // TODO Fulu: replace with `ssz.fulu.Metadata` once `custody_group_count` is more widely supported /** Based on Ethereum Consensus [Metadata object](https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/phase0/p2p-interface.md#metadata) */ - metadata: ssz.fulu.Metadata, + metadata: ssz.altair.Metadata, }, {jsonCase: "eth2"} ); @@ -56,7 +57,9 @@ export const SyncingStatusType = new ContainerType( {jsonCase: "eth2"} ); -export type NetworkIdentity = ValueOf; +export type NetworkIdentity = ValueOf & { + metadata: Partial; +}; export type PeerState = "disconnected" | "connecting" | "connected" | "disconnecting"; export type PeerDirection = "inbound" | "outbound"; @@ -190,7 +193,23 @@ export function getDefinitions(_config: ChainForkConfig): RouteDefinitions { + const json = NetworkIdentityType.toJson(data); + (json as {metadata: {custody_group_count: number | undefined}}).metadata.custody_group_count = + data.metadata.custodyGroupCount; + return json; + }, + fromJson: (json) => { + const data = NetworkIdentityType.fromJson(json); + (data.metadata as Partial).custodyGroupCount = ( + json as {metadata: {custody_group_count: number | undefined}} + ).metadata.custody_group_count; + return data; + }, + }, meta: EmptyMetaCodec, }, }, From 08de67c077c5d76266135458c01330bc04c3b57e Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sat, 26 Apr 2025 20:45:59 +0100 Subject: [PATCH 3/3] Organize imports --- packages/api/src/beacon/routes/node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/beacon/routes/node.ts b/packages/api/src/beacon/routes/node.ts index 9ce947f45960..4d705b909bf5 100644 --- a/packages/api/src/beacon/routes/node.ts +++ b/packages/api/src/beacon/routes/node.ts @@ -1,6 +1,6 @@ import {ContainerType, ValueOf} from "@chainsafe/ssz"; import {ChainForkConfig} from "@lodestar/config"; -import {ssz, stringType, fulu} from "@lodestar/types"; +import {fulu, ssz, stringType} from "@lodestar/types"; import { ArrayOf, EmptyArgs,