Skip to content

Commit

Permalink
Fix column sidecars incorrectly produced when there are no blobs.
Browse files Browse the repository at this point in the history
  • Loading branch information
jimmygchen committed Apr 19, 2024
1 parent 5b4f9de commit 99a41e8
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 8 deletions.
93 changes: 92 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 3 additions & 1 deletion beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions consensus/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
36 changes: 30 additions & 6 deletions consensus/types/src/data_column_sidecar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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()
Expand Down
1 change: 1 addition & 0 deletions crypto/kzg/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ ethereum_serde_utils = { workspace = true }
hex = { workspace = true }
ethereum_hashing = { workspace = true }
c-kzg = { workspace = true }
mockall = { workspace = true }
19 changes: 19 additions & 0 deletions crypto/kzg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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> {
Expand Down Expand Up @@ -191,6 +194,22 @@ 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;
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;

Expand Down

0 comments on commit 99a41e8

Please sign in to comment.