From 7508106650d545948e8bbe147a782c82c4052998 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sat, 7 Jan 2023 04:44:23 -0500 Subject: [PATCH] Use an explicit SubaddressIndex type --- coins/monero/src/wallet/address.rs | 27 +++++++++++++-- coins/monero/src/wallet/mod.rs | 53 ++++++++++++------------------ coins/monero/src/wallet/scan.rs | 51 +++++++++++++++++++--------- coins/monero/tests/scan.rs | 37 ++++++++++----------- processor/src/coin/mod.rs | 2 +- processor/src/coin/monero.rs | 28 ++++++++-------- 6 files changed, 115 insertions(+), 83 deletions(-) diff --git a/coins/monero/src/wallet/address.rs b/coins/monero/src/wallet/address.rs index 09ff5aca..b0f09927 100644 --- a/coins/monero/src/wallet/address.rs +++ b/coins/monero/src/wallet/address.rs @@ -27,13 +27,36 @@ pub enum AddressType { Featured(bool, Option<[u8; 8]>, bool), } +#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] +pub struct SubaddressIndex { + pub(crate) account: u32, + pub(crate) address: u32, +} + +impl SubaddressIndex { + pub const fn new(account: u32, address: u32) -> Option { + if (account == 0) && (address == 0) { + return None; + } + Some(SubaddressIndex { account, address }) + } + + pub fn account(&self) -> u32 { + self.account + } + + pub fn address(&self) -> u32 { + self.address + } +} + /// 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), + Subaddress(SubaddressIndex), + Featured(Option, Option<[u8; 8]>, bool), } impl AddressType { diff --git a/coins/monero/src/wallet/mod.rs b/coins/monero/src/wallet/mod.rs index ae393ab4..48ee3a34 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, AddressSpec, AddressMeta, MoneroAddress}; +use address::{Network, AddressType, SubaddressIndex, AddressSpec, AddressMeta, MoneroAddress}; mod scan; pub use scan::{ReceivedOutput, SpendableOutput}; @@ -106,31 +106,23 @@ impl ViewPair { ViewPair { spend, view } } - fn subaddress_derivation(&self, index: (u32, u32)) -> Scalar { - if index == (0, 0) { - return Scalar::zero(); - } - + fn subaddress_derivation(&self, index: SubaddressIndex) -> Scalar { hash_to_scalar(&Zeroizing::new( [ b"SubAddr\0".as_ref(), Zeroizing::new(self.view.to_bytes()).as_ref(), - &index.0.to_le_bytes(), - &index.1.to_le_bytes(), + &index.account().to_le_bytes(), + &index.address().to_le_bytes(), ] .concat(), )) } - fn subaddress_keys(&self, index: (u32, u32)) -> Option<(EdwardsPoint, EdwardsPoint)> { - if index == (0, 0) { - return None; - } - + fn subaddress_keys(&self, index: SubaddressIndex) -> (EdwardsPoint, EdwardsPoint) { let scalar = self.subaddress_derivation(index); let spend = self.spend + (&scalar * &ED25519_BASEPOINT_TABLE); let view = self.view.deref() * spend; - Some((spend, view)) + (spend, view) } /// Returns an address with the provided specification. @@ -144,21 +136,18 @@ impl ViewPair { 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::Subaddress(index) => { + (spend, view) = self.subaddress_keys(index); + AddressMeta::new(network, AddressType::Subaddress) } 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; + if let Some(index) = subaddress { + (spend, view) = self.subaddress_keys(index); } - AddressMeta::new(network, AddressType::Featured(is_subaddress, payment_id, guaranteed)) + AddressMeta::new( + network, + AddressType::Featured(subaddress.is_some(), payment_id, guaranteed), + ) } }; @@ -173,7 +162,8 @@ impl ViewPair { #[derive(Clone)] pub struct Scanner { pair: ViewPair, - pub(crate) subaddresses: HashMap, + // Also contains the spend key as None + pub(crate) subaddresses: HashMap>, pub(crate) burning_bug: Option>, } @@ -212,7 +202,7 @@ impl Scanner { // TODO: Should this take in a DB access handle to ensure output keys are saved? pub fn from_view(pair: ViewPair, burning_bug: Option>) -> Scanner { let mut subaddresses = HashMap::new(); - subaddresses.insert(pair.spend.compress(), (0, 0)); + subaddresses.insert(pair.spend.compress(), None); Scanner { pair, subaddresses, burning_bug } } @@ -221,9 +211,8 @@ impl Scanner { // 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); - } + pub fn register_subaddress(&mut self, subaddress: SubaddressIndex) { + let (spend, _) = self.pair.subaddress_keys(subaddress); + self.subaddresses.insert(spend.compress(), Some(subaddress)); } } diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs index 29dfdd07..5b80dbaf 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -1,3 +1,5 @@ +use std::io; + use zeroize::{Zeroize, ZeroizeOnDrop}; use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar, edwards::EdwardsPoint}; @@ -8,7 +10,10 @@ use crate::{ transaction::{Input, Timelock, Transaction}, block::Block, rpc::{Rpc, RpcError}, - wallet::{PaymentId, Extra, Scanner, uniqueness, shared_key, amount_decryption, commitment_mask}, + wallet::{ + PaymentId, Extra, address::SubaddressIndex, Scanner, uniqueness, shared_key, amount_decryption, + commitment_mask, + }, }; /// An absolute output ID, defined as its transaction hash and output index. @@ -26,7 +31,7 @@ impl AbsoluteId { res } - pub fn deserialize(r: &mut R) -> std::io::Result { + pub fn read(r: &mut R) -> io::Result { Ok(AbsoluteId { tx: read_bytes(r)?, o: read_byte(r)? }) } } @@ -50,7 +55,7 @@ impl OutputData { res } - pub fn deserialize(r: &mut R) -> std::io::Result { + pub fn read(r: &mut R) -> io::Result { Ok(OutputData { key: read_point(r)?, key_offset: read_scalar(r)?, @@ -62,9 +67,8 @@ impl OutputData { /// The metadata for an output. #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct Metadata { - // Does not have to be an Option since the 0 subaddress is the main address /// The subaddress this output was sent to. - pub subaddress: (u32, u32), + pub subaddress: Option, /// The payment ID included with this output. /// This will be gibberish if the payment ID wasn't intended for the recipient or wasn't included. // Could be an Option, as extra doesn't necessarily have a payment ID, yet all Monero TXs should @@ -77,8 +81,13 @@ pub struct Metadata { impl Metadata { pub fn serialize(&self) -> Vec { let mut res = Vec::with_capacity(4 + 4 + 8 + 1); - res.extend(self.subaddress.0.to_le_bytes()); - res.extend(self.subaddress.1.to_le_bytes()); + if let Some(subaddress) = self.subaddress { + res.push(1); + res.extend(subaddress.account().to_le_bytes()); + res.extend(subaddress.address().to_le_bytes()); + } else { + res.push(0); + } res.extend(self.payment_id); res.extend(u32::try_from(self.arbitrary_data.len()).unwrap().to_le_bytes()); @@ -89,9 +98,18 @@ impl Metadata { res } - pub fn deserialize(r: &mut R) -> std::io::Result { + pub fn read(r: &mut R) -> io::Result { + let subaddress = if read_byte(r)? == 1 { + Some( + SubaddressIndex::new(read_u32(r)?, read_u32(r)?) + .ok_or(io::Error::new(io::ErrorKind::Other, "invalid subaddress in metadata"))?, + ) + } else { + None + }; + Ok(Metadata { - subaddress: (read_u32(r)?, read_u32(r)?), + subaddress, payment_id: read_bytes(r)?, arbitrary_data: { let mut data = vec![]; @@ -137,11 +155,11 @@ impl ReceivedOutput { serialized } - pub fn deserialize(r: &mut R) -> std::io::Result { + pub fn deserialize(r: &mut R) -> io::Result { Ok(ReceivedOutput { - absolute: AbsoluteId::deserialize(r)?, - data: OutputData::deserialize(r)?, - metadata: Metadata::deserialize(r)?, + absolute: AbsoluteId::read(r)?, + data: OutputData::read(r)?, + metadata: Metadata::read(r)?, }) } } @@ -188,7 +206,7 @@ impl SpendableOutput { serialized } - pub fn deserialize(r: &mut R) -> std::io::Result { + pub fn read(r: &mut R) -> io::Result { Ok(SpendableOutput { output: ReceivedOutput::deserialize(r)?, global_index: read_u64(r)? }) } } @@ -291,7 +309,10 @@ 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_derivation(subaddress); + let mut key_offset = shared_key; + if let Some(subaddress) = subaddress { + key_offset += 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/scan.rs b/coins/monero/tests/scan.rs index ccc2f906..af42ea96 100644 --- a/coins/monero/tests/scan.rs +++ b/coins/monero/tests/scan.rs @@ -1,6 +1,6 @@ use rand::RngCore; -use monero_serai::transaction::Transaction; +use monero_serai::{transaction::Transaction, wallet::address::SubaddressIndex}; mod runner; @@ -24,22 +24,19 @@ test!( scan_subaddress, ( |_, mut builder: Builder, _| async move { - let subaddress = (0, 1); + let subaddress = SubaddressIndex::new(0, 1).unwrap(); 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.add_payment(view.address(Network::Mainnet, AddressSpec::Subaddress(subaddress)), 5); (builder.build().unwrap(), (scanner, subaddress)) }, - |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move { + |_, tx: Transaction, _, mut state: (Scanner, SubaddressIndex)| 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); + assert_eq!(output.metadata.subaddress, Some(state.1)); }, ), ); @@ -86,7 +83,7 @@ test!( scan_featured_subaddress, ( |_, mut builder: Builder, _| async move { - let subaddress = (0, 2); + let subaddress = SubaddressIndex::new(0, 2).unwrap(); let view = runner::random_address().1; let mut scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); @@ -98,10 +95,10 @@ test!( ); (builder.build().unwrap(), (scanner, subaddress)) }, - |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move { + |_, tx: Transaction, _, mut state: (Scanner, SubaddressIndex)| 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); + assert_eq!(output.metadata.subaddress, Some(state.1)); }, ), ); @@ -133,7 +130,7 @@ test!( scan_featured_integrated_subaddress, ( |_, mut builder: Builder, _| async move { - let subaddress = (0, 3); + let subaddress = SubaddressIndex::new(0, 3).unwrap(); let view = runner::random_address().1; let mut scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); @@ -151,11 +148,11 @@ test!( ); (builder.build().unwrap(), (scanner, payment_id, subaddress)) }, - |_, tx: Transaction, _, mut state: (Scanner, [u8; 8], (u32, u32))| async move { + |_, tx: Transaction, _, mut state: (Scanner, [u8; 8], SubaddressIndex)| 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); + assert_eq!(output.metadata.subaddress, Some(state.2)); }, ), ); @@ -182,7 +179,7 @@ test!( scan_guaranteed_subaddress, ( |_, mut builder: Builder, _| async move { - let subaddress = (1, 0); + let subaddress = SubaddressIndex::new(1, 0).unwrap(); let view = runner::random_address().1; let mut scanner = Scanner::from_view(view.clone(), None); @@ -194,10 +191,10 @@ test!( ); (builder.build().unwrap(), (scanner, subaddress)) }, - |_, tx: Transaction, _, mut state: (Scanner, (u32, u32))| async move { + |_, tx: Transaction, _, mut state: (Scanner, SubaddressIndex)| 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); + assert_eq!(output.metadata.subaddress, Some(state.1)); }, ), ); @@ -229,7 +226,7 @@ test!( scan_guaranteed_integrated_subaddress, ( |_, mut builder: Builder, _| async move { - let subaddress = (1, 1); + let subaddress = SubaddressIndex::new(1, 1).unwrap(); let view = runner::random_address().1; let mut scanner = Scanner::from_view(view.clone(), None); @@ -247,11 +244,11 @@ test!( ); (builder.build().unwrap(), (scanner, payment_id, subaddress)) }, - |_, tx: Transaction, _, mut state: (Scanner, [u8; 8], (u32, u32))| async move { + |_, tx: Transaction, _, mut state: (Scanner, [u8; 8], SubaddressIndex)| 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); + assert_eq!(output.metadata.subaddress, Some(state.2)); }, ), ); diff --git a/processor/src/coin/mod.rs b/processor/src/coin/mod.rs index 313dd2b3..b5fb9065 100644 --- a/processor/src/coin/mod.rs +++ b/processor/src/coin/mod.rs @@ -40,7 +40,7 @@ pub trait Output: Sized + Clone { fn amount(&self) -> u64; fn serialize(&self) -> Vec; - fn deserialize(reader: &mut R) -> std::io::Result; + fn read(reader: &mut R) -> std::io::Result; } #[async_trait] diff --git a/processor/src/coin/monero.rs b/processor/src/coin/monero.rs index 94f270c2..58afe3d8 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, AddressSpec, MoneroAddress}, + address::{Network, SubaddressIndex, AddressSpec, MoneroAddress}, Fee, SpendableOutput, SignableTransaction as MSignableTransaction, TransactionMachine, }, }; @@ -41,9 +41,9 @@ impl From for Output { } } -const EXTERNAL_SUBADDRESS: (u32, u32) = (0, 0); -const BRANCH_SUBADDRESS: (u32, u32) = (1, 0); -const CHANGE_SUBADDRESS: (u32, u32) = (2, 0); +const EXTERNAL_SUBADDRESS: Option = SubaddressIndex::new(0, 0); +const BRANCH_SUBADDRESS: Option = SubaddressIndex::new(1, 0); +const CHANGE_SUBADDRESS: Option = SubaddressIndex::new(2, 0); impl OutputTrait for Output { // While we could use (tx, o), using the key ensures we won't be susceptible to the burning bug. @@ -72,8 +72,8 @@ impl OutputTrait for Output { self.0.serialize() } - fn deserialize(reader: &mut R) -> std::io::Result { - SpendableOutput::deserialize(reader).map(Output) + fn read(reader: &mut R) -> std::io::Result { + SpendableOutput::read(reader).map(Output) } } @@ -101,17 +101,19 @@ impl Monero { ViewPair::new(spend.0, self.view.clone()) } - fn address_internal(&self, spend: dfg::EdwardsPoint, subaddress: (u32, u32)) -> MoneroAddress { - self - .view_pair(spend) - .address(Network::Mainnet, AddressSpec::Featured(Some(subaddress), None, true)) + fn address_internal( + &self, + spend: dfg::EdwardsPoint, + subaddress: Option, + ) -> MoneroAddress { + self.view_pair(spend).address(Network::Mainnet, AddressSpec::Featured(subaddress, None, true)) } fn scanner(&self, spend: dfg::EdwardsPoint) -> Scanner { let mut scanner = Scanner::from_view(self.view_pair(spend), None); - scanner.register_subaddress(EXTERNAL_SUBADDRESS); // Pointless as (0, 0) is already registered - scanner.register_subaddress(BRANCH_SUBADDRESS); - scanner.register_subaddress(CHANGE_SUBADDRESS); + debug_assert!(EXTERNAL_SUBADDRESS.is_none()); + scanner.register_subaddress(BRANCH_SUBADDRESS.unwrap()); + scanner.register_subaddress(CHANGE_SUBADDRESS.unwrap()); scanner }