Use an explicit SubaddressIndex type

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<SubaddressIndex> {
+    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),
+  Subaddress(SubaddressIndex),
-  Featured(Option<(u32, u32)>, Option<[u8; 8]>, bool),
+  Featured(Option<SubaddressIndex>, Option<[u8; 8]>, bool),
 }
 
 impl AddressType {

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 {
+  fn subaddress_derivation(&self, index: SubaddressIndex) -> Scalar {
-    if index == (0, 0) {
-      return Scalar::zero();
-    }
-
     hash_to_scalar(&Zeroizing::new(
       [
         b"SubAddr\0".as_ref(),
         Zeroizing::new(self.view.to_bytes()).as_ref(),
-        &index.0.to_le_bytes(),
+        &index.account().to_le_bytes(),
-        &index.1.to_le_bytes(),
+        &index.address().to_le_bytes(),
       ]
       .concat(),
     ))
   }
 
-  fn subaddress_keys(&self, index: (u32, u32)) -> Option<(EdwardsPoint, EdwardsPoint)> {
+  fn subaddress_keys(&self, index: SubaddressIndex) -> (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))
+    (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) => {
+      AddressSpec::Subaddress(index) => {
-        if let Some(keys) = self.subaddress_keys((i1, i2)) {
+        (spend, view) = self.subaddress_keys(index);
-          (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(index) = subaddress {
-        if let Some(Some(keys)) = subaddress.map(|subaddress| self.subaddress_keys(subaddress)) {
+          (spend, view) = self.subaddress_keys(index);
-          (spend, view) = keys;
-          is_subaddress = true;
         }
-        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<CompressedEdwardsY, (u32, u32)>,
+  // Also contains the spend key as None
+  pub(crate) subaddresses: HashMap<CompressedEdwardsY, Option<SubaddressIndex>>,
   pub(crate) burning_bug: Option<HashSet<CompressedEdwardsY>>,
 }
 
@@ -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<HashSet<CompressedEdwardsY>>) -> 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)) {
+  pub fn register_subaddress(&mut self, subaddress: SubaddressIndex) {
-    if let Some((spend, _)) = self.pair.subaddress_keys(subaddress) {
+    let (spend, _) = self.pair.subaddress_keys(subaddress);
-      self.subaddresses.insert(spend.compress(), subaddress);
+    self.subaddresses.insert(spend.compress(), Some(subaddress));
-    }
   }
 }

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: std::io::Read>(r: &mut R) -> std::io::Result<AbsoluteId> {
+  pub fn read<R: io::Read>(r: &mut R) -> io::Result<AbsoluteId> {
     Ok(AbsoluteId { tx: read_bytes(r)?, o: read_byte(r)? })
   }
 }
@@ -50,7 +55,7 @@ impl OutputData {
     res
   }
 
-  pub fn deserialize<R: std::io::Read>(r: &mut R) -> std::io::Result<OutputData> {
+  pub fn read<R: io::Read>(r: &mut R) -> io::Result<OutputData> {
     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<SubaddressIndex>,
   /// 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<u8> {
     let mut res = Vec::with_capacity(4 + 4 + 8 + 1);
-    res.extend(self.subaddress.0.to_le_bytes());
+    if let Some(subaddress) = self.subaddress {
-    res.extend(self.subaddress.1.to_le_bytes());
+      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: std::io::Read>(r: &mut R) -> std::io::Result<Metadata> {
+  pub fn read<R: io::Read>(r: &mut R) -> io::Result<Metadata> {
+    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: std::io::Read>(r: &mut R) -> std::io::Result<ReceivedOutput> {
+  pub fn deserialize<R: io::Read>(r: &mut R) -> io::Result<ReceivedOutput> {
     Ok(ReceivedOutput {
-      absolute: AbsoluteId::deserialize(r)?,
+      absolute: AbsoluteId::read(r)?,
-      data: OutputData::deserialize(r)?,
+      data: OutputData::read(r)?,
-      metadata: Metadata::deserialize(r)?,
+      metadata: Metadata::read(r)?,
     })
   }
 }
@@ -188,7 +206,7 @@ impl SpendableOutput {
     serialized
   }
 
-  pub fn deserialize<R: std::io::Read>(r: &mut R) -> std::io::Result<SpendableOutput> {
+  pub fn read<R: io::Read>(r: &mut R) -> io::Result<SpendableOutput> {
     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();
 

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(
+      builder.add_payment(view.address(Network::Mainnet, AddressSpec::Subaddress(subaddress)), 5);
-        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 {
+    |_, 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));
     },
   ),
 );

processor/src/coin/mod.rs

@@ -40,7 +40,7 @@ pub trait Output: Sized + Clone {
   fn amount(&self) -> u64;
 
   fn serialize(&self) -> Vec<u8>;
-  fn deserialize<R: std::io::Read>(reader: &mut R) -> std::io::Result<Self>;
+  fn read<R: std::io::Read>(reader: &mut R) -> std::io::Result<Self>;
 }
 
 #[async_trait]

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<SpendableOutput> for Output {
   }
 }
 
-const EXTERNAL_SUBADDRESS: (u32, u32) = (0, 0);
+const EXTERNAL_SUBADDRESS: Option<SubaddressIndex> = SubaddressIndex::new(0, 0);
-const BRANCH_SUBADDRESS: (u32, u32) = (1, 0);
+const BRANCH_SUBADDRESS: Option<SubaddressIndex> = SubaddressIndex::new(1, 0);
-const CHANGE_SUBADDRESS: (u32, u32) = (2, 0);
+const CHANGE_SUBADDRESS: Option<SubaddressIndex> = 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<R: std::io::Read>(reader: &mut R) -> std::io::Result<Self> {
+  fn read<R: std::io::Read>(reader: &mut R) -> std::io::Result<Self> {
-    SpendableOutput::deserialize(reader).map(Output)
+    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 {
+  fn address_internal(
-    self
+    &self,
-      .view_pair(spend)
+    spend: dfg::EdwardsPoint,
-      .address(Network::Mainnet, AddressSpec::Featured(Some(subaddress), None, true))
+    subaddress: Option<SubaddressIndex>,
+  ) -> 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
+    debug_assert!(EXTERNAL_SUBADDRESS.is_none());
-    scanner.register_subaddress(BRANCH_SUBADDRESS);
+    scanner.register_subaddress(BRANCH_SUBADDRESS.unwrap());
-    scanner.register_subaddress(CHANGE_SUBADDRESS);
+    scanner.register_subaddress(CHANGE_SUBADDRESS.unwrap());
     scanner
   }