Make max_blobs_per_block a config parameter (#6329)

* First pass

* Add restrictions to RuntimeVariableList api

* Use empty_uninitialized and fix warnings

* Fix some todos

* Merge branch 'unstable' into max-blobs-preset

* Fix take impl on RuntimeFixedList

* cleanup

* Fix test compilations

* Fix some more tests

* Fix test from unstable

* Merge branch 'unstable' into max-blobs-preset

* Merge remote-tracking branch 'origin/unstable' into max-blobs-preset

* Remove footgun function

* Minor simplifications

* Move from preset to config

* Fix typo

* Revert "Remove footgun function"

This reverts commit de01f923c7.

* Try fixing tests

* Thread through ChainSpec

* Fix release tests

* Move RuntimeFixedVector into module and rename

* Add test

* Remove empty RuntimeVarList awefullness

* Fix tests

* Simplify BlobSidecarListFromRoot

* Merge remote-tracking branch 'origin/unstable' into max-blobs-preset

* Bump quota to account for new target (6)

* Remove clone

* Fix issue from review

* Try to remove ugliness

* Merge branch 'unstable' into max-blobs-preset

* Fix max value

* Fix doctest

* Fix formatting

* Fix max check

* Delete hardcoded max_blobs_per_block in RPC limits

* Merge remote-tracking branch 'origin/unstable' into max-blobs-preset

consensus/types/src/beacon_block_body.rs

@@ -13,8 +13,6 @@ use tree_hash_derive::TreeHash;
 
 pub type KzgCommitments<E> =
     VariableList<KzgCommitment, <E as EthSpec>::MaxBlobCommitmentsPerBlock>;
-pub type KzgCommitmentOpts<E> =
-    FixedVector<Option<KzgCommitment>, <E as EthSpec>::MaxBlobsPerBlock>;
 
 /// The number of leaves (including padding) on the `BeaconBlockBody` Merkle tree.
 ///

consensus/types/src/blob_sidecar.rs

@@ -1,10 +1,10 @@
 use crate::test_utils::TestRandom;
 use crate::{
-    beacon_block_body::BLOB_KZG_COMMITMENTS_INDEX, BeaconBlockHeader, BeaconStateError, Blob,
-    Epoch, EthSpec, FixedVector, Hash256, SignedBeaconBlockHeader, Slot, VariableList,
+    beacon_block_body::BLOB_KZG_COMMITMENTS_INDEX, AbstractExecPayload, BeaconBlockHeader,
+    BeaconStateError, Blob, ChainSpec, Epoch, EthSpec, FixedVector, ForkName,
+    ForkVersionDeserialize, Hash256, KzgProofs, RuntimeFixedVector, RuntimeVariableList,
+    SignedBeaconBlock, SignedBeaconBlockHeader, Slot, VariableList,
 };
-use crate::{AbstractExecPayload, ForkName};
-use crate::{ForkVersionDeserialize, KzgProofs, SignedBeaconBlock};
 use bls::Signature;
 use derivative::Derivative;
 use kzg::{Blob as KzgBlob, Kzg, KzgCommitment, KzgProof, BYTES_PER_BLOB, BYTES_PER_FIELD_ELEMENT};
@@ -30,19 +30,6 @@ pub struct BlobIdentifier {
     pub index: u64,
 }
 
-impl BlobIdentifier {
-    pub fn get_all_blob_ids<E: EthSpec>(block_root: Hash256) -> Vec<BlobIdentifier> {
-        let mut blob_ids = Vec::with_capacity(E::max_blobs_per_block());
-        for i in 0..E::max_blobs_per_block() {
-            blob_ids.push(BlobIdentifier {
-                block_root,
-                index: i as u64,
-            });
-        }
-        blob_ids
-    }
-}
-
 impl PartialOrd for BlobIdentifier {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
         Some(self.cmp(other))
@@ -291,19 +278,23 @@ impl<E: EthSpec> BlobSidecar<E> {
         blobs: BlobsList<E>,
         block: &SignedBeaconBlock<E>,
         kzg_proofs: KzgProofs<E>,
+        spec: &ChainSpec,
     ) -> Result<BlobSidecarList<E>, BlobSidecarError> {
         let mut blob_sidecars = vec![];
         for (i, (kzg_proof, blob)) in kzg_proofs.iter().zip(blobs).enumerate() {
             let blob_sidecar = BlobSidecar::new(i, blob, block, *kzg_proof)?;
             blob_sidecars.push(Arc::new(blob_sidecar));
         }
-        Ok(VariableList::from(blob_sidecars))
+        Ok(RuntimeVariableList::from_vec(
+            blob_sidecars,
+            spec.max_blobs_per_block(block.epoch()) as usize,
+        ))
     }
 }
 
-pub type BlobSidecarList<E> = VariableList<Arc<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>;
-pub type FixedBlobSidecarList<E> =
-    FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>;
+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>;
 
 impl<E: EthSpec> ForkVersionDeserialize for BlobSidecarList<E> {

consensus/types/src/chain_spec.rs

@@ -237,6 +237,7 @@ pub struct ChainSpec {
     pub max_request_data_column_sidecars: u64,
     pub min_epochs_for_blob_sidecars_requests: u64,
     pub blob_sidecar_subnet_count: u64,
+    max_blobs_per_block: u64,
 
     /*
      * Networking Derived
@@ -616,6 +617,17 @@ impl ChainSpec {
         }
     }
 
+    /// Return the value of `MAX_BLOBS_PER_BLOCK` appropriate for the fork at `epoch`.
+    pub fn max_blobs_per_block(&self, epoch: Epoch) -> u64 {
+        self.max_blobs_per_block_by_fork(self.fork_name_at_epoch(epoch))
+    }
+
+    /// Return the value of `MAX_BLOBS_PER_BLOCK` appropriate for `fork`.
+    pub fn max_blobs_per_block_by_fork(&self, _fork_name: ForkName) -> u64 {
+        // TODO(electra): add Electra blobs per block change here
+        self.max_blobs_per_block
+    }
+
     pub fn data_columns_per_subnet(&self) -> usize {
         self.number_of_columns
             .safe_div(self.data_column_sidecar_subnet_count as usize)
@@ -859,6 +871,7 @@ impl ChainSpec {
             max_request_data_column_sidecars: default_max_request_data_column_sidecars(),
             min_epochs_for_blob_sidecars_requests: default_min_epochs_for_blob_sidecars_requests(),
             blob_sidecar_subnet_count: default_blob_sidecar_subnet_count(),
+            max_blobs_per_block: default_max_blobs_per_block(),
 
             /*
              * Derived Deneb Specific
@@ -1187,6 +1200,7 @@ impl ChainSpec {
             max_request_data_column_sidecars: default_max_request_data_column_sidecars(),
             min_epochs_for_blob_sidecars_requests: 16384,
             blob_sidecar_subnet_count: default_blob_sidecar_subnet_count(),
+            max_blobs_per_block: default_max_blobs_per_block(),
 
             /*
              * Derived Deneb Specific
@@ -1388,6 +1402,9 @@ pub struct Config {
     #[serde(default = "default_blob_sidecar_subnet_count")]
     #[serde(with = "serde_utils::quoted_u64")]
     blob_sidecar_subnet_count: u64,
+    #[serde(default = "default_max_blobs_per_block")]
+    #[serde(with = "serde_utils::quoted_u64")]
+    max_blobs_per_block: u64,
 
     #[serde(default = "default_min_per_epoch_churn_limit_electra")]
     #[serde(with = "serde_utils::quoted_u64")]
@@ -1523,6 +1540,12 @@ const fn default_blob_sidecar_subnet_count() -> u64 {
     6
 }
 
+/// Its important to keep this consistent with the deneb preset value for
+/// `MAX_BLOBS_PER_BLOCK` else we might run into consensus issues.
+const fn default_max_blobs_per_block() -> u64 {
+    6
+}
+
 const fn default_min_per_epoch_churn_limit_electra() -> u64 {
     128_000_000_000
 }
@@ -1745,6 +1768,7 @@ impl Config {
             max_request_data_column_sidecars: spec.max_request_data_column_sidecars,
             min_epochs_for_blob_sidecars_requests: spec.min_epochs_for_blob_sidecars_requests,
             blob_sidecar_subnet_count: spec.blob_sidecar_subnet_count,
+            max_blobs_per_block: spec.max_blobs_per_block,
 
             min_per_epoch_churn_limit_electra: spec.min_per_epoch_churn_limit_electra,
             max_per_epoch_activation_exit_churn_limit: spec
@@ -1822,6 +1846,7 @@ impl Config {
             max_request_data_column_sidecars,
             min_epochs_for_blob_sidecars_requests,
             blob_sidecar_subnet_count,
+            max_blobs_per_block,
 
             min_per_epoch_churn_limit_electra,
             max_per_epoch_activation_exit_churn_limit,
@@ -1890,6 +1915,7 @@ impl Config {
             max_request_data_column_sidecars,
             min_epochs_for_blob_sidecars_requests,
             blob_sidecar_subnet_count,
+            max_blobs_per_block,
 
             min_per_epoch_churn_limit_electra,
             max_per_epoch_activation_exit_churn_limit,

consensus/types/src/data_column_sidecar.rs

@@ -1,7 +1,7 @@
 use crate::beacon_block_body::{KzgCommitments, BLOB_KZG_COMMITMENTS_INDEX};
 use crate::test_utils::TestRandom;
 use crate::BeaconStateError;
-use crate::{BeaconBlockHeader, EthSpec, Hash256, KzgProofs, SignedBeaconBlockHeader, Slot};
+use crate::{BeaconBlockHeader, Epoch, EthSpec, Hash256, KzgProofs, SignedBeaconBlockHeader, Slot};
 use bls::Signature;
 use derivative::Derivative;
 use kzg::Error as KzgError;
@@ -11,7 +11,6 @@ use safe_arith::ArithError;
 use serde::{Deserialize, Serialize};
 use ssz::Encode;
 use ssz_derive::{Decode, Encode};
-use ssz_types::typenum::Unsigned;
 use ssz_types::Error as SszError;
 use ssz_types::{FixedVector, VariableList};
 use std::hash::Hash;
@@ -68,6 +67,10 @@ impl<E: EthSpec> DataColumnSidecar<E> {
         self.signed_block_header.message.slot
     }
 
+    pub fn epoch(&self) -> Epoch {
+        self.slot().epoch(E::slots_per_epoch())
+    }
+
     pub fn block_root(&self) -> Hash256 {
         self.signed_block_header.message.tree_hash_root()
     }
@@ -110,18 +113,16 @@ impl<E: EthSpec> DataColumnSidecar<E> {
         .len()
     }
 
-    pub fn max_size() -> usize {
+    pub fn max_size(max_blobs_per_block: usize) -> usize {
         Self {
             index: 0,
-            column: VariableList::new(vec![Cell::<E>::default(); E::MaxBlobsPerBlock::to_usize()])
-                .unwrap(),
+            column: VariableList::new(vec![Cell::<E>::default(); max_blobs_per_block]).unwrap(),
             kzg_commitments: VariableList::new(vec![
                 KzgCommitment::empty_for_testing();
-                E::MaxBlobsPerBlock::to_usize()
+                max_blobs_per_block
             ])
             .unwrap(),
-            kzg_proofs: VariableList::new(vec![KzgProof::empty(); E::MaxBlobsPerBlock::to_usize()])
-                .unwrap(),
+            kzg_proofs: VariableList::new(vec![KzgProof::empty(); max_blobs_per_block]).unwrap(),
             signed_block_header: SignedBeaconBlockHeader {
                 message: BeaconBlockHeader::empty(),
                 signature: Signature::empty(),

consensus/types/src/eth_spec.rs

@@ -4,8 +4,7 @@ use safe_arith::SafeArith;
 use serde::{Deserialize, Serialize};
 use ssz_types::typenum::{
     bit::B0, UInt, U0, U1, U1024, U1048576, U1073741824, U1099511627776, U128, U131072, U134217728,
-    U16, U16777216, U2, U2048, U256, U262144, U32, U4, U4096, U512, U6, U625, U64, U65536, U8,
-    U8192,
+    U16, U16777216, U2, U2048, U256, U262144, U32, U4, U4096, U512, U625, U64, U65536, U8, U8192,
 };
 use ssz_types::typenum::{U17, U9};
 use std::fmt::{self, Debug};
@@ -109,7 +108,6 @@ pub trait EthSpec:
     /*
      * New in Deneb
      */
-    type MaxBlobsPerBlock: Unsigned + Clone + Sync + Send + Debug + PartialEq + Unpin;
     type MaxBlobCommitmentsPerBlock: Unsigned + Clone + Sync + Send + Debug + PartialEq + Unpin;
     type FieldElementsPerBlob: Unsigned + Clone + Sync + Send + Debug + PartialEq;
     type BytesPerFieldElement: Unsigned + Clone + Sync + Send + Debug + PartialEq;
@@ -281,11 +279,6 @@ pub trait EthSpec:
         Self::MaxWithdrawalsPerPayload::to_usize()
     }
 
-    /// Returns the `MAX_BLOBS_PER_BLOCK` constant for this specification.
-    fn max_blobs_per_block() -> usize {
-        Self::MaxBlobsPerBlock::to_usize()
-    }
-
     /// Returns the `MAX_BLOB_COMMITMENTS_PER_BLOCK` constant for this specification.
     fn max_blob_commitments_per_block() -> usize {
         Self::MaxBlobCommitmentsPerBlock::to_usize()
@@ -421,7 +414,6 @@ impl EthSpec for MainnetEthSpec {
     type GasLimitDenominator = U1024;
     type MinGasLimit = U5000;
     type MaxExtraDataBytes = U32;
-    type MaxBlobsPerBlock = U6;
     type MaxBlobCommitmentsPerBlock = U4096;
     type BytesPerFieldElement = U32;
     type FieldElementsPerBlob = U4096;
@@ -505,7 +497,6 @@ impl EthSpec for MinimalEthSpec {
         MinGasLimit,
         MaxExtraDataBytes,
         MaxBlsToExecutionChanges,
-        MaxBlobsPerBlock,
         BytesPerFieldElement,
         PendingDepositsLimit,
         MaxPendingDepositsPerEpoch,
@@ -559,7 +550,6 @@ impl EthSpec for GnosisEthSpec {
     type SlotsPerEth1VotingPeriod = U1024; // 64 epochs * 16 slots per epoch
     type MaxBlsToExecutionChanges = U16;
     type MaxWithdrawalsPerPayload = U8;
-    type MaxBlobsPerBlock = U6;
     type MaxBlobCommitmentsPerBlock = U4096;
     type FieldElementsPerBlob = U4096;
     type BytesPerFieldElement = U32;

consensus/types/src/lib.rs

@@ -108,6 +108,7 @@ pub mod data_column_sidecar;
 pub mod data_column_subnet_id;
 pub mod light_client_header;
 pub mod non_zero_usize;
+pub mod runtime_fixed_vector;
 pub mod runtime_var_list;
 
 pub use crate::activation_queue::ActivationQueue;
@@ -223,6 +224,7 @@ pub use crate::preset::{
 pub use crate::proposer_preparation_data::ProposerPreparationData;
 pub use crate::proposer_slashing::ProposerSlashing;
 pub use crate::relative_epoch::{Error as RelativeEpochError, RelativeEpoch};
+pub use crate::runtime_fixed_vector::RuntimeFixedVector;
 pub use crate::runtime_var_list::RuntimeVariableList;
 pub use crate::selection_proof::SelectionProof;
 pub use crate::shuffling_id::AttestationShufflingId;

consensus/types/src/preset.rs

@@ -205,8 +205,6 @@ impl CapellaPreset {
 #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "UPPERCASE")]
 pub struct DenebPreset {
-    #[serde(with = "serde_utils::quoted_u64")]
-    pub max_blobs_per_block: u64,
     #[serde(with = "serde_utils::quoted_u64")]
     pub max_blob_commitments_per_block: u64,
     #[serde(with = "serde_utils::quoted_u64")]
@@ -216,7 +214,6 @@ pub struct DenebPreset {
 impl DenebPreset {
     pub fn from_chain_spec<E: EthSpec>(_spec: &ChainSpec) -> Self {
         Self {
-            max_blobs_per_block: E::max_blobs_per_block() as u64,
             max_blob_commitments_per_block: E::max_blob_commitments_per_block() as u64,
             field_elements_per_blob: E::field_elements_per_blob() as u64,
         }

consensus/types/src/runtime_fixed_vector.rs

@@ -0,0 +1,81 @@
+//! Emulates a fixed size array but with the length set at runtime.
+//!
+//! The length of the list cannot be changed once it is set.
+
+#[derive(Clone, Debug)]
+pub struct RuntimeFixedVector<T> {
+    vec: Vec<T>,
+    len: usize,
+}
+
+impl<T: Clone + Default> RuntimeFixedVector<T> {
+    pub fn new(vec: Vec<T>) -> Self {
+        let len = vec.len();
+        Self { vec, len }
+    }
+
+    pub fn to_vec(&self) -> Vec<T> {
+        self.vec.clone()
+    }
+
+    pub fn as_slice(&self) -> &[T] {
+        self.vec.as_slice()
+    }
+
+    #[allow(clippy::len_without_is_empty)]
+    pub fn len(&self) -> usize {
+        self.len
+    }
+
+    pub fn into_vec(self) -> Vec<T> {
+        self.vec
+    }
+
+    pub fn default(max_len: usize) -> Self {
+        Self {
+            vec: vec![T::default(); max_len],
+            len: max_len,
+        }
+    }
+
+    pub fn take(&mut self) -> Self {
+        let new = std::mem::take(&mut self.vec);
+        *self = Self::new(vec![T::default(); self.len]);
+        Self {
+            vec: new,
+            len: self.len,
+        }
+    }
+}
+
+impl<T> std::ops::Deref for RuntimeFixedVector<T> {
+    type Target = [T];
+
+    fn deref(&self) -> &[T] {
+        &self.vec[..]
+    }
+}
+
+impl<T> std::ops::DerefMut for RuntimeFixedVector<T> {
+    fn deref_mut(&mut self) -> &mut [T] {
+        &mut self.vec[..]
+    }
+}
+
+impl<T> IntoIterator for RuntimeFixedVector<T> {
+    type Item = T;
+    type IntoIter = std::vec::IntoIter<T>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.vec.into_iter()
+    }
+}
+
+impl<'a, T> IntoIterator for &'a RuntimeFixedVector<T> {
+    type Item = &'a T;
+    type IntoIter = std::slice::Iter<'a, T>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.vec.iter()
+    }
+}

consensus/types/src/runtime_var_list.rs

@@ -2,7 +2,7 @@ use derivative::Derivative;
 use serde::{Deserialize, Serialize};
 use ssz::Decode;
 use ssz_types::Error;
-use std::ops::{Deref, DerefMut, Index, IndexMut};
+use std::ops::{Deref, Index, IndexMut};
 use std::slice::SliceIndex;
 
 /// Emulates a SSZ `List`.
@@ -10,6 +10,8 @@ use std::slice::SliceIndex;
 /// An ordered, heap-allocated, variable-length, homogeneous collection of `T`, with no more than
 /// `max_len` values.
 ///
+/// To ensure there are no inconsistent states, we do not allow any mutating operation if `max_len` is not set.
+///
 /// ## Example
 ///
 /// ```
@@ -35,6 +37,7 @@ use std::slice::SliceIndex;
 ///
 /// // Push a value to if it _does_ exceed the maximum.
 /// assert!(long.push(6).is_err());
+///
 /// ```
 #[derive(Debug, Clone, Serialize, Deserialize, Derivative)]
 #[derivative(PartialEq, Eq, Hash(bound = "T: std::hash::Hash"))]
@@ -65,7 +68,7 @@ impl<T> RuntimeVariableList<T> {
         Self { vec, max_len }
     }
 
-    /// Create an empty list.
+    /// Create an empty list with the given `max_len`.
     pub fn empty(max_len: usize) -> Self {
         Self {
             vec: vec![],
@@ -77,6 +80,10 @@ impl<T> RuntimeVariableList<T> {
         self.vec.as_slice()
     }
 
+    pub fn as_mut_slice(&mut self) -> &mut [T] {
+        self.vec.as_mut_slice()
+    }
+
     /// Returns the number of values presently in `self`.
     pub fn len(&self) -> usize {
         self.vec.len()
@@ -88,6 +95,8 @@ impl<T> RuntimeVariableList<T> {
     }
 
     /// Returns the type-level maximum length.
+    ///
+    /// Returns `None` if self is uninitialized with a max_len.
     pub fn max_len(&self) -> usize {
         self.max_len
     }
@@ -169,12 +178,6 @@ impl<T> Deref for RuntimeVariableList<T> {
     }
 }
 
-impl<T> DerefMut for RuntimeVariableList<T> {
-    fn deref_mut(&mut self) -> &mut [T] {
-        &mut self.vec[..]
-    }
-}
-
 impl<'a, T> IntoIterator for &'a RuntimeVariableList<T> {
     type Item = &'a T;
     type IntoIter = std::slice::Iter<'a, T>;