Merge pull request #1930 from get10101/fix/funding-amount

Smarter coin selection for DLC funding transaction

crates/ln-dlc-node/Cargo.toml

@@ -10,6 +10,7 @@ description = "A common interface for using Lightning and DLC channels side-by-s
 anyhow = { version = "1", features = ["backtrace"] }
 async-trait = "0.1.71"
 bdk = { version = "0.28.0", default-features = false, features = ["key-value-db", "use-esplora-blocking", "std"] }
+bdk_coin_select = "0.2.0"
 bip39 = { version = "2", features = ["rand_core"] }
 bitcoin = "0.29.2"
 dlc = { version = "0.4.0" }

crates/ln-dlc-node/src/ldk_node_wallet.rs

@@ -15,22 +15,28 @@ use bdk::FeeRate;
 use bdk::SignOptions;
 use bdk::SyncOptions;
 use bdk::TransactionDetails;
+use bdk_coin_select::metrics::LowestFee;
+use bdk_coin_select::Candidate;
+use bdk_coin_select::ChangePolicy;
+use bdk_coin_select::CoinSelector;
+use bdk_coin_select::DrainWeights;
+use bdk_coin_select::Target;
 use bitcoin::consensus::encode::serialize_hex;
 use bitcoin::psbt::PartiallySignedTransaction;
 use bitcoin::Address;
 use bitcoin::Amount;
 use bitcoin::BlockHash;
-use bitcoin::Network;
 use bitcoin::OutPoint;
 use bitcoin::Script;
 use bitcoin::Transaction;
+use bitcoin::TxIn;
 use bitcoin::Txid;
+use bitcoin::VarInt;
 use dlc_manager::Utxo;
 use lightning::chain::chaininterface::BroadcasterInterface;
 use lightning::chain::chaininterface::ConfirmationTarget;
 use parking_lot::Mutex;
 use parking_lot::MutexGuard;
-use rust_bitcoin_coin_selection::select_coins;
 use std::sync::Arc;
 use std::time::Instant;
 use tokio::sync::RwLock;
@@ -50,6 +56,8 @@ where
     inner: Mutex<bdk::Wallet<D>>,
     settings: RwLock<WalletSettings>,
     fee_rate_estimator: Arc<F>,
+    // Only cleared upon restart. This means that if a locked outpoint ends up unspent, it will
+    // remain locked until the binary is restarted.
     locked_outpoints: Mutex<Vec<OutPoint>>,
     node_storage: Arc<N>,
 }
@@ -170,6 +178,10 @@ where
             .address)
     }
 
+    pub(crate) fn get_new_address(&self) -> Result<Address> {
+        Ok(self.bdk_lock().get_address(AddressIndex::New)?.address)
+    }
+
     pub fn is_mine(&self, script: &Script) -> Result<bool> {
         Ok(self.bdk_lock().is_mine(script)?)
     }
@@ -183,58 +195,121 @@ where
         Ok(utxos)
     }
 
