diff --git a/coins/monero/src/lib.rs b/coins/monero/src/lib.rs index 70c306aa..43b8ddc1 100644 --- a/coins/monero/src/lib.rs +++ b/coins/monero/src/lib.rs @@ -55,7 +55,13 @@ pub(crate) fn INV_EIGHT() -> Scalar { pub enum Protocol { v14, v16, - Custom { ring_len: usize, bp_plus: bool, optimal_rct_type: RctType }, + Custom { + ring_len: usize, + bp_plus: bool, + optimal_rct_type: RctType, + view_tags: bool, + v16_fee: bool, + }, } impl Protocol { @@ -88,16 +94,37 @@ impl Protocol { } } + /// Whether or not the specified version uses view tags. + pub fn view_tags(&self) -> bool { + match self { + Protocol::v14 => false, + Protocol::v16 => true, + Protocol::Custom { view_tags, .. } => *view_tags, + } + } + + /// Whether or not the specified version uses the fee algorithm from Monero + /// hard fork version 16 (released in v18 binaries). + pub fn v16_fee(&self) -> bool { + match self { + Protocol::v14 => false, + Protocol::v16 => true, + Protocol::Custom { v16_fee, .. } => *v16_fee, + } + } + pub(crate) fn write(&self, w: &mut W) -> io::Result<()> { match self { Protocol::v14 => w.write_all(&[0, 14]), Protocol::v16 => w.write_all(&[0, 16]), - Protocol::Custom { ring_len, bp_plus, optimal_rct_type } => { + Protocol::Custom { ring_len, bp_plus, optimal_rct_type, view_tags, v16_fee } => { // Custom, version 0 w.write_all(&[1, 0])?; w.write_all(&u16::try_from(*ring_len).unwrap().to_le_bytes())?; w.write_all(&[u8::from(*bp_plus)])?; - w.write_all(&[optimal_rct_type.to_byte()]) + w.write_all(&[optimal_rct_type.to_byte()])?; + w.write_all(&[u8::from(*view_tags)])?; + w.write_all(&[u8::from(*v16_fee)]) } } } @@ -121,6 +148,16 @@ impl Protocol { }, optimal_rct_type: RctType::from_byte(read_byte(r)?) .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid RctType serialization"))?, + view_tags: match read_byte(r)? { + 0 => false, + 1 => true, + _ => Err(io::Error::new(io::ErrorKind::Other, "invalid bool serialization"))?, + }, + v16_fee: match read_byte(r)? { + 0 => false, + 1 => true, + _ => Err(io::Error::new(io::ErrorKind::Other, "invalid bool serialization"))?, + }, }, _ => { Err(io::Error::new(io::ErrorKind::Other, "unrecognized custom protocol serialization"))? diff --git a/coins/monero/src/ringct/bulletproofs/mod.rs b/coins/monero/src/ringct/bulletproofs/mod.rs index c4d17369..e7e071f5 100644 --- a/coins/monero/src/ringct/bulletproofs/mod.rs +++ b/coins/monero/src/ringct/bulletproofs/mod.rs @@ -37,24 +37,41 @@ pub enum Bulletproofs { } impl Bulletproofs { - pub(crate) fn fee_weight(plus: bool, outputs: usize) -> usize { - let fields = if plus { 6 } else { 9 }; + fn bp_fields(plus: bool) -> usize { + if plus { + 6 + } else { + 9 + } + } - // TODO: Shouldn't this use u32/u64? + // https://github.com/monero-project/monero/blob/94e67bf96bbc010241f29ada6abc89f49a81759c/ + // src/cryptonote_basic/cryptonote_format_utils.cpp#L106-L124 + pub(crate) fn calculate_bp_clawback(plus: bool, n_outputs: usize) -> (usize, usize) { #[allow(non_snake_case)] - let mut LR_len = usize::try_from(usize::BITS - (outputs - 1).leading_zeros()).unwrap(); - let padded_outputs = 1 << LR_len; + let mut LR_len = 0; + let mut n_padded_outputs = 1; + while n_padded_outputs < n_outputs { + LR_len += 1; + n_padded_outputs = 1 << LR_len; + } LR_len += LOG_N; - let len = (fields + (2 * LR_len)) * 32; - len + - if padded_outputs <= 2 { - 0 - } else { - let base = ((fields + (2 * (LOG_N + 1))) * 32) / 2; - let size = (fields + (2 * LR_len)) * 32; - ((base * padded_outputs) - size) * 4 / 5 - } + let mut bp_clawback = 0; + if n_padded_outputs > 2 { + let fields = Bulletproofs::bp_fields(plus); + let base = ((fields + (2 * (LOG_N + 1))) * 32) / 2; + let size = (fields + (2 * LR_len)) * 32; + bp_clawback = ((base * n_padded_outputs) - size) * 4 / 5; + } + + (bp_clawback, LR_len) + } + + pub(crate) fn fee_weight(plus: bool, outputs: usize) -> usize { + #[allow(non_snake_case)] + let (bp_clawback, LR_len) = Bulletproofs::calculate_bp_clawback(plus, outputs); + 32 * (Bulletproofs::bp_fields(plus) + (2 * LR_len)) + 2 + bp_clawback } /// Prove the list of commitments are within [0 .. 2^64). diff --git a/coins/monero/src/ringct/mod.rs b/coins/monero/src/ringct/mod.rs index 6288417d..a61e7c61 100644 --- a/coins/monero/src/ringct/mod.rs +++ b/coins/monero/src/ringct/mod.rs @@ -124,8 +124,9 @@ pub struct RctBase { } impl RctBase { - pub(crate) fn fee_weight(outputs: usize) -> usize { - 1 + 8 + (outputs * (8 + 32)) + pub(crate) fn fee_weight(outputs: usize, fee: u64) -> usize { + // 1 byte for the RCT signature type + 1 + (outputs * (8 + 32)) + varint_len(fee) } pub fn write(&self, w: &mut W, rct_type: RctType) -> io::Result<()> { @@ -211,6 +212,7 @@ pub enum RctPrunable { impl RctPrunable { pub(crate) fn fee_weight(protocol: Protocol, inputs: usize, outputs: usize) -> usize { + // 1 byte for number of BPs (technically a VarInt, yet there's always just zero or one) 1 + Bulletproofs::fee_weight(protocol.bp_plus(), outputs) + (inputs * (Clsag::fee_weight(protocol.ring_len()) + 32)) } @@ -377,8 +379,8 @@ impl RctSignatures { } } - pub(crate) fn fee_weight(protocol: Protocol, inputs: usize, outputs: usize) -> usize { - RctBase::fee_weight(outputs) + RctPrunable::fee_weight(protocol, inputs, outputs) + pub(crate) fn fee_weight(protocol: Protocol, inputs: usize, outputs: usize, fee: u64) -> usize { + RctBase::fee_weight(outputs, fee) + RctPrunable::fee_weight(protocol, inputs, outputs) } pub fn write(&self, w: &mut W) -> io::Result<()> { diff --git a/coins/monero/src/rpc/mod.rs b/coins/monero/src/rpc/mod.rs index 863b56e9..cacbd200 100644 --- a/coins/monero/src/rpc/mod.rs +++ b/coins/monero/src/rpc/mod.rs @@ -19,7 +19,7 @@ use crate::{ serialize::*, transaction::{Input, Timelock, Transaction}, block::Block, - wallet::Fee, + wallet::{FeePriority, Fee}, }; #[cfg(feature = "http_rpc")] @@ -27,6 +27,11 @@ mod http; #[cfg(feature = "http_rpc")] pub use http::*; +// Number of blocks the fee estimate will be valid for +// https://github.com/monero-project/monero/blob/94e67bf96bbc010241f29ada6abc89f49a81759c/ +// src/wallet/wallet2.cpp#L121 +const GRACE_BLOCKS_FOR_FEE_ESTIMATE: u64 = 10; + #[derive(Deserialize, Debug)] pub struct EmptyResponse {} #[derive(Deserialize, Debug)] @@ -66,6 +71,10 @@ pub enum RpcError { PrunedTransaction, #[cfg_attr(feature = "std", error("invalid transaction ({0:?})"))] InvalidTransaction([u8; 32]), + #[cfg_attr(feature = "std", error("unexpected fee response"))] + InvalidFee, + #[cfg_attr(feature = "std", error("invalid priority"))] + InvalidPriority, } fn rpc_hex(value: &str) -> Result, RpcError> { @@ -338,7 +347,6 @@ impl Rpc { txid: [u8; 32], } - #[allow(dead_code)] #[derive(Deserialize, Debug)] struct OIndexes { o_indexes: Vec, @@ -498,13 +506,11 @@ impl Rpc { from: usize, to: usize, ) -> Result, RpcError> { - #[allow(dead_code)] #[derive(Deserialize, Debug)] struct Distribution { distribution: Vec, } - #[allow(dead_code)] #[derive(Deserialize, Debug)] struct Distributions { distributions: Vec, @@ -585,19 +591,83 @@ impl Rpc { .collect() } - /// Get the currently estimated fee from the node. This may be manipulated to unsafe levels and - /// MUST be sanity checked. - // TODO: Take a sanity check argument - pub async fn get_fee(&self) -> Result { - #[allow(dead_code)] + async fn get_fee_v14(&self, priority: FeePriority) -> Result { #[derive(Deserialize, Debug)] - struct FeeResponse { + struct FeeResponseV14 { + status: String, fee: u64, quantization_mask: u64, } - let res: FeeResponse = self.json_rpc_call("get_fee_estimate", None).await?; - Ok(Fee { per_weight: res.fee, mask: res.quantization_mask }) + // https://github.com/monero-project/monero/blob/94e67bf96bbc010241f29ada6abc89f49a81759c/ + // src/wallet/wallet2.cpp#L7569-L7584 + // https://github.com/monero-project/monero/blob/94e67bf96bbc010241f29ada6abc89f49a81759c/ + // src/wallet/wallet2.cpp#L7660-L7661 + let priority_idx = + usize::try_from(if priority.fee_priority() == 0 { 1 } else { priority.fee_priority() - 1 }) + .map_err(|_| RpcError::InvalidPriority)?; + let multipliers = [1, 5, 25, 1000]; + if priority_idx >= multipliers.len() { + // though not an RPC error, it seems sensible to treat as such + Err(RpcError::InvalidPriority)?; + } + let fee_multiplier = multipliers[priority_idx]; + + let res: FeeResponseV14 = self + .json_rpc_call( + "get_fee_estimate", + Some(json!({ "grace_blocks": GRACE_BLOCKS_FOR_FEE_ESTIMATE })), + ) + .await?; + + if res.status != "OK" { + Err(RpcError::InvalidFee)?; + } + + Ok(Fee { per_weight: res.fee * fee_multiplier, mask: res.quantization_mask }) + } + + /// Get the currently estimated fee from the node. + /// + /// This may be manipulated to unsafe levels and MUST be sanity checked. + // TODO: Take a sanity check argument + pub async fn get_fee(&self, protocol: Protocol, priority: FeePriority) -> Result { + // TODO: Implement wallet2's adjust_priority which by default automatically uses a lower + // priority than provided depending on the backlog in the pool + if protocol.v16_fee() { + #[derive(Deserialize, Debug)] + struct FeeResponse { + status: String, + fees: Vec, + quantization_mask: u64, + } + + let res: FeeResponse = self + .json_rpc_call( + "get_fee_estimate", + Some(json!({ "grace_blocks": GRACE_BLOCKS_FOR_FEE_ESTIMATE })), + ) + .await?; + + // https://github.com/monero-project/monero/blob/94e67bf96bbc010241f29ada6abc89f49a81759c/ + // src/wallet/wallet2.cpp#L7615-L7620 + let priority_idx = usize::try_from(if priority.fee_priority() >= 4 { + 3 + } else { + priority.fee_priority().saturating_sub(1) + }) + .map_err(|_| RpcError::InvalidPriority)?; + + if res.status != "OK" { + Err(RpcError::InvalidFee) + } else if priority_idx >= res.fees.len() { + Err(RpcError::InvalidPriority) + } else { + Ok(Fee { per_weight: res.fees[priority_idx], mask: res.quantization_mask }) + } + } else { + self.get_fee_v14(priority).await + } } pub async fn publish_transaction(&self, tx: &Transaction) -> Result<(), RpcError> { diff --git a/coins/monero/src/serialize.rs b/coins/monero/src/serialize.rs index 29baaeed..80559195 100644 --- a/coins/monero/src/serialize.rs +++ b/coins/monero/src/serialize.rs @@ -11,8 +11,18 @@ use curve25519_dalek::{ const VARINT_CONTINUATION_MASK: u8 = 0b1000_0000; -pub(crate) fn varint_len(varint: usize) -> usize { - ((usize::try_from(usize::BITS - varint.leading_zeros()).unwrap().saturating_sub(1)) / 7) + 1 +mod sealed { + pub trait VarInt: TryInto {} + impl VarInt for u8 {} + impl VarInt for u32 {} + impl VarInt for u64 {} + impl VarInt for usize {} +} + +// This will panic if the VarInt exceeds u64::MAX +pub(crate) fn varint_len(varint: U) -> usize { + let varint_u64: u64 = varint.try_into().map_err(|_| "varint exceeded u64").unwrap(); + ((usize::try_from(u64::BITS - varint_u64.leading_zeros()).unwrap().saturating_sub(1)) / 7) + 1 } pub(crate) fn write_byte(byte: &u8, w: &mut W) -> io::Result<()> { diff --git a/coins/monero/src/transaction.rs b/coins/monero/src/transaction.rs index 84e40fe5..cab01ece 100644 --- a/coins/monero/src/transaction.rs +++ b/coins/monero/src/transaction.rs @@ -14,7 +14,7 @@ use curve25519_dalek::{ use crate::{ Protocol, hash, serialize::*, - ringct::{RctBase, RctPrunable, RctSignatures}, + ringct::{bulletproofs::Bulletproofs, RctType, RctBase, RctPrunable, RctSignatures}, }; #[derive(Clone, PartialEq, Eq, Debug)] @@ -24,11 +24,10 @@ pub enum Input { } impl Input { - // Worst-case predictive len - pub(crate) fn fee_weight(ring_len: usize) -> usize { + pub(crate) fn fee_weight(offsets_weight: usize) -> usize { + // Uses 1 byte for the input type // Uses 1 byte for the VarInt amount due to amount being 0 - // Uses 1 byte for the VarInt encoding of the length of the ring as well - 1 + 1 + 1 + (8 * ring_len) + 32 + 1 + 1 + offsets_weight + 32 } pub fn write(&self, w: &mut W) -> io::Result<()> { @@ -86,8 +85,10 @@ pub struct Output { } impl Output { - pub(crate) fn fee_weight() -> usize { - 1 + 1 + 32 + 1 + pub(crate) fn fee_weight(view_tags: bool) -> usize { + // Uses 1 byte for the output type + // Uses 1 byte for the VarInt amount due to amount being 0 + 1 + 1 + 32 + if view_tags { 1 } else { 0 } } pub fn write(&self, w: &mut W) -> io::Result<()> { @@ -185,13 +186,19 @@ pub struct TransactionPrefix { } impl TransactionPrefix { - pub(crate) fn fee_weight(ring_len: usize, inputs: usize, outputs: usize, extra: usize) -> usize { + pub(crate) fn fee_weight( + decoy_weights: &[usize], + outputs: usize, + view_tags: bool, + extra: usize, + ) -> usize { // Assumes Timelock::None since this library won't let you create a TX with a timelock + // 1 input for every decoy weight 1 + 1 + - varint_len(inputs) + - (inputs * Input::fee_weight(ring_len)) + - 1 + - (outputs * Output::fee_weight()) + + varint_len(decoy_weights.len()) + + decoy_weights.iter().map(|&offsets_weight| Input::fee_weight(offsets_weight)).sum::() + + varint_len(outputs) + + (outputs * Output::fee_weight(view_tags)) + varint_len(extra) + extra } @@ -253,12 +260,13 @@ pub struct Transaction { impl Transaction { pub(crate) fn fee_weight( protocol: Protocol, - inputs: usize, + decoy_weights: &[usize], outputs: usize, extra: usize, + fee: u64, ) -> usize { - TransactionPrefix::fee_weight(protocol.ring_len(), inputs, outputs, extra) + - RctSignatures::fee_weight(protocol, inputs, outputs) + TransactionPrefix::fee_weight(decoy_weights, outputs, protocol.view_tags(), extra) + + RctSignatures::fee_weight(protocol, decoy_weights.len(), outputs, fee) } pub fn write(&self, w: &mut W) -> io::Result<()> { @@ -378,4 +386,39 @@ impl Transaction { hash(&sig_hash) } + + fn is_rct_bulletproof(&self) -> bool { + match &self.rct_signatures.rct_type() { + RctType::Bulletproofs | RctType::BulletproofsCompactAmount | RctType::Clsag => true, + RctType::Null | + RctType::MlsagAggregate | + RctType::MlsagIndividual | + RctType::BulletproofsPlus => false, + } + } + + fn is_rct_bulletproof_plus(&self) -> bool { + match &self.rct_signatures.rct_type() { + RctType::BulletproofsPlus => true, + RctType::Null | + RctType::MlsagAggregate | + RctType::MlsagIndividual | + RctType::Bulletproofs | + RctType::BulletproofsCompactAmount | + RctType::Clsag => false, + } + } + + /// Calculate the transaction's weight. + pub fn weight(&self) -> usize { + let blob_size = self.serialize().len(); + + let bp = self.is_rct_bulletproof(); + let bp_plus = self.is_rct_bulletproof_plus(); + if !(bp || bp_plus) { + blob_size + } else { + blob_size + Bulletproofs::calculate_bp_clawback(bp_plus, self.prefix.outputs.len()).0 + } + } } diff --git a/coins/monero/src/wallet/decoys.rs b/coins/monero/src/wallet/decoys.rs index 81eeedab..fd1b8bcd 100644 --- a/coins/monero/src/wallet/decoys.rs +++ b/coins/monero/src/wallet/decoys.rs @@ -15,6 +15,7 @@ use rand_distr::num_traits::Float; use curve25519_dalek::edwards::EdwardsPoint; use crate::{ + serialize::varint_len, wallet::SpendableOutput, rpc::{RpcError, RpcConnection, Rpc}, }; @@ -135,12 +136,17 @@ fn offset(ring: &[u64]) -> Vec { /// Decoy data, containing the actual member as well (at index `i`). #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct Decoys { - pub i: u8, - pub offsets: Vec, - pub ring: Vec<[EdwardsPoint; 2]>, + pub(crate) i: u8, + pub(crate) offsets: Vec, + pub(crate) ring: Vec<[EdwardsPoint; 2]>, } +#[allow(clippy::len_without_is_empty)] impl Decoys { + pub fn fee_weight(offsets: &[u64]) -> usize { + varint_len(offsets.len()) + offsets.iter().map(|offset| varint_len(*offset)).sum::() + } + pub fn len(&self) -> usize { self.offsets.len() } diff --git a/coins/monero/src/wallet/mod.rs b/coins/monero/src/wallet/mod.rs index 8c2a2530..455c1c73 100644 --- a/coins/monero/src/wallet/mod.rs +++ b/coins/monero/src/wallet/mod.rs @@ -26,11 +26,11 @@ use address::{Network, AddressType, SubaddressIndex, AddressSpec, AddressMeta, M mod scan; pub use scan::{ReceivedOutput, SpendableOutput, Timelocked}; -pub(crate) mod decoys; -pub(crate) use decoys::Decoys; +pub mod decoys; +pub use decoys::Decoys; mod send; -pub use send::{Fee, TransactionError, Change, SignableTransaction, Eventuality}; +pub use send::{FeePriority, Fee, TransactionError, Change, SignableTransaction, Eventuality}; #[cfg(feature = "std")] pub use send::SignableTransactionBuilder; #[cfg(feature = "multisig")] diff --git a/coins/monero/src/wallet/send/builder.rs b/coins/monero/src/wallet/send/builder.rs index 7c575750..080504c0 100644 --- a/coins/monero/src/wallet/send/builder.rs +++ b/coins/monero/src/wallet/send/builder.rs @@ -5,18 +5,18 @@ use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; use crate::{ Protocol, wallet::{ - address::MoneroAddress, Fee, SpendableOutput, Change, SignableTransaction, TransactionError, - extra::MAX_ARBITRARY_DATA_SIZE, + address::MoneroAddress, Fee, SpendableOutput, Change, Decoys, SignableTransaction, + TransactionError, extra::MAX_ARBITRARY_DATA_SIZE, }, }; #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] struct SignableTransactionBuilderInternal { protocol: Protocol, - fee: Fee, + fee_rate: Fee, r_seed: Option>, - inputs: Vec, + inputs: Vec<(SpendableOutput, Decoys)>, payments: Vec<(MoneroAddress, u64)>, change_address: Option, data: Vec>, @@ -25,10 +25,10 @@ struct SignableTransactionBuilderInternal { impl SignableTransactionBuilderInternal { // Takes in the change address so users don't miss that they have to manually set one // If they don't, all leftover funds will become part of the fee - fn new(protocol: Protocol, fee: Fee, change_address: Option) -> Self { + fn new(protocol: Protocol, fee_rate: Fee, change_address: Option) -> Self { Self { protocol, - fee, + fee_rate, r_seed: None, inputs: vec![], payments: vec![], @@ -41,10 +41,10 @@ impl SignableTransactionBuilderInternal { self.r_seed = Some(r_seed); } - fn add_input(&mut self, input: SpendableOutput) { + fn add_input(&mut self, input: (SpendableOutput, Decoys)) { self.inputs.push(input); } - fn add_inputs(&mut self, inputs: &[SpendableOutput]) { + fn add_inputs(&mut self, inputs: &[(SpendableOutput, Decoys)]) { self.inputs.extend(inputs.iter().cloned()); } @@ -90,10 +90,10 @@ impl SignableTransactionBuilder { Self(self.0.clone()) } - pub fn new(protocol: Protocol, fee: Fee, change_address: Option) -> Self { + pub fn new(protocol: Protocol, fee_rate: Fee, change_address: Option) -> Self { Self(Arc::new(RwLock::new(SignableTransactionBuilderInternal::new( protocol, - fee, + fee_rate, change_address, )))) } @@ -103,11 +103,11 @@ impl SignableTransactionBuilder { self.shallow_copy() } - pub fn add_input(&mut self, input: SpendableOutput) -> Self { + pub fn add_input(&mut self, input: (SpendableOutput, Decoys)) -> Self { self.0.write().unwrap().add_input(input); self.shallow_copy() } - pub fn add_inputs(&mut self, inputs: &[SpendableOutput]) -> Self { + pub fn add_inputs(&mut self, inputs: &[(SpendableOutput, Decoys)]) -> Self { self.0.write().unwrap().add_inputs(inputs); self.shallow_copy() } @@ -138,7 +138,7 @@ impl SignableTransactionBuilder { read.payments.clone(), read.change_address.clone(), read.data.clone(), - read.fee, + read.fee_rate, ) } } diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index a2fc9309..a8d90b03 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -35,7 +35,7 @@ use crate::{ RctBase, RctPrunable, RctSignatures, }, transaction::{Input, Output, Timelock, TransactionPrefix, Transaction}, - rpc::{RpcError, RpcConnection, Rpc}, + rpc::RpcError, wallet::{ address::{Network, AddressSpec, MoneroAddress}, ViewPair, SpendableOutput, Decoys, PaymentId, ExtraField, Extra, key_image_sort, uniqueness, @@ -130,6 +130,8 @@ pub enum TransactionError { NoInputs, #[cfg_attr(feature = "std", error("no outputs"))] NoOutputs, + #[cfg_attr(feature = "std", error("invalid number of decoys"))] + InvalidDecoyQuantity, #[cfg_attr(feature = "std", error("only one output and no change address"))] NoChange, #[cfg_attr(feature = "std", error("too many outputs"))] @@ -153,40 +155,26 @@ pub enum TransactionError { FrostError(FrostError), } -async fn prepare_inputs( - rng: &mut R, - rpc: &Rpc, - ring_len: usize, - inputs: &[SpendableOutput], +fn prepare_inputs( + inputs: &[(SpendableOutput, Decoys)], spend: &Zeroizing, tx: &mut Transaction, ) -> Result, EdwardsPoint, ClsagInput)>, TransactionError> { let mut signable = Vec::with_capacity(inputs.len()); - // Select decoys - let decoys = Decoys::select( - rng, - rpc, - ring_len, - rpc.get_height().await.map_err(TransactionError::RpcError)? - 1, - inputs, - ) - .await - .map_err(TransactionError::RpcError)?; - - for (i, input) in inputs.iter().enumerate() { + for (i, (input, decoys)) in inputs.iter().enumerate() { let input_spend = Zeroizing::new(input.key_offset() + spend.deref()); let image = generate_key_image(&input_spend); signable.push(( input_spend, image, - ClsagInput::new(input.commitment().clone(), decoys[i].clone()) + ClsagInput::new(input.commitment().clone(), decoys.clone()) .map_err(TransactionError::ClsagError)?, )); tx.prefix.inputs.push(Input::ToKey { amount: None, - key_offsets: decoys[i].offsets.clone(), + key_offsets: decoys.offsets.clone(), key_image: signable[i].1, }); } @@ -203,6 +191,58 @@ async fn prepare_inputs( Ok(signable) } +// Deterministically calculate what the TX weight and fee will be. +fn calculate_weight_and_fee( + protocol: Protocol, + decoy_weights: &[usize], + n_outputs: usize, + extra: usize, + fee_rate: Fee, +) -> (usize, u64) { + // Starting the fee at 0 here is different than core Monero's wallet2.cpp, which starts its fee + // calculation with an estimate. + // + // This difference is okay in practice because wallet2 still ends up using a fee calculated from + // a TX's weight, as calculated later in this function. + // + // See this PR highlighting wallet2's behavior: + // https://github.com/monero-project/monero/pull/8882 + // + // Even with that PR, if the estimated fee's VarInt byte length is larger than the calculated + // fee's, the wallet can theoretically use a fee not based on the actual TX weight. This does not + // occur in practice as it's nearly impossible for wallet2 to estimate a fee that is larger + // than the calculated fee today, and on top of that, even more unlikely for that estimate's + // VarInt to be larger in byte length than the calculated fee's. + let mut weight = 0usize; + let mut fee = 0u64; + + let mut done = false; + let mut iters = 0; + let max_iters = 5; + while !done { + weight = Transaction::fee_weight(protocol, decoy_weights, n_outputs, extra, fee); + + let fee_calculated_from_weight = fee_rate.calculate_fee_from_weight(weight); + + // Continue trying to use the fee calculated from the tx's weight + done = fee_calculated_from_weight == fee; + + fee = fee_calculated_from_weight; + + #[cfg(test)] + debug_assert!(iters < max_iters, "Reached max fee calculation attempts"); + // Should never happen because the fee VarInt byte length shouldn't change *every* single iter. + // `iters` reaching `max_iters` is unexpected. + if iters >= max_iters { + // Fail-safe break to ensure funds are still spendable + break; + } + iters += 1; + } + + (weight, fee) +} + /// Fee struct, defined as a per-unit cost and a mask for rounding purposes. #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub struct Fee { @@ -211,8 +251,38 @@ pub struct Fee { } impl Fee { - pub fn calculate(&self, weight: usize) -> u64 { - ((((self.per_weight * u64::try_from(weight).unwrap()) - 1) / self.mask) + 1) * self.mask + pub fn calculate_fee_from_weight(&self, weight: usize) -> u64 { + let fee = (((self.per_weight * u64::try_from(weight).unwrap()) + self.mask - 1) / self.mask) * + self.mask; + debug_assert_eq!(weight, self.calculate_weight_from_fee(fee), "Miscalculated weight from fee"); + fee + } + + pub fn calculate_weight_from_fee(&self, fee: u64) -> usize { + usize::try_from(fee / self.per_weight).unwrap() + } +} + +/// Fee priority, determining how quickly a transaction is included in a block. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[allow(non_camel_case_types)] +pub enum FeePriority { + Lowest, + Low, + Medium, + High, + Custom { priority: u32 }, +} + +impl FeePriority { + pub(crate) fn fee_priority(&self) -> u32 { + match self { + FeePriority::Lowest => 0, + FeePriority::Low => 1, + FeePriority::Medium => 2, + FeePriority::High => 3, + FeePriority::Custom { priority, .. } => *priority, + } } } @@ -240,10 +310,11 @@ pub struct Eventuality { pub struct SignableTransaction { protocol: Protocol, r_seed: Option>, - inputs: Vec, + inputs: Vec<(SpendableOutput, Decoys)>, payments: Vec, data: Vec>, fee: u64, + fee_rate: Fee, } /// Specification for a change output. @@ -282,6 +353,49 @@ impl Change { } } +fn need_additional(payments: &[InternalPayment]) -> (bool, bool) { + let mut has_change_view = false; + let subaddresses = payments + .iter() + .filter(|payment| match *payment { + InternalPayment::Payment(payment) => payment.0.is_subaddress(), + InternalPayment::Change(change, _) => { + if change.view.is_some() { + has_change_view = true; + // It should not be possible to construct a change specification to a subaddress with a + // view key + debug_assert!(!change.address.is_subaddress()); + } + change.address.is_subaddress() + } + }) + .count() != + 0; + + // We need additional keys if we have any subaddresses + let mut additional = subaddresses; + // Unless the above change view key path is taken + if (payments.len() == 2) && has_change_view { + additional = false; + } + + (subaddresses, additional) +} + +fn sanity_check_change_payment(payments: &[InternalPayment], has_change_address: bool) { + debug_assert_eq!( + payments + .iter() + .filter(|payment| match *payment { + InternalPayment::Payment(_) => false, + InternalPayment::Change(_, _) => true, + }) + .count(), + if has_change_address { 1 } else { 0 }, + "Unexpected number of change outputs" + ); +} + impl SignableTransaction { /// Create a signable transaction. /// @@ -295,7 +409,7 @@ impl SignableTransaction { pub fn new( protocol: Protocol, r_seed: Option>, - inputs: Vec, + inputs: Vec<(SpendableOutput, Decoys)>, payments: Vec<(MoneroAddress, u64)>, change_address: Option, data: Vec>, @@ -328,6 +442,12 @@ impl SignableTransaction { Err(TransactionError::NoOutputs)?; } + for (_, decoys) in &inputs { + if decoys.len() != protocol.ring_len() { + Err(TransactionError::InvalidDecoyQuantity)?; + } + } + for part in &data { if part.len() > MAX_ARBITRARY_DATA_SIZE { Err(TransactionError::TooMuchData)?; @@ -338,13 +458,31 @@ impl SignableTransaction { if (payments.len() == 1) && change_address.is_none() { Err(TransactionError::NoChange)?; } + + // Get the outgoing amount ignoring fees + let out_amount = payments.iter().map(|payment| payment.1).sum::(); + let outputs = payments.len() + usize::from(change_address.is_some()); + if outputs > MAX_OUTPUTS { + Err(TransactionError::TooManyOutputs)?; + } + + // Collect payments in a container that includes a change output if a change address is provided + let mut payments = payments.into_iter().map(InternalPayment::Payment).collect::>(); + if change_address.is_some() { + // Push a 0 amount change output that we'll use to do fee calculations. + // We'll modify the change amount after calculating the fee + payments.push(InternalPayment::Change(change_address.clone().unwrap(), 0)); + } + + // Determine if we'll need additional pub keys in tx extra + let (_, additional) = need_additional(&payments); + // Add a dummy payment ID if there's only 2 payments has_payment_id |= outputs == 2; // Calculate the extra length - // Assume additional keys are needed in order to cause a worst-case estimation - let extra = Extra::fee_weight(outputs, true, has_payment_id, data.as_ref()); + let extra = Extra::fee_weight(outputs, additional, has_payment_id, data.as_ref()); // https://github.com/monero-project/monero/pull/8733 const MAX_EXTRA_SIZE: usize = 1060; @@ -352,48 +490,67 @@ impl SignableTransaction { Err(TransactionError::TooMuchData)?; } - // This is a extremely heavy fee weight estimation which can only be trusted for two things - // 1) Ensuring we have enough for whatever fee we end up using - // 2) Ensuring we aren't over the max size - let estimated_tx_size = Transaction::fee_weight(protocol, inputs.len(), outputs, extra); + // Caclculate weight of decoys + let decoy_weights = + inputs.iter().map(|(_, decoy)| Decoys::fee_weight(&decoy.offsets)).collect::>(); + + // Deterministically calculate tx weight and fee + let (weight, fee) = + calculate_weight_and_fee(protocol, &decoy_weights, outputs, extra, fee_rate); // The actual limit is half the block size, and for the minimum block size of 300k, that'd be // 150k // wallet2 will only create transactions up to 100k bytes however const MAX_TX_SIZE: usize = 100_000; - - // This uses the weight (estimated_tx_size) despite the BP clawback - // The clawback *increases* the weight, so this will over-estimate, yet it's still safe - if estimated_tx_size >= MAX_TX_SIZE { + if weight >= MAX_TX_SIZE { Err(TransactionError::TooLargeTransaction)?; } - // Calculate the fee. - let fee = fee_rate.calculate(estimated_tx_size); - // Make sure we have enough funds - let in_amount = inputs.iter().map(|input| input.commitment().amount).sum::(); - let out_amount = payments.iter().map(|payment| payment.1).sum::() + fee; - if in_amount < out_amount { - Err(TransactionError::NotEnoughFunds(in_amount, out_amount))?; + let in_amount = inputs.iter().map(|(input, _)| input.commitment().amount).sum::(); + if in_amount < (out_amount + fee) { + Err(TransactionError::NotEnoughFunds(in_amount, out_amount + fee))?; } - if outputs > MAX_OUTPUTS { - Err(TransactionError::TooManyOutputs)?; + // Sanity check we have the expected number of change outputs + sanity_check_change_payment(&payments, change_address.is_some()); + + // Modify the amount of the change output + if change_address.is_some() { + let change_payment = payments.last_mut().unwrap(); + debug_assert!(matches!(change_payment, InternalPayment::Change(_, _))); + *change_payment = + InternalPayment::Change(change_address.clone().unwrap(), in_amount - out_amount - fee); } - let mut payments = payments.into_iter().map(InternalPayment::Payment).collect::>(); - if let Some(change) = change_address { - payments.push(InternalPayment::Change(change, in_amount - out_amount)); - } + // Sanity check the change again after modifying + sanity_check_change_payment(&payments, change_address.is_some()); - Ok(SignableTransaction { protocol, r_seed, inputs, payments, data, fee }) + // Sanity check outgoing amount + fee == incoming amount + debug_assert_eq!( + payments + .iter() + .map(|payment| match *payment { + InternalPayment::Payment(payment) => payment.1, + InternalPayment::Change(_, amount) => amount, + }) + .sum::() + + fee, + in_amount, + "Outgoing amount + fee != incoming amount" + ); + + Ok(SignableTransaction { protocol, r_seed, inputs, payments, data, fee, fee_rate }) } pub fn fee(&self) -> u64 { self.fee } + pub fn fee_rate(&self) -> Fee { + self.fee_rate + } + #[allow(clippy::type_complexity)] fn prepare_payments( seed: &Zeroizing<[u8; 32]>, @@ -425,30 +582,7 @@ impl SignableTransaction { // If any of these outputs are to a subaddress, we need keys distinct to them // The only time this *does not* force having additional keys is when the only other output // is a change output we have the view key for, enabling rewriting rA to aR - let mut has_change_view = false; - let subaddresses = payments - .iter() - .filter(|payment| match *payment { - InternalPayment::Payment(payment) => payment.0.is_subaddress(), - InternalPayment::Change(change, _) => { - if change.view.is_some() { - has_change_view = true; - // It should not be possible to construct a change specification to a subaddress with a - // view key - debug_assert!(!change.address.is_subaddress()); - } - change.address.is_subaddress() - } - }) - .count() != - 0; - - // We need additional keys if we have any subaddresses - let mut additional = subaddresses; - // Unless the above change view key path is taken - if (payments.len() == 2) && has_change_view { - additional = false; - } + let (subaddresses, additional) = need_additional(payments); let modified_change_ecdh = subaddresses && (!additional); // If we're using the aR rewrite, update tx_public_key from rG to rB @@ -567,7 +701,7 @@ impl SignableTransaction { /// with the specified seed. This eventuality can be compared to on-chain transactions to see /// if the transaction has already been signed and published. pub fn eventuality(&self) -> Option { - let inputs = self.inputs.iter().map(SpendableOutput::key).collect::>(); + let inputs = self.inputs.iter().map(|(input, _)| input.key()).collect::>(); let (tx_key, additional, outputs, id) = Self::prepare_payments( self.r_seed.as_ref()?, &inputs, @@ -607,7 +741,7 @@ impl SignableTransaction { let (tx_key, additional, outputs, id) = Self::prepare_payments( &r_seed, - &self.inputs.iter().map(SpendableOutput::key).collect::>(), + &self.inputs.iter().map(|(input, _)| input.key()).collect::>(), &mut self.payments, uniqueness, ); @@ -629,7 +763,7 @@ impl SignableTransaction { &mut self.data, ); - let mut fee = self.inputs.iter().map(|input| input.commitment().amount).sum::(); + let mut fee = self.inputs.iter().map(|(input, _)| input.commitment().amount).sum::(); let mut tx_outputs = Vec::with_capacity(outputs.len()); let mut encrypted_amounts = Vec::with_capacity(outputs.len()); for output in &outputs { @@ -637,10 +771,11 @@ impl SignableTransaction { tx_outputs.push(Output { amount: None, key: output.dest.compress(), - view_tag: Some(output.view_tag).filter(|_| matches!(self.protocol, Protocol::v16)), + view_tag: Some(output.view_tag).filter(|_| self.protocol.view_tags()), }); encrypted_amounts.push(EncryptedAmount::Compact { amount: output.amount }); } + debug_assert_eq!(self.fee, fee, "transaction will use an unexpected fee"); ( Transaction { @@ -667,14 +802,13 @@ impl SignableTransaction { } /// Sign this transaction. - pub async fn sign( + pub async fn sign( mut self, rng: &mut R, - rpc: &Rpc, spend: &Zeroizing, ) -> Result { let mut images = Vec::with_capacity(self.inputs.len()); - for input in &self.inputs { + for (input, _) in &self.inputs { let mut offset = Zeroizing::new(spend.deref() + input.key_offset()); if (offset.deref() * &ED25519_BASEPOINT_TABLE) != input.key() { Err(TransactionError::WrongPrivateKey)?; @@ -695,8 +829,7 @@ impl SignableTransaction { ), ); - let signable = - prepare_inputs(rng, rpc, self.protocol.ring_len(), &self.inputs, spend, &mut tx).await?; + let signable = prepare_inputs(&self.inputs, spend, &mut tx)?; let clsag_pairs = Clsag::sign(rng, signable, mask_sum, tx.signature_hash()); match tx.rct_signatures.prunable { @@ -707,6 +840,13 @@ impl SignableTransaction { } _ => unreachable!("attempted to sign a TX which wasn't CLSAG"), } + + debug_assert_eq!( + self.fee_rate.calculate_fee_from_weight(tx.weight()), + tx.rct_signatures.base.fee, + "transaction used unexpected fee", + ); + Ok(tx) } } @@ -768,7 +908,7 @@ impl Eventuality { if (&Output { amount: None, key: expected.dest.compress(), - view_tag: Some(expected.view_tag).filter(|_| matches!(self.protocol, Protocol::v16)), + view_tag: Some(expected.view_tag).filter(|_| self.protocol.view_tags()), } != actual) || (Some(&expected.commitment.calculate()) != tx.rct_signatures.base.commitments.get(o)) || (Some(&EncryptedAmount::Compact { amount: expected.amount }) != diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index d60d9a5a..d352894a 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -31,10 +31,7 @@ use crate::{ RctPrunable, }, transaction::{Input, Transaction}, - rpc::{RpcConnection, Rpc}, - wallet::{ - TransactionError, InternalPayment, SignableTransaction, Decoys, key_image_sort, uniqueness, - }, + wallet::{TransactionError, InternalPayment, SignableTransaction, key_image_sort, uniqueness}, }; /// FROST signing machine to produce a signed transaction. @@ -44,8 +41,6 @@ pub struct TransactionMachine { i: Participant, transcript: RecommendedTranscript, - decoys: Vec, - // Hashed key and scalar offset key_images: Vec<(EdwardsPoint, Scalar)>, inputs: Vec>>>, @@ -58,8 +53,6 @@ pub struct TransactionSignMachine { i: Participant, transcript: RecommendedTranscript, - decoys: Vec, - key_images: Vec<(EdwardsPoint, Scalar)>, inputs: Vec>>>, clsags: Vec>, @@ -75,12 +68,10 @@ pub struct TransactionSignatureMachine { impl SignableTransaction { /// Create a FROST signing machine out of this signable transaction. /// The height is the Monero blockchain height to synchronize around. - pub async fn multisig( + pub fn multisig( self, - rpc: &Rpc, keys: ThresholdKeys, mut transcript: RecommendedTranscript, - height: usize, ) -> Result { let mut inputs = vec![]; for _ in 0 .. self.inputs.len() { @@ -97,11 +88,6 @@ impl SignableTransaction { transcript.domain_separate(b"monero_transaction"); - // Include the height we're using for our data - // The data itself will be included, making this unnecessary, yet a lot of this is technically - // unnecessary. Anything which further increases security at almost no cost should be followed - transcript.append_message(b"height", u64::try_from(height).unwrap().to_le_bytes()); - // Also include the spend_key as below only the key offset is included, so this transcripts the // sum product // Useful as transcripting the sum product effectively transcripts the key image, further @@ -112,7 +98,7 @@ impl SignableTransaction { transcript.append_message(b"r_seed", r_seed); } - for input in &self.inputs { + for (input, decoys) in &self.inputs { // These outputs can only be spent once. Therefore, it forces all RNGs derived from this // transcript (such as the one used to create one time keys) to be unique transcript.append_message(b"input_hash", input.output.absolute.tx); @@ -120,6 +106,16 @@ impl SignableTransaction { // Not including this, with a doxxed list of payments, would allow brute forcing the inputs // to determine RNG seeds and therefore the true spends transcript.append_message(b"input_shared_key", input.key_offset().to_bytes()); + + // Ensure all signers are signing the same rings + transcript.append_message(b"real_spend", [decoys.i]); + for (i, ring_member) in decoys.ring.iter().enumerate() { + transcript + .append_message(b"ring_member", [u8::try_from(i).expect("ring size exceeded 255")]); + transcript.append_message(b"ring_member_offset", decoys.offsets[i].to_le_bytes()); + transcript.append_message(b"ring_member_key", ring_member[0].compress().to_bytes()); + transcript.append_message(b"ring_member_commitment", ring_member[1].compress().to_bytes()); + } } for payment in &self.payments { @@ -139,7 +135,7 @@ impl SignableTransaction { } let mut key_images = vec![]; - for (i, input) in self.inputs.iter().enumerate() { + for (i, (input, _)) in self.inputs.iter().enumerate() { // Check this the right set of keys let offset = keys.offset(dfg::Scalar(input.key_offset())); if offset.group_key().0 != input.key() { @@ -149,36 +145,17 @@ impl SignableTransaction { let clsag = ClsagMultisig::new(transcript.clone(), input.key(), inputs[i].clone()); key_images.push(( clsag.H, - keys.current_offset().unwrap_or(dfg::Scalar::ZERO).0 + self.inputs[i].key_offset(), + keys.current_offset().unwrap_or(dfg::Scalar::ZERO).0 + self.inputs[i].0.key_offset(), )); clsags.push(AlgorithmMachine::new(clsag, offset)); } - // Select decoys - // Ideally, this would be done post entropy, instead of now, yet doing so would require sign - // to be async which isn't preferable. This should be suitably competent though - // While this inability means we can immediately create the input, moving it out of the - // Arc RwLock, keeping it within an Arc RwLock keeps our options flexible - let decoys = Decoys::select( - // Using a seeded RNG with a specific height, committed to above, should make these decoys - // committed to. They'll also be committed to later via the TX message as a whole - &mut ChaCha20Rng::from_seed(transcript.rng_seed(b"decoys")), - rpc, - self.protocol.ring_len(), - height, - &self.inputs, - ) - .await - .map_err(TransactionError::RpcError)?; - Ok(TransactionMachine { signable: self, i: keys.params().i(), transcript, - decoys, - key_images, inputs, clsags, @@ -224,8 +201,6 @@ impl PreprocessMachine for TransactionMachine { i: self.i, transcript: self.transcript, - decoys: self.decoys, - key_images: self.key_images, inputs: self.inputs, clsags, @@ -349,10 +324,11 @@ impl SignMachine for TransactionSignMachine { // Sort the inputs, as expected let mut sorted = Vec::with_capacity(self.clsags.len()); while !self.clsags.is_empty() { + let (inputs, decoys) = self.signable.inputs.swap_remove(0); sorted.push(( images.swap_remove(0), - self.signable.inputs.swap_remove(0), - self.decoys.swap_remove(0), + inputs, + decoys, self.inputs.swap_remove(0), self.clsags.swap_remove(0), commitments.swap_remove(0), diff --git a/coins/monero/tests/runner.rs b/coins/monero/tests/runner.rs index 8a480563..6121854e 100644 --- a/coins/monero/tests/runner.rs +++ b/coins/monero/tests/runner.rs @@ -14,8 +14,9 @@ use monero_serai::{ wallet::{ ViewPair, Scanner, address::{Network, AddressType, AddressSpec, AddressMeta, MoneroAddress}, - SpendableOutput, + SpendableOutput, Fee, }, + transaction::Transaction, }; pub fn random_address() -> (Scalar, ViewPair, MoneroAddress) { @@ -72,6 +73,18 @@ pub async fn get_miner_tx_output(rpc: &Rpc, view: &ViewPair) -> Spendab scanner.scan(rpc, &block).await.unwrap().swap_remove(0).ignore_timelock().swap_remove(0) } +/// Make sure the weight and fee match the expected calculation. +pub fn check_weight_and_fee(tx: &Transaction, fee_rate: Fee) { + let fee = tx.rct_signatures.base.fee; + + let weight = tx.weight(); + let expected_weight = fee_rate.calculate_weight_from_fee(fee); + assert_eq!(weight, expected_weight); + + let expected_fee = fee_rate.calculate_fee_from_weight(weight); + assert_eq!(fee, expected_fee); +} + pub async fn rpc() -> Rpc { let rpc = HttpRpc::new("http://127.0.0.1:18081".to_string()).unwrap(); @@ -154,12 +167,15 @@ macro_rules! test { use monero_serai::{ random_scalar, wallet::{ - address::{Network, AddressSpec}, ViewPair, Scanner, Change, SignableTransaction, - SignableTransactionBuilder, + address::{Network, AddressSpec}, ViewPair, Scanner, Change, Decoys, FeePriority, + SignableTransaction, SignableTransactionBuilder, }, }; - use runner::{random_address, rpc, mine_until_unlocked, get_miner_tx_output}; + use runner::{ + random_address, rpc, mine_until_unlocked, get_miner_tx_output, + check_weight_and_fee, + }; type Builder = SignableTransactionBuilder; @@ -192,9 +208,11 @@ macro_rules! test { let miner_tx = get_miner_tx_output(&rpc, &view).await; + let protocol = rpc.get_protocol().await.unwrap(); + let builder = SignableTransactionBuilder::new( - rpc.get_protocol().await.unwrap(), - rpc.get_fee().await.unwrap(), + protocol, + rpc.get_fee(protocol, FeePriority::Low).await.unwrap(), Some(Change::new( &ViewPair::new( &random_scalar(&mut OsRng) * &ED25519_BASEPOINT_TABLE, @@ -205,13 +223,12 @@ macro_rules! test { ); let sign = |tx: SignableTransaction| { - let rpc = rpc.clone(); let spend = spend.clone(); #[cfg(feature = "multisig")] let keys = keys.clone(); async move { if !multisig { - tx.sign(&mut OsRng, &rpc, &spend).await.unwrap() + tx.sign(&mut OsRng, &spend).await.unwrap() } else { #[cfg(not(feature = "multisig"))] panic!("Multisig branch called without the multisig feature"); @@ -224,12 +241,9 @@ macro_rules! test { tx .clone() .multisig( - &rpc, keys[&i].clone(), RecommendedTranscript::new(b"Monero Serai Test Transaction"), - rpc.get_height().await.unwrap() - 10, ) - .await .unwrap(), ); } @@ -245,13 +259,25 @@ macro_rules! test { let temp = Box::new({ let mut builder = builder.clone(); - builder.add_input(miner_tx); - let (tx, state) = ($first_tx)(rpc.clone(), builder, next_addr).await; + let decoys = Decoys::select( + &mut OsRng, + &rpc, + protocol.ring_len(), + rpc.get_height().await.unwrap() - 1, + &[miner_tx.clone()] + ) + .await + .unwrap(); + builder.add_input((miner_tx, decoys.first().unwrap().clone())); + + let (tx, state) = ($first_tx)(rpc.clone(), builder, next_addr).await; + let fee_rate = tx.fee_rate().clone(); let signed = sign(tx).await; rpc.publish_transaction(&signed).await.unwrap(); mine_until_unlocked(&rpc, &random_address().2.to_string(), signed.hash()).await; let tx = rpc.get_transaction(signed.hash()).await.unwrap(); + check_weight_and_fee(&tx, fee_rate); let scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); ($first_checks)(rpc.clone(), tx, scanner, state).await @@ -261,16 +287,18 @@ macro_rules! test { $( let (tx, state) = ($tx)( + protocol, rpc.clone(), builder.clone(), next_addr, *carried_state.downcast().unwrap() ).await; - + let fee_rate = tx.fee_rate().clone(); let signed = sign(tx).await; rpc.publish_transaction(&signed).await.unwrap(); mine_until_unlocked(&rpc, &random_address().2.to_string(), signed.hash()).await; let tx = rpc.get_transaction(signed.hash()).await.unwrap(); + check_weight_and_fee(&tx, fee_rate); #[allow(unused_assignments)] { let scanner = diff --git a/coins/monero/tests/send.rs b/coins/monero/tests/send.rs index 8acac581..dfc7cf58 100644 --- a/coins/monero/tests/send.rs +++ b/coins/monero/tests/send.rs @@ -1,11 +1,44 @@ +use rand_core::OsRng; + use monero_serai::{ transaction::Transaction, - wallet::{extra::Extra, address::SubaddressIndex, ReceivedOutput, SpendableOutput}, - rpc::Rpc, + wallet::{ + extra::Extra, address::SubaddressIndex, ReceivedOutput, SpendableOutput, Decoys, + SignableTransactionBuilder, + }, + rpc::{Rpc, HttpRpc}, + Protocol, }; mod runner; +// Set up inputs, select decoys, then add them to the TX builder +async fn add_inputs( + protocol: Protocol, + rpc: &Rpc, + outputs: Vec, + builder: &mut SignableTransactionBuilder, +) { + let mut spendable_outputs = Vec::with_capacity(outputs.len()); + for output in outputs { + spendable_outputs.push(SpendableOutput::from(rpc, output).await.unwrap()); + } + + let decoys = Decoys::select( + &mut OsRng, + rpc, + protocol.ring_len(), + rpc.get_height().await.unwrap() - 1, + &spendable_outputs, + ) + .await + .unwrap(); + + let inputs = spendable_outputs.into_iter().zip(decoys.into_iter()).collect::>(); + + builder.add_inputs(&inputs); +} + test!( spend_miner_output, ( @@ -37,10 +70,8 @@ test!( }, ), ( - |rpc, mut builder: Builder, addr, outputs: Vec| async move { - for output in outputs { - builder.add_input(SpendableOutput::from(&rpc, output).await.unwrap()); - } + |protocol: Protocol, rpc, mut builder: Builder, addr, outputs: Vec| async move { + add_inputs(protocol, &rpc, outputs, &mut builder).await; builder.add_payment(addr, 6); (builder.build().unwrap(), ()) }, @@ -69,18 +100,20 @@ test!( }, ), ( - |rpc: Rpc<_>, _, _, mut outputs: Vec| async move { + |protocol, rpc: Rpc<_>, _, _, outputs: Vec| async move { + use monero_serai::wallet::FeePriority; + let change_view = ViewPair::new( &random_scalar(&mut OsRng) * &ED25519_BASEPOINT_TABLE, Zeroizing::new(random_scalar(&mut OsRng)), ); let mut builder = SignableTransactionBuilder::new( - rpc.get_protocol().await.unwrap(), - rpc.get_fee().await.unwrap(), + protocol, + rpc.get_fee(protocol, FeePriority::Low).await.unwrap(), Some(Change::new(&change_view, false)), ); - builder.add_input(SpendableOutput::from(&rpc, outputs.swap_remove(0)).await.unwrap()); + add_inputs(protocol, &rpc, vec![outputs.first().unwrap().clone()], &mut builder).await; // Send to a subaddress let sub_view = ViewPair::new( @@ -116,3 +149,127 @@ test!( }, ), ); + +test!( + spend_one_input_to_one_output_plus_change, + ( + |_, mut builder: Builder, addr| async move { + builder.add_payment(addr, 2000000000000); + (builder.build().unwrap(), ()) + }, + |_, tx: Transaction, mut scanner: Scanner, _| async move { + let mut outputs = scanner.scan_transaction(&tx).not_locked(); + outputs.sort_by(|x, y| x.commitment().amount.cmp(&y.commitment().amount)); + assert_eq!(outputs[0].commitment().amount, 2000000000000); + outputs + }, + ), + ( + |protocol: Protocol, rpc, mut builder: Builder, addr, outputs: Vec| async move { + add_inputs(protocol, &rpc, outputs, &mut builder).await; + builder.add_payment(addr, 2); + (builder.build().unwrap(), ()) + }, + |_, tx: Transaction, mut scanner: Scanner, _| async move { + let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 2); + }, + ), +); + +test!( + spend_max_outputs, + ( + |_, mut builder: Builder, addr| async move { + builder.add_payment(addr, 1000000000000); + (builder.build().unwrap(), ()) + }, + |_, tx: Transaction, mut scanner: Scanner, _| async move { + let mut outputs = scanner.scan_transaction(&tx).not_locked(); + outputs.sort_by(|x, y| x.commitment().amount.cmp(&y.commitment().amount)); + assert_eq!(outputs[0].commitment().amount, 1000000000000); + outputs + }, + ), + ( + |protocol: Protocol, rpc, mut builder: Builder, addr, outputs: Vec| async move { + add_inputs(protocol, &rpc, outputs, &mut builder).await; + + for i in 0 .. 15 { + builder.add_payment(addr, i + 1); + } + (builder.build().unwrap(), ()) + }, + |_, tx: Transaction, mut scanner: Scanner, _| async move { + let mut scanned_tx = scanner.scan_transaction(&tx).not_locked(); + + let mut output_amounts = HashSet::new(); + for i in 0 .. 15 { + output_amounts.insert((i + 1) as u64); + } + for _ in 0 .. 15 { + let output = scanned_tx.swap_remove(0); + let amount = output.commitment().amount; + assert!(output_amounts.contains(&amount)); + output_amounts.remove(&amount); + } + }, + ), +); + +test!( + spend_max_outputs_to_subaddresses, + ( + |_, mut builder: Builder, addr| async move { + builder.add_payment(addr, 1000000000000); + (builder.build().unwrap(), ()) + }, + |_, tx: Transaction, mut scanner: Scanner, _| async move { + let mut outputs = scanner.scan_transaction(&tx).not_locked(); + outputs.sort_by(|x, y| x.commitment().amount.cmp(&y.commitment().amount)); + assert_eq!(outputs[0].commitment().amount, 1000000000000); + outputs + }, + ), + ( + |protocol: Protocol, rpc, mut builder: Builder, _, outputs: Vec| async move { + add_inputs(protocol, &rpc, outputs, &mut builder).await; + + let view = runner::random_address().1; + let mut scanner = Scanner::from_view(view.clone(), Some(HashSet::new())); + + let mut subaddresses = vec![]; + for i in 0 .. 15 { + let subaddress = SubaddressIndex::new(0, i + 1).unwrap(); + scanner.register_subaddress(subaddress); + + builder.add_payment( + view.address(Network::Mainnet, AddressSpec::Subaddress(subaddress)), + (i + 1) as u64, + ); + subaddresses.push(subaddress); + } + + (builder.build().unwrap(), (scanner, subaddresses)) + }, + |_, tx: Transaction, _, mut state: (Scanner, Vec)| async move { + use std::collections::HashMap; + + let mut scanned_tx = state.0.scan_transaction(&tx).not_locked(); + + let mut output_amounts_by_subaddress = HashMap::new(); + for i in 0 .. 15 { + output_amounts_by_subaddress.insert((i + 1) as u64, state.1[i]); + } + for _ in 0 .. 15 { + let output = scanned_tx.swap_remove(0); + let amount = output.commitment().amount; + + assert!(output_amounts_by_subaddress.contains_key(&amount)); + assert_eq!(output.metadata.subaddress, Some(output_amounts_by_subaddress[&amount])); + + output_amounts_by_subaddress.remove(&amount); + } + }, + ), +); diff --git a/coins/monero/tests/wallet2_compatibility.rs b/coins/monero/tests/wallet2_compatibility.rs index 9bd50161..ed41a12e 100644 --- a/coins/monero/tests/wallet2_compatibility.rs +++ b/coins/monero/tests/wallet2_compatibility.rs @@ -84,19 +84,30 @@ async fn from_wallet_rpc_to_self(spec: AddressSpec) { .unwrap(); let tx_hash: [u8; 32] = tx.tx_hash.0.try_into().unwrap(); + // TODO: Needs https://github.com/monero-project/monero/pull/8882 + // let fee_rate = daemon_rpc + // // Uses `FeePriority::Low` instead of `FeePriority::Lowest` because wallet RPC + // // adjusts `monero_rpc::TransferPriority::Default` up by 1 + // .get_fee(daemon_rpc.get_protocol().await.unwrap(), FeePriority::Low) + // .await + // .unwrap(); + // unlock it runner::mine_until_unlocked(&daemon_rpc, &wallet_rpc_addr.to_string(), tx_hash).await; - // create the scanner + // Create the scanner let mut scanner = Scanner::from_view(view_pair, Some(HashSet::new())); if let AddressSpec::Subaddress(index) = spec { scanner.register_subaddress(index); } - // retrieve it and confirm + // Retrieve it and scan it let tx = daemon_rpc.get_transaction(tx_hash).await.unwrap(); let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0); + // TODO: Needs https://github.com/monero-project/monero/pull/8882 + // runner::check_weight_and_fee(&tx, fee_rate); + match spec { AddressSpec::Subaddress(index) => assert_eq!(output.metadata.subaddress, Some(index)), AddressSpec::Integrated(payment_id) => { diff --git a/processor/src/coins/monero.rs b/processor/src/coins/monero.rs index bb83a58a..208dfff7 100644 --- a/processor/src/coins/monero.rs +++ b/processor/src/coins/monero.rs @@ -4,7 +4,10 @@ use async_trait::async_trait; use zeroize::Zeroizing; -use transcript::RecommendedTranscript; +use rand_core::SeedableRng; +use rand_chacha::ChaCha20Rng; + +use transcript::{Transcript, RecommendedTranscript}; use ciphersuite::group::{ff::Field, Group}; use dalek_ff_group::{Scalar, EdwardsPoint}; @@ -18,8 +21,8 @@ use monero_serai::{ wallet::{ ViewPair, Scanner, address::{Network, SubaddressIndex, AddressSpec}, - Fee, SpendableOutput, Change, TransactionError, SignableTransaction as MSignableTransaction, - Eventuality, TransactionMachine, + Fee, SpendableOutput, Change, Decoys, TransactionError, + SignableTransaction as MSignableTransaction, Eventuality, TransactionMachine, }, }; @@ -129,8 +132,6 @@ impl EventualityTrait for Eventuality { pub struct SignableTransaction { keys: ThresholdKeys, transcript: RecommendedTranscript, - // Monero height, defined as the length of the chain - height: usize, actual: MSignableTransaction, } @@ -398,6 +399,25 @@ impl Coin for Monero { // Check a fork hasn't occurred which this processor hasn't been updated for assert_eq!(protocol, self.rpc.get_protocol().await.map_err(|_| CoinError::ConnectionError)?); + let spendable_outputs = plan.inputs.iter().cloned().map(|input| input.0).collect::>(); + + let mut transcript = plan.transcript(); + + // All signers need to select the same decoys + // All signers use the same height and a seeded RNG to make sure they do so. + let decoys = Decoys::select( + &mut ChaCha20Rng::from_seed(transcript.rng_seed(b"decoys")), + &self.rpc, + protocol.ring_len(), + block_number + 1, + &spendable_outputs, + ) + .await + .map_err(|_| CoinError::ConnectionError) + .unwrap(); + + let inputs = spendable_outputs.into_iter().zip(decoys.into_iter()).collect::>(); + let signable = |plan: &mut Plan, tx_fee: Option<_>| { // Monero requires at least two outputs // If we only have one output planned, add a dummy payment @@ -432,7 +452,7 @@ impl Coin for Monero { // This perfectly binds the plan while simultaneously allowing verifying the plan was // executed with no additional communication Some(Zeroizing::new(plan.id())), - plan.inputs.iter().cloned().map(|input| input.0).collect(), + inputs.clone(), payments, plan.change.map(|key| { Change::fingerprintable(Self::address_internal(key, CHANGE_SUBADDRESS).into()) @@ -447,6 +467,7 @@ impl Coin for Monero { } TransactionError::NoInputs | TransactionError::NoOutputs | + TransactionError::InvalidDecoyQuantity | TransactionError::NoChange | TransactionError::TooManyOutputs | TransactionError::TooMuchData | @@ -483,8 +504,7 @@ impl Coin for Monero { let signable = SignableTransaction { keys, - transcript: plan.transcript(), - height: block_number + 1, + transcript, actual: match signable(&mut plan, Some(tx_fee))? { Some(signable) => signable, None => return Ok((None, branch_outputs)), @@ -498,17 +518,10 @@ impl Coin for Monero { &self, transaction: SignableTransaction, ) -> Result { - transaction - .actual - .clone() - .multisig( - &self.rpc, - transaction.keys.clone(), - transaction.transcript.clone(), - transaction.height, - ) - .await - .map_err(|_| CoinError::ConnectionError) + match transaction.actual.clone().multisig(transaction.keys.clone(), transaction.transcript) { + Ok(machine) => Ok(machine), + Err(e) => panic!("failed to create a multisig machine for TX: {e}"), + } } async fn publish_transaction(&self, tx: &Self::Transaction) -> Result<(), CoinError> { @@ -536,7 +549,9 @@ impl Coin for Monero { #[cfg(test)] async fn get_fee(&self) -> Self::Fee { - self.rpc.get_fee().await.unwrap() + use monero_serai::wallet::FeePriority; + + self.rpc.get_fee(self.rpc.get_protocol().await.unwrap(), FeePriority::Low).await.unwrap() } #[cfg(test)] @@ -566,6 +581,7 @@ impl Coin for Monero { async fn test_send(&self, address: Self::Address) -> Block { use zeroize::Zeroizing; use rand_core::OsRng; + use monero_serai::wallet::FeePriority; let new_block = self.get_latest_block_number().await.unwrap() + 1; for _ in 0 .. 80 { @@ -583,17 +599,31 @@ impl Coin for Monero { // The dust should always be sufficient for the fee let fee = Monero::DUST; + let protocol = self.rpc.get_protocol().await.unwrap(); + + let decoys = Decoys::select( + &mut OsRng, + &self.rpc, + protocol.ring_len(), + self.rpc.get_height().await.unwrap() - 1, + &outputs, + ) + .await + .unwrap(); + + let inputs = outputs.into_iter().zip(decoys.into_iter()).collect::>(); + let tx = MSignableTransaction::new( - self.rpc.get_protocol().await.unwrap(), + protocol, None, - outputs, + inputs, vec![(address.into(), amount - fee)], Some(Change::fingerprintable(Self::test_address().into())), vec![], - self.rpc.get_fee().await.unwrap(), + self.rpc.get_fee(protocol, FeePriority::Low).await.unwrap(), ) .unwrap() - .sign(&mut OsRng, &self.rpc, &Zeroizing::new(Scalar::ONE.0)) + .sign(&mut OsRng, &Zeroizing::new(Scalar::ONE.0)) .await .unwrap();