Fix remaining bugs in monero-wallet tests

coins/monero/primitives/src/lib.rs

@@ -147,7 +147,7 @@ impl Decoys {
   pub fn positions(&self) -> Vec<u64> {
     let mut res = Vec::with_capacity(self.len());
     res.push(self.offsets[0]);
-    for m in 1 .. res.len() {
+    for m in 1 .. self.len() {
       res.push(res[m - 1] + self.offsets[m]);
     }
     res

coins/monero/rpc/src/lib.rs

@@ -788,7 +788,7 @@ pub trait Rpc: Sync + Clone + Debug {
         )
         .await?
     } else {
-      Vec::new()
+      vec![]
     };
 
     // TODO: https://github.com/serai-dex/serai/issues/104
@@ -811,6 +811,7 @@ pub trait Rpc: Sync + Clone + Debug {
         Ok(Some([key, rpc_point(&out.mask)?]).filter(|_| {
           if fingerprintable_canonical {
             // TODO: Are timelock blocks by height or number?
+            // TODO: This doesn't check the default timelock has been passed
             Timelock::Block(height) >= txs[i].prefix().additional_timelock
           } else {
             out.unlocked

coins/monero/wallet/src/send/tx_keys.rs

@@ -12,30 +12,31 @@ use crate::{
   primitives::{keccak256, Commitment},
   ringct::EncryptedAmount,
   SharedKeyDerivations,
-  send::{InternalPayment, SignableTransaction},
+  send::{InternalPayment, SignableTransaction, key_image_sort},
 };
 
-fn seeded_rng(
-  dst: &'static [u8],
-  outgoing_view_key: &Zeroizing<[u8; 32]>,
-  output_keys: impl Iterator<Item = EdwardsPoint>,
-) -> ChaCha20Rng {
-  // Apply the DST
-  let mut transcript = Zeroizing::new(vec![u8::try_from(dst.len()).unwrap()]);
-  transcript.extend(dst);
-  // Bind to the outgoing view key to prevent foreign entities from rebuilding the transcript
-  transcript.extend(outgoing_view_key.as_slice());
-  // Ensure uniqueness across transactions by binding to a use-once object
-  // The output key is also binding to the output's key image, making this use-once
-  for key in output_keys {
-    transcript.extend(key.compress().to_bytes());
-  }
-  ChaCha20Rng::from_seed(keccak256(&transcript))
-}
-
 impl SignableTransaction {
   pub(crate) fn seeded_rng(&self, dst: &'static [u8]) -> ChaCha20Rng {
-    seeded_rng(dst, &self.outgoing_view_key, self.inputs.iter().map(|(input, _)| input.key()))
+    // Apply the DST
+    let mut transcript = Zeroizing::new(vec![u8::try_from(dst.len()).unwrap()]);
+    transcript.extend(dst);
+
+    // Bind to the outgoing view key to prevent foreign entities from rebuilding the transcript
+    transcript.extend(self.outgoing_view_key.as_slice());
+
+    // Ensure uniqueness across transactions by binding to a use-once object
+    // The keys for the inputs is binding to their key images, making them use-once
+    let mut input_keys = self.inputs.iter().map(|(input, _)| input.key()).collect::<Vec<_>>();
+    // We sort the inputs mid-way through TX construction, so apply our own sort to ensure a
+    // consistent order
+    // We use the key image sort as it's applicable and well-defined, not because these are key
+    // images
+    input_keys.sort_by(key_image_sort);
+    for key in input_keys {
+      transcript.extend(key.compress().to_bytes());
+    }
+
+    ChaCha20Rng::from_seed(keccak256(&transcript))
   }
 
   fn has_payments_to_subaddresses(&self) -> bool {

coins/monero/wallet/tests/runner/mod.rs

@@ -286,8 +286,8 @@ macro_rules! test {
               }
             };
 
-            assert_eq!(&eventuality.extra(), &tx.prefix().extra);
-            assert!(eventuality.matches(&tx));
+            assert_eq!(&eventuality.extra(), &tx.prefix().extra, "eventuality extra was distinct");
+            assert!(eventuality.matches(&tx), "eventuality didn't match");
 
             tx
           };

coins/monero/wallet/tests/scan.rs

@@ -72,21 +72,17 @@ test!(
   scan_guaranteed,
   (
     |_, mut builder: Builder, _| async move {
-      let subaddress = SubaddressIndex::new(0, 2).unwrap();
-
       let view = runner::random_guaranteed_address().1;
-      let mut scanner = GuaranteedScanner::new(view.clone());
-      scanner.register_subaddress(subaddress);
-
+      let scanner = GuaranteedScanner::new(view.clone());
       builder.add_payment(view.address(Network::Mainnet, None, None), 5);
-      (builder.build().unwrap(), (scanner, subaddress))
+      (builder.build().unwrap(), scanner)
     },
-    |rpc, block, tx: Transaction, _, mut state: (GuaranteedScanner, SubaddressIndex)| async move {
+    |rpc, block, tx: Transaction, _, mut scanner: GuaranteedScanner| async move {
       let output =
-        state.0.scan(&rpc, &block).await.unwrap().not_additionally_locked().swap_remove(0);
+        scanner.scan(&rpc, &block).await.unwrap().not_additionally_locked().swap_remove(0);
       assert_eq!(output.transaction(), tx.hash());
       assert_eq!(output.commitment().amount, 5);
-      assert_eq!(output.subaddress(), Some(state.1));
+      assert_eq!(output.subaddress(), None);
     },
   ),
 );