Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add further bitcoin::Amount usage on public APIs #1426

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 12 additions & 15 deletions crates/chain/src/tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ use crate::{
use alloc::collections::vec_deque::VecDeque;
use alloc::sync::Arc;
use alloc::vec::Vec;
use bitcoin::{Amount, OutPoint, Script, Transaction, TxOut, Txid};
use bitcoin::{Amount, OutPoint, Script, SignedAmount, Transaction, TxOut, Txid};
use core::fmt::{self, Formatter};
use core::{
convert::Infallible,
Expand Down Expand Up @@ -182,7 +182,7 @@ pub enum CalculateFeeError {
/// Missing `TxOut` for one or more of the inputs of the tx
MissingTxOut(Vec<OutPoint>),
/// When the transaction is invalid according to the graph it has a negative fee
NegativeFee(i64),
NegativeFee(SignedAmount),
ValuedMammal marked this conversation as resolved.
Show resolved Hide resolved
}

impl fmt::Display for CalculateFeeError {
Expand All @@ -196,7 +196,7 @@ impl fmt::Display for CalculateFeeError {
CalculateFeeError::NegativeFee(fee) => write!(
f,
"transaction is invalid according to the graph and has negative fee: {}",
fee
fee.display_dynamic()
),
}
}
Expand Down Expand Up @@ -307,7 +307,7 @@ impl<A> TxGraph<A> {
})
}

/// 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.
/// Returns `OK(_)` if we have all the [`TxOut`]s being spent by `tx` in the graph (either as
/// the full transactions or individual txouts).
///
Expand All @@ -318,20 +318,20 @@ impl<A> TxGraph<A> {
/// Note `tx` does not have to be in the graph for this to work.
///
/// [`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> {
if tx.is_coinbase() {
return Ok(0);
return Ok(Amount::ZERO);
}

let (inputs_sum, missing_outputs) = tx.input.iter().fold(
(0_i64, Vec::new()),
(SignedAmount::ZERO, Vec::new()),
ValuedMammal marked this conversation as resolved.
Show resolved Hide resolved
|(mut sum, mut missing_outpoints), txin| match self.get_txout(txin.previous_output) {
None => {
missing_outpoints.push(txin.previous_output);
(sum, missing_outpoints)
}
Some(txout) => {
sum += txout.value.to_sat() as i64;
sum += txout.value.to_signed().expect("valid `SignedAmount`");
(sum, missing_outpoints)
}
},
Expand All @@ -343,15 +343,12 @@ impl<A> TxGraph<A> {
let outputs_sum = tx
.output
.iter()
.map(|txout| txout.value.to_sat() as i64)
.sum::<i64>();
.map(|txout| txout.value.to_signed().expect("valid `SignedAmount`"))
.sum::<SignedAmount>();

let fee = inputs_sum - outputs_sum;
if fee < 0 {
Err(CalculateFeeError::NegativeFee(fee))
} else {
Ok(fee as u64)
}
fee.to_unsigned()
.map_err(|_| CalculateFeeError::NegativeFee(fee))
}

/// The transactions spending from this output.
Expand Down
10 changes: 5 additions & 5 deletions crates/chain/tests/test_tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use bdk_chain::{
Anchor, Append, BlockId, ChainOracle, ChainPosition, ConfirmationHeightAnchor,
};
use bitcoin::{
absolute, hashes::Hash, transaction, Amount, BlockHash, OutPoint, ScriptBuf, Transaction, TxIn,
TxOut, Txid,
absolute, hashes::Hash, transaction, Amount, BlockHash, OutPoint, ScriptBuf, SignedAmount,
Transaction, TxIn, TxOut, Txid,
};
use common::*;
use core::iter;
Expand Down Expand Up @@ -466,14 +466,14 @@ fn test_calculate_fee() {
}],
};

assert_eq!(graph.calculate_fee(&tx), Ok(100));
assert_eq!(graph.calculate_fee(&tx), Ok(Amount::from_sat(100)));

tx.input.remove(2);

// fee would be negative, should return CalculateFeeError::NegativeFee
assert_eq!(
graph.calculate_fee(&tx),
Err(CalculateFeeError::NegativeFee(-200))
Err(CalculateFeeError::NegativeFee(SignedAmount::from_sat(-200)))
);

