feat(wallet)!: remove TransactionDetails from bdk::Wallet API

Added - spk_txout_index::TotalSentReceived struct for tx total sent and received amounts - Wallet::total_send_received and SpkTxOutIndex::total_sent_received functions - Wallet::calculate_fee and Wallet::calculate_fee_rate functions - Wallet::error::CalculateFeeError BREAKING CHANGES: Removed - TransactionDetails struct Changed - Wallet::get_tx now returns CanonicalTx instead of TransactionDetails - TxBuilder::finish now returns only a PartiallySignedTransaction
bitcoindevkit · Aug 1, 2023 · 1667496 · 1667496
1 parent 8f38e96
commit 1667496
Show file tree

Hide file tree

Showing 14 changed files with 462 additions and 341 deletions.
diff --git a/crates/bdk/src/error.rs b/crates/bdk/src/error.rs
@@ -9,6 +9,10 @@
 // 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};
@@ -89,7 +93,17 @@ 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),

diff --git a/crates/bdk/src/keys/mod.rs b/crates/bdk/src/keys/mod.rs
@@ -754,7 +754,7 @@ fn expand_multi_keys<Pk: IntoDescriptorKey<Ctx>, Ctx: ScriptContext>(
     let (key_map, valid_networks) = key_maps_networks.into_iter().fold(
         (KeyMap::default(), any_network()),
         |(mut keys_acc, net_acc), (key, net)| {
-            keys_acc.extend(key.into_iter());
+            keys_acc.extend(key);
             let net_acc = merge_networks(&net_acc, &net);
 
             (keys_acc, net_acc)

diff --git a/crates/bdk/src/lib.rs b/crates/bdk/src/lib.rs
@@ -29,7 +29,7 @@ extern crate bip39;
 
 #[allow(unused_imports)]
 #[macro_use]
-pub(crate) mod error;
+pub mod error;
 pub mod descriptor;
 pub mod keys;
 pub mod psbt;

diff --git a/crates/bdk/src/types.rs b/crates/bdk/src/types.rs
@@ -14,8 +14,8 @@ use core::convert::AsRef;
 use core::ops::Sub;
 
 use bdk_chain::ConfirmationTime;
-use bitcoin::blockdata::transaction::{OutPoint, Transaction, TxOut};
-use bitcoin::{hash_types::Txid, util::psbt};
+use bitcoin::blockdata::transaction::{OutPoint, TxOut};
+use bitcoin::util::psbt;
 
 use serde::{Deserialize, Serialize};
 
@@ -234,40 +234,6 @@ impl Utxo {
     }
 }
 
-/// A wallet transaction
-#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
-pub struct TransactionDetails {
-    /// Optional transaction
-    pub transaction: Option<Transaction>,
-    /// Transaction id
-    pub txid: Txid,
-    /// Received value (sats)
-    /// Sum of owned outputs of this transaction.
-    pub received: u64,
-    /// Sent value (sats)
-    /// Sum of owned inputs of this transaction.
-    pub sent: u64,
-    /// Fee value in sats if it was available.
-    pub fee: Option<u64>,
-    /// If the transaction is confirmed, contains height and Unix timestamp of the block containing the
-    /// transaction, unconfirmed transaction contains `None`.
-    pub confirmation_time: ConfirmationTime,
-}
-
-impl PartialOrd for TransactionDetails {
-    fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl Ord for TransactionDetails {
-    fn cmp(&self, other: &Self) -> core::cmp::Ordering {
-        self.confirmation_time
-            .cmp(&other.confirmation_time)
-            .then_with(|| self.txid.cmp(&other.txid))
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

diff --git a/crates/bdk/src/wallet/coin_selection.rs b/crates/bdk/src/wallet/coin_selection.rs
@@ -81,7 +81,7 @@
 //! // create wallet, sync, ...
 //!
 //! let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
-//! let (psbt, details) = {
+//! let psbt = {
 //!     let mut builder = wallet.build_tx().coin_selection(AlwaysSpendEverything);
 //!     builder.add_recipient(to_address.script_pubkey(), 50_000);
 //!     builder.finish()?

diff --git a/crates/bdk/src/wallet/mod.rs b/crates/bdk/src/wallet/mod.rs
@@ -26,7 +26,7 @@ use bdk_chain::{
     local_chain::{self, LocalChain, UpdateNotConnectedError},
     tx_graph::{CanonicalTx, TxGraph},
     Append, BlockId, ChainPosition, ConfirmationTime, ConfirmationTimeAnchor, FullTxOut,
-    IndexedTxGraph, Persist, PersistBackend,
+    IndexedTxGraph, Persist, PersistBackend, TotalSentReceived,
 };
 use bitcoin::consensus::encode::serialize;
 use bitcoin::secp256k1::Secp256k1;
@@ -65,7 +65,7 @@ use crate::descriptor::{
     calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorMeta,
     ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils,
 };
-use crate::error::{Error, MiniscriptPsbtError};
+use crate::error::{CalculateFeeError, Error, MiniscriptPsbtError};
 use crate::psbt::PsbtUtils;
 use crate::signer::SignerError;
 use crate::types::*;
@@ -427,27 +427,49 @@ impl<D> Wallet<D> {
             .next()
     }
 
-    /// Return a single transactions made and received by the wallet
+    /// Calculates the fee of a given transaction. Returns 0 if `tx` is a coinbase transaction.
     ///
-    /// Optionally fill the [`TransactionDetails::transaction`] field with the raw transaction if
-    /// `include_raw` is `true`.
-    pub fn get_tx(&self, txid: Txid, include_raw: bool) -> Option<TransactionDetails> {
+    /// 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()),
+        }
+    }
+
+    /// Calculate the `FeeRate` for a given transaction.
+    ///
+    /// Note `tx` does not have to be in the graph for this to work.
+    pub fn calculate_fee_rate(&self, tx: &Transaction) -> Result<FeeRate, CalculateFeeError> {
+        self.calculate_fee(tx).map(|fee| {
+            let weight = tx.weight();
+            FeeRate::from_wu(fee, weight)
+        })
+    }
+
+    /// Return `TotalSentReceived` for a `Transaction` in relation to the `Wallet` and it's
+    /// descriptors.
+    pub fn total_sent_received(&self, tx: &Transaction) -> TotalSentReceived {
+        self.indexed_graph.index.total_sent_received(tx)
+    }
+
+    /// Return a single `CanonicalTx` made and received by the wallet or `None` if it doesn't
+    /// exist in the wallet
+    pub fn get_tx(
+        &self,
+        txid: Txid,
+    ) -> Option<CanonicalTx<'_, Transaction, ConfirmationTimeAnchor>> {
         let graph = self.indexed_graph.graph();
 
-        let canonical_tx = CanonicalTx {
+        Some(CanonicalTx {
             observed_as: graph.get_chain_position(
                 &self.chain,
                 self.chain.tip().unwrap_or_default(),
                 txid,
             )?,
             node: graph.get_tx_node(txid)?,
-        };
-
-        Some(new_tx_details(
-            &self.indexed_graph,
-            canonical_tx,
-            include_raw,
-        ))
+        })
     }
 
     /// Add a new checkpoint to the wallet's internal view of the chain.
@@ -599,7 +621,7 @@ impl<D> Wallet<D> {
     /// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
     /// # let mut wallet = doctest_wallet!();
     /// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
-    /// let (psbt, details) = {
+    /// let psbt = {
     ///    let mut builder =  wallet.build_tx();
     ///    builder
     ///        .add_recipient(to_address.script_pubkey(), 50_000);
@@ -624,7 +646,7 @@ impl<D> Wallet<D> {
         &mut self,
         coin_selection: Cs,
         params: TxParams,
-    ) -> Result<(psbt::PartiallySignedTransaction, TransactionDetails), Error>
+    ) -> Result<psbt::PartiallySignedTransaction, Error>
     where
         D: PersistBackend<ChangeSet>,
     {
@@ -969,20 +991,8 @@ impl<D> Wallet<D> {
         // sort input/outputs according to the chosen algorithm
         params.ordering.sort_tx(&mut tx);
 
-        let txid = tx.txid();
-        let sent = coin_selection.local_selected_amount();
         let psbt = self.complete_transaction(tx, coin_selection.selected, params)?;
-
-        let transaction_details = TransactionDetails {
-            transaction: None,
-            txid,
-            confirmation_time: ConfirmationTime::Unconfirmed { last_seen: 0 },
-            received,
-            sent,
-            fee: Some(fee_amount),
-        };
-
-        Ok((psbt, transaction_details))
+        Ok(psbt)
     }
 
     /// Bump the fee of a transaction previously created with this wallet.
@@ -1001,7 +1011,7 @@ impl<D> Wallet<D> {
     /// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
     /// # let mut wallet = doctest_wallet!();
     /// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
-    /// let (mut psbt, _) = {
+    /// let mut psbt = {
     ///     let mut builder = wallet.build_tx();
     ///     builder
     ///         .add_recipient(to_address.script_pubkey(), 50_000)
@@ -1011,7 +1021,7 @@ impl<D> Wallet<D> {
     /// let _ = wallet.sign(&mut psbt, SignOptions::default())?;
     /// let tx = psbt.extract_tx();
     /// // broadcast tx but it's taking too long to confirm so we want to bump the fee
-    /// let (mut psbt, _) =  {
+    /// let mut psbt =  {
     ///     let mut builder = wallet.build_fee_bump(tx.txid())?;
     ///     builder
     ///         .fee_rate(FeeRate::from_sat_per_vb(5.0));
@@ -1171,7 +1181,7 @@ impl<D> Wallet<D> {
     /// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
     /// # let mut wallet = doctest_wallet!();
     /// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
-    /// let (mut psbt, _) = {
+    /// let mut psbt = {
     ///     let mut builder = wallet.build_tx();
     ///     builder.add_recipient(to_address.script_pubkey(), 50_000);
     ///     builder.finish()?
@@ -1721,7 +1731,7 @@ impl<D> Wallet<D> {
         Ok(changed)
     }
 
-    /// Commits all curently [`staged`] changed to the persistence backend returning and error when
+    /// Commits all currently [`staged`] changed to the persistence backend returning and error when
     /// this fails.
     ///
     /// This returns whether the `update` resulted in any changes.
@@ -1812,61 +1822,6 @@ fn new_local_utxo(
     }
 }
 
-fn new_tx_details(
-    indexed_graph: &IndexedTxGraph<ConfirmationTimeAnchor, KeychainTxOutIndex<KeychainKind>>,
-    canonical_tx: CanonicalTx<'_, Transaction, ConfirmationTimeAnchor>,
-    include_raw: bool,
-) -> TransactionDetails {
-    let graph = indexed_graph.graph();
-    let index = &indexed_graph.index;
-    let tx = canonical_tx.node.tx;
-
-    let received = tx
-        .output
-        .iter()
-        .map(|txout| {
-            if index.index_of_spk(&txout.script_pubkey).is_some() {
-                txout.value
-            } else {
-                0
-            }
-        })
-        .sum();
-
-    let sent = tx
-        .input
-        .iter()
-        .map(|txin| {
-            if let Some((_, txout)) = index.txout(txin.previous_output) {
-                txout.value
-            } else {
-                0
-            }
-        })
-        .sum();
-
-    let inputs = tx
-        .input
-        .iter()
-        .map(|txin| {
-            graph
-                .get_txout(txin.previous_output)
-                .map(|txout| txout.value)
-        })
-        .sum::<Option<u64>>();
-    let outputs = tx.output.iter().map(|txout| txout.value).sum();
-    let fee = inputs.map(|inputs| inputs.saturating_sub(outputs));
-
-    TransactionDetails {
-        transaction: if include_raw { Some(tx.clone()) } else { None },
-        txid: canonical_tx.node.txid,
-        received,
-        sent,
-        fee,
-        confirmation_time: canonical_tx.observed_as.cloned().into(),
-    }
-}
-
 #[macro_export]
 #[doc(hidden)]
 /// Macro for getting a wallet for use in a doctest

diff --git a/crates/bdk/src/wallet/tx_builder.rs b/crates/bdk/src/wallet/tx_builder.rs
@@ -32,7 +32,7 @@
 //!     .do_not_spend_change()
 //!     // Turn on RBF signaling
 //!     .enable_rbf();
-//! let (psbt, tx_details) = tx_builder.finish()?;
+//! let psbt = tx_builder.finish()?;
 //! # Ok::<(), bdk::Error>(())
 //! ```
 
@@ -48,10 +48,7 @@ use bitcoin::{LockTime, OutPoint, Script, Sequence, Transaction};
 
 use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
 use super::ChangeSet;
-use crate::{
-    types::{FeeRate, KeychainKind, LocalUtxo, WeightedUtxo},
-    TransactionDetails,
-};
+use crate::types::{FeeRate, KeychainKind, LocalUtxo, WeightedUtxo};
 use crate::{Error, Utxo, Wallet};
 /// Context in which the [`TxBuilder`] is valid
 pub trait TxBuilderContext: core::fmt::Debug + Default + Clone {}
@@ -85,7 +82,7 @@ impl TxBuilderContext for BumpFee {}
 /// # let addr1 = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap();
 /// # let addr2 = addr1.clone();
 /// // chaining
-/// let (psbt1, details) = {
+/// let psbt1 = {
 ///     let mut builder = wallet.build_tx();
 ///     builder
 ///         .ordering(TxOrdering::Untouched)
@@ -95,7 +92,7 @@ impl TxBuilderContext for BumpFee {}
 /// };
 ///
 /// // non-chaining
-/// let (psbt2, details) = {
+/// let psbt2 = {
 ///     let mut builder = wallet.build_tx();
 ///     builder.ordering(TxOrdering::Untouched);
 ///     for addr in &[addr1, addr2] {
@@ -527,7 +524,7 @@ impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D,
     /// Returns the [`BIP174`] "PSBT" and summary details about the transaction.
     ///
     /// [`BIP174`]: https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki
-    pub fn finish(self) -> Result<(Psbt, TransactionDetails), Error>
+    pub fn finish(self) -> Result<Psbt, Error>
     where
         D: PersistBackend<ChangeSet>,
     {
@@ -637,7 +634,7 @@ impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
     ///     .drain_to(to_address.script_pubkey())
     ///     .fee_rate(FeeRate::from_sat_per_vb(5.0))
     ///     .enable_rbf();
-    /// let (psbt, tx_details) = tx_builder.finish()?;
+    /// let psbt = tx_builder.finish()?;
     /// # Ok::<(), bdk::Error>(())
     /// ```
     ///