diff --git a/src/common.rs b/src/common.rs index 3f96234..87ccf9d 100644 --- a/src/common.rs +++ b/src/common.rs @@ -326,7 +326,7 @@ pub mod test_helpers { #[cfg(test)] /// Test module for common functionality -pub mod test { +mod test { use super::*; use crate::util::create_rng; diff --git a/src/net.rs b/src/net.rs index 2d305ff..464a39b 100644 --- a/src/net.rs +++ b/src/net.rs @@ -168,6 +168,9 @@ pub struct DkgPrivateBegin { pub signer_ids: Vec, /// Key IDs who responded in time for this DKG round pub key_ids: Vec, + /// Include DkgPublicShares to avoid p2p related message delivery + /// order issues when signers communicate directly with each other + pub dkg_public_shares: HashMap, } 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, /// Key IDs who responded in time for this DKG round pub key_ids: Vec, + /// Include DkgPrivateShares to avoid p2p related message delivery + /// order issues when signers communicate directly with each other + pub dkg_private_shares: HashMap, } 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 { diff --git a/src/state_machine/coordinator/fire.rs b/src/state_machine/coordinator/fire.rs index 87c0d7e..5e79bf0 100644 --- a/src/state_machine/coordinator/fire.rs +++ b/src/state_machine/coordinator/fire.rs @@ -424,11 +424,19 @@ impl Coordinator { .keys() .flat_map(|signer_id| self.config.signer_key_ids[signer_id].clone()) .collect::>(); - + 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 Coordinator { .keys() .flat_map(|signer_id| self.config.signer_key_ids[signer_id].clone()) .collect::>(); + 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 @@ -1275,7 +1292,7 @@ impl CoordinatorTrait for Coordinator { #[cfg(test)] /// Test module for coordinator functionality -pub mod test { +mod test { use crate::{ curve::{point::Point, scalar::Scalar}, net::{ @@ -1530,17 +1547,27 @@ pub mod test { #[test] fn missing_public_keys_dkg_v1() { - missing_public_keys_dkg::(10, 1); + missing_public_keys_dkg::(10, 1, false); + } + #[test] + fn missing_public_keys_dkg_v1_embed() { + missing_public_keys_dkg::(10, 1, true); } #[test] fn missing_public_keys_dkg_v2() { - missing_public_keys_dkg::(10, 1); + missing_public_keys_dkg::(10, 1, false); + } + + #[test] + fn missing_public_keys_dkg_v2_embed() { + missing_public_keys_dkg::(10, 1, true); } fn missing_public_keys_dkg( num_signers: u32, keys_per_signer: u32, + embed_public_private_shares: bool, ) -> (Vec>, Vec>) { let timeout = Duration::from_millis(1024); let expire = Duration::from_millis(1280); @@ -1553,6 +1580,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 @@ -1611,17 +1639,28 @@ pub mod test { #[test] fn minimum_signers_dkg_v1() { - minimum_signers_dkg::(10, 2); + minimum_signers_dkg::(10, 2, false); } #[test] fn minimum_signers_dkg_v2() { - minimum_signers_dkg::(10, 2); + minimum_signers_dkg::(10, 2, false); + } + + #[test] + fn minimum_signers_dkg_v1_embed() { + minimum_signers_dkg::(10, 2, true); + } + + #[test] + fn minimum_signers_dkg_v2_embed() { + minimum_signers_dkg::(10, 2, true); } fn minimum_signers_dkg( num_signers: u32, keys_per_signer: u32, + embed_public_private_shares: bool, ) -> (Vec>, Vec>) { let timeout = Duration::from_millis(1024); let expire = Duration::from_millis(1280); @@ -1634,6 +1673,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 @@ -1766,15 +1806,27 @@ pub mod test { #[test] fn insufficient_signers_dkg_v1() { - insufficient_signers_dkg::(); + insufficient_signers_dkg::(false); } #[test] fn insufficient_signers_dkg_v2() { - insufficient_signers_dkg::(); + insufficient_signers_dkg::(false); + } + + #[test] + fn insufficient_signers_dkg_v1_embed() { + insufficient_signers_dkg::(true); + } + + #[test] + fn insufficient_signers_dkg_v2_embed() { + insufficient_signers_dkg::(true); } - fn insufficient_signers_dkg() { + fn insufficient_signers_dkg( + embed_public_private_shares: bool, + ) { let timeout = Duration::from_millis(1024); let expire = Duration::from_millis(1280); let num_signers = 10; @@ -1787,6 +1839,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 @@ -2215,20 +2268,35 @@ pub mod test { #[test] fn minimum_signers_sign_v1() { - minimum_signers_sign::(); + minimum_signers_sign::(false); } #[test] fn minimum_signers_sign_v2() { - minimum_signers_sign::(); + minimum_signers_sign::(false); } - fn minimum_signers_sign() { + #[test] + fn minimum_signers_sign_v1_embed() { + minimum_signers_sign::(true); + } + + #[test] + fn minimum_signers_sign_v2_embed() { + minimum_signers_sign::(true); + } + + fn minimum_signers_sign( + embed_public_private_shares: bool, + ) { let num_signers = 10; let keys_per_signer = 2; - let (mut coordinators, mut signers) = - minimum_signers_dkg::(num_signers, keys_per_signer); + let (mut coordinators, mut signers) = minimum_signers_dkg::( + 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 @@ -2299,20 +2367,35 @@ pub mod test { #[test] fn missing_public_keys_sign_v1() { - missing_public_keys_sign::(); + missing_public_keys_sign::(false); + } + + #[test] + fn missing_public_keys_sign_v2() { + missing_public_keys_sign::(false); } #[test] - fn minimum_missing_public_keys_sign_v2() { - missing_public_keys_sign::(); + fn missing_public_keys_sign_v1_embed() { + missing_public_keys_sign::(true); } - fn missing_public_keys_sign() { + #[test] + fn missing_public_keys_sign_v2_embed() { + missing_public_keys_sign::(true); + } + + fn missing_public_keys_sign( + embed_public_private_shares: bool, + ) { let num_signers = 10; let keys_per_signer = 2; - let (mut coordinators, mut signers) = - minimum_signers_dkg::(num_signers, keys_per_signer); + let (mut coordinators, mut signers) = minimum_signers_dkg::( + 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"); @@ -2386,15 +2469,27 @@ pub mod test { #[test] fn insufficient_signers_sign_v1() { - insufficient_signers_sign::(); + insufficient_signers_sign::(false); } #[test] fn insufficient_signers_sign_v2() { - insufficient_signers_sign::(); + insufficient_signers_sign::(false); + } + + #[test] + fn insufficient_signers_sign_v1_embed() { + insufficient_signers_sign::(true); + } + + #[test] + fn insufficient_signers_sign_v2_embed() { + insufficient_signers_sign::(true); } - fn insufficient_signers_sign() { + fn insufficient_signers_sign( + embed_public_private_shares: bool, + ) { let num_signers = 5; let keys_per_signer = 2; let (mut coordinators, mut signers) = @@ -2406,6 +2501,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(); diff --git a/src/state_machine/coordinator/frost.rs b/src/state_machine/coordinator/frost.rs index c4b6145..a850b5f 100644 --- a/src/state_machine/coordinator/frost.rs +++ b/src/state_machine/coordinator/frost.rs @@ -229,10 +229,27 @@ impl Coordinator { dkg_id = %self.current_dkg_id, "Starting Private Share Distribution" ); + let dkg_public_shares = if self.config.embed_public_private_shares { + let mut public_shares = HashMap::new(); + for id in 0..self.config.num_signers { + match self.dkg_public_shares.get(&id) { + Some(shares) => { + public_shares.insert(id, shares.clone()); + } + None => { + return Err(Error::MissingPublicShares(id)); + } + } + } + public_shares + } 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 +268,27 @@ impl Coordinator { dkg_id = %self.current_dkg_id, "Starting DKG End Distribution" ); + let dkg_private_shares = if self.config.embed_public_private_shares { + let mut private_shares = HashMap::new(); + for id in 0..self.config.num_signers { + match self.dkg_private_shares.get(&id) { + Some(shares) => { + private_shares.insert(id, shares.clone()); + } + None => { + return Err(Error::MissingPrivateShares(id)); + } + } + } + private_shares + } 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 +821,7 @@ impl CoordinatorTrait for Coordinator { #[cfg(test)] /// Test module for coordinator functionality -pub mod test { +mod test { use crate::{ curve::scalar::Scalar, net::{DkgBegin, Message, NonceRequest, Packet, SignatureType}, diff --git a/src/state_machine/coordinator/mod.rs b/src/state_machine/coordinator/mod.rs index 8d6bdfb..2bdf3cd 100644 --- a/src/state_machine/coordinator/mod.rs +++ b/src/state_machine/coordinator/mod.rs @@ -6,7 +6,7 @@ use crate::{ state_machine::{DkgFailure, OperationResult}, taproot::SchnorrProof, }; -use core::{cmp::PartialEq, fmt::Debug}; +use core::{cmp::PartialEq, fmt::Debug, num::TryFromIntError}; use hashbrown::{HashMap, HashSet}; use std::{ collections::BTreeMap, @@ -78,6 +78,12 @@ pub enum Error { /// Missing message response information for a signing round #[error("Missing message nonce information")] MissingMessageNonceInfo, + /// Missing DkgPublicShares for a signer_id + #[error("Missing DkgPublicShares for signer_id {0}")] + MissingPublicShares(u32), + /// Missing DkgPrivateShares for a signer_id + #[error("Missing DkgPrivateShares for signer_id {0}")] + MissingPrivateShares(u32), /// DKG failure from signers #[error("DKG failure from signers")] DkgFailure(HashMap), @@ -89,6 +95,9 @@ pub enum Error { /// Supplied party polynomial contained duplicate party IDs #[error("Supplied party polynomials contained a duplicate party ID")] DuplicatePartyId, + #[error("integer conversion error")] + /// An error during integer conversion operations + TryFromInt, } impl From for Error { @@ -97,6 +106,12 @@ impl From for Error { } } +impl From for Error { + fn from(_e: TryFromIntError) -> Self { + Self::TryFromInt + } +} + /// Config fields common to all Coordinators #[derive(Default, Clone, Debug, PartialEq)] pub struct Config { @@ -124,6 +139,8 @@ pub struct Config { pub signer_key_ids: HashMap>, /// ECDSA public keys as Point objects indexed by signer_id pub signer_public_keys: HashMap, + /// Embed public and private shares into DkgBegin messages + pub embed_public_private_shares: bool, } impl Config { @@ -147,6 +164,7 @@ impl Config { sign_timeout: None, signer_key_ids: Default::default(), signer_public_keys: Default::default(), + embed_public_private_shares: false, } } @@ -165,6 +183,7 @@ impl Config { sign_timeout: Option, signer_key_ids: HashMap>, signer_public_keys: HashMap, + embed_public_private_shares: bool, ) -> Self { Config { num_signers, @@ -179,6 +198,7 @@ impl Config { sign_timeout, signer_key_ids, signer_public_keys, + embed_public_private_shares, } } } @@ -438,9 +458,12 @@ pub mod test { None, None, None, + false, ) } + #[allow(static_mut_refs)] + #[allow(clippy::too_many_arguments)] pub fn setup_with_timeouts( num_signers: u32, keys_per_signer: u32, @@ -449,6 +472,7 @@ pub mod test { dkg_end_timeout: Option, nonce_timeout: Option, sign_timeout: Option, + embed_public_private_shares: bool, ) -> (Vec, Vec>) { INIT.call_once(|| { tracing_subscriber::registry() @@ -505,6 +529,7 @@ pub mod test { signer_key_ids[&(signer_id as u32)].clone(), *private_key, public_keys.clone(), + embed_public_private_shares, &mut rng, ) }) @@ -525,6 +550,7 @@ pub mod test { sign_timeout, signer_key_ids_set.clone(), signer_public_keys.clone(), + embed_public_private_shares, ); Coordinator::new(config) }) diff --git a/src/state_machine/signer/mod.rs b/src/state_machine/signer/mod.rs index e45128d..d636634 100644 --- a/src/state_machine/signer/mod.rs +++ b/src/state_machine/signer/mod.rs @@ -1,3 +1,4 @@ +use core::num::TryFromIntError; use hashbrown::{HashMap, HashSet}; use rand_core::{CryptoRng, RngCore}; use std::collections::{BTreeMap, BTreeSet}; @@ -61,6 +62,15 @@ pub enum Error { /// An encryption error occurred #[error("Encryption error: {0}")] Encryption(#[from] EncryptionError), + #[error("integer conversion error")] + /// An error during integer conversion operations + TryFromInt, +} + +impl From for Error { + fn from(_e: TryFromIntError) -> Self { + Self::TryFromInt + } } /// The saved state required to reconstruct a signer @@ -110,6 +120,8 @@ pub struct SavedState { pub dkg_private_begin_msg: Option, /// the DKG end begin message received in this round pub dkg_end_begin_msg: Option, + /// Embed public and private shares into DkgBegin messages + pub embed_public_private_shares: bool, } /// A state machine for a signing round @@ -159,6 +171,8 @@ pub struct Signer { pub dkg_private_begin_msg: Option, /// the DKG end begin message received in this round pub dkg_end_begin_msg: Option, + /// Embed public and private shares into DkgBegin messages + pub embed_public_private_shares: bool, } impl Signer { @@ -172,6 +186,7 @@ impl Signer { key_ids: Vec, network_private_key: Scalar, public_keys: PublicKeys, + embed_public_private_shares: bool, rng: &mut R, ) -> Self { assert!(threshold <= total_keys); @@ -209,6 +224,7 @@ impl Signer { dkg_private_shares: Default::default(), dkg_private_begin_msg: Default::default(), dkg_end_begin_msg: Default::default(), + embed_public_private_shares, } } @@ -235,6 +251,7 @@ impl Signer { dkg_private_shares: state.dkg_private_shares.clone(), dkg_private_begin_msg: state.dkg_private_begin_msg.clone(), dkg_end_begin_msg: state.dkg_end_begin_msg.clone(), + embed_public_private_shares: state.embed_public_private_shares, } } @@ -261,6 +278,7 @@ impl Signer { dkg_private_shares: self.dkg_private_shares.clone(), dkg_private_begin_msg: self.dkg_private_begin_msg.clone(), dkg_end_begin_msg: self.dkg_end_begin_msg.clone(), + embed_public_private_shares: self.embed_public_private_shares, } } @@ -313,10 +331,20 @@ impl Signer { Message::DkgPrivateBegin(dkg_private_begin) => { self.dkg_private_begin(dkg_private_begin, rng) } - Message::DkgEndBegin(dkg_end_begin) => self.dkg_end_begin(dkg_end_begin), - Message::DkgPublicShares(dkg_public_shares) => self.dkg_public_share(dkg_public_shares), + Message::DkgEndBegin(dkg_end_begin) => self.dkg_end_begin(dkg_end_begin, rng), + Message::DkgPublicShares(dkg_public_shares) => { + if !self.embed_public_private_shares { + self.dkg_public_share(dkg_public_shares) + } else { + Ok(vec![]) + } + } Message::DkgPrivateShares(dkg_private_shares) => { - self.dkg_private_shares(dkg_private_shares, rng) + if !self.embed_public_private_shares { + self.dkg_private_shares(dkg_private_shares, rng) + } else { + Ok(vec![]) + } } Message::SignatureShareRequest(sign_share_request) => { self.sign_share_request(sign_share_request, rng) @@ -352,7 +380,7 @@ impl Signer { let mut missing_public_shares = HashSet::new(); let mut missing_private_shares = HashSet::new(); let mut bad_public_shares = HashSet::new(); - let threshold: usize = self.threshold.try_into().unwrap(); + let threshold: usize = self.threshold.try_into()?; if let Some(dkg_end_begin) = &self.dkg_end_begin_msg { for signer_id in &dkg_end_begin.signer_ids { if let Some(shares) = self.dkg_public_shares.get(signer_id) { @@ -712,6 +740,10 @@ impl Signer { .cloned() .collect::>(); + for (_, shares) in &dkg_private_begin.dkg_public_shares { + let _ = self.dkg_public_share(shares)?; + } + self.dkg_private_begin_msg = Some(dkg_private_begin.clone()); self.move_to(State::DkgPrivateDistribute)?; @@ -760,11 +792,19 @@ impl Signer { } /// handle incoming DkgEndBegin - pub fn dkg_end_begin(&mut self, dkg_end_begin: &DkgEndBegin) -> Result, Error> { + pub fn dkg_end_begin( + &mut self, + dkg_end_begin: &DkgEndBegin, + rng: &mut R, + ) -> Result, Error> { let msgs = vec![]; self.dkg_end_begin_msg = Some(dkg_end_begin.clone()); + for (_, shares) in &dkg_end_begin.dkg_private_shares { + let _ = self.dkg_private_shares(shares, rng)?; + } + info!( signer_id = %self.signer_id, dkg_id = %self.dkg_id, @@ -905,7 +945,9 @@ impl StateMachine for Signer } #[cfg(test)] /// Test module for signer functionality -pub mod test { +mod test { + use hashbrown::HashMap; + use crate::{ common::PolyCommitment, curve::{ecdsa, scalar::Scalar}, @@ -940,6 +982,7 @@ pub mod test { vec![1], Default::default(), Default::default(), + true, &mut rng, ); let public_share = DkgPublicShares { @@ -977,6 +1020,7 @@ pub mod test { vec![1], Default::default(), Default::default(), + true, &mut rng, ); // publich_shares_done starts out as false @@ -1015,8 +1059,17 @@ pub mod test { public_keys.signers.insert(0, public_key.clone()); public_keys.key_ids.insert(1, public_key.clone()); - let mut signer = - Signer::::new(1, 1, 1, 0, vec![1], private_key, public_keys, &mut rng); + let mut signer = Signer::::new( + 1, + 1, + 1, + 0, + vec![1], + private_key, + public_keys, + true, + &mut rng, + ); // can_dkg_end starts out as false assert!(!signer.can_dkg_end()); @@ -1028,10 +1081,17 @@ pub mod test { let _ = signer .process(&dkg_public_shares[0], &mut rng) .expect("failed to process DkgPublicShares"); + let mut public_shares = HashMap::new(); + if let Message::DkgPublicShares(shares) = &dkg_public_shares[0] { + public_shares.insert(0, shares.clone()); + } else { + panic!(""); + } let dkg_private_begin = Message::DkgPrivateBegin(DkgPrivateBegin { dkg_id: 1, signer_ids: vec![0], key_ids: vec![1], + dkg_public_shares: public_shares, }); let dkg_private_shares = signer .process(&dkg_private_begin, &mut rng) @@ -1039,13 +1099,20 @@ pub mod test { let _ = signer .process(&dkg_private_shares[0], &mut rng) .expect("failed to process DkgPrivateShares"); + let mut private_shares = HashMap::new(); + if let Message::DkgPrivateShares(shares) = &dkg_private_shares[0] { + private_shares.insert(0u32, shares.clone()); + } else { + panic!(""); + } let dkg_end_begin = DkgEndBegin { dkg_id: 1, signer_ids: vec![0], key_ids: vec![1], + dkg_private_shares: private_shares, }; let _ = signer - .dkg_end_begin(&dkg_end_begin) + .dkg_end_begin(&dkg_end_begin, &mut rng) .expect("failed to process DkgPrivateShares"); // can_dkg_end should be true @@ -1068,6 +1135,7 @@ pub mod test { .with(fmt::layer()) .with(EnvFilter::from_default_env()) .init();*/ + let mut rng = create_rng(); let mut signer = Signer::::new( 1, @@ -1077,6 +1145,7 @@ pub mod test { vec![1], Default::default(), Default::default(), + true, &mut rng, );