Merge pull request #14 from LLFourn/simplify-change-policy

Simplify change policy

Author: LLFourn

.github/workflows/test.yml

@@ -0,0 +1,56 @@
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+
+# Make sure CI fails on all warnings, including Clippy lints
+env:
+    RUSTFLAGS: "-Dwarnings"
+    RUSTDOCFLAGS: "-Dwarnings"
+
+jobs:
+  fmt:
+    name: Rustfmt
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+        with:
+          components: rustfmt
+      - run: cargo fmt --all -- --check
+
+  clippy_check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+        with:
+          components: clippy
+      - uses: Swatinem/rust-cache@v2
+      - run: cargo clippy --all-targets --all-features --tests
+
+  build-msrv:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/[email protected]
+        # don't want dev-dependencies for MSRV check
+      - run: sed -i 's/\[dev-dependencies]/[ignore-this-warning-fren]/g' Cargo.toml
+      - run: cargo build --release
+
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+      - run: cargo test --release
+
+  doc-build:
+     name: doc-build
+     runs-on: ubuntu-latest
+     steps:
+       - uses: actions/checkout@v4
+       - uses: dtolnay/rust-toolchain@stable
+       - run:  cargo doc --no-deps

Cargo.toml

@@ -15,11 +15,12 @@ readme = "README.md"
 [dependencies]
 # No dependencies! Please do not add any please!
 
-[dev-dependencies]
-rand = "0.7"
-proptest = "0.10"
-bitcoin = "0.30"
 
 [features]
 default = ["std"]
 std = []
+
+[dev-dependencies]
+rand = "0.8"
+proptest = "1.4"
+bitcoin = "0.30"

README.md

@@ -3,6 +3,9 @@
 `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.
 
+> ⚠ 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
@@ -11,18 +14,12 @@ and mandatory inputs (if any).
 
 ```rust
 use std::str::FromStr;
-use bdk_coin_select::{ CoinSelector, Candidate, TR_KEYSPEND_SATISFACTION_WEIGHT, TXIN_BASE_WEIGHT };
+use bdk_coin_select::{ CoinSelector, Candidate, TR_KEYSPEND_TXIN_WEIGHT};
 use bitcoin::{ Address, Network, Transaction, TxIn, TxOut };
 
-// You should use miniscript to figure out the satisfaction weight for your coins!
-const TR_INPUT_WEIGHT: u32 = TXIN_BASE_WEIGHT + TR_KEYSPEND_SATISFACTION_WEIGHT;
-
 // The address where we want to send our coins.
 let recipient_addr = 
