Merge #21: No more base weight

c650306 fix: `CoinSelector::implied_fee` (志宇)
4d0ae4c docs: fix typos (志宇)
2e26aa5 fix: rm `Drain::none` method (志宇)
8eb582b [lowest-fee] ignore failure when selection is not possible (LLFourn)
064996c [lowest-fee] Fix off by one in test (LLFourn)
956685a [changeless] Revert number of candidates (LLFourn)
c6f0682 Remove typo in lowest_fee comment (LLFourn)
76e67a3 Move ChangePolicy to drain.rs (LLFourn)
7e82c3f Remove base_weight, put weight in Target (LLFourn)

Pull request description:

  Fixes #1

  On top of #19

  - CoinSelector no longer tracks anything but input weight
  - Previously the value of the target outputs was in `Target` but the
    weights were accounted for in CoinSelector. Now they're in all in
    target.
  - This allows us to actually figure out how many outputs there are and
    therefore the actual weight of the transaction accounting for the varint for the number of outputs.

  This wasn't what the issue had in mind but it was easier to take the `base_weight` out of `CoinSelector` and put it in `Target` rather than put `Target` in `CoinSelector`. Getting rid of `base_weight` is a more crucial change than expected because rust bitcoin changed what `Transaction::weight` returns for empty output transactions recently so using it to determine `base_weight` will get different answers between versions (this breaks our weight tests but this PR will fix it I think if we uprade dev deps). We only need to know the total weight of the outputs and how many there are now to get the right answers for weight.

ACKs for top commit:
  evanlinjin:
    ACK c650306

Tree-SHA512: bd2d6bba15a172b56451d13a9560b6221a6f005417857e1cf7f0cd7022cebd0c2e4d6ff78dac9b99690771297351f90eb3fea26f85aa34a2841c91cfe0d73f9c

Author: evanlinjin

CHANGELOG.md

@@ -4,4 +4,7 @@
 - Remove `min_fee` in favour of `replace` which allows you to replace a transaction
 - Remove `Drain` argument from `CoinSelector::select_until_target_met` because adding a drain won't
   change when the target is met.
+- No more `base_weight` in `CoinSelector`. Weight of the outputs is tracked in `target`.
+- You now account for the number of outputs in both drain and target and their weight.
+- Removed waste metric because it was pretty broken and took a lot to maintain
 

README.md

@@ -1,36 +1,40 @@
 # BDK Coin Selection
 
-`bdk_coin_select` is a tool to help you select inputs for making Bitcoin (ticker: BTC) transactions.
-It's got zero dependencies so you can paste it into your project without concern.
+`bdk_coin_select` is a zero-dependency tool to help you select inputs for making Bitcoin (ticker: BTC) transactions.
 
 > ⚠ This work is only ready to use by those who expect (potentially catastrophic) bugs and will have
 > the time to investigate them and contribute back to this crate.
 
