diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 9d74400336..e1cf40f65f 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1249,7 +1249,10 @@ impl BeaconChain { pub fn get_blobs(&self, block_root: &Hash256) -> Result, Error> { match self.store.get_blobs(block_root)? { Some(blobs) => Ok(blobs), - None => Ok(BlobSidecarList::default()), + None => Ok(BlobSidecarList::empty( + // TODO(pawan): fix this + self.spec.max_blobs_per_block(Epoch::new(0)) as usize, + )), } } diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 99fc5d9d0c..4a39a9d253 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -17,7 +17,8 @@ use std::time::Duration; use tree_hash::TreeHash; use types::blob_sidecar::BlobIdentifier; use types::{ - BeaconStateError, BlobSidecar, Epoch, EthSpec, Hash256, SignedBeaconBlockHeader, Slot, + BeaconStateError, BlobSidecar, Epoch, EthSpec, Hash256, RuntimeVariableList, + SignedBeaconBlockHeader, Slot, }; /// An error occurred while validating a gossip blob. @@ -172,10 +173,7 @@ impl From for GossipBlobError { } } -pub type GossipVerifiedBlobList = VariableList< - GossipVerifiedBlob, - <::EthSpec as EthSpec>::MaxBlobsPerBlock, ->; +pub type GossipVerifiedBlobList = RuntimeVariableList>; /// A wrapper around a `BlobSidecar` that indicates it has been approved for re-gossiping on /// the p2p network. @@ -399,7 +397,7 @@ pub fn validate_blob_sidecar_for_gossip( // since we only subscribe to `MaxBlobsPerBlock` subnets over gossip network. // We include this check only for completeness. // Getting this error would imply something very wrong with our networking decoding logic. - if blob_index >= T::EthSpec::max_blobs_per_block() as u64 { + if blob_index >= chain.spec.max_blobs_per_block(blob_epoch) { return Err(GossipBlobError::InvalidSubnet { expected: subnet, received: blob_index, diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index d9662d59f9..6582096bef 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -779,7 +779,9 @@ fn build_gossip_verified_blobs( GossipVerifiedBlob::new(Arc::new(blob), i as u64, chain)?; gossip_verified_blobs.push(gossip_verified_blob); } - let gossip_verified_blobs = VariableList::from(gossip_verified_blobs); + let max_len = chain.spec.max_blobs_per_block(block.epoch()) as usize; + let gossip_verified_blobs = + RuntimeVariableList::from_vec(gossip_verified_blobs, max_len); Ok::<_, BlockContentsError>(gossip_verified_blobs) }) .transpose() diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index b271f0a2f9..67643318f2 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -181,23 +181,6 @@ impl RpcBlock { }) } - pub fn new_from_fixed( - block_root: Hash256, - block: Arc>, - blobs: FixedBlobSidecarList, - ) -> Result { - let filtered = blobs - .into_iter() - .filter_map(|b| b.clone()) - .collect::>(); - let blobs = if filtered.is_empty() { - None - } else { - Some(VariableList::from(filtered)) - }; - Self::new(Some(block_root), block, blobs) - } - #[allow(clippy::type_complexity)] pub fn deconstruct( self, diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 470cee713f..cef7c528f6 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -200,7 +200,7 @@ impl DataAvailabilityChecker { .ok_or(AvailabilityCheckError::SlotClockError)?; let verified_blobs = - KzgVerifiedBlobList::new(Vec::from(blobs).into_iter().flatten(), kzg, seen_timestamp) + KzgVerifiedBlobList::new(blobs.into_vec().into_iter().flatten(), kzg, seen_timestamp) .map_err(AvailabilityCheckError::Kzg)?; self.availability_cache @@ -384,14 +384,13 @@ impl DataAvailabilityChecker { blocks: Vec>, ) -> Result>, AvailabilityCheckError> { let mut results = Vec::with_capacity(blocks.len()); - let all_blobs: BlobSidecarList = blocks + let all_blobs = blocks .iter() .filter(|block| self.blobs_required_for_block(block.as_block())) // this clone is cheap as it's cloning an Arc .filter_map(|block| block.blobs().cloned()) .flatten() - .collect::>() - .into(); + .collect::>(); // verify kzg for all blobs at once if !all_blobs.is_empty() { diff --git a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs index 4863982b55..9c33051ae0 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs @@ -16,9 +16,10 @@ use std::collections::HashSet; use std::num::NonZeroUsize; use std::sync::Arc; use types::blob_sidecar::BlobIdentifier; +use types::runtime_var_list::RuntimeFixedList; use types::{ BlobSidecar, ChainSpec, ColumnIndex, DataColumnIdentifier, DataColumnSidecar, - DataColumnSidecarList, Epoch, EthSpec, Hash256, SignedBeaconBlock, + DataColumnSidecarList, Epoch, EthSpec, Hash256, RuntimeVariableList, SignedBeaconBlock, }; pub type DataColumnsToPublish = Option>; @@ -32,7 +33,7 @@ pub type DataColumnsToPublish = Option>; #[derive(Clone)] pub struct PendingComponents { pub block_root: Hash256, - pub verified_blobs: FixedVector>, E::MaxBlobsPerBlock>, + pub verified_blobs: RuntimeFixedList>>, pub verified_data_columns: Vec>, pub executed_block: Option>, pub reconstruction_started: bool, @@ -50,9 +51,7 @@ impl PendingComponents { } /// Returns an immutable reference to the fixed vector of cached blobs. - pub fn get_cached_blobs( - &self, - ) -> &FixedVector>, E::MaxBlobsPerBlock> { + pub fn get_cached_blobs(&self) -> &RuntimeFixedList>> { &self.verified_blobs } @@ -73,9 +72,7 @@ impl PendingComponents { } /// Returns a mutable reference to the fixed vector of cached blobs. - pub fn get_cached_blobs_mut( - &mut self, - ) -> &mut FixedVector>, E::MaxBlobsPerBlock> { + pub fn get_cached_blobs_mut(&mut self) -> &mut RuntimeFixedList>> { &mut self.verified_blobs } @@ -147,10 +144,7 @@ impl PendingComponents { /// Blobs are only inserted if: /// 1. The blob entry at the index is empty and no block exists. /// 2. The block exists and its commitment matches the blob's commitment. - pub fn merge_blobs( - &mut self, - blobs: FixedVector>, E::MaxBlobsPerBlock>, - ) { + pub fn merge_blobs(&mut self, blobs: RuntimeFixedList>>) { for (index, blob) in blobs.iter().cloned().enumerate() { let Some(blob) = blob else { continue }; self.merge_single_blob(index, blob); @@ -194,7 +188,7 @@ impl PendingComponents { /// Blobs that don't match the new block's commitments are evicted. pub fn merge_block(&mut self, block: DietAvailabilityPendingExecutedBlock) { self.insert_block(block); - let reinsert = std::mem::take(self.get_cached_blobs_mut()); + let reinsert = self.get_cached_blobs_mut().take(); self.merge_blobs(reinsert); } @@ -223,10 +217,11 @@ impl PendingComponents { } /// Returns an empty `PendingComponents` object with the given block root. - pub fn empty(block_root: Hash256) -> Self { + pub fn empty(block_root: Hash256, max_len: usize) -> Self { Self { block_root, - verified_blobs: FixedVector::default(), + // TODO(pawan): just make this a vec potentially + verified_blobs: RuntimeFixedList::new(vec![None; max_len]), verified_data_columns: vec![], executed_block: None, reconstruction_started: false, @@ -280,7 +275,12 @@ impl PendingComponents { else { return Err(AvailabilityCheckError::Unexpected); }; - (Some(VariableList::new(verified_blobs)?), None) + let max_len = + spec.max_blobs_per_block(diet_executed_block.as_block().epoch()) as usize; + ( + Some(RuntimeVariableList::new(verified_blobs, max_len)?), + None, + ) } BlockImportRequirement::CustodyColumns(_) => { let verified_data_columns = verified_data_columns @@ -477,7 +477,8 @@ impl DataAvailabilityCheckerInner { epoch: Epoch, kzg_verified_blobs: I, ) -> Result, AvailabilityCheckError> { - let mut fixed_blobs = FixedVector::default(); + let mut fixed_blobs = + RuntimeFixedList::new(vec![None; self.spec.max_blobs_per_block(epoch) as usize]); for blob in kzg_verified_blobs { if let Some(blob_opt) = fixed_blobs.get_mut(blob.blob_index() as usize) { @@ -491,7 +492,9 @@ impl DataAvailabilityCheckerInner { let mut pending_components = write_lock .pop_entry(&block_root) .map(|(_, v)| v) - .unwrap_or_else(|| PendingComponents::empty(block_root)); + .unwrap_or_else(|| { + PendingComponents::empty(block_root, self.spec.max_blobs_per_block(epoch) as usize) + }); // Merge in the blobs. pending_components.merge_blobs(fixed_blobs); @@ -527,7 +530,9 @@ impl DataAvailabilityCheckerInner { let mut pending_components = write_lock .pop_entry(&block_root) .map(|(_, v)| v) - .unwrap_or_else(|| PendingComponents::empty(block_root)); + .unwrap_or_else(|| { + PendingComponents::empty(block_root, self.spec.max_blobs_per_block(epoch) as usize) + }); // Merge in the data columns. pending_components.merge_data_columns(kzg_verified_data_columns)?; @@ -614,7 +619,9 @@ impl DataAvailabilityCheckerInner { let mut pending_components = write_lock .pop_entry(&block_root) .map(|(_, v)| v) - .unwrap_or_else(|| PendingComponents::empty(block_root)); + .unwrap_or_else(|| { + PendingComponents::empty(block_root, self.spec.max_blobs_per_block(epoch) as usize) + }); // Merge in the block. pending_components.merge_block(diet_executed_block); diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 55c1ee9e98..db0847073a 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -188,9 +188,10 @@ fn build_data_column_sidecars( spec: &ChainSpec, ) -> Result, String> { let number_of_columns = spec.number_of_columns; - let mut columns = vec![Vec::with_capacity(E::max_blobs_per_block()); number_of_columns]; - let mut column_kzg_proofs = - vec![Vec::with_capacity(E::max_blobs_per_block()); number_of_columns]; + let max_blobs_per_block = + spec.max_blobs_per_block(signed_block_header.message.slot.epoch(E::slots_per_epoch())) as usize; + let mut columns = vec![Vec::with_capacity(max_blobs_per_block); number_of_columns]; + let mut column_kzg_proofs = vec![Vec::with_capacity(max_blobs_per_block); number_of_columns]; for (blob_cells, blob_cell_proofs) in blob_cells_and_proofs_vec { // we iterate over each column, and we construct the column from "top to bottom", diff --git a/beacon_node/beacon_chain/src/observed_data_sidecars.rs b/beacon_node/beacon_chain/src/observed_data_sidecars.rs index 601241dd8a..7cc5f2e92a 100644 --- a/beacon_node/beacon_chain/src/observed_data_sidecars.rs +++ b/beacon_node/beacon_chain/src/observed_data_sidecars.rs @@ -23,7 +23,7 @@ pub trait ObservableDataSidecar { fn slot(&self) -> Slot; fn block_proposer_index(&self) -> u64; fn index(&self) -> u64; - fn max_num_of_items(spec: &ChainSpec) -> usize; + fn max_num_of_items(spec: &ChainSpec, slot: Slot) -> usize; } impl ObservableDataSidecar for BlobSidecar { @@ -39,8 +39,8 @@ impl ObservableDataSidecar for BlobSidecar { self.index } - fn max_num_of_items(_spec: &ChainSpec) -> usize { - E::max_blobs_per_block() + fn max_num_of_items(spec: &ChainSpec, slot: Slot) -> usize { + spec.max_blobs_per_block(slot.epoch(E::slots_per_epoch())) as usize } } @@ -57,7 +57,7 @@ impl ObservableDataSidecar for DataColumnSidecar { self.index } - fn max_num_of_items(spec: &ChainSpec) -> usize { + fn max_num_of_items(spec: &ChainSpec, _slot: Slot) -> usize { spec.number_of_columns } } @@ -102,7 +102,9 @@ impl ObservedDataSidecars { slot: data_sidecar.slot(), proposer: data_sidecar.block_proposer_index(), }) - .or_insert_with(|| HashSet::with_capacity(T::max_num_of_items(&self.spec))); + .or_insert_with(|| { + HashSet::with_capacity(T::max_num_of_items(&self.spec, data_sidecar.slot())) + }); let did_not_exist = data_indices.insert(data_sidecar.index()); Ok(!did_not_exist) @@ -122,7 +124,7 @@ impl ObservedDataSidecars { } fn sanitize_data_sidecar(&self, data_sidecar: &T) -> Result<(), Error> { - if data_sidecar.index() >= T::max_num_of_items(&self.spec) as u64 { + if data_sidecar.index() >= T::max_num_of_items(&self.spec, data_sidecar.slot()) as u64 { return Err(Error::InvalidDataIndex(data_sidecar.index())); } let finalized_slot = self.finalized_slot; diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index b28d221da7..cd2e355caf 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1991,7 +1991,7 @@ where let (block, blob_items) = block_contents; let sidecars = blob_items - .map(|(proofs, blobs)| BlobSidecar::build_sidecars(blobs, &block, proofs)) + .map(|(proofs, blobs)| BlobSidecar::build_sidecars(blobs, &block, proofs, &self.spec)) .transpose() .unwrap(); let block_hash: SignedBeaconBlockHash = self @@ -2017,7 +2017,7 @@ where let (block, blob_items) = block_contents; let sidecars = blob_items - .map(|(proofs, blobs)| BlobSidecar::build_sidecars(blobs, &block, proofs)) + .map(|(proofs, blobs)| BlobSidecar::build_sidecars(blobs, &block, proofs, &self.spec)) .transpose() .unwrap(); let block_root = block.canonical_root(); @@ -2636,7 +2636,8 @@ pub fn generate_rand_block_and_blobs( // Get either zero blobs or a random number of blobs between 1 and Max Blobs. let payload: &mut FullPayloadDeneb = &mut message.body.execution_payload; let num_blobs = match num_blobs { - NumBlobs::Random => rng.gen_range(1..=E::max_blobs_per_block()), + // TODO(pawan): thread the chainspec value here + NumBlobs::Random => rng.gen_range(1..=6), NumBlobs::Number(n) => n, NumBlobs::None => 0, }; @@ -2656,7 +2657,8 @@ pub fn generate_rand_block_and_blobs( // Get either zero blobs or a random number of blobs between 1 and Max Blobs. let payload: &mut FullPayloadElectra = &mut message.body.execution_payload; let num_blobs = match num_blobs { - NumBlobs::Random => rng.gen_range(1..=E::max_blobs_per_block()), + // TODO(pawan): thread the chainspec value here + NumBlobs::Random => rng.gen_range(1..=6), NumBlobs::Number(n) => n, NumBlobs::None => 0, }; diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index d299eebec8..aa50ede84a 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -352,10 +352,11 @@ where let anchor_block = SignedBeaconBlock::from_ssz_bytes(&anchor_block_bytes, &spec) .map_err(|e| format!("Unable to parse weak subj block SSZ: {:?}", e))?; let anchor_blobs = if anchor_block.message().body().has_blobs() { + let max_blobs_len = spec.max_blobs_per_block(anchor_block.epoch()) as usize; let anchor_blobs_bytes = anchor_blobs_bytes .ok_or("Blobs for checkpoint must be provided using --checkpoint-blobs")?; Some( - BlobSidecarList::from_ssz_bytes(&anchor_blobs_bytes) + BlobSidecarList::from_ssz_bytes(&anchor_blobs_bytes, max_blobs_len) .map_err(|e| format!("Unable to parse weak subj blobs SSZ: {e:?}"))?, ) } else { diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index 216c3b7844..a18ac711aa 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -673,7 +673,8 @@ impl ExecutionBlockGenerator { ForkName::Deneb | ForkName::Electra => { // get random number between 0 and Max Blobs let mut rng = self.rng.lock(); - let num_blobs = rng.gen::() % (E::max_blobs_per_block() + 1); + // TODO(pawan): thread the chainspec value here somehow + let num_blobs = rng.gen::() % 6; let (bundle, transactions) = generate_blobs(num_blobs)?; for tx in Vec::from(transactions) { execution_payload diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index f35df2f5e8..e3ed15ebc2 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -279,13 +279,14 @@ impl BlockId { .get_blobs(&root) .map_err(warp_utils::reject::beacon_chain_error)?; + let max_len = blob_sidecar_list.max_len(); let blob_sidecar_list_filtered = match indices.indices { Some(vec) => { let list = blob_sidecar_list .into_iter() .filter(|blob_sidecar| vec.contains(&blob_sidecar.index)) .collect(); - BlobSidecarList::new(list) + BlobSidecarList::new(list, max_len) .map_err(|e| warp_utils::reject::custom_server_error(format!("{:?}", e)))? } None => blob_sidecar_list, diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index bbdfc31d43..9f1d17677e 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -22,7 +22,8 @@ use tree_hash::TreeHash; use types::{ AbstractExecPayload, BeaconBlockRef, BlobSidecarList, BlockImportSource, DataColumnSidecarList, DataColumnSubnetId, EthSpec, ExecPayload, ExecutionBlockHash, ForkName, FullPayload, - FullPayloadBellatrix, Hash256, SignedBeaconBlock, SignedBlindedBeaconBlock, VariableList, + FullPayloadBellatrix, Hash256, RuntimeVariableList, SignedBeaconBlock, + SignedBlindedBeaconBlock, VariableList, }; use warp::http::StatusCode; use warp::{reply::Response, Rejection, Reply}; @@ -198,7 +199,10 @@ pub async fn publish_block>(); - VariableList::from(blobs) + RuntimeVariableList::from_vec( + blobs, + chain.spec.max_blobs_per_block(block.epoch()) as usize, + ) }); let data_cols_opt = gossip_verified_data_columns .as_ref() diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index a96b9d1b16..0ff469aafb 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -24,6 +24,13 @@ use types::{ pub type MaxErrorLen = U256; pub const MAX_ERROR_LEN: u64 = 256; +/// The max number of blobs we expect in the configs to set for compile time params. +/// Note: This value is an estimate that we should use only for rate limiting, +/// bounds checking and other non-consensus critical operations. +/// +/// For exact value, we should always check the chainspec. +pub const MAX_BLOBS_PER_BLOCK_CEILING: u64 = 16; + /// Wrapper over SSZ List to represent error message in rpc responses. #[derive(Debug, Clone)] pub struct ErrorType(pub VariableList); @@ -326,8 +333,13 @@ pub struct BlobsByRangeRequest { } impl BlobsByRangeRequest { + /// This function provides an upper bound on number of blobs expected in + /// a certain slot range. + /// + /// Note: **must not** use for anything consensus critical, only for + /// bounds checking and rate limiting. pub fn max_blobs_requested(&self) -> u64 { - self.count.saturating_mul(E::max_blobs_per_block() as u64) + self.count.saturating_mul(MAX_BLOBS_PER_BLOCK_CEILING as u64) } } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index f4bdf6450b..1b560f1d1e 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -92,7 +92,7 @@ pub static SIGNED_BEACON_BLOCK_DENEB_MAX: LazyLock = LazyLock::new(|| { *SIGNED_BEACON_BLOCK_CAPELLA_MAX_WITHOUT_PAYLOAD + types::ExecutionPayload::::max_execution_payload_deneb_size() // adding max size of execution payload (~16gb) + ssz::BYTES_PER_LENGTH_OFFSET // Adding the additional offsets for the `ExecutionPayload` - + (::ssz_fixed_len() * ::max_blobs_per_block()) + + (::ssz_fixed_len() * MAX_BLOBS_PER_BLOCK_CEILING as usize) + ssz::BYTES_PER_LENGTH_OFFSET }); // Length offset for the blob commitments field. // @@ -100,7 +100,7 @@ pub static SIGNED_BEACON_BLOCK_ELECTRA_MAX: LazyLock = LazyLock::new(|| { *SIGNED_BEACON_BLOCK_ELECTRA_MAX_WITHOUT_PAYLOAD + types::ExecutionPayload::::max_execution_payload_electra_size() // adding max size of execution payload (~16gb) + ssz::BYTES_PER_LENGTH_OFFSET // Adding the additional ssz offset for the `ExecutionPayload` field - + (::ssz_fixed_len() * ::max_blobs_per_block()) + + (::ssz_fixed_len() * MAX_BLOBS_PER_BLOCK_CEILING as usize) + ssz::BYTES_PER_LENGTH_OFFSET }); // Length offset for the blob commitments field. @@ -636,7 +636,7 @@ pub fn rpc_blob_limits() -> RpcLimits { pub fn rpc_data_column_limits() -> RpcLimits { RpcLimits::new( DataColumnSidecar::::empty().as_ssz_bytes().len(), - DataColumnSidecar::::max_size(), + DataColumnSidecar::::max_size(MAX_BLOBS_PER_BLOCK_CEILING as usize), ) } diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index 966ce55fab..d642a2b4dc 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -8,7 +8,8 @@ use std::{ sync::Arc, }; use types::{ - BlobSidecar, ChainSpec, ColumnIndex, DataColumnSidecar, EthSpec, Hash256, SignedBeaconBlock, + BlobSidecar, ChainSpec, ColumnIndex, DataColumnSidecar, EthSpec, Hash256, RuntimeVariableList, + SignedBeaconBlock, }; #[derive(Debug)] @@ -31,6 +32,7 @@ pub struct RangeBlockComponentsRequest { num_custody_column_requests: Option, /// The peers the request was made to. pub(crate) peer_ids: Vec, + max_blobs_per_block: usize, } impl RangeBlockComponentsRequest { @@ -39,6 +41,7 @@ impl RangeBlockComponentsRequest { expects_custody_columns: Option>, num_custody_column_requests: Option, peer_ids: Vec, + max_blobs_per_block: usize, ) -> Self { Self { blocks: <_>::default(), @@ -51,6 +54,7 @@ impl RangeBlockComponentsRequest { expects_custody_columns, num_custody_column_requests, peer_ids, + max_blobs_per_block, } } @@ -100,7 +104,7 @@ impl RangeBlockComponentsRequest { let mut responses = Vec::with_capacity(blocks.len()); let mut blob_iter = blobs.into_iter().peekable(); for block in blocks.into_iter() { - let mut blob_list = Vec::with_capacity(E::max_blobs_per_block()); + let mut blob_list = Vec::with_capacity(self.max_blobs_per_block); while { let pair_next_blob = blob_iter .peek() @@ -111,7 +115,7 @@ impl RangeBlockComponentsRequest { blob_list.push(blob_iter.next().ok_or("Missing next blob".to_string())?); } - let mut blobs_buffer = vec![None; E::max_blobs_per_block()]; + let mut blobs_buffer = vec![None; self.max_blobs_per_block]; for blob in blob_list { let blob_index = blob.index as usize; let Some(blob_opt) = blobs_buffer.get_mut(blob_index) else { @@ -123,7 +127,11 @@ impl RangeBlockComponentsRequest { *blob_opt = Some(blob); } } - let blobs = VariableList::from(blobs_buffer.into_iter().flatten().collect::>()); + let blobs = RuntimeVariableList::new( + blobs_buffer.into_iter().flatten().collect::>(), + self.max_blobs_per_block, + ) + .map_err(|_| "Blobs returned exceeds max length".to_string())?; responses.push(RpcBlock::new(None, block, Some(blobs)).map_err(|e| format!("{e:?}"))?) } diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index d6ce14adb1..866268b6fd 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -1120,6 +1120,7 @@ impl SyncManager { .network .range_block_and_blob_response(id, block_or_blob) { + let epoch = resp.sender_id.batch_id(); match resp.responses { Ok(blocks) => { match resp.sender_id { @@ -1163,6 +1164,7 @@ impl SyncManager { resp.expects_custody_columns, None, vec![], + self.chain.spec.max_blobs_per_block(epoch) as usize, ), ); // inform range that the request needs to be treated as failed diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 3da6f92cfe..8d954628eb 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -36,8 +36,8 @@ use std::time::Duration; use tokio::sync::mpsc; use types::blob_sidecar::FixedBlobSidecarList; use types::{ - BlobSidecar, ColumnIndex, DataColumnSidecar, DataColumnSidecarList, EthSpec, Hash256, - SignedBeaconBlock, Slot, + chain_spec, BlobSidecar, ChainSpec, ColumnIndex, DataColumnSidecar, DataColumnSidecarList, + EthSpec, Hash256, SignedBeaconBlock, Slot, }; pub mod custody; @@ -61,6 +61,15 @@ pub enum RangeRequestId { }, } +impl RangeRequestId { + pub fn batch_id(&self) -> BatchId { + match self { + RangeRequestId::RangeSync { batch_id, .. } => *batch_id, + RangeRequestId::BackfillSync { batch_id, .. } => *batch_id, + } + } +} + #[derive(Debug)] pub enum RpcEvent { StreamTermination, @@ -422,11 +431,14 @@ impl SyncNetworkContext { (None, None) }; + // TODO(pawan): this would break if a batch contains multiple epochs + let max_blobs_len = self.chain.spec.max_blobs_per_block(epoch); let info = RangeBlockComponentsRequest::new( expected_blobs, expects_custody_columns, num_of_custody_column_req, requested_peers, + max_blobs_len as usize, ); self.range_block_components_requests .insert(id, (sender_id, info)); @@ -977,9 +989,16 @@ impl SyncNetworkContext { RpcEvent::Response(blob, seen_timestamp) => { let request = request.get_mut(); match request.add_response(blob) { - Ok(Some(blobs)) => to_fixed_blob_sidecar_list(blobs) - .map(|blobs| (blobs, seen_timestamp)) - .map_err(|e| (e.into(), request.resolve())), + Ok(Some(blobs)) => { + let max_len = if let Some(blob) = blobs.first() { + self.chain.spec.max_blobs_per_block(blob.epoch()) as usize + } else { + 6 + }; + to_fixed_blob_sidecar_list(blobs, max_len) + .map(|blobs| (blobs, seen_timestamp)) + .map_err(|e| (e.into(), request.resolve())) + } Ok(None) => return None, Err(e) => Err((e.into(), request.resolve())), } @@ -1218,8 +1237,11 @@ impl SyncNetworkContext { fn to_fixed_blob_sidecar_list( blobs: Vec>>, + max_len: usize, ) -> Result, LookupVerifyError> { - let mut fixed_list = FixedBlobSidecarList::default(); + // TODO(pawan): have a method on fixed runtime vector that just initializes a raw vec with max_len = None + // to signify an empty fixed runtime vector + let mut fixed_list = FixedBlobSidecarList::new(vec![None; max_len]); for blob in blobs.into_iter() { let index = blob.index as usize; *fixed_list diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index fecd8e3744..0addb61937 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -1669,7 +1669,23 @@ impl, Cold: ItemStore> HotColdDB .get_bytes(DBColumn::BeaconBlob.into(), block_root.as_bytes())? { Some(ref blobs_bytes) => { - let blobs = BlobSidecarList::from_ssz_bytes(blobs_bytes)?; + // We insert a VariableList of BlobSidecars into the db, but retrieve + // a plain vec since we don't know the length limit of the list without + // knowing the slot. + // The encoding of a VariableList is same as a regular vec. + let blobs = BlobSidecarVec::from_ssz_bytes(blobs_bytes)?; + let max_blobs_per_block = blobs + .first() + .map(|blob| { + self.spec + .max_blobs_per_block(blob.slot().epoch(E::slots_per_epoch())) + }) + // This is the case where we have no blobs for the slot, doesn't matter what value we keep for max here + // TODO(pawan): double check that this is the case + // we could also potentially deal with just vecs in the db since we only add length validated sidecar + // lists to the db + .unwrap_or(6); + let blobs = BlobSidecarList::from_vec(blobs, max_blobs_per_block as usize); self.block_cache .lock() .put_blobs(*block_root, blobs.clone()); diff --git a/beacon_node/store/src/impls/execution_payload.rs b/beacon_node/store/src/impls/execution_payload.rs index 14fc10ad6d..3f7c4172bc 100644 --- a/beacon_node/store/src/impls/execution_payload.rs +++ b/beacon_node/store/src/impls/execution_payload.rs @@ -1,7 +1,7 @@ use crate::{DBColumn, Error, StoreItem}; use ssz::{Decode, Encode}; use types::{ - BlobSidecarList, EthSpec, ExecutionPayload, ExecutionPayloadBellatrix, ExecutionPayloadCapella, + EthSpec, ExecutionPayload, ExecutionPayloadBellatrix, ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadElectra, }; @@ -26,7 +26,6 @@ impl_store_item!(ExecutionPayloadBellatrix); impl_store_item!(ExecutionPayloadCapella); impl_store_item!(ExecutionPayloadDeneb); impl_store_item!(ExecutionPayloadElectra); -impl_store_item!(BlobSidecarList); /// This fork-agnostic implementation should be only used for writing. /// diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index e7655b453a..7b2ca90055 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -391,10 +391,12 @@ pub fn partially_verify_execution_payload = VariableList::MaxBlobCommitmentsPerBlock>; -pub type KzgCommitmentOpts = - FixedVector, ::MaxBlobsPerBlock>; /// The number of leaves (including padding) on the `BeaconBlockBody` Merkle tree. /// diff --git a/consensus/types/src/blob_sidecar.rs b/consensus/types/src/blob_sidecar.rs index 0f7dbb2673..c2981e14cc 100644 --- a/consensus/types/src/blob_sidecar.rs +++ b/consensus/types/src/blob_sidecar.rs @@ -1,10 +1,13 @@ use crate::test_utils::TestRandom; -use crate::ForkName; use crate::{ beacon_block_body::BLOB_KZG_COMMITMENTS_INDEX, BeaconBlockHeader, BeaconStateError, Blob, Epoch, EthSpec, FixedVector, Hash256, SignedBeaconBlockHeader, Slot, VariableList, }; -use crate::{ForkVersionDeserialize, KzgProofs, SignedBeaconBlock}; +use crate::{ + runtime_var_list::RuntimeFixedList, ForkVersionDeserialize, KzgProofs, RuntimeVariableList, + SignedBeaconBlock, +}; +use crate::{ChainSpec, ForkName}; use bls::Signature; use derivative::Derivative; use kzg::{Blob as KzgBlob, Kzg, KzgCommitment, KzgProof, BYTES_PER_BLOB, BYTES_PER_FIELD_ELEMENT}; @@ -30,19 +33,6 @@ pub struct BlobIdentifier { pub index: u64, } -impl BlobIdentifier { - pub fn get_all_blob_ids(block_root: Hash256) -> Vec { - 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 { Some(self.cmp(other)) @@ -260,19 +250,24 @@ impl BlobSidecar { blobs: BlobsList, block: &SignedBeaconBlock, kzg_proofs: KzgProofs, + spec: &ChainSpec, ) -> Result, 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 = VariableList>, ::MaxBlobsPerBlock>; -pub type FixedBlobSidecarList = - FixedVector>>, ::MaxBlobsPerBlock>; +pub type BlobSidecarList = RuntimeVariableList>>; +/// Alias for a non length-constrained list of `BlobSidecar`s. +pub type BlobSidecarVec = Vec>>; +pub type FixedBlobSidecarList = RuntimeFixedList>>>; pub type BlobsList = VariableList, ::MaxBlobCommitmentsPerBlock>; impl ForkVersionDeserialize for BlobSidecarList { diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 10b00d5ba1..5af0f349de 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -230,6 +230,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 @@ -607,6 +608,16 @@ impl ChainSpec { } } + /// Returns the deneb preset value if peerdas epoch hasn't hit. + /// Otherwise, returns the value obtained from the config.yaml. + pub fn max_blobs_per_block(&self, epoch: Epoch) -> u64 { + if self.is_peer_das_enabled_for_epoch(epoch) { + self.max_blobs_per_block + } else { + default_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) @@ -843,6 +854,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 @@ -1164,6 +1176,8 @@ 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(), + // TODO(pawan): check if gnosis preset values match + max_blobs_per_block: default_max_blobs_per_block(), /* * Derived Deneb Specific @@ -1364,6 +1378,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")] @@ -1499,6 +1516,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 } @@ -1718,6 +1741,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 @@ -1795,6 +1819,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, @@ -1863,6 +1888,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, diff --git a/consensus/types/src/data_column_sidecar.rs b/consensus/types/src/data_column_sidecar.rs index 90c05aea1f..ea0fb2b14a 100644 --- a/consensus/types/src/data_column_sidecar.rs +++ b/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 crate::{BeaconStateError, ChainSpec}; use bls::Signature; use derivative::Derivative; use kzg::Error as KzgError; @@ -110,18 +110,16 @@ impl DataColumnSidecar { .len() } - pub fn max_size() -> usize { + pub fn max_size(max_blobs_per_block: usize) -> usize { Self { index: 0, - column: VariableList::new(vec![Cell::::default(); E::MaxBlobsPerBlock::to_usize()]) - .unwrap(), + column: VariableList::new(vec![Cell::::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(), diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index 09ef8e3c1a..3d496d214e 100644 --- a/consensus/types/src/eth_spec.rs +++ b/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; @@ -280,11 +278,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() @@ -415,7 +408,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; @@ -498,7 +490,6 @@ impl EthSpec for MinimalEthSpec { MinGasLimit, MaxExtraDataBytes, MaxBlsToExecutionChanges, - MaxBlobsPerBlock, BytesPerFieldElement, PendingBalanceDepositsLimit, MaxConsolidationRequestsPerPayload, @@ -551,7 +542,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; diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 68d48ec7c8..4db50c30e1 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -138,7 +138,9 @@ pub use crate::beacon_block_body::{ pub use crate::beacon_block_header::BeaconBlockHeader; pub use crate::beacon_committee::{BeaconCommittee, OwnedBeaconCommittee}; pub use crate::beacon_state::{Error as BeaconStateError, *}; -pub use crate::blob_sidecar::{BlobIdentifier, BlobSidecar, BlobSidecarList, BlobsList}; +pub use crate::blob_sidecar::{ + BlobIdentifier, BlobSidecar, BlobSidecarList, BlobSidecarVec, BlobsList, +}; pub use crate::bls_to_execution_change::BlsToExecutionChange; pub use crate::chain_spec::{ChainSpec, Config, Domain}; pub use crate::checkpoint::Checkpoint; diff --git a/consensus/types/src/preset.rs b/consensus/types/src/preset.rs index 2c576ed332..0d067d3fe9 100644 --- a/consensus/types/src/preset.rs +++ b/consensus/types/src/preset.rs @@ -208,8 +208,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")] @@ -219,7 +217,6 @@ pub struct DenebPreset { impl DenebPreset { pub fn from_chain_spec(_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, } diff --git a/consensus/types/src/runtime_var_list.rs b/consensus/types/src/runtime_var_list.rs index af4ee87c15..4c6d6e2d92 100644 --- a/consensus/types/src/runtime_var_list.rs +++ b/consensus/types/src/runtime_var_list.rs @@ -214,6 +214,67 @@ where } } +#[derive(Clone, Debug)] +pub struct RuntimeFixedList { + vec: Vec, + len: usize, +} + +impl RuntimeFixedList { + // TODO(pawan): no need to take len + pub fn new(vec: Vec) -> Self { + let len = vec.len(); + Self { vec, len } + } + + pub fn to_vec(&self) -> Vec { + self.vec.clone() + } + + pub fn as_slice(&self) -> &[T] { + self.vec.as_slice() + } + + pub fn len(&self) -> usize { + self.len + } + + pub fn into_vec(self) -> Vec { + self.vec + } + + pub fn take(&mut self) -> Self { + let new = std::mem::take(&mut self.vec); + Self { + vec: new, + len: self.len, + } + } +} + +impl std::ops::Deref for RuntimeFixedList { + type Target = [T]; + + fn deref(&self) -> &[T] { + &self.vec[..] + } +} + +impl std::ops::DerefMut for RuntimeFixedList { + fn deref_mut(&mut self) -> &mut [T] { + &mut self.vec[..] + } +} + +impl<'a, T> IntoIterator for &'a RuntimeFixedList { + type Item = &'a T; + type IntoIter = std::slice::Iter<'a, T>; + + fn into_iter(self) -> Self::IntoIter { + self.vec.iter() + } +} + #[cfg(test)] mod test { use super::*;