Skip to content

Commit

Permalink
embed public and private shares into DkgPrivateBegin and DkgEndBegin …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
xoloki committed Dec 19, 2024
1 parent e3ab997 commit 95fe283
Show file tree
Hide file tree
Showing 6 changed files with 259 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
13 changes: 13 additions & 0 deletions src/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
}
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down
142 changes: 119 additions & 23 deletions src/state_machine/coordinator/fire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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::{
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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) =
Expand All @@ -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();

Expand Down
18 changes: 17 additions & 1 deletion src/state_machine/coordinator/frost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(""),
Expand Down Expand Up @@ -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},
Expand Down
Loading

0 comments on commit 95fe283

Please sign in to comment.