diff --git a/doc/API.md b/doc/API.md index f126fffb..2d57697e 100644 --- a/doc/API.md +++ b/doc/API.md @@ -10,6 +10,8 @@ Commands must be sent as valid JSONRPC 2.0 requests, ending with a `\n`. | [`stop`](#stop) | Stops the minisafe daemon | | [`getinfo`](#getinfo) | Get general information about the daemon | | [`getnewaddress`](#getnewaddress) | Get a new receiving address | +| [`listspendtxs`](#listspendtxs) | List all stored Spend transactions | +| [`delspendtx`](#delspendtx) | Delete a stored Spend transaction | # Reference @@ -130,3 +132,44 @@ This command does not return anything for now. | Field | Type | Description | | -------------- | --------- | ---------------------------------------------------- | + + +### `listspendtxs` + +List stored Spend transactions. + +#### Request + +This command does not take any parameter for now. + +| Field | Type | Description | +| ------------- | ----------------- | ----------------------------------------------------------- | + +#### Response + +| Field | Type | Description | +| -------------- | ------------- | ---------------------------------------------------------------- | +| `spend_txs` | array | Array of Spend tx entries | + +##### Spend tx entry + +| 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. | + + +### `delspendtx` + +#### Request + +| Field | Type | Description | +| -------- | ------ | --------------------------------------------------- | +| `txid` | string | Hex encoded txid of the Spend transaction to delete | + +#### Response + +This command does not return anything for now. + +| Field | Type | Description | +| -------------- | --------- | ---------------------------------------------------- | diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 4a364f19..32487278 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()); } @@ -418,6 +418,24 @@ impl DaemonControl { Ok(()) } + + pub fn list_spend(&self) -> ListSpendResult { + let mut db_conn = self.db.connection(); + let spend_txs = db_conn + .list_spend() + .into_iter() + .map(|psbt| { + let change_index = change_index(&psbt).map(|i| i.try_into().expect("insane usize")); + ListSpendEntry { psbt, change_index } + }) + .collect(); + ListSpendResult { spend_txs } + } + + pub fn delete_spend(&self, txid: &bitcoin::Txid) { + let mut db_conn = self.db.connection(); + db_conn.delete_spend(txid); + } } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -462,6 +480,18 @@ pub struct CreateSpendResult { pub psbt: Psbt, } +#[derive(Debug, Clone, Serialize, Deserialize)] +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)] +pub struct ListSpendResult { + pub spend_txs: Vec, +} + #[cfg(test)] mod tests { use super::*; 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/src/database/mod.rs b/src/database/mod.rs index 2bf5214b..fc6d4c44 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -76,6 +76,12 @@ pub trait DatabaseConnection { /// Insert a new Spend transaction or replace an existing one. fn store_spend(&mut self, psbt: &Psbt); + + /// List all existing Spend transactions. + fn list_spend(&mut self) -> Vec; + + /// Delete a Spend transaction from database. + fn delete_spend(&mut self, txid: &bitcoin::Txid); } // FIXME: if possible, avoid reallocating. @@ -171,6 +177,17 @@ impl DatabaseConnection for SqliteConn { fn store_spend(&mut self, psbt: &Psbt) { self.store_spend(psbt) } + + fn list_spend(&mut self) -> Vec { + self.list_spend() + .into_iter() + .map(|db_spend| db_spend.psbt) + .collect() + } + + fn delete_spend(&mut self, txid: &bitcoin::Txid) { + self.delete_spend(txid) + } } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index 55b76171..a076ac7a 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -382,6 +382,27 @@ impl SqliteConn { }) .expect("Db must not fail"); } + + pub fn list_spend(&mut self) -> Vec { + db_query( + &mut self.conn, + "SELECT * FROM spend_transactions", + rusqlite::params![], + |row| row.try_into(), + ) + .expect("Db must not fail") + } + + pub fn delete_spend(&mut self, txid: &bitcoin::Txid) { + db_exec(&mut self.conn, |db_tx| { + db_tx.execute( + "DELETE FROM spend_transactions WHERE txid = ?1", + rusqlite::params![txid.to_vec()], + )?; + Ok(()) + }) + .expect("Db must not fail"); + } } #[cfg(test)] diff --git a/src/jsonrpc/api.rs b/src/jsonrpc/api.rs index 90f224c6..9a08ad35 100644 --- a/src/jsonrpc/api.rs +++ b/src/jsonrpc/api.rs @@ -60,6 +60,18 @@ fn update_spend(control: &DaemonControl, params: Params) -> Result Result { + let txid = params + .get(0, "txid") + .ok_or(Error::invalid_params("Missing 'txid' parameter."))? + .as_str() + .and_then(|s| bitcoin::Txid::from_str(&s).ok()) + .ok_or(Error::invalid_params("Invalid 'feerate' parameter."))?; + control.delete_spend(&txid); + + Ok(serde_json::json!({})) +} + /// Handle an incoming JSONRPC2 request. pub fn handle_request(control: &DaemonControl, req: Request) -> Result { let result = match req.method.as_str() { @@ -69,9 +81,16 @@ pub fn handle_request(control: &DaemonControl, req: Request) -> Result { + let params = req + .params + .ok_or(Error::invalid_params("Missing 'txid' parameter."))?; + delete_spend(control, params)? + } "getinfo" => serde_json::json!(&control.get_info()), "getnewaddress" => serde_json::json!(&control.get_new_address()), "listcoins" => serde_json::json!(&control.list_coins()), + "listspendtxs" => serde_json::json!(&control.list_spend()), "stop" => serde_json::json!({}), "updatespend" => { let params = req diff --git a/src/testutils.rs b/src/testutils.rs index 5cefaf93..2aaa0883 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -170,6 +170,20 @@ impl DatabaseConnection for DummyDbConn { fn spend_tx(&mut self, txid: &bitcoin::Txid) -> Option { self.db.read().unwrap().spend_txs.get(txid).cloned() } + + fn list_spend(&mut self) -> Vec { + self.db + .read() + .unwrap() + .spend_txs + .values() + .cloned() + .collect() + } + + fn delete_spend(&mut self, txid: &bitcoin::Txid) { + self.db.write().unwrap().spend_txs.remove(txid); + } } pub struct DummyMinisafe { diff --git a/tests/test_framework/serializations.py b/tests/test_framework/serializations.py index c0cdc4f5..c6961877 100644 --- a/tests/test_framework/serializations.py +++ b/tests/test_framework/serializations.py @@ -551,6 +551,11 @@ class CTransaction(object): self.sha256 = uint256_from_str(hash256(self.serialize_without_witness())) self.hash = encode(hash256(self.serialize())[::-1], "hex_codec").decode("ascii") + def txid(self): + if self.sha256 is None: + self.calc_sha256() + return ser_uint256(self.sha256)[::-1] + def is_valid(self): self.calc_sha256() for tout in self.vout: diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 84e077fa..89030860 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -95,6 +95,61 @@ def test_create_spend(minisafed, bitcoind): bitcoind.rpc.sendrawtransaction(signed_tx_hex) +def test_list_spend(minisafed, bitcoind): + # 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"] + 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(): int(value_a * COIN // 2), + } + res = minisafed.rpc.createspend(outpoints, destinations, 6) + assert "psbt" in res + + addr = minisafed.rpc.getnewaddress()["address"] + 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(): int((value_a + value_b) * COIN - 1_000), + } + res_b = minisafed.rpc.createspend(outpoints, destinations, 2) + assert "psbt" in res_b + + # Store them both in DB. + assert len(minisafed.rpc.listspendtxs()["spend_txs"]) == 0 + minisafed.rpc.updatespend(res["psbt"]) + minisafed.rpc.updatespend(res_b["psbt"]) + + # 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 + 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 + + # If we delete the first one, we'll get only the second one. + first_psbt = PSBT() + first_psbt.deserialize(res["psbt"]) + minisafed.rpc.delspendtx(first_psbt.tx.txid().hex()) + list_res = minisafed.rpc.listspendtxs()["spend_txs"] + assert len(list_res) == 1 + assert list_res[0]["psbt"] == res_b["psbt"] + + # If we delete the second one, result will be empty. + second_psbt = PSBT() + second_psbt.deserialize(res_b["psbt"]) + minisafed.rpc.delspendtx(second_psbt.tx.txid().hex()) + list_res = minisafed.rpc.listspendtxs()["spend_txs"] + assert len(list_res) == 0 + + def test_update_spend(minisafed, bitcoind): # Start by creating a Spend PSBT addr = minisafed.rpc.getnewaddress()["address"] @@ -108,7 +163,52 @@ def test_update_spend(minisafed, bitcoind): assert "psbt" in res # Now update it + assert len(minisafed.rpc.listspendtxs()["spend_txs"]) == 0 minisafed.rpc.updatespend(res["psbt"]) + list_res = minisafed.rpc.listspendtxs()["spend_txs"] + assert len(list_res) == 1 + assert list_res[0]["psbt"] == res["psbt"] - # TODO: check it's stored once we implement 'listspendtxs' - # TODO: check with added signatures once we implement 'listspendtxs' + # Keep a copy for later. + psbt_no_sig = PSBT() + psbt_no_sig.deserialize(res["psbt"]) + + # We can add a signature and update it + psbt_sig_a = PSBT() + psbt_sig_a.deserialize(res["psbt"]) + dummy_pk_a = bytes.fromhex( + "0375e00eb72e29da82b89367947f29ef34afb75e8654f6ea368e0acdfd92976b7c" + ) + dummy_sig_a = bytes.fromhex( + "304402202b925395cfeaa0171a7a92982bb4891acc4a312cbe7691d8375d36796d5b570a0220378a8ab42832848e15d1aedded5fb360fedbdd6c39226144e527f0f1e19d5398" + ) + psbt_sig_a.inputs[0].partial_sigs[dummy_pk_a] = dummy_sig_a + psbt_sig_a_ser = psbt_sig_a.serialize() + minisafed.rpc.updatespend(psbt_sig_a_ser) + + # We'll get it when querying + list_res = minisafed.rpc.listspendtxs()["spend_txs"] + assert len(list_res) == 1 + assert list_res[0]["psbt"] == psbt_sig_a_ser + + # We can add another signature to the empty PSBT and update it again + psbt_sig_b = PSBT() + psbt_sig_b.deserialize(res["psbt"]) + dummy_pk_b = bytes.fromhex( + "03a1b26313f430c4b15bb1fdce663207659d8cac749a0e53d70eff01874496feff" + ) + dummy_sig_b = bytes.fromhex( + "3044022005aebcd649fb8965f0591710fb3704931c3e8118ee60dd44917479f63ceba6d4022018b212900e5a80e9452366894de37f0d02fb9c89f1e94f34fb6ed7fd71c15c41" + ) + psbt_sig_b.inputs[0].partial_sigs[dummy_pk_b] = dummy_sig_b + psbt_sig_b_ser = psbt_sig_b.serialize() + minisafed.rpc.updatespend(psbt_sig_b_ser) + + # It will have merged both. + list_res = minisafed.rpc.listspendtxs()["spend_txs"] + assert len(list_res) == 1 + psbt_merged = PSBT() + psbt_merged.deserialize(list_res[0]["psbt"]) + assert len(psbt_merged.inputs[0].partial_sigs) == 2 + assert psbt_merged.inputs[0].partial_sigs[dummy_pk_a] == dummy_sig_a + assert psbt_merged.inputs[0].partial_sigs[dummy_pk_b] == dummy_sig_b