Support signing Monero TXs with multiple inputs

Remove's CLSAG's msg Rc for the msg available through AlgorithmMachine. 
Potentially slightly more inefficient, as it needs to be converted from 
a slice to a [u8; 32], yet removes a re-impl.

Also removes a match for an if.

coins/monero/src/clsag/multisig.rs

@@ -85,16 +85,15 @@ pub struct Multisig {
   AH: (dfg::EdwardsPoint, dfg::EdwardsPoint),
 
   details: Rc<RefCell<Option<Details>>>,
-  msg: Rc<RefCell<Option<[u8; 32]>>>,
 
+  msg: Option<[u8; 32]>,
   interim: Option<Interim>
 }
 
 impl Multisig {
   pub fn new(
     transcript: Transcript,
-    details: Rc<RefCell<Option<Details>>>,
+    details: Rc<RefCell<Option<Details>>>
-    msg: Rc<RefCell<Option<[u8; 32]>>>,
   ) -> Result<Multisig, MultisigError> {
     Ok(
       Multisig {
@@ -105,8 +104,8 @@ impl Multisig {
         AH: (dfg::EdwardsPoint::identity(), dfg::EdwardsPoint::identity()),
 
         details,
-        msg,
 
+        msg: None,
         interim: None
       }
     )
@@ -123,10 +122,6 @@ impl Multisig {
   fn mask(&self) -> Scalar {
     self.details.borrow().as_ref().unwrap().mask
   }
-
-  fn msg(&self) -> [u8; 32] {
-    *self.msg.borrow().as_ref().unwrap()
-  }
 }
 
 impl Algorithm<Ed25519> for Multisig {
@@ -168,7 +163,6 @@ impl Algorithm<Ed25519> for Multisig {
       self.transcript.domain_separate(b"CLSAG");
       self.input().transcript(&mut self.transcript);
       self.transcript.append_message(b"mask", &self.mask().to_bytes());
-      self.transcript.append_message(b"message", &self.msg());
     }
 
     let share = read_dleq(
@@ -208,7 +202,7 @@ impl Algorithm<Ed25519> for Multisig {
     nonce_sum: dfg::EdwardsPoint,
     b: dfg::Scalar,
     nonce: dfg::Scalar,
-    _: &[u8]
+    msg: &[u8]
   ) -> dfg::Scalar {
     // Apply the binding factor to the H variant of the nonce
     self.AH.0 += self.AH.1 * b;
@@ -220,13 +214,15 @@ impl Algorithm<Ed25519> for Multisig {
     // input commitment masks)
     let mut rng = ChaCha12Rng::from_seed(self.transcript.rng_seed(b"decoy_responses", None));
 
+    self.msg = Some(msg.try_into().expect("CLSAG message should be 32-bytes"));
+
     #[allow(non_snake_case)]
     let (clsag, pseudo_out, p, c) = sign_core(
       &mut rng,
       &self.image,
       &self.input(),
       self.mask(),
-      &self.msg(),
+      &self.msg.as_ref().unwrap(),
       nonce_sum.0,
       self.AH.0.0
     );
@@ -246,7 +242,13 @@ impl Algorithm<Ed25519> for Multisig {
     let interim = self.interim.as_ref().unwrap();
     let mut clsag = interim.clsag.clone();
     clsag.s[usize::from(self.input().decoys.i)] = Key { key: (sum.0 - interim.c).to_bytes() };
-    if verify(&clsag, &self.input().decoys.ring, &self.image, &interim.pseudo_out, &self.msg()).is_ok() {
+    if verify(
+      &clsag,
+      &self.input().decoys.ring,
+      &self.image,
+      &interim.pseudo_out,
+      &self.msg.as_ref().unwrap()
+    ).is_ok() {
       return Some((clsag, interim.pseudo_out));
     }
     return None;

coins/monero/src/transaction/mod.rs

@@ -95,11 +95,9 @@ pub fn scan(tx: &Transaction, view: Scalar, spend: EdwardsPoint) -> Vec<Spendabl
 
   let mut res = vec![];
   for o in 0 .. tx.prefix.outputs.len() {
-    let output_key = match tx.prefix.outputs[o].target {
+    let output_key = if let TxOutTarget::ToKey { key } = tx.prefix.outputs[o].target {
-      TxOutTarget::ToScript { .. } => None,
+      key.point.decompress()
-      TxOutTarget::ToScriptHash { .. } => None,
+    } else { None };
-      TxOutTarget::ToKey { key } => key.point.decompress()
-    };
     if output_key.is_none() {
       continue;
     }
@@ -160,6 +158,7 @@ fn amount_encryption(amount: u64, key: Scalar) -> Hash8 {
 }
 
 #[allow(non_snake_case)]
+#[derive(Clone, Debug)]
 struct Output {
   R: EdwardsPoint,
   dest: EdwardsPoint,
@@ -200,8 +199,6 @@ async fn prepare_inputs<R: RngCore + CryptoRng>(
   spend: &Scalar,
   tx: &mut Transaction
 ) -> Result<Vec<(Scalar, EdwardsPoint, clsag::Input)>, TransactionError> {
-  // TODO sort inputs
-
   let mut signable = Vec::with_capacity(inputs.len());
 
   // Select decoys
@@ -229,9 +226,20 @@ async fn prepare_inputs<R: RngCore + CryptoRng>(
     });
   }
 
+  signable.sort_by(|x, y| x.1.compress().to_bytes().cmp(&y.1.compress().to_bytes()).reverse());
+  tx.prefix.inputs.sort_by(|x, y| if let (
+    TxIn::ToKey{ k_image: x, ..},
+    TxIn::ToKey{ k_image: y, ..}
+  ) = (x, y) {
+    x.image.cmp(&y.image).reverse()
+  } else {
+    panic!("TxIn wasn't ToKey")
+  });
+
   Ok(signable)
 }
 
+#[derive(Clone, Debug)]
 pub struct SignableTransaction {
   inputs: Vec<SpendableOutput>,
   payments: Vec<(Address, u64)>,

coins/monero/src/transaction/multisig.rs

@@ -29,10 +29,9 @@ pub struct TransactionMachine {
 
   decoys: Vec<Decoys>,
 
-  our_images: Vec<EdwardsPoint>,
+  images: Vec<EdwardsPoint>,
   output_masks: Option<Scalar>,
   inputs: Vec<Rc<RefCell<Option<clsag::Details>>>>,
-  msg: Rc<RefCell<Option<[u8; 32]>>>,
   clsags: Vec<AlgorithmMachine<Ed25519, clsag::Multisig>>,
 
   tx: Option<Transaction>
@@ -45,14 +44,16 @@ impl SignableTransaction {
     rng: &mut R,
     rpc: &Rpc,
     height: usize,
-    keys: Rc<MultisigKeys<Ed25519>>,
+    keys: MultisigKeys<Ed25519>,
     included: &[usize]
   ) -> Result<TransactionMachine, TransactionError> {
-    let mut our_images = vec![];
+    let mut images = vec![];
-    our_images.resize(self.inputs.len(), EdwardsPoint::identity());
+    images.resize(self.inputs.len(), EdwardsPoint::identity());
     let mut inputs = vec![];
-    inputs.resize(self.inputs.len(), Rc::new(RefCell::new(None)));
+    for _ in 0 .. self.inputs.len() {
-    let msg = Rc::new(RefCell::new(None));
+      // Doesn't resize as that will use a single Rc for the entire Vec
+      inputs.push(Rc::new(RefCell::new(None)));
+    }
     let mut clsags = vec![];
 
     // Create a RNG out of the input shared keys, which either requires the view key or being every
@@ -96,8 +97,7 @@ impl SignableTransaction {
         AlgorithmMachine::new(
           clsag::Multisig::new(
             transcript.clone(),
-            inputs[i].clone(),
+            inputs[i].clone()
-            msg.clone()
           ).map_err(|e| TransactionError::MultisigError(e))?,
           Rc::new(keys.offset(dalek_ff_group::Scalar(input.key_offset))),
           included
@@ -115,10 +115,9 @@ impl SignableTransaction {
 
       decoys,
 
-      our_images,
+      images,
       output_masks: None,
       inputs,
-      msg,
       clsags,
 
       tx: None
@@ -142,7 +141,7 @@ impl StateMachine for TransactionMachine {
     for (i, clsag) in self.clsags.iter_mut().enumerate() {
       let preprocess = clsag.preprocess(rng)?;
       // First 64 bytes are FROST's commitments
-      self.our_images[i] += CompressedEdwardsY(preprocess[64 .. 96].try_into().unwrap()).decompress().unwrap();
+      self.images[i] += CompressedEdwardsY(preprocess[64 .. 96].try_into().unwrap()).decompress().unwrap();
       serialized.extend(&preprocess);
     }
 
@@ -209,63 +208,79 @@ impl StateMachine for TransactionMachine {
     }
 
     let mut rng = ChaCha12Rng::from_seed(self.transcript.rng_seed(b"pseudo_out_masks", None));
-    let mut sum_pseudo_outs = Scalar::zero();
     for c in 0 .. self.clsags.len() {
       // Calculate the key images in order to update the TX
       // Multisig will parse/calculate/validate this as needed, yet doing so here as well provides
       // the easiest API overall
-      let mut image = self.our_images[c];
       for (l, serialized) in commitments.iter().enumerate().filter(|(_, s)| s.is_some()) {
-        image += CompressedEdwardsY(
+        self.images[c] += CompressedEdwardsY(
           serialized.as_ref().unwrap()[((c * clsag_len) + 64) .. ((c * clsag_len) + 96)]
             .try_into().map_err(|_| FrostError::InvalidCommitment(l))?
         ).decompress().ok_or(FrostError::InvalidCommitment(l))?;
       }
+    }
 
-      // TODO sort inputs
+    let mut commitments = (0 .. self.inputs.len()).map(|c| commitments.iter().map(
+      |commitments| commitments.clone().map(
+        |commitments| commitments[(c * clsag_len) .. ((c * clsag_len) + clsag_len)].to_vec()
+      )
+    ).collect::<Vec<_>>()).collect::<Vec<_>>();
+
+    let mut sorted = Vec::with_capacity(self.decoys.len());
+    while self.decoys.len() != 0 {
+      sorted.push((
+        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());
+
+    let mut sum_pseudo_outs = Scalar::zero();
+    while sorted.len() != 0 {
+      let value = sorted.remove(0);
 
       let mut mask = random_scalar(&mut rng);
-      if c == (self.clsags.len() - 1) {
+      if sorted.len() == 0 {
         mask = self.output_masks.unwrap() - sum_pseudo_outs;
       } else {
         sum_pseudo_outs += mask;
       }
 
-      self.inputs[c].replace(
+      tx.prefix.inputs.push(
+        TxIn::ToKey {
+          amount: VarInt(0),
+          key_offsets: value.1.offsets.clone(),
+          k_image: KeyImage { image: Hash(value.2.compress().to_bytes()) }
+        }
+      );
+
+      value.3.replace(
         Some(
           clsag::Details::new(
             clsag::Input::new(
-              self.signable.inputs[c].commitment,
+              value.0.commitment,
-              self.decoys[c].clone()
+              value.1
             ).map_err(|_| panic!("Signing an input which isn't present in the ring we created for it"))?,
             mask
           )
         )
       );
 
-      tx.prefix.inputs.push(
+      self.clsags.push(value.4);
-        TxIn::ToKey {
+      commitments.push(value.5);
-          amount: VarInt(0),
-          key_offsets: self.decoys[c].offsets.clone(),
-          k_image: KeyImage { image: Hash(image.compress().to_bytes()) }
-        }
-      );
     }
 
-    self.msg.replace(Some(tx.signature_hash().unwrap().0));
+    let msg = tx.signature_hash().unwrap().0;
     self.tx = Some(tx);
 
     // Iterate over each CLSAG calling sign
     let mut serialized = Vec::with_capacity(self.clsags.len() * 32);
     for (c, clsag) in self.clsags.iter_mut().enumerate() {
-      serialized.extend(&clsag.sign(
+      serialized.extend(&clsag.sign(&commitments[c], &msg)?);
-        &commitments.iter().map(
-          |commitments| commitments.clone().map(
-            |commitments| commitments[(c * clsag_len) .. ((c * clsag_len) + clsag_len)].to_vec()
-          )
-        ).collect::<Vec<_>>(),
-        &vec![]
-      )?);
     }
 
     Ok(serialized)

coins/monero/tests/clsag.rs

@@ -109,16 +109,15 @@ fn clsag_multisig() -> Result<(), MultisigError> {
               ).unwrap(),
               mask_sum
             )
-          ))),
+          )))
-          Rc::new(RefCell::new(Some([1; 32])))
         ).unwrap(),
-        keys[i - 1].clone(),
+        Rc::new(keys[i - 1].clone()),
         &(1 ..= THRESHOLD).collect::<Vec<usize>>()
       ).unwrap()
     );
   }
 
-  let mut signatures = sign(&mut machines, keys);
+  let mut signatures = sign(&mut machines, &[1; 32]);
   let signature = signatures.swap_remove(0);
   for s in 0 .. (t - 1) {
     // Verify the commitments and the non-decoy s scalar are identical to every other signature

coins/monero/tests/frost.rs

@@ -1,7 +1,5 @@
 #![cfg(feature = "multisig")]
 
-use std::rc::Rc;
-
 use rand::rngs::OsRng;
 
 use ff::Field;
@@ -17,7 +15,7 @@ use monero_serai::frost::Ed25519;
 pub const THRESHOLD: usize = 3;
 pub const PARTICIPANTS: usize = 5;
 
-pub fn generate_keys() -> (Vec<Rc<MultisigKeys<Ed25519>>>, Scalar) {
+pub fn generate_keys() -> (Vec<MultisigKeys<Ed25519>>, Scalar) {
   let mut params = vec![];
   let mut machines = vec![];
   let mut commitments = vec![vec![]];
@@ -54,7 +52,7 @@ pub fn generate_keys() -> (Vec<Rc<MultisigKeys<Ed25519>>>, Scalar) {
     our_secret_shares.extend(
       secret_shares.iter().map(|shares| shares[i].clone()).collect::<Vec<Vec<u8>>>()
     );
-    keys.push(Rc::new(machines[i - 1].complete(our_secret_shares).unwrap().clone()));
+    keys.push(machines[i - 1].complete(our_secret_shares).unwrap().clone());
   }
 
   let mut group_private = Scalar::zero();
@@ -70,12 +68,8 @@ pub fn generate_keys() -> (Vec<Rc<MultisigKeys<Ed25519>>>, Scalar) {
 }
 
 #[allow(dead_code)] // Currently has some false positive
-pub fn sign<S, M: sign::StateMachine<Signature = S>>(
+pub fn sign<S, M: sign::StateMachine<Signature = S>>(machines: &mut Vec<M>, msg: &[u8]) -> Vec<S> {
-  machines: &mut Vec<M>,
-  keys: Vec<Rc<MultisigKeys<Ed25519>>>
-) -> Vec<S> {
   assert!(machines.len() >= THRESHOLD);
-  assert!(keys.len() >= machines.len());
 
   let mut commitments = Vec::with_capacity(PARTICIPANTS + 1);
   commitments.resize(PARTICIPANTS + 1, None);
@@ -93,7 +87,7 @@ pub fn sign<S, M: sign::StateMachine<Signature = S>>(
           .enumerate()
           .map(|(idx, value)| if idx == i { None } else { value.to_owned() })
           .collect::<Vec<Option<Vec<u8>>>>(),
-        &vec![]
+        msg
       ).unwrap()
     );
   }

coins/monero/tests/send.rs

@@ -1,25 +1,62 @@
+use std::sync::Mutex;
+
+use lazy_static::lazy_static;
+
 use rand::rngs::OsRng;
 
+#[cfg(feature = "multisig")]
+use blake2::{digest::Update, Digest, Blake2b512};
+
 use curve25519_dalek::constants::ED25519_BASEPOINT_TABLE;
+#[cfg(feature = "multisig")]
+use dalek_ff_group::Scalar;
 
 use monero::{
   network::Network,
   util::{key::PublicKey, address::Address}
 };
+#[cfg(feature = "multisig")]
+use monero::cryptonote::hash::Hashable;
 
 use monero_serai::{random_scalar, transaction::{self, SignableTransaction}};
 
 mod rpc;
 use crate::rpc::{rpc, mine_block};
 
-#[tokio::test]
+#[cfg(feature = "multisig")]
-pub async fn send() {
+mod frost;
+#[cfg(feature = "multisig")]
+use crate::frost::{THRESHOLD, generate_keys, sign};
+
+lazy_static! {
+  static ref SEQUENTIAL: Mutex<()> = Mutex::new(());
+}
+
+pub async fn send_core(test: usize, multisig: bool) {
+  let _guard = SEQUENTIAL.lock().unwrap();
   let rpc = rpc().await;
 
   // Generate an address
-  let view = random_scalar(&mut OsRng);
   let spend = random_scalar(&mut OsRng);
-  let spend_pub = &spend * &ED25519_BASEPOINT_TABLE;
+  #[allow(unused_mut)]
+  let mut view = random_scalar(&mut OsRng);
+  #[allow(unused_mut)]
+  let mut spend_pub = &spend * &ED25519_BASEPOINT_TABLE;
+
+  #[cfg(feature = "multisig")]
+  let (keys, _) = generate_keys();
+  #[cfg(feature = "multisig")]
+  let t = keys[0].params().t();
+
+  if multisig {
+    #[cfg(not(feature = "multisig"))]
+    panic!("Running a multisig test without the multisig feature");
+    #[cfg(feature = "multisig")]
+    {
+      view = Scalar::from_hash(Blake2b512::new().chain("Monero Serai Transaction Test")).0;
+      spend_pub = keys[0].group_key().0;
+    }
+  }
 
   let addr = Address::standard(
     Network::Mainnet,
@@ -27,26 +64,99 @@ pub async fn send() {
     PublicKey { point: (&view * &ED25519_BASEPOINT_TABLE).compress() }
   );
 
+  // TODO
   let fee_per_byte = 50000000;
   let fee = fee_per_byte * 2000;
 
-  let mut tx;
+  let start = rpc.get_height().await.unwrap();
-  let mut output;
+  for _ in 0 .. 7 {
-  let mut amount;
+    mine_block(&rpc, &addr.to_string()).await.unwrap();
-  for i in 0 .. 2 {
+  }
-    let start = rpc.get_height().await.unwrap();
+
-    for _ in 0 .. 7 {
+  let mut tx = None;
-      mine_block(&rpc, &addr.to_string()).await.unwrap();
+  // Allow tests to test variable transactions
+  for i in 0 .. [2, 1][test] {
+    let mut outputs = vec![];
+    let mut amount = 0;
+    // Test spending both a miner output and a normal output
+    if test == 0 {
+      if i == 0 {
+        tx = Some(rpc.get_block_transactions(start).await.unwrap().swap_remove(0));
+      }
+
+      let output = transaction::scan(tx.as_ref().unwrap(), view, spend_pub).swap_remove(0);
+      // Test creating a zero change output and a non-zero change output
+      amount = output.commitment.amount - u64::try_from(i).unwrap();
+      outputs.push(output);
+
+    // Test spending multiple inputs
+    } else if test == 1 {
+      if i != 0 {
+        continue;
+      }
+
+      for i in (start + 1) .. (start + 9) {
+        let tx = rpc.get_block_transactions(i).await.unwrap().swap_remove(0);
+        let output = transaction::scan(&tx, view, spend_pub).swap_remove(0);
+        amount += output.commitment.amount;
+        outputs.push(output);
+      }
     }
 
-    // Test both a miner output and a normal output
+    let mut signable = SignableTransaction::new(
-    tx = rpc.get_block_transactions(start).await.unwrap().swap_remove(i);
+      outputs, vec![(addr, amount - fee)], addr, fee_per_byte
-    output = transaction::scan(&tx, view, spend_pub).swap_remove(0);
+    ).unwrap();
-    // Test creating a zero change output and a non-zero change output
+
-    amount = output.commitment.amount - fee - u64::try_from(i).unwrap();
+    if !multisig {
-    let tx = SignableTransaction::new(
+      tx = Some(signable.sign(&mut OsRng, &rpc, &spend).await.unwrap());
-      vec![output], vec![(addr, amount)], addr, fee_per_byte
+    } else {
-    ).unwrap().sign(&mut OsRng, &rpc, &spend).await.unwrap();
+      #[cfg(feature = "multisig")]
-    rpc.publish_transaction(&tx).await.unwrap();
+      {
+        let mut machines = Vec::with_capacity(t);
+        for i in 1 ..= t {
+          machines.push(
+            signable.clone().multisig(
+              b"Monero Serai Test Transaction".to_vec(),
+              &mut OsRng,
+              &rpc,
+              rpc.get_height().await.unwrap() - 10,
+              keys[i - 1].clone(),
+              &(1 ..= THRESHOLD).collect::<Vec<usize>>()
+            ).await.unwrap()
+          );
+        }
+
+        let mut txs = sign(&mut machines, &vec![]);
+        for s in 0 .. (t - 1) {
+          assert_eq!(txs[s].hash(), txs[0].hash());
+        }
+        tx = Some(txs.swap_remove(0));
+      }
+    }
+
+    rpc.publish_transaction(tx.as_ref().unwrap()).await.unwrap();
+    mine_block(&rpc, &addr.to_string()).await.unwrap();
   }
 }
+
+#[tokio::test]
+pub async fn send_single_input() {
+  send_core(0, false).await;
+}
+
+#[tokio::test]
+pub async fn send_multiple_inputs() {
+  send_core(1, false).await;
+}
+
+#[cfg(feature = "multisig")]
+#[tokio::test]
+pub async fn multisig_send_single_input() {
+  send_core(0, true).await;
+}
+
+#[cfg(feature = "multisig")]
+#[tokio::test]
+pub async fn multisig_send_multiple_inputs() {
+  send_core(1, true).await;
+}

coins/monero/tests/send_multisig.rs

@@ -1,75 +0,0 @@
-#![cfg(feature = "multisig")]
-
-use rand::rngs::OsRng;
-
-use blake2::{digest::Update, Digest, Blake2b512};
-
-use curve25519_dalek::constants::ED25519_BASEPOINT_TABLE;
-use dalek_ff_group::Scalar;
-
-use monero::{
-  cryptonote::hash::Hashable,
-  network::Network,
-  util::{key::PublicKey, address::Address}
-};
-
-use monero_serai::transaction::{self, SignableTransaction};
-
-mod rpc;
-use crate::rpc::{rpc, mine_block};
-
-mod frost;
-use crate::frost::{THRESHOLD, generate_keys, sign};
-
-#[tokio::test]
-pub async fn send_multisig() {
-  let rpc = rpc().await;
-
-  let fee_per_byte = 50000000;
-  let fee = fee_per_byte * 2000;
-
-  let (keys, _) = generate_keys();
-  let t = keys[0].params().t();
-
-  // Generate an address
-  let view = Scalar::from_hash(Blake2b512::new().chain("Monero Serai Transaction Test")).0;
-  let spend = keys[0].group_key().0;
-  let addr = Address::standard(
-    Network::Mainnet,
-    PublicKey { point: spend.compress() },
-    PublicKey { point: (&view * &ED25519_BASEPOINT_TABLE).compress() }
-  );
-
-  // Mine blocks to that address
-  let start = rpc.get_height().await.unwrap();
-  for _ in 0 .. 7 {
-    mine_block(&rpc, &addr.to_string()).await.unwrap();
-  }
-
-  // Get the input TX
-  let tx = rpc.get_block_transactions(start).await.unwrap().swap_remove(0);
-  let output = transaction::scan(&tx, view, spend).swap_remove(0);
-  let amount = output.commitment.amount - fee;
-
-  let mut machines = Vec::with_capacity(t);
-  for i in 1 ..= t {
-    machines.push(
-      SignableTransaction::new(
-        vec![output.clone()], vec![(addr, amount)], addr, fee_per_byte
-      ).unwrap().multisig(
-        b"Monero Serai Test Transaction".to_vec(),
-        &mut OsRng,
-        &rpc,
-        rpc.get_height().await.unwrap() - 10,
-        keys[i - 1].clone(),
-        &(1 ..= THRESHOLD).collect::<Vec<usize>>()
-      ).await.unwrap()
-    );
-  }
-
-  let txs = sign(&mut machines, keys);
-  for s in 0 .. (t - 1) {
-    assert_eq!(txs[s].hash(), txs[0].hash());
-  }
-  rpc.publish_transaction(&txs[0]).await.unwrap();
-}