From 117171f24ff5bd6731d9e5e50e4515a03013a9eb Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 24 Oct 2022 13:13:22 +0200 Subject: [PATCH] commands: use a separate key chain for change addresses --- src/commands/mod.rs | 37 +++++++++++++++++------------- tests/test_spend.py | 55 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 15 deletions(-) create mode 100644 tests/test_spend.py diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 6f0e9a9a..da16d6c5 100644 --- a/src/commands/mod.rs +++ b/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: &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(); diff --git a/tests/test_spend.py b/tests/test_spend.py new file mode 100644 index 00000000..508d9c06 --- /dev/null +++ b/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)