-    pub fn get_utxos_for_amount(
+    pub fn get_utxos_for_dlc_funding_transaction(
         &self,
         amount: u64,
-        lock_utxos: bool,
-        network: Network,
+        fee_rate: Option<u64>,
+        should_lock_utxos: bool,
     ) -> Result<Vec<Utxo>> {
-        let utxos = self.get_utxos()?;
-        // get temporarily reserved utxo from in-memory storage
+        let network = {
+            let bdk = self.bdk_lock();
+            bdk.network()
+        };
+
+        let fee_rate = fee_rate.map(|fee_rate| fee_rate as f32).unwrap_or_else(|| {
+            self.get_fee_rate(ConfirmationTarget::Normal)
+                .as_sat_per_vb()
+        });
+
+        // Get temporarily reserved UTXOs from in-memory storage.
         let mut reserved_outpoints = self.locked_outpoints.lock();
 
-        // filter reserved utxos from all known utxos to not accidentally double spend and those who
-        // have actually been spent already
+        let utxos = self.get_utxos()?;
+
+        // Filter out reserved and spent UTXOs to prevent double-spending attempts.
         let utxos = utxos
             .iter()
             .filter(|utxo| !reserved_outpoints.contains(&utxo.outpoint))
             .filter(|utxo| !utxo.is_spent)
             .collect::<Vec<_>>();
 
-        let mut utxos = utxos
-            .into_iter()
-            .map(|x| UtxoWrap {
-                utxo: Utxo {
-                    tx_out: x.txout.clone(),
-                    outpoint: x.outpoint,
-                    address: Address::from_script(&x.txout.script_pubkey, network)
-                        .expect("to be a valid address"),
-                    redeem_script: Default::default(),
-                    reserved: false,
-                },
+        let candidates = utxos
+            .iter()
+            .map(|utxo| {
+                let tx_in = TxIn {
+                    previous_output: utxo.outpoint,
+                    ..Default::default()
+                };
+
+                // Inspired by `rust-bitcoin:0.30.2`.
+                let segwit_weight = {
+                    let legacy_weight = {
+                        let script_sig_size = tx_in.script_sig.len();
+                        (36 + VarInt(script_sig_size as u64).len() + script_sig_size + 4) * 4
+                    };
+
+                    legacy_weight + tx_in.witness.serialized_len()
+                };
+
+                // The 10101 wallet always generates SegWit addresses.
+                //
+                // TODO: Rework this once we use Taproot.
+                let is_witness_program = true;
+
+                Candidate::new(utxo.txout.value, segwit_weight as u32, is_witness_program)
             })
             .collect::<Vec<_>>();
 
-        // select enough utxos for our needs
-        let selected_local_utxos = select_coins(amount, 20, &mut utxos);
-        match selected_local_utxos {
-            None => Ok(vec![]),
-            Some(selected_local_utxos) => {
-                // update our temporarily reserved utxos with the selected once.
-                // note: this storage is only cleared up on a restart, meaning, if the protocol
-                // fails later on, the utxos will remain reserved
-                if lock_utxos {
-                    for utxo in selected_local_utxos.clone() {
-                        reserved_outpoints.push(utxo.utxo.outpoint);
-                    }
-                }
+        // This is a standard base weight (without inputs or change outputs) for on-chain DLCs. We
+        // assume that this value is still correct for DLC channels.
+        let funding_tx_base_weight = 212;
+
+        let target = Target {
+            feerate: bdk_coin_select::FeeRate::from_sat_per_vb(fee_rate),
+            min_fee: 0,
+            value: amount,
+        };
+
+        let mut coin_selector = CoinSelector::new(&candidates, funding_tx_base_weight);
 
-                Ok(selected_local_utxos
-                    .into_iter()
-                    .map(|utxo| utxo.utxo)
-                    .collect())
+        let dust_limit = 0;
+        let long_term_feerate = bdk_coin_select::FeeRate::default_min_relay_fee();
+
+        let change_policy = ChangePolicy::min_value_and_waste(
+            DrainWeights::default(),
+            dust_limit,
+            target.feerate,
+            long_term_feerate,
+        );
+
+        let metric = LowestFee {
+            target,
+            long_term_feerate,
+            change_policy,
+        };
+
+        coin_selector
+            .run_bnb(metric, 100_000)
+            .context("Failed to select coins")?;
+
+        debug_assert!(coin_selector.is_target_met(target));
+
+        let indices = coin_selector.selected_indices();
+
+        let mut selected_utxos: Vec<Utxo> = Vec::with_capacity(indices.len());
+        for index in indices {
+            let utxo = &utxos[*index];
+
+            let address = Address::from_script(&utxo.txout.script_pubkey, network)
+                .expect("to be a valid address");
+
+            let utxo = Utxo {
+                tx_out: utxo.txout.clone(),
+                outpoint: utxo.outpoint,
+                address,
+                redeem_script: Script::new(),
+                reserved: false,
+            };
+
+            if should_lock_utxos {
+                // Add selected UTXOs to reserve to prevent future double-spend attempts.
+                reserved_outpoints.push(utxo.outpoint);
             }
+
+            selected_utxos.push(utxo);
         }
+
+        Ok(selected_utxos)
     }
 
     /// Build the PSBT for sending funds to a given script and signs it

crates/ln-dlc-node/src/ln_dlc_wallet.rs

@@ -122,6 +122,10 @@ impl<S: TenTenOneStorage, N: Storage> LnDlcWallet<S, N> {
         self.address_cache.read().clone()
     }
 
+    pub fn new_address(&self) -> Result<Address> {
+        self.ldk_wallet().get_new_address()
+    }
+
     pub fn is_mine(&self, script: &Script) -> Result<bool> {
         self.ldk_wallet().is_mine(script)
     }
@@ -270,23 +274,20 @@ impl<S: TenTenOneStorage, N: Storage> dlc_manager::Wallet for LnDlcWallet<S, N>
         Ok(sk)
     }
 
+    // This is only used to create the funding transaction of a DLC or a DLC channel.
     fn get_utxos_for_amount(
         &self,
         amount: u64,
-        _: Option<u64>,
+        fee_rate: Option<u64>,
         lock_utxos: bool,
     ) -> Result<Vec<Utxo>, Error> {
         let utxos = self
             .ldk_wallet()
-            .get_utxos_for_amount(amount, lock_utxos, self.network)
+            .get_utxos_for_dlc_funding_transaction(amount, fee_rate, lock_utxos)
             .map_err(|error| {
-                Error::InvalidState(format!("Could not find utxos for mount {error:?}"))
+                Error::InvalidState(format!("Could not find utxos for amount: {error:?}"))
             })?;
-        if utxos.is_empty() {
-            return Err(Error::InvalidState(
-                "Not enough UTXOs for amount".to_string(),
-            ));
-        }
+
         Ok(utxos)
     }
 

crates/ln-dlc-node/src/node/wallet.rs

@@ -59,6 +59,12 @@ impl<S: TenTenOneStorage, N: Storage> Node<S, N> {
         self.wallet.unused_address()
     }
 
+    pub fn get_new_address(&self) -> Result<Address> {
+        self.wallet
+            .new_address()
+            .context("Failed to get new address")
+    }
+
     pub fn get_blockchain_height(&self) -> Result<u64> {
         self.wallet
             .get_blockchain_height()