Tidy Borromean, fix bugs in last commit, replace todo! with unreachable!

coins/monero/Cargo.toml

@@ -96,6 +96,9 @@ std = [
   "serde/std",
   "serde_json/std",
 ]
+
 http_rpc = ["digest_auth", "reqwest"]
 multisig = ["transcript", "frost", "dleq", "std"]
+experimental = []
+
 default = ["std", "http_rpc"]

coins/monero/generators/src/lib.rs

@@ -5,15 +5,12 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
-use core::cell::OnceCell;
+use std_shims::sync::OnceLock;
-use std_shims::sync::Mutex;
 
 use sha3::{Digest, Keccak256};
 
 use curve25519_dalek::edwards::{EdwardsPoint as DalekPoint, CompressedEdwardsY};
-use curve25519_dalek::scalar::Scalar;
 
-use std_shims::vec::Vec;
 use group::{Group, GroupEncoding};
 use dalek_ff_group::EdwardsPoint;
 
@@ -42,19 +39,16 @@ pub fn H() -> DalekPoint {
 static H_POW_2_CELL: OnceLock<[DalekPoint; 64]> = OnceLock::new();
 /// Monero's alternate generator `H`, multiplied by 2**i for i in 1 ..= 64.
 #[allow(non_snake_case)]
-pub fn H_pow_2() -> &[DalekPoint; 64] {
+pub fn H_pow_2() -> &'static [DalekPoint; 64] {
   H_POW_2_CELL.get_or_init(|| {
     let mut res = [H(); 64];
     for i in 1 .. 64 {
-      res[i] = res[i - 1].double();
+      res[i] = res[i - 1] + res[i - 1];
     }
     res
   })
 }
 
-#[allow(non_snake_case)]
-fn generate_H2() -> [DalekPoint; 64] {
-
 const MAX_M: usize = 16;
 const N: usize = 64;
 const MAX_MN: usize = MAX_M * N;

coins/monero/src/ringct/borromean.rs

@@ -1,39 +1,36 @@
 #![allow(non_snake_case)]
 
-use std::fmt::Debug;
+use core::fmt::Debug;
-use std::io::{self, Read, Write};
+use std_shims::io::{self, Read, Write};
 
-use curve25519_dalek::edwards::EdwardsPoint;
+use curve25519_dalek::{traits::Identity, scalar::Scalar, edwards::EdwardsPoint};
-use curve25519_dalek::scalar::Scalar;
-use curve25519_dalek::traits::Identity;
 
-use monero_generators::H2;
+use monero_generators::H_pow_2;
+use crate::{hash_to_scalar, serialize::*};
 
-use crate::hash_to_scalar;
+/// 64 Borromean ring signatures, modified to be aggregated with a shared challenge.
-use crate::serialize::*;
-
-/// A Borromean signature.
 ///
-/// Note: This type keeps the data as raw bytes as Monero has
+/// This type keeps the data as raw bytes as Monero has some transactions with unreduced scalars in
-/// some transactions with unreduced scalars in this field, we
+/// this field. While we could use `from_bytes_mod_order`, we'd then not be able  to encode this
-/// could use `from_bytes_mod_order` but then we would not be able
+/// back into it's original form.
-/// to encode this back into it's original form.
 ///
+/// Those scalars also have a custom reduction algorithm...
 #[derive(Clone, PartialEq, Eq, Debug)]
-pub struct BorroSig {
+pub struct BorromeanSignatures {
   pub s0: [[u8; 32]; 64],
   pub s1: [[u8; 32]; 64],
   pub ee: [u8; 32],
 }
 
-impl BorroSig {
+impl BorromeanSignatures {
-  pub fn read<R: Read>(r: &mut R) -> io::Result<BorroSig> {
+  pub fn read<R: Read>(r: &mut R) -> io::Result<BorromeanSignatures> {
-    Ok(BorroSig {
+    Ok(BorromeanSignatures {
       s0: read_array(read_bytes, r)?,
       s1: read_array(read_bytes, r)?,
       ee: read_bytes(r)?,
     })
   }
+
   pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
     for s0 in self.s0.iter() {
       w.write_all(s0)?;
@@ -43,66 +40,63 @@ impl BorroSig {
     }
     w.write_all(&self.ee)
   }
+
+  #[cfg(feature = "experimental")]
+  fn verify(&self, keys_a: &[EdwardsPoint], keys_b: &[EdwardsPoint]) -> bool {
+    let mut transcript = [0; 2048];
+    for i in 0 .. 64 {
+      // TODO: These aren't the correct reduction
+      // TODO: Can either of these be tightened?
+      let LL = EdwardsPoint::vartime_double_scalar_mul_basepoint(
+        &Scalar::from_bytes_mod_order(self.ee),
+        &keys_a[i],
+        &Scalar::from_bytes_mod_order(self.s0[i]),
+      );
+      let LV = EdwardsPoint::vartime_double_scalar_mul_basepoint(
+        &hash_to_scalar(LL.compress().as_bytes()),
+        &keys_b[i],
+        &Scalar::from_bytes_mod_order(self.s1[i]),
+      );
+      transcript[i .. ((i + 1) * 32)].copy_from_slice(LV.compress().as_bytes());
+    }
+
+    // TODO: This isn't the correct reduction
+    // TODO: Can this be tightened to from_canonical_bytes?
+    hash_to_scalar(&transcript) == Scalar::from_bytes_mod_order(self.ee)
+  }
 }
 
+/// A range proof premised on Borromean ring signatures.
 #[derive(Clone, PartialEq, Eq, Debug)]
-pub struct RangeSig {
+pub struct BorromeanRange {
-  pub asig: BorroSig,
+  pub sig: BorromeanSignatures,
-  pub Ci: [EdwardsPoint; 64],
+  pub bit_commitments: [EdwardsPoint; 64],
 }
 
-impl RangeSig {
+impl BorromeanRange {
-  pub fn read<R: Read>(r: &mut R) -> io::Result<RangeSig> {
+  pub fn read<R: Read>(r: &mut R) -> io::Result<BorromeanRange> {
-    Ok(RangeSig { asig: BorroSig::read(r)?, Ci: read_array(read_point, r)? })
+    Ok(BorromeanRange {
+      sig: BorromeanSignatures::read(r)?,
+      bit_commitments: read_array(read_point, r)?,
+    })
   }
   pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
-    self.asig.write(w)?;
+    self.sig.write(w)?;
-    write_raw_vec(write_point, &self.Ci, w)
+    write_raw_vec(write_point, &self.bit_commitments, w)
   }
 
+  #[cfg(feature = "experimental")]
   pub fn verify(&self, commitment: &EdwardsPoint) -> bool {
-    let mut P1 = Vec::with_capacity(64);
+    if &self.bit_commitments.iter().sum::<EdwardsPoint>() != commitment {
-    let mut P2 = Vec::with_capacity(64);
+      return false;
-    let mut bbs0 = Vec::with_capacity(64);
+    }
-    let mut bbs1 = Vec::with_capacity(64);
-
-    let bbee = Scalar::from_bytes_mod_order(self.asig.ee);
-
-    let mut C_temp = EdwardsPoint::identity();
 
+    let H_pow_2 = H_pow_2();
+    let mut commitments_sub_one = [EdwardsPoint::identity(); 64];
     for i in 0 .. 64 {
-      bbs0.push(Scalar::from_bytes_mod_order(self.asig.s0[i]));
+      commitments_sub_one[i] = self.bit_commitments[i] - H_pow_2[i];
-      bbs1.push(Scalar::from_bytes_mod_order(self.asig.s1[i]));
-
-      P1.push(self.Ci[i]);
-      P2.push(P1[i] - H2[i]);
-
-      C_temp += P1[i];
     }
 
-    if &C_temp != commitment {
+    self.sig.verify(&self.bit_commitments, &commitments_sub_one)
-      false
-    } else {
-      verify_borromean(P1, P2, bbee, bbs0, bbs1)
-    }
   }
 }
-
-fn verify_borromean(
-  P1: Vec<EdwardsPoint>,
-  P2: Vec<EdwardsPoint>,
-  bbee: Scalar,
-  bbs0: Vec<Scalar>,
-  bbs1: Vec<Scalar>,
-) -> bool {
-  let mut LV: Vec<u8> = Vec::with_capacity(2048);
-  for i in 0 .. 64 {
-    let LL = EdwardsPoint::vartime_double_scalar_mul_basepoint(&bbee, &P1[i], &bbs0[i]);
-    let chash = hash_to_scalar(LL.compress().as_bytes());
-    let LV_temp = EdwardsPoint::vartime_double_scalar_mul_basepoint(&chash, &P2[i], &bbs1[i]);
-    LV.extend(LV_temp.compress().as_bytes());
-  }
-  let eecomp = hash_to_scalar(&LV);
-
-  eecomp == bbee
-}

coins/monero/src/ringct/mod.rs

@@ -15,7 +15,7 @@ pub use hash_to_point::{raw_hash_to_point, hash_to_point};
 pub mod clsag;
 /// MLSAG struct, along with verifying functionality.
 pub mod mlsag;
-/// RangeSig struct, along with verifying functionality.
+/// BorromeanRange struct, along with verifying functionality.
 pub mod borromean;
 /// Bulletproofs(+) structs, along with proving and verifying functionality.
 pub mod bulletproofs;
@@ -23,7 +23,7 @@ pub mod bulletproofs;
 use crate::{
   Protocol,
   serialize::*,
-  ringct::{clsag::Clsag, mlsag::MgSig, bulletproofs::Bulletproofs, borromean::RangeSig},
+  ringct::{clsag::Clsag, mlsag::MgSig, bulletproofs::Bulletproofs, borromean::BorromeanRange},
 };
 
 /// Generate a key image for a given key. Defined as `x * hash_to_point(xG)`.
@@ -110,7 +110,7 @@ impl RctBase {
 pub enum RctPrunable {
   Null,
   Borromean {
-    range_sigs: Vec<RangeSig>,
+    range_sigs: Vec<BorromeanRange>,
     mlsags: Vec<MgSig>,
     simple: bool,
   },
@@ -165,7 +165,7 @@ impl RctPrunable {
     match self {
       RctPrunable::Null => Ok(()),
       RctPrunable::Borromean { range_sigs, mlsags, simple: _ } => {
-        write_raw_vec(RangeSig::write, range_sigs, w)?;
+        write_raw_vec(BorromeanRange::write, range_sigs, w)?;
         write_raw_vec(MgSig::write, mlsags, w)
       }
       RctPrunable::BulletProof { bulletproofs, mlsags, pseudo_outs, v2 } => {
@@ -201,7 +201,7 @@ impl RctPrunable {
     Ok(match rct_type {
       0 => RctPrunable::Null,
       1 | 2 => RctPrunable::Borromean {
-        range_sigs: read_raw_vec(RangeSig::read, outputs, r)?,
+        range_sigs: read_raw_vec(BorromeanRange::read, outputs, r)?,
         mlsags: decoys.iter().map(|d| MgSig::read(*d, r)).collect::<Result<_, _>>()?,
         simple: rct_type == 2,
       },

coins/monero/src/wallet/send/mod.rs

@@ -708,9 +708,7 @@ impl SignableTransaction {
         clsags.append(&mut clsag_pairs.iter().map(|clsag| clsag.0.clone()).collect::<Vec<_>>());
         pseudo_outs.append(&mut clsag_pairs.iter().map(|clsag| clsag.1).collect::<Vec<_>>());
       }
-      _ => {
+      _ => unreachable!("attempted to sign a TX which wasn't CLSAG"),
-        todo!()
-      }
     }
     Ok(tx)
   }

coins/monero/src/wallet/send/multisig.rs

@@ -430,7 +430,7 @@ impl SignatureMachine<Transaction> for TransactionSignatureMachine {
           pseudo_outs.push(pseudo_out);
         }
       }
-      _ => todo!(""),
+      _ => unreachable!("attempted to sign a multisig TX which wasn't CLSAG"),
     }
     Ok(tx)
   }