-## Constructing the `CoinSelector`
-
-The main structure is [`CoinSelector`](crate::CoinSelector). To construct it, we specify a list of
-candidate UTXOs and a transaction `base_weight`. The `base_weight` includes the recipient outputs
-and mandatory inputs (if any).
+## Synopis
 
 ```rust
 use std::str::FromStr;
-use bdk_coin_select::{ CoinSelector, Candidate, TR_KEYSPEND_TXIN_WEIGHT};
+use bdk_coin_select::{ CoinSelector, Candidate, TR_KEYSPEND_TXIN_WEIGHT, Drain, FeeRate, Target, ChangePolicy, TargetOutputs, TargetFee, DrainWeights};
 use bitcoin::{ Address, Network, Transaction, TxIn, TxOut };
 
-// The address where we want to send our coins.
 let recipient_addr = 
     Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46").unwrap();
 
+let outputs = vec![TxOut {
+    value: 3_500_000,
+    script_pubkey: recipient_addr.payload.script_pubkey(),
+}];
+
+let target = Target {
+    outputs: TargetOutputs::fund_outputs(outputs.iter().map(|output| (output.weight() as u32, output.value))),
+    fee: TargetFee::from_feerate(FeeRate::from_sat_per_vb(42.0))
+};
+
 let candidates = vec![
     Candidate {
         // How many inputs does this candidate represents. Needed so we can 
         // figure out the weight of the varint that encodes the number of inputs
         input_count: 1,
         // the value of the input
         value: 1_000_000,
-        // the total weight of the input(s).
+        // the total weight of the input(s) including their witness/scriptSig
         // you may need to use miniscript to figure out the correct value here.
-        weight: TR_KEYSPEND_TXIN_WEIGHT, 
+        weight: TR_KEYSPEND_TXIN_WEIGHT,
         // wether it's a segwit input. Needed so we know whether to include the
         // segwit header in total weight calculations.
         is_segwit: true
@@ -45,91 +49,54 @@ let candidates = vec![
     }
 ];
 
-let base_tx = Transaction {
-    input: vec![],
-    // include your recipient outputs here
-    output: vec![TxOut {
-        value: 900_000,
-        script_pubkey: recipient_addr.payload.script_pubkey(),
-    }],
-    lock_time: bitcoin::absolute::LockTime::from_height(0).unwrap(),
-    version: 0x02,
-};
-let base_weight = base_tx.weight().to_wu() as u32;
-println!("base weight: {}", base_weight);
-
 // You can now select coins!
-let mut coin_selector = CoinSelector::new(&candidates, base_weight);
+let mut coin_selector = CoinSelector::new(&candidates);
 coin_selector.select(0);
+
+assert!(!coin_selector.is_target_met(target), "we didn't select enough");
+println!("we didn't select enough yet we're missing: {}", coin_selector.missing(target));
+coin_selector.select(1);
+assert!(coin_selector.is_target_met(target), "we should have enough now");
+
+// Now we need to know if we need a change output to drain the excess if we overshot too much
+//
+// We don't need to know exactly which change output we're going to use yet but we assume it's a taproot output
+// that we'll use a keyspend to spend from.
+let drain_weights = DrainWeights::TR_KEYSPEND; 
+// Our policy is to only add a change output if the value is over 1_000 sats
+let change_policy = ChangePolicy::min_value(drain_weights, 1_000);
+let change = coin_selector.drain(target, change_policy);
+if change.is_some() {
+    println!("We need to add our change output to the transaction with {} value", change.value);
+} else {
+    println!("Yay we don't need to add a change output");
+}
 ```
 
-## Change Policy
+## Automatic selection with Branch and Bound
 
-A change policy determines whether the drain output(s) should be in the final solution. The
-determination is simple: if the excess value is above a threshold then the drain should be added. To
-construct a change policy you always provide `DrainWeights` which tell the coin selector the weight
-cost of adding the drain. `DrainWeights` includes two weights. One is the weight of the drain
-output(s). The other is the weight of spending the drain output later on (the input weight).
+You can use methods such as [`CoinSelector::select`] to manually select coins, or methods such as
+[`CoinSelector::select_until_target_met`] for a rudimentary automatic selection. Probably you want
+to use [`CoinSelector::run_bnb`] to do this in a smart way.
 
+Built-in metrics are provided in the [`metrics`] submodule. Currently, only the
+[`LowestFee`](metrics::LowestFee) metric is considered stable. Note you *can* try and write your own
+metric by implementing the [`BnbMetric`] yourself but we don't recommend this.
 
 ```rust
 use std::str::FromStr;
-use bdk_coin_select::{CoinSelector, Candidate, DrainWeights, TXIN_BASE_WEIGHT, ChangePolicy, TR_KEYSPEND_TXIN_WEIGHT};
-use bitcoin::{Address, Network, Transaction, TxIn, TxOut};
-let base_tx = Transaction {
-    input: vec![],
-    output: vec![/* include your recipient outputs here */],
-    lock_time: bitcoin::absolute::LockTime::from_height(0).unwrap(),
-    version: 0x02,
-};
-let base_weight = base_tx.weight().to_wu() as u32;
-
-// The change output that may or may not be included in the final transaction.
-let drain_addr =
-    Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46")
-    .expect("address must be valid")
-    .require_network(Network::Testnet)
-    .expect("network must match");
-
-// The drain output(s) may or may not be included in the final tx. We calculate
-// the drain weight to include the output length varint weight changes from
-// including the drain output(s).
-let drain_output_weight = {
-    let mut tx_with_drain = base_tx.clone();
-    tx_with_drain.output.push(TxOut {
-        script_pubkey: drain_addr.script_pubkey(),
-        ..Default::default()
-    });
-    tx_with_drain.weight().to_wu() as u32 - base_weight
-};
-println!("drain output weight: {}", drain_output_weight);
-
-let drain_weights = DrainWeights {
-    output_weight: drain_output_weight,
-    spend_weight: TR_KEYSPEND_TXIN_WEIGHT,
-};
-
-// This constructs a change policy that creates change when the change value is
-// greater than or equal to the dust limit.
-let change_policy = ChangePolicy::min_value(
-    drain_weights,
-    drain_addr.script_pubkey().dust_value().to_sat(),
-);
-```
-
-## Branch and Bound
+use bdk_coin_select::{ Candidate, CoinSelector, FeeRate, Target, TargetFee, TargetOutputs, ChangePolicy, TR_KEYSPEND_TXIN_WEIGHT, TR_DUST_RELAY_MIN_VALUE};
+use bdk_coin_select::metrics::LowestFee;
+use bitcoin::{ Address, Network, Transaction, TxIn, TxOut };
 
