Check that KzgCommitments are consistent across all sidecars in batch…

… during verification (#6029) Squashed commit of the following: commit af1adf7 Merge: 0472f04 bf300b3 Author: Jimmy Chen <[email protected]> Date: Fri Jul 12 14:33:35 2024 +1000 Merge branch 'das' into fix-kzg-batch-verification # Conflicts: # beacon_node/beacon_chain/src/kzg_utils.rs commit 0472f04 Author: Jimmy Chen <[email protected]> Date: Mon Jul 1 23:32:24 2024 +1000 Fix incorrect `kzg_commitments` passed to CKZG and add test.
sigp · Jul 12, 2024 · fd38584 · fd38584
1 parent ff08e96
commit fd38584
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 27 deletions.
diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs
@@ -1,3 +1,4 @@
+use itertools::Itertools;
 use kzg::{Blob as KzgBlob, Bytes48, CellRef as KzgCellRef, Error as KzgError, Kzg};
 use std::sync::Arc;
 use types::data_column_sidecar::Cell;
@@ -38,6 +39,8 @@ pub fn validate_data_column<'a, E: EthSpec, I>(
 where
     I: Iterator<Item = &'a Arc<DataColumnSidecar<E>>> + Clone,
 {
+    let data_column_iter = data_column_iter.sorted_by_key(|d| d.index);
+
     let cells = data_column_iter
         .clone()
         .flat_map(|data_column| data_column.column.iter().map(ssz_cell_to_crypto_cell::<E>))
@@ -63,13 +66,16 @@ where
 
     let commitments = data_column_iter
         .clone()
-        .flat_map(|data_column| {
-            data_column
-                .kzg_commitments
-                .iter()
-                .map(|&commitment| Bytes48::from(commitment))
+        .map(|d| d.kzg_commitments.clone())
+        .unique()
+        .exactly_one()
+        .map(|kzg_commitments| {
+            kzg_commitments
+                .into_iter()
+                .map(Bytes48::from)
+                .collect::<Vec<_>>()
         })
-        .collect::<Vec<_>>();
+        .map_err(|_| KzgError::InconsistentKzgCommitments)?;
 
     kzg.verify_cell_proof_batch(&cells, &proofs, &coordinates, &commitments)
 }
@@ -134,32 +140,30 @@ pub fn verify_kzg_proof<E: EthSpec>(
 
 #[cfg(test)]
 mod test {
+    use super::*;
+    use crate::test_utils::KZG;
     use bls::Signature;
-    use eth2_network_config::TRUSTED_SETUP_BYTES;
-    use kzg::{Kzg, KzgCommitment, TrustedSetup};
+    use eth2::types::BlobsBundle;
+    use execution_layer::test_utils::generate_blobs;
     use types::{
-        beacon_block_body::KzgCommitments, BeaconBlock, BeaconBlockDeneb, Blob, BlobsList,
-        ChainSpec, DataColumnSidecar, EmptyBlock, EthSpec, MainnetEthSpec, SignedBeaconBlock,
+        BeaconBlock, BeaconBlockDeneb, BlobsList, ChainSpec, DataColumnSidecar, EmptyBlock,
+        EthSpec, MainnetEthSpec, SignedBeaconBlock,
     };
 
+    type E = MainnetEthSpec;
+
     #[test]
     fn build_and_reconstruct() {
-        type E = MainnetEthSpec;
         let num_of_blobs = 6;
         let spec = E::default_spec();
         let (signed_block, blob_sidecars) = create_test_block_and_blobs::<E>(num_of_blobs, &spec);
 
-        let trusted_setup: TrustedSetup = serde_json::from_reader(TRUSTED_SETUP_BYTES)
-            .map_err(|e| format!("Unable to read trusted setup file: {}", e))
-            .expect("should have trusted setup");
-        let kzg = Kzg::new_from_trusted_setup(trusted_setup).expect("should create kzg");
-
         let column_sidecars =
-            DataColumnSidecar::build_sidecars(&blob_sidecars, &signed_block, &kzg, &spec).unwrap();
+            DataColumnSidecar::build_sidecars(&blob_sidecars, &signed_block, &KZG, &spec).unwrap();
 
         // Now reconstruct
         let reconstructed_columns = DataColumnSidecar::reconstruct(
-            &kzg,
+            &KZG,
             &column_sidecars.iter().as_slice()[0..column_sidecars.len() / 2],
             &spec,
         )
@@ -170,23 +174,36 @@ mod test {
         }
     }
 
+    #[test]
+    fn validate_data_columns() {
+        let num_of_blobs = 6;
+        let spec = E::default_spec();
+        let (signed_block, blob_sidecars) = create_test_block_and_blobs::<E>(num_of_blobs, &spec);
+
+        let column_sidecars =
+            DataColumnSidecar::build_sidecars(&blob_sidecars, &signed_block, &KZG, &spec).unwrap();
+
+        validate_data_column::<E, _>(&KZG, column_sidecars.iter()).unwrap()
+    }
+
     fn create_test_block_and_blobs<E: EthSpec>(
         num_of_blobs: usize,
         spec: &ChainSpec,
     ) -> (SignedBeaconBlock<E>, BlobsList<E>) {
         let mut block = BeaconBlock::Deneb(BeaconBlockDeneb::empty(spec));
         let mut body = block.body_mut();
-        let blob_kzg_commitments = body.blob_kzg_commitments_mut().unwrap();
-        *blob_kzg_commitments =
-            KzgCommitments::<E>::new(vec![KzgCommitment::empty_for_testing(); num_of_blobs])
-                .unwrap();
 
-        let signed_block = SignedBeaconBlock::from_block(block, Signature::empty());
+        let (bundle, _transactions) = generate_blobs(num_of_blobs).unwrap();
+
+        let BlobsBundle::<E> {
+            commitments,
+            proofs: _,
+            blobs,
+        } = bundle;
 
-        let blobs = (0..num_of_blobs)
-            .map(|_| Blob::<E>::default())
-            .collect::<Vec<_>>()
-            .into();
+        *body.blob_kzg_commitments_mut().unwrap() = commitments;
+
+        let signed_block = SignedBeaconBlock::from_block(block, Signature::empty());
 
         (signed_block, blobs)
     }

diff --git a/crypto/kzg/src/lib.rs b/crypto/kzg/src/lib.rs
@@ -34,6 +34,8 @@ pub enum Error {
     KzgVerificationFailed,
     /// Misc indexing error
     InconsistentArrayLength(String),
+    /// KZG commitments do not match across data columns
+    InconsistentKzgCommitments,
 }
 
 impl From<c_kzg::Error> for Error {