spend: avoid direct access to our Bitcoin backend

We introduce a trait to get the wallet transaction correponding to the
transaction input in order to encapsulate the spend module.

src/commands/mod.rs

@@ -10,7 +10,7 @@ use crate::{
     descriptors,
     spend::{
         check_output_value, create_spend, sanity_check_psbt, unsigned_tx_max_vbytes, AddrInfo,
-        CandidateCoin, CreateSpendRes, SpendCreationError, SpendOutputAddress,
+        CandidateCoin, CreateSpendRes, SpendCreationError, SpendOutputAddress, TxGetter,
     },
     DaemonControl, VERSION,
 };
@@ -25,7 +25,7 @@ use utils::{
 use std::{
     collections::{hash_map, BTreeMap, HashMap, HashSet},
     convert::TryInto,
-    fmt,
+    fmt, sync,
 };
 
 use miniscript::{
@@ -154,6 +154,32 @@ impl fmt::Display for RbfErrorInfo {
     }
 }
 
+/// A wallet transaction getter which fetches the transaction from our Bitcoin backend with a cache
+/// to avoid needless redundant calls. Note the cache holds an Option<> so we also avoid redundant
+/// calls when the txid isn't known by our Bitcoin backend.
+struct BitcoindTxGetter<'a> {
+    bitcoind: &'a sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
+    cache: HashMap<bitcoin::Txid, Option<bitcoin::Transaction>>,
+}
+
+impl<'a> BitcoindTxGetter<'a> {
+    pub fn new(bitcoind: &'a sync::Arc<sync::Mutex<dyn BitcoinInterface>>) -> Self {
+        Self {
+            bitcoind,
+            cache: HashMap::new(),
+        }
+    }
+}
+
+impl<'a> TxGetter for BitcoindTxGetter<'a> {
+    fn get_tx(&mut self, txid: &bitcoin::Txid) -> Option<bitcoin::Transaction> {
+        if let hash_map::Entry::Vacant(entry) = self.cache.entry(*txid) {
+            entry.insert(self.bitcoind.wallet_transaction(txid).map(|wtx| wtx.0));
+        }
+        self.cache.get(txid).cloned().flatten()
+    }
+}
+
 impl DaemonControl {
     // Get the derived descriptor for this coin
     fn derived_desc(&self, coin: &Coin) -> descriptors::DerivedSinglePathLianaDesc {
@@ -393,6 +419,7 @@ impl DaemonControl {
             return Err(CommandError::InvalidFeerate(feerate_vb));
         }
         let mut db_conn = self.db.connection();
+        let mut tx_getter = BitcoindTxGetter::new(&self.bitcoin);
 
         // Check the destination addresses are valid for the network and
         // sanity check each output's value.
@@ -458,7 +485,7 @@ impl DaemonControl {
         let CreateSpendRes { psbt, has_change } = create_spend(
             &self.config.main_descriptor,
             &self.secp,
-            &self.bitcoin,
+            &mut tx_getter,
             &destinations_checked,
             &candidate_coins,
             feerate_vb,
@@ -605,6 +632,7 @@ impl DaemonControl {
         feerate_vb: Option<u64>,
     ) -> Result<CreateSpendResult, CommandError> {
         let mut db_conn = self.db.connection();
+        let mut tx_getter = BitcoindTxGetter::new(&self.bitcoin);
 
         if is_cancel && feerate_vb.is_some() {
             return Err(CommandError::RbfError(RbfErrorInfo::SuperfluousFeerate));
@@ -782,7 +810,7 @@ impl DaemonControl {
             } = match create_spend(
                 &self.config.main_descriptor,
                 &self.secp,
-                &self.bitcoin,
+                &mut tx_getter,
                 &destinations,
                 &candidate_coins,
                 feerate_vb,

src/spend.rs

@@ -1,13 +1,6 @@
-use crate::{bitcoin::BitcoinInterface, database::Coin, descriptors};
+use crate::{database::Coin, descriptors};
 
-use std::{
-    collections::{
-        hash_map::{self, HashMap},
-        BTreeMap,
-    },
-    convert::TryInto,
-    fmt, sync,
-};
+use std::{collections::BTreeMap, convert::TryInto, fmt};
 
 pub use bdk_coin_select::InsufficientFunds;
 use bdk_coin_select::{
@@ -401,6 +394,12 @@ pub struct SpendOutputAddress {
     pub info: Option<AddrInfo>,
 }
 
+/// A trait for getting a wallet transaction by its txid.
+pub trait TxGetter {
+    /// Get a wallet transaction. Allows for a cache by making the access mutable.
+    fn get_tx(&mut self, txid: &bitcoin::Txid) -> Option<bitcoin::Transaction>;
+}
+
 pub struct CreateSpendRes {
     /// The created PSBT.
     pub psbt: Psbt,
@@ -411,7 +410,7 @@ pub struct CreateSpendRes {
 pub fn create_spend(
     main_descriptor: &descriptors::LianaDescriptor,
     secp: &secp256k1::Secp256k1<secp256k1::VerifyOnly>,
-    bitcoin: &sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
+    tx_getter: &mut impl TxGetter,
     destinations: &[(SpendOutputAddress, bitcoin::Amount)],
     candidate_coins: &[CandidateCoin],
     feerate_vb: u64,
@@ -544,16 +543,7 @@ pub fn create_spend(
 
     // Iterate through selected coins and add necessary information to the PSBT inputs.
     let mut psbt_ins = Vec::with_capacity(selected_coins.len());
-    let mut spent_txs = HashMap::with_capacity(selected_coins.len());
     for coin in &selected_coins {
-        // Fetch the transaction that created it if necessary
-        if let hash_map::Entry::Vacant(e) = spent_txs.entry(coin.outpoint) {
-            let tx = bitcoin
-                .wallet_transaction(&coin.outpoint.txid)
-                .ok_or(SpendCreationError::FetchingTransaction(coin.outpoint))?;
-            e.insert(tx.0);
-        }
-
         tx.input.push(bitcoin::TxIn {
             previous_output: coin.outpoint,
             sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME,
@@ -568,7 +558,7 @@ pub fn create_spend(
             value: coin.amount.to_sat(),
             script_pubkey: coin_desc.script_pubkey(),
         });
-        let non_witness_utxo = spent_txs.get(&coin.outpoint).cloned();
+        let non_witness_utxo = tx_getter.get_tx(&coin.outpoint.txid);
         let bip32_derivation = coin_desc.bip32_derivations();
         psbt_ins.push(PsbtIn {
             witness_script,