Support for different blob target and max values #14678

terencechain · 2024-11-27T20:32:05Z

This PR adds support for potential new blob target and max values (based on hard forks) in the configuration, with updates applied throughout the codebase. The detailed change list is provided below.

Notes This PR does not implement `EIP-7691: Blob throughput increase`; it is the first part of the PR, allowing a custom blob target and max count. The EIP will be included in the following PR, as we aim to keep each PR small and easy to review.

Config

Removed fieldparams package's MaxBlobsPerBlock for mainnet and minimal configurations.
Added DeprecatedMaxBlobsPerBlock to the config package which is mainly used for import through yaml
Introduced helpers MaxBlobsPerBlock and TargetBlobsPerBlock that determine the appropriate blob count for a given slot based on the active fork.

Proposer Requesting Header from Relayer

Updated ToProto() to ToProto(Slot) for the proposer getHeader response to the relayer endpoint. The proposer passes the slot, and ToProto ensures the builder's bid's KZG commitment does not exceed MaxBlobsPerBlock for the current slot.

Blockchain Package Checking DA

Added slot to missingIndices for calculating MaxBlobsPerBlock as a safety check ensuring KZG commitments in the beacon block body do not exceed the limit.
Updated forRoot in blobNotifierMap to use slot for constructing the correct number of channels (MaxBlobsPerBlock). This manages the NotifyIndex listener.

Sync Notify New Blob to Blockchain Package

Added slot to sendBlobEvent in notifyIndex to construct the correct size for the seen map and downstream notifier channel.

State Transition Process for New Payload

Added slot to verifyBlobCommitmentCount to ensure KZG commitments do not exceed the MaxBlobsPerBlock limit. This is part of the state transition check, using the state.slot before processing the payload to match the block slot.

DAS Cache

Removed safeCommitmentArray as it assumed a fixed MaxBlobsPerBlock.
Updated functions like filter, stash, and commitmentsToCheck:
- commitmentsToCheck still ensures KZG commitments in the block do not exceed the max blob count.
- stash now handles cases where e.scs is nil, which was previously unsupported.

Blob Storage

Added slot to indices for MaxBlobsPerBlock checks and for constructing the mask object.
Changed blobIndexMask to a slice instead of an array.
Updated hasIndex and AllAvailable to include additional checks ensuring indices do not exceed the length of the mask.
If v.mask is nil, a new blobIndexMask is constructed.

Beacon API RPC

Updated initial URL index checks to validate indices against MaxBlobsPerBlock based on Electra fork configurations.
Added a check to ensure indices do not exceed KZG commitments in beacon-chain/rpc/lookup/blocker.go.

Proposer

Ensured correct MaxBlobsPerBlock is used when fetching payload headers from the builder. This check is duplicated but not removed in this PR.

Initial Sync - Backfill

Changed blobVerifierMap to use a slice of BlobVerifier instead of a hardcoded length.

Sync - RPC

Added slot to constructPendingBlobsRequest for database index validation.
Added slot to blobBatchLimit for constructing the correct block batch limit.
Updated SendBlobSidecarByRoot to verify that the blob sidecars in the response do not exceed MaxBlobsPerBlock.

Sync - Subscribe

Added slot to all usages of database index changes.
Added a new validation rule ensuring the KZG commitment check is based on the blob's slot.

prestonvanloon · 2024-12-17T14:31:28Z

api/client/builder/types.go

@@ -1008,12 +1009,13 @@ func (ehr *ExecHeaderResponseDeneb) ToProto() (*eth.SignedBuilderBidDeneb, error
 }

 // ToProto creates a BuilderBidDeneb Proto from BuilderBidDeneb.
