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 2 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
2 changes: 2 additions & 0 deletions Cargo.lock

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

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 @@ -493,7 +493,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 @@ -5617,10 +5617,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
6 changes: 2 additions & 4 deletions beacon_node/beacon_chain/src/blob_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,10 +566,8 @@ 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 = chain.kzg.as_ref();

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

Expand Down
11 changes: 3 additions & 8 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 @@ -684,11 +684,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
22 changes: 6 additions & 16 deletions beacon_node/beacon_chain/src/data_availability_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,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>,
log: Logger,
spec: ChainSpec,
}
Expand Down Expand Up @@ -95,7 +95,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,
log: &Logger,
Expand Down Expand Up @@ -189,9 +189,7 @@ 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 kzg = self.kzg.as_ref();

let seen_timestamp = self
.slot_clock
Expand All @@ -214,9 +212,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
epoch: Epoch,
custody_columns: DataColumnSidecarList<T::EthSpec>,
) -> Result<Availability<T::EthSpec>, AvailabilityCheckError> {
let Some(kzg) = self.kzg.as_ref() else {
return Err(AvailabilityCheckError::KzgNotInitialized);
};
let kzg = self.kzg.as_ref();

// TODO(das): report which column is invalid for proper peer scoring
// TODO(das): batch KZG verification here
Expand Down Expand Up @@ -310,10 +306,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
}
Some(blob_list) => {
let verified_blobs = if self.blobs_required_for_block(&block) {
let kzg = self
.kzg
.as_ref()
.ok_or(AvailabilityCheckError::KzgNotInitialized)?;
let kzg = self.kzg.as_ref();
verify_kzg_for_blob_list(blob_list.iter(), kzg)
.map_err(AvailabilityCheckError::Kzg)?;
Some(blob_list)
Expand Down Expand Up @@ -353,10 +346,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)?;
let kzg = self.kzg.as_ref();
verify_kzg_for_blob_list(all_blobs.iter(), kzg)?;
}

Expand Down
5 changes: 1 addition & 4 deletions beacon_node/beacon_chain/src/data_column_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,7 @@ pub fn validate_data_column_sidecar_for_gossip<T: BeaconChainTypes>(
let parent_block = verify_parent_block_and_finalized_descendant(data_column.clone(), chain)?;
verify_slot_higher_than_parent(&parent_block, column_slot)?;
verify_proposer_and_signature(&data_column, &parent_block, chain)?;
let kzg = chain
.kzg
.clone()
.ok_or(GossipDataColumnError::KzgNotInitialized)?;
let kzg = chain.kzg.clone();
michaelsproul marked this conversation as resolved.
Show resolved Hide resolved
let kzg_verified_data_column = verify_kzg_for_data_column(data_column.clone(), &kzg)
.map_err(GossipDataColumnError::InvalidKzgProof)?;

Expand Down
7 changes: 3 additions & 4 deletions beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,12 +507,12 @@ where
let validator_keypairs = self
.validator_keypairs
.expect("cannot build without validator keypairs");
let kzg = spec.deneb_fork_epoch.map(|_| KZG.clone());
let kzg = spec.deneb_fork_epoch.map(|_| KZG.clone()).unwrap();

let validator_monitor_config = self.validator_monitor_config.unwrap_or_default();

let chain_config = self.chain_config.unwrap_or_default();
let mut builder = BeaconChainBuilder::new(self.eth_spec_instance)
let mut builder = BeaconChainBuilder::new(self.eth_spec_instance, kzg.clone())
.logger(log.clone())
.custom_spec(spec.clone())
.store(self.store.expect("cannot build without store"))
Expand All @@ -531,8 +531,7 @@ where
log.clone(),
5,
)))
.validator_monitor_config(validator_monitor_config)
.kzg(kzg);
.validator_monitor_config(validator_monitor_config);