-You can use methods such as [`CoinSelector::select`] to manually select coins, or methods such as
-[`CoinSelector::select_until_target_met`] for a rudimentary automatic selection. However, if you 
-wish to automatically select coins to optimize for a given metric, [`CoinSelector::run_bnb`] can be
-used.
+let recipient_addr =
+    Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46").unwrap();
 
-Built-in metrics are provided in the [`metrics`] submodule. Currently, only the 
-[`LowestFee`](metrics::LowestFee) metric is considered stable.
+let outputs = vec![TxOut {
+    value: 210_000,
+    script_pubkey: recipient_addr.payload.script_pubkey(),
+}];
 
-```rust
-use bdk_coin_select::{ Candidate, CoinSelector, FeeRate, Target, TargetFee, ChangePolicy, TR_KEYSPEND_TXIN_WEIGHT };
-use bdk_coin_select::metrics::LowestFee;
 let candidates = [
     Candidate {
         input_count: 1,
@@ -150,34 +117,35 @@ let candidates = [
         is_segwit: true
     }
 ];
-let base_weight = 0;
 let drain_weights = bdk_coin_select::DrainWeights::default();
-let dust_limit = 0;
+// You could determine this by looking at the user's transaction history and taking an average of the feerate.
 let long_term_feerate = FeeRate::from_sat_per_vb(10.0);
 
-let mut coin_selector = CoinSelector::new(&candidates, base_weight);
+let mut coin_selector = CoinSelector::new(&candidates);
 
 let target = Target {
     fee: TargetFee::from_feerate(FeeRate::from_sat_per_vb(15.0)),
-    value: 210_000,
+    outputs: TargetOutputs::fund_outputs(outputs.iter().map(|output| (output.weight() as u32, output.value))),
 };
 
+// The change output must be at least this size to be relayed.
+// To choose it you need to know the kind of script pubkey on your change txout.
+// Here we assume it's a taproot output
+let dust_limit = TR_DUST_RELAY_MIN_VALUE;
+
 // We use a change policy that introduces a change output if doing so reduces
-// the "waste" and that the change output's value is at least that of the 
-// `dust_limit`.
+// the "waste" (i.e. adding change doesn't increase the fees we'd pay if we factor in the cost to spend the output later on).
 let change_policy = ChangePolicy::min_value_and_waste(
     drain_weights,
     dust_limit,
     target.fee.rate,
     long_term_feerate,
 );
 
-// This metric minimizes transaction fees paid over time. The 
-// `long_term_feerate` is used to calculate the additional fee from spending 
-// the change output in the future.
+// The LowestFee metric tries make selections that minimize your total fees paid over time.
 let metric = LowestFee {
     target,
-    long_term_feerate,
+    long_term_feerate, // used to calculate the cost of spending th change output if the future
     change_policy
 };
 
@@ -203,79 +171,6 @@ match coin_selector.run_bnb(metric, 100_000) {
 
 ```
 
