diff --git a/Cargo.lock b/Cargo.lock index 99b3d3b492..5e0520843e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3162,6 +3162,7 @@ dependencies = [ "tokio", "tracing", "url", + "utils", "vbs", "vec1", "workspace-hack", diff --git a/crates/hotshot/src/tasks/mod.rs b/crates/hotshot/src/tasks/mod.rs index 1d50c16bd7..e69ed4aaa2 100644 --- a/crates/hotshot/src/tasks/mod.rs +++ b/crates/hotshot/src/tasks/mod.rs @@ -159,14 +159,8 @@ pub fn add_network_message_task< // Wait for a message from the network message = network.recv_message().fuse() => { // Make sure the message did not fail - let message = match message { - Ok(message) => { - message - } - Err(e) => { - tracing::trace!("Failed to receive message: {:?}", e); - continue; - } + let Ok(message) = message else { + continue; }; // Deserialize the message diff --git a/crates/hotshot/src/traits/networking/memory_network.rs b/crates/hotshot/src/traits/networking/memory_network.rs index 5925a85eff..9aa8adfef2 100644 --- a/crates/hotshot/src/traits/networking/memory_network.rs +++ b/crates/hotshot/src/traits/networking/memory_network.rs @@ -309,7 +309,7 @@ impl ConnectedNetwork for MemoryNetwork { .iter() { if !recipients.contains(&node.0) { - tracing::error!("Skipping node because not in recipient list: {:?}", &node.0); + tracing::trace!("Skipping node because not in recipient list: {:?}", &node.0); continue; } // TODO delay/drop etc here diff --git a/crates/testing/Cargo.toml b/crates/testing/Cargo.toml index b05bbd9dde..bb582f3e06 100644 --- a/crates/testing/Cargo.toml +++ b/crates/testing/Cargo.toml @@ -45,6 +45,7 @@ tide-disco = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } url = { workspace = true } +utils = { path = "../utils" } vbs = { workspace = true } vec1 = { workspace = true } workspace-hack = { version = "0.1", path = "../workspace-hack" } diff --git a/crates/testing/src/consistency_task.rs b/crates/testing/src/consistency_task.rs index 5c0dc3192d..bf1bf02141 100644 --- a/crates/testing/src/consistency_task.rs +++ b/crates/testing/src/consistency_task.rs @@ -7,7 +7,7 @@ #![allow(clippy::unwrap_or_default)] use std::{collections::BTreeMap, marker::PhantomData}; -use anyhow::{bail, ensure, Context, Result}; +use async_broadcast::Sender; use async_trait::async_trait; use committable::Committable; use hotshot_example_types::block_types::TestBlockHeader; @@ -17,11 +17,13 @@ use hotshot_types::{ message::UpgradeLock, traits::node_implementation::{ConsensusTime, NodeType, Versions}, }; +use tokio::task::JoinHandle; +use utils::anytrace::*; use crate::{ overall_safety_task::OverallSafetyPropertiesDescription, test_builder::TransactionValidator, - test_task::{TestResult, TestTaskState}, + test_task::{spawn_timeout_task, TestEvent, TestResult, TestTaskState}, }; /// Map from views to leaves for a single node, allowing multiple leaves for each view (because the node may a priori send us multiple leaves for a given view). @@ -100,7 +102,12 @@ async fn validate_node_map( child .extends_upgrade(parent, &upgrade_lock.decided_upgrade_certificate) .await - .context("Leaf {child} does not extend its parent {parent}")?; + .context(|e| { + error!( + "Leaf {child:?} does not extend its parent {parent:?}: {}", + e + ) + })?; // We want to make sure the commitment matches, // but allow for the possibility that we may have skipped views in between. @@ -138,7 +145,7 @@ fn sanitize_network_map( result.insert( *node, sanitize_node_map(node_map) - .context(format!("Node {node} produced inconsistent leaves."))?, + .context(|e| error!("Node {node} produced inconsistent leaves: {}", e))?, ); } @@ -159,7 +166,7 @@ async fn invert_network_map( for (node_id, node_map) in network_map.iter() { validate_node_map::(node_map) .await - .context(format!("Node {node_id} has an invalid leaf history"))?; + .context(|e| error!("Node {node_id} has an invalid leaf history: {}", e))?; // validate each node's leaf map for (view, leaf) in node_map.iter() { @@ -186,9 +193,13 @@ fn sanitize_view_map( ensure!( node_leaves.len() <= 1, - leaf_map.iter().fold( - format!("The network does not agree on view {view:?}."), - |acc, (node, leaf)| { format!("{acc}\n\nNode {node} sent us leaf:\n\n{leaf:?}") } + error!( + "The network does not agree on the following views: {}", + leaf_map + .iter() + .fold(format!("\n\nView {view:?}:"), |acc, (node, leaf)| { + format!("{acc}\n\nNode {node} sent us leaf:\n\n{leaf:?}") + }) ) ); @@ -207,18 +218,29 @@ fn sanitize_view_map( Ok(result) } +enum TestProgress { + Incomplete, + Finished, +} + /// Data availability task state pub struct ConsistencyTask { /// A map from node ids to (leaves keyed on view number) pub consensus_leaves: NetworkMap, /// safety task requirements - pub safety_properties: OverallSafetyPropertiesDescription, + pub safety_properties: OverallSafetyPropertiesDescription, /// whether we should have seen an upgrade certificate or not pub ensure_upgrade: bool, + /// a list of errors accumulated by the task + pub errors: Vec, + /// channel used to shutdown the test + pub test_sender: Sender, /// phantom marker pub _pd: PhantomData, /// function used to validate the number of transactions committed in each block pub validate_transactions: TransactionValidator, + /// running timeout task + pub timeout_task: JoinHandle<()>, } impl, V: Versions> ConsistencyTask { @@ -228,6 +250,15 @@ impl, V: Versions> ConsistencyTas let inverted_map = invert_network_map::(&sanitized_network_map).await?; let sanitized_view_map = sanitize_view_map(&inverted_map)?; + let num_successful_views = sanitized_view_map.iter().len(); + + // check that we've succeeded in enough views + ensure!( + num_successful_views >= self.safety_properties.num_successful_views, + "Not enough successful views: expected {:?} but got {:?}", + self.safety_properties.num_successful_views, + num_successful_views, + ); let expected_upgrade = self.ensure_upgrade; let actual_upgrade = sanitized_view_map.iter().fold(false, |acc, (_view, leaf)| { @@ -253,6 +284,90 @@ impl, V: Versions> ConsistencyTas Ok(()) } + + async fn partial_validate(&self) -> Result { + self.check_view_success().await?; + self.check_view_failure().await?; + + self.check_total_successes().await + } + + fn add_error(&mut self, error: Error) { + self.errors.push(error); + } + + async fn handle_result(&mut self, result: Result) { + match result { + Ok(TestProgress::Finished) => { + let _ = self.test_sender.broadcast(TestEvent::Shutdown).await; + } + Err(e) => { + self.add_error(e); + let _ = self.test_sender.broadcast(TestEvent::Shutdown).await; + } + Ok(TestProgress::Incomplete) => {} + } + } + + async fn check_total_successes(&self) -> Result { + let sanitized_network_map = sanitize_network_map(&self.consensus_leaves)?; + + let inverted_map = invert_network_map::(&sanitized_network_map).await?; + + if inverted_map.len() >= self.safety_properties.num_successful_views { + Ok(TestProgress::Finished) + } else { + Ok(TestProgress::Incomplete) + } + } + pub async fn check_view_success(&self) -> Result<()> { + for (node_id, node_map) in self.consensus_leaves.iter() { + for (view, leaf) in node_map { + ensure!( + !self + .safety_properties + .expected_view_failures + .contains(view), + "Expected a view failure, but got a decided leaf for view {:?} from node {:?}.\n\nLeaf:\n\n{:?}", + view, + node_id, + leaf + ); + } + } + + Ok(()) + } + + pub async fn check_view_failure(&self) -> Result<()> { + let sanitized_network_map = sanitize_network_map(&self.consensus_leaves)?; + + let mut inverted_map = invert_network_map::(&sanitized_network_map).await?; + + let (current_view, _) = inverted_map + .pop_last() + .context(error!("Leaf map is empty, which should be impossible"))?; + let Some((last_view, _)) = inverted_map.pop_last() else { + // the view cannot fail if there wasn't a prior view in the map. + return Ok(()); + }; + + // filter out views we expected to (possibly) fail + let unexpected_failed_views: Vec<_> = (*(last_view + 1)..*current_view) + .filter(|view| { + !self.safety_properties.expected_view_failures.contains(view) + && !self.safety_properties.possible_view_failures.contains(view) + }) + .collect(); + + ensure!( + unexpected_failed_views.is_empty(), + "Unexpected failed views: {:?}", + unexpected_failed_views + ); + + Ok(()) + } } #[async_trait] @@ -268,23 +383,46 @@ impl, V: Versions> TestTaskState .. } = message { - let map = &mut self.consensus_leaves.entry(id).or_insert(BTreeMap::new()); + { + let mut timeout_task = spawn_timeout_task( + self.test_sender.clone(), + self.safety_properties.decide_timeout, + ); + + std::mem::swap(&mut self.timeout_task, &mut timeout_task); + + timeout_task.abort(); + } + + for leaf_info in leaf_chain.iter().rev() { + let map = &mut self.consensus_leaves.entry(id).or_insert(BTreeMap::new()); - leaf_chain.iter().for_each(|leaf_info| { map.entry(leaf_info.leaf.view_number()) .and_modify(|vec| vec.push(leaf_info.leaf.clone())) .or_insert(vec![leaf_info.leaf.clone()]); - }); + + let result = self.partial_validate().await; + + self.handle_result(result).await; + } } Ok(()) } async fn check(&self) -> TestResult { + self.timeout_task.abort(); + + let mut errors: Vec<_> = self.errors.iter().map(|e| e.to_string()).collect(); + if let Err(e) = self.validate().await { - return TestResult::Fail(Box::new(e)); + errors.push(e.to_string()); } - TestResult::Pass + if !errors.is_empty() { + TestResult::Fail(Box::new(errors)) + } else { + TestResult::Pass + } } } diff --git a/crates/testing/src/overall_safety_task.rs b/crates/testing/src/overall_safety_task.rs index c824cc19b6..32d7a01e3a 100644 --- a/crates/testing/src/overall_safety_task.rs +++ b/crates/testing/src/overall_safety_task.rs @@ -5,36 +5,14 @@ // along with the HotShot repository. If not, see . use std::{ - collections::{hash_map::Entry, HashMap, HashSet}, - sync::Arc, + collections::{HashMap, HashSet}, + time::Duration, }; -use anyhow::Result; -use async_broadcast::Sender; -use async_lock::RwLock; -use async_trait::async_trait; -use hotshot::{traits::TestableNodeImplementation, HotShotError}; -use hotshot_types::{ - data::Leaf2, - error::RoundTimedoutState, - event::{Event, EventType, LeafChain}, - simple_certificate::QuorumCertificate2, - traits::{ - block_contents::BlockHeader, - election::Membership, - node_implementation::{ConsensusTime, NodeType, Versions}, - BlockPayload, - }, - vid::VidCommitment, -}; +use hotshot_types::traits::node_implementation::NodeType; use thiserror::Error; use tracing::error; -use crate::{ - test_runner::Node, - test_task::{TestEvent, TestResult, TestTaskState}, -}; - /// convenience type alias for state and block pub type StateAndBlock = (Vec, Vec); @@ -86,588 +64,9 @@ pub enum OverallSafetyTaskErr { ViewTimeout, } -/// Data availability task state -pub struct OverallSafetyTask, V: Versions> { - /// handles - pub handles: Arc>>>, - /// ctx - pub ctx: RoundCtx, - /// configure properties - pub properties: OverallSafetyPropertiesDescription, - /// error - pub error: Option>>, - /// sender to test event channel - pub test_sender: Sender, - /// Number of blocks in an epoch, zero means there are no epochs - pub epoch_height: u64, -} - -impl, V: Versions> - OverallSafetyTask -{ - async fn handle_view_failure(&mut self, num_failed_views: usize, view_number: TYPES::View) { - let expected_views_to_fail = &mut self.properties.expected_views_to_fail; - - self.ctx.failed_views.insert(view_number); - if self.ctx.failed_views.len() > num_failed_views { - let _ = self.test_sender.broadcast(TestEvent::Shutdown).await; - self.error = Some(Box::new(OverallSafetyTaskErr::::TooManyFailures( - self.ctx.failed_views.clone(), - ))); - } else if !expected_views_to_fail.is_empty() { - match expected_views_to_fail.entry(view_number) { - Entry::Occupied(mut view_seen) => { - *view_seen.get_mut() = true; - } - Entry::Vacant(_v) => { - let _ = self.test_sender.broadcast(TestEvent::Shutdown).await; - self.error = Some(Box::new( - OverallSafetyTaskErr::::InconsistentFailedViews { - expected_failed_views: expected_views_to_fail.keys().cloned().collect(), - actual_failed_views: self.ctx.failed_views.clone(), - }, - )); - } - } - } - } -} - -#[async_trait] -impl, V: Versions> TestTaskState - for OverallSafetyTask -{ - type Event = Event; - - /// Handles an event from one of multiple receivers. - async fn handle_event(&mut self, (message, id): (Self::Event, usize)) -> Result<()> { - let memberships_arc = Arc::clone( - &self - .handles - .read() - .await - .first() - .unwrap() - .handle - .memberships, - ); - let public_key = self.handles.read().await[id].handle.public_key(); - let OverallSafetyPropertiesDescription:: { - check_leaf, - check_block, - num_failed_views, - num_successful_views, - transaction_threshold, - .. - }: OverallSafetyPropertiesDescription = self.properties.clone(); - let Event { view_number, event } = message; - let keys: Option> = match event { - EventType::Error { error } => { - let cur_epoch = self.handles.read().await[id] - .handle - .consensus() - .read() - .await - .cur_epoch(); - if !memberships_arc - .read() - .await - .has_stake(&public_key, cur_epoch) - { - // Return early, this event comes from a node not belonging to the current epoch - return Ok(()); - } - self.ctx - .insert_error_to_context(view_number, id, error.clone()); - None - } - EventType::Decide { - leaf_chain, - qc, - block_size: _, - } => { - // Skip the genesis leaf. - if leaf_chain.last().unwrap().leaf.view_number() == TYPES::View::genesis() { - return Ok(()); - } - let mut keys = Vec::default(); - let mut leaf_qc = (*qc).clone(); - let mut leaf_chain_vec = leaf_chain.to_vec(); - while !leaf_chain_vec.is_empty() { - let paired_up = (leaf_chain_vec.clone(), leaf_qc.clone()); - let leaf_info = leaf_chain_vec.first().unwrap(); - let mut txns = HashSet::new(); - if let Some(ref payload) = leaf_info.leaf.block_payload() { - for txn in payload - .transaction_commitments(leaf_info.leaf.block_header().metadata()) - { - txns.insert(txn); - } - } - let maybe_block_size = if txns.is_empty() { - None - } else { - Some(txns.len().try_into()?) - }; - match self.ctx.round_results.entry(leaf_info.leaf.view_number()) { - Entry::Occupied(mut o) => { - let entry = o.get_mut(); - let key = entry - .insert_into_result( - id, - paired_up, - maybe_block_size, - &memberships_arc, - &public_key, - self.epoch_height, - ) - .await; - keys.push(key); - } - Entry::Vacant(v) => { - let mut round_result = RoundResult::default(); - let key = round_result - .insert_into_result( - id, - paired_up, - maybe_block_size, - &memberships_arc, - &public_key, - self.epoch_height, - ) - .await; - if key.is_some() { - v.insert(round_result); - keys.push(key); - } - } - } - leaf_qc = leaf_chain_vec.first().unwrap().leaf.justify_qc(); - leaf_chain_vec.remove(0); - } - Some(keys.into_iter().flatten().collect()) - } - EventType::ReplicaViewTimeout { view_number } => { - let cur_epoch = self.handles.read().await[id] - .handle - .consensus() - .read() - .await - .cur_epoch(); - if !memberships_arc - .read() - .await - .has_stake(&public_key, cur_epoch) - { - // Return early, this event comes from a node not belonging to the current epoch - return Ok(()); - } - let error = Arc::new(HotShotError::::ViewTimedOut { - view_number, - state: RoundTimedoutState::TestCollectRoundEventsTimedOut, - }); - self.ctx.insert_error_to_context(view_number, id, error); - None - } - _ => return Ok(()), - }; - - if let Some(keys) = keys { - for key in keys { - let key_epoch = key.epoch(self.epoch_height); - let memberships_reader = memberships_arc.read().await; - let key_len = memberships_reader.total_nodes(key_epoch); - let key_threshold = memberships_reader.success_threshold(key_epoch).get() as usize; - drop(memberships_reader); - - let key_view_number = key.view_number(); - let key_view = self.ctx.round_results.get_mut(&key_view_number).unwrap(); - key_view.update_status( - key_threshold, - key_len, - &key, - check_leaf, - check_block, - transaction_threshold, - ); - match key_view.status.clone() { - ViewStatus::Ok => { - self.ctx.successful_views.insert(key_view_number); - // if a view succeeds remove it from the failed views - self.ctx.failed_views.remove(&key_view_number); - if self.ctx.successful_views.len() >= num_successful_views { - let _ = self.test_sender.broadcast(TestEvent::Shutdown).await; - } - } - ViewStatus::Failed => { - self.handle_view_failure(num_failed_views, key_view_number) - .await; - } - ViewStatus::Err(e) => { - let _ = self.test_sender.broadcast(TestEvent::Shutdown).await; - self.error = Some(Box::new(e)); - return Ok(()); - } - ViewStatus::InProgress => {} - } - } - } else { - let Some(view) = self.ctx.round_results.get_mut(&view_number) else { - return Ok(()); - }; - let cur_epoch = self.handles.read().await[id] - .handle - .consensus() - .read() - .await - .cur_epoch(); - - let memberships_reader = memberships_arc.read().await; - let len = memberships_reader.total_nodes(cur_epoch); - let threshold = memberships_reader.success_threshold(cur_epoch).get() as usize; - drop(memberships_reader); - - if view.check_if_failed(threshold, len) { - view.status = ViewStatus::Failed; - self.handle_view_failure(num_failed_views, view_number) - .await; - } - } - Ok(()) - } - - async fn check(&self) -> TestResult { - if let Some(e) = &self.error { - return TestResult::Fail(e.clone()); - } - - let OverallSafetyPropertiesDescription:: { - check_leaf: _, - check_block: _, - num_failed_views: num_failed_rounds_total, - num_successful_views, - threshold_calculator: _, - transaction_threshold: _, - expected_views_to_fail, - }: OverallSafetyPropertiesDescription = self.properties.clone(); - - let views_count = self.ctx.failed_views.len() + self.ctx.successful_views.len(); - let results_count = self.ctx.round_results.len(); - - // This can cause tests to crash if we do the subtracting to get `num_incomplete_views` below - // So lets fail return an error instead - // Use this check instead of saturating_sub as that could hide a real problem - if views_count > results_count { - return TestResult::Fail(Box::new( - OverallSafetyTaskErr::::NotEnoughRoundResults { - results_count, - views_count, - }, - )); - } - let num_incomplete_views = results_count - views_count; - - if self.ctx.successful_views.len() < num_successful_views { - return TestResult::Fail(Box::new(OverallSafetyTaskErr::::NotEnoughDecides { - got: self.ctx.successful_views.len(), - expected: num_successful_views, - })); - } - - if self.ctx.failed_views.len() + num_incomplete_views > num_failed_rounds_total { - return TestResult::Fail(Box::new(OverallSafetyTaskErr::::TooManyFailures( - self.ctx.failed_views.clone(), - ))); - } - - if !expected_views_to_fail - .values() - .all(|&view_failed| view_failed) - { - return TestResult::Fail(Box::new( - OverallSafetyTaskErr::::InconsistentFailedViews { - actual_failed_views: self.ctx.failed_views.clone(), - expected_failed_views: expected_views_to_fail.keys().cloned().collect(), - }, - )); - } - - // We should really be able to include a check like this: - // - // if self.ctx.failed_views.len() < num_failed_rounds_total { - // return TestResult::Fail(Box::new(OverallSafetyTaskErr::::NotEnoughFailures { - // expected: num_failed_rounds_total, - // failed_views: self.ctx.failed_views.clone(), - // })); - // } - // - // but we have several tests where it's not possible to fail pin down an exact number of failures (just from async timing issues, if nothing else). Ideally, we should refactor some of the failure count logic for this. - - TestResult::Pass - } -} - -/// Result of running a round of consensus -#[derive(Debug)] -pub struct RoundResult { - /// Transactions that were submitted - // pub txns: Vec, - - /// Nodes that committed this round - /// id -> (leaf, qc) - success_nodes: HashMap, QuorumCertificate2)>, - - /// Nodes that failed to commit this round - pub failed_nodes: HashMap>>, - - /// whether or not the round succeeded (for a custom defn of succeeded) - pub status: ViewStatus, - - /// NOTE: technically a map is not needed - /// left one anyway for ease of viewing - /// leaf -> # entries decided on that leaf - pub leaf_map: HashMap, usize>, - - /// block -> # entries decided on that block - pub block_map: HashMap, - - /// number of transactions -> number of nodes reporting that number - pub num_txns_map: HashMap, -} - -impl Default for RoundResult { - fn default() -> Self { - Self { - success_nodes: HashMap::default(), - failed_nodes: HashMap::default(), - leaf_map: HashMap::default(), - block_map: HashMap::default(), - num_txns_map: HashMap::default(), - status: ViewStatus::InProgress, - } - } -} - -/// smh my head I shouldn't need to implement this -/// Rust doesn't realize I doesn't need to implement default -impl Default for RoundCtx { - fn default() -> Self { - Self { - round_results: HashMap::default(), - failed_views: HashSet::default(), - successful_views: HashSet::default(), - } - } -} - -/// context for a round -/// TODO eventually we want these to just be futures -/// that we poll when things are event driven -/// this context will be passed around -#[derive(Debug)] -pub struct RoundCtx { - /// results from previous rounds - /// view number -> round result - pub round_results: HashMap>, - /// during the run view refactor - pub failed_views: HashSet, - /// successful views - pub successful_views: HashSet, -} - -impl RoundCtx { - /// inserts an error into the context - pub fn insert_error_to_context( - &mut self, - view_number: TYPES::View, - idx: usize, - error: Arc>, - ) { - match self.round_results.entry(view_number) { - Entry::Occupied(mut o) => match o.get_mut().failed_nodes.entry(idx as u64) { - Entry::Occupied(mut o2) => { - *o2.get_mut() = error; - } - Entry::Vacant(v) => { - v.insert(error); - } - }, - Entry::Vacant(v) => { - let mut round_result = RoundResult::default(); - round_result.failed_nodes.insert(idx as u64, error); - v.insert(round_result); - } - } - } -} - -impl RoundResult { - /// insert into round result - #[allow(clippy::unit_arg)] - pub async fn insert_into_result( - &mut self, - idx: usize, - result: (LeafChain, QuorumCertificate2), - maybe_block_size: Option, - membership: &Arc>, - public_key: &TYPES::SignatureKey, - epoch_height: u64, - ) -> Option> { - let maybe_leaf = result.0.first(); - if let Some(leaf_info) = maybe_leaf { - let leaf = &leaf_info.leaf; - let epoch = leaf.epoch(epoch_height); - if !membership.read().await.has_stake(public_key, epoch) { - // The node doesn't belong to the epoch, don't count towards total successes count - return None; - } - if self.success_nodes.contains_key(&(idx as u64)) { - // The success of this node was previously counted, don't continue - return None; - } - self.success_nodes.insert(idx as u64, result.clone()); - match self.leaf_map.entry(leaf.clone()) { - std::collections::hash_map::Entry::Occupied(mut o) => { - *o.get_mut() += 1; - } - std::collections::hash_map::Entry::Vacant(v) => { - v.insert(1); - } - } - - let payload_commitment = leaf.payload_commitment(); - - match self.block_map.entry(payload_commitment) { - std::collections::hash_map::Entry::Occupied(mut o) => { - *o.get_mut() += 1; - } - std::collections::hash_map::Entry::Vacant(v) => { - v.insert(1); - } - } - - if let Some(num_txns) = maybe_block_size { - match self.num_txns_map.entry(num_txns) { - Entry::Occupied(mut o) => { - *o.get_mut() += 1; - } - Entry::Vacant(v) => { - v.insert(1); - } - } - } - return Some(leaf.clone()); - } - None - } - - /// check if the test failed due to not enough nodes getting through enough views - pub fn check_if_failed(&mut self, threshold: usize, total_num_nodes: usize) -> bool { - let num_failed = self.failed_nodes.len(); - total_num_nodes - num_failed < threshold - } - /// determines whether or not the round passes - /// also do a safety check - #[allow(clippy::too_many_arguments, clippy::let_unit_value)] - pub fn update_status( - &mut self, - threshold: usize, - total_num_nodes: usize, - key: &Leaf2, - check_leaf: bool, - check_block: bool, - transaction_threshold: u64, - ) { - let num_decided = self.success_nodes.len(); - let num_failed = self.failed_nodes.len(); - - if check_leaf && self.leaf_map.len() != 1 { - let (quorum_leaf, count) = self - .leaf_map - .iter() - .max_by(|(_, v), (_, other_val)| v.cmp(other_val)) - .unwrap(); - if *count >= threshold { - for leaf in self.leaf_map.keys() { - if leaf.view_number() > quorum_leaf.view_number() { - error!("LEAF MAP (that is mismatched) IS: {:?}", self.leaf_map); - self.status = ViewStatus::Err(OverallSafetyTaskErr::MismatchedLeaf); - return; - } - } - } - } - - if check_block && self.block_map.len() != 1 { - self.status = ViewStatus::Err(OverallSafetyTaskErr::InconsistentBlocks); - error!("Check blocks failed. Block map IS: {:?}", self.block_map); - return; - } - - if transaction_threshold >= 1 { - if self.num_txns_map.len() > 1 { - self.status = ViewStatus::Err(OverallSafetyTaskErr::InconsistentTxnsNum { - map: self.num_txns_map.clone(), - }); - return; - } - if let Some((n_txn, _)) = self.num_txns_map.iter().last() { - if *n_txn < transaction_threshold { - tracing::error!("not enough transactions for view {:?}", key.view_number()); - self.status = ViewStatus::Failed; - return; - } - } - } - - // check for success - if num_decided >= threshold { - // decide on if we've succeeded. - // if so, set state and return - // if not, return error - // if neither, continue through - - let block_key = key.payload_commitment(); - - if *self.block_map.get(&block_key).unwrap() == threshold - && *self.leaf_map.get(key).unwrap() == threshold - { - self.status = ViewStatus::Ok; - return; - } - } - - let is_success_possible = total_num_nodes - num_failed >= threshold; - if !is_success_possible { - self.status = ViewStatus::Failed; - } - } - - /// generate leaves - #[must_use] - pub fn gen_leaves(&self) -> HashMap, usize> { - let mut leaves = HashMap::, usize>::new(); - - for (leaf_vec, _) in self.success_nodes.values() { - let most_recent_leaf = leaf_vec.iter().last(); - if let Some(leaf_info) = most_recent_leaf { - match leaves.entry(leaf_info.leaf.clone()) { - std::collections::hash_map::Entry::Occupied(mut o) => { - *o.get_mut() += 1; - } - std::collections::hash_map::Entry::Vacant(v) => { - v.insert(1); - } - } - } - } - leaves - } -} - /// cross node safety properties -#[derive(Clone)] -pub struct OverallSafetyPropertiesDescription { +#[derive(Clone, Debug)] +pub struct OverallSafetyPropertiesDescription { /// required number of successful views pub num_successful_views: usize, /// whether or not to check the leaf @@ -679,39 +78,26 @@ pub struct OverallSafetyPropertiesDescription { /// if n > 0, check that at least n transactions are decided upon if such information /// is available pub transaction_threshold: u64, - /// num of total rounds allowed to fail - pub num_failed_views: usize, - /// threshold calculator. Given number of live and total nodes, provide number of successes - /// required to mark view as successful - pub threshold_calculator: Arc usize + Send + Sync>, - /// pass in the views that we expect to fail - pub expected_views_to_fail: HashMap, -} - -impl std::fmt::Debug for OverallSafetyPropertiesDescription { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("OverallSafetyPropertiesDescription") - .field("num successful views", &self.num_successful_views) - .field("check leaf", &self.check_leaf) - .field("check_block", &self.check_block) - .field("num_failed_rounds_total", &self.num_failed_views) - .field("transaction_threshold", &self.transaction_threshold) - .field("expected views to fail", &self.expected_views_to_fail) - .finish_non_exhaustive() - } + /// pass in the views that we expect to fail. + /// + /// the test should fail if any view on this list succeeds. + pub expected_view_failures: Vec, + /// pass in the views that may or may not fail. + pub possible_view_failures: Vec, + /// how long to wait between external events before timing out the test + pub decide_timeout: Duration, } -impl Default for OverallSafetyPropertiesDescription { +impl Default for OverallSafetyPropertiesDescription { fn default() -> Self { Self { num_successful_views: 50, check_leaf: false, check_block: true, - num_failed_views: 0, transaction_threshold: 0, - // very strict - threshold_calculator: Arc::new(|_num_live, num_total| 2 * num_total / 3 + 1), - expected_views_to_fail: HashMap::new(), + expected_view_failures: vec![], + possible_view_failures: vec![], + decide_timeout: Duration::from_secs(4), } } } diff --git a/crates/testing/src/spinning_task.rs b/crates/testing/src/spinning_task.rs index 17f783a008..088586712f 100644 --- a/crates/testing/src/spinning_task.rs +++ b/crates/testing/src/spinning_task.rs @@ -9,7 +9,6 @@ use std::{ sync::Arc, }; -use anyhow::Result; use async_broadcast::broadcast; use async_lock::RwLock; use async_trait::async_trait; @@ -38,6 +37,7 @@ use hotshot_types::{ vote::HasViewNumber, ValidatorConfig, }; +use utils::anytrace::*; use crate::{ test_launcher::Network, diff --git a/crates/testing/src/test_builder.rs b/crates/testing/src/test_builder.rs index b2182e4d34..f6b042f3cd 100644 --- a/crates/testing/src/test_builder.rs +++ b/crates/testing/src/test_builder.rs @@ -6,7 +6,6 @@ use std::{collections::HashMap, num::NonZeroUsize, rc::Rc, sync::Arc, time::Duration}; -use anyhow::{ensure, Result}; use async_lock::RwLock; use hotshot::{ tasks::EventTransformerState, @@ -24,6 +23,7 @@ use hotshot_types::{ HotShotConfig, PeerConfig, ValidatorConfig, }; use tide_disco::Url; +use utils::anytrace::*; use vec1::Vec1; use super::{ @@ -125,7 +125,7 @@ pub struct TestDescription, V: Ver /// `HotShotInitializer::from_reload` in the spinning task. pub skip_late: bool, /// overall safety property description - pub overall_safety_properties: OverallSafetyPropertiesDescription, + pub overall_safety_properties: OverallSafetyPropertiesDescription, /// spinning properties pub spinning_properties: SpinningTaskDescription, /// txns timing @@ -354,14 +354,9 @@ impl, V: Versions> TestDescription let num_nodes_with_stake = 100; Self { - overall_safety_properties: OverallSafetyPropertiesDescription:: { + overall_safety_properties: OverallSafetyPropertiesDescription { num_successful_views: 50, - check_leaf: true, - check_block: true, - num_failed_views: 15, - transaction_threshold: 0, - threshold_calculator: Arc::new(|_active, total| (2 * total / 3 + 1)), - expected_views_to_fail: HashMap::new(), + ..OverallSafetyPropertiesDescription::default() }, timing_data: TimingData { next_view_timeout: 2000, @@ -378,14 +373,9 @@ impl, V: Versions> TestDescription pub fn default_multiple_rounds() -> Self { let num_nodes_with_stake = 10; TestDescription:: { - overall_safety_properties: OverallSafetyPropertiesDescription:: { + overall_safety_properties: OverallSafetyPropertiesDescription { num_successful_views: 20, - check_leaf: true, - check_block: true, - num_failed_views: 8, - transaction_threshold: 0, - threshold_calculator: Arc::new(|_active, total| (2 * total / 3 + 1)), - expected_views_to_fail: HashMap::new(), + ..OverallSafetyPropertiesDescription::default() }, timing_data: TimingData { ..TimingData::default() diff --git a/crates/testing/src/test_runner.rs b/crates/testing/src/test_runner.rs index 618ffa49e9..545004f4d2 100644 --- a/crates/testing/src/test_runner.rs +++ b/crates/testing/src/test_runner.rs @@ -46,10 +46,7 @@ use tokio::{spawn, task::JoinHandle}; use tracing::info; use super::{ - completion_task::CompletionTask, - consistency_task::ConsistencyTask, - overall_safety_task::{OverallSafetyTask, RoundCtx}, - txn_task::TxnTask, + completion_task::CompletionTask, consistency_task::ConsistencyTask, txn_task::TxnTask, }; use crate::{ block_builder::{BuilderTask, TestBuilderImplementation}, @@ -57,7 +54,7 @@ use crate::{ spinning_task::{ChangeNode, NodeAction, SpinningTask}, test_builder::create_test_handle, test_launcher::{Network, TestLauncher}, - test_task::{TestResult, TestTask}, + test_task::{spawn_timeout_task, TestResult, TestTask}, txn_task::TxnTaskDescription, view_sync_task::ViewSyncTask, }; @@ -205,21 +202,18 @@ where event_rxs.clone(), test_receiver.clone(), ); - // add safety task - let overall_safety_task_state = OverallSafetyTask { - handles: Arc::clone(&handles), - epoch_height: launcher.metadata.test_config.epoch_height, - ctx: RoundCtx::default(), - properties: launcher.metadata.overall_safety_properties.clone(), - error: None, - test_sender, - }; let consistency_task_state = ConsistencyTask { consensus_leaves: BTreeMap::new(), - safety_properties: launcher.metadata.overall_safety_properties, + safety_properties: launcher.metadata.overall_safety_properties.clone(), + test_sender: test_sender.clone(), + errors: vec![], ensure_upgrade: launcher.metadata.upgrade_view.is_some(), validate_transactions: launcher.metadata.validate_transactions, + timeout_task: spawn_timeout_task( + test_sender.clone(), + launcher.metadata.overall_safety_properties.decide_timeout, + ), _pd: PhantomData, }; @@ -229,12 +223,6 @@ where test_receiver.clone(), ); - let overall_safety_task = TestTask::>::new( - overall_safety_task_state, - event_rxs.clone(), - test_receiver.clone(), - ); - // add view sync task let view_sync_task_state = ViewSyncTask { hit_view_sync: HashSet::new(), @@ -273,7 +261,6 @@ where task_futs.push(task.run()); } - task_futs.push(overall_safety_task.run()); task_futs.push(consistency_task.run()); task_futs.push(view_sync_task.run()); task_futs.push(spinning_task.run()); diff --git a/crates/testing/src/test_task.rs b/crates/testing/src/test_task.rs index fd51a5a7e0..4abe39c01f 100644 --- a/crates/testing/src/test_task.rs +++ b/crates/testing/src/test_task.rs @@ -6,7 +6,6 @@ use std::{num::NonZeroUsize, sync::Arc, time::Duration}; -use anyhow::Result; use async_broadcast::{Receiver, Sender}; use async_lock::RwLock; use async_trait::async_trait; @@ -29,6 +28,7 @@ use tokio::{ time::{sleep, timeout}, }; use tracing::error; +use utils::anytrace::*; use crate::test_runner::Node; @@ -40,6 +40,14 @@ pub enum TestResult { Fail(Box), } +pub fn spawn_timeout_task(test_sender: Sender, timeout: Duration) -> JoinHandle<()> { + tokio::spawn(async move { + sleep(timeout).await; + + let _ = test_sender.broadcast(TestEvent::Shutdown).await; + }) +} + #[async_trait] /// Type for mutable task state that can be used as the state for a `Task` pub trait TestTaskState: Send { diff --git a/crates/testing/src/view_sync_task.rs b/crates/testing/src/view_sync_task.rs index 914c8279cd..b0ff45dbdd 100644 --- a/crates/testing/src/view_sync_task.rs +++ b/crates/testing/src/view_sync_task.rs @@ -6,11 +6,11 @@ use std::{collections::HashSet, marker::PhantomData, sync::Arc}; -use anyhow::Result; use async_trait::async_trait; use hotshot_task_impls::events::HotShotEvent; use hotshot_types::traits::node_implementation::{NodeType, TestableNodeImplementation}; use thiserror::Error; +use utils::anytrace::*; use crate::test_task::{TestResult, TestTaskState}; diff --git a/crates/testing/tests/tests_1/libp2p.rs b/crates/testing/tests/tests_1/libp2p.rs index e0886b2c59..a183d09d5b 100644 --- a/crates/testing/tests/tests_1/libp2p.rs +++ b/crates/testing/tests/tests_1/libp2p.rs @@ -82,8 +82,8 @@ async fn libp2p_network_failures_2() { metadata.spinning_properties = SpinningTaskDescription { node_changes: vec![(3, dead_nodes)], }; - // 2 nodes fail triggering view sync, expect no other timeouts - metadata.overall_safety_properties.num_failed_views = 1; + metadata.overall_safety_properties.expected_view_failures = vec![10, 11]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(12); // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 15; diff --git a/crates/testing/tests/tests_1/test_success.rs b/crates/testing/tests/tests_1/test_success.rs index c4ca20b9de..3dfcf9b407 100644 --- a/crates/testing/tests/tests_1/test_success.rs +++ b/crates/testing/tests/tests_1/test_success.rs @@ -62,7 +62,6 @@ cross_tests!( }; metadata.test_config.epoch_height = 0; - metadata.overall_safety_properties.num_failed_views = 0; metadata.overall_safety_properties.num_successful_views = 0; let mut config = DelayConfig::default(); let delay_settings = DelaySettings { @@ -95,7 +94,6 @@ cross_tests!( }; metadata.test_config.epoch_height = 0; - metadata.overall_safety_properties.num_failed_views = 0; metadata.overall_safety_properties.num_successful_views = 10; let mut config = DelayConfig::default(); let mut delay_settings = DelaySettings { @@ -129,8 +127,6 @@ cross_tests!( metadata.test_config.epoch_height = 0; metadata.test_config.num_bootstrap = 10; - metadata.overall_safety_properties.num_failed_views = 0; - metadata.view_sync_properties = ViewSyncTaskDescription::Threshold(0, 0); metadata diff --git a/crates/testing/tests/tests_1/test_with_failures_2.rs b/crates/testing/tests/tests_1/test_with_failures_2.rs index f9c7f860b5..4354e1cb7f 100644 --- a/crates/testing/tests/tests_1/test_with_failures_2.rs +++ b/crates/testing/tests/tests_1/test_with_failures_2.rs @@ -6,7 +6,7 @@ // TODO: Remove this after integration #![allow(unused_imports)] -use std::collections::HashMap; +use std::{collections::HashMap, time::Duration}; use hotshot_example_types::{ node_types::{ @@ -62,10 +62,10 @@ cross_tests!( node_changes: vec![(5, dead_nodes)] }; - // 2 nodes fail triggering view sync, expect no other timeouts - metadata.overall_safety_properties.num_failed_views = 2; + metadata.overall_safety_properties.expected_view_failures = vec![9,10,11]; // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 13; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(20); metadata } @@ -97,13 +97,13 @@ cross_tests!( node_changes: vec![(view_spin_node_down, dead_nodes)] }; - // node 3 is leader twice when we shut down - metadata.overall_safety_properties.num_failed_views = 2; - metadata.overall_safety_properties.expected_views_to_fail = HashMap::from([ + metadata.overall_safety_properties.expected_view_failures = vec![ // next views after turning node off - (ViewNumber::new(view_spin_node_down + 1), false), - (ViewNumber::new(view_spin_node_down + 2), false) - ]); + view_spin_node_down, + view_spin_node_down + 1, + view_spin_node_down + 2 + ]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(24); // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 13; diff --git a/crates/testing/tests/tests_2/catchup.rs b/crates/testing/tests/tests_2/catchup.rs index 51b6459d90..4ac6630613 100644 --- a/crates/testing/tests/tests_2/catchup.rs +++ b/crates/testing/tests/tests_2/catchup.rs @@ -53,7 +53,7 @@ async fn test_catchup() { metadata.spinning_properties = SpinningTaskDescription { // Start the nodes before their leadership. - node_changes: vec![(13, catchup_node)], + node_changes: vec![(10, catchup_node)], }; metadata.completion_task_description = @@ -65,7 +65,7 @@ async fn test_catchup() { metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, - num_failed_views: 0, + expected_view_failures: vec![], ..Default::default() }; @@ -117,7 +117,6 @@ async fn test_catchup_cdn() { }, ); metadata.overall_safety_properties = OverallSafetyPropertiesDescription { - num_failed_views: 0, ..Default::default() }; @@ -171,7 +170,6 @@ async fn test_catchup_one_node() { metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, - num_failed_views: 0, ..Default::default() }; @@ -231,7 +229,6 @@ async fn test_catchup_in_view_sync() { }, ); metadata.overall_safety_properties = OverallSafetyPropertiesDescription { - num_failed_views: 0, ..Default::default() }; @@ -280,7 +277,7 @@ async fn test_catchup_reload() { metadata.spinning_properties = SpinningTaskDescription { // Start the nodes before their leadership. - node_changes: vec![(13, catchup_node)], + node_changes: vec![(10, catchup_node)], }; metadata.completion_task_description = @@ -292,6 +289,7 @@ async fn test_catchup_reload() { metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, + expected_view_failures: vec![], ..Default::default() }; @@ -342,7 +340,9 @@ cross_tests!( metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, - num_failed_views: 15, + expected_view_failures: vec![13], + possible_view_failures: vec![12, 14], + decide_timeout: Duration::from_secs(20), ..Default::default() }; @@ -395,7 +395,9 @@ cross_tests!( metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, - num_failed_views: 15, + expected_view_failures: vec![13], + possible_view_failures: vec![12, 14], + decide_timeout: Duration::from_secs(20), ..Default::default() }; @@ -454,7 +456,9 @@ cross_tests!( metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, - num_failed_views: 30, + expected_view_failures: vec![12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34], + possible_view_failures: vec![35], + decide_timeout: Duration::from_secs(120), ..Default::default() }; diff --git a/crates/testing/tests/tests_3/byzantine_tests.rs b/crates/testing/tests/tests_3/byzantine_tests.rs index 68f1ad3a5c..a0d4599444 100644 --- a/crates/testing/tests/tests_3/byzantine_tests.rs +++ b/crates/testing/tests/tests_3/byzantine_tests.rs @@ -1,9 +1,4 @@ -use std::{ - collections::{HashMap, HashSet}, - rc::Rc, - sync::Arc, - time::Duration, -}; +use std::{collections::HashSet, rc::Rc, sync::Arc, time::Duration}; use async_lock::RwLock; use hotshot_example_types::{ @@ -21,13 +16,8 @@ use hotshot_testing::{ test_builder::{Behaviour, TestDescription}, }; use hotshot_types::{ - data::ViewNumber, message::{GeneralConsensusMessage, MessageKind, SequencingMessage}, - traits::{ - election::Membership, - network::TransmitType, - node_implementation::{ConsensusTime, NodeType}, - }, + traits::{election::Membership, network::TransmitType, node_implementation::NodeType}, vote::HasViewNumber, }; @@ -84,7 +74,9 @@ cross_tests!( }.set_num_nodes(12,12); metadata.test_config.epoch_height = 0; - metadata.overall_safety_properties.num_failed_views = 15; + metadata.overall_safety_properties.num_successful_views = 10; + metadata.overall_safety_properties.expected_view_failures = vec![3, 4]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(12); metadata }, ); @@ -122,11 +114,9 @@ cross_tests!( }.set_num_nodes(5,5); metadata.test_config.epoch_height = 0; - metadata.overall_safety_properties.num_failed_views = 2; - metadata.overall_safety_properties.expected_views_to_fail = HashMap::from([ - (ViewNumber::new(7), false), - (ViewNumber::new(12), false) - ]); + metadata.overall_safety_properties.expected_view_failures = vec![6, 7, 11, 12]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(20); + metadata }, ); @@ -245,10 +235,8 @@ cross_tests!( }.set_num_nodes(10,10); metadata.test_config.epoch_height = 0; - metadata.overall_safety_properties.num_failed_views = 1; - metadata.overall_safety_properties.expected_views_to_fail = HashMap::from([ - (ViewNumber::new(14), false), - ]); + metadata.overall_safety_properties.expected_view_failures = vec![13, 14]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(12); metadata }, ); diff --git a/crates/testing/tests/tests_3/test_with_failures_half_f.rs b/crates/testing/tests/tests_3/test_with_failures_half_f.rs index 71eb77ab4f..5bb264e456 100644 --- a/crates/testing/tests/tests_3/test_with_failures_half_f.rs +++ b/crates/testing/tests/tests_3/test_with_failures_half_f.rs @@ -4,6 +4,8 @@ // You should have received a copy of the MIT License // along with the HotShot repository. If not, see . +use std::time::Duration; + use hotshot_example_types::{ node_types::{Libp2pImpl, MemoryImpl, PushCdnImpl, TestVersions}, state_types::TestTypes, @@ -47,9 +49,10 @@ cross_tests!( node_changes: vec![(5, dead_nodes)] }; - metadata.overall_safety_properties.num_failed_views = 3; + metadata.overall_safety_properties.expected_view_failures = vec![16, 17, 18, 19]; // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 22; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(20); metadata } ); diff --git a/crates/testing/tests/tests_4/byzantine_tests.rs b/crates/testing/tests/tests_4/byzantine_tests.rs index d9173aebd3..3823f6c9f8 100644 --- a/crates/testing/tests/tests_4/byzantine_tests.rs +++ b/crates/testing/tests/tests_4/byzantine_tests.rs @@ -48,9 +48,8 @@ // let num_nodes_with_stake = 15; // metadata.num_nodes_with_stake = num_nodes_with_stake; // metadata.da_staked_committee_size = num_nodes_with_stake; -// metadata.overall_safety_properties.num_failed_views = 20; // metadata.overall_safety_properties.num_successful_views = 20; -// metadata.overall_safety_properties.expected_views_to_fail = HashMap::from([ +// metadata.overall_safety_properties.expected_view_failures = HashMap::from([ // (ViewNumber::new(6), false), // (ViewNumber::new(10), false), // (ViewNumber::new(14), false), diff --git a/crates/testing/tests/tests_4/test_with_failures_f.rs b/crates/testing/tests/tests_4/test_with_failures_f.rs index 1a2467ad6b..dbfa66df00 100644 --- a/crates/testing/tests/tests_4/test_with_failures_f.rs +++ b/crates/testing/tests/tests_4/test_with_failures_f.rs @@ -4,6 +4,8 @@ // You should have received a copy of the MIT License // along with the HotShot repository. If not, see . +use std::time::Duration; + use hotshot_example_types::{ node_types::{Libp2pImpl, MemoryImpl, PushCdnImpl, TestVersions}, state_types::TestTypes, @@ -24,9 +26,10 @@ cross_tests!( Metadata: { let mut metadata = TestDescription::default_more_nodes(); metadata.test_config.epoch_height = 0; - metadata.overall_safety_properties.num_failed_views = 6; + metadata.overall_safety_properties.expected_view_failures = vec![13, 14, 15, 16, 17, 18, 19]; // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 20; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(60); metadata.test_config.num_bootstrap = 14; // The first 14 (i.e., 20 - f) nodes are in the DA committee and we may shutdown the // remaining 6 (i.e., f) nodes. We could remove this restriction after fixing the diff --git a/crates/testing/tests/tests_5/broken_3_chain.rs b/crates/testing/tests/tests_5/broken_3_chain.rs index e785e0ae42..4ce03b0e9c 100644 --- a/crates/testing/tests/tests_5/broken_3_chain.rs +++ b/crates/testing/tests/tests_5/broken_3_chain.rs @@ -56,9 +56,9 @@ async fn broken_3_chain() { metadata.num_nodes_with_stake = 10; metadata.da_staked_committee_size = 10; metadata.start_nodes = 10; - metadata.overall_safety_properties.num_failed_views = 100; // Check whether we see at least 10 decides metadata.overall_safety_properties.num_successful_views = 10; + metadata.overall_safety_properties.expected_view_failures = vec![2, 3, 5, 6, 8, 9]; metadata .gen_launcher(0) diff --git a/crates/testing/tests/tests_5/combined_network.rs b/crates/testing/tests/tests_5/combined_network.rs index 57bff51daa..1d185cb194 100644 --- a/crates/testing/tests/tests_5/combined_network.rs +++ b/crates/testing/tests/tests_5/combined_network.rs @@ -33,7 +33,6 @@ async fn test_combined_network() { ..Default::default() }, overall_safety_properties: OverallSafetyPropertiesDescription { - num_failed_views: 0, num_successful_views: 25, ..Default::default() }, @@ -66,7 +65,6 @@ async fn test_combined_network_cdn_crash() { ..Default::default() }, overall_safety_properties: OverallSafetyPropertiesDescription { - num_failed_views: 0, num_successful_views: 35, ..Default::default() }, @@ -114,7 +112,6 @@ async fn test_combined_network_reup() { ..Default::default() }, overall_safety_properties: OverallSafetyPropertiesDescription { - num_failed_views: 0, num_successful_views: 35, ..Default::default() }, @@ -166,7 +163,6 @@ async fn test_combined_network_half_dc() { ..Default::default() }, overall_safety_properties: OverallSafetyPropertiesDescription { - num_failed_views: 0, num_successful_views: 35, ..Default::default() }, diff --git a/crates/testing/tests/tests_5/push_cdn.rs b/crates/testing/tests/tests_5/push_cdn.rs index 5d569da0c2..48960db33d 100644 --- a/crates/testing/tests/tests_5/push_cdn.rs +++ b/crates/testing/tests/tests_5/push_cdn.rs @@ -28,7 +28,6 @@ async fn push_cdn_network() { ..Default::default() }, overall_safety_properties: OverallSafetyPropertiesDescription { - num_failed_views: 0, num_successful_views: 35, ..Default::default() }, diff --git a/crates/testing/tests/tests_5/test_with_failures.rs b/crates/testing/tests/tests_5/test_with_failures.rs index 45ff52948b..1fb3f1f636 100644 --- a/crates/testing/tests/tests_5/test_with_failures.rs +++ b/crates/testing/tests/tests_5/test_with_failures.rs @@ -4,6 +4,8 @@ // You should have received a copy of the MIT License // along with the HotShot repository. If not, see . +use std::time::Duration; + use hotshot_example_types::{ node_types::{Libp2pImpl, MemoryImpl, PushCdnImpl, TestVersions}, state_types::TestTypes, @@ -37,7 +39,8 @@ cross_tests!( metadata.spinning_properties = SpinningTaskDescription { node_changes: vec![(5, dead_nodes)] }; - metadata.overall_safety_properties.num_failed_views = 1; + metadata.overall_safety_properties.expected_view_failures = vec![18,19]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(16); metadata.overall_safety_properties.num_successful_views = 25; metadata } diff --git a/crates/testing/tests/tests_5/timeout.rs b/crates/testing/tests/tests_5/timeout.rs index 19fed7605d..2f674a6e1c 100644 --- a/crates/testing/tests/tests_5/timeout.rs +++ b/crates/testing/tests/tests_5/timeout.rs @@ -37,8 +37,8 @@ async fn test_timeout() { metadata.timing_data = timing_data; metadata.overall_safety_properties = OverallSafetyPropertiesDescription { - num_failed_views: 4, - num_successful_views: 25, + expected_view_failures: vec![9, 10, 19, 20], + num_successful_views: 20, ..Default::default() }; @@ -95,7 +95,6 @@ async fn test_timeout_libp2p() { metadata.timing_data = timing_data; metadata.overall_safety_properties = OverallSafetyPropertiesDescription { - num_failed_views: 25, num_successful_views: 25, ..Default::default() }; diff --git a/crates/testing/tests/tests_5/unreliable_network.rs b/crates/testing/tests/tests_5/unreliable_network.rs index d9a3701166..fa5e7b67d8 100644 --- a/crates/testing/tests/tests_5/unreliable_network.rs +++ b/crates/testing/tests/tests_5/unreliable_network.rs @@ -94,7 +94,6 @@ async fn libp2p_network_async() { let mut metadata: TestDescription = TestDescription { overall_safety_properties: OverallSafetyPropertiesDescription { check_leaf: true, - num_failed_views: 50, ..Default::default() }, completion_task_description: CompletionTaskDescription::TimeBasedCompletionTaskBuilder( @@ -142,7 +141,6 @@ async fn test_memory_network_async() { let mut metadata: TestDescription = TestDescription { overall_safety_properties: OverallSafetyPropertiesDescription { check_leaf: true, - num_failed_views: 5000, ..Default::default() }, // allow more time to pass in CI @@ -189,7 +187,6 @@ async fn test_memory_network_partially_sync() { let mut metadata: TestDescription = TestDescription { overall_safety_properties: OverallSafetyPropertiesDescription { - num_failed_views: 0, ..Default::default() }, // allow more time to pass in CI @@ -235,7 +232,6 @@ async fn libp2p_network_partially_sync() { let mut metadata: TestDescription = TestDescription { overall_safety_properties: OverallSafetyPropertiesDescription { - num_failed_views: 0, ..Default::default() }, completion_task_description: CompletionTaskDescription::TimeBasedCompletionTaskBuilder( diff --git a/crates/testing/tests/tests_6/test_epochs.rs b/crates/testing/tests/tests_6/test_epochs.rs index 595a02da7f..67cace8ce3 100644 --- a/crates/testing/tests/tests_6/test_epochs.rs +++ b/crates/testing/tests/tests_6/test_epochs.rs @@ -4,7 +4,7 @@ // You should have received a copy of the MIT License // along with the HotShot repository. If not, see . -use std::{collections::HashMap, time::Duration}; +use std::time::Duration; use hotshot_example_types::{ node_types::{ @@ -24,7 +24,6 @@ use hotshot_testing::{ test_builder::{TestDescription, TimingData}, view_sync_task::ViewSyncTaskDescription, }; -use hotshot_types::{data::ViewNumber, traits::node_implementation::ConsensusTime}; cross_tests!( TestName: test_success_with_epochs, @@ -95,8 +94,7 @@ cross_tests!( }; metadata.test_config.epoch_height = 10; - metadata.overall_safety_properties.num_failed_views = 0; - metadata.overall_safety_properties.num_successful_views = 0; + metadata.overall_safety_properties.num_successful_views = 50; let mut config = DelayConfig::default(); let delay_settings = DelaySettings { delay_option: DelayOptions::Random, @@ -128,7 +126,6 @@ cross_tests!( }; metadata.test_config.epoch_height = 10; - metadata.overall_safety_properties.num_failed_views = 0; metadata.overall_safety_properties.num_successful_views = 30; let mut config = DelayConfig::default(); let mut delay_settings = DelaySettings { @@ -162,8 +159,6 @@ cross_tests!( metadata.test_config.num_bootstrap = 10; metadata.test_config.epoch_height = 10; - metadata.overall_safety_properties.num_failed_views = 0; - metadata.view_sync_properties = ViewSyncTaskDescription::Threshold(0, 0); metadata @@ -223,7 +218,6 @@ cross_tests!( node_changes: vec![(1, dead_nodes)] }; metadata.overall_safety_properties.num_successful_views = 1; - metadata.overall_safety_properties.num_failed_views = 0; metadata }, ); @@ -280,17 +274,10 @@ cross_tests!( node_changes: vec![(5, dead_nodes)] }; - // 2 nodes fail triggering view sync, expect no other timeouts - metadata.overall_safety_properties.num_failed_views = 5; // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 20; - metadata.overall_safety_properties.expected_views_to_fail = HashMap::from([ - (ViewNumber::new(5), false), - (ViewNumber::new(11), false), - (ViewNumber::new(17), false), - (ViewNumber::new(23), false), - (ViewNumber::new(29), false), - ]); + metadata.overall_safety_properties.expected_view_failures = vec![4, 5, 10, 11, 17, 22, 23, 28, 29, 34, 35]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(20); metadata } @@ -320,12 +307,12 @@ cross_tests!( }; // node 5 is leader twice when we shut down - metadata.overall_safety_properties.num_failed_views = 2; - metadata.overall_safety_properties.expected_views_to_fail = HashMap::from([ - // next views after turning node off - (ViewNumber::new(view_spin_node_down + 1), false), - (ViewNumber::new(view_spin_node_down + 2), false) - ]); + metadata.overall_safety_properties.expected_view_failures = vec![ + view_spin_node_down, + view_spin_node_down + 1, + view_spin_node_down + 2 + ]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(20); // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 13; @@ -338,9 +325,9 @@ cross_tests!( ); cross_tests!( - TestName: test_with_failures_half_f_epochs, + TestName: test_with_failures_half_f_epochs_1, Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl], - Types: [TestTypes, TestTwoStakeTablesTypes], + Types: [TestTypes], Versions: [EpochsTestVersions], Ignore: false, Metadata: { @@ -364,7 +351,8 @@ cross_tests!( node_changes: vec![(5, dead_nodes)] }; - metadata.overall_safety_properties.num_failed_views = 3; + metadata.overall_safety_properties.expected_view_failures = vec![16, 17, 18, 19]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(24); // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 19; metadata @@ -372,14 +360,101 @@ cross_tests!( ); cross_tests!( - TestName: test_with_failures_f_epochs, + TestName: test_with_failures_half_f_epochs_2, Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl], - Types: [TestTypes, TestTwoStakeTablesTypes], + Types: [TestTwoStakeTablesTypes], + Versions: [EpochsTestVersions], + Ignore: false, + Metadata: { + let mut metadata = TestDescription::default_more_nodes(); + metadata.test_config.epoch_height = 10; + // The first 14 (i.e., 20 - f) nodes are in the DA committee and we may shutdown the + // remaining 6 (i.e., f) nodes. We could remove this restriction after fixing the + // following issue. + let dead_nodes = vec![ + ChangeNode { + idx: 17, + updown: NodeAction::Down, + }, + ChangeNode { + idx: 18, + updown: NodeAction::Down, + }, + ChangeNode { + idx: 19, + updown: NodeAction::Down, + }, + ]; + + metadata.spinning_properties = SpinningTaskDescription { + node_changes: vec![(5, dead_nodes)] + }; + + metadata.overall_safety_properties.expected_view_failures = vec![7, 8, 9, 18, 19]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(20); + // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts + metadata.overall_safety_properties.num_successful_views = 19; + metadata + } +); + +cross_tests!( + TestName: test_with_failures_f_epochs_1, + Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl], + Types: [TestTypes], + Versions: [EpochsTestVersions], + Ignore: false, + Metadata: { + let mut metadata = TestDescription::default_more_nodes(); + metadata.overall_safety_properties.expected_view_failures = vec![13, 14, 15, 16, 17, 18, 19]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(60); + // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts + metadata.overall_safety_properties.num_successful_views = 15; + let dead_nodes = vec![ + ChangeNode { + idx: 14, + updown: NodeAction::Down, + }, + ChangeNode { + idx: 15, + updown: NodeAction::Down, + }, + ChangeNode { + idx: 16, + updown: NodeAction::Down, + }, + ChangeNode { + idx: 17, + updown: NodeAction::Down, + }, + ChangeNode { + idx: 18, + updown: NodeAction::Down, + }, + ChangeNode { + idx: 19, + updown: NodeAction::Down, + }, + ]; + + metadata.spinning_properties = SpinningTaskDescription { + node_changes: vec![(5, dead_nodes)] + }; + + metadata + } +); + +cross_tests!( + TestName: test_with_failures_f_epochs_2, + Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl], + Types: [TestTwoStakeTablesTypes], Versions: [EpochsTestVersions], Ignore: false, Metadata: { let mut metadata = TestDescription::default_more_nodes(); - metadata.overall_safety_properties.num_failed_views = 6; + metadata.overall_safety_properties.expected_view_failures = vec![6, 7, 8, 9, 17, 18, 19]; + metadata.overall_safety_properties.decide_timeout = Duration::from_secs(60); // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts metadata.overall_safety_properties.num_successful_views = 15; let dead_nodes = vec![ @@ -456,7 +531,9 @@ cross_tests!( metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, - num_failed_views: 15, + expected_view_failures: vec![10], + possible_view_failures: vec![9, 11], + decide_timeout: Duration::from_secs(20), ..Default::default() }; @@ -503,7 +580,9 @@ cross_tests!( metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, - num_failed_views: 15, + expected_view_failures: vec![10], + possible_view_failures: vec![9, 11], + decide_timeout: Duration::from_secs(20), ..Default::default() }; @@ -512,9 +591,67 @@ cross_tests!( ); cross_tests!( - TestName: test_staggered_restart_with_epochs, + TestName: test_staggered_restart_with_epochs_1, Impls: [CombinedImpl], - Types: [TestTypes, TestTwoStakeTablesTypes], + Types: [TestTwoStakeTablesTypes], + Versions: [EpochsTestVersions], + Ignore: false, + Metadata: { + let mut metadata = TestDescription::default().set_num_nodes(20,4); + + let mut down_da_nodes = vec![]; + for i in 2..4 { + down_da_nodes.push(ChangeNode { + idx: i, + updown: NodeAction::RestartDown(20), + }); + } + + let mut down_regular_nodes = vec![]; + for i in 4..20 { + down_regular_nodes.push(ChangeNode { + idx: i, + updown: NodeAction::RestartDown(0), + }); + } + // restart the last da so it gets the new libp2p routing table + for i in 0..2 { + down_regular_nodes.push(ChangeNode { + idx: i, + updown: NodeAction::RestartDown(0), + }); + } + + metadata.spinning_properties = SpinningTaskDescription { + node_changes: vec![(10, down_da_nodes), (30, down_regular_nodes)], + }; + metadata.view_sync_properties = + hotshot_testing::view_sync_task::ViewSyncTaskDescription::Threshold(0, 50); + + // Give the test some extra time because we are purposely timing out views + metadata.completion_task_description = + CompletionTaskDescription::TimeBasedCompletionTaskBuilder( + TimeBasedCompletionTaskDescription { + duration: Duration::from_secs(240), + }, + ); + metadata.overall_safety_properties = OverallSafetyPropertiesDescription { + // Make sure we keep committing rounds after the catchup, but not the full 50. + num_successful_views: 22, + expected_view_failures: vec![10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33], + possible_view_failures: vec![34], + decide_timeout: Duration::from_secs(120), + ..Default::default() + }; + + metadata + }, +); + +cross_tests!( + TestName: test_staggered_restart_with_epochs_2, + Impls: [CombinedImpl], + Types: [TestTypes], Versions: [EpochsTestVersions], Ignore: false, Metadata: { @@ -559,7 +696,9 @@ cross_tests!( metadata.overall_safety_properties = OverallSafetyPropertiesDescription { // Make sure we keep committing rounds after the catchup, but not the full 50. num_successful_views: 22, - num_failed_views: 30, + expected_view_failures: vec![10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31], + possible_view_failures: vec![32], + decide_timeout: Duration::from_secs(120), ..Default::default() }; @@ -581,8 +720,8 @@ cross_tests!( }; let overall_safety_properties = OverallSafetyPropertiesDescription { - num_failed_views: 0, num_successful_views: 35, + decide_timeout: Duration::from_secs(8), ..Default::default() };