Allow runtime config of MAX_REQUEST_BLOB_SIDECARS (#6964)

With `MAX_REQUEST_BLOB_SIDECARS` becoming dynamic, we can no longer use
the value from network config in the `List` datatype and have to rely on
an upper bound constant for the maximum supported value instead and then
check the actual length at runtime.

As a side effect, this also extends `BlobIdentifierList` to use the
correct electra specific request limit instead of the Deneb constant.

Author: etan-status

beacon_chain/spec/datatypes/constants.nim

@@ -93,7 +93,3 @@ const
 
   # https://github.com/ethereum/consensus-specs/blob/v1.5.0-beta.0/specs/electra/beacon-chain.md#execution-1
   MAX_BLOBS_PER_BLOCK_ELECTRA* = 9'u64
-
-  # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.10/specs/electra/p2p-interface.md#configuration
-  MAX_REQUEST_BLOB_SIDECARS_ELECTRA* =
-    MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK_ELECTRA

beacon_chain/spec/network.nim

@@ -30,10 +30,6 @@ const
   # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.9/specs/altair/light-client/p2p-interface.md#configuration
   MAX_REQUEST_LIGHT_CLIENT_UPDATES* = 128
 
-  # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.5/specs/deneb/p2p-interface.md#configuration
-  MAX_REQUEST_BLOB_SIDECARS*: uint64 =
-    MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK
-
   # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.10/specs/fulu/p2p-interface.md#configuration
   MAX_REQUEST_DATA_COLUMN_SIDECARS*: uint64 =
     MAX_REQUEST_BLOCKS_DENEB * NUMBER_OF_COLUMNS

beacon_chain/spec/presets.nim

@@ -32,6 +32,7 @@ const
   MESSAGE_DOMAIN_VALID_SNAPPY*: array[4, byte] = [0x01, 0x00, 0x00, 0x00]
 
   MAX_SUPPORTED_BLOBS_PER_BLOCK*: uint64 = 9  # revisit getShortMap(Blobs) if >9
+  MAX_SUPPORTED_REQUEST_BLOB_SIDECARS*: uint64 = 1152
 
 type
   Version* = distinct array[4, byte]
@@ -869,15 +870,14 @@ proc readRuntimeConfig*(
   checkCompatibility ATTESTATION_SUBNET_PREFIX_BITS
 
   checkCompatibility MAX_REQUEST_BLOCKS_DENEB
-  checkCompatibility MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK,
-                     "MAX_REQUEST_BLOB_SIDECARS"
   checkCompatibility BLOB_SIDECAR_SUBNET_COUNT
   checkCompatibility MAX_BLOBS_PER_BLOCK_ELECTRA
-  checkCompatibility MAX_REQUEST_BLOB_SIDECARS_ELECTRA
 
   for suffix in ["", "_ELECTRA"]:
     checkCompatibility MAX_SUPPORTED_BLOBS_PER_BLOCK,
                        "MAX_BLOBS_PER_BLOCK" & suffix, `<=`
+    checkCompatibility MAX_SUPPORTED_REQUEST_BLOB_SIDECARS,
+                       "MAX_REQUEST_BLOB_SIDECARS" & suffix, `<=`
 
   # https://github.com/ethereum/consensus-specs/blob/v1.5.0-beta.0/specs/phase0/fork-choice.md#configuration
   # Isn't being used as a preset in the usual way: at any time, there's one correct value

beacon_chain/sync/sync_manager.nim

@@ -92,7 +92,7 @@ type
   BeaconBlocksRes =
     NetRes[List[ref ForkedSignedBeaconBlock, Limit MAX_REQUEST_BLOCKS]]
   BlobSidecarsRes =
-    NetRes[List[ref BlobSidecar, Limit(MAX_REQUEST_BLOB_SIDECARS_ELECTRA)]]
+    NetRes[List[ref BlobSidecar, Limit(MAX_SUPPORTED_REQUEST_BLOB_SIDECARS)]]
 
   SyncBlockData* = object
     blocks*: seq[ref ForkedSignedBeaconBlock]

beacon_chain/sync/sync_protocol.nim

@@ -37,8 +37,10 @@ type
     slot: Slot
 
   BlockRootsList* = List[Eth2Digest, Limit MAX_REQUEST_BLOCKS]
-  BlobIdentifierList* = List[BlobIdentifier, Limit (MAX_REQUEST_BLOB_SIDECARS)]
-  DataColumnIdentifierList* = List[DataColumnIdentifier, Limit (MAX_REQUEST_DATA_COLUMN_SIDECARS)]
+  BlobIdentifierList* = List[
+    BlobIdentifier, Limit MAX_SUPPORTED_REQUEST_BLOB_SIDECARS]
+  DataColumnIdentifierList* = List[
+    DataColumnIdentifier, Limit (MAX_REQUEST_DATA_COLUMN_SIDECARS)]
 
 proc readChunkPayload*(
     conn: Connection, peer: Peer, MsgType: type (ref ForkedSignedBeaconBlock)):
@@ -108,11 +110,13 @@ proc readChunkPayload*(
 
 template getBlobSidecarsByRoot(
     versionNumber: static string, peer: Peer, dag: ChainDAGRef, response: auto,
-    blobIds: BlobIdentifierList) =
+    blobIds: BlobIdentifierList, maxReqSidecars: uint64) =
   trace "got v" & versionNumber & " blobs range request",
     peer, len = blobIds.len
   if blobIds.len == 0:
     raise newException(InvalidInputsError, "No blobs requested")
+  if blobIds.lenu64 > maxReqSidecars:
+    raise newException(InvalidInputsError, "Exceeding blob request limit")
 
   let count = blobIds.len
 
@@ -145,8 +149,8 @@ template getBlobSidecarsByRoot(
 
 template getBlobSidecarsByRange(
     versionNumber: static string, peer: Peer, dag: ChainDAGRef, response: auto,
-    startSlot: Slot, reqCount: uint64, blobsPerBlock: static uint64,
-    maxReqSidecars: static uint64) =
+    startSlot: Slot, reqCount: uint64, blobsPerBlock: uint64,
+    maxReqSidecars: uint64) =
   trace "got v" & versionNumber & " blobs range request",
     peer, startSlot, count = reqCount
   if reqCount == 0:
@@ -161,9 +165,9 @@ template getBlobSidecarsByRange(
   if startSlot.epoch < epochBoundary:
     raise newException(ResourceUnavailableError, BlobsOutOfRange)
 
-  var blockIds: array[int(maxReqSidecars), BlockId]
+  var blockIds: array[MAX_SUPPORTED_REQUEST_BLOB_SIDECARS.int, BlockId]
   let
-    count = int min(reqCount, blockIds.lenu64)
+    count = int min(reqCount, maxReqSidecars)
     endIndex = count - 1
     startIndex =
       dag.getBlockRange(startSlot, blockIds.toOpenArray(0, endIndex))
@@ -334,7 +338,7 @@ p2pProtocol BeaconSync(version = 1,
       peer: Peer,
       blobIds: BlobIdentifierList,
       response: MultipleChunksResponse[
-        ref BlobSidecar, Limit(MAX_REQUEST_BLOB_SIDECARS_ELECTRA)])
+        ref BlobSidecar, Limit(MAX_SUPPORTED_REQUEST_BLOB_SIDECARS)])
       {.async, libp2pProtocol("blob_sidecars_by_root", 1).} =
     # TODO Semantically, this request should return a non-ref, but doing so
     #      runs into extreme inefficiency due to the compiler introducing
@@ -348,15 +352,17 @@ p2pProtocol BeaconSync(version = 1,
     #      client call that returns `seq[ref BlobSidecar]` will
     #      will be generated by the libp2p macro - we guarantee that seq items
     #      are `not-nil` in the implementation
-    getBlobSidecarsByRoot("1", peer, peer.networkState.dag, response, blobIds)
+    getBlobSidecarsByRoot(
+      "1", peer, peer.networkState.dag, response, blobIds,
+      peer.networkState.dag.cfg.MAX_REQUEST_BLOB_SIDECARS_ELECTRA)
 
   # https://github.com/ethereum/consensus-specs/blob/v1.3.0/specs/deneb/p2p-interface.md#blobsidecarsbyrange-v1
   proc blobSidecarsByRange(
       peer: Peer,
       startSlot: Slot,
       reqCount: uint64,
       response: MultipleChunksResponse[
-        ref BlobSidecar, Limit(MAX_REQUEST_BLOB_SIDECARS_ELECTRA)])
+        ref BlobSidecar, Limit(MAX_SUPPORTED_REQUEST_BLOB_SIDECARS)])
       {.async, libp2pProtocol("blob_sidecars_by_range", 1).} =
     # TODO This code is more complicated than it needs to be, since the type
     #      of the multiple chunks response is not actually used in this server
@@ -368,7 +374,8 @@ p2pProtocol BeaconSync(version = 1,
     #      are `not-nil` in the implementation
     getBlobSidecarsByRange(
       "1", peer, peer.networkState.dag, response, startSlot, reqCount,
-      MAX_BLOBS_PER_BLOCK_ELECTRA, MAX_REQUEST_BLOB_SIDECARS_ELECTRA)
+      peer.networkState.dag.cfg.MAX_BLOBS_PER_BLOCK_ELECTRA,
+      peer.networkState.dag.cfg.MAX_REQUEST_BLOB_SIDECARS_ELECTRA)
 
   # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.10/specs/fulu/p2p-interface.md#datacolumnsidecarsbyroot-v1
   proc dataColumnSidecarsByRoot(
@@ -420,7 +427,7 @@ p2pProtocol BeaconSync(version = 1,
     debug "Data column root request done",
       peer, roots = colIds.len, count, found
 
-# https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.10/specs/fulu/p2p-interface.md#datacolumnsidecarsbyrange-v1
+  # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.10/specs/fulu/p2p-interface.md#datacolumnsidecarsbyrange-v1
   proc dataColumnSidecarsByRange(
       peer: Peer,
       startSlot: Slot,