absolutebi/serai
1
0
Fork 0
mirror of https://github.com/serai-dex/serai.git synced 2025-12-10 13:09:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

Distinguish fee from necessary_fee in monero-wallet

Browse Source 
If there's no change, the fee is difference of the inputs to the outputs. The
prior code wouldn't check that amount is greater than or equal to the necessary
fee, and returning the would-be change amount as the fee isn't necessarily
helpful.

Now the fee is validated in such cases and the necessary fee is returned,
enabling operating off of that.
This commit is contained in:
Luke Parker
2024-07-06 22:22:32 -04:00
parent 9e4d83bb2c
commit d88bd70f7a
4 changed files with 57 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
coins/monero/wallet/src/send/mod.rs
View File

@@ -193,18 +193,20 @@ pub enum SendError {
/// This transaction could not pay for itself.
#[cfg_attr(
feature = "std",
error("not enough funds (inputs {inputs}, outputs {outputs}, fee {fee:?})")
error(
"not enough funds (inputs {inputs}, outputs {outputs}, necessary_fee {necessary_fee:?})"
)
)]
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.
/// The fee necessary to 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>,
necessary_fee: Option<u64>,
},
/// This transaction is being signed with the wrong private key.
#[cfg_attr(feature = "std", error("wrong spend private key"))]
@@ -321,22 +323,19 @@ impl SignableTransaction {
InternalPayment::Change(_, _) => None,
})
.sum::<u64>();
// Necessary so weight_and_fee doesn't underflow
if in_amount < payments_amount {
Err(SendError::NotEnoughFunds { inputs: in_amount, outputs: payments_amount, fee: None })?;
}
let (weight, fee) = self.weight_and_fee();
if in_amount < (payments_amount + fee) {
let (weight, necessary_fee) = self.weight_and_necessary_fee();
if in_amount < (payments_amount + necessary_fee) {
Err(SendError::NotEnoughFunds {
inputs: in_amount,
outputs: payments_amount,
fee: Some(fee),
necessary_fee: Some(necessary_fee),
})?;
}
// 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
// TODO: Cite
const MAX_TX_SIZE: usize = 100_000;
if weight >= MAX_TX_SIZE {
Err(SendError::TooLargeTransaction)?;
@@ -394,9 +393,12 @@ impl SignableTransaction {
self.fee_rate
}
/// The fee this transaction will use.
pub fn fee(&self) -> u64 {
self.weight_and_fee().1
/// The fee this transaction requires.
///
/// This is distinct from the fee this transaction will use. If no change output is specified,
/// all unspent coins will be shunted to the fee.
pub fn necessary_fee(&self) -> u64 {
self.weight_and_necessary_fee().1
}
/// Write a SignableTransaction.

55
coins/monero/wallet/src/send/tx.rs
View File

@@ -105,7 +105,7 @@ impl SignableTransaction {
serialized
}
pub(crate) fn weight_and_fee(&self) -> (usize, u64) {
pub(crate) fn weight_and_necessary_fee(&self) -> (usize, u64) {
/*
This transaction is variable length to:
- The decoy offsets (fixed)
@@ -223,22 +223,6 @@ impl SignableTransaction {
1
};
// If we don't have a change output, the difference is the fee
if !self.payments.iter().any(|payment| matches!(payment, InternalPayment::Change(_, _))) {
let inputs = self.inputs.iter().map(|input| input.0.commitment().amount).sum::<u64>();
let payments = self
.payments
.iter()
.filter_map(|payment| match payment {
InternalPayment::Payment(_, amount) => Some(amount),
InternalPayment::Change(_, _) => None,
})
.sum::<u64>();
// Safe since the constructor checks inputs > payments before any calls to weight_and_fee
let fee = inputs - payments;
return (base_weight + varint_len(fee), fee);
}
// We now have the base weight, without the fee encoded
// The fee itself will impact the weight as its encoding is [1, 9] bytes long
let mut possible_weights = Vec::with_capacity(9);
@@ -255,17 +239,17 @@ impl SignableTransaction {
// We now look for the fee whose length matches the length used to derive it
let mut weight_and_fee = None;
for (len, possible_fee) in possible_fees.into_iter().enumerate() {
let len = 1 + len;
debug_assert!(1 <= len);
debug_assert!(len <= 9);
for (fee_len, possible_fee) in possible_fees.into_iter().enumerate() {
let fee_len = 1 + fee_len;
debug_assert!(1 <= fee_len);
debug_assert!(fee_len <= 9);
// We use the first fee whose encoded length is not larger than the length used within this
// weight
// This should be because the lengths are equal, yet means if somehow none are equal, this
// will still terminate successfully
if varint_len(possible_fee) <= len {
weight_and_fee = Some((base_weight + len, possible_fee));
if varint_len(possible_fee) <= fee_len {
weight_and_fee = Some((base_weight + fee_len, possible_fee));
break;
}
}
@@ -304,7 +288,30 @@ impl SignableTransactionWithKeyImages {
},
proofs: Some(RctProofs {
base: RctBase {
fee: self.intent.weight_and_fee().1,
fee: if self
.intent
.payments
.iter()
.any(|payment| matches!(payment, InternalPayment::Change(_, _)))
{
// The necessary fee is the fee
self.intent.weight_and_necessary_fee().1
} else {
// If we don't have a change output, the difference is the fee
let inputs =
self.intent.inputs.iter().map(|input| input.0.commitment().amount).sum::<u64>();
let payments = self
.intent
.payments
.iter()
.filter_map(|payment| match payment {
InternalPayment::Payment(_, amount) => Some(amount),
InternalPayment::Change(_, _) => None,
})
.sum::<u64>();
// Safe since the constructor checks inputs >= (payments + fee)
inputs - payments
},
encrypted_amounts,
pseudo_outs: vec![],
commitments,

6
coins/monero/wallet/src/send/tx_keys.rs
View File

@@ -217,9 +217,9 @@ impl SignableTransaction {
InternalPayment::Change(_, _) => None,
})
.sum::<u64>();
let fee = self.weight_and_fee().1;
// Safe since the constructor checked this
inputs - (payments + fee)
let necessary_fee = self.weight_and_necessary_fee().1;
// Safe since the constructor checked this TX has enough funds for itself
inputs - (payments + necessary_fee)
}
};
let commitment = Commitment::new(shared_key_derivations.commitment_mask(), amount);

16
processor/src/networks/monero.rs
View File

@@ -159,7 +159,7 @@ impl EventualityTrait for Eventuality {
pub struct SignableTransaction(MSignableTransaction);
impl SignableTransactionTrait for SignableTransaction {
fn fee(&self) -> u64 {
self.0.fee()
self.0.necessary_fee()
}
}
@@ -293,7 +293,7 @@ impl Monero {
let fee = fees.get(fees.len() / 2).copied().unwrap_or(0);
// TODO: Set a sane minimum fee
const MINIMUM_FEE: u64 = 5_000_000;
const MINIMUM_FEE: u64 = 50_000;
Ok(FeeRate::new(fee.max(MINIMUM_FEE), 10000).unwrap())
}
@@ -383,7 +383,7 @@ impl Monero {
) {
Ok(signable) => Ok(Some({
if calculating_fee {
MakeSignableTransactionResult::Fee(signable.fee())
MakeSignableTransactionResult::Fee(signable.necessary_fee())
} else {
MakeSignableTransactionResult::SignableTransaction(signable)
}
@@ -405,17 +405,17 @@ impl Monero {
SendError::MultiplePaymentIds => {
panic!("multiple payment IDs despite not supporting integrated addresses");
}
SendError::NotEnoughFunds { inputs, outputs, fee } => {
SendError::NotEnoughFunds { inputs, outputs, necessary_fee } => {
log::debug!(
"Monero NotEnoughFunds. inputs: {:?}, outputs: {:?}, fee: {fee:?}",
"Monero NotEnoughFunds. inputs: {:?}, outputs: {:?}, necessary_fee: {necessary_fee:?}",
inputs,
outputs
);
match fee {
Some(fee) => {
match necessary_fee {
Some(necessary_fee) => {
// If we're solely calculating the fee, return the fee this TX will cost
if calculating_fee {
Ok(Some(MakeSignableTransactionResult::Fee(fee)))
Ok(Some(MakeSignableTransactionResult::Fee(necessary_fee)))
} else {
// If we're actually trying to make the TX, return None
Ok(None)