fix pr issues

coins/monero/src/wallet/address.rs

@@ -182,14 +182,6 @@ impl<B: AddressBytes> ToString for Address<B> {
 }
 
 impl<B: AddressBytes> Address<B> {
-  /// Generates an Address type according to the specification provided in the meta.
-  ///
-  /// WARNING: Specification on the meta are not "commands" and are
-  /// just "specifications" for the address. This function relies on the
-  /// caller to make sure all the passed parameters makes sense and assumes
-  /// specifications provided in the meta are correct and applicable for the passed in keys.
-  /// For example, passing `AddressType::Subaddress` type in the meta wont generate
-  /// a correct subaddress if the keys aren't already for a valid subaddress.
   pub fn new(meta: AddressMeta<B>, spend: EdwardsPoint, view: EdwardsPoint) -> Self {
     Address { meta, spend, view }
   }

coins/monero/src/wallet/mod.rs

@@ -124,18 +124,16 @@ impl ViewPair {
     ))
   }
 
-  /// returns the MoneroAddress type according to specifications in the meta
+  /// Returns an address with the provided metadata.
   pub fn address(&self, meta: AddressMeta<MoneroAddressBytes>) -> MoneroAddress {
     MoneroAddress::new(meta, self.spend, self.view.deref() * &ED25519_BASEPOINT_TABLE)
   }
 }
 
 /// Transaction scanner.
