Small Improvements (#4980)

* initial changes

* use arc<blobsidecar> in vector

* Utilize new pattern for KzgVerifiedBlob

* fmt

* Update beacon_node/beacon_chain/src/blob_verification.rs

Co-authored-by: realbigsean <seananderson33@GMAIL.com>

* forgot to save..

* lint

* fmt.. again

---------

Co-authored-by: realbigsean <seananderson33@GMAIL.com>

beacon_node/beacon_chain/src/blob_verification.rs

@@ -276,6 +276,10 @@ impl<T: EthSpec> Ord for KzgVerifiedBlob<T> {
 }
 
 impl<T: EthSpec> KzgVerifiedBlob<T> {
+    pub fn new(blob: Arc<BlobSidecar<T>>, kzg: &Kzg) -> Result<Self, KzgError> {
+        verify_kzg_for_blob(blob, kzg)
+    }
+
     pub fn to_blob(self) -> Arc<BlobSidecar<T>> {
         self.blob
     }
@@ -289,14 +293,12 @@ impl<T: EthSpec> KzgVerifiedBlob<T> {
     pub fn blob_index(&self) -> u64 {
         self.blob.index
     }
-}
-
-#[cfg(test)]
-impl<T: EthSpec> KzgVerifiedBlob<T> {
-    pub fn new(blob: BlobSidecar<T>) -> Self {
-        Self {
-            blob: Arc::new(blob),
-        }
+    /// Construct a `KzgVerifiedBlob` that is assumed to be valid.
+    ///
+    /// This should ONLY be used for testing.
+    #[cfg(test)]
+    pub fn __assumed_valid(blob: Arc<BlobSidecar<T>>) -> Self {
+        Self { blob }
     }
 }
 
@@ -599,7 +601,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
         .as_ref()
         .ok_or(GossipBlobError::KzgNotInitialized)?;
     let kzg_verified_blob =
-        verify_kzg_for_blob(blob_sidecar, kzg).map_err(GossipBlobError::KzgError)?;
+        KzgVerifiedBlob::new(blob_sidecar, kzg).map_err(GossipBlobError::KzgError)?;
 
     Ok(GossipVerifiedBlob {
         block_root,

beacon_node/beacon_chain/src/data_availability_checker.rs

@@ -1,4 +1,4 @@
-use crate::blob_verification::{verify_kzg_for_blob, verify_kzg_for_blob_list, GossipVerifiedBlob};
+use crate::blob_verification::{verify_kzg_for_blob_list, GossipVerifiedBlob, KzgVerifiedBlob};
 use crate::block_verification_types::{
     AvailabilityPendingExecutedBlock, AvailableExecutedBlock, RpcBlock,
 };
@@ -199,10 +199,9 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
     ) -> Result<Availability<T::EthSpec>, AvailabilityCheckError> {
         let mut verified_blobs = vec![];
         if let Some(kzg) = self.kzg.as_ref() {
-            for blob in blobs.iter().flatten() {
-                verified_blobs.push(
-                    verify_kzg_for_blob(blob.clone(), kzg).map_err(AvailabilityCheckError::Kzg)?,
-                );
+            for blob in Vec::from(blobs).into_iter().flatten() {
+                verified_blobs
+                    .push(KzgVerifiedBlob::new(blob, kzg).map_err(AvailabilityCheckError::Kzg)?);
             }
         } else {
             return Err(AvailabilityCheckError::KzgNotInitialized);

beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs

@@ -278,8 +278,8 @@ pub mod tests {
 
     type Setup<E> = (
         SignedBeaconBlock<E>,
-        FixedVector<Option<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
-        FixedVector<Option<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
+        FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>,
+        FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>,
     );
 
     pub fn pre_setup() -> Setup<E> {
@@ -290,20 +290,20 @@ pub mod tests {
 
         for blob in blobs_vec {
             if let Some(b) = blobs.get_mut(blob.index as usize) {
-                *b = Some(blob);
+                *b = Some(Arc::new(blob));
             }
         }
 
         let mut invalid_blobs: FixedVector<
-            Option<BlobSidecar<E>>,
+            Option<Arc<BlobSidecar<E>>>,
             <E as EthSpec>::MaxBlobsPerBlock,
         > = FixedVector::default();
         for (index, blob) in blobs.iter().enumerate() {
-            let mut invalid_blob_opt = blob.clone();
-            if let Some(invalid_blob) = invalid_blob_opt.as_mut() {
-                invalid_blob.kzg_commitment = KzgCommitment::random_for_test(&mut rng);
+            if let Some(invalid_blob) = blob {
+                let mut blob_copy = invalid_blob.as_ref().clone();
+                blob_copy.kzg_commitment = KzgCommitment::random_for_test(&mut rng);
+                *invalid_blobs.get_mut(index).unwrap() = Some(Arc::new(blob_copy));
             }
-            *invalid_blobs.get_mut(index).unwrap() = invalid_blob_opt;
         }
 
         (block, blobs, invalid_blobs)
@@ -317,8 +317,8 @@ pub mod tests {
 
     pub fn setup_processing_components(
         block: SignedBeaconBlock<E>,
-        valid_blobs: FixedVector<Option<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
-        invalid_blobs: FixedVector<Option<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
+        valid_blobs: FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>,
+        invalid_blobs: FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>,
     ) -> ProcessingViewSetup<E> {
         let commitments = block
             .message()
@@ -349,16 +349,16 @@ pub mod tests {
 
     pub fn setup_pending_components(
         block: SignedBeaconBlock<E>,
-        valid_blobs: FixedVector<Option<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
-        invalid_blobs: FixedVector<Option<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
+        valid_blobs: FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>,
+        invalid_blobs: FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>,
     ) -> PendingComponentsSetup<E> {
         let blobs = FixedVector::from(
             valid_blobs
                 .iter()
                 .map(|blob_opt| {
                     blob_opt
                         .as_ref()
-                        .map(|blob| KzgVerifiedBlob::new(blob.clone()))
+                        .map(|blob| KzgVerifiedBlob::__assumed_valid(blob.clone()))
                 })
                 .collect::<Vec<_>>(),
         );
@@ -368,7 +368,7 @@ pub mod tests {
                 .map(|blob_opt| {
                     blob_opt
                         .as_ref()
-                        .map(|blob| KzgVerifiedBlob::new(blob.clone()))
+                        .map(|blob| KzgVerifiedBlob::__assumed_valid(blob.clone()))
                 })
                 .collect::<Vec<_>>(),
         );
@@ -402,21 +402,12 @@ pub mod tests {
 
     pub fn setup_child_components(
         block: SignedBeaconBlock<E>,
-        valid_blobs: FixedVector<Option<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
-        invalid_blobs: FixedVector<Option<BlobSidecar<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
+        valid_blobs: FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>,
+        invalid_blobs: FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>,
     ) -> ChildComponentsSetup<E> {
-        let blobs = FixedVector::from(
-            valid_blobs
-                .into_iter()
-                .map(|blob_opt| blob_opt.clone().map(Arc::new))
-                .collect::<Vec<_>>(),
-        );
-        let invalid_blobs = FixedVector::from(
-            invalid_blobs
-                .into_iter()
-                .map(|blob_opt| blob_opt.clone().map(Arc::new))
-                .collect::<Vec<_>>(),
-        );
+        let blobs = FixedVector::from(valid_blobs.into_iter().cloned().collect::<Vec<_>>());
+        let invalid_blobs =
+            FixedVector::from(invalid_blobs.into_iter().cloned().collect::<Vec<_>>());
         (Arc::new(block), blobs, invalid_blobs)
     }
 

beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs

@@ -738,7 +738,7 @@ impl ssz::Decode for OverflowKey {
 mod test {
     use super::*;
     use crate::{
-        blob_verification::{validate_blob_sidecar_for_gossip, GossipVerifiedBlob},
+        blob_verification::GossipVerifiedBlob,
         block_verification::PayloadVerificationOutcome,
         block_verification_types::{AsBlock, BlockImportData},
         data_availability_checker::STATE_LRU_CAPACITY,
@@ -925,7 +925,7 @@ mod test {
                 .into_iter()
                 .map(|sidecar| {
                     let subnet = sidecar.index;
-                    validate_blob_sidecar_for_gossip(sidecar, subnet, &harness.chain)
+                    GossipVerifiedBlob::new(sidecar, subnet, &harness.chain)
                         .expect("should validate blob")
                 })
                 .collect()