Fix some TODOs, make key share in net info optional.

src/dynamic_honey_badger/builder.rs

@@ -30,7 +30,6 @@ where
     N: Ord,
 {
     fn default() -> Self {
-        // TODO: Use the defaults from `HoneyBadgerBuilder`.
         DynamicHoneyBadgerBuilder {
             era: 0,
             rng: Box::new(rand::thread_rng()),

src/dynamic_honey_badger/dynamic_honey_badger.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 use std::{fmt, result};
 
 use bincode;
-use crypto::{PublicKey, SecretKey, SecretKeyShare, Signature};
+use crypto::{PublicKey, SecretKey, Signature};
 use derivative::Derivative;
 use log::debug;
 use rand::{self, Rand, Rng};
@@ -98,7 +98,7 @@ where
     ) -> Result<(Self, Step<C, N>)> {
         let netinfo = NetworkInfo::new(
             our_id,
-            SecretKeyShare::default(), // TODO: Should be an option?
+            None,
             join_plan.pub_key_set,
             secret_key,
             join_plan.pub_keys,
@@ -390,9 +390,7 @@ where
         debug!("{}: Restarting DKG for {:?}.", self, pub_keys);
         let params = self.honey_badger.params().clone();
         self.restart_honey_badger(era, params);
-        // TODO: This needs to be the same as `num_faulty` will be in the _new_
-        // `NetworkInfo` if the change goes through. It would be safer to deduplicate.
-        let threshold = (pub_keys.len() - 1) / 3;
+        let threshold = util::max_faulty(pub_keys.len());
         let sk = self.netinfo.secret_key().clone();
         let our_id = self.our_id().clone();
         let (key_gen, part) =

src/network_info.rs

@@ -3,7 +3,7 @@ use std::collections::{BTreeMap, BTreeSet};
 use crypto::{self, PublicKey, PublicKeySet, PublicKeyShare, SecretKey, SecretKeyShare};
 use rand;
 
-use NodeIdT;
+use {util, NodeIdT};
 
 /// Common data shared between algorithms: the nodes' IDs and key shares.
 #[derive(Debug, Clone)]
@@ -12,8 +12,7 @@ pub struct NetworkInfo<N> {
     num_nodes: usize,
     num_faulty: usize,
     is_validator: bool,
-    // TODO: Should this be an option? It only makes sense for validators.
-    secret_key_share: SecretKeyShare,
+    secret_key_share: Option<SecretKeyShare>,
     secret_key: SecretKey,
     public_key_set: PublicKeySet,
     public_key_shares: BTreeMap<N, PublicKeyShare>,
@@ -27,9 +26,9 @@ impl<N: NodeIdT> NetworkInfo<N> {
     /// All nodes in the network must share the same public information. Validators' IDs must be
     /// keys in the `public_keys` map, and their secret key share must match their share in the
     /// `public_key_set`.
-    pub fn new(
+    pub fn new<SKS: Into<Option<SecretKeyShare>>>(
         our_id: N,
-        secret_key_share: SecretKeyShare,
+        secret_key_share: SKS,
         public_key_set: PublicKeySet,
         secret_key: SecretKey,
         public_keys: BTreeMap<N, PublicKey>,
@@ -48,9 +47,9 @@ impl<N: NodeIdT> NetworkInfo<N> {
         NetworkInfo {
             our_id,
             num_nodes,
-            num_faulty: (num_nodes - 1) / 3,
+            num_faulty: util::max_faulty(num_nodes),
             is_validator,
-            secret_key_share,
+            secret_key_share: secret_key_share.into(),
             secret_key,
             public_key_set,
             public_key_shares,
@@ -91,10 +90,10 @@ impl<N: NodeIdT> NetworkInfo<N> {
         self.num_nodes - self.num_faulty
     }
 
-    /// Returns our secret key share for threshold cryptography.
+    /// Returns our secret key share for threshold cryptography, or `None` if not a validator.
     #[inline]
-    pub fn secret_key_share(&self) -> &SecretKeyShare {
-        &self.secret_key_share
+    pub fn secret_key_share(&self) -> Option<&SecretKeyShare> {
+        self.secret_key_share.as_ref()
     }
 
     /// Returns our secret key for encryption and signing.
@@ -166,7 +165,7 @@ impl<N: NodeIdT> NetworkInfo<N> {
         use crypto::SecretKeySet;
 
         let all_ids: BTreeSet<N> = ids.into_iter().collect();
-        let num_faulty = (all_ids.len() - 1) / 3;
+        let num_faulty = util::max_faulty(all_ids.len());
 
         // Generate the keys for threshold cryptography.
         let sk_set = SecretKeySet::random(num_faulty, rng);

src/sync_key_gen.rs

@@ -462,8 +462,7 @@ impl<N: NodeIdT> SyncKeyGen<N> {
     /// All participating nodes must have handled the exact same sequence of `Part` and `Ack`
     /// messages before calling this method. Otherwise their key shares will not match.
     pub fn into_network_info(self) -> Result<NetworkInfo<N>, Error> {
-        let (pk_set, opt_sk_share) = self.generate()?;
-        let sk_share = opt_sk_share.unwrap_or_default(); // TODO: Make this an option.
+        let (pk_set, sk_share) = self.generate()?;
         let netinfo = NetworkInfo::new(self.our_id, sk_share, pk_set, self.sec_key, self.pub_keys);
         Ok(netinfo)
     }

src/threshold_decrypt.rs

@@ -135,11 +135,11 @@ impl<N: NodeIdT> ThresholdDecrypt<N> {
         let mut step = Step::default();
         step.fault_log.extend(self.remove_invalid_shares());
         self.had_input = true;
-        if !self.netinfo.is_validator() {
-            step.extend(self.try_output()?);
-            return Ok(step);
-        }
-        let share = self.netinfo.secret_key_share().decrypt_share_no_verify(&ct);
+        // TODO: Remove `cloned()` once non-lexical lifetimes are stable.
+        let share = match self.netinfo.secret_key_share().cloned() {
+            Some(sks) => sks.decrypt_share_no_verify(&ct),
+            None => return Ok(step.join(self.try_output()?)), // Not a validator.
+        };
         let our_id = self.our_id().clone();
         let msg = Target::All.message(Message(share.clone()));
         step.messages.push(msg);

src/threshold_sign.rs

@@ -143,10 +143,11 @@ impl<N: NodeIdT> ThresholdSign<N> {
         self.had_input = true;
         let mut step = Step::default();
         step.fault_log.extend(self.remove_invalid_shares());
-        if !self.netinfo.is_validator() {
-            return Ok(step.join(self.try_output()?));
-        }
-        let msg = Message(self.netinfo.secret_key_share().sign_g2(hash));
+        // TODO: Remove `cloned()` once non-lexical lifetimes are stable.
+        let msg = match self.netinfo.secret_key_share().cloned() {
+            Some(sks) => Message(sks.sign_g2(hash)),
+            None => return Ok(step.join(self.try_output()?)), // Not a validator.
+        };
         step.messages.push(Target::All.message(msg.clone()));
         let id = self.our_id().clone();
         step.extend(self.handle_message(&id, msg)?);

src/util.rs

@@ -36,3 +36,10 @@ pub fn fmt_rng<T>(_: T, f: &mut fmt::Formatter) -> fmt::Result {
 pub fn fmt_hex<T: AsRef<[u8]>>(bytes: T, f: &mut fmt::Formatter) -> fmt::Result {
     write!(f, "{:10}", HexFmt(bytes))
 }
+
+/// Given a number of nodes, returns the maximum number of faulty nodes that can be tolerated: the
+/// greatest number less than one third of `n`.
+#[inline]
+pub fn max_faulty(n: usize) -> usize {
+    (n - 1) / 3
+}

tests/broadcast.rs

@@ -19,7 +19,7 @@ use log::info;
 use rand::Rng;
 
 use hbbft::broadcast::{Broadcast, Message};
-use hbbft::{DistAlgorithm, NetworkInfo, Target, TargetedMessage};
+use hbbft::{util, DistAlgorithm, NetworkInfo, Target, TargetedMessage};
 use network::{
     Adversary, MessageScheduler, MessageWithSender, NodeId, RandomAdversary, SilentAdversary,
     TestNetwork, TestNode,
@@ -110,7 +110,7 @@ where
         .chain(once(rng.gen_range(6, 20)))
         .chain(once(rng.gen_range(30, 50)));
     for size in sizes {
-        let num_faulty_nodes = (size - 1) / 3;
+        let num_faulty_nodes = util::max_faulty(size);
         let num_good_nodes = size - num_faulty_nodes;
         info!(
             "Network size: {} good nodes, {} faulty nodes",

tests/dynamic_honey_badger.rs

@@ -23,7 +23,7 @@ use rand::{Isaac64Rng, Rng};
 use hbbft::dynamic_honey_badger::{Batch, Change, ChangeState, DynamicHoneyBadger, Input};
 use hbbft::sender_queue::{SenderQueue, Step};
 use hbbft::transaction_queue::TransactionQueue;
-use hbbft::NetworkInfo;
+use hbbft::{util, NetworkInfo};
 
 use network::{Adversary, MessageScheduler, NodeId, SilentAdversary, TestNetwork, TestNode};
 
@@ -134,7 +134,7 @@ where
     let sizes = vec![2, 3, 5, rng.gen_range(6, 10)];
     for size in sizes {
         // The test is removing one correct node, so we allow fewer faulty ones.
-        let num_adv_nodes = (size - 2) / 3;
+        let num_adv_nodes = util::max_faulty(size - 1);
         let num_good_nodes = size - num_adv_nodes;
         info!(
             "Network size: {} good nodes, {} faulty nodes",

tests/honey_badger.rs

@@ -24,7 +24,7 @@ use rand::Rng;
 use hbbft::honey_badger::{Batch, EncryptionSchedule, HoneyBadger, MessageContent};
 use hbbft::sender_queue::{self, SenderQueue, Step};
 use hbbft::transaction_queue::TransactionQueue;
-use hbbft::{threshold_decrypt, DistAlgorithm, NetworkInfo, Target, TargetedMessage};
+use hbbft::{threshold_decrypt, util, DistAlgorithm, NetworkInfo, Target, TargetedMessage};
 
 use network::{
     Adversary, MessageScheduler, MessageWithSender, NodeId, RandomAdversary, SilentAdversary,
@@ -101,6 +101,7 @@ impl Adversary<UsizeHoneyBadger> for FaultyShareAdversary {
                         .encrypt(fake_proposal);
                     let share = adv_node
                         .secret_key_share()
+                        .expect("missing adversary key share")
                         .decrypt_share(&fake_ciphertext)
                         .expect("decryption share");
                     // Send the share to remote nodes.
@@ -207,7 +208,7 @@ where
     let mut rng = rand::thread_rng();
     let sizes = vec![1, 2, 3, 5, rng.gen_range(6, 10)];
     for size in sizes {
-        let num_adv_nodes = (size - 1) / 3;
+        let num_adv_nodes = util::max_faulty(size);
         let num_good_nodes = size - num_adv_nodes;
         info!(
             "Network size: {} good nodes, {} faulty nodes",

tests/network/mod.rs

@@ -3,7 +3,6 @@ use std::fmt::{self, Debug};
 use std::mem;
 use std::sync::Arc;
 
-use crypto::SecretKeyShare;
 use log::{debug, warn};
 use rand::{self, Rng};
 use rand_derive::Rand;
@@ -412,7 +411,7 @@ where
             let node_ni = netinfos.values().next().unwrap();
             NetworkInfo::new(
                 NodeId(good_num + adv_num),
-                SecretKeyShare::default(),
+                None,
                 node_ni.public_key_set().clone(),
                 rng.gen(),
                 node_ni.public_key_map().clone(),

tests/queueing_honey_badger.rs

@@ -23,7 +23,7 @@ use rand::{Isaac64Rng, Rng};
 use hbbft::dynamic_honey_badger::DynamicHoneyBadger;
 use hbbft::queueing_honey_badger::{Batch, Change, ChangeState, Input, QueueingHoneyBadger};
 use hbbft::sender_queue::{Message, SenderQueue, Step};
-use hbbft::NetworkInfo;
+use hbbft::{util, NetworkInfo};
 
 use network::{Adversary, MessageScheduler, NodeId, SilentAdversary, TestNetwork, TestNode};
 
@@ -118,7 +118,7 @@ where
     let sizes = vec![3, 5, rng.gen_range(6, 10)];
     for size in sizes {
         // The test is removing one correct node, so we allow fewer faulty ones.
-        let num_adv_nodes = (size - 2) / 3;
+        let num_adv_nodes = util::max_faulty(size - 1);
         let num_good_nodes = size - num_adv_nodes;
         info!(
             "Network size: {} good nodes, {} faulty nodes",

tests/sync_key_gen.rs

@@ -11,6 +11,7 @@ use std::collections::BTreeMap;
 use crypto::{PublicKey, SecretKey};
 
 use hbbft::sync_key_gen::{PartOutcome, SyncKeyGen};
+use hbbft::util;
 
 fn test_sync_key_gen_with(threshold: usize, node_num: usize) {
     // Generate individual key pairs for encryption. These are not suitable for threshold schemes.
@@ -100,7 +101,7 @@ fn test_sync_key_gen() {
     let _ = env_logger::try_init();
 
     for &node_num in &[1, 2, 3, 4, 8, 15] {
-        let threshold = (node_num - 1) / 3;
+        let threshold = util::max_faulty(node_num);
         test_sync_key_gen_with(threshold, node_num);
     }
 }

tests/threshold_sign.rs

@@ -19,7 +19,7 @@ use rand_derive::Rand;
 use serde_derive::{Deserialize, Serialize};
 
 use crypto::Signature;
-use hbbft::threshold_sign::ThresholdSign;
+use hbbft::{threshold_sign::ThresholdSign, util};
 
 use network::{Adversary, MessageScheduler, NodeId, SilentAdversary, TestNetwork, TestNode};
 
@@ -93,7 +93,7 @@ where
     }
 
     for size in sizes {
-        let num_faulty_nodes = (size - 1) / 3;
+        let num_faulty_nodes = util::max_faulty(size);
         let num_good_nodes = size - num_faulty_nodes;
         info!(
             "Network size: {} good nodes, {} faulty nodes",