Monero: fix decoy selection algo and add test for latest spendable (#384)

* Monero: fix decoy selection algo and add test for latest spendable

- DSA only selected coinbase outputs and didn't match the wallet2
implementation
- Added test to make sure DSA will select a decoy output from the
most recent unlocked block
- Made usage of "height" in DSA consistent with other usage of
"height" in Monero code (height == num blocks in chain)
- Rely on monerod RPC response for output's unlocked status

* xmr runner tests mine until outputs are unlocked

* fingerprintable canoncial select decoys

* Separate fingerprintable canonical function

Makes it simpler for callers who are unconcered with consistent
canonical output selection across multiple clients to rely on
the simpler Decoy::select and not worry about fingerprintable
canonical

* fix merge conflicts

* Put back TODO for issue #104

* Fix incorrect check on distribution len

The RingCT distribution on mainnet doesn't start until well after
genesis, so the distribution length is expected to be < height.

To be clear, this was my mistake from this series of changes
to the DSA. I noticed this mistake because the DSA would error
when running on mainnet.

coins/monero/tests/decoys.rs

@@ -0,0 +1,162 @@
+use monero_serai::{
+  transaction::Transaction,
+  wallet::SpendableOutput,
+  rpc::{Rpc, OutputResponse},
+  Protocol, DEFAULT_LOCK_WINDOW,
+};
+
+mod runner;
+
+test!(
+  select_latest_output_as_decoy_canonical,
+  (
+    // First make an initial tx0
+    |_, mut builder: Builder, addr| async move {
+      builder.add_payment(addr, 2000000000000);
+      (builder.build().unwrap(), ())
+    },
+    |rpc: Rpc<_>, tx: Transaction, mut scanner: Scanner, _| async move {
+      let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 2000000000000);
+      SpendableOutput::from(&rpc, output).await.unwrap()
+    },
+  ),
+  (
+    // Then make a second tx1
+    |protocol: Protocol, rpc: Rpc<_>, mut builder: Builder, addr, state: _| async move {
+      let output_tx0: SpendableOutput = state;
+      let decoys = Decoys::fingerprintable_canonical_select(
+        &mut OsRng,
+        &rpc,
+        protocol.ring_len(),
+        rpc.get_height().await.unwrap(),
+        &[output_tx0.clone()],
+      )
+      .await
+      .unwrap();
+
+      let inputs = [output_tx0.clone()].into_iter().zip(decoys).collect::<Vec<_>>();
+      builder.add_inputs(&inputs);
+      builder.add_payment(addr, 1000000000000);
+
+      (builder.build().unwrap(), (protocol, output_tx0))
+    },
+    // Then make sure DSA selects freshly unlocked output from tx1 as a decoy
+    |rpc: Rpc<_>, tx: Transaction, mut scanner: Scanner, state: (_, _)| async move {
+      use rand_core::OsRng;
+
+      let height = rpc.get_height().await.unwrap();
+
+      let output_tx1 =
+        SpendableOutput::from(&rpc, scanner.scan_transaction(&tx).not_locked().swap_remove(0))
+          .await
+          .unwrap();
+
+      // Make sure output from tx1 is in the block in which it unlocks
+      let out_tx1: OutputResponse =
+        rpc.get_outs(&[output_tx1.global_index]).await.unwrap().swap_remove(0);
+      assert_eq!(out_tx1.height, height - DEFAULT_LOCK_WINDOW);
+      assert!(out_tx1.unlocked);
+
+      // Select decoys using spendable output from tx0 as the real, and make sure DSA selects
+      // the freshly unlocked output from tx1 as a decoy
+      let (protocol, output_tx0): (Protocol, SpendableOutput) = state;
+      let mut selected_fresh_decoy = false;
+      let mut attempts = 1000;
+      while !selected_fresh_decoy && attempts > 0 {
+        let decoys = Decoys::fingerprintable_canonical_select(
+          &mut OsRng, // TODO: use a seeded RNG to consistently select the latest output
+          &rpc,
+          protocol.ring_len(),
+          height,
+          &[output_tx0.clone()],
+        )
+        .await
+        .unwrap();
+
+        selected_fresh_decoy = decoys[0].indexes().contains(&output_tx1.global_index);
+        attempts -= 1;
+      }
+
+      assert!(selected_fresh_decoy);
+      assert_eq!(height, rpc.get_height().await.unwrap());
+    },
+  ),
+);
+
+test!(
+  select_latest_output_as_decoy,
+  (
+    // First make an initial tx0
+    |_, mut builder: Builder, addr| async move {
+      builder.add_payment(addr, 2000000000000);
+      (builder.build().unwrap(), ())
+    },
+    |rpc: Rpc<_>, tx: Transaction, mut scanner: Scanner, _| async move {
+      let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0);
+      assert_eq!(output.commitment().amount, 2000000000000);
+      SpendableOutput::from(&rpc, output).await.unwrap()
+    },
+  ),
+  (
+    // Then make a second tx1
+    |protocol: Protocol, rpc: Rpc<_>, mut builder: Builder, addr, state: _| async move {
+      let output_tx0: SpendableOutput = state;
+      let decoys = Decoys::select(
+        &mut OsRng,
+        &rpc,
+        protocol.ring_len(),
+        rpc.get_height().await.unwrap(),
+        &[output_tx0.clone()],
+      )
+      .await
+      .unwrap();
+
+      let inputs = [output_tx0.clone()].into_iter().zip(decoys).collect::<Vec<_>>();
+      builder.add_inputs(&inputs);
+      builder.add_payment(addr, 1000000000000);
+
+      (builder.build().unwrap(), (protocol, output_tx0))
+    },
+    // Then make sure DSA selects freshly unlocked output from tx1 as a decoy
+    |rpc: Rpc<_>, tx: Transaction, mut scanner: Scanner, state: (_, _)| async move {
+      use rand_core::OsRng;
+
+      let height = rpc.get_height().await.unwrap();
+
+      let output_tx1 =
+        SpendableOutput::from(&rpc, scanner.scan_transaction(&tx).not_locked().swap_remove(0))
+          .await
+          .unwrap();
+
+      // Make sure output from tx1 is in the block in which it unlocks
+      let out_tx1: OutputResponse =
+        rpc.get_outs(&[output_tx1.global_index]).await.unwrap().swap_remove(0);
+      assert_eq!(out_tx1.height, height - DEFAULT_LOCK_WINDOW);
+      assert!(out_tx1.unlocked);
+
+      // Select decoys using spendable output from tx0 as the real, and make sure DSA selects
+      // the freshly unlocked output from tx1 as a decoy
+      let (protocol, output_tx0): (Protocol, SpendableOutput) = state;
+      let mut selected_fresh_decoy = false;
+      let mut attempts = 1000;
+      while !selected_fresh_decoy && attempts > 0 {
+        let decoys = Decoys::select(
+          &mut OsRng, // TODO: use a seeded RNG to consistently select the latest output
+          &rpc,
+          protocol.ring_len(),
+          height,
+          &[output_tx0.clone()],
+        )
+        .await
+        .unwrap();
+
+        selected_fresh_decoy = decoys[0].indexes().contains(&output_tx1.global_index);
+        attempts -= 1;
+      }
+
+      assert!(selected_fresh_decoy);
+      assert_eq!(height, rpc.get_height().await.unwrap());
+    },
+  ),
+);