builder = if let Some(mutator) = self.initial_mutator {
mutator(builder)
Expand Down
4 changes: 2 additions & 2 deletions beacon_node/beacon_chain/tests/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ async fn blob_sidecar_event_on_process_gossip_blob() {
let mut blob_event_receiver = event_handler.subscribe_blob_sidecar();

// build and process a gossip verified blob
let kzg = harness.chain.kzg.as_ref().unwrap();
let kzg = harness.chain.kzg.as_ref();
let mut rng = StdRng::seed_from_u64(0xDEADBEEF0BAD5EEDu64);
let sidecar = BlobSidecar::random_valid(&mut rng, kzg)
.map(Arc::new)
Expand Down Expand Up @@ -59,7 +59,7 @@ async fn blob_sidecar_event_on_process_rpc_blobs() {
let mut blob_event_receiver = event_handler.subscribe_blob_sidecar();

// build and process multiple rpc blobs
let kzg = harness.chain.kzg.as_ref().unwrap();
let kzg = harness.chain.kzg.as_ref();
let mut rng = StdRng::seed_from_u64(0xDEADBEEF0BAD5EEDu64);

let blob_1 = BlobSidecar::random_valid(&mut rng, kzg)
Expand Down
31 changes: 16 additions & 15 deletions beacon_node/client/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,22 @@ where
None
};

let builder = BeaconChainBuilder::new(eth_spec_instance)
let kzg = if let Some(trusted_setup) = config.clone().trusted_setup {
let kzg_err_msg = |e| format!("Failed to load trusted setup: {:?}", e);

if spec.is_peer_das_scheduled() {
Arc::new(
Kzg::new_from_trusted_setup_das_enabled(trusted_setup).map_err(kzg_err_msg)?,
)
} else {
Arc::new(Kzg::new_from_trusted_setup(trusted_setup).map_err(kzg_err_msg)?)
}
} else {
// TODO(kzg) this causes us failed tests i.e. http_server_genesis_state
panic!("Failed to load KZG");
michaelsproul marked this conversation as resolved.
Show resolved Hide resolved
};

let builder = BeaconChainBuilder::new(eth_spec_instance, kzg.clone())
.logger(context.log().clone())
.store(store)
.task_executor(context.executor.clone())
Expand Down Expand Up @@ -623,20 +638,6 @@ where
ClientGenesis::FromStore => builder.resume_from_db().map(|v| (v, None))?,
};

let beacon_chain_builder = if let Some(trusted_setup) = config.trusted_setup {
let kzg_err_msg = |e| format!("Failed to load trusted setup: {:?}", e);

let kzg = if spec.is_peer_das_scheduled() {
Kzg::new_from_trusted_setup_das_enabled(trusted_setup).map_err(kzg_err_msg)?
} else {
Kzg::new_from_trusted_setup(trusted_setup).map_err(kzg_err_msg)?
};

beacon_chain_builder.kzg(Some(Arc::new(kzg)))
} else {
beacon_chain_builder
};

if config.sync_eth1_chain {
self.eth1_service = eth1_service_option;
}
Expand Down
2 changes: 2 additions & 0 deletions beacon_node/network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ edition = { workspace = true }
sloggers = { workspace = true }
genesis = { workspace = true }
matches = "0.1.8"
serde_json = { workspace = true }
slog-term = { workspace = true }
slog-async = { workspace = true }
eth2 = { workspace = true }
gossipsub = { workspace = true }
eth2_network_config = { workspace = true }

[dependencies]
async-channel = { workspace = true }
Expand Down
11 changes: 9 additions & 2 deletions beacon_node/network/src/subnet_service/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use super::*;
use beacon_chain::{
builder::{BeaconChainBuilder, Witness},
eth1_chain::CachingEth1Backend,
BeaconChain,
BeaconChain, Kzg, TrustedSetup,
};
use eth2_network_config::TRUSTED_SETUP_BYTES;
use futures::prelude::*;
use genesis::{generate_deterministic_keypairs, interop_genesis_state, DEFAULT_ETH1_BLOCK_HASH};
use lighthouse_network::NetworkConfig;
Expand Down Expand Up @@ -45,12 +46,18 @@ impl TestBeaconChain {
let store =
HotColdDB::open_ephemeral(StoreConfig::default(), spec.clone(), log.clone()).unwrap();

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 (shutdown_tx, _) = futures::channel::mpsc::channel(1);

let test_runtime = TestRuntime::default();

let chain = Arc::new(
BeaconChainBuilder::new(MainnetEthSpec)
BeaconChainBuilder::new(MainnetEthSpec, kzg.clone())
.logger(log.clone())
.custom_spec(spec.clone())
.store(Arc::new(store))
Expand Down
Loading