From b2c962cd3e798b692614d61799b11101d243e7d0 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sat, 6 Jul 2024 03:24:38 -0400 Subject: [PATCH] Fix remaining bugs in monero-wallet tests --- coins/monero/primitives/src/lib.rs | 2 +- coins/monero/rpc/src/lib.rs | 3 +- coins/monero/wallet/src/send/tx_keys.rs | 41 +++++++++++++------------ coins/monero/wallet/tests/runner/mod.rs | 4 +-- coins/monero/wallet/tests/scan.rs | 14 +++------ 5 files changed, 31 insertions(+), 33 deletions(-) diff --git a/coins/monero/primitives/src/lib.rs b/coins/monero/primitives/src/lib.rs index 12d717375..848bed9df 100644 --- a/coins/monero/primitives/src/lib.rs +++ b/coins/monero/primitives/src/lib.rs @@ -147,7 +147,7 @@ impl Decoys { pub fn positions(&self) -> Vec { 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 diff --git a/coins/monero/rpc/src/lib.rs b/coins/monero/rpc/src/lib.rs index 48e594458..a8dde92b2 100644 --- a/coins/monero/rpc/src/lib.rs +++ b/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 diff --git a/coins/monero/wallet/src/send/tx_keys.rs b/coins/monero/wallet/src/send/tx_keys.rs index db63affb3..b54212357 100644 --- a/coins/monero/wallet/src/send/tx_keys.rs +++ b/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, -) -> 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::>(); + // 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 { diff --git a/coins/monero/wallet/tests/runner/mod.rs b/coins/monero/wallet/tests/runner/mod.rs index c2e18a497..d491a34a9 100644 --- a/coins/monero/wallet/tests/runner/mod.rs +++ b/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 }; diff --git a/coins/monero/wallet/tests/scan.rs b/coins/monero/wallet/tests/scan.rs index 1ad743955..b2a51c60c 100644 --- a/coins/monero/wallet/tests/scan.rs +++ b/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); }, ), );