diff --git a/coins/monero/src/wallet/address.rs b/coins/monero/src/wallet/address.rs index 063c7e6f..09ff5aca 100644 --- a/coins/monero/src/wallet/address.rs +++ b/coins/monero/src/wallet/address.rs @@ -27,6 +27,15 @@ pub enum AddressType { Featured(bool, Option<[u8; 8]>, bool), } +/// Address specification. Used internally to create addresses. +#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] +pub enum AddressSpec { + Standard, + Integrated([u8; 8]), + Subaddress(u32, u32), + Featured(Option<(u32, u32)>, Option<[u8; 8]>, bool), +} + impl AddressType { pub fn subaddress(&self) -> bool { matches!(self, AddressType::Subaddress) || matches!(self, AddressType::Featured(true, ..)) diff --git a/coins/monero/src/wallet/mod.rs b/coins/monero/src/wallet/mod.rs index 5b776d2d..ae393ab4 100644 --- a/coins/monero/src/wallet/mod.rs +++ b/coins/monero/src/wallet/mod.rs @@ -16,7 +16,7 @@ pub(crate) use extra::{PaymentId, ExtraField, Extra}; /// Address encoding and decoding functionality. pub mod address; -use address::{Network, AddressType, AddressMeta, MoneroAddress}; +use address::{Network, AddressType, AddressSpec, AddressMeta, MoneroAddress}; mod scan; pub use scan::{ReceivedOutput, SpendableOutput}; @@ -106,7 +106,7 @@ impl ViewPair { ViewPair { spend, view } } - pub(crate) fn subaddress(&self, index: (u32, u32)) -> Scalar { + fn subaddress_derivation(&self, index: (u32, u32)) -> Scalar { if index == (0, 0) { return Scalar::zero(); } @@ -121,6 +121,49 @@ impl ViewPair { .concat(), )) } + + fn subaddress_keys(&self, index: (u32, u32)) -> Option<(EdwardsPoint, EdwardsPoint)> { + if index == (0, 0) { + return None; + } + + let scalar = self.subaddress_derivation(index); + let spend = self.spend + (&scalar * &ED25519_BASEPOINT_TABLE); + let view = self.view.deref() * spend; + Some((spend, view)) + } + + /// Returns an address with the provided specification. + pub fn address(&self, network: Network, spec: AddressSpec) -> MoneroAddress { + let mut spend = self.spend; + let mut view: EdwardsPoint = self.view.deref() * &ED25519_BASEPOINT_TABLE; + + // construct the address meta + let meta = match spec { + AddressSpec::Standard => AddressMeta::new(network, AddressType::Standard), + AddressSpec::Integrated(payment_id) => { + AddressMeta::new(network, AddressType::Integrated(payment_id)) + } + AddressSpec::Subaddress(i1, i2) => { + if let Some(keys) = self.subaddress_keys((i1, i2)) { + (spend, view) = keys; + AddressMeta::new(network, AddressType::Subaddress) + } else { + AddressMeta::new(network, AddressType::Standard) + } + } + AddressSpec::Featured(subaddress, payment_id, guaranteed) => { + let mut is_subaddress = false; + if let Some(Some(keys)) = subaddress.map(|subaddress| self.subaddress_keys(subaddress)) { + (spend, view) = keys; + is_subaddress = true; + } + AddressMeta::new(network, AddressType::Featured(is_subaddress, payment_id, guaranteed)) + } + }; + + MoneroAddress::new(meta, spend, view) + } } /// Transaction scanner. @@ -130,7 +173,6 @@ impl ViewPair { #[derive(Clone)] pub struct Scanner { pair: ViewPair, - network: Network, pub(crate) subaddresses: HashMap, pub(crate) burning_bug: Option>, } @@ -138,7 +180,6 @@ pub struct Scanner { impl Zeroize for Scanner { fn zeroize(&mut self) { self.pair.zeroize(); - self.network.zeroize(); // These may not be effective, unfortunately for (mut key, mut value) in self.subaddresses.drain() { @@ -163,59 +204,26 @@ impl ZeroizeOnDrop for Scanner {} impl Scanner { /// Create a Scanner from a ViewPair. - /// The network is used for generating subaddresses. /// burning_bug is a HashSet of used keys, intended to prevent key reuse which would burn funds. /// When an output is successfully scanned, the output key MUST be saved to disk. /// When a new scanner is created, ALL saved output keys must be passed in to be secure. /// If None is passed, a modified shared key derivation is used which is immune to the burning /// bug (specifically the Guaranteed feature from Featured Addresses). // TODO: Should this take in a DB access handle to ensure output keys are saved? - pub fn from_view( - pair: ViewPair, - network: Network, - burning_bug: Option>, - ) -> Scanner { + pub fn from_view(pair: ViewPair, burning_bug: Option>) -> Scanner { let mut subaddresses = HashMap::new(); subaddresses.insert(pair.spend.compress(), (0, 0)); - Scanner { pair, network, subaddresses, burning_bug } + Scanner { pair, subaddresses, burning_bug } } - /// Return the main address for this view pair. - pub fn address(&self) -> MoneroAddress { - MoneroAddress::new( - AddressMeta::new( - self.network, - if self.burning_bug.is_none() { - AddressType::Featured(false, None, true) - } else { - AddressType::Standard - }, - ), - self.pair.spend, - self.pair.view.deref() * &ED25519_BASEPOINT_TABLE, - ) - } - - /// Return the specified subaddress for this view pair. - pub fn subaddress(&mut self, index: (u32, u32)) -> MoneroAddress { - if index == (0, 0) { - return self.address(); + /// Register a subaddress. + // There used to be an address function here, yet it wasn't safe. It could generate addresses + // incompatible with the Scanner. While we could return None for that, then we have the issue + // of runtime failures to generate an address. + // Removing that API was the simplest option. + pub fn register_subaddress(&mut self, subaddress: (u32, u32)) { + if let Some((spend, _)) = self.pair.subaddress_keys(subaddress) { + self.subaddresses.insert(spend.compress(), subaddress); } - - let spend = self.pair.spend + (&self.pair.subaddress(index) * &ED25519_BASEPOINT_TABLE); - self.subaddresses.insert(spend.compress(), index); - - MoneroAddress::new( - AddressMeta::new( - self.network, - if self.burning_bug.is_none() { - AddressType::Featured(true, None, true) - } else { - AddressType::Subaddress - }, - ), - spend, - self.pair.view.deref() * spend, - ) } } diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs index 2dff5ed5..cefa3d1e 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -293,7 +293,7 @@ impl Scanner { // If we did though, it'd enable bypassing the included burning bug protection debug_assert!(output_key.is_torsion_free()); - let key_offset = shared_key + self.pair.subaddress(subaddress); + let key_offset = shared_key + self.pair.subaddress_derivation(subaddress); // Since we've found an output to us, get its amount let mut commitment = Commitment::zero(); diff --git a/coins/monero/tests/add_data.rs b/coins/monero/tests/add_data.rs index 1808472a..a620a73d 100644 --- a/coins/monero/tests/add_data.rs +++ b/coins/monero/tests/add_data.rs @@ -1,4 +1,4 @@ -use monero_serai::{rpc::Rpc, wallet::TransactionError, transaction::Transaction}; +use monero_serai::{wallet::TransactionError, transaction::Transaction}; mod runner; @@ -15,8 +15,7 @@ test!( builder.add_payment(addr, 5); (builder.build().unwrap(), (arbitrary_data,)) }, - |rpc: Rpc, signed: Transaction, mut scanner: Scanner, data: (Vec,)| async move { - let tx = rpc.get_transaction(signed.hash()).await.unwrap(); + |_, tx: Transaction, mut scanner: Scanner, data: (Vec,)| async move { let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0); assert_eq!(output.commitment().amount, 5); assert_eq!(output.arbitrary_data()[0], data.0); @@ -39,8 +38,7 @@ test!( builder.add_payment(addr, 5); (builder.build().unwrap(), data) }, - |rpc: Rpc, signed: Transaction, mut scanner: Scanner, data: Vec| async move { - let tx = rpc.get_transaction(signed.hash()).await.unwrap(); + |_, tx: Transaction, mut scanner: Scanner, data: Vec| async move { let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0); assert_eq!(output.commitment().amount, 5); assert_eq!(output.arbitrary_data(), vec![data; 5]); @@ -65,8 +63,7 @@ test!( builder.add_payment(addr, 5); (builder.build().unwrap(), data) }, - |rpc: Rpc, signed: Transaction, mut scanner: Scanner, data: Vec| async move { - let tx = rpc.get_transaction(signed.hash()).await.unwrap(); + |_, tx: Transaction, mut scanner: Scanner, data: Vec| async move { let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0); assert_eq!(output.commitment().amount, 5); assert_eq!(output.arbitrary_data(), vec![data]); diff --git a/coins/monero/tests/runner.rs b/coins/monero/tests/runner.rs index fe9239d7..8c202422 100644 --- a/coins/monero/tests/runner.rs +++ b/coins/monero/tests/runner.rs @@ -1,4 +1,5 @@ use core::ops::Deref; +use std::collections::HashSet; use lazy_static::lazy_static; @@ -12,8 +13,9 @@ use tokio::sync::Mutex; use monero_serai::{ Protocol, random_scalar, wallet::{ - ViewPair, - address::{Network, AddressType, AddressMeta, MoneroAddress}, + ViewPair, Scanner, + address::{Network, AddressType, AddressSpec, AddressMeta, MoneroAddress}, + SpendableOutput, }, rpc::Rpc, }; @@ -56,6 +58,21 @@ pub async fn mine_until_unlocked(rpc: &Rpc, addr: &str, tx_hash: [u8; 32]) { rpc.generate_blocks(addr, 9).await.unwrap(); } +// Mines 60 blocks and returns an unlocked miner TX output. +pub async fn get_miner_tx_output(rpc: &Rpc, view: &ViewPair) -> SpendableOutput { + let mut scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + + // Mine 60 blocks to unlock a miner TX + let start = rpc.get_height().await.unwrap(); + rpc + .generate_blocks(&view.address(Network::Mainnet, AddressSpec::Standard).to_string(), 60) + .await + .unwrap(); + + let block = rpc.get_block(start).await.unwrap(); + scanner.scan(rpc, &block).await.unwrap().swap_remove(0).ignore_timelock().swap_remove(0) +} + pub async fn rpc() -> Rpc { let rpc = Rpc::new("http://127.0.0.1:18081".to_string()).unwrap(); @@ -137,12 +154,12 @@ macro_rules! test { use monero_serai::{ random_scalar, wallet::{ - address::Network, ViewPair, Scanner, SignableTransaction, + address::{Network, AddressSpec}, ViewPair, Scanner, SignableTransaction, SignableTransactionBuilder, }, }; - use runner::{random_address, rpc, mine_until_unlocked}; + use runner::{random_address, rpc, mine_until_unlocked, get_miner_tx_output}; type Builder = SignableTransactionBuilder; @@ -168,28 +185,12 @@ macro_rules! test { keys[&1].group_key().0 }; - let view = ViewPair::new(spend_pub, Zeroizing::new(random_scalar(&mut OsRng))); - let rpc = rpc().await; - let (addr, miner_tx) = { - let mut scanner = - Scanner::from_view(view.clone(), Network::Mainnet, Some(HashSet::new())); - let addr = scanner.address(); + let view = ViewPair::new(spend_pub, Zeroizing::new(random_scalar(&mut OsRng))); + let addr = view.address(Network::Mainnet, AddressSpec::Standard); - // mine 60 blocks to unlock a miner tx - let start = rpc.get_height().await.unwrap(); - rpc.generate_blocks(&addr.to_string(), 60).await.unwrap(); - - let block = rpc.get_block(start).await.unwrap(); - ( - addr, - scanner.scan( - &rpc, - &block - ).await.unwrap().swap_remove(0).ignore_timelock().swap_remove(0) - ) - }; + let miner_tx = get_miner_tx_output(&rpc, &view).await; let builder = SignableTransactionBuilder::new( rpc.get_protocol().await.unwrap(), @@ -246,7 +247,7 @@ macro_rules! test { mine_until_unlocked(&rpc, &random_address().2.to_string(), signed.hash()).await; let tx = rpc.get_transaction(signed.hash()).await.unwrap(); let scanner = - Scanner::from_view(view.clone(), Network::Mainnet, Some(HashSet::new())); + Scanner::from_view(view.clone(), Some(HashSet::new())); ($first_checks)(rpc.clone(), tx, scanner, state).await }); #[allow(unused_variables, unused_mut, unused_assignments)] @@ -267,7 +268,7 @@ macro_rules! test { #[allow(unused_assignments)] { let scanner = - Scanner::from_view(view.clone(), Network::Mainnet, Some(HashSet::new())); + Scanner::from_view(view.clone(), Some(HashSet::new())); carried_state = Box::new(($checks)(rpc.clone(), tx, scanner, state).await); } diff --git a/coins/monero/tests/scan.rs b/coins/monero/tests/scan.rs new file mode 100644 index 00000000..ccc2f906 --- /dev/null +++ b/coins/monero/tests/scan.rs @@ -0,0 +1,257 @@ +use rand::RngCore; + +use monero_serai::transaction::Transaction; + +mod runner; + +test!( + scan_standard_address, + ( + |_, mut builder: Builder, _| async move { + let view = runner::random_address().1; + let scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + builder.add_payment(view.address(Network::Mainnet, AddressSpec::Standard), 5); + (builder.build().unwrap(), scanner) + }, + |_, tx: Transaction, _, mut state: Scanner| async move { + let output = state.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + }, + ), +); + +test!( + scan_subaddress, + ( + |_, mut builder: Builder, _| async move { + let subaddress = (0, 1); + + let view = runner::random_address().1; + let mut scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + scanner.register_subaddress(subaddress); + + builder.add_payment( + view.address(Network::Mainnet, AddressSpec::Subaddress(subaddress.0, subaddress.1)), + 5, + ); + (builder.build().unwrap(), (scanner, subaddress)) + }, + |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move { + let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.metadata.subaddress, state.1); + }, + ), +); + +test!( + scan_integrated_address, + ( + |_, mut builder: Builder, _| async move { + let view = runner::random_address().1; + let scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + + let mut payment_id = [0u8; 8]; + OsRng.fill_bytes(&mut payment_id); + + builder.add_payment(view.address(Network::Mainnet, AddressSpec::Integrated(payment_id)), 5); + (builder.build().unwrap(), (scanner, payment_id)) + }, + |_, tx: Transaction, _, mut state: (Scanner, [u8; 8])| async move { + let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.metadata.payment_id, state.1); + }, + ), +); + +test!( + scan_featured_standard, + ( + |_, mut builder: Builder, _| async move { + let view = runner::random_address().1; + let scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + builder + .add_payment(view.address(Network::Mainnet, AddressSpec::Featured(None, None, false)), 5); + (builder.build().unwrap(), scanner) + }, + |_, tx: Transaction, _, mut state: Scanner| async move { + let output = state.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + }, + ), +); + +test!( + scan_featured_subaddress, + ( + |_, mut builder: Builder, _| async move { + let subaddress = (0, 2); + + let view = runner::random_address().1; + let mut scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + scanner.register_subaddress(subaddress); + + builder.add_payment( + view.address(Network::Mainnet, AddressSpec::Featured(Some(subaddress), None, false)), + 5, + ); + (builder.build().unwrap(), (scanner, subaddress)) + }, + |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move { + let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.metadata.subaddress, state.1); + }, + ), +); + +test!( + scan_featured_integrated, + ( + |_, mut builder: Builder, _| async move { + let view = runner::random_address().1; + let scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + let mut payment_id = [0u8; 8]; + OsRng.fill_bytes(&mut payment_id); + + builder.add_payment( + view.address(Network::Mainnet, AddressSpec::Featured(None, Some(payment_id), false)), + 5, + ); + (builder.build().unwrap(), (scanner, payment_id)) + }, + |_, tx: Transaction, _, mut state: (Scanner, [u8; 8])| async move { + let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.metadata.payment_id, state.1); + }, + ), +); + +test!( + scan_featured_integrated_subaddress, + ( + |_, mut builder: Builder, _| async move { + let subaddress = (0, 3); + + let view = runner::random_address().1; + let mut scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + scanner.register_subaddress(subaddress); + + let mut payment_id = [0u8; 8]; + OsRng.fill_bytes(&mut payment_id); + + builder.add_payment( + view.address( + Network::Mainnet, + AddressSpec::Featured(Some(subaddress), Some(payment_id), false), + ), + 5, + ); + (builder.build().unwrap(), (scanner, payment_id, subaddress)) + }, + |_, tx: Transaction, _, mut state: (Scanner, [u8; 8], (u32, u32))| async move { + let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.metadata.payment_id, state.1); + assert_eq!(output.metadata.subaddress, state.2); + }, + ), +); + +test!( + scan_guaranteed_standard, + ( + |_, mut builder: Builder, _| async move { + let view = runner::random_address().1; + let scanner = Scanner::from_view(view.clone(), None); + + builder + .add_payment(view.address(Network::Mainnet, AddressSpec::Featured(None, None, true)), 5); + (builder.build().unwrap(), scanner) + }, + |_, tx: Transaction, _, mut state: Scanner| async move { + let output = state.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + }, + ), +); + +test!( + scan_guaranteed_subaddress, + ( + |_, mut builder: Builder, _| async move { + let subaddress = (1, 0); + + let view = runner::random_address().1; + let mut scanner = Scanner::from_view(view.clone(), None); + scanner.register_subaddress(subaddress); + + builder.add_payment( + view.address(Network::Mainnet, AddressSpec::Featured(Some(subaddress), None, true)), + 5, + ); + (builder.build().unwrap(), (scanner, subaddress)) + }, + |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move { + let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.metadata.subaddress, state.1); + }, + ), +); + +test!( + scan_guaranteed_integrated, + ( + |_, mut builder: Builder, _| async move { + let view = runner::random_address().1; + let scanner = Scanner::from_view(view.clone(), None); + let mut payment_id = [0u8; 8]; + OsRng.fill_bytes(&mut payment_id); + + builder.add_payment( + view.address(Network::Mainnet, AddressSpec::Featured(None, Some(payment_id), true)), + 5, + ); + (builder.build().unwrap(), (scanner, payment_id)) + }, + |_, tx: Transaction, _, mut state: (Scanner, [u8; 8])| async move { + let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.metadata.payment_id, state.1); + }, + ), +); + +test!( + scan_guaranteed_integrated_subaddress, + ( + |_, mut builder: Builder, _| async move { + let subaddress = (1, 1); + + let view = runner::random_address().1; + let mut scanner = Scanner::from_view(view.clone(), None); + scanner.register_subaddress(subaddress); + + let mut payment_id = [0u8; 8]; + OsRng.fill_bytes(&mut payment_id); + + builder.add_payment( + view.address( + Network::Mainnet, + AddressSpec::Featured(Some(subaddress), Some(payment_id), true), + ), + 5, + ); + (builder.build().unwrap(), (scanner, payment_id, subaddress)) + }, + |_, tx: Transaction, _, mut state: (Scanner, [u8; 8], (u32, u32))| async move { + let output = state.0.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.metadata.payment_id, state.1); + assert_eq!(output.metadata.subaddress, state.2); + }, + ), +); diff --git a/processor/src/coin/monero.rs b/processor/src/coin/monero.rs index b1656194..dc14fc0d 100644 --- a/processor/src/coin/monero.rs +++ b/processor/src/coin/monero.rs @@ -14,7 +14,7 @@ use monero_serai::{ rpc::Rpc, wallet::{ ViewPair, Scanner, - address::{Network, MoneroAddress}, + address::{Network, AddressSpec, MoneroAddress}, Fee, SpendableOutput, SignableTransaction as MSignableTransaction, TransactionMachine, }, }; @@ -75,23 +75,28 @@ impl Monero { Monero { rpc: Rpc::new(url).unwrap(), view: Zeroizing::new(additional_key::(0).0) } } + fn view_pair(&self, spend: dfg::EdwardsPoint) -> ViewPair { + ViewPair::new(spend.0, self.view.clone()) + } + fn scanner(&self, spend: dfg::EdwardsPoint) -> Scanner { - Scanner::from_view(ViewPair::new(spend.0, self.view.clone()), Network::Mainnet, None) + Scanner::from_view(self.view_pair(spend), None) } #[cfg(test)] - fn empty_scanner() -> Scanner { + fn test_view_pair() -> ViewPair { use group::Group; - Scanner::from_view( - ViewPair::new(*dfg::EdwardsPoint::generator(), Zeroizing::new(Scalar::one())), - Network::Mainnet, - Some(std::collections::HashSet::new()), - ) + ViewPair::new(*dfg::EdwardsPoint::generator(), Zeroizing::new(Scalar::one())) } #[cfg(test)] - fn empty_address() -> MoneroAddress { - Self::empty_scanner().address() + fn test_scanner() -> Scanner { + Scanner::from_view(Self::test_view_pair(), Some(std::collections::HashSet::new())) + } + + #[cfg(test)] + fn test_address() -> MoneroAddress { + Self::test_view_pair().address(Network::Mainnet, AddressSpec::Standard) } } @@ -121,7 +126,7 @@ impl Coin for Monero { const MAX_OUTPUTS: usize = 16; fn address(&self, key: dfg::EdwardsPoint) -> Self::Address { - self.scanner(key).address() + self.view_pair(key).address(Network::Mainnet, AddressSpec::Featured(None, None, true)) } async fn get_latest_block_number(&self) -> Result { @@ -228,7 +233,7 @@ impl Coin for Monero { Some(serde_json::json!({ "method": "generateblocks", "params": { - "wallet_address": Self::empty_address().to_string(), + "wallet_address": Self::test_address().to_string(), "amount_of_blocks": 10 }, })), @@ -249,7 +254,7 @@ impl Coin for Monero { self.mine_block().await; } - let outputs = Self::empty_scanner() + let outputs = Self::test_scanner() .scan(&self.rpc, &self.rpc.get_block(new_block).await.unwrap()) .await .unwrap() @@ -262,7 +267,7 @@ impl Coin for Monero { self.rpc.get_protocol().await.unwrap(), outputs, vec![(address, amount - fee)], - Some(Self::empty_address()), + Some(Self::test_address()), vec![], self.rpc.get_fee().await.unwrap(), )