Skip to content

Commit

Permalink
add functionality to fail out if we fail unexpected view
Browse files Browse the repository at this point in the history
  • Loading branch information
lukeiannucci committed Jul 30, 2024
1 parent 19a2ef3 commit d8ca319
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 13 deletions.
2 changes: 1 addition & 1 deletion crates/testing/src/consistency_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub struct ConsistencyTask<TYPES: NodeType> {
/// A map from node ids to (leaves keyed on view number)
pub consensus_leaves: NetworkMap<TYPES>,
/// safety task requirements
pub safety_properties: OverallSafetyPropertiesDescription,
pub safety_properties: OverallSafetyPropertiesDescription<TYPES>,
}

#[async_trait]
Expand Down
45 changes: 37 additions & 8 deletions crates/testing/src/overall_safety_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ pub enum OverallSafetyTaskErr<TYPES: NodeType> {

failed_views: HashSet<TYPES::Time>,
},
InconsistentFailedViews {
expected_failed_views: HashSet<TYPES::Time>,
actual_failed_views: HashSet<TYPES::Time>,
},
}

/// Data availability task state
Expand All @@ -80,7 +84,7 @@ pub struct OverallSafetyTask<TYPES: NodeType, I: TestableNodeImplementation<TYPE
/// ctx
pub ctx: RoundCtx<TYPES>,
/// configure properties
pub properties: OverallSafetyPropertiesDescription,
pub properties: OverallSafetyPropertiesDescription<TYPES>,
/// error
pub error: Option<Box<OverallSafetyTaskErr<TYPES>>>,
/// sender to test event channel
Expand All @@ -95,14 +99,15 @@ impl<TYPES: NodeType, I: TestableNodeImplementation<TYPES>> TestTaskState

/// Handles an event from one of multiple receivers.
async fn handle_event(&mut self, (message, id): (Self::Event, usize)) -> Result<()> {
let OverallSafetyPropertiesDescription {
let OverallSafetyPropertiesDescription::<TYPES> {
check_leaf,
check_block,
num_failed_views,
num_successful_views,
threshold_calculator,
transaction_threshold,
}: OverallSafetyPropertiesDescription = self.properties.clone();
expected_views_to_fail,
}: OverallSafetyPropertiesDescription<TYPES> = self.properties.clone();
let Event { view_number, event } = message;
let key = match event {
EventType::Error { error } => {
Expand Down Expand Up @@ -175,6 +180,16 @@ impl<TYPES: NodeType, I: TestableNodeImplementation<TYPES>> TestTaskState
Some(Box::new(OverallSafetyTaskErr::<TYPES>::TooManyFailures {
failed_views: self.ctx.failed_views.clone(),
}));
} else if !expected_views_to_fail.is_empty()
&& !expected_views_to_fail.contains(&view_number)
{
let _ = self.test_sender.broadcast(TestEvent::Shutdown).await;
self.error = Some(Box::new(
OverallSafetyTaskErr::<TYPES>::InconsistentFailedViews {
expected_failed_views: expected_views_to_fail.clone(),
actual_failed_views: self.ctx.failed_views.clone(),
},
));
}
return Ok(());
}
Expand All @@ -195,6 +210,16 @@ impl<TYPES: NodeType, I: TestableNodeImplementation<TYPES>> TestTaskState
self.error = Some(Box::new(OverallSafetyTaskErr::<TYPES>::TooManyFailures {
failed_views: self.ctx.failed_views.clone(),
}));
} else if !expected_views_to_fail.is_empty()
&& !expected_views_to_fail.contains(&view_number)
{
let _ = self.test_sender.broadcast(TestEvent::Shutdown).await;
self.error = Some(Box::new(
OverallSafetyTaskErr::<TYPES>::InconsistentFailedViews {
expected_failed_views: expected_views_to_fail.clone(),
actual_failed_views: self.ctx.failed_views.clone(),
},
));
}
return Ok(());
}
Expand All @@ -206,14 +231,15 @@ impl<TYPES: NodeType, I: TestableNodeImplementation<TYPES>> TestTaskState
return TestResult::Fail(e.clone());
}

let OverallSafetyPropertiesDescription {
let OverallSafetyPropertiesDescription::<TYPES> {
check_leaf: _,
check_block: _,
num_failed_views: num_failed_rounds_total,
num_successful_views,
threshold_calculator: _,
transaction_threshold: _,
}: OverallSafetyPropertiesDescription = self.properties.clone();
expected_views_to_fail: _,
}: OverallSafetyPropertiesDescription<TYPES> = self.properties.clone();

