Merge #1076: Bump bdk_coin_select to 0.3.0

43ecd94 spend: change parameter type for rbf (jp1ac4)
6376909 commands: remove rbf rule 4 logic (jp1ac4)
936d7e9 spend: bump bdk_coin_select to 0.3.0 (jp1ac4)

Pull request description:

  This is to resolve #923.

  The `score` method of the `LowestFee` metric has been fixed upstream and so our temporary partial fix from #867 is no longer required.

  The `min_fee` parameter of `select_coins_for_spend`, if positive, now ensures that RBF rule 4 is satisfied. Accordingly, it has been renamed to `replaced_fee` and made an `Option`. We could potentially have used the `SpendTxFees` enum as a parameter directly instead of `feerate_vb` and `replaced_fee`, but `feerate_vb` is currently `f32` rather than `u64` and so I kept them as separate parameters.

  Thanks to how the `replaced_fee` parameter works, the fee iteration approach used in `rbf_psbt` to ensure the replacement satisfies [RBF rule 4](https://github.com/bitcoin/bitcoin/blob/master/doc/policy/mempool-replacements.md#current-replace-by-fee-policy) is no longer required.

  `base_weight` is no longer stored in `CoinSelector` and instead the output weights are stored in `Target`. This means that the `output_weight` of `DrainWeights` no longer needs to take into account a potential change in output count varint as this is now handled by bdk_coin_select.

  The min value for change no longer includes the threshold itself and so we have to subtract 1 from our change policy's min value (see bitcoindevkit/coin-select#14 (comment)). We already have a [test](https://github.com/wizardsardine/liana/blob/master/src/commands/mod.rs#L1653-#L1654) that fails without this subtraction as it expects a change output of 5000 sats.

ACKs for top commit:
  darosior:
    ACK 43ecd94

Tree-SHA512: a064bafef13abefcb8c4b640cfc4017eb288c62891a8b486add33c1499e7061bf262d6aadabbdd4c191ed9b81e3931b382c8c8445e6bb9c1b052380caf14b3f9

Cargo.toml

@@ -29,7 +29,7 @@ nonblocking_shutdown = []
 miniscript = { version = "11.0", features = ["serde", "compiler", "base64"] }
 
 # Coin selection algorithms for spend transaction creation.
-bdk_coin_select = { version = "0.1.0" }
+bdk_coin_select = "0.3"
 
 # Don't reinvent the wheel
 dirs = "5.0"

src/commands/mod.rs

@@ -897,27 +897,40 @@ impl DaemonControl {
         if !is_cancel {
             candidate_coins.extend(&confirmed_cands);
         }
-        // Try with increasing fee until fee paid by replacement transaction is high enough.
-        // Replacement fee must be at least:
-        // sum of fees paid by original transactions + incremental feerate * replacement size.
-        // Loop will continue until either we find a suitable replacement or we have insufficient funds.
-        let mut replacement_vsize = 0;
-        for incremental_feerate in 0.. {
-            let min_fee = descendant_fees.to_sat() + replacement_vsize * incremental_feerate;
-            let CreateSpendRes {
-                psbt: rbf_psbt,
-                has_change,
-                warnings,
-            } = match create_spend(
+        // The replaced fee is the fee of the transaction being replaced and its descendants. Coin selection
+        // will ensure that the replacement transaction additionally pays for its own weight as per
+        // RBF rule 4.
+        let replaced_fee = descendant_fees.to_sat();
+        // This loop can have up to 2 iterations in the case of cancel and otherwise only 1.
+        loop {
+            match create_spend(
                 &self.config.main_descriptor,
                 &self.secp,
                 &mut tx_getter,
                 &destinations,
                 &candidate_coins,
-                SpendTxFees::Rbf(feerate_vb, min_fee),
+                SpendTxFees::Rbf(feerate_vb, replaced_fee),
                 change_address.clone(),
             ) {
-                Ok(psbt) => psbt,
+                Ok(CreateSpendRes {
+                    psbt,
+                    has_change,
+                    warnings,
+                }) => {
+                    // In case of success, make sure to update our next derivation index if any address
+                    // used in the transaction outputs was from the future.
+                    for (addr, _) in destinations {
+                        self.maybe_increase_next_deriv_index(&mut db_conn, &addr.info);
+                    }
+                    if has_change {
+                        self.maybe_increase_next_deriv_index(&mut db_conn, &change_address.info);
+                    }
+
+                    return Ok(CreateSpendResult::Success {
+                        psbt,
+                        warnings: warnings.iter().map(|w| w.to_string()).collect(),
+                    });
+                }
                 Err(SpendCreationError::CoinSelection(e)) => {
                     // If we get a coin selection error due to insufficient funds and we want to cancel the
                     // transaction, then set all previous coins as mandatory and add confirmed coins as
@@ -936,32 +949,7 @@ impl DaemonControl {
                     return Err(e.into());
                 }
             };
-            replacement_vsize = self
-                .config
-                .main_descriptor
-                .unsigned_tx_max_vbytes(&rbf_psbt.unsigned_tx);
-
-            // Make sure it satisfies RBF rule 4.
-            if rbf_psbt.fee().expect("has already been sanity checked")
-                >= descendant_fees + bitcoin::Amount::from_sat(replacement_vsize)
-            {
-                // In case of success, make sure to update our next derivation index if any address
-                // used in the transaction outputs was from the future.
-                for (addr, _) in destinations {
-                    self.maybe_increase_next_deriv_index(&mut db_conn, &addr.info);
-                }
-                if has_change {
-                    self.maybe_increase_next_deriv_index(&mut db_conn, &change_address.info);
-                }
-
-                return Ok(CreateSpendResult::Success {
-                    psbt: rbf_psbt,
-                    warnings: warnings.iter().map(|w| w.to_string()).collect(),
-                });
-            }
         }
-
-        unreachable!("We keep increasing the min fee until we run out of funds or satisfy rule 4.")
     }
 
     /// Trigger a rescan of the block chain for transactions involving our main descriptor between