commands: use a separate key chain for change addresses

src/commands/mod.rs

@@ -20,7 +20,6 @@ use std::{
 use miniscript::{
     bitcoin::{
         self,
-        util::bip32,
         util::psbt::{Input as PsbtIn, Output as PsbtOut, PartiallySignedTransaction as Psbt},
     },
     psbt::PsbtExt,
@@ -170,12 +169,14 @@ fn serializable_size<T: bitcoin::consensus::Encodable + ?Sized>(t: &T) -> u64 {
 }
 
 impl DaemonControl {
-    // Get the descriptor at this derivation index
-    fn derived_desc(&self, index: bip32::ChildNumber) -> descriptors::DerivedInheritanceDescriptor {
-        self.config
-            .main_descriptor
-            .receive_descriptor()
-            .derive(index, &self.secp)
+    // Get the derived descriptor for this coin
+    fn derived_desc(&self, coin: &Coin) -> descriptors::DerivedInheritanceDescriptor {
+        let desc = if coin.is_change {
+            self.config.main_descriptor.change_descriptor()
+        } else {
+            self.config.main_descriptor.receive_descriptor()
+        };
+        desc.derive(coin.derivation_index, &self.secp)
     }
 }
 
@@ -204,7 +205,10 @@ impl DaemonControl {
         // TODO: should we wrap around instead of failing?
         db_conn.increment_receive_index(&self.secp);
         let address = self
-            .derived_desc(index)
+            .config
+            .main_descriptor
+            .receive_descriptor()
+            .derive(index, &self.secp)
             .address(self.config.bitcoin_config.network);
         GetAddressResult { address }
     }
@@ -279,7 +283,7 @@ impl DaemonControl {
                 ..bitcoin::TxIn::default()
             });
 
-            let coin_desc = self.derived_desc(coin.derivation_index);
+            let coin_desc = self.derived_desc(coin);
             sat_vb += desc_sat_vb(&coin_desc);
             let witness_script = Some(coin_desc.witness_script());
             let witness_utxo = Some(bitcoin::TxOut {
@@ -341,14 +345,15 @@ impl DaemonControl {
         // an added output* (for the change).
         if nochange_feerate_vb > feerate_vb {
             // Get the change address to create a dummy change txo.
-            // TODO: decent change management
-            let first_coin = coins
-                .get(coins_outpoints.get(0).expect("We checked it wasn't empty"))
-                .expect("We checked they were all present");
-            let coin_desc = self.derived_desc(first_coin.derivation_index);
+            let change_desc = self
+                .config
+                .main_descriptor
+                .receive_descriptor()
+                .derive(db_conn.change_index(), &self.secp);
+            db_conn.increment_change_index(&self.secp);
             let mut change_txo = bitcoin::TxOut {
                 value: std::u64::MAX,
-                script_pubkey: coin_desc.script_pubkey(),
+                script_pubkey: change_desc.script_pubkey(),
             };
             // Serialized size is equal to the virtual size for an output.
             let change_vb: u64 = serializable_size(&change_txo);
@@ -555,6 +560,8 @@ mod tests {
     use crate::testutils::*;
     use std::str::FromStr;
 
+    use bitcoin::util::bip32;
+
     #[test]
     fn getinfo() {
         let ms = DummyMinisafe::new();

tests/test_spend.py

@@ -0,0 +1,55 @@
+from fixtures import *
+from test_framework.serializations import PSBT
+from test_framework.utils import wait_for
+
+
+def test_spend_change(minisafed, bitcoind):
+    """We can spend a coin that was received on a change address."""
+    # Receive a coin on a receive address
+    addr = minisafed.rpc.getnewaddress()["address"]
+    txid = bitcoind.rpc.sendtoaddress(addr, 0.01)
+    bitcoind.generate_block(1, wait_for_mempool=txid)
+    wait_for(lambda: len(minisafed.rpc.listcoins()["coins"]) == 1)
+
+    # Create a transaction that will spend this coin to 1) one of our receive
+    # addresses 2) an external address 3) one of our change addresses.
+    outpoints = [c["outpoint"] for c in minisafed.rpc.listcoins()["coins"]]
+    destinations = {
+        bitcoind.rpc.getnewaddress(): 100_000,
+        minisafed.rpc.getnewaddress()["address"]: 100_000,
+    }
+    res = minisafed.rpc.createspend(outpoints, destinations, 2)
+    assert "psbt" in res
+
+    # The transaction must contain a change output.
+    spend_psbt = PSBT.from_base64(res["psbt"])
+    assert len(spend_psbt.o) == 3
+    assert len(spend_psbt.tx.vout) == 3
+
+    # Sign and broadcast this first Spend transaction.
+    signed_psbt = minisafed.sign_psbt(spend_psbt)
+    minisafed.rpc.updatespend(signed_psbt.to_base64())
+    spend_txid = signed_psbt.tx.txid().hex()
+    minisafed.rpc.broadcastspend(spend_txid)
+    bitcoind.generate_block(1, wait_for_mempool=spend_txid)
+    wait_for(lambda: len(minisafed.rpc.listcoins()["coins"]) == 3)
+
+    # Now create a new transaction that spends the change output as well as
+    # the output sent to the receive address.
+    outpoints = [
+        c["outpoint"]
+        for c in minisafed.rpc.listcoins()["coins"]
+        if c["spend_info"] is None
+    ]
+    destinations = {
+        bitcoind.rpc.getnewaddress(): 100_000,
+    }
+    res = minisafed.rpc.createspend(outpoints, destinations, 2)
+    spend_psbt = PSBT.from_base64(res["psbt"])
+
+    # We can sign and broadcast it.
+    signed_psbt = minisafed.sign_psbt(spend_psbt)
+    minisafed.rpc.updatespend(signed_psbt.to_base64())
+    spend_txid = signed_psbt.tx.txid().hex()
+    minisafed.rpc.broadcastspend(spend_txid)
+    bitcoind.generate_block(1, wait_for_mempool=spend_txid)