let num_incomplete_views = self.ctx.round_results.len()
- self.ctx.successful_views.len()
Expand Down Expand Up @@ -495,7 +521,7 @@ impl<TYPES: NodeType> RoundResult<TYPES> {

/// cross node safety properties
#[derive(Clone)]
pub struct OverallSafetyPropertiesDescription {
pub struct OverallSafetyPropertiesDescription<TYPES: NodeType> {
/// required number of successful views
pub num_successful_views: usize,
/// whether or not to check the leaf
Expand All @@ -512,9 +538,11 @@ pub struct OverallSafetyPropertiesDescription {
/// threshold calculator. Given number of live and total nodes, provide number of successes
/// required to mark view as successful
pub threshold_calculator: Arc<dyn Fn(usize, usize) -> usize + Send + Sync>,
/// pass in the views that we expect to fail
pub expected_views_to_fail: HashSet<TYPES::Time>,
}

impl std::fmt::Debug for OverallSafetyPropertiesDescription {
impl<TYPES: NodeType> std::fmt::Debug for OverallSafetyPropertiesDescription<TYPES> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("OverallSafetyPropertiesDescription")
.field("num successful views", &self.num_successful_views)
Expand All @@ -526,7 +554,7 @@ impl std::fmt::Debug for OverallSafetyPropertiesDescription {
}
}

impl Default for OverallSafetyPropertiesDescription {
impl<TYPES: NodeType> Default for OverallSafetyPropertiesDescription<TYPES> {
fn default() -> Self {
Self {
num_successful_views: 50,
Expand All @@ -536,6 +564,7 @@ impl Default for OverallSafetyPropertiesDescription {
transaction_threshold: 0,
// very strict
threshold_calculator: Arc::new(|_num_live, num_total| 2 * num_total / 3 + 1),
expected_views_to_fail: HashSet::new(),
}
}
}
16 changes: 12 additions & 4 deletions crates/testing/src/test_builder.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
use std::{collections::HashMap, num::NonZeroUsize, rc::Rc, sync::Arc, time::Duration};
use std::{
collections::{HashMap, HashSet},
num::NonZeroUsize,
rc::Rc,
sync::Arc,
time::Duration,
};

use hotshot::{
tasks::EventTransformerState,
Expand Down Expand Up @@ -67,7 +73,7 @@ pub struct TestDescription<TYPES: NodeType, I: NodeImplementation<TYPES>> {
/// Size of the non-staked DA committee for the test
pub da_non_staked_committee_size: usize,
/// overall safety property description
pub overall_safety_properties: OverallSafetyPropertiesDescription,
pub overall_safety_properties: OverallSafetyPropertiesDescription<TYPES>,
/// spinning properties
pub spinning_properties: SpinningTaskDescription,
/// txns timing
Expand Down Expand Up @@ -230,13 +236,14 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>> TestDescription<TYPES, I> {
num_nodes_with_stake,
num_nodes_without_stake,
start_nodes: num_nodes_with_stake,
overall_safety_properties: OverallSafetyPropertiesDescription {
overall_safety_properties: OverallSafetyPropertiesDescription::<TYPES> {
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: HashSet::new(),
},
timing_data: TimingData {
next_view_timeout: 2000,
Expand All @@ -263,13 +270,14 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>> TestDescription<TYPES, I> {
num_nodes_with_stake,
num_nodes_without_stake,
start_nodes: num_nodes_with_stake,
overall_safety_properties: OverallSafetyPropertiesDescription {
overall_safety_properties: OverallSafetyPropertiesDescription::<TYPES> {
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: HashSet::new(),
},
timing_data: TimingData {
start_delay: 120_000,
Expand Down
4 changes: 4 additions & 0 deletions crates/testing/tests/tests_1/test_with_failures_2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ use hotshot_testing::{
spinning_task::{ChangeNode, SpinningTaskDescription, UpDown},
test_builder::TestDescription,
};
use hotshot_types::data::ViewNumber;
use hotshot_types::traits::node_implementation::ConsensusTime;
use std::time::Duration;
use std::collections::HashSet;

#[cfg(async_executor_impl = "async-std")]
use {hotshot::tasks::DishonestLeader, hotshot_testing::test_builder::Behaviour, std::rc::Rc};
Expand Down Expand Up @@ -84,6 +87,7 @@ cross_tests!(
};

metadata.overall_safety_properties.num_failed_views = 1;
metadata.overall_safety_properties.expected_views_to_fail = HashSet::from([ViewNumber::new(8)]);
metadata
},
);

0 comments on commit d8ca319

Please sign in to comment.