From 9b5d88da721ae5cb315c686c1f0ca65b4b26f976 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Wed, 19 Jul 2023 13:47:09 -0300 Subject: [PATCH] refactor Lagrange coefficient computation (#436) * refactor Lagrange coefficient computation * A line * Apply suggestions from code review Co-authored-by: Deirdre Connolly * address review comments; make compute_lagrange_coefficients() not pub by default --------- Co-authored-by: Deirdre Connolly --- frost-core/src/error.rs | 14 +++- frost-core/src/frost.rs | 83 +++++++++++++++------ frost-core/src/frost/keys.rs | 47 ++++-------- frost-core/src/frost/keys/repairable.rs | 52 +++++-------- frost-core/src/tests/ciphersuite_generic.rs | 4 +- frost-core/src/tests/repairable.rs | 26 ++++--- frost-ed25519/src/keys/repairable.rs | 4 +- frost-ed448/src/keys/repairable.rs | 4 +- frost-p256/src/keys/repairable.rs | 4 +- frost-ristretto255/src/keys/repairable.rs | 4 +- frost-secp256k1/src/keys/repairable.rs | 4 +- 11 files changed, 132 insertions(+), 114 deletions(-) diff --git a/frost-core/src/error.rs b/frost-core/src/error.rs index 60adce3..8845dd3 100644 --- a/frost-core/src/error.rs +++ b/frost-core/src/error.rs @@ -24,8 +24,14 @@ pub enum Error { #[error("Malformed identifier is unserializable.")] MalformedIdentifier, /// This identifier is duplicated. - #[error("Duplicated identifier.")] - DuplicatedIdentifier, + #[error("Duplicated identifiers.")] + DuplicatedIdentifiers, + /// This identifier does not belong to a participant in the signing process. + #[error("Unknown identifier.")] + UnknownIdentifier, + /// Incorrect number of identifiers. + #[error("Incorrect number of identifiers.")] + IncorrectNumberOfIdentifiers, /// The encoding of a signing key was malformed. #[error("Malformed signing key encoding.")] MalformedSigningKey, @@ -125,8 +131,10 @@ where | Error::DKGNotSupported | Error::FieldError(_) | Error::GroupError(_) - | Error::DuplicatedIdentifier + | Error::DuplicatedIdentifiers | Error::InvalidCoefficient + | Error::UnknownIdentifier + | Error::IncorrectNumberOfIdentifiers | Error::IdentifierDerivationNotSupported => None, } } diff --git a/frost-core/src/frost.rs b/frost-core/src/frost.rs index 7717a41..217ff3d 100644 --- a/frost-core/src/frost.rs +++ b/frost-core/src/frost.rs @@ -11,7 +11,7 @@ //! Sharing, where shares are generated using Shamir Secret Sharing. use std::{ - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, BTreeSet, HashMap}, fmt::{self, Debug}, ops::Index, }; @@ -148,34 +148,69 @@ where } } -/// Generates the lagrange coefficient for the i'th participant. +/// Generates a lagrange coefficient. +/// +/// The Lagrange polynomial for a set of points (x_j, y_j) for 0 <= j <= k +/// is ∑_{i=0}^k y_i.ℓ_i(x), where ℓ_i(x) is the Lagrange basis polynomial: +/// +/// ℓ_i(x) = ∏_{0≤j≤k; j≠i} (x - x_j) / (x_i - x_j). +/// +/// This computes ℓ_j(x) for the set of points `xs` and for the j corresponding +/// to the given xj. +/// +/// If `x` is None, it uses 0 for it (since Identifiers can't be 0) +#[cfg_attr(feature = "internals", visibility::make(pub))] +fn compute_lagrange_coefficient( + x_set: &BTreeSet>, + x: Option>, + x_i: Identifier, +) -> Result, Error> { + if x_set.is_empty() { + return Err(Error::IncorrectNumberOfIdentifiers); + } + let mut num = <::Field>::one(); + let mut den = <::Field>::one(); + + let mut x_i_found = false; + + for x_j in x_set.iter() { + if x_i == *x_j { + x_i_found = true; + continue; + } + + if let Some(x) = x { + num *= x - *x_j; + den *= x_i - *x_j; + } else { + // Both signs inverted just to avoid requiring Neg (-*xj) + num *= *x_j; + den *= *x_j - x_i; + } + } + if !x_i_found { + return Err(Error::UnknownIdentifier); + } + + Ok(num + * <::Field>::invert(&den).map_err(|_| Error::DuplicatedIdentifiers)?) +} + +/// Generates the lagrange coefficient for the i'th participant (for `signer_id`). #[cfg_attr(feature = "internals", visibility::make(pub))] fn derive_interpolating_value( signer_id: &Identifier, signing_package: &SigningPackage, ) -> Result, Error> { - let zero = <::Field>::zero(); - - let mut num = <::Field>::one(); - let mut den = <::Field>::one(); - - for commitment_identifier in signing_package.signing_commitments().keys() { - if *commitment_identifier == *signer_id { - continue; - } - - num *= *commitment_identifier; - - den *= *commitment_identifier - *signer_id; - } - - if den == zero { - return Err(Error::DuplicatedShares); - } - - let lagrange_coeff = num * <::Field>::invert(&den).unwrap(); - - Ok(lagrange_coeff) + compute_lagrange_coefficient( + &signing_package + .signing_commitments() + .keys() + .cloned() + .collect(), + None, + *signer_id, + ) } /// Generated by the coordinator of the signing operation and distributed to diff --git a/frost-core/src/frost/keys.rs b/frost-core/src/frost/keys.rs index 1e9c488..bc613b1 100644 --- a/frost-core/src/frost/keys.rs +++ b/frost-core/src/frost/keys.rs @@ -2,7 +2,7 @@ #![allow(clippy::type_complexity)] use std::{ - collections::{HashMap, HashSet}, + collections::{BTreeSet, HashMap, HashSet}, convert::TryFrom, default::Default, fmt::{self, Debug}, @@ -23,6 +23,8 @@ use crate::{ #[cfg(feature = "serde")] use crate::{ElementSerialization, ScalarSerialization}; +use super::compute_lagrange_coefficient; + pub mod dkg; pub mod repairable; @@ -716,7 +718,7 @@ pub(crate) fn generate_secret_shares( let identifiers_set: HashSet<_> = identifiers.iter().collect(); if identifiers_set.len() != identifiers.len() { - return Err(Error::DuplicatedIdentifier); + return Err(Error::DuplicatedIdentifiers); } for id in identifiers { @@ -754,39 +756,20 @@ pub fn reconstruct( let mut secret = <::Field>::zero(); - // Compute the Lagrange coefficients - for (i_idx, i, secret_share) in secret_shares + let identifiers: BTreeSet<_> = secret_shares .iter() - .enumerate() - .map(|(idx, s)| (idx, s.identifier, s)) - { - let mut num = <::Field>::one(); - let mut den = <::Field>::one(); + .map(|s| s.identifier()) + .cloned() + .collect(); - for (j_idx, j) in secret_shares - .iter() - .enumerate() - .map(|(idx, s)| (idx, s.identifier)) - { - if j_idx == i_idx { - continue; - } + if identifiers.len() != secret_shares.len() { + return Err(Error::DuplicatedIdentifiers); + } - // numerator *= j - num *= j; - - // denominator *= j - i - den *= j - i; - } - - // If at this step, the denominator is zero in the scalar field, there must be a duplicate - // secret share. - if den == <::Field>::zero() { - return Err(Error::DuplicatedShares); - } - - // Save numerator * 1/denominator in the scalar field - let lagrange_coefficient = num * <::Field>::invert(&den).unwrap(); + // Compute the Lagrange coefficients + for secret_share in secret_shares.iter() { + let lagrange_coefficient = + compute_lagrange_coefficient(&identifiers, None, secret_share.identifier)?; // Compute y = f(0) via polynomial interpolation of these t-of-n solutions ('points) of f secret = secret + (lagrange_coefficient * secret_share.value.0); diff --git a/frost-core/src/frost/keys/repairable.rs b/frost-core/src/frost/keys/repairable.rs index fb8a9c2..34f20b5 100644 --- a/frost-core/src/frost/keys/repairable.rs +++ b/frost-core/src/frost/keys/repairable.rs @@ -4,9 +4,12 @@ //! The RTS is used to help a signer (participant) repair their lost share. This is achieved //! using a subset of the other signers know here as `helpers`. -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; -use crate::{frost::Identifier, Ciphersuite, CryptoRng, Field, Group, RngCore, Scalar}; +use crate::{ + frost::{compute_lagrange_coefficient, Identifier}, + Ciphersuite, CryptoRng, Error, Field, Group, RngCore, Scalar, +}; use super::{generate_coefficients, SecretShare, SigningShare, VerifiableSecretSharingCommitment}; @@ -22,10 +25,18 @@ pub fn repair_share_step_1( share_i: &SecretShare, rng: &mut R, participant: Identifier, -) -> HashMap, Scalar> { +) -> Result, Scalar>, Error> { + if helpers.is_empty() { + return Err(Error::IncorrectNumberOfIdentifiers); + } + let xset: BTreeSet<_> = helpers.iter().cloned().collect(); + if xset.len() != helpers.len() { + return Err(Error::DuplicatedIdentifiers); + } + let rand_val: Vec> = generate_coefficients::(helpers.len() - 1, rng); - compute_last_random_value(helpers, share_i, &rand_val, participant) + compute_last_random_value(&xset, share_i, &rand_val, participant) } /// Compute the last delta value given the (generated uniformly at random) remaining ones @@ -33,13 +44,13 @@ pub fn repair_share_step_1( /// /// Returns a HashMap mapping which value should be sent to which participant. fn compute_last_random_value( - helpers: &[Identifier], + helpers: &BTreeSet>, share_i: &SecretShare, random_values: &Vec>, participant: Identifier, -) -> HashMap, Scalar> { +) -> Result, Scalar>, Error> { // Calculate Lagrange Coefficient for helper_i - let zeta_i = compute_lagrange_coefficient(helpers, participant, share_i.identifier); + let zeta_i = compute_lagrange_coefficient(helpers, Some(participant), share_i.identifier)?; let lhs = zeta_i * share_i.value.0; @@ -55,32 +66,9 @@ fn compute_last_random_value( sum_i_deltas = sum_i_deltas + *v; } - out.insert(helpers[helpers.len() - 1], lhs - sum_i_deltas); + out.insert(*helpers.last().unwrap(), lhs - sum_i_deltas); - out -} - -/// Compute the i-th Lagrange coefficient evaluated at `participant`, i.e. -/// computes `zeta_i` such that f(participant) is the sum of all `zeta_i * share_i` -/// for each `i` in `helpers`. -pub fn compute_lagrange_coefficient( - helpers: &[Identifier], - participant: Identifier, - helper_i: Identifier, -) -> Scalar { - let mut num = <::Field>::one(); - let mut den = <::Field>::one(); - - for j in helpers.iter() { - if helper_i == *j { - continue; - } - - num *= participant - *j; - den *= helper_i - *j; - } - - num * <::Field>::invert(&den).unwrap() + Ok(out) } /// Communication round diff --git a/frost-core/src/tests/ciphersuite_generic.rs b/frost-core/src/tests/ciphersuite_generic.rs index a15cc25..3e43b0d 100644 --- a/frost-core/src/tests/ciphersuite_generic.rs +++ b/frost-core/src/tests/ciphersuite_generic.rs @@ -55,7 +55,7 @@ pub fn check_share_generation(mut rng: R assert_eq!( frost::keys::reconstruct::(&secret_shares).unwrap_err(), - Error::DuplicatedShares + Error::DuplicatedIdentifiers ); } @@ -372,7 +372,7 @@ pub fn check_sign_with_dealer_and_identifiers(mut rng: R) { // Each helper generates random values for each helper - let helper_1_deltas = repair_share_step_1(&helpers, helper_1, &mut rng, participant.identifier); - let helper_4_deltas = repair_share_step_1(&helpers, helper_4, &mut rng, participant.identifier); - let helper_5_deltas = repair_share_step_1(&helpers, helper_5, &mut rng, participant.identifier); + let helper_1_deltas = + repair_share_step_1(&helpers, helper_1, &mut rng, participant.identifier).unwrap(); + let helper_4_deltas = + repair_share_step_1(&helpers, helper_4, &mut rng, participant.identifier).unwrap(); + let helper_5_deltas = + repair_share_step_1(&helpers, helper_5, &mut rng, participant.identifier).unwrap(); // Each helper calculates their sigma from the random values received from the other helpers @@ -130,10 +130,14 @@ pub fn check_repair_share_step_1(mut rng ]; // Generate deltas for helper 4 - let deltas = repair_share_step_1(&helpers, helper_4, &mut rng, participant.identifier); + let deltas = repair_share_step_1(&helpers, helper_4, &mut rng, participant.identifier).unwrap(); - let lagrange_coefficient = - compute_lagrange_coefficient(&helpers, participant.identifier, helpers[1]); + let lagrange_coefficient = compute_lagrange_coefficient( + &helpers.iter().cloned().collect(), + Some(participant.identifier), + helpers[1], + ) + .unwrap(); let mut rhs = <::Field>::zero(); for (_k, v) in deltas { diff --git a/frost-ed25519/src/keys/repairable.rs b/frost-ed25519/src/keys/repairable.rs index edac670..5234099 100644 --- a/frost-ed25519/src/keys/repairable.rs +++ b/frost-ed25519/src/keys/repairable.rs @@ -9,8 +9,8 @@ use std::collections::HashMap; // This is imported separately to make `gencode` work. // (if it were below, the position of the import would vary between ciphersuites // after `cargo fmt`) -use crate::Ed25519Sha512; use crate::{frost, Ciphersuite, CryptoRng, Identifier, RngCore, Scalar}; +use crate::{Ed25519Sha512, Error}; use super::{SecretShare, VerifiableSecretSharingCommitment}; @@ -26,7 +26,7 @@ pub fn repair_share_step_1( share_i: &SecretShare, rng: &mut R, participant: Identifier, -) -> HashMap { +) -> Result, Error> { frost::keys::repairable::repair_share_step_1(helpers, share_i, rng, participant) } diff --git a/frost-ed448/src/keys/repairable.rs b/frost-ed448/src/keys/repairable.rs index 2e8fbac..64a95aa 100644 --- a/frost-ed448/src/keys/repairable.rs +++ b/frost-ed448/src/keys/repairable.rs @@ -9,8 +9,8 @@ use std::collections::HashMap; // This is imported separately to make `gencode` work. // (if it were below, the position of the import would vary between ciphersuites // after `cargo fmt`) -use crate::Ed448Shake256; use crate::{frost, Ciphersuite, CryptoRng, Identifier, RngCore, Scalar}; +use crate::{Ed448Shake256, Error}; use super::{SecretShare, VerifiableSecretSharingCommitment}; @@ -26,7 +26,7 @@ pub fn repair_share_step_1( share_i: &SecretShare, rng: &mut R, participant: Identifier, -) -> HashMap { +) -> Result, Error> { frost::keys::repairable::repair_share_step_1(helpers, share_i, rng, participant) } diff --git a/frost-p256/src/keys/repairable.rs b/frost-p256/src/keys/repairable.rs index 5aa9728..33e09a1 100644 --- a/frost-p256/src/keys/repairable.rs +++ b/frost-p256/src/keys/repairable.rs @@ -9,8 +9,8 @@ use std::collections::HashMap; // This is imported separately to make `gencode` work. // (if it were below, the position of the import would vary between ciphersuites // after `cargo fmt`) -use crate::P256Sha256; use crate::{frost, Ciphersuite, CryptoRng, Identifier, RngCore, Scalar}; +use crate::{Error, P256Sha256}; use super::{SecretShare, VerifiableSecretSharingCommitment}; @@ -26,7 +26,7 @@ pub fn repair_share_step_1( share_i: &SecretShare, rng: &mut R, participant: Identifier, -) -> HashMap { +) -> Result, Error> { frost::keys::repairable::repair_share_step_1(helpers, share_i, rng, participant) } diff --git a/frost-ristretto255/src/keys/repairable.rs b/frost-ristretto255/src/keys/repairable.rs index 5b325f0..a74019a 100644 --- a/frost-ristretto255/src/keys/repairable.rs +++ b/frost-ristretto255/src/keys/repairable.rs @@ -9,8 +9,8 @@ use std::collections::HashMap; // This is imported separately to make `gencode` work. // (if it were below, the position of the import would vary between ciphersuites // after `cargo fmt`) -use crate::Ristretto255Sha512; use crate::{frost, Ciphersuite, CryptoRng, Identifier, RngCore, Scalar}; +use crate::{Error, Ristretto255Sha512}; use super::{SecretShare, VerifiableSecretSharingCommitment}; @@ -26,7 +26,7 @@ pub fn repair_share_step_1( share_i: &SecretShare, rng: &mut R, participant: Identifier, -) -> HashMap { +) -> Result, Error> { frost::keys::repairable::repair_share_step_1(helpers, share_i, rng, participant) } diff --git a/frost-secp256k1/src/keys/repairable.rs b/frost-secp256k1/src/keys/repairable.rs index badf2d7..ce4fe12 100644 --- a/frost-secp256k1/src/keys/repairable.rs +++ b/frost-secp256k1/src/keys/repairable.rs @@ -9,8 +9,8 @@ use std::collections::HashMap; // This is imported separately to make `gencode` work. // (if it were below, the position of the import would vary between ciphersuites // after `cargo fmt`) -use crate::Secp256K1Sha256; use crate::{frost, Ciphersuite, CryptoRng, Identifier, RngCore, Scalar}; +use crate::{Error, Secp256K1Sha256}; use super::{SecretShare, VerifiableSecretSharingCommitment}; @@ -26,7 +26,7 @@ pub fn repair_share_step_1( share_i: &SecretShare, rng: &mut R, participant: Identifier, -) -> HashMap { +) -> Result, Error> { frost::keys::repairable::repair_share_step_1(helpers, share_i, rng, participant) }