From 8b665b554a473f2a50398521a3dfbf6954b0212a Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 3 Sep 2024 18:38:19 -0700 Subject: [PATCH] cleanup --- beacon_node/beacon_chain/src/kzg_utils.rs | 5 ++-- .../lighthouse_network/src/rpc/methods.rs | 9 ++++---- .../network/src/sync/network_context.rs | 4 ++-- consensus/types/src/blob_sidecar.rs | 2 +- consensus/types/src/runtime_var_list.rs | 23 ++++++++++++++++--- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index db0847073a..64e960f0f3 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -188,8 +188,9 @@ fn build_data_column_sidecars( spec: &ChainSpec, ) -> Result, String> { let number_of_columns = spec.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 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]; diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 0ff469aafb..5d788552da 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -27,7 +27,7 @@ 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; @@ -335,11 +335,12 @@ 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. + /// bounds checking and rate limiting. pub fn max_blobs_requested(&self) -> u64 { - self.count.saturating_mul(MAX_BLOBS_PER_BLOCK_CEILING as u64) + self.count + .saturating_mul(MAX_BLOBS_PER_BLOCK_CEILING) } } diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 4b255fee4d..a489ed7057 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -35,8 +35,8 @@ use std::time::Duration; use tokio::sync::mpsc; use types::blob_sidecar::FixedBlobSidecarList; use types::{ - BlobSidecar, ColumnIndex, DataColumnSidecar, DataColumnSidecarList, EthSpec, - Hash256, SignedBeaconBlock, Slot, + BlobSidecar, ColumnIndex, DataColumnSidecar, DataColumnSidecarList, EthSpec, Hash256, + SignedBeaconBlock, Slot, }; pub mod custody; diff --git a/consensus/types/src/blob_sidecar.rs b/consensus/types/src/blob_sidecar.rs index 108c807b47..9acbd8d95c 100644 --- a/consensus/types/src/blob_sidecar.rs +++ b/consensus/types/src/blob_sidecar.rs @@ -265,7 +265,7 @@ impl BlobSidecar { } pub type BlobSidecarList = RuntimeVariableList>>; -/// Alias for a non length-constrained list of `BlobSidecar`s. +/// Alias for a non length-constrained list of `BlobSidecar`s. pub type FixedBlobSidecarList = RuntimeFixedList>>>; pub type BlobsList = VariableList, ::MaxBlobCommitmentsPerBlock>; diff --git a/consensus/types/src/runtime_var_list.rs b/consensus/types/src/runtime_var_list.rs index b24be185da..8d03535f34 100644 --- a/consensus/types/src/runtime_var_list.rs +++ b/consensus/types/src/runtime_var_list.rs @@ -10,6 +10,11 @@ use std::slice::SliceIndex; /// An ordered, heap-allocated, variable-length, homogeneous collection of `T`, with no more than /// `max_len` values. /// +/// In cases where the `max_length` of the container is unknown at time of initialization, we provide +/// a `Self::empty_uninitialized` constructor that initializes a runtime list without setting the max_len. +/// +/// To ensure there are no inconsistent states, we do not allow any mutating operation if `max_len` is not set. +/// /// ## Example /// /// ``` @@ -35,6 +40,16 @@ use std::slice::SliceIndex; /// /// // Push a value to if it _does_ exceed the maximum. /// assert!(long.push(6).is_err()); +/// +/// let mut uninit = RuntimeVariableList::empty_unitialized(); +/// assert!(uninit.push(5).is_err()); +/// +/// // Set max_len to allow mutation. +/// uninit.set_max_len(5usize); +/// +/// uninit.push(5).unwrap(); +/// assert_eq!(&uninit[..], &[5]); +/// /// ``` #[derive(Debug, Clone, Serialize, Deserialize, Derivative)] #[derivative(PartialEq, Eq, Hash(bound = "T: std::hash::Hash"))] @@ -73,7 +88,7 @@ impl RuntimeVariableList { } } - /// Create an empty list. + /// Create an empty list with the given `max_len`. pub fn empty(max_len: usize) -> Self { Self { vec: vec![], @@ -95,7 +110,7 @@ impl RuntimeVariableList { /// Returns an instance of `Self` with max_len = None. /// /// No mutating operation can be performed on an uninitialized instance - /// without first setting max_len. + /// without first setting `max_len`. pub fn empty_uninitialized() -> Self { Self { vec: vec![], @@ -129,7 +144,7 @@ impl RuntimeVariableList { /// Returns `Err(())` when appending `value` would exceed the maximum length. pub fn push(&mut self, value: T) -> Result<(), Error> { let Some(max_len) = self.max_len else { - // TODO(pawan): set a better error + // TODO(pawan): set a better error? return Err(Error::MissingLengthInformation); }; if self.vec.len() < max_len { @@ -247,6 +262,7 @@ where } } +/// Emulates a SSZ `Vector`. #[derive(Clone, Debug)] pub struct RuntimeFixedList { vec: Vec, @@ -267,6 +283,7 @@ impl RuntimeFixedList { self.vec.as_slice() } + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.len }