// If we have an unknown outpoint, fee should return CalculateFeeError::MissingTxOut.
Expand Down Expand Up @@ -505,7 +505,7 @@ fn test_calculate_fee_on_coinbase() {

let graph = TxGraph::<()>::default();

assert_eq!(graph.calculate_fee(&tx), Ok(0));
assert_eq!(graph.calculate_fee(&tx), Ok(Amount::ZERO));
}

// `test_walk_ancestors` uses the following transaction structure:
Expand Down
3 changes: 2 additions & 1 deletion crates/electrum/tests/test_electrum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ fn scan_detects_confirmed_tx() -> anyhow::Result<()> {
.fee
.expect("Fee must exist")
.abs()
.to_sat() as u64;
.to_unsigned()
.expect("valid `SignedAmount`");

// Check that the calculated fee matches the fee from the transaction data.
assert_eq!(fee, tx_fee);
Expand Down
3 changes: 2 additions & 1 deletion crates/esplora/tests/async_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ pub async fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
.fee
.expect("Fee must exist")
.abs()
.to_sat() as u64;
.to_unsigned()
.expect("valid `Amount`");

// Check that the calculated fee matches the fee from the transaction data.
assert_eq!(fee, tx_fee);
Expand Down
3 changes: 2 additions & 1 deletion crates/esplora/tests/blocking_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
.fee
.expect("Fee must exist")
.abs()
.to_sat() as u64;
.to_unsigned()
.expect("valid `Amount`");

// Check that the calculated fee matches the fee from the transaction data.
assert_eq!(fee, tx_fee);
Expand Down
15 changes: 5 additions & 10 deletions crates/wallet/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)
}
}
8 changes: 4 additions & 4 deletions crates/wallet/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,
ValuedMammal marked this conversation as resolved.
Show resolved Hide resolved
},
/// When bumping a tx the fee rate requested is lower than required
FeeRateTooLow {
Expand Down Expand Up @@ -160,7 +160,7 @@ impl fmt::Display for CreateTxError {
)
}
CreateTxError::FeeTooLow { required } => {
write!(f, "Fee to low: required {} sat", required)
write!(f, "Fee to low: required {}", required.display_dynamic())
}
CreateTxError::FeeRateTooLow { required } => {
write!(
Expand Down
24 changes: 11 additions & 13 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,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 @@ -1004,7 +1004,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 @@ -1036,8 +1036,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 @@ -1465,7 +1464,7 @@ impl Wallet {
if let Some(previous_fee) = params.bumping_fee {
if fee < previous_fee.absolute {
return Err(CreateTxError::FeeTooLow {
required: previous_fee.absolute,
required: Amount::from_sat(previous_fee.absolute),
});
}
}
Expand Down Expand Up @@ -1498,9 +1497,8 @@ impl Wallet {
return Err(CreateTxError::NoUtxosSelected);
}

// 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;
let mut outgoing = Amount::ZERO;
let mut received = Amount::ZERO;

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

Expand All @@ -1513,7 +1511,7 @@ impl Wallet {
}

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

let new_out = TxOut {
Expand All @@ -1523,7 +1521,7 @@ impl Wallet {

tx.output.push(new_out);

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

fee_amount += (fee_rate * tx.weight()).to_sat();
Expand Down Expand Up @@ -1565,7 +1563,7 @@ impl Wallet {
required_utxos,
optional_utxos,
fee_rate,
outgoing + fee_amount,
outgoing.to_sat() + fee_amount,
&drain_script,
)?;
fee_amount += coin_selection.fee_amount;
Expand Down Expand Up @@ -1613,7 +1611,7 @@ impl Wallet {
} => fee_amount += remaining_amount,
Change { amount, fee } => {
if self.is_mine(&drain_script) {
received += amount;
received += Amount::from_sat(*amount);
}
fee_amount += fee;

Expand Down Expand Up @@ -1797,7 +1795,7 @@ impl Wallet {
.collect(),
utxos: original_utxos,
bumping_fee: Some(tx_builder::PreviousFee {
absolute: fee,
absolute: fee.to_sat(),
rate: fee_rate,
}),
..Default::default()
Expand Down
8 changes: 4 additions & 4 deletions crates/wallet/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,10 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
}

/// 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
Expand Down
Loading
Loading