From 7685ff301ceb12fef442d6f7e7c137b1f6ad39b8 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 10 Dec 2024 13:55:37 +0000 Subject: [PATCH 01/13] ci(mergify): upgrade configuration to current format (#794) Co-authored-by: Mergify <37929162+mergify[bot]@users.noreply.github.com> --- .mergify.yml | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index dff68c01c..491837eb1 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -1,28 +1,33 @@ queue_rules: - name: main - allow_inplace_checks: True - allow_checks_interruption: True - speculative_checks: 1 - batch_size: 2 - # Wait for a few minutes to embark 2 tickets together in a merge train - batch_max_wait_time: "3 minutes" - conditions: + queue_conditions: + - base=main + - -draft + - label!=do-not-merge + merge_conditions: # Mergify automatically applies status check, approval, and conversation rules, # which are the same as the GitHub main branch protection rules # https://docs.mergify.com/conditions/#about-branch-protection - base=main + allow_inplace_checks: true + batch_size: 2 + # Wait for a few minutes to embark 2 tickets together in a merge train + batch_max_wait_time: "3 minutes" + merge_method: squash pull_request_rules: - name: main queue triggered when CI passes with 1 review + conditions: [] + actions: + queue: + +priority_rules: + - name: Priority rule from queue `main` conditions: - # This queue handles a PR if: - # - it targets main - # - is not in draft - # including automated dependabot PRs. - base=main - -draft - label!=do-not-merge - actions: - queue: - name: main - method: squash \ No newline at end of file + allow_checks_interruption: true + +merge_queue: + max_parallel_checks: 1 From d6c9f7314038064f170dfc3aaf079e044c9eb559 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Tue, 10 Dec 2024 11:16:41 -0300 Subject: [PATCH 02/13] core: prevent creating a zero identifier with deserialization (#797) * core: prevent creating a zero identifier with deserialization * move test to file which is not copied to other ciphersuites --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- frost-core/src/identifier.rs | 2 +- frost-secp256k1/src/tests/deserialize.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/frost-core/src/identifier.rs b/frost-core/src/identifier.rs index 0ca710c38..9dc4e8fc8 100644 --- a/frost-core/src/identifier.rs +++ b/frost-core/src/identifier.rs @@ -69,7 +69,7 @@ where /// Deserialize an Identifier from a serialized buffer. /// Returns an error if it attempts to deserialize zero. pub fn deserialize(bytes: &[u8]) -> Result> { - Ok(Self(SerializableScalar::deserialize(bytes)?)) + Self::new(SerializableScalar::deserialize(bytes)?.0) } } diff --git a/frost-secp256k1/src/tests/deserialize.rs b/frost-secp256k1/src/tests/deserialize.rs index a744832be..bb8d555bd 100644 --- a/frost-secp256k1/src/tests/deserialize.rs +++ b/frost-secp256k1/src/tests/deserialize.rs @@ -36,3 +36,12 @@ fn check_deserialize_identity() { let r = ::Group::deserialize(&encoded_identity); assert_eq!(r, Err(GroupError::MalformedElement)); } + +// Test if deserializing the identifier 0 fails. +// https://github.com/ZcashFoundation/frost/issues/793 +#[test] +fn check_zero_identifier_deserialization() { + let arr: [u8; 32] = [0; 32]; + let r = Identifier::deserialize(&arr); + assert_eq!(r, Err(Error::FieldError(FieldError::InvalidZeroScalar))); +} From dee685d596e4a9c9bbff90893fa795d49a16385e Mon Sep 17 00:00:00 2001 From: natalie Date: Wed, 11 Dec 2024 08:57:49 +0000 Subject: [PATCH 03/13] Refresh Shares with DKG (#766) * Refresh shares with DKG (#663) * Add verification step for round2_packages for refreshing shares with DKG (#663) * Clean up clippy issues for correct indexing with refreshing shares with DKG (#663) * Import refresh tests for all ciphersuites (#663) * Fix formatting (#663) --- frost-core/src/keys/refresh.rs | 269 +++++++++++++++++- frost-core/src/tests/ciphersuite_generic.rs | 2 +- frost-core/src/tests/refresh.rs | 198 ++++++++++++- frost-ed25519/src/lib.rs | 1 + frost-ed25519/tests/integration_tests.rs | 7 + frost-ed448/src/lib.rs | 1 + frost-ed448/tests/integration_tests.rs | 7 + frost-p256/tests/integration_tests.rs | 7 + frost-ristretto255/tests/integration_tests.rs | 7 + frost-secp256k1/src/lib.rs | 1 + frost-secp256k1/tests/integration_tests.rs | 7 + 11 files changed, 498 insertions(+), 9 deletions(-) diff --git a/frost-core/src/keys/refresh.rs b/frost-core/src/keys/refresh.rs index 53a3cdd91..79f61185e 100644 --- a/frost-core/src/keys/refresh.rs +++ b/frost-core/src/keys/refresh.rs @@ -8,14 +8,18 @@ use alloc::collections::BTreeMap; use alloc::vec::Vec; use crate::{ + keys::dkg::{compute_proof_of_knowledge, round1, round2}, keys::{ - generate_coefficients, generate_secret_shares, validate_num_of_signers, - CoefficientCommitment, PublicKeyPackage, SigningKey, SigningShare, VerifyingShare, + evaluate_polynomial, generate_coefficients, generate_secret_polynomial, + generate_secret_shares, validate_num_of_signers, CoefficientCommitment, PublicKeyPackage, + SigningKey, SigningShare, VerifyingShare, }, - Ciphersuite, CryptoRng, Error, Field, Group, Identifier, RngCore, + Ciphersuite, CryptoRng, Error, Field, Group, Header, Identifier, RngCore, }; -use super::{KeyPackage, SecretShare, VerifiableSecretSharingCommitment}; +use core::iter; + +use super::{dkg::round1::Package, KeyPackage, SecretShare, VerifiableSecretSharingCommitment}; /// Generates new zero key shares and a public key package using a trusted /// dealer Building a new public key package is done by taking the verifying @@ -114,3 +118,260 @@ pub fn refresh_share( Ok(new_key_package) } + +/// Part 1 of refresh share with DKG. A refreshing_key is generated and a new package and secret_package are generated. +/// The identity commitment is removed from the packages. +pub fn refresh_dkg_part_1( + identifier: Identifier, + max_signers: u16, + min_signers: u16, + mut rng: R, +) -> Result<(round1::SecretPackage, round1::Package), Error> { + validate_num_of_signers::(min_signers, max_signers)?; + + // Build refreshing shares + let refreshing_key = SigningKey { + scalar: <::Field>::zero(), + }; + + // Round 1, Step 1 + let coefficients = generate_coefficients::(min_signers as usize - 1, &mut rng); + + let (coefficients, commitment) = + generate_secret_polynomial(&refreshing_key, max_signers, min_signers, coefficients)?; + + // Remove identity element from coefficients + let mut coeff_comms = commitment.0; + coeff_comms.remove(0); + let commitment = VerifiableSecretSharingCommitment::new(coeff_comms.clone()); + + let proof_of_knowledge = + compute_proof_of_knowledge(identifier, &coefficients, &commitment, &mut rng)?; + + let secret_package = round1::SecretPackage { + identifier, + coefficients: coefficients.clone(), + commitment: commitment.clone(), + min_signers, + max_signers, + }; + let package = round1::Package { + header: Header::default(), + commitment, + proof_of_knowledge, + }; + + Ok((secret_package, package)) +} + +/// Part 2 of refresh share with DKG. The identity commitment needs to be added back into the secret package. +pub fn refresh_dkg_part2( + mut secret_package: round1::SecretPackage, + round1_packages: &BTreeMap, round1::Package>, +) -> Result< + ( + round2::SecretPackage, + BTreeMap, round2::Package>, + ), + Error, +> { + if round1_packages.len() != (secret_package.max_signers - 1) as usize { + return Err(Error::IncorrectNumberOfPackages); + } + + // The identity commitment needs to be added to the VSS commitment for secret package + let identity_commitment: Vec> = + vec![CoefficientCommitment::new(C::Group::identity())]; + + let refreshing_secret_share_commitments: Vec> = identity_commitment + .into_iter() + .chain(secret_package.commitment.0.clone()) + .collect(); + + secret_package.commitment = + VerifiableSecretSharingCommitment::::new(refreshing_secret_share_commitments); + + let mut round2_packages = BTreeMap::new(); + + for (sender_identifier, round1_package) in round1_packages { + // The identity commitment needs to be added to the VSS commitment for every round 1 package + let identity_commitment: Vec> = + vec![CoefficientCommitment::new(C::Group::identity())]; + + let refreshing_share_commitments: Vec> = identity_commitment + .into_iter() + .chain(round1_package.commitment.0.clone()) + .collect(); + + if refreshing_share_commitments.clone().len() != secret_package.min_signers as usize { + return Err(Error::IncorrectNumberOfCommitments); + } + + let ell = *sender_identifier; + + // Round 1, Step 5 + // We don't need to verify the proof of knowledge + + // Round 2, Step 1 + // + // > Each P_i securely sends to each other participant P_ℓ a secret share (ℓ, f_i(ℓ)), + // > deleting f_i and each share afterward except for (i, f_i(i)), + // > which they keep for themselves. + let signing_share = SigningShare::from_coefficients(&secret_package.coefficients, ell); + + round2_packages.insert( + ell, + round2::Package { + header: Header::default(), + signing_share, + }, + ); + } + let fii = evaluate_polynomial(secret_package.identifier, &secret_package.coefficients); + + Ok(( + round2::SecretPackage { + identifier: secret_package.identifier, + commitment: secret_package.commitment, + secret_share: fii, + min_signers: secret_package.min_signers, + max_signers: secret_package.max_signers, + }, + round2_packages, + )) +} + +/// This is the step that actually refreshes the shares. New public key packages +/// and key packages are created. +pub fn refresh_dkg_shares( + round2_secret_package: &round2::SecretPackage, + round1_packages: &BTreeMap, round1::Package>, + round2_packages: &BTreeMap, round2::Package>, + old_pub_key_package: PublicKeyPackage, + old_key_package: KeyPackage, +) -> Result<(KeyPackage, PublicKeyPackage), Error> { + // Add identity commitment back into round1_packages + let mut new_round_1_packages = BTreeMap::new(); + for (sender_identifier, round1_package) in round1_packages { + // The identity commitment needs to be added to the VSS commitment for every round 1 package + let identity_commitment: Vec> = + vec![CoefficientCommitment::new(C::Group::identity())]; + + let refreshing_share_commitments: Vec> = identity_commitment + .into_iter() + .chain(round1_package.commitment.0.clone()) + .collect(); + + let new_commitments = + VerifiableSecretSharingCommitment::::new(refreshing_share_commitments); + + let new_round_1_package = Package { + header: round1_package.header, + commitment: new_commitments, + proof_of_knowledge: round1_package.proof_of_knowledge, + }; + + new_round_1_packages.insert(*sender_identifier, new_round_1_package); + } + + if new_round_1_packages.len() != (round2_secret_package.max_signers - 1) as usize { + return Err(Error::IncorrectNumberOfPackages); + } + if new_round_1_packages.len() != round2_packages.len() { + return Err(Error::IncorrectNumberOfPackages); + } + if new_round_1_packages + .keys() + .any(|id| !round2_packages.contains_key(id)) + { + return Err(Error::IncorrectPackage); + } + + let mut signing_share = <::Field>::zero(); + + for (sender_identifier, round2_package) in round2_packages { + // Round 2, Step 2 + // + // > Each P_i verifies their shares by calculating: + // > g^{f_ℓ(i)} ≟ ∏^{t−1}_{k=0} φ^{i^k mod q}_{ℓk}, aborting if the + // > check fails. + let ell = *sender_identifier; + let f_ell_i = round2_package.signing_share; + + let commitment = &new_round_1_packages + .get(&ell) + .ok_or(Error::PackageNotFound)? + .commitment; + + // The verification is exactly the same as the regular SecretShare verification; + // however the required components are in different places. + // Build a temporary SecretShare so what we can call verify(). + let secret_share = SecretShare { + header: Header::default(), + identifier: round2_secret_package.identifier, + signing_share: f_ell_i, + commitment: commitment.clone(), + }; + + // Verify the share. We don't need the result. + let _ = secret_share.verify()?; + + // Round 2, Step 3 + // + // > Each P_i calculates their long-lived private signing share by computing + // > s_i = ∑^n_{ℓ=1} f_ℓ(i), stores s_i securely, and deletes each f_ℓ(i). + signing_share = signing_share + f_ell_i.to_scalar(); + } + + signing_share = signing_share + round2_secret_package.secret_share; + + // Build new signing share + let old_signing_share = old_key_package.signing_share.to_scalar(); + signing_share = signing_share + old_signing_share; + let signing_share = SigningShare::new(signing_share); + + // Round 2, Step 4 + // + // > Each P_i calculates their public verification share Y_i = g^{s_i}. + let verifying_share = signing_share.into(); + + let commitments: BTreeMap<_, _> = new_round_1_packages + .iter() + .map(|(id, package)| (*id, &package.commitment)) + .chain(iter::once(( + round2_secret_package.identifier, + &round2_secret_package.commitment, + ))) + .collect(); + + let zero_shares_public_key_package = PublicKeyPackage::from_dkg_commitments(&commitments)?; + + let mut new_verifying_shares = BTreeMap::new(); + + for (identifier, verifying_share) in zero_shares_public_key_package.verifying_shares { + let new_verifying_share = verifying_share.to_element() + + old_pub_key_package + .verifying_shares + .get(&identifier) + .ok_or(Error::UnknownIdentifier)? + .to_element(); + new_verifying_shares.insert(identifier, VerifyingShare::new(new_verifying_share)); + } + + let public_key_package = PublicKeyPackage { + header: old_pub_key_package.header, + verifying_shares: new_verifying_shares, + verifying_key: old_pub_key_package.verifying_key, + }; + + let key_package = KeyPackage { + header: Header::default(), + identifier: round2_secret_package.identifier, + signing_share, + verifying_share, + verifying_key: public_key_package.verifying_key, + min_signers: round2_secret_package.min_signers, + }; + + Ok((key_package, public_key_package)) +} diff --git a/frost-core/src/tests/ciphersuite_generic.rs b/frost-core/src/tests/ciphersuite_generic.rs index 3271f26ac..2cf01831e 100644 --- a/frost-core/src/tests/ciphersuite_generic.rs +++ b/frost-core/src/tests/ciphersuite_generic.rs @@ -566,7 +566,7 @@ fn check_part3_errors( } /// Check that calling dkg::part3() with distinct sets of participants fail. -fn check_part3_different_participants( +pub fn check_part3_different_participants( max_signers: u16, round2_secret_packages: BTreeMap, frost::keys::dkg::round2::SecretPackage>, received_round1_packages: BTreeMap< diff --git a/frost-core/src/tests/refresh.rs b/frost-core/src/tests/refresh.rs index 29330b90b..b35ee0d42 100644 --- a/frost-core/src/tests/refresh.rs +++ b/frost-core/src/tests/refresh.rs @@ -1,17 +1,22 @@ //! Test for Refreshing shares -use std::collections::BTreeMap; - use rand_core::{CryptoRng, RngCore}; use crate::keys::generate_with_dealer; -use crate::keys::refresh::{compute_refreshing_shares, refresh_share}; +use crate::keys::refresh::{ + compute_refreshing_shares, refresh_dkg_part2, refresh_dkg_part_1, refresh_share, +}; use crate::{self as frost}; use crate::{ keys::{KeyPackage, PublicKeyPackage, SecretShare}, - Ciphersuite, Error, Identifier, + Ciphersuite, Error, Identifier, Signature, VerifyingKey, }; +use crate::tests::ciphersuite_generic::check_part3_different_participants; + +use alloc::collections::BTreeMap; +use alloc::vec::Vec; + use super::ciphersuite_generic::check_sign; /// We want to test that recover share matches the original share @@ -239,3 +244,188 @@ pub fn check_refresh_shares_with_dealer_serialisation( + mut rng: R, +) -> (Vec, Signature, VerifyingKey) +where + C::Group: core::cmp::PartialEq, +{ + //////////////////////////////////////////////////////////////////////////// + // Old Key generation + //////////////////////////////////////////////////////////////////////////// + + let old_max_signers = 5; + let min_signers = 3; + let (old_shares, pub_key_package) = generate_with_dealer( + old_max_signers, + min_signers, + frost::keys::IdentifierList::Default, + &mut rng, + ) + .unwrap(); + + let mut old_key_packages: BTreeMap, KeyPackage> = BTreeMap::new(); + + for (k, v) in old_shares { + let key_package = KeyPackage::try_from(v).unwrap(); + old_key_packages.insert(k, key_package); + } + + //////////////////////////////////////////////////////////////////////////// + // Key generation, Round 1 + //////////////////////////////////////////////////////////////////////////// + + let max_signers = 4; + let min_signers = 3; + + let remaining_ids = vec![ + Identifier::try_from(4).unwrap(), + Identifier::try_from(2).unwrap(), + Identifier::try_from(3).unwrap(), + Identifier::try_from(1).unwrap(), + ]; + + // Keep track of each participant's round 1 secret package. + // In practice each participant will keep its copy; no one + // will have all the participant's packages. + let mut round1_secret_packages: BTreeMap< + frost::Identifier, + frost::keys::dkg::round1::SecretPackage, + > = BTreeMap::new(); + + // Keep track of all round 1 packages sent to the given participant. + // This is used to simulate the broadcast; in practice the packages + // will be sent through some communication channel. + let mut received_round1_packages: BTreeMap< + frost::Identifier, + BTreeMap, frost::keys::dkg::round1::Package>, + > = BTreeMap::new(); + + // For each participant, perform the first part of the DKG protocol. + // In practice, each participant will perform this on their own environments. + for participant_identifier in remaining_ids.clone() { + let (round1_secret_package, round1_package) = + refresh_dkg_part_1(participant_identifier, max_signers, min_signers, &mut rng).unwrap(); + + // Store the participant's secret package for later use. + // In practice each participant will store it in their own environment. + round1_secret_packages.insert(participant_identifier, round1_secret_package); + + // "Send" the round 1 package to all other participants. In this + // test this is simulated using a BTreeMap; in practice this will be + // sent through some communication channel. + for receiver_participant_identifier in remaining_ids.clone() { + if receiver_participant_identifier == participant_identifier { + continue; + } + received_round1_packages + .entry(receiver_participant_identifier) + .or_default() + .insert(participant_identifier, round1_package.clone()); + } + } + + //////////////////////////////////////////////////////////////////////////// + // Key generation, Round 2 + //////////////////////////////////////////////////////////////////////////// + // Keep track of each participant's round 2 secret package. + // In practice each participant will keep its copy; no one + // will have all the participant's packages. + let mut round2_secret_packages = BTreeMap::new(); + + // Keep track of all round 2 packages sent to the given participant. + // This is used to simulate the broadcast; in practice the packages + // will be sent through some communication channel. + let mut received_round2_packages = BTreeMap::new(); + + // For each participant, perform the second part of the DKG protocol. + // In practice, each participant will perform this on their own environments. + for participant_identifier in remaining_ids.clone() { + let round1_secret_package = round1_secret_packages + .remove(&participant_identifier) + .unwrap(); + let round1_packages = &received_round1_packages[&participant_identifier]; + let (round2_secret_package, round2_packages) = + refresh_dkg_part2(round1_secret_package, round1_packages).expect("should work"); + + // Store the participant's secret package for later use. + // In practice each participant will store it in their own environment. + round2_secret_packages.insert(participant_identifier, round2_secret_package); + + // "Send" the round 2 package to all other participants. In this + // test this is simulated using a BTreeMap; in practice this will be + // sent through some communication channel. + // Note that, in contrast to the previous part, here each other participant + // gets its own specific package. + for (receiver_identifier, round2_package) in round2_packages { + received_round2_packages + .entry(receiver_identifier) + .or_insert_with(BTreeMap::new) + .insert(participant_identifier, round2_package); + } + } + + //////////////////////////////////////////////////////////////////////////// + // Key generation, final computation + //////////////////////////////////////////////////////////////////////////// + + // Keep track of each participant's long-lived key package. + // In practice each participant will keep its copy; no one + // will have all the participant's packages. + let mut key_packages = BTreeMap::new(); + + // Map of the verifying key of each participant. + // Used by the signing test that follows. + let mut verifying_keys = BTreeMap::new(); + // The group public key, used by the signing test that follows. + let mut verifying_key = None; + // For each participant, store the set of verifying keys they have computed. + // This is used to check if the set is correct (the same) for all participants. + // In practice, if there is a Coordinator, only they need to store the set. + // If there is not, then all candidates must store their own sets. + // The verifying keys are used to verify the signature shares produced + // for each signature before being aggregated. + let mut pubkey_packages_by_participant = BTreeMap::new(); + + check_part3_different_participants( + max_signers, + round2_secret_packages.clone(), + received_round1_packages.clone(), + received_round2_packages.clone(), + ); + + // For each participant, this is where they refresh thair shares + // In practice, each participant will perform this on their own environments. + for participant_identifier in remaining_ids.clone() { + let (key_package, pubkey_package_for_participant) = + frost::keys::refresh::refresh_dkg_shares( + &round2_secret_packages[&participant_identifier], + &received_round1_packages[&participant_identifier], + &received_round2_packages[&participant_identifier], + pub_key_package.clone(), + old_key_packages[&participant_identifier].clone(), + ) + .unwrap(); + verifying_keys.insert(participant_identifier, key_package.verifying_share); + // Test if all verifying_key are equal + if let Some(previous_verifying_key) = verifying_key { + assert_eq!(previous_verifying_key, key_package.verifying_key) + } + verifying_key = Some(key_package.verifying_key); + key_packages.insert(participant_identifier, key_package); + pubkey_packages_by_participant + .insert(participant_identifier, pubkey_package_for_participant); + } + + // Test if the set of verifying keys is correct for all participants. + for verifying_keys_for_participant in pubkey_packages_by_participant.values() { + assert!(verifying_keys_for_participant.verifying_shares == verifying_keys); + } + + let pubkeys = frost::keys::PublicKeyPackage::new(verifying_keys, verifying_key.unwrap()); + + // Proceed with the signing test. + check_sign(min_signers, key_packages, rng, pubkeys).unwrap() +} diff --git a/frost-ed25519/src/lib.rs b/frost-ed25519/src/lib.rs index 1e33b2d9c..9c4bce8e8 100644 --- a/frost-ed25519/src/lib.rs +++ b/frost-ed25519/src/lib.rs @@ -326,6 +326,7 @@ pub mod keys { pub type VerifiableSecretSharingCommitment = frost::keys::VerifiableSecretSharingCommitment; pub mod dkg; + pub mod refresh; pub mod repairable; } diff --git a/frost-ed25519/tests/integration_tests.rs b/frost-ed25519/tests/integration_tests.rs index 6c5647882..c8c27fc2a 100644 --- a/frost-ed25519/tests/integration_tests.rs +++ b/frost-ed25519/tests/integration_tests.rs @@ -180,6 +180,13 @@ fn check_refresh_shares_with_dealer_fails_with_invalid_identifier() { >(max_signers, min_signers, &identifiers, error, rng); } +#[test] +fn check_refresh_shares_with_dkg() { + let rng = thread_rng(); + + frost_core::tests::refresh::check_refresh_shares_with_dkg::(rng); +} + #[test] fn check_sign_with_dealer() { let rng = thread_rng(); diff --git a/frost-ed448/src/lib.rs b/frost-ed448/src/lib.rs index 4ceb707e8..56523561d 100644 --- a/frost-ed448/src/lib.rs +++ b/frost-ed448/src/lib.rs @@ -321,6 +321,7 @@ pub mod keys { pub type VerifiableSecretSharingCommitment = frost::keys::VerifiableSecretSharingCommitment; pub mod dkg; + pub mod refresh; pub mod repairable; } diff --git a/frost-ed448/tests/integration_tests.rs b/frost-ed448/tests/integration_tests.rs index 70061503c..b96d83287 100644 --- a/frost-ed448/tests/integration_tests.rs +++ b/frost-ed448/tests/integration_tests.rs @@ -180,6 +180,13 @@ fn check_refresh_shares_with_dealer_fails_with_invalid_identifier() { >(max_signers, min_signers, &identifiers, error, rng); } +#[test] +fn check_refresh_shares_with_dkg() { + let rng = thread_rng(); + + frost_core::tests::refresh::check_refresh_shares_with_dkg::(rng); +} + #[test] fn check_sign_with_dealer() { let rng = thread_rng(); diff --git a/frost-p256/tests/integration_tests.rs b/frost-p256/tests/integration_tests.rs index 8d44312db..acd3f81e9 100644 --- a/frost-p256/tests/integration_tests.rs +++ b/frost-p256/tests/integration_tests.rs @@ -180,6 +180,13 @@ fn check_refresh_shares_with_dealer_fails_with_invalid_identifier() { >(max_signers, min_signers, &identifiers, error, rng); } +#[test] +fn check_refresh_shares_with_dkg() { + let rng = thread_rng(); + + frost_core::tests::refresh::check_refresh_shares_with_dkg::(rng); +} + #[test] fn check_sign_with_dealer() { let rng = thread_rng(); diff --git a/frost-ristretto255/tests/integration_tests.rs b/frost-ristretto255/tests/integration_tests.rs index af536ac3c..743620271 100644 --- a/frost-ristretto255/tests/integration_tests.rs +++ b/frost-ristretto255/tests/integration_tests.rs @@ -181,6 +181,13 @@ fn check_refresh_shares_with_dealer_fails_with_invalid_identifier() { >(max_signers, min_signers, &identifiers, error, rng); } +#[test] +fn check_refresh_shares_with_dkg() { + let rng = thread_rng(); + + frost_core::tests::refresh::check_refresh_shares_with_dkg::(rng); +} + #[test] fn check_sign_with_dealer() { let rng = thread_rng(); diff --git a/frost-secp256k1/src/lib.rs b/frost-secp256k1/src/lib.rs index 903ffda30..eec2c8ee8 100644 --- a/frost-secp256k1/src/lib.rs +++ b/frost-secp256k1/src/lib.rs @@ -336,6 +336,7 @@ pub mod keys { pub type VerifiableSecretSharingCommitment = frost::keys::VerifiableSecretSharingCommitment; pub mod dkg; + pub mod refresh; pub mod repairable; } diff --git a/frost-secp256k1/tests/integration_tests.rs b/frost-secp256k1/tests/integration_tests.rs index 9581384b9..d098f266f 100644 --- a/frost-secp256k1/tests/integration_tests.rs +++ b/frost-secp256k1/tests/integration_tests.rs @@ -180,6 +180,13 @@ fn check_refresh_shares_with_dealer_fails_with_invalid_identifier() { >(max_signers, min_signers, &identifiers, error, rng); } +#[test] +fn check_refresh_shares_with_dkg() { + let rng = thread_rng(); + + frost_core::tests::refresh::check_refresh_shares_with_dkg::(rng); +} + #[test] fn check_sign_with_dealer() { let rng = thread_rng(); From 061d1cb2036c8ce038392346d37588e0d696ec6f Mon Sep 17 00:00:00 2001 From: StackOverflowExcept1on <109800286+StackOverflowExcept1on@users.noreply.github.com> Date: Wed, 11 Dec 2024 22:50:54 +0300 Subject: [PATCH 04/13] fix(frost-secp256k1-tr): add missing `pub mod refresh` and missing test (#806) --- frost-secp256k1-tr/src/lib.rs | 1 + frost-secp256k1-tr/tests/integration_tests.rs | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/frost-secp256k1-tr/src/lib.rs b/frost-secp256k1-tr/src/lib.rs index 5310ced08..3aa51605f 100644 --- a/frost-secp256k1-tr/src/lib.rs +++ b/frost-secp256k1-tr/src/lib.rs @@ -787,6 +787,7 @@ pub mod keys { } pub mod dkg; + pub mod refresh; pub mod repairable; } diff --git a/frost-secp256k1-tr/tests/integration_tests.rs b/frost-secp256k1-tr/tests/integration_tests.rs index 9187285b3..176aad67c 100644 --- a/frost-secp256k1-tr/tests/integration_tests.rs +++ b/frost-secp256k1-tr/tests/integration_tests.rs @@ -181,6 +181,13 @@ fn check_refresh_shares_with_dealer_fails_with_invalid_identifier() { >(max_signers, min_signers, &identifiers, error, rng); } +#[test] +fn check_refresh_shares_with_dkg() { + let rng = thread_rng(); + + frost_core::tests::refresh::check_refresh_shares_with_dkg::(rng); +} + #[test] fn check_sign_with_dealer() { let rng = thread_rng(); From db7888699736e41f844a32397aa4c5b4f79a2c5a Mon Sep 17 00:00:00 2001 From: Dimitris Apostolou Date: Thu, 26 Dec 2024 20:43:54 +0200 Subject: [PATCH 05/13] Fix typos (#819) --- book/src/tutorial/refreshing-shares.md | 2 +- book/src/zcash/ywallet-demo.md | 8 ++++---- frost-core/src/keys.rs | 2 +- frost-core/src/lib.rs | 2 +- frost-core/src/scalar_mul.rs | 2 +- frost-core/src/tests/refresh.rs | 2 +- frost-core/src/traits.rs | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/book/src/tutorial/refreshing-shares.md b/book/src/tutorial/refreshing-shares.md index 5a65f1acc..a32a3b8e2 100644 --- a/book/src/tutorial/refreshing-shares.md +++ b/book/src/tutorial/refreshing-shares.md @@ -30,7 +30,7 @@ The refreshed `KeyPackage` contents must be stored securely and the original Applications should first ensure that all participants who refreshed their `KeyPackages` were actually able to do so successfully, before deleting their old `KeyPackages`. How this is done is up to the application; it might require -sucessfully generating a signature with all of those participants. +successfully generating a signature with all of those participants. ``` ```admonish danger diff --git a/book/src/zcash/ywallet-demo.md b/book/src/zcash/ywallet-demo.md index 71bb3ca0e..7f358368b 100644 --- a/book/src/zcash/ywallet-demo.md +++ b/book/src/zcash/ywallet-demo.md @@ -37,9 +37,9 @@ cargo run --bin trusted-dealer -- -C redpallas This will by default generate a 2-of-3 key shares. The public key package will be written to `public-key-package.json`, while key packages will be -written to `key-package-1.json` through `-3`. You can change the threhsold, +written to `key-package-1.json` through `-3`. You can change the threshold, number of shares and file names using the command line; append `-- -h` -to the commend above for the command line help. +to the command above for the command line help. ```admonish info If you want to use DKG instead of Trusted Dealer, instead of the command above, @@ -198,7 +198,7 @@ cargo run --bin participant -- -C redpallas --http --key-package key-package-1.j ``` (We are using "alice" again. There's nothing stopping a Coordinator from being a -Partcipant too!) +Participant too!) ### Participant 2 (bob) @@ -212,7 +212,7 @@ cargo run --bin participant -- -C redpallas --http --key-package key-package-2.j ### Coordinator Go back to the Coordinator CLI. The protocol should run and complete -succesfully. It will print the final FROST-generated signature. Hurrah! Copy it +successfully. It will print the final FROST-generated signature. Hurrah! Copy it (just the hex value). Go back to the signer and paste the signature. It will write the raw signed diff --git a/frost-core/src/keys.rs b/frost-core/src/keys.rs index 9834665d7..0e78c98b5 100644 --- a/frost-core/src/keys.rs +++ b/frost-core/src/keys.rs @@ -319,7 +319,7 @@ where Self(coefficients) } - /// Returns serialized coefficent commitments + /// Returns serialized coefficient commitments pub fn serialize(&self) -> Result>, Error> { self.0 .iter() diff --git a/frost-core/src/lib.rs b/frost-core/src/lib.rs index 25c8ff990..76078d908 100644 --- a/frost-core/src/lib.rs +++ b/frost-core/src/lib.rs @@ -419,7 +419,7 @@ where ) -> Result, Vec)>, Error> { let mut binding_factor_input_prefix = Vec::new(); - // The length of a serialized verifying key of the same cipersuite does + // The length of a serialized verifying key of the same ciphersuite does // not change between runs of the protocol, so we don't need to hash to // get a fixed length. binding_factor_input_prefix.extend_from_slice(verifying_key.serialize()?.as_ref()); diff --git a/frost-core/src/scalar_mul.rs b/frost-core/src/scalar_mul.rs index 7e6fb3fa3..e8215c443 100644 --- a/frost-core/src/scalar_mul.rs +++ b/frost-core/src/scalar_mul.rs @@ -1,4 +1,4 @@ -//! Non-adjacent form (NAF) implementations for fast batch scalar multiplcation +//! Non-adjacent form (NAF) implementations for fast batch scalar multiplication // We expect slicings in this module to never panic due to algorithmic // constraints. diff --git a/frost-core/src/tests/refresh.rs b/frost-core/src/tests/refresh.rs index b35ee0d42..ca0171973 100644 --- a/frost-core/src/tests/refresh.rs +++ b/frost-core/src/tests/refresh.rs @@ -396,7 +396,7 @@ where received_round2_packages.clone(), ); - // For each participant, this is where they refresh thair shares + // For each participant, this is where they refresh their shares // In practice, each participant will perform this on their own environments. for participant_identifier in remaining_ids.clone() { let (key_package, pubkey_package_for_participant) = diff --git a/frost-core/src/traits.rs b/frost-core/src/traits.rs index 4e3f959d7..91a5056af 100644 --- a/frost-core/src/traits.rs +++ b/frost-core/src/traits.rs @@ -225,7 +225,7 @@ pub trait Ciphersuite: Copy + Clone + PartialEq + Debug + 'static { // protocol if required. /// Optional. Do regular (non-FROST) signing with a [`SigningKey`]. Called - /// by [`SigningKey::sign()`]. This is not used by FROST. Can be overriden + /// by [`SigningKey::sign()`]. This is not used by FROST. Can be overridden /// if required which is useful if FROST signing has been changed by the /// other Ciphersuite trait methods and regular signing should be changed /// accordingly to match. From fa5c842aebbb308084e4034a68d5f3e0ba7aa2e8 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Fri, 27 Dec 2024 18:32:54 -0300 Subject: [PATCH 06/13] ci: fail on doc warnings (#809) --- .github/workflows/main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1e91bca7c..dbc0e1d5b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -176,6 +176,8 @@ jobs: docs: name: Check Rust doc runs-on: ubuntu-latest + env: + RUSTDOCFLAGS: -D warnings steps: - uses: actions/checkout@v4.2.2 From 39b61ec9da75c73476f1573a0781fb87069d86b1 Mon Sep 17 00:00:00 2001 From: Armin Sabouri Date: Mon, 30 Dec 2024 08:30:48 -0500 Subject: [PATCH 07/13] fix(frost-secp256k1-tr): empty merkle root tweak should still hash the x-only pk (#815) Per BIP-341 if there is no script paths the internal key should still be tapTweak'd by tG where t = TaggedHash(P_x). Before this commit the internal key and the taproot output key are the same if no script paths are used. This is because the tweak is the 0 scalar value so Q = P + tG = P. It is worth noting that Bitcoin's consensus would still accept a non-taptweak'd internal key as it verifies a signature against whatever pk is used in the witness program. So the outputs are still spendable, however it deviates from the spec. --- frost-secp256k1-tr/src/lib.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/frost-secp256k1-tr/src/lib.rs b/frost-secp256k1-tr/src/lib.rs index 3aa51605f..f566f72ea 100644 --- a/frost-secp256k1-tr/src/lib.rs +++ b/frost-secp256k1-tr/src/lib.rs @@ -208,7 +208,11 @@ fn tweak>( merkle_root: Option, ) -> Scalar { match merkle_root { - None => Secp256K1ScalarField::zero(), + None => { + let mut hasher = tagged_hash("TapTweak"); + hasher.update(public_key.to_affine().x()); + hasher_to_scalar(hasher) + } Some(root) => { let mut hasher = tagged_hash("TapTweak"); hasher.update(public_key.to_affine().x()); @@ -481,10 +485,9 @@ impl Ciphersuite for Secp256K1Sha256TR { // > key should commit to an unspendable script path instead of having // > no script path. This can be achieved by computing the output key // > point as Q = P + int(hashTapTweak(bytes(P)))G. - let merkle_root = [0u8; 0]; Ok(( - key_package.tweak(Some(&merkle_root)), - public_key_package.tweak(Some(&merkle_root)), + key_package.tweak::<&[u8]>(None), + public_key_package.tweak::<&[u8]>(None), )) } } @@ -862,12 +865,8 @@ pub mod round2 { key_package: &keys::KeyPackage, merkle_root: Option<&[u8]>, ) -> Result { - if merkle_root.is_some() { - let key_package = key_package.clone().tweak(merkle_root); - frost::round2::sign(signing_package, signer_nonces, &key_package) - } else { - frost::round2::sign(signing_package, signer_nonces, key_package) - } + let key_package = key_package.clone().tweak(merkle_root); + frost::round2::sign(signing_package, signer_nonces, &key_package) } } @@ -904,12 +903,8 @@ pub fn aggregate_with_tweak( public_key_package: &keys::PublicKeyPackage, merkle_root: Option<&[u8]>, ) -> Result { - if merkle_root.is_some() { - let public_key_package = public_key_package.clone().tweak(merkle_root); - frost::aggregate(signing_package, signature_shares, &public_key_package) - } else { - frost::aggregate(signing_package, signature_shares, public_key_package) - } + let public_key_package = public_key_package.clone().tweak(merkle_root); + frost::aggregate(signing_package, signature_shares, &public_key_package) } /// A signing key for a Schnorr signature on FROST(secp256k1, SHA-256). From fa3664b0698e323369d3444776a57ae302e63377 Mon Sep 17 00:00:00 2001 From: Armin Sabouri Date: Tue, 10 Dec 2024 16:16:05 -0500 Subject: [PATCH 08/13] feature: introduce signing params --- frost-secp256k1-tr/src/lib.rs | 63 ++++++++++++++++++---- frost-secp256k1-tr/tests/tweaking_tests.rs | 12 +++-- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/frost-secp256k1-tr/src/lib.rs b/frost-secp256k1-tr/src/lib.rs index f566f72ea..632d030d0 100644 --- a/frost-secp256k1-tr/src/lib.rs +++ b/frost-secp256k1-tr/src/lib.rs @@ -95,6 +95,32 @@ impl Field for Secp256K1ScalarField { } } +/// The ciphersuite-specific signing parameters which are fed into +/// signing code to ensure correctly compliant signatures are computed. +#[derive(Debug, Clone, Eq, PartialEq, Default)] +pub struct SigningParameters { + /// The tapscript merkle tree root which must be committed to and agreed upon + /// in advance by all participants in the signing round. + /// + /// If set to `None` (the default), then no taproot tweak will be committed to in the signature. + /// Best practice suggested by BIP341 is to commit to an empty merkle root in cases + /// where no tapscript tweak is needed, i.e. by supplying `&[0; u8]` as the merkle root. + /// This prevents hiding of taproot commitments inside a linearly aggregated key. + /// + /// However, for FROST, this is not strictly required as the group key cannot be + /// poisoned as long as the DKG procedure is conducted correctly. + /// Thus, the [`Default`] trait implementation of taproot `SigningParameters` + /// sets `tapscript_merkle_root` to `None`. + /// + /// If 3rd party observers outside the FROST group must be able to verify there + /// is no hidden script-spending path embedded in the FROST group's taproot output key, + /// then you should set `tapscript_merkle_root` to `Some(vec![])`, which proves + /// the tapscript commitment for the tweaked output key is unspendable. + pub tapscript_merkle_root: Option>, + /// Additional Tweak + pub additional_tweak: Option>, +} + /// An implementation of the FROST(secp256k1, SHA-256) ciphersuite group. #[derive(Clone, Copy, PartialEq, Eq)] pub struct Secp256K1Group; @@ -485,9 +511,18 @@ impl Ciphersuite for Secp256K1Sha256TR { // > key should commit to an unspendable script path instead of having // > no script path. This can be achieved by computing the output key // > point as Q = P + int(hashTapTweak(bytes(P)))G. + // let merkle_root = [0u8; 0].to_vec(); + // TODO(armins) double check this is correct + // There should be no additional tweaks present at the time of dkg + // But does this mean that we can only sign with a additional tweak of 0 bytes? idk rn + let additional_tweak = None; + let signing_parameters = SigningParameters { + tapscript_merkle_root: None, + additional_tweak, + }; Ok(( - key_package.tweak::<&[u8]>(None), - public_key_package.tweak::<&[u8]>(None), + key_package.tweak(&signing_parameters), + public_key_package.tweak(&signing_parameters), )) } } @@ -746,12 +781,15 @@ pub mod keys { /// Trait for tweaking a key component following BIP-341 pub trait Tweak: EvenY { /// Convert the given type to add a tweak. - fn tweak>(self, merkle_root: Option) -> Self; + fn tweak(self, signing_parameters: &SigningParameters) -> Self; } impl Tweak for PublicKeyPackage { - fn tweak>(self, merkle_root: Option) -> Self { - let t = tweak(&self.verifying_key().to_element(), merkle_root); + fn tweak(self, signing_parameters: &SigningParameters) -> Self { + let t = tweak( + &self.verifying_key().to_element(), + signing_parameters.tapscript_merkle_root.clone(), + ); let tp = ProjectivePoint::GENERATOR * t; let public_key_package = self.into_even_y(None); let verifying_key = @@ -771,8 +809,11 @@ pub mod keys { } impl Tweak for KeyPackage { - fn tweak>(self, merkle_root: Option) -> Self { - let t = tweak(&self.verifying_key().to_element(), merkle_root); + fn tweak(self, signing_parameters: &SigningParameters) -> Self { + let t = tweak( + &self.verifying_key().to_element(), + signing_parameters.tapscript_merkle_root.clone(), + ); let tp = ProjectivePoint::GENERATOR * t; let key_package = self.into_even_y(None); let verifying_key = VerifyingKey::new(key_package.verifying_key().to_element() + tp); @@ -863,9 +904,9 @@ pub mod round2 { signing_package: &SigningPackage, signer_nonces: &round1::SigningNonces, key_package: &keys::KeyPackage, - merkle_root: Option<&[u8]>, + signing_parameters: &SigningParameters, ) -> Result { - let key_package = key_package.clone().tweak(merkle_root); + let key_package = key_package.clone().tweak(signing_parameters); frost::round2::sign(signing_package, signer_nonces, &key_package) } } @@ -901,9 +942,9 @@ pub fn aggregate_with_tweak( signing_package: &SigningPackage, signature_shares: &BTreeMap, public_key_package: &keys::PublicKeyPackage, - merkle_root: Option<&[u8]>, + signing_parameters: &SigningParameters, ) -> Result { - let public_key_package = public_key_package.clone().tweak(merkle_root); + let public_key_package = public_key_package.clone().tweak(signing_parameters); frost::aggregate(signing_package, signature_shares, &public_key_package) } diff --git a/frost-secp256k1-tr/tests/tweaking_tests.rs b/frost-secp256k1-tr/tests/tweaking_tests.rs index 3fc74aefb..bed4a0ce8 100644 --- a/frost-secp256k1-tr/tests/tweaking_tests.rs +++ b/frost-secp256k1-tr/tests/tweaking_tests.rs @@ -46,6 +46,10 @@ fn check_tweaked_sign_with_dealer() -> Result<(), Box> { let mut signature_shares = BTreeMap::new(); let message = "message to sign".as_bytes(); let signing_package = frost::SigningPackage::new(commitments_map, message); + let signing_parameters = SigningParameters { + tapscript_merkle_root: Some(merkle_root), + additional_tweak: None, + }; for participant_identifier in nonces_map.keys() { let key_package = &key_packages[participant_identifier]; @@ -54,7 +58,7 @@ fn check_tweaked_sign_with_dealer() -> Result<(), Box> { &signing_package, nonces, key_package, - Some(&merkle_root), + Some(&signing_parameters), )?; signature_shares.insert(*participant_identifier, signature_share); } @@ -63,7 +67,7 @@ fn check_tweaked_sign_with_dealer() -> Result<(), Box> { &signing_package, &signature_shares, &pubkey_package, - Some(&merkle_root), + Some(&signing_parameters), )?; pubkey_package @@ -71,7 +75,7 @@ fn check_tweaked_sign_with_dealer() -> Result<(), Box> { .verify(message, &group_signature) .expect_err("signature should not be valid for untweaked pubkey_package"); - let pubkey_package_tweaked = pubkey_package.clone().tweak(Some(&merkle_root)); + let pubkey_package_tweaked = pubkey_package.clone().tweak(&signing_parameters); pubkey_package_tweaked .verifying_key() .verify(message, &group_signature) @@ -92,7 +96,7 @@ fn check_tweaked_sign_with_dealer() -> Result<(), Box> { .to_affine() .x() .into(), - &merkle_root, + &signing_parameters.tapscript_merkle_root.unwrap(), ); let tr_output_point = pubkey_package_tweaked From 702e2b6ab303ec7dddc10a59a2dad11afafc3840 Mon Sep 17 00:00:00 2001 From: Armin Sabouri Date: Thu, 12 Dec 2024 15:49:45 -0500 Subject: [PATCH 09/13] WIP --- frost-secp256k1-tr/src/lib.rs | 72 +++++++++++-- frost-secp256k1-tr/tests/tweaking_tests.rs | 117 +++++++++++++++++++++ 2 files changed, 178 insertions(+), 11 deletions(-) diff --git a/frost-secp256k1-tr/src/lib.rs b/frost-secp256k1-tr/src/lib.rs index 632d030d0..195048213 100644 --- a/frost-secp256k1-tr/src/lib.rs +++ b/frost-secp256k1-tr/src/lib.rs @@ -209,6 +209,17 @@ const CONTEXT_STRING: &str = "FROST-secp256k1-SHA256-TR-v1"; #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct Secp256K1Sha256TR; +/// Take arbitrary data and convert it to a scalar +pub fn additional_tweak_scalar>( + public_key: &<::Group as Group>::Element, + data: T, +) -> Scalar { + let mut hasher = Sha256::new(); + hasher.update(public_key.to_affine().x()); + hasher.update(data); + hasher_to_scalar(hasher) +} + /// Digest the hasher to a Scalar fn hasher_to_scalar(hasher: Sha256) -> Scalar { // This is acceptable because secp256k1 curve order is close to 2^256, @@ -248,6 +259,18 @@ fn tweak>( } } +/// Create the pre-taptweak'd tweaked internal key +pub fn tweaked_internal_key>( + public_key: &VerifyingKey, + additional_tweak: T, +) -> VerifyingKey { + let mut pk = public_key.into_even_y(None).to_element(); + let e = additional_tweak_scalar(&pk, additional_tweak); + let eg = ProjectivePoint::GENERATOR * e; + pk = eg + pk; + VerifyingKey::new(pk) +} + // Negate a Nonce fn negate_nonce(nonce: &frost_core::round1::Nonce) -> frost_core::round1::Nonce { frost_core::round1::Nonce::::from_scalar(-nonce.to_scalar()) @@ -786,21 +809,33 @@ pub mod keys { impl Tweak for PublicKeyPackage { fn tweak(self, signing_parameters: &SigningParameters) -> Self { + let mut internal_pk = self.verifying_key().clone(); + if let Some(additional_tweak) = &signing_parameters.additional_tweak { + internal_pk = tweaked_internal_key(&internal_pk, additional_tweak); + } + let public_key_package = self.into_even_y(None); let t = tweak( - &self.verifying_key().to_element(), + &internal_pk.to_element(), signing_parameters.tapscript_merkle_root.clone(), ); - let tp = ProjectivePoint::GENERATOR * t; - let public_key_package = self.into_even_y(None); + + let e = additional_tweak_scalar( + &public_key_package.verifying_key().to_element(), + signing_parameters.clone().additional_tweak.unwrap(), + ); + let tweak_scalar = t + e; + let eg = ProjectivePoint::GENERATOR * e; + let tg = ProjectivePoint::GENERATOR * t; + let verifying_key = - VerifyingKey::new(public_key_package.verifying_key().to_element() + tp); + VerifyingKey::new(public_key_package.verifying_key().to_element() + eg + tg); // Recreate verifying share map with negated VerifyingShares // values. let verifying_shares: BTreeMap<_, _> = public_key_package .verifying_shares() .iter() .map(|(i, vs)| { - let vs = VerifyingShare::new(vs.to_element() + tp); + let vs = VerifyingShare::new(vs.to_element() + eg + tg); (*i, vs) }) .collect(); @@ -810,16 +845,31 @@ pub mod keys { impl Tweak for KeyPackage { fn tweak(self, signing_parameters: &SigningParameters) -> Self { + let mut internal_pk = self.verifying_key().clone(); + if let Some(additional_tweak) = &signing_parameters.additional_tweak { + internal_pk = tweaked_internal_key(&internal_pk, additional_tweak); + } + let key_package = self.into_even_y(None); let t = tweak( - &self.verifying_key().to_element(), + &internal_pk.to_element(), signing_parameters.tapscript_merkle_root.clone(), ); - let tp = ProjectivePoint::GENERATOR * t; - let key_package = self.into_even_y(None); - let verifying_key = VerifyingKey::new(key_package.verifying_key().to_element() + tp); - let signing_share = SigningShare::new(key_package.signing_share().to_scalar() + t); + + let e = additional_tweak_scalar( + &key_package.verifying_key().to_element(), + signing_parameters.clone().additional_tweak.unwrap(), + ); + let tweak_scalar = t + e; + + let eg = ProjectivePoint::GENERATOR * e; + let tg = ProjectivePoint::GENERATOR * t; + + let verifying_key = + VerifyingKey::new(key_package.verifying_key().to_element() + eg + tg); + let signing_share = SigningShare::new(key_package.signing_share().to_scalar() + tweak_scalar); let verifying_share = - VerifyingShare::new(key_package.verifying_share().to_element() + tp); + VerifyingShare::new(key_package.verifying_share().to_element() + eg + tg); + KeyPackage::new( *key_package.identifier(), signing_share, diff --git a/frost-secp256k1-tr/tests/tweaking_tests.rs b/frost-secp256k1-tr/tests/tweaking_tests.rs index bed4a0ce8..5f4f223d5 100644 --- a/frost-secp256k1-tr/tests/tweaking_tests.rs +++ b/frost-secp256k1-tr/tests/tweaking_tests.rs @@ -9,6 +9,123 @@ use frost_secp256k1_tr::*; mod helpers; +#[test] +fn check_additional_tweaked_sign_with_dealer() -> Result<(), Box> { + use frost_secp256k1_tr as frost; + use rand::thread_rng; + use std::collections::BTreeMap; + + let merkle_root: Vec = vec![12; 32]; + let additional_tweak: Vec = vec![12; 20]; + let mut rng = thread_rng(); + let max_signers = 5; + let min_signers = 3; + /* KEY GENERATION */ + let (shares, pubkey_package) = frost::keys::generate_with_dealer( + max_signers, + min_signers, + frost::keys::IdentifierList::Default, + &mut rng, + )?; + let mut key_packages: BTreeMap<_, _> = BTreeMap::new(); + for (identifier, secret_share) in shares { + let key_package = frost::keys::KeyPackage::try_from(secret_share)?; + key_packages.insert(identifier, key_package); + } + + /* ROUND 1 */ + + let mut nonces_map = BTreeMap::new(); + let mut commitments_map = BTreeMap::new(); + + for participant_index in 1..=min_signers { + let participant_identifier = participant_index.try_into().expect("should be nonzero"); + let key_package = &key_packages[&participant_identifier]; + let (nonces, commitments) = frost::round1::commit(key_package.signing_share(), &mut rng); + nonces_map.insert(participant_identifier, nonces); + commitments_map.insert(participant_identifier, commitments); + } + + let mut signature_shares = BTreeMap::new(); + let message = "message to sign".as_bytes(); + let signing_package = frost::SigningPackage::new(commitments_map, message); + let signing_parameters = SigningParameters { + tapscript_merkle_root: Some(merkle_root), + additional_tweak: Some(additional_tweak), + }; + + for participant_identifier in nonces_map.keys() { + let key_package = &key_packages[participant_identifier]; + let nonces = &nonces_map[participant_identifier]; + let signature_share = frost::round2::sign_with_tweak( + &signing_package, + nonces, + key_package, + Some(&signing_parameters), + )?; + signature_shares.insert(*participant_identifier, signature_share); + } + + let group_signature = frost::aggregate_with_tweak( + &signing_package, + &signature_shares, + &pubkey_package, + Some(&signing_parameters), + )?; + + pubkey_package + .verifying_key() + .verify(message, &group_signature) + .expect_err("signature should not be valid for untweaked pubkey_package"); + + let pubkey_package_tweaked = pubkey_package.clone().tweak(&signing_parameters); + pubkey_package_tweaked + .verifying_key() + .verify(message, &group_signature) + .expect("signature should be valid for tweaked pubkey_package"); + + helpers::verify_signature( + message, + &group_signature, + pubkey_package_tweaked.verifying_key(), + ); + + println!("Made it here"); + + // Confirm the internal (untweaked) group key can be provided to access + // script spending paths under the output (tweaked) group key. + // let (expected_parity, expected_tr_output_pubkey) = taproot_tweak_pubkey( + // pubkey_package + // .verifying_key() + // .to_element() + // .to_affine() + // .x() + // .into(), + // &signing_parameters.tapscript_merkle_root.unwrap(), + // ); + + // let tr_output_point = pubkey_package_tweaked + // .verifying_key() + // .to_element() + // .to_affine(); + + // let tr_output_pubkey: [u8; 32] = tr_output_point.x().into(); + // let tr_output_parity: bool = tr_output_point.y_is_odd().into(); + + // assert_eq!( + // tr_output_pubkey, expected_tr_output_pubkey, + // "taproot output pubkey does not match" + // ); + + // assert_eq!( + // tr_output_parity, expected_parity, + // "taproot output pubkey parity bit does not match" + // ); + + Ok(()) +} + + #[test] fn check_tweaked_sign_with_dealer() -> Result<(), Box> { use frost_secp256k1_tr as frost; From 1b87811b9d102def0f09cd6269ae6bf66eb06dad Mon Sep 17 00:00:00 2001 From: Armin Sabouri Date: Fri, 13 Dec 2024 09:37:44 -0500 Subject: [PATCH 10/13] additional tweak can be optional --- frost-secp256k1-tr/src/lib.rs | 72 ++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/frost-secp256k1-tr/src/lib.rs b/frost-secp256k1-tr/src/lib.rs index 195048213..54e0886b5 100644 --- a/frost-secp256k1-tr/src/lib.rs +++ b/frost-secp256k1-tr/src/lib.rs @@ -814,28 +814,33 @@ pub mod keys { internal_pk = tweaked_internal_key(&internal_pk, additional_tweak); } let public_key_package = self.into_even_y(None); - let t = tweak( - &internal_pk.to_element(), - signing_parameters.tapscript_merkle_root.clone(), - ); - - let e = additional_tweak_scalar( - &public_key_package.verifying_key().to_element(), - signing_parameters.clone().additional_tweak.unwrap(), - ); - let tweak_scalar = t + e; - let eg = ProjectivePoint::GENERATOR * e; - let tg = ProjectivePoint::GENERATOR * t; + let tweak_point = { + let t = tweak( + &internal_pk.to_element(), + signing_parameters.tapscript_merkle_root.clone(), + ); + let tg = ProjectivePoint::GENERATOR * t; + if let Some(additional_tweak) = &signing_parameters.additional_tweak { + let e = additional_tweak_scalar( + &public_key_package.verifying_key().to_element(), + additional_tweak, + ); + let eg = ProjectivePoint::GENERATOR * e; + eg + tg + } else { + tg + } + }; let verifying_key = - VerifyingKey::new(public_key_package.verifying_key().to_element() + eg + tg); + VerifyingKey::new(public_key_package.verifying_key().to_element() + tweak_point); // Recreate verifying share map with negated VerifyingShares // values. let verifying_shares: BTreeMap<_, _> = public_key_package .verifying_shares() .iter() .map(|(i, vs)| { - let vs = VerifyingShare::new(vs.to_element() + eg + tg); + let vs = VerifyingShare::new(vs.to_element() + tweak_point); (*i, vs) }) .collect(); @@ -846,29 +851,34 @@ pub mod keys { impl Tweak for KeyPackage { fn tweak(self, signing_parameters: &SigningParameters) -> Self { let mut internal_pk = self.verifying_key().clone(); + let key_package = self.into_even_y(None); if let Some(additional_tweak) = &signing_parameters.additional_tweak { internal_pk = tweaked_internal_key(&internal_pk, additional_tweak); } - let key_package = self.into_even_y(None); - let t = tweak( - &internal_pk.to_element(), - signing_parameters.tapscript_merkle_root.clone(), - ); - - let e = additional_tweak_scalar( - &key_package.verifying_key().to_element(), - signing_parameters.clone().additional_tweak.unwrap(), - ); - let tweak_scalar = t + e; - - let eg = ProjectivePoint::GENERATOR * e; - let tg = ProjectivePoint::GENERATOR * t; + let (tweak_point, tweak_scalar) = { + let t = tweak( + &internal_pk.to_element(), + signing_parameters.tapscript_merkle_root.clone(), + ); + let tg = ProjectivePoint::GENERATOR * t; + if let Some(additional_tweak) = &signing_parameters.additional_tweak { + let e = additional_tweak_scalar( + &key_package.verifying_key().to_element(), + additional_tweak, + ); + let eg = ProjectivePoint::GENERATOR * e; + (eg + tg, t + e) + } else { + (tg, t) + } + }; let verifying_key = - VerifyingKey::new(key_package.verifying_key().to_element() + eg + tg); - let signing_share = SigningShare::new(key_package.signing_share().to_scalar() + tweak_scalar); + VerifyingKey::new(key_package.verifying_key().to_element() + tweak_point); + let signing_share = + SigningShare::new(key_package.signing_share().to_scalar() + tweak_scalar); let verifying_share = - VerifyingShare::new(key_package.verifying_share().to_element() + eg + tg); + VerifyingShare::new(key_package.verifying_share().to_element() + tweak_point); KeyPackage::new( *key_package.identifier(), From 847a565ced650357b32e96306f49da110e09d62d Mon Sep 17 00:00:00 2001 From: Armin Sabouri Date: Mon, 16 Dec 2024 15:22:14 -0500 Subject: [PATCH 11/13] impl tweak for vk --- frost-secp256k1-tr/src/lib.rs | 28 ++++++++++++++++++++++ frost-secp256k1-tr/tests/tweaking_tests.rs | 4 ++++ 2 files changed, 32 insertions(+) diff --git a/frost-secp256k1-tr/src/lib.rs b/frost-secp256k1-tr/src/lib.rs index 54e0886b5..507d9d9d4 100644 --- a/frost-secp256k1-tr/src/lib.rs +++ b/frost-secp256k1-tr/src/lib.rs @@ -807,6 +807,34 @@ pub mod keys { fn tweak(self, signing_parameters: &SigningParameters) -> Self; } + impl Tweak for VerifyingKey { + fn tweak(self, signing_parameters: &SigningParameters) -> Self { + let mut internal_pk = self.clone(); + if let Some(additional_tweak) = &signing_parameters.additional_tweak { + internal_pk = tweaked_internal_key(&internal_pk, additional_tweak); + } + + let tweak_point = { + let t = tweak( + &internal_pk.to_element(), + signing_parameters.tapscript_merkle_root.clone(), + ); + let tg = ProjectivePoint::GENERATOR * t; + if let Some(additional_tweak) = &signing_parameters.additional_tweak { + let e = additional_tweak_scalar( + &self.to_element(), + additional_tweak, + ); + let eg = ProjectivePoint::GENERATOR * e; + eg + tg + } else { + tg + } + }; + VerifyingKey::new(self.to_element() + tweak_point) + } + } + impl Tweak for PublicKeyPackage { fn tweak(self, signing_parameters: &SigningParameters) -> Self { let mut internal_pk = self.verifying_key().clone(); diff --git a/frost-secp256k1-tr/tests/tweaking_tests.rs b/frost-secp256k1-tr/tests/tweaking_tests.rs index 5f4f223d5..09affc7bc 100644 --- a/frost-secp256k1-tr/tests/tweaking_tests.rs +++ b/frost-secp256k1-tr/tests/tweaking_tests.rs @@ -84,6 +84,10 @@ fn check_additional_tweaked_sign_with_dealer() -> Result<(), Box> { .verify(message, &group_signature) .expect("signature should be valid for tweaked pubkey_package"); + let vk_tweaked = pubkey_package.verifying_key().tweak(&signing_parameters); + + assert_eq!(vk_tweaked, pubkey_package_tweaked.verifying_key().clone()); + helpers::verify_signature( message, &group_signature, From c018ae388c7a10fb9534391b8d09cb07230b78c4 Mon Sep 17 00:00:00 2001 From: Armin Sabouri Date: Mon, 30 Dec 2024 09:17:02 -0500 Subject: [PATCH 12/13] fix: tweaking tests should pass non-optional tweak params --- frost-secp256k1-tr/tests/tweaking_tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frost-secp256k1-tr/tests/tweaking_tests.rs b/frost-secp256k1-tr/tests/tweaking_tests.rs index 09affc7bc..0b37823d9 100644 --- a/frost-secp256k1-tr/tests/tweaking_tests.rs +++ b/frost-secp256k1-tr/tests/tweaking_tests.rs @@ -61,7 +61,7 @@ fn check_additional_tweaked_sign_with_dealer() -> Result<(), Box> { &signing_package, nonces, key_package, - Some(&signing_parameters), + &signing_parameters, )?; signature_shares.insert(*participant_identifier, signature_share); } @@ -70,7 +70,7 @@ fn check_additional_tweaked_sign_with_dealer() -> Result<(), Box> { &signing_package, &signature_shares, &pubkey_package, - Some(&signing_parameters), + &signing_parameters, )?; pubkey_package @@ -179,7 +179,7 @@ fn check_tweaked_sign_with_dealer() -> Result<(), Box> { &signing_package, nonces, key_package, - Some(&signing_parameters), + &signing_parameters, )?; signature_shares.insert(*participant_identifier, signature_share); } @@ -188,7 +188,7 @@ fn check_tweaked_sign_with_dealer() -> Result<(), Box> { &signing_package, &signature_shares, &pubkey_package, - Some(&signing_parameters), + &signing_parameters, )?; pubkey_package From b4c5ab017bb0d182f3c56fd80b95d6375a4bfeea Mon Sep 17 00:00:00 2001 From: Armin Sabouri Date: Thu, 9 Jan 2025 11:56:16 -0500 Subject: [PATCH 13/13] fix: tweaking vk should apply final tweak point against even y key --- frost-secp256k1-tr/src/lib.rs | 4 +- frost-secp256k1-tr/tests/tweaking_tests.rs | 47 +++++++++++++++++++--- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/frost-secp256k1-tr/src/lib.rs b/frost-secp256k1-tr/src/lib.rs index 507d9d9d4..a44b47c62 100644 --- a/frost-secp256k1-tr/src/lib.rs +++ b/frost-secp256k1-tr/src/lib.rs @@ -811,6 +811,7 @@ pub mod keys { fn tweak(self, signing_parameters: &SigningParameters) -> Self { let mut internal_pk = self.clone(); if let Some(additional_tweak) = &signing_parameters.additional_tweak { + // Note that tweaked internal key will tweak with a even y key internal_pk = tweaked_internal_key(&internal_pk, additional_tweak); } @@ -831,7 +832,8 @@ pub mod keys { tg } }; - VerifyingKey::new(self.to_element() + tweak_point) + let vk_y_even = self.into_even_y(None); + VerifyingKey::new(vk_y_even.to_element() + tweak_point) } } diff --git a/frost-secp256k1-tr/tests/tweaking_tests.rs b/frost-secp256k1-tr/tests/tweaking_tests.rs index 0b37823d9..8b8580795 100644 --- a/frost-secp256k1-tr/tests/tweaking_tests.rs +++ b/frost-secp256k1-tr/tests/tweaking_tests.rs @@ -8,13 +8,51 @@ use sha2::{Digest, Sha256}; use frost_secp256k1_tr::*; mod helpers; +use frost_secp256k1_tr as frost; +use rand::thread_rng; +use std::collections::BTreeMap; #[test] -fn check_additional_tweaked_sign_with_dealer() -> Result<(), Box> { - use frost_secp256k1_tr as frost; - use rand::thread_rng; - use std::collections::BTreeMap; +fn check_tweaked_vk() -> Result<(), Box> { + let num_iterations = 1_000; + let additional_tweak: Vec = vec![12; 20]; + let mut rng = thread_rng(); + let max_signers = 5; + let min_signers = 3; + for i in 0..num_iterations { + println!("Iteration {}", i); + /* KEY GENERATION */ + let (shares, pubkey_package) = frost::keys::generate_with_dealer( + max_signers, + min_signers, + frost::keys::IdentifierList::Default, + &mut rng, + )?; + let mut key_packages: BTreeMap<_, _> = BTreeMap::new(); + for (identifier, secret_share) in shares { + let key_package = frost::keys::KeyPackage::try_from(secret_share)?; + key_packages.insert(identifier, key_package); + } + + let signing_parameters = SigningParameters { + tapscript_merkle_root: None, + additional_tweak: Some(additional_tweak.clone()), + }; + + let vk = pubkey_package.verifying_key(); + + let vk_tweaked = vk.tweak(&signing_parameters); + let pubkey_package_tweaked = pubkey_package.clone().tweak(&signing_parameters); + + assert_eq!(vk_tweaked, pubkey_package_tweaked.verifying_key().clone()); + } + + Ok(()) +} + +#[test] +fn check_additional_tweaked_sign_with_dealer() -> Result<(), Box> { let merkle_root: Vec = vec![12; 32]; let additional_tweak: Vec = vec![12; 20]; let mut rng = thread_rng(); @@ -129,7 +167,6 @@ fn check_additional_tweaked_sign_with_dealer() -> Result<(), Box> { Ok(()) } - #[test] fn check_tweaked_sign_with_dealer() -> Result<(), Box> { use frost_secp256k1_tr as frost;