Make CLSAG signing private

Also adds a bit more documentation and does a bit more tidying.

coins/monero/src/ringct/bulletproofs/original.rs

@@ -33,6 +33,7 @@ pub(crate) fn hadamard_fold(
   res
 }
 
+/// Internal structure representing a Bulletproof.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct OriginalStruct {
   pub(crate) A: DalekPoint,

coins/monero/src/ringct/bulletproofs/plus/aggregate_range_proof.rs

@@ -51,6 +51,7 @@ impl AggregateRangeWitness {
   }
 }
 
+/// Internal structure representing a Bulletproof+, as used in Monero.
 #[derive(Clone, PartialEq, Eq, Debug, Zeroize)]
 pub struct AggregateRangeProof {
   pub(crate) A: EdwardsPoint,

coins/monero/src/ringct/clsag/mod.rs

@@ -26,31 +26,36 @@ use crate::{
 #[cfg(feature = "multisig")]
 mod multisig;
 #[cfg(feature = "multisig")]
-pub use multisig::{ClsagDetails, ClsagAddendum, ClsagMultisig};
+pub(crate) use multisig::{ClsagDetails, ClsagAddendum, ClsagMultisig};
 
-/// Errors returned when CLSAG signing fails.
+/// Errors when working with CLSAGs.
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 #[cfg_attr(feature = "std", derive(thiserror::Error))]
 pub enum ClsagError {
-  #[cfg_attr(feature = "std", error("internal error ({0})"))]
+  /// The ring was invalid (such as being too small or too large).
-  InternalError(&'static str),
   #[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 commitment opening provided did not match the ring member's.
   #[cfg_attr(feature = "std", error("invalid commitment"))]
   InvalidCommitment,
+  /// The key image was invalid (such as being identity or torsioned)
   #[cfg_attr(feature = "std", error("invalid key image"))]
   InvalidImage,
+  /// The `D` component was invalid.
   #[cfg_attr(feature = "std", error("invalid D"))]
   InvalidD,
+  /// The `s` vector was invalid.
   #[cfg_attr(feature = "std", error("invalid s"))]
   InvalidS,
+  /// The `c1` variable was invalid.
   #[cfg_attr(feature = "std", error("invalid c1"))]
   InvalidC1,
 }
 
-/// Input being signed for.
+/// Context on the ring member being signed for.
 #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)]
 pub struct ClsagInput {
   // The actual commitment for the true spend
@@ -63,7 +68,7 @@ impl ClsagInput {
   pub fn new(commitment: Commitment, decoys: Decoys) -> Result<ClsagInput, ClsagError> {
     let n = decoys.len();
     if n > u8::MAX.into() {
-      Err(ClsagError::InternalError("max ring size in this library is u8 max"))?;
+      Err(ClsagError::InvalidRing)?;
     }
     let n = u8::try_from(n).unwrap();
     if decoys.i >= n {
@@ -213,9 +218,16 @@ fn core(
 /// CLSAG signature, as used in Monero.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct Clsag {
-  pub D: EdwardsPoint,
+  D: EdwardsPoint,
-  pub s: Vec<Scalar>,
+  pub(crate) s: Vec<Scalar>,
-  pub c1: Scalar,
+  pub(crate) c1: Scalar,
+}
+
+pub(crate) struct ClsagSignCore {
+  incomplete_clsag: Clsag,
+  pseudo_out: EdwardsPoint,
+  key_challenge: Scalar,
+  challenged_mask: Scalar,
 }
 
 impl Clsag {
@@ -229,28 +241,34 @@ impl Clsag {
     msg: &[u8; 32],
     A: EdwardsPoint,
     AH: EdwardsPoint,
-  ) -> (Clsag, EdwardsPoint, Scalar, Scalar) {
+  ) -> ClsagSignCore {
     let r: usize = input.decoys.i.into();
 
     let pseudo_out = Commitment::new(mask, input.commitment.amount).calculate();
-    let z = input.commitment.mask - mask;
+    let mask_delta = input.commitment.mask - mask;
 
     let H = hash_to_point(&input.decoys.ring[r][0]);
-    let D = H * z;
+    let D = H * mask_delta;
     let mut s = Vec::with_capacity(input.decoys.ring.len());
     for _ in 0 .. input.decoys.ring.len() {
       s.push(random_scalar(rng));
     }
-    let ((D, p, c), c1) =
+    let ((D, c_p, c_c), c1) =
       core(&input.decoys.ring, I, &pseudo_out, msg, &D, &s, &Mode::Sign(r, A, AH));
 
-    (Clsag { D, s, c1 }, pseudo_out, p, c * z)
+    ClsagSignCore {
+      incomplete_clsag: Clsag { D, s, c1 },
+      pseudo_out,
+      key_challenge: c_p,
+      challenged_mask: c_c * mask_delta,
+    }
   }
 
   /// Generate CLSAG signatures for the given inputs.
+  ///
   /// inputs is of the form (private key, key image, input).
   /// sum_outputs is for the sum of the outputs' commitment masks.
-  pub fn sign<R: RngCore + CryptoRng>(
+  pub(crate) fn sign<R: RngCore + CryptoRng>(
     rng: &mut R,
     mut inputs: Vec<(Zeroizing<Scalar>, EdwardsPoint, ClsagInput)>,
     sum_outputs: Scalar,
@@ -267,20 +285,25 @@ impl Clsag {
       }
 
       let mut nonce = Zeroizing::new(random_scalar(rng));
-      let (mut clsag, pseudo_out, p, c) = Clsag::sign_core(
+      let ClsagSignCore { mut incomplete_clsag, pseudo_out, key_challenge, challenged_mask } =
-        rng,
+        Clsag::sign_core(
-        &inputs[i].1,
+          rng,
-        &inputs[i].2,
+          &inputs[i].1,
-        mask,
+          &inputs[i].2,
-        &msg,
+          mask,
-        nonce.deref() * ED25519_BASEPOINT_TABLE,
+          &msg,
-        nonce.deref() *
+          nonce.deref() * ED25519_BASEPOINT_TABLE,
-          hash_to_point(&inputs[i].2.decoys.ring[usize::from(inputs[i].2.decoys.i)][0]),
+          nonce.deref() *
-      );
+            hash_to_point(&inputs[i].2.decoys.ring[usize::from(inputs[i].2.decoys.i)][0]),
+        );
       // 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)
-      clsag.s[usize::from(inputs[i].2.decoys.i)] = nonce.deref() - ((p * inputs[i].0.deref()) + c);
+      incomplete_clsag.s[usize::from(inputs[i].2.decoys.i)] =
+        nonce.deref() - ((key_challenge * inputs[i].0.deref()) + challenged_mask);
+      let clsag = incomplete_clsag;
+
+      // Zeroize private keys and nonces.
       inputs[i].0.zeroize();
       nonce.zeroize();
 
@@ -310,7 +333,7 @@ impl Clsag {
     if ring.len() != self.s.len() {
       Err(ClsagError::InvalidS)?;
     }
-    if I.is_identity() {
+    if I.is_identity() || (!I.is_torsion_free()) {
       Err(ClsagError::InvalidImage)?;
     }
 
@@ -330,12 +353,14 @@ impl Clsag {
     (ring_len * 32) + 32 + 32
   }
 
+  /// Write the CLSAG to a writer.
   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.
   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/src/ringct/clsag/multisig.rs

@@ -57,13 +57,13 @@ impl ClsagInput {
 
 /// CLSAG input and the mask to use for it.
 #[derive(Clone, Debug, Zeroize, ZeroizeOnDrop)]
-pub struct ClsagDetails {
+pub(crate) struct ClsagDetails {
   input: ClsagInput,
   mask: Scalar,
 }
 
 impl ClsagDetails {
-  pub fn new(input: ClsagInput, mask: Scalar) -> ClsagDetails {
+  pub(crate) fn new(input: ClsagInput, mask: Scalar) -> ClsagDetails {
     ClsagDetails { input, mask }
   }
 }
@@ -93,7 +93,7 @@ struct Interim {
 /// FROST algorithm for producing a CLSAG signature.
 #[allow(non_snake_case)]
 #[derive(Clone, Debug)]
-pub struct ClsagMultisig {
+pub(crate) struct ClsagMultisig {
   transcript: RecommendedTranscript,
 
   pub(crate) H: EdwardsPoint,
@@ -107,7 +107,7 @@ pub struct ClsagMultisig {
 }
 
 impl ClsagMultisig {
-  pub fn new(
+  pub(crate) fn new(
     transcript: RecommendedTranscript,
     output_key: EdwardsPoint,
     details: Arc<RwLock<Option<ClsagDetails>>>,
@@ -219,8 +219,7 @@ impl Algorithm<Ed25519> for ClsagMultisig {
 
     self.msg = Some(msg.try_into().expect("CLSAG message should be 32-bytes"));
 
-    #[allow(non_snake_case)]
+    let sign_core = Clsag::sign_core(
-    let (clsag, pseudo_out, p, c) = Clsag::sign_core(
       &mut rng,
       &self.image.expect("verifying a share despite never processing any addendums").0,
       &self.input(),
@@ -229,10 +228,15 @@ impl Algorithm<Ed25519> for ClsagMultisig {
       nonce_sums[0][0].0,
       nonce_sums[0][1].0,
     );
-    self.interim = Some(Interim { p, c, clsag, pseudo_out });
+    self.interim = Some(Interim {
+      p: sign_core.key_challenge,
+      c: sign_core.challenged_mask,
+      clsag: sign_core.incomplete_clsag,
+      pseudo_out: sign_core.pseudo_out,
+    });
 
     // r - p x, where p is the challenge for the keys
-    *nonces[0] - dfg::Scalar(p) * view.secret_share().deref()
+    *nonces[0] - dfg::Scalar(sign_core.key_challenge) * view.secret_share().deref()
   }
 
   #[must_use]