feat(tx_graph)!: change TxGraph::calculate_fee to return Result<u64,CalculateFeeError>

added
- tx_graph::CalculateFeeError enum

BREAKING CHANGES:

changed
- TxGraph::calculate_fee function to return Result<u64,CalculateFeeError> instead of Option<i64>
This commit is contained in:
Steve Myers 2023-08-01 12:42:37 -05:00
parent b4c31cd5ba
commit d443fe7f66
No known key found for this signature in database
GPG Key ID: 8105A46B22C2D051
6 changed files with 72 additions and 59 deletions

View File

@ -9,10 +9,6 @@
// You may not use this file except in accordance with one or both of these
// licenses.
//! Errors
//!
//! This module defines the errors that can be thrown by [`crate`] functions.
use crate::bitcoin::Network;
use crate::{descriptor, wallet};
use alloc::{string::String, vec::Vec};
@ -93,17 +89,7 @@ pub enum Error {
Psbt(bitcoin::psbt::Error),
}
/// Errors returned by `Wallet::calculate_fee`.
#[derive(Debug)]
pub enum CalculateFeeError {
/// Missing `TxOut` for one of the inputs of the tx
MissingTxOut,
/// When the transaction is invalid according to the graph it has a negative fee
NegativeFee(i64),
}
/// Errors returned by miniscript when updating inconsistent PSBTs
#[allow(missing_docs)] // TODO add docs
#[derive(Debug, Clone)]
pub enum MiniscriptPsbtError {
Conversion(miniscript::descriptor::ConversionError),

View File

@ -29,7 +29,7 @@ extern crate bip39;
#[allow(unused_imports)]
#[macro_use]
pub mod error;
pub(crate) mod error;
pub mod descriptor;
pub mod keys;
pub mod psbt;

View File

@ -40,6 +40,7 @@ use core::fmt;
use core::ops::Deref;
use miniscript::psbt::{PsbtExt, PsbtInputExt, PsbtInputSatisfier};
use bdk_chain::tx_graph::CalculateFeeError;
#[allow(unused_imports)]
use log::{debug, error, info, trace};
@ -66,7 +67,7 @@ use crate::descriptor::{
calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorMeta,
ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils,
};
use crate::error::{CalculateFeeError, Error, MiniscriptPsbtError};
use crate::error::{Error, MiniscriptPsbtError};
use crate::psbt::PsbtUtils;
use crate::signer::SignerError;
use crate::types::*;
@ -434,11 +435,7 @@ impl<D> Wallet<D> {
///
/// Note `tx` does not have to be in the graph for this to work.
pub fn calculate_fee(&self, tx: &Transaction) -> Result<u64, CalculateFeeError> {
match self.indexed_graph.graph().calculate_fee(tx) {
None => Err(CalculateFeeError::MissingTxOut),
Some(fee) if fee < 0 => Err(CalculateFeeError::NegativeFee(fee)),
Some(fee) => Ok(u64::try_from(fee).unwrap()),
}
self.indexed_graph.graph().calculate_fee(tx)
}
/// Calculate the `FeeRate` for a given transaction.
@ -1072,13 +1069,12 @@ impl<D> Wallet<D> {
return Err(Error::IrreplaceableTransaction);
}
let fee = graph.calculate_fee(&tx).ok_or(Error::FeeRateUnavailable)?;
if fee < 0 {
// It's available but it's wrong so let's say it's unavailable
return Err(Error::FeeRateUnavailable)?;
}
let fee = fee as u64;
let feerate = FeeRate::from_wu(fee, tx.weight());
let fee = self
.calculate_fee(&tx)
.map_err(|_| Error::FeeRateUnavailable)?;
let fee_rate = self
.calculate_fee_rate(&tx)
.map_err(|_| Error::FeeRateUnavailable)?;
// remove the inputs from the tx and process them
let original_txin = tx.input.drain(..).collect::<Vec<_>>();
@ -1162,7 +1158,7 @@ impl<D> Wallet<D> {
utxos: original_utxos,
bumping_fee: Some(tx_builder::PreviousFee {
absolute: fee,
rate: feerate.as_sat_per_vb(),
rate: fee_rate.as_sat_per_vb(),
}),
..Default::default()
};

View File

@ -6,6 +6,7 @@ use bdk::wallet::coin_selection::LargestFirstCoinSelection;
use bdk::wallet::AddressIndex::*;
use bdk::wallet::{AddressIndex, AddressInfo, Balance, Wallet};
use bdk::{Error, FeeRate, KeychainKind};
use bdk_chain::tx_graph::CalculateFeeError;
use bdk_chain::COINBASE_MATURITY;
use bdk_chain::{BlockId, ConfirmationTime};
use bitcoin::hashes::Hash;
@ -104,7 +105,7 @@ fn test_get_funded_wallet_sent_and_received() {
fn test_get_funded_wallet_tx_fees() {
let (wallet, _) = get_funded_wallet(get_test_wpkh());
assert_eq!(wallet.get_balance().confirmed, 50000);
let mut tx_fee_amounts: Vec<(Txid, Result<u64, bdk::error::CalculateFeeError>)> = wallet
let mut tx_fee_amounts: Vec<(Txid, Result<u64, CalculateFeeError>)> = wallet
.transactions()
.map(|ct| {
let fee = wallet.calculate_fee(ct.node.tx);
@ -116,7 +117,7 @@ fn test_get_funded_wallet_tx_fees() {
assert_eq!(tx_fee_amounts.len(), 2);
assert_matches!(
tx_fee_amounts.get(1),
Some((_, Err(bdk::error::CalculateFeeError::MissingTxOut)))
Some((_, Err(CalculateFeeError::MissingTxOut(_))))
);
assert_matches!(tx_fee_amounts.get(0), Some((_, Ok(1000))))
}
@ -125,7 +126,7 @@ fn test_get_funded_wallet_tx_fees() {
fn test_get_funded_wallet_tx_fee_rate() {
let (wallet, _) = get_funded_wallet(get_test_wpkh());
assert_eq!(wallet.get_balance().confirmed, 50000);
let mut tx_fee_rates: Vec<(Txid, Result<FeeRate, bdk::error::CalculateFeeError>)> = wallet
let mut tx_fee_rates: Vec<(Txid, Result<FeeRate, CalculateFeeError>)> = wallet
.transactions()
.map(|ct| {
let fee_rate = wallet.calculate_fee_rate(ct.node.tx);
@ -137,7 +138,7 @@ fn test_get_funded_wallet_tx_fee_rate() {
assert_eq!(tx_fee_rates.len(), 2);
assert_matches!(
tx_fee_rates.get(1),
Some((_, Err(bdk::error::CalculateFeeError::MissingTxOut)))
Some((_, Err(CalculateFeeError::MissingTxOut(_))))
);
assert_matches!(tx_fee_rates.get(0), Some((_, Ok(_))))
}

View File

@ -135,6 +135,15 @@ pub struct CanonicalTx<'a, T, A> {
pub tx_node: TxNode<'a, T, A>,
}
/// Errors returned by `TxGraph::calculate_fee`.
#[derive(Debug, PartialEq, Eq)]
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),
}
impl<A> TxGraph<A> {
/// Iterate over all tx outputs known by [`TxGraph`].
///
@ -236,25 +245,33 @@ impl<A> TxGraph<A> {
}
/// Calculates the fee of a given transaction. Returns 0 if `tx` is a coinbase transaction.
/// Returns `Some(_)` if we have all the `TxOut`s being spent by `tx` in the graph (either as
/// the full transactions or individual txouts). If the returned value is negative, then the
/// transaction is invalid according to the graph.
///
/// Returns `None` if we're missing an input for the tx in the graph.
/// Returns `OK(_)` if we have all the `TxOut`s being spent by `tx` in the graph (either as
/// the full transactions or individual txouts).
///
/// Note `tx` does not have to be in the graph for this to work.
pub fn calculate_fee(&self, tx: &Transaction) -> Option<i64> {
pub fn calculate_fee(&self, tx: &Transaction) -> Result<u64, CalculateFeeError> {
if tx.is_coin_base() {
return Some(0);
return Ok(0);
}
let inputs_sum = tx
.input
.iter()
.map(|txin| {
self.get_txout(txin.previous_output)
.map(|txout| txout.value as i64)
})
.sum::<Option<i64>>()?;
let inputs_sum = tx.input.iter().fold(
(0_u64, Vec::new()),
|(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;
(sum, missing_outpoints)
}
},
);
let inputs_sum = if inputs_sum.1.is_empty() {
Ok(inputs_sum.0 as i64)
} else {
Err(CalculateFeeError::MissingTxOut(inputs_sum.1))
}?;
let outputs_sum = tx
.output
@ -262,7 +279,12 @@ impl<A> TxGraph<A> {
.map(|txout| txout.value as i64)
.sum::<i64>();
Some(inputs_sum - outputs_sum)
let fee = inputs_sum - outputs_sum;
if fee < 0 {
Err(CalculateFeeError::NegativeFee(fee))
} else {
Ok(fee as u64)
}
}
/// The transactions spending from this output.

View File

@ -1,5 +1,6 @@
#[macro_use]
mod common;
use bdk_chain::tx_graph::CalculateFeeError;
use bdk_chain::{
collections::*,
local_chain::LocalChain,
@ -453,22 +454,29 @@ fn test_calculate_fee() {
}],
};
assert_eq!(graph.calculate_fee(&tx), Some(100));
assert_eq!(graph.calculate_fee(&tx), Ok(100));
tx.input.remove(2);
// fee would be negative
assert_eq!(graph.calculate_fee(&tx), Some(-200));
// fee would be negative, should return CalculateFeeError::NegativeFee
assert_eq!(
graph.calculate_fee(&tx),
Err(CalculateFeeError::NegativeFee(-200))
);
// If we have an unknown outpoint, fee should return None.
tx.input.push(TxIn {
previous_output: OutPoint {
// If we have an unknown outpoint, fee should return CalculateFeeError::MissingTxOut.
let outpoint = OutPoint {
txid: h!("unknown_txid"),
vout: 0,
},
};
tx.input.push(TxIn {
previous_output: outpoint,
..Default::default()
});
assert_eq!(graph.calculate_fee(&tx), None);
assert_eq!(
graph.calculate_fee(&tx),
Err(CalculateFeeError::MissingTxOut(vec!(outpoint)))
);
}
#[test]
@ -485,7 +493,7 @@ fn test_calculate_fee_on_coinbase() {
let graph = TxGraph::<()>::default();
assert_eq!(graph.calculate_fee(&tx), Some(0));
assert_eq!(graph.calculate_fee(&tx), Ok(0));
}
#[test]