Skip to content

Commit

Permalink
feat: use Amount on calculate_fee, fee_absolute, fee_amount a…
Browse files Browse the repository at this point in the history
…nd others

- update to use `bitcoin::Amount` on `CreateTxError::FeeTooLow` variant.
- update to use `bitcoin::Amount` on `Wallet::calculate_fee()`.
- update to use `bitcoin::Amount` on `PreviousFee` `absolute` field.
- update to use `bitcoin::Amount` on `FeePolicy::FeeAmount` variant.
- update to use `bitcoin::Amount` on `FeePolicy::fee_absolute()`.
- update to use `bitcoin::SignedAmount` on
  `CalculateFeeError::NegativeFee` variant.
- update to use `bitcoin::Amount` on `TxGraph::calculate_fee()`.
- update to use `bitcoin::Amount` on `PsbUtils::fee_amount()`
  • Loading branch information
oleonardolima committed May 6, 2024
1 parent dcd2d47 commit fb4a337
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 124 deletions.
15 changes: 5 additions & 10 deletions crates/bdk/src/psbt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub trait PsbtUtils {

/// The total transaction fee amount, sum of input amounts minus sum of output amounts, in sats.
/// If the PSBT is missing a TxOut for an input returns None.
fn fee_amount(&self) -> Option<u64>;
fn fee_amount(&self) -> Option<Amount>;

/// The transaction's fee rate. This value will only be accurate if calculated AFTER the
/// `Psbt` is finalized and all witness/signature data is added to the
Expand All @@ -49,18 +49,13 @@ impl PsbtUtils for Psbt {
}
}

