chore: consistently pass fork info to requestSszTypeByMethod (#7411)

We need to consistently pass fork info to `requestSszTypeByMethod` as otherwise it might use the incorrect type, eg. in case of Electra we would use wrong list limit for `BlobSidecarsByRoot` method in some places. This addresses last outstanding item from #7309.
ChainSafe · Jan 30, 2025 · 418e81e · 418e81e
1 parent 78a5e72
commit 418e81e
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 16 deletions.
diff --git a/packages/beacon-node/src/network/network.ts b/packages/beacon-node/src/network/network.ts
@@ -527,7 +527,7 @@ export class Network implements INetwork {
     request: Req
   ): AsyncIterable<ResponseIncoming> {
     const fork = this.config.getForkName(this.clock.currentSlot);
-    const requestType = requestSszTypeByMethod(this.config, fork)[method];
+    const requestType = requestSszTypeByMethod(fork, this.config)[method];
     const requestData = requestType ? requestType.serialize(request as never) : new Uint8Array();
 
     // ReqResp outgoing request, emit from main thread to worker

diff --git a/packages/beacon-node/src/network/reqresp/ReqRespBeaconNode.ts b/packages/beacon-node/src/network/reqresp/ReqRespBeaconNode.ts
@@ -156,7 +156,7 @@ export class ReqRespBeaconNode extends ReqResp {
 
     // Overwrite placeholder requestData from main thread with correct sequenceNumber
     if (method === ReqRespMethod.Ping) {
-      requestData = requestSszTypeByMethod(this.config)[ReqRespMethod.Ping].serialize(
+      requestData = requestSszTypeByMethod(ForkName.phase0, this.config)[ReqRespMethod.Ping].serialize(
         this.metadataController.seqNumber
       );
     }
@@ -208,7 +208,7 @@ export class ReqRespBeaconNode extends ReqResp {
     request: Req
   ): AsyncIterable<ResponseIncoming> {
     const fork = ForkName[ForkSeq[this.currentRegisteredFork] as ForkName];
-    const requestType = requestSszTypeByMethod(this.config, fork)[method];
+    const requestType = requestSszTypeByMethod(fork, this.config)[method];
     const requestData = requestType ? requestType.serialize(request as never) : new Uint8Array();
     return this.sendRequestWithoutEncoding(peerId, method, versions, requestData);
   }

diff --git a/packages/beacon-node/src/network/reqresp/protocols.ts b/packages/beacon-node/src/network/reqresp/protocols.ts
@@ -107,7 +107,7 @@ function toProtocol(protocol: ProtocolSummary) {
     encoding: Encoding.SSZ_SNAPPY,
     contextBytes: toContextBytes(protocol.contextBytesType, config),
     inboundRateLimits: rateLimitQuotas(fork, config)[protocol.method],
-    requestSizes: requestSszTypeByMethod(config)[protocol.method],
+    requestSizes: requestSszTypeByMethod(fork, config)[protocol.method],
     responseSizes: (fork) => responseSszTypeByMethod[protocol.method](fork, protocol.version),
   });
 }

diff --git a/packages/beacon-node/src/network/reqresp/rateLimit.ts b/packages/beacon-node/src/network/reqresp/rateLimit.ts
@@ -7,8 +7,7 @@ import {
   isForkBlobs,
 } from "@lodestar/params";
 import {InboundRateLimitQuota} from "@lodestar/reqresp";
-import {ReqRespMethod, RequestBodyByMethod} from "./types.js";
-import {requestSszTypeByMethod} from "./types.js";
+import {ReqRespMethod, RequestBodyByMethod, requestSszTypeByMethod} from "./types.js";
 
 export const rateLimitQuotas: (fork: ForkName, config: BeaconConfig) => Record<ReqRespMethod, InboundRateLimitQuota> = (
   fork,
@@ -34,22 +33,22 @@ export const rateLimitQuotas: (fork: ForkName, config: BeaconConfig) => Record<R
   [ReqRespMethod.BeaconBlocksByRange]: {
     // Rationale: https://github.com/sigp/lighthouse/blob/bf533c8e42cc73c35730e285c21df8add0195369/beacon_node/lighthouse_network/src/rpc/mod.rs#L118-L130
     byPeer: {quota: isForkBlobs(fork) ? MAX_REQUEST_BLOCKS_DENEB : MAX_REQUEST_BLOCKS, quotaTimeMs: 10_000},
-    getRequestCount: getRequestCountFn(config, ReqRespMethod.BeaconBlocksByRange, (req) => req.count),
+    getRequestCount: getRequestCountFn(fork, config, ReqRespMethod.BeaconBlocksByRange, (req) => req.count),
   },
   [ReqRespMethod.BeaconBlocksByRoot]: {
     // Rationale: https://github.com/sigp/lighthouse/blob/bf533c8e42cc73c35730e285c21df8add0195369/beacon_node/lighthouse_network/src/rpc/mod.rs#L118-L130
     byPeer: {quota: isForkBlobs(fork) ? MAX_REQUEST_BLOCKS_DENEB : MAX_REQUEST_BLOCKS, quotaTimeMs: 10_000},
-    getRequestCount: getRequestCountFn(config, ReqRespMethod.BeaconBlocksByRoot, (req) => req.length),
+    getRequestCount: getRequestCountFn(fork, config, ReqRespMethod.BeaconBlocksByRoot, (req) => req.length),
   },
   [ReqRespMethod.BlobSidecarsByRange]: {
     // Rationale: MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK
     byPeer: {quota: config.getMaxRequestBlobSidecars(fork), quotaTimeMs: 10_000},
-    getRequestCount: getRequestCountFn(config, ReqRespMethod.BlobSidecarsByRange, (req) => req.count),
+    getRequestCount: getRequestCountFn(fork, config, ReqRespMethod.BlobSidecarsByRange, (req) => req.count),
   },
   [ReqRespMethod.BlobSidecarsByRoot]: {
     // Rationale: quota of BeaconBlocksByRoot * MAX_BLOBS_PER_BLOCK
     byPeer: {quota: config.getMaxRequestBlobSidecars(fork), quotaTimeMs: 10_000},
-    getRequestCount: getRequestCountFn(config, ReqRespMethod.BlobSidecarsByRoot, (req) => req.length),
+    getRequestCount: getRequestCountFn(fork, config, ReqRespMethod.BlobSidecarsByRoot, (req) => req.length),
   },
   [ReqRespMethod.LightClientBootstrap]: {
     // As similar in the nature of `Status` protocol so we use the same rate limits.
@@ -58,7 +57,7 @@ export const rateLimitQuotas: (fork: ForkName, config: BeaconConfig) => Record<R
   [ReqRespMethod.LightClientUpdatesByRange]: {
     // Same rationale as for BeaconBlocksByRange
     byPeer: {quota: MAX_REQUEST_LIGHT_CLIENT_UPDATES, quotaTimeMs: 10_000},
-    getRequestCount: getRequestCountFn(config, ReqRespMethod.LightClientUpdatesByRange, (req) => req.count),
+    getRequestCount: getRequestCountFn(fork, config, ReqRespMethod.LightClientUpdatesByRange, (req) => req.count),
   },
   [ReqRespMethod.LightClientFinalityUpdate]: {
     // Finality updates should not be requested more than once per epoch.
@@ -74,11 +73,12 @@ export const rateLimitQuotas: (fork: ForkName, config: BeaconConfig) => Record<R
 
 // Helper to produce a getRequestCount function
 function getRequestCountFn<T extends ReqRespMethod>(
+  fork: ForkName,
   config: BeaconConfig,
   method: T,
   fn: (req: RequestBodyByMethod[T]) => number
 ): (reqData: Uint8Array) => number {
-  const type = requestSszTypeByMethod(config)[method];
+  const type = requestSszTypeByMethod(fork, config)[method];
   return (reqData: Uint8Array) => {
     try {
       return (type && fn(type.deserialize(reqData))) ?? 1;

diff --git a/packages/beacon-node/src/network/reqresp/types.ts b/packages/beacon-node/src/network/reqresp/types.ts
@@ -70,13 +70,12 @@ type ResponseBodyByMethod = {
 };
 
 /** Request SSZ type for each method and ForkName */
-// TODO Electra: Currently setting default fork to deneb because not every caller of requestSszTypeByMethod can provide fork info
 export const requestSszTypeByMethod: (
-  config: BeaconConfig,
-  fork?: ForkName
+  fork: ForkName,
+  config: BeaconConfig
 ) => {
   [K in ReqRespMethod]: RequestBodyByMethod[K] extends null ? null : Type<RequestBodyByMethod[K]>;
-} = (config, fork = ForkName.deneb) => ({
+} = (fork, config) => ({
   [ReqRespMethod.Status]: ssz.phase0.Status,
   [ReqRespMethod.Goodbye]: ssz.phase0.Goodbye,
   [ReqRespMethod.Ping]: ssz.phase0.Ping,