refactor(cpfp): improve owned output selection to balance dust avoidance and fee efficiency

Author: aagbotemi

examples/common.rs

@@ -188,7 +188,14 @@ impl Wallet {
         let canon_utxos = CanonicalUnspents::new(self.canonical_txs());
 
         let script_pubkey = self.next_address().unwrap().script_pubkey();
-        let cpfpset = canon_utxos.build_cpfp_set_from_txids(parent_txids, self.graph.graph())?;
+        let ownership_check =
+            |outpoint: OutPoint| -> bool { self.graph.index.txout(outpoint).is_some() };
+
+        let cpfpset = canon_utxos.build_cpfp_set_from_txids(
+            parent_txids,
+            self.graph.graph(),
+            ownership_check,
+        )?;
         let plans = cpfpset
             .selected_outpoints
             .iter()

examples/cpfp.rs

@@ -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..3 {
+    for i in 0..4 {
         let low_fee_selection = wallet
             .all_candidates()
             .regroup(group_by_spk())

src/canonical_unspents.rs

@@ -3,8 +3,8 @@ use alloc::vec::Vec;
 use core::fmt;
 
 use crate::{
-    collections::HashMap, input::CoinbaseMismatch, CPFPError, CPFPSet, 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};
@@ -21,6 +21,22 @@ pub struct CanonicalUnspents {
     spends: HashMap<OutPoint, Txid>,
 }
 
+fn select_upper_middle_output(candidates: &[(OutPoint, &TxOut)]) -> OutPoint {
+    let len = candidates.len();
+
+    let index = match len {
+        1..=2 => len - 1, // select the largest for small sets
+        3..=5 => len - 2, // select second largest for medium sets
+        _ => {
+            let upper_third_start = (len * 2) / 3;
+            let upper_third_end = len - 1;
+            (upper_third_start + upper_third_end) / 2
+        }
+    };
+
+    candidates[index].0
+}
+
 impl CanonicalUnspents {
     /// Construct [`CanonicalUnspents`] from an iterator of txs with confirmation status.
     pub fn new<T>(canonical_txs: impl IntoIterator<Item = TxWithStatus<T>>) -> Self
@@ -128,11 +144,15 @@ impl CanonicalUnspents {
     }
 
     /// Constructs a '[CPFPSet]' from a set of parent transaction IDs.
-    pub fn build_cpfp_set_from_txids(
+    pub fn build_cpfp_set_from_txids<F>(
         &self,
         parent_txids: impl IntoIterator<Item = Txid>,
         graph: &TxGraph,
-    ) -> Result<CPFPSet, CPFPError> {
+        ownership_check: F,
+    ) -> Result<CPFPSet, CPFPError>
+    where
+        F: Fn(OutPoint) -> bool + Clone,
+    {
         let parent_txids: Vec<Txid> = parent_txids.into_iter().collect();
 
         const MAX_ANCESTORS: usize = 25;
@@ -148,8 +168,9 @@ impl CanonicalUnspents {
                 weight += tx.weight();
                 fee += graph.calculate_fee(tx)?;
 
-                let base_outpoint = self.select_largest_unspent_output(tx.clone())?;
-                outpoints.push(base_outpoint);
+                let selected_outpoint =
+                    self.select_owned_unspent_output(tx.clone(), ownership_check.clone())?;
+                outpoints.push(selected_outpoint);
 
                 Ok((weight, fee, outpoints))
             },
@@ -158,13 +179,17 @@ impl CanonicalUnspents {
         Ok(CPFPSet::new(parent_fee, parent_weight, selected_outpoints))
     }
 
-    /// Selects the largest unspent output from a given transaction
-    pub fn select_largest_unspent_output(
+    /// Selects the unspent output from a given transaction
+    pub fn select_owned_unspent_output<F>(
         &self,
         tx: Arc<Transaction>,
-    ) -> Result<OutPoint, CPFPError> {
+        ownership_check: F,
+    ) -> Result<OutPoint, CPFPError>
+    where
+        F: Fn(OutPoint) -> bool,
+    {
         let txid = tx.compute_txid();
-        let outpoint = tx
+        let mut candidates: Vec<_> = tx
             .output
             .iter()
             .enumerate()
@@ -177,12 +202,22 @@ impl CanonicalUnspents {
                     txout,
                 )
             })
-            .filter(|(op, _)| self.is_unspent(*op))
-            .max_by_key(|(_, txout)| txout.value)
-            .map(|(op, _)| op)
-            .ok_or(CPFPError::NoUnspentOutput(txid))?;
+            .filter(|(op, tx_out)| {
+                // Must be unspent, owned and above dust threshold
+                self.is_unspent(*op)
+                    && ownership_check(*op)
+                    && tx_out.value >= tx_out.script_pubkey.minimal_non_dust()
+            })
+            .collect();
+
+        if candidates.is_empty() {
+            return Err(CPFPError::NoUnspentOutput(txid));
+        }
+
+        candidates.sort_by_key(|(_, txout)| txout.value);
+        let selected_outpoint = select_upper_middle_output(&candidates);
 
-        Ok(outpoint)
+        Ok(selected_outpoint)
     }
 
     /// Whether outpoint is a leaf (unspent).

src/cpfp.rs

@@ -39,22 +39,21 @@ impl CPFPSet {
         script_pubkey: &ScriptBuf,
         plans: impl IntoIterator<Item = Plan>,
     ) -> Result<Selection, CPFPError> {
-        let mut inputs = Vec::new();
-        let mut total_input_value = Amount::ZERO;
-
-        // Get inputs from selected outpoints
-        for (outpoint, plan) in self.selected_outpoints.iter().zip(plans) {
-            if let Some(input) = canon_utxos.try_get_unspent(*outpoint, plan) {
-                total_input_value += input.prev_txout().value;
-                inputs.push(input);
-            } else {
-                return Err(CPFPError::NoUnspentOutput(outpoint.txid));
-            }
-        }
+        let inputs: Vec<Input> = self
+            .selected_outpoints
+            .iter()
+            .zip(plans)
+            .map(|(op, plan)| {
+                canon_utxos
+                    .try_get_unspent(*op, plan)
+                    .ok_or(CPFPError::NoUnspentOutput(op.txid))
+            })
+            .collect::<Result<Vec<Input>, CPFPError>>()?;
 
         if inputs.is_empty() {
             return Err(CPFPError::NoSpendableOutputs);
         }
+        let total_input_value: Amount = inputs.iter().map(|input| input.prev_txout().value).sum();
 
         let child_weight = self.estimate_child_tx_weight(&inputs, script_pubkey);