From f08faeadffb4f859d3738fa3a86a5f3c7264bcfa Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Mon, 5 Aug 2024 06:06:56 -0400 Subject: [PATCH] Have the DKG explicitly declare how to interpolate its shares Removes the hack for MuSig where we multiply keys by the inverse of their lagrange interpolation factor. --- crypto/dkg/src/evrf/mod.rs | 3 +- crypto/dkg/src/lib.rs | 89 +++++++++++++++----- crypto/dkg/src/musig.rs | 34 +++----- crypto/dkg/src/pedpop.rs | 3 +- crypto/dkg/src/promote.rs | 1 + crypto/dkg/src/tests/mod.rs | 6 +- networks/monero/ringct/clsag/src/multisig.rs | 7 +- networks/monero/wallet/src/send/multisig.rs | 29 +++---- networks/monero/wallet/tests/runner/mod.rs | 2 +- processor/src/networks/monero.rs | 2 +- 10 files changed, 108 insertions(+), 68 deletions(-) diff --git a/crypto/dkg/src/evrf/mod.rs b/crypto/dkg/src/evrf/mod.rs index b64435a7..1213fed3 100644 --- a/crypto/dkg/src/evrf/mod.rs +++ b/crypto/dkg/src/evrf/mod.rs @@ -88,7 +88,7 @@ use multiexp::multiexp_vartime; use generalized_bulletproofs::arithmetic_circuit_proof::*; use ec_divisors::DivisorCurve; -use crate::{Participant, ThresholdParams, ThresholdCore, ThresholdKeys}; +use crate::{Participant, ThresholdParams, Interpolation, ThresholdCore, ThresholdKeys}; pub(crate) mod proof; use proof::*; @@ -571,6 +571,7 @@ impl EvrfDkg { res.push(ThresholdKeys::from(ThresholdCore { params: ThresholdParams::new(self.t, self.n, i).unwrap(), + interpolation: Interpolation::Lagrange, secret_share, group_key: self.group_key, verification_shares: self.verification_shares.clone(), diff --git a/crypto/dkg/src/lib.rs b/crypto/dkg/src/lib.rs index a5423b0d..32c73963 100644 --- a/crypto/dkg/src/lib.rs +++ b/crypto/dkg/src/lib.rs @@ -209,25 +209,42 @@ mod lib { } } - /// Calculate the lagrange coefficient for a signing set. - pub fn lagrange(i: Participant, included: &[Participant]) -> F { - let i_f = F::from(u64::from(u16::from(i))); + #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] + #[cfg_attr(feature = "borsh", derive(borsh::BorshSerialize))] + pub(crate) enum Interpolation { + None, + Lagrange, + } - let mut num = F::ONE; - let mut denom = F::ONE; - for l in included { - if i == *l { - continue; + impl Interpolation { + pub(crate) fn interpolation_factor( + self, + i: Participant, + included: &[Participant], + ) -> F { + match self { + Interpolation::None => F::ONE, + Interpolation::Lagrange => { + let i_f = F::from(u64::from(u16::from(i))); + + let mut num = F::ONE; + let mut denom = F::ONE; + for l in included { + if i == *l { + continue; + } + + let share = F::from(u64::from(u16::from(*l))); + num *= share; + denom *= share - i_f; + } + + // Safe as this will only be 0 if we're part of the above loop + // (which we have an if case to avoid) + num * denom.invert().unwrap() + } } - - let share = F::from(u64::from(u16::from(*l))); - num *= share; - denom *= share - i_f; } - - // Safe as this will only be 0 if we're part of the above loop - // (which we have an if case to avoid) - num * denom.invert().unwrap() } /// Keys and verification shares generated by a DKG. @@ -236,6 +253,8 @@ mod lib { pub struct ThresholdCore { /// Threshold Parameters. pub(crate) params: ThresholdParams, + /// The interpolation method used. + pub(crate) interpolation: Interpolation, /// Secret share key. pub(crate) secret_share: Zeroizing, @@ -250,6 +269,7 @@ mod lib { fmt .debug_struct("ThresholdCore") .field("params", &self.params) + .field("interpolation", &self.interpolation) .field("group_key", &self.group_key) .field("verification_shares", &self.verification_shares) .finish_non_exhaustive() @@ -259,6 +279,7 @@ mod lib { impl Zeroize for ThresholdCore { fn zeroize(&mut self) { self.params.zeroize(); + self.interpolation.zeroize(); self.secret_share.zeroize(); self.group_key.zeroize(); for share in self.verification_shares.values_mut() { @@ -270,14 +291,19 @@ mod lib { impl ThresholdCore { pub(crate) fn new( params: ThresholdParams, + interpolation: Interpolation, secret_share: Zeroizing, verification_shares: HashMap, ) -> ThresholdCore { let t = (1 ..= params.t()).map(Participant).collect::>(); ThresholdCore { params, + interpolation, secret_share, - group_key: t.iter().map(|i| verification_shares[i] * lagrange::(*i, &t)).sum(), + group_key: t + .iter() + .map(|i| verification_shares[i] * interpolation.interpolation_factor::(*i, &t)) + .sum(), verification_shares, } } @@ -308,6 +334,10 @@ mod lib { writer.write_all(&self.params.t.to_le_bytes())?; writer.write_all(&self.params.n.to_le_bytes())?; writer.write_all(&self.params.i.to_bytes())?; + writer.write_all(match self.interpolation { + Interpolation::None => &[0], + Interpolation::Lagrange => &[1], + })?; let mut share_bytes = self.secret_share.to_repr(); writer.write_all(share_bytes.as_ref())?; share_bytes.as_mut().zeroize(); @@ -356,6 +386,14 @@ mod lib { ) }; + let mut interpolation = [0]; + reader.read_exact(&mut interpolation)?; + let interpolation = match interpolation[0] { + 0 => Interpolation::None, + 1 => Interpolation::Lagrange, + _ => Err(io::Error::other("invalid interpolation method"))?, + }; + let secret_share = Zeroizing::new(C::read_F(reader)?); let mut verification_shares = HashMap::new(); @@ -365,6 +403,7 @@ mod lib { Ok(ThresholdCore::new( ThresholdParams::new(t, n, i).map_err(|_| io::Error::other("invalid parameters"))?, + interpolation, secret_share, verification_shares, )) @@ -387,6 +426,7 @@ mod lib { /// View of keys, interpolated and offset for usage. #[derive(Clone)] pub struct ThresholdView { + interpolation: Interpolation, offset: C::F, group_key: C::G, included: Vec, @@ -399,6 +439,7 @@ mod lib { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt .debug_struct("ThresholdView") + .field("interpolation", &self.interpolation) .field("offset", &self.offset) .field("group_key", &self.group_key) .field("included", &self.included) @@ -484,12 +525,13 @@ mod lib { included.sort(); let mut secret_share = Zeroizing::new( - lagrange::(self.params().i(), &included) * self.secret_share().deref(), + self.core.interpolation.interpolation_factor::(self.params().i(), &included) * + self.secret_share().deref(), ); let mut verification_shares = self.verification_shares(); for (i, share) in &mut verification_shares { - *share *= lagrange::(*i, &included); + *share *= self.core.interpolation.interpolation_factor::(*i, &included); } // The offset is included by adding it to the participant with the lowest ID @@ -500,6 +542,7 @@ mod lib { *verification_shares.get_mut(&included[0]).unwrap() += C::generator() * offset; Ok(ThresholdView { + interpolation: self.core.interpolation, offset, group_key: self.group_key(), secret_share, @@ -532,6 +575,14 @@ mod lib { &self.included } + /// Return the interpolation factor for a signer. + pub fn interpolation_factor(&self, participant: Participant) -> Option { + if !self.included.contains(&participant) { + None? + } + Some(self.interpolation.interpolation_factor(participant, &self.included)) + } + /// Return the interpolated, offset secret share. pub fn secret_share(&self) -> &Zeroizing { &self.secret_share diff --git a/crypto/dkg/src/musig.rs b/crypto/dkg/src/musig.rs index 4d6b54c8..0998e537 100644 --- a/crypto/dkg/src/musig.rs +++ b/crypto/dkg/src/musig.rs @@ -7,8 +7,6 @@ use std_shims::collections::HashMap; #[cfg(feature = "std")] use zeroize::Zeroizing; -#[cfg(feature = "std")] -use ciphersuite::group::ff::Field; use ciphersuite::{ group::{Group, GroupEncoding}, Ciphersuite, @@ -16,7 +14,7 @@ use ciphersuite::{ use crate::DkgError; #[cfg(feature = "std")] -use crate::{Participant, ThresholdParams, ThresholdCore, lagrange}; +use crate::{Participant, ThresholdParams, Interpolation, ThresholdCore}; fn check_keys(keys: &[C::G]) -> Result> { if keys.is_empty() { @@ -110,32 +108,20 @@ pub fn musig( // Calculate verification shares let mut verification_shares = HashMap::new(); - // When this library offers a ThresholdView for a specific signing set, it applies the lagrange - // factor - // Since this is a n-of-n scheme, there's only one possible signing set, and one possible - // lagrange factor - // In the name of simplicity, we define the group key as the sum of all bound keys - // Accordingly, the secret share must be multiplied by the inverse of the lagrange factor, along - // with all verification shares - // This is less performant than simply defining the group key as the sum of all post-lagrange - // bound keys, yet the simplicity is preferred - let included = (1 ..= keys_len) - // This error also shouldn't be possible, for the same reasons as documented above - .map(|l| Participant::new(l).ok_or(DkgError::InvalidSigningSet)) - .collect::, _>>()?; let mut group_key = C::G::identity(); - for (l, p) in included.iter().enumerate() { - let bound = keys[l] * binding[l]; + for (l, (key, binding)) in keys.iter().zip(binding).enumerate() { + let bound = *key * binding; group_key += bound; - let lagrange_inv = lagrange::(*p, &included).invert().unwrap(); - if params.i() == *p { - *secret_share *= lagrange_inv; - } - verification_shares.insert(*p, bound * lagrange_inv); + // These errors also shouldn't be possible, for the same reasons as documented above + verification_shares.insert( + Participant::new(1 + u16::try_from(l).map_err(|_| DkgError::InvalidSigningSet)?) + .ok_or(DkgError::InvalidSigningSet)?, + bound, + ); } debug_assert_eq!(C::generator() * secret_share.deref(), verification_shares[¶ms.i()]); debug_assert_eq!(musig_key::(context, keys).unwrap(), group_key); - Ok(ThresholdCore { params, secret_share, group_key, verification_shares }) + Ok(ThresholdCore::new(params, Interpolation::None, secret_share, verification_shares)) } diff --git a/crypto/dkg/src/pedpop.rs b/crypto/dkg/src/pedpop.rs index 578c3bcc..37af59d2 100644 --- a/crypto/dkg/src/pedpop.rs +++ b/crypto/dkg/src/pedpop.rs @@ -22,7 +22,7 @@ use multiexp::{multiexp_vartime, BatchVerifier}; use schnorr::SchnorrSignature; use crate::{ - Participant, DkgError, ThresholdParams, ThresholdCore, validate_map, + Participant, DkgError, ThresholdParams, Interpolation, ThresholdCore, validate_map, encryption::{ ReadWrite, EncryptionKeyMessage, EncryptedMessage, Encryption, Decryption, EncryptionKeyProof, DecryptionError, @@ -477,6 +477,7 @@ impl KeyMachine { encryption: encryption.into_decryption(), result: Some(ThresholdCore { params, + interpolation: Interpolation::Lagrange, secret_share: secret, group_key: stripes[0], verification_shares, diff --git a/crypto/dkg/src/promote.rs b/crypto/dkg/src/promote.rs index 7cad4f23..d7a98b59 100644 --- a/crypto/dkg/src/promote.rs +++ b/crypto/dkg/src/promote.rs @@ -113,6 +113,7 @@ impl> GeneratorPromotion< Ok(ThresholdKeys { core: Arc::new(ThresholdCore::new( params, + self.base.core.interpolation, self.base.secret_share().clone(), verification_shares, )), diff --git a/crypto/dkg/src/tests/mod.rs b/crypto/dkg/src/tests/mod.rs index 99d68b3d..2a2d25b3 100644 --- a/crypto/dkg/src/tests/mod.rs +++ b/crypto/dkg/src/tests/mod.rs @@ -6,7 +6,7 @@ use rand_core::{RngCore, CryptoRng}; use ciphersuite::{group::ff::Field, Ciphersuite}; -use crate::{Participant, ThresholdCore, ThresholdKeys, lagrange, musig::musig as musig_fn}; +use crate::{Participant, ThresholdCore, ThresholdKeys, musig::musig as musig_fn}; mod musig; pub use musig::test_musig; @@ -46,7 +46,9 @@ pub fn recover_key(keys: &HashMap> let included = keys.keys().copied().collect::>(); let group_private = keys.iter().fold(C::F::ZERO, |accum, (i, keys)| { - accum + (lagrange::(*i, &included) * keys.secret_share().deref()) + accum + + (first.core.interpolation.interpolation_factor::(*i, &included) * + keys.secret_share().deref()) }); assert_eq!(C::generator() * group_private, first.group_key(), "failed to recover keys"); group_private diff --git a/networks/monero/ringct/clsag/src/multisig.rs b/networks/monero/ringct/clsag/src/multisig.rs index bfbb8fc5..70cba19e 100644 --- a/networks/monero/ringct/clsag/src/multisig.rs +++ b/networks/monero/ringct/clsag/src/multisig.rs @@ -20,7 +20,6 @@ use group::{ use transcript::{Transcript, RecommendedTranscript}; use dalek_ff_group as dfg; use frost::{ - dkg::lagrange, curve::Ed25519, Participant, FrostError, ThresholdKeys, ThresholdView, algorithm::{WriteAddendum, Algorithm}, @@ -233,8 +232,10 @@ impl Algorithm for ClsagMultisig { .append_message(b"key_image_share", addendum.key_image_share.compress().to_bytes()); // Accumulate the interpolated share - let interpolated_key_image_share = - addendum.key_image_share * lagrange::(l, view.included()); + let interpolated_key_image_share = addendum.key_image_share * + view + .interpolation_factor(l) + .ok_or(FrostError::InternalError("processing addendum of non-participant"))?; *self.image.as_mut().unwrap() += interpolated_key_image_share; self diff --git a/networks/monero/wallet/src/send/multisig.rs b/networks/monero/wallet/src/send/multisig.rs index b3d58ba5..d60c5a33 100644 --- a/networks/monero/wallet/src/send/multisig.rs +++ b/networks/monero/wallet/src/send/multisig.rs @@ -14,7 +14,6 @@ use transcript::{Transcript, RecommendedTranscript}; use frost::{ curve::Ed25519, Participant, FrostError, ThresholdKeys, - dkg::lagrange, sign::{ Preprocess, CachedPreprocess, SignatureShare, PreprocessMachine, SignMachine, SignatureMachine, AlgorithmMachine, AlgorithmSignMachine, AlgorithmSignatureMachine, @@ -34,7 +33,7 @@ use crate::send::{SendError, SignableTransaction, key_image_sort}; pub struct TransactionMachine { signable: SignableTransaction, - i: Participant, + keys: ThresholdKeys, // The key image generator, and the scalar offset from the spend key key_image_generators_and_offsets: Vec<(EdwardsPoint, Scalar)>, @@ -45,7 +44,7 @@ pub struct TransactionMachine { pub struct TransactionSignMachine { signable: SignableTransaction, - i: Participant, + keys: ThresholdKeys, key_image_generators_and_offsets: Vec<(EdwardsPoint, Scalar)>, clsags: Vec<(ClsagMultisigMaskSender, AlgorithmSignMachine)>, @@ -61,7 +60,7 @@ pub struct TransactionSignatureMachine { impl SignableTransaction { /// Create a FROST signing machine out of this signable transaction. - pub fn multisig(self, keys: &ThresholdKeys) -> Result { + pub fn multisig(self, keys: ThresholdKeys) -> Result { let mut clsags = vec![]; let mut key_image_generators_and_offsets = vec![]; @@ -85,12 +84,7 @@ impl SignableTransaction { clsags.push((clsag_mask_send, AlgorithmMachine::new(clsag, offset))); } - Ok(TransactionMachine { - signable: self, - i: keys.params().i(), - key_image_generators_and_offsets, - clsags, - }) + Ok(TransactionMachine { signable: self, keys, key_image_generators_and_offsets, clsags }) } } @@ -120,7 +114,7 @@ impl PreprocessMachine for TransactionMachine { TransactionSignMachine { signable: self.signable, - i: self.i, + keys: self.keys, key_image_generators_and_offsets: self.key_image_generators_and_offsets, clsags, @@ -173,12 +167,12 @@ impl SignMachine for TransactionSignMachine { // We do not need to be included here, yet this set of signers has yet to be validated // We explicitly remove ourselves to ensure we aren't included twice, if we were redundantly // included - commitments.remove(&self.i); + commitments.remove(&self.keys.params().i()); // Find out who's included let mut included = commitments.keys().copied().collect::>(); // This push won't duplicate due to the above removal - included.push(self.i); + included.push(self.keys.params().i()); // unstable sort may reorder elements of equal order // Given our lack of duplicates, we should have no elements of equal order included.sort_unstable(); @@ -192,12 +186,15 @@ impl SignMachine for TransactionSignMachine { } // Convert the serialized nonces commitments to a parallelized Vec + let view = self.keys.view(included.clone()).map_err(|_| { + FrostError::InvalidSigningSet("couldn't form an interpolated view of the key") + })?; let mut commitments = (0 .. self.clsags.len()) .map(|c| { included .iter() .map(|l| { - let preprocess = if *l == self.i { + let preprocess = if *l == self.keys.params().i() { self.our_preprocess[c].clone() } else { commitments.get_mut(l).ok_or(FrostError::MissingParticipant(*l))?[c].clone() @@ -206,7 +203,7 @@ impl SignMachine for TransactionSignMachine { // While here, calculate the key image as needed to call sign // The CLSAG algorithm will independently calculate the key image/verify these shares key_images[c] += - preprocess.addendum.key_image_share().0 * lagrange::(*l, &included).0; + preprocess.addendum.key_image_share().0 * view.interpolation_factor(*l).unwrap().0; Ok((*l, preprocess)) }) @@ -217,7 +214,7 @@ impl SignMachine for TransactionSignMachine { // The above inserted our own preprocess into these maps (which is unnecessary) // Remove it now for map in &mut commitments { - map.remove(&self.i); + map.remove(&self.keys.params().i()); } // The actual TX will have sorted its inputs by key image diff --git a/networks/monero/wallet/tests/runner/mod.rs b/networks/monero/wallet/tests/runner/mod.rs index 4d042b53..b9d40737 100644 --- a/networks/monero/wallet/tests/runner/mod.rs +++ b/networks/monero/wallet/tests/runner/mod.rs @@ -281,7 +281,7 @@ macro_rules! test { { let mut machines = HashMap::new(); for i in (1 ..= THRESHOLD).map(|i| Participant::new(i).unwrap()) { - machines.insert(i, tx.clone().multisig(&keys[&i]).unwrap()); + machines.insert(i, tx.clone().multisig(keys[&i].clone()).unwrap()); } frost::tests::sign_without_caching(&mut OsRng, machines, &[]) diff --git a/processor/src/networks/monero.rs b/processor/src/networks/monero.rs index 54a3af24..d5320eb5 100644 --- a/processor/src/networks/monero.rs +++ b/processor/src/networks/monero.rs @@ -657,7 +657,7 @@ impl Network for Monero { keys: ThresholdKeys, transaction: SignableTransaction, ) -> Result { - match transaction.0.clone().multisig(&keys) { + match transaction.0.clone().multisig(keys) { Ok(machine) => Ok(machine), Err(e) => panic!("failed to create a multisig machine for TX: {e}"), }