From 865dee80e5194e101eae7bda20f1133e49a4ed89 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 14 Jun 2024 16:17:51 -0400 Subject: [PATCH] Document and clean clsag --- coins/monero/primitives/src/lib.rs | 5 + coins/monero/ringct/clsag/README.md | 2 + coins/monero/ringct/clsag/src/lib.rs | 123 ++++++++++++--------- coins/monero/ringct/clsag/src/multisig.rs | 125 ++++++++++++---------- coins/monero/src/tests/clsag.rs | 33 +++--- coins/monero/src/wallet/send/mod.rs | 33 ++++-- coins/monero/src/wallet/send/multisig.rs | 49 ++++----- 7 files changed, 209 insertions(+), 161 deletions(-) diff --git a/coins/monero/primitives/src/lib.rs b/coins/monero/primitives/src/lib.rs index 75f48410..1697b887 100644 --- a/coins/monero/primitives/src/lib.rs +++ b/coins/monero/primitives/src/lib.rs @@ -151,4 +151,9 @@ impl Decoys { pub fn ring(&self) -> &[[EdwardsPoint; 2]] { &self.ring } + + // The [key, commitment] pair of the signer. + pub fn signer_ring_members(&self) -> [EdwardsPoint; 2] { + self.ring[usize::from(self.signer_index)] + } } diff --git a/coins/monero/ringct/clsag/README.md b/coins/monero/ringct/clsag/README.md index a589d69c..2af88f09 100644 --- a/coins/monero/ringct/clsag/README.md +++ b/coins/monero/ringct/clsag/README.md @@ -2,5 +2,7 @@ The CLSAG linkable ring signature, as defined by the Monero protocol. +Additionally included is a FROST-based threshold multisignature algorithm. + This library is usable under no-std when the `std` feature (on by default) is disabled. diff --git a/coins/monero/ringct/clsag/src/lib.rs b/coins/monero/ringct/clsag/src/lib.rs index ae467869..5f599eef 100644 --- a/coins/monero/ringct/clsag/src/lib.rs +++ b/coins/monero/ringct/clsag/src/lib.rs @@ -28,7 +28,7 @@ use monero_primitives::{INV_EIGHT, BASEPOINT_PRECOMP, Commitment, Decoys, keccak #[cfg(feature = "multisig")] mod multisig; #[cfg(feature = "multisig")] -pub use multisig::{ClsagDetails, ClsagAddendum, ClsagMultisig}; +pub use multisig::{ClsagAddendum, ClsagMultisig}; /// Errors when working with CLSAGs. #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -37,9 +37,9 @@ pub enum ClsagError { /// The ring was invalid (such as being too small or too large). #[cfg_attr(feature = "std", error("invalid ring"))] InvalidRing, - /// The specified ring member was invalid (index, ring size). - #[cfg_attr(feature = "std", error("invalid ring member (member {0}, ring size {1})"))] - InvalidRingMember(u8, u8), + /// The discrete logarithm of the key, scaling G, wasn't equivalent to the signing ring member. + #[cfg_attr(feature = "std", error("invalid commitment"))] + InvalidKey, /// The commitment opening provided did not match the ring member's. #[cfg_attr(feature = "std", error("invalid commitment"))] InvalidCommitment, @@ -57,32 +57,28 @@ pub enum ClsagError { InvalidC1, } -/// Context on the ring member being signed for. +/// Context on the input being signed for. #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] -pub struct ClsagInput { - // The actual commitment for the true spend - pub commitment: Commitment, - // True spend index, offsets, and ring - pub decoys: Decoys, +pub struct ClsagContext { + // The opening for the commitment of the signing ring member + commitment: Commitment, + // Selected ring members' positions, signer index, and ring + decoys: Decoys, } -impl ClsagInput { - pub fn new(commitment: Commitment, decoys: Decoys) -> Result { - let n = decoys.len(); - if n > u8::MAX.into() { +impl ClsagContext { + /// Create a new context, as necessary for signing. + pub fn new(decoys: Decoys, commitment: Commitment) -> Result { + if decoys.len() > u8::MAX.into() { Err(ClsagError::InvalidRing)?; } - let n = u8::try_from(n).unwrap(); - if decoys.signer_index() >= n { - Err(ClsagError::InvalidRingMember(decoys.signer_index(), n))?; - } // Validate the commitment matches - if decoys.ring()[usize::from(decoys.signer_index())][1] != commitment.calculate() { + if decoys.signer_ring_members()[1] != commitment.calculate() { Err(ClsagError::InvalidCommitment)?; } - Ok(ClsagInput { commitment, decoys }) + Ok(ClsagContext { commitment, decoys }) } } @@ -93,6 +89,7 @@ enum Mode { } // Core of the CLSAG algorithm, applicable to both sign and verify with minimal differences +// // Said differences are covered via the above Mode fn core( ring: &[[EdwardsPoint; 2]], @@ -217,15 +214,16 @@ fn core( ((D_INV_EIGHT, c * mu_P, c * mu_C), c1) } -/// CLSAG signature, as used in Monero. +/// The CLSAG signature, as used in Monero. #[derive(Clone, PartialEq, Eq, Debug)] pub struct Clsag { D: EdwardsPoint, - pub(crate) s: Vec, + s: Vec, + // TODO: Remove pub pub c1: Scalar, } -pub(crate) struct ClsagSignCore { +struct ClsagSignCore { incomplete_clsag: Clsag, pseudo_out: EdwardsPoint, key_challenge: Scalar, @@ -235,10 +233,10 @@ pub(crate) struct ClsagSignCore { impl Clsag { // Sign core is the extension of core as needed for signing, yet is shared between single signer // and multisig, hence why it's still core - pub(crate) fn sign_core( + fn sign_core( rng: &mut R, I: &EdwardsPoint, - input: &ClsagInput, + input: &ClsagContext, mask: Scalar, msg: &[u8; 32], A: EdwardsPoint, @@ -266,23 +264,54 @@ impl Clsag { } } - /// Generate CLSAG signatures for the given inputs. + /// Sign CLSAG signatures for the provided inputs. /// - /// inputs is of the form (private key, key image, input). - /// sum_outputs is for the sum of the outputs' commitment masks. + /// Monero ensures the rerandomized input commitments have the same value as the outputs by + /// checking `sum(rerandomized_input_commitments) - sum(output_commitments) == 0`. This requires + /// not only the amounts balance, yet also + /// `sum(input_commitment_masks) - sum(output_commitment_masks)`. + /// + /// Monero solves this by following the wallet protocol to determine each output commitment's + /// randomness, then using random masks for all but the last input. The last input is + /// rerandomized to the necessary mask for the equation to balance. + /// + /// Due to Monero having this behavior, it only makes sense to sign CLSAGs as a list, hence this + /// API being the way it is. + /// + /// `inputs` is of the form (discrete logarithm of the key, context). + /// + /// `sum_outputs` is for the sum of the output commitments' masks. pub fn sign( rng: &mut R, - mut inputs: Vec<(Zeroizing, EdwardsPoint, ClsagInput)>, + mut inputs: Vec<(Zeroizing, ClsagContext)>, sum_outputs: Scalar, msg: [u8; 32], - ) -> Vec<(Clsag, EdwardsPoint)> { + ) -> Result, ClsagError> { + // Create the key images + let mut key_image_generators = vec![]; + let mut key_images = vec![]; + for input in &inputs { + let key = input.1.decoys.signer_ring_members()[0]; + + // Check the key is consistent + if (ED25519_BASEPOINT_TABLE * input.0.deref()) != key { + Err(ClsagError::InvalidKey)?; + } + + let key_image_generator = hash_to_point(key.compress().0); + key_image_generators.push(key_image_generator); + key_images.push(key_image_generator * input.0.deref()); + } + let mut res = Vec::with_capacity(inputs.len()); let mut sum_pseudo_outs = Scalar::ZERO; for i in 0 .. inputs.len() { - let mut mask = Scalar::random(rng); + let mask; + // If this is the last input, set the mask as described above if i == (inputs.len() - 1) { mask = sum_outputs - sum_pseudo_outs; } else { + mask = Scalar::random(rng); sum_pseudo_outs += mask; } @@ -290,22 +319,17 @@ impl Clsag { let ClsagSignCore { mut incomplete_clsag, pseudo_out, key_challenge, challenged_mask } = Clsag::sign_core( rng, + &key_images[i], &inputs[i].1, - &inputs[i].2, mask, &msg, nonce.deref() * ED25519_BASEPOINT_TABLE, - nonce.deref() * - hash_to_point( - inputs[i].2.decoys.ring()[usize::from(inputs[i].2.decoys.signer_index())][0] - .compress() - .0, - ), + nonce.deref() * key_image_generators[i], ); - // Effectively r - cx, except cx is (c_p x) + (c_c z), where z is the delta between a ring - // member's commitment and our input commitment (which will only have a known discrete log - // over G if the amounts cancel out) - incomplete_clsag.s[usize::from(inputs[i].2.decoys.signer_index())] = + // Effectively r - c x, except c x is (c_p x) + (c_c z), where z is the delta between the + // ring member's commitment and our pseudo-out commitment (which will only have a known + // discrete log over G if the amounts cancel out) + incomplete_clsag.s[usize::from(inputs[i].1.decoys.signer_index())] = nonce.deref() - ((key_challenge * inputs[i].0.deref()) + challenged_mask); let clsag = incomplete_clsag; @@ -314,16 +338,16 @@ impl Clsag { nonce.zeroize(); debug_assert!(clsag - .verify(inputs[i].2.decoys.ring(), &inputs[i].1, &pseudo_out, &msg) + .verify(inputs[i].1.decoys.ring(), &key_images[i], &pseudo_out, &msg) .is_ok()); res.push((clsag, pseudo_out)); } - res + Ok(res) } - /// Verify the CLSAG signature against the given Transaction data. + /// Verify a CLSAG signature for the provided context. pub fn verify( &self, ring: &[[EdwardsPoint; 2]], @@ -331,8 +355,8 @@ impl Clsag { pseudo_out: &EdwardsPoint, msg: &[u8; 32], ) -> Result<(), ClsagError> { - // Preliminary checks. s, c1, and points must also be encoded canonically, which isn't checked - // here + // Preliminary checks + // s, c1, and points must also be encoded canonically, which is checked at time of decode if ring.is_empty() { Err(ClsagError::InvalidRing)?; } @@ -355,18 +379,19 @@ impl Clsag { Ok(()) } + /// The weight a CLSAG will take within a Monero transaction. pub fn fee_weight(ring_len: usize) -> usize { (ring_len * 32) + 32 + 32 } - /// Write the CLSAG to a writer. + /// Write a CLSAG. pub fn write(&self, w: &mut W) -> io::Result<()> { write_raw_vec(write_scalar, &self.s, w)?; w.write_all(&self.c1.to_bytes())?; write_point(&self.D, w) } - /// Read a CLSAG from a reader. + /// Read a CLSAG. pub fn read(decoys: usize, r: &mut R) -> io::Result { Ok(Clsag { s: read_raw_vec(read_scalar, decoys, r)?, c1: read_scalar(r)?, D: read_point(r)? }) } diff --git a/coins/monero/ringct/clsag/src/multisig.rs b/coins/monero/ringct/clsag/src/multisig.rs index 82d0f88c..06ae1a58 100644 --- a/coins/monero/ringct/clsag/src/multisig.rs +++ b/coins/monero/ringct/clsag/src/multisig.rs @@ -3,12 +3,12 @@ use std_shims::{ io::{self, Read, Write}, collections::HashMap, }; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex}; use rand_core::{RngCore, CryptoRng, SeedableRng}; use rand_chacha::ChaCha20Rng; -use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; +use zeroize::{Zeroize, Zeroizing}; use curve25519_dalek::{scalar::Scalar, edwards::EdwardsPoint}; @@ -28,20 +28,20 @@ use frost::{ use monero_generators::hash_to_point; -use crate::{ClsagInput, Clsag}; +use crate::{ClsagContext, Clsag}; -impl ClsagInput { +impl ClsagContext { fn transcript(&self, transcript: &mut T) { // Doesn't domain separate as this is considered part of the larger CLSAG proof // Ring index - transcript.append_message(b"real_spend", [self.decoys.signer_index()]); + transcript.append_message(b"signer_index", [self.decoys.signer_index()]); // Ring for (i, pair) in self.decoys.ring().iter().enumerate() { - // Doesn't include global output indexes as CLSAG doesn't care and won't be affected by it + // Doesn't include global output indexes as CLSAG doesn't care/won't be affected by it // They're just a unreliable reference to this data which will be included in the message - // if in use + // if somehow relevant transcript.append_message(b"member", [u8::try_from(i).expect("ring size exceeded 255")]); // This also transcripts the key image generator since it's derived from this key transcript.append_message(b"key", pair[0].compress().to_bytes()); @@ -49,33 +49,26 @@ impl ClsagInput { } // Doesn't include the commitment's parts as the above ring + index includes the commitment - // The only potential malleability would be if the G/H relationship is known breaking the + // The only potential malleability would be if the G/H relationship is known, breaking the // discrete log problem, which breaks everything already } } -/// CLSAG input and the mask to use for it. -#[derive(Clone, Debug, Zeroize, ZeroizeOnDrop)] -pub struct ClsagDetails { - input: ClsagInput, - mask: Scalar, -} - -impl ClsagDetails { - pub fn new(input: ClsagInput, mask: Scalar) -> ClsagDetails { - ClsagDetails { input, mask } - } -} - -/// Addendum produced during the FROST signing process with relevant data. +/// Addendum produced during the signing process. #[derive(Clone, PartialEq, Eq, Zeroize, Debug)] pub struct ClsagAddendum { - pub key_image: dfg::EdwardsPoint, + key_image_share: dfg::EdwardsPoint, +} + +impl ClsagAddendum { + pub fn key_image_share(&self) -> dfg::EdwardsPoint { + self.key_image_share + } } impl WriteAddendum for ClsagAddendum { fn write(&self, writer: &mut W) -> io::Result<()> { - writer.write_all(self.key_image.compress().to_bytes().as_ref()) + writer.write_all(self.key_image_share.compress().to_bytes().as_ref()) } } @@ -89,66 +82,77 @@ struct Interim { pseudo_out: EdwardsPoint, } -/// FROST algorithm for producing a CLSAG signature. +/// FROST-inspired algorithm for producing a CLSAG signature. #[allow(non_snake_case)] #[derive(Clone, Debug)] pub struct ClsagMultisig { transcript: RecommendedTranscript, - pub H: EdwardsPoint, + key_image_generator: EdwardsPoint, key_image_shares: HashMap<[u8; 32], dfg::EdwardsPoint>, image: Option, - details: Arc>>, + context: ClsagContext, + + mask_mutex: Arc>>, + mask: Option, msg: Option<[u8; 32]>, interim: Option, } impl ClsagMultisig { + /// Construct a new instance of multisignature CLSAG signing. + /// + /// Before this has its `process_addendum` called, a mask must be set. Else this will panic. pub fn new( transcript: RecommendedTranscript, - output_key: EdwardsPoint, - details: Arc>>, + context: ClsagContext, + mask_mutex: Arc>>, ) -> ClsagMultisig { ClsagMultisig { transcript, - H: hash_to_point(output_key.compress().0), + key_image_generator: hash_to_point(context.decoys.signer_ring_members()[0].compress().0), key_image_shares: HashMap::new(), image: None, - details, + context, + + mask_mutex, + mask: None, msg: None, interim: None, } } - fn input(&self) -> ClsagInput { - (*self.details.read().unwrap()).as_ref().unwrap().input.clone() - } - - fn mask(&self) -> Scalar { - (*self.details.read().unwrap()).as_ref().unwrap().mask + /// The key image generator used by the signer. + pub fn key_image_generator(&self) -> EdwardsPoint { + self.key_image_generator } } impl Algorithm for ClsagMultisig { type Transcript = RecommendedTranscript; type Addendum = ClsagAddendum; + // We output the CLSAG and the key image, which requires an interactive protocol to obtain type Signature = (Clsag, EdwardsPoint); + // We need the nonce represented against both G and the key image generator fn nonces(&self) -> Vec> { - vec![vec![dfg::EdwardsPoint::generator(), dfg::EdwardsPoint(self.H)]] + vec![vec![dfg::EdwardsPoint::generator(), dfg::EdwardsPoint(self.key_image_generator)]] } + // We also publish our share of the key image fn preprocess_addendum( &mut self, _rng: &mut R, keys: &ThresholdKeys, ) -> ClsagAddendum { - ClsagAddendum { key_image: dfg::EdwardsPoint(self.H) * keys.secret_share().deref() } + ClsagAddendum { + key_image_share: dfg::EdwardsPoint(self.key_image_generator) * keys.secret_share().deref(), + } } fn read_addendum(&self, reader: &mut R) -> io::Result { @@ -162,7 +166,7 @@ impl Algorithm for ClsagMultisig { Err(io::Error::other("non-canonical key image"))?; } - Ok(ClsagAddendum { key_image: xH }) + Ok(ClsagAddendum { key_image_share: xH }) } fn process_addendum( @@ -174,21 +178,27 @@ impl Algorithm for ClsagMultisig { if self.image.is_none() { self.transcript.domain_separate(b"CLSAG"); // Transcript the ring - self.input().transcript(&mut self.transcript); + self.context.transcript(&mut self.transcript); + // Fetch the mask from the Mutex + // We set it to a variable to ensure our view of it is consistent + // It was this or a mpsc channel... std doesn't have oneshot :/ + self.mask = Some(self.mask_mutex.lock().unwrap().unwrap()); // Transcript the mask - self.transcript.append_message(b"mask", self.mask().to_bytes()); + self.transcript.append_message(b"mask", self.mask.expect("mask wasn't set").to_bytes()); // Init the image to the offset - self.image = Some(dfg::EdwardsPoint(self.H) * view.offset()); + self.image = Some(dfg::EdwardsPoint(self.key_image_generator) * view.offset()); } // Transcript this participant's contribution self.transcript.append_message(b"participant", l.to_bytes()); - self.transcript.append_message(b"key_image_share", addendum.key_image.compress().to_bytes()); + self + .transcript + .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 * lagrange::(l, view.included()); + addendum.key_image_share * lagrange::(l, view.included()); *self.image.as_mut().unwrap() += interpolated_key_image_share; self @@ -210,19 +220,22 @@ impl Algorithm for ClsagMultisig { msg: &[u8], ) -> dfg::Scalar { // Use the transcript to get a seeded random number generator + // // The transcript contains private data, preventing passive adversaries from recreating this - // process even if they have access to commitments (specifically, the ring index being signed - // for, along with the mask which should not only require knowing the shared keys yet also the - // input commitment masks) + // process even if they have access to the commitments/key image share broadcast so far + // + // Specifically, the transcript contains the signer's index within the ring, along with the + // opening of the commitment being re-randomized (and what it's re-randomized to) let mut rng = ChaCha20Rng::from_seed(self.transcript.rng_seed(b"decoy_responses")); + // TODO: Accept the message preimage and remove this panic self.msg = Some(msg.try_into().expect("CLSAG message should be 32-bytes")); let sign_core = Clsag::sign_core( &mut rng, &self.image.expect("verifying a share despite never processing any addendums").0, - &self.input(), - self.mask(), + &self.context, + self.mask.expect("mask wasn't set"), self.msg.as_ref().unwrap(), nonce_sums[0][0].0, nonce_sums[0][1].0, @@ -247,12 +260,12 @@ impl Algorithm for ClsagMultisig { ) -> Option { let interim = self.interim.as_ref().unwrap(); let mut clsag = interim.clsag.clone(); - // We produced shares as `r - p x`, yet the signature is `r - p x - c x` + // We produced shares as `r - p x`, yet the signature is actually `r - p x - c x` // Substract `c x` (saved as `c`) now - clsag.s[usize::from(self.input().decoys.signer_index())] = sum.0 - interim.c; + clsag.s[usize::from(self.context.decoys.signer_index())] = sum.0 - interim.c; if clsag .verify( - self.input().decoys.ring(), + self.context.decoys.ring(), &self.image.expect("verifying a signature despite never processing any addendums").0, &interim.pseudo_out, self.msg.as_ref().unwrap(), @@ -296,11 +309,11 @@ impl Algorithm for ClsagMultisig { let key_image_share = self.key_image_shares[&verification_share.to_bytes()]; - // Hash every variable relevant here, using the hahs output as the random weight + // Hash every variable relevant here, using the hash output as the random weight let mut weight_transcript = RecommendedTranscript::new(b"monero-serai v0.1 ClsagMultisig::verify_share"); weight_transcript.append_message(b"G", dfg::EdwardsPoint::generator().to_bytes()); - weight_transcript.append_message(b"H", self.H.to_bytes()); + weight_transcript.append_message(b"H", self.key_image_generator.to_bytes()); weight_transcript.append_message(b"xG", verification_share.to_bytes()); weight_transcript.append_message(b"xH", key_image_share.to_bytes()); weight_transcript.append_message(b"rG", nonces[0][0].to_bytes()); @@ -318,7 +331,7 @@ impl Algorithm for ClsagMultisig { ]; let mut part_two = vec![ - (weight * share, dfg::EdwardsPoint(self.H)), + (weight * share, dfg::EdwardsPoint(self.key_image_generator)), // -(R.1 - pK) == -R.1 + pK (-weight, nonces[0][1]), (weight * dfg::Scalar(interim.p), key_image_share), diff --git a/coins/monero/src/tests/clsag.rs b/coins/monero/src/tests/clsag.rs index fe488729..bc708d76 100644 --- a/coins/monero/src/tests/clsag.rs +++ b/coins/monero/src/tests/clsag.rs @@ -1,6 +1,5 @@ use core::ops::Deref; -#[cfg(feature = "multisig")] -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex}; use zeroize::Zeroizing; use rand_core::{RngCore, OsRng}; @@ -17,11 +16,11 @@ use crate::{ wallet::Decoys, ringct::{ generate_key_image, - clsag::{ClsagInput, Clsag}, + clsag::{ClsagContext, Clsag}, }, }; #[cfg(feature = "multisig")] -use crate::ringct::clsag::{ClsagDetails, ClsagMultisig}; +use crate::ringct::clsag::ClsagMultisig; #[cfg(feature = "multisig")] use frost::{ @@ -56,24 +55,24 @@ fn clsag() { .push([dest.deref() * ED25519_BASEPOINT_TABLE, Commitment::new(mask, amount).calculate()]); } - let image = generate_key_image(&secrets.0); let (mut clsag, pseudo_out) = Clsag::sign( &mut OsRng, vec![( - secrets.0, - image, - ClsagInput::new( - Commitment::new(secrets.1, AMOUNT), + secrets.0.clone(), + ClsagContext::new( Decoys::new((1 ..= RING_LEN).collect(), u8::try_from(real).unwrap(), ring.clone()) .unwrap(), + Commitment::new(secrets.1, AMOUNT), ) .unwrap(), )], Scalar::random(&mut OsRng), msg, ) + .unwrap() .swap_remove(0); + let image = generate_key_image(&secrets.0); clsag.verify(&ring, &image, &pseudo_out, &msg).unwrap(); // make sure verification fails if we throw a random `c1` at it. @@ -105,18 +104,14 @@ fn clsag_multisig() { ring.push([dest, Commitment::new(mask, amount).calculate()]); } - let mask_sum = Scalar::random(&mut OsRng); let algorithm = ClsagMultisig::new( RecommendedTranscript::new(b"Monero Serai CLSAG Test"), - keys[&Participant::new(1).unwrap()].group_key().0, - Arc::new(RwLock::new(Some(ClsagDetails::new( - ClsagInput::new( - Commitment::new(randomness, AMOUNT), - Decoys::new((1 ..= RING_LEN).collect(), RING_INDEX, ring.clone()).unwrap(), - ) - .unwrap(), - mask_sum, - )))), + ClsagContext::new( + Decoys::new((1 ..= RING_LEN).collect(), RING_INDEX, ring.clone()).unwrap(), + Commitment::new(randomness, AMOUNT), + ) + .unwrap(), + Arc::new(Mutex::new(Some(Scalar::random(&mut OsRng)))), ); sign( diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index a6cb107e..50a126b3 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -32,7 +32,7 @@ use crate::{ }, ringct::{ generate_key_image, - clsag::{ClsagError, ClsagInput, Clsag}, + clsag::{ClsagError, ClsagContext, Clsag}, bulletproofs::{MAX_OUTPUTS, Bulletproof}, RctBase, RctPrunable, RctSignatures, }, @@ -168,28 +168,34 @@ fn prepare_inputs( inputs: &[(SpendableOutput, Decoys)], spend: &Zeroizing, tx: &mut Transaction, -) -> Result, EdwardsPoint, ClsagInput)>, TransactionError> { +) -> Result, ClsagContext)>, TransactionError> { let mut signable = Vec::with_capacity(inputs.len()); - for (i, (input, decoys)) in inputs.iter().enumerate() { + for (input, decoys) in inputs { let input_spend = Zeroizing::new(input.key_offset() + spend.deref()); let image = generate_key_image(&input_spend); signable.push(( input_spend, - image, - ClsagInput::new(input.commitment().clone(), decoys.clone()) + ClsagContext::new(decoys.clone(), input.commitment().clone()) .map_err(TransactionError::ClsagError)?, )); tx.prefix.inputs.push(Input::ToKey { amount: None, key_offsets: decoys.offsets().to_vec(), - key_image: signable[i].1, + key_image: image, }); } - signable.sort_by(|x, y| x.1.compress().to_bytes().cmp(&y.1.compress().to_bytes()).reverse()); - tx.prefix.inputs.sort_by(|x, y| { + // We now need to sort the inputs by their key image + // We take the transaction's inputs, temporarily + let mut tx_inputs = Vec::with_capacity(inputs.len()); + std::mem::swap(&mut tx_inputs, &mut tx.prefix.inputs); + + // Then we join them with their signable contexts + let mut joint = tx_inputs.into_iter().zip(signable).collect::>(); + // Perform the actual sort + joint.sort_by(|(x, _), (y, _)| { if let (Input::ToKey { key_image: x, .. }, Input::ToKey { key_image: y, .. }) = (x, y) { x.compress().to_bytes().cmp(&y.compress().to_bytes()).reverse() } else { @@ -197,6 +203,14 @@ fn prepare_inputs( } }); + // We now re-create the consumed signable (tx.prefix.inputs already having an empty vector) and + // split the joint iterator back into two Vecs + let mut signable = Vec::with_capacity(inputs.len()); + for (input, signable_i) in joint { + tx.prefix.inputs.push(input); + signable.push(signable_i); + } + Ok(signable) } @@ -875,7 +889,8 @@ impl SignableTransaction { let signable = prepare_inputs(&self.inputs, spend, &mut tx)?; - let clsag_pairs = Clsag::sign(rng, signable, mask_sum, tx.signature_hash()); + let clsag_pairs = Clsag::sign(rng, signable, mask_sum, tx.signature_hash()) + .map_err(|_| TransactionError::WrongPrivateKey)?; match tx.rct_signatures.prunable { RctPrunable::Null => panic!("Signing for RctPrunable::Null"), RctPrunable::Clsag { ref mut clsags, ref mut pseudo_outs, .. } => { diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index b3b5f4de..770d44c4 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -3,7 +3,7 @@ use std_shims::{ io::{self, Read}, collections::HashMap, }; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex}; use zeroize::Zeroizing; @@ -27,7 +27,7 @@ use frost::{ use crate::{ ringct::{ - clsag::{ClsagInput, ClsagDetails, ClsagAddendum, ClsagMultisig}, + clsag::{ClsagContext, ClsagAddendum, ClsagMultisig}, RctPrunable, }, transaction::{Input, Transaction}, @@ -43,7 +43,7 @@ pub struct TransactionMachine { // Hashed key and scalar offset key_images: Vec<(EdwardsPoint, Scalar)>, - inputs: Vec>>>, + clsag_mask_mutexes: Vec>>>, clsags: Vec>, } @@ -54,7 +54,7 @@ pub struct TransactionSignMachine { transcript: RecommendedTranscript, key_images: Vec<(EdwardsPoint, Scalar)>, - inputs: Vec>>>, + clsag_mask_mutexes: Vec>>>, clsags: Vec>, our_preprocess: Vec>, @@ -73,10 +73,10 @@ impl SignableTransaction { keys: &ThresholdKeys, mut transcript: RecommendedTranscript, ) -> Result { - let mut inputs = vec![]; + let mut clsag_mask_mutexes = vec![]; for _ in 0 .. self.inputs.len() { // Doesn't resize as that will use a single Rc for the entire Vec - inputs.push(Arc::new(RwLock::new(None))); + clsag_mask_mutexes.push(Arc::new(Mutex::new(None))); } let mut clsags = vec![]; @@ -139,16 +139,18 @@ impl SignableTransaction { } let mut key_images = vec![]; - for (i, (input, _)) in self.inputs.iter().enumerate() { + for (i, (input, decoys)) in self.inputs.iter().enumerate() { // Check this the right set of keys let offset = keys.offset(dfg::Scalar(input.key_offset())); if offset.group_key().0 != input.key() { Err(TransactionError::WrongPrivateKey)?; } - let clsag = ClsagMultisig::new(transcript.clone(), input.key(), inputs[i].clone()); + let context = ClsagContext::new(decoys.clone(), input.commitment()) + .map_err(TransactionError::ClsagError)?; + let clsag = ClsagMultisig::new(transcript.clone(), context, clsag_mask_mutexes[i].clone()); key_images.push(( - clsag.H, + clsag.key_image_generator(), keys.current_offset().unwrap_or(dfg::Scalar::ZERO).0 + self.inputs[i].0.key_offset(), )); clsags.push(AlgorithmMachine::new(clsag, offset)); @@ -156,12 +158,10 @@ impl SignableTransaction { Ok(TransactionMachine { signable: self, - i: keys.params().i(), transcript, - key_images, - inputs, + clsag_mask_mutexes, clsags, }) } @@ -206,7 +206,7 @@ impl PreprocessMachine for TransactionMachine { transcript: self.transcript, key_images: self.key_images, - inputs: self.inputs, + clsag_mask_mutexes: self.clsag_mask_mutexes, clsags, our_preprocess, @@ -296,7 +296,8 @@ impl SignMachine for TransactionSignMachine { // 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 // outputs may need these in order to guarantee uniqueness) - images[c] += preprocess.addendum.key_image.0 * lagrange::(*l, &included).0; + images[c] += + preprocess.addendum.key_image_share().0 * lagrange::(*l, &included).0; Ok((*l, preprocess)) }) @@ -330,12 +331,10 @@ impl SignMachine for TransactionSignMachine { // Sort the inputs, as expected let mut sorted = Vec::with_capacity(self.clsags.len()); while !self.clsags.is_empty() { - let (inputs, decoys) = self.signable.inputs.swap_remove(0); sorted.push(( images.swap_remove(0), - inputs, - decoys, - self.inputs.swap_remove(0), + self.signable.inputs.swap_remove(0).1, + self.clsag_mask_mutexes.swap_remove(0), self.clsags.swap_remove(0), commitments.swap_remove(0), )); @@ -353,22 +352,16 @@ impl SignMachine for TransactionSignMachine { } else { sum_pseudo_outs += mask; } + *value.2.lock().unwrap() = Some(mask); tx.prefix.inputs.push(Input::ToKey { amount: None, - key_offsets: value.2.offsets().to_vec(), + key_offsets: value.1.offsets().to_vec(), key_image: value.0, }); - *value.3.write().unwrap() = Some(ClsagDetails::new( - ClsagInput::new(value.1.commitment().clone(), value.2).map_err(|_| { - panic!("Signing an input which isn't present in the ring we created for it") - })?, - mask, - )); - - self.clsags.push(value.4); - commitments.push(value.5); + self.clsags.push(value.3); + commitments.push(value.4); } let msg = tx.signature_hash();