Document and clean clsag

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)]
+  }
 }

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.

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).
+  /// The discrete logarithm of the key, scaling G, wasn't equivalent to the signing ring member.
-  #[cfg_attr(feature = "std", error("invalid ring member (member {0}, ring size {1})"))]
+  #[cfg_attr(feature = "std", error("invalid commitment"))]
-  InvalidRingMember(u8, u8),
+  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 {
+pub struct ClsagContext {
-  // The actual commitment for the true spend
+  // The opening for the commitment of the signing ring member
-  pub commitment: Commitment,
+  commitment: Commitment,
-  // True spend index, offsets, and ring
+  // Selected ring members' positions, signer index, and ring
-  pub decoys: Decoys,
+  decoys: Decoys,
 }
 
-impl ClsagInput {
+impl ClsagContext {
-  pub fn new(commitment: Commitment, decoys: Decoys) -> Result<ClsagInput, ClsagError> {
+  /// Create a new context, as necessary for signing.
-    let n = decoys.len();
+  pub fn new(decoys: Decoys, commitment: Commitment) -> Result<ClsagContext, ClsagError> {
-    if n > u8::MAX.into() {
+    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<Scalar>,
+  s: Vec<Scalar>,
+  // 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<R: RngCore + CryptoRng>(
+  fn sign_core<R: RngCore + CryptoRng>(
     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).
+  /// Monero ensures the rerandomized input commitments have the same value as the outputs by
-  /// sum_outputs is for the sum of the outputs' commitment masks.
+  /// 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<R: RngCore + CryptoRng>(
     rng: &mut R,
-    mut inputs: Vec<(Zeroizing<Scalar>, EdwardsPoint, ClsagInput)>,
+    mut inputs: Vec<(Zeroizing<Scalar>, ClsagContext)>,
     sum_outputs: Scalar,
     msg: [u8; 32],
-  ) -> Vec<(Clsag, EdwardsPoint)> {
+  ) -> Result<Vec<(Clsag, EdwardsPoint)>, 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() *
+          nonce.deref() * key_image_generators[i],
-            hash_to_point(
-              inputs[i].2.decoys.ring()[usize::from(inputs[i].2.decoys.signer_index())][0]
-                .compress()
-                .0,
-            ),
         );
-      // Effectively r - cx, except cx is (c_p x) + (c_c z), where z is the delta between a ring
+      // Effectively r - c x, except c x is (c_p x) + (c_c z), where z is the delta between the
-      // member's commitment and our input commitment (which will only have a known discrete log
+      // ring member's commitment and our pseudo-out commitment (which will only have a known
-      // over G if the amounts cancel out)
+      // discrete log over G if the amounts cancel out)
-      incomplete_clsag.s[usize::from(inputs[i].2.decoys.signer_index())] =
+      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
+    // Preliminary checks
-    // here
+    // 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<W: 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<R: Read>(decoys: usize, r: &mut R) -> io::Result<Clsag> {
     Ok(Clsag { s: read_raw_vec(read_scalar, decoys, r)?, c1: read_scalar(r)?, D: read_point(r)? })
   }

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<T: 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.
+/// Addendum produced during the signing process.
-#[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.
 #[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<W: 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<dfg::EdwardsPoint>,
 
-  details: Arc<RwLock<Option<ClsagDetails>>>,
+  context: ClsagContext,
+
+  mask_mutex: Arc<Mutex<Option<Scalar>>>,
+  mask: Option<Scalar>,
 
   msg: Option<[u8; 32]>,
   interim: Option<Interim>,
 }
 
 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,
+    context: ClsagContext,
-    details: Arc<RwLock<Option<ClsagDetails>>>,
+    mask_mutex: Arc<Mutex<Option<Scalar>>>,
   ) -> 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 {
+  /// The key image generator used by the signer.
-    (*self.details.read().unwrap()).as_ref().unwrap().input.clone()
+  pub fn key_image_generator(&self) -> EdwardsPoint {
-  }
+    self.key_image_generator
-
-  fn mask(&self) -> Scalar {
-    (*self.details.read().unwrap()).as_ref().unwrap().mask
   }
 }
 
 impl Algorithm<Ed25519> 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<dfg::EdwardsPoint>> {
-    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<R: RngCore + CryptoRng>(
     &mut self,
     _rng: &mut R,
     keys: &ThresholdKeys<Ed25519>,
   ) -> 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<R: Read>(&self, reader: &mut R) -> io::Result<ClsagAddendum> {
@@ -162,7 +166,7 @@ impl Algorithm<Ed25519> 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<Ed25519> 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::<dfg::Scalar>(l, view.included());
+      addendum.key_image_share * lagrange::<dfg::Scalar>(l, view.included());
     *self.image.as_mut().unwrap() += interpolated_key_image_share;
 
     self
@@ -210,19 +220,22 @@ impl Algorithm<Ed25519> 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
+    // process even if they have access to the commitments/key image share broadcast so far
-    // for, along with the mask which should not only require knowing the shared keys yet also the
+    //
-    // input commitment masks)
+    // 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.context,
-      self.mask(),
+      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<Ed25519> for ClsagMultisig {
   ) -> Option<Self::Signature> {
     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<Ed25519> 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<Ed25519> 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),

coins/monero/src/tests/clsag.rs

@@ -1,6 +1,5 @@
 use core::ops::Deref;
-#[cfg(feature = "multisig")]
+use std::sync::{Arc, Mutex};
-use std::sync::{Arc, RwLock};
 
 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,
+        secrets.0.clone(),
-        image,
+        ClsagContext::new(
-        ClsagInput::new(
-          Commitment::new(secrets.1, AMOUNT),
           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,
+    ClsagContext::new(
-    Arc::new(RwLock::new(Some(ClsagDetails::new(
-      ClsagInput::new(
-        Commitment::new(randomness, AMOUNT),
       Decoys::new((1 ..= RING_LEN).collect(), RING_INDEX, ring.clone()).unwrap(),
+      Commitment::new(randomness, AMOUNT),
     )
     .unwrap(),
-      mask_sum,
+    Arc::new(Mutex::new(Some(Scalar::random(&mut OsRng)))),
-    )))),
   );
 
   sign(

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<Scalar>,
   tx: &mut Transaction,
-) -> Result<Vec<(Zeroizing<Scalar>, EdwardsPoint, ClsagInput)>, TransactionError> {
+) -> Result<Vec<(Zeroizing<Scalar>, 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,
+      ClsagContext::new(decoys.clone(), input.commitment().clone())
-      ClsagInput::new(input.commitment().clone(), decoys.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());
+  // We now need to sort the inputs by their key image
-  tx.prefix.inputs.sort_by(|x, y| {
+  // 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::<Vec<_>>();
+  // 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, .. } => {

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<Arc<RwLock<Option<ClsagDetails>>>>,
+  clsag_mask_mutexes: Vec<Arc<Mutex<Option<Scalar>>>>,
   clsags: Vec<AlgorithmMachine<Ed25519, ClsagMultisig>>,
 }
 
@@ -54,7 +54,7 @@ pub struct TransactionSignMachine {
   transcript: RecommendedTranscript,
 
   key_images: Vec<(EdwardsPoint, Scalar)>,
-  inputs: Vec<Arc<RwLock<Option<ClsagDetails>>>>,
+  clsag_mask_mutexes: Vec<Arc<Mutex<Option<Scalar>>>>,
   clsags: Vec<AlgorithmSignMachine<Ed25519, ClsagMultisig>>,
 
   our_preprocess: Vec<Preprocess<Ed25519, ClsagAddendum>>,
@@ -73,10 +73,10 @@ impl SignableTransaction {
     keys: &ThresholdKeys<Ed25519>,
     mut transcript: RecommendedTranscript,
   ) -> Result<TransactionMachine, TransactionError> {
-    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<Transaction> 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::<dfg::Scalar>(*l, &included).0;
+            images[c] +=
+              preprocess.addendum.key_image_share().0 * lagrange::<dfg::Scalar>(*l, &included).0;
 
             Ok((*l, preprocess))
           })
@@ -330,12 +331,10 @@ impl SignMachine<Transaction> 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,
+        self.signable.inputs.swap_remove(0).1,
-        decoys,
+        self.clsag_mask_mutexes.swap_remove(0),
-        self.inputs.swap_remove(0),
         self.clsags.swap_remove(0),
         commitments.swap_remove(0),
       ));
@@ -353,22 +352,16 @@ impl SignMachine<Transaction> 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(
+      self.clsags.push(value.3);
-        ClsagInput::new(value.1.commitment().clone(), value.2).map_err(|_| {
+      commitments.push(value.4);
-          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);
     }
 
     let msg = tx.signature_hash();