embed public and private shares into DkgPrivateBegin and DkgEndBegin messages

make embedding of public and private shares configurable

reenable start_private_shares test now that embedding is configurable

inc major semver for backwards incompatible API changes

test modules do not need to be public unless someone is using them externally

allow a static mut ref to an atomic variable used for test log initialization; allow many arguments to Signer::new

allow many arguments to setup_with_timeouts

fmt and code change for rng PR rebase

rebase fixes

add TryFromInt to signer and coordinator errors so we can remove more unwrap calls

remove unwrap in signer state machine

Author: xoloki

src/common.rs

@@ -321,7 +321,7 @@ pub mod test_helpers {
 
 #[cfg(test)]
 /// Test module for common functionality
-pub mod test {
+mod test {
     use super::*;
     use crate::util::create_rng;
 

src/net.rs

@@ -168,6 +168,9 @@ pub struct DkgPrivateBegin {
     pub signer_ids: Vec<u32>,
     /// Key IDs who responded in time for this DKG round
     pub key_ids: Vec<u32>,
+    /// Include DkgPublicShares to avoid p2p related message delivery
+    /// order issues when signers communicate directly with each other
+    pub dkg_public_shares: HashMap<u32, DkgPublicShares>,
 }
 
 impl Signable for DkgPrivateBegin {
@@ -179,6 +182,9 @@ impl Signable for DkgPrivateBegin {
         }
         for signer_id in &self.signer_ids {
             hasher.update(signer_id.to_be_bytes());
+            if let Some(shares) = self.dkg_public_shares.get(signer_id) {
+                shares.hash(hasher);
+            }
         }
     }
 }
@@ -228,6 +234,9 @@ pub struct DkgEndBegin {
     pub signer_ids: Vec<u32>,
     /// Key IDs who responded in time for this DKG round
     pub key_ids: Vec<u32>,
+    /// Include DkgPrivateShares to avoid p2p related message delivery
+    /// order issues when signers communicate directly with each other
+    pub dkg_private_shares: HashMap<u32, DkgPrivateShares>,
 }
 
 impl Signable for DkgEndBegin {
@@ -239,6 +248,9 @@ impl Signable for DkgEndBegin {
         }
         for signer_id in &self.signer_ids {
             hasher.update(signer_id.to_be_bytes());
+            if let Some(shares) = self.dkg_private_shares.get(signer_id) {
+                shares.hash(hasher);
+            }
         }
     }
 }
@@ -660,6 +672,7 @@ mod test {
             dkg_id: 0,
             key_ids: Default::default(),
             signer_ids: Default::default(),
+            dkg_public_shares: Default::default(),
         };
         let msg = Message::DkgBegin(dkg_begin.clone());
         let coordinator_packet_dkg_begin = Packet {

src/state_machine/coordinator/fire.rs

@@ -424,11 +424,19 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
             .keys()
             .flat_map(|signer_id| self.config.signer_key_ids[signer_id].clone())
             .collect::<Vec<u32>>();
-
+        let dkg_public_shares = if self.config.embed_public_private_shares {
+            self.dkg_public_shares
+                .iter()
+                .map(|(id, share)| (*id, share.clone()))
+                .collect()
+        } else {
+            Default::default()
+        };
         let dkg_begin = DkgPrivateBegin {
             dkg_id: self.current_dkg_id,
             key_ids: active_key_ids,
             signer_ids: self.dkg_public_shares.keys().cloned().collect(),
+            dkg_public_shares,
         };
         let dkg_private_begin_msg = Packet {
             sig: dkg_begin
@@ -458,11 +466,20 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
             .keys()
             .flat_map(|signer_id| self.config.signer_key_ids[signer_id].clone())
             .collect::<Vec<u32>>();
+        let dkg_private_shares = if self.config.embed_public_private_shares {
+            self.dkg_private_shares
+                .iter()
+                .map(|(id, share)| (*id, share.clone()))
+                .collect()
+        } else {
+            Default::default()
+        };
 
         let dkg_end_begin = DkgEndBegin {
             dkg_id: self.current_dkg_id,
             key_ids: active_key_ids,
             signer_ids: self.dkg_private_shares.keys().cloned().collect(),
+            dkg_private_shares,
         };
         let dkg_end_begin_msg = Packet {
             sig: dkg_end_begin
@@ -1270,7 +1287,7 @@ impl<Aggregator: AggregatorTrait> CoordinatorTrait for Coordinator<Aggregator> {
 
 #[cfg(test)]
 /// Test module for coordinator functionality
-pub mod test {
+mod test {
     use crate::{
         curve::{point::Point, scalar::Scalar},
         net::{
@@ -1524,17 +1541,27 @@ pub mod test {
 
     #[test]
     fn missing_public_keys_dkg_v1() {
-        missing_public_keys_dkg::<v1::Aggregator, v1::Signer>(10, 1);
+        missing_public_keys_dkg::<v1::Aggregator, v1::Signer>(10, 1, false);
+    }
+    #[test]
+    fn missing_public_keys_dkg_v1_embed() {
+        missing_public_keys_dkg::<v1::Aggregator, v1::Signer>(10, 1, true);
     }
 
     #[test]
     fn missing_public_keys_dkg_v2() {
-        missing_public_keys_dkg::<v2::Aggregator, v2::Signer>(10, 1);
+        missing_public_keys_dkg::<v2::Aggregator, v2::Signer>(10, 1, false);
+    }
+
+    #[test]
+    fn missing_public_keys_dkg_v2_embed() {
+        missing_public_keys_dkg::<v2::Aggregator, v2::Signer>(10, 1, true);
     }
 
     fn missing_public_keys_dkg<Aggregator: AggregatorTrait, SignerType: SignerTrait>(
         num_signers: u32,
         keys_per_signer: u32,
+        embed_public_private_shares: bool,
     ) -> (Vec<FireCoordinator<Aggregator>>, Vec<Signer<SignerType>>) {
         let timeout = Duration::from_millis(1024);
         let expire = Duration::from_millis(1280);
@@ -1547,6 +1574,7 @@ pub mod test {
                 Some(timeout),
                 Some(timeout),
                 Some(timeout),
+                embed_public_private_shares,
             );
 
         // Start a DKG round where we will not allow all signers to recv DkgBegin, so they will not respond with DkgPublicShares
@@ -1605,17 +1633,28 @@ pub mod test {
 
     #[test]
     fn minimum_signers_dkg_v1() {
-        minimum_signers_dkg::<v1::Aggregator, v1::Signer>(10, 2);
+        minimum_signers_dkg::<v1::Aggregator, v1::Signer>(10, 2, false);
     }
 
     #[test]
     fn minimum_signers_dkg_v2() {
-        minimum_signers_dkg::<v2::Aggregator, v2::Signer>(10, 2);
+        minimum_signers_dkg::<v2::Aggregator, v2::Signer>(10, 2, false);
+    }
+
+    #[test]
+    fn minimum_signers_dkg_v1_embed() {
+        minimum_signers_dkg::<v1::Aggregator, v1::Signer>(10, 2, true);
+    }
+
+    #[test]
+    fn minimum_signers_dkg_v2_embed() {
+        minimum_signers_dkg::<v2::Aggregator, v2::Signer>(10, 2, true);
     }
 
     fn minimum_signers_dkg<Aggregator: AggregatorTrait, SignerType: SignerTrait>(
         num_signers: u32,
         keys_per_signer: u32,
+        embed_public_private_shares: bool,
     ) -> (Vec<FireCoordinator<Aggregator>>, Vec<Signer<SignerType>>) {
         let timeout = Duration::from_millis(1024);
         let expire = Duration::from_millis(1280);
@@ -1628,6 +1667,7 @@ pub mod test {
                 Some(timeout),
                 Some(timeout),
                 Some(timeout),
+                embed_public_private_shares,
             );
 
         // Start a DKG round where we will not allow all signers to recv DkgBegin, so they will not respond with DkgPublicShares
@@ -1760,15 +1800,27 @@ pub mod test {
 
     #[test]
     fn insufficient_signers_dkg_v1() {
-        insufficient_signers_dkg::<v1::Aggregator, v1::Signer>();
+        insufficient_signers_dkg::<v1::Aggregator, v1::Signer>(false);
     }
 
     #[test]
     fn insufficient_signers_dkg_v2() {
-        insufficient_signers_dkg::<v2::Aggregator, v2::Signer>();
+        insufficient_signers_dkg::<v2::Aggregator, v2::Signer>(false);
+    }
+
+    #[test]
+    fn insufficient_signers_dkg_v1_embed() {
+        insufficient_signers_dkg::<v1::Aggregator, v1::Signer>(true);
+    }
+
+    #[test]
+    fn insufficient_signers_dkg_v2_embed() {
+        insufficient_signers_dkg::<v2::Aggregator, v2::Signer>(true);
     }
 
-    fn insufficient_signers_dkg<Aggregator: AggregatorTrait, Signer: SignerTrait>() {
+    fn insufficient_signers_dkg<Aggregator: AggregatorTrait, Signer: SignerTrait>(
+        embed_public_private_shares: bool,
+    ) {
         let timeout = Duration::from_millis(1024);
         let expire = Duration::from_millis(1280);
         let num_signers = 10;
@@ -1781,6 +1833,7 @@ pub mod test {
             Some(timeout),
             Some(timeout),
             Some(timeout),
+            embed_public_private_shares,
         );
 
         // Start a DKG round where we will not allow all signers to recv DkgBegin, so they will not respond with DkgPublicShares
@@ -2209,20 +2262,35 @@ pub mod test {
 
     #[test]
     fn minimum_signers_sign_v1() {
-        minimum_signers_sign::<v1::Aggregator, v1::Signer>();
+        minimum_signers_sign::<v1::Aggregator, v1::Signer>(false);
     }
 
     #[test]
     fn minimum_signers_sign_v2() {
-        minimum_signers_sign::<v2::Aggregator, v2::Signer>();
+        minimum_signers_sign::<v2::Aggregator, v2::Signer>(false);
     }
 
-    fn minimum_signers_sign<Aggregator: AggregatorTrait, Signer: SignerTrait>() {
+    #[test]
+    fn minimum_signers_sign_v1_embed() {
+        minimum_signers_sign::<v1::Aggregator, v1::Signer>(true);
+    }
+
+    #[test]
+    fn minimum_signers_sign_v2_embed() {
+        minimum_signers_sign::<v2::Aggregator, v2::Signer>(true);
+    }
+
+    fn minimum_signers_sign<Aggregator: AggregatorTrait, Signer: SignerTrait>(
+        embed_public_private_shares: bool,
+    ) {
         let num_signers = 10;
         let keys_per_signer = 2;
 
-        let (mut coordinators, mut signers) =
-            minimum_signers_dkg::<Aggregator, Signer>(num_signers, keys_per_signer);
+        let (mut coordinators, mut signers) = minimum_signers_dkg::<Aggregator, Signer>(
+            num_signers,
+            keys_per_signer,
+            embed_public_private_shares,
+        );
         let config = coordinators.first().unwrap().get_config();
 
         // Figure out how many signers we can remove and still be above the threshold
@@ -2293,20 +2361,35 @@ pub mod test {
 
     #[test]
     fn missing_public_keys_sign_v1() {
-        missing_public_keys_sign::<v1::Aggregator, v1::Signer>();
+        missing_public_keys_sign::<v1::Aggregator, v1::Signer>(false);
+    }
+
+    #[test]
+    fn missing_public_keys_sign_v2() {
+        missing_public_keys_sign::<v2::Aggregator, v2::Signer>(false);
     }
 
     #[test]
-    fn minimum_missing_public_keys_sign_v2() {
-        missing_public_keys_sign::<v2::Aggregator, v2::Signer>();
+    fn missing_public_keys_sign_v1_embed() {
+        missing_public_keys_sign::<v1::Aggregator, v1::Signer>(true);
     }
 
-    fn missing_public_keys_sign<Aggregator: AggregatorTrait, Signer: SignerTrait>() {
+    #[test]
+    fn missing_public_keys_sign_v2_embed() {
+        missing_public_keys_sign::<v2::Aggregator, v2::Signer>(true);
+    }
+
+    fn missing_public_keys_sign<Aggregator: AggregatorTrait, Signer: SignerTrait>(
+        embed_public_private_shares: bool,
+    ) {
         let num_signers = 10;
         let keys_per_signer = 2;
 
-        let (mut coordinators, mut signers) =
-            minimum_signers_dkg::<Aggregator, Signer>(num_signers, keys_per_signer);
+        let (mut coordinators, mut signers) = minimum_signers_dkg::<Aggregator, Signer>(
+            num_signers,
+            keys_per_signer,
+            embed_public_private_shares,
+        );
 
         // Let us also remove that signers public key from the config including all of its key ids
         let mut removed_signer = signers.pop().expect("Failed to pop signer");
@@ -2380,15 +2463,27 @@ pub mod test {
 
     #[test]
     fn insufficient_signers_sign_v1() {
-        insufficient_signers_sign::<v1::Aggregator, v1::Signer>();
+        insufficient_signers_sign::<v1::Aggregator, v1::Signer>(false);
     }
 
     #[test]
     fn insufficient_signers_sign_v2() {
-        insufficient_signers_sign::<v2::Aggregator, v2::Signer>();
+        insufficient_signers_sign::<v2::Aggregator, v2::Signer>(false);
+    }
+
+    #[test]
+    fn insufficient_signers_sign_v1_embed() {
+        insufficient_signers_sign::<v1::Aggregator, v1::Signer>(true);
+    }
+
+    #[test]
+    fn insufficient_signers_sign_v2_embed() {
+        insufficient_signers_sign::<v2::Aggregator, v2::Signer>(true);
     }
 
-    fn insufficient_signers_sign<Aggregator: AggregatorTrait, Signer: SignerTrait>() {
+    fn insufficient_signers_sign<Aggregator: AggregatorTrait, Signer: SignerTrait>(
+        embed_public_private_shares: bool,
+    ) {
         let num_signers = 5;
         let keys_per_signer = 2;
         let (mut coordinators, mut signers) =
@@ -2400,6 +2495,7 @@ pub mod test {
                 None,
                 Some(Duration::from_millis(128)),
                 Some(Duration::from_millis(128)),
+                embed_public_private_shares,
             );
         let config = coordinators.first().unwrap().get_config();
 

src/state_machine/coordinator/frost.rs

@@ -229,10 +229,18 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
             dkg_id = %self.current_dkg_id,
             "Starting Private Share Distribution"
         );
+        let dkg_public_shares = if self.config.embed_public_private_shares {
+            (0..self.config.num_signers)
+                .map(|id| (id, self.dkg_public_shares[&id].clone()))
+                .collect()
+        } else {
+            Default::default()
+        };
         let dkg_begin = DkgPrivateBegin {
             dkg_id: self.current_dkg_id,
             key_ids: (1..self.config.num_keys + 1).collect(),
             signer_ids: (0..self.config.num_signers).collect(),
+            dkg_public_shares,
         };
         let dkg_private_begin_msg = Packet {
             sig: dkg_begin
@@ -251,10 +259,18 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
             dkg_id = %self.current_dkg_id,
             "Starting DKG End Distribution"
         );
+        let dkg_private_shares = if self.config.embed_public_private_shares {
+            (0..self.config.num_signers)
+                .map(|id| (id, self.dkg_private_shares[&id].clone()))
+                .collect()
+        } else {
+            Default::default()
+        };
         let dkg_begin = DkgEndBegin {
             dkg_id: self.current_dkg_id,
             key_ids: (0..self.config.num_keys).collect(),
             signer_ids: (0..self.config.num_signers).collect(),
+            dkg_private_shares,
         };
         let dkg_end_begin_msg = Packet {
             sig: dkg_begin.sign(&self.config.message_private_key).expect(""),
@@ -787,7 +803,7 @@ impl<Aggregator: AggregatorTrait> CoordinatorTrait for Coordinator<Aggregator> {
 
 #[cfg(test)]
 /// Test module for coordinator functionality
-pub mod test {
+mod test {
     use crate::{
         curve::scalar::Scalar,
         net::{DkgBegin, Message, NonceRequest, Packet, SignatureType},