Merge #793

793: Lock used UTXOs r=thomaseizinger a=thomaseizinger

Fixes #737.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

Cargo.lock

@@ -1441,7 +1441,7 @@ dependencies = [
 [[package]]
 name = "maia"
 version = "0.1.0"
-source = "git+https://github.com/comit-network/maia#4dd8adaf88f4c9ec244412319ac5e4ed67b509ec"
+source = "git+https://github.com/comit-network/maia#5e6d44882ced923c19f45c55a825b961c9b1e396"
 dependencies = [
  "anyhow",
  "bdk",

daemon/src/bdk_ext.rs

@@ -0,0 +1,34 @@
+use anyhow::Result;
+use bdk::bitcoin::util::bip32::ExtendedPrivKey;
+use bdk::bitcoin::{self, Amount, Network};
+use rand::{CryptoRng, RngCore};
+
+pub fn new_test_wallet(
+    rng: &mut (impl RngCore + CryptoRng),
+    utxo_amount: Amount,
+    num_utxos: u8,
+) -> Result<bdk::Wallet<(), bdk::database::MemoryDatabase>> {
+    use bdk::{populate_test_db, testutils};
+
+    let mut seed = [0u8; 32];
+    rng.fill_bytes(&mut seed);
+
+    let key = ExtendedPrivKey::new_master(Network::Regtest, &seed)?;
+    let descriptors = testutils!(@descriptors (&format!("wpkh({}/*)", key)));
+
+    let mut database = bdk::database::MemoryDatabase::new();
+
+    for index in 0..num_utxos {
+        populate_test_db!(
+            &mut database,
+            testutils! {
+                @tx ( (@external descriptors, index as u32) => utxo_amount.as_sat() ) (@confirmations 1)
+            },
+            Some(100)
+        );
+    }
+
+    let wallet = bdk::Wallet::new_offline(&descriptors.0, None, Network::Regtest, database)?;
+
+    Ok(wallet)
+}

daemon/src/lib.rs

@@ -23,6 +23,7 @@ pub mod sqlx_ext; // Must come first because it is a macro.
 pub mod actors;
 pub mod address_map;
 pub mod auth;
+pub mod bdk_ext;
 pub mod bitmex_price_feed;
 pub mod cfd_actors;
 pub mod collab_settlement_taker;

daemon/src/wallet.rs

@@ -3,13 +3,16 @@ use anyhow::{bail, Context, Result};
 use bdk::bitcoin::consensus::encode::serialize_hex;
 use bdk::bitcoin::util::bip32::ExtendedPrivKey;
 use bdk::bitcoin::util::psbt::PartiallySignedTransaction;
-use bdk::bitcoin::{Address, Amount, PublicKey, Script, Transaction, Txid};
+use bdk::bitcoin::{Address, Amount, OutPoint, PublicKey, Script, Transaction, Txid};
 use bdk::blockchain::{ElectrumBlockchain, NoopProgress};
+use bdk::database::BatchDatabase;
+use bdk::wallet::tx_builder::TxOrdering;
 use bdk::wallet::AddressIndex;
 use bdk::{electrum_client, FeeRate, KeychainKind, SignOptions};
-use maia::{PartyParams, WalletExt};
+use maia::{PartyParams, TxBuilderExt};
 use rocket::serde::json::Value;
 
+use std::collections::HashSet;
 use std::path::Path;
 use xtra_productivity::xtra_productivity;
 
@@ -17,6 +20,7 @@ const DUST_AMOUNT: u64 = 546;
 
 pub struct Actor {
     wallet: bdk::Wallet<ElectrumBlockchain, bdk::database::SqliteDatabase>,
+    used_utxos: HashSet<OutPoint>,
 }
 
 #[derive(thiserror::Error, Debug, Clone, Copy)]
@@ -42,7 +46,10 @@ impl Actor {
             ElectrumBlockchain::from(client),
         )?;
 
-        Ok(Self { wallet })
+        Ok(Self {
+            wallet,
+            used_utxos: HashSet::default(),
+        })
     }
 
     /// Calculates the maximum "giveable" amount of this wallet.
@@ -63,6 +70,7 @@ impl Actor {
         let dummy_script = Script::from(vec![0u8; locking_script_size]);
         tx_builder.drain_to(dummy_script);
         tx_builder.fee_rate(fee_rate);
+        tx_builder.unspendable(self.used_utxos.iter().copied().collect());
         tx_builder.drain_wallet();
 
         let response = tx_builder.finish();
@@ -123,7 +131,14 @@ impl Actor {
             identity_pk,
         }: BuildPartyParams,
     ) -> Result<PartyParams> {
-        self.wallet.build_party_params(amount, identity_pk)
+        let psbt = self.wallet.build_lock_tx(amount, &mut self.used_utxos)?;
+
+        Ok(PartyParams {
+            lock_psbt: psbt,
+            identity_pk,
+            lock_amount: amount,
+            address: self.wallet.get_address(AddressIndex::New)?.address,
+        })
     }
 
     pub fn handle_try_broadcast_transaction(
@@ -258,9 +273,55 @@ impl From<RpcErrorCode> for i64 {
     }
 }
 
+/// Module private trait to faciliate testing.
+///
+/// Implementing this generically on `bdk::Wallet` allows us to call it on a dummy wallet in the
+/// test.
+trait BuildLockTx {
+    fn build_lock_tx(
+        &mut self,
+        amount: Amount,
+        used_utxos: &mut HashSet<OutPoint>,
+    ) -> Result<PartiallySignedTransaction>;
+}
+
+impl<B, D> BuildLockTx for bdk::Wallet<B, D>
+where
+    D: BatchDatabase,
+{
+    fn build_lock_tx(
+        &mut self,
+        amount: Amount,
+        used_utxos: &mut HashSet<OutPoint>,
+    ) -> Result<PartiallySignedTransaction> {
+        let mut builder = self.build_tx();
+
+        builder
+            .ordering(TxOrdering::Bip69Lexicographic) // TODO: I think this is pointless but we did this in maia.
+            .fee_rate(FeeRate::from_sat_per_vb(1.0))
+            .unspendable(used_utxos.iter().copied().collect())
+            .add_2of2_multisig_recipient(amount);
+
+        let (psbt, _) = builder.finish()?;
+
+        let used_inputs = psbt
+            .global
+            .unsigned_tx
+            .input
+            .iter()
+            .map(|input| input.previous_output);
+        used_utxos.extend(used_inputs);
+
+        Ok(psbt)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::bdk_ext::new_test_wallet;
+    use rand::thread_rng;
+    use std::collections::HashSet;
 
     #[test]
     fn parse_error_response() {
@@ -270,4 +331,43 @@ mod tests {
 
         assert_eq!(code, -27);
     }
+
+    #[test]
+    fn creating_two_lock_transactions_uses_different_utxos() {
+        let mut wallet = new_test_wallet(&mut thread_rng(), Amount::from_sat(1000), 10).unwrap();
+        let mut used_utxos = HashSet::new();
+
+        let lock_tx_1 = wallet
+            .build_lock_tx(Amount::from_sat(2500), &mut used_utxos)
+            .unwrap();
+        let lock_tx_2 = wallet
+            .build_lock_tx(Amount::from_sat(2500), &mut used_utxos)
+            .unwrap();
+
+        let mut utxos_in_transaction = HashSet::new();
+        utxos_in_transaction.extend(
+            lock_tx_1
+                .global
+                .unsigned_tx
+                .input
+                .iter()
+                .map(|i| i.previous_output),
+        );
+        utxos_in_transaction.extend(
+            lock_tx_2
+                .global
+                .unsigned_tx
+                .input
+                .iter()
+                .map(|i| i.previous_output),
+        );
+
+        // 2 TX a 2500 sats with UTXOs worth 1000s = 6 inputs
+        // If there are 6 UTXOs in the HashSet, we know that they are all different (HashSets don't
+        // allow duplicates!)
+        let expected_num_utxos = 6;
+
+        assert_eq!(utxos_in_transaction.len(), expected_num_utxos);
+        assert_eq!(utxos_in_transaction, used_utxos);
+    }
 }

daemon/tests/harness/maia.rs

@@ -1,42 +1,7 @@
-use anyhow::Result;
-use bdk::bitcoin;
-use bdk::bitcoin::util::bip32::ExtendedPrivKey;
-use bdk::bitcoin::{Amount, Network};
-use maia::secp256k1_zkp::rand::{CryptoRng, RngCore};
 use maia::secp256k1_zkp::{schnorrsig, SecretKey};
 use maia::Announcement;
 use std::str::FromStr;
 
-pub fn dummy_wallet(
-    rng: &mut (impl RngCore + CryptoRng),
-    utxo_amount: Amount,
-    num_utxos: u8,
-) -> Result<bdk::Wallet<(), bdk::database::MemoryDatabase>> {
-    use bdk::{populate_test_db, testutils};
-
-    let mut seed = [0u8; 32];
-    rng.fill_bytes(&mut seed);
-
-    let key = ExtendedPrivKey::new_master(Network::Regtest, &seed)?;
-    let descriptors = testutils!(@descriptors (&format!("wpkh({}/*)", key)));
-
-    let mut database = bdk::database::MemoryDatabase::new();
-
-    for index in 0..num_utxos {
-        populate_test_db!(
-            &mut database,
-            testutils! {
-                @tx ( (@external descriptors, index as u32) => utxo_amount.as_sat() ) (@confirmations 1)
-            },
-            Some(100)
-        );
-    }
-
-    let wallet = bdk::Wallet::new_offline(&descriptors.0, None, Network::Regtest, database)?;
-
-    Ok(wallet)
-}
-
 #[allow(dead_code)]
 pub struct OliviaData {
     id: String,

daemon/tests/harness/mocks/wallet.rs

@@ -1,11 +1,14 @@
-use crate::harness::maia::dummy_wallet;
 use anyhow::Result;
 use bdk::bitcoin::util::psbt::PartiallySignedTransaction;
 use bdk::bitcoin::{ecdsa, Amount, Txid};
+use bdk::wallet::tx_builder::TxOrdering;
+use bdk::wallet::AddressIndex;
+use bdk::FeeRate;
+use daemon::bdk_ext::new_test_wallet;
 use daemon::model::{Timestamp, WalletInfo};
 use daemon::wallet;
 use maia::secp256k1_zkp::Secp256k1;
-use maia::{PartyParams, WalletExt};
+use maia::{PartyParams, TxBuilderExt};
 use mockall::*;
 use rand::thread_rng;
 use std::sync::Arc;
@@ -72,10 +75,21 @@ fn dummy_wallet_info() -> Result<WalletInfo> {
 
 pub fn build_party_params(msg: wallet::BuildPartyParams) -> Result<PartyParams> {
     let mut rng = thread_rng();
-    let wallet = dummy_wallet(&mut rng, Amount::from_btc(0.4).unwrap(), 5).unwrap();
+    let wallet = new_test_wallet(&mut rng, Amount::from_btc(0.4).unwrap(), 5).unwrap();
 
-    let party_params = wallet
-        .build_party_params(msg.amount, msg.identity_pk)
-        .unwrap();
-    Ok(party_params)
+    let mut builder = wallet.build_tx();
+
+    builder
+        .ordering(TxOrdering::Bip69Lexicographic) // TODO: I think this is pointless but we did this in maia.
+        .fee_rate(FeeRate::from_sat_per_vb(1.0))
+        .add_2of2_multisig_recipient(msg.amount);
+
+    let (psbt, _) = builder.finish()?;
+
+    Ok(PartyParams {
+        lock_psbt: psbt,
+        identity_pk: msg.identity_pk,
+        lock_amount: msg.amount,
+        address: wallet.get_address(AddressIndex::New)?.address,
+    })
 }