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 402dac1fa8c..5e6322ae95a 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 @@ -1279,7 +1279,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, &spec); + generate_rand_block_and_blobs::(ForkName::Deneb, NumBlobs::Random, &mut rng); let max_len = spec.max_blobs_per_block(block.epoch()) as usize; let mut blobs: RuntimeFixedVector>>> = RuntimeFixedVector::default(max_len); diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 382775ab50f..5f8d440d341 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -463,7 +463,7 @@ mod test { #[track_caller] fn test_validate_data_columns(kzg: &Kzg, spec: &ChainSpec) { - let num_of_blobs = 6; + let num_of_blobs = 2; let (signed_block, blobs, proofs) = create_test_fulu_block_and_blobs::(num_of_blobs, spec); let blob_refs = blobs.iter().collect::>(); @@ -489,7 +489,8 @@ mod test { #[track_caller] fn test_build_data_columns(kzg: &Kzg, spec: &ChainSpec) { - let num_of_blobs = 6; + // Using at least 2 blobs to make sure we're arranging the data columns correctly. + let num_of_blobs = 2; let (signed_block, blobs, proofs) = create_test_fulu_block_and_blobs::(num_of_blobs, spec); @@ -529,6 +530,7 @@ mod test { #[track_caller] fn test_reconstruct_data_columns(kzg: &Kzg, spec: &ChainSpec) { + // Using at least 2 blobs to make sure we're arranging the data columns correctly. let num_of_blobs = 2; let (signed_block, blobs, proofs) = create_test_fulu_block_and_blobs::(num_of_blobs, spec); @@ -552,6 +554,7 @@ mod test { #[track_caller] fn test_reconstruct_data_columns_unordered(kzg: &Kzg, spec: &ChainSpec) { + // Using at least 2 blobs to make sure we're arranging the data columns correctly. let num_of_blobs = 2; let (signed_block, blobs, proofs) = create_test_fulu_block_and_blobs::(num_of_blobs, spec); @@ -573,7 +576,7 @@ mod test { #[track_caller] fn test_reconstruct_blobs_from_data_columns(kzg: &Kzg, spec: &ChainSpec) { - let num_of_blobs = 6; + let num_of_blobs = 3; let (signed_block, blobs, proofs) = create_test_fulu_block_and_blobs::(num_of_blobs, spec); let blob_refs = blobs.iter().collect::>(); @@ -583,7 +586,8 @@ mod test { // Now reconstruct let signed_blinded_block = signed_block.into(); - let blob_indices = vec![3, 4, 5]; + // Using at least 2 blobs to make sure we're arranging the data columns correctly. + let blob_indices = vec![1, 2]; let reconstructed_blobs = reconstruct_blobs( kzg, &column_sidecars.iter().as_slice()[0..column_sidecars.len() / 2], diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 38797d0264d..1f030c3c086 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -172,23 +172,28 @@ fn make_rng() -> Mutex { Mutex::new(StdRng::seed_from_u64(0x0DDB1A5E5BAD5EEDu64)) } -/// Return a `ChainSpec` suitable for test usage. -/// -/// If the `fork_from_env` feature is enabled, read the fork to use from the FORK_NAME environment -/// variable. Otherwise use the default spec. -pub fn test_spec() -> ChainSpec { - let mut spec = if cfg!(feature = "fork_from_env") { +pub fn fork_name_from_env() -> Option { + if cfg!(feature = "fork_from_env") { let fork_name = std::env::var(FORK_NAME_ENV_VAR).unwrap_or_else(|e| { panic!( "{} env var must be defined when using fork_from_env: {:?}", FORK_NAME_ENV_VAR, e ) }); - let fork = ForkName::from_str(fork_name.as_str()).unwrap(); - fork.make_genesis_spec(E::default_spec()) + Some(ForkName::from_str(fork_name.as_str()).unwrap()) } else { - E::default_spec() - }; + None + } +} + +/// Return a `ChainSpec` suitable for test usage. +/// +/// If the `fork_from_env` feature is enabled, read the fork to use from the FORK_NAME environment +/// variable. Otherwise use the default spec. +pub fn test_spec() -> ChainSpec { + let mut spec = fork_name_from_env() + .map(|fork| fork.make_genesis_spec(E::default_spec())) + .unwrap_or_else(|| E::default_spec()); // Set target aggregators to a high value by default. spec.target_aggregators_per_committee = DEFAULT_TARGET_AGGREGATORS; @@ -3245,96 +3250,49 @@ pub enum NumBlobs { None, } +macro_rules! add_blob_transactions { + ($message:expr, $payload_type:ty, $num_blobs:expr, $rng:expr, $fork_name:expr) => {{ + let num_blobs = match $num_blobs { + NumBlobs::Random => $rng.random_range(1..=2), + NumBlobs::Number(n) => n, + NumBlobs::None => 0, + }; + let (bundle, transactions) = + execution_layer::test_utils::generate_blobs::(num_blobs, $fork_name).unwrap(); + + let payload: &mut $payload_type = &mut $message.body.execution_payload; + payload.execution_payload.transactions = <_>::default(); + for tx in Vec::from(transactions) { + payload.execution_payload.transactions.push(tx).unwrap(); + } + $message.body.blob_kzg_commitments = bundle.commitments.clone(); + bundle + }}; +} + 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 { SignedBeaconBlock::Deneb(SignedBeaconBlockDeneb { ref mut message, .. - }) => { - // 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 { - NumBlobs::Random => rng.random_range(1..=max_blobs), - NumBlobs::Number(n) => n, - NumBlobs::None => 0, - }; - let (bundle, transactions) = - execution_layer::test_utils::generate_blobs::(num_blobs, fork_name).unwrap(); - - payload.execution_payload.transactions = <_>::default(); - for tx in Vec::from(transactions) { - payload.execution_payload.transactions.push(tx).unwrap(); - } - message.body.blob_kzg_commitments = bundle.commitments.clone(); - bundle - } + }) => add_blob_transactions!(message, FullPayloadDeneb, num_blobs, rng, fork_name), SignedBeaconBlock::Electra(SignedBeaconBlockElectra { ref mut message, .. - }) => { - // 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 { - NumBlobs::Random => rng.random_range(1..=max_blobs), - NumBlobs::Number(n) => n, - NumBlobs::None => 0, - }; - let (bundle, transactions) = - execution_layer::test_utils::generate_blobs::(num_blobs, fork_name).unwrap(); - payload.execution_payload.transactions = <_>::default(); - for tx in Vec::from(transactions) { - payload.execution_payload.transactions.push(tx).unwrap(); - } - message.body.blob_kzg_commitments = bundle.commitments.clone(); - bundle - } + }) => add_blob_transactions!(message, FullPayloadElectra, num_blobs, rng, fork_name), SignedBeaconBlock::Fulu(SignedBeaconBlockFulu { ref mut message, .. - }) => { - // Get either zero blobs or a random number of blobs between 1 and Max Blobs. - let payload: &mut FullPayloadFulu = &mut message.body.execution_payload; - let num_blobs = match num_blobs { - NumBlobs::Random => rng.random_range(1..=max_blobs), - NumBlobs::Number(n) => n, - NumBlobs::None => 0, - }; - let (bundle, transactions) = - execution_layer::test_utils::generate_blobs::(num_blobs, fork_name).unwrap(); - payload.execution_payload.transactions = <_>::default(); - for tx in Vec::from(transactions) { - payload.execution_payload.transactions.push(tx).unwrap(); - } - message.body.blob_kzg_commitments = bundle.commitments.clone(); - bundle - } + }) => add_blob_transactions!(message, FullPayloadFulu, num_blobs, rng, fork_name), SignedBeaconBlock::Gloas(SignedBeaconBlockGloas { ref mut message, .. - }) => { - // Get either zero blobs or a random number of blobs between 1 and Max Blobs. - let payload: &mut FullPayloadGloas = &mut message.body.execution_payload; - let num_blobs = match num_blobs { - NumBlobs::Random => rng.random_range(1..=max_blobs), - NumBlobs::Number(n) => n, - NumBlobs::None => 0, - }; - let (bundle, transactions) = - execution_layer::test_utils::generate_blobs::(num_blobs, fork_name).unwrap(); - payload.execution_payload.transactions = <_>::default(); - for tx in Vec::from(transactions) { - payload.execution_payload.transactions.push(tx).unwrap(); - } - message.body.blob_kzg_commitments = bundle.commitments.clone(); - bundle - } + }) => add_blob_transactions!(message, FullPayloadGloas, num_blobs, rng, fork_name), _ => return (block, blob_sidecars), }; @@ -3375,7 +3333,7 @@ pub fn generate_rand_block_and_data_columns( SignedBeaconBlock>, DataColumnSidecarList, ) { - let (block, _blobs) = generate_rand_block_and_blobs(fork_name, num_blobs, rng, spec); + let (block, _blobs) = generate_rand_block_and_blobs(fork_name, num_blobs, rng); let data_columns = generate_data_column_sidecars_from_block(&block, spec); (block, data_columns) } diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 7dfef50ea11..95740730a13 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -297,19 +297,20 @@ async fn chain_segment_full_segment() { #[tokio::test] async fn chain_segment_varying_chunk_size() { + let (chain_segment, chain_segment_blobs) = get_chain_segment().await; + let blocks: Vec> = chain_segment_blocks(&chain_segment, &chain_segment_blobs) + .into_iter() + .collect(); + for chunk_size in &[1, 2, 3, 5, 31, 32, 33, 42] { let harness = get_harness(VALIDATOR_COUNT, NodeCustodyType::Fullnode); - let (chain_segment, chain_segment_blobs) = get_chain_segment().await; - let blocks: Vec> = chain_segment_blocks(&chain_segment, &chain_segment_blobs) - .into_iter() - .collect(); harness .chain .slot_clock .set_slot(blocks.last().unwrap().slot().as_u64()); - for chunk in blocks.chunks(*chunk_size) { + for chunk in blocks.clone().chunks(*chunk_size) { harness .chain .process_chain_segment(chunk.to_vec(), NotifyExecutionLayer::Yes) diff --git a/beacon_node/beacon_chain/tests/events.rs b/beacon_node/beacon_chain/tests/events.rs index 466058eea38..86bdb03dafd 100644 --- a/beacon_node/beacon_chain/tests/events.rs +++ b/beacon_node/beacon_chain/tests/events.rs @@ -1,20 +1,26 @@ use beacon_chain::blob_verification::GossipVerifiedBlob; use beacon_chain::data_column_verification::GossipVerifiedDataColumn; -use beacon_chain::test_utils::{BeaconChainHarness, generate_data_column_sidecars_from_block}; +use beacon_chain::test_utils::{ + BeaconChainHarness, fork_name_from_env, generate_data_column_sidecars_from_block, test_spec, +}; use eth2::types::{EventKind, SseBlobSidecar, SseDataColumnSidecar}; use rand::SeedableRng; use rand::rngs::StdRng; use std::sync::Arc; use types::blob_sidecar::FixedBlobSidecarList; use types::test_utils::TestRandom; -use types::{BlobSidecar, DataColumnSidecar, EthSpec, ForkName, MinimalEthSpec, Slot}; +use types::{BlobSidecar, DataColumnSidecar, EthSpec, MinimalEthSpec, Slot}; type E = MinimalEthSpec; /// Verifies that a blob event is emitted when a gossip verified blob is received via gossip or the publish block API. #[tokio::test] async fn blob_sidecar_event_on_process_gossip_blob() { - let spec = Arc::new(ForkName::Deneb.make_genesis_spec(E::default_spec())); + if fork_name_from_env().is_some_and(|f| !f.deneb_enabled() || f.fulu_enabled()) { + return; + }; + + let spec = Arc::new(test_spec::()); let harness = BeaconChainHarness::builder(E::default()) .spec(spec) .deterministic_keypairs(8) @@ -48,7 +54,11 @@ async fn blob_sidecar_event_on_process_gossip_blob() { /// Verifies that a data column event is emitted when a gossip verified data column is received via gossip or the publish block API. #[tokio::test] async fn data_column_sidecar_event_on_process_gossip_data_column() { - let spec = Arc::new(ForkName::Fulu.make_genesis_spec(E::default_spec())); + if fork_name_from_env().is_some_and(|f| !f.fulu_enabled()) { + return; + }; + + let spec = Arc::new(test_spec::()); let harness = BeaconChainHarness::builder(E::default()) .spec(spec) .deterministic_keypairs(8) @@ -93,7 +103,11 @@ async fn data_column_sidecar_event_on_process_gossip_data_column() { /// Verifies that a blob event is emitted when blobs are received via RPC. #[tokio::test] async fn blob_sidecar_event_on_process_rpc_blobs() { - let spec = Arc::new(ForkName::Deneb.make_genesis_spec(E::default_spec())); + if fork_name_from_env().is_some_and(|f| !f.deneb_enabled() || f.fulu_enabled()) { + return; + }; + + let spec = Arc::new(test_spec::()); let harness = BeaconChainHarness::builder(E::default()) .spec(spec) .deterministic_keypairs(8) @@ -112,7 +126,7 @@ async fn blob_sidecar_event_on_process_rpc_blobs() { let slot = head_state.slot() + 1; let ((signed_block, opt_blobs), _) = harness.make_block(head_state, slot).await; let (kzg_proofs, blobs) = opt_blobs.unwrap(); - assert!(blobs.len() > 2); + assert_eq!(blobs.len(), 2); let blob_1 = Arc::new(BlobSidecar::new(0, blobs[0].clone(), &signed_block, kzg_proofs[0]).unwrap()); @@ -144,7 +158,11 @@ async fn blob_sidecar_event_on_process_rpc_blobs() { #[tokio::test] async fn data_column_sidecar_event_on_process_rpc_columns() { - let spec = Arc::new(ForkName::Fulu.make_genesis_spec(E::default_spec())); + if fork_name_from_env().is_some_and(|f| !f.fulu_enabled()) { + return; + }; + + let spec = Arc::new(test_spec::()); let harness = BeaconChainHarness::builder(E::default()) .spec(spec.clone()) .deterministic_keypairs(8) diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 53e841692e6..409fffaa468 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -7,11 +7,11 @@ use beacon_chain::custody_context::CUSTODY_CHANGE_DA_EFFECTIVE_DELAY_SECONDS; use beacon_chain::data_availability_checker::AvailableBlock; use beacon_chain::historical_data_columns::HistoricalDataColumnError; use beacon_chain::schema_change::migrate_schema; -use beacon_chain::test_utils::SyncCommitteeStrategy; use beacon_chain::test_utils::{ AttestationStrategy, BeaconChainHarness, BlockStrategy, DiskHarnessType, get_kzg, mock_execution_layer_from_parts, test_spec, }; +use beacon_chain::test_utils::{SyncCommitteeStrategy, fork_name_from_env}; use beacon_chain::{ BeaconChain, BeaconChainError, BeaconChainTypes, BeaconSnapshot, BlockError, ChainConfig, NotifyExecutionLayer, ServerSentEventHandler, WhenSlotSkipped, @@ -3211,12 +3211,13 @@ async fn test_import_historical_data_columns_batch() { for block in block_root_iter { let (block_root, _) = block.unwrap(); let data_columns = harness.chain.store.get_data_columns(&block_root).unwrap(); - assert!(data_columns.is_some()); - for data_column in data_columns.unwrap() { + for data_column in data_columns.unwrap_or_default() { data_columns_list.push(data_column); } } + assert!(!data_columns_list.is_empty()); + harness .extend_chain( (E::slots_per_epoch() * 4) as usize, @@ -3255,8 +3256,18 @@ async fn test_import_historical_data_columns_batch() { for block in block_root_iter { let (block_root, _) = block.unwrap(); - let data_columns = harness.chain.store.get_data_columns(&block_root).unwrap(); - assert!(data_columns.is_some()) + if !harness + .get_block(block_root.into()) + .unwrap() + .message() + .body() + .blob_kzg_commitments() + .unwrap() + .is_empty() + { + let data_columns = harness.chain.store.get_data_columns(&block_root).unwrap(); + assert!(data_columns.is_some()) + }; } } @@ -3290,9 +3301,8 @@ async fn test_import_historical_data_columns_batch_mismatched_block_root() { for block in block_root_iter { let (block_root, _) = block.unwrap(); let data_columns = harness.chain.store.get_data_columns(&block_root).unwrap(); - assert!(data_columns.is_some()); - for data_column in data_columns.unwrap() { + for data_column in data_columns.unwrap_or_default() { let mut data_column = (*data_column).clone(); if data_column.index % 2 == 0 { data_column.signed_block_header.message.body_root = Hash256::ZERO; @@ -3301,6 +3311,7 @@ async fn test_import_historical_data_columns_batch_mismatched_block_root() { data_columns_list.push(Arc::new(data_column)); } } + assert!(!data_columns_list.is_empty()); harness .extend_chain( @@ -3347,7 +3358,11 @@ async fn test_import_historical_data_columns_batch_mismatched_block_root() { // be imported. #[tokio::test] async fn test_import_historical_data_columns_batch_no_block_found() { - let spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); + if fork_name_from_env().is_some_and(|f| !f.fulu_enabled()) { + return; + }; + + let spec = test_spec::(); let db_path = tempdir().unwrap(); let store = get_store_generic(&db_path, StoreConfig::default(), spec); let start_slot = Slot::new(1); @@ -3374,15 +3389,16 @@ async fn test_import_historical_data_columns_batch_no_block_found() { for block in block_root_iter { let (block_root, _) = block.unwrap(); let data_columns = harness.chain.store.get_data_columns(&block_root).unwrap(); - assert!(data_columns.is_some()); - for data_column in data_columns.unwrap() { + for data_column in data_columns.unwrap_or_default() { let mut data_column = (*data_column).clone(); data_column.signed_block_header.message.body_root = Hash256::ZERO; data_columns_list.push(Arc::new(data_column)); } } + assert!(!data_columns_list.is_empty()); + harness .extend_chain( (E::slots_per_epoch() * 4) as usize, @@ -4108,6 +4124,12 @@ async fn deneb_prune_blobs_no_finalization() { /// Check that blob pruning does not fail trying to prune across the fork boundary. #[tokio::test] async fn prune_blobs_across_fork_boundary() { + // This test covers earlier forks and only need to be executed once. + // Note: this test is quite expensive (building a chain to epoch 15) and we should revisit this + if fork_name_from_env() != Some(ForkName::latest_stable()) { + return; + } + let mut spec = ForkName::Capella.make_genesis_spec(E::default_spec()); let deneb_fork_epoch = Epoch::new(4); @@ -4124,6 +4146,7 @@ async fn prune_blobs_across_fork_boundary() { let store = get_store_generic(&db_path, StoreConfig::default(), spec); let harness = get_harness(store.clone(), LOW_VALIDATOR_COUNT); + harness.execution_block_generator().set_min_blob_count(1); let blocks_to_deneb_finalization = E::slots_per_epoch() * 7; let blocks_to_electra_finalization = E::slots_per_epoch() * 4; @@ -4279,7 +4302,7 @@ async fn prune_blobs_across_fork_boundary() { // Fulu fork epochs // Pruning should have been triggered assert!(store.get_blob_info().oldest_blob_slot <= Some(oldest_slot)); - // Oldest blost slot should never be greater than the first fulu slot + // Oldest blob slot should never be greater than the first fulu slot let fulu_first_slot = fulu_fork_epoch.start_slot(E::slots_per_epoch()); assert!(store.get_blob_info().oldest_blob_slot <= Some(fulu_first_slot)); // Blobs should not exist post-Fulu @@ -4764,7 +4787,7 @@ async fn fulu_prune_data_columns_margin_test(margin: u64) { check_data_column_existence(&harness, oldest_data_column_slot, harness.head_slot(), true); } -/// Check tat there are data column sidecars (or not) at every slot in the range. +/// Check that there are data column sidecars (or not) at every slot in the range. fn check_data_column_existence( harness: &TestHarness, start_slot: Slot, diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index bc927e19b41..c671c92163d 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -1453,8 +1453,7 @@ mod test { impl Tester { pub fn new(with_auth: bool) -> Self { - let spec = Arc::new(MainnetEthSpec::default_spec()); - let server = MockServer::unit_testing(spec); + let server = MockServer::unit_testing(); 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 4836f9307c8..77eb0c2963a 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 @@ -13,6 +13,7 @@ use rand::{Rng, SeedableRng, rngs::StdRng}; use serde::{Deserialize, Serialize}; use ssz::Decode; use ssz_types::VariableList; +use std::cmp::max; use std::collections::HashMap; use std::sync::Arc; use tree_hash::TreeHash; @@ -157,7 +158,6 @@ pub struct ExecutionBlockGenerator { pub blobs_bundles: HashMap>, pub kzg: Option>, rng: Arc>, - spec: Arc, } fn make_rng() -> Arc> { @@ -177,7 +177,6 @@ impl ExecutionBlockGenerator { prague_time: Option, osaka_time: Option, amsterdam_time: Option, - spec: Arc, kzg: Option>, ) -> Self { let mut generator = Self { @@ -200,7 +199,6 @@ impl ExecutionBlockGenerator { blobs_bundles: <_>::default(), kzg, rng: make_rng(), - spec, }; generator.insert_pow_block(0).unwrap(); @@ -732,11 +730,10 @@ impl ExecutionBlockGenerator { let fork_name = execution_payload.fork_name(); if fork_name.deneb_enabled() { - // get random number between 0 and Max Blobs + // get random number between 0 and 1 blobs by default + // For tests that need higher blob count, consider adding a `set_max_blob_count` method let mut rng = self.rng.lock(); - // TODO(EIP-7892): see FIXME below - // FIXME: this will break with BPO forks. This function needs to calculate the epoch based on block timestamp.. - let max_blobs = self.spec.max_blobs_per_block_within_fork(fork_name) as usize; + let max_blobs = max(1, self.min_blobs_count); let num_blobs = rng.random_range(self.min_blobs_count..=max_blobs); let (bundle, transactions) = generate_blobs(num_blobs, fork_name)?; for tx in Vec::from(transactions) { @@ -977,7 +974,6 @@ 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), @@ -988,7 +984,6 @@ 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 9e587d4e590..73c998956ca 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 @@ -63,7 +63,6 @@ impl MockExecutionLayer { prague_time, osaka_time, amsterdam_time, - spec.clone(), kzg, ); diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index 712c773dda0..8f129715606 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -22,7 +22,7 @@ use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use std::sync::{Arc, LazyLock}; use tokio::{runtime, sync::oneshot}; use tracing::info; -use types::{ChainSpec, EthSpec, ExecutionBlockHash, Uint256}; +use types::{EthSpec, ExecutionBlockHash, Uint256}; use warp::{Filter, Rejection, http::StatusCode}; use crate::EngineCapabilities; @@ -114,7 +114,7 @@ pub struct MockServer { } impl MockServer { - pub fn unit_testing(chain_spec: Arc) -> Self { + pub fn unit_testing() -> Self { Self::new( &runtime::Handle::current(), JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap(), @@ -126,7 +126,6 @@ impl MockServer { None, // FIXME(electra): should this be the default? None, // FIXME(fulu): should this be the default? None, // FIXME(gloas): should this be the default? - chain_spec, None, ) } @@ -134,7 +133,6 @@ impl MockServer { pub fn new_with_config( handle: &runtime::Handle, config: MockExecutionConfig, - spec: Arc, kzg: Option>, ) -> Self { create_test_tracing_subscriber(); @@ -161,7 +159,6 @@ impl MockServer { prague_time, osaka_time, amsterdam_time, - spec, kzg, ); @@ -226,7 +223,6 @@ impl MockServer { prague_time: Option, osaka_time: Option, amsterdam_time: Option, - spec: Arc, kzg: Option>, ) -> Self { Self::new_with_config( @@ -243,7 +239,6 @@ impl MockServer { osaka_time, amsterdam_time, }, - spec, kzg, ) } diff --git a/beacon_node/http_api/tests/broadcast_validation_tests.rs b/beacon_node/http_api/tests/broadcast_validation_tests.rs index 9427f6fdf35..1b79c13d76f 100644 --- a/beacon_node/http_api/tests/broadcast_validation_tests.rs +++ b/beacon_node/http_api/tests/broadcast_validation_tests.rs @@ -822,6 +822,14 @@ pub async fn blinded_gossip_invalid() { tester.harness.advance_slot(); + // Ensure there's at least one blob in the block, so we don't run into failures when the + // block generator logic changes, as different errors could be returned: + // * Invalidity of blocks: `NotFinalizedDescendant` + // * Invalidity of blobs: `ParentUnknown` + tester + .harness + .execution_block_generator() + .set_min_blob_count(1); let (blinded_block, _) = tester .harness .make_blinded_block_with_modifier(chain_state_before, slot, |b| { @@ -837,21 +845,20 @@ pub async fn blinded_gossip_invalid() { assert!(response.is_err()); let error_response: eth2::Error = response.err().unwrap(); + assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); + let pre_finalized_block_root = Hash256::zero(); - /* mandated by Beacon API spec */ - if tester.harness.spec.is_fulu_scheduled() { - // XXX: this should be a 400 but is a 500 due to the mock-builder being janky - assert_eq!( - error_response.status(), - Some(StatusCode::INTERNAL_SERVER_ERROR) - ); + let expected_error_msg = if tester.harness.spec.is_fulu_scheduled() { + format!( + "BAD_REQUEST: NotFinalizedDescendant {{ block_parent_root: {pre_finalized_block_root:?} }}" + ) } else { - assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error( - error_response, - format!("BAD_REQUEST: ParentUnknown {{ parent_root: {pre_finalized_block_root:?} }}"), - ); - } + // Since Deneb, the invalidity of the blobs will be detected prior to the invalidity of the + // block. + format!("BAD_REQUEST: ParentUnknown {{ parent_root: {pre_finalized_block_root:?} }}") + }; + + assert_server_message_error(error_response, expected_error_msg); } /// Process a blinded block that is invalid, but valid on gossip. @@ -1647,6 +1654,10 @@ pub async fn block_seen_on_gossip_with_some_blobs_or_columns() { ) .await; tester.harness.advance_slot(); + tester + .harness + .execution_block_generator() + .set_min_blob_count(2); let slot_a = Slot::new(num_initial); let slot_b = slot_a + 1; diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index dc2fd4ae440..3791e0090a4 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -178,6 +178,9 @@ impl ApiTester { "precondition: current slot is one after head" ); + // Set a min blob count for the next block for get_blobs testing + harness.execution_block_generator().set_min_blob_count(2); + let (next_block, _next_state) = harness .make_block(head.beacon_state.clone(), harness.chain.slot().unwrap()) .await; @@ -1855,7 +1858,7 @@ impl ApiTester { } pub async fn test_get_blob_sidecars(self, use_indices: bool) -> Self { - let block_id = BlockId(CoreBlockId::Finalized); + let block_id = BlockId(CoreBlockId::Head); let (block_root, _, _) = block_id.root(&self.chain).unwrap(); let (block, _, _) = block_id.full_block(&self.chain).await.unwrap(); let num_blobs = block.num_expected_blobs(); @@ -1888,7 +1891,7 @@ impl ApiTester { } pub async fn test_get_blobs(self, versioned_hashes: bool) -> Self { - let block_id = BlockId(CoreBlockId::Finalized); + let block_id = BlockId(CoreBlockId::Head); let (block_root, _, _) = block_id.root(&self.chain).unwrap(); let (block, _, _) = block_id.full_block(&self.chain).await.unwrap(); let num_blobs = block.num_expected_blobs(); @@ -1926,7 +1929,7 @@ impl ApiTester { } pub async fn test_get_blobs_post_fulu_full_node(self, versioned_hashes: bool) -> Self { - let block_id = BlockId(CoreBlockId::Finalized); + let block_id = BlockId(CoreBlockId::Head); let (block_root, _, _) = block_id.root(&self.chain).unwrap(); let (block, _, _) = block_id.full_block(&self.chain).await.unwrap(); @@ -7853,6 +7856,8 @@ async fn get_blobs_post_fulu_supernode() { config.spec.fulu_fork_epoch = Some(Epoch::new(0)); ApiTester::new_from_config(config) + .await + .test_post_beacon_blocks_valid() .await // We can call the same get_blobs function in this test // because the function will call get_blobs_by_versioned_hashes which handles peerDAS post-Fulu @@ -7873,6 +7878,8 @@ async fn get_blobs_post_fulu_full_node() { config.spec.fulu_fork_epoch = Some(Epoch::new(0)); ApiTester::new_from_config(config) + .await + .test_post_beacon_blocks_valid() .await .test_get_blobs_post_fulu_full_node(false) .await diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index d5858c23f11..01929cbf906 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -517,11 +517,10 @@ mod tests { #[test] fn no_blobs_into_responses() { - let spec = test_spec::(); let mut rng = XorShiftRng::from_seed([42; 16]); let blocks = (0..4) .map(|_| { - generate_rand_block_and_blobs::(ForkName::Base, NumBlobs::None, &mut rng, &spec) + generate_rand_block_and_blobs::(ForkName::Base, NumBlobs::None, &mut rng) .0 .into() }) @@ -540,19 +539,13 @@ mod tests { #[test] fn empty_blobs_into_responses() { - let spec = test_spec::(); let mut rng = XorShiftRng::from_seed([42; 16]); let blocks = (0..4) .map(|_| { // Always generate some blobs. - generate_rand_block_and_blobs::( - ForkName::Deneb, - NumBlobs::Number(3), - &mut rng, - &spec, - ) - .0 - .into() + generate_rand_block_and_blobs::(ForkName::Deneb, NumBlobs::Number(3), &mut rng) + .0 + .into() }) .collect::>>>(); diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index fc641861754..63bcd176f52 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -194,7 +194,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, &self.spec) + generate_rand_block_and_blobs::(fork_name, num_blobs, rng) } fn rand_block_and_data_columns( @@ -1146,10 +1146,8 @@ 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, &spec); + let (block, _) = generate_rand_block_and_blobs::(ForkName::Base, NumBlobs::None, &mut rng); assert_eq!( block.canonical_root(), Hash256::from_slice( diff --git a/consensus/types/src/test_utils/generate_random_block_and_blobs.rs b/consensus/types/src/test_utils/generate_random_block_and_blobs.rs index 0f52e485a8a..8f4908291ee 100644 --- a/consensus/types/src/test_utils/generate_random_block_and_blobs.rs +++ b/consensus/types/src/test_utils/generate_random_block_and_blobs.rs @@ -77,7 +77,7 @@ mod test { #[test] fn test_verify_blob_inclusion_proof() { let (_block, blobs) = - generate_rand_block_and_blobs::(ForkName::Deneb, 6, &mut rng()); + generate_rand_block_and_blobs::(ForkName::Deneb, 2, &mut rng()); for blob in blobs { assert!(blob.verify_blob_sidecar_inclusion_proof()); } @@ -115,7 +115,7 @@ mod test { #[test] fn test_verify_blob_inclusion_proof_invalid() { let (_block, blobs) = - generate_rand_block_and_blobs::(ForkName::Deneb, 6, &mut rng()); + generate_rand_block_and_blobs::(ForkName::Deneb, 1, &mut rng()); for mut blob in blobs { blob.kzg_commitment_inclusion_proof = FixedVector::random_for_test(&mut rng()); diff --git a/lcli/src/mock_el.rs b/lcli/src/mock_el.rs index ee6485b2388..d6bdfb0d712 100644 --- a/lcli/src/mock_el.rs +++ b/lcli/src/mock_el.rs @@ -44,7 +44,7 @@ pub fn run(mut env: Environment, matches: &ArgMatches) -> Result< amsterdam_time, }; let kzg = None; - let server: MockServer = MockServer::new_with_config(&handle, config, spec, kzg); + let server: MockServer = MockServer::new_with_config(&handle, config, 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 df191ed5af7..e49d11ee1eb 100644 --- a/testing/node_test_rig/src/lib.rs +++ b/testing/node_test_rig/src/lib.rs @@ -248,14 +248,8 @@ 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 = context.eth2_config.spec.clone(); Self { - server: MockServer::new_with_config( - &context.executor.handle().unwrap(), - config, - spec, - None, - ), + server: MockServer::new_with_config(&context.executor.handle().unwrap(), config, None), datadir, } }