WIP: Do RBF properly as according to bitcoin core policy

evanlinjin · evanlinjin · commit 12f036cb8868 · 2025-04-08T14:36:20.000+10:00
diff --git a/src/coin_control.rs b/src/coin_control.rs
@@ -82,14 +82,22 @@ impl<'g, C: ChainOracle<Error = Infallible>> CoinControl<'g, C> {
         let mut canonical = tx_graph
             .canonical_iter(chain, chain_tip)
             .map(|r| r.expect("infallible"))
-            .map(|(txid, _, _)| txid)
+            .flat_map(|(txid, tx, _)| {
+                tx.input
+                    .iter()
+                    .map(|txin| txin.previous_output.txid)
+                    .chain([txid])
+                    .collect::<Vec<_>>()
+            })
             .collect::<HashSet<Txid>>();
         let exclude = replace
             .into_iter()
             .filter_map(|txid| tx_graph.get_tx(txid))
             .flat_map(|tx| {
+                let txid = tx.compute_txid();
                 tx_graph
-                    .walk_conflicts(&tx, move |_, txid| Some(txid))
+                    .walk_descendants(txid, move |_, txid| Some(txid))
+                    .chain(core::iter::once(txid))
                     .collect::<Vec<_>>()
             });
         for txid in exclude {
@@ -134,7 +142,7 @@ impl<'g, C: ChainOracle<Error = Infallible>> CoinControl<'g, C> {
             .tx_graph
             .get_tx_node(outpoint.txid)
             .ok_or(ExcludeInputReason::DoesNotExist)?;
-        if self.canonical.contains(&tx_node.txid) {
+        if !self.canonical.contains(&tx_node.txid) {
             return Err(ExcludeInputReason::NotCanonical);
         }
         if self.is_spent(outpoint) {
diff --git a/src/input.rs b/src/input.rs
@@ -309,6 +309,15 @@ impl Input {
         false
     }
 
+    /// Confirmations of this tx.
+    pub fn confirmations(&self, tip_height: absolute::Height) -> u32 {
+        self.status.map_or(0, |status| {
+            tip_height
+                .to_consensus_u32()
+                .saturating_sub(status.height.to_consensus_u32().saturating_sub(1))
+        })
+    }
+
     /// Whether this output can be spent now.
     pub fn is_spendable_now(&self, tip_height: absolute::Height, tip_time: absolute::Time) -> bool {
         !self.is_immature(tip_height) && !self.is_timelocked(tip_height, tip_time)
@@ -405,6 +414,15 @@ impl InputGroup {
             .all(|input| input.is_spendable_now(tip_height, tip_time))
     }
 
+    /// Returns the tx confirmation count this is the smallest in this group.
+    pub fn min_confirmations(&self, tip_height: absolute::Height) -> u32 {
+        self.inputs()
+            .iter()
+            .map(|input| input.confirmations(tip_height))
+            .min()
+            .expect("group must not be empty")
+    }
+
     /// Total value of all contained inputs.
     pub fn value(&self) -> Amount {
         self.inputs().iter().map(|input| input.txout.value).sum()
diff --git a/src/input_candidates.rs b/src/input_candidates.rs
@@ -1,9 +1,13 @@
+use crate::collections::HashSet;
 use alloc::vec::Vec;
 use bdk_coin_select::Candidate;
+use bitcoin::OutPoint;
+use miniscript::bitcoin;
 
 use crate::InputGroup;
 
 /// Candidates ready for coin selection.
+#[must_use]
 #[derive(Debug, Clone)]
 pub struct InputCandidates {
     must_select_count: usize,
@@ -40,6 +44,62 @@ impl InputCandidates {
         }
     }
 
+    /// Try create a new [`InputCandidates`] with the provided `outpoints` as "must spend".
+    ///
+    /// TODO: This can be optimized later. Right API first.
+    ///
+    /// # Error
+    ///
+    /// Returns the original [`InputCandidates`] if any outpoint is not found.
+    pub fn with_must_select(self, outpoints: HashSet<OutPoint>) -> Result<Self, Self> {
+        let (must_select, can_select) = self.groups.iter().partition::<Vec<_>, _>(|group| {
+            group
+                .inputs()
+                .iter()
+                .any(|input| outpoints.contains(&input.prev_outpoint()))
+        });
+
+        // `must_select` must contaon all outpoints.
+        let must_select_map = must_select
+            .iter()
+            .flat_map(|group| group.inputs().iter().map(|input| input.prev_outpoint()))
+            .collect::<HashSet<OutPoint>>();
+        if !must_select_map.is_superset(&outpoints) {
+            return Err(self);
+        }
+
+        let must_select_count = must_select.len();
+        let groups = must_select
+            .into_iter()
+            .chain(can_select)
+            .cloned()
+            .collect::<Vec<_>>();
+        let cs_candidates = groups
+            .iter()
+            .map(|group| Candidate {
+                value: group.value().to_sat(),
+                weight: group.weight(),
+                input_count: group.input_count(),
+                is_segwit: group.is_segwit(),
+            })
+            .collect();
+        Ok(InputCandidates {
+            must_select_count,
+            groups,
+            cs_candidates,
+        })
+    }
+
+    /// Whether the outpoint is contained in our candidates.
+    pub fn contains(&self, outpoint: OutPoint) -> bool {
+        self.groups.iter().any(|group| {
+            group
+                .inputs()
+                .iter()
+                .any(|input| input.prev_outpoint() == outpoint)
+        })
+    }
+
     /// Iterate all groups (both must_select and can_select).
     pub fn iter(&self) -> impl ExactSizeIterator<Item = (InputGroup, Candidate)> + '_ {
         self.groups
diff --git a/src/selection.rs b/src/selection.rs
@@ -1,20 +1,19 @@
 use core::fmt::{Debug, Display};
 use std::vec::Vec;
 
-use bdk_coin_select::float::Ordf32;
-use bdk_coin_select::metrics::LowestFee;
-use bdk_coin_select::{
-    ChangePolicy, CoinSelector, DrainWeights, FeeRate, NoBnbSolution, Target, TargetFee,
-    TargetOutputs,
-};
-use bitcoin::{absolute, transaction, Amount, TxOut};
+use bdk_coin_select::FeeRate;
+use bitcoin::{absolute, transaction};
 use miniscript::bitcoin;
 use miniscript::psbt::PsbtExt;
 
-use crate::{DefiniteDescriptor, Finalizer, Input, InputCandidates, Output};
+use crate::{Finalizer, Input, Output};
 
 const FALLBACK_SEQUENCE: bitcoin::Sequence = bitcoin::Sequence::ENABLE_LOCKTIME_NO_RBF;
 
+pub(crate) fn cs_feerate(feerate: bitcoin::FeeRate) -> bdk_coin_select::FeeRate {
+    FeeRate::from_sat_per_wu(feerate.to_sat_per_kwu() as f32 / 1000.0)
+}
+
 /// Final selection of inputs and outputs.
 #[derive(Debug, Clone)]
 pub struct Selection {
@@ -24,45 +23,6 @@ pub struct Selection {
     pub outputs: Vec<Output>,
 }
 
-/// Parameters for creating tx.
-#[derive(Debug, Clone)]
-pub struct SelectionParams {
-    /// To derive change output.
-    ///
-    /// Will error if this is unsatisfiable descriptor.
-    pub change_descriptor: DefiniteDescriptor,
-
-    /// Feerate target!
-    pub target_feerate: bitcoin::FeeRate,
-
-    /// Uses `target_feerate` as a fallback.
-    pub long_term_feerate: Option<bitcoin::FeeRate>,
-
-    /// Outputs that must be included.
-    pub target_outputs: Vec<Output>,
-
-    /// Max rounds of branch-and-bound.
-    /// TODO: Remove.
-    pub max_rounds: usize,
-}
-
-impl SelectionParams {
-    /// With default params.
-    pub fn new(
-        change_descriptor: DefiniteDescriptor,
-        target_outputs: Vec<Output>,
-        target_feerate: bitcoin::FeeRate,
-    ) -> Self {
-        Self {
-            change_descriptor,
-            target_feerate,
-            long_term_feerate: None,
-            target_outputs,
-            max_rounds: 100_000,
-        }
-    }
-}
-
 /// Parameters for creating a psbt.
 #[derive(Debug, Clone)]
 pub struct PsbtParams {
@@ -76,6 +36,9 @@ pub struct PsbtParams {
     /// It is best practive to set this to the latest block height to avoid fee sniping.
     pub fallback_locktime: absolute::LockTime,
 
+    /// Yes.
+    pub fallback_sequence: bitcoin::Sequence,
+
     /// Recommended.
     pub mandate_full_tx_for_segwit_v0: bool,
 }
@@ -85,40 +48,12 @@ impl Default for PsbtParams {
         Self {
             version: transaction::Version::TWO,
             fallback_locktime: absolute::LockTime::ZERO,
+            fallback_sequence: FALLBACK_SEQUENCE,
             mandate_full_tx_for_segwit_v0: true,
         }
     }
 }
 
-/// Selection Metrics.
-pub struct SelectionMetrics {
-    /// Selection score.
-    pub score: Ordf32,
-    /// Whether there is a change output in this selection.
-    pub has_change: bool,
-}
-
-/// When create_tx fails.
-#[derive(Debug)]
-pub enum SelectionError {
-    /// No solution.
-    NoSolution(NoBnbSolution),
-    /// Cannot satisfy change descriptor.
-    CannotSatisfyChangeDescriptor(miniscript::Error),
-}
-
-impl Display for SelectionError {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self {
-            SelectionError::NoSolution(no_bnb_solution) => Display::fmt(&no_bnb_solution, f),
-            SelectionError::CannotSatisfyChangeDescriptor(error) => Display::fmt(&error, f),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for SelectionError {}
-
 /// Occurs when creating a psbt fails.
 #[derive(Debug)]
 pub enum CreatePsbtError {
@@ -160,100 +95,6 @@ impl core::fmt::Display for CreatePsbtError {
 impl std::error::Error for CreatePsbtError {}
 
 impl Selection {
-    /// Constructor
-    pub fn new(
-        candidates: InputCandidates,
-        params: SelectionParams,
-    ) -> Result<(Self, SelectionMetrics), SelectionError> {
-        fn convert_feerate(feerate: bitcoin::FeeRate) -> bdk_coin_select::FeeRate {
-            FeeRate::from_sat_per_wu(feerate.to_sat_per_kwu() as f32 / 1000.0)
-        }
-
-        let cs_candidates = candidates.coin_select_candidates();
-
-        let target_feerate = convert_feerate(params.target_feerate);
-        let long_term_feerate =
-            convert_feerate(params.long_term_feerate.unwrap_or(params.target_feerate));
-        println!("target_feerate: {} sats/vb", target_feerate.as_sat_vb());
-
-        let target = Target {
-            fee: TargetFee::from_feerate(target_feerate),
-            outputs: TargetOutputs::fund_outputs(
-                params
-                    .target_outputs
-                    .iter()
-                    .map(|output| (output.txout().weight().to_wu(), output.value.to_sat())),
-            ),
-        };
-
-        let change_policy = ChangePolicy::min_value_and_waste(
-            DrainWeights {
-                output_weight: (TxOut {
-                    script_pubkey: params.change_descriptor.script_pubkey(),
-                    value: Amount::ZERO,
-                })
-                .weight()
-                .to_wu(),
-                spend_weight: params
-                    .change_descriptor
-                    .max_weight_to_satisfy()
-                    .map_err(SelectionError::CannotSatisfyChangeDescriptor)?
-                    .to_wu(),
-                n_outputs: 1,
-            },
-            params
-                .change_descriptor
-                .script_pubkey()
-                .minimal_non_dust()
-                .to_sat(),
-            target_feerate,
-            long_term_feerate,
-        );
-
-        let bnb_metric = LowestFee {
-            target,
-            long_term_feerate,
-            change_policy,
-        };
-
-        let mut selector = CoinSelector::new(cs_candidates);
-
-        // Select input candidates that must be spent.
-        for index in 0..candidates.must_select_len() {
-            selector.select(index);
-        }
-
-        // We assume that this still works if the current selection is already a solution.
-        let score = selector
-            .run_bnb(bnb_metric, params.max_rounds)
-            .map_err(SelectionError::NoSolution)?;
-
-        let maybe_drain = selector.drain(target, change_policy);
-        Ok((
-            Selection {
-                inputs: selector
-                    .apply_selection(candidates.groups())
-                    .flat_map(|group| group.inputs())
-                    .cloned()
-                    .collect::<Vec<Input>>(),
-                outputs: {
-                    let mut outputs = params.target_outputs;
-                    if maybe_drain.is_some() {
-                        outputs.push(Output::with_descriptor(
-                            params.change_descriptor,
-                            Amount::from_sat(maybe_drain.value),
-                        ));
-                    }
-                    outputs
-                },
-            },
-            SelectionMetrics {
-                score,
-                has_change: maybe_drain.is_some(),
-            },
-        ))
-    }
-
     /// Returns none if there is a mismatch of units in `locktimes`.
     ///
     /// As according to BIP-64...
@@ -298,7 +139,7 @@ impl Selection {
                 .map(|input| bitcoin::TxIn {
                     previous_output: input.prev_outpoint(),
                     // TODO: Custom fallback sequence?
-                    sequence: input.sequence().unwrap_or(FALLBACK_SEQUENCE),
+                    sequence: input.sequence().unwrap_or(params.fallback_sequence),
                     ..Default::default()
                 })
                 .collect(),
diff --git a/src/selector.rs b/src/selector.rs
diff --git a/tests/synopsis.rs b/tests/synopsis.rs
