refactor(cpfp): remove coin selection and calculate fee from package rate

Author: aagbotemi

examples/common.rs

@@ -9,8 +9,7 @@ use bdk_chain::{
 use bdk_coin_select::DrainWeights;
 use bdk_testenv::{bitcoincore_rpc::RpcApi, TestEnv};
 use bdk_tx::{
-    CPFPParams, CPFPSet, CanonicalUnspents, Input, InputCandidates, RbfParams, TxStatus,
-    TxWithStatus,
+    CanonicalUnspents, Input, InputCandidates, RbfParams, Selection, TxStatus, TxWithStatus,
 };
 use bitcoin::{absolute, Address, BlockHash, FeeRate, OutPoint, Transaction, Txid};
 use miniscript::{
@@ -89,8 +88,6 @@ impl Wallet {
         Ok((tip_height, tip_time))
     }
 
-    // TODO: Maybe create an `AssetsBuilder` or `AssetsExt` that makes it easier to add
-    // assets from descriptors, etc.
     pub fn assets(&self) -> Assets {
         let index = &self.graph.index;
         let tip = self.chain.tip().block_id();
@@ -207,50 +204,28 @@ impl Wallet {
         ))
     }
 
-    pub fn cpfp_candidates(
-        &self,
+    pub fn create_cpfp_transaction(
+        &mut self,
         parent_txids: impl IntoIterator<Item = Txid>,
-        tip_height: absolute::Height,
-        target_feerate: FeeRate,
-    ) -> anyhow::Result<(InputCandidates, CPFPParams)> {
+        target_package_feerate: FeeRate,
+    ) -> anyhow::Result<Selection> {
         let assets = self.assets();
         let canon_utxos = CanonicalUnspents::new(self.canonical_txs());
-        let index = &self.graph.index;
-        let parent_txids: Vec<Txid> = parent_txids.into_iter().collect();
 
-        let cpfpset = CPFPSet::new(parent_txids.clone(), self.graph.graph(), tip_height)?;
-        let cpfp_params = cpfpset.selector_cpfp_params(target_feerate);
-
-        let must_select = cpfpset
-            .must_select_largest_input_of_each_parent(&canon_utxos)?
-            .into_iter()
-            .map(|op| {
-                let plan = self
-                    .plan_of_output(op, &assets)
-                    .ok_or_else(|| anyhow::anyhow!("failed to derive plan for outpoint {}", op))?;
-                canon_utxos.try_get_unspent(op, plan).ok_or_else(|| {
-                    anyhow::anyhow!("failed to get unspent input for outpoint {}", op)
-                })
-            })
-            .collect::<Result<Vec<Input>, _>>()?;
-
-        // Select other spendable outputs as optional candidates
-        let can_select = index
-            .outpoints()
+        let script_pubkey = self.next_address().unwrap().script_pubkey();
+        let cpfpset = canon_utxos.build_cpfp_set_from_txids(parent_txids, self.graph.graph())?;
+        let plans = cpfpset
+            .selected_outpoints
             .iter()
-            .filter_map(|(_, op)| {
-                if canon_utxos.is_unspent(*op) {
-                    self.plan_of_output(*op, &assets)
-                        .map(|plan| canon_utxos.try_get_unspent(*op, plan))
-                } else {
-                    None
-                }
-            })
-            .flatten();
+            .filter_map(|op| self.plan_of_output(*op, &assets));
 
-        let candidates = InputCandidates::new(must_select, can_select)
-            .filter(cpfpset.candidate_filter(&canon_utxos, tip_height));
+        let selection = cpfpset.create_cpfp_transaction(
+            &canon_utxos,
+            target_package_feerate,
+            &script_pubkey,
+            plans,
+        )?;
 
-        Ok((candidates, cpfp_params))
+        Ok(selection)
     }
 }

examples/cpfp.rs

@@ -5,7 +5,7 @@ use bdk_tx::{
     filter_unspendable_now, group_by_spk, selection_algorithm_lowest_fee_bnb, ChangePolicyType,
     Output, PsbtParams, SelectorParams, Signer,
 };
-use bitcoin::{key::Secp256k1, Amount, FeeRate, Sequence, Transaction};
+use bitcoin::{absolute::LockTime, key::Secp256k1, Amount, FeeRate, Sequence, Transaction};
 use miniscript::Descriptor;
 
 mod common;
@@ -40,7 +40,7 @@ fn main() -> anyhow::Result<()> {
     // Create two low-fee parent transactions
     let (tip_height, tip_time) = wallet.tip_info(env.rpc_client())?;
     let mut parent_txids = vec![];
-    for i in 0..2 {
+    for i in 0..3 {
         let low_fee_selection = wallet
             .all_candidates()
             .regroup(group_by_spk())
@@ -85,27 +85,14 @@ fn main() -> anyhow::Result<()> {
     }
 
     // Create CPFP transaction to boost both parents
-    let (cpfp_candidates, _) = wallet.cpfp_candidates(
+    let cpfp_selection = wallet.create_cpfp_transaction(
         parent_txids.clone(),
-        tip_height,
-        FeeRate::from_sat_per_vb_unchecked(10),
+        FeeRate::from_sat_per_vb_unchecked(10), // user specified
     )?;
-    let cpfp_selection = cpfp_candidates
-        .regroup(group_by_spk())
-        .filter(filter_unspendable_now(tip_height, tip_time))
-        .into_selection(
-            selection_algorithm_lowest_fee_bnb(FeeRate::from_sat_per_vb_unchecked(1), 100_000),
-            SelectorParams::new(
-                FeeRate::from_sat_per_vb_unchecked(30), // Higher fee rate for 10 sat/vB combined
-                vec![],                                 // No additional outputs, maximize change
-                internal.at_derivation_index(2)?,
-                ChangePolicyType::NoDustAndLeastWaste {
-                    longterm_feerate: FeeRate::from_sat_per_vb_unchecked(1),
-                },
-            ),
-        )?;
+
     let mut cpfp_psbt = cpfp_selection.create_psbt(PsbtParams {
         fallback_sequence: Sequence::MAX,
+        fallback_locktime: LockTime::ZERO,
         ..Default::default()
     })?;
     let cpfp_finalizer = cpfp_selection.into_finalizer();

examples/synopsis.rs

@@ -142,7 +142,6 @@ fn main() -> anyhow::Result<()> {
                     change_weight: wallet.change_weight(),
                     // This ensures that we satisfy mempool-replacement policy rules 4 and 6.
                     replace: Some(rbf_params),
-                    cpfp: None,
                 },
             )?;
 

src/canonical_unspents.rs

@@ -2,12 +2,13 @@ use alloc::sync::Arc;
 use alloc::vec::Vec;
 use core::fmt;
 
-use bitcoin::{psbt, OutPoint, Sequence, Transaction, TxOut, Txid};
-use miniscript::{bitcoin, plan::Plan};
-
 use crate::{
-    collections::HashMap, input::CoinbaseMismatch, FromPsbtInputError, Input, RbfSet, TxStatus,
+    collections::HashMap, input::CoinbaseMismatch, CPFPError, CPFPSet, FromPsbtInputError, Input,
+    RbfSet, TxStatus,
 };
+use bdk_chain::TxGraph;
+use bitcoin::{psbt, Amount, OutPoint, Sequence, Transaction, TxOut, Txid, Weight};
+use miniscript::{bitcoin, plan::Plan};
 
 /// Tx with confirmation status.
 pub type TxWithStatus<T> = (T, Option<TxStatus>);
@@ -126,6 +127,64 @@ impl CanonicalUnspents {
         )
     }
 
+    /// Constructs a '[CPFPSet]' from a set of parent transaction IDs.
+    pub fn build_cpfp_set_from_txids(
+        &self,
+        parent_txids: impl IntoIterator<Item = Txid>,
+        graph: &TxGraph,
+    ) -> Result<CPFPSet, CPFPError> {
+        let parent_txids: Vec<Txid> = parent_txids.into_iter().collect();
+
+        const MAX_ANCESTORS: usize = 25;
+        if parent_txids.len() > MAX_ANCESTORS {
+            return Err(CPFPError::ExcessUnconfirmedAncestor);
+        }
+
+        let (parent_weight, parent_fee, selected_outpoints) = parent_txids.iter().try_fold(
+            (Weight::ZERO, Amount::ZERO, Vec::new()),
+            |(mut weight, mut fee, mut outpoints), txid| -> Result<_, CPFPError> {
+                let tx = self.get_tx(txid).ok_or(CPFPError::MissingParent(*txid))?;
+
+                weight += tx.weight();
+                fee += graph.calculate_fee(tx)?;
+
+                let base_outpoint = self.select_largest_unspent_output(tx.clone())?;
+                outpoints.push(base_outpoint);
+
+                Ok((weight, fee, outpoints))
+            },
+        )?;
+
+        Ok(CPFPSet::new(parent_fee, parent_weight, selected_outpoints))
+    }
+
+    /// Selects the largest unspent output from a given transaction
+    pub fn select_largest_unspent_output(
+        &self,
+        tx: Arc<Transaction>,
+    ) -> Result<OutPoint, CPFPError> {
+        let txid = tx.compute_txid();
+        let outpoint = tx
+            .output
+            .iter()
+            .enumerate()
+            .map(|(vout, txout)| {
+                (
+                    OutPoint {
+                        txid,
+                        vout: vout as u32,
+                    },
+                    txout,
+                )
+            })
+            .filter(|(op, _)| self.is_unspent(*op))
+            .max_by_key(|(_, txout)| txout.value)
+            .map(|(op, _)| op)
+            .ok_or(CPFPError::NoUnspentOutput(txid))?;
+
+        Ok(outpoint)
+    }
+
     /// Whether outpoint is a leaf (unspent).
     pub fn is_unspent(&self, outpoint: OutPoint) -> bool {
         if self.spends.contains_key(&outpoint) {
@@ -229,11 +288,6 @@ impl CanonicalUnspents {
     pub fn get_tx(&self, txid: &Txid) -> Option<&Arc<Transaction>> {
         self.txs.get(txid)
     }
-
-    /// Retrieves the transaction ID that spends a given output, if any.
-    pub fn get_spend(&self, outpoint: &OutPoint) -> Option<&Txid> {
-        self.spends.get(outpoint)
-    }
 }
 
 /// Canonical unspents error