Cite source for obscure wallet protocol rules

coins/monero/rpc/src/lib.rs

@@ -829,16 +829,16 @@ pub trait DecoyRpc: Sync + Clone + Debug {
   /// The timelock being satisfied is distinct from being free of the 10-block lock applied to all
   /// Monero transactions.
   ///
-  /// The node is trusted for if the output is unlocked unless `fingerprintable_canonical` is set
-  /// to true. If `fingerprintable_canonical` is set to true, the node's local view isn't used, yet
-  /// the transaction's timelock is checked to be unlocked at the specified `height`. This offers a
-  /// canonical decoy selection, yet is fingerprintable as time-based timelocks aren't evaluated
-  /// (and considered locked, preventing their selection).
+  /// The node is trusted for if the output is unlocked unless `fingerprintable_deterministic` is
+  /// set to true. If `fingerprintable_deterministic` is set to true, the node's local view isn't
+  /// used, yet the transaction's timelock is checked to be unlocked at the specified `height`.
+  /// This offers a deterministic decoy selection, yet is fingerprintable as time-based timelocks
+  /// aren't evaluated (and considered locked, preventing their selection).
   async fn get_unlocked_outputs(
     &self,
     indexes: &[u64],
     height: usize,
-    fingerprintable_canonical: bool,
+    fingerprintable_deterministic: bool,
   ) -> Result<Vec<Option<[EdwardsPoint; 2]>>, RpcError>;
 }
 
