From 1caa6a96065cdba1ce2e3c909ee6c9027618403d Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 24 Jun 2022 08:40:14 -0400 Subject: [PATCH] Enforce FROST StateMachine progression via the type system A comment on the matter was made in https://github.com/serai-dex/serai/issues/12. While I do believe the API is slightly worse, I appreciate the explicitness. --- coins/monero/src/wallet/send/multisig.rs | 199 +++++++++++++---------- crypto/frost/src/key_gen.rs | 130 +++++---------- crypto/frost/src/lib.rs | 5 - crypto/frost/src/sign.rs | 159 +++++++----------- crypto/frost/src/tests/mod.rs | 80 ++++----- crypto/frost/src/tests/vectors.rs | 33 ++-- processor/src/coins/monero.rs | 1 - processor/src/lib.rs | 4 +- processor/src/wallet.rs | 16 +- 9 files changed, 276 insertions(+), 351 deletions(-) diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index 84dc63d7..f03fbf36 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -6,7 +6,13 @@ use rand_chacha::ChaCha12Rng; use curve25519_dalek::{traits::Identity, scalar::Scalar, edwards::{EdwardsPoint, CompressedEdwardsY}}; use transcript::Transcript as TranscriptTrait; -use frost::{FrostError, MultisigKeys, MultisigParams, sign::{State, StateMachine, AlgorithmMachine}}; +use frost::{ + FrostError, MultisigKeys, + sign::{ + PreprocessMachine, SignMachine, SignatureMachine, + AlgorithmMachine, AlgorithmSignMachine, AlgorithmSignatureMachine + } +}; use crate::{ frost::{Transcript, Ed25519}, @@ -24,14 +30,27 @@ pub struct TransactionMachine { decoys: Vec, - our_preprocess: Vec, - - images: Vec, - output_masks: Option, inputs: Vec>>>, - clsags: Vec>, + clsags: Vec> +} - tx: Option +pub struct TransactionSignMachine { + signable: SignableTransaction, + i: u16, + included: Vec, + transcript: Transcript, + + decoys: Vec, + + inputs: Vec>>>, + clsags: Vec>, + + our_preprocess: Vec +} + +pub struct TransactionSignatureMachine { + tx: Transaction, + clsags: Vec> } impl SignableTransaction { @@ -43,8 +62,6 @@ impl SignableTransaction { height: usize, mut included: Vec ) -> Result { - let mut images = vec![]; - images.resize(self.inputs.len(), EdwardsPoint::identity()); let mut inputs = vec![]; for _ in 0 .. self.inputs.len() { // Doesn't resize as that will use a single Rc for the entire Vec @@ -118,43 +135,38 @@ impl SignableTransaction { &self.inputs ).await.map_err(|e| TransactionError::RpcError(e))?; - Ok(TransactionMachine { - signable: self, - i: keys.params().i(), - included, - transcript, + Ok( + TransactionMachine { + signable: self, + i: keys.params().i(), + included, + transcript, - decoys, + decoys, - our_preprocess: vec![], - - images, - output_masks: None, - inputs, - clsags, - - tx: None - }) + inputs, + clsags + } + ) } } -impl StateMachine for TransactionMachine { +impl PreprocessMachine for TransactionMachine { type Signature = Transaction; + type SignMachine = TransactionSignMachine; fn preprocess( - &mut self, + mut self, rng: &mut R - ) -> Result, FrostError> { - if self.state() != State::Fresh { - Err(FrostError::InvalidSignTransition(State::Fresh, self.state()))?; - } - + ) -> (TransactionSignMachine, Vec) { // Iterate over each CLSAG calling preprocess let mut serialized = Vec::with_capacity(self.clsags.len() * (64 + ClsagMultisig::serialized_len())); - for clsag in self.clsags.iter_mut() { - serialized.extend(&clsag.preprocess(rng)?); - } - self.our_preprocess = serialized.clone(); + let clsags = self.clsags.drain(..).map(|clsag| { + let (clsag, preprocess) = clsag.preprocess(rng); + serialized.extend(&preprocess); + clsag + }).collect(); + let our_preprocess = serialized.clone(); // We could add further entropy here, and previous versions of this library did so // As of right now, the multisig's key, the inputs being spent, and the FROST data itself @@ -165,18 +177,33 @@ impl StateMachine for TransactionMachine { // increase privacy. If they're not sent in plain text, or are otherwise inaccessible, they // already offer sufficient entropy. That's why further entropy is not included - Ok(serialized) + ( + TransactionSignMachine { + signable: self.signable, + i: self.i, + included: self.included, + transcript: self.transcript, + + decoys: self.decoys, + + inputs: self.inputs, + clsags, + + our_preprocess, + }, + serialized + ) } +} + +impl SignMachine for TransactionSignMachine { + type SignatureMachine = TransactionSignatureMachine; fn sign( - &mut self, + mut self, mut commitments: HashMap>, msg: &[u8] - ) -> Result, FrostError> { - if self.state() != State::Preprocessed { - Err(FrostError::InvalidSignTransition(State::Preprocessed, self.state()))?; - } - + ) -> Result<(TransactionSignatureMachine, Vec), FrostError> { if msg.len() != 0 { Err( FrostError::InternalError( @@ -189,7 +216,7 @@ impl StateMachine for TransactionMachine { // While each CLSAG will do this as they need to for security, they have their own transcripts // cloned from this TX's initial premise's transcript. For our TX transcript to have the CLSAG // data for entropy, it'll have to be added ourselves - commitments.insert(self.i, self.our_preprocess.clone()); + commitments.insert(self.i, self.our_preprocess); for l in &self.included { self.transcript.append_message(b"participant", &(*l).to_be_bytes()); // FROST itself will error if this is None, so let it @@ -201,30 +228,33 @@ impl StateMachine for TransactionMachine { // FROST commitments, image, H commitments, and their proofs let clsag_len = 64 + ClsagMultisig::serialized_len(); - let mut commitments = (0 .. self.clsags.len()).map(|c| commitments.iter().map( - |(l, commitments)| (*l, commitments[(c * clsag_len) .. ((c + 1) * clsag_len)].to_vec()) + // Convert the unified commitments to a Vec of the individual commitments + let mut commitments = (0 .. self.clsags.len()).map(|_| commitments.iter_mut().map( + |(l, commitments)| (*l, commitments.drain(.. clsag_len).collect::>()) ).collect::>()).collect::>(); + // Calculate the key images + // Clsag will parse/calculate/validate this as needed, yet doing so here as well provides + // the easiest API overall, as this is where the TX is (which needs the key images in its + // message), along with where the outputs are determined (where our change output needs these + // to be unique) + let mut images = vec![EdwardsPoint::identity(); self.clsags.len()]; for c in 0 .. self.clsags.len() { - // Calculate the key images - // Multisig will parse/calculate/validate this as needed, yet doing so here as well provides - // the easiest API overall, as this is where the TX is (which needs the key images in its - // message), along with where the outputs are determined (where our change output needs these - // to be unique) for (l, preprocess) in &commitments[c] { - self.images[c] += CompressedEdwardsY( + images[c] += CompressedEdwardsY( preprocess[64 .. 96].try_into().map_err(|_| FrostError::InvalidCommitment(*l))? ).decompress().ok_or(FrostError::InvalidCommitment(*l))?; } } // Create the actual transaction + let output_masks; let mut tx = { - // Calculate uniqueness - let mut images = self.images.clone(); - images.sort_by(key_image_sort); + let mut sorted_images = images.clone(); + sorted_images.sort_by(key_image_sort); - let (commitments, output_masks) = self.signable.prepare_outputs( + let commitments; + (commitments, output_masks) = self.signable.prepare_outputs( &mut ChaCha12Rng::from_seed(self.transcript.rng_seed(b"tx_keys")), uniqueness( &images.iter().map(|image| Input::ToKey { @@ -234,7 +264,6 @@ impl StateMachine for TransactionMachine { }).collect::>() ) ); - self.output_masks = Some(output_masks); self.signable.prepare_transaction( &commitments, @@ -245,18 +274,19 @@ impl StateMachine for TransactionMachine { ) }; - let mut sorted = Vec::with_capacity(self.decoys.len()); - while self.decoys.len() != 0 { + // Sort the inputs, as expected + let mut sorted = Vec::with_capacity(self.clsags.len()); + while self.clsags.len() != 0 { sorted.push(( + images.swap_remove(0), self.signable.inputs.swap_remove(0), self.decoys.swap_remove(0), - self.images.swap_remove(0), self.inputs.swap_remove(0), self.clsags.swap_remove(0), commitments.swap_remove(0) )); } - sorted.sort_by(|x, y| x.2.compress().to_bytes().cmp(&y.2.compress().to_bytes()).reverse()); + sorted.sort_by(|x, y| key_image_sort(&x.0, &y.0)); let mut rng = ChaCha12Rng::from_seed(self.transcript.rng_seed(b"pseudo_out_masks")); let mut sum_pseudo_outs = Scalar::zero(); @@ -265,7 +295,7 @@ impl StateMachine for TransactionMachine { let mut mask = random_scalar(&mut rng); if sorted.len() == 0 { - mask = self.output_masks.unwrap() - sum_pseudo_outs; + mask = output_masks - sum_pseudo_outs; } else { sum_pseudo_outs += mask; } @@ -273,16 +303,16 @@ impl StateMachine for TransactionMachine { tx.prefix.inputs.push( Input::ToKey { amount: 0, - key_offsets: value.1.offsets.clone(), - key_image: value.2 + key_offsets: value.2.offsets.clone(), + key_image: value.0 } ); *value.3.write().unwrap() = Some( ClsagDetails::new( ClsagInput::new( - value.0.commitment, - value.1 + value.1.commitment, + value.2 ).map_err(|_| panic!("Signing an input which isn't present in the ring we created for it"))?, mask ) @@ -293,30 +323,31 @@ impl StateMachine for TransactionMachine { } let msg = tx.signature_hash(); - self.tx = Some(tx); // Iterate over each CLSAG calling sign let mut serialized = Vec::with_capacity(self.clsags.len() * 32); - for clsag in self.clsags.iter_mut() { - serialized.extend(&clsag.sign(commitments.remove(0), &msg)?); - } + let clsags = self.clsags.drain(..).map(|clsag| { + let (clsag, share) = clsag.sign(commitments.remove(0), &msg)?; + serialized.extend(&share); + Ok(clsag) + }).collect::>()?; - Ok(serialized) + Ok((TransactionSignatureMachine { tx, clsags }, serialized)) } +} - fn complete(&mut self, shares: HashMap>) -> Result { - if self.state() != State::Signed { - Err(FrostError::InvalidSignTransition(State::Signed, self.state()))?; - } - - let mut tx = self.tx.take().unwrap(); +impl SignatureMachine for TransactionSignatureMachine { + fn complete(self, mut shares: HashMap>) -> Result { + let mut tx = self.tx; match tx.rct_signatures.prunable { RctPrunable::Null => panic!("Signing for RctPrunable::Null"), RctPrunable::Clsag { ref mut clsags, ref mut pseudo_outs, .. } => { - for (c, clsag) in self.clsags.iter_mut().enumerate() { - let (clsag, pseudo_out) = clsag.complete(shares.iter().map( - |(l, shares)| (*l, shares[(c * 32) .. ((c + 1) * 32)].to_vec()) - ).collect::>())?; + for clsag in self.clsags { + let (clsag, pseudo_out) = clsag.complete( + shares.iter_mut().map( + |(l, shares)| (*l, shares.drain(.. 32).collect()) + ).collect::>() + )?; clsags.push(clsag); pseudo_outs.push(pseudo_out); } @@ -324,12 +355,4 @@ impl StateMachine for TransactionMachine { } Ok(tx) } - - fn multisig_params(&self) -> MultisigParams { - self.clsags[0].multisig_params() - } - - fn state(&self) -> State { - self.clsags[0].state() - } } diff --git a/crypto/frost/src/key_gen.rs b/crypto/frost/src/key_gen.rs index fd5f13bb..061260c6 100644 --- a/crypto/frost/src/key_gen.rs +++ b/crypto/frost/src/key_gen.rs @@ -1,5 +1,4 @@ -use core::fmt; -use std::collections::HashMap; +use std::{marker::PhantomData, collections::HashMap}; use rand_core::{RngCore, CryptoRng}; @@ -271,100 +270,76 @@ fn complete_r2( ) } -/// State of a Key Generation machine -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum State { - Fresh, - GeneratedCoefficients, - GeneratedSecretShares, - Complete, -} - -impl fmt::Display for State { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", self) - } -} - -/// State machine which manages key generation -#[allow(non_snake_case)] -pub struct StateMachine { +pub struct KeyGenMachine { params: MultisigParams, context: String, - state: State, - coefficients: Option>, - our_commitments: Option>, - secret: Option, - commitments: Option>> + _curve: PhantomData, } -impl StateMachine { +pub struct SecretShareMachine { + params: MultisigParams, + context: String, + coefficients: Vec, + our_commitments: Vec, +} + +pub struct KeyMachine { + params: MultisigParams, + secret: C::F, + commitments: HashMap>, +} + +impl KeyGenMachine { /// Creates a new machine to generate a key for the specified curve in the specified multisig // The context string must be unique among multisigs - pub fn new(params: MultisigParams, context: String) -> StateMachine { - StateMachine { - params, - context, - state: State::Fresh, - coefficients: None, - our_commitments: None, - secret: None, - commitments: None - } + pub fn new(params: MultisigParams, context: String) -> KeyGenMachine { + KeyGenMachine { params, context, _curve: PhantomData } } /// Start generating a key according to the FROST DKG spec /// Returns a serialized list of commitments to be sent to all parties over an authenticated /// channel. If any party submits multiple sets of commitments, they MUST be treated as malicious pub fn generate_coefficients( - &mut self, + self, rng: &mut R - ) -> Result, FrostError> { - if self.state != State::Fresh { - Err(FrostError::InvalidKeyGenTransition(State::Fresh, self.state))?; - } - - let (coefficients, serialized) = generate_key_r1::( - rng, - &self.params, - &self.context, - ); - - self.coefficients = Some(coefficients); - self.our_commitments = Some(serialized.clone()); - self.state = State::GeneratedCoefficients; - Ok(serialized) + ) -> (SecretShareMachine, Vec) { + let (coefficients, serialized) = generate_key_r1::(rng, &self.params, &self.context); + ( + SecretShareMachine { + params: self.params, + context: self.context, + coefficients, + our_commitments: serialized.clone() + }, + serialized, + ) } +} +impl SecretShareMachine { /// Continue generating a key /// Takes in everyone else's commitments, which are expected to be in a Vec where participant /// index = Vec index. An empty vector is expected at index 0 to allow for this. An empty vector /// is also expected at index i which is locally handled. Returns a byte vector representing a /// secret share for each other participant which should be encrypted before sending pub fn generate_secret_shares( - &mut self, + self, rng: &mut R, commitments: HashMap>, - ) -> Result>, FrostError> { - if self.state != State::GeneratedCoefficients { - Err(FrostError::InvalidKeyGenTransition(State::GeneratedCoefficients, self.state))?; - } - + ) -> Result<(KeyMachine, HashMap>), FrostError> { let (secret, commitments, shares) = generate_key_r2::( rng, &self.params, &self.context, - self.coefficients.take().unwrap(), - self.our_commitments.take().unwrap(), + self.coefficients, + self.our_commitments, commitments, )?; - - self.secret = Some(secret); - self.commitments = Some(commitments); - self.state = State::GeneratedSecretShares; - Ok(shares) + Ok((KeyMachine { params: self.params, secret, commitments }, shares)) } +} +impl KeyMachine { /// Complete key generation /// Takes in everyone elses' shares submitted to us as a Vec, expecting participant index = /// Vec index with an empty vector at index 0 and index i. Returns a byte vector representing the @@ -372,31 +347,10 @@ impl StateMachine { /// must report completion without issue before this key can be considered usable, yet you should /// wait for all participants to report as such pub fn complete( - &mut self, + self, rng: &mut R, shares: HashMap>, ) -> Result, FrostError> { - if self.state != State::GeneratedSecretShares { - Err(FrostError::InvalidKeyGenTransition(State::GeneratedSecretShares, self.state))?; - } - - let keys = complete_r2( - rng, - self.params, - self.secret.take().unwrap(), - self.commitments.take().unwrap(), - shares, - )?; - - self.state = State::Complete; - Ok(keys) - } - - pub fn params(&self) -> MultisigParams { - self.params.clone() - } - - pub fn state(&self) -> State { - self.state + complete_r2(rng, self.params, self.secret, self.commitments, shares) } } diff --git a/crypto/frost/src/lib.rs b/crypto/frost/src/lib.rs index cd5d8ee2..f3b8b2bd 100644 --- a/crypto/frost/src/lib.rs +++ b/crypto/frost/src/lib.rs @@ -181,11 +181,6 @@ pub enum FrostError { InvalidProofOfKnowledge(u16), #[error("invalid share (participant {0})")] InvalidShare(u16), - #[error("invalid key generation state machine transition (expected {0}, was {1})")] - InvalidKeyGenTransition(key_gen::State, key_gen::State), - - #[error("invalid sign state machine transition (expected {0}, was {1})")] - InvalidSignTransition(sign::State, sign::State), #[error("internal error ({0})")] InternalError(String), diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 987cec0e..289165aa 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -236,31 +236,21 @@ fn complete>( ) } -/// State of a Sign machine -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum State { - Fresh, - Preprocessed, - Signed, - Complete, -} - -impl fmt::Display for State { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", self) - } -} - -pub trait StateMachine { +pub trait PreprocessMachine { type Signature: Clone + PartialEq + fmt::Debug; + type SignMachine: SignMachine; /// Perform the preprocessing round required in order to sign /// Returns a byte vector which must be transmitted to all parties selected for this signing /// process, over an authenticated channel fn preprocess( - &mut self, + self, rng: &mut R - ) -> Result, FrostError>; + ) -> (Self::SignMachine, Vec); +} + +pub trait SignMachine { + type SignatureMachine: SignatureMachine; /// Sign a message /// Takes in the participant's commitments, which are expected to be in a Vec where participant @@ -268,29 +258,33 @@ pub trait StateMachine { /// index i which is locally handled. Returns a byte vector representing a share of the signature /// for every other participant to receive, over an authenticated channel fn sign( - &mut self, + self, commitments: HashMap>, msg: &[u8], - ) -> Result, FrostError>; + ) -> Result<(Self::SignatureMachine, Vec), FrostError>; +} +pub trait SignatureMachine { /// Complete signing /// Takes in everyone elses' shares submitted to us as a Vec, expecting participant index = /// Vec index with None at index 0 and index i. Returns a byte vector representing the serialized /// signature - fn complete(&mut self, shares: HashMap>) -> Result; - - fn multisig_params(&self) -> MultisigParams; - - fn state(&self) -> State; + fn complete(self, shares: HashMap>) -> Result; } /// State machine which manages signing for an arbitrary signature algorithm -#[allow(non_snake_case)] pub struct AlgorithmMachine> { + params: Params +} + +pub struct AlgorithmSignMachine> { params: Params, - state: State, - preprocess: Option>, - sign: Option>, + preprocess: PreprocessPackage, +} + +pub struct AlgorithmSignatureMachine> { + params: Params, + sign: Package, } impl> AlgorithmMachine { @@ -300,85 +294,52 @@ impl> AlgorithmMachine { keys: Arc>, included: &[u16], ) -> Result, FrostError> { - Ok( - AlgorithmMachine { - params: Params::new(algorithm, keys, included)?, - state: State::Fresh, - preprocess: None, - sign: None, - } - ) + Ok(AlgorithmMachine { params: Params::new(algorithm, keys, included)? }) } - pub(crate) fn unsafe_override_preprocess(&mut self, preprocess: PreprocessPackage) { - if self.state != State::Fresh { - // This would be unacceptable, yet this is pub(crate) and explicitly labelled unsafe - // It's solely used in a testing environment, which is how it's justified - Err::<(), _>(FrostError::InvalidSignTransition(State::Fresh, self.state)).unwrap(); - } - self.preprocess = Some(preprocess); - self.state = State::Preprocessed; + pub(crate) fn unsafe_override_preprocess( + self, + preprocess: PreprocessPackage + ) -> (AlgorithmSignMachine, Vec) { + let serialized = preprocess.serialized.clone(); + (AlgorithmSignMachine { params: self.params, preprocess }, serialized) } } -impl> StateMachine for AlgorithmMachine { +impl> PreprocessMachine for AlgorithmMachine { type Signature = A::Signature; + type SignMachine = AlgorithmSignMachine; fn preprocess( - &mut self, + self, rng: &mut R - ) -> Result, FrostError> { - if self.state != State::Fresh { - Err(FrostError::InvalidSignTransition(State::Fresh, self.state))?; - } - let preprocess = preprocess::(rng, &mut self.params); + ) -> (Self::SignMachine, Vec) { + let mut params = self.params; + let preprocess = preprocess::(rng, &mut params); let serialized = preprocess.serialized.clone(); - self.preprocess = Some(preprocess); - self.state = State::Preprocessed; - Ok(serialized) - } - - fn sign( - &mut self, - commitments: HashMap>, - msg: &[u8], - ) -> Result, FrostError> { - if self.state != State::Preprocessed { - Err(FrostError::InvalidSignTransition(State::Preprocessed, self.state))?; - } - - let (sign, serialized) = sign_with_share( - &mut self.params, - self.preprocess.take().unwrap(), - commitments, - msg, - )?; - - self.sign = Some(sign); - self.state = State::Signed; - Ok(serialized) - } - - fn complete(&mut self, shares: HashMap>) -> Result { - if self.state != State::Signed { - Err(FrostError::InvalidSignTransition(State::Signed, self.state))?; - } - - let signature = complete( - &self.params, - self.sign.take().unwrap(), - shares, - )?; - - self.state = State::Complete; - Ok(signature) - } - - fn multisig_params(&self) -> MultisigParams { - self.params.multisig_params().clone() - } - - fn state(&self) -> State { - self.state + (AlgorithmSignMachine { params, preprocess }, serialized) + } +} + +impl> SignMachine for AlgorithmSignMachine { + type SignatureMachine = AlgorithmSignatureMachine; + + fn sign( + self, + commitments: HashMap>, + msg: &[u8] + ) -> Result<(Self::SignatureMachine, Vec), FrostError> { + let mut params = self.params; + let (sign, serialized) = sign_with_share(&mut params, self.preprocess, commitments, msg)?; + Ok((AlgorithmSignatureMachine { params, sign }, serialized)) + } +} + +impl< + C: Curve, + A: Algorithm +> SignatureMachine for AlgorithmSignatureMachine { + fn complete(self, shares: HashMap>) -> Result { + complete(&self.params, self.sign, shares) } } diff --git a/crypto/frost/src/tests/mod.rs b/crypto/frost/src/tests/mod.rs index 78bc7425..fa45f6f1 100644 --- a/crypto/frost/src/tests/mod.rs +++ b/crypto/frost/src/tests/mod.rs @@ -8,9 +8,9 @@ use crate::{ Curve, MultisigParams, MultisigKeys, lagrange, - key_gen, + key_gen::KeyGenMachine, algorithm::Algorithm, - sign::{StateMachine, AlgorithmMachine} + sign::{PreprocessMachine, SignMachine, SignatureMachine, AlgorithmMachine} }; // Test suites for public usage @@ -37,49 +37,36 @@ pub fn clone_without( pub fn key_gen( rng: &mut R ) -> HashMap>> { - let mut params = HashMap::new(); let mut machines = HashMap::new(); - let mut commitments = HashMap::new(); for i in 1 ..= PARTICIPANTS { - params.insert( - i, - MultisigParams::new( - THRESHOLD, - PARTICIPANTS, - i - ).unwrap() - ); - machines.insert( - i, - key_gen::StateMachine::::new( - params[&i], - "FROST Test key_gen".to_string() - ) - ); - commitments.insert( - i, - machines.get_mut(&i).unwrap().generate_coefficients(rng).unwrap() + let machine = KeyGenMachine::::new( + MultisigParams::new(THRESHOLD, PARTICIPANTS, i).unwrap(), + "FROST Test key_gen".to_string() ); + let (machine, these_commitments) = machine.generate_coefficients(rng); + machines.insert(i, machine); + commitments.insert(i, these_commitments); } let mut secret_shares = HashMap::new(); - for (l, machine) in machines.iter_mut() { - secret_shares.insert( - *l, + let mut machines = machines.drain().map(|(l, machine)| { + let (machine, shares) = machine.generate_secret_shares( + rng, // clone_without isn't necessary, as this machine's own data will be inserted without // conflict, yet using it ensures the machine's own data is actually inserted as expected - machine.generate_secret_shares(rng, clone_without(&commitments, l)).unwrap() - ); - } + clone_without(&commitments, &l) + ).unwrap(); + secret_shares.insert(l, shares); + (l, machine) + }).collect::>(); let mut verification_shares = None; let mut group_key = None; - let mut keys = HashMap::new(); - for (i, machine) in machines.iter_mut() { + machines.drain().map(|(i, machine)| { let mut our_secret_shares = HashMap::new(); for (l, shares) in &secret_shares { - if i == l { + if i == *l { continue; } our_secret_shares.insert(*l, shares[&i].clone()); @@ -98,10 +85,8 @@ pub fn key_gen( } assert_eq!(group_key.unwrap(), these_keys.group_key()); - keys.insert(*i, Arc::new(these_keys)); - } - - keys + (i, Arc::new(these_keys)) + }).collect::>() } pub fn recover(keys: &HashMap>) -> C::F { @@ -147,27 +132,28 @@ pub fn algorithm_machines>( ).collect() } -pub fn sign( +pub fn sign( rng: &mut R, mut machines: HashMap, msg: &[u8] ) -> M::Signature { let mut commitments = HashMap::new(); - for (i, machine) in machines.iter_mut() { - commitments.insert(*i, machine.preprocess(rng).unwrap()); - } + let mut machines = machines.drain().map(|(i, machine)| { + let (machine, preprocess) = machine.preprocess(rng); + commitments.insert(i, preprocess); + (i, machine) + }).collect::>(); let mut shares = HashMap::new(); - for (i, machine) in machines.iter_mut() { - shares.insert( - *i, - machine.sign(clone_without(&commitments, i), msg).unwrap() - ); - } + let mut machines = machines.drain().map(|(i, machine)| { + let (machine, share) = machine.sign(clone_without(&commitments, &i), msg).unwrap(); + shares.insert(i, share); + (i, machine) + }).collect::>(); let mut signature = None; - for (i, machine) in machines.iter_mut() { - let sig = machine.complete(clone_without(&shares, i)).unwrap(); + for (i, machine) in machines.drain() { + let sig = machine.complete(clone_without(&shares, &i)).unwrap(); if signature.is_none() { signature = Some(sig.clone()); } diff --git a/crypto/frost/src/tests/vectors.rs b/crypto/frost/src/tests/vectors.rs index db46de1f..e0def162 100644 --- a/crypto/frost/src/tests/vectors.rs +++ b/crypto/frost/src/tests/vectors.rs @@ -5,7 +5,7 @@ use rand_core::{RngCore, CryptoRng}; use crate::{ Curve, MultisigKeys, algorithm::{Schnorr, Hram}, - sign::{PreprocessPackage, StateMachine, AlgorithmMachine}, + sign::{PreprocessPackage, SignMachine, SignatureMachine, AlgorithmMachine}, tests::{curve::test_curve, schnorr::test_schnorr, recover} }; @@ -92,33 +92,40 @@ pub fn test_with_vectors< let mut commitments = HashMap::new(); let mut c = 0; - for (i, machine) in machines.iter_mut() { + let mut machines = machines.drain(..).map(|(i, machine)| { let nonces = [ C::F_from_slice(&hex::decode(vectors.nonces[c][0]).unwrap()).unwrap(), C::F_from_slice(&hex::decode(vectors.nonces[c][1]).unwrap()).unwrap() ]; + c += 1; let mut serialized = C::G_to_bytes(&(C::GENERATOR * nonces[0])); serialized.extend(&C::G_to_bytes(&(C::GENERATOR * nonces[1]))); - machine.unsafe_override_preprocess( + let (machine, serialized) = machine.unsafe_override_preprocess( PreprocessPackage { nonces, serialized: serialized.clone() } ); - commitments.insert(*i, serialized); - c += 1; - } + commitments.insert(i, serialized); + (i, machine) + }).collect::>(); let mut shares = HashMap::new(); c = 0; - for (i, machine) in machines.iter_mut() { - let share = machine.sign(commitments.clone(), &hex::decode(vectors.msg).unwrap()).unwrap(); - assert_eq!(share, hex::decode(vectors.sig_shares[c]).unwrap()); - shares.insert(*i, share); - c += 1; - } + let mut machines = machines.drain(..).map(|(i, machine)| { + let (machine, share) = machine.sign( + commitments.clone(), + &hex::decode(vectors.msg).unwrap() + ).unwrap(); - for (_, machine) in machines.iter_mut() { + assert_eq!(share, hex::decode(vectors.sig_shares[c]).unwrap()); + c += 1; + + shares.insert(i, share); + (i, machine) + }).collect::>(); + + for (_, machine) in machines.drain() { let sig = machine.complete(shares.clone()).unwrap(); let mut serialized = C::G_to_bytes(&sig.R); serialized.extend(C::F_to_bytes(&sig.s)); diff --git a/processor/src/coins/monero.rs b/processor/src/coins/monero.rs index 55226981..a9757e97 100644 --- a/processor/src/coins/monero.rs +++ b/processor/src/coins/monero.rs @@ -1,7 +1,6 @@ use std::sync::Arc; use async_trait::async_trait; -use rand_core::OsRng; use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar}; diff --git a/processor/src/lib.rs b/processor/src/lib.rs index 2a615ea5..d990a4c5 100644 --- a/processor/src/lib.rs +++ b/processor/src/lib.rs @@ -3,7 +3,7 @@ use std::{marker::Send, sync::Arc, collections::HashMap}; use async_trait::async_trait; use thiserror::Error; -use frost::{Curve, FrostError, MultisigKeys, sign::StateMachine}; +use frost::{Curve, FrostError, MultisigKeys, sign::PreprocessMachine}; pub(crate) use monero_serai::frost::Transcript; @@ -57,7 +57,7 @@ pub trait Coin { type Output: Output; type SignableTransaction; - type TransactionMachine: StateMachine; + type TransactionMachine: PreprocessMachine; type Address: Send; diff --git a/processor/src/wallet.rs b/processor/src/wallet.rs index be58a838..cf3e731e 100644 --- a/processor/src/wallet.rs +++ b/processor/src/wallet.rs @@ -4,7 +4,7 @@ use rand_core::OsRng; use transcript::Transcript as TranscriptTrait; -use frost::{Curve, MultisigKeys, sign::StateMachine}; +use frost::{Curve, MultisigKeys, sign::{PreprocessMachine, SignMachine, SignatureMachine}}; use crate::{Transcript, CoinError, SignError, Output, Coin, Network}; @@ -344,17 +344,17 @@ impl Wallet { prepared: C::SignableTransaction, included: Vec ) -> Result<(Vec, Vec<::Id>), SignError> { - let mut attempt = self.coin.attempt_send( + let attempt = self.coin.attempt_send( prepared, &included ).await.map_err(|e| SignError::CoinError(e))?; - let commitments = network.round( - attempt.preprocess(&mut OsRng).unwrap() - ).await.map_err(|e| SignError::NetworkError(e))?; - let shares = network.round( - attempt.sign(commitments, b"").map_err(|e| SignError::FrostError(e))? - ).await.map_err(|e| SignError::NetworkError(e))?; + let (attempt, commitments) = attempt.preprocess(&mut OsRng); + let commitments = network.round(commitments).await.map_err(|e| SignError::NetworkError(e))?; + + let (attempt, share) = attempt.sign(commitments, b"").map_err(|e| SignError::FrostError(e))?; + let shares = network.round(share).await.map_err(|e| SignError::NetworkError(e))?; + let tx = attempt.complete(shares).map_err(|e| SignError::FrostError(e))?; self.coin.publish_transaction(&tx).await.map_err(|e| SignError::CoinError(e))