feat(tx_graph)!: change TxGraph::calculate_fee to return Result<u64,C…

…alculateFeeError>

added
- tx_graph::CalculateFeeError enum

BREAKING CHANGES:

changed
- TxGraph::calculate_fee function to return Result<u64,CalculateFeeError> instead of Option<i64>

crates/bdk/src/error.rs

@@ -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::util::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),

crates/bdk/src/lib.rs

@@ -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;

crates/bdk/src/wallet/mod.rs

@@ -41,6 +41,7 @@ use miniscript::psbt::{PsbtExt, PsbtInputExt, PsbtInputSatisfier};
 
 #[allow(unused_imports)]
 use log::{debug, error, info, trace};
+use bdk_chain::tx_graph::CalculateFeeError;
 
 pub mod coin_selection;
 pub mod export;
@@ -65,7 +66,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::*;
@@ -431,11 +432,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.
@@ -1065,13 +1062,8 @@ 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<_>>();
@@ -1154,7 +1146,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()
         };

crates/bdk/tests/wallet.rs

@@ -17,6 +17,7 @@ use bitcoin::{
 };
 use bitcoin::{BlockHash, Txid};
 use core::str::FromStr;
+use bdk_chain::tx_graph::CalculateFeeError;
 
 mod common;
 use common::*;
@@ -100,7 +101,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);
@@ -112,7 +113,7 @@ fn test_get_funded_wallet_tx_fees() {
     assert_eq!(tx_fee_amounts.len(), 2);
     assert_matches!(
         tx_fee_amounts.get(0),
-        Some((_, Err(bdk::error::CalculateFeeError::MissingTxOut)))
+        Some((_, Err(CalculateFeeError::MissingTxOut(_))))
     );
     assert_matches!(tx_fee_amounts.get(1), Some((_, Ok(1000))))
 }
@@ -121,7 +122,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);
@@ -133,7 +134,7 @@ fn test_get_funded_wallet_tx_fee_rate() {
     assert_eq!(tx_fee_rates.len(), 2);
     assert_matches!(
         tx_fee_rates.get(0),
-        Some((_, Err(bdk::error::CalculateFeeError::MissingTxOut)))
+        Some((_, Err(CalculateFeeError::MissingTxOut(_))))
     );
     assert_matches!(tx_fee_rates.get(1), Some((_, Ok(_))))
 }

crates/chain/Cargo.toml

@@ -24,6 +24,7 @@ miniscript = { version = "9.0.0", optional = true, default-features = false }
 
 [dev-dependencies]
 rand = "0.8"
+assert_matches = "1.5.0"
 
 [features]
 default = ["std"]

crates/chain/src/tx_graph.rs

@@ -140,6 +140,15 @@ pub struct CanonicalTx<'a, T, A> {
     pub node: TxNode<'a, T, A>,
 }
 
+/// Errors returned by `TxGraph::calculate_fee`.
+#[derive(Debug)]
+pub enum CalculateFeeError {
+    /// Missing `TxOut` for one of the inputs of the tx
+    MissingTxOut(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`].
     ///
@@ -241,33 +250,36 @@ 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)
+                    .ok_or(CalculateFeeError::MissingTxOut(txin.previous_output))
             })
-            .sum::<Option<i64>>()?;
+            .sum::<Result<i64, CalculateFeeError>>()?;
 
         let outputs_sum = tx
             .output
             .iter()
             .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(u64::try_from(fee).unwrap())
+        }
     }
 
     /// The transactions spending from this output.

crates/chain/tests/test_tx_graph.rs

@@ -11,6 +11,8 @@ use bitcoin::{
 };
 use core::iter;
 use std::vec;
+use bdk_chain::tx_graph::CalculateFeeError;
+use assert_matches::assert_matches;
 
 #[test]
 fn insert_txouts() {
@@ -442,12 +444,12 @@ fn test_calculate_fee() {
         }],
     };
 
-    assert_eq!(graph.calculate_fee(&tx), Some(100));
+    assert_matches!(graph.calculate_fee(&tx), Ok(100));
 
     tx.input.remove(2);
 
     // fee would be negative
-    assert_eq!(graph.calculate_fee(&tx), Some(-200));
+    assert_matches!(graph.calculate_fee(&tx), Err(CalculateFeeError::NegativeFee(-200)));
 
     // If we have an unknown outpoint, fee should return None.
     tx.input.push(TxIn {
@@ -457,7 +459,7 @@ fn test_calculate_fee() {
         },
         ..Default::default()
     });
-    assert_eq!(graph.calculate_fee(&tx), None);
+    assert_matches!(graph.calculate_fee(&tx), Err(CalculateFeeError::MissingTxOut(_)));
 }
 
 #[test]
@@ -474,7 +476,7 @@ fn test_calculate_fee_on_coinbase() {
 
     let graph = TxGraph::<()>::default();
 
-    assert_eq!(graph.calculate_fee(&tx), Some(0));
+    assert_matches!(graph.calculate_fee(&tx), Ok(0));
 }
 
 #[test]