From 798ffc9b28994d32d08469a15212d189eeef4a9d Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 14 Jun 2024 17:12:46 -0400 Subject: [PATCH] Add a dedicated send/recv CLSAG mask struct Abstracts the types used internally. Also moves the tests from monero-serai to monero-clsag. --- coins/monero/ringct/clsag/Cargo.toml | 3 + coins/monero/ringct/clsag/src/lib.rs | 8 ++- coins/monero/ringct/clsag/src/multisig.rs | 65 ++++++++++++++----- .../clsag.rs => ringct/clsag/src/tests.rs} | 21 +++--- coins/monero/src/tests/mod.rs | 1 - coins/monero/src/wallet/send/multisig.rs | 24 +++---- 6 files changed, 75 insertions(+), 47 deletions(-) rename coins/monero/{src/tests/clsag.rs => ringct/clsag/src/tests.rs} (88%) diff --git a/coins/monero/ringct/clsag/Cargo.toml b/coins/monero/ringct/clsag/Cargo.toml index 1e540273..4aa528c1 100644 --- a/coins/monero/ringct/clsag/Cargo.toml +++ b/coins/monero/ringct/clsag/Cargo.toml @@ -39,6 +39,9 @@ monero-io = { path = "../../io", version = "0.1", default-features = false } monero-generators = { path = "../../generators", version = "0.4", default-features = false } monero-primitives = { path = "../../primitives", version = "0.1", default-features = false } +[dev-dependencies] +frost = { package = "modular-frost", path = "../../../../crypto/frost", default-features = false, features = ["ed25519", "tests"] } + [features] std = [ "std-shims/std", diff --git a/coins/monero/ringct/clsag/src/lib.rs b/coins/monero/ringct/clsag/src/lib.rs index 5f599eef..e999f7c6 100644 --- a/coins/monero/ringct/clsag/src/lib.rs +++ b/coins/monero/ringct/clsag/src/lib.rs @@ -28,7 +28,10 @@ use monero_primitives::{INV_EIGHT, BASEPOINT_PRECOMP, Commitment, Decoys, keccak #[cfg(feature = "multisig")] mod multisig; #[cfg(feature = "multisig")] -pub use multisig::{ClsagAddendum, ClsagMultisig}; +pub use multisig::{ClsagMultisigMaskSender, ClsagAddendum, ClsagMultisig}; + +#[cfg(all(feature = "std", test))] +mod tests; /// Errors when working with CLSAGs. #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -219,8 +222,7 @@ fn core( pub struct Clsag { D: EdwardsPoint, s: Vec, - // TODO: Remove pub - pub c1: Scalar, + c1: Scalar, } struct ClsagSignCore { diff --git a/coins/monero/ringct/clsag/src/multisig.rs b/coins/monero/ringct/clsag/src/multisig.rs index 06ae1a58..f7265f85 100644 --- a/coins/monero/ringct/clsag/src/multisig.rs +++ b/coins/monero/ringct/clsag/src/multisig.rs @@ -1,9 +1,9 @@ use core::{ops::Deref, fmt::Debug}; use std_shims::{ + sync::{Arc, Mutex}, io::{self, Read, Write}, collections::HashMap, }; -use std::sync::{Arc, Mutex}; use rand_core::{RngCore, CryptoRng, SeedableRng}; use rand_chacha::ChaCha20Rng; @@ -54,6 +54,35 @@ impl ClsagContext { } } +/// A channel to send the mask to use for the pseudo-out (rerandomized commitment) with. +/// +/// A mask must be sent along this channel before any preprocess addendums are handled. Breaking +/// this rule will cause a panic. +#[derive(Clone, Debug)] +pub struct ClsagMultisigMaskSender { + buf: Arc>>, +} +#[derive(Clone, Debug)] +struct ClsagMultisigMaskReceiver { + buf: Arc>>, +} +impl ClsagMultisigMaskSender { + fn new() -> (ClsagMultisigMaskSender, ClsagMultisigMaskReceiver) { + let buf = Arc::new(Mutex::new(None)); + (ClsagMultisigMaskSender { buf: buf.clone() }, ClsagMultisigMaskReceiver { buf }) + } + + /// Send a mask to a CLSAG multisig instance. + pub fn send(self, mask: Scalar) { + *self.buf.lock() = Some(mask); + } +} +impl ClsagMultisigMaskReceiver { + fn recv(self) -> Scalar { + self.buf.lock().unwrap() + } +} + /// Addendum produced during the signing process. #[derive(Clone, PartialEq, Eq, Zeroize, Debug)] pub struct ClsagAddendum { @@ -61,6 +90,7 @@ pub struct ClsagAddendum { } impl ClsagAddendum { + /// The key image share within this addendum. pub fn key_image_share(&self) -> dfg::EdwardsPoint { self.key_image_share } @@ -94,7 +124,7 @@ pub struct ClsagMultisig { context: ClsagContext, - mask_mutex: Arc>>, + mask_recv: Option, mask: Option, msg: Option<[u8; 32]>, @@ -108,23 +138,26 @@ impl ClsagMultisig { pub fn new( transcript: RecommendedTranscript, context: ClsagContext, - mask_mutex: Arc>>, - ) -> ClsagMultisig { - ClsagMultisig { - transcript, + ) -> (ClsagMultisig, ClsagMultisigMaskSender) { + let (mask_send, mask_recv) = ClsagMultisigMaskSender::new(); + ( + ClsagMultisig { + transcript, - key_image_generator: hash_to_point(context.decoys.signer_ring_members()[0].compress().0), - key_image_shares: HashMap::new(), - image: None, + key_image_generator: hash_to_point(context.decoys.signer_ring_members()[0].compress().0), + key_image_shares: HashMap::new(), + image: None, - context, + context, - mask_mutex, - mask: None, + mask_recv: Some(mask_recv), + mask: None, - msg: None, - interim: None, - } + msg: None, + interim: None, + }, + mask_send, + ) } /// The key image generator used by the signer. @@ -182,7 +215,7 @@ impl Algorithm for ClsagMultisig { // 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()); + self.mask = Some(self.mask_recv.take().unwrap().recv()); // Transcript the mask self.transcript.append_message(b"mask", self.mask.expect("mask wasn't set").to_bytes()); diff --git a/coins/monero/src/tests/clsag.rs b/coins/monero/ringct/clsag/src/tests.rs similarity index 88% rename from coins/monero/src/tests/clsag.rs rename to coins/monero/ringct/clsag/src/tests.rs index bc708d76..ba71d69c 100644 --- a/coins/monero/src/tests/clsag.rs +++ b/coins/monero/ringct/clsag/src/tests.rs @@ -1,5 +1,4 @@ use core::ops::Deref; -use std::sync::{Arc, Mutex}; use zeroize::Zeroizing; use rand_core::{RngCore, OsRng}; @@ -11,16 +10,11 @@ use transcript::{Transcript, RecommendedTranscript}; #[cfg(feature = "multisig")] use frost::curve::Ed25519; -use crate::{ - Commitment, - wallet::Decoys, - ringct::{ - generate_key_image, - clsag::{ClsagContext, Clsag}, - }, -}; +use monero_generators::hash_to_point; +use monero_primitives::{Commitment, Decoys}; +use crate::{ClsagContext, Clsag}; #[cfg(feature = "multisig")] -use crate::ringct::clsag::ClsagMultisig; +use crate::ClsagMultisig; #[cfg(feature = "multisig")] use frost::{ @@ -72,7 +66,8 @@ fn clsag() { .unwrap() .swap_remove(0); - let image = generate_key_image(&secrets.0); + let image = + hash_to_point((ED25519_BASEPOINT_TABLE * secrets.0.deref()).compress().0) * secrets.0.deref(); clsag.verify(&ring, &image, &pseudo_out, &msg).unwrap(); // make sure verification fails if we throw a random `c1` at it. @@ -104,15 +99,15 @@ fn clsag_multisig() { ring.push([dest, Commitment::new(mask, amount).calculate()]); } - let algorithm = ClsagMultisig::new( + let (algorithm, mask_send) = ClsagMultisig::new( RecommendedTranscript::new(b"Monero Serai CLSAG Test"), ClsagContext::new( Decoys::new((1 ..= RING_LEN).collect(), RING_INDEX, ring.clone()).unwrap(), Commitment::new(randomness, AMOUNT), ) .unwrap(), - Arc::new(Mutex::new(Some(Scalar::random(&mut OsRng)))), ); + mask_send.send(Scalar::random(&mut OsRng)); sign( &mut OsRng, diff --git a/coins/monero/src/tests/mod.rs b/coins/monero/src/tests/mod.rs index 33d56f22..8a5b7ab4 100644 --- a/coins/monero/src/tests/mod.rs +++ b/coins/monero/src/tests/mod.rs @@ -1,5 +1,4 @@ mod unreduced_scalar; -mod clsag; mod bulletproofs; mod address; mod seed; diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index 770d44c4..596e5bb1 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -3,7 +3,6 @@ use std_shims::{ io::{self, Read}, collections::HashMap, }; -use std::sync::{Arc, Mutex}; use zeroize::Zeroizing; @@ -27,7 +26,7 @@ use frost::{ use crate::{ ringct::{ - clsag::{ClsagContext, ClsagAddendum, ClsagMultisig}, + clsag::{ClsagContext, ClsagMultisigMaskSender, ClsagAddendum, ClsagMultisig}, RctPrunable, }, transaction::{Input, Transaction}, @@ -43,7 +42,7 @@ pub struct TransactionMachine { // Hashed key and scalar offset key_images: Vec<(EdwardsPoint, Scalar)>, - clsag_mask_mutexes: Vec>>>, + clsag_mask_sends: Vec, clsags: Vec>, } @@ -54,7 +53,7 @@ pub struct TransactionSignMachine { transcript: RecommendedTranscript, key_images: Vec<(EdwardsPoint, Scalar)>, - clsag_mask_mutexes: Vec>>>, + clsag_mask_sends: Vec, clsags: Vec>, our_preprocess: Vec>, @@ -73,11 +72,7 @@ impl SignableTransaction { keys: &ThresholdKeys, mut transcript: RecommendedTranscript, ) -> Result { - 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 - clsag_mask_mutexes.push(Arc::new(Mutex::new(None))); - } + let mut clsag_mask_sends = vec![]; let mut clsags = vec![]; // Create a RNG out of the input shared keys, which either requires the view key or being every @@ -148,7 +143,8 @@ impl SignableTransaction { let context = ClsagContext::new(decoys.clone(), input.commitment()) .map_err(TransactionError::ClsagError)?; - let clsag = ClsagMultisig::new(transcript.clone(), context, clsag_mask_mutexes[i].clone()); + let (clsag, clsag_mask_send) = ClsagMultisig::new(transcript.clone(), context); + clsag_mask_sends.push(clsag_mask_send); key_images.push(( clsag.key_image_generator(), keys.current_offset().unwrap_or(dfg::Scalar::ZERO).0 + self.inputs[i].0.key_offset(), @@ -161,7 +157,7 @@ impl SignableTransaction { i: keys.params().i(), transcript, key_images, - clsag_mask_mutexes, + clsag_mask_sends, clsags, }) } @@ -206,7 +202,7 @@ impl PreprocessMachine for TransactionMachine { transcript: self.transcript, key_images: self.key_images, - clsag_mask_mutexes: self.clsag_mask_mutexes, + clsag_mask_sends: self.clsag_mask_sends, clsags, our_preprocess, @@ -334,7 +330,7 @@ impl SignMachine for TransactionSignMachine { sorted.push(( images.swap_remove(0), self.signable.inputs.swap_remove(0).1, - self.clsag_mask_mutexes.swap_remove(0), + self.clsag_mask_sends.swap_remove(0), self.clsags.swap_remove(0), commitments.swap_remove(0), )); @@ -352,7 +348,7 @@ impl SignMachine for TransactionSignMachine { } else { sum_pseudo_outs += mask; } - *value.2.lock().unwrap() = Some(mask); + value.2.send(mask); tx.prefix.inputs.push(Input::ToKey { amount: None,