-## Finalizing a Selection
-
-- [`is_target_met`] checks whether the current state of [`CoinSelector`] meets the [`Target`].
-- [`apply_selection`] applies the selection to the original list of candidate `TxOut`s.
-
-[`is_target_met`]: crate::CoinSelector::is_target_met
-[`apply_selection`]: crate::CoinSelector::apply_selection
-[`CoinSelector`]: crate::CoinSelector
-[`Target`]: crate::Target
-
-```rust
-use bdk_coin_select::{CoinSelector, Candidate, DrainWeights, Target, ChangePolicy, TR_KEYSPEND_TXIN_WEIGHT, Drain};
-use bitcoin::{Amount, TxOut, Address};
-let base_weight = 0_u32;
-let drain_weights = DrainWeights::TR_KEYSPEND;
-use core::str::FromStr;
-
-// A random target, as an example.
-let target = Target {
-    value: 21_000,
-    ..Default::default()
-};
-// Am arbitary drain policy, for the example.
-let change_policy = ChangePolicy::min_value(drain_weights, 1337);
-
-// This is a list of candidate txouts for coin selection. If a txout is picked,
-// our transaction's input will spend it.
-let candidate_txouts = vec![
-    TxOut {
-        value: 100_000,
-        script_pubkey: Address::from_str("bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr").unwrap().payload.script_pubkey(),
-    },
-    TxOut {
-        value: 150_000,
-        script_pubkey: Address::from_str("bc1p4qhjn9zdvkux4e44uhx8tc55attvtyu358kutcqkudyccelu0was9fqzwh").unwrap().payload.script_pubkey(),
-    },
-    TxOut {
-        value: 200_000,
-        script_pubkey: Address::from_str("bc1p0d0rhyynq0awa9m8cqrcr8f5nxqx3aw29w4ru5u9my3h0sfygnzs9khxz8").unwrap().payload.script_pubkey()
-    }
-];
-// We transform the candidate txouts into something `CoinSelector` can
-// understand.
-let candidates = candidate_txouts
-    .iter()
-    .map(|txout| Candidate {
-        input_count: 1,
-        value: txout.value,
-        weight: TR_KEYSPEND_TXIN_WEIGHT, // you need to figure out the weight of the txin somehow
-        is_segwit: txout.script_pubkey.is_witness_program(),
-    })
-    .collect::<Vec<_>>();
-
-let mut selector = CoinSelector::new(&candidates, base_weight);
-selector
-    .select_until_target_met(target)
-    .expect("we've got enough coins");
-
-// Get a list of coins that are selected.
-let selected_coins = selector
-    .apply_selection(&candidate_txouts)
-    .collect::<Vec<_>>();
-assert_eq!(selected_coins.len(), 1);
-
-// Determine whether we should add a change output.
-let drain = selector.drain(target, change_policy);
-
-if drain.is_some() {
-    // add our change output to the transaction
-    let change_value = drain.value;
-}
-```
-
 # Minimum Supported Rust Version (MSRV)
 
 This library is compiles on rust v1.54 and above

src/change_policy.rs

@@ -1,50 +1 @@
-//! This module contains a collection of change policies.
-//!
-//! A change policy determines whether a given coin selection (presented by [`CoinSelector`]) should
-//! construct a transaction with a change output. A change policy is represented as a function of
-//! type `Fn(&CoinSelector, Target) -> Drain`.
 
-#[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't
-use crate::float::FloatExt;
-use crate::{DrainWeights, FeeRate};
-
-/// Describes when a change output (although it could represent several) should be added that drains
-/// the excess in the coin selection. It includes the `drain_weights` to account for the cost of
-/// adding this outupt(s).
-#[derive(Clone, Copy, Debug, PartialEq)]
-pub struct ChangePolicy {
-    /// The minimum amount of excesss there needs to be add a change output.
-    pub min_value: u64,
-    /// The weights of the drain that would be added according to the policy.
-    pub drain_weights: DrainWeights,
-}
-
-impl ChangePolicy {
-    /// Construct a change policy that creates change when the change value is greater than
-    /// `min_value`.
-    pub fn min_value(drain_weights: DrainWeights, min_value: u64) -> Self {
-        Self {
-            drain_weights,
-            min_value,
-        }
-    }
-
-    /// Construct a change policy that creates change when it would reduce the transaction waste
-    /// given that `min_value` is respected.
-    pub fn min_value_and_waste(
-        drain_weights: DrainWeights,
-        min_value: u64,
-        target_feerate: FeeRate,
-        long_term_feerate: FeeRate,
-    ) -> Self {
-        // The output waste of a changeless solution is the excess.
-        let waste_with_change = drain_weights
-            .waste(target_feerate, long_term_feerate)
-            .ceil() as u64;
-
-        Self {
-            drain_weights,
-            min_value: waste_with_change.max(min_value),
-        }
-    }
-}