-/// - This scanner is capable of generating subaddresses, additionally scanning for
+/// This scanner is capable of generating subaddresses, additionally scanning for them once they've
-/// them once they've been explicitly generated by using `subaddress()` member function.
+/// been explicitly generated. If the burning bug is attempted, any secondary outputs will be
-/// It wont be able to track the outputs for subaddresses that aren't generated this way.
+/// ignored.
-/// - If the burning bug is attempted, any secondary outputs will be ignored.
-/// or standard address type if none passed.
 #[derive(Clone)]
 pub struct Scanner {
   pair: ViewPair,
@@ -176,13 +174,8 @@ impl Scanner {
   /// burning_bug is a HashSet of used keys, intended to prevent key reuse which would burn funds.
   /// When an output is successfully scanned, the output key MUST be saved to disk.
   /// When a new scanner is created, ALL saved output keys must be passed in to be secure.
-  /// If None is passed, a modified shared key derivation is used to generate new addresses
+  /// If None is passed, a modified shared key derivation is used which is immune to the burning
-  /// through this scanner which is immune to the burning bug (specifically
+  /// bug (specifically the Guaranteed feature from Featured Addresses).
-  /// the Guaranteed feature from Featured Addresses).
-  ///
-  /// If you want to use guaranteed feature for both generating addresses and
-  /// decoding outputs with such addresses pass `None` to the `burning_bug`. if you
-  /// just wanna support standard monero addresses then pass `Some`.
   // TODO: Should this take in a DB access handle to ensure output keys are saved?
   pub fn from_view(
     pair: ViewPair,
@@ -207,7 +200,7 @@ impl Scanner {
     self.pair.address(meta)
   }
 
-  /// Returns the integrated address for a given payment id.
+  /// Returns the integrated address for a given payment ID.
   pub fn integrated_address(&self, payment_id: [u8; 8]) -> MoneroAddress {
     let meta = AddressMeta::new(
       self.network,
@@ -245,29 +238,29 @@ impl Scanner {
 
   /// Returns a featured address.
   ///
-  /// if you created the scanner with `Some` value for `burning_bug` be aware that
+  /// A `Scanner` will fail to scan this address unless it is appropriate to the `Scanner`.
-  /// even though you can pass `true` for the `guaranteed` parameter and generate
+  /// If the `Scanner` was defined as not handling the burning bug, by passing `None` for
-  /// a guaranteed address, scanner wont be able decode incoming outputs for this address.
+  /// `burning_bug` upon construction, this must be a guaranteed address
-  /// If you want to support guaranteed addresses please create a scanner with `None` value to
+  /// in order to be scanned by it.
-  /// `burning_bug`. Reverse of the case also holds.
   pub fn featured_address(
     &mut self,
-    index: Option<(u32, u32)>,
+    subaddress: Option<(u32, u32)>,
     payment_id: Option<[u8; 8]>,
     guaranteed: bool,
   ) -> MoneroAddress {
-    let subaddress = index.is_some() && (index.unwrap() != (0, 0));
+    let is_subaddress = subaddress.is_some() && (subaddress.unwrap() != (0, 0));
 
     let mut spend = self.pair.spend;
     let mut view: EdwardsPoint = self.pair.view.deref() * &ED25519_BASEPOINT_TABLE;
-    if subaddress {
+    if is_subaddress {
-      spend = self.pair.spend + (&self.pair.subaddress(index.unwrap()) * &ED25519_BASEPOINT_TABLE);
+      spend =
-      self.subaddresses.insert(spend.compress(), index.unwrap());
+        self.pair.spend + (&self.pair.subaddress(subaddress.unwrap()) * &ED25519_BASEPOINT_TABLE);
+      self.subaddresses.insert(spend.compress(), subaddress.unwrap());
       view = self.pair.view.deref() * spend;
     }
 
     let meta =
-      AddressMeta::new(self.network, AddressType::Featured(subaddress, payment_id, guaranteed));
+      AddressMeta::new(self.network, AddressType::Featured(is_subaddress, payment_id, guaranteed));
     MoneroAddress::new(meta, spend, view)
   }
 }

coins/monero/tests/runner.rs

@@ -1,6 +1,5 @@
 use core::ops::Deref;
-use std::sync::Mutex;
+use std::{sync::Mutex, collections::HashSet};
-use std::collections::HashSet;
 
 use lazy_static::lazy_static;
 

coins/monero/tests/scan.rs

@@ -1,160 +1,213 @@
-use std::collections::HashSet;
+use rand::RngCore;
-use hex_literal::hex;
 
-use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar};
+use monero_serai::{transaction::Transaction};
-use rand_core::OsRng;
-use zeroize::Zeroizing;
-
-use monero_serai::{
-  wallet::{
-    address::{Network, MoneroAddress},
-    ViewPair, SignableTransaction, Scanner,
-  },
-};
 
 mod runner;
 
-const ADDR_SPEND: [u8; 32] =
+test!(
-  hex!("bf02a79d9e30ea76872e565722517924151793c535832e4353e54dc0be698001");
+  scan_standard_address,
-const ADDR_VIEW: [u8; 32] =
+  (
-  hex!("c300b798c000f3c5da4ebc5a288418fe9060d623c9830b7abc3d9e7a3152eb08");
+    |_, mut builder: Builder, _| async move {
-
+      let scanner =
-pub struct AddressInfo {
+        Scanner::from_view(runner::random_address().1, Network::Mainnet, Some(HashSet::new()));
-  address: String,
+      builder.add_payment(scanner.address(), 5);
-  payment_id: Option<[u8; 8]>,
+      (builder.build().unwrap(), (scanner,))
-}
+    },
-
+    |_, tx: Transaction, _, mut state: (Scanner,)| async move {
-pub async fn scan_incomings(addr_scanner: &mut Scanner, addresses: &Vec<AddressInfo>) {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
-  // get an output to spend
+      assert_eq!(output.commitment().amount, 5);
-  let (miner_spend, miner_view, miner_addr) = runner::random_address();
+    },
-  let mut miner_scanner =
+  ),
-    Scanner::from_view(miner_view.clone(), Network::Mainnet, Some(HashSet::new()));
-  let rpc = runner::rpc().await;
-  let mut input = runner::get_miner_tx_output(&rpc, &miner_view).await;
-
-  // chain params
-  let fee = rpc.get_fee().await.unwrap();
-  let start = rpc.get_height().await.unwrap() - 59; // -60 input is already grabbed above
-  let protocol = rpc.get_protocol().await.unwrap();
-  let amount = 1000000;
-
-  for (i, addr) in addresses.iter().enumerate() {
-    let mut tx = SignableTransaction::new(
-      protocol,
-      vec![input],
-      vec![(MoneroAddress::from_str(Network::Mainnet, &addr.address).unwrap(), amount)],
-      Some(miner_addr),
-      vec![],
-      fee,
-    )
-    .unwrap()
-    .sign(&mut OsRng, &rpc, &Zeroizing::new(miner_spend))
-    .await
-    .unwrap();
-
-    // submit and unlock tx
-    rpc.publish_transaction(&tx).await.unwrap();
-    runner::mine_until_unlocked(&rpc, &miner_addr.to_string(), tx.hash()).await;
-
-    // get the tx and confirm receipt
-    tx = rpc.get_transaction(tx.hash()).await.unwrap();
-    let output = addr_scanner.scan_transaction(&tx).not_locked().swap_remove(0);
-    assert_eq!(output.commitment().amount, amount);
-    if addr.payment_id.is_some() {
-      assert_eq!(output.metadata.payment_id, addr.payment_id.unwrap());
-    }
-
-    // pick another input to spend for the  next address
-    let block = rpc.get_block(start + i).await.unwrap();
-    input = miner_scanner
-      .scan(&rpc, &block)
-      .await
-      .unwrap()
-      .swap_remove(0)
-      .ignore_timelock()
-      .swap_remove(0);
-  }
-}
-
-async_sequential!(
-  async fn scan_all_standard_addresses() {
-    // scanner
-    let spend_pub = &Scalar::from_bits(ADDR_SPEND) * &ED25519_BASEPOINT_TABLE;
-    let mut scanner = Scanner::from_view(
-      ViewPair::new(spend_pub, Zeroizing::new(Scalar::from_bits(ADDR_VIEW))),
-      Network::Mainnet,
-      Some(HashSet::new()),
-    );
-
-    // generate all types of addresses
-    let payment_ids = vec![
-      [46, 48, 134, 34, 245, 148, 243, 195],
-      [153, 176, 98, 204, 151, 27, 197, 168],
-      [88, 37, 149, 111, 171, 108, 120, 181],
-    ];
-    let mut addresses = vec![];
-    // standard versions
-    addresses.push(AddressInfo { address: scanner.address().to_string(), payment_id: None });
-    addresses
-      .push(AddressInfo { address: scanner.subaddress((0, 1)).to_string(), payment_id: None });
-    addresses.push(AddressInfo {
-      address: scanner.integrated_address(payment_ids[0]).to_string(),
-      payment_id: Some(payment_ids[0]),
-    });
-    // featured versions
-    addresses.push(AddressInfo {
-      address: scanner.featured_address(Some((0, 2)), Some(payment_ids[1]), false).to_string(),
-      payment_id: Some(payment_ids[1]),
-    });
-    addresses.push(AddressInfo {
-      address: scanner.featured_address(None, None, false).to_string(),
-      payment_id: None,
-    });
-    addresses.push(AddressInfo {
-      address: scanner.featured_address(Some((0, 3)), None, false).to_string(),
-      payment_id: None,
-    });
-    addresses.push(AddressInfo {
-      address: scanner.featured_address(None, Some(payment_ids[2]), false).to_string(),
-      payment_id: Some(payment_ids[2]),
-    });
-
-    // send to &  test receive from addresses
-    scan_incomings(&mut scanner, &addresses).await;
-  }
 );
 
-async_sequential!(
+test!(
-  async fn scan_all_guaranteed_addresses() {
+  scan_subaddress,
-    // scanner
+  (
-    let spend_pub = &Scalar::from_bits(ADDR_SPEND) * &ED25519_BASEPOINT_TABLE;
+    |_, mut builder: Builder, _| async move {
-    let mut scanner = Scanner::from_view(
+      let mut scanner =
-      ViewPair::new(spend_pub, Zeroizing::new(Scalar::from_bits(ADDR_VIEW))),
+        Scanner::from_view(runner::random_address().1, Network::Mainnet, Some(HashSet::new()));
-      Network::Mainnet,
+      let subaddress_index = (0, 1);
-      None,
+      builder.add_payment(scanner.subaddress(subaddress_index), 5);
-    );
+      (builder.build().unwrap(), (scanner, subaddress_index))
-
+    },
-    // generate all types of addresses
+    |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move {
-    let payment_ids =
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
-      vec![[88, 37, 149, 111, 171, 108, 120, 181], [125, 69, 155, 152, 140, 160, 157, 186]];
+      assert_eq!(output.commitment().amount, 5);
-    let mut addresses = vec![];
+      assert_eq!(output.metadata.subaddress, state.1);
-    // featured (false, none, true)
+    },
-    addresses.push(AddressInfo { address: scanner.address().to_string(), payment_id: None });
+  ),
-    // featured (false, some, true)
+);
-    addresses.push(AddressInfo {
+
-      address: scanner.integrated_address(payment_ids[0]).to_string(),
+test!(
-      payment_id: Some(payment_ids[0]),
+  scan_integrated_address,
-    });
+  (
-    // featured (true, none, true)
+    |_, mut builder: Builder, _| async move {
-    addresses
+      let scanner =
-      .push(AddressInfo { address: scanner.subaddress((0, 1)).to_string(), payment_id: None });
+        Scanner::from_view(runner::random_address().1, Network::Mainnet, Some(HashSet::new()));
-    // featured (true, some, true)
+      let mut payment_id = [0u8; 8];
-    addresses.push(AddressInfo {
+      OsRng.fill_bytes(&mut payment_id);
-      address: scanner.featured_address(Some((0, 2)), Some(payment_ids[1]), true).to_string(),
+
-      payment_id: Some(payment_ids[1]),
+      builder.add_payment(scanner.integrated_address(payment_id), 5);
-    });
+      (builder.build().unwrap(), (scanner, payment_id))
-
+    },
-    // send to &  test receive from addresses
+    |_, tx: Transaction, _, mut state: (Scanner, [u8; 8])| async move {
-    scan_incomings(&mut scanner, &addresses).await;
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
-  }
+      assert_eq!(output.commitment().amount, 5);
+      assert_eq!(output.metadata.payment_id, state.1);
+    },
+  ),
+);
+
+test!(
+  scan_featured_standard,
+  (
+    |_, mut builder: Builder, _| async move {
+      let mut scanner =
+        Scanner::from_view(runner::random_address().1, Network::Mainnet, Some(HashSet::new()));
+      builder.add_payment(scanner.featured_address(None, None, false), 5);
+      (builder.build().unwrap(), (scanner,))
+    },
+    |_, tx: Transaction, _, mut state: (Scanner,)| async move {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 5);
+    },
+  ),
+);
+
+test!(
+  scan_featured_subaddress,
+  (
+    |_, mut builder: Builder, _| async move {
+      let mut scanner =
+        Scanner::from_view(runner::random_address().1, Network::Mainnet, Some(HashSet::new()));
+      let subaddress_index = (0, 2);
+      builder.add_payment(scanner.featured_address(Some(subaddress_index), None, false), 5);
+      (builder.build().unwrap(), (scanner, subaddress_index))
+    },
+    |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 5);
+      assert_eq!(output.metadata.subaddress, state.1);
+    },
+  ),
+);
+
+test!(
+  scan_featured_integrated,
+  (
+    |_, mut builder: Builder, _| async move {
+      let mut scanner =
+        Scanner::from_view(runner::random_address().1, Network::Mainnet, Some(HashSet::new()));
+      let mut payment_id = [0u8; 8];
+      OsRng.fill_bytes(&mut payment_id);
+
+      builder.add_payment(scanner.featured_address(None, Some(payment_id), false), 5);
+      (builder.build().unwrap(), (scanner, payment_id))
+    },
+    |_, tx: Transaction, _, mut state: (Scanner, [u8; 8])| async move {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 5);
+      assert_eq!(output.metadata.payment_id, state.1);
+    },
+  ),
+);
+
+test!(
+  scan_featured_integrated_subaddress,
+  (
+    |_, mut builder: Builder, _| async move {
+      let mut scanner =
+        Scanner::from_view(runner::random_address().1, Network::Mainnet, Some(HashSet::new()));
+      let subaddress_index = (0, 3);
+
+      let mut payment_id = [0u8; 8];
+      OsRng.fill_bytes(&mut payment_id);
+
+      builder
+        .add_payment(scanner.featured_address(Some(subaddress_index), Some(payment_id), false), 5);
+      (builder.build().unwrap(), (scanner, payment_id, subaddress_index))
+    },
+    |_, tx: Transaction, _, mut state: (Scanner, [u8; 8], (u32, u32))| async move {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 5);
+      assert_eq!(output.metadata.payment_id, state.1);
+      assert_eq!(output.metadata.subaddress, state.2);
+    },
+  ),
+);
+
+test!(
+  scan_guaranteed_standard,
+  (
+    |_, mut builder: Builder, _| async move {
+      let scanner = Scanner::from_view(runner::random_address().1, Network::Mainnet, None);
+
+      builder.add_payment(scanner.address(), 5);
+      (builder.build().unwrap(), (scanner,))
+    },
+    |_, tx: Transaction, _, mut state: (Scanner,)| async move {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 5);
+    },
+  ),
+);
+
+test!(
+  scan_guaranteed_subaddress,
+  (
+    |_, mut builder: Builder, _| async move {
+      let mut scanner = Scanner::from_view(runner::random_address().1, Network::Mainnet, None);
+      let subaddress_index = (0, 1);
+
+      builder.add_payment(scanner.subaddress(subaddress_index), 5);
+      (builder.build().unwrap(), (scanner, subaddress_index))
+    },
+    |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 5);
+      assert_eq!(output.metadata.subaddress, state.1);
+    },
+  ),
+);
+
+test!(
+  scan_guaranteed_integrated,
+  (
+    |_, mut builder: Builder, _| async move {
+      let scanner = Scanner::from_view(runner::random_address().1, Network::Mainnet, None);
+      let mut payment_id = [0u8; 8];
+      OsRng.fill_bytes(&mut payment_id);
+
+      builder.add_payment(scanner.integrated_address(payment_id), 5);
+      (builder.build().unwrap(), (scanner, payment_id))
+    },
+    |_, tx: Transaction, _, mut state: (Scanner, [u8; 8])| async move {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 5);
+      assert_eq!(output.metadata.payment_id, state.1);
+    },
+  ),
+);
+
+test!(
+  scan_guaranteed_integrated_subaddress,
+  (
+    |_, mut builder: Builder, _| async move {
+      let mut scanner = Scanner::from_view(runner::random_address().1, Network::Mainnet, None);
+      let subaddress_index = (0, 2);
+
+      let mut payment_id = [0u8; 8];
+      OsRng.fill_bytes(&mut payment_id);
+
+      builder
+        .add_payment(scanner.featured_address(Some(subaddress_index), Some(payment_id), true), 5);
+      (builder.build().unwrap(), (scanner, payment_id, subaddress_index))
+    },
+    |_, tx: Transaction, _, mut state: (Scanner, [u8; 8], (u32, u32))| async move {
+      let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 5);
+      assert_eq!(output.metadata.payment_id, state.1);
+      assert_eq!(output.metadata.subaddress, state.2);
+    },
+  ),
 );