-    Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46")
-    .expect("address must be valid")
-    .require_network(Network::Testnet)
-    .expect("network must match");
+    Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46").unwrap();
 
 let candidates = vec![
     Candidate {
@@ -32,7 +29,8 @@ let candidates = vec![
         // the value of the input
         value: 1_000_000,
         // the total weight of the input(s).
-        weight: TR_INPUT_WEIGHT,
+        // you may need to use miniscript to figure out the correct value here.
+        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
@@ -41,7 +39,7 @@ let candidates = vec![
         // A candidate can represent multiple inputs in the case where you 
         // always want some inputs to be spent together.
         input_count: 2,
-        weight: 2*TR_INPUT_WEIGHT,
+        weight: 2*TR_KEYSPEND_TXIN_WEIGHT,
         value: 3_000_000,
         is_segwit: true
     }
@@ -52,7 +50,7 @@ let base_tx = Transaction {
     // include your recipient outputs here
     output: vec![TxOut {
         value: 900_000,
-        script_pubkey: recipient_addr.script_pubkey(),
+        script_pubkey: recipient_addr.payload.script_pubkey(),
     }],
     lock_time: bitcoin::absolute::LockTime::from_height(0).unwrap(),
     version: 0x02,
@@ -67,30 +65,25 @@ coin_selector.select(0);
 
 ## Change Policy
 
-A change policy determines whether the drain output(s) should be in the final solution. A change
-policy is represented by a closure of signature `Fn(&CoinSelector, Target) -> Drain`. We provide 3
-built-in change policies; `min_value`, `min_waste` and `min_value_and_waste` (refer to the 
-[module-level docs](crate::change_policy) for more).
+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).
 
-Typically, to construct a change policy, the [`DrainWeights`] need to be provided. `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).
 
 ```rust
-# use std::str::FromStr;
-# use bdk_coin_select::{ CoinSelector, Candidate, DrainWeights, TXIN_BASE_WEIGHT };
-# use bitcoin::{ Address, Network, Transaction, TxIn, TxOut };
-use bdk_coin_select::change_policy::min_value;
-# const TR_SATISFACTION_WEIGHT: u32 = 66;
-# const TR_INPUT_WEIGHT: u32 = TXIN_BASE_WEIGHT + TR_SATISFACTION_WEIGHT;
-# let base_tx = Transaction {
-#     input: vec![],
-#     // include your recipient outputs here
-#     output: vec![],
-#     lock_time: bitcoin::absolute::LockTime::from_height(0).unwrap(),
-#     version: 1,
-# };
-# let base_weight = base_tx.weight().to_wu() as u32;
+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};
+const TR_SATISFACTION_WEIGHT: u32 = 66;
+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 =
@@ -114,12 +107,12 @@ println!("drain output weight: {}", drain_output_weight);
 
 let drain_weights = DrainWeights {
     output_weight: drain_output_weight,
-    spend_weight: TR_INPUT_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 = min_value(
+let change_policy = ChangePolicy::min_value(
     drain_weights,
     drain_addr.script_pubkey().dust_value().to_sat(),
 );
@@ -136,14 +129,13 @@ Built-in metrics are provided in the [`metrics`] submodule. Currently, only the
 [`LowestFee`](metrics::LowestFee) metric is considered stable.
 
 ```rust
-use bdk_coin_select::{ Candidate, CoinSelector, FeeRate, Target };
+use bdk_coin_select::{ Candidate, CoinSelector, FeeRate, Target, ChangePolicy };
 use bdk_coin_select::metrics::LowestFee;
-use bdk_coin_select::change_policy::min_value_and_waste;
-# let candidates = [];
-# let base_weight = 0;
-# let drain_weights = bdk_coin_select::DrainWeights::default();
-# let dust_limit = 0;
-# let long_term_feerate = FeeRate::default_min_relay_fee();
+let candidates = [];
+let base_weight = 0;
+let drain_weights = bdk_coin_select::DrainWeights::default();
+let dust_limit = 0;
+let long_term_feerate = FeeRate::default_min_relay_fee();
 
 let mut coin_selector = CoinSelector::new(&candidates, base_weight);
 
@@ -156,9 +148,10 @@ let target = Target {
 // 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`.
-let change_policy = min_value_and_waste(
+let change_policy = ChangePolicy::min_value_and_waste(
     drain_weights,
     dust_limit,
+    target.feerate,
     long_term_feerate,
 );
 
@@ -168,7 +161,7 @@ let change_policy = min_value_and_waste(
 let metric = LowestFee {
     target,
     long_term_feerate,
-    change_policy: &change_policy,
+    change_policy
 };
 
 // We run the branch and bound algorithm with a max round limit of 100,000.
@@ -180,10 +173,10 @@ match coin_selector.run_bnb(metric, 100_000) {
         let selection = coin_selector
             .apply_selection(&candidates)
             .collect::<Vec<_>>();
-        let change = change_policy(&coin_selector, target);
+        let change = coin_selector.drain(target, change_policy);
 
         println!("we selected {} inputs", selection.len());
-        println!("are we including the change output? {}", change.is_some());
+        println!("We are including a change output of {} value (0 means not change)", change.value);
     }
 };
 ```
@@ -199,26 +192,34 @@ match coin_selector.run_bnb(metric, 100_000) {
 [`Target`]: crate::Target
 
 ```rust
-use bdk_coin_select::{ CoinSelector, Candidate, DrainWeights, Target };
-use bdk_coin_select::change_policy::min_value;
-use bitcoin::{ Amount, ScriptBuf, TxOut };
-# let base_weight = 0_u32;
-# let drain_weights = DrainWeights::new_tr_keyspend();
+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::new_tr_keyspend();
+use core::str::FromStr;
 
 // A random target, as an example.
 let target = Target {
     value: 21_000,
     ..Default::default()
 };
-// A random drain policy, as an example.
-let drain_policy = min_value(drain_weights, 0);
+// 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: ScriptBuf::new(),
+        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 
@@ -228,19 +229,19 @@ let candidates = candidate_txouts
     .map(|txout| Candidate {
         input_count: 1,
         value: txout.value,
-        weight: txout.weight() as u32,
+        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);
 let _result = selector
-    .select_until_target_met(target, drain_policy(&selector, target));
+    .select_until_target_met(target,  Drain::none());
 
 // Determine what the drain output will be, based on our selection.
-let drain = drain_policy(&selector, target);
+let drain = selector.drain(target, change_policy);
 
-// Check that selection is finished!
+// In theory the target must always still be met at this point
 assert!(selector.is_target_met(target, drain));
 
 // Get a list of coins that are selected.
@@ -252,7 +253,7 @@ assert_eq!(selected_coins.len(), 1);
 
 # Minimum Supported Rust Version (MSRV)
 
-This library should compile with Rust 1.54.0.
+This library is tested to compile on 1.54
 
 To build with the MSRV, you will need to pin the following dependencies: