Skip to content

Allow runtime config of MAX_REQUEST_BLOB_SIDECARS #6964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tersec merged 1 commit into from
Feb 25, 2025
Merged

Allow runtime config of MAX_REQUEST_BLOB_SIDECARS #6964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions beacon_chain/spec/datatypes/constants.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines -98 to -99
Copy link
Copy Markdown
Contributor Author

@etan-status etan-status Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In presets.nim we have the same equivalence check dynamically:

  checkCompatibility MAX_REQUEST_BLOCKS_DENEB * cfg.MAX_BLOBS_PER_BLOCK,
                     "MAX_REQUEST_BLOB_SIDECARS"
  checkCompatibility MAX_REQUEST_BLOCKS_DENEB * cfg.MAX_BLOBS_PER_BLOCK_ELECTRA,
                     "MAX_REQUEST_BLOB_SIDECARS_ELECTRA"

4 changes: 0 additions & 4 deletions beacon_chain/spec/network.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions beacon_chain/spec/presets.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion beacon_chain/sync/sync_manager.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,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]
Expand Down
31 changes: 19 additions & 12 deletions beacon_chain/sync/sync_protocol.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)):
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand All @@ -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))
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down