Fix column sidecars incorrectly produced when there are no blobs.

Author: jimmygchen

Cargo.lock

@@ -119,6 +119,8 @@ libsecp256k1 = "0.7"
 log = "0.4"
 lru = "0.12"
 maplit = "1"
+mockall = "0.12"
+mockall_double = "0.3"
 num_cpus = "1"
 parking_lot = "0.12"
 paste = "1"

Cargo.toml

@@ -773,6 +773,8 @@ fn build_gossip_verified_data_columns<T: BeaconChainTypes>(
     gossip_verified_blobs: Option<&GossipVerifiedBlobList<T>>,
 ) -> Result<Option<GossipVerifiedDataColumnList<T>>, BlockContentsError<T::EthSpec>> {
     gossip_verified_blobs
+        // Only attempt to build data columns if blobs is non empty to avoid skewing the metrics.
+        .filter(|b| !b.is_empty())
         .map(|blobs| {
             // NOTE: we expect KZG to be initialized if the blobs are present
             let kzg = chain
@@ -786,7 +788,7 @@ fn build_gossip_verified_data_columns<T: BeaconChainTypes>(
             let blob_sidecar_list = BlobSidecarList::new(blob_sidecar_list)
                 .map_err(DataColumnSidecarError::SszError)?;
             let timer = metrics::start_timer(&metrics::DATA_COLUMN_SIDECAR_COMPUTATION);
-            let sidecars = DataColumnSidecar::build_sidecars(&blob_sidecar_list, &block, kzg)?;
+            let sidecars = DataColumnSidecar::build_sidecars(&blob_sidecar_list, block, kzg)?;
             drop(timer);
             let mut gossip_verified_data_columns = vec![];
             for sidecar in sidecars {

beacon_node/beacon_chain/src/block_verification.rs

@@ -60,6 +60,7 @@ eth2_network_config = { workspace = true }
 state_processing = { workspace = true }
 tokio = { workspace = true }
 paste = { workspace = true }
+mockall_double = { workspace = true }
 
 [features]
 default = ["sqlite", "legacy-arith"]

consensus/types/Cargo.toml

@@ -7,8 +7,12 @@ use crate::{
 };
 use bls::Signature;
 use derivative::Derivative;
-use kzg::{Blob as KzgBlob, Error as KzgError, Kzg};
+#[cfg_attr(test, double)]
+use kzg::Kzg;
+use kzg::{Blob as KzgBlob, Error as KzgError};
 use kzg::{KzgCommitment, KzgProof};
+#[cfg(test)]
+use mockall_double::double;
 use safe_arith::ArithError;
 use serde::{Deserialize, Serialize};
 use ssz::Encode;
@@ -76,6 +80,9 @@ impl<E: EthSpec> DataColumnSidecar<E> {
         block: &SignedBeaconBlock<E>,
         kzg: &Kzg,
     ) -> Result<DataColumnSidecarList<E>, DataColumnSidecarError> {
+        if blobs.is_empty() {
+            return Ok(DataColumnSidecarList::empty());
+        }
         let kzg_commitments = block
             .message()
             .body()
@@ -239,6 +246,7 @@ pub type FixedDataColumnSidecarList<E> =
 
 #[cfg(test)]
 mod test {
+    use super::*;
     use crate::beacon_block::EmptyBlock;
     use crate::beacon_block_body::KzgCommitments;
     use crate::eth_spec::EthSpec;
@@ -247,10 +255,24 @@ mod test {
         DataColumnSidecar, MainnetEthSpec, SignedBeaconBlock,
     };
     use bls::Signature;
-    use eth2_network_config::TRUSTED_SETUP_BYTES;
-    use kzg::{Kzg, KzgCommitment, KzgProof, TrustedSetup};
+    use kzg::{KzgCommitment, KzgProof};
     use std::sync::Arc;
 
+    #[test]
+    fn test_build_sidecars_empty() {
+        type E = MainnetEthSpec;
+        let num_of_blobs = 0;
+        let spec = E::default_spec();
+        let (signed_block, blob_sidecars) =
+            create_test_block_and_blob_sidecars::<E>(num_of_blobs, &spec);
+
+        let mock_kzg = Arc::new(Kzg::default());
+        let column_sidecars =
+            DataColumnSidecar::build_sidecars(&blob_sidecars, &signed_block, &mock_kzg).unwrap();
+
+        assert!(column_sidecars.is_empty());
+    }
+
     #[test]
     fn test_build_sidecars() {
         type E = MainnetEthSpec;
@@ -259,11 +281,13 @@ mod test {
         let (signed_block, blob_sidecars) =
             create_test_block_and_blob_sidecars::<E>(num_of_blobs, &spec);
 
-        let trusted_setup: TrustedSetup = serde_json::from_reader(TRUSTED_SETUP_BYTES).unwrap();
-        let kzg = Arc::new(Kzg::new_from_trusted_setup(trusted_setup).unwrap());
+        let mut mock_kzg = Kzg::default();
+        mock_kzg
+            .expect_compute_cells_and_proofs()
+            .returning(kzg::mock::compute_cells_and_proofs);
 
         let column_sidecars =
-            DataColumnSidecar::build_sidecars(&blob_sidecars, &signed_block, &kzg).unwrap();
+            DataColumnSidecar::build_sidecars(&blob_sidecars, &signed_block, &mock_kzg).unwrap();
 
         let block_kzg_commitments = signed_block
             .message()

consensus/types/src/data_column_sidecar.rs

@@ -17,3 +17,4 @@ ethereum_serde_utils = { workspace = true }
 hex = { workspace = true }
 ethereum_hashing = { workspace = true }
 c-kzg = { workspace = true }
+mockall = { workspace = true }

crypto/kzg/Cargo.toml

@@ -14,6 +14,8 @@ pub use c_kzg::{
     BYTES_PER_FIELD_ELEMENT, BYTES_PER_PROOF, FIELD_ELEMENTS_PER_BLOB,
 };
 use c_kzg::{Cell, CELLS_PER_BLOB};
+use mockall::automock;
+
 #[derive(Debug)]
 pub enum Error {
     /// An error from the underlying kzg library.
@@ -34,6 +36,7 @@ pub struct Kzg {
     trusted_setup: KzgSettings,
 }
 
+#[automock]
 impl Kzg {
     /// Load the kzg trusted setup parameters from a vec of G1 and G2 points.
     pub fn new_from_trusted_setup(trusted_setup: TrustedSetup) -> Result<Self, Error> {
@@ -191,6 +194,24 @@ impl Kzg {
     }
 }
 
+pub mod mock {
+    use crate::{Error, KzgProof};
+    use c_kzg::{Blob, Cell, CELLS_PER_BLOB};
+
+    pub const MOCK_KZG_BYTES_PER_CELL: usize = 2048;
+
+    #[allow(clippy::type_complexity)]
+    pub fn compute_cells_and_proofs(
+        _blob: &Blob,
+    ) -> Result<(Box<[Cell; CELLS_PER_BLOB]>, Box<[KzgProof; CELLS_PER_BLOB]>), Error> {
+        let empty_cell = Cell::new([0; MOCK_KZG_BYTES_PER_CELL]);
+        Ok((
+            Box::new([empty_cell; CELLS_PER_BLOB]),
+            Box::new([KzgProof::empty(); CELLS_PER_BLOB]),
+        ))
+    }
+}
+
 impl TryFrom<TrustedSetup> for Kzg {
     type Error = Error;