diff --git a/beacon_node/lighthouse_network/src/types/topics.rs b/beacon_node/lighthouse_network/src/types/topics.rs index a06c65679d6..bdd62aa7cb9 100644 --- a/beacon_node/lighthouse_network/src/types/topics.rs +++ b/beacon_node/lighthouse_network/src/types/topics.rs @@ -3,7 +3,13 @@ use serde::{Deserialize, Serialize}; use std::collections::HashSet; use strum::AsRefStr; use typenum::Unsigned; -use types::{ChainSpec, DataColumnSubnetId, EthSpec, ForkName, SubnetId, SyncSubnetId}; +use types::{ + ChainSpec, EthSpec, + attestation::SubnetId, + data::{DataColumnSubnetId, all_data_column_sidecar_subnets_from_spec}, + fork::ForkName, + sync_committee::SyncSubnetId, +}; use crate::Subnet; @@ -115,7 +121,7 @@ pub fn is_fork_non_core_topic(topic: &GossipTopic, _fork_name: ForkName) -> bool pub fn all_topics_at_fork(fork: ForkName, spec: &ChainSpec) -> Vec { // Compute the worst case of all forks - let sampling_subnets = HashSet::from_iter(spec.all_data_column_sidecar_subnets()); + let sampling_subnets = HashSet::from_iter(all_data_column_sidecar_subnets_from_spec(spec)); let opts = TopicConfig { enable_light_client_server: true, subscribe_all_subnets: true, diff --git a/consensus/types/src/core/chain_spec.rs b/consensus/types/src/core/chain_spec.rs index c02456540c1..a90a39a69d8 100644 --- a/consensus/types/src/core/chain_spec.rs +++ b/consensus/types/src/core/chain_spec.rs @@ -8,7 +8,7 @@ use safe_arith::{ArithError, SafeArith}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde_utils::quoted_u64::MaybeQuoted; use ssz::Encode; -use ssz_types::{RuntimeVariableList, VariableList}; +use ssz_types::RuntimeVariableList; use tree_hash::TreeHash; use crate::{ @@ -16,7 +16,6 @@ use crate::{ APPLICATION_DOMAIN_BUILDER, Address, ApplicationDomain, EnrForkId, Epoch, EthSpec, EthSpecId, ExecutionBlockHash, Hash256, MainnetEthSpec, Slot, Uint256, }, - data::{BlobIdentifier, DataColumnSubnetId, DataColumnsByRootIdentifier}, fork::{Fork, ForkData, ForkName}, }; @@ -823,10 +822,6 @@ impl ChainSpec { } } - pub fn all_data_column_sidecar_subnets(&self) -> impl Iterator { - (0..self.data_column_sidecar_subnet_count).map(DataColumnSubnetId::new) - } - /// Worst-case compressed length for a given payload of size n when using snappy. /// /// https://github.com/google/snappy/blob/32ded457c0b1fe78ceb8397632c416568d6714a0/snappy.cc#L218C1-L218C47 @@ -2110,37 +2105,41 @@ fn max_blocks_by_root_request_common(max_request_blocks: u64) -> usize { .len() } -fn max_blobs_by_root_request_common(max_request_blob_sidecars: u64) -> usize { - let max_request_blob_sidecars = max_request_blob_sidecars as usize; - let empty_blob_identifier = BlobIdentifier { - block_root: Hash256::zero(), - index: 0, - }; +// Simplified function which precomputes the size of a `List` of `BlobIdentifiers`. +pub(crate) fn max_blobs_by_root_request_common(max_request_blob_sidecars: u64) -> usize { + // BlobIdentifier is a fixed-size struct with two fields: + // - block_root: Hash256 (32 bytes) + // - index: u64 (8 bytes) + // Total per element: 32 + 8 = 40 bytes + // Since BlobIdentifier is fixed-size, the outer List does not add any byte overhead. + let blob_identifier_ssz_size = 40_usize; - RuntimeVariableList::::new( - vec![empty_blob_identifier; max_request_blob_sidecars], - max_request_blob_sidecars, - ) - .expect("creating a RuntimeVariableList of size `max_request_blob_sidecars` should succeed") - .as_ssz_bytes() - .len() + (max_request_blob_sidecars as usize) + .safe_mul(blob_identifier_ssz_size) + .expect("should not overflow") } -fn max_data_columns_by_root_request_common(max_request_blocks: u64) -> usize { - let max_request_blocks = max_request_blocks as usize; - - let empty_data_columns_by_root_id = DataColumnsByRootIdentifier { - block_root: Hash256::zero(), - columns: VariableList::repeat_full(0), - }; - - RuntimeVariableList::>::new( - vec![empty_data_columns_by_root_id; max_request_blocks], - max_request_blocks, - ) - .expect("creating a RuntimeVariableList of size `max_request_blocks` should succeed") - .as_ssz_bytes() - .len() +// Simplified function which precomputes the size of a `List` of `DataColumnIdentifiers`. +pub(crate) fn max_data_columns_by_root_request_common( + max_request_blocks: u64, +) -> usize { + // DataColumnsByRootIdentifier is a variable-size struct with two fields: + // - block_root: Hash256 (32 bytes) + // - columns: List (4 byte offset + n × 8 bytes) + // Since DataColumnsByRootIdentifier is variable-size, the outer List adds a + // 4-byte offset per element. + // Total per element: 4 (outer offset) + 32 (block_root) + 4 (columns offset) + n × 8 + let column_index_ssz_size = 8_usize; + let ssz_fixed_size = 40_usize; + + let data_columns_by_root_identifier_ssz_size = column_index_ssz_size + .safe_mul(E::number_of_columns()) + .and_then(|b| b.safe_add(ssz_fixed_size)) + .expect("should not overflow"); + + (max_request_blocks as usize) + .safe_mul(data_columns_by_root_identifier_ssz_size) + .expect("should not overflow") } fn default_max_blocks_by_root_request() -> usize { diff --git a/consensus/types/src/core/mod.rs b/consensus/types/src/core/mod.rs index 33d5856deaa..4e583fbc672 100644 --- a/consensus/types/src/core/mod.rs +++ b/consensus/types/src/core/mod.rs @@ -38,6 +38,11 @@ pub use signing_data::{SignedRoot, SigningData}; pub use slot_data::SlotData; pub use slot_epoch::{Epoch, Slot}; +#[cfg(test)] +pub(crate) use chain_spec::{ + max_blobs_by_root_request_common, max_data_columns_by_root_request_common, +}; + pub type Hash256 = alloy_primitives::B256; pub type Uint256 = alloy_primitives::U256; pub type Hash64 = alloy_primitives::B64; diff --git a/consensus/types/src/data/blob_sidecar.rs b/consensus/types/src/data/blob_sidecar.rs index 709e556933b..638491d6d72 100644 --- a/consensus/types/src/data/blob_sidecar.rs +++ b/consensus/types/src/data/blob_sidecar.rs @@ -300,3 +300,39 @@ pub type BlobSidecarList = RuntimeVariableList>>; /// Alias for a non length-constrained list of `BlobSidecar`s. pub type FixedBlobSidecarList = RuntimeFixedVector>>>; pub type BlobsList = VariableList, ::MaxBlobCommitmentsPerBlock>; + +#[cfg(test)] +mod tests { + use super::*; + use crate::core::max_blobs_by_root_request_common; + use fixed_bytes::FixedBytesExtended; + + // This is the "correct" implementation of max_blobs_by_root_request. + // This test ensures that the simplified implementation doesn't deviate from it. + fn max_blobs_by_root_request_implementation(max_request_blob_sidecars: u64) -> usize { + let max_request_blob_sidecars = max_request_blob_sidecars as usize; + let empty_blob_identifier = BlobIdentifier { + block_root: Hash256::zero(), + index: 0, + }; + + RuntimeVariableList::::new( + vec![empty_blob_identifier; max_request_blob_sidecars], + max_request_blob_sidecars, + ) + .expect("creating a RuntimeVariableList of size `max_request_blob_sidecars` should succeed") + .as_ssz_bytes() + .len() + } + + #[test] + fn max_blobs_by_root_request_matches_simplified() { + for n in [0, 1, 2, 8, 16, 32, 64, 128, 256, 512, 768, 1024, 1152] { + assert_eq!( + max_blobs_by_root_request_common(n), + max_blobs_by_root_request_implementation(n), + "Mismatch at n={n}" + ); + } + } +} diff --git a/consensus/types/src/data/data_column_sidecar.rs b/consensus/types/src/data/data_column_sidecar.rs index 71d821f83ef..c37f9568a6f 100644 --- a/consensus/types/src/data/data_column_sidecar.rs +++ b/consensus/types/src/data/data_column_sidecar.rs @@ -170,3 +170,43 @@ impl From for DataColumnSidecarError { Self::SszError(e) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::core::{MainnetEthSpec, max_data_columns_by_root_request_common}; + use fixed_bytes::FixedBytesExtended; + use ssz_types::RuntimeVariableList; + + // This is the "correct" implementation of max_data_columns_by_root_request. + // This test ensures that the simplified implementation doesn't deviate from it. + fn max_data_columns_by_root_request_implementation( + max_request_blocks: u64, + ) -> usize { + let max_request_blocks = max_request_blocks as usize; + + let empty_data_columns_by_root_id = DataColumnsByRootIdentifier { + block_root: Hash256::zero(), + columns: VariableList::repeat_full(0), + }; + + RuntimeVariableList::>::new( + vec![empty_data_columns_by_root_id; max_request_blocks], + max_request_blocks, + ) + .expect("creating a RuntimeVariableList of size `max_request_blocks` should succeed") + .as_ssz_bytes() + .len() + } + + #[test] + fn max_data_columns_by_root_request_matches_simplified() { + for n in [0, 1, 2, 8, 16, 32, 64, 128, 256, 512, 1024] { + assert_eq!( + max_data_columns_by_root_request_common::(n), + max_data_columns_by_root_request_implementation::(n), + "Mismatch at n={n}" + ); + } + } +} diff --git a/consensus/types/src/data/data_column_subnet_id.rs b/consensus/types/src/data/data_column_subnet_id.rs index c30ebbba20e..0a05cd8936f 100644 --- a/consensus/types/src/data/data_column_subnet_id.rs +++ b/consensus/types/src/data/data_column_subnet_id.rs @@ -72,3 +72,9 @@ impl From<&DataColumnSubnetId> for u64 { val.0 } } + +pub fn all_data_column_sidecar_subnets_from_spec( + spec: &ChainSpec, +) -> impl Iterator { + (0..spec.data_column_sidecar_subnet_count).map(DataColumnSubnetId::new) +} diff --git a/consensus/types/src/data/mod.rs b/consensus/types/src/data/mod.rs index 10d062bada9..4c165ba4726 100644 --- a/consensus/types/src/data/mod.rs +++ b/consensus/types/src/data/mod.rs @@ -15,7 +15,7 @@ pub use data_column_sidecar::{ Cell, ColumnIndex, DataColumn, DataColumnSidecar, DataColumnSidecarError, DataColumnSidecarList, DataColumnsByRootIdentifier, }; -pub use data_column_subnet_id::DataColumnSubnetId; +pub use data_column_subnet_id::{DataColumnSubnetId, all_data_column_sidecar_subnets_from_spec}; use crate::core::EthSpec; use ssz_types::FixedVector;