From 4ba961b2cbcd5188c1d9b2e6396d605b2427285e Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 12 Jul 2024 02:19:21 -0400 Subject: [PATCH] Cite source for obscure wallet protocol rules --- coins/monero/rpc/src/lib.rs | 20 +++++++------- coins/monero/wallet/address/src/lib.rs | 6 ++++- coins/monero/wallet/src/decoys.rs | 9 +++---- coins/monero/wallet/src/extra.rs | 35 ++++++++++++++++--------- coins/monero/wallet/src/scan.rs | 5 ++-- coins/monero/wallet/src/send/tx.rs | 8 +++--- coins/monero/wallet/src/send/tx_keys.rs | 3 ++- 7 files changed, 50 insertions(+), 36 deletions(-) diff --git a/coins/monero/rpc/src/lib.rs b/coins/monero/rpc/src/lib.rs index ad2c3239..70b1a04a 100644 --- a/coins/monero/rpc/src/lib.rs +++ b/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>, RpcError>; } @@ -972,12 +972,12 @@ impl DecoyRpc for R { &self, indexes: &[u64], height: usize, - fingerprintable_canonical: bool, + fingerprintable_deterministic: bool, ) -> Result>, RpcError> { let outs: Vec = 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::, _>>()?, @@ -1005,7 +1005,7 @@ impl 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 diff --git a/coins/monero/wallet/address/src/lib.rs b/coins/monero/wallet/address/src/lib.rs index 96cbee19..35bdc24b 100644 --- a/coins/monero/wallet/address/src/lib.rs +++ b/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"), diff --git a/coins/monero/wallet/src/decoys.rs b/coins/monero/wallet/src/decoys.rs index 7abd3ce2..4a4faae5 100644 --- a/coins/monero/wallet/src/decoys.rs +++ b/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, 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( ring_len: usize, height: usize, input: &WalletOutput, - // TODO: Decide "canonical" or "deterministic" (updating RPC terminology accordingly) - fingerprintable_canonical: bool, + fingerprintable_deterministic: bool, ) -> Result { // 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( height, input.relative_id.index_on_blockchain, ring_len, - fingerprintable_canonical, + fingerprintable_deterministic, ) .await?; diff --git a/coins/monero/wallet/src/extra.rs b/coins/monero/wallet/src/extra.rs index 55f087f8..521d69ae 100644 --- a/coins/monero/wallet/src/extra.rs +++ b/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, Option>)> { let mut keys = vec![]; let mut additional = None; @@ -255,18 +258,24 @@ impl Extra { pub(crate) fn new(key: EdwardsPoint, additional: Vec) -> 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) { + 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(&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: &mut R) -> io::Result { 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) } diff --git a/coins/monero/wallet/src/scan.rs b/coins/monero/wallet/src/scan.rs index a1f64f48..73c98522 100644 --- a/coins/monero/wallet/src/scan.rs +++ b/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)] diff --git a/coins/monero/wallet/src/send/tx.rs b/coins/monero/wallet/src/send/tx.rs index 70bb8d62..65962211 100644 --- a/coins/monero/wallet/src/send/tx.rs +++ b/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); diff --git a/coins/monero/wallet/src/send/tx_keys.rs b/coins/monero/wallet/src/send/tx_keys.rs index 628f643e..7edc9ffd 100644 --- a/coins/monero/wallet/src/send/tx_keys.rs +++ b/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 {