From 92d8b91be994a4b96b41961738599259051de65c Mon Sep 17 00:00:00 2001 From: Justin Berman Date: Mon, 19 Feb 2024 18:34:10 -0800 Subject: [PATCH] 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/src/lib.rs | 4 + coins/monero/src/rpc/mod.rs | 96 +++--- coins/monero/src/transaction.rs | 2 + coins/monero/src/wallet/decoys.rs | 324 ++++++++++++-------- coins/monero/tests/decoys.rs | 162 ++++++++++ coins/monero/tests/runner.rs | 24 +- coins/monero/tests/send.rs | 4 +- processor/src/networks/monero.rs | 6 +- tests/full-stack/src/tests/mint_and_burn.rs | 6 +- tests/processor/src/networks.rs | 4 +- 10 files changed, 444 insertions(+), 188 deletions(-) create mode 100644 coins/monero/tests/decoys.rs diff --git a/coins/monero/src/lib.rs b/coins/monero/src/lib.rs index c8a8407a..6d9c0a6b 100644 --- a/coins/monero/src/lib.rs +++ b/coins/monero/src/lib.rs @@ -46,6 +46,10 @@ pub mod wallet; #[cfg(test)] mod tests; +pub const DEFAULT_LOCK_WINDOW: usize = 10; +pub const COINBASE_LOCK_WINDOW: usize = 60; +pub const BLOCK_TIME: usize = 120; + static INV_EIGHT_CELL: OnceLock = OnceLock::new(); #[allow(non_snake_case)] pub(crate) fn INV_EIGHT() -> Scalar { diff --git a/coins/monero/src/rpc/mod.rs b/coins/monero/src/rpc/mod.rs index d124928c..c0a8eae2 100644 --- a/coins/monero/src/rpc/mod.rs +++ b/coins/monero/src/rpc/mod.rs @@ -54,6 +54,15 @@ struct TransactionsResponse { txs: Vec, } +#[derive(Deserialize, Debug)] +pub struct OutputResponse { + pub height: usize, + pub unlocked: bool, + key: String, + mask: String, + txid: String, +} + #[derive(Clone, PartialEq, Eq, Debug)] #[cfg_attr(feature = "std", derive(thiserror::Error))] pub enum RpcError { @@ -534,29 +543,15 @@ impl Rpc { Ok(distributions.distributions.swap_remove(0).distribution) } - /// Get the specified outputs from the RingCT (zero-amount) pool, but only return them if their - /// timelock has been satisfied. - /// - /// The timelock being satisfied is distinct from being free of the 10-block lock applied to all - /// Monero transactions. - pub async fn get_unlocked_outputs( - &self, - indexes: &[u64], - height: usize, - ) -> Result>, RpcError> { + /// Get the specified outputs from the RingCT (zero-amount) pool + pub async fn get_outs(&self, indexes: &[u64]) -> Result, RpcError> { #[derive(Deserialize, Debug)] - struct Out { - key: String, - mask: String, - txid: String, + struct OutsResponse { + status: String, + outs: Vec, } - #[derive(Deserialize, Debug)] - struct Outs { - outs: Vec, - } - - let outs: Outs = self + let res: OutsResponse = self .rpc_call( "get_outs", Some(json!({ @@ -569,15 +564,39 @@ impl Rpc { ) .await?; - let txs = self - .get_transactions( - &outs.outs.iter().map(|out| hash_hex(&out.txid)).collect::, _>>()?, - ) - .await?; + if res.status != "OK" { + Err(RpcError::InvalidNode("bad response to get_outs".to_string()))?; + } + + Ok(res.outs) + } + + /// Get the specified outputs from the RingCT (zero-amount) pool, but only return them if their + /// timelock has been satisfied. + /// + /// The timelock being satisfied is distinct from being free of the 10-block lock applied to all + /// Monero transactions. + pub async fn get_unlocked_outputs( + &self, + indexes: &[u64], + height: usize, + fingerprintable_canonical: bool, + ) -> Result>, RpcError> { + let outs: Vec = self.get_outs(indexes).await?; + + // Only need to fetch txs to do canonical check on timelock + let txs = if fingerprintable_canonical { + self + .get_transactions( + &outs.iter().map(|out| hash_hex(&out.txid)).collect::, _>>()?, + ) + .await? + } else { + Vec::new() + }; // TODO: https://github.com/serai-dex/serai/issues/104 outs - .outs .iter() .enumerate() .map(|(i, out)| { @@ -593,10 +612,13 @@ impl Rpc { ) else { return Ok(None); }; - Ok( - Some([key, rpc_point(&out.mask)?]) - .filter(|_| Timelock::Block(height) >= txs[i].prefix.timelock), - ) + Ok(Some([key, rpc_point(&out.mask)?]).filter(|_| { + if fingerprintable_canonical { + Timelock::Block(height) >= txs[i].prefix.timelock + } else { + out.unlocked + } + })) }) .collect() } @@ -713,13 +735,14 @@ impl Rpc { &self, address: &str, block_count: usize, - ) -> Result, RpcError> { + ) -> Result<(Vec<[u8; 32]>, usize), RpcError> { #[derive(Debug, Deserialize)] struct BlocksResponse { blocks: Vec, + height: usize, } - let block_strs = self + let res = self .json_rpc_call::( "generateblocks", Some(json!({ @@ -727,13 +750,12 @@ impl Rpc { "amount_of_blocks": block_count })), ) - .await? - .blocks; + .await?; - let mut blocks = Vec::with_capacity(block_strs.len()); - for block in block_strs { + let mut blocks = Vec::with_capacity(res.blocks.len()); + for block in res.blocks { blocks.push(hash_hex(&block)?); } - Ok(blocks) + Ok((blocks, res.height)) } } diff --git a/coins/monero/src/transaction.rs b/coins/monero/src/transaction.rs index 20ad4009..89d489fe 100644 --- a/coins/monero/src/transaction.rs +++ b/coins/monero/src/transaction.rs @@ -161,7 +161,9 @@ impl Timelock { impl PartialOrd for Timelock { fn partial_cmp(&self, other: &Self) -> Option { match (self, other) { + (Timelock::None, Timelock::None) => Some(Ordering::Equal), (Timelock::None, _) => Some(Ordering::Less), + (_, Timelock::None) => Some(Ordering::Greater), (Timelock::Block(a), Timelock::Block(b)) => a.partial_cmp(b), (Timelock::Time(a), Timelock::Time(b)) => a.partial_cmp(b), _ => None, diff --git a/coins/monero/src/wallet/decoys.rs b/coins/monero/src/wallet/decoys.rs index e3b9776f..b0282f37 100644 --- a/coins/monero/src/wallet/decoys.rs +++ b/coins/monero/src/wallet/decoys.rs @@ -21,15 +21,13 @@ use crate::{ serialize::varint_len, wallet::SpendableOutput, rpc::{RpcError, RpcConnection, Rpc}, + DEFAULT_LOCK_WINDOW, COINBASE_LOCK_WINDOW, BLOCK_TIME, }; -const LOCK_WINDOW: usize = 10; -const MATURITY: u64 = 60; const RECENT_WINDOW: usize = 15; -const BLOCK_TIME: usize = 120; const BLOCKS_PER_YEAR: usize = 365 * 24 * 60 * 60 / BLOCK_TIME; #[allow(clippy::cast_precision_loss)] -const TIP_APPLICATION: f64 = (LOCK_WINDOW * BLOCK_TIME) as f64; +const TIP_APPLICATION: f64 = (DEFAULT_LOCK_WINDOW * BLOCK_TIME) as f64; // TODO: Resolve safety of this in case a reorg occurs/the network changes // TODO: Update this when scanning a block, as possible @@ -52,8 +50,10 @@ async fn select_n<'a, R: RngCore + CryptoRng, RPC: RpcConnection>( real: &[u64], used: &mut HashSet, count: usize, + fingerprintable_canonical: bool, ) -> Result, RpcError> { - if height >= rpc.get_height().await? { + // 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"))?; } @@ -64,6 +64,8 @@ async fn select_n<'a, R: RngCore + CryptoRng, RPC: RpcConnection>( // 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) let mut candidates = Vec::with_capacity(remaining); while candidates.len() != remaining { #[cfg(test)] @@ -117,7 +119,14 @@ async fn select_n<'a, R: RngCore + CryptoRng, RPC: RpcConnection>( } } - for (i, output) in rpc.get_unlocked_outputs(&candidates, height).await?.iter_mut().enumerate() { + // 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) { continue; @@ -141,6 +150,154 @@ fn offset(ring: &[u64]) -> Vec { res } +async fn select_decoys( + rng: &mut R, + rpc: &Rpc, + ring_len: usize, + height: usize, + inputs: &[SpendableOutput], + fingerprintable_canonical: bool, +) -> Result, RpcError> { + #[cfg(feature = "cache-distribution")] + #[cfg(not(feature = "std"))] + let mut distribution = DISTRIBUTION().lock(); + #[cfg(feature = "cache-distribution")] + #[cfg(feature = "std")] + let mut distribution = DISTRIBUTION().lock().await; + + #[cfg(not(feature = "cache-distribution"))] + 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.global_index); + 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.saturating_sub(1)).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 decoy candidates"))?; + } + + #[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::::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]; + if high.saturating_sub(COINBASE_LOCK_WINDOW as u64) < + u64::try_from(inputs.len() * ring_len).unwrap() + { + Err(RpcError::InternalError("not enough coinbase candidates"))?; + } + + // 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( + rng, + rpc, + &distribution, + height, + high, + per_second, + &real, + &mut used, + inputs.len() * decoy_count, + 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::>(); + ring.push(o); + 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::>() { + // 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); + } + } + + // 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)); + } + + // The other sanity check rule is about duplicates, yet we already enforce unique ring + // members + } + + res.push(Decoys { + // Binary searches for the real spend since we don't know where it sorted to + i: u8::try_from(ring.partition_point(|x| x.0 < o.0)).unwrap(), + offsets: offset(&ring.iter().map(|output| output.0).collect::>()), + ring: ring.iter().map(|output| output.1).collect(), + }); + } + + Ok(res) +} + /// Decoy data, containing the actual member as well (at index `i`). #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct Decoys { @@ -159,7 +316,16 @@ impl Decoys { self.offsets.len() } - /// Select decoys using the same distribution as Monero. + pub fn indexes(&self) -> Vec { + let mut res = vec![self.offsets[0]; self.len()]; + for m in 1 .. res.len() { + res[m] = res[m - 1] + self.offsets[m]; + } + res + } + + /// 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. pub async fn select( rng: &mut R, rpc: &Rpc, @@ -167,132 +333,24 @@ impl Decoys { height: usize, inputs: &[SpendableOutput], ) -> Result, RpcError> { - #[cfg(feature = "cache-distribution")] - #[cfg(not(feature = "std"))] - let mut distribution = DISTRIBUTION().lock(); - #[cfg(feature = "cache-distribution")] - #[cfg(feature = "std")] - let mut distribution = DISTRIBUTION().lock().await; + select_decoys(rng, rpc, ring_len, height, inputs, false).await + } - #[cfg(not(feature = "cache-distribution"))] - 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.global_index); - outputs.push((real[real.len() - 1], [input.key(), input.commitment().calculate()])); - } - - if distribution.len() <= height { - 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 + 1); // height is inclusive, and 0 is a valid height - - let high = distribution[distribution.len() - 1]; - #[allow(clippy::cast_precision_loss)] - let per_second = { - let blocks = distribution.len().min(BLOCKS_PER_YEAR); - let outputs = high - distribution[distribution.len().saturating_sub(blocks + 1)]; - (outputs as f64) / ((blocks * BLOCK_TIME) as f64) - }; - - let mut used = HashSet::::new(); - for o in &outputs { - used.insert(o.0); - } - - // TODO: Create a TX with less than the target amount, as allowed by the protocol - if (high - MATURITY) < u64::try_from(inputs.len() * ring_len).unwrap() { - Err(RpcError::InternalError("not enough decoy candidates"))?; - } - - // 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( - rng, - rpc, - &distribution, - height, - high, - per_second, - &real, - &mut used, - inputs.len() * decoy_count, - ) - .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::>(); - ring.push(o); - 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::>() { - // 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); - } - } - - // 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(), - ) - .await?, - ); - ring.sort_by(|a, b| a.0.cmp(&b.0)); - } - - // The other sanity check rule is about duplicates, yet we already enforce unique ring - // members - } - - res.push(Decoys { - // Binary searches for the real spend since we don't know where it sorted to - i: u8::try_from(ring.partition_point(|x| x.0 < o.0)).unwrap(), - offsets: offset(&ring.iter().map(|output| output.0).collect::>()), - ring: ring.iter().map(|output| output.1).collect(), - }); - } - - Ok(res) + /// 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. + /// + /// 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 + pub async fn fingerprintable_canonical_select( + rng: &mut R, + rpc: &Rpc, + ring_len: usize, + height: usize, + inputs: &[SpendableOutput], + ) -> Result, RpcError> { + select_decoys(rng, rpc, ring_len, height, inputs, true).await } } diff --git a/coins/monero/tests/decoys.rs b/coins/monero/tests/decoys.rs new file mode 100644 index 00000000..037726bc --- /dev/null +++ b/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::>(); + 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::>(); + 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()); + }, + ), +); diff --git a/coins/monero/tests/runner.rs b/coins/monero/tests/runner.rs index f99fb7b1..9cef6c21 100644 --- a/coins/monero/tests/runner.rs +++ b/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, 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, 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 = 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(); diff --git a/coins/monero/tests/send.rs b/coins/monero/tests/send.rs index ece54737..4c338eb6 100644 --- a/coins/monero/tests/send.rs +++ b/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 diff --git a/processor/src/networks/monero.rs b/processor/src/networks/monero.rs index 1def02ea..8d58ee1a 100644 --- a/processor/src/networks/monero.rs +++ b/processor/src/networks/monero.rs @@ -338,7 +338,7 @@ impl Monero { // 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::select( + let decoys = Decoys::fingerprintable_canonical_select( &mut ChaCha20Rng::from_seed(transcript.rng_seed(b"decoys")), &self.rpc, protocol.ring_len(), @@ -742,11 +742,11 @@ impl Network for Monero { let protocol = self.rpc.get_protocol().await.unwrap(); - let decoys = Decoys::select( + let decoys = Decoys::fingerprintable_canonical_select( &mut OsRng, &self.rpc, protocol.ring_len(), - self.rpc.get_height().await.unwrap() - 1, + self.rpc.get_height().await.unwrap(), &outputs, ) .await diff --git a/tests/full-stack/src/tests/mint_and_burn.rs b/tests/full-stack/src/tests/mint_and_burn.rs index 6ce22e8c..74bb8203 100644 --- a/tests/full-stack/src/tests/mint_and_burn.rs +++ b/tests/full-stack/src/tests/mint_and_burn.rs @@ -100,7 +100,7 @@ async fn mint_and_burn_test() { let rpc = producer_handles.monero(ops).await; let mut res = Vec::with_capacity(count); for _ in 0 .. count { - let block = rpc.get_block(rpc.generate_blocks(&addr, 1).await.unwrap()[0]).await.unwrap(); + let block = rpc.get_block(rpc.generate_blocks(&addr, 1).await.unwrap().0[0]).await.unwrap(); let mut txs = Vec::with_capacity(block.txs.len()); for tx in &block.txs { @@ -360,11 +360,11 @@ async fn mint_and_burn_test() { .unwrap() .swap_remove(0); - let decoys = Decoys::select( + let decoys = Decoys::fingerprintable_canonical_select( &mut OsRng, &rpc, Protocol::v16.ring_len(), - rpc.get_height().await.unwrap() - 1, + rpc.get_height().await.unwrap(), &[output.clone()], ) .await diff --git a/tests/processor/src/networks.rs b/tests/processor/src/networks.rs index e0aec77f..882b9e89 100644 --- a/tests/processor/src/networks.rs +++ b/tests/processor/src/networks.rs @@ -308,11 +308,11 @@ impl Wallet { .expect("prior transaction was never published"), ); } - let mut decoys = Decoys::select( + let mut decoys = Decoys::fingerprintable_canonical_select( &mut OsRng, &rpc, Protocol::v16.ring_len(), - rpc.get_height().await.unwrap() - 1, + rpc.get_height().await.unwrap(), &these_inputs, ) .await