From d5bd10add8270c1e0aae1855e6f8a96649323e24 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sat, 1 Oct 2022 13:35:19 +0200 Subject: [PATCH] commands: add a change_index field to listspendtxs entries --- doc/API.md | 7 ++++--- src/commands/mod.rs | 10 +++++++--- src/commands/utils.rs | 24 ++++++++++++++++++++++++ tests/test_rpc.py | 23 ++++++++++++++--------- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/doc/API.md b/doc/API.md index 15994554..6946a928 100644 --- a/doc/API.md +++ b/doc/API.md @@ -152,6 +152,7 @@ This command does not take any parameter for now. ##### Spend tx entry -| Field | Type | Description | -| ------------- | ----------------- | ----------------------------------------------------------- | -| `psbt` | string | Base64-encoded PSBT of the Spend transaction. | +| Field | Type | Description | +| -------------- | ----------------- | ----------------------------------------------------------------------- | +| `psbt` | string | Base64-encoded PSBT of the Spend transaction. | +| `change_index` | int or null | Index of the change output in the transaction outputs, if there is one. | diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 94380a95..a7249f83 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -9,7 +9,7 @@ use crate::{ database::{Coin, DatabaseInterface}, descriptors, DaemonControl, VERSION, }; -use utils::{deser_amount_from_sats, deser_psbt_base64, ser_amount, ser_base64}; +use utils::{change_index, deser_amount_from_sats, deser_psbt_base64, ser_amount, ser_base64}; use std::{ collections::{BTreeMap, HashMap}, @@ -287,7 +287,7 @@ impl DaemonControl { script_pubkey: address.script_pubkey(), }); // TODO: if it's an address of ours, signal it as change to signing devices by adding - // the BIP32 derivation path to the PSBT input. + // the BIP32 derivation path to the PSBT output. psbt_outs.push(PsbtOut::default()); } @@ -424,7 +424,10 @@ impl DaemonControl { let spend_txs = db_conn .list_spend() .into_iter() - .map(|psbt| ListSpendEntry { psbt }) + .map(|psbt| { + let change_index = change_index(&psbt).map(|i| i.try_into().expect("insane usize")); + ListSpendEntry { psbt, change_index } + }) .collect(); ListSpendResult { spend_txs } } @@ -476,6 +479,7 @@ pub struct CreateSpendResult { pub struct ListSpendEntry { #[serde(serialize_with = "ser_base64", deserialize_with = "deser_psbt_base64")] pub psbt: Psbt, + pub change_index: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/commands/utils.rs b/src/commands/utils.rs index a34975b3..90dbc398 100644 --- a/src/commands/utils.rs +++ b/src/commands/utils.rs @@ -32,3 +32,27 @@ where let psbt = consensus::deserialize(&s).map_err(de::Error::custom)?; Ok(psbt) } + +// Utility to gather the index of a change output in a Psbt, if there is one. +// FIXME: this is temporary! This is based on create_spend's behaviour that reuses the +// first coin address and doesn't shuffle the outputs! +pub fn change_index(psbt: &Psbt) -> Option { + // We always set the witness UTxO in the PSBTs we create. + let first_coin_spk = match psbt.inputs[0] + .witness_utxo + .as_ref() + .map(|o| &o.script_pubkey) + { + Some(spk) => spk, + None => return None, + }; + + let tx = &psbt.global.unsigned_tx; + for i in (0..tx.output.len()).rev() { + if &tx.output[i].script_pubkey == first_coin_spk { + return Some(i); + } + } + + None +} diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 9d8ac87e..df69060f 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -96,23 +96,26 @@ def test_create_spend(minisafed, bitcoind): def test_list_spend(minisafed, bitcoind): - # Start by creating two conflicting Spend PSBTs + # Start by creating two conflicting Spend PSBTs. The first one will have a change + # output but not the second one. addr = minisafed.rpc.getnewaddress()["address"] - bitcoind.rpc.sendtoaddress(addr, 0.2567) + value_a = 0.2567 + bitcoind.rpc.sendtoaddress(addr, value_a) wait_for(lambda: len(minisafed.rpc.listcoins()["coins"]) == 1) outpoints = [c["outpoint"] for c in minisafed.rpc.listcoins()["coins"]] destinations = { - bitcoind.rpc.getnewaddress(): 200_000, + bitcoind.rpc.getnewaddress(): int(value_a * COIN // 2), } res = minisafed.rpc.createspend(outpoints, destinations, 6) assert "psbt" in res addr = minisafed.rpc.getnewaddress()["address"] - bitcoind.rpc.sendtoaddress(addr, 0.0987) + value_b = 0.0987 + bitcoind.rpc.sendtoaddress(addr, value_b) wait_for(lambda: len(minisafed.rpc.listcoins()["coins"]) == 2) outpoints = [c["outpoint"] for c in minisafed.rpc.listcoins()["coins"]] destinations = { - bitcoind.rpc.getnewaddress(): 400_000, + bitcoind.rpc.getnewaddress(): int((value_a + value_b) * COIN - 1_000), } res_b = minisafed.rpc.createspend(outpoints, destinations, 2) assert "psbt" in res_b @@ -122,12 +125,14 @@ def test_list_spend(minisafed, bitcoind): minisafed.rpc.updatespend(res["psbt"]) minisafed.rpc.updatespend(res_b["psbt"]) - # Listing all Spend transactions will list them both. + # Listing all Spend transactions will list them both. It'll tell us which one has + # change and which one doesn't. list_res = minisafed.rpc.listspendtxs()["spend_txs"] assert len(list_res) == 2 - all_psbts = [entry["psbt"] for entry in list_res] - assert res["psbt"] in all_psbts - assert res_b["psbt"] in all_psbts + first_psbt = next(entry for entry in list_res if entry["psbt"] == res["psbt"]) + assert first_psbt["change_index"] == 1 + second_psbt = next(entry for entry in list_res if entry["psbt"] == res_b["psbt"]) + assert second_psbt["change_index"] is None def test_update_spend(minisafed, bitcoind):