From 1b4545cd9d05a001a8295c526a1e83d1e4f83fd3 Mon Sep 17 00:00:00 2001
From: Jimmy Chen <jchen.tc@gmail.com>
Date: Wed, 18 Oct 2023 06:52:54 +0000
Subject: [PATCH] Remove blob clones in KZG verification (#4852)

## Issue Addressed

This PR removes two instances of blob clones during blob verification that may not be necessary.
---
 beacon_node/beacon_chain/src/beacon_chain.rs       |  2 +-
 beacon_node/beacon_chain/src/blob_verification.rs  | 14 ++++----------
 beacon_node/beacon_chain/src/kzg_utils.rs          | 10 +++++-----
 .../src/cases/kzg_verify_blob_kzg_proof.rs         |  2 +-
 .../src/cases/kzg_verify_blob_kzg_proof_batch.rs   |  2 +-
 5 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
index a45caba79..78e1cc46f 100644
--- a/beacon_node/beacon_chain/src/beacon_chain.rs
+++ b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -5123,7 +5123,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     kzg_utils::validate_blobs::<T::EthSpec>(
                         kzg,
                         expected_kzg_commitments,
-                        blobs,
+                        blobs.iter().collect(),
                         &kzg_proofs,
                     )
                     .map_err(BlockProductionError::KzgError)?;
diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs
index 1c8bc9be8..b535f3104 100644
--- a/beacon_node/beacon_chain/src/blob_verification.rs
+++ b/beacon_node/beacon_chain/src/blob_verification.rs
@@ -502,8 +502,7 @@ pub fn verify_kzg_for_blob<T: EthSpec>(
     kzg: &Kzg<T::Kzg>,
 ) -> Result<KzgVerifiedBlob<T>, AvailabilityCheckError> {
     let _timer = crate::metrics::start_timer(&crate::metrics::KZG_VERIFICATION_SINGLE_TIMES);
-    //TODO(sean) remove clone
-    if validate_blob::<T>(kzg, blob.blob.clone(), blob.kzg_commitment, blob.kzg_proof)
+    if validate_blob::<T>(kzg, &blob.blob, blob.kzg_commitment, blob.kzg_proof)
         .map_err(AvailabilityCheckError::Kzg)?
     {
         Ok(KzgVerifiedBlob { blob })
@@ -524,15 +523,10 @@ pub fn verify_kzg_for_blob_list<T: EthSpec>(
     let _timer = crate::metrics::start_timer(&crate::metrics::KZG_VERIFICATION_BATCH_TIMES);
     let (blobs, (commitments, proofs)): (Vec<_>, (Vec<_>, Vec<_>)) = blob_list
         .iter()
-        .map(|blob| (blob.blob.clone(), (blob.kzg_commitment, blob.kzg_proof)))
+        .map(|blob| (&blob.blob, (blob.kzg_commitment, blob.kzg_proof)))
         .unzip();
-    if validate_blobs::<T>(
-        kzg,
-        commitments.as_slice(),
-        blobs.as_slice(),
-        proofs.as_slice(),
-    )
-    .map_err(AvailabilityCheckError::Kzg)?
+    if validate_blobs::<T>(kzg, commitments.as_slice(), blobs, proofs.as_slice())
+        .map_err(AvailabilityCheckError::Kzg)?
     {
         Ok(())
     } else {
diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs
index 144e21367..b4c70befa 100644
--- a/beacon_node/beacon_chain/src/kzg_utils.rs
+++ b/beacon_node/beacon_chain/src/kzg_utils.rs
@@ -12,12 +12,12 @@ fn ssz_blob_to_crypto_blob<T: EthSpec>(
 /// Validate a single blob-commitment-proof triplet from a `BlobSidecar`.
 pub fn validate_blob<T: EthSpec>(
     kzg: &Kzg<T::Kzg>,
-    blob: Blob<T>,
+    blob: &Blob<T>,
     kzg_commitment: KzgCommitment,
     kzg_proof: KzgProof,
 ) -> Result<bool, KzgError> {
     kzg.verify_blob_kzg_proof(
-        &ssz_blob_to_crypto_blob::<T>(&blob)?,
+        &ssz_blob_to_crypto_blob::<T>(blob)?,
         kzg_commitment,
         kzg_proof,
     )
@@ -27,12 +27,12 @@ pub fn validate_blob<T: EthSpec>(
 pub fn validate_blobs<T: EthSpec>(
     kzg: &Kzg<T::Kzg>,
     expected_kzg_commitments: &[KzgCommitment],
-    blobs: &[Blob<T>],
+    blobs: Vec<&Blob<T>>,
     kzg_proofs: &[KzgProof],
 ) -> Result<bool, KzgError> {
     let blobs = blobs
-        .iter()
-        .map(|blob| ssz_blob_to_crypto_blob::<T>(blob)) // Avoid this clone
+        .into_iter()
+        .map(|blob| ssz_blob_to_crypto_blob::<T>(blob))
         .collect::<Result<Vec<_>, KzgError>>()?;
 
     kzg.verify_blob_kzg_proof_batch(&blobs, expected_kzg_commitments, kzg_proofs)
diff --git a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs
index 2d18c9bdc..ff918cac1 100644
--- a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs
+++ b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs
@@ -91,7 +91,7 @@ impl<E: EthSpec> Case for KZGVerifyBlobKZGProof<E> {
 
         let kzg = get_kzg::<E::Kzg>()?;
         let result = parse_input(&self.input).and_then(|(blob, commitment, proof)| {
-            validate_blob::<E>(&kzg, blob, commitment, proof)
+            validate_blob::<E>(&kzg, &blob, commitment, proof)
                 .map_err(|e| Error::InternalError(format!("Failed to validate blob: {:?}", e)))
         });
 
diff --git a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs
index cc941618a..43c2c8cf6 100644
--- a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs
+++ b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs
@@ -54,7 +54,7 @@ impl<E: EthSpec> Case for KZGVerifyBlobKZGProofBatch<E> {
 
         let kzg = get_kzg::<E::Kzg>()?;
         let result = parse_input(&self.input).and_then(|(commitments, blobs, proofs)| {
-            validate_blobs::<E>(&kzg, &commitments, &blobs, &proofs)
+            validate_blobs::<E>(&kzg, &commitments, blobs.iter().collect(), &proofs)
                 .map_err(|e| Error::InternalError(format!("Failed to validate blobs: {:?}", e)))
         });