coins/monero/tests/runner.rs

@@ -17,6 +17,7 @@ use monero_serai::{
     SpendableOutput, Fee,
   },
   transaction::Transaction,
+  DEFAULT_LOCK_WINDOW,
 };
 
 pub fn random_address() -> (Scalar, ViewPair, MoneroAddress) {
@@ -36,7 +37,6 @@ pub fn random_address() -> (Scalar, ViewPair, MoneroAddress) {
 
 // TODO: Support transactions already on-chain
 // TODO: Don't have a side effect of mining blocks more blocks than needed under race conditions
-// TODO: mine as much as needed instead of default 10 blocks
 pub async fn mine_until_unlocked(rpc: &Rpc<HttpRpc>, addr: &str, tx_hash: [u8; 32]) {
   // mine until tx is in a block
   let mut height = rpc.get_height().await.unwrap();
@@ -46,15 +46,23 @@ pub async fn mine_until_unlocked(rpc: &Rpc<HttpRpc>, addr: &str, tx_hash: [u8; 3
     found = match block.txs.iter().find(|&&x| x == tx_hash) {
       Some(_) => true,
       None => {
-        rpc.generate_blocks(addr, 1).await.unwrap();
-        height += 1;
+        height = rpc.generate_blocks(addr, 1).await.unwrap().1 + 1;
         false
       }
     }
   }
 
-  // mine 9 more blocks to unlock the tx
-  rpc.generate_blocks(addr, 9).await.unwrap();
+  // Mine until tx's outputs are unlocked
+  let o_indexes: Vec<u64> = rpc.get_o_indexes(tx_hash).await.unwrap();
+  while rpc
+    .get_outs(&o_indexes)
+    .await
+    .unwrap()
+    .into_iter()
+    .all(|o| (!(o.unlocked && height >= (o.height + DEFAULT_LOCK_WINDOW))))
+  {
+    height = rpc.generate_blocks(addr, 1).await.unwrap().1 + 1;
+  }
 }
 
 // Mines 60 blocks and returns an unlocked miner TX output.
@@ -260,12 +268,12 @@ macro_rules! test {
           let temp = Box::new({
             let mut builder = builder.clone();
 
-            let decoys = Decoys::select(
+            let decoys = Decoys::fingerprintable_canonical_select(
               &mut OsRng,
               &rpc,
               protocol.ring_len(),
-              rpc.get_height().await.unwrap() - 1,
-              &[miner_tx.clone()]
+              rpc.get_height().await.unwrap(),
+              &[miner_tx.clone()],
             )
             .await
             .unwrap();

coins/monero/tests/send.rs

@@ -24,11 +24,11 @@ async fn add_inputs(
     spendable_outputs.push(SpendableOutput::from(rpc, output).await.unwrap());
   }
 
-  let decoys = Decoys::select(
+  let decoys = Decoys::fingerprintable_canonical_select(
     &mut OsRng,
     rpc,
     protocol.ring_len(),
-    rpc.get_height().await.unwrap() - 1,
+    rpc.get_height().await.unwrap(),
     &spendable_outputs,
   )
   .await