Clean decoy selection code

Cargo.lock

@@ -4932,7 +4932,6 @@ dependencies = [
 name = "monero-wallet"
 version = "0.1.0"
 dependencies = [
- "async-trait",
  "curve25519-dalek",
  "dalek-ff-group",
  "flexible-transcript",

coins/monero/primitives/src/lib.rs

@@ -137,13 +137,23 @@ impl Commitment {
 }
 
 /// Decoy data, as used for producing Monero's ring signatures.
-#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)]
+#[derive(Clone, PartialEq, Eq, Zeroize, ZeroizeOnDrop)]
 pub struct Decoys {
   offsets: Vec<u64>,
   signer_index: u8,
   ring: Vec<[EdwardsPoint; 2]>,
 }
 
+impl core::fmt::Debug for Decoys {
+  fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
+    fmt
+      .debug_struct("Decoys")
+      .field("offsets", &self.offsets)
+      .field("ring", &self.ring)
+      .finish_non_exhaustive()
+  }
+}
+
 #[allow(clippy::len_without_is_empty)]
 impl Decoys {
   /// Create a new instance of decoy data.

coins/monero/rpc/src/lib.rs

@@ -910,7 +910,10 @@ pub trait Rpc: Sync + Clone + Debug {
     }
 
     let res: SendRawResponse = self
-      .rpc_call("send_raw_transaction", Some(json!({ "tx_as_hex": hex::encode(tx.serialize()) })))
+      .rpc_call(
+        "send_raw_transaction",
+        Some(json!({ "tx_as_hex": hex::encode(tx.serialize()), "do_sanity_checks": false })),
+      )
       .await?;
 
     if res.status != "OK" {

coins/monero/wallet/Cargo.toml

@@ -18,7 +18,6 @@ workspace = true
 [dependencies]
 std-shims = { path = "../../../common/std-shims", version = "^0.1.1", default-features = false }
 
-async-trait = { version = "0.1", default-features = false }
 thiserror = { version = "1", default-features = false, optional = true }
 
 zeroize = { version = "^1.5", default-features = false, features = ["zeroize_derive"] }

coins/monero/wallet/src/decoys.rs

@@ -1,19 +1,19 @@
-// TODO: Clean this
+use std_shims::{io, vec::Vec, collections::HashSet};
 
-use std_shims::{vec::Vec, collections::HashSet};
-
-use zeroize::Zeroize;
+use zeroize::{Zeroize, ZeroizeOnDrop};
 
 use rand_core::{RngCore, CryptoRng};
 use rand_distr::{Distribution, Gamma};
 #[cfg(not(feature = "std"))]
 use rand_distr::num_traits::Float;
 
-use curve25519_dalek::edwards::EdwardsPoint;
+use curve25519_dalek::{Scalar, EdwardsPoint};
 
 use crate::{
   DEFAULT_LOCK_WINDOW, COINBASE_LOCK_WINDOW, BLOCK_TIME,
+  primitives::{Commitment, Decoys},
   rpc::{RpcError, Rpc},
+  output::OutputData,
   WalletOutput,
 };
 
@@ -22,45 +22,79 @@ const BLOCKS_PER_YEAR: usize = 365 * 24 * 60 * 60 / BLOCK_TIME;
 #[allow(clippy::cast_precision_loss)]
 const TIP_APPLICATION: f64 = (DEFAULT_LOCK_WINDOW * BLOCK_TIME) as f64;
 
-#[allow(clippy::too_many_arguments)]
-async fn select_n<'a, R: RngCore + CryptoRng>(
-  rng: &mut R,
+async fn select_n(
+  rng: &mut (impl RngCore + CryptoRng),
   rpc: &impl Rpc,
-  distribution: &[u64],
   height: usize,
-  high: u64,
-  per_second: f64,
-  real: &[u64],
-  used: &mut HashSet<u64>,
-  count: usize,
+  real_output: u64,
+  ring_len: usize,
   fingerprintable_canonical: bool,
 ) -> Result<Vec<(u64, [EdwardsPoint; 2])>, RpcError> {
-  // TODO: consider removing this extra RPC and expect the caller to handle it
-  if fingerprintable_canonical && (height > rpc.get_height().await?) {
-    // TODO: Don't use InternalError for the caller's failure
-    Err(RpcError::InternalError("decoys being requested from too young blocks".to_string()))?;
+  if height < DEFAULT_LOCK_WINDOW {
+    Err(RpcError::InternalError("not enough blocks to select decoys".to_string()))?;
+  }
+  if height > rpc.get_height().await? {
+    Err(RpcError::InternalError(
+      "decoys being requested from blocks this node doesn't have".to_string(),
+    ))?;
   }
 
-  #[cfg(test)]
+  // Get the distribution
+  let distribution = rpc.get_output_distribution(.. height).await?;
+  let highest_output_exclusive_bound = distribution[distribution.len() - DEFAULT_LOCK_WINDOW];
+  // This assumes that each miner TX had one output (as sane) and checks we have sufficient
+  // outputs even when excluding them (due to their own timelock requirements)
+  // Considering this a temporal error for very new chains, it's sufficiently sane to have
+  if highest_output_exclusive_bound.saturating_sub(u64::try_from(COINBASE_LOCK_WINDOW).unwrap()) <
+    u64::try_from(ring_len).unwrap()
+  {
+    Err(RpcError::InternalError("not enough decoy candidates".to_string()))?;
+  }
+
+  // Determine the outputs per second
+  #[allow(clippy::cast_precision_loss)]
+  let per_second = {
+    let blocks = distribution.len().min(BLOCKS_PER_YEAR);
+    let initial = distribution[distribution.len().saturating_sub(blocks + 1)];
+    let outputs = distribution[distribution.len() - 1].saturating_sub(initial);
+    (outputs as f64) / ((blocks * BLOCK_TIME) as f64)
+  };
+
+  // Don't select the real output
+  let mut do_not_select = HashSet::new();
+  do_not_select.insert(real_output);
+
+  let decoy_count = ring_len - 1;
+  let mut res = Vec::with_capacity(decoy_count);
+
   let mut iters = 0;
-  let mut confirmed = Vec::with_capacity(count);
-  // Retries on failure. Retries are obvious as decoys, yet should be minimal
-  while confirmed.len() != count {
-    let remaining = count - confirmed.len();
-    // TODO: over-request candidates in case some are locked to avoid needing
-    // round trips to the daemon (and revealing obvious decoys to the daemon)
+  // Iterates until we have enough decoys
+  // If an iteration only returns a partial set of decoys, the remainder will be obvious as decoys
+  // to the RPC
+  // The length of that remainder is expected to be minimal
+  while res.len() != decoy_count {
+    iters += 1;
+    #[cfg(not(test))]
+    const MAX_ITERS: usize = 10;
+    // When testing on fresh chains, increased iterations can be useful and we don't necessitate
+    // reasonable performance
+    #[cfg(test)]
+    const MAX_ITERS: usize = 100;
+    // Ensure this isn't infinitely looping
+    // We check both that we aren't at the maximum amount of iterations and that the not-yet
+    // selected candidates exceed the amount of candidates necessary to trigger the next iteration
+    if (iters == MAX_ITERS) ||
+      ((highest_output_exclusive_bound - u64::try_from(do_not_select.len()).unwrap()) <
+        u64::try_from(ring_len).unwrap())
+    {
+      Err(RpcError::InternalError("hit decoy selection round limit".to_string()))?;
+    }
+
+    let remaining = decoy_count - res.len();
     let mut candidates = Vec::with_capacity(remaining);
     while candidates.len() != remaining {
-      #[cfg(test)]
-      {
-        iters += 1;
-        // This is cheap and on fresh chains, a lot of rounds may be needed
-        if iters == 100 {
-          Err(RpcError::InternalError("hit decoy selection round limit".to_string()))?;
-        }
-      }
-
-      // Use a gamma distribution
+      // Use a gamma distribution, as Monero does
+      // TODO: Cite these constants
       let mut age = Gamma::<f64>::new(19.28, 1.0 / 1.61).unwrap().sample(rng).exp();
       #[allow(clippy::cast_precision_loss)]
       if age > TIP_APPLICATION {
@@ -72,16 +106,19 @@ async fn select_n<'a, R: RngCore + CryptoRng>(
 
       #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
       let o = (age * per_second) as u64;
-      if o < high {
-        let i = distribution.partition_point(|s| *s < (high - 1 - o));
+      if o < highest_output_exclusive_bound {
+        // Find which block this points to
+        let i = distribution.partition_point(|s| *s < (highest_output_exclusive_bound - 1 - o));
         let prev = i.saturating_sub(1);
         let n = distribution[i] - distribution[prev];
         if n != 0 {
+          // Select an output from within this block
           let o = distribution[prev] + (rng.next_u64() % n);
-          if !used.contains(&o) {
-            // It will either actually be used, or is unusable and this prevents trying it again
-            used.insert(o);
+          if !do_not_select.contains(&o) {
             candidates.push(o);
+            // This output will either be used or is unusable
+            // In either case, we should not try it again
+            do_not_select.insert(o);
           }
         }
       }
@@ -90,47 +127,39 @@ async fn select_n<'a, R: RngCore + CryptoRng>(
     // If this is the first time we're requesting these outputs, include the real one as well
     // Prevents the node we're connected to from having a list of known decoys and then seeing a
     // TX which uses all of them, with one additional output (the true spend)
-    let mut real_indexes = HashSet::with_capacity(real.len());
-    if confirmed.is_empty() {
-      for real in real {
-        candidates.push(*real);
-      }
+    let real_index = if iters == 0 {
+      candidates.push(real_output);
       // Sort candidates so the real spends aren't the ones at the end
       candidates.sort();
-      for real in real {
-        real_indexes.insert(candidates.binary_search(real).unwrap());
-      }
-    }
+      Some(candidates.binary_search(&real_output).unwrap())
+    } else {
+      None
+    };
 
-    // TODO: make sure that the real output is included in the response, and
-    // that mask and key are equal to expected
     for (i, output) in rpc
       .get_unlocked_outputs(&candidates, height, fingerprintable_canonical)
       .await?
       .iter_mut()
       .enumerate()
     {
-      // Don't include the real spend as a decoy, despite requesting it
-      if real_indexes.contains(&i) {
+      // We could check the returned info is equivalent to our expectations, yet that'd allow the
+      // node to malleate the returned info to see if they can cause this error (allowing them to
+      // figure out the output being spent)
+      //
+      // Some degree of this attack (forcing resampling/trying to observe errors) is likely
+      // always possible
+      if real_index == Some(i) {
         continue;
       }
 
+      // If this is an unlocked output, push it to the result
       if let Some(output) = output.take() {
-        confirmed.push((candidates[i], output));
+        res.push((candidates[i], output));
       }
     }
   }
 
-  Ok(confirmed)
-}
-
-fn offset(ring: &[u64]) -> Vec<u64> {
-  let mut res = vec![ring[0]];
-  res.resize(ring.len(), 0);
-  for m in (1 .. ring.len()).rev() {
-    res[m] = ring[m] - ring[m - 1];
-  }
-  res
+  Ok(res)
 }
 
 async fn select_decoys<R: RngCore + CryptoRng>(
@@ -138,199 +167,142 @@ async fn select_decoys<R: RngCore + CryptoRng>(
   rpc: &impl Rpc,
   ring_len: usize,
   height: usize,
-  inputs: &[WalletOutput],
+  input: &WalletOutput,
+  // TODO: Decide "canonical" or "deterministic" (updating RPC terminology accordingly)
   fingerprintable_canonical: bool,
-) -> Result<Vec<Decoys>, RpcError> {
-  let mut distribution = vec![];
-
-  let decoy_count = ring_len - 1;
-
-  // Convert the inputs in question to the raw output data
-  let mut real = Vec::with_capacity(inputs.len());
-  let mut outputs = Vec::with_capacity(inputs.len());
-  for input in inputs {
-    real.push(input.relative_id.index_on_blockchain);
-    outputs.push((real[real.len() - 1], [input.key(), input.commitment().calculate()]));
-  }
-
-  if distribution.len() < height {
-    // TODO: verify distribution elems are strictly increasing
-    let extension = rpc.get_output_distribution(distribution.len() .. height).await?;
-    distribution.extend(extension);
-  }
-  // If asked to use an older height than previously asked, truncate to ensure accuracy
-  // Should never happen, yet risks desyncing if it did
-  distribution.truncate(height);
-
-  if distribution.len() < DEFAULT_LOCK_WINDOW {
-    Err(RpcError::InternalError("not enough blocks to select decoys".to_string()))?;
-  }
-
-  #[allow(clippy::cast_precision_loss)]
-  let per_second = {
-    let blocks = distribution.len().min(BLOCKS_PER_YEAR);
-    let initial = distribution[distribution.len().saturating_sub(blocks + 1)];
-    let outputs = distribution[distribution.len() - 1].saturating_sub(initial);
-    (outputs as f64) / ((blocks * BLOCK_TIME) as f64)
-  };
-
-  let mut used = HashSet::<u64>::new();
-  for o in &outputs {
-    used.insert(o.0);
-  }
-
-  // TODO: Create a TX with less than the target amount, as allowed by the protocol
-  let high = distribution[distribution.len() - DEFAULT_LOCK_WINDOW];
-  // This assumes that each miner TX had one output (as sane) and checks we have sufficient
-  // outputs even when excluding them (due to their own timelock requirements)
-  if high.saturating_sub(u64::try_from(COINBASE_LOCK_WINDOW).unwrap()) <
-    u64::try_from(inputs.len() * ring_len).unwrap()
-  {
-    Err(RpcError::InternalError("not enough decoy candidates".to_string()))?;
-  }
-
+) -> Result<Decoys, RpcError> {
   // Select all decoys for this transaction, assuming we generate a sane transaction
   // We should almost never naturally generate an insane transaction, hence why this doesn't
   // bother with an overage
-  let mut decoys = select_n(
+  let decoys = select_n(
     rng,
     rpc,
-    &distribution,
     height,
-    high,
-    per_second,
-    &real,
-    &mut used,
-    inputs.len() * decoy_count,
+    input.relative_id.index_on_blockchain,
+    ring_len,
     fingerprintable_canonical,
   )
   .await?;
-  real.zeroize();
 
-  let mut res = Vec::with_capacity(inputs.len());
-  for o in outputs {
-    // Grab the decoys for this specific output
-    let mut ring = decoys.drain((decoys.len() - decoy_count) ..).collect::<Vec<_>>();
-    ring.push(o);
-    ring.sort_by(|a, b| a.0.cmp(&b.0));
+  // Form the complete ring
+  let mut ring = decoys;
+  ring.push((input.relative_id.index_on_blockchain, [input.key(), input.commitment().calculate()]));
+  ring.sort_by(|a, b| a.0.cmp(&b.0));
 
-    // Sanity checks are only run when 1000 outputs are available in Monero
-    // We run this check whenever the highest output index, which we acknowledge, is > 500
-    // This means we assume (for presumably test blockchains) the height being used has not had
-    // 500 outputs since while itself not being a sufficiently mature blockchain
-    // Considering Monero's p2p layer doesn't actually check transaction sanity, it should be
-    // fine for us to not have perfectly matching rules, especially since this code will infinite
-    // loop if it can't determine sanity, which is possible with sufficient inputs on
-    // sufficiently small chains
-    if high > 500 {
-      // Make sure the TX passes the sanity check that the median output is within the last 40%
-      let target_median = high * 3 / 5;
-      while ring[ring_len / 2].0 < target_median {
-        // If it's not, update the bottom half with new values to ensure the median only moves up
-        for removed in ring.drain(0 .. (ring_len / 2)).collect::<Vec<_>>() {
-          // If we removed the real spend, add it back
-          if removed.0 == o.0 {
-            ring.push(o);
-          } else {
-            // We could not remove this, saving CPU time and removing low values as
-            // possibilities, yet it'd increase the amount of decoys required to create this
-            // transaction and some removed outputs may be the best option (as we drop the first
-            // half, not just the bottom n)
-            used.remove(&removed.0);
-          }
-        }
+  /*
+    Monero does have sanity checks which it applies to the selected ring.
 
-        // Select new outputs until we have a full sized ring again
-        ring.extend(
-          select_n(
-            rng,
-            rpc,
-            &distribution,
-            height,
-            high,
-            per_second,
-            &[],
-            &mut used,
-            ring_len - ring.len(),
-            fingerprintable_canonical,
-          )
-          .await?,
-        );
-        ring.sort_by(|a, b| a.0.cmp(&b.0));
-      }
+    They're statistically unlikely to be hit and only occur when the transaction is published over
+    the RPC (so they are not a relay rule). The RPC allows disabling them, which monero-rpc does to
+    ensure they don't pose a problem.
 
-      // The other sanity check rule is about duplicates, yet we already enforce unique ring
-      // members
+    They aren't worth the complexity to implement here, especially since they're non-deterministic.
+  */
+
+  // We need to convert our positional indexes to offset indexes
+  let mut offsets = Vec::with_capacity(ring.len());
+  {
+    offsets.push(ring[0].0);
+    for m in 1 .. ring.len() {
+      offsets.push(ring[m].0 - ring[m - 1].0);
     }
-
-    res.push(
-      Decoys::new(
-        offset(&ring.iter().map(|output| output.0).collect::<Vec<_>>()),
-        // Binary searches for the real spend since we don't know where it sorted to
-        u8::try_from(ring.partition_point(|x| x.0 < o.0)).unwrap(),
-        ring.iter().map(|output| output.1).collect(),
-      )
-      .unwrap(),
-    );
   }
 
-  Ok(res)
+  Ok(
+    Decoys::new(
+      offsets,
+      // Binary searches for the real spend since we don't know where it sorted to
+      u8::try_from(ring.partition_point(|x| x.0 < input.relative_id.index_on_blockchain)).unwrap(),
+      ring.into_iter().map(|output| output.1).collect(),
+    )
+    .unwrap(),
+  )
 }
 
-pub use monero_serai::primitives::Decoys;
+/// An output with decoys selected.
+#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)]
+pub struct OutputWithDecoys {
+  output: OutputData,
+  decoys: Decoys,
+}
 
-// TODO: Remove this trait
-/// TODO: Document
-#[cfg(feature = "std")]
-#[async_trait::async_trait]
-pub trait DecoySelection {
-  /// Select decoys using the same distribution as Monero. Relies on the monerod RPC
-  /// response for an output's unlocked status, minimizing trips to the daemon.
-  async fn select<R: Send + Sync + RngCore + CryptoRng>(
-    rng: &mut R,
+impl OutputWithDecoys {
+  /// Select decoys for this output.
+  pub async fn new(
+    rng: &mut (impl Send + Sync + RngCore + CryptoRng),
     rpc: &impl Rpc,
     ring_len: usize,
     height: usize,
-    inputs: &[WalletOutput],
-  ) -> Result<Vec<Decoys>, RpcError>;
+    output: WalletOutput,
+  ) -> Result<OutputWithDecoys, RpcError> {
+    let decoys = select_decoys(rng, rpc, ring_len, height, &output, false).await?;
+    Ok(OutputWithDecoys { output: output.data.clone(), decoys })
+  }
 
-  /// If no reorg has occurred and an honest RPC, any caller who passes the same height to this
-  /// function will use the same distribution to select decoys. It is fingerprintable
-  /// because a caller using this will not be able to select decoys that are timelocked
-  /// with a timestamp. Any transaction which includes timestamp timelocked decoys in its
-  /// rings could not be constructed using this function.
+  /// Select a set of decoys for this output with a deterministic process.
   ///
-  /// TODO: upstream change to monerod get_outs RPC to accept a height param for checking
-  /// output's unlocked status and remove all usage of fingerprintable_canonical
-  async fn fingerprintable_canonical_select<R: Send + Sync + RngCore + CryptoRng>(
-    rng: &mut R,
+  /// This function will always output the same set of decoys when called with the same arguments.
+  /// This makes it very useful in multisignature contexts, where instead of having one participant
+  /// select the decoys, everyone can locally select the decoys while coming to the same result.
+  ///
+  /// The set of decoys selected may be fingerprintable as having been produced by this
+  /// methodology.
+  pub async fn fingerprintable_deterministic_new(
+    rng: &mut (impl Send + Sync + RngCore + CryptoRng),
     rpc: &impl Rpc,
     ring_len: usize,
     height: usize,
-    inputs: &[WalletOutput],
-  ) -> Result<Vec<Decoys>, RpcError>;
-}
-
-#[cfg(feature = "std")]
-#[async_trait::async_trait]
-impl DecoySelection for Decoys {
-  async fn select<R: Send + Sync + RngCore + CryptoRng>(
-    rng: &mut R,
-    rpc: &impl Rpc,
-    ring_len: usize,
-    height: usize,
-    inputs: &[WalletOutput],
-  ) -> Result<Vec<Decoys>, RpcError> {
-    select_decoys(rng, rpc, ring_len, height, inputs, false).await
+    output: WalletOutput,
+  ) -> Result<OutputWithDecoys, RpcError> {
+    let decoys = select_decoys(rng, rpc, ring_len, height, &output, true).await?;
+    Ok(OutputWithDecoys { output: output.data.clone(), decoys })
   }
 
-  async fn fingerprintable_canonical_select<R: Send + Sync + RngCore + CryptoRng>(
-    rng: &mut R,
-    rpc: &impl Rpc,
-    ring_len: usize,
-    height: usize,
-    inputs: &[WalletOutput],
-  ) -> Result<Vec<Decoys>, RpcError> {
-    select_decoys(rng, rpc, ring_len, height, inputs, true).await
+  /// The key this output may be spent by.
+  pub fn key(&self) -> EdwardsPoint {
+    self.output.key()
+  }
+
+  /// The scalar to add to the private spend key for it to be the discrete logarithm of this
+  /// output's key.
+  pub fn key_offset(&self) -> Scalar {
+    self.output.key_offset
+  }
+
+  /// The commitment this output created.
+  pub fn commitment(&self) -> &Commitment {
+    &self.output.commitment
+  }
+
+  /// The decoys this output selected.
+  pub fn decoys(&self) -> &Decoys {
+    &self.decoys
+  }
+
+  /// Write the OutputWithDecoys.
+  ///
+  /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
+  /// defined serialization.
+  pub fn write<W: io::Write>(&self, w: &mut W) -> io::Result<()> {
+    self.output.write(w)?;
+    self.decoys.write(w)
+  }
+
+  /// Serialize the OutputWithDecoys to a `Vec<u8>`.
+  ///
+  /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
+  /// defined serialization.
+  pub fn serialize(&self) -> Vec<u8> {
+    let mut serialized = Vec::with_capacity(128);
+    self.write(&mut serialized).unwrap();
+    serialized
+  }
+
+  /// Read an OutputWithDecoys.
+  ///
+  /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
+  /// defined serialization.
+  pub fn read<R: io::Read>(r: &mut R) -> io::Result<Self> {
+    Ok(Self { output: OutputData::read(r)?, decoys: Decoys::read(r)? })
   }
 }

coins/monero/wallet/src/lib.rs

@@ -35,15 +35,8 @@ pub use output::WalletOutput;
 mod scan;
 pub use scan::{Scanner, GuaranteedScanner};
 
-#[cfg(feature = "std")]
 mod decoys;
-#[cfg(not(feature = "std"))]
-mod decoys {
-  pub use monero_serai::primitives::Decoys;
-  /// TODO: Document/remove
-  pub trait DecoySelection {}
-}
-pub use decoys::{DecoySelection, Decoys};
+pub use decoys::OutputWithDecoys;
 
 /// Structs and functionality for sending transactions.
 pub mod send;

coins/monero/wallet/src/output.rs

@@ -52,21 +52,15 @@ impl AbsoluteId {
 
 /// An output's relative ID.
 ///
-/// This id defined as the block which contains the transaction creating the output and the
-/// output's index on the blockchain.
+/// This is defined as the output's index on the blockchain.
 #[derive(Clone, PartialEq, Eq, Zeroize, ZeroizeOnDrop)]
 pub(crate) struct RelativeId {
-  pub(crate) block: [u8; 32],
   pub(crate) index_on_blockchain: u64,
 }
 
 impl core::fmt::Debug for RelativeId {
   fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
-    fmt
-      .debug_struct("RelativeId")
-      .field("block", &hex::encode(self.block))
-      .field("index_on_blockchain", &self.index_on_blockchain)
-      .finish()
+    fmt.debug_struct("RelativeId").field("index_on_blockchain", &self.index_on_blockchain).finish()
   }
 }
 
@@ -76,7 +70,6 @@ impl RelativeId {
   /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
   /// defined serialization.
   fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
-    w.write_all(&self.block)?;
     w.write_all(&self.index_on_blockchain.to_le_bytes())
   }
 
@@ -85,18 +78,16 @@ impl RelativeId {
   /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
   /// defined serialization.
   fn read<R: Read>(r: &mut R) -> io::Result<Self> {
-    Ok(RelativeId { block: read_bytes(r)?, index_on_blockchain: read_u64(r)? })
+    Ok(RelativeId { index_on_blockchain: read_u64(r)? })
   }
 }
 
-/// The data within an output as necessary to spend an output, and the output's additional
-/// timelock.
+/// The data within an output, as necessary to spend the output.
 #[derive(Clone, PartialEq, Eq, Zeroize, ZeroizeOnDrop)]
 pub(crate) struct OutputData {
   pub(crate) key: EdwardsPoint,
   pub(crate) key_offset: Scalar,
   pub(crate) commitment: Commitment,
-  pub(crate) additional_timelock: Timelock,
 }
 
 impl core::fmt::Debug for OutputData {
@@ -106,33 +97,55 @@ impl core::fmt::Debug for OutputData {
       .field("key", &hex::encode(self.key.compress().0))
       .field("key_offset", &hex::encode(self.key_offset.to_bytes()))
       .field("commitment", &self.commitment)
-      .field("additional_timelock", &self.additional_timelock)
       .finish()
   }
 }
 
 impl OutputData {
-  // Write the OutputData.
+  /// The key this output may be spent by.
+  pub(crate) fn key(&self) -> EdwardsPoint {
+    self.key
+  }
+
+  /// The scalar to add to the private spend key for it to be the discrete logarithm of this
+  /// output's key.
+  pub(crate) fn key_offset(&self) -> Scalar {
+    self.key_offset
+  }
+
+  /// The commitment this output created.
+  pub(crate) fn commitment(&self) -> &Commitment {
+    &self.commitment
+  }
+
+  /// Write the OutputData.
   ///
   /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
   /// defined serialization.
-  fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
+  pub(crate) fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
     w.write_all(&self.key.compress().to_bytes())?;
     w.write_all(&self.key_offset.to_bytes())?;
-    self.commitment.write(w)?;
-    self.additional_timelock.write(w)
+    self.commitment.write(w)
   }
 
+  /*
+  /// Serialize the OutputData to a `Vec<u8>`.
+  pub fn serialize(&self) -> Vec<u8> {
+    let mut res = Vec::with_capacity(32 + 32 + 40);
+    self.write(&mut res).unwrap();
+    res
+  }
+  */
+
   /// Read an OutputData.
   ///
   /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
   /// defined serialization.
-  fn read<R: Read>(r: &mut R) -> io::Result<OutputData> {
+  pub(crate) fn read<R: Read>(r: &mut R) -> io::Result<OutputData> {
     Ok(OutputData {
       key: read_point(r)?,
       key_offset: read_scalar(r)?,
       commitment: Commitment::read(r)?,
-      additional_timelock: Timelock::read(r)?,
     })
   }
 }
@@ -140,6 +153,7 @@ impl OutputData {
 /// The metadata for an output.
 #[derive(Clone, PartialEq, Eq, Zeroize, ZeroizeOnDrop)]
 pub(crate) struct Metadata {
+  pub(crate) additional_timelock: Timelock,
   pub(crate) subaddress: Option<SubaddressIndex>,
   pub(crate) payment_id: Option<PaymentId>,
   pub(crate) arbitrary_data: Vec<Vec<u8>>,
@@ -149,6 +163,7 @@ impl core::fmt::Debug for Metadata {
   fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
     fmt
       .debug_struct("Metadata")
+      .field("additional_timelock", &self.additional_timelock)
       .field("subaddress", &self.subaddress)
       .field("payment_id", &self.payment_id)
       .field("arbitrary_data", &self.arbitrary_data.iter().map(hex::encode).collect::<Vec<_>>())
@@ -162,6 +177,8 @@ impl Metadata {
   /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
   /// defined serialization.
   fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
+    self.additional_timelock.write(w)?;
+
     if let Some(subaddress) = self.subaddress {
       w.write_all(&[1])?;
       w.write_all(&subaddress.account().to_le_bytes())?;
@@ -190,6 +207,8 @@ impl Metadata {
   /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
   /// defined serialization.
   fn read<R: Read>(r: &mut R) -> io::Result<Metadata> {
+    let additional_timelock = Timelock::read(r)?;
+
     let subaddress = match read_byte(r)? {
       0 => None,
       1 => Some(
@@ -200,6 +219,7 @@ impl Metadata {
     };
 
     Ok(Metadata {
+      additional_timelock,
       subaddress,
       payment_id: if read_byte(r)? == 1 { PaymentId::read(r).ok() } else { None },
       arbitrary_data: {
@@ -214,7 +234,7 @@ impl Metadata {
   }
 }
 
-/// A received output.
+/// A scanned output and all associated data.
 ///
 /// This struct contains all data necessary to spend this output, or handle it as a payment.
 ///
@@ -244,11 +264,6 @@ impl WalletOutput {
     self.absolute_id.index_in_transaction
   }
 
-  /// The block containing the transaction which created this output.
-  pub fn block(&self) -> [u8; 32] {
-    self.relative_id.block
-  }
-
   /// The index of the output on the blockchain.
   pub fn index_on_blockchain(&self) -> u64 {
     self.relative_id.index_on_blockchain
@@ -256,18 +271,18 @@ impl WalletOutput {
 
   /// The key this output may be spent by.
   pub fn key(&self) -> EdwardsPoint {
-    self.data.key
+    self.data.key()
   }
 
   /// The scalar to add to the private spend key for it to be the discrete logarithm of this
   /// output's key.
   pub fn key_offset(&self) -> Scalar {
-    self.data.key_offset
+    self.data.key_offset()
   }
 
   /// The commitment this output created.
   pub fn commitment(&self) -> &Commitment {
-    &self.data.commitment
+    self.data.commitment()
   }
 
   /// The additional timelock this output is subject to.
@@ -276,7 +291,7 @@ impl WalletOutput {
   /// on-chain during which they cannot be spent. Outputs may be additionally timelocked. This
   /// function only returns the additional timelock.
   pub fn additional_timelock(&self) -> Timelock {
-    self.data.additional_timelock
+    self.metadata.additional_timelock
   }
 
   /// The index of the subaddress this output was identified as sent to.

coins/monero/wallet/src/scan.rs

@@ -107,7 +107,6 @@ impl InternalScanner {
 
   fn scan_transaction(
     &self,
-    block_hash: [u8; 32],
     tx_start_index_on_blockchain: u64,
     tx: &Transaction,
   ) -> Result<Timelocked, RpcError> {
@@ -224,16 +223,15 @@ impl InternalScanner {
             index_in_transaction: o.try_into().unwrap(),
           },
           relative_id: RelativeId {
-            block: block_hash,
             index_on_blockchain: tx_start_index_on_blockchain + u64::try_from(o).unwrap(),
           },
-          data: OutputData {
-            key: output_key,
-            key_offset,
-            commitment,
+          data: OutputData { key: output_key, key_offset, commitment },
+          metadata: Metadata {
             additional_timelock: tx.prefix().additional_timelock,
+            subaddress,
+            payment_id,
+            arbitrary_data: extra.data(),
           },
-          metadata: Metadata { subaddress, payment_id, arbitrary_data: extra.data() },
         });
 
         // Break to prevent public keys from being included multiple times, triggering multiple
@@ -253,8 +251,6 @@ impl InternalScanner {
       )))?;
     }
 
-    let block_hash = block.hash();
-
     // We obtain all TXs in full
     let mut txs = vec![block.miner_transaction.clone()];
     txs.extend(rpc.get_transactions(&block.transactions).await?);
@@ -327,7 +323,7 @@ impl InternalScanner {
       {
         let mut this_txs_outputs = vec![];
         core::mem::swap(
-          &mut self.scan_transaction(block_hash, tx_start_index_on_blockchain, &tx)?.0,
+          &mut self.scan_transaction(tx_start_index_on_blockchain, &tx)?.0,
           &mut this_txs_outputs,
         );
         res.0.extend(this_txs_outputs);
@@ -379,27 +375,6 @@ impl Scanner {
     self.0.register_subaddress(subaddress)
   }
 
-  /*
-  /// Scan a transaction.
-  ///
-  /// This takes in the block hash the transaction is contained in. This method is NOT recommended
-  /// and MUST be used carefully. The node will receive a request for the output indexes of the
-  /// specified transactions, which may de-anonymize which transactions belong to a user.
-  pub async fn scan_transaction(
-    &self,
-    rpc: &impl Rpc,
-    block_hash: [u8; 32],
-    tx: &Transaction,
-  ) -> Result<Timelocked, RpcError> {
-    // This isn't technically illegal due to a lack of minimum output rules for a while
-    let Some(tx_start_index_on_blockchain) =
-      rpc.get_o_indexes(tx.hash()).await?.first().copied() else {
-        return Ok(Timelocked(vec![]))
-      };
-    self.0.scan_transaction(block_hash, tx_start_index_on_blockchain, tx)
-  }
-  */
-
   /// Scan a block.
   pub async fn scan(&mut self, rpc: &impl Rpc, block: &Block) -> Result<Timelocked, RpcError> {
     self.0.scan(rpc, block).await
@@ -429,27 +404,6 @@ impl GuaranteedScanner {
     self.0.register_subaddress(subaddress)
   }
 
-  /*
-  /// Scan a transaction.
-  ///
-  /// This takes in the block hash the transaction is contained in. This method is NOT recommended
-  /// and MUST be used carefully. The node will receive a request for the output indexes of the
-  /// specified transactions, which may de-anonymize which transactions belong to a user.
-  pub async fn scan_transaction(
-    &self,
-    rpc: &impl Rpc,
-    block_hash: [u8; 32],
-    tx: &Transaction,
-  ) -> Result<Timelocked, RpcError> {
-    // This isn't technically illegal due to a lack of minimum output rules for a while
-    let Some(tx_start_index_on_blockchain) =
-      rpc.get_o_indexes(tx.hash()).await?.first().copied() else {
-        return Ok(Timelocked(vec![]))
-      };
-    self.0.scan_transaction(block_hash, tx_start_index_on_blockchain, tx)
-  }
-  */
-
   /// Scan a block.
   pub async fn scan(&mut self, rpc: &impl Rpc, block: &Block) -> Result<Timelocked, RpcError> {
     self.0.scan(rpc, block).await

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

@@ -17,7 +17,6 @@ use frost::FrostError;
 use crate::{
   io::*,
   generators::{MAX_COMMITMENTS, hash_to_point},
-  primitives::Decoys,
   ringct::{
     clsag::{ClsagError, ClsagContext, Clsag},
     RctType, RctPrunable, RctProofs,
@@ -26,7 +25,7 @@ use crate::{
   extra::MAX_ARBITRARY_DATA_SIZE,
   address::{Network, MoneroAddress},
   rpc::FeeRate,
-  ViewPair, GuaranteedViewPair, WalletOutput,
+  ViewPair, GuaranteedViewPair, OutputWithDecoys,
 };
 
 mod tx_keys;
@@ -231,7 +230,7 @@ pub enum SendError {
 pub struct SignableTransaction {
   rct_type: RctType,
   outgoing_view_key: Zeroizing<[u8; 32]>,
-  inputs: Vec<(WalletOutput, Decoys)>,
+  inputs: Vec<OutputWithDecoys>,
   payments: Vec<InternalPayment>,
   data: Vec<Vec<u8>>,
   fee_rate: FeeRate,
@@ -252,9 +251,9 @@ impl SignableTransaction {
     if self.inputs.is_empty() {
       Err(SendError::NoInputs)?;
     }
-    for (_, decoys) in &self.inputs {
+    for input in &self.inputs {
       // TODO: Add a function for the ring length
-      if decoys.len() !=
+      if input.decoys().len() !=
         match self.rct_type {
           RctType::ClsagBulletproof => 11,
           RctType::ClsagBulletproofPlus => 16,
@@ -314,7 +313,7 @@ impl SignableTransaction {
     }
 
     // Make sure we have enough funds
-    let in_amount = self.inputs.iter().map(|(input, _)| input.commitment().amount).sum::<u64>();
+    let in_amount = self.inputs.iter().map(|input| input.commitment().amount).sum::<u64>();
     let payments_amount = self
       .payments
       .iter()
@@ -356,7 +355,7 @@ impl SignableTransaction {
   pub fn new(
     rct_type: RctType,
     outgoing_view_key: Zeroizing<[u8; 32]>,
-    inputs: Vec<(WalletOutput, Decoys)>,
+    inputs: Vec<OutputWithDecoys>,
     payments: Vec<(MoneroAddress, u64)>,
     change: Change,
     data: Vec<Vec<u8>>,
@@ -406,11 +405,6 @@ impl SignableTransaction {
   /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
   /// defined serialization.
   pub fn write<W: io::Write>(&self, w: &mut W) -> io::Result<()> {
-    fn write_input<W: io::Write>(input: &(WalletOutput, Decoys), w: &mut W) -> io::Result<()> {
-      input.0.write(w)?;
-      input.1.write(w)
-    }
-
     fn write_payment<W: io::Write>(payment: &InternalPayment, w: &mut W) -> io::Result<()> {
       match payment {
         InternalPayment::Payment(addr, amount) => {
@@ -433,7 +427,7 @@ impl SignableTransaction {
 
     write_byte(&u8::from(self.rct_type), w)?;
     w.write_all(self.outgoing_view_key.as_slice())?;
-    write_vec(write_input, &self.inputs, w)?;
+    write_vec(OutputWithDecoys::write, &self.inputs, w)?;
     write_vec(write_payment, &self.payments, w)?;
     write_vec(|data, w| write_vec(write_byte, data, w), &self.data, w)?;
     self.fee_rate.write(w)
@@ -454,10 +448,6 @@ impl SignableTransaction {
   /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
   /// defined serialization.
   pub fn read<R: io::Read>(r: &mut R) -> io::Result<SignableTransaction> {
-    fn read_input(r: &mut impl io::Read) -> io::Result<(WalletOutput, Decoys)> {
-      Ok((WalletOutput::read(r)?, Decoys::read(r)?))
-    }
-
     fn read_address<R: io::Read>(r: &mut R) -> io::Result<MoneroAddress> {
       String::from_utf8(read_vec(read_byte, r)?)
         .ok()
@@ -484,7 +474,7 @@ impl SignableTransaction {
       rct_type: RctType::try_from(read_byte(r)?)
         .map_err(|()| io::Error::other("unsupported/invalid RctType"))?,
       outgoing_view_key: Zeroizing::new(read_bytes(r)?),
-      inputs: read_vec(read_input, r)?,
+      inputs: read_vec(OutputWithDecoys::read, r)?,
       payments: read_vec(read_payment, r)?,
       data: read_vec(|r| read_vec(read_byte, r), r)?,
       fee_rate: FeeRate::read(r)?,
@@ -522,7 +512,7 @@ impl SignableTransaction {
   ) -> Result<Transaction, SendError> {
     // Calculate the key images
     let mut key_images = vec![];
-    for (input, _) in &self.inputs {
+    for input in &self.inputs {
       let input_key = Zeroizing::new(sender_spend_key.deref() + input.key_offset());
       if (input_key.deref() * ED25519_BASEPOINT_TABLE) != input.key() {
         Err(SendError::WrongPrivateKey)?;
@@ -536,12 +526,12 @@ impl SignableTransaction {
 
     // Prepare the CLSAG signatures
     let mut clsag_signs = Vec::with_capacity(tx.intent.inputs.len());
-    for (input, decoys) in &tx.intent.inputs {
+    for input in &tx.intent.inputs {
       // Re-derive the input key as this will be in a different order
       let input_key = Zeroizing::new(sender_spend_key.deref() + input.key_offset());
       clsag_signs.push((
         input_key,
-        ClsagContext::new(decoys.clone(), input.commitment().clone())
+        ClsagContext::new(input.decoys().clone(), input.commitment().clone())
           .map_err(SendError::ClsagError)?,
       ));
     }

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

@@ -65,14 +65,14 @@ impl SignableTransaction {
     let mut clsags = vec![];
 
     let mut key_image_generators_and_offsets = vec![];
-    for (i, (input, decoys)) in self.inputs.iter().enumerate() {
+    for input in &self.inputs {
       // Check this is the right set of keys
       let offset = keys.offset(dfg::Scalar(input.key_offset()));
       if offset.group_key().0 != input.key() {
         Err(SendError::WrongPrivateKey)?;
       }
 
-      let context = ClsagContext::new(decoys.clone(), input.commitment().clone())
+      let context = ClsagContext::new(input.decoys().clone(), input.commitment().clone())
         .map_err(SendError::ClsagError)?;
       let (clsag, clsag_mask_send) = ClsagMultisig::new(
         RecommendedTranscript::new(b"Monero Multisignature Transaction"),
@@ -80,7 +80,7 @@ impl SignableTransaction {
       );
       key_image_generators_and_offsets.push((
         clsag.key_image_generator(),
-        keys.current_offset().unwrap_or(dfg::Scalar::ZERO).0 + self.inputs[i].0.key_offset(),
+        keys.current_offset().unwrap_or(dfg::Scalar::ZERO).0 + input.key_offset(),
       ));
       clsags.push((clsag_mask_send, AlgorithmMachine::new(clsag, offset)));
     }

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

@@ -23,10 +23,10 @@ impl SignableTransaction {
     debug_assert_eq!(self.inputs.len(), key_images.len());
 
     let mut res = Vec::with_capacity(self.inputs.len());
-    for ((_, decoys), key_image) in self.inputs.iter().zip(key_images) {
+    for (input, key_image) in self.inputs.iter().zip(key_images) {
       res.push(Input::ToKey {
         amount: None,
-        key_offsets: decoys.offsets().to_vec(),
+        key_offsets: input.decoys().offsets().to_vec(),
         key_image: *key_image,
       });
     }
@@ -299,7 +299,7 @@ impl SignableTransactionWithKeyImages {
           } else {
             // If we don't have a change output, the difference is the fee
             let inputs =
-              self.intent.inputs.iter().map(|input| input.0.commitment().amount).sum::<u64>();
+              self.intent.inputs.iter().map(|input| input.commitment().amount).sum::<u64>();
             let payments = self
               .intent
               .payments

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

@@ -11,7 +11,7 @@ use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, Scalar, EdwardsPoint}
 use crate::{
   primitives::{keccak256, Commitment},
   ringct::EncryptedAmount,
-  SharedKeyDerivations,
+  SharedKeyDerivations, OutputWithDecoys,
   send::{InternalPayment, SignableTransaction, key_image_sort},
 };
 
@@ -26,7 +26,7 @@ impl SignableTransaction {
 
     // 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<_>>();
+    let mut input_keys = self.inputs.iter().map(OutputWithDecoys::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
@@ -208,7 +208,7 @@ impl SignableTransaction {
       let amount = match payment {
         InternalPayment::Payment(_, amount) => *amount,
         InternalPayment::Change(_, _) => {
-          let inputs = self.inputs.iter().map(|input| input.0.commitment().amount).sum::<u64>();
+          let inputs = self.inputs.iter().map(|input| input.commitment().amount).sum::<u64>();
           let payments = self
             .payments
             .iter()

coins/monero/wallet/tests/decoys.rs

@@ -28,18 +28,17 @@ test!(
     // Then make a second tx1
     |rct_type: RctType, rpc: SimpleRequestRpc, mut builder: Builder, addr, state: _| async move {
       let output_tx0: WalletOutput = state;
-      let decoys = Decoys::fingerprintable_canonical_select(
+
+      let input = OutputWithDecoys::fingerprintable_deterministic_new(
         &mut OsRng,
         &rpc,
         ring_len(rct_type),
         rpc.get_height().await.unwrap(),
-        &[output_tx0.clone()],
+        output_tx0.clone(),
       )
       .await
       .unwrap();
-
-      let inputs = [output_tx0.clone()].into_iter().zip(decoys).collect::<Vec<_>>();
-      builder.add_inputs(&inputs);
+      builder.add_input(input);
       builder.add_payment(addr, 1000000000000);
 
       (builder.build().unwrap(), (rct_type, output_tx0))
@@ -66,17 +65,19 @@ test!(
       let mut selected_fresh_decoy = false;
       let mut attempts = 1000;
       while !selected_fresh_decoy && attempts > 0 {
-        let decoys = Decoys::fingerprintable_canonical_select(
+        let decoys = OutputWithDecoys::fingerprintable_deterministic_new(
           &mut OsRng, // TODO: use a seeded RNG to consistently select the latest output
           &rpc,
           ring_len(rct_type),
           height,
-          &[output_tx0.clone()],
+          output_tx0.clone(),
         )
         .await
-        .unwrap();
+        .unwrap()
+        .decoys()
+        .clone();
 
-        selected_fresh_decoy = decoys[0].positions().contains(&most_recent_o_index);
+        selected_fresh_decoy = decoys.positions().contains(&most_recent_o_index);
         attempts -= 1;
       }
 
@@ -107,18 +108,16 @@ test!(
     |rct_type: RctType, rpc, mut builder: Builder, addr, output_tx0: WalletOutput| async move {
       let rpc: SimpleRequestRpc = rpc;
 
-      let decoys = Decoys::select(
+      let input = OutputWithDecoys::new(
         &mut OsRng,
         &rpc,
         ring_len(rct_type),
         rpc.get_height().await.unwrap(),
-        &[output_tx0.clone()],
+        output_tx0.clone(),
       )
       .await
       .unwrap();
-
-      let inputs = [output_tx0.clone()].into_iter().zip(decoys).collect::<Vec<_>>();
-      builder.add_inputs(&inputs);
+      builder.add_input(input);
       builder.add_payment(addr, 1000000000000);
 
       (builder.build().unwrap(), (rct_type, output_tx0))
@@ -145,17 +144,19 @@ test!(
       let mut selected_fresh_decoy = false;
       let mut attempts = 1000;
       while !selected_fresh_decoy && attempts > 0 {
-        let decoys = Decoys::select(
+        let decoys = OutputWithDecoys::new(
           &mut OsRng, // TODO: use a seeded RNG to consistently select the latest output
           &rpc,
           ring_len(rct_type),
           height,
-          &[output_tx0.clone()],
+          output_tx0.clone(),
         )
         .await
-        .unwrap();
+        .unwrap()
+        .decoys()
+        .clone();
 
-        selected_fresh_decoy = decoys[0].positions().contains(&most_recent_o_index);
+        selected_fresh_decoy = decoys.positions().contains(&most_recent_o_index);
         attempts -= 1;
       }
 

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

@@ -1,11 +1,10 @@
 use zeroize::{Zeroize, Zeroizing};
 
 use monero_wallet::{
-  primitives::Decoys,
   ringct::RctType,
   rpc::FeeRate,
   address::MoneroAddress,
-  WalletOutput,
+  OutputWithDecoys,
   send::{Change, SendError, SignableTransaction},
   extra::MAX_ARBITRARY_DATA_SIZE,
 };
@@ -15,7 +14,7 @@ use monero_wallet::{
 pub struct SignableTransactionBuilder {
   rct_type: RctType,
   outgoing_view_key: Zeroizing<[u8; 32]>,
-  inputs: Vec<(WalletOutput, Decoys)>,
+  inputs: Vec<OutputWithDecoys>,
   payments: Vec<(MoneroAddress, u64)>,
   change: Change,
   data: Vec<Vec<u8>>,
@@ -40,12 +39,12 @@ impl SignableTransactionBuilder {
     }
   }
 
-  pub fn add_input(&mut self, input: (WalletOutput, Decoys)) -> &mut Self {
+  pub fn add_input(&mut self, input: OutputWithDecoys) -> &mut Self {
     self.inputs.push(input);
     self
   }
   #[allow(unused)]
-  pub fn add_inputs(&mut self, inputs: &[(WalletOutput, Decoys)]) -> &mut Self {
+  pub fn add_inputs(&mut self, inputs: &[OutputWithDecoys]) -> &mut Self {
     self.inputs.extend(inputs.iter().cloned());
     self
   }

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

@@ -198,13 +198,10 @@ macro_rules! test {
         };
 
         use monero_wallet::{
-          primitives::Decoys,
           ringct::RctType,
           rpc::FeePriority,
           address::Network,
-          ViewPair,
-          DecoySelection,
-          Scanner,
+          ViewPair, Scanner, OutputWithDecoys,
           send::{Change, SignableTransaction, Eventuality},
         };
 
@@ -300,16 +297,14 @@ macro_rules! test {
           let temp = Box::new({
             let mut builder = builder.clone();
 
-            let decoys = Decoys::fingerprintable_canonical_select(
+            let input = OutputWithDecoys::fingerprintable_deterministic_new(
               &mut OsRng,
               &rpc,
               ring_len(rct_type),
               rpc.get_height().await.unwrap(),
-              &[miner_tx.clone()],
-            )
-            .await
-            .unwrap();
-            builder.add_input((miner_tx, decoys.first().unwrap().clone()));
+              miner_tx,
+            ).await.unwrap();
+            builder.add_input(input);
 
             let (tx, state) = ($first_tx)(rpc.clone(), builder, next_addr).await;
             let fee_rate = tx.fee_rate().clone();

coins/monero/wallet/tests/send.rs

@@ -4,8 +4,8 @@ use rand_core::OsRng;
 
 use monero_simple_request_rpc::SimpleRequestRpc;
 use monero_wallet::{
-  primitives::Decoys, ringct::RctType, transaction::Transaction, rpc::Rpc,
-  address::SubaddressIndex, extra::Extra, WalletOutput, DecoySelection,
+  ringct::RctType, transaction::Transaction, rpc::Rpc, address::SubaddressIndex, extra::Extra,
+  WalletOutput, OutputWithDecoys,
 };
 
 mod runner;
@@ -18,19 +18,19 @@ async fn add_inputs(
   outputs: Vec<WalletOutput>,
   builder: &mut SignableTransactionBuilder,
 ) {
-  let decoys = Decoys::fingerprintable_canonical_select(
-    &mut OsRng,
-    rpc,
-    ring_len(rct_type),
-    rpc.get_height().await.unwrap(),
-    &outputs,
-  )
-  .await
-  .unwrap();
-
-  let inputs = outputs.into_iter().zip(decoys).collect::<Vec<_>>();
-
-  builder.add_inputs(&inputs);
+  for output in outputs {
+    builder.add_input(
+      OutputWithDecoys::fingerprintable_deterministic_new(
+        &mut OsRng,
+        rpc,
+        ring_len(rct_type),
+        rpc.get_height().await.unwrap(),
+        output,
+      )
+      .await
+      .unwrap(),
+    );
+  }
 }
 
 test!(

processor/src/networks/monero.rs

@@ -20,7 +20,7 @@ use monero_wallet::{
   block::Block,
   rpc::{FeeRate, RpcError, Rpc},
   address::{Network as MoneroNetwork, SubaddressIndex},
-  ViewPair, GuaranteedViewPair, WalletOutput, GuaranteedScanner, DecoySelection, Decoys,
+  ViewPair, GuaranteedViewPair, WalletOutput, OutputWithDecoys, GuaranteedScanner,
   send::{
     SendError, Change, SignableTransaction as MSignableTransaction, Eventuality, TransactionMachine,
   },
@@ -322,30 +322,31 @@ impl Monero {
       _ => panic!("Monero hard forked and the processor wasn't updated for it"),
     };
 
-    let spendable_outputs = inputs.iter().map(|input| input.0.clone()).collect::<Vec<_>>();
-
     let mut transcript =
       RecommendedTranscript::new(b"Serai Processor Monero Transaction Transcript");
     transcript.append_message(b"plan", plan_id);
 
     // All signers need to select the same decoys
     // All signers use the same height and a seeded RNG to make sure they do so.
-    let decoys = Decoys::fingerprintable_canonical_select(
-      &mut ChaCha20Rng::from_seed(transcript.rng_seed(b"decoys")),
-      &self.rpc,
-      // TODO: Have Decoys take RctType
-      match rct_type {
-        RctType::ClsagBulletproof => 11,
-        RctType::ClsagBulletproofPlus => 16,
-        _ => panic!("selecting decoys for an unsupported RctType"),
-      },
-      block_number + 1,
-      &spendable_outputs,
-    )
-    .await
-    .map_err(map_rpc_err)?;
-
-    let inputs = spendable_outputs.into_iter().zip(decoys).collect::<Vec<_>>();
+    let mut inputs_actual = Vec::with_capacity(inputs.len());
+    for input in inputs {
+      inputs_actual.push(
+        OutputWithDecoys::fingerprintable_deterministic_new(
+          &mut ChaCha20Rng::from_seed(transcript.rng_seed(b"decoys")),
+          &self.rpc,
+          // TODO: Have Decoys take RctType
+          match rct_type {
+            RctType::ClsagBulletproof => 11,
+            RctType::ClsagBulletproofPlus => 16,
+            _ => panic!("selecting decoys for an unsupported RctType"),
+          },
+          block_number + 1,
+          input.0.clone(),
+        )
+        .await
+        .map_err(map_rpc_err)?,
+      );
+    }
 
     // Monero requires at least two outputs
     // If we only have one output planned, add a dummy payment
@@ -375,7 +376,7 @@ impl Monero {
       rct_type,
       // Use the plan ID as the outgoing view key
       Zeroizing::new(*plan_id),
-      inputs.clone(),
+      inputs_actual,
       payments,
       Change::fingerprintable(change.as_ref().map(|change| change.clone().into())),
       vec![],
@@ -400,7 +401,7 @@ impl Monero {
         SendError::TooMuchArbitraryData |
         SendError::TooLargeTransaction |
         SendError::WrongPrivateKey => {
-          panic!("created an Monero invalid transaction: {e}");
+          panic!("created an invalid Monero transaction: {e}");
         }
         SendError::MultiplePaymentIds => {
           panic!("multiple payment IDs despite not supporting integrated addresses");
@@ -736,10 +737,11 @@ impl Network for Monero {
     }
 
     let new_block = self.rpc.get_block_by_number(new_block).await.unwrap();
-    let outputs =
+    let mut outputs =
       Self::test_scanner().scan(&self.rpc, &new_block).await.unwrap().ignore_additional_timelock();
+    let output = outputs.swap_remove(0);
 
-    let amount = outputs[0].commitment().amount;
+    let amount = output.commitment().amount;
     // The dust should always be sufficient for the fee
     let fee = Monero::DUST;
 
@@ -749,7 +751,7 @@ impl Network for Monero {
       _ => panic!("Monero hard forked and the processor wasn't updated for it"),
     };
 
-    let decoys = Decoys::fingerprintable_canonical_select(
+    let output = OutputWithDecoys::fingerprintable_deterministic_new(
       &mut OsRng,
       &self.rpc,
       match rct_type {
@@ -758,19 +760,17 @@ impl Network for Monero {
         _ => panic!("selecting decoys for an unsupported RctType"),
       },
       self.rpc.get_height().await.unwrap(),
-      &outputs,
+      output,
     )
     .await
     .unwrap();
 
-    let inputs = outputs.into_iter().zip(decoys).collect::<Vec<_>>();
-
     let mut outgoing_view_key = Zeroizing::new([0; 32]);
     OsRng.fill_bytes(outgoing_view_key.as_mut());
     let tx = MSignableTransaction::new(
       rct_type,
       outgoing_view_key,
-      inputs,
+      vec![output],
       vec![(address.into(), amount - fee)],
       Change::fingerprintable(Some(Self::test_address().into())),
       vec![],

tests/full-stack/src/tests/mint_and_burn.rs

@@ -348,7 +348,7 @@ async fn mint_and_burn_test() {
         ringct::RctType,
         rpc::{FeePriority, Rpc},
         address::{Network, AddressType, MoneroAddress},
-        ViewPair, Scanner, DecoySelection, Decoys,
+        ViewPair, Scanner, OutputWithDecoys,
         send::{Change, SignableTransaction},
       };
 
@@ -363,23 +363,22 @@ async fn mint_and_burn_test() {
         .additional_timelock_satisfied_by(rpc.get_height().await.unwrap(), 0)
         .swap_remove(0);
 
-      let decoys = Decoys::fingerprintable_canonical_select(
+      let input = OutputWithDecoys::fingerprintable_deterministic_new(
         &mut OsRng,
         &rpc,
         16,
         rpc.get_height().await.unwrap(),
-        &[output.clone()],
+        output.clone(),
       )
       .await
-      .unwrap()
-      .swap_remove(0);
+      .unwrap();
 
       let mut outgoing_view_key = Zeroizing::new([0; 32]);
       OsRng.fill_bytes(outgoing_view_key.as_mut());
       let tx = SignableTransaction::new(
         RctType::ClsagBulletproofPlus,
         outgoing_view_key,
-        vec![(output, decoys)],
+        vec![input],
         vec![(
           MoneroAddress::new(
             Network::Mainnet,

tests/processor/src/networks.rs

@@ -412,7 +412,7 @@ impl Wallet {
           ringct::RctType,
           rpc::{FeePriority, Rpc},
           address::{Network, AddressType, Address},
-          Scanner, DecoySelection, Decoys,
+          Scanner, OutputWithDecoys,
           send::{Change, SignableTransaction},
         };
         use processor::{additional_key, networks::Monero};
@@ -422,30 +422,35 @@ impl Wallet {
 
         // Prepare inputs
         let current_height = rpc.get_height().await.unwrap();
-        let mut inputs = vec![];
+        let mut outputs = vec![];
         for block in last_tx.0 .. current_height {
           let block = rpc.get_block_by_number(block).await.unwrap();
           if (block.miner_transaction.hash() == last_tx.1) ||
             block.transactions.contains(&last_tx.1)
           {
-            inputs = Scanner::new(view_pair.clone())
+            outputs = Scanner::new(view_pair.clone())
               .scan(&rpc, &block)
               .await
               .unwrap()
               .ignore_additional_timelock();
           }
         }
-        assert!(!inputs.is_empty());
+        assert!(!outputs.is_empty());
 
-        let mut decoys = Decoys::fingerprintable_canonical_select(
-          &mut OsRng,
-          &rpc,
-          16,
-          rpc.get_height().await.unwrap(),
-          &inputs,
-        )
-        .await
-        .unwrap();
+        let mut inputs = Vec::with_capacity(outputs.len());
+        for output in outputs {
+          inputs.push(
+            OutputWithDecoys::fingerprintable_deterministic_new(
+              &mut OsRng,
+              &rpc,
+              16,
+              rpc.get_height().await.unwrap(),
+              output,
+            )
+            .await
+            .unwrap(),
+          );
+        }
 
         let to_spend_key = decompress_point(<[u8; 32]>::try_from(to.as_ref()).unwrap()).unwrap();
         let to_view_key = additional_key::<Monero>(0);
@@ -467,7 +472,7 @@ impl Wallet {
         let tx = SignableTransaction::new(
           RctType::ClsagBulletproofPlus,
           outgoing_view_key,
-          inputs.drain(..).zip(decoys.drain(..)).collect(),
+          inputs,
           vec![(to_addr, AMOUNT)],
           Change::new(view_pair),
           data,