Use Monero-compatible additional TX keys

This still sends a fingerprinting flare up if you send to a subaddress which
needs to be fixed. Despite that, Monero no should no longer fail to scan TXs
from monero-serai regarding additional keys.

Previously it failed becuase we supplied one key as THE key, and n-1 as
additional. Monero expects n for additional.

This does correctly select when to use THE key versus when to use the additional
key when sending. That removes the ability for recipients to fingerprint
monero-serai by receiving to a standard address yet needing to use an additional
key.

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(

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<B: AddressBytes> AddressMeta<B> {
     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<B: AddressBytes> Address<B> {
     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()
   }
 }
 

coins/monero/src/wallet/extra.rs

@@ -149,13 +149,11 @@ impl Extra {
     res
   }
 
-  pub(crate) fn new(mut keys: Vec<EdwardsPoint>) -> Extra {
+  pub(crate) fn new(key: EdwardsPoint, additional: Vec<EdwardsPoint>) -> 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
   }

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<Scalar>,
   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

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 },

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<R: RngCore + CryptoRng>(
-    rng: &mut R,
+  fn new(
+    r: &Zeroizing<Scalar>,
     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();