fix: refactor to add and use nodeid computation and clear out nodeid …

…tracking nodeid cleanup for network
ChainSafe · Aug 2, 2024 · 0debaf4 · 0debaf4
1 parent a8058b8
commit 0debaf4
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 74 deletions.
diff --git a/packages/beacon-node/src/network/core/networkCore.ts b/packages/beacon-node/src/network/core/networkCore.ts
@@ -2,8 +2,6 @@ import {Connection, PeerId} from "@libp2p/interface";
 import {multiaddr} from "@multiformats/multiaddr";
 import {PublishOpts} from "@chainsafe/libp2p-gossipsub/types";
 import {PeerScoreStatsDump} from "@chainsafe/libp2p-gossipsub/dist/src/score/peer-score.js";
-import {fromHexString} from "@chainsafe/ssz";
-import {ENR} from "@chainsafe/enr";
 import {routes} from "@lodestar/api";
 import {BeaconConfig} from "@lodestar/config";
 import type {LoggerNode} from "@lodestar/logger/node";
@@ -19,7 +17,7 @@ import {Eth2Gossipsub, getCoreTopicsAtFork} from "../gossip/index.js";
 import {SyncnetsService} from "../subnets/syncnetsService.js";
 import {FORK_EPOCH_LOOKAHEAD, getActiveForks} from "../forks.js";
 import {NetworkOptions} from "../options.js";
-import {CommitteeSubscription, IAttnetsService} from "../subnets/interface.js";
+import {CommitteeSubscription, IAttnetsService, computeNodeId} from "../subnets/interface.js";
 import {MetadataController} from "../metadata.js";
 import {createNodeJsLibp2p} from "../libp2p/index.js";
 import {PeersData} from "../peers/peersData.js";
