Various corrections to multisig API

coins/monero/src/clsag/mod.rs

@@ -71,13 +71,17 @@ impl Input {
 
   #[cfg(feature = "multisig")]
   pub fn context(&self) -> Vec<u8> {
+    // image is extraneous in practice as the image should be in the msg AND the addendum when TX
+    // signing. This just ensures CLSAG guarantees its integrity, even when others won't
     let mut context = self.image.compress().to_bytes().to_vec();
+    // Ring index
+    context.extend(&u8::try_from(self.i).unwrap().to_le_bytes());
+    // Ring
     for pair in &self.ring {
-      // Doesn't include mixins[i] as CLSAG doesn't care and won't be affected by it
+      // Doesn't include key offsets as CLSAG doesn't care and won't be affected by it
       context.extend(&pair[0].compress().to_bytes());
       context.extend(&pair[1].compress().to_bytes());
     }
-    context.extend(&u8::try_from(self.i).unwrap().to_le_bytes());
     // Doesn't include commitment as the above ring + index includes the commitment
     context
   }

coins/monero/src/clsag/multisig.rs

@@ -35,51 +35,45 @@ struct ClsagSignInterim {
 #[allow(non_snake_case)]
 #[derive(Clone, Debug)]
 pub struct Multisig {
-  seed: [u8; 32],
   b: Vec<u8>,
-  AH: dfg::EdwardsPoint,
+  AH0: dfg::EdwardsPoint,
+  AH1: dfg::EdwardsPoint,
 
-  msg: [u8; 32],
   input: Input,
 
+  msg: Option<[u8; 32]>,
   interim: Option<ClsagSignInterim>
 }
 
 impl Multisig {
-  pub fn new<R: RngCore + CryptoRng + SeedableRng>(
+  pub fn new(
-    rng: &mut R,
-    msg: [u8; 32],
     input: Input
   ) -> Result<Multisig, MultisigError> {
-    let mut seed = [0; 32];
-    rng.fill_bytes(&mut seed);
-
     Ok(
       Multisig {
-        seed,
         b: vec![],
-        AH: dfg::EdwardsPoint::identity(),
+        AH0: dfg::EdwardsPoint::identity(),
+        AH1: dfg::EdwardsPoint::identity(),
 
-        msg,
         input,
 
+        msg: None,
         interim: None
       }
     )
   }
+
+  pub fn set_msg(
+    &mut self,
+    msg: [u8; 32]
+  ) {
+    self.msg = Some(msg);
+  }
 }
 
 impl Algorithm<Ed25519> for Multisig {
   type Signature = (Clsag, EdwardsPoint);
 
-  fn context(&self) -> Vec<u8> {
-    let mut context = vec![];
-    context.extend(&self.seed);
-    context.extend(&self.msg);
-    context.extend(&self.input.context());
-    context
-  }
-
   // We arguably don't have to commit to at all thanks to xG and yG being committed to, both of
   // those being proven to have the same scalar as xH and yH, yet it doesn't hurt
   fn addendum_commit_len() -> usize {
@@ -95,8 +89,7 @@ impl Algorithm<Ed25519> for Multisig {
     let H = hash_to_point(&view.group_key().0);
     let h0 = nonces[0].0 * H;
     let h1 = nonces[1].0 * H;
-    // 32 + 32 + 64 + 64
+    let mut serialized = Vec::with_capacity(32 + 32 + 64 + 64);
-    let mut serialized = Vec::with_capacity(192);
     serialized.extend(h0.compress().to_bytes());
     serialized.extend(h1.compress().to_bytes());
     serialized.extend(&DLEqProof::prove(rng, &nonces[0].0, &H, &h0).serialize());
@@ -109,7 +102,6 @@ impl Algorithm<Ed25519> for Multisig {
     _: &ParamsView<Ed25519>,
     l: usize,
     commitments: &[dfg::EdwardsPoint; 2],
-    p: &dfg::Scalar,
     serialized: &[u8]
   ) -> Result<(), FrostError> {
     if serialized.len() != 192 {
@@ -121,6 +113,7 @@ impl Algorithm<Ed25519> for Multisig {
 
     let h0 = <Ed25519 as Curve>::G_from_slice(&serialized[0 .. 32]).map_err(|_| FrostError::InvalidCommitment(l))?;
     DLEqProof::deserialize(&serialized[64 .. 128]).ok_or(FrostError::InvalidCommitment(l))?.verify(
+      l,
       &alt,
       &commitments[0],
       &h0
@@ -128,6 +121,7 @@ impl Algorithm<Ed25519> for Multisig {
 
     let h1 = <Ed25519 as Curve>::G_from_slice(&serialized[32 .. 64]).map_err(|_| FrostError::InvalidCommitment(l))?;
     DLEqProof::deserialize(&serialized[128 .. 192]).ok_or(FrostError::InvalidCommitment(l))?.verify(
+      l,
       &alt,
       &commitments[1],
       &h1
@@ -135,11 +129,26 @@ impl Algorithm<Ed25519> for Multisig {
 
     self.b.extend(&l.to_le_bytes());
     self.b.extend(&serialized[0 .. 64]);
-    self.AH += h0 + (h1 * p);
+    self.AH0 += h0;
+    self.AH1 += h1;
 
     Ok(())
   }
 
+  fn context(&self) -> Vec<u8> {
+    let mut context = vec![];
+    context.extend(&self.msg.unwrap());
+    context.extend(&self.input.context());
+    context
+  }
+
+  fn process_binding(
+    &mut self,
+    p: &dfg::Scalar,
+  ) {
+    self.AH0 += self.AH1 * p;
+  }
+
   fn sign_share(
     &mut self,
     view: &ParamsView<Ed25519>,
@@ -149,7 +158,9 @@ impl Algorithm<Ed25519> for Multisig {
   ) -> dfg::Scalar {
     // Use everyone's commitments to derive a random source all signers can agree upon
     // Cannot be manipulated to effect and all signers must, and will, know this
-    // Uses a parent seed (part of context) as well just to enable further privacy options
+    // Uses the context as well to prevent passive observers of messages from being able to break
+    // privacy, as the context includes the index of the output in the ring, which can only be
+    // known if you have the view key and know which of the wallet's TXOs is being spent
     let mut seed = b"CLSAG_randomness".to_vec();
     seed.extend(&self.context());
     seed.extend(&self.b);
@@ -159,11 +170,11 @@ impl Algorithm<Ed25519> for Multisig {
     #[allow(non_snake_case)]
     let (clsag, c, mu_C, z, mu_P, C_out) = sign_core(
       &mut rng,
-      &self.msg,
+      &self.msg.unwrap(),
       &self.input,
       mask,
       nonce_sum.0,
-      self.AH.0
+      self.AH0.0
     );
     self.interim = Some(ClsagSignInterim { c: c * mu_P, s: c * mu_C * z, clsag, C_out });
 
@@ -182,7 +193,7 @@ impl Algorithm<Ed25519> for Multisig {
 
     let mut clsag = interim.clsag.clone();
     clsag.s[self.input.i] = Key { key: (sum.0 - interim.s).to_bytes() };
-    if verify(&clsag, &self.msg, self.input.image, &self.input.ring, interim.C_out) {
+    if verify(&clsag, &self.msg.unwrap(), self.input.image, &self.input.ring, interim.C_out) {
       return Some((clsag, interim.C_out));
     }
     return None;

coins/monero/src/frost.rs

@@ -26,8 +26,8 @@ use crate::random_scalar;
 pub enum MultisigError {
   #[error("internal error ({0})")]
   InternalError(String),
-  #[error("invalid discrete log equality proof")]
+  #[error("invalid discrete log equality proof {0}")]
-  InvalidDLEqProof,
+  InvalidDLEqProof(usize),
   #[error("invalid key image {0}")]
   InvalidKeyImage(usize)
 }
@@ -145,6 +145,7 @@ impl DLEqProof {
 
   pub fn verify(
     &self,
+    l: usize,
     H: &DPoint,
     primary: &DPoint,
     alt: &DPoint
@@ -165,7 +166,7 @@ impl DLEqProof {
 
     // Take the opportunity to ensure a lack of torsion in key images/randomness commitments
     if (!primary.is_torsion_free()) || (!alt.is_torsion_free()) || (c != expected_c) {
-      Err(MultisigError::InvalidDLEqProof)?;
+      Err(MultisigError::InvalidDLEqProof(l))?;
     }
 
     Ok(())

coins/monero/src/key_image/mod.rs

@@ -9,7 +9,7 @@ use crate::hash_to_point;
 #[cfg(feature = "multisig")]
 mod multisig;
 #[cfg(feature = "multisig")]
-pub use crate::key_image::multisig::{Package, multisig};
+pub use crate::key_image::multisig::{generate_share, verify_share};
 
 pub fn generate(secret: &Scalar) -> EdwardsPoint {
   secret * hash_to_point(&(secret * &ED25519_BASEPOINT_TABLE))

coins/monero/src/key_image/multisig.rs

@@ -1,34 +1,17 @@
 use rand_core::{RngCore, CryptoRng};
 
-use curve25519_dalek::edwards::EdwardsPoint;
+use curve25519_dalek::edwards::{EdwardsPoint, CompressedEdwardsY};
-use dalek_ff_group::Scalar;
+use frost::sign::ParamsView;
-use frost::{MultisigKeys, sign::lagrange};
 
 use crate::{hash_to_point, frost::{MultisigError, Ed25519, DLEqProof}};
 
-#[derive(Clone)]
 #[allow(non_snake_case)]
-pub struct Package {
+pub fn generate_share<R: RngCore + CryptoRng>(
-  // Don't serialize
-  H: EdwardsPoint,
-  i: usize,
-
-  // Serialize
-  image: EdwardsPoint,
-  proof: DLEqProof
-}
-
-#[allow(non_snake_case)]
-pub fn multisig<R: RngCore + CryptoRng>(
   rng: &mut R,
-  keys: &MultisigKeys<Ed25519>,
+  view: &ParamsView<Ed25519>
-  included: &[usize]
+) -> (Vec<u8>, Vec<u8>) {
-) -> Package {
+  let H = hash_to_point(&view.group_key().0);
-  let i = keys.params().i();
+  let image = view.secret_share().0 * H;
-  let secret = (keys.secret_share() * lagrange::<Scalar>(i, included)).0;
-
-  let H = hash_to_point(&keys.group_key().0);
-  let image = secret * H;
   // Includes a proof. Since:
   // sum(lagranged_secrets) = group_private
   // group_private * G = output_key
@@ -37,39 +20,32 @@ pub fn multisig<R: RngCore + CryptoRng>(
   // lagranged_secret * G is known. lagranged_secret * H is being sent
   // Any discrete log equality proof confirms the same secret was used,
   // forming a valid key_image share
-  Package { H, i, image, proof: DLEqProof::prove(rng, &secret, &H, &image) }
+  (
+    image.compress().to_bytes().to_vec(),
+    DLEqProof::prove(rng, &view.secret_share().0, &H, &image).serialize()
+  )
 }
 
-#[allow(non_snake_case)]
+pub fn verify_share(
-impl Package {
+  view: &ParamsView<Ed25519>,
-  pub fn resolve(
+  l: usize,
-    self,
+  share: &[u8]
-    shares: Vec<Option<(EdwardsPoint, Package)>>
+) -> Result<(EdwardsPoint, Vec<u8>), MultisigError> {
-  ) -> Result<EdwardsPoint, MultisigError> {
+  if share.len() < 96 {
-    let mut included = vec![self.i];
+    Err(MultisigError::InvalidDLEqProof(l))?;
-    for i in 1 .. shares.len() {
-      if shares[i].is_some() {
-        included.push(i);
-      }
   }
+  let image = CompressedEdwardsY(
+    share[0 .. 32].try_into().unwrap()
+  ).decompress().ok_or(MultisigError::InvalidKeyImage(l))?;
+  let proof = DLEqProof::deserialize(
+    &share[(share.len() - 64) .. share.len()]
+  ).ok_or(MultisigError::InvalidDLEqProof(l))?;
+  proof.verify(
+    l,
+    &hash_to_point(&view.group_key().0),
+    &view.verification_share(l),
+    &image
+  ).map_err(|_| MultisigError::InvalidKeyImage(l))?;
 
-    let mut image = self.image;
+  Ok((image, share[32 .. (share.len() - 64)].to_vec()))
-    for i in 0 .. shares.len() {
-      if shares[i].is_none() {
-        continue;
-      }
-
-      let (other, shares) = shares[i].as_ref().unwrap();
-      let other = other * lagrange::<Scalar>(i, &included).0;
-
-      // Verify their proof
-      let share = shares.image;
-      shares.proof.verify(&self.H, &other, &share).map_err(|_| MultisigError::InvalidKeyImage(i))?;
-
-      // Add their share to the image
-      image += share;
-    }
-
-    Ok(image)
-  }
 }

coins/monero/tests/clsag.rs

@@ -1,5 +1,4 @@
-use rand::{RngCore, SeedableRng, rngs::OsRng};
+use rand::{RngCore, rngs::OsRng};
-use rand_chacha::ChaCha12Rng;
 
 use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar};
 
@@ -8,7 +7,7 @@ use monero_serai::{random_scalar, Commitment, frost::MultisigError, key_image, c
 #[cfg(feature = "multisig")]
 mod frost;
 #[cfg(feature = "multisig")]
-use crate::frost::{THRESHOLD, PARTICIPANTS, generate_keys, sign};
+use crate::frost::{generate_keys, sign};
 
 const RING_INDEX: u8 = 3;
 const RING_LEN: u64 = 11;
@@ -57,24 +56,9 @@ fn test_multisig() -> Result<(), MultisigError> {
   let (keys, group_private) = generate_keys();
   let t = keys[0].params().t();
 
-  let mut images = vec![];
-  images.resize(PARTICIPANTS + 1, None);
-  let included = (1 ..= THRESHOLD).collect::<Vec<usize>>();
-  for i in &included {
-    let i = *i;
-    images[i] = Some(
-      (
-        keys[0].verification_shares()[i].0,
-        key_image::multisig(&mut OsRng, &keys[i - 1], &included)
-      )
-    );
-  }
-
   let msg = [1; 32];
 
-  images.push(None);
+  let image = key_image::generate(&group_private.0);
-  let ki_used = images.swap_remove(1).unwrap().1;
-  let image = ki_used.resolve(images).unwrap();
 
   let randomness = random_scalar(&mut OsRng);
   let mut ring = vec![];
@@ -95,14 +79,13 @@ fn test_multisig() -> Result<(), MultisigError> {
   }
 
   let mut algorithms = Vec::with_capacity(t);
-  for _ in 1 ..= t {
+  for i in 1 ..= t {
     algorithms.push(
       clsag::Multisig::new(
-        &mut ChaCha12Rng::seed_from_u64(1),
-        msg,
         clsag::Input::new(image, ring.clone(), RING_INDEX, Commitment::new(randomness, AMOUNT)).unwrap()
       ).unwrap()
     );
+    algorithms[i - 1].set_msg(msg);
   }
 
   let mut signatures = sign(algorithms, keys);

coins/monero/tests/frost.rs

@@ -2,13 +2,14 @@
 
 use std::rc::Rc;
 
+use rand_core::{RngCore, CryptoRng};
 use rand::rngs::OsRng;
 
 use ff::Field;
-use dalek_ff_group::{ED25519_BASEPOINT_TABLE, Scalar};
+use dalek_ff_group::{ED25519_BASEPOINT_TABLE, Scalar, EdwardsPoint};
 
 use frost::{
-  MultisigParams, MultisigKeys,
+  FrostError, MultisigParams, MultisigKeys,
   key_gen, algorithm::Algorithm, sign::{self, lagrange}
 };
 
@@ -17,6 +18,49 @@ use monero_serai::frost::Ed25519;
 pub const THRESHOLD: usize = 5;
 pub const PARTICIPANTS: usize = 8;
 
+#[derive(Clone)]
+pub struct DummyAlgorithm;
+impl Algorithm<Ed25519> for DummyAlgorithm {
+  type Signature = ();
+
+  fn addendum_commit_len() -> usize { unimplemented!() }
+
+  fn preprocess_addendum<R: RngCore + CryptoRng>(
+    _: &mut R,
+    _: &sign::ParamsView<Ed25519>,
+    _: &[Scalar; 2],
+  ) -> Vec<u8> { unimplemented!() }
+
+  fn process_addendum(
+    &mut self,
+    _: &sign::ParamsView<Ed25519>,
+    _: usize,
+    _: &[EdwardsPoint; 2],
+    _: &[u8],
+  ) -> Result<(), FrostError> { unimplemented!() }
+
+  fn context(&self) -> Vec<u8> { unimplemented!() }
+
+  fn process_binding(&mut self, _: &Scalar) { unimplemented!() }
+
+  fn sign_share(
+    &mut self,
+    _: &sign::ParamsView<Ed25519>,
+    _: EdwardsPoint,
+    _: Scalar,
+    _: &[u8],
+  ) -> Scalar { unimplemented!() }
+
+  fn verify(&self, _: EdwardsPoint, _: EdwardsPoint, _: Scalar) -> Option<Self::Signature> { unimplemented!() }
+
+  fn verify_share(
+    &self,
+    _: EdwardsPoint,
+    _: EdwardsPoint,
+    _: Scalar,
+  ) -> bool { unimplemented!() }
+}
+
 pub fn generate_keys() -> (Vec<Rc<MultisigKeys<Ed25519>>>, Scalar) {
   let mut params = vec![];
   let mut machines = vec![];

coins/monero/tests/key_image.rs

@@ -2,10 +2,14 @@
 
 use rand::{RngCore, rngs::OsRng};
 
+use curve25519_dalek::{traits::Identity, edwards::EdwardsPoint};
+
 use monero_serai::{frost::MultisigError, key_image};
 
+use ::frost::sign;
+
 mod frost;
-use crate::frost::{THRESHOLD, PARTICIPANTS, generate_keys};
+use crate::frost::{THRESHOLD, PARTICIPANTS, DummyAlgorithm, generate_keys};
 
 #[test]
 fn test() -> Result<(), MultisigError> {
@@ -18,23 +22,33 @@ fn test() -> Result<(), MultisigError> {
   }
   included.sort();
 
-  let mut packages = vec![];
+  let mut views = vec![];
-  packages.resize(PARTICIPANTS + 1, None);
+  let mut shares = vec![];
-  for i in &included {
+  for i in 1 ..= PARTICIPANTS {
-    let i = *i;
+    if included.contains(&i) {
-    packages[i] = Some(
+      // If they were included, include their view
-      (
+      views.push(sign::Params::new(DummyAlgorithm, keys[i - 1].clone(), &included).unwrap().view());
-        keys[0].verification_shares()[i].0,
+      let share = key_image::generate_share(&mut OsRng, &views[i - 1]);
-        key_image::multisig(&mut OsRng, &keys[i - 1], &included)
+      let mut serialized = share.0;
-      )
+      serialized.extend(b"abc");
-    );
+      serialized.extend(&share.1);
+      shares.push(serialized);
+    } else {
+      // If they weren't included, include dummy data
+      // Uses the view of someone actually included as Params::new verifies inclusion
+      views.push(sign::Params::new(DummyAlgorithm, keys[included[0] - 1].clone(), &included).unwrap().view());
+      shares.push(vec![]);
+    }
   }
 
-  for i in included {
+  for i in &included {
-    let mut packages = packages.clone();
+    let mut multi_image = EdwardsPoint::identity();
-    packages.push(None);
+    for l in &included {
-    let package = packages.swap_remove(i).unwrap().1;
+      let share = key_image::verify_share(&views[i - 1], *l, &shares[l - 1]).unwrap();
-    assert_eq!(image, package.resolve(packages).unwrap());
+      assert_eq!(share.1, b"abc");
+      multi_image += share.0;
+    }
+    assert_eq!(image, multi_image);
   }
 
   Ok(())

sign/frost/src/algorithm.rs

@@ -16,7 +16,6 @@ pub trait Algorithm<C: Curve>: Clone {
 
   /// Generate an addendum to FROST"s preprocessing stage
   fn preprocess_addendum<R: RngCore + CryptoRng>(
-    &mut self,
     rng: &mut R,
     params: &sign::ParamsView<C>,
     nonces: &[C::F; 2],
@@ -100,7 +99,6 @@ impl<C: Curve, H: Hram<C>> Algorithm<C> for Schnorr<C, H> {
   }
 
   fn preprocess_addendum<R: RngCore + CryptoRng>(
-    &mut self,
     _: &mut R,
     _: &sign::ParamsView<C>,
     _: &[C::F; 2],

sign/frost/src/sign.rs

@@ -70,7 +70,7 @@ impl<C: Curve, A: Algorithm<C>> Params<C, A> {
     algorithm: A,
     keys: Rc<MultisigKeys<C>>,
     included: &[usize],
-) -> Result<Params<C, A>, FrostError> {
+  ) -> Result<Params<C, A>, FrostError> {
     let mut included = included.to_vec();
     (&mut included).sort_unstable();
 
@@ -126,6 +126,10 @@ impl<C: Curve, A: Algorithm<C>> Params<C, A> {
   pub fn multisig_params(&self) -> MultisigParams {
     self.keys.params
   }
+
+  pub fn view(&self) -> ParamsView<C> {
+    self.view.clone()
+  }
 }
 
 struct PreprocessPackage<C: Curve> {
@@ -146,7 +150,7 @@ fn preprocess<R: RngCore + CryptoRng, C: Curve, A: Algorithm<C>>(
   serialized.extend(&C::G_to_bytes(&commitments[1]));
 
   serialized.extend(
-    &params.algorithm.preprocess_addendum(
+    &A::preprocess_addendum(
       rng,
       &params.view,
       &nonces