Merge #560: Apply coin selection for spend

cfa0f91 commands: auto-select coins if none provided (jp1ac4)

Pull request description:

  These are some initial changes towards #51.

  I've added a `selectcoinsforspend` command that applies BnB coin selection using a waste metric.

  This coin selection is then used in `createspend` if no coins are specified.

  @darosior The changes are still in their early stages so I'm creating this draft PR to facilitate discussion about the approach in general and specific details.

ACKs for top commit:
  darosior:
    ACK cfa0f91

Tree-SHA512: 2b94a8f4d335366e477fff54fa51d478ef459e2e729bac00a5d4ac21d04667409cb685642f27fd1936456a05a8d76d23483e45a24f5d342f9a26de904bb6639c

Cargo.toml

@@ -28,6 +28,8 @@ nonblocking_shutdown = []
 # For managing transactions (it re-exports the bitcoin crate)
 miniscript = { version = "10.0", features = ["serde", "compiler", "base64"] }
 
+bdk_coin_select = { git = "https://github.com/evanlinjin/bdk", branch = "new_bdk_coin_select" }
+
 # Don't reinvent the wheel
 dirs = "5.0"
 

doc/API.md

@@ -143,6 +143,9 @@ A coin may have one of the following four statuses:
 
 Create a transaction spending one or more of our coins. All coins must exist and not be spent.
 
