Skip to content

Commit

Permalink
Make BeaconChain::kzg field mandatory (#6267)
Browse files Browse the repository at this point in the history
* make kzg field required

* update todo

* always load trusted setup WIP

* fmt

* use new rust_eth_kzg version

* merge conlficts

* add kzg fn with trusted setup disabled

* as_slice

* add kzg with no precomp

* ignore udep for kzg

* refactor kzg init

* fix peerdas kzg schedule

* fix

* udeps

* uuuudeps

* merge conflict resolved

* merge conflict

* merge conflicts

* resolve TODO

* update

* move kzg to a test util fn

* remove trusted setup default impl

* lint fmt

* fix failing test

* lint

* fix test

* Merge branch 'unstable' into beacon-chain-kzg-field-required
  • Loading branch information
eserilev authored Sep 23, 2024
1 parent 18c97a7 commit b619f1a
Show file tree
Hide file tree
Showing 33 changed files with 188 additions and 210 deletions.
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)?;
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

0 comments on commit b619f1a

Please sign in to comment.