Add fault checks to the tests.

The `network` test module now verifies that correct nodes are never
reported as faulty.

The `DuplicateAck` fault is removed for now, because the same ack is
usually handled multiple times in DHB.

src/dynamic_honey_badger/dynamic_honey_badger.rs

@@ -280,17 +280,9 @@ where
                     .retain(|skgm| !key_gen_messages.contains(skgm));
                 for SignedKeyGenMsg(epoch, s_id, kg_msg, sig) in key_gen_messages {
                     if epoch != self.start_epoch {
-                        info!(
-                            "Obsolete or premature key generation message: {:?}.",
-                            kg_msg
-                        );
                         let fault_kind = FaultKind::InvalidKeyGenMessageEpoch;
                         step.fault_log.append(id.clone(), fault_kind);
                     } else if !self.verify_signature(&s_id, &sig, &kg_msg)? {
-                        info!(
-                            "Invalid signature in {:?}'s batch from {:?} for: {:?}.",
-                            id, s_id, kg_msg
-                        );
                         let fault_kind = FaultKind::InvalidKeyGenMessageSignature;
                         step.fault_log.append(id.clone(), fault_kind);
                     } else {

src/fault_log.rs

@@ -12,8 +12,6 @@ pub enum AckMessageFault {
     NodeCount,
     #[fail(display = "Sender does not exist")]
     SenderExist,
-    #[fail(display = "Duplicate ack")]
-    DuplicateAck,
     #[fail(display = "Value decryption failed")]
     ValueDecryption,
     #[fail(display = "Value deserialization failed")]

src/lib.rs

@@ -137,6 +137,7 @@ extern crate tiny_keccak;
 
 pub extern crate threshold_crypto as crypto;
 
+mod fault_log;
 mod messaging;
 mod network_info;
 mod traits;
@@ -145,7 +146,6 @@ pub mod binary_agreement;
 pub mod broadcast;
 pub mod coin;
 pub mod dynamic_honey_badger;
-pub mod fault_log;
 pub mod honey_badger;
 pub mod queueing_honey_badger;
 pub mod subset;
@@ -155,6 +155,7 @@ pub mod transaction_queue;
 pub mod util;
 
 pub use crypto::pairing;
+pub use fault_log::{AckMessageFault, Fault, FaultKind, FaultLog};
 pub use messaging::{SourcedMessage, Target, TargetedMessage};
 pub use network_info::NetworkInfo;
 pub use traits::{Contribution, DistAlgorithm, Message, NodeIdT, Step};

src/sync_key_gen.rs

@@ -475,7 +475,7 @@ impl<N: NodeIdT> SyncKeyGen<N> {
             .get_mut(&proposer_idx)
             .ok_or_else(|| Fault::SenderExist)?;
         if !part.acks.insert(sender_idx) {
-            return Err(Fault::DuplicateAck);
+            return Ok(());
         }
         let our_idx = match self.our_idx {
             Some(our_idx) => our_idx,

tests/network/mod.rs

@@ -7,7 +7,7 @@ use crypto::SecretKeyShare;
 use rand::{self, Rng};
 
 use hbbft::dynamic_honey_badger::Batch;
-use hbbft::{Contribution, DistAlgorithm, NetworkInfo, Step, Target, TargetedMessage};
+use hbbft::{Contribution, DistAlgorithm, Fault, NetworkInfo, Step, Target, TargetedMessage};
 
 /// A node identifier. In the tests, nodes are simply numbered.
 #[derive(Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Clone, Copy, Serialize, Deserialize, Rand)]
@@ -25,6 +25,8 @@ pub struct TestNode<D: DistAlgorithm> {
     outputs: Vec<D::Output>,
     /// Outgoing messages to be sent to other nodes.
     messages: VecDeque<TargetedMessage<D::Message, D::NodeId>>,
+    /// Collected fault logs.
+    faults: Vec<Fault<D::NodeId>>,
 }
 
 impl<D: DistAlgorithm> TestNode<D> {
@@ -44,6 +46,7 @@ impl<D: DistAlgorithm> TestNode<D> {
         let step = self.algo.handle_input(input).expect("input");
         self.outputs.extend(step.output);
         self.messages.extend(step.messages);
+        self.faults.extend(step.fault_log.0);
     }
 
     /// Returns the internal algorithm's instance.
@@ -60,6 +63,7 @@ impl<D: DistAlgorithm> TestNode<D> {
             queue: VecDeque::new(),
             outputs: step.output.into_iter().collect(),
             messages: step.messages,
+            faults: step.fault_log.0,
         }
     }
 
@@ -73,9 +77,10 @@ impl<D: DistAlgorithm> TestNode<D> {
             .expect("handling message");
         self.outputs.extend(step.output);
         self.messages.extend(step.messages);
+        self.faults.extend(step.fault_log.0);
     }
 
-    /// Checks whether the node has messages to process
+    /// Checks whether the node has messages to process.
     fn is_idle(&self) -> bool {
         self.queue.is_empty()
     }
@@ -480,6 +485,8 @@ where
         }
         while !self.observer.queue.is_empty() {
             self.observer.handle_message();
+            let faults: Vec<_> = self.observer.faults.drain(..).collect();
+            self.check_faults(faults);
         }
     }
 
@@ -501,7 +508,7 @@ where
         let id = self.adversary.pick_node(&self.nodes);
 
         // The node handles the incoming message and creates new outgoing ones to be dispatched.
-        let msgs: Vec<_> = {
+        let (msgs, faults): (Vec<_>, Vec<_>) = {
             let mut node = self.nodes.get_mut(&id).unwrap();
 
             // Ensure the adversary is playing fair by selecting a node that will result in actual
@@ -513,8 +520,12 @@ where
             );
 
             node.handle_message();
-            node.messages.drain(..).collect()
+            (
+                node.messages.drain(..).collect(),
+                node.faults.drain(..).collect(),
+            )
         };
+        self.check_faults(faults);
         self.dispatch_messages(id, msgs);
 
         id
@@ -522,11 +533,15 @@ where
 
     /// Inputs a value in node `id`.
     pub fn input(&mut self, id: NodeId, value: D::Input) {
-        let msgs: Vec<_> = {
+        let (msgs, faults): (Vec<_>, Vec<_>) = {
             let mut node = self.nodes.get_mut(&id).expect("input instance");
             node.handle_input(value);
-            node.messages.drain(..).collect()
+            (
+                node.messages.drain(..).collect(),
+                node.faults.drain(..).collect(),
+            )
         };
+        self.check_faults(faults);
         self.dispatch_messages(id, msgs);
     }
 
@@ -541,6 +556,15 @@ where
             self.input(id, value.clone());
         }
     }
+
+    /// Verifies that no correct node is reported as faulty.
+    fn check_faults<I: IntoIterator<Item = Fault<D::NodeId>>>(&self, faults: I) {
+        for fault in faults {
+            if self.nodes.contains_key(&fault.node_id) {
+                panic!("Unexpected fault: {:?}", fault);
+            }
+        }
+    }
 }
 
 impl<A: Adversary<D>, C, D> TestNetwork<A, D>