@@ -972,12 +972,12 @@ impl<R: Rpc> DecoyRpc for R {
     &self,
     indexes: &[u64],
     height: usize,
-    fingerprintable_canonical: bool,
+    fingerprintable_deterministic: bool,
   ) -> Result<Vec<Option<[EdwardsPoint; 2]>>, RpcError> {
     let outs: Vec<OutputResponse> = self.get_outs(indexes).await?;
 
-    // Only need to fetch txs to do canonical check on timelock
-    let txs = if fingerprintable_canonical {
+    // Only need to fetch txs to do deterministic check on timelock
+    let txs = if fingerprintable_deterministic {
       self
         .get_transactions(
           &outs.iter().map(|out| hash_hex(&out.txid)).collect::<Result<Vec<_>, _>>()?,
@@ -1005,7 +1005,7 @@ impl<R: Rpc> DecoyRpc for R {
           return Ok(None);
         };
         Ok(Some([key, rpc_point(&out.mask)?]).filter(|_| {
-          if fingerprintable_canonical {
+          if fingerprintable_deterministic {
             // 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

coins/monero/wallet/address/src/lib.rs

@@ -164,15 +164,19 @@ impl AddressBytes {
   }
 }
 
-// TODO: Cite origin
+// https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+//   /src/cryptonote_config.h#L216-L225
+// https://gist.github.com/kayabaNerve/01c50bbc35441e0bbdcee63a9d823789 for featured
 const MONERO_MAINNET_BYTES: AddressBytes = match AddressBytes::new(18, 19, 42, 70) {
   Some(bytes) => bytes,
   None => panic!("mainnet byte constants conflicted"),
 };
+// https://github.com/monero-project/monero/blob/master/src/cryptonote_config.h#L277-L281
 const MONERO_STAGENET_BYTES: AddressBytes = match AddressBytes::new(24, 25, 36, 86) {
   Some(bytes) => bytes,
   None => panic!("stagenet byte constants conflicted"),
 };
+// https://github.com/monero-project/monero/blob/master/src/cryptonote_config.h#L262-L266
 const MONERO_TESTNET_BYTES: AddressBytes = match AddressBytes::new(53, 54, 63, 111) {
   Some(bytes) => bytes,
   None => panic!("testnet byte constants conflicted"),

coins/monero/wallet/src/decoys.rs

@@ -28,7 +28,7 @@ async fn select_n(
   height: usize,
   real_output: u64,
   ring_len: usize,
-  fingerprintable_canonical: bool,
+  fingerprintable_deterministic: bool,
 ) -> Result<Vec<(u64, [EdwardsPoint; 2])>, RpcError> {
   if height < DEFAULT_LOCK_WINDOW {
     Err(RpcError::InternalError("not enough blocks to select decoys".to_string()))?;
@@ -141,7 +141,7 @@ async fn select_n(
     };
 
     for (i, output) in rpc
-      .get_unlocked_outputs(&candidates, height, fingerprintable_canonical)
+      .get_unlocked_outputs(&candidates, height, fingerprintable_deterministic)
       .await?
       .iter_mut()
       .enumerate()
@@ -172,8 +172,7 @@ async fn select_decoys<R: RngCore + CryptoRng>(
   ring_len: usize,
   height: usize,
   input: &WalletOutput,
-  // TODO: Decide "canonical" or "deterministic" (updating RPC terminology accordingly)
-  fingerprintable_canonical: bool,
+  fingerprintable_deterministic: bool,
 ) -> Result<Decoys, RpcError> {
   // 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
@@ -184,7 +183,7 @@ async fn select_decoys<R: RngCore + CryptoRng>(
     height,
     input.relative_id.index_on_blockchain,
     ring_len,
-    fingerprintable_canonical,
+    fingerprintable_deterministic,
   )
   .await?;
 

coins/monero/wallet/src/extra.rs

@@ -204,7 +204,10 @@ impl Extra {
   ///
   /// This returns all keys specified with `PublicKey` and the first set of keys specified with
   /// `PublicKeys`, so long as they're well-formed.
-  // TODO: Cite this
+  // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c45
+  //   /src/wallet/wallet2.cpp#L2290-L2300
+  // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+  // /src/wallet/wallet2.cpp#L2337-L2340
   pub fn keys(&self) -> Option<(Vec<EdwardsPoint>, Option<Vec<EdwardsPoint>>)> {
     let mut keys = vec![];
     let mut additional = None;
@@ -255,18 +258,24 @@ impl Extra {
 
   pub(crate) fn new(key: EdwardsPoint, additional: Vec<EdwardsPoint>) -> Extra {
     let mut res = Extra(Vec::with_capacity(3));
-    res.push(ExtraField::PublicKey(key));
+    // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+    //   /src/cryptonote_basic/cryptonote_format_utils.cpp#L627-L633
+    // We only support pushing nonces which come after these in the sort order
+    res.0.push(ExtraField::PublicKey(key));
     if !additional.is_empty() {
-      res.push(ExtraField::PublicKeys(additional));
+      res.0.push(ExtraField::PublicKeys(additional));
     }
     res
   }
 
-  pub(crate) fn push(&mut self, field: ExtraField) {
-    self.0.push(field);
+  pub(crate) fn push_nonce(&mut self, nonce: Vec<u8>) {
+    self.0.push(ExtraField::Nonce(nonce));
   }
 
   /// Write the Extra.
+  ///
+  /// This is not of deterministic length nor length-prefixed. It should only be written to a
+  /// buffer which will be delimited.
   pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
     for field in &self.0 {
       field.write(w)?;
@@ -281,17 +290,19 @@ impl Extra {
     buf
   }
 
-  // TODO: Is this supposed to silently drop trailing gibberish?
   /// Read an `Extra`.
+  ///
+  /// This is not of deterministic length nor length-prefixed. It should only be read from a buffer
+  /// already delimited.
   #[allow(clippy::unnecessary_wraps)]
   pub fn read<R: BufRead>(r: &mut R) -> io::Result<Extra> {
     let mut res = Extra(vec![]);
-    let mut field;
-    while {
-      field = ExtraField::read(r);
-      field.is_ok()
-    } {
-      res.0.push(field.unwrap());
+    // Extra reads until EOF
+    // We take a BufRead so we can detect when the buffer is empty
+    // `fill_buf` returns the current buffer, filled if empty, only empty if the reader is
+    // exhausted
+    while !r.fill_buf()?.is_empty() {
+      res.0.push(ExtraField::read(r)?);
     }
     Ok(res)
   }

coins/monero/wallet/src/scan.rs

@@ -135,9 +135,8 @@ impl InternalScanner {
       // This will be None if there's no additional keys, Some(None) if there's additional keys
       // yet not one for this output (which is non-standard), and Some(Some(_)) if there's an
       // additional key for this output
-      // https://github.com/monero-project/monero/
-      //   blob/04a1e2875d6e35e27bb21497988a6c822d319c28/
-      //   src/cryptonote_basic/cryptonote_format_utils.cpp#L1062
+      // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+      //   /src/cryptonote_basic/cryptonote_format_utils.cpp#L1060-L1070
       let additional = additional.as_ref().map(|additional| additional.get(o));
 
       #[allow(clippy::manual_let_else)]

coins/monero/wallet/src/send/tx.rs

@@ -13,7 +13,7 @@ use crate::{
     RctProofs,
   },
   transaction::{Input, Output, Timelock, TransactionPrefix, Transaction},
-  extra::{ARBITRARY_DATA_MARKER, PaymentId, ExtraField, Extra},
+  extra::{ARBITRARY_DATA_MARKER, PaymentId, Extra},
   send::{InternalPayment, SignableTransaction, SignableTransactionWithKeyImages},
 };
 
@@ -74,7 +74,7 @@ impl SignableTransaction {
       let id = (u64::from_le_bytes(id) ^ u64::from_le_bytes(*id_xor)).to_le_bytes();
       let mut id_vec = Vec::with_capacity(1 + 8);
       PaymentId::Encrypted(id).write(&mut id_vec).unwrap();
-      extra.push(ExtraField::Nonce(id_vec));
+      extra.push_nonce(id_vec);
     } else {
       // If there's no payment ID, we push a dummy (as wallet2 does) if there's only one payment
       if (self.payments.len() == 2) &&
@@ -89,7 +89,7 @@ impl SignableTransaction {
         let mut id_vec = Vec::with_capacity(1 + 8);
         // The dummy payment ID is [0; 8], which when xor'd with the mask, is just the mask
         PaymentId::Encrypted(*payment_id_xor).write(&mut id_vec).unwrap();
-        extra.push(ExtraField::Nonce(id_vec));
+        extra.push_nonce(id_vec);
       }
     }
 
@@ -97,7 +97,7 @@ impl SignableTransaction {
     for part in &self.data {
       let mut arb = vec![ARBITRARY_DATA_MARKER];
       arb.extend(part);
-      extra.push(ExtraField::Nonce(arb));
+      extra.push_nonce(arb);
     }
 
     let mut serialized = Vec::with_capacity(32 * amount_of_keys);

coins/monero/wallet/src/send/tx_keys.rs

@@ -186,7 +186,8 @@ impl SignableTransaction {
       let mut additional_keys_pub = vec![];
       for (additional_key, payment) in additional_keys.into_iter().zip(&self.payments) {
         let addr = payment.address();
-        // TODO: Double check this against wallet2
+        // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+        //   /src/device/device_default.cpp#L308-L312
         if addr.is_subaddress() {
           additional_keys_pub.push(additional_key.deref() * addr.spend());
         } else {