Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make BeaconChain::kzg field mandatory #6267

Merged
merged 27 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion Cargo.lock

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

10 changes: 3 additions & 7 deletions beacon_node/beacon_chain/benches/benches.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::sync::Arc;

use beacon_chain::kzg_utils::{blobs_to_data_column_sidecars, reconstruct_data_columns};
use beacon_chain::test_utils::get_kzg;
use criterion::{black_box, criterion_group, criterion_main, Criterion};

use bls::Signature;
use eth2_network_config::TRUSTED_SETUP_BYTES;
use kzg::{Kzg, KzgCommitment, TrustedSetup};
use kzg::KzgCommitment;
use types::{
beacon_block_body::KzgCommitments, BeaconBlock, BeaconBlockDeneb, Blob, BlobsList, ChainSpec,
EmptyBlock, EthSpec, MainnetEthSpec, SignedBeaconBlock,
Expand Down Expand Up @@ -35,11 +35,7 @@ fn all_benches(c: &mut Criterion) {
type E = MainnetEthSpec;
let spec = Arc::new(E::default_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 = Arc::new(Kzg::new_from_trusted_setup(trusted_setup).expect("should create kzg"));

let kzg = get_kzg(&spec);
for blob_count in [1, 2, 3, 6] {
let kzg = kzg.clone();
let (signed_block, blob_sidecars) = create_test_block_and_blobs::<E>(blob_count, &spec);
Expand Down
8 changes: 3 additions & 5 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ pub struct BeaconChain<T: BeaconChainTypes> {
/// they are collected and combined.
pub data_availability_checker: Arc<DataAvailabilityChecker<T>>,
/// The KZG trusted setup used by this chain.
pub kzg: Option<Arc<Kzg>>,
pub kzg: Arc<Kzg>,
}

pub enum BeaconBlockResponseWrapper<E: EthSpec> {
Expand Down Expand Up @@ -5682,10 +5682,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {

let kzg_proofs = Vec::from(proofs);

let kzg = self
.kzg
.as_ref()
.ok_or(BlockProductionError::TrustedSetupNotInitialized)?;
michaelsproul marked this conversation as resolved.
Show resolved Hide resolved
let kzg = self.kzg.as_ref();

kzg_utils::validate_blobs::<T::EthSpec>(
kzg,
expected_kzg_commitments,
Expand Down
15 changes: 3 additions & 12 deletions beacon_node/beacon_chain/src/blob_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,6 @@ pub enum GossipBlobError {
index: u64,
},

/// `Kzg` struct hasn't been initialized. This is an internal error.
///
/// ## Peer scoring
///
/// The peer isn't faulty, This is an internal error.
KzgNotInitialized,

/// The kzg verification failed.
///
/// ## Peer scoring
Expand Down Expand Up @@ -559,11 +552,9 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
}

// Kzg verification for gossip blob sidecar
let kzg = chain
.kzg
.as_ref()
.ok_or(GossipBlobError::KzgNotInitialized)?;
let kzg_verified_blob = KzgVerifiedBlob::new(blob_sidecar, kzg, seen_timestamp)
let kzg = chain.kzg.as_ref();

let kzg_verified_blob = KzgVerifiedBlob::new(blob_sidecar.clone(), kzg, seen_timestamp)
.map_err(GossipBlobError::KzgError)?;
let blob_sidecar = &kzg_verified_blob.blob;

Expand Down
10 changes: 1 addition & 9 deletions beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,19 +789,11 @@ fn build_gossip_verified_data_columns<T: BeaconChainTypes>(
// 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
.kzg
.as_ref()
.ok_or(BlockContentsError::DataColumnError(
GossipDataColumnError::KzgNotInitialized,
))?;

let mut timer = metrics::start_timer_vec(
&metrics::DATA_COLUMN_SIDECAR_COMPUTATION,
&[&blobs.len().to_string()],
);
let sidecars = blobs_to_data_column_sidecars(&blobs, block, kzg, &chain.spec)
let sidecars = blobs_to_data_column_sidecars(&blobs, block, &chain.kzg, &chain.spec)
.discard_timer_on_break(&mut timer)?;
drop(timer);
let mut gossip_verified_data_columns = vec![];
Expand Down
17 changes: 7 additions & 10 deletions beacon_node/beacon_chain/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ pub struct BeaconChainBuilder<T: BeaconChainTypes> {
// Pending I/O batch that is constructed during building and should be executed atomically
// alongside `PersistedBeaconChain` storage when `BeaconChainBuilder::build` is called.
pending_io_batch: Vec<KeyValueStoreOp>,
kzg: Option<Arc<Kzg>>,
kzg: Arc<Kzg>,
task_executor: Option<TaskExecutor>,
validator_monitor_config: Option<ValidatorMonitorConfig>,
import_all_data_columns: bool,
Expand All @@ -120,7 +120,7 @@ where
///
/// The `_eth_spec_instance` parameter is only supplied to make concrete the `E` trait.
/// This should generally be either the `MinimalEthSpec` or `MainnetEthSpec` types.
pub fn new(_eth_spec_instance: E) -> Self {
pub fn new(_eth_spec_instance: E, kzg: Arc<Kzg>) -> Self {
Self {
store: None,
store_migrator_config: None,
Expand All @@ -143,7 +143,7 @@ where
beacon_graffiti: GraffitiOrigin::default(),
slasher: None,
pending_io_batch: vec![],
kzg: None,
kzg,
task_executor: None,
validator_monitor_config: None,
import_all_data_columns: false,
Expand Down Expand Up @@ -694,11 +694,6 @@ where
self
}

pub fn kzg(mut self, kzg: Option<Arc<Kzg>>) -> Self {
self.kzg = kzg;
self
}

/// Consumes `self`, returning a `BeaconChain` if all required parameters have been supplied.
///
/// An error will be returned at runtime if all required parameters have not been configured.
Expand Down Expand Up @@ -1157,7 +1152,7 @@ fn descriptive_db_error(item: &str, error: &StoreError) -> String {
#[cfg(test)]
mod test {
use super::*;
use crate::test_utils::EphemeralHarnessType;
use crate::test_utils::{get_kzg, EphemeralHarnessType};
use ethereum_hashing::hash;
use genesis::{
generate_deterministic_keypairs, interop_genesis_state, DEFAULT_ETH1_BLOCK_HASH,
Expand Down Expand Up @@ -1204,7 +1199,9 @@ mod test {
let (shutdown_tx, _) = futures::channel::mpsc::channel(1);
let runtime = TestRuntime::default();

let chain = Builder::new(MinimalEthSpec)
let kzg = get_kzg(&spec);

let chain = Builder::new(MinimalEthSpec, kzg)
.logger(log.clone())
.store(Arc::new(store))
.task_executor(runtime.task_executor.clone())
Expand Down
55 changes: 16 additions & 39 deletions beacon_node/beacon_chain/src/data_availability_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub const STATE_LRU_CAPACITY: usize = STATE_LRU_CAPACITY_NON_ZERO.get();
pub struct DataAvailabilityChecker<T: BeaconChainTypes> {
availability_cache: Arc<DataAvailabilityCheckerInner<T>>,
slot_clock: T::SlotClock,
kzg: Option<Arc<Kzg>>,
kzg: Arc<Kzg>,
spec: Arc<ChainSpec>,
}

Expand Down Expand Up @@ -97,7 +97,7 @@ impl<E: EthSpec> Debug for Availability<E> {
impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
pub fn new(
slot_clock: T::SlotClock,
kzg: Option<Arc<Kzg>>,
kzg: Arc<Kzg>,
store: BeaconStore<T>,
import_all_data_columns: bool,
spec: ChainSpec,
Expand Down Expand Up @@ -190,18 +190,17 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
epoch: Epoch,
blobs: FixedBlobSidecarList<T::EthSpec>,
) -> Result<Availability<T::EthSpec>, AvailabilityCheckError> {
let Some(kzg) = self.kzg.as_ref() else {
return Err(AvailabilityCheckError::KzgNotInitialized);
};

let seen_timestamp = self
.slot_clock
.now_duration()
.ok_or(AvailabilityCheckError::SlotClockError)?;

let verified_blobs =
KzgVerifiedBlobList::new(Vec::from(blobs).into_iter().flatten(), kzg, seen_timestamp)
.map_err(AvailabilityCheckError::Kzg)?;
let verified_blobs = KzgVerifiedBlobList::new(
Vec::from(blobs).into_iter().flatten(),
&self.kzg,
seen_timestamp,
)
.map_err(AvailabilityCheckError::Kzg)?;

self.availability_cache
.put_kzg_verified_blobs(block_root, epoch, verified_blobs)
Expand All @@ -217,23 +216,20 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
custody_columns: DataColumnSidecarList<T::EthSpec>,
) -> Result<(Availability<T::EthSpec>, DataColumnsToPublish<T::EthSpec>), AvailabilityCheckError>
{
let Some(kzg) = self.kzg.as_ref() else {
return Err(AvailabilityCheckError::KzgNotInitialized);
};

// TODO(das): report which column is invalid for proper peer scoring
// TODO(das): batch KZG verification here
let verified_custody_columns = custody_columns
.into_iter()
.map(|column| {
Ok(KzgVerifiedCustodyDataColumn::from_asserted_custody(
KzgVerifiedDataColumn::new(column, kzg).map_err(AvailabilityCheckError::Kzg)?,
KzgVerifiedDataColumn::new(column, &self.kzg)
.map_err(AvailabilityCheckError::Kzg)?,
))
})
.collect::<Result<Vec<_>, AvailabilityCheckError>>()?;

self.availability_cache.put_kzg_verified_data_columns(
kzg,
&self.kzg,
block_root,
epoch,
verified_custody_columns,
Expand Down Expand Up @@ -269,9 +265,6 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
gossip_data_columns: Vec<GossipVerifiedDataColumn<T>>,
) -> Result<(Availability<T::EthSpec>, DataColumnsToPublish<T::EthSpec>), AvailabilityCheckError>
{
let Some(kzg) = self.kzg.as_ref() else {
return Err(AvailabilityCheckError::KzgNotInitialized);
};
let epoch = slot.epoch(T::EthSpec::slots_per_epoch());

let custody_columns = gossip_data_columns
Expand All @@ -280,7 +273,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
.collect::<Vec<_>>();

self.availability_cache.put_kzg_verified_data_columns(
kzg,
&self.kzg,
block_root,
epoch,
custody_columns,
Expand Down Expand Up @@ -314,11 +307,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
let (block_root, block, blobs, data_columns) = block.deconstruct();
if self.blobs_required_for_block(&block) {
return if let Some(blob_list) = blobs.as_ref() {
let kzg = self
.kzg
.as_ref()
.ok_or(AvailabilityCheckError::KzgNotInitialized)?;
verify_kzg_for_blob_list(blob_list.iter(), kzg)
verify_kzg_for_blob_list(blob_list.iter(), &self.kzg)
.map_err(AvailabilityCheckError::Kzg)?;
Ok(MaybeAvailableBlock::Available(AvailableBlock {
block_root,
Expand All @@ -334,15 +323,11 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
}
if self.data_columns_required_for_block(&block) {
return if let Some(data_column_list) = data_columns.as_ref() {
let kzg = self
.kzg
.as_ref()
.ok_or(AvailabilityCheckError::KzgNotInitialized)?;
verify_kzg_for_data_column_list(
data_column_list
.iter()
.map(|custody_column| custody_column.as_data_column()),
kzg,
&self.kzg,
)
.map_err(AvailabilityCheckError::Kzg)?;
Ok(MaybeAvailableBlock::Available(AvailableBlock {
Expand Down Expand Up @@ -395,11 +380,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {

// verify kzg for all blobs at once
if !all_blobs.is_empty() {
let kzg = self
.kzg
.as_ref()
.ok_or(AvailabilityCheckError::KzgNotInitialized)?;
verify_kzg_for_blob_list(all_blobs.iter(), kzg)?;
verify_kzg_for_blob_list(all_blobs.iter(), &self.kzg)?;
}

let all_data_columns = blocks
Expand All @@ -415,11 +396,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {

// verify kzg for all data columns at once
if !all_data_columns.is_empty() {
let kzg = self
.kzg
.as_ref()
.ok_or(AvailabilityCheckError::KzgNotInitialized)?;
verify_kzg_for_data_column_list(all_data_columns.iter(), kzg)?;
verify_kzg_for_data_column_list(all_data_columns.iter(), &self.kzg)?;
}

for block in blocks {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use types::{BeaconStateError, Hash256};
#[derive(Debug)]
pub enum Error {
Kzg(KzgError),
KzgNotInitialized,
KzgVerificationFailed,
KzgCommitmentMismatch {
blob_commitment: KzgCommitment,
Expand Down Expand Up @@ -36,8 +35,7 @@ pub enum ErrorCategory {
impl Error {
pub fn category(&self) -> ErrorCategory {
match self {
Error::KzgNotInitialized
| Error::SszTypes(_)
Error::SszTypes(_)
| Error::MissingBlobs
| Error::MissingCustodyColumns
| Error::StoreError(_)
Expand Down
Loading