diff --git a/coins/monero/src/tests/address.rs b/coins/monero/src/tests/address.rs index 3950d6d1..76f75a3e 100644 --- a/coins/monero/src/tests/address.rs +++ b/coins/monero/src/tests/address.rs @@ -33,9 +33,9 @@ fn standard_address() { let addr = MoneroAddress::from_str(Network::Mainnet, STANDARD).unwrap(); assert_eq!(addr.meta.network, Network::Mainnet); assert_eq!(addr.meta.kind, AddressType::Standard); - assert!(!addr.meta.kind.subaddress()); + assert!(!addr.meta.kind.is_subaddress()); assert_eq!(addr.meta.kind.payment_id(), None); - assert!(!addr.meta.kind.guaranteed()); + assert!(!addr.meta.kind.is_guaranteed()); assert_eq!(addr.spend.compress().to_bytes(), SPEND); assert_eq!(addr.view.compress().to_bytes(), VIEW); assert_eq!(addr.to_string(), STANDARD); @@ -46,9 +46,9 @@ fn integrated_address() { let addr = MoneroAddress::from_str(Network::Mainnet, INTEGRATED).unwrap(); assert_eq!(addr.meta.network, Network::Mainnet); assert_eq!(addr.meta.kind, AddressType::Integrated(PAYMENT_ID)); - assert!(!addr.meta.kind.subaddress()); + assert!(!addr.meta.kind.is_subaddress()); assert_eq!(addr.meta.kind.payment_id(), Some(PAYMENT_ID)); - assert!(!addr.meta.kind.guaranteed()); + assert!(!addr.meta.kind.is_guaranteed()); assert_eq!(addr.spend.compress().to_bytes(), SPEND); assert_eq!(addr.view.compress().to_bytes(), VIEW); assert_eq!(addr.to_string(), INTEGRATED); @@ -59,9 +59,9 @@ fn subaddress() { let addr = MoneroAddress::from_str(Network::Mainnet, SUBADDRESS).unwrap(); assert_eq!(addr.meta.network, Network::Mainnet); assert_eq!(addr.meta.kind, AddressType::Subaddress); - assert!(addr.meta.kind.subaddress()); + assert!(addr.meta.kind.is_subaddress()); assert_eq!(addr.meta.kind.payment_id(), None); - assert!(!addr.meta.kind.guaranteed()); + assert!(!addr.meta.kind.is_guaranteed()); assert_eq!(addr.spend.compress().to_bytes(), SUB_SPEND); assert_eq!(addr.view.compress().to_bytes(), SUB_VIEW); assert_eq!(addr.to_string(), SUBADDRESS); @@ -100,9 +100,9 @@ fn featured() { assert_eq!(addr.spend, spend); assert_eq!(addr.view, view); - assert_eq!(addr.subaddress(), subaddress); + assert_eq!(addr.is_subaddress(), subaddress); assert_eq!(addr.payment_id(), payment_id); - assert_eq!(addr.guaranteed(), guaranteed); + assert_eq!(addr.is_guaranteed(), guaranteed); } } } @@ -151,10 +151,10 @@ fn featured_vectors() { assert_eq!(addr.spend, spend); assert_eq!(addr.view, view); - assert_eq!(addr.subaddress(), vector.subaddress); + assert_eq!(addr.is_subaddress(), vector.subaddress); assert_eq!(vector.integrated, vector.payment_id.is_some()); assert_eq!(addr.payment_id(), vector.payment_id); - assert_eq!(addr.guaranteed(), vector.guaranteed); + assert_eq!(addr.is_guaranteed(), vector.guaranteed); assert_eq!( MoneroAddress::new( diff --git a/coins/monero/src/wallet/address.rs b/coins/monero/src/wallet/address.rs index ad7777d2..9a232d2a 100644 --- a/coins/monero/src/wallet/address.rs +++ b/coins/monero/src/wallet/address.rs @@ -60,7 +60,7 @@ pub enum AddressSpec { } impl AddressType { - pub fn subaddress(&self) -> bool { + pub fn is_subaddress(&self) -> bool { matches!(self, AddressType::Subaddress) || matches!(self, AddressType::Featured { subaddress: true, .. }) } @@ -75,7 +75,7 @@ impl AddressType { } } - pub fn guaranteed(&self) -> bool { + pub fn is_guaranteed(&self) -> bool { matches!(self, AddressType::Featured { guaranteed: true, .. }) } } @@ -169,16 +169,16 @@ impl AddressMeta { meta.ok_or(AddressError::InvalidByte) } - pub fn subaddress(&self) -> bool { - self.kind.subaddress() + pub fn is_subaddress(&self) -> bool { + self.kind.is_subaddress() } pub fn payment_id(&self) -> Option<[u8; 8]> { self.kind.payment_id() } - pub fn guaranteed(&self) -> bool { - self.kind.guaranteed() + pub fn is_guaranteed(&self) -> bool { + self.kind.is_guaranteed() } } @@ -285,16 +285,16 @@ impl Address { self.meta.network } - pub fn subaddress(&self) -> bool { - self.meta.subaddress() + pub fn is_subaddress(&self) -> bool { + self.meta.is_subaddress() } pub fn payment_id(&self) -> Option<[u8; 8]> { self.meta.payment_id() } - pub fn guaranteed(&self) -> bool { - self.meta.guaranteed() + pub fn is_guaranteed(&self) -> bool { + self.meta.is_guaranteed() } } diff --git a/coins/monero/src/wallet/extra.rs b/coins/monero/src/wallet/extra.rs index 2a0b9aa2..f213fecf 100644 --- a/coins/monero/src/wallet/extra.rs +++ b/coins/monero/src/wallet/extra.rs @@ -149,13 +149,11 @@ impl Extra { res } - pub(crate) fn new(mut keys: Vec) -> Extra { + pub(crate) fn new(key: EdwardsPoint, additional: Vec) -> Extra { let mut res = Extra(Vec::with_capacity(3)); - if !keys.is_empty() { - res.push(ExtraField::PublicKey(keys[0])); - } - if keys.len() > 1 { - res.push(ExtraField::PublicKeys(keys.drain(1 ..).collect())); + res.push(ExtraField::PublicKey(key)); + if !additional.is_empty() { + res.push(ExtraField::PublicKeys(additional)); } res } diff --git a/coins/monero/src/wallet/mod.rs b/coins/monero/src/wallet/mod.rs index 383eb09a..511b5be3 100644 --- a/coins/monero/src/wallet/mod.rs +++ b/coins/monero/src/wallet/mod.rs @@ -54,12 +54,12 @@ pub(crate) fn uniqueness(inputs: &[Input]) -> [u8; 32] { #[allow(non_snake_case)] pub(crate) fn shared_key( uniqueness: Option<[u8; 32]>, - s: &Scalar, + s: &Zeroizing, P: &EdwardsPoint, o: usize, ) -> (u8, Scalar, [u8; 8]) { // 8Ra - let mut output_derivation = (s * P).mul_by_cofactor().compress().to_bytes().to_vec(); + let mut output_derivation = (s.deref() * P).mul_by_cofactor().compress().to_bytes().to_vec(); let mut payment_id_xor = [0; 8]; payment_id_xor diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs index 7fd1a027..a86cbf9b 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -296,6 +296,7 @@ impl Scanner { } let output_key = output_key.unwrap(); + // TODO: Only use THE key or the matching additional key. Not any key for key in &keys { let (view_tag, shared_key, payment_id_xor) = shared_key( if self.burning_bug.is_none() { Some(uniqueness(&tx.prefix.inputs)) } else { None }, diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index 9aa3e8b8..bf169299 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -7,7 +7,9 @@ use rand::seq::SliceRandom; use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; +use group::Group; use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar, edwards::EdwardsPoint}; +use dalek_ff_group as dfg; #[cfg(feature = "multisig")] use frost::FrostError; @@ -47,24 +49,23 @@ struct SendOutput { } impl SendOutput { - fn new( - rng: &mut R, + fn new( + r: &Zeroizing, unique: [u8; 32], output: (usize, (MoneroAddress, u64)), ) -> (SendOutput, Option<[u8; 8]>) { let o = output.0; let output = output.1; - let r = random_scalar(rng); let (view_tag, shared_key, payment_id_xor) = - shared_key(Some(unique).filter(|_| output.0.meta.kind.guaranteed()), &r, &output.0.view, o); + shared_key(Some(unique).filter(|_| output.0.is_guaranteed()), r, &output.0.view, o); ( SendOutput { - R: if !output.0.meta.kind.subaddress() { - &r * &ED25519_BASEPOINT_TABLE + R: if !output.0.is_subaddress() { + r.deref() * &ED25519_BASEPOINT_TABLE } else { - r * output.0.spend + r.deref() * output.0.spend }, view_tag, dest: ((&shared_key * &ED25519_BASEPOINT_TABLE) + output.0.spend), @@ -281,11 +282,26 @@ impl SignableTransaction { // Shuffle the payments self.payments.shuffle(rng); + // Used for all non-subaddress outputs, or if there's only one subaddress output and a change + let tx_key = Zeroizing::new(random_scalar(rng)); + // TODO: Support not needing additional when one subaddress and non-subaddress change + let additional = self.payments.iter().filter(|payment| payment.0.is_subaddress()).count() != 0; + // Actually create the outputs let mut outputs = Vec::with_capacity(self.payments.len()); let mut id = None; for payment in self.payments.drain(..).enumerate() { - let (output, payment_id) = SendOutput::new(rng, uniqueness, payment); + // If this is a subaddress, generate a dedicated r. Else, reuse the TX key + let dedicated = Zeroizing::new(random_scalar(&mut *rng)); + let use_dedicated = additional && payment.1 .0.is_subaddress(); + let r = if use_dedicated { &dedicated } else { &tx_key }; + + let (mut output, payment_id) = SendOutput::new(r, uniqueness, payment); + // If this used the tx_key, randomize its R + if !use_dedicated { + output.R = dfg::EdwardsPoint::random(&mut *rng).0; + } + outputs.push(output); id = id.or(payment_id); } @@ -308,7 +324,10 @@ impl SignableTransaction { // Create the TX extra let extra = { - let mut extra = Extra::new(outputs.iter().map(|output| output.R).collect()); + let mut extra = Extra::new( + tx_key.deref() * &ED25519_BASEPOINT_TABLE, + if additional { outputs.iter().map(|output| output.R).collect() } else { vec![] }, + ); let mut id_vec = Vec::with_capacity(1 + 8); PaymentId::Encrypted(id).write(&mut id_vec).unwrap();