diff --git a/Cargo.lock b/Cargo.lock index 7683e67624..5a63ab1e72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6081,6 +6081,7 @@ dependencies = [ "metrics", "operation_pool", "parking_lot", + "paste", "rand 0.8.5", "rand 0.9.2", "rand_chacha 0.3.1", diff --git a/Makefile b/Makefile index 9e2b1d24c5..0995a869f4 100644 --- a/Makefile +++ b/Makefile @@ -36,8 +36,12 @@ PROFILE ?= release RECENT_FORKS_BEFORE_GLOAS=electra fulu # List of all recent hard forks. This list is used to set env variables for http_api tests +# Include phase0 to test the code paths in sync that are pre blobs RECENT_FORKS=electra fulu gloas +# For network tests include phase0 to cover genesis syncing (blocks without blobs or columns) +TEST_NETWORK_FORKS=phase0 $(RECENT_FORKS_BEFORE_GLOAS) + # Extra flags for Cargo CARGO_INSTALL_EXTRA_FLAGS?= @@ -226,12 +230,15 @@ test-op-pool-%: # Run the tests in the `network` crate for all known forks. # TODO(EIP-7732) Extend to support gloas by using RECENT_FORKS instead -test-network: $(patsubst %,test-network-%,$(RECENT_FORKS_BEFORE_GLOAS)) +test-network: $(patsubst %,test-network-%,$(TEST_NETWORK_FORKS)) test-network-%: - env FORK_NAME=$* cargo nextest run --release \ - --features "fork_from_env,$(TEST_FEATURES)" \ + env FORK_NAME=$* cargo nextest run --no-fail-fast --release \ + --features "fork_from_env,fake_crypto,$(TEST_FEATURES)" \ -p network + env FORK_NAME=$* cargo nextest run --no-fail-fast --release \ + --features "fork_from_env,$(TEST_FEATURES)" \ + -p network crypto_on # Run the tests in the `slasher` crate for all supported database backends. test-slasher: diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index 6a028e6c98..f98cd40d08 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -287,21 +287,6 @@ pub struct BlockImportData { pub consensus_context: ConsensusContext, } -impl BlockImportData { - pub fn __new_for_test( - block_root: Hash256, - state: BeaconState, - parent_block: SignedBeaconBlock>, - ) -> Self { - Self { - block_root, - state, - parent_block, - consensus_context: ConsensusContext::new(Slot::new(0)), - } - } -} - /// Trait for common block operations. pub trait AsBlock { fn slot(&self) -> Slot; 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 f7bd646f82..7260a4aca0 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 @@ -698,6 +698,8 @@ impl DataAvailabilityCheckerInner { pub fn remove_pre_execution_block(&self, block_root: &Hash256) { // The read lock is immediately dropped so we can safely remove the block from the cache. if let Some(BlockProcessStatus::NotValidated(_, _)) = self.get_cached_block(block_root) { + // If the block is execution invalid, this status is permanent and idempotent to this + // block_root. We drop its components (e.g. columns) because they will never be useful. self.critical.write().pop(block_root); } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index f816dbac53..096a0516fc 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -818,7 +818,11 @@ where } pub fn get_full_block(&self, block_root: &Hash256) -> RpcBlock { - let block = self.chain.get_blinded_block(block_root).unwrap().unwrap(); + let block = self + .chain + .get_blinded_block(block_root) + .unwrap() + .unwrap_or_else(|| panic!("block root does not exist in harness {block_root:?}")); let full_block = self.chain.store.make_full_block(block_root, block).unwrap(); self.build_rpc_block_from_store_blobs(Some(*block_root), Arc::new(full_block)) } diff --git a/beacon_node/beacon_processor/src/lib.rs b/beacon_node/beacon_processor/src/lib.rs index d3e9133542..33a00bfa49 100644 --- a/beacon_node/beacon_processor/src/lib.rs +++ b/beacon_node/beacon_processor/src/lib.rs @@ -243,12 +243,15 @@ impl From for WorkEvent { }, }, ReadyWork::RpcBlock(QueuedRpcBlock { - beacon_block_root: _, + beacon_block_root, process_fn, ignore_fn: _, }) => Self { drop_during_sync: false, - work: Work::RpcBlock { process_fn }, + work: Work::RpcBlock { + process_fn, + beacon_block_root, + }, }, ReadyWork::IgnoredRpcBlock(IgnoredRpcBlock { process_fn }) => Self { drop_during_sync: false, @@ -389,6 +392,7 @@ pub enum Work { GossipLightClientFinalityUpdate(BlockingFn), GossipLightClientOptimisticUpdate(BlockingFn), RpcBlock { + beacon_block_root: Hash256, process_fn: AsyncFn, }, RpcBlobs { @@ -479,7 +483,7 @@ pub enum WorkType { } impl Work { - fn str_id(&self) -> &'static str { + pub fn str_id(&self) -> &'static str { self.to_type().into() } @@ -1432,7 +1436,10 @@ impl BeaconProcessor { beacon_block_root: _, process_fn, } => task_spawner.spawn_async(process_fn), - Work::RpcBlock { process_fn } + Work::RpcBlock { + process_fn, + beacon_block_root: _, + } | Work::RpcBlobs { process_fn } | Work::RpcCustodyColumn(process_fn) | Work::ColumnReconstruction(process_fn) => task_spawner.spawn_async(process_fn), 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 6b247a4cd4..8591359f15 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 @@ -18,6 +18,7 @@ use ssz_types::VariableList; use std::cmp::max; use std::collections::HashMap; use std::sync::Arc; +use tracing::warn; use tree_hash::TreeHash; use tree_hash_derive::TreeHash; use types::{ @@ -537,6 +538,21 @@ impl ExecutionBlockGenerator { .contains_key(&forkchoice_state.finalized_block_hash); if unknown_head_block_hash || unknown_safe_block_hash || unknown_finalized_block_hash { + if unknown_head_block_hash { + warn!(?head_block_hash, "Received unknown head block hash"); + } + if unknown_safe_block_hash { + warn!( + safe_block_hash = ?forkchoice_state.safe_block_hash, + "Received unknown safe block hash" + ); + } + if unknown_finalized_block_hash { + warn!( + finalized_block_hash = ?forkchoice_state.finalized_block_hash, + "Received unknown finalized block hash" + ) + } return Ok(JsonForkchoiceUpdatedV1Response { payload_status: JsonPayloadStatusV1 { status: JsonPayloadStatusV1Status::Syncing, diff --git a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs index 2168ed8961..53eb3b5166 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -5,6 +5,7 @@ use crate::test_utils::{DEFAULT_CLIENT_VERSION, DEFAULT_MOCK_EL_PAYLOAD_VALUE_WE use serde::{Deserialize, de::DeserializeOwned}; use serde_json::Value as JsonValue; use std::sync::Arc; +use tracing::debug; pub const GENERIC_ERROR_CODE: i64 = -1234; pub const BAD_PARAMS_ERROR_CODE: i64 = -32602; @@ -28,6 +29,8 @@ pub async fn handle_rpc( .ok_or_else(|| "missing/invalid params field".to_string()) .map_err(|s| (s, GENERIC_ERROR_CODE))?; + debug!(method, "Mock execution engine"); + match method { ETH_SYNCING => ctx .syncing_response @@ -517,6 +520,12 @@ pub async fn handle_rpc( _ => unreachable!(), }; + debug!( + ?payload_attributes, + ?forkchoice_state, + "ENGINE_FORKCHOICE_UPDATED" + ); + // validate method called correctly according to fork time if let Some(pa) = payload_attributes.as_ref() { match ctx diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 34d8efccd1..b75ca72eda 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -731,7 +731,7 @@ where } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, IntoStaticStr)] pub enum RequestType { Status(StatusMessage), Goodbye(GoodbyeReason), diff --git a/beacon_node/lighthouse_network/src/service/api_types.rs b/beacon_node/lighthouse_network/src/service/api_types.rs index f1a4d87de7..d0323bab52 100644 --- a/beacon_node/lighthouse_network/src/service/api_types.rs +++ b/beacon_node/lighthouse_network/src/service/api_types.rs @@ -135,7 +135,7 @@ pub struct CustodyId { pub struct CustodyRequester(pub SingleLookupReqId); /// Application level requests sent to the network. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub enum AppRequestId { Sync(SyncRequestId), Router, diff --git a/beacon_node/network/Cargo.toml b/beacon_node/network/Cargo.toml index 78dc0c48a7..68c77252ab 100644 --- a/beacon_node/network/Cargo.toml +++ b/beacon_node/network/Cargo.toml @@ -8,6 +8,7 @@ edition = { workspace = true } # NOTE: This can be run via cargo build --bin lighthouse --features network/disable-backfill disable-backfill = [] fork_from_env = ["beacon_chain/fork_from_env"] +fake_crypto = ["bls/fake_crypto", "kzg/fake_crypto"] portable = ["beacon_chain/portable"] test_logger = [] @@ -57,6 +58,7 @@ k256 = "0.13.4" kzg = { workspace = true } libp2p = { workspace = true } matches = "0.1.8" +paste = { workspace = true } rand_08 = { package = "rand", version = "0.8.5" } rand_chacha = "0.9.0" rand_chacha_03 = { package = "rand_chacha", version = "0.3.1" } diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index fd67fcde82..e1adf860de 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -526,7 +526,10 @@ impl NetworkBeaconProcessor { ); self.try_send(BeaconWorkEvent { drop_during_sync: false, - work: Work::RpcBlock { process_fn }, + work: Work::RpcBlock { + process_fn, + beacon_block_root: block_root, + }, }) } diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index a6b3ea9e4b..629a42c688 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -219,7 +219,7 @@ impl NetworkBeaconProcessor { // to be sent from the peers if we already have them. let publish_blobs = false; self.fetch_engine_blobs_and_publish(signed_beacon_block, block_root, publish_blobs) - .await + .await; } _ => {} } diff --git a/beacon_node/network/src/network_beacon_processor/tests.rs b/beacon_node/network/src/network_beacon_processor/tests.rs index 49b1c0c262..aa03ee931d 100644 --- a/beacon_node/network/src/network_beacon_processor/tests.rs +++ b/beacon_node/network/src/network_beacon_processor/tests.rs @@ -940,20 +940,20 @@ async fn data_column_reconstruction_at_deadline() { .set_current_time(slot_start + Duration::from_millis(reconstruction_deadline_millis)); let min_columns_for_reconstruction = E::number_of_columns() / 2; + + // Enqueue all columns first - at deadline, reconstruction races with gossip drain for i in 0..min_columns_for_reconstruction { rig.enqueue_gossip_data_columns(i); - rig.assert_event_journal_completes(&[WorkType::GossipDataColumnSidecar]) - .await; } - // Since we're at the reconstruction deadline, reconstruction should be triggered immediately - rig.assert_event_journal_with_timeout( - &[WorkType::ColumnReconstruction.into()], - Duration::from_millis(50), - false, - false, - ) - .await; + // Expect all gossip events + reconstruction + let mut expected_events: Vec = (0..min_columns_for_reconstruction) + .map(|_| WorkType::GossipDataColumnSidecar) + .collect(); + expected_events.push(WorkType::ColumnReconstruction); + + rig.assert_event_journal_contains_ordered(&expected_events) + .await; } // Test the column reconstruction is delayed for columns that arrive for a previous slot. diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index 9065f05753..cbf65505ef 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -121,15 +121,24 @@ pub struct BlockLookups { // TODO: Why not index lookups by block_root? single_block_lookups: FnvHashMap>, + + /// Used for testing assertions + metrics: BlockLookupsMetrics, } #[cfg(test)] use lighthouse_network::service::api_types::Id; #[cfg(test)] -/// Tuple of `SingleLookupId`, requested block root, awaiting parent block root (if any), -/// and list of peers that claim to have imported this set of block components. -pub(crate) type BlockLookupSummary = (Id, Hash256, Option, Vec); +#[derive(Debug)] +pub(crate) struct BlockLookupSummary { + /// Lookup ID + pub id: Id, + /// Requested block root + pub block_root: Hash256, + /// List of peers that claim to have imported this set of block components. + pub peers: Vec, +} impl BlockLookups { pub fn new() -> Self { @@ -138,9 +147,15 @@ impl BlockLookups { IGNORED_CHAINS_CACHE_EXPIRY_SECONDS, )), single_block_lookups: Default::default(), + metrics: <_>::default(), } } + #[cfg(test)] + pub(crate) fn metrics(&self) -> &BlockLookupsMetrics { + &self.metrics + } + #[cfg(test)] pub(crate) fn insert_ignored_chain(&mut self, block_root: Hash256) { self.ignored_chains.insert(block_root); @@ -155,7 +170,11 @@ impl BlockLookups { pub(crate) fn active_single_lookups(&self) -> Vec { self.single_block_lookups .iter() - .map(|(id, l)| (*id, l.block_root(), l.awaiting_parent(), l.all_peers())) + .map(|(id, l)| BlockLookupSummary { + id: *id, + block_root: l.block_root(), + peers: l.all_peers(), + }) .collect() } @@ -306,7 +325,7 @@ impl BlockLookups { // attributability. A peer can send us garbage blocks over blocks_by_root, and // then correct blocks via blocks_by_range. - self.drop_lookup_and_children(*lookup_id); + self.drop_lookup_and_children(*lookup_id, "chain_too_long"); } else { // Should never happen error!( @@ -414,6 +433,7 @@ impl BlockLookups { "Created block lookup" ); metrics::inc_counter(&metrics::SYNC_LOOKUP_CREATED); + self.metrics.created_lookups += 1; let result = lookup.continue_requests(cx); if self.on_lookup_result(id, result, "new_current_lookup", cx) { @@ -513,8 +533,11 @@ impl BlockLookups { /* Error responses */ pub fn peer_disconnected(&mut self, peer_id: &PeerId) { - for (_, lookup) in self.single_block_lookups.iter_mut() { + for (id, lookup) in self.single_block_lookups.iter_mut() { lookup.remove_peer(peer_id); + if lookup.has_no_peers() { + debug!(%id, "Lookup has no peers"); + } } } @@ -566,7 +589,8 @@ impl BlockLookups { let action = match result { BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(_)) - | BlockProcessingResult::Err(BlockError::DuplicateFullyImported(..)) => { + | BlockProcessingResult::Err(BlockError::DuplicateFullyImported(..)) + | BlockProcessingResult::Err(BlockError::GenesisBlock) => { // Successfully imported request_state.on_processing_success()?; Action::Continue @@ -747,6 +771,15 @@ impl BlockLookups { let lookup_result = if imported { Ok(LookupResult::Completed) } else { + // A lookup may be in the following state: + // - Block awaiting processing from a different source + // - Blobs downloaded processed, and inserted into the da_checker + // + // At this point the block fails processing (e.g. execution engine offline) and it is + // removed from the da_checker. Note that ALL components are removed from the da_checker + // so when we re-download and process the block we get the error + // MissingComponentsAfterAllProcessed and get stuck. + lookup.reset_requests(); lookup.continue_requests(cx) }; let id = *id; @@ -779,14 +812,17 @@ impl BlockLookups { /// Drops `dropped_id` lookup and all its children recursively. Lookups awaiting a parent need /// the parent to make progress to resolve, therefore we must drop them if the parent is /// dropped. - pub fn drop_lookup_and_children(&mut self, dropped_id: SingleLookupId) { + pub fn drop_lookup_and_children(&mut self, dropped_id: SingleLookupId, reason: &'static str) { if let Some(dropped_lookup) = self.single_block_lookups.remove(&dropped_id) { debug!( id = ?dropped_id, block_root = ?dropped_lookup.block_root(), awaiting_parent = ?dropped_lookup.awaiting_parent(), + reason, "Dropping lookup" ); + metrics::inc_counter_vec(&metrics::SYNC_LOOKUP_DROPPED, &[reason]); + self.metrics.dropped_lookups += 1; let child_lookups = self .single_block_lookups @@ -796,7 +832,7 @@ impl BlockLookups { .collect::>(); for id in child_lookups { - self.drop_lookup_and_children(id); + self.drop_lookup_and_children(id, reason); } } } @@ -814,8 +850,13 @@ impl BlockLookups { Ok(LookupResult::Pending) => true, // no action Ok(LookupResult::Completed) => { if let Some(lookup) = self.single_block_lookups.remove(&id) { - debug!(block = ?lookup.block_root(), id, "Dropping completed lookup"); + debug!( + block = ?lookup.block_root(), + id, + "Dropping completed lookup" + ); metrics::inc_counter(&metrics::SYNC_LOOKUP_COMPLETED); + self.metrics.completed_lookups += 1; // Block imported, continue the requests of pending child blocks self.continue_child_lookups(lookup.block_root(), cx); self.update_metrics(); @@ -829,8 +870,7 @@ impl BlockLookups { Err(LookupRequestError::UnknownLookup) => false, Err(error) => { debug!(id, source, ?error, "Dropping lookup on request error"); - metrics::inc_counter_vec(&metrics::SYNC_LOOKUP_DROPPED, &[error.into()]); - self.drop_lookup_and_children(id); + self.drop_lookup_and_children(id, error.into()); self.update_metrics(); false } @@ -897,7 +937,7 @@ impl BlockLookups { %block_root, "Dropping lookup with no peers" ); - self.drop_lookup_and_children(lookup_id); + self.drop_lookup_and_children(lookup_id, "no_peers"); } } @@ -946,7 +986,7 @@ impl BlockLookups { } metrics::inc_counter(&metrics::SYNC_LOOKUPS_STUCK); - self.drop_lookup_and_children(ancestor_stuck_lookup.id); + self.drop_lookup_and_children(ancestor_stuck_lookup.id, "lookup_stuck"); } } @@ -1022,3 +1062,10 @@ impl BlockLookups { } } } + +#[derive(Default, Clone, Debug)] +pub(crate) struct BlockLookupsMetrics { + pub created_lookups: usize, + pub dropped_lookups: usize, + pub completed_lookups: usize, +} diff --git a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs index 43bfe29a84..919526c238 100644 --- a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs @@ -109,6 +109,12 @@ impl SingleBlockLookup { } } + /// Reset the status of all internal requests + pub fn reset_requests(&mut self) { + self.block_request_state = BlockRequestState::new(self.block_root); + self.component_requests = ComponentRequests::WaitingForBlock; + } + /// Return the slot of this lookup's block if it's currently cached as `AwaitingProcessing` pub fn peek_downloaded_block_slot(&self) -> Option { self.block_request_state diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 096ed9c328..c1ab6221dd 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -70,6 +70,7 @@ use slot_clock::SlotClock; use std::ops::Sub; use std::sync::Arc; use std::time::Duration; +use strum::IntoStaticStr; use tokio::sync::mpsc; use tracing::{debug, error, info, trace}; use types::{ @@ -90,7 +91,7 @@ pub const SLOT_IMPORT_TOLERANCE: usize = 32; /// arbitrary number that covers a full slot, but allows recovery if sync get stuck for a few slots. const NOTIFIED_UNKNOWN_ROOT_EXPIRY_SECONDS: u64 = 30; -#[derive(Debug)] +#[derive(Debug, IntoStaticStr)] /// A message that can be sent to the sync manager thread. pub enum SyncMessage { /// A useful peer has been discovered. @@ -323,17 +324,18 @@ impl SyncManager { } #[cfg(test)] - pub(crate) fn active_single_lookups(&self) -> Vec { - self.block_lookups.active_single_lookups() + pub(crate) fn send_sync_message(&mut self, sync_message: SyncMessage<::EthSpec>) { + self.network.send_sync_message(sync_message); } #[cfg(test)] - pub(crate) fn active_parent_lookups(&self) -> Vec> { - self.block_lookups - .active_parent_lookups() - .iter() - .map(|c| c.chain.clone()) - .collect() + pub(crate) fn block_lookups(&self) -> &BlockLookups { + &self.block_lookups + } + + #[cfg(test)] + pub(crate) fn range_sync(&self) -> &RangeSync { + &self.range_sync } #[cfg(test)] @@ -512,17 +514,18 @@ impl SyncManager { /// there is no way to guarantee that libp2p always emits a error along with /// the disconnect. fn peer_disconnect(&mut self, peer_id: &PeerId) { - // Inject a Disconnected error on all requests associated with the disconnected peer - // to retry all batches/lookups - for sync_request_id in self.network.peer_disconnected(peer_id) { - self.inject_error(*peer_id, sync_request_id, RPCError::Disconnected); - } - // Remove peer from all data structures self.range_sync.peer_disconnect(&mut self.network, peer_id); let _ = self.backfill_sync.peer_disconnected(peer_id); self.block_lookups.peer_disconnected(peer_id); + // Inject a Disconnected error on all requests associated with the disconnected peer + // to retry all batches/lookups. Only after removing the peer from the data structures to + // avoid sending retry requests to the disconnecting peer. + for sync_request_id in self.network.peer_disconnected(peer_id) { + self.inject_error(*peer_id, sync_request_id, RPCError::Disconnected); + } + // Regardless of the outcome, we update the sync status. self.update_sync_state(); } diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 542625b8a3..7e2c0d9a94 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -17,7 +17,7 @@ use crate::sync::block_lookups::SingleLookupId; use crate::sync::block_sidecar_coupling::CouplingError; use crate::sync::network_context::requests::BlobsByRootSingleBlockRequest; use crate::sync::range_data_column_batch_request::RangeDataColumnBatchRequest; -use beacon_chain::block_verification_types::RpcBlock; +use beacon_chain::block_verification_types::{AsBlock, RpcBlock}; use beacon_chain::{BeaconChain, BeaconChainTypes, BlockProcessStatus, EngineState}; use custody::CustodyRequestResult; use fnv::FnvHashMap; @@ -1095,13 +1095,14 @@ impl SyncNetworkContext { })?; // Include only the blob indexes not yet imported (received through gossip) - let custody_indexes_to_fetch = self + let mut custody_indexes_to_fetch = self .chain .sampling_columns_for_epoch(current_epoch) .iter() .copied() .filter(|index| !custody_indexes_imported.contains(index)) .collect::>(); + custody_indexes_to_fetch.sort_unstable(); if custody_indexes_to_fetch.is_empty() { // No indexes required, do not issue any request @@ -1595,7 +1596,7 @@ impl SyncNetworkContext { ) .map_err(|_| SendErrorProcessor::SendError)?; - debug!(block = ?block_root, id, "Sending block for processing"); + debug!(block = ?block_root, block_slot = %block.slot(), id, "Sending block for processing"); // Lookup sync event safety: If `beacon_processor.send_rpc_beacon_block` returns Ok() sync // must receive a single `SyncMessage::BlockComponentProcessed` with this process type beacon_processor diff --git a/beacon_node/network/src/sync/network_context/custody.rs b/beacon_node/network/src/sync/network_context/custody.rs index 61ae95ee70..de5d9b6e0b 100644 --- a/beacon_node/network/src/sync/network_context/custody.rs +++ b/beacon_node/network/src/sync/network_context/custody.rs @@ -198,7 +198,14 @@ impl ActiveCustodyRequest { cx: &mut SyncNetworkContext, ) -> CustodyRequestResult { let _guard = self.span.clone().entered(); - if self.column_requests.values().all(|r| r.is_downloaded()) { + let total_requests = self.column_requests.len(); + let completed_requests = self + .column_requests + .values() + .filter(|r| r.is_downloaded()) + .count(); + + if completed_requests >= total_requests { // All requests have completed successfully. let mut peers = HashMap::>::new(); let mut seen_timestamps = vec![]; @@ -222,6 +229,7 @@ impl ActiveCustodyRequest { let active_request_count_by_peer = cx.active_request_count_by_peer(); let mut columns_to_request_by_peer = HashMap::>::new(); + let mut columns_without_peers = vec![]; let lookup_peers = self.lookup_peers.read(); // Create deterministic hasher per request to ensure consistent peer ordering within // this request (avoiding fragmentation) while varying selection across different requests @@ -256,6 +264,7 @@ impl ActiveCustodyRequest { return Err(Error::NoPeer(*column_index)); } else { // Do not issue requests if there is no custody peer on this column + columns_without_peers.push(*column_index); } } } @@ -270,10 +279,13 @@ impl ActiveCustodyRequest { lookup_peers = lookup_peers.len(), "Requesting {} columns from {} peers", columns_requested_count, peer_requests, ); - } else { + } else if !columns_without_peers.is_empty() { debug!( lookup_peers = lookup_peers.len(), - "No column peers found for look up", + total_requests, + completed_requests, + ?columns_without_peers, + "No column peers found for lookup", ); } @@ -288,7 +300,7 @@ impl ActiveCustodyRequest { }, // If peer is in the lookup peer set, it claims to have imported the block and // must have its columns in custody. In that case, set `true = enforce max_requests` - // and downscore if data_columns_by_root does not returned the expected custody + // and downscore if data_columns_by_root does not return the expected custody // columns. For the rest of peers, don't downscore if columns are missing. lookup_peers.contains(&peer_id), ) diff --git a/beacon_node/network/src/sync/range_sync/chain_collection.rs b/beacon_node/network/src/sync/range_sync/chain_collection.rs index 1d57ee6c3d..b91b88b55c 100644 --- a/beacon_node/network/src/sync/range_sync/chain_collection.rs +++ b/beacon_node/network/src/sync/range_sync/chain_collection.rs @@ -41,6 +41,13 @@ pub enum RangeSyncState { pub type SyncChainStatus = Result, &'static str>; +#[cfg(test)] +#[derive(Default, Debug)] +pub struct ChainCollectionMetrics { + pub chains_added: usize, + pub chains_removed: usize, +} + /// A collection of finalized and head chains currently being processed. pub struct ChainCollection { /// The beacon chain for processing. @@ -51,6 +58,9 @@ pub struct ChainCollection { head_chains: FnvHashMap>, /// The current sync state of the process. state: RangeSyncState, + #[cfg(test)] + /// Used for testing assertions + metrics: ChainCollectionMetrics, } impl ChainCollection { @@ -60,12 +70,23 @@ impl ChainCollection { finalized_chains: FnvHashMap::default(), head_chains: FnvHashMap::default(), state: RangeSyncState::Idle, + #[cfg(test)] + metrics: <_>::default(), } } + #[cfg(test)] + pub(crate) fn metrics(&self) -> &ChainCollectionMetrics { + &self.metrics + } + /// Updates the Syncing state of the collection after a chain is removed. fn on_chain_removed(&mut self, id: &ChainId, was_syncing: bool, sync_type: RangeSyncType) { metrics::inc_counter_vec(&metrics::SYNCING_CHAINS_REMOVED, &[sync_type.as_str()]); + #[cfg(test)] + { + self.metrics.chains_removed += 1; + } self.update_metrics(); match self.state { @@ -510,6 +531,10 @@ impl ChainCollection { ); collection.insert(id, new_chain); metrics::inc_counter_vec(&metrics::SYNCING_CHAINS_ADDED, &[sync_type.as_str()]); + #[cfg(test)] + { + self.metrics.chains_added += 1; + } self.update_metrics(); } } diff --git a/beacon_node/network/src/sync/range_sync/range.rs b/beacon_node/network/src/sync/range_sync/range.rs index c9656ad1d0..86625444be 100644 --- a/beacon_node/network/src/sync/range_sync/range.rs +++ b/beacon_node/network/src/sync/range_sync/range.rs @@ -98,6 +98,11 @@ where self.failed_chains.keys().copied().collect() } + #[cfg(test)] + pub(crate) fn metrics(&self) -> &super::chain_collection::ChainCollectionMetrics { + self.chains.metrics() + } + pub fn state(&self) -> SyncChainStatus { self.chains.state() } diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index b6e96737d6..769a11d976 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -1,79 +1,171 @@ +use super::*; use crate::NetworkMessage; -use crate::network_beacon_processor::NetworkBeaconProcessor; -use crate::sync::block_lookups::{ - BlockLookupSummary, PARENT_DEPTH_TOLERANCE, SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS, -}; +use crate::network_beacon_processor::{InvalidBlockStorage, NetworkBeaconProcessor}; +use crate::sync::block_lookups::{BlockLookupSummary, PARENT_DEPTH_TOLERANCE}; use crate::sync::{ SyncMessage, manager::{BlockProcessType, BlockProcessingResult, SyncManager}, }; -use std::sync::Arc; -use std::time::Duration; - -use super::*; - -use crate::sync::block_lookups::common::ResponseType; -use beacon_chain::observed_data_sidecars::Observe; +use beacon_chain::blob_verification::KzgVerifiedBlob; +use beacon_chain::custody_context::NodeCustodyType; use beacon_chain::{ - AvailabilityPendingExecutedBlock, AvailabilityProcessingStatus, BlockError, - PayloadVerificationOutcome, PayloadVerificationStatus, - blob_verification::GossipVerifiedBlob, - block_verification_types::{AsBlock, BlockImportData}, - custody_context::NodeCustodyType, + AvailabilityProcessingStatus, BlockError, NotifyExecutionLayer, + block_verification_types::{AsBlock, AvailableBlockData}, data_availability_checker::Availability, test_utils::{ - BeaconChainHarness, EphemeralHarnessType, NumBlobs, generate_rand_block_and_blobs, - generate_rand_block_and_data_columns, test_spec, + AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType, NumBlobs, + generate_rand_block_and_blobs, test_spec, }, - validator_monitor::timestamp_now, }; -use beacon_processor::WorkEvent; +use beacon_processor::{BeaconProcessorChannels, DuplicateCache, Work, WorkEvent}; +use educe::Educe; +use itertools::Itertools; use lighthouse_network::discovery::CombinedKey; use lighthouse_network::{ NetworkConfig, NetworkGlobals, PeerId, - rpc::{RPCError, RequestType, RpcErrorResponse}, - service::api_types::{ - AppRequestId, DataColumnsByRootRequestId, DataColumnsByRootRequester, Id, - SingleLookupReqId, SyncRequestId, - }, + rpc::{RPCError, RequestType}, + service::api_types::{AppRequestId, SyncRequestId}, types::SyncState, }; use slot_clock::{SlotClock, TestingSlotClock}; +use std::sync::Arc; +use std::time::Duration; use tokio::sync::mpsc; use tracing::info; use types::{ - BeaconState, BeaconStateBase, BlobSidecar, BlockImportSource, DataColumnSidecar, EthSpec, - ForkContext, ForkName, Hash256, MinimalEthSpec as E, SignedBeaconBlock, Slot, - data::ColumnIndex, - test_utils::{SeedableRng, TestRandom, XorShiftRng}, + BlobSidecar, BlockImportSource, ColumnIndex, DataColumnSidecar, EthSpec, ForkContext, ForkName, + Hash256, MinimalEthSpec as E, SignedBeaconBlock, Slot, + test_utils::{SeedableRng, XorShiftRng}, }; const D: Duration = Duration::new(0, 0); -const PARENT_FAIL_TOLERANCE: u8 = SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS; -type DCByRootIds = Vec; -type DCByRootId = (SyncRequestId, Vec); -impl TestRig { - pub fn test_setup() -> Self { - Self::test_setup_with_custody_type(NodeCustodyType::Fullnode) +/// Configuration for how the test rig should respond to sync requests. +/// +/// Controls simulated peer behavior during lookup tests, including RPC errors, +/// invalid responses, and custom block processing results. Use builder methods +/// to configure specific failure scenarios. +#[derive(Default, Educe)] +#[educe(Debug)] +pub struct SimulateConfig { + return_rpc_error: Option, + return_wrong_blocks_n_times: usize, + return_wrong_sidecar_for_block_n_times: usize, + return_no_blocks_n_times: usize, + return_no_data_n_times: usize, + return_too_few_data_n_times: usize, + return_no_columns_on_indices_n_times: usize, + return_no_columns_on_indices: Vec, + skip_by_range_routes: bool, + // Use a callable fn because BlockProcessingResult does not implement Clone + #[educe(Debug(ignore))] + process_result_conditional: + Option Option + Send + Sync>>, + // Import a block directly before processing it (for simulating race conditions) + import_block_before_process: HashSet, +} + +impl SimulateConfig { + fn new() -> Self { + Self::default() } - pub fn test_setup_with_custody_type(node_custody_type: NodeCustodyType) -> Self { + fn happy_path() -> Self { + Self::default() + } + + fn return_no_blocks_always(mut self) -> Self { + self.return_no_blocks_n_times = usize::MAX; + self + } + + fn return_no_blocks_once(mut self) -> Self { + self.return_no_blocks_n_times = 1; + self + } + + fn return_no_data_once(mut self) -> Self { + self.return_no_data_n_times = 1; + self + } + + fn return_wrong_blocks_once(mut self) -> Self { + self.return_wrong_blocks_n_times = 1; + self + } + + fn return_wrong_sidecar_for_block_once(mut self) -> Self { + self.return_wrong_sidecar_for_block_n_times = 1; + self + } + + fn return_too_few_data_once(mut self) -> Self { + self.return_too_few_data_n_times = 1; + self + } + + fn return_no_columns_on_indices(mut self, indices: &[ColumnIndex], times: usize) -> Self { + self.return_no_columns_on_indices_n_times = times; + self.return_no_columns_on_indices = indices.to_vec(); + self + } + + fn return_rpc_error(mut self, error: RPCError) -> Self { + self.return_rpc_error = Some(error); + self + } + + fn no_range_sync(mut self) -> Self { + self.skip_by_range_routes = true; + self + } + + fn with_process_result(mut self, f: F) -> Self + where + F: Fn() -> BlockProcessingResult + Send + Sync + 'static, + { + self.process_result_conditional = Some(Box::new(move |_| Some(f()))); + self + } + + fn with_import_block_before_process(mut self, block_root: Hash256) -> Self { + self.import_block_before_process.insert(block_root); + self + } +} + +fn genesis_fork() -> ForkName { + test_spec::().fork_name_at_slot::(Slot::new(0)) +} + +pub(crate) struct TestRigConfig { + fulu_test_type: FuluTestType, + /// Override the node custody type derived from `fulu_test_type` + node_custody_type_override: Option, +} + +impl TestRig { + pub(crate) fn new(test_rig_config: TestRigConfig) -> Self { // Use `fork_from_env` logic to set correct fork epochs - let spec = test_spec::(); + let spec = Arc::new(test_spec::()); + let clock = TestingSlotClock::new( + Slot::new(0), + Duration::from_secs(0), + Duration::from_secs(12), + ); // Initialise a new beacon chain let harness = BeaconChainHarness::>::builder(E) - .spec(Arc::new(spec)) + .spec(spec.clone()) .deterministic_keypairs(1) .fresh_ephemeral_store() .mock_execution_layer() - .testing_slot_clock(TestingSlotClock::new( - Slot::new(0), - Duration::from_secs(0), - Duration::from_secs(12), - )) - .node_custody_type(node_custody_type) + .testing_slot_clock(clock.clone()) + .node_custody_type( + test_rig_config + .node_custody_type_override + .unwrap_or_else(|| test_rig_config.fulu_test_type.we_node_custody_type()), + ) .build(); let chain = harness.chain.clone(); @@ -93,12 +185,23 @@ impl TestRig { network_config, chain.spec.clone(), )); - let (beacon_processor, beacon_processor_rx) = NetworkBeaconProcessor::null_for_testing( - globals, + + let BeaconProcessorChannels { + beacon_processor_tx, + beacon_processor_rx, + } = <_>::default(); + + let beacon_processor = NetworkBeaconProcessor { + beacon_processor_send: beacon_processor_tx, + duplicate_cache: DuplicateCache::default(), + chain: chain.clone(), + // TODO: What is this sender used for? + network_tx: mpsc::unbounded_channel().0, sync_tx, - chain.clone(), - harness.runtime.task_executor.clone(), - ); + network_globals: globals.clone(), + invalid_block_storage: InvalidBlockStorage::Disabled, + executor: harness.runtime.task_executor.clone(), + }; let fork_name = chain.spec.fork_name_at_slot::(chain.slot().unwrap()); @@ -119,6 +222,7 @@ impl TestRig { network_rx, network_rx_queue: vec![], sync_rx, + sync_rx_queue: vec![], rng_08, rng, network_globals: beacon_processor.network_globals.clone(), @@ -132,36 +236,985 @@ impl TestRig { ), harness, fork_name, + network_blocks_by_root: <_>::default(), + network_blocks_by_slot: <_>::default(), + penalties: <_>::default(), + seen_lookups: <_>::default(), + requests: <_>::default(), + complete_strategy: <_>::default(), + initial_block_lookups_metrics: <_>::default(), + fulu_test_type: test_rig_config.fulu_test_type, } } - fn test_setup_after_deneb_before_fulu() -> Option { - let r = Self::test_setup(); - if r.after_deneb() && !r.fork_name.fulu_enabled() { - Some(r) + pub fn default() -> Self { + // Before Fulu, FuluTestType is irrelevant + Self::new(TestRigConfig { + fulu_test_type: FuluTestType::WeFullnodeThemSupernode, + node_custody_type_override: None, + }) + } + + pub fn with_custody_type(node_custody_type: NodeCustodyType) -> Self { + Self::new(TestRigConfig { + fulu_test_type: FuluTestType::WeFullnodeThemSupernode, + node_custody_type_override: Some(node_custody_type), + }) + } + + /// Runs the sync simulation until all event queues are empty. + /// + /// Processes events from sync_rx (sink), beacon processor, and network queues in fixed + /// priority order each tick. Handles completed work before pulling new requests. + async fn simulate(&mut self, complete_strategy: SimulateConfig) { + self.complete_strategy = complete_strategy; + self.log(&format!( + "Running simulate with config {:?}", + self.complete_strategy + )); + + let mut i = 0; + + loop { + i += 1; + + // Record current status + for BlockLookupSummary { + id, + block_root, + peers, + .. + } in self.active_single_lookups() + { + let lookup = self.seen_lookups.entry(id).or_insert(SeenLookup { + id, + block_root, + seen_peers: <_>::default(), + }); + for peer in peers { + lookup.seen_peers.insert(peer); + } + } + + // Drain all channels into queues + while let Ok(ev) = self.network_rx.try_recv() { + self.network_rx_queue.push(ev); + } + while let Ok(ev) = self.beacon_processor_rx.try_recv() { + self.beacon_processor_rx_queue.push(ev); + } + while let Ok(ev) = self.sync_rx.try_recv() { + self.sync_rx_queue.push(ev); + } + + // Process one event per tick in fixed priority: sink → processor → network + if !self.sync_rx_queue.is_empty() { + let sync_message = self.sync_rx_queue.remove(0); + self.log(&format!( + "Tick {i}: sync_rx event: {}", + Into::<&'static str>::into(&sync_message) + )); + self.sync_manager.handle_message(sync_message); + } else if !self.beacon_processor_rx_queue.is_empty() { + let event = self.beacon_processor_rx_queue.remove(0); + self.log(&format!("Tick {i}: beacon_processor event: {event:?}")); + match event.work { + Work::RpcBlock { + process_fn, + beacon_block_root, + } => { + // Import block before processing if configured (for simulating race conditions) + if self + .complete_strategy + .import_block_before_process + .contains(&beacon_block_root) + { + self.log(&format!( + "Importing block {} before processing (race condition simulation)", + beacon_block_root + )); + self.import_block_by_root(beacon_block_root).await; + } + + if let Some(f) = self.complete_strategy.process_result_conditional.as_ref() + && let Some(result) = f(beacon_block_root) + { + let id = self.lookup_by_root(beacon_block_root).id; + self.log(&format!( + "Sending custom process result to lookup id {id}: {result:?}" + )); + self.push_sync_message(SyncMessage::BlockComponentProcessed { + process_type: BlockProcessType::SingleBlock { id }, + result, + }); + } else { + process_fn.await + } + } + Work::RpcBlobs { process_fn } + | Work::RpcCustodyColumn(process_fn) + | Work::ChainSegment(process_fn) => process_fn.await, + Work::Reprocess(_) => {} // ignore + other => panic!("Unsupported Work event {}", other.str_id()), + } + } else if !self.network_rx_queue.is_empty() { + let event = self.network_rx_queue.remove(0); + self.log(&format!("Tick {i}: network_rx event: {event:?}")); + match event { + NetworkMessage::SendRequest { + peer_id, + request, + app_request_id, + } => { + self.simulate_on_request(peer_id, request, app_request_id); + } + NetworkMessage::ReportPeer { peer_id, msg, .. } => { + self.penalties.push(ReportedPenalty { peer_id, msg }); + } + _ => {} + } + } else { + break; + } + } + + self.log("No more events in simulation"); + self.log(&format!( + "Lookup metrics: {:?}", + self.sync_manager.block_lookups().metrics() + )); + self.log(&format!( + "Range sync metrics: {:?}", + self.sync_manager.range_sync().metrics() + )); + self.log(&format!( + "Max known slot: {}, Head slot: {}", + self.max_known_slot(), + self.head_slot() + )); + self.log(&format!("Penalties: {:?}", self.penalties)); + self.log(&format!( + "Total requests {}: {:?}", + self.requests.len(), + self.requests_count() + )) + } + + fn simulate_on_request( + &mut self, + peer_id: PeerId, + request: RequestType, + app_req_id: AppRequestId, + ) { + self.requests.push((request.clone(), app_req_id)); + + if let AppRequestId::Sync(req_id) = app_req_id + && let Some(error) = self.complete_strategy.return_rpc_error.take() + { + self.log(&format!( + "Completing request {req_id:?} to {peer_id} with RPCError {error:?}" + )); + self.send_sync_message(SyncMessage::RpcError { + sync_request_id: req_id, + peer_id, + error, + }); + return; + } + + match (request, app_req_id) { + (RequestType::BlocksByRoot(req), AppRequestId::Sync(req_id)) => { + let blocks = + req.block_roots() + .iter() + .filter_map(|block_root| { + if self.complete_strategy.return_no_blocks_n_times > 0 { + self.complete_strategy.return_no_blocks_n_times -= 1; + None + } else if self.complete_strategy.return_wrong_blocks_n_times > 0 { + self.complete_strategy.return_wrong_blocks_n_times -= 1; + Some(Arc::new(self.rand_block())) + } else { + Some(self.network_blocks_by_root + .get(block_root) + .unwrap_or_else(|| { + panic!("Test consumer requested unknown block: {block_root:?}") + }) + .block_cloned()) + } + }) + .collect::>(); + + self.send_rpc_blocks_response(req_id, peer_id, &blocks); + } + + (RequestType::BlobsByRoot(req), AppRequestId::Sync(req_id)) => { + if self.complete_strategy.return_no_data_n_times > 0 { + self.complete_strategy.return_no_data_n_times -= 1; + return self.send_rpc_blobs_response(req_id, peer_id, &[]); + } + + let mut blobs = req + .blob_ids + .iter() + .map(|id| { + self.network_blocks_by_root + .get(&id.block_root) + .unwrap_or_else(|| { + panic!("Test consumer requested unknown block: {id:?}") + }) + .block_data() + .and_then(|d| d.blobs()) + .unwrap_or_else(|| panic!("Block {id:?} has no blobs")) + .iter() + .find(|blob| blob.index == id.index) + .unwrap_or_else(|| panic!("Blob id {id:?} not avail")) + .clone() + }) + .collect::>(); + + if self.complete_strategy.return_too_few_data_n_times > 0 { + self.complete_strategy.return_too_few_data_n_times -= 1; + blobs.pop(); + } + + if self + .complete_strategy + .return_wrong_sidecar_for_block_n_times + > 0 + { + self.complete_strategy + .return_wrong_sidecar_for_block_n_times -= 1; + let first = blobs.first_mut().expect("empty blobs"); + let mut blob = Arc::make_mut(first).clone(); + blob.signed_block_header.message.body_root = Hash256::ZERO; + *first = Arc::new(blob); + } + + self.send_rpc_blobs_response(req_id, peer_id, &blobs); + } + + (RequestType::DataColumnsByRoot(req), AppRequestId::Sync(req_id)) => { + if self.complete_strategy.return_no_data_n_times > 0 { + self.complete_strategy.return_no_data_n_times -= 1; + return self.send_rpc_columns_response(req_id, peer_id, &[]); + } + + let will_omit_columns = req.data_column_ids.iter().any(|id| { + id.columns.iter().any(|c| { + self.complete_strategy + .return_no_columns_on_indices + .contains(c) + }) + }); + let columns_to_omit = if will_omit_columns + && self.complete_strategy.return_no_columns_on_indices_n_times > 0 + { + self.log(&format!("OMIT {:?}", req)); + self.complete_strategy.return_no_columns_on_indices_n_times -= 1; + self.complete_strategy.return_no_columns_on_indices.clone() + } else { + vec![] + }; + + let mut columns = req + .data_column_ids + .iter() + .flat_map(|id| { + let block_columns = self + .network_blocks_by_root + .get(&id.block_root) + .unwrap_or_else(|| { + panic!("Test consumer requested unknown block: {id:?}") + }) + .block_data() + .and_then(|d| d.data_columns()) + .unwrap_or_else(|| panic!("Block id {id:?} has no columns")); + id.columns + .iter() + .filter(|index| !columns_to_omit.contains(index)) + .map(move |index| { + block_columns + .iter() + .find(|c| *c.index() == *index) + .unwrap_or_else(|| { + panic!("Column {index:?} {:?} not found", id.block_root) + }) + .clone() + }) + }) + .collect::>(); + + if self.complete_strategy.return_too_few_data_n_times > 0 { + self.complete_strategy.return_too_few_data_n_times -= 1; + columns.pop(); + } + + if self + .complete_strategy + .return_wrong_sidecar_for_block_n_times + > 0 + { + self.complete_strategy + .return_wrong_sidecar_for_block_n_times -= 1; + let first = columns.first_mut().expect("empty columns"); + let column = Arc::make_mut(first); + column + .signed_block_header_mut() + .expect("not fulu") + .message + .body_root = Hash256::ZERO; + } + self.send_rpc_columns_response(req_id, peer_id, &columns); + } + + (RequestType::BlocksByRange(req), AppRequestId::Sync(req_id)) => { + if self.complete_strategy.skip_by_range_routes { + return; + } + let blocks = (*req.start_slot()..req.start_slot() + req.count()) + .filter_map(|slot| { + self.network_blocks_by_slot + .get(&Slot::new(slot)) + .map(|block| block.block_cloned()) + }) + .collect::>(); + + self.send_rpc_blocks_response(req_id, peer_id, &blocks); + } + + (RequestType::BlobsByRange(req), AppRequestId::Sync(req_id)) => { + if self.complete_strategy.skip_by_range_routes { + return; + } + + // Note: This function is permissive, blocks may have zero blobs and it won't + // error. Some caveats: + // - The genesis block never has blobs + // - Some blocks may not have blobs as the blob count is random + let blobs = (req.start_slot..req.start_slot + req.count) + .filter_map(|slot| self.network_blocks_by_slot.get(&Slot::new(slot))) + .filter_map(|block| block.block_data().and_then(|d| d.blobs())) + .flat_map(|blobs| blobs.into_iter()) + .collect::>(); + self.send_rpc_blobs_response(req_id, peer_id, &blobs); + } + + (RequestType::DataColumnsByRange(req), AppRequestId::Sync(req_id)) => { + if self.complete_strategy.skip_by_range_routes { + return; + } + // Note: This function is permissive, blocks may have zero columns and it won't + // error. Some caveats: + // - The genesis block never has columns + // - Some blocks may not have columns as the blob count is random + let columns = (req.start_slot..req.start_slot + req.count) + .filter_map(|slot| self.network_blocks_by_slot.get(&Slot::new(slot))) + .filter_map(|block| block.block_data().and_then(|d| d.data_columns())) + .flat_map(|columns| { + columns + .into_iter() + .filter(|c| req.columns.contains(c.index())) + }) + .collect::>(); + self.send_rpc_columns_response(req_id, peer_id, &columns); + } + + (RequestType::Status(_req), AppRequestId::Router) => { + // Ignore Status requests for now + } + + other => panic!("Request not supported: {app_req_id:?} {other:?}"), + } + } + + fn send_rpc_blocks_response( + &mut self, + sync_request_id: SyncRequestId, + peer_id: PeerId, + blocks: &[Arc>], + ) { + let slots = blocks.iter().map(|block| block.slot()).collect::>(); + self.log(&format!( + "Completing request {sync_request_id:?} to {peer_id} with blocks {slots:?}" + )); + + for block in blocks { + self.push_sync_message(SyncMessage::RpcBlock { + sync_request_id, + peer_id, + beacon_block: Some(block.clone()), + seen_timestamp: D, + }); + } + self.push_sync_message(SyncMessage::RpcBlock { + sync_request_id, + peer_id, + beacon_block: None, + seen_timestamp: D, + }); + } + + fn send_rpc_blobs_response( + &mut self, + sync_request_id: SyncRequestId, + peer_id: PeerId, + blobs: &[Arc>], + ) { + let slots = blobs + .iter() + .map(|block| block.slot()) + .unique() + .collect::>(); + self.log(&format!( + "Completing request {sync_request_id:?} to {peer_id} with blobs {slots:?}" + )); + + for blob in blobs { + self.push_sync_message(SyncMessage::RpcBlob { + sync_request_id, + peer_id, + blob_sidecar: Some(blob.clone()), + seen_timestamp: D, + }); + } + self.push_sync_message(SyncMessage::RpcBlob { + sync_request_id, + peer_id, + blob_sidecar: None, + seen_timestamp: D, + }); + } + + fn send_rpc_columns_response( + &mut self, + sync_request_id: SyncRequestId, + peer_id: PeerId, + columns: &[Arc>], + ) { + let slots = columns + .iter() + .map(|block| block.slot()) + .unique() + .collect::>(); + let indices = columns + .iter() + .map(|column| *column.index()) + .unique() + .collect::>(); + self.log(&format!( + "Completing request {sync_request_id:?} to {peer_id} with columns {slots:?} indices {indices:?}" + )); + + for column in columns { + self.push_sync_message(SyncMessage::RpcDataColumn { + sync_request_id, + peer_id, + data_column: Some(column.clone()), + seen_timestamp: D, + }); + } + self.push_sync_message(SyncMessage::RpcDataColumn { + sync_request_id, + peer_id, + data_column: None, + seen_timestamp: D, + }); + } + + // Preparation steps + + /// Returns the block root of the tip of the built chain + async fn build_chain(&mut self, block_count: usize) -> Hash256 { + let mut blocks = vec![]; + + // Initialise a new beacon chain + let external_harness = BeaconChainHarness::>::builder(E) + .spec(self.harness.spec.clone()) + .deterministic_keypairs(1) + .fresh_ephemeral_store() + .mock_execution_layer() + .testing_slot_clock(self.harness.chain.slot_clock.clone()) + // Make the external harness a supernode so all columns are available + .node_custody_type(NodeCustodyType::Supernode) + .build(); + // Ensure all blocks have data. Otherwise, the triggers for unknown blob parent and unknown + // data column parent fail. + external_harness + .execution_block_generator() + .set_min_blob_count(1); + + // Add genesis block for completeness + let genesis_block = external_harness.get_head_block(); + self.network_blocks_by_root + .insert(genesis_block.canonical_root(), genesis_block.clone()); + self.network_blocks_by_slot + .insert(genesis_block.slot(), genesis_block); + + for i in 0..block_count { + external_harness.advance_slot(); + let block_root = external_harness + .extend_chain( + 1, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + .await; + let block = external_harness.get_full_block(&block_root); + let block_root = block.canonical_root(); + let block_slot = block.slot(); + self.network_blocks_by_root + .insert(block_root, block.clone()); + self.network_blocks_by_slot.insert(block_slot, block); + self.log(&format!( + "Produced block {} index {i} in external harness", + block_slot, + )); + blocks.push((block_slot, block_root)); + } + + // Re-log to have a nice list of block roots at the end + for block in &blocks { + self.log(&format!("Build chain {block:?}")); + } + + // Auto-update the clock on the main harness to accept the blocks + self.harness + .set_current_slot(external_harness.get_current_slot()); + + blocks.last().expect("empty blocks").1 + } + + fn corrupt_last_block_signature(&mut self) { + let rpc_block = self.get_last_block().clone(); + let mut block = (*rpc_block.block_cloned()).clone(); + let blobs = rpc_block.block_data().and_then(|d| d.blobs()); + let columns = rpc_block.block_data().and_then(|d| d.data_columns()); + *block.signature_mut() = self.valid_signature(); + self.re_insert_block(Arc::new(block), blobs, columns); + } + + fn valid_signature(&mut self) -> bls::Signature { + let keypair = bls::Keypair::random(); + let msg = Hash256::random(); + keypair.sk.sign(msg) + } + + fn corrupt_last_blob_proposer_signature(&mut self) { + let rpc_block = self.get_last_block().clone(); + let block = rpc_block.block_cloned(); + let mut blobs = rpc_block + .block_data() + .and_then(|d| d.blobs()) + .expect("no blobs") + .into_iter() + .collect::>(); + let columns = rpc_block.block_data().and_then(|d| d.data_columns()); + let first = blobs.first_mut().expect("empty blobs"); + Arc::make_mut(first).signed_block_header.signature = self.valid_signature(); + let max_blobs = + self.harness + .spec + .max_blobs_per_block(block.slot().epoch(E::slots_per_epoch())) as usize; + let blobs = + types::BlobSidecarList::new(blobs, max_blobs).expect("invalid blob sidecar list"); + self.re_insert_block(block, Some(blobs), columns); + } + + fn corrupt_last_blob_kzg_proof(&mut self) { + let rpc_block = self.get_last_block().clone(); + let block = rpc_block.block_cloned(); + let mut blobs = rpc_block + .block_data() + .and_then(|d| d.blobs()) + .expect("no blobs") + .into_iter() + .collect::>(); + let columns = rpc_block.block_data().and_then(|d| d.data_columns()); + let first = blobs.first_mut().expect("empty blobs"); + Arc::make_mut(first).kzg_proof = kzg::KzgProof::empty(); + let max_blobs = + self.harness + .spec + .max_blobs_per_block(block.slot().epoch(E::slots_per_epoch())) as usize; + let blobs = + types::BlobSidecarList::new(blobs, max_blobs).expect("invalid blob sidecar list"); + self.re_insert_block(block, Some(blobs), columns); + } + + fn corrupt_last_column_proposer_signature(&mut self) { + let rpc_block = self.get_last_block().clone(); + let block = rpc_block.block_cloned(); + let blobs = rpc_block.block_data().and_then(|d| d.blobs()); + let mut columns = rpc_block + .block_data() + .and_then(|d| d.data_columns()) + .expect("no columns"); + let first = columns.first_mut().expect("empty columns"); + Arc::make_mut(first) + .signed_block_header_mut() + .expect("not fulu") + .signature = self.valid_signature(); + self.re_insert_block(block, blobs, Some(columns)); + } + + fn corrupt_last_column_kzg_proof(&mut self) { + let rpc_block = self.get_last_block().clone(); + let block = rpc_block.block_cloned(); + let blobs = rpc_block.block_data().and_then(|d| d.blobs()); + let mut columns = rpc_block + .block_data() + .and_then(|d| d.data_columns()) + .expect("no columns"); + let first = columns.first_mut().expect("empty columns"); + let column = Arc::make_mut(first); + let proof = column.kzg_proofs_mut().first_mut().expect("no kzg proofs"); + *proof = kzg::KzgProof::empty(); + self.re_insert_block(block, blobs, Some(columns)); + } + + fn get_last_block(&self) -> &RpcBlock { + let (_, last_block) = self + .network_blocks_by_root + .iter() + .max_by_key(|(_, block)| block.slot()) + .expect("no blocks"); + last_block + } + + fn re_insert_block( + &mut self, + block: Arc>, + blobs: Option>, + columns: Option>, + ) { + self.network_blocks_by_slot.clear(); + self.network_blocks_by_root.clear(); + let block_root = block.canonical_root(); + let block_slot = block.slot(); + let block_data = if let Some(columns) = columns { + Some(AvailableBlockData::new_with_data_columns(columns)) + } else if let Some(blobs) = blobs { + Some(AvailableBlockData::new_with_blobs(blobs)) + } else { + Some(AvailableBlockData::NoData) + }; + let rpc_block = RpcBlock::new( + block, + block_data, + &self.harness.chain.data_availability_checker, + self.harness.chain.spec.clone(), + ) + .unwrap(); + self.network_blocks_by_slot + .insert(block_slot, rpc_block.clone()); + self.network_blocks_by_root.insert(block_root, rpc_block); + } + + /// Trigger a lookup with the last created block + fn trigger_with_last_block(&mut self) { + let peer_id = match self.fulu_test_type.them_node_custody_type() { + NodeCustodyType::Fullnode => self.new_connected_peer(), + NodeCustodyType::Supernode | NodeCustodyType::SemiSupernode => { + self.new_connected_supernode_peer() + } + }; + let last_block = self.get_last_block().canonical_root(); + self.trigger_unknown_block_from_attestation(last_block, peer_id); + } + + fn block_at_slot(&self, slot: u64) -> Arc> { + self.network_blocks_by_slot + .get(&Slot::new(slot)) + .unwrap_or_else(|| panic!("No block for slot {slot}")) + .block_cloned() + } + + fn block_root_at_slot(&self, slot: u64) -> Hash256 { + self.block_at_slot(slot).canonical_root() + } + + fn trigger_with_block_at_slot(&mut self, slot: u64) { + let peer_id = self.new_connected_supernode_peer(); + let block = self.block_at_slot(slot); + self.trigger_unknown_block_from_attestation(block.canonical_root(), peer_id); + } + + async fn build_chain_and_trigger_last_block(&mut self, block_count: usize) { + self.build_chain(block_count).await; + self.trigger_with_last_block(); + } + + /// Import a block directly into the chain without going through lookup sync + async fn import_block_by_root(&mut self, block_root: Hash256) { + let rpc_block = self + .network_blocks_by_root + .get(&block_root) + .unwrap_or_else(|| panic!("No block for root {block_root}")) + .clone(); + + self.harness + .chain + .process_block( + block_root, + rpc_block, + NotifyExecutionLayer::Yes, + BlockImportSource::Gossip, + || Ok(()), + ) + .await + .unwrap(); + + self.harness.chain.recompute_head_at_current_slot().await; + } + + fn trigger_with_last_unknown_block_parent(&mut self) { + let peer_id = self.new_connected_supernode_peer(); + let last_block = self.get_last_block().block_cloned(); + self.trigger_unknown_parent_block(peer_id, last_block); + } + + fn trigger_with_last_unknown_blob_parent(&mut self) { + let peer_id = self.new_connected_supernode_peer(); + let blobs = self + .get_last_block() + .block_data() + .and_then(|d| d.blobs()) + .expect("no blobs"); + let blob = blobs.first().expect("empty blobs"); + self.trigger_unknown_parent_blob(peer_id, blob.clone()); + } + + fn trigger_with_last_unknown_data_column_parent(&mut self) { + let peer_id = self.new_connected_supernode_peer(); + let columns = self + .get_last_block() + .block_data() + .and_then(|d| d.data_columns()) + .expect("No data columns"); + let column = columns.first().expect("empty columns"); + self.trigger_unknown_parent_column(peer_id, column.clone()); + } + + // Post-test assertions + + fn head_slot(&self) -> Slot { + self.harness.chain.head().head_slot() + } + + fn assert_head_slot(&self, slot: u64) { + assert_eq!(self.head_slot(), Slot::new(slot), "Unexpected head slot"); + } + + fn max_known_slot(&self) -> Slot { + self.network_blocks_by_slot + .keys() + .max() + .copied() + .expect("no blocks") + } + + fn assert_penalties(&self, expected_penalties: &[&'static str]) { + let penalties = self + .penalties + .iter() + .map(|penalty| penalty.msg) + .collect::>(); + if penalties != expected_penalties { + panic!( + "Expected penalties: {:#?} but got {:#?}", + expected_penalties, + self.penalties + .iter() + .map(|p| format!("{} for peer {}", p.msg, p.peer_id)) + .collect::>() + ); + } + } + + fn assert_penalties_of_type(&self, expected_penalty: &'static str) { + if self.penalties.is_empty() { + panic!("No penalties but expected some of type {expected_penalty}"); + } + let non_matching_penalties = self + .penalties + .iter() + .filter(|penalty| penalty.msg != expected_penalty) + .collect::>(); + if !non_matching_penalties.is_empty() { + panic!( + "Found non-matching penalties to {}: {:?}", + expected_penalty, non_matching_penalties + ); + } + } + + fn assert_no_penalties(&mut self) { + if !self.penalties.is_empty() { + panic!("Some downscore events: {:?}", self.penalties); + } + } + fn assert_failed_lookup_sync(&mut self) { + assert!(self.created_lookups() > 0, "no created lookups"); + assert_eq!(self.completed_lookups(), 0, "some completed lookups"); + assert_eq!( + self.dropped_lookups(), + self.created_lookups(), + "not all dropped. Current lookups {:?}", + self.active_single_lookups(), + ); + self.assert_empty_network(); + self.assert_no_active_lookups(); + } + + fn assert_successful_lookup_sync(&mut self) { + assert!(self.created_lookups() > 0, "no created lookups"); + assert_eq!(self.dropped_lookups(), 0, "some dropped lookups"); + assert_eq!( + self.completed_lookups(), + self.created_lookups(), + "not all lookups completed. Current lookups {:?}", + self.active_single_lookups(), + ); + self.assert_empty_network(); + self.assert_no_active_lookups(); + } + + /// There is a lookup created with the block that triggers the unknown message that can't be + /// completed because it has zero peers + fn assert_successful_lookup_sync_parent_trigger(&mut self) { + assert!(self.created_lookups() > 0, "no created lookups"); + assert_eq!( + self.completed_lookups() + 1, + self.created_lookups(), + "all completed" + ); + assert_eq!(self.dropped_lookups(), 0, "some dropped lookups"); + self.assert_empty_network(); + } + + fn assert_pending_lookup_sync(&self) { + assert!(self.created_lookups() > 0, "no created lookups"); + assert_eq!(self.dropped_lookups(), 0, "some dropped lookups"); + assert_eq!(self.completed_lookups(), 0, "some completed lookups"); + } + + /// Assert there is at least one range sync chain created and that all sync chains completed + fn assert_successful_range_sync(&self) { + assert!( + self.range_sync_chains_added() > 0, + "No created range sync chains" + ); + assert_eq!( + self.range_sync_chains_added(), + self.range_sync_chains_removed(), + "Not all chains completed" + ); + } + + fn lookup_at_slot(&self, slot: u64) -> &SeenLookup { + let block_root = self.block_root_at_slot(slot); + self.seen_lookups + .values() + .find(|lookup| lookup.block_root == block_root) + .unwrap_or_else(|| panic!("No lookup for block_root {block_root} of slot {slot}")) + } + + fn assert_peers_at_lookup_of_slot(&self, slot: u64, expected_peers: usize) { + let lookup = self.lookup_at_slot(slot); + if lookup.seen_peers.len() != expected_peers { + panic!( + "Expected lookup of slot {slot} to have {expected_peers} peers but had {:?}", + lookup.seen_peers + ) + } + } + + /// Total count of unique lookups created + fn created_lookups(&self) -> usize { + // Subtract initial value to allow resetting metrics mid test + self.sync_manager.block_lookups().metrics().created_lookups + - self.initial_block_lookups_metrics.created_lookups + } + + /// Total count of lookups completed or dropped + fn dropped_lookups(&self) -> usize { + // Subtract initial value to allow resetting metrics mid test + self.sync_manager.block_lookups().metrics().dropped_lookups + - self.initial_block_lookups_metrics.dropped_lookups + } + + fn completed_lookups(&self) -> usize { + // Subtract initial value to allow resetting metrics mid test + self.sync_manager + .block_lookups() + .metrics() + .completed_lookups + - self.initial_block_lookups_metrics.completed_lookups + } + + fn capture_metrics_baseline(&mut self) { + self.initial_block_lookups_metrics = self.sync_manager.block_lookups().metrics().clone() + } + + /// Returns the last lookup seen with matching block_root + fn lookup_by_root(&self, block_root: Hash256) -> &SeenLookup { + self.seen_lookups + .values() + .filter(|lookup| lookup.block_root == block_root) + .max_by_key(|lookup| lookup.id) + .unwrap_or_else(|| panic!("No loookup for block_root {block_root}")) + } + + fn range_sync_chains_added(&self) -> usize { + self.sync_manager.range_sync().metrics().chains_added + } + + fn range_sync_chains_removed(&self) -> usize { + self.sync_manager.range_sync().metrics().chains_removed + } + + fn custody_columns(&self) -> &[ColumnIndex] { + self.harness + .chain + .data_availability_checker + .custody_context() + .custody_columns_for_epoch(None, &self.harness.spec) + } + + // Test setup + + fn new_after_deneb() -> Option { + genesis_fork().deneb_enabled().then(Self::default) + } + + fn new_after_deneb_before_fulu() -> Option { + let fork = genesis_fork(); + if fork.deneb_enabled() && !fork.fulu_enabled() { + Some(Self::default()) } else { None } } - pub fn test_setup_after_fulu() -> Option { - let r = Self::test_setup(); - if r.fork_name.fulu_enabled() { - Some(r) - } else { - None - } + pub fn new_fulu_peer_test(fulu_test_type: FuluTestType) -> Option { + genesis_fork().fulu_enabled().then(|| { + Self::new(TestRigConfig { + fulu_test_type, + node_custody_type_override: None, + }) + }) } pub fn log(&self, msg: &str) { info!(msg, "TEST_RIG"); } - pub fn after_deneb(&self) -> bool { + pub fn is_after_deneb(&self) -> bool { self.fork_name.deneb_enabled() } - pub fn after_fulu(&self) -> bool { + pub fn is_after_fulu(&self) -> bool { self.fork_name.fulu_enabled() } @@ -170,8 +1223,16 @@ impl TestRig { self.send_sync_message(SyncMessage::UnknownParentBlock(peer_id, block, block_root)) } - fn trigger_unknown_parent_blob(&mut self, peer_id: PeerId, blob: BlobSidecar) { - self.send_sync_message(SyncMessage::UnknownParentBlob(peer_id, blob.into())); + fn trigger_unknown_parent_blob(&mut self, peer_id: PeerId, blob: Arc>) { + self.send_sync_message(SyncMessage::UnknownParentBlob(peer_id, blob)); + } + + fn trigger_unknown_parent_column( + &mut self, + peer_id: PeerId, + column: Arc>, + ) { + self.send_sync_message(SyncMessage::UnknownParentDataColumn(peer_id, column)); } fn trigger_unknown_block_from_attestation(&mut self, block_root: Hash256, peer_id: PeerId) { @@ -180,13 +1241,6 @@ impl TestRig { )); } - /// Drain all sync messages in the sync_rx attached to the beacon processor - fn drain_sync_rx(&mut self) { - while let Ok(sync_message) = self.sync_rx.try_recv() { - self.send_sync_message(sync_message); - } - } - fn rand_block(&mut self) -> SignedBeaconBlock { self.rand_block_and_blobs(NumBlobs::None).0 } @@ -200,105 +1254,36 @@ impl TestRig { generate_rand_block_and_blobs::(fork_name, num_blobs, rng) } - fn rand_block_and_data_columns( - &mut self, - ) -> (SignedBeaconBlock, Vec>>) { - let num_blobs = NumBlobs::Number(1); - generate_rand_block_and_data_columns::( - self.fork_name, - num_blobs, - &mut self.rng, - &self.harness.spec, - ) - } - - pub fn rand_block_and_parent( - &mut self, - ) -> (SignedBeaconBlock, SignedBeaconBlock, Hash256, Hash256) { - let parent = self.rand_block(); - let parent_root = parent.canonical_root(); - let mut block = self.rand_block(); - *block.message_mut().parent_root_mut() = parent_root; - let block_root = block.canonical_root(); - (parent, block, parent_root, block_root) - } - pub fn send_sync_message(&mut self, sync_message: SyncMessage) { self.sync_manager.handle_message(sync_message); } + pub fn push_sync_message(&mut self, sync_message: SyncMessage) { + self.sync_manager.send_sync_message(sync_message); + } + fn active_single_lookups(&self) -> Vec { - self.sync_manager.active_single_lookups() + self.sync_manager.block_lookups().active_single_lookups() } fn active_single_lookups_count(&self) -> usize { - self.sync_manager.active_single_lookups().len() - } - - fn active_parent_lookups(&self) -> Vec> { - self.sync_manager.active_parent_lookups() - } - - fn active_parent_lookups_count(&self) -> usize { - self.sync_manager.active_parent_lookups().len() - } - - fn active_range_sync_chain(&self) -> (RangeSyncType, Slot, Slot) { - self.sync_manager.get_range_sync_chains().unwrap().unwrap() + self.active_single_lookups().len() } fn assert_single_lookups_count(&self, count: usize) { assert_eq!( self.active_single_lookups_count(), count, - "Unexpected count of single lookups. Current lookups: {:?}", + "Unexpected count of single lookups. Current lookups: {:#?}", self.active_single_lookups() ); } - fn assert_parent_lookups_count(&self, count: usize) { - assert_eq!( - self.active_parent_lookups_count(), - count, - "Unexpected count of parent lookups. Parent lookups: {:?}. Current lookups: {:?}", - self.active_parent_lookups(), - self.active_single_lookups() - ); - } - - fn assert_lookup_is_active(&self, block_root: Hash256) { - let lookups = self.sync_manager.active_single_lookups(); - if !lookups.iter().any(|l| l.1 == block_root) { - panic!("Expected lookup {block_root} to be the only active: {lookups:?}"); - } - } - - fn assert_lookup_peers(&self, block_root: Hash256, mut expected_peers: Vec) { - let mut lookup = self - .sync_manager - .active_single_lookups() - .into_iter() - .find(|l| l.1 == block_root) - .unwrap_or_else(|| panic!("no lookup for {block_root}")); - lookup.3.sort(); - expected_peers.sort(); - assert_eq!( - lookup.3, expected_peers, - "unexpected peers on lookup {block_root}" - ); - } - fn insert_ignored_chain(&mut self, block_root: Hash256) { + self.log(&format!("Inserting block in ignored chains {block_root:?}")); self.sync_manager.insert_ignored_chain(block_root); } - fn assert_not_ignored_chain(&mut self, chain_hash: Hash256) { - let chains = self.sync_manager.get_ignored_chains(); - if chains.contains(&chain_hash) { - panic!("ignored chains contain {chain_hash:?}: {chains:?}"); - } - } - fn assert_ignored_chain(&mut self, chain_hash: Hash256) { let chains = self.sync_manager.get_ignored_chains(); if !chains.contains(&chain_hash) { @@ -306,16 +1291,8 @@ impl TestRig { } } - fn find_single_lookup_for(&self, block_root: Hash256) -> Id { - self.active_single_lookups() - .iter() - .find(|l| l.1 == block_root) - .unwrap_or_else(|| panic!("no single block lookup found for {block_root}")) - .0 - } - #[track_caller] - fn expect_no_active_single_lookups(&self) { + fn assert_no_active_single_lookups(&self) { assert!( self.active_single_lookups().is_empty(), "expect no single block lookups: {:?}", @@ -324,13 +1301,8 @@ impl TestRig { } #[track_caller] - fn expect_no_active_lookups(&self) { - self.expect_no_active_single_lookups(); - } - - fn expect_no_active_lookups_empty_network(&mut self) { - self.expect_no_active_lookups(); - self.expect_empty_network(); + fn assert_no_active_lookups(&self) { + self.assert_no_active_single_lookups(); } pub fn new_connected_peer(&mut self) -> PeerId { @@ -340,367 +1312,62 @@ impl TestRig { .peers .write() .__add_connected_peer_testing_only(false, &self.harness.spec, key); - self.log(&format!("Added new peer for testing {peer_id:?}")); + + // Assumes custody subnet count == column count + let custody_subnets = self + .network_globals + .peers + .read() + .peer_info(&peer_id) + .expect("Peer should be known") + .custody_subnets_iter() + .copied() + .collect::>(); + let peer_custody_str = + if custody_subnets.len() == self.harness.spec.number_of_custody_groups as usize { + "all".to_owned() + } else { + format!("{custody_subnets:?}") + }; + + self.log(&format!( + "Added new peer for testing {peer_id:?}, custody: {peer_custody_str}" + )); peer_id } pub fn new_connected_supernode_peer(&mut self) -> PeerId { let key = self.determinstic_key(); - self.network_globals + let peer_id = self + .network_globals .peers .write() - .__add_connected_peer_testing_only(true, &self.harness.spec, key) + .__add_connected_peer_testing_only(true, &self.harness.spec, key); + self.log(&format!( + "Added new peer for testing {peer_id:?}, custody: supernode" + )); + peer_id } fn determinstic_key(&mut self) -> CombinedKey { k256::ecdsa::SigningKey::random(&mut self.rng_08).into() } - pub fn new_connected_peers_for_peerdas(&mut self) { - // Enough sampling peers with few columns - for _ in 0..100 { - self.new_connected_peer(); - } - // One supernode peer to ensure all columns have at least one peer - self.new_connected_supernode_peer(); - } - - fn parent_chain_processed_success( - &mut self, - chain_hash: Hash256, - blocks: &[Arc>], - ) { - // Send import events for all pending parent blocks - for _ in blocks { - self.parent_block_processed_imported(chain_hash); - } - // Send final import event for the block that triggered the lookup - self.single_block_component_processed_imported(chain_hash); - } - - /// Locate a parent lookup chain with tip hash `chain_hash` - fn find_oldest_parent_lookup(&self, chain_hash: Hash256) -> Hash256 { - let parent_chain = self - .active_parent_lookups() - .into_iter() - .find(|chain| chain.first() == Some(&chain_hash)) - .unwrap_or_else(|| { - panic!( - "No parent chain with chain_hash {chain_hash:?}: Parent lookups {:?} Single lookups {:?}", - self.active_parent_lookups(), - self.active_single_lookups(), - ) - }); - *parent_chain.last().unwrap() - } - - fn parent_block_processed(&mut self, chain_hash: Hash256, result: BlockProcessingResult) { - let id = self.find_single_lookup_for(self.find_oldest_parent_lookup(chain_hash)); - self.single_block_component_processed(id, result); - } - - fn parent_blob_processed(&mut self, chain_hash: Hash256, result: BlockProcessingResult) { - let id = self.find_single_lookup_for(self.find_oldest_parent_lookup(chain_hash)); - self.single_blob_component_processed(id, result); - } - - fn parent_block_processed_imported(&mut self, chain_hash: Hash256) { - self.parent_block_processed( - chain_hash, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(chain_hash)), - ); - } - - fn single_block_component_processed(&mut self, id: Id, result: BlockProcessingResult) { - self.send_sync_message(SyncMessage::BlockComponentProcessed { - process_type: BlockProcessType::SingleBlock { id }, - result, - }) - } - - fn single_block_component_processed_imported(&mut self, block_root: Hash256) { - let id = self.find_single_lookup_for(block_root); - self.single_block_component_processed( - id, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)), - ) - } - - fn single_blob_component_processed(&mut self, id: Id, result: BlockProcessingResult) { - self.send_sync_message(SyncMessage::BlockComponentProcessed { - process_type: BlockProcessType::SingleBlob { id }, - result, - }) - } - - fn parent_lookup_block_response( - &mut self, - id: SingleLookupReqId, - peer_id: PeerId, - beacon_block: Option>>, - ) { - self.log("parent_lookup_block_response"); - self.send_sync_message(SyncMessage::RpcBlock { - sync_request_id: SyncRequestId::SingleBlock { id }, - peer_id, - beacon_block, - seen_timestamp: D, - }); - } - - fn single_lookup_block_response( - &mut self, - id: SingleLookupReqId, - peer_id: PeerId, - beacon_block: Option>>, - ) { - self.log("single_lookup_block_response"); - self.send_sync_message(SyncMessage::RpcBlock { - sync_request_id: SyncRequestId::SingleBlock { id }, - peer_id, - beacon_block, - seen_timestamp: D, - }); - } - - fn parent_lookup_blob_response( - &mut self, - id: SingleLookupReqId, - peer_id: PeerId, - blob_sidecar: Option>>, - ) { - self.log(&format!( - "parent_lookup_blob_response {:?}", - blob_sidecar.as_ref().map(|b| b.index) - )); - self.send_sync_message(SyncMessage::RpcBlob { - sync_request_id: SyncRequestId::SingleBlob { id }, - peer_id, - blob_sidecar, - seen_timestamp: D, - }); - } - - fn single_lookup_blob_response( - &mut self, - id: SingleLookupReqId, - peer_id: PeerId, - blob_sidecar: Option>>, - ) { - self.send_sync_message(SyncMessage::RpcBlob { - sync_request_id: SyncRequestId::SingleBlob { id }, - peer_id, - blob_sidecar, - seen_timestamp: D, - }); - } - - fn complete_single_lookup_blob_download( - &mut self, - id: SingleLookupReqId, - peer_id: PeerId, - blobs: Vec>, - ) { - for blob in blobs { - self.single_lookup_blob_response(id, peer_id, Some(blob.into())); - } - self.single_lookup_blob_response(id, peer_id, None); - } - - fn complete_single_lookup_blob_lookup_valid( - &mut self, - id: SingleLookupReqId, - peer_id: PeerId, - blobs: Vec>, - import: bool, - ) { - let block_root = blobs.first().unwrap().block_root(); - let block_slot = blobs.first().unwrap().slot(); - self.complete_single_lookup_blob_download(id, peer_id, blobs); - self.expect_block_process(ResponseType::Blob); - self.single_blob_component_processed( - id.lookup_id, - if import { - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)) - } else { - BlockProcessingResult::Ok(AvailabilityProcessingStatus::MissingComponents( - block_slot, block_root, - )) - }, - ); - } - - fn complete_lookup_block_download(&mut self, block: SignedBeaconBlock) { - let block_root = block.canonical_root(); - let id = self.expect_block_lookup_request(block_root); - self.expect_empty_network(); - let peer_id = self.new_connected_peer(); - self.single_lookup_block_response(id, peer_id, Some(block.into())); - self.single_lookup_block_response(id, peer_id, None); - } - - fn complete_lookup_block_import_valid(&mut self, block_root: Hash256, import: bool) { - self.expect_block_process(ResponseType::Block); - let id = self.find_single_lookup_for(block_root); - self.single_block_component_processed( - id, - if import { - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)) - } else { - BlockProcessingResult::Ok(AvailabilityProcessingStatus::MissingComponents( - Slot::new(0), - block_root, - )) - }, - ) - } - - fn complete_single_lookup_block_valid(&mut self, block: SignedBeaconBlock, import: bool) { - let block_root = block.canonical_root(); - self.complete_lookup_block_download(block); - self.complete_lookup_block_import_valid(block_root, import) - } - - fn parent_lookup_failed(&mut self, id: SingleLookupReqId, peer_id: PeerId, error: RPCError) { - self.send_sync_message(SyncMessage::RpcError { - peer_id, - sync_request_id: SyncRequestId::SingleBlock { id }, - error, - }) - } - - fn parent_lookup_failed_unavailable(&mut self, id: SingleLookupReqId, peer_id: PeerId) { - self.parent_lookup_failed( - id, - peer_id, - RPCError::ErrorResponse( - RpcErrorResponse::ResourceUnavailable, - "older than deneb".into(), - ), - ); - } - - fn single_lookup_failed(&mut self, id: SingleLookupReqId, peer_id: PeerId, error: RPCError) { - self.send_sync_message(SyncMessage::RpcError { - peer_id, - sync_request_id: SyncRequestId::SingleBlock { id }, - error, - }) - } - - fn complete_valid_block_request( - &mut self, - id: SingleLookupReqId, - block: Arc>, - missing_components: bool, - ) { - // Complete download - let peer_id = PeerId::random(); - let slot = block.slot(); - let block_root = block.canonical_root(); - self.single_lookup_block_response(id, peer_id, Some(block)); - self.single_lookup_block_response(id, peer_id, None); - // Expect processing and resolve with import - self.expect_block_process(ResponseType::Block); - self.single_block_component_processed( - id.lookup_id, - if missing_components { - BlockProcessingResult::Ok(AvailabilityProcessingStatus::MissingComponents( - slot, block_root, - )) - } else { - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(block_root)) - }, - ) - } - - fn complete_valid_custody_request( - &mut self, - ids: DCByRootIds, - data_columns: Vec>>, - missing_components: bool, - ) { - let lookup_id = if let SyncRequestId::DataColumnsByRoot(DataColumnsByRootRequestId { - requester: DataColumnsByRootRequester::Custody(id), - .. - }) = ids.first().unwrap().0 - { - id.requester.0.lookup_id - } else { - panic!("not a custody requester") - }; - - let first_column = data_columns.first().cloned().unwrap(); - - for id in ids { - self.log(&format!("return valid data column for {id:?}")); - let indices = &id.1; - let columns_to_send = indices - .iter() - .map(|&i| data_columns[i as usize].clone()) - .collect::>(); - self.complete_data_columns_by_root_request(id, &columns_to_send); - } - - // Expect work event - self.expect_rpc_custody_column_work_event(); - - // Respond with valid result - self.send_sync_message(SyncMessage::BlockComponentProcessed { - process_type: BlockProcessType::SingleCustodyColumn(lookup_id), - result: if missing_components { - BlockProcessingResult::Ok(AvailabilityProcessingStatus::MissingComponents( - first_column.slot(), - first_column.block_root(), - )) - } else { - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported( - first_column.block_root(), - )) - }, - }); - } - - fn complete_data_columns_by_root_request( - &mut self, - (sync_request_id, _): DCByRootId, - data_columns: &[Arc>], - ) { - let peer_id = PeerId::random(); - for data_column in data_columns { - // Send chunks - self.send_sync_message(SyncMessage::RpcDataColumn { - sync_request_id, - peer_id, - data_column: Some(data_column.clone()), - seen_timestamp: timestamp_now(), - }); - } - // Send stream termination - self.send_sync_message(SyncMessage::RpcDataColumn { - sync_request_id, - peer_id, - data_column: None, - seen_timestamp: timestamp_now(), - }); - } - - /// Return RPCErrors for all active requests of peer - fn rpc_error_all_active_requests(&mut self, disconnected_peer_id: PeerId) { - self.drain_network_rx(); - while let Ok(sync_request_id) = self.pop_received_network_event(|ev| match ev { - NetworkMessage::SendRequest { - peer_id, - app_request_id: AppRequestId::Sync(id), - .. - } if *peer_id == disconnected_peer_id => Some(*id), - _ => None, - }) { - self.send_sync_message(SyncMessage::RpcError { - peer_id: disconnected_peer_id, - sync_request_id, - error: RPCError::Disconnected, - }); + pub fn new_connected_peers_for_peerdas(&mut self) -> Vec { + match self.fulu_test_type.them_node_custody_type() { + NodeCustodyType::Fullnode => { + // Enough sampling peers with few columns + let mut peers = (0..100) + .map(|_| self.new_connected_peer()) + .collect::>(); + // One supernode peer to ensure all columns have at least one peer + peers.push(self.new_connected_supernode_peer()); + peers + } + NodeCustodyType::Supernode | NodeCustodyType::SemiSupernode => { + let peer = self.new_connected_supernode_peer(); + vec![peer] + } } } @@ -708,6 +1375,22 @@ impl TestRig { self.send_sync_message(SyncMessage::Disconnect(peer_id)); } + fn get_connected_peers(&self) -> Vec { + self.network_globals + .peers + .read() + .peers() + .map(|(peer, _)| *peer) + .collect::>() + } + + fn disconnect_all_peers(&mut self) { + for peer in self.get_connected_peers() { + self.log(&format!("Disconnecting peer {peer}")); + self.send_sync_message(SyncMessage::Disconnect(peer)); + } + } + fn drain_network_rx(&mut self) { while let Ok(event) = self.network_rx.try_recv() { self.network_rx_queue.push(event); @@ -764,7 +1447,7 @@ impl TestRig { } } - pub fn expect_empty_processor(&mut self) { + pub fn assert_empty_processor(&mut self) { self.drain_processor_rx(); if !self.beacon_processor_rx_queue.is_empty() { panic!( @@ -774,215 +1457,8 @@ impl TestRig { } } - fn find_block_lookup_request( - &mut self, - for_block: Hash256, - ) -> Result { - self.pop_received_network_event(|ev| match ev { - NetworkMessage::SendRequest { - peer_id: _, - request: RequestType::BlocksByRoot(request), - app_request_id: AppRequestId::Sync(SyncRequestId::SingleBlock { id }), - } if request.block_roots().to_vec().contains(&for_block) => Some(*id), - _ => None, - }) - } - #[track_caller] - fn expect_block_lookup_request(&mut self, for_block: Hash256) -> SingleLookupReqId { - self.find_block_lookup_request(for_block) - .unwrap_or_else(|e| panic!("Expected block request for {for_block:?}: {e}")) - } - - fn find_blob_lookup_request( - &mut self, - for_block: Hash256, - ) -> Result { - self.pop_received_network_event(|ev| match ev { - NetworkMessage::SendRequest { - peer_id: _, - request: RequestType::BlobsByRoot(request), - app_request_id: AppRequestId::Sync(SyncRequestId::SingleBlob { id }), - } if request - .blob_ids - .to_vec() - .iter() - .any(|r| r.block_root == for_block) => - { - Some(*id) - } - _ => None, - }) - } - - #[track_caller] - fn expect_blob_lookup_request(&mut self, for_block: Hash256) -> SingleLookupReqId { - self.find_blob_lookup_request(for_block) - .unwrap_or_else(|e| panic!("Expected blob request for {for_block:?}: {e}")) - } - - #[track_caller] - fn expect_block_parent_request(&mut self, for_block: Hash256) -> SingleLookupReqId { - self.pop_received_network_event(|ev| match ev { - NetworkMessage::SendRequest { - peer_id: _, - request: RequestType::BlocksByRoot(request), - app_request_id: AppRequestId::Sync(SyncRequestId::SingleBlock { id }), - } if request.block_roots().to_vec().contains(&for_block) => Some(*id), - _ => None, - }) - .unwrap_or_else(|e| panic!("Expected block parent request for {for_block:?}: {e}")) - } - - fn expect_no_requests_for(&mut self, block_root: Hash256) { - if let Ok(request) = self.find_block_lookup_request(block_root) { - panic!("Expected no block request for {block_root:?} found {request:?}"); - } - if let Ok(request) = self.find_blob_lookup_request(block_root) { - panic!("Expected no blob request for {block_root:?} found {request:?}"); - } - } - - #[track_caller] - fn expect_blob_parent_request(&mut self, for_block: Hash256) -> SingleLookupReqId { - self.pop_received_network_event(|ev| match ev { - NetworkMessage::SendRequest { - peer_id: _, - request: RequestType::BlobsByRoot(request), - app_request_id: AppRequestId::Sync(SyncRequestId::SingleBlob { id }), - } if request - .blob_ids - .to_vec() - .iter() - .all(|r| r.block_root == for_block) => - { - Some(*id) - } - _ => None, - }) - .unwrap_or_else(|e| panic!("Expected blob parent request for {for_block:?}: {e}")) - } - - /// Retrieves an unknown number of requests for data columns of `block_root`. Because peer ENRs - /// are random, and peer selection is random, the total number of batched requests is unknown. - fn expect_data_columns_by_root_requests( - &mut self, - block_root: Hash256, - count: usize, - ) -> DCByRootIds { - let mut requests: DCByRootIds = vec![]; - loop { - let req = self - .pop_received_network_event(|ev| match ev { - NetworkMessage::SendRequest { - peer_id: _, - request: RequestType::DataColumnsByRoot(request), - app_request_id: - AppRequestId::Sync(id @ SyncRequestId::DataColumnsByRoot { .. }), - } => { - let matching = request - .data_column_ids - .iter() - .find(|id| id.block_root == block_root)?; - - let indices = matching.columns.iter().copied().collect(); - Some((*id, indices)) - } - _ => None, - }) - .unwrap_or_else(|e| { - panic!("Expected more DataColumnsByRoot requests for {block_root:?}: {e}") - }); - requests.push(req); - - // Should never infinite loop because sync does not send requests for 0 columns - if requests.iter().map(|r| r.1.len()).sum::() >= count { - return requests; - } - } - } - - fn expect_only_data_columns_by_root_requests( - &mut self, - for_block: Hash256, - count: usize, - ) -> DCByRootIds { - let ids = self.expect_data_columns_by_root_requests(for_block, count); - self.expect_empty_network(); - ids - } - - #[track_caller] - fn expect_block_process(&mut self, response_type: ResponseType) { - match response_type { - ResponseType::Block => self - .pop_received_processor_event(|ev| { - (ev.work_type() == beacon_processor::WorkType::RpcBlock).then_some(()) - }) - .unwrap_or_else(|e| panic!("Expected block work event: {e}")), - ResponseType::Blob => self - .pop_received_processor_event(|ev| { - (ev.work_type() == beacon_processor::WorkType::RpcBlobs).then_some(()) - }) - .unwrap_or_else(|e| panic!("Expected blobs work event: {e}")), - ResponseType::CustodyColumn => self - .pop_received_processor_event(|ev| { - (ev.work_type() == beacon_processor::WorkType::RpcCustodyColumn).then_some(()) - }) - .unwrap_or_else(|e| panic!("Expected column work event: {e}")), - } - } - - fn expect_rpc_custody_column_work_event(&mut self) { - self.pop_received_processor_event(|ev| { - if ev.work_type() == beacon_processor::WorkType::RpcCustodyColumn { - Some(()) - } else { - None - } - }) - .unwrap_or_else(|e| panic!("Expected RPC custody column work: {e}")) - } - - #[allow(dead_code)] - fn expect_no_work_event(&mut self) { - self.drain_processor_rx(); - assert!(self.network_rx_queue.is_empty()); - } - - fn expect_no_penalty_for(&mut self, peer_id: PeerId) { - self.drain_network_rx(); - let downscore_events = self - .network_rx_queue - .iter() - .filter_map(|ev| match ev { - NetworkMessage::ReportPeer { - peer_id: p_id, msg, .. - } if p_id == &peer_id => Some(msg), - _ => None, - }) - .collect::>(); - if !downscore_events.is_empty() { - panic!("Some downscore events for {peer_id}: {downscore_events:?}"); - } - } - - #[track_caller] - fn expect_parent_chain_process(&mut self) { - match self.beacon_processor_rx.try_recv() { - Ok(work) => { - // Parent chain sends blocks one by one - assert_eq!(work.work_type(), beacon_processor::WorkType::RpcBlock); - } - other => panic!( - "Expected rpc_block from chain segment process, found {:?}", - other - ), - } - } - - #[track_caller] - pub fn expect_empty_network(&mut self) { + pub fn assert_empty_network(&mut self) { self.drain_network_rx(); if !self.network_rx_queue.is_empty() { let n = self.network_rx_queue.len(); @@ -993,115 +1469,52 @@ impl TestRig { } } - #[track_caller] - fn expect_empty_beacon_processor(&mut self) { - match self.beacon_processor_rx.try_recv() { - Err(mpsc::error::TryRecvError::Empty) => {} // ok - Ok(event) => panic!("expected empty beacon processor: {:?}", event), - other => panic!("unexpected err {:?}", other), - } - } - - #[track_caller] - pub fn expect_penalty(&mut self, peer_id: PeerId, expect_penalty_msg: &'static str) { - let penalty_msg = self - .pop_received_network_event(|ev| match ev { - NetworkMessage::ReportPeer { - peer_id: p_id, msg, .. - } if p_id == &peer_id => Some(msg.to_owned()), - _ => None, - }) - .unwrap_or_else(|_| { - panic!( - "Expected '{expect_penalty_msg}' penalty for peer {peer_id}: {:#?}", - self.network_rx_queue - ) - }); - assert_eq!( - penalty_msg, expect_penalty_msg, - "Unexpected penalty msg for {peer_id}" - ); - self.log(&format!("Found expected penalty {penalty_msg}")); - } - - pub fn block_with_parent_and_blobs( + async fn import_block_to_da_checker( &mut self, - parent_root: Hash256, - num_blobs: NumBlobs, - ) -> (SignedBeaconBlock, Vec>) { - let (mut block, mut blobs) = self.rand_block_and_blobs(num_blobs); - *block.message_mut().parent_root_mut() = parent_root; - blobs.iter_mut().for_each(|blob| { - blob.signed_block_header = block.signed_block_header(); - }); - (block, blobs) - } - - pub fn rand_blockchain(&mut self, depth: usize) -> Vec>> { - let mut blocks = Vec::>>::with_capacity(depth); - for slot in 0..depth { - let parent = blocks - .last() - .map(|b| b.canonical_root()) - .unwrap_or_else(Hash256::random); - let mut block = self.rand_block(); - *block.message_mut().parent_root_mut() = parent; - *block.message_mut().slot_mut() = slot.into(); - blocks.push(block.into()); - } - self.log(&format!( - "Blockchain dump {:#?}", - blocks - .iter() - .map(|b| format!( - "block {} {} parent {}", - b.slot(), - b.canonical_root(), - b.parent_root() - )) - .collect::>() - )); - blocks - } - - fn insert_block_to_da_checker(&mut self, block: Arc>) { - let state = BeaconState::Base(BeaconStateBase::random_for_test(&mut self.rng)); - let parent_block = self.rand_block(); - let import_data = BlockImportData::::__new_for_test( - block.canonical_root(), - state, - parent_block.into(), - ); - let payload_verification_outcome = PayloadVerificationOutcome { - payload_verification_status: PayloadVerificationStatus::Verified, - is_valid_merge_transition_block: false, - }; - let executed_block = - AvailabilityPendingExecutedBlock::new(block, import_data, payload_verification_outcome); - match self - .harness + block: Arc>, + ) -> AvailabilityProcessingStatus { + // Simulate importing block from another source. Don't use GossipVerified as it checks with + // the clock, which does not match the timestamp in the payload. + let block_root = block.canonical_root(); + let rpc_block = RpcBlock::BlockOnly { block_root, block }; + self.harness .chain - .data_availability_checker - .put_executed_block(executed_block) - .unwrap() - { - Availability::Available(_) => panic!("block removed from da_checker, available"), - Availability::MissingComponents(block_root) => { + .process_block( + block_root, + rpc_block, + NotifyExecutionLayer::Yes, + BlockImportSource::Gossip, + || Ok(()), + ) + .await + .expect("Error processing block") + } + + async fn insert_block_to_da_chain_and_assert_missing_componens( + &mut self, + block: Arc>, + ) { + match self.import_block_to_da_checker(block).await { + AvailabilityProcessingStatus::Imported(_) => { + panic!("block removed from da_checker, available") + } + AvailabilityProcessingStatus::MissingComponents(_, block_root) => { self.log(&format!("inserted block to da_checker {block_root:?}")) } - }; + } } - fn insert_blob_to_da_checker(&mut self, blob: BlobSidecar) { + fn insert_blob_to_da_checker(&mut self, blob: Arc>) { match self .harness .chain .data_availability_checker - .put_gossip_verified_blobs( + .put_kzg_verified_blobs( blob.block_root(), - std::iter::once(GossipVerifiedBlob::<_, Observe>::__assumed_valid( - blob.into(), - )), + std::iter::once( + KzgVerifiedBlob::new(blob, &self.harness.chain.kzg, Duration::new(0, 0)) + .expect("Invalid blob"), + ), ) .unwrap() { @@ -1112,7 +1525,11 @@ impl TestRig { }; } - fn insert_block_to_availability_cache(&mut self, block: Arc>) { + fn insert_block_to_da_checker_as_pre_execution(&mut self, block: Arc>) { + self.log(&format!( + "Inserting block to availability_cache as pre_execution_block {:?}", + block.canonical_root() + )); self.harness .chain .data_availability_checker @@ -1121,6 +1538,9 @@ impl TestRig { } fn simulate_block_gossip_processing_becomes_invalid(&mut self, block_root: Hash256) { + self.log(&format!( + "Marking block {block_root:?} in da_checker as execution error" + )); self.harness .chain .data_availability_checker @@ -1132,19 +1552,38 @@ impl TestRig { }); } - fn simulate_block_gossip_processing_becomes_valid_missing_components( + async fn simulate_block_gossip_processing_becomes_valid( &mut self, block: Arc>, ) { let block_root = block.canonical_root(); - self.insert_block_to_da_checker(block); + match self.import_block_to_da_checker(block).await { + AvailabilityProcessingStatus::Imported(block_root) => { + self.log(&format!( + "insert block to da_checker and it imported {block_root:?}" + )); + } + AvailabilityProcessingStatus::MissingComponents(_, _) => { + panic!("block not imported after adding to da_checker"); + } + } self.send_sync_message(SyncMessage::GossipBlockProcessResult { block_root, imported: false, }); } + + fn requests_count(&self) -> HashMap<&'static str, usize> { + let mut requests_count = HashMap::new(); + for (request, _) in &self.requests { + *requests_count + .entry(Into::<&'static str>::into(request)) + .or_default() += 1; + } + requests_count + } } #[test] @@ -1161,1558 +1600,803 @@ fn stable_rng() { ); } -#[test] -fn test_single_block_lookup_happy_path() { - let mut rig = TestRig::test_setup(); - let block = rig.rand_block(); - let peer_id = rig.new_connected_peer(); - let block_root = block.canonical_root(); - // Trigger the request - rig.trigger_unknown_block_from_attestation(block_root, peer_id); - let id = rig.expect_block_lookup_request(block_root); +macro_rules! run_lookups_tests_for_depths { + ($($depth:literal),+ $(,)?) => { + paste::paste! { + $( + #[tokio::test] + async fn []() { + happy_path_unknown_attestation($depth).await; + } - // The peer provides the correct block, should not be penalized. Now the block should be sent - // for processing. - rig.single_lookup_block_response(id, peer_id, Some(block.into())); - rig.expect_empty_network(); - rig.expect_block_process(ResponseType::Block); + #[tokio::test] + async fn []() { + happy_path_unknown_block_parent($depth).await; + } - // The request should still be active. - assert_eq!(rig.active_single_lookups_count(), 1); + #[tokio::test] + async fn []() { + happy_path_unknown_data_parent($depth).await; + } - // Send the stream termination. Peer should have not been penalized, and the request removed - // after processing. - rig.single_lookup_block_response(id, peer_id, None); - rig.single_block_component_processed_imported(block_root); - rig.expect_empty_network(); - rig.expect_no_active_lookups(); + #[tokio::test] + async fn []() { + happy_path_multiple_triggers($depth).await; + } + + #[tokio::test] + async fn []() { + bad_peer_empty_block_response($depth).await; + } + + #[tokio::test] + async fn []() { + bad_peer_empty_data_response($depth).await; + } + + #[tokio::test] + async fn []() { + bad_peer_too_few_data_response($depth).await; + } + + #[tokio::test] + async fn []() { + bad_peer_wrong_block_response($depth).await; + } + + #[tokio::test] + async fn []() { + bad_peer_wrong_data_response($depth).await; + } + + #[tokio::test] + async fn []() { + bad_peer_rpc_failure($depth).await; + } + + #[tokio::test] + async fn []() { + too_many_download_failures($depth).await; + } + + #[tokio::test] + async fn []() { + too_many_processing_failures($depth).await; + } + + #[tokio::test] + async fn []() { + peer_disconnected_then_rpc_error($depth).await; + } + )+ + } + }; } -// Tests that if a peer does not respond with a block, we downscore and retry the block only -#[test] -fn test_single_block_lookup_empty_response() { - let mut r = TestRig::test_setup(); +run_lookups_tests_for_depths!(1, 2); - let block = r.rand_block(); - let block_root = block.canonical_root(); - let peer_id = r.new_connected_peer(); - - // Trigger the request - r.trigger_unknown_block_from_attestation(block_root, peer_id); - let id = r.expect_block_lookup_request(block_root); - - // The peer does not have the block. It should be penalized. - r.single_lookup_block_response(id, peer_id, None); - r.expect_penalty(peer_id, "NotEnoughResponsesReturned"); - // it should be retried - let id = r.expect_block_lookup_request(block_root); - // Send the right block this time. - r.single_lookup_block_response(id, peer_id, Some(block.into())); - r.expect_block_process(ResponseType::Block); - r.single_block_component_processed_imported(block_root); - r.expect_no_active_lookups(); +/// Assert that lookup sync succeeds with the happy case +async fn happy_path_unknown_attestation(depth: usize) { + let mut r = TestRig::default(); + // We get attestation for a block descendant (depth) blocks of current head + r.build_chain_and_trigger_last_block(depth).await; + // Complete the request with good peer behaviour + r.simulate(SimulateConfig::happy_path()).await; + r.assert_successful_lookup_sync(); } -#[test] -fn test_single_block_lookup_wrong_response() { - let mut rig = TestRig::test_setup(); - - let block_hash = Hash256::random(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_block_from_attestation(block_hash, peer_id); - let id = rig.expect_block_lookup_request(block_hash); - - // Peer sends something else. It should be penalized. - let bad_block = rig.rand_block(); - rig.single_lookup_block_response(id, peer_id, Some(bad_block.into())); - rig.expect_penalty(peer_id, "UnrequestedBlockRoot"); - rig.expect_block_lookup_request(block_hash); // should be retried - - // Send the stream termination. This should not produce an additional penalty. - rig.single_lookup_block_response(id, peer_id, None); - rig.expect_empty_network(); +async fn happy_path_unknown_block_parent(depth: usize) { + let mut r = TestRig::default(); + r.build_chain(depth).await; + r.trigger_with_last_unknown_block_parent(); + r.simulate(SimulateConfig::happy_path()).await; + // All lookups should NOT complete on this test, however note the following for the tip lookup, + // it's the lookup for the tip block which has 0 peers and a block cached: + // - before deneb the block is cached, so it's sent for processing, and success + // - before fulu the block is cached, but we can't fetch blobs so it's stuck + // - after fulu the block is cached, we start a custody request and since we use the global pool + // of peers we DO have 1 connected synced supernode peer, which gives us the columns and the + // lookup succeeds + if r.is_after_deneb() && !r.is_after_fulu() { + r.assert_successful_lookup_sync_parent_trigger() + } else { + r.assert_successful_lookup_sync(); + } } -#[test] -fn test_single_block_lookup_failure() { - let mut rig = TestRig::test_setup(); - - let block_hash = Hash256::random(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_block_from_attestation(block_hash, peer_id); - let id = rig.expect_block_lookup_request(block_hash); - - // The request fails. RPC failures are handled elsewhere so we should not penalize the peer. - rig.single_lookup_failed(id, peer_id, RPCError::UnsupportedProtocol); - rig.expect_block_lookup_request(block_hash); - rig.expect_empty_network(); +/// Assert that sync completes from a GossipUnknownParentBlob / UnknownDataColumnParent +async fn happy_path_unknown_data_parent(depth: usize) { + let Some(mut r) = TestRig::new_after_deneb() else { + return; + }; + r.build_chain(depth).await; + if r.is_after_fulu() { + r.trigger_with_last_unknown_data_column_parent(); + } else if r.is_after_deneb() { + r.trigger_with_last_unknown_blob_parent(); + } + r.simulate(SimulateConfig::happy_path()).await; + r.assert_successful_lookup_sync_parent_trigger(); } -#[test] -fn test_single_block_lookup_peer_disconnected_then_rpc_error() { - let mut rig = TestRig::test_setup(); +/// Assert that multiple trigger types don't create extra lookups +async fn happy_path_multiple_triggers(depth: usize) { + let mut r = TestRig::default(); + // + 1, because the unknown parent trigger needs two new blocks + r.build_chain(depth + 1).await; + r.trigger_with_last_block(); + r.trigger_with_last_block(); + r.trigger_with_last_unknown_block_parent(); + r.trigger_with_last_unknown_block_parent(); + if r.is_after_fulu() { + r.trigger_with_last_unknown_data_column_parent(); + } else if r.is_after_deneb() { + r.trigger_with_last_unknown_blob_parent(); + } + r.simulate(SimulateConfig::happy_path()).await; + assert_eq!(r.created_lookups(), depth + 1, "Don't create extra lookups"); + r.assert_successful_lookup_sync(); +} - let block_hash = Hash256::random(); - let peer_id = rig.new_connected_peer(); +// Test bad behaviour of peers - // Trigger the request. - rig.trigger_unknown_block_from_attestation(block_hash, peer_id); - let id = rig.expect_block_lookup_request(block_hash); +/// Assert that if peer responds with no blocks, we downscore, and retry the same lookup +async fn bad_peer_empty_block_response(depth: usize) { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(depth).await; + // Simulate that peer returns empty response once, then good behaviour + r.simulate(SimulateConfig::new().return_no_blocks_once()) + .await; + // We register a penalty, retry and complete sync successfully + r.assert_penalties(&["NotEnoughResponsesReturned"]); + r.assert_successful_lookup_sync(); + // TODO(tree-sync) For post-deneb assert that the blobs are not re-fetched + // TODO(tree-sync) Assert that a single lookup is created (no drops) +} + +/// Assert that if peer responds with no blobs / columns, we downscore, and retry the same lookup +async fn bad_peer_empty_data_response(depth: usize) { + let Some(mut r) = TestRig::new_after_deneb() else { + return; + }; + r.build_chain_and_trigger_last_block(depth).await; + r.simulate(SimulateConfig::new().return_no_data_once()) + .await; + // We register a penalty, retry and complete sync successfully + r.assert_penalties(&["NotEnoughResponsesReturned"]); + r.assert_successful_lookup_sync(); + // TODO(tree-sync) Assert that a single lookup is created (no drops) +} + +/// Assert that if peer responds with not enough blobs / columns, we downscore, and retry the same +/// lookup +async fn bad_peer_too_few_data_response(depth: usize) { + let Some(mut r) = TestRig::new_after_deneb() else { + return; + }; + r.build_chain_and_trigger_last_block(depth).await; + r.simulate(SimulateConfig::new().return_too_few_data_once()) + .await; + // We register a penalty, retry and complete sync successfully + r.assert_penalties(&["NotEnoughResponsesReturned"]); + r.assert_successful_lookup_sync(); + // TODO(tree-sync) Assert that a single lookup is created (no drops) +} + +/// Assert that if peer responds with bad blocks, we downscore, and retry the same lookup +async fn bad_peer_wrong_block_response(depth: usize) { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(depth).await; + r.simulate(SimulateConfig::new().return_wrong_blocks_once()) + .await; + r.assert_penalties(&["UnrequestedBlockRoot"]); + r.assert_successful_lookup_sync(); + + // TODO(tree-sync) Assert that a single lookup is created (no drops) +} + +/// Assert that if peer responds with bad blobs / columns, we downscore, and retry the same lookup +async fn bad_peer_wrong_data_response(depth: usize) { + let Some(mut r) = TestRig::new_after_deneb() else { + return; + }; + r.build_chain_and_trigger_last_block(depth).await; + r.simulate(SimulateConfig::new().return_wrong_sidecar_for_block_once()) + .await; + // We register a penalty, retry and complete sync successfully + r.assert_penalties(&["UnrequestedBlockRoot"]); + r.assert_successful_lookup_sync(); + // TODO(tree-sync) Assert that a single lookup is created (no drops) +} + +/// Assert that on network error, we DON'T downscore, and retry the same lookup +async fn bad_peer_rpc_failure(depth: usize) { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(depth).await; + r.simulate(SimulateConfig::new().return_rpc_error(RPCError::UnsupportedProtocol)) + .await; + r.assert_no_penalties(); + r.assert_successful_lookup_sync(); +} + +// Test retry logic + +/// Assert that on too many download failures the lookup fails, but we can still sync +async fn too_many_download_failures(depth: usize) { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(depth).await; + // Simulate that a peer always returns empty + r.simulate(SimulateConfig::new().return_no_blocks_always()) + .await; + // We register multiple penalties, the lookup fails and sync does not progress + r.assert_penalties_of_type("NotEnoughResponsesReturned"); + r.assert_failed_lookup_sync(); + + // Trigger sync again for same block, and complete successfully. + // Asserts that the lookup is not on a blacklist + r.capture_metrics_baseline(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_successful_lookup_sync(); +} + +/// Assert that on too many processing failures the lookup fails, but we can still sync +async fn too_many_processing_failures(depth: usize) { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(depth).await; + // Simulate that a peer always returns empty + r.simulate( + SimulateConfig::new() + .with_process_result(|| BlockProcessingResult::Err(BlockError::BlockSlotLimitReached)), + ) + .await; + // We register multiple penalties, the lookup fails and sync does not progress + r.assert_penalties_of_type("lookup_block_processing_failure"); + r.assert_failed_lookup_sync(); + + // Trigger sync again for same block, and complete successfully. + // Asserts that the lookup is not on a blacklist + r.capture_metrics_baseline(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_successful_lookup_sync(); +} + +#[tokio::test] +/// Assert that multiple trigger types don't create extra lookups +async fn unknown_parent_does_not_add_peers_to_itself() { + let Some(mut r) = TestRig::new_after_deneb() else { + return; + }; + // 2, because the unknown parent trigger needs two new blocks + r.build_chain(2).await; + r.trigger_with_last_unknown_block_parent(); + r.trigger_with_last_unknown_block_parent(); + if r.is_after_fulu() { + r.trigger_with_last_unknown_data_column_parent(); + } else if r.is_after_deneb() { + r.trigger_with_last_unknown_blob_parent(); + } + r.simulate(SimulateConfig::happy_path()).await; + r.assert_peers_at_lookup_of_slot(2, 0); + r.assert_peers_at_lookup_of_slot(1, 3); + assert_eq!(r.created_lookups(), 2, "Don't create extra lookups"); + // All lookups should NOT complete on this test, however note the following for the tip lookup, + // it's the lookup for the tip block which has 0 peers and a block cached: + // - before fulu the block is cached, but we can't fetch blobs so it's stuck + // - after fulu the block is cached, we start a custody request and since we use the global pool + // of peers we DO have >1 connected synced supernode peer, which gives us the columns and the + // lookup succeeds + if r.is_after_fulu() { + r.assert_successful_lookup_sync() + } else { + r.assert_successful_lookup_sync_parent_trigger(); + } +} + +#[tokio::test] +/// Assert that if the beacon processor returns Ignored, the lookup is dropped +async fn test_single_block_lookup_ignored_response() { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(1).await; + // Send an Ignored response, the request should be dropped + r.simulate(SimulateConfig::new().with_process_result(|| BlockProcessingResult::Ignored)) + .await; + // The block was not actually imported + r.assert_head_slot(0); + assert_eq!(r.created_lookups(), 1, "no created lookups"); + assert_eq!(r.dropped_lookups(), 1, "no dropped lookups"); + assert_eq!(r.completed_lookups(), 0, "some completed lookups"); +} + +#[tokio::test] +/// Assert that if the beacon processor returns DuplicateFullyImported, the lookup completes successfully +async fn test_single_block_lookup_duplicate_response() { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(1).await; + // Send a DuplicateFullyImported response, the lookup should complete successfully + r.simulate(SimulateConfig::new().with_process_result(|| { + BlockProcessingResult::Err(BlockError::DuplicateFullyImported(Hash256::ZERO)) + })) + .await; + // The block was not actually imported + r.assert_head_slot(0); + r.assert_successful_lookup_sync(); +} + +/// Assert that when peers disconnect the lookups are not dropped (kept with zero peers) +async fn peer_disconnected_then_rpc_error(depth: usize) { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(depth).await; + r.assert_single_lookups_count(1); // The peer disconnect event reaches sync before the rpc error. - rig.peer_disconnected(peer_id); + r.disconnect_all_peers(); // The lookup is not removed as it can still potentially make progress. - rig.assert_single_lookups_count(1); - // The request fails. - rig.single_lookup_failed(id, peer_id, RPCError::Disconnected); - rig.expect_block_lookup_request(block_hash); - // The request should be removed from the network context on disconnection. - rig.expect_empty_network(); + r.assert_single_lookups_count(1); + r.simulate(SimulateConfig::new().return_rpc_error(RPCError::Disconnected)) + .await; + + // Regardless of depth, only the initial lookup is created, because the peer disconnects before + // being able to download the block + assert_eq!(r.created_lookups(), 1, "no created lookups"); + assert_eq!(r.completed_lookups(), 0, "some completed lookups"); + assert_eq!(r.dropped_lookups(), 0, "some dropped lookups"); + r.assert_empty_network(); + r.assert_single_lookups_count(1); } -#[test] -fn test_single_block_lookup_becomes_parent_request() { - let mut rig = TestRig::test_setup(); - - let block = Arc::new(rig.rand_block()); - let block_root = block.canonical_root(); - let parent_root = block.parent_root(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_block_from_attestation(block.canonical_root(), peer_id); - let id = rig.expect_block_parent_request(block_root); - - // The peer provides the correct block, should not be penalized. Now the block should be sent - // for processing. - rig.single_lookup_block_response(id, peer_id, Some(block.clone())); - rig.expect_empty_network(); - rig.expect_block_process(ResponseType::Block); - - // The request should still be active. - assert_eq!(rig.active_single_lookups_count(), 1); - - // Send the stream termination. Peer should have not been penalized, and the request moved to a - // parent request after processing. - rig.single_block_component_processed( - id.lookup_id, - BlockProcessingResult::Err(BlockError::ParentUnknown { - parent_root: block.parent_root(), - }), - ); - assert_eq!(rig.active_single_lookups_count(), 2); // 2 = current + parent - rig.expect_block_parent_request(parent_root); - rig.expect_empty_network(); - assert_eq!(rig.active_parent_lookups_count(), 1); -} - -#[test] -fn test_parent_lookup_happy_path() { - let mut rig = TestRig::test_setup(); - - let (parent, block, parent_root, block_root) = rig.rand_block_and_parent(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_parent_block(peer_id, block.into()); - let id = rig.expect_block_parent_request(parent_root); - - // Peer sends the right block, it should be sent for processing. Peer should not be penalized. - rig.parent_lookup_block_response(id, peer_id, Some(parent.into())); - // No request of blobs because the block has not data - rig.expect_empty_network(); - rig.expect_block_process(ResponseType::Block); - rig.expect_empty_network(); - - // Add peer to child lookup to prevent it being dropped - rig.trigger_unknown_block_from_attestation(block_root, peer_id); - // Processing succeeds, now the rest of the chain should be sent for processing. - rig.parent_block_processed( - block_root, - BlockError::DuplicateFullyImported(block_root).into(), - ); - rig.expect_parent_chain_process(); - rig.parent_chain_processed_success(block_root, &[]); - rig.expect_no_active_lookups_empty_network(); -} - -#[test] -fn test_parent_lookup_wrong_response() { - let mut rig = TestRig::test_setup(); - - let (parent, block, parent_root, block_root) = rig.rand_block_and_parent(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_parent_block(peer_id, block.into()); - let id1 = rig.expect_block_parent_request(parent_root); - - // Peer sends the wrong block, peer should be penalized and the block re-requested. - let bad_block = rig.rand_block(); - rig.parent_lookup_block_response(id1, peer_id, Some(bad_block.into())); - rig.expect_penalty(peer_id, "UnrequestedBlockRoot"); - let id2 = rig.expect_block_parent_request(parent_root); - - // Send the stream termination for the first request. This should not produce extra penalties. - rig.parent_lookup_block_response(id1, peer_id, None); - rig.expect_empty_network(); - - // Send the right block this time. - rig.parent_lookup_block_response(id2, peer_id, Some(parent.into())); - rig.expect_block_process(ResponseType::Block); - - // Add peer to child lookup to prevent it being dropped - rig.trigger_unknown_block_from_attestation(block_root, peer_id); - // Processing succeeds, now the rest of the chain should be sent for processing. - rig.parent_block_processed_imported(block_root); - rig.expect_parent_chain_process(); - rig.parent_chain_processed_success(block_root, &[]); - rig.expect_no_active_lookups_empty_network(); -} - -#[test] -fn test_parent_lookup_rpc_failure() { - let mut rig = TestRig::test_setup(); - - let (parent, block, parent_root, block_root) = rig.rand_block_and_parent(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_parent_block(peer_id, block.into()); - let id = rig.expect_block_parent_request(parent_root); - - // The request fails. It should be tried again. - rig.parent_lookup_failed_unavailable(id, peer_id); - let id = rig.expect_block_parent_request(parent_root); - - // Send the right block this time. - rig.parent_lookup_block_response(id, peer_id, Some(parent.into())); - rig.expect_block_process(ResponseType::Block); - - // Add peer to child lookup to prevent it being dropped - rig.trigger_unknown_block_from_attestation(block_root, peer_id); - // Processing succeeds, now the rest of the chain should be sent for processing. - rig.parent_block_processed_imported(block_root); - rig.expect_parent_chain_process(); - rig.parent_chain_processed_success(block_root, &[]); - rig.expect_no_active_lookups_empty_network(); -} - -#[test] -fn test_parent_lookup_too_many_attempts() { - let mut rig = TestRig::test_setup(); - - let block = rig.rand_block(); - let parent_root = block.parent_root(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_parent_block(peer_id, block.into()); - for i in 1..=PARENT_FAIL_TOLERANCE { - let id = rig.expect_block_parent_request(parent_root); - // Blobs are only requested in the first iteration as this test only retries blocks - - if i % 2 == 0 { - // make sure every error is accounted for - // The request fails. It should be tried again. - rig.parent_lookup_failed_unavailable(id, peer_id); - } else { - // Send a bad block this time. It should be tried again. - let bad_block = rig.rand_block(); - rig.parent_lookup_block_response(id, peer_id, Some(bad_block.into())); - // Send the stream termination - - // Note, previously we would send the same lookup id with a stream terminator, - // we'd ignore it because we'd intrepret it as an unrequested response, since - // we already got one response for the block. I'm not sure what the intent is - // for having this stream terminator line in this test at all. Receiving an invalid - // block and a stream terminator with the same Id now results in two failed attempts, - // I'm unsure if this is how it should behave? - // - rig.parent_lookup_block_response(id, peer_id, None); - rig.expect_penalty(peer_id, "UnrequestedBlockRoot"); - } +#[tokio::test] +/// Assert that when creating multiple lookups their parent-child relation is discovered and we add +/// peers recursively from child to parent. +async fn lookups_form_chain() { + let depth = 5; + let mut r = TestRig::default(); + r.build_chain(depth).await; + for slot in (1..=depth).rev() { + r.trigger_with_block_at_slot(slot as u64); } + // TODO(tree-sync): Assert that there are `depth` disjoint chains + r.simulate(SimulateConfig::happy_path()).await; + r.assert_successful_lookup_sync(); - rig.expect_no_active_lookups_empty_network(); + // Assert that the peers are added to ancestor lookups, + // - The lookup with max slot has 1 peer + // - The lookup with min slot has all the peers + for slot in 1..=(depth as u64) { + let lookup = r.lookup_by_root(r.block_root_at_slot(slot)); + assert_eq!( + lookup.seen_peers.len(), + 1 + depth - slot as usize, + "Unexpected peer count for lookup at slot {slot}" + ); + } } -#[test] -fn test_parent_lookup_too_many_download_attempts_no_blacklist() { - let mut rig = TestRig::test_setup(); +#[tokio::test] +/// Assert that if a lookup chain (by appending ancestors) is too long we drop it +async fn test_parent_lookup_too_deep_grow_ancestor_one() { + let mut r = TestRig::default(); + r.build_chain(PARENT_DEPTH_TOLERANCE + 1).await; + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; - let (parent, block, parent_root, block_root) = rig.rand_block_and_parent(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_parent_block(peer_id, block.into()); - for i in 1..=PARENT_FAIL_TOLERANCE { - rig.assert_not_ignored_chain(block_root); - let id = rig.expect_block_parent_request(parent_root); - if i % 2 != 0 { - // The request fails. It should be tried again. - rig.parent_lookup_failed_unavailable(id, peer_id); - } else { - // Send a bad block this time. It should be tried again. - let bad_block = rig.rand_block(); - rig.parent_lookup_block_response(id, peer_id, Some(bad_block.into())); - rig.expect_penalty(peer_id, "UnrequestedBlockRoot"); - } - } - - rig.assert_not_ignored_chain(block_root); - rig.assert_not_ignored_chain(parent.canonical_root()); - rig.expect_no_active_lookups_empty_network(); -} - -#[test] -fn test_parent_lookup_too_many_processing_attempts_must_blacklist() { - const PROCESSING_FAILURES: u8 = PARENT_FAIL_TOLERANCE / 2 + 1; - let mut rig = TestRig::test_setup(); - let (parent, block, parent_root, block_root) = rig.rand_block_and_parent(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_parent_block(peer_id, block.into()); - - rig.log("Fail downloading the block"); - for _ in 0..(PARENT_FAIL_TOLERANCE - PROCESSING_FAILURES) { - let id = rig.expect_block_parent_request(parent_root); - // The request fails. It should be tried again. - rig.parent_lookup_failed_unavailable(id, peer_id); - } - - rig.log("Now fail processing a block in the parent request"); - for _ in 0..PROCESSING_FAILURES { - let id = rig.expect_block_parent_request(parent_root); - // Blobs are only requested in the previous first iteration as this test only retries blocks - rig.assert_not_ignored_chain(block_root); - // send the right parent but fail processing - rig.parent_lookup_block_response(id, peer_id, Some(parent.clone().into())); - rig.parent_block_processed(block_root, BlockError::BlockSlotLimitReached.into()); - rig.parent_lookup_block_response(id, peer_id, None); - rig.expect_penalty(peer_id, "lookup_block_processing_failure"); - } - - rig.assert_not_ignored_chain(block_root); - rig.expect_no_active_lookups_empty_network(); -} - -#[test] -fn test_parent_lookup_too_deep_grow_ancestor() { - let mut rig = TestRig::test_setup(); - let mut blocks = rig.rand_blockchain(PARENT_DEPTH_TOLERANCE); - - let peer_id = rig.new_connected_peer(); - let trigger_block = blocks.pop().unwrap(); - let chain_hash = trigger_block.canonical_root(); - rig.trigger_unknown_parent_block(peer_id, trigger_block); - - for block in blocks.into_iter().rev() { - let id = rig.expect_block_parent_request(block.canonical_root()); - // the block - rig.parent_lookup_block_response(id, peer_id, Some(block.clone())); - // the stream termination - rig.parent_lookup_block_response(id, peer_id, None); - // the processing request - rig.expect_block_process(ResponseType::Block); - // the processing result - rig.parent_block_processed( - chain_hash, - BlockProcessingResult::Err(BlockError::ParentUnknown { - parent_root: block.parent_root(), - }), - ) - } - - // Should create a new syncing chain - rig.drain_sync_rx(); - assert_eq!( - rig.active_range_sync_chain(), - ( - RangeSyncType::Head, - Slot::new(0), - Slot::new(PARENT_DEPTH_TOLERANCE as u64 - 1) - ) - ); + r.assert_head_slot(PARENT_DEPTH_TOLERANCE as u64 + 1); + r.assert_no_penalties(); // Should not penalize peer, but network is not clear because of the blocks_by_range requests - rig.expect_no_penalty_for(peer_id); - rig.assert_ignored_chain(chain_hash); + // r.assert_ignored_chain(chain_hash); + // + // Assert that chain is in failed chains + // Assert that there were 0 lookups completed, 33 dropped + // Assert that there were 1 range sync chains + // Bound resources: + // - Limit amount of requests + // - Limit the types of sync used + assert_eq!(r.completed_lookups(), 0, "no completed lookups"); + assert_eq!( + r.dropped_lookups(), + PARENT_DEPTH_TOLERANCE, + "All lookups dropped" + ); + r.assert_successful_range_sync(); +} + +#[tokio::test] +async fn test_parent_lookup_too_deep_grow_ancestor_zero() { + let mut r = TestRig::default(); + r.build_chain(PARENT_DEPTH_TOLERANCE).await; + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + + r.assert_head_slot(PARENT_DEPTH_TOLERANCE as u64); + r.assert_no_penalties(); + assert_eq!( + r.completed_lookups(), + PARENT_DEPTH_TOLERANCE, + "completed all lookups" + ); + assert_eq!(r.dropped_lookups(), 0, "no dropped lookups"); } // Regression test for https://github.com/sigp/lighthouse/pull/7118 // 8042 UPDATE: block was previously added to the failed_chains cache, now it's inserted into the -// ignored chains cache. The regression test still applies as the chaild lookup is not created -#[test] -fn test_child_lookup_not_created_for_ignored_chain_parent_after_processing() { - // GIVEN: A parent chain longer than PARENT_DEPTH_TOLERANCE. - let mut rig = TestRig::test_setup(); - let mut blocks = rig.rand_blockchain(PARENT_DEPTH_TOLERANCE + 1); - let peer_id = rig.new_connected_peer(); - - // The child of the trigger block to be used to extend the chain. - let trigger_block_child = blocks.pop().unwrap(); - // The trigger block that starts the lookup. - let trigger_block = blocks.pop().unwrap(); - let tip_root = trigger_block.canonical_root(); - - // Trigger the initial unknown parent block for the tip. - rig.trigger_unknown_parent_block(peer_id, trigger_block.clone()); - - // Simulate the lookup chain building up via `ParentUnknown` errors. - for block in blocks.into_iter().rev() { - let id = rig.expect_block_parent_request(block.canonical_root()); - rig.parent_lookup_block_response(id, peer_id, Some(block.clone())); - rig.parent_lookup_block_response(id, peer_id, None); - rig.expect_block_process(ResponseType::Block); - rig.parent_block_processed( - tip_root, - BlockProcessingResult::Err(BlockError::ParentUnknown { - parent_root: block.parent_root(), - }), - ); - } +// ignored chains cache. The regression test still applies as the child lookup is not created +#[tokio::test] +async fn test_child_lookup_not_created_for_ignored_chain_parent_after_processing() { + let mut r = TestRig::default(); + let depth = PARENT_DEPTH_TOLERANCE + 1; + r.build_chain(depth + 1).await; + r.trigger_with_block_at_slot(depth as u64); + r.simulate(SimulateConfig::new().no_range_sync()).await; // At this point, the chain should have been deemed too deep and pruned. // The tip root should have been inserted into ignored chains. - rig.assert_ignored_chain(tip_root); - rig.expect_no_penalty_for(peer_id); + // Ensure no blocks have been synced + r.assert_head_slot(0); + r.assert_no_active_lookups(); + r.assert_no_penalties(); + r.assert_ignored_chain(r.block_at_slot(depth as u64).canonical_root()); // WHEN: Trigger the extending block that points to the tip. - let trigger_block_child_root = trigger_block_child.canonical_root(); - rig.trigger_unknown_block_from_attestation(trigger_block_child_root, peer_id); - let id = rig.expect_block_lookup_request(trigger_block_child_root); - rig.single_lookup_block_response(id, peer_id, Some(trigger_block_child.clone())); - rig.single_lookup_block_response(id, peer_id, None); - rig.expect_block_process(ResponseType::Block); - rig.single_block_component_processed( - id.lookup_id, - BlockProcessingResult::Err(BlockError::ParentUnknown { - parent_root: tip_root, - }), - ); - + let peer = r.new_connected_peer(); + r.trigger_unknown_parent_block(peer, r.block_at_slot(depth as u64 + 1)); // THEN: The extending block should not create a lookup because the tip was inserted into // ignored chains. - rig.expect_no_active_lookups(); - rig.expect_no_penalty_for(peer_id); - rig.expect_empty_network(); + r.assert_no_active_lookups(); + r.assert_no_penalties(); + r.assert_empty_network(); } -#[test] -fn test_parent_lookup_too_deep_grow_tip() { - let mut rig = TestRig::test_setup(); - let blocks = rig.rand_blockchain(PARENT_DEPTH_TOLERANCE - 1); - let peer_id = rig.new_connected_peer(); - let tip = blocks.last().unwrap().clone(); - - for block in blocks.into_iter() { - let block_root = block.canonical_root(); - rig.trigger_unknown_block_from_attestation(block_root, peer_id); - let id = rig.expect_block_parent_request(block_root); - rig.single_lookup_block_response(id, peer_id, Some(block.clone())); - rig.single_lookup_block_response(id, peer_id, None); - rig.expect_block_process(ResponseType::Block); - rig.single_block_component_processed( - id.lookup_id, - BlockError::ParentUnknown { - parent_root: block.parent_root(), - } - .into(), - ); +#[tokio::test] +/// Assert that if a lookup chain (by appending tips) is too long we drop it +async fn test_parent_lookup_too_deep_grow_tip() { + let depth = PARENT_DEPTH_TOLERANCE + 1; + let mut r = TestRig::default(); + r.build_chain(depth).await; + for slot in (1..=depth).rev() { + r.trigger_with_block_at_slot(slot as u64); } + r.simulate(SimulateConfig::happy_path()).await; - // Should create a new syncing chain - rig.drain_sync_rx(); + // Even if the chain is longer than `PARENT_DEPTH_TOLERANCE` because the lookups are created all + // at once they chain by sections and it's possible that the oldest ancestors start processing + // before the full chain is connected. + assert!(r.created_lookups() > 0, "no created lookups"); assert_eq!( - rig.active_range_sync_chain(), - ( - RangeSyncType::Head, - Slot::new(0), - Slot::new(PARENT_DEPTH_TOLERANCE as u64 - 2) - ) + r.completed_lookups(), + r.created_lookups(), + "not all completed lookups" ); + assert_eq!(r.dropped_lookups(), 0, "some dropped lookups"); + r.assert_successful_lookup_sync(); // Should not penalize peer, but network is not clear because of the blocks_by_range requests - rig.expect_no_penalty_for(peer_id); - rig.assert_ignored_chain(tip.canonical_root()); + r.assert_no_penalties(); } -#[test] -fn test_lookup_peer_disconnected_no_peers_left_while_request() { - let mut rig = TestRig::test_setup(); - let peer_id = rig.new_connected_peer(); - let trigger_block = rig.rand_block(); - rig.trigger_unknown_parent_block(peer_id, trigger_block.into()); - rig.peer_disconnected(peer_id); - rig.rpc_error_all_active_requests(peer_id); - // Erroring all rpc requests and disconnecting the peer shouldn't remove the requests - // from the lookups map as they can still progress. - rig.assert_single_lookups_count(2); -} - -#[test] -fn test_lookup_disconnection_peer_left() { - let mut rig = TestRig::test_setup(); - let peer_ids = (0..2).map(|_| rig.new_connected_peer()).collect::>(); - let disconnecting_peer = *peer_ids.first().unwrap(); - let block_root = Hash256::random(); - // lookup should have two peers associated with the same block - for peer_id in peer_ids.iter() { - rig.trigger_unknown_block_from_attestation(block_root, *peer_id); - } - // Disconnect the first peer only, which is the one handling the request - rig.peer_disconnected(disconnecting_peer); - rig.rpc_error_all_active_requests(disconnecting_peer); - rig.assert_single_lookups_count(1); -} - -#[test] -fn test_lookup_add_peers_to_parent() { - let mut r = TestRig::test_setup(); - let peer_id_1 = r.new_connected_peer(); - let peer_id_2 = r.new_connected_peer(); - let blocks = r.rand_blockchain(5); - let last_block_root = blocks.last().unwrap().canonical_root(); - // Create a chain of lookups - for block in &blocks { - r.trigger_unknown_parent_block(peer_id_1, block.clone()); - } - r.trigger_unknown_block_from_attestation(last_block_root, peer_id_2); - for block in blocks.iter().take(blocks.len() - 1) { - // Parent has the original unknown parent event peer + new peer - r.assert_lookup_peers(block.canonical_root(), vec![peer_id_1, peer_id_2]); - } - // Child lookup only has the unknown attestation peer - r.assert_lookup_peers(last_block_root, vec![peer_id_2]); -} - -#[test] -fn test_skip_creating_ignored_parent_lookup() { - let mut rig = TestRig::test_setup(); - let (_, block, parent_root, _) = rig.rand_block_and_parent(); - let peer_id = rig.new_connected_peer(); - rig.insert_ignored_chain(parent_root); - rig.trigger_unknown_parent_block(peer_id, block.into()); - rig.expect_no_penalty_for(peer_id); +#[tokio::test] +async fn test_skip_creating_ignored_parent_lookup() { + let mut r = TestRig::default(); + r.build_chain(2).await; + r.insert_ignored_chain(r.block_root_at_slot(1)); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_no_penalties(); // Both current and parent lookup should not be created - rig.expect_no_active_lookups(); + r.assert_no_active_lookups(); } -#[test] -fn test_single_block_lookup_ignored_response() { - let mut rig = TestRig::test_setup(); +#[tokio::test] +/// Assert that if the oldest block in a chain is already imported (DuplicateFullyImported), +/// the remaining blocks in the chain are still processed successfully. This tests a race +/// condition where a block gets imported elsewhere while the lookup is processing. +/// +/// The processing sequence is: +/// - Block 3: UnknownParent (needs block 2) +/// - Block 2: UnknownParent (needs block 1) +/// - Block 1: About to be processed, but gets imported via gossip (race condition) +/// - Block 1: DuplicateFullyImported (already in chain from race) +/// - Block 2: Import ok (parent block 1 is available) +/// - Block 3: Import ok (parent block 2 is available) +async fn test_same_chain_race_condition() { + let mut r = TestRig::default(); + r.build_chain(3).await; - let block = rig.rand_block(); - let peer_id = rig.new_connected_peer(); + let block_1_root = r.block_root_at_slot(1); - // Trigger the request - rig.trigger_unknown_block_from_attestation(block.canonical_root(), peer_id); - let id = rig.expect_block_lookup_request(block.canonical_root()); + // Trigger a lookup with block 3. This creates a parent lookup chain that will + // request blocks 3 → 2 → 1. + r.trigger_with_block_at_slot(3); - // The peer provides the correct block, should not be penalized. Now the block should be sent - // for processing. - rig.single_lookup_block_response(id, peer_id, Some(block.into())); - rig.expect_empty_network(); - rig.expect_block_process(ResponseType::Block); + // Configure simulate to import block 1 right before it's processed by the lookup. + // This simulates the race condition where block 1 arrives via gossip at the same + // time the lookup is trying to process it. + r.simulate(SimulateConfig::new().with_import_block_before_process(block_1_root)) + .await; - // The request should still be active. - assert_eq!(rig.active_single_lookups_count(), 1); - - // Send the stream termination. Peer should have not been penalized, and the request removed - // after processing. - rig.single_lookup_block_response(id, peer_id, None); - // Send an Ignored response, the request should be dropped - rig.single_block_component_processed(id.lookup_id, BlockProcessingResult::Ignored); - rig.expect_no_active_lookups_empty_network(); + // The chain should complete successfully with head at slot 3, proving that + // the lookup correctly handled the DuplicateFullyImported for block 1 and + // continued processing blocks 2 and 3. + r.assert_head_slot(3); + r.assert_successful_lookup_sync(); } -#[test] -fn test_parent_lookup_ignored_response() { - let mut rig = TestRig::test_setup(); - - let (parent, block, parent_root, block_root) = rig.rand_block_and_parent(); - let peer_id = rig.new_connected_peer(); - - // Trigger the request - rig.trigger_unknown_parent_block(peer_id, block.clone().into()); - let id = rig.expect_block_parent_request(parent_root); - // Note: single block lookup for current `block` does not trigger any request because it does - // not have blobs, and the block is already cached - - // Peer sends the right block, it should be sent for processing. Peer should not be penalized. - rig.parent_lookup_block_response(id, peer_id, Some(parent.into())); - rig.expect_block_process(ResponseType::Block); - rig.expect_empty_network(); - - // Return an Ignored result. The request should be dropped - rig.parent_block_processed(block_root, BlockProcessingResult::Ignored); - rig.expect_empty_network(); - rig.expect_no_active_lookups(); -} - -/// This is a regression test. -#[test] -fn test_same_chain_race_condition() { - let mut rig = TestRig::test_setup(); - - // if we use one or two blocks it will match on the hash or the parent hash, so make a longer - // chain. - let depth = 4; - let mut blocks = rig.rand_blockchain(depth); - let peer_id = rig.new_connected_peer(); - let trigger_block = blocks.pop().unwrap(); - let chain_hash = trigger_block.canonical_root(); - rig.trigger_unknown_parent_block(peer_id, trigger_block.clone()); - - for (i, block) in blocks.clone().into_iter().rev().enumerate() { - let id = rig.expect_block_parent_request(block.canonical_root()); - // the block - rig.parent_lookup_block_response(id, peer_id, Some(block.clone())); - // the stream termination - rig.parent_lookup_block_response(id, peer_id, None); - // the processing request - rig.expect_block_process(ResponseType::Block); - // the processing result - if i + 2 == depth { - rig.log(&format!("Block {i} was removed and is already known")); - rig.parent_block_processed( - chain_hash, - BlockError::DuplicateFullyImported(block.canonical_root()).into(), - ) - } else { - rig.log(&format!("Block {i} ParentUnknown")); - rig.parent_block_processed( - chain_hash, - BlockProcessingResult::Err(BlockError::ParentUnknown { - parent_root: block.parent_root(), - }), - ) - } - } - - // Try to get this block again while the chain is being processed. We should not request it again. - let peer_id = rig.new_connected_peer(); - rig.trigger_unknown_parent_block(peer_id, trigger_block.clone()); - rig.expect_empty_network(); - - // Add a peer to the tip child lookup which has zero peers - rig.trigger_unknown_block_from_attestation(trigger_block.canonical_root(), peer_id); - - rig.log("Processing succeeds, now the rest of the chain should be sent for processing."); - for block in blocks.iter().skip(1).chain(&[trigger_block]) { - rig.expect_parent_chain_process(); - rig.single_block_component_processed_imported(block.canonical_root()); - } - rig.expect_no_active_lookups_empty_network(); -} - -#[test] -fn block_in_da_checker_skips_download() { - let Some(mut r) = TestRig::test_setup_after_deneb_before_fulu() else { +#[tokio::test] +/// Assert that if the lookup's block is in the da_checker we don't download it again +async fn block_in_da_checker_skips_download() { + // Only in Deneb, as the block needs blobs to remain in the da_checker + let Some(mut r) = TestRig::new_after_deneb_before_fulu() else { return; }; - let (block, blobs) = r.rand_block_and_blobs(NumBlobs::Number(1)); - let block_root = block.canonical_root(); - let peer_id = r.new_connected_peer(); - r.insert_block_to_da_checker(block.into()); - r.trigger_unknown_block_from_attestation(block_root, peer_id); - // Should not trigger block request - let id = r.expect_blob_lookup_request(block_root); - r.expect_empty_network(); - // Resolve blob and expect lookup completed - r.complete_single_lookup_blob_lookup_valid(id, peer_id, blobs, true); - r.expect_no_active_lookups(); + // Add block to da_checker + // Complete test with happy path + // Assert that there were no requests for blocks + r.build_chain(1).await; + r.insert_block_to_da_chain_and_assert_missing_componens(r.block_at_slot(1)) + .await; + r.trigger_with_block_at_slot(1); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_successful_lookup_sync(); + assert_eq!( + r.requests + .iter() + .filter(|(request, _)| matches!(request, RequestType::BlocksByRoot(_))) + .collect::>(), + Vec::<&(RequestType, AppRequestId)>::new(), + "There should be no block requests" + ); } -#[test] -fn block_in_processing_cache_becomes_invalid() { - let Some(mut r) = TestRig::test_setup_after_deneb_before_fulu() else { +#[tokio::test] +async fn block_in_processing_cache_becomes_invalid() { + let Some(mut r) = TestRig::new_after_deneb_before_fulu() else { return; }; - let (block, blobs) = r.rand_block_and_blobs(NumBlobs::Number(1)); - let block_root = block.canonical_root(); - let peer_id = r.new_connected_peer(); - r.insert_block_to_availability_cache(block.clone().into()); - r.trigger_unknown_block_from_attestation(block_root, peer_id); - // Should trigger blob request - let id = r.expect_blob_lookup_request(block_root); - // Should not trigger block request - r.expect_empty_network(); + r.build_chain(1).await; + let block = r.block_at_slot(1); + r.insert_block_to_da_checker_as_pre_execution(block.clone()); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_pending_lookup_sync(); + // Here the only active lookup is waiting for the block to finish processing + // Simulate invalid block, removing it from processing cache - r.simulate_block_gossip_processing_becomes_invalid(block_root); + r.simulate_block_gossip_processing_becomes_invalid(block.canonical_root()); // Should download block, then issue blobs request - r.complete_lookup_block_download(block); - // Should not trigger block or blob request - r.expect_empty_network(); - r.complete_lookup_block_import_valid(block_root, false); - // Resolve blob and expect lookup completed - r.complete_single_lookup_blob_lookup_valid(id, peer_id, blobs, true); - r.expect_no_active_lookups(); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_successful_lookup_sync(); } -#[test] -fn block_in_processing_cache_becomes_valid_imported() { - let Some(mut r) = TestRig::test_setup_after_deneb_before_fulu() else { +#[tokio::test] +async fn block_in_processing_cache_becomes_valid_imported() { + let Some(mut r) = TestRig::new_after_deneb_before_fulu() else { return; }; - let (block, blobs) = r.rand_block_and_blobs(NumBlobs::Number(1)); - let block_root = block.canonical_root(); - let peer_id = r.new_connected_peer(); - r.insert_block_to_availability_cache(block.clone().into()); - r.trigger_unknown_block_from_attestation(block_root, peer_id); - // Should trigger blob request - let id = r.expect_blob_lookup_request(block_root); - // Should not trigger block request - r.expect_empty_network(); + r.build_chain(1).await; + let block = r.block_at_slot(1); + r.insert_block_to_da_checker_as_pre_execution(block.clone()); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_pending_lookup_sync(); + // Here the only active lookup is waiting for the block to finish processing + // Resolve the block from processing step - r.simulate_block_gossip_processing_becomes_valid_missing_components(block.into()); + r.simulate_block_gossip_processing_becomes_valid(block) + .await; // Should not trigger block or blob request - r.expect_empty_network(); + r.assert_empty_network(); // Resolve blob and expect lookup completed - r.complete_single_lookup_blob_lookup_valid(id, peer_id, blobs, true); - r.expect_no_active_lookups(); + r.assert_no_active_lookups(); } // IGNORE: wait for change that delays blob fetching to knowing the block -#[ignore] -#[test] -fn blobs_in_da_checker_skip_download() { - let Some(mut r) = TestRig::test_setup_after_deneb_before_fulu() else { +#[tokio::test] +async fn blobs_in_da_checker_skip_download() { + let Some(mut r) = TestRig::new_after_deneb_before_fulu() else { return; }; - let (block, blobs) = r.rand_block_and_blobs(NumBlobs::Number(1)); - let block_root = block.canonical_root(); - let peer_id = r.new_connected_peer(); - for blob in blobs { - r.insert_blob_to_da_checker(blob); + r.build_chain(1).await; + let block = r.get_last_block().clone(); + let blobs = block + .block_data() + .and_then(|d| d.blobs()) + .expect("block with no blobs"); + for blob in &blobs { + r.insert_blob_to_da_checker(blob.clone()); } - r.trigger_unknown_block_from_attestation(block_root, peer_id); - // Should download and process the block - r.complete_single_lookup_block_valid(block, true); - // Should not trigger blob request - r.expect_empty_network(); - r.expect_no_active_lookups(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + + r.assert_successful_lookup_sync(); + assert_eq!( + r.requests + .iter() + .filter(|(request, _)| matches!(request, RequestType::BlobsByRoot(_))) + .collect::>(), + Vec::<&(RequestType, AppRequestId)>::new(), + "There should be no blob requests" + ); } -#[test] -fn custody_lookup_happy_path() { - let Some(mut r) = TestRig::test_setup_after_fulu() else { +macro_rules! fulu_peer_matrix_tests { + ( + [$($name:ident => $variant:expr),+ $(,)?] + ) => { + paste::paste! { + $( + #[tokio::test] + async fn []() { + custody_lookup_happy_path($variant).await; + } + + #[tokio::test] + async fn []() { + custody_lookup_some_custody_failures($variant).await; + } + + #[tokio::test] + async fn []() { + custody_lookup_permanent_custody_failures($variant).await; + } + )+ + } + }; +} + +fulu_peer_matrix_tests!( + [ + we_supernode_them_supernode => FuluTestType::WeSupernodeThemSupernode, + we_supernode_them_fullnodes => FuluTestType::WeSupernodeThemFullnodes, + we_fullnode_them_supernode => FuluTestType::WeFullnodeThemSupernode, + we_fullnode_them_fullnodes => FuluTestType::WeFullnodeThemFullnodes, + ] +); + +async fn custody_lookup_happy_path(test_type: FuluTestType) { + let Some(mut r) = TestRig::new_fulu_peer_test(test_type) else { return; }; - let spec = E::default_spec(); + r.build_chain(1).await; r.new_connected_peers_for_peerdas(); - let (block, data_columns) = r.rand_block_and_data_columns(); - let block_root = block.canonical_root(); - let peer_id = r.new_connected_peer(); - r.trigger_unknown_block_from_attestation(block_root, peer_id); - // Should not request blobs - let id = r.expect_block_lookup_request(block.canonical_root()); - r.complete_valid_block_request(id, block.into(), true); - // for each slot we download `samples_per_slot` columns - let sample_column_count = spec.samples_per_slot * spec.data_columns_per_group::(); - let custody_ids = - r.expect_only_data_columns_by_root_requests(block_root, sample_column_count as usize); - r.complete_valid_custody_request(custody_ids, data_columns, false); - r.expect_no_active_lookups(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + r.assert_no_penalties(); + r.assert_successful_lookup_sync(); } +async fn custody_lookup_some_custody_failures(test_type: FuluTestType) { + let Some(mut r) = TestRig::new_fulu_peer_test(test_type) else { + return; + }; + let block_root = r.build_chain(1).await; + // Send the same trigger from all peers, so that the lookup has all peers + for peer in r.new_connected_peers_for_peerdas() { + r.trigger_unknown_block_from_attestation(block_root, peer); + } + let custody_columns = r.custody_columns(); + r.simulate(SimulateConfig::new().return_no_columns_on_indices(&custody_columns[..4], 3)) + .await; + r.assert_penalties_of_type("NotEnoughResponsesReturned"); + r.assert_successful_lookup_sync(); +} + +async fn custody_lookup_permanent_custody_failures(test_type: FuluTestType) { + let Some(mut r) = TestRig::new_fulu_peer_test(test_type) else { + return; + }; + let block_root = r.build_chain(1).await; + + // Send the same trigger from all peers, so that the lookup has all peers + for peer in r.new_connected_peers_for_peerdas() { + r.trigger_unknown_block_from_attestation(block_root, peer); + } + + let custody_columns = r.custody_columns(); + r.simulate( + SimulateConfig::new().return_no_columns_on_indices(&custody_columns[..2], usize::MAX), + ) + .await; + // Every peer that does not return a column is part of the lookup because it claimed to have + // imported the lookup, so we will penalize. + r.assert_penalties_of_type("NotEnoughResponsesReturned"); + r.assert_failed_lookup_sync(); +} + +// We supernode, diverse peers +// We not supernode, diverse peers + // TODO(das): Test retries of DataColumnByRoot: // - Expect request for column_index // - Respond with bad data // - Respond with stream terminator // ^ The stream terminator should be ignored and not close the next retry -mod deneb_only { - use super::*; - use beacon_chain::{ - block_verification_types::{AsBlock, RpcBlock}, - data_availability_checker::AvailabilityCheckError, - }; - use std::collections::VecDeque; - - struct DenebTester { - rig: TestRig, - block: Arc>, - blobs: Vec>>, - parent_block_roots: Vec, - parent_block: VecDeque>>, - parent_blobs: VecDeque>>>, - unknown_parent_block: Option>>, - unknown_parent_blobs: Option>>>, - peer_id: PeerId, - block_req_id: Option, - parent_block_req_id: Option, - blob_req_id: Option, - parent_blob_req_id: Option, - slot: Slot, - block_root: Hash256, - } - - enum RequestTrigger { - AttestationUnknownBlock, - GossipUnknownParentBlock(usize), - GossipUnknownParentBlob(usize), - } - - impl RequestTrigger { - fn num_parents(&self) -> usize { - match self { - RequestTrigger::AttestationUnknownBlock => 0, - RequestTrigger::GossipUnknownParentBlock(num_parents) => *num_parents, - RequestTrigger::GossipUnknownParentBlob(num_parents) => *num_parents, - } - } - } - - impl DenebTester { - fn new(request_trigger: RequestTrigger) -> Option { - let Some(mut rig) = TestRig::test_setup_after_deneb_before_fulu() else { - return None; - }; - let (block, blobs) = rig.rand_block_and_blobs(NumBlobs::Random); - let mut block = Arc::new(block); - let mut blobs = blobs.into_iter().map(Arc::new).collect::>(); - let slot = block.slot(); - - let num_parents = request_trigger.num_parents(); - let mut parent_block_chain = VecDeque::with_capacity(num_parents); - let mut parent_blobs_chain = VecDeque::with_capacity(num_parents); - let mut parent_block_roots = vec![]; - for _ in 0..num_parents { - // Set the current block as the parent. - let parent_root = block.canonical_root(); - let parent_block = block.clone(); - let parent_blobs = blobs.clone(); - parent_block_chain.push_front(parent_block); - parent_blobs_chain.push_front(parent_blobs); - parent_block_roots.push(parent_root); - - // Create the next block. - let (child_block, child_blobs) = - rig.block_with_parent_and_blobs(parent_root, NumBlobs::Random); - let mut child_block = Arc::new(child_block); - let mut child_blobs = child_blobs.into_iter().map(Arc::new).collect::>(); - - // Update the new block to the current block. - std::mem::swap(&mut child_block, &mut block); - std::mem::swap(&mut child_blobs, &mut blobs); - } - let block_root = block.canonical_root(); - - let peer_id = rig.new_connected_peer(); - - // Trigger the request - let (block_req_id, blob_req_id, parent_block_req_id, parent_blob_req_id) = - match request_trigger { - RequestTrigger::AttestationUnknownBlock => { - rig.send_sync_message(SyncMessage::UnknownBlockHashFromAttestation( - peer_id, block_root, - )); - let block_req_id = rig.expect_block_lookup_request(block_root); - (Some(block_req_id), None, None, None) - } - RequestTrigger::GossipUnknownParentBlock { .. } => { - rig.send_sync_message(SyncMessage::UnknownParentBlock( - peer_id, - block.clone(), - block_root, - )); - - let parent_root = block.parent_root(); - let parent_block_req_id = rig.expect_block_parent_request(parent_root); - rig.expect_empty_network(); // expect no more requests - (None, None, Some(parent_block_req_id), None) - } - RequestTrigger::GossipUnknownParentBlob { .. } => { - let single_blob = blobs.first().cloned().unwrap(); - let parent_root = single_blob.block_parent_root(); - rig.send_sync_message(SyncMessage::UnknownParentBlob(peer_id, single_blob)); - - let parent_block_req_id = rig.expect_block_parent_request(parent_root); - rig.expect_empty_network(); // expect no more requests - (None, None, Some(parent_block_req_id), None) - } - }; - - Some(Self { - rig, - block, - blobs, - parent_block: parent_block_chain, - parent_blobs: parent_blobs_chain, - parent_block_roots, - unknown_parent_block: None, - unknown_parent_blobs: None, - peer_id, - block_req_id, - parent_block_req_id, - blob_req_id, - parent_blob_req_id, - slot, - block_root, - }) - } - - fn trigger_unknown_block_from_attestation(mut self) -> Self { - let block_root = self.block.canonical_root(); - self.rig - .trigger_unknown_block_from_attestation(block_root, self.peer_id); - self - } - - fn parent_block_response(mut self) -> Self { - self.rig.expect_empty_network(); - let block = self.parent_block.pop_front().unwrap().clone(); - let _ = self.unknown_parent_block.insert(block.clone()); - self.rig.parent_lookup_block_response( - self.parent_block_req_id.expect("parent request id"), - self.peer_id, - Some(block), - ); - - self.rig.assert_parent_lookups_count(1); - self - } - - fn parent_block_response_expect_blobs(mut self) -> Self { - self.rig.expect_empty_network(); - let block = self.parent_block.pop_front().unwrap().clone(); - let _ = self.unknown_parent_block.insert(block.clone()); - self.rig.parent_lookup_block_response( - self.parent_block_req_id.expect("parent request id"), - self.peer_id, - Some(block), - ); - - // Expect blobs request after sending block - let s = self.expect_parent_blobs_request(); - - s.rig.assert_parent_lookups_count(1); - s - } - - fn parent_blob_response(mut self) -> Self { - let blobs = self.parent_blobs.pop_front().unwrap(); - let _ = self.unknown_parent_blobs.insert(blobs.clone()); - for blob in &blobs { - self.rig.parent_lookup_blob_response( - self.parent_blob_req_id.expect("parent blob request id"), - self.peer_id, - Some(blob.clone()), - ); - assert_eq!(self.rig.active_parent_lookups_count(), 1); - } - self.rig.parent_lookup_blob_response( - self.parent_blob_req_id.expect("parent blob request id"), - self.peer_id, - None, - ); - - self - } - - fn block_response_triggering_process(self) -> Self { - let mut me = self.block_response_and_expect_blob_request(); - me.rig.expect_block_process(ResponseType::Block); - - // The request should still be active. - assert_eq!(me.rig.active_single_lookups_count(), 1); - me - } - - fn block_response_and_expect_blob_request(mut self) -> Self { - // The peer provides the correct block, should not be penalized. Now the block should be sent - // for processing. - self.rig.single_lookup_block_response( - self.block_req_id.expect("block request id"), - self.peer_id, - Some(self.block.clone()), - ); - // After responding with block the node will issue a blob request - let mut s = self.expect_blobs_request(); - - s.rig.expect_empty_network(); - - // The request should still be active. - s.rig.assert_lookup_is_active(s.block.canonical_root()); - s - } - - fn blobs_response(mut self) -> Self { - self.rig - .log(&format!("blobs response {}", self.blobs.len())); - for blob in &self.blobs { - self.rig.single_lookup_blob_response( - self.blob_req_id.expect("blob request id"), - self.peer_id, - Some(blob.clone()), - ); - self.rig - .assert_lookup_is_active(self.block.canonical_root()); - } - self.rig.single_lookup_blob_response( - self.blob_req_id.expect("blob request id"), - self.peer_id, - None, - ); - self - } - - fn blobs_response_was_valid(mut self) -> Self { - self.rig.expect_empty_network(); - if !self.blobs.is_empty() { - self.rig.expect_block_process(ResponseType::Blob); - } - self - } - - fn expect_empty_beacon_processor(mut self) -> Self { - self.rig.expect_empty_beacon_processor(); - self - } - - fn empty_block_response(mut self) -> Self { - self.rig.single_lookup_block_response( - self.block_req_id.expect("block request id"), - self.peer_id, - None, - ); - self - } - - fn empty_blobs_response(mut self) -> Self { - self.rig.single_lookup_blob_response( - self.blob_req_id.expect("blob request id"), - self.peer_id, - None, - ); - self - } - - fn empty_parent_blobs_response(mut self) -> Self { - self.rig.parent_lookup_blob_response( - self.parent_blob_req_id.expect("blob request id"), - self.peer_id, - None, - ); - self - } - - fn block_missing_components(mut self) -> Self { - self.rig.single_block_component_processed( - self.block_req_id.expect("block request id").lookup_id, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::MissingComponents( - self.block.slot(), - self.block_root, - )), - ); - self.rig.expect_empty_network(); - self.rig.assert_single_lookups_count(1); - self - } - - fn blob_imported(mut self) -> Self { - self.rig.single_blob_component_processed( - self.blob_req_id.expect("blob request id").lookup_id, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(self.block_root)), - ); - self.rig.expect_empty_network(); - self.rig.assert_single_lookups_count(0); - self - } - - fn block_imported(mut self) -> Self { - // Missing blobs should be the request is not removed, the outstanding blobs request should - // mean we do not send a new request. - self.rig.single_block_component_processed( - self.block_req_id - .or(self.blob_req_id) - .expect("block request id") - .lookup_id, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(self.block_root)), - ); - self.rig.expect_empty_network(); - self.rig.assert_single_lookups_count(0); - self - } - - fn parent_block_imported(mut self) -> Self { - let parent_root = *self.parent_block_roots.first().unwrap(); - self.rig - .log(&format!("parent_block_imported {parent_root:?}")); - self.rig.parent_block_processed( - self.block_root, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(parent_root)), - ); - self.rig.expect_no_requests_for(parent_root); - self.rig.assert_parent_lookups_count(0); - self - } - - fn parent_block_missing_components(mut self) -> Self { - let parent_root = *self.parent_block_roots.first().unwrap(); - self.rig - .log(&format!("parent_block_missing_components {parent_root:?}")); - self.rig.parent_block_processed( - self.block_root, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::MissingComponents( - Slot::new(0), - parent_root, - )), - ); - self.rig.expect_no_requests_for(parent_root); - self - } - - fn parent_blob_imported(mut self) -> Self { - let parent_root = *self.parent_block_roots.first().unwrap(); - self.rig - .log(&format!("parent_blob_imported {parent_root:?}")); - self.rig.parent_blob_processed( - self.block_root, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(parent_root)), - ); - - self.rig.expect_no_requests_for(parent_root); - self.rig.assert_parent_lookups_count(0); - self - } - - fn parent_block_unknown_parent(mut self) -> Self { - self.rig.log("parent_block_unknown_parent"); - let block = self.unknown_parent_block.take().unwrap(); - // Now this block is the one we expect requests from - self.block = block.clone(); - let block = RpcBlock::new( - block, - None, - &self.rig.harness.chain.data_availability_checker, - self.rig.harness.chain.spec.clone(), - ) - .unwrap(); - self.rig.parent_block_processed( - self.block_root, - BlockProcessingResult::Err(BlockError::ParentUnknown { - parent_root: block.parent_root(), - }), - ); - assert_eq!(self.rig.active_parent_lookups_count(), 1); - self - } - - fn invalid_parent_processed(mut self) -> Self { - self.rig.parent_block_processed( - self.block_root, - BlockProcessingResult::Err(BlockError::BlockSlotLimitReached), - ); - assert_eq!(self.rig.active_parent_lookups_count(), 1); - self - } - - fn invalid_block_processed(mut self) -> Self { - self.rig.single_block_component_processed( - self.block_req_id.expect("block request id").lookup_id, - BlockProcessingResult::Err(BlockError::BlockSlotLimitReached), - ); - self.rig.assert_single_lookups_count(1); - self - } - - fn invalid_blob_processed(mut self) -> Self { - self.rig.log("invalid_blob_processed"); - self.rig.single_blob_component_processed( - self.blob_req_id.expect("blob request id").lookup_id, - BlockProcessingResult::Err(BlockError::AvailabilityCheck( - AvailabilityCheckError::InvalidBlobs(kzg::Error::KzgVerificationFailed), - )), - ); - self.rig.assert_single_lookups_count(1); - self - } - - fn missing_components_from_block_request(mut self) -> Self { - self.rig.single_block_component_processed( - self.block_req_id.expect("block request id").lookup_id, - BlockProcessingResult::Ok(AvailabilityProcessingStatus::MissingComponents( - self.slot, - self.block_root, - )), - ); - // Add block to da_checker so blobs request can continue - self.rig.insert_block_to_da_checker(self.block.clone()); - - self.rig.assert_single_lookups_count(1); - self - } - - fn complete_current_block_and_blobs_lookup(self) -> Self { - self.expect_block_request() - .block_response_and_expect_blob_request() - .blobs_response() - // TODO: Should send blobs for processing - .expect_block_process() - .block_imported() - } - - fn log(self, msg: &str) -> Self { - self.rig.log(msg); - self - } - - fn parent_block_then_empty_parent_blobs(self) -> Self { - self.log( - " Return empty blobs for parent, block errors with missing components, downscore", - ) - .parent_block_response() - .expect_parent_blobs_request() - .empty_parent_blobs_response() - .expect_penalty("NotEnoughResponsesReturned") - .log("Re-request parent blobs, succeed and import parent") - .expect_parent_blobs_request() - .parent_blob_response() - .expect_block_process() - .parent_block_missing_components() - // Insert new peer into child request before completing parent - .trigger_unknown_block_from_attestation() - .parent_blob_imported() - } - - fn expect_penalty(mut self, expect_penalty_msg: &'static str) -> Self { - self.rig.expect_penalty(self.peer_id, expect_penalty_msg); - self - } - fn expect_no_penalty(mut self) -> Self { - self.rig.expect_empty_network(); - self - } - fn expect_no_penalty_and_no_requests(mut self) -> Self { - self.rig.expect_empty_network(); - self - } - fn expect_block_request(mut self) -> Self { - let id = self - .rig - .expect_block_lookup_request(self.block.canonical_root()); - self.block_req_id = Some(id); - self - } - fn expect_blobs_request(mut self) -> Self { - let id = self - .rig - .expect_blob_lookup_request(self.block.canonical_root()); - self.blob_req_id = Some(id); - self - } - fn expect_parent_block_request(mut self) -> Self { - let id = self - .rig - .expect_block_parent_request(self.block.parent_root()); - self.parent_block_req_id = Some(id); - self - } - fn expect_parent_blobs_request(mut self) -> Self { - let id = self - .rig - .expect_blob_parent_request(self.block.parent_root()); - self.parent_blob_req_id = Some(id); - self - } - fn expect_no_blobs_request(mut self) -> Self { - self.rig.expect_empty_network(); - self - } - fn expect_no_block_request(mut self) -> Self { - self.rig.expect_empty_network(); - self - } - fn invalidate_blobs_too_few(mut self) -> Self { - self.blobs.pop().expect("blobs"); - self - } - fn expect_block_process(mut self) -> Self { - self.rig.expect_block_process(ResponseType::Block); - self - } - fn expect_no_active_lookups(self) -> Self { - self.rig.expect_no_active_lookups(); - self - } - fn search_parent_dup(mut self) -> Self { - self.rig - .trigger_unknown_parent_block(self.peer_id, self.block.clone()); - self - } - } - - #[test] - fn single_block_and_blob_lookup_block_returned_first_attestation() { - let Some(tester) = DenebTester::new(RequestTrigger::AttestationUnknownBlock) else { - return; - }; - tester - .block_response_and_expect_blob_request() - .blobs_response() - .block_missing_components() // blobs not yet imported - .blobs_response_was_valid() - .blob_imported(); // now blobs resolve as imported - } - - #[test] - fn single_block_response_then_empty_blob_response_attestation() { - let Some(tester) = DenebTester::new(RequestTrigger::AttestationUnknownBlock) else { - return; - }; - tester - .block_response_and_expect_blob_request() - .missing_components_from_block_request() - .empty_blobs_response() - .expect_penalty("NotEnoughResponsesReturned") - .expect_blobs_request() - .expect_no_block_request(); - } - - #[test] - fn single_invalid_block_response_then_blob_response_attestation() { - let Some(tester) = DenebTester::new(RequestTrigger::AttestationUnknownBlock) else { - return; - }; - tester - .block_response_triggering_process() - .invalid_block_processed() - .expect_penalty("lookup_block_processing_failure") - .expect_block_request() - .expect_no_blobs_request() - .blobs_response() - // blobs not sent for processing until the block is processed - .expect_no_penalty_and_no_requests(); - } - - #[test] - fn single_block_response_then_invalid_blob_response_attestation() { - let Some(tester) = DenebTester::new(RequestTrigger::AttestationUnknownBlock) else { - return; - }; - tester - .block_response_triggering_process() - .missing_components_from_block_request() - .blobs_response() - .invalid_blob_processed() - .expect_penalty("lookup_blobs_processing_failure") - .expect_blobs_request() - .expect_no_block_request(); - } - - #[test] - fn single_block_response_then_too_few_blobs_response_attestation() { - let Some(tester) = DenebTester::new(RequestTrigger::AttestationUnknownBlock) else { - return; - }; - tester - .block_response_triggering_process() - .missing_components_from_block_request() - .invalidate_blobs_too_few() - .blobs_response() - .expect_penalty("NotEnoughResponsesReturned") - .expect_blobs_request() - .expect_no_block_request(); - } - - // Test peer returning block that has unknown parent, and a new lookup is created - #[test] - fn parent_block_unknown_parent() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock(1)) else { - return; - }; - tester - .expect_empty_beacon_processor() - .parent_block_response_expect_blobs() - .parent_blob_response() - .expect_block_process() - .parent_block_unknown_parent() - .expect_parent_block_request() - .expect_empty_beacon_processor(); - } - - // Test peer returning invalid (processing) block, expect retry - #[test] - fn parent_block_invalid_parent() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock(1)) else { - return; - }; - tester - .parent_block_response_expect_blobs() - .parent_blob_response() - .expect_block_process() - .invalid_parent_processed() - .expect_penalty("lookup_block_processing_failure") - .expect_parent_block_request() - .expect_empty_beacon_processor(); - } - - // Tests that if a peer does not respond with a block, we downscore and retry the block only - #[test] - fn empty_block_is_retried() { - let Some(tester) = DenebTester::new(RequestTrigger::AttestationUnknownBlock) else { - return; - }; - tester - .empty_block_response() - .expect_penalty("NotEnoughResponsesReturned") - .expect_block_request() - .expect_no_blobs_request() - .block_response_and_expect_blob_request() - .blobs_response() - .block_imported() - .expect_no_active_lookups(); - } - - #[test] - fn parent_block_then_empty_parent_blobs() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock(1)) else { - return; - }; - tester - .parent_block_then_empty_parent_blobs() - .log("resolve original block trigger blobs request and import") - // Should not have block request, it is cached - .expect_blobs_request() - // TODO: Should send blobs for processing - .block_imported() - .expect_no_active_lookups(); - } - - #[test] - fn parent_blob_unknown_parent() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob(1)) else { - return; - }; - tester - .expect_empty_beacon_processor() - .parent_block_response_expect_blobs() - .parent_blob_response() - .expect_block_process() - .parent_block_unknown_parent() - .expect_parent_block_request() - .expect_empty_beacon_processor(); - } - - #[test] - fn parent_blob_invalid_parent() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob(1)) else { - return; - }; - tester - .expect_empty_beacon_processor() - .parent_block_response_expect_blobs() - .parent_blob_response() - .expect_block_process() - .invalid_parent_processed() - .expect_penalty("lookup_block_processing_failure") - .expect_parent_block_request() - // blobs are not sent until block is processed - .expect_empty_beacon_processor(); - } - - #[test] - fn parent_block_and_blob_lookup_parent_returned_first_blob_trigger() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob(1)) else { - return; - }; - tester - .parent_block_response() - .expect_parent_blobs_request() - .parent_blob_response() - .expect_block_process() - .trigger_unknown_block_from_attestation() - .parent_block_imported() - .complete_current_block_and_blobs_lookup() - .expect_no_active_lookups(); - } - - #[test] - fn parent_block_then_empty_parent_blobs_blob_trigger() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob(1)) else { - return; - }; - tester - .parent_block_then_empty_parent_blobs() - .log("resolve original block trigger blobs request and import") - .complete_current_block_and_blobs_lookup() - .expect_no_active_lookups(); - } - - #[test] - fn parent_blob_unknown_parent_chain() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob(2)) else { - return; - }; - tester - .expect_empty_beacon_processor() - .parent_block_response_expect_blobs() - .parent_blob_response() - .expect_no_penalty() - .expect_block_process() - .parent_block_unknown_parent() - .expect_parent_block_request() - .expect_empty_beacon_processor() - .parent_block_response() - .expect_parent_blobs_request() - .parent_blob_response() - .expect_no_penalty() - .expect_block_process(); - } - - #[test] - fn unknown_parent_block_dup() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock(1)) else { - return; - }; - tester - .search_parent_dup() - .expect_no_blobs_request() - .expect_no_block_request(); - } - - #[test] - fn unknown_parent_blob_dup() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob(1)) else { - return; - }; - tester - .search_parent_dup() - .expect_no_blobs_request() - .expect_no_block_request(); - } - - // This test no longer applies, we don't issue requests for child lookups - // Keep for after updating rules on fetching blocks only first - #[ignore] - #[test] - fn no_peer_penalty_when_rpc_response_already_known_from_gossip() { - let Some(mut r) = TestRig::test_setup_after_deneb_before_fulu() else { - return; - }; - let (block, blobs) = r.rand_block_and_blobs(NumBlobs::Number(2)); - let block_root = block.canonical_root(); - let blob_0 = blobs[0].clone(); - let blob_1 = blobs[1].clone(); - let peer_a = r.new_connected_peer(); - let peer_b = r.new_connected_peer(); - // Send unknown parent block lookup - r.trigger_unknown_parent_block(peer_a, block.into()); - // Expect network request for blobs - let id = r.expect_blob_lookup_request(block_root); - // Peer responses with blob 0 - r.single_lookup_blob_response(id, peer_a, Some(blob_0.into())); - // Blob 1 is received via gossip unknown parent blob from a different peer - r.trigger_unknown_parent_blob(peer_b, blob_1.clone()); - // Original peer sends blob 1 via RPC - r.single_lookup_blob_response(id, peer_a, Some(blob_1.into())); - // Assert no downscore event for original peer - r.expect_no_penalty_for(peer_a); +// These `crypto_on` tests assert that the fake_crytpo feature works as expected. We run only the +// `crypto_on` tests without the fake_crypto feature and make sure that processing fails, = to +// assert that signatures and kzg proofs are checked +#[tokio::test] +async fn crypto_on_fail_with_invalid_block_signature() { + let mut r = TestRig::default(); + r.build_chain(1).await; + r.corrupt_last_block_signature(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + if cfg!(feature = "fake_crypto") { + r.assert_successful_lookup_sync(); + r.assert_no_penalties(); + } else { + r.assert_failed_lookup_sync(); + r.assert_penalties_of_type("lookup_block_processing_failure"); + } +} + +#[tokio::test] +async fn crypto_on_fail_with_bad_blob_proposer_signature() { + let Some(mut r) = TestRig::new_after_deneb_before_fulu() else { + return; + }; + r.build_chain(1).await; + r.corrupt_last_blob_proposer_signature(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + if cfg!(feature = "fake_crypto") { + r.assert_successful_lookup_sync(); + r.assert_no_penalties(); + } else { + r.assert_failed_lookup_sync(); + r.assert_penalties_of_type("lookup_blobs_processing_failure"); + } +} + +#[tokio::test] +async fn crypto_on_fail_with_bad_blob_kzg_proof() { + let Some(mut r) = TestRig::new_after_deneb_before_fulu() else { + return; + }; + r.build_chain(1).await; + r.corrupt_last_blob_kzg_proof(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + if cfg!(feature = "fake_crypto") { + r.assert_successful_lookup_sync(); + r.assert_no_penalties(); + } else { + r.assert_failed_lookup_sync(); + r.assert_penalties_of_type("lookup_blobs_processing_failure"); + } +} + +#[tokio::test] +async fn crypto_on_fail_with_bad_column_proposer_signature() { + let Some(mut r) = TestRig::new_fulu_peer_test(FuluTestType::WeSupernodeThemSupernode) else { + return; + }; + r.build_chain(1).await; + r.corrupt_last_column_proposer_signature(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + if cfg!(feature = "fake_crypto") { + r.assert_successful_lookup_sync(); + r.assert_no_penalties(); + } else { + r.assert_failed_lookup_sync(); + r.assert_penalties_of_type("lookup_custody_column_processing_failure"); + } +} + +#[tokio::test] +async fn crypto_on_fail_with_bad_column_kzg_proof() { + let Some(mut r) = TestRig::new_fulu_peer_test(FuluTestType::WeSupernodeThemSupernode) else { + return; + }; + r.build_chain(1).await; + r.corrupt_last_column_kzg_proof(); + r.trigger_with_last_block(); + r.simulate(SimulateConfig::happy_path()).await; + if cfg!(feature = "fake_crypto") { + r.assert_successful_lookup_sync(); + r.assert_no_penalties(); + } else { + r.assert_failed_lookup_sync(); + r.assert_penalties_of_type("lookup_custody_column_processing_failure"); } } diff --git a/beacon_node/network/src/sync/tests/mod.rs b/beacon_node/network/src/sync/tests/mod.rs index dcc7e3e49d..f00cf5841d 100644 --- a/beacon_node/network/src/sync/tests/mod.rs +++ b/beacon_node/network/src/sync/tests/mod.rs @@ -1,13 +1,19 @@ use crate::NetworkMessage; use crate::sync::SyncMessage; +use crate::sync::block_lookups::BlockLookupsMetrics; use crate::sync::manager::SyncManager; -use crate::sync::range_sync::RangeSyncType; +use crate::sync::tests::lookups::SimulateConfig; +use beacon_chain::block_verification_types::RpcBlock; use beacon_chain::builder::Witness; +use beacon_chain::custody_context::NodeCustodyType; use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType}; use beacon_processor::WorkEvent; -use lighthouse_network::NetworkGlobals; +use lighthouse_network::rpc::RequestType; +use lighthouse_network::service::api_types::{AppRequestId, Id}; +use lighthouse_network::{NetworkGlobals, PeerId}; use rand_chacha::ChaCha20Rng; use slot_clock::ManualSlotClock; +use std::collections::{HashMap, HashSet}; use std::fs::OpenOptions; use std::io::Write; use std::sync::{Arc, Once}; @@ -16,7 +22,7 @@ use tokio::sync::mpsc; use tracing_subscriber::fmt::MakeWriter; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; -use types::{ForkName, MinimalEthSpec as E}; +use types::{ForkName, Hash256, MinimalEthSpec as E, Slot}; mod lookups; mod range; @@ -58,6 +64,8 @@ struct TestRig { network_rx_queue: Vec>, /// Receiver for `SyncMessage` from the network sync_rx: mpsc::UnboundedReceiver>, + /// Stores all `SyncMessage`s received from `sync_rx` + sync_rx_queue: Vec>, /// To send `SyncMessage`. For sending RPC responses or block processing results to sync. sync_manager: SyncManager, /// To manipulate sync state and peer connection status @@ -68,6 +76,65 @@ struct TestRig { rng_08: rand_chacha_03::ChaCha20Rng, rng: ChaCha20Rng, fork_name: ForkName, + /// Blocks that will be used in the test but may not be known to `harness` yet. + network_blocks_by_root: HashMap>, + network_blocks_by_slot: HashMap>, + penalties: Vec, + /// All seen lookups through the test run + seen_lookups: HashMap, + /// Registry of all requests done by the test + requests: Vec<(RequestType, AppRequestId)>, + /// Persistent config on how to complete request + complete_strategy: SimulateConfig, + /// Metrics values to allow a reset + initial_block_lookups_metrics: BlockLookupsMetrics, + /// Fulu test type + fulu_test_type: FuluTestType, +} + +enum FuluTestType { + WeSupernodeThemSupernode, + WeSupernodeThemFullnodes, + WeFullnodeThemSupernode, + WeFullnodeThemFullnodes, +} + +impl FuluTestType { + fn we_node_custody_type(&self) -> NodeCustodyType { + match self { + Self::WeSupernodeThemSupernode | Self::WeSupernodeThemFullnodes => { + NodeCustodyType::Supernode + } + Self::WeFullnodeThemSupernode | Self::WeFullnodeThemFullnodes => { + NodeCustodyType::Fullnode + } + } + } + + fn them_node_custody_type(&self) -> NodeCustodyType { + match self { + Self::WeSupernodeThemSupernode | Self::WeFullnodeThemSupernode => { + NodeCustodyType::Supernode + } + Self::WeSupernodeThemFullnodes | Self::WeFullnodeThemFullnodes => { + NodeCustodyType::Fullnode + } + } + } +} + +#[derive(Debug)] +struct SeenLookup { + /// Lookup's Id + id: Id, + block_root: Hash256, + seen_peers: HashSet, +} + +#[derive(Debug)] +struct ReportedPenalty { + pub peer_id: PeerId, + pub msg: &'static str, } // Environment variable to read if `fork_from_env` feature is enabled. diff --git a/beacon_node/network/src/sync/tests/range.rs b/beacon_node/network/src/sync/tests/range.rs index 6f129bc8f0..67395ccd25 100644 --- a/beacon_node/network/src/sync/tests/range.rs +++ b/beacon_node/network/src/sync/tests/range.rs @@ -185,7 +185,7 @@ impl TestRig { } #[track_caller] - fn expect_chain_segments(&mut self, count: usize) { + fn assert_chain_segments(&mut self, count: usize) { for i in 0..count { self.pop_received_processor_event(|ev| { (ev.work_type() == beacon_processor::WorkType::ChainSegment).then_some(()) @@ -235,7 +235,7 @@ impl TestRig { panic!("Should have a BlocksByRange request, filter {request_filter:?}: {e:?}") }); - let by_range_data_requests = if self.after_fulu() { + let by_range_data_requests = if self.is_after_fulu() { let mut data_columns_requests = vec![]; while let Ok(data_columns_request) = self.pop_received_network_event(|ev| match ev { NetworkMessage::SendRequest { @@ -254,7 +254,7 @@ impl TestRig { panic!("Found zero DataColumnsByRange requests, filter {request_filter:?}"); } ByRangeDataRequestIds::PostPeerDAS(data_columns_requests) - } else if self.after_deneb() { + } else if self.is_after_deneb() { let (id, peer) = self .pop_received_network_event(|ev| match ev { NetworkMessage::SendRequest { @@ -489,7 +489,7 @@ fn build_rpc_block( fn head_chain_removed_while_finalized_syncing() { // NOTE: this is a regression test. // Added in PR https://github.com/sigp/lighthouse/pull/2821 - let mut rig = TestRig::test_setup(); + let mut rig = TestRig::default(); // Get a peer with an advanced head let head_peer = rig.add_head_peer(); @@ -514,11 +514,11 @@ fn head_chain_removed_while_finalized_syncing() { async fn state_update_while_purging() { // NOTE: this is a regression test. // Added in PR https://github.com/sigp/lighthouse/pull/2827 - let mut rig = TestRig::test_setup_with_custody_type(NodeCustodyType::SemiSupernode); + let mut rig = TestRig::with_custody_type(NodeCustodyType::SemiSupernode); // Create blocks on a separate harness // SemiSupernode ensures enough columns are stored for sampling + custody RPC block validation - let mut rig_2 = TestRig::test_setup_with_custody_type(NodeCustodyType::SemiSupernode); + let mut rig_2 = TestRig::with_custody_type(NodeCustodyType::SemiSupernode); // Need to create blocks that can be inserted into the fork-choice and fit the "known // conditions" below. let head_peer_block = rig_2.create_canonical_block().await; @@ -550,7 +550,7 @@ async fn state_update_while_purging() { #[test] fn pause_and_resume_on_ee_offline() { - let mut rig = TestRig::test_setup(); + let mut rig = TestRig::default(); // add some peers let peer1 = rig.add_head_peer(); @@ -559,7 +559,7 @@ fn pause_and_resume_on_ee_offline() { // send the response to the request rig.find_and_complete_blocks_by_range_request(filter().peer(peer1).epoch(0)); // the beacon processor shouldn't have received any work - rig.expect_empty_processor(); + rig.assert_empty_processor(); // while the ee is offline, more peers might arrive. Add a new finalized peer. let _peer2 = rig.add_finalized_peer(); @@ -570,14 +570,14 @@ fn pause_and_resume_on_ee_offline() { // epoch for the other batch. So we can either filter by epoch of by sync type. rig.find_and_complete_blocks_by_range_request(filter().epoch(0)); // the beacon processor shouldn't have received any work - rig.expect_empty_processor(); + rig.assert_empty_processor(); // make the beacon processor available again. // update_execution_engine_state implicitly calls resume // now resume range, we should have two processing requests in the beacon processor. rig.update_execution_engine_state(EngineState::Online); // The head chain and finalized chain (2) should be in the processing queue - rig.expect_chain_segments(2); + rig.assert_chain_segments(2); } /// To attempt to finalize the peer's status finalized checkpoint we synced to its finalized epoch + @@ -587,7 +587,7 @@ const EXTRA_SYNCED_EPOCHS: u64 = 2 + 1; #[test] fn finalized_sync_enough_global_custody_peers_few_chain_peers() { // Run for all forks - let mut r = TestRig::test_setup(); + let mut r = TestRig::default(); let advanced_epochs: u64 = 2; let remote_info = r.finalized_remote_info_advanced_by(advanced_epochs.into()); @@ -604,7 +604,7 @@ fn finalized_sync_enough_global_custody_peers_few_chain_peers() { #[test] fn finalized_sync_not_enough_custody_peers_on_start() { - let mut r = TestRig::test_setup(); + let mut r = TestRig::default(); // Only run post-PeerDAS if !r.fork_name.fulu_enabled() { return; @@ -621,7 +621,7 @@ fn finalized_sync_not_enough_custody_peers_on_start() { // Because we don't have enough peers on all columns we haven't sent any request. // NOTE: There's a small chance that this single peer happens to custody exactly the set we // expect, in that case the test will fail. Find a way to make the test deterministic. - r.expect_empty_network(); + r.assert_empty_network(); // Generate enough peers and supernodes to cover all custody columns let peer_count = 100; diff --git a/crypto/bls/src/impls/fake_crypto.rs b/crypto/bls/src/impls/fake_crypto.rs index e7eee05077..5fe0c3baab 100644 --- a/crypto/bls/src/impls/fake_crypto.rs +++ b/crypto/bls/src/impls/fake_crypto.rs @@ -49,7 +49,9 @@ impl TPublicKey for PublicKey { } fn serialize_uncompressed(&self) -> [u8; PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN] { - panic!("fake_crypto does not support uncompressed keys") + let mut bytes = [0; PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN]; + bytes[0..PUBLIC_KEY_BYTES_LEN].copy_from_slice(&self.0); + bytes } fn deserialize(bytes: &[u8]) -> Result { @@ -58,8 +60,17 @@ impl TPublicKey for PublicKey { Ok(pubkey) } - fn deserialize_uncompressed(_: &[u8]) -> Result { - panic!("fake_crypto does not support uncompressed keys") + fn deserialize_uncompressed(bytes: &[u8]) -> Result { + if bytes.len() == PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN { + let mut pubkey = Self([0; PUBLIC_KEY_BYTES_LEN]); + pubkey.0.copy_from_slice(&bytes[0..PUBLIC_KEY_BYTES_LEN]); + Ok(pubkey) + } else { + Err(Error::InvalidByteLength { + got: bytes.len(), + expected: PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN, + }) + } } } @@ -97,7 +108,7 @@ pub struct Signature([u8; SIGNATURE_BYTES_LEN]); impl Signature { fn infinity() -> Self { - Self([0; SIGNATURE_BYTES_LEN]) + Self(INFINITY_SIGNATURE) } } @@ -213,7 +224,11 @@ impl TSecretKey for SecretKey { } fn public_key(&self) -> PublicKey { - PublicKey::infinity() + let mut bytes = [0; PUBLIC_KEY_BYTES_LEN]; + bytes[0] = 0x01; + let to_copy = std::cmp::min(self.0.len(), bytes.len() - 1); + bytes[1..1 + to_copy].copy_from_slice(&self.0[..to_copy]); + PublicKey(bytes) } fn sign(&self, _msg: Hash256) -> Signature { diff --git a/crypto/kzg/Cargo.toml b/crypto/kzg/Cargo.toml index 5a36eb74f7..d2558663d5 100644 --- a/crypto/kzg/Cargo.toml +++ b/crypto/kzg/Cargo.toml @@ -5,6 +5,10 @@ authors = ["Pawan Dhananjay "] edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[features] +default = [] +fake_crypto = [] + [dependencies] arbitrary = { workspace = true } c-kzg = { workspace = true } diff --git a/crypto/kzg/src/lib.rs b/crypto/kzg/src/lib.rs index 0fe95b7723..66499dad8e 100644 --- a/crypto/kzg/src/lib.rs +++ b/crypto/kzg/src/lib.rs @@ -134,6 +134,9 @@ impl Kzg { kzg_commitment: KzgCommitment, kzg_proof: KzgProof, ) -> Result<(), Error> { + if cfg!(feature = "fake_crypto") { + return Ok(()); + } if !self.trusted_setup.verify_blob_kzg_proof( blob, &kzg_commitment.into(), @@ -155,6 +158,9 @@ impl Kzg { kzg_commitments: &[KzgCommitment], kzg_proofs: &[KzgProof], ) -> Result<(), Error> { + if cfg!(feature = "fake_crypto") { + return Ok(()); + } let commitments_bytes = kzg_commitments .iter() .map(|comm| Bytes48::from(*comm)) @@ -204,6 +210,9 @@ impl Kzg { y: &Bytes32, kzg_proof: KzgProof, ) -> Result { + if cfg!(feature = "fake_crypto") { + return Ok(true); + } self.trusted_setup .verify_kzg_proof(&kzg_commitment.into(), z, y, &kzg_proof.into()) .map_err(Into::into) @@ -240,6 +249,9 @@ impl Kzg { indices: Vec, kzg_commitments: &[Bytes48], ) -> Result<(), (Option, Error)> { + if cfg!(feature = "fake_crypto") { + return Ok(()); + } let mut column_groups: HashMap> = HashMap::new(); let expected_len = cells.len();