Finish documenting monero-serai

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<u8>`.
+  ///
+  /// This is not a Monero protocol defined struct, and this is accordingly not a Monero protocol
+  /// defined serialization.
   pub fn serialize(&self) -> Vec<u8> {
     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<FeeRate> {
     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<Vec<Transaction>, 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.

coins/monero/wallet/src/decoys.rs

@@ -1,3 +1,5 @@
+// TODO: Clean this
+
 use std_shims::{vec::Vec, collections::HashSet};
 
 use zeroize::Zeroize;
@@ -277,29 +279,10 @@ async fn select_decoys<R: RngCore + CryptoRng>(
 pub use monero_serai::primitives::Decoys;
 
 // TODO: Remove this trait
+/// TODO: Document
 #[cfg(feature = "std")]
 #[async_trait::async_trait]
 pub trait DecoySelection {
-  async fn select<R: Send + Sync + RngCore + CryptoRng>(
-    rng: &mut R,
-    rpc: &impl Rpc,
-    ring_len: usize,
-    height: usize,
-    inputs: &[WalletOutput],
-  ) -> Result<Vec<Decoys>, RpcError>;
-
-  async fn fingerprintable_canonical_select<R: Send + Sync + RngCore + CryptoRng>(
-    rng: &mut R,
-    rpc: &impl Rpc,
-    ring_len: usize,
-    height: usize,
-    inputs: &[WalletOutput],
-  ) -> Result<Vec<Decoys>, RpcError>;
-}
-
-#[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<R: Send + Sync + RngCore + CryptoRng>(
@@ -308,9 +291,7 @@ impl DecoySelection for Decoys {
     ring_len: usize,
     height: usize,
     inputs: &[WalletOutput],
-  ) -> Result<Vec<Decoys>, RpcError> {
-    select_decoys(rng, rpc, ring_len, height, inputs, false).await
-  }
+  ) -> Result<Vec<Decoys>, 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
@@ -320,6 +301,28 @@ impl DecoySelection for Decoys {
   ///
   /// 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<R: Send + Sync + RngCore + CryptoRng>(
+    rng: &mut R,
+    rpc: &impl Rpc,
+    ring_len: usize,
+    height: usize,
+    inputs: &[WalletOutput],
+  ) -> Result<Vec<Decoys>, RpcError>;
+}
+
+#[cfg(feature = "std")]
+#[async_trait::async_trait]
+impl DecoySelection for Decoys {
+  async fn select<R: Send + Sync + RngCore + CryptoRng>(
+    rng: &mut R,
+    rpc: &impl Rpc,
+    ring_len: usize,
+    height: usize,
+    inputs: &[WalletOutput],
+  ) -> Result<Vec<Decoys>, RpcError> {
+    select_decoys(rng, rpc, ring_len, height, inputs, false).await
+  }
+
   async fn fingerprintable_canonical_select<R: Send + Sync + RngCore + CryptoRng>(
     rng: &mut R,
     rpc: &impl Rpc,

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<W: 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<u8>`.
+  pub fn serialize(&self) -> Vec<u8> {
+    let mut res = Vec::with_capacity(1 + 8);
+    self.write(&mut res).unwrap();
+    res
+  }
+
+  /// Read a PaymentId.
   pub fn read<R: Read>(r: &mut R) -> io::Result<PaymentId> {
     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<u8>),
+  /// 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<EdwardsPoint>),
+  /// 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<u8>),
 }
 
 impl ExtraField {
+  /// Write the ExtraField.
   pub fn write<W: 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<u8>`.
+  pub fn serialize(&self) -> Vec<u8> {
+    let mut res = Vec::with_capacity(1 + 8);
+    self.write(&mut res).unwrap();
+    res
+  }
+
+  /// Read an ExtraField.
   pub fn read<R: BufRead>(r: &mut R) -> io::Result<ExtraField> {
     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<ExtraField>);
 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<EdwardsPoint>, Option<Vec<EdwardsPoint>>)> {
     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<PaymentId> {
     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<Vec<u8>> {
     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<W: 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<u8>`.
   pub fn serialize(&self) -> Vec<u8> {
     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: BufRead>(r: &mut R) -> io::Result<Extra> {
     let mut res = Extra(vec![]);

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};

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<u64> },
+  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<u64>,
+  },
+  /// 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),

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<Ed25519, ClsagMultisig>)>,
 }
 
+/// Second FROST machine to produce a signed transaction.
 pub struct TransactionSignMachine {
   signable: SignableTransaction,
 
@@ -52,6 +53,7 @@ pub struct TransactionSignMachine {
   our_preprocess: Vec<Preprocess<Ed25519, ClsagAddendum>>,
 }
 
+/// Final FROST machine to produce a signed transaction.
 pub struct TransactionSignatureMachine {
   tx: Transaction,
   clsags: Vec<AlgorithmSignatureMachine<Ed25519, ClsagMultisig>>,

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

coins/monero/wallet/tests/runner/builder.rs

@@ -63,7 +63,7 @@ impl SignableTransactionBuilder {
   #[allow(unused)]
   pub fn add_data(&mut self, data: Vec<u8>) -> Result<&mut Self, SendError> {
     if data.len() > MAX_ARBITRARY_DATA_SIZE {
-      Err(SendError::TooMuchData)?;
+      Err(SendError::TooMuchArbitraryData)?;
     }
     self.data.push(data);
     Ok(self)

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();
       }
 

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")
       }

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}");