Add Byzantine Test where we have a dishonest leader

crates/hotshot/src/tasks/mod.rs

@@ -2,7 +2,7 @@
 
 /// Provides trait to create task states from a `SystemContextHandle`
 pub mod task_state;
-use std::{sync::Arc, time::Duration};
+use std::{collections::HashSet, sync::Arc, time::Duration};
 
 use async_broadcast::broadcast;
 use async_compatibility_layer::art::{async_sleep, async_spawn};
@@ -25,6 +25,8 @@ use hotshot_task_impls::{
 };
 use hotshot_types::{
     constants::EVENT_CHANNEL_SIZE,
+    data::QuorumProposal,
+    message::Proposal,
     message::{Messages, VersionedMessage},
     traits::{
         network::ConnectedNetwork,
@@ -408,6 +410,91 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>> EventTransformerState<TYPES,
     }
 }
 
+#[derive(Debug)]
+/// An `EventHandlerState` that modifies justify_qc to that of previous view to mock dishonest leader
+pub struct DishonestLeader<TYPES: NodeType, I: NodeImplementation<TYPES>> {
+    /// store events from previous views
+    pub validated_proposals: Vec<QuorumProposal<TYPES>>,
+    /// how many times current node has been elected leader and sent proposal
+    pub total_proposals_from_node: u64,
+    /// which proposals to be dishonest at
+    pub dishonest_at_proposal_numbers: HashSet<u64>,
+    /// how far back to look for a QC
+    pub view_look_back: usize,
+    /// phantom
+    pub _phantom: std::marker::PhantomData<I>,
+}
+
+/// add trait to handle proposal events for a dishonest leader
+trait DishonestProposal<TYPES: NodeType, I: NodeImplementation<TYPES>> {
+    /// send dishonest proposal
+    fn handle_proposal_send_event(
+        &self,
+        event: &HotShotEvent<TYPES>,
+        proposal: &Proposal<TYPES, QuorumProposal<TYPES>>,
+        sender: &TYPES::SignatureKey,
+    ) -> Vec<HotShotEvent<TYPES>>;
+}
+
+impl<TYPES: NodeType, I: NodeImplementation<TYPES>> DishonestProposal<TYPES, I>
+    for DishonestLeader<TYPES, I>
+{
+    fn handle_proposal_send_event(
+        &self,
+        event: &HotShotEvent<TYPES>,
+        proposal: &Proposal<TYPES, QuorumProposal<TYPES>>,
+        sender: &TYPES::SignatureKey,
+    ) -> Vec<HotShotEvent<TYPES>> {
+        let length = self.validated_proposals.len();
+        if !self
+            .dishonest_at_proposal_numbers
+            .contains(&self.total_proposals_from_node)
+            || length == 0
+        {
+            return vec![event.clone()];
+        }
+
+        // grab qc from specified look back
+        let proposal_from_lookback = if length - 1 < self.view_look_back {
+            // if look back is to far just take the first proposal
+            self.validated_proposals[0].clone()
+        } else {
+            let index = (self.validated_proposals.len() - 1) - self.view_look_back;
+            self.validated_proposals[index].clone()
+        };
+        let mut dishonest_proposal = proposal.clone();
+        dishonest_proposal.data.justify_qc = proposal_from_lookback.justify_qc;
+
+        vec![HotShotEvent::QuorumProposalSend(
+            dishonest_proposal,
+            sender.clone(),
+        )]
+    }
+}
+
+#[async_trait]
+impl<TYPES: NodeType, I: NodeImplementation<TYPES> + std::fmt::Debug>
+    EventTransformerState<TYPES, I> for DishonestLeader<TYPES, I>
+{
+    async fn recv_handler(&mut self, event: &HotShotEvent<TYPES>) -> Vec<HotShotEvent<TYPES>> {
+        vec![event.clone()]
+    }
+
+    async fn send_handler(&mut self, event: &HotShotEvent<TYPES>) -> Vec<HotShotEvent<TYPES>> {
+        match event {
+            HotShotEvent::QuorumProposalSend(proposal, sender) => {
+                self.total_proposals_from_node += 1;
+                return self.handle_proposal_send_event(event, proposal, sender);
+            }
+            HotShotEvent::QuorumProposalValidated(proposal, _) => {
+                self.validated_proposals.push(proposal.clone());
+            }
+            _ => {}
+        }
+        vec![event.clone()]
+    }
+}
+
 /// adds tasks for sending/receiving messages to/from the network.
 pub async fn add_network_tasks<TYPES: NodeType, I: NodeImplementation<TYPES>>(
     handle: &mut SystemContextHandle<TYPES, I>,

crates/testing/src/consistency_task.rs

@@ -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]

crates/testing/src/overall_safety_task.rs

@@ -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
@@ -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
@@ -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 } => {
@@ -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(());
                 }
@@ -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(());
         }
@@ -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()
@@ -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
@@ -512,21 +538,24 @@ 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)
             .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()
     }
 }
 
-impl Default for OverallSafetyPropertiesDescription {
+impl<TYPES: NodeType> Default for OverallSafetyPropertiesDescription<TYPES> {
     fn default() -> Self {
         Self {
             num_successful_views: 50,
@@ -536,6 +565,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(),
         }
     }
 }

crates/testing/src/test_builder.rs

@@ -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,
@@ -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
@@ -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,
@@ -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,

crates/testing/tests/tests_1/test_with_failures_2.rs

@@ -1,16 +1,23 @@
 // TODO: Remove this after integration
 #![allow(unused_imports)]
-
 use hotshot_example_types::{
     node_types::{Libp2pImpl, MemoryImpl, PushCdnImpl},
     state_types::TestTypes,
 };
 use hotshot_macros::cross_tests;
 use hotshot_testing::{
     block_builder::SimpleBuilderImplementation,
+    completion_task::{CompletionTaskDescription, TimeBasedCompletionTaskDescription},
     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};
 // Test that a good leader can succeed in the view directly after view sync
 #[cfg(not(feature = "dependency-tasks"))]
 cross_tests!(
@@ -52,3 +59,43 @@ cross_tests!(
         metadata
     }
 );
+
+#[cfg(async_executor_impl = "async-std")]
+cross_tests!(
+    TestName: dishonest_leader,
+    Impls: [MemoryImpl],
+    Types: [TestTypes],
+    Ignore: false,
+    Metadata: {
+        let behaviour = Rc::new(|node_id| {
+                let dishonest_leader = DishonestLeader::<TestTypes, MemoryImpl> {
+                    dishonest_at_proposal_numbers: HashSet::from([2, 3]),
+                    validated_proposals: Vec::new(),
+                    total_proposals_from_node: 0,
+                    view_look_back: 1,
+                    _phantom: std::marker::PhantomData
+                };
+                match node_id {
+                    2 => Behaviour::Byzantine(Box::new(dishonest_leader)),
+                    _ => Behaviour::Standard,
+                }
+            });
+
+        let mut metadata = TestDescription {
+            // allow more time to pass in CI
+            completion_task_description: CompletionTaskDescription::TimeBasedCompletionTaskBuilder(
+                                             TimeBasedCompletionTaskDescription {
+                                                 duration: Duration::from_secs(60),
+                                             },
+                                         ),
+            behaviour,
+            ..TestDescription::default()
+        };
+
+        metadata.overall_safety_properties.num_failed_views = 2;
+        metadata.num_nodes_with_stake = 5;
+        metadata.overall_safety_properties.expected_views_to_fail = 
+            HashSet::from([ViewNumber::new(7), ViewNumber::new(12)]);
+        metadata
+    },
+);