From c6e048a9b2d5ae36b44e2f89935af2161a93dd4a Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 4 Jul 2024 00:43:50 -0400 Subject: [PATCH] Finish documenting monero-serai --- coins/monero/rpc/src/lib.rs | 14 +++- coins/monero/wallet/src/decoys.rs | 23 +++--- coins/monero/wallet/src/extra.rs | 70 +++++++++++++++++-- coins/monero/wallet/src/lib.rs | 4 +- coins/monero/wallet/src/send/mod.rs | 59 ++++++++++++++-- coins/monero/wallet/src/send/multisig.rs | 4 +- coins/monero/wallet/tests/add_data.rs | 2 +- coins/monero/wallet/tests/runner/builder.rs | 2 +- .../wallet/tests/wallet2_compatibility.rs | 5 +- processor/src/networks/bitcoin.rs | 4 +- processor/src/networks/monero.rs | 2 +- 11 files changed, 157 insertions(+), 32 deletions(-) diff --git a/coins/monero/rpc/src/lib.rs b/coins/monero/rpc/src/lib.rs index 14e5ed7e3..454e004a6 100644 --- a/coins/monero/rpc/src/lib.rs +++ b/coins/monero/rpc/src/lib.rs @@ -85,12 +85,18 @@ impl FeeRate { } /// Write the FeeRate. + /// + /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol + /// defined serialization. pub fn write(&self, w: &mut impl io::Write) -> io::Result<()> { w.write_all(&self.per_weight.to_le_bytes())?; w.write_all(&self.mask.to_le_bytes()) } /// Serialize the FeeRate to a `Vec`. + /// + /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol + /// defined serialization. pub fn serialize(&self) -> Vec { let mut res = Vec::with_capacity(16); self.write(&mut res).unwrap(); @@ -98,6 +104,9 @@ impl FeeRate { } /// Read a FeeRate. + /// + /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol + /// defined serialization. pub fn read(r: &mut impl io::Read) -> io::Result { Ok(FeeRate { per_weight: read_u64(r)?, mask: read_u64(r)? }) } @@ -486,7 +495,10 @@ pub trait Rpc: Sync + Clone + Debug { &self, number: usize, ) -> Result, RpcError> { - self.get_block_transactions(self.get_block_hash(number).await?).await + let block = self.get_block_by_number(number).await?; + let mut res = vec![block.miner_transaction]; + res.extend(self.get_transactions(&block.transactions).await?); + Ok(res) } /// Get the output indexes of the specified transaction. diff --git a/coins/monero/wallet/src/decoys.rs b/coins/monero/wallet/src/decoys.rs index 4d3ca8be9..d98d62e9a 100644 --- a/coins/monero/wallet/src/decoys.rs +++ b/coins/monero/wallet/src/decoys.rs @@ -1,3 +1,5 @@ +// TODO: Clean this + use std_shims::{vec::Vec, collections::HashSet}; use zeroize::Zeroize; @@ -277,9 +279,12 @@ async fn select_decoys( pub use monero_serai::primitives::Decoys; // TODO: Remove this trait +/// TODO: Document #[cfg(feature = "std")] #[async_trait::async_trait] pub trait DecoySelection { + /// Select decoys using the same distribution as Monero. Relies on the monerod RPC + /// response for an output's unlocked status, minimizing trips to the daemon. async fn select( rng: &mut R, rpc: &impl Rpc, @@ -288,6 +293,14 @@ pub trait DecoySelection { inputs: &[WalletOutput], ) -> Result, RpcError>; + /// If no reorg has occurred and an honest RPC, any caller who passes the same height to this + /// function will use the same distribution to select decoys. It is fingerprintable + /// because a caller using this will not be able to select decoys that are timelocked + /// with a timestamp. Any transaction which includes timestamp timelocked decoys in its + /// rings could not be constructed using this function. + /// + /// TODO: upstream change to monerod get_outs RPC to accept a height param for checking + /// output's unlocked status and remove all usage of fingerprintable_canonical async fn fingerprintable_canonical_select( rng: &mut R, rpc: &impl Rpc, @@ -300,8 +313,6 @@ pub trait DecoySelection { #[cfg(feature = "std")] #[async_trait::async_trait] impl DecoySelection for Decoys { - /// Select decoys using the same distribution as Monero. Relies on the monerod RPC - /// response for an output's unlocked status, minimizing trips to the daemon. async fn select( rng: &mut R, rpc: &impl Rpc, @@ -312,14 +323,6 @@ impl DecoySelection for Decoys { select_decoys(rng, rpc, ring_len, height, inputs, false).await } - /// If no reorg has occurred and an honest RPC, any caller who passes the same height to this - /// function will use the same distribution to select decoys. It is fingerprintable - /// because a caller using this will not be able to select decoys that are timelocked - /// with a timestamp. Any transaction which includes timestamp timelocked decoys in its - /// rings could not be constructed using this function. - /// - /// TODO: upstream change to monerod get_outs RPC to accept a height param for checking - /// output's unlocked status and remove all usage of fingerprintable_canonical async fn fingerprintable_canonical_select( rng: &mut R, rpc: &impl Rpc, diff --git a/coins/monero/wallet/src/extra.rs b/coins/monero/wallet/src/extra.rs index a24f31337..b67a4c707 100644 --- a/coins/monero/wallet/src/extra.rs +++ b/coins/monero/wallet/src/extra.rs @@ -10,20 +10,26 @@ use curve25519_dalek::edwards::EdwardsPoint; use monero_serai::io::*; -pub const MAX_TX_EXTRA_PADDING_COUNT: usize = 255; -pub const MAX_TX_EXTRA_NONCE_SIZE: usize = 255; +pub(crate) const MAX_TX_EXTRA_PADDING_COUNT: usize = 255; +const MAX_TX_EXTRA_NONCE_SIZE: usize = 255; -pub const PAYMENT_ID_MARKER: u8 = 0; -pub const ENCRYPTED_PAYMENT_ID_MARKER: u8 = 1; +const PAYMENT_ID_MARKER: u8 = 0; +const ENCRYPTED_PAYMENT_ID_MARKER: u8 = 1; // Used as it's the highest value not interpretable as a continued VarInt -pub const ARBITRARY_DATA_MARKER: u8 = 127; +pub(crate) const ARBITRARY_DATA_MARKER: u8 = 127; +/// The max amount of data which will fit within a blob of arbitrary data. // 1 byte is used for the marker pub const MAX_ARBITRARY_DATA_SIZE: usize = MAX_TX_EXTRA_NONCE_SIZE - 1; +/// A Payment ID. +/// +/// This is a legacy method of identifying why Monero was sent to the receiver. #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub enum PaymentId { + /// A deprecated form of payment ID which is no longer supported. Unencrypted([u8; 32]), + /// An encrypted payment ID. Encrypted([u8; 8]), } @@ -42,6 +48,7 @@ impl BitXor<[u8; 8]> for PaymentId { } impl PaymentId { + /// Write the PaymentId. pub fn write(&self, w: &mut W) -> io::Result<()> { match self { PaymentId::Unencrypted(id) => { @@ -56,6 +63,14 @@ impl PaymentId { Ok(()) } + /// Serialize the PaymentId to a `Vec`. + pub fn serialize(&self) -> Vec { + let mut res = Vec::with_capacity(1 + 8); + self.write(&mut res).unwrap(); + res + } + + /// Read a PaymentId. pub fn read(r: &mut R) -> io::Result { Ok(match read_byte(r)? { 0 => PaymentId::Unencrypted(read_bytes(r)?), @@ -65,18 +80,39 @@ impl PaymentId { } } -// Doesn't bother with padding nor MinerGate +/// A field within the TX extra. #[derive(Clone, PartialEq, Eq, Debug, Zeroize)] pub enum ExtraField { + /// Padding. + /// + /// This is a block of zeroes within the TX extra. Padding(usize), + /// The transaction key. + /// + /// This is a commitment to the randomness used for deriving outputs. PublicKey(EdwardsPoint), + /// The nonce field. + /// + /// This is used for data, such as payment IDs. Nonce(Vec), + /// The field for merge-mining. + /// + /// This is used within miner transactions who are merge-mining Monero to specify the foreign + /// block they mined. MergeMining(usize, [u8; 32]), + /// The additional transaction keys. + /// + /// These are the per-output commitments to the randomness used for deriving outputs. PublicKeys(Vec), + /// The 'mysterious' Minergate tag. + /// + /// This was used by a closed source entity without documentation. Support for parsing it was + /// added to reduce extra which couldn't be decoded. MysteriousMinergate(Vec), } impl ExtraField { + /// Write the ExtraField. pub fn write(&self, w: &mut W) -> io::Result<()> { match self { ExtraField::Padding(size) => { @@ -110,6 +146,14 @@ impl ExtraField { Ok(()) } + /// Serialize the ExtraField to a `Vec`. + pub fn serialize(&self) -> Vec { + let mut res = Vec::with_capacity(1 + 8); + self.write(&mut res).unwrap(); + res + } + + /// Read an ExtraField. pub fn read(r: &mut R) -> io::Result { Ok(match read_byte(r)? { 0 => ExtraField::Padding({ @@ -151,9 +195,15 @@ impl ExtraField { } } +/// The result of decoding a transaction's extra field. #[derive(Clone, PartialEq, Eq, Debug, Zeroize)] pub struct Extra(pub(crate) Vec); impl Extra { + /// The keys within this 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 pub fn keys(&self) -> Option<(Vec, Option>)> { let mut keys = vec![]; let mut additional = None; @@ -174,6 +224,8 @@ impl Extra { } } + /// The payment ID embedded within this extra. + // TODO: Monero distinguishes encrypted/unencrypted payment ID retrieval pub fn payment_id(&self) -> Option { for field in &self.0 { if let ExtraField::Nonce(data) = field { @@ -183,6 +235,9 @@ impl Extra { None } + /// The arbitrary data within this extra. + /// + /// This uses a marker custom to monero-wallet. pub fn data(&self) -> Vec> { let mut res = vec![]; for field in &self.0 { @@ -208,6 +263,7 @@ impl Extra { self.0.push(field); } + /// Write the Extra. pub fn write(&self, w: &mut W) -> io::Result<()> { for field in &self.0 { field.write(w)?; @@ -215,6 +271,7 @@ impl Extra { Ok(()) } + /// Serialize the Extra to a `Vec`. pub fn serialize(&self) -> Vec { let mut buf = vec![]; self.write(&mut buf).unwrap(); @@ -222,6 +279,7 @@ impl Extra { } // TODO: Is this supposed to silently drop trailing gibberish? + /// Read an `Extra`. #[allow(clippy::unnecessary_wraps)] pub fn read(r: &mut R) -> io::Result { let mut res = Extra(vec![]); diff --git a/coins/monero/wallet/src/lib.rs b/coins/monero/wallet/src/lib.rs index e93cacd40..2fa2e8048 100644 --- a/coins/monero/wallet/src/lib.rs +++ b/coins/monero/wallet/src/lib.rs @@ -1,6 +1,6 @@ #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![doc = include_str!("../README.md")] -// #![deny(missing_docs)] // TODO +#![deny(missing_docs)] #![cfg_attr(not(feature = "std"), no_std)] use zeroize::{Zeroize, Zeroizing}; @@ -23,6 +23,7 @@ pub use monero_address as address; mod view_pair; pub use view_pair::{ViewPair, GuaranteedViewPair}; +/// Structures and functionality for working with transactions' extra fields. pub mod extra; pub(crate) use extra::{PaymentId, Extra}; @@ -37,6 +38,7 @@ mod decoys; #[cfg(not(feature = "std"))] mod decoys { pub use monero_serai::primitives::Decoys; + /// TODO: Document/remove pub trait DecoySelection {} } pub use decoys::{DecoySelection, Decoys}; diff --git a/coins/monero/wallet/src/send/mod.rs b/coins/monero/wallet/src/send/mod.rs index 16f2a36db..b3b255051 100644 --- a/coins/monero/wallet/src/send/mod.rs +++ b/coins/monero/wallet/src/send/mod.rs @@ -69,7 +69,6 @@ impl Change { /// /// This take the view key as Monero assumes it has the view key for change outputs. It optimizes /// its wallet protocol accordingly. - // TODO: Accept AddressSpec, not `guaranteed: bool` pub fn new(view: &ViewPair) -> Change { Change(ChangeEnum::AddressWithView( // Which network doesn't matter as the derivations will all be the same @@ -79,6 +78,10 @@ impl Change { )) } + /// Create a change output specification for a guaranteed view pair. + /// + /// This take the view key as Monero assumes it has the view key for change outputs. It optimizes + /// its wallet protocol accordingly. pub fn guaranteed(view: &GuaranteedViewPair) -> Change { Change(ChangeEnum::AddressWithView( view.address( @@ -146,46 +149,78 @@ impl fmt::Debug for InternalPayment { } } +/// An error while sending Monero. #[derive(Clone, PartialEq, Eq, Debug)] #[cfg_attr(feature = "std", derive(thiserror::Error))] pub enum SendError { + /// The RingCT type to produce proofs for this transaction with weren't supported. #[cfg_attr(feature = "std", error("this library doesn't yet support that RctType"))] UnsupportedRctType, + /// The transaction had no inputs specified. #[cfg_attr(feature = "std", error("no inputs"))] NoInputs, + /// The decoy quantity was invalid for the specified RingCT type. #[cfg_attr(feature = "std", error("invalid number of decoys"))] InvalidDecoyQuantity, + /// The transaction had no outputs specified. #[cfg_attr(feature = "std", error("no outputs"))] NoOutputs, + /// The transaction had too many outputs specified. #[cfg_attr(feature = "std", error("too many outputs"))] TooManyOutputs, + /// The transaction did not have a change output, and did not have two outputs. + /// + /// Monero requires all transactions have at least two outputs, assuming one payment and one + /// change (or at least one dummy and one change). Accordingly, specifying no change and only + /// one payment prevents creating a valid transaction #[cfg_attr(feature = "std", error("only one output and no change address"))] NoChange, + /// Multiple addresses had payment IDs specified. + /// + /// Only one payment ID is allowed per transaction. #[cfg_attr(feature = "std", error("multiple addresses with payment IDs"))] MultiplePaymentIds, + /// Too much arbitrary data was specified. #[cfg_attr(feature = "std", error("too much data"))] - TooMuchData, - #[cfg_attr(feature = "std", error("too many inputs/too much arbitrary data"))] + TooMuchArbitraryData, + /// The created transaction was too large. + #[cfg_attr(feature = "std", error("too large of a transaction"))] TooLargeTransaction, + /// This transaction could not pay for itself. #[cfg_attr( feature = "std", error("not enough funds (inputs {inputs}, outputs {outputs}, fee {fee:?})") )] - NotEnoughFunds { inputs: u64, outputs: u64, fee: Option }, + NotEnoughFunds { + /// The amount of funds the inputs contributed. + inputs: u64, + /// The amount of funds the outputs required. + outputs: u64, + /// The fee which would be paid on top. + /// + /// If this is None, it is because the fee was not calculated as the outputs alone caused this + /// error. + fee: Option, + }, + /// This transaction is being signed with the wrong private key. #[cfg_attr(feature = "std", error("wrong spend private key"))] WrongPrivateKey, + /// This transaction was read from a bytestream which was malicious. #[cfg_attr( feature = "std", error("this SignableTransaction was created by deserializing a malicious serialization") )] MaliciousSerialization, + /// There was an error when working with the CLSAGs. #[cfg_attr(feature = "std", error("clsag error ({0})"))] ClsagError(ClsagError), + /// There was an error when working with FROST. #[cfg(feature = "multisig")] #[cfg_attr(feature = "std", error("frost error {0}"))] FrostError(FrostError), } +/// A signable transaction. #[derive(Clone, PartialEq, Eq, Debug, Zeroize)] pub struct SignableTransaction { rct_type: RctType, @@ -260,7 +295,7 @@ impl SignableTransaction { // Check the length of each arbitrary data for part in &self.data { if part.len() > MAX_ARBITRARY_DATA_SIZE { - Err(SendError::TooMuchData)?; + Err(SendError::TooMuchArbitraryData)?; } } @@ -268,7 +303,7 @@ impl SignableTransaction { // https://github.com/monero-project/monero/pull/8733 const MAX_EXTRA_SIZE: usize = 1060; if self.extra().len() > MAX_EXTRA_SIZE { - Err(SendError::TooMuchData)?; + Err(SendError::TooMuchArbitraryData)?; } // Make sure we have enough funds @@ -305,6 +340,15 @@ impl SignableTransaction { Ok(()) } + /// Create a new SignableTransaction. + /// + /// `outgoing_view_key` is used to seed the RNGs for this transaction. Anyone with knowledge of + /// the outgoing view key will be able to identify a transaction produced with this methodology, + /// and the data within it. Accordingly, it must be treated as a private key. + /// + /// `data` represents arbitrary data which will be embedded into the transaction's `extra` field. + /// The embedding occurs using an `ExtraField::Nonce` with a custom marker byte (as to not + /// conflict with a payment ID). pub fn new( rct_type: RctType, outgoing_view_key: Zeroizing<[u8; 32]>, @@ -340,10 +384,12 @@ impl SignableTransaction { Ok(res) } + /// The fee rate this transaction uses. pub fn fee_rate(&self) -> FeeRate { self.fee_rate } + /// The fee this transaction will use. pub fn fee(&self) -> u64 { self.weight_and_fee().1 } @@ -461,6 +507,7 @@ impl SignableTransaction { SignableTransactionWithKeyImages { intent: self, key_images } } + /// Sign this transaction. pub fn sign( self, rng: &mut (impl RngCore + CryptoRng), diff --git a/coins/monero/wallet/src/send/multisig.rs b/coins/monero/wallet/src/send/multisig.rs index 2cb0e24b7..79e8d5f37 100644 --- a/coins/monero/wallet/src/send/multisig.rs +++ b/coins/monero/wallet/src/send/multisig.rs @@ -30,7 +30,7 @@ use monero_serai::{ }; use crate::send::{SendError, SignableTransaction, key_image_sort}; -/// FROST signing machine to produce a signed transaction. +/// Initial FROST machine to produce a signed transaction. pub struct TransactionMachine { signable: SignableTransaction, @@ -41,6 +41,7 @@ pub struct TransactionMachine { clsags: Vec<(ClsagMultisigMaskSender, AlgorithmMachine)>, } +/// Second FROST machine to produce a signed transaction. pub struct TransactionSignMachine { signable: SignableTransaction, @@ -52,6 +53,7 @@ pub struct TransactionSignMachine { our_preprocess: Vec>, } +/// Final FROST machine to produce a signed transaction. pub struct TransactionSignatureMachine { tx: Transaction, clsags: Vec>, diff --git a/coins/monero/wallet/tests/add_data.rs b/coins/monero/wallet/tests/add_data.rs index 51d8edf04..6aa57dbca 100644 --- a/coins/monero/wallet/tests/add_data.rs +++ b/coins/monero/wallet/tests/add_data.rs @@ -60,7 +60,7 @@ test!( let mut data = vec![b'a'; MAX_ARBITRARY_DATA_SIZE + 1]; // Make sure we get an error if we try to add it to the TX - assert_eq!(builder.add_data(data.clone()), Err(SendError::TooMuchData)); + assert_eq!(builder.add_data(data.clone()), Err(SendError::TooMuchArbitraryData)); // Reduce data size and retry. The data will now be 255 bytes long (including the added // marker), exactly diff --git a/coins/monero/wallet/tests/runner/builder.rs b/coins/monero/wallet/tests/runner/builder.rs index 0e5a59213..df42a1da7 100644 --- a/coins/monero/wallet/tests/runner/builder.rs +++ b/coins/monero/wallet/tests/runner/builder.rs @@ -63,7 +63,7 @@ impl SignableTransactionBuilder { #[allow(unused)] pub fn add_data(&mut self, data: Vec) -> Result<&mut Self, SendError> { if data.len() > MAX_ARBITRARY_DATA_SIZE { - Err(SendError::TooMuchData)?; + Err(SendError::TooMuchArbitraryData)?; } self.data.push(data); Ok(self) diff --git a/coins/monero/wallet/tests/wallet2_compatibility.rs b/coins/monero/wallet/tests/wallet2_compatibility.rs index c6ba13da0..5348e243c 100644 --- a/coins/monero/wallet/tests/wallet2_compatibility.rs +++ b/coins/monero/wallet/tests/wallet2_compatibility.rs @@ -8,7 +8,7 @@ use monero_wallet::{ transaction::Transaction, rpc::Rpc, address::{Network, SubaddressIndex, MoneroAddress}, - extra::{MAX_TX_EXTRA_NONCE_SIZE, Extra, PaymentId}, + extra::{MAX_ARBITRARY_DATA_SIZE, Extra, PaymentId}, Scanner, }; @@ -347,8 +347,7 @@ test!( // Make 2 data that is the full 255 bytes for _ in 0 .. 2 { - // Subtract 1 since we prefix data with 127 - let data = vec![b'a'; MAX_TX_EXTRA_NONCE_SIZE - 1]; + let data = vec![b'a'; MAX_ARBITRARY_DATA_SIZE]; builder.add_data(data).unwrap(); } diff --git a/processor/src/networks/bitcoin.rs b/processor/src/networks/bitcoin.rs index 183444b12..43c266c4c 100644 --- a/processor/src/networks/bitcoin.rs +++ b/processor/src/networks/bitcoin.rs @@ -465,7 +465,9 @@ impl Bitcoin { Err(TransactionError::NoOutputs | TransactionError::NotEnoughFunds) => Ok(None), // amortize_fee removes payments which fall below the dust threshold Err(TransactionError::DustPayment) => panic!("dust payment despite removing dust"), - Err(TransactionError::TooMuchData) => panic!("too much data despite not specifying data"), + Err(TransactionError::TooMuchData) => { + panic!("too much data despite not specifying data") + } Err(TransactionError::TooLowFee) => { panic!("created a transaction whose fee is below the minimum") } diff --git a/processor/src/networks/monero.rs b/processor/src/networks/monero.rs index a316a8002..60363bbc2 100644 --- a/processor/src/networks/monero.rs +++ b/processor/src/networks/monero.rs @@ -388,7 +388,7 @@ impl Monero { SendError::NoOutputs | SendError::TooManyOutputs | SendError::NoChange | - SendError::TooMuchData | + SendError::TooMuchArbitraryData | SendError::TooLargeTransaction | SendError::WrongPrivateKey => { panic!("created an Monero invalid transaction: {e}");