@@ -194,11 +192,7 @@ export class NetworkCore implements INetworkCore {
     // should be called before AttnetsService constructor so that node subscribe to deterministic attnet topics
     await gossip.start();
 
-    const enr = opts.discv5?.enr;
-    const nodeId = enr ? fromHexString(ENR.decodeTxt(enr).nodeId) : null;
-    if (nodeId === null) {
-      throw Error("null node id");
-    }
+    const nodeId = computeNodeId(peerId);
     const attnetsService = new AttnetsService(config, clock, gossip, metadata, logger, metrics, nodeId, opts);
     const syncnetsService = new SyncnetsService(config, clock, gossip, metadata, logger, metrics, opts);
 

diff --git a/packages/beacon-node/src/network/peers/discover.ts b/packages/beacon-node/src/network/peers/discover.ts
@@ -1,7 +1,7 @@
 import {Multiaddr} from "@multiformats/multiaddr";
 import type {PeerId, PeerInfo} from "@libp2p/interface";
 import {ENR} from "@chainsafe/enr";
-import {fromHexString, toHexString} from "@chainsafe/ssz";
+import {toHexString} from "@chainsafe/ssz";
 import {BeaconConfig} from "@lodestar/config";
 import {pruneSetToMax, sleep} from "@lodestar/utils";
 import {ATTESTATION_SUBNET_COUNT, SYNC_COMMITTEE_SUBNET_COUNT} from "@lodestar/params";
@@ -13,7 +13,7 @@ import {ENRKey, SubnetType} from "../metadata.js";
 import {getConnectionsMap, prettyPrintPeerId} from "../util.js";
 import {Discv5Worker} from "../discv5/index.js";
 import {LodestarDiscv5Opts} from "../discv5/types.js";
-import {NodeId} from "../subnets/interface.js";
+import {NodeId, computeNodeId} from "../subnets/interface.js";
 import {getCustodyColumnSubnets} from "../../util/dataColumns.js";
 import {deserializeEnrSubnets, zeroAttnets, zeroSyncnets} from "./utils/enrSubnetsDeserialize.js";
 import {IPeerRpcScoreStore, ScoreState} from "./score/index.js";
@@ -23,8 +23,6 @@ const MAX_CACHED_ENRS = 100;
 /** Max age a cached ENR will be considered for dial */
 const MAX_CACHED_ENR_AGE_MS = 5 * 60 * 1000;
 
-const MAX_CACHED_NODEIDS = 10000;
-
 export type PeerDiscoveryOpts = {
   maxPeers: number;
   discv5FirstQueryDelayMs: number;
@@ -36,7 +34,7 @@ export type PeerDiscoveryOpts = {
 };
 
 export type PeerDiscoveryModules = {
-  nodeId: NodeId,
+  nodeId: NodeId;
   libp2p: Libp2p;
   peerRpcScores: IPeerRpcScoreStore;
   metrics: NetworkCoreMetrics | null;
@@ -103,8 +101,6 @@ export class PeerDiscovery {
   private logger: LoggerNode;
   private config: BeaconConfig;
   private cachedENRs = new Map<PeerIdStr, CachedENR>();
-  private peerIdToNodeId = new Map<PeerIdStr, NodeId>();
-  private peerIdToMyEnr = new Map<PeerIdStr, ENR>();
   private peerIdToCustodySubnetCount = new Map<PeerIdStr, number>();
   private randomNodeQuery: QueryStatus = {code: QueryStatusCode.NotActive};
   private peersToConnect = 0;
@@ -349,12 +345,6 @@ export class PeerDiscovery {
     }
     // async due to some crypto that's no longer necessary
     const peerId = await enr.peerId();
-
-    const nodeId = fromHexString(enr.nodeId);
-    this.peerIdToNodeId.set(peerId.toString(), nodeId);
-    this.peerIdToMyEnr.set(peerId.toString(), enr);
-    pruneSetToMax(this.peerIdToNodeId, MAX_CACHED_NODEIDS);
-
     // tcp multiaddr is known to be be present, checked inside the worker
     const multiaddrTCP = enr.getLocationMultiaddr(ENRKey.tcp);
     if (!multiaddrTCP) {
@@ -393,8 +383,8 @@ export class PeerDiscovery {
     syncnets: boolean[],
     custodySubnetCount: number
   ): DiscoveredPeerStatus {
-    const nodeId = this.peerIdToNodeId.get(peerId.toString());
-    this.logger.warn("handleDiscoveredPeer", {nodeId: nodeId ? toHexString(nodeId) : null, peerId: peerId.toString()});
+    const nodeId = computeNodeId(peerId);
+    this.logger.warn("handleDiscoveredPeer", {nodeId: toHexString(nodeId), peerId: peerId.toString()});
     try {
       // Check if peer is not banned or disconnected
       if (this.peerRpcScores.getScoreState(peerId) !== ScoreState.Healthy) {
@@ -442,10 +432,7 @@ export class PeerDiscovery {
   }
 
   private shouldDialPeer(peer: CachedENR): boolean {
-    const nodeId = this.peerIdToNodeId.get(peer.peerId.toString());
-    if (nodeId === undefined) {
-      return false;
-    }
+    const nodeId = computeNodeId(peer.peerId);
     const peerCustodySubnetCount = peer.custodySubnetCount;
     const peerCustodySubnets = getCustodyColumnSubnets(nodeId, peerCustodySubnetCount);
 

diff --git a/packages/beacon-node/src/network/peers/peerManager.ts b/packages/beacon-node/src/network/peers/peerManager.ts
@@ -1,6 +1,5 @@
 import {Connection, PeerId} from "@libp2p/interface";
-import {ENR} from "@chainsafe/enr";
-import {BitArray, toHexString, fromHexString} from "@chainsafe/ssz";
+import {BitArray, toHexString} from "@chainsafe/ssz";
 import {SYNC_COMMITTEE_SUBNET_COUNT} from "@lodestar/params";
 import {BeaconConfig} from "@lodestar/config";
 import {Metadata, altair, phase0} from "@lodestar/types";
@@ -12,7 +11,7 @@ import {NetworkEvent, INetworkEventBus, NetworkEventData} from "../events.js";
 import {Libp2p} from "../interface.js";
 import {ReqRespMethod} from "../reqresp/ReqRespBeaconNode.js";
 import {getConnection, getConnectionsMap, prettyPrintPeerId} from "../util.js";
-import {NodeId, SubnetsService} from "../subnets/index.js";
+import {NodeId, SubnetsService, computeNodeId} from "../subnets/index.js";
 import {SubnetType} from "../metadata.js";
 import {Eth2Gossipsub} from "../gossip/gossipsub.js";
 import {StatusCache} from "../statusCache.js";
@@ -392,11 +391,7 @@ export class PeerManager {
       peerData.relevantStatus = RelevantPeerStatus.relevant;
     }
     if (getConnection(this.libp2p, peer.toString())) {
-      const nodeId = peerData?.nodeId ?? this.discovery?.["peerIdToNodeId"].get(peer.toString());
-      if (nodeId === undefined) {
-        this.logger.error("onStatus with nodeId=undefined", {peerId: peer.toString()});
-        return;
-      }
+      const nodeId = peerData?.nodeId ?? computeNodeId(peer);
       const custodySubnetCount =
         peerData?.custodySubnetCount ?? this.discovery?.["peerIdToCustodySubnetCount"].get(peer.toString());
 
@@ -411,7 +406,7 @@ export class PeerManager {
       const hasMinCustodyMatchingColumns = matchingSubnetsNum >= this.config.CUSTODY_REQUIREMENT;
 
       this.logger.warn(`onStatus ${custodySubnetCount == undefined ? "undefined custody count assuming 4" : ""}`, {
-        nodeId: nodeId ? toHexString(nodeId) : undefined,
+        nodeId: toHexString(nodeId),
         peerId: peer.toString(),
         custodySubnetCount,
         hasAllColumns,
@@ -420,19 +415,17 @@ export class PeerManager {
       });
 
       if (this.opts.onlyConnectToBiggerDataNodes && !hasAllColumns) {
-        const enr = this.discovery?.["peerIdToMyEnr"].get(peer.toString());
-        this.logger.debug(
-          `ignoring peercontected onlyConnectToBiggerDataNodes=true hasAllColumns=${hasAllColumns}`,
-          exportENRToJSON(enr)
-        );
+        this.logger.debug(`ignoring peercontected onlyConnectToBiggerDataNodes=true hasAllColumns=${hasAllColumns}`, {
+          nodeId: toHexString(nodeId),
+          peerId: peer.toString(),
+        });
         return;
       }
 
       if (this.opts.onlyConnectToMinimalCustodyOverlapNodes && !hasMinCustodyMatchingColumns) {
-        const enr = this.discovery?.["peerIdToMyEnr"].get(peer.toString());
         this.logger.debug(
           `ignoring peercontected onlyConnectToMinimalCustodyOverlapNodes=true hasMinCustodyMatchingColumns=${hasMinCustodyMatchingColumns}`,
-          exportENRToJSON(enr)
+          {nodeId: toHexString(nodeId), peerId: peer.toString()}
         );
         return;
       }
@@ -649,7 +642,7 @@ export class PeerManager {
     // NOTE: libp2p may emit two "peer:connect" events: One for inbound, one for outbound
     // If that happens, it's okay. Only the "outbound" connection triggers immediate action
     const now = Date.now();
-    const nodeId = this.discovery?.["peerIdToNodeId"].get(remotePeer.toString()) ?? null;
+    const nodeId = computeNodeId(remotePeer);
     const custodySubnetCount = this.discovery?.["peerIdToCustodySubnetCount"].get(remotePeer.toString()) ?? null;
     const peerData: PeerData = {
       lastReceivedMsgUnixTsMs: direction === "outbound" ? 0 : now,
@@ -791,14 +784,3 @@ export class PeerManager {
     metrics.peersSync.set(syncPeers);
   }
 }
-
-function exportENRToJSON(enr?: ENR): Record<string, string | undefined> | undefined {
-  if (enr === undefined) {
-    return undefined;
-  }
-  return {
-    ip4: enr.kvs.get("ip")?.toString(),
-    csc: enr.kvs.get("csc")?.toString(),
-    nodeId: enr.nodeId,
-  };
-}
diff --git a/packages/beacon-node/src/network/reqresp/beaconBlocksMaybeBlobsByRange.ts b/packages/beacon-node/src/network/reqresp/beaconBlocksMaybeBlobsByRange.ts
@@ -1,4 +1,4 @@
-import {ENR} from "@chainsafe/enr";
+import {toHexString} from "@chainsafe/ssz";
 import {ChainForkConfig} from "@lodestar/config";
 import {deneb, Epoch, phase0, SignedBeaconBlock, Slot, electra, ssz} from "@lodestar/types";
 import {ForkSeq, NUMBER_OF_COLUMNS, ForkName} from "@lodestar/params";
@@ -12,16 +12,14 @@ import {
   BlockInputDataBlobs,
   BlockInputDataDataColumns,
   DataColumnsSource,
-  DataColumnsCacheMap,
   BlockInputType,
   getBlockInputDataColumns,
 } from "../../chain/blocks/types.js";
 import {PeerIdStr} from "../../util/peerId.js";
 import {INetwork, WithBytes, WithOptionalBytes} from "../interface.js";
 import {CustodyConfig} from "../../util/dataColumns.js";
-import {Network} from "../network.js";
-import {NetworkCore} from "../core/networkCore.js";
 import {getEmptyBlockInputCacheEntry} from "../../chain/seenCache/seenGossipBlockInput.js";
+import {computeNodeId} from "../subnets/index.js";
 
 export type PartialDownload = null | {blocks: BlockInput[]; pendingDataColumns: number[]};
 export async function beaconBlocksMaybeBlobsByRange(
@@ -284,16 +282,14 @@ export function matchBlockWithDataColumns(
         );
 
         if (dataColumnSidecars.length !== requestedColumns.length || !requestedColumnsPresent) {
-          const peerEnr = ((network as Network)["core"] as NetworkCore)["peerManager"]?.["discovery"]?.[
-            "peerIdToMyEnr"
-          ].get(peerId);
           console.log(
             "matchBlockWithDataColumns",
             `Missing or mismatching dataColumnSidecars from peerId=${peerId} for blockSlot=${block.data.message.slot} with numColumns=${custodyColumnsLen} dataColumnSidecars=${dataColumnSidecars.length} requestedColumnsPresent=${requestedColumnsPresent} received dataColumnIndexes=${dataColumnIndexes.join(",")} requested=${requestedColumns.join(",")}`,
             {
               allBlocks: allBlocks.length,
               allDataColumnSidecars: allDataColumnSidecars.length,
-              peerEnr: exportENRToJSON(peerEnr),
+              peerId,
+              nodeId: toHexString(computeNodeId(peerId)),
               blobKzgCommitmentsLen,
             }
           );
@@ -363,14 +359,3 @@ export function matchBlockWithDataColumns(
   }
   return blockInputs;
 }
-
-function exportENRToJSON(enr?: ENR): Record<string, string | undefined> | undefined {
-  if (enr === undefined) {
-    return undefined;
-  }
-  return {
-    ip4: enr.kvs.get("ip")?.toString(),
-    csc: enr.kvs.get("csc")?.toString(),
-    nodeId: enr.nodeId,
-  };
-}
diff --git a/packages/beacon-node/src/network/subnets/interface.ts b/packages/beacon-node/src/network/subnets/interface.ts
@@ -1,3 +1,7 @@
+import {nodeId as computeENRNodeId, IDScheme} from "@chainsafe/enr";
+import type {PeerId} from "@libp2p/interface";
+import {peerIdFromString} from "@libp2p/peer-id";
+import {fromHexString} from "@chainsafe/ssz";
 import {ForkName} from "@lodestar/params";
 import {Bytes32, Slot, ValidatorIndex} from "@lodestar/types";
 import {RequestedSubnet} from "../peers/utils/index.js";
@@ -48,3 +52,16 @@ export type GossipSubscriber = {
 
 // uint256 in the spec
 export type NodeId = Bytes32;
+export function computeNodeId(peerIdOrStr: PeerId | PeerIdStr) {
+  let peerId;
+  if (typeof peerId === "string") {
+    peerId = peerIdFromString(peerId);
+  } else {
+    peerId = peerIdOrStr as PeerId;
+  }
+
+  if (peerId.publicKey === undefined) {
+    throw Error("Undefined publicKey");
+  }
+  return fromHexString(computeENRNodeId(IDScheme.v4, peerId.publicKey.slice(4)));
+}