Reqresp. Add todos

ChainSafe · Dec 17, 2024 · 1722971 · 1722971
1 parent 6b48588
commit 1722971
Show file tree

Hide file tree

Showing 8 changed files with 34 additions and 11 deletions.
diff --git a/packages/beacon-node/src/network/network.ts b/packages/beacon-node/src/network/network.ts
@@ -4,7 +4,7 @@ import {PeerId} from "@libp2p/interface";
 import {routes} from "@lodestar/api";
 import {BeaconConfig} from "@lodestar/config";
 import {LoggerNode} from "@lodestar/logger/node";
-import {ForkSeq} from "@lodestar/params";
+import {ForkName, ForkSeq, isForkPostElectra} from "@lodestar/params";
 import {ResponseIncoming} from "@lodestar/reqresp";
 import {computeStartSlotAtEpoch, computeTimeAtSlot} from "@lodestar/state-transition";
 import {
@@ -501,17 +501,30 @@ export class Network implements INetwork {
     peerId: PeerIdStr,
     request: deneb.BlobSidecarsByRangeRequest
   ): Promise<deneb.BlobSidecar[]> {
+    const fork = this.config.getForkName(this.clock.currentSlot);
     return collectMaxResponseTyped(
-      this.sendReqRespRequest(peerId, ReqRespMethod.BlobSidecarsByRange, [Version.V1], request),
+      this.sendReqRespRequest(
+        peerId,
+        ReqRespMethod.BlobSidecarsByRange,
+        [isForkPostElectra(fork) ? Version.V2 : Version.V1],
+        request
+      ),
       // request's count represent the slots, so the actual max count received could be slots * blobs per slot
-      request.count * this.config.MAX_BLOBS_PER_BLOCK,
+      request.count *
+        (isForkPostElectra(fork) ? this.config.MAX_BLOBS_PER_BLOCK_ELECTRA : this.config.MAX_BLOBS_PER_BLOCK),
       responseSszTypeByMethod[ReqRespMethod.BlobSidecarsByRange]
     );
   }
 
   async sendBlobSidecarsByRoot(peerId: PeerIdStr, request: BlobSidecarsByRootRequest): Promise<deneb.BlobSidecar[]> {
+    const fork = this.config.getForkName(this.clock.currentSlot);
     return collectMaxResponseTyped(
-      this.sendReqRespRequest(peerId, ReqRespMethod.BlobSidecarsByRoot, [Version.V1], request), // TODO
+      this.sendReqRespRequest(
+        peerId,
+        ReqRespMethod.BlobSidecarsByRoot,
+        [isForkPostElectra(fork) ? Version.V2 : Version.V1],
+        request
+      ),
       request.length,
       responseSszTypeByMethod[ReqRespMethod.BlobSidecarsByRoot]
     );

diff --git a/packages/beacon-node/src/network/reqresp/ReqRespBeaconNode.ts b/packages/beacon-node/src/network/reqresp/ReqRespBeaconNode.ts
@@ -251,6 +251,7 @@ export class ReqRespBeaconNode extends ReqResp {
     }
 
     if (ForkSeq[fork] >= ForkSeq.deneb) {
+      // TODO Electra: Consider deprecating BlobSidecarsByRootV1 and BlobSidecarsByRangeV1 at fork boundary or after Electra is stable
       protocolsAtFork.push(
         [protocols.BlobSidecarsByRoot(this.config), this.getHandler(ReqRespMethod.BlobSidecarsByRoot)],
         [protocols.BlobSidecarsByRange(this.config), this.getHandler(ReqRespMethod.BlobSidecarsByRange)]

diff --git a/packages/beacon-node/src/network/reqresp/beaconBlocksMaybeBlobsByRoot.ts b/packages/beacon-node/src/network/reqresp/beaconBlocksMaybeBlobsByRoot.ts
@@ -228,7 +228,7 @@ export async function unavailableBeaconBlobsByRoot(
   }
 
   if (networkReqIdentifiers.length > 0) {
-    networkResBlobSidecars = await network.sendBlobSidecarsByRoot(peerId, networkReqIdentifiers); // TODO
+    networkResBlobSidecars = await network.sendBlobSidecarsByRoot(peerId, networkReqIdentifiers);
     metrics?.blockInputFetchStats.dataPromiseBlobsFinallyAvailableFromNetwork.inc(networkResBlobSidecars.length);
     if (blockTriedBefore) {
       metrics?.blockInputFetchStats.dataPromiseBlobsRetriedAvailableFromNetwork.inc(networkResBlobSidecars.length);

diff --git a/packages/beacon-node/src/network/reqresp/handlers/beaconBlocksByRange.ts b/packages/beacon-node/src/network/reqresp/handlers/beaconBlocksByRange.ts
@@ -85,6 +85,7 @@ export function validateBeaconBlocksByRangeRequest(
   // step > 1 is deprecated, see https://github.com/ethereum/consensus-specs/pull/2856
 
   if (count > MAX_REQUEST_BLOCKS) {
+    // TODO: This is probably not right as `BeaconBlocksByRangeV2` takes at most `MAX_REQUEST_BLOCKS_DENEB`
     count = MAX_REQUEST_BLOCKS;
   }
 

diff --git a/packages/beacon-node/src/network/reqresp/handlers/index.ts b/packages/beacon-node/src/network/reqresp/handlers/index.ts
@@ -1,9 +1,10 @@
+import {ForkName} from "@lodestar/params";
 import {ProtocolHandler} from "@lodestar/reqresp";
 import {ssz} from "@lodestar/types";
 import {IBeaconChain} from "../../../chain/index.js";
 import {IBeaconDb} from "../../../db/index.js";
 import {BlobSidecarsByRootRequestType} from "../../../util/types.js";
-import {GetReqRespHandlerFn, ReqRespMethod} from "../types.js";
+import {GetReqRespHandlerFn, ReqRespMethod, Version} from "../types.js";
 import {onBeaconBlocksByRange} from "./beaconBlocksByRange.js";
 import {onBeaconBlocksByRoot} from "./beaconBlocksByRoot.js";
 import {onBlobSidecarsByRange} from "./blobSidecarsByRange.js";
@@ -38,8 +39,9 @@ export function getReqRespHandlers({db, chain}: {db: IBeaconDb; chain: IBeaconCh
       return onBeaconBlocksByRoot(body, chain, db);
     },
     [ReqRespMethod.BlobSidecarsByRoot]: (req) => {
+      const fork = req.version === Version.V2 ? ForkName.electra : ForkName.deneb;
       const body = BlobSidecarsByRootRequestType(fork, chain.config).deserialize(req.data);
-      return onBlobSidecarsByRoot(body, chain, db)
+      return onBlobSidecarsByRoot(body, chain, db);
     },
     [ReqRespMethod.BlobSidecarsByRange]: (req) => {
       const body = ssz.deneb.BlobSidecarsByRangeRequest.deserialize(req.data);

diff --git a/packages/beacon-node/src/network/reqresp/rateLimit.ts b/packages/beacon-node/src/network/reqresp/rateLimit.ts
@@ -34,12 +34,14 @@ export const rateLimitQuotas: (config: ChainConfig) => Record<ReqRespMethod, Inb
   },
   [ReqRespMethod.BlobSidecarsByRange]: {
     // Rationale: MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK
+    // TODO Electra: Stays as `MAX_REQUEST_BLOB_SIDECARS` until we have fork-aware `byPeer` and set it to `MAX_REQUEST_BLOB_SIDECARS_ELECTRA`
     byPeer: {quota: config.MAX_REQUEST_BLOB_SIDECARS, quotaTimeMs: 10_000},
     getRequestCount: getRequestCountFn(config, ReqRespMethod.BlobSidecarsByRange, (req) => req.count),
   },
   [ReqRespMethod.BlobSidecarsByRoot]: {
     // Rationale: quota of BeaconBlocksByRoot * MAX_BLOBS_PER_BLOCK
-    byPeer: {quota: config.MAX_REQUEST_BLOB_SIDECARS, quotaTimeMs: 10_000}, // TODO 
+    // TODO Electra: Stays as `MAX_REQUEST_BLOB_SIDECARS` until we have fork-aware `byPeer` and set it to `MAX_REQUEST_BLOB_SIDECARS_ELECTRA`
+    byPeer: {quota: config.MAX_REQUEST_BLOB_SIDECARS, quotaTimeMs: 10_000},
     getRequestCount: getRequestCountFn(config, ReqRespMethod.BlobSidecarsByRoot, (req) => req.length),
   },
   [ReqRespMethod.LightClientBootstrap]: {

diff --git a/packages/beacon-node/src/network/reqresp/types.ts b/packages/beacon-node/src/network/reqresp/types.ts
@@ -70,6 +70,7 @@ type ResponseBodyByMethod = {
 };
 
 /** Request SSZ type for each method and ForkName */
+// TODO Electra: Make requestSszTypeByMethod fork-aware so we can get the correct BlobSidecarsByRootRequestType
 export const requestSszTypeByMethod: (config: ChainConfig) => {
   [K in ReqRespMethod]: RequestBodyByMethod[K] extends null ? null : Type<RequestBodyByMethod[K]>;
 } = (config) => ({
@@ -80,7 +81,7 @@ export const requestSszTypeByMethod: (config: ChainConfig) => {
   [ReqRespMethod.BeaconBlocksByRange]: ssz.phase0.BeaconBlocksByRangeRequest,
   [ReqRespMethod.BeaconBlocksByRoot]: ssz.phase0.BeaconBlocksByRootRequest,
   [ReqRespMethod.BlobSidecarsByRange]: ssz.deneb.BlobSidecarsByRangeRequest,
-  [ReqRespMethod.BlobSidecarsByRoot]: BlobSidecarsByRootRequestType(fork, config),
+  [ReqRespMethod.BlobSidecarsByRoot]: BlobSidecarsByRootRequestType(ForkName.deneb, config),
   [ReqRespMethod.LightClientBootstrap]: ssz.Root,
   [ReqRespMethod.LightClientUpdatesByRange]: ssz.altair.LightClientUpdatesByRange,
   [ReqRespMethod.LightClientFinalityUpdate]: null,

diff --git a/packages/beacon-node/src/util/types.ts b/packages/beacon-node/src/util/types.ts
@@ -1,6 +1,6 @@
 import {ContainerType, ListCompositeType, ValueOf} from "@chainsafe/ssz";
 import {ChainConfig} from "@lodestar/config";
-import { ForkName, isForkPostElectra } from "@lodestar/params";
+import {ForkName, isForkPostElectra} from "@lodestar/params";
 import {ssz} from "@lodestar/types";
 
 // Misc SSZ types used only in the beacon-node package, no need to upstream to types
@@ -16,5 +16,8 @@ export const signedBLSToExecutionChangeVersionedType = new ContainerType(
 export type SignedBLSToExecutionChangeVersioned = ValueOf<typeof signedBLSToExecutionChangeVersionedType>;
 
 export const BlobSidecarsByRootRequestType = (fork: ForkName, config: ChainConfig) =>
-  new ListCompositeType(ssz.deneb.BlobIdentifier, isForkPostElectra(fork) ? config.MAX_REQUEST_BLOB_SIDECARS_ELECTRA : config.MAX_REQUEST_BLOB_SIDECARS); // TODO
+  new ListCompositeType(
+    ssz.deneb.BlobIdentifier,
+    isForkPostElectra(fork) ? config.MAX_REQUEST_BLOB_SIDECARS_ELECTRA : config.MAX_REQUEST_BLOB_SIDECARS
+  );
 export type BlobSidecarsByRootRequest = ValueOf<ReturnType<typeof BlobSidecarsByRootRequestType>>;