fn fee_amount(&self) -> Option<u64> {
fn fee_amount(&self) -> Option<Amount> {
let tx = &self.unsigned_tx;
let utxos: Option<Vec<TxOut>> = (0..tx.input.len()).map(|i| self.get_utxo_for(i)).collect();

utxos.map(|inputs| {
let input_amount: u64 = inputs.iter().map(|i| i.value.to_sat()).sum();
let output_amount: u64 = self
.unsigned_tx
.output
.iter()
.map(|o| o.value.to_sat())
.sum();
let input_amount: Amount = inputs.iter().map(|i| i.value).sum();
let output_amount: Amount = self.unsigned_tx.output.iter().map(|o| o.value).sum();
input_amount
.checked_sub(output_amount)
.expect("input amount must be greater than output amount")
Expand All @@ -70,6 +65,6 @@ impl PsbtUtils for Psbt {
fn fee_rate(&self) -> Option<FeeRate> {
let fee_amount = self.fee_amount();
let weight = self.clone().extract_tx().ok()?.weight();
fee_amount.map(|fee| Amount::from_sat(fee) / weight)
fee_amount.map(|fee| fee / weight)
}
}
6 changes: 3 additions & 3 deletions crates/bdk/src/wallet/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::descriptor::DescriptorError;
use crate::wallet::coin_selection;
use crate::{descriptor, KeychainKind};
use alloc::string::String;
use bitcoin::{absolute, psbt, OutPoint, Sequence, Txid};
use bitcoin::{absolute, psbt, Amount, OutPoint, Sequence, Txid};
use core::fmt;

/// Errors returned by miniscript when updating inconsistent PSBTs
Expand Down Expand Up @@ -78,8 +78,8 @@ pub enum CreateTxError {
},
/// When bumping a tx the absolute fee requested is lower than replaced tx absolute fee
FeeTooLow {
/// Required fee absolute value (satoshi)
required: u64,
/// Required fee absolute value [`Amount`]
required: Amount,
},
/// When bumping a tx the fee rate requested is lower than required
FeeRateTooLow {
Expand Down
30 changes: 15 additions & 15 deletions crates/bdk/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ impl Wallet {
self.persist.stage(ChangeSet::from(additions));
}

/// Calculates the fee of a given transaction. Returns 0 if `tx` is a coinbase transaction.
/// Calculates the fee of a given transaction. Returns [`Amount::ZERO`] if `tx` is a coinbase transaction.
///
/// To calculate the fee for a [`Transaction`] with inputs not owned by this wallet you must
/// manually insert the TxOut(s) into the tx graph using the [`insert_txout`] function.
Expand All @@ -917,7 +917,7 @@ impl Wallet {
/// let fee = wallet.calculate_fee(tx).expect("fee");
/// ```
/// [`insert_txout`]: Self::insert_txout
pub fn calculate_fee(&self, tx: &Transaction) -> Result<u64, CalculateFeeError> {
pub fn calculate_fee(&self, tx: &Transaction) -> Result<Amount, CalculateFeeError> {
self.indexed_graph.graph().calculate_fee(tx)
}

Expand Down Expand Up @@ -949,8 +949,7 @@ impl Wallet {
/// ```
/// [`insert_txout`]: Self::insert_txout
pub fn calculate_fee_rate(&self, tx: &Transaction) -> Result<FeeRate, CalculateFeeError> {
self.calculate_fee(tx)
.map(|fee| Amount::from_sat(fee) / tx.weight())
self.calculate_fee(tx).map(|fee| fee / tx.weight())
}

/// Compute the `tx`'s sent and received [`Amount`]s.
Expand Down Expand Up @@ -1405,7 +1404,7 @@ impl Wallet {
});
}
}
(rate, 0)
(rate, Amount::ZERO)
}
};

Expand All @@ -1421,8 +1420,9 @@ impl Wallet {
}

// we keep it as a float while we accumulate it, and only round it at the end
let mut outgoing: u64 = 0;
let mut received: u64 = 0;
// (it's internally implemented as a float as Amount(u64))
let mut outgoing = Amount::ZERO;
let mut received = Amount::ZERO;

let recipients = params.recipients.iter().map(|(r, v)| (r, *v));

Expand All @@ -1435,7 +1435,7 @@ impl Wallet {
}

if self.is_mine(script_pubkey) {
received += value;
received += Amount::from_sat(value);
}

let new_out = TxOut {
Expand All @@ -1445,10 +1445,10 @@ impl Wallet {

tx.output.push(new_out);

outgoing += value;
outgoing += Amount::from_sat(value);
}

fee_amount += (fee_rate * tx.weight()).to_sat();
fee_amount += fee_rate * tx.weight();

if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeAllowed
&& internal_descriptor.is_none()
Expand Down Expand Up @@ -1484,10 +1484,10 @@ impl Wallet {
required_utxos,
optional_utxos,
fee_rate,
outgoing + fee_amount,
(outgoing + fee_amount).to_sat(), // FIXME: (@leonardo) I think this should be also updated to expect `Amount`
&drain_script,
)?;
fee_amount += coin_selection.fee_amount;
fee_amount += Amount::from_sat(coin_selection.fee_amount);
let excess = &coin_selection.excess;

tx.input = coin_selection
Expand Down Expand Up @@ -1529,12 +1529,12 @@ impl Wallet {
match excess {
NoChange {
remaining_amount, ..
} => fee_amount += remaining_amount,
} => fee_amount += Amount::from_sat(*remaining_amount),
Change { amount, fee } => {
if self.is_mine(&drain_script) {
received += amount;
received += Amount::from_sat(*amount);
}
fee_amount += fee;
fee_amount += Amount::from_sat(*fee);

// create drain output
let drain_output = TxOut {
Expand Down
14 changes: 7 additions & 7 deletions crates/bdk/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,14 @@ pub(crate) struct TxParams {

#[derive(Clone, Copy, Debug)]
pub(crate) struct PreviousFee {
pub absolute: u64,
pub absolute: Amount,
pub rate: FeeRate,
}

#[derive(Debug, Clone, Copy)]
pub(crate) enum FeePolicy {
FeeRate(FeeRate),
FeeAmount(u64),
FeeAmount(Amount),
}

impl Default for FeePolicy {
Expand Down Expand Up @@ -204,15 +204,15 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
}

/// Set an absolute fee
/// The fee_absolute method refers to the absolute transaction fee in satoshis (sats).
/// If anyone sets both the fee_absolute method and the fee_rate method,
/// the FeePolicy enum will be set by whichever method was called last,
/// as the FeeRate and FeeAmount are mutually exclusive.
/// The fee_absolute method refers to the absolute transaction fee in [`Amount`].
/// If anyone sets both the `fee_absolute` method and the `fee_rate` method,
/// the [`FeePolicy`] enum will be set by whichever method was called last,
/// as the [`FeeRate`] and [`FeeAmount`] are mutually exclusive.
///
/// Note that this is really a minimum absolute fee -- it's possible to
/// overshoot it slightly since adding a change output to drain the remaining
/// excess might not be viable.
pub fn fee_absolute(&mut self, fee_amount: u64) -> &mut Self {
pub fn fee_absolute(&mut self, fee_amount: Amount) -> &mut Self {
self.params.fee_policy = Some(FeePolicy::FeeAmount(fee_amount));
self
}
Expand Down
Loading

0 comments on commit fb4a337

Please sign in to comment.