From 52117f43ba276df30d8e7d4eecc864f211885f81 Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Wed, 6 Dec 2023 14:51:40 -0600 Subject: [PATCH] Small Improvements (#4980) * initial changes * use arc in vector * Utilize new pattern for KzgVerifiedBlob * fmt * Update beacon_node/beacon_chain/src/blob_verification.rs Co-authored-by: realbigsean * forgot to save.. * lint * fmt.. again --------- Co-authored-by: realbigsean --- .../beacon_chain/src/blob_verification.rs | 20 ++++---- .../src/data_availability_checker.rs | 9 ++-- .../availability_view.rs | 47 ++++++++----------- .../overflow_lru_cache.rs | 4 +- 4 files changed, 36 insertions(+), 44 deletions(-) diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index e2a1f0928f0..05457adab32 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -276,6 +276,10 @@ impl Ord for KzgVerifiedBlob { } impl KzgVerifiedBlob { + pub fn new(blob: Arc>, kzg: &Kzg) -> Result { + verify_kzg_for_blob(blob, kzg) + } + pub fn to_blob(self) -> Arc> { self.blob } @@ -289,14 +293,12 @@ impl KzgVerifiedBlob { pub fn blob_index(&self) -> u64 { self.blob.index } -} - -#[cfg(test)] -impl KzgVerifiedBlob { - pub fn new(blob: BlobSidecar) -> 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>) -> Self { + Self { blob } } } @@ -599,7 +601,7 @@ pub fn validate_blob_sidecar_for_gossip( .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, diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 2fcb3b7a9e8..6b327246a2e 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/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 DataAvailabilityChecker { ) -> Result, 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); diff --git a/beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs b/beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs index f013cf649a8..776f81ee545 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs @@ -278,8 +278,8 @@ pub mod tests { type Setup = ( SignedBeaconBlock, - FixedVector>, ::MaxBlobsPerBlock>, - FixedVector>, ::MaxBlobsPerBlock>, + FixedVector>>, ::MaxBlobsPerBlock>, + FixedVector>>, ::MaxBlobsPerBlock>, ); pub fn pre_setup() -> Setup { @@ -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>, + Option>>, ::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, - valid_blobs: FixedVector>, ::MaxBlobsPerBlock>, - invalid_blobs: FixedVector>, ::MaxBlobsPerBlock>, + valid_blobs: FixedVector>>, ::MaxBlobsPerBlock>, + invalid_blobs: FixedVector>>, ::MaxBlobsPerBlock>, ) -> ProcessingViewSetup { let commitments = block .message() @@ -349,8 +349,8 @@ pub mod tests { pub fn setup_pending_components( block: SignedBeaconBlock, - valid_blobs: FixedVector>, ::MaxBlobsPerBlock>, - invalid_blobs: FixedVector>, ::MaxBlobsPerBlock>, + valid_blobs: FixedVector>>, ::MaxBlobsPerBlock>, + invalid_blobs: FixedVector>>, ::MaxBlobsPerBlock>, ) -> PendingComponentsSetup { let blobs = FixedVector::from( valid_blobs @@ -358,7 +358,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::>(), ); @@ -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::>(), ); @@ -402,21 +402,12 @@ pub mod tests { pub fn setup_child_components( block: SignedBeaconBlock, - valid_blobs: FixedVector>, ::MaxBlobsPerBlock>, - invalid_blobs: FixedVector>, ::MaxBlobsPerBlock>, + valid_blobs: FixedVector>>, ::MaxBlobsPerBlock>, + invalid_blobs: FixedVector>>, ::MaxBlobsPerBlock>, ) -> ChildComponentsSetup { - let blobs = FixedVector::from( - valid_blobs - .into_iter() - .map(|blob_opt| blob_opt.clone().map(Arc::new)) - .collect::>(), - ); - let invalid_blobs = FixedVector::from( - invalid_blobs - .into_iter() - .map(|blob_opt| blob_opt.clone().map(Arc::new)) - .collect::>(), - ); + let blobs = FixedVector::from(valid_blobs.into_iter().cloned().collect::>()); + let invalid_blobs = + FixedVector::from(invalid_blobs.into_iter().cloned().collect::>()); (Arc::new(block), blobs, invalid_blobs) } 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 36d7c2acad8..7997a2e5e36 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 @@ -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()