Remove data dependency from from core module in `consensus/types (#8694)

#8652


  Implements "simplified" versions of `max_blobs_by_root_request` and `max_data_columns_by_root_request` which do not depend on type information from the `data` module. I've also added tests which test the original implementation against the simplified one to ensure they don't deviate.

Also moves `all_data_column_sidecar_subnets` from a method on `ChainSpec` to a function which just takes `ChainSpec` as an argument.


Co-Authored-By: Mac L <mjladson@pm.me>

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<E: EthSpec>(fork: ForkName, spec: &ChainSpec) -> Vec<GossipKind> {
     // 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,

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<Item = DataColumnSubnetId> {
-        (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::<BlobIdentifier>::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<E: EthSpec>(max_request_blocks: u64) -> usize {
-    let max_request_blocks = max_request_blocks as usize;
+// Simplified function which precomputes the size of a `List` of `DataColumnIdentifiers`.
+pub(crate) fn max_data_columns_by_root_request_common<E: EthSpec>(
+    max_request_blocks: u64,
+) -> usize {
+    // DataColumnsByRootIdentifier is a variable-size struct with two fields:
+    // - block_root: Hash256 (32 bytes)
+    // - columns: List<ColumnIndex, NumberOfColumns> (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 empty_data_columns_by_root_id = DataColumnsByRootIdentifier {
-        block_root: Hash256::zero(),
-        columns: VariableList::repeat_full(0),
-    };
+    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");
 
-    RuntimeVariableList::<DataColumnsByRootIdentifier<E>>::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()
+    (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 {

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;

consensus/types/src/data/blob_sidecar.rs

@@ -300,3 +300,39 @@ pub type BlobSidecarList<E> = RuntimeVariableList<Arc<BlobSidecar<E>>>;
 /// Alias for a non length-constrained list of `BlobSidecar`s.
 pub type FixedBlobSidecarList<E> = RuntimeFixedVector<Option<Arc<BlobSidecar<E>>>>;
 pub type BlobsList<E> = VariableList<Blob<E>, <E as EthSpec>::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::<BlobIdentifier>::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}"
+            );
+        }
+    }
+}

consensus/types/src/data/data_column_sidecar.rs

@@ -170,3 +170,43 @@ impl From<SszError> 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<E: EthSpec>(
+        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::<DataColumnsByRootIdentifier<E>>::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::<MainnetEthSpec>(n),
+                max_data_columns_by_root_request_implementation::<MainnetEthSpec>(n),
+                "Mismatch at n={n}"
+            );
+        }
+    }
+}

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<Item = DataColumnSubnetId> {
+    (0..spec.data_column_sidecar_subnet_count).map(DataColumnSubnetId::new)
+}

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;