-func (bb *BuilderBidDeneb) ToProto() (*eth.BuilderBidDeneb, error) {
+func (bb *BuilderBidDeneb) ToProto(s types.Slot) (*eth.BuilderBidDeneb, error) {


Why not have this be the max blob count instead of the slot? That would be a better abstraction since this method doesn't actually care about the slot, it only needs it to derive the max blob count

prestonvanloon · 2024-12-17T14:34:29Z

config/params/config.go

+	MaxBlobsPerBlock           int `yaml:"MAX_BLOBS_PER_BLOCK" spec:"true"`            // MaxBlobsPerBlock defines the max blobs could exist in a block.
+	MaxBlobsPerBlockElectra    int `yaml:"MAX_BLOBS_PER_BLOCK_ELECTRA" spec:"true"`    // MaxBlobsPerBlockElectra defines the max blobs could exist in a block post Electra hard fork.
+	TargetBlobsPerBlockElectra int `yaml:"TARGET_BLOBS_PER_BLOCK_ELECTRA" spec:"true"` // TargetBlobsPerBlockElectra defines the target number of blobs per block post Electra hard fork.


Why is it an int when everything else is uint64?

It's in the description :)
Tried both approaches and int is generally more friendly to work with, less conversion in most places

prestonvanloon · 2024-12-17T14:38:39Z

config/params/config.go

@@ -280,6 +280,11 @@ type BeaconChainConfig struct {
 	AttestationSubnetPrefixBits     uint64          `yaml:"ATTESTATION_SUBNET_PREFIX_BITS" spec:"true"`     // AttestationSubnetPrefixBits is defined as (ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS).
 	SubnetsPerNode                  uint64          `yaml:"SUBNETS_PER_NODE" spec:"true"`                   // SubnetsPerNode is the number of long-lived subnets a beacon node should be subscribed to.
 	NodeIdBits                      uint64          `yaml:"NODE_ID_BITS" spec:"true"`                       // NodeIdBits defines the bit length of a node id.
+
+	// Blobs Values
+	MaxBlobsPerBlock           int `yaml:"MAX_BLOBS_PER_BLOCK" spec:"true"`            // MaxBlobsPerBlock defines the max blobs could exist in a block.


I think having this as a public config value is going to be a potential footgun or lead to misuse. Is there a way that this can only be accessed via the helper method? If there is a single place in the production codepath that is using params.BeaconConfig().MaxBlobsPerBlock instead of params.BeaconConfig().MaxBlobCount(slot) then it's going to be a nasty bug at the fork.

prestonvanloon · 2024-12-19T14:16:58Z

CHANGELOG.md

@@ -78,6 +78,7 @@ Notable features:
 - Save light client updates and bootstraps in DB.
 - Added more comprehensive tests for `BlockToLightClientHeader`. [PR](https://github.com/prysmaticlabs/prysm/pull/14699)
 - Added light client feature flag check to RPC handlers. [PR](https://github.com/prysmaticlabs/prysm/pull/14736)
+- Added support to update target and max blob count to different values per hard fork config.


This is the wrong section

…blob-count

prestonvanloon · 2024-12-19T14:40:59Z

beacon-chain/blockchain/service.go

@@ -166,7 +171,7 @@ func NewService(ctx context.Context, opts ...Option) (*Service, error) {
 	ctx, cancel := context.WithCancel(ctx)
 	bn := &blobNotifierMap{
 		notifiers: make(map[[32]byte]chan uint64),
-		seenIndex: make(map[[32]byte][fieldparams.MaxBlobsPerBlock]bool),
+		seenIndex: make(map[[32]byte][]bool),


probably a micro optimization that can be in another PR, but a bitfield is cheaper on memory.

terencechain force-pushed the custom-blob-count branch from 33bafb6 to d70c358 Compare November 27, 2024 21:35

terencechain force-pushed the custom-blob-count branch 6 times, most recently from 837eb16 to 68d9ce7 Compare December 9, 2024 20:18

terencechain marked this pull request as ready for review December 9, 2024 20:18

terencechain requested review from prestonvanloon and a team as code owners December 9, 2024 20:18

terencechain requested review from nalepae and saolyn December 9, 2024 20:18

terencechain force-pushed the custom-blob-count branch 6 times, most recently from 3d81330 to 4599ebd Compare December 11, 2024 00:08

prestonvanloon requested changes Dec 17, 2024

View reviewed changes

terencechain mentioned this pull request Dec 17, 2024

Implement consensus spec v1.5.0-alpha.10 #14733

Merged

terencechain force-pushed the custom-blob-count branch 3 times, most recently from 8e83e77 to 8e2b3eb Compare December 19, 2024 00:57

Add support for different blob target and max

a76b72c

terencechain force-pushed the custom-blob-count branch from 8e2b3eb to a76b72c Compare December 19, 2024 01:00

prestonvanloon previously approved these changes Dec 19, 2024

View reviewed changes

Merge branch 'develop' into custom-blob-count

e591ff6

terencechain dismissed prestonvanloon’s stale review via e591ff6 December 19, 2024 14:07

prestonvanloon reviewed Dec 19, 2024

View reviewed changes

Fix change log to right section

5697114

terencechain enabled auto-merge December 19, 2024 14:34

Merge branch 'develop' of github.com:prysmaticlabs/prysm into custom-…

f03587d

…blob-count

prestonvanloon approved these changes Dec 19, 2024

View reviewed changes

terencechain added this pull request to the merge queue Dec 19, 2024

Merged via the queue into develop with commit bc69ab8 Dec 19, 2024
15 checks passed

terencechain deleted the custom-blob-count branch December 19, 2024 15:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for different blob target and max values #14678

Support for different blob target and max values #14678

terencechain commented Nov 27, 2024 •

edited

Loading

prestonvanloon Dec 17, 2024

prestonvanloon Dec 17, 2024

terencechain Dec 17, 2024

prestonvanloon Dec 17, 2024

prestonvanloon Dec 19, 2024

prestonvanloon Dec 19, 2024

Support for different blob target and max values #14678

Support for different blob target and max values #14678

Conversation

terencechain commented Nov 27, 2024 • edited Loading

Config

Proposer Requesting Header from Relayer

Blockchain Package Checking DA

Sync Notify New Blob to Blockchain Package

State Transition Process for New Payload

DAS Cache

Blob Storage

Beacon API RPC

Proposer

Initial Sync - Backfill

Sync - RPC

Sync - Subscribe

prestonvanloon Dec 17, 2024

Choose a reason for hiding this comment

prestonvanloon Dec 17, 2024

Choose a reason for hiding this comment

terencechain Dec 17, 2024

Choose a reason for hiding this comment

prestonvanloon Dec 17, 2024

Choose a reason for hiding this comment

prestonvanloon Dec 19, 2024

Choose a reason for hiding this comment

prestonvanloon Dec 19, 2024

Choose a reason for hiding this comment

terencechain commented Nov 27, 2024 •

edited

Loading