From e4bfe71cd1f0a2784d0bd57f85b2f5d8cf503ac1 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 6 Jan 2025 17:05:30 +1100 Subject: [PATCH] Thread through ChainSpec --- .../overflow_lru_cache.rs | 2 +- beacon_node/beacon_chain/src/test_utils.rs | 19 +++++++++---------- .../execution_layer/src/engine_api/http.rs | 3 ++- .../test_utils/execution_block_generator.rs | 13 +++++++++++-- .../src/test_utils/mock_execution_layer.rs | 9 +++++---- .../execution_layer/src/test_utils/mod.rs | 9 +++++++-- .../src/sync/block_sidecar_coupling.rs | 13 +++++++++++-- beacon_node/network/src/sync/tests/lookups.rs | 6 ++++-- consensus/types/src/chain_spec.rs | 15 ++++++++------- lcli/src/mock_el.rs | 5 +++-- testing/node_test_rig/src/lib.rs | 9 ++++++++- 11 files changed, 69 insertions(+), 34 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs index d5351a5dc2..4cb425b74a 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs @@ -1178,7 +1178,7 @@ mod pending_components_tests { let mut rng = StdRng::seed_from_u64(0xDEADBEEF0BAD5EEDu64); let spec = test_spec::(); let (block, blobs_vec) = - generate_rand_block_and_blobs::(ForkName::Deneb, NumBlobs::Random, &mut rng); + generate_rand_block_and_blobs::(ForkName::Deneb, NumBlobs::Random, &mut rng, &spec); let max_len = spec.max_blobs_per_block(block.epoch()) as usize; let mut blobs: RuntimeFixedList>>> = RuntimeFixedList::default(max_len); diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index a792f5f404..a722669c2d 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -511,7 +511,7 @@ where pub fn mock_execution_layer_with_config(mut self) -> Self { let mock = mock_execution_layer_from_parts::( - self.spec.as_ref().expect("cannot build without spec"), + self.spec.clone().expect("cannot build without spec"), self.runtime.task_executor.clone(), ); self.execution_layer = Some(mock.el.clone()); @@ -611,7 +611,7 @@ where } pub fn mock_execution_layer_from_parts( - spec: &ChainSpec, + spec: Arc, task_executor: TaskExecutor, ) -> MockExecutionLayer { let shanghai_time = spec.capella_fork_epoch.map(|epoch| { @@ -624,7 +624,7 @@ pub fn mock_execution_layer_from_parts( HARNESS_GENESIS_TIME + spec.seconds_per_slot * E::slots_per_epoch() * epoch.as_u64() }); - let kzg = get_kzg(spec); + let kzg = get_kzg(&spec); MockExecutionLayer::new( task_executor, @@ -633,7 +633,7 @@ pub fn mock_execution_layer_from_parts( cancun_time, prague_time, Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()), - spec.clone(), + spec, Some(kzg), ) } @@ -2816,11 +2816,12 @@ pub fn generate_rand_block_and_blobs( fork_name: ForkName, num_blobs: NumBlobs, rng: &mut impl Rng, + spec: &ChainSpec, ) -> (SignedBeaconBlock>, Vec>) { let inner = map_fork_name!(fork_name, BeaconBlock, <_>::random_for_test(rng)); let mut block = SignedBeaconBlock::from_block(inner, types::Signature::random_for_test(rng)); - + let max_blobs = spec.max_blobs_per_block(block.epoch()) as usize; let mut blob_sidecars = vec![]; let bundle = match block { @@ -2830,8 +2831,7 @@ pub fn generate_rand_block_and_blobs( // Get either zero blobs or a random number of blobs between 1 and Max Blobs. let payload: &mut FullPayloadDeneb = &mut message.body.execution_payload; let num_blobs = match num_blobs { - // TODO(pawan): thread the chainspec value here - NumBlobs::Random => rng.gen_range(1..=6), + NumBlobs::Random => rng.gen_range(1..=max_blobs), NumBlobs::Number(n) => n, NumBlobs::None => 0, }; @@ -2851,8 +2851,7 @@ pub fn generate_rand_block_and_blobs( // Get either zero blobs or a random number of blobs between 1 and Max Blobs. let payload: &mut FullPayloadElectra = &mut message.body.execution_payload; let num_blobs = match num_blobs { - // TODO(pawan): thread the chainspec value here - NumBlobs::Random => rng.gen_range(1..=6), + NumBlobs::Random => rng.gen_range(1..=max_blobs), NumBlobs::Number(n) => n, NumBlobs::None => 0, }; @@ -2906,7 +2905,7 @@ pub fn generate_rand_block_and_data_columns( DataColumnSidecarList, ) { let kzg = get_kzg(spec); - let (block, blobs) = generate_rand_block_and_blobs(fork_name, num_blobs, rng); + let (block, blobs) = generate_rand_block_and_blobs(fork_name, num_blobs, rng, spec); let blob_refs = blobs.iter().map(|b| &b.blob).collect::>(); let data_columns = blobs_to_data_column_sidecars(&blob_refs, &block, &kzg, spec).unwrap(); diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 33dc60d037..830eb9d5d9 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -1321,7 +1321,8 @@ mod test { impl Tester { pub fn new(with_auth: bool) -> Self { - let server = MockServer::unit_testing(); + let spec = Arc::new(MainnetEthSpec::default_spec()); + let server = MockServer::unit_testing(spec); let rpc_url = SensitiveUrl::parse(&server.url()).unwrap(); let echo_url = SensitiveUrl::parse(&format!("{}/echo", server.url())).unwrap(); diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index a190b3fb75..7dd39d58ad 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -153,6 +153,7 @@ pub struct ExecutionBlockGenerator { pub blobs_bundles: HashMap>, pub kzg: Option>, rng: Arc>, + spec: Arc, } fn make_rng() -> Arc> { @@ -162,6 +163,7 @@ fn make_rng() -> Arc> { } impl ExecutionBlockGenerator { + #[allow(clippy::too_many_arguments)] pub fn new( terminal_total_difficulty: Uint256, terminal_block_number: u64, @@ -169,6 +171,7 @@ impl ExecutionBlockGenerator { shanghai_time: Option, cancun_time: Option, prague_time: Option, + spec: Arc, kzg: Option>, ) -> Self { let mut gen = Self { @@ -188,6 +191,7 @@ impl ExecutionBlockGenerator { blobs_bundles: <_>::default(), kzg, rng: make_rng(), + spec, }; gen.insert_pow_block(0).unwrap(); @@ -671,8 +675,11 @@ impl ExecutionBlockGenerator { if execution_payload.fork_name().deneb_enabled() { // get random number between 0 and Max Blobs let mut rng = self.rng.lock(); - // TODO(pawan): thread the chainspec value here somehow - let num_blobs = rng.gen::() % (6 + 1); + let max_blobs = self + .spec + .max_blobs_per_block_by_fork(execution_payload.fork_name()) + as usize; + let num_blobs = rng.gen::() % (max_blobs + 1); let (bundle, transactions) = generate_blobs(num_blobs)?; for tx in Vec::from(transactions) { execution_payload @@ -875,6 +882,7 @@ mod test { const TERMINAL_DIFFICULTY: u64 = 10; const TERMINAL_BLOCK: u64 = 10; const DIFFICULTY_INCREMENT: u64 = 1; + let spec = Arc::new(MainnetEthSpec::default_spec()); let mut generator: ExecutionBlockGenerator = ExecutionBlockGenerator::new( Uint256::from(TERMINAL_DIFFICULTY), @@ -883,6 +891,7 @@ mod test { None, None, None, + spec, None, ); diff --git a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs index 48372a39be..f472d21482 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs @@ -13,7 +13,7 @@ pub struct MockExecutionLayer { pub server: MockServer, pub el: ExecutionLayer, pub executor: TaskExecutor, - pub spec: ChainSpec, + pub spec: Arc, } impl MockExecutionLayer { @@ -29,7 +29,7 @@ impl MockExecutionLayer { None, None, Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()), - spec, + Arc::new(spec), None, ) } @@ -42,7 +42,7 @@ impl MockExecutionLayer { cancun_time: Option, prague_time: Option, jwt_key: Option, - spec: ChainSpec, + spec: Arc, kzg: Option>, ) -> Self { let handle = executor.handle().unwrap(); @@ -57,6 +57,7 @@ impl MockExecutionLayer { shanghai_time, cancun_time, prague_time, + spec.clone(), kzg, ); @@ -320,7 +321,7 @@ impl MockExecutionLayer { pub async fn with_terminal_block<'a, U, V>(self, func: U) -> Self where - U: Fn(ChainSpec, ExecutionLayer, Option) -> V, + U: Fn(Arc, ExecutionLayer, Option) -> V, V: Future, { let terminal_block_number = self diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index faf6d4ef0b..726874ddb0 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -21,7 +21,7 @@ use std::marker::PhantomData; use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use std::sync::{Arc, LazyLock}; use tokio::{runtime, sync::oneshot}; -use types::{EthSpec, ExecutionBlockHash, Uint256}; +use types::{ChainSpec, EthSpec, ExecutionBlockHash, Uint256}; use warp::{http::StatusCode, Filter, Rejection}; use crate::EngineCapabilities; @@ -107,7 +107,7 @@ pub struct MockServer { } impl MockServer { - pub fn unit_testing() -> Self { + pub fn unit_testing(chain_spec: Arc) -> Self { Self::new( &runtime::Handle::current(), JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap(), @@ -117,6 +117,7 @@ impl MockServer { None, // FIXME(capella): should this be the default? None, // FIXME(deneb): should this be the default? None, // FIXME(electra): should this be the default? + chain_spec, None, ) } @@ -124,6 +125,7 @@ impl MockServer { pub fn new_with_config( handle: &runtime::Handle, config: MockExecutionConfig, + spec: Arc, kzg: Option>, ) -> Self { let MockExecutionConfig { @@ -145,6 +147,7 @@ impl MockServer { shanghai_time, cancun_time, prague_time, + spec, kzg, ); @@ -208,6 +211,7 @@ impl MockServer { shanghai_time: Option, cancun_time: Option, prague_time: Option, + spec: Arc, kzg: Option>, ) -> Self { Self::new_with_config( @@ -222,6 +226,7 @@ impl MockServer { cancun_time, prague_time, }, + spec, kzg, ) } diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index 0d0a5b8081..7a234eaef0 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -256,7 +256,10 @@ mod tests { let peer_id = PeerId::random(); let mut rng = XorShiftRng::from_seed([42; 16]); let blocks = (0..4) - .map(|_| generate_rand_block_and_blobs::(ForkName::Base, NumBlobs::None, &mut rng).0) + .map(|_| { + generate_rand_block_and_blobs::(ForkName::Base, NumBlobs::None, &mut rng, &spec) + .0 + }) .collect::>(); let max_len = spec.max_blobs_per_block(blocks.first().unwrap().epoch()) as usize; let mut info = @@ -281,7 +284,13 @@ mod tests { let blocks = (0..4) .map(|_| { // Always generate some blobs. - generate_rand_block_and_blobs::(ForkName::Deneb, NumBlobs::Number(3), &mut rng).0 + generate_rand_block_and_blobs::( + ForkName::Deneb, + NumBlobs::Number(3), + &mut rng, + &spec, + ) + .0 }) .collect::>(); let max_len = spec.max_blobs_per_block(blocks.first().unwrap().epoch()) as usize; diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 8c88ddb62d..ee96db0f63 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -216,7 +216,7 @@ impl TestRig { ) -> (SignedBeaconBlock, Vec>) { let fork_name = self.fork_name; let rng = &mut self.rng; - generate_rand_block_and_blobs::(fork_name, num_blobs, rng) + generate_rand_block_and_blobs::(fork_name, num_blobs, rng, &self.spec) } fn rand_block_and_data_columns( @@ -1331,8 +1331,10 @@ impl TestRig { #[test] fn stable_rng() { + let spec = types::MainnetEthSpec::default_spec(); let mut rng = XorShiftRng::from_seed([42; 16]); - let (block, _) = generate_rand_block_and_blobs::(ForkName::Base, NumBlobs::None, &mut rng); + let (block, _) = + generate_rand_block_and_blobs::(ForkName::Base, NumBlobs::None, &mut rng, &spec); assert_eq!( block.canonical_root(), Hash256::from_slice( diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index c1d87354a8..05d5142853 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -606,14 +606,15 @@ impl ChainSpec { } } - /// Returns the deneb preset value if peerdas epoch hasn't hit. - /// Otherwise, returns the value obtained from the config.yaml. + /// Return the value of `MAX_BLOBS_PER_BLOCK` appropriate for the fork at `epoch`. pub fn max_blobs_per_block(&self, epoch: Epoch) -> u64 { - if self.is_peer_das_enabled_for_epoch(epoch) { - self.max_blobs_per_block - } else { - default_max_blobs_per_block() - } + self.max_blobs_per_block_by_fork(self.fork_name_at_epoch(epoch)) + } + + /// Return the value of `MAX_BLOBS_PER_BLOCK` appropriate for `fork`. + pub fn max_blobs_per_block_by_fork(&self, _fork_name: ForkName) -> u64 { + // TODO(electra): add Electra blobs per block change here + self.max_blobs_per_block } pub fn data_columns_per_subnet(&self) -> usize { diff --git a/lcli/src/mock_el.rs b/lcli/src/mock_el.rs index 8d3220b1df..aad7c89dda 100644 --- a/lcli/src/mock_el.rs +++ b/lcli/src/mock_el.rs @@ -9,6 +9,7 @@ use execution_layer::{ }; use std::net::Ipv4Addr; use std::path::PathBuf; +use std::sync::Arc; use types::*; pub fn run(mut env: Environment, matches: &ArgMatches) -> Result<(), String> { @@ -21,7 +22,7 @@ pub fn run(mut env: Environment, matches: &ArgMatches) -> Result< let prague_time = parse_optional(matches, "prague-time")?; let handle = env.core_context().executor.handle().unwrap(); - let spec = &E::default_spec(); + let spec = Arc::new(E::default_spec()); let jwt_key = JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap(); std::fs::write(jwt_path, hex::encode(DEFAULT_JWT_SECRET)).unwrap(); @@ -39,7 +40,7 @@ pub fn run(mut env: Environment, matches: &ArgMatches) -> Result< prague_time, }; let kzg = None; - let server: MockServer = MockServer::new_with_config(&handle, config, kzg); + let server: MockServer = MockServer::new_with_config(&handle, config, spec, kzg); if all_payloads_valid { eprintln!( diff --git a/testing/node_test_rig/src/lib.rs b/testing/node_test_rig/src/lib.rs index ac01c84b9d..6e632ccf54 100644 --- a/testing/node_test_rig/src/lib.rs +++ b/testing/node_test_rig/src/lib.rs @@ -7,6 +7,7 @@ use environment::RuntimeContext; use eth2::{reqwest::ClientBuilder, BeaconNodeHttpClient, Timeouts}; use sensitive_url::SensitiveUrl; use std::path::PathBuf; +use std::sync::Arc; use std::time::Duration; use std::time::{SystemTime, UNIX_EPOCH}; use tempfile::{Builder as TempBuilder, TempDir}; @@ -248,8 +249,14 @@ impl LocalExecutionNode { if let Err(e) = std::fs::write(jwt_file_path, config.jwt_key.hex_string()) { panic!("Failed to write jwt file {}", e); } + let spec = Arc::new(E::default_spec()); Self { - server: MockServer::new_with_config(&context.executor.handle().unwrap(), config, None), + server: MockServer::new_with_config( + &context.executor.handle().unwrap(), + config, + spec, + None, + ), datadir, } }