+If no coins are specified in `outpoints`, they will be selected automatically from the set of
+confirmed coins (see [`listcoins`](#listcoins) for coin status definitions).
+
 Will error if the given coins are not sufficient to cover the transaction cost at 90% (or more) of
 the given feerate. If on the contrary the transaction is more than sufficiently funded, it will
 create a change output when economically rationale to do so.

src/commands/utils.rs

@@ -1,8 +1,17 @@
-use std::str::FromStr;
+use bdk_coin_select::{
+    change_policy, metrics::LowestFee, Candidate, CoinSelector, DrainWeights, FeeRate,
+    InsufficientFunds, Target, TXIN_BASE_WEIGHT,
+};
+use log::warn;
+use std::{convert::TryInto, str::FromStr};
 
 use miniscript::bitcoin::{self, consensus, hashes::hex::FromHex};
 use serde::{de, Deserialize, Deserializer, Serializer};
 
+use crate::database::Coin;
+
+use super::{CandidateCoin, DUST_OUTPUT_SATS, LONG_TERM_FEERATE_VB};
+
 pub fn deser_fromstr<'de, D, T>(deserializer: D) -> Result<T, D::Error>
 where
     D: Deserializer<'de>,
@@ -62,3 +71,119 @@ where
     let s = Vec::from_hex(&s).map_err(de::Error::custom)?;
     consensus::deserialize(&s).map_err(de::Error::custom)
 }
+
+/// Select coins for spend.
+///
+/// Returns the selected coins and the change amount, which could be zero.
+///
+/// `candidate_coins` are the coins to consider for selection.
+///
+/// `base_tx` is the transaction to select coins for. It should be without any inputs
+/// and without a change output, but with all non-change outputs added.
+///
+/// `change_txo` is the change output to add if needed (with any value).
+///
+/// `feerate_vb` is the minimum feerate (in sats/vb). Note that the selected coins
+/// and change may result in a slightly lower feerate than this as the underlying
+/// function instead uses a minimum feerate of `feerate_vb / 4.0` sats/wu.
+///
+/// `min_fee` is the minimum fee (in sats) that the selection must have.
+///
+/// `max_sat_weight` is the maximum size difference (in vb) of
+/// an input in the transaction before and after satisfaction.
+pub fn select_coins_for_spend(
+    candidate_coins: &[CandidateCoin],
+    base_tx: bitcoin::Transaction,
+    change_txo: bitcoin::TxOut,
+    feerate_vb: f32,
+    min_fee: u64,
+    max_sat_weight: u32,
+) -> Result<(Vec<Coin>, bitcoin::Amount), InsufficientFunds> {
+    let out_value_nochange = base_tx.output.iter().map(|o| o.value).sum();
+
+    // Create the coin selector from the given candidates. NOTE: the coin selector keeps track
+    // of the original ordering of candidates so we can select any mandatory candidates using their
+    // original indices.
+    let base_weight: u32 = base_tx
+        .weight()
+        .to_wu()
+        .try_into()
+        .expect("Transaction weight must fit in u32");
+    let max_input_weight = TXIN_BASE_WEIGHT + max_sat_weight;
+    let candidates: Vec<Candidate> = candidate_coins
+        .iter()
+        .map(|cand| Candidate {
+            input_count: 1,
+            value: cand.coin.amount.to_sat(),
+            weight: max_input_weight,
+            is_segwit: true, // We only support receiving on Segwit scripts.
+        })
+        .collect();
+    let mut selector = CoinSelector::new(&candidates, base_weight);
+    for (i, cand) in candidate_coins.iter().enumerate() {
+        if cand.must_select {
+            // It's fine because the index passed to `select` refers to the original candidates ordering
+            // (and in any case the ordering of candidates is still the same in the coin selector).
+            selector.select(i);
+        }
+    }
+
+    // Now set the change policy. We use a policy which ensures no change output is created with a
+    // lower value than our custom dust limit. NOTE: the change output weight must account for a
+    // potential difference in the size of the outputs count varint. This is why we take the whole
+    // change txo as argument and compute the weight difference below.
+    let long_term_feerate = FeeRate::from_sat_per_vb(LONG_TERM_FEERATE_VB);
+    let drain_weights = DrainWeights {
+        output_weight: {
+            let mut tx_with_change = base_tx;
+            tx_with_change.output.push(change_txo);
+            tx_with_change
+                .weight()
+                .to_wu()
+                .checked_sub(base_weight.into())
+                .expect("base_weight can't be larger")
+                .try_into()
+                .expect("tx size must always fit in u32")
+        },
+        spend_weight: max_input_weight,
+    };
+    let change_policy =
+        change_policy::min_value_and_waste(drain_weights, DUST_OUTPUT_SATS, long_term_feerate);
+
+    // Finally, run the coin selection algorithm. We use a BnB with 100k iterations and if it
+    // couldn't find any solution we fall back to selecting coins by descending value.
+    let target = Target {
+        value: out_value_nochange,
+        feerate: FeeRate::from_sat_per_vb(feerate_vb),
+        min_fee,
+    };
+    if let Err(e) = selector.run_bnb(
+        LowestFee {
+            target,
+            long_term_feerate,
+            change_policy: &change_policy,
+        },
+        100_000,
+    ) {
+        warn!(
+            "Coin selection error: '{}'. Selecting coins by descending value per weight unit...",
+            e.to_string()
+        );
+        selector.sort_candidates_by_descending_value_pwu();
+        // If more coins still need to be selected to meet target, then `change_policy(&selector, target)`
+        // will give `Drain::none()`, i.e. no change, and this will simply select more coins until
+        // they cover the target.
+        selector.select_until_target_met(target, change_policy(&selector, target))?;
+    }
+    // By now, selection is complete and we can check how much change to give according to our policy.
+    let drain = change_policy(&selector, target);
+    let change_amount = bitcoin::Amount::from_sat(drain.value);
+    Ok((
+        selector
+            .selected_indices()
+            .iter()
+            .map(|i| candidate_coins[*i].coin)
+            .collect(),
+        change_amount,
+    ))
+}

src/jsonrpc/mod.rs

@@ -151,7 +151,7 @@ impl error::Error for Error {}
 impl From<commands::CommandError> for Error {
     fn from(e: commands::CommandError) -> Error {
         match e {
-            commands::CommandError::NoOutpoint
+            commands::CommandError::NoOutpointForSelfSend
             | commands::CommandError::UnknownOutpoint(..)
             | commands::CommandError::InvalidFeerate(..)
             | commands::CommandError::AlreadySpent(..)
@@ -170,6 +170,7 @@ impl From<commands::CommandError> for Error {
             }
             commands::CommandError::FetchingTransaction(..)
             | commands::CommandError::SanityCheckFailure(_)
+            | commands::CommandError::CoinSelectionError(..)
             | commands::CommandError::RescanTrigger(..) => {
                 Error::new(ErrorCode::InternalError, e.to_string())
             }

tests/test_rpc.py

@@ -399,12 +399,13 @@ def test_create_spend(lianad, bitcoind):
     assert len(spend_psbt.o) == 4
     assert len(spend_psbt.tx.vout) == 4
 
-    # The transaction must contain the spent transaction for each input
-    spent_txs = [bitcoind.rpc.gettransaction(op[:64]) for op in outpoints]
-    for i, psbt_in in enumerate(spend_psbt.i):
-        assert psbt_in.map[PSBT_IN_NON_WITNESS_UTXO] == bytes.fromhex(
-            spent_txs[i]["hex"]
-        )
+    # The transaction must contain the spent transaction for each input.
+    # We don't make assumptions about the ordering of PSBT inputs.
+    assert sorted(
+        [psbt_in.map[PSBT_IN_NON_WITNESS_UTXO] for psbt_in in spend_psbt.i]
+    ) == sorted(
+        [bytes.fromhex(bitcoind.rpc.gettransaction(op[:64])["hex"]) for op in outpoints]
+    )
 
     # We can sign it and broadcast it.
     sign_and_broadcast(lianad, bitcoind, PSBT.from_base64(res["psbt"]))

tests/test_spend.py

@@ -219,3 +219,106 @@ def test_send_to_self(lianad, bitcoind):
         c for c in lianad.rpc.listcoins()["coins"] if c["spend_info"] is None
     )
     wait_for(lambda: len(list(unspent_coins())) == 1)
+
+
+def test_coin_selection(lianad, bitcoind):
+    """We can create a spend using coin selection."""
+    # Send to an (external) address.
+    dest_100_000 = {bitcoind.rpc.getnewaddress(): 100_000}
+    # Coin selection is not possible if we have no coins.
+    assert len(lianad.rpc.listcoins()["coins"]) == 0
+    with pytest.raises(
+        RpcError,
+        match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
+    ):
+        lianad.rpc.createspend(dest_100_000, [], 2)
+
+    # Receive a coin in an unconfirmed deposit transaction.
+    recv_addr = lianad.rpc.getnewaddress()["address"]
+    deposit = bitcoind.rpc.sendtoaddress(recv_addr, 0.0008)  # 80_000 sats
+    wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 1)
+    # There are still no confirmed coins to use as candidates for selection.
+    assert len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 0
+    assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1
+    with pytest.raises(
+        RpcError,
+        match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
+    ):
+        lianad.rpc.createspend(dest_100_000, [], 2)
+
+    # Confirm coin.
+    bitcoind.generate_block(1, wait_for_mempool=deposit)
+    wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
+
+    # Insufficient funds for coin selection.
+    with pytest.raises(
+        RpcError,
+        match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
+    ):
+        lianad.rpc.createspend(dest_100_000, [], 2)
+
+    # Reduce spend amount.
+    dest_30_000 = {bitcoind.rpc.getnewaddress(): 30_000}
+    res = lianad.rpc.createspend(dest_30_000, [], 2)
+    assert "psbt" in res
+
+    # The transaction must contain a change output.
+    spend_psbt = PSBT.from_base64(res["psbt"])
+    assert len(spend_psbt.o) == 2
+    assert len(spend_psbt.tx.vout) == 2
+
+    # Sign and broadcast this Spend transaction.
+    signed_psbt = lianad.signer.sign_psbt(spend_psbt)
+    lianad.rpc.updatespend(signed_psbt.to_base64())
+    spend_txid = signed_psbt.tx.txid().hex()
+    lianad.rpc.broadcastspend(spend_txid)
+
+    wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 2)
+    coins = lianad.rpc.listcoins()["coins"]
+    # Check that change output is unconfirmed.
+    assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1
+    assert len(lianad.rpc.listcoins(["spending"])["coins"]) == 1
+    # Check we cannot use coins as candidates if they are spending/spent or unconfirmed.
+    with pytest.raises(
+        RpcError,
+        match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
+    ):
+        lianad.rpc.createspend(dest_30_000, [], 2)
+
+    # Now confirm the Spend.
+    bitcoind.generate_block(1, wait_for_mempool=spend_txid)
+    wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
+    # But its value is not enough for this Spend.
+    dest_60_000 = {bitcoind.rpc.getnewaddress(): 60_000}
+    with pytest.raises(
+        RpcError,
+        match="Coin selection error: 'Insufficient funds. Missing \\d+ sats.'",
+    ):
+        lianad.rpc.createspend(dest_60_000, [], 2)
+
+    # Get another coin to check coin selection with more than one candidate.
+    recv_addr = lianad.rpc.getnewaddress()["address"]
+    deposit = bitcoind.rpc.sendtoaddress(recv_addr, 0.0002)  # 20_000 sats
+    bitcoind.generate_block(1, wait_for_mempool=deposit)
+    wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 2)
+
+    res = lianad.rpc.createspend(dest_60_000, [], 2)
+    assert "psbt" in res
+
+    # The transaction must contain a change output.
+    auto_psbt = PSBT.from_base64(res["psbt"])
+    assert len(auto_psbt.o) == 2
+    assert len(auto_psbt.tx.vout) == 2
+
+    # Now create a transaction with manual coin selection using the same outpoints.
+    outpoints = [
+        f"{txin.prevout.hash:064x}:{txin.prevout.n}" for txin in auto_psbt.tx.vin
+    ]
+    res_manual = lianad.rpc.createspend(dest_60_000, outpoints, 2)
+    manual_psbt = PSBT.from_base64(res_manual["psbt"])
+
+    # Recipient details are the same for both.
+    assert auto_psbt.tx.vout[0].nValue == manual_psbt.tx.vout[0].nValue
+    assert auto_psbt.tx.vout[0].scriptPubKey == manual_psbt.tx.vout[0].scriptPubKey
+    # Change amount is the same (change address will be different).
+    assert auto_psbt.tx.vout[1].nValue == manual_psbt.tx.vout[1].nValue