From 8e7e46a52c74fbd2c14f3647e5395c5737153fb1 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 23 Nov 2022 11:46:25 +0100 Subject: [PATCH 1/3] bitcoind: detect spent coins for send-to-self transactions We weren't specifying include_change therefore those were not part of listsinceblock result and never detected as spent! --- src/bitcoin/d/mod.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 782dff46..f088f8d8 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -708,8 +708,17 @@ impl BitcoinD { // Now we can get all transactions related to us since the spent transaction confirmed. // We'll use it to locate the spender. - let lsb_res = - self.make_wallet_request("listsinceblock", ¶ms!(Json::String(block_hash))); + // TODO: merge this with the existing list_since_block method. + let lsb_res = self.make_wallet_request( + "listsinceblock", + ¶ms!( + Json::String(block_hash), + Json::Number(1.into()), // Default for min_confirmations for the returned + Json::Bool(true), // Whether to include watchonly + Json::Bool(false), // Whether to include an array of txs that were removed in reorgs + Json::Bool(true) // Whether to include UTxOs treated as change. + ), + ); let transactions = lsb_res .get("transactions") .and_then(Json::as_array) From 478707df27f411c1ca983def07b6f804369a6e82 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 23 Nov 2022 11:48:47 +0100 Subject: [PATCH 2/3] bitcoind: detect coins as spent even if they are unconfirmed --- src/bitcoin/d/mod.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index f088f8d8..9394adb9 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -683,20 +683,19 @@ impl BitcoinD { /// and iterating through each of those to check if it spends the transaction we are interested /// in (requiring an other RPC call for each!!). pub fn get_spender_txid(&self, spent_outpoint: &bitcoin::OutPoint) -> Option { - // Get the hash of the block parent of the spent transaction's block. + // Get the hash of the spent transaction's block parent. If the spent transaction is still + // unconfirmed, just use the tip. let req = self.make_wallet_request( "gettransaction", ¶ms!(Json::String(spent_outpoint.txid.to_string())), ); - let spent_tx_height = match req.get("blockheight").and_then(Json::as_i64) { - Some(h) => h, - // FIXME: we assume it's confirmed. If we were to change the logic in the poller, we'd - // need to handle it here. - None => return None, + let list_since_height = match req.get("blockheight").and_then(Json::as_i64) { + Some(h) => h as i32, + None => self.chain_tip().height, }; let block_hash = if let Ok(res) = self.make_fallible_node_request( "getblockhash", - ¶ms!(Json::Number((spent_tx_height - 1).into())), + ¶ms!(Json::Number((list_since_height - 1).into())), ) { res.as_str() .expect("'getblockhash' result isn't a string") From 5721ab0fdeb5927b4a53a90d4820e30d3dcfea6a Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 23 Nov 2022 11:42:36 +0100 Subject: [PATCH 3/3] qa: test spent coins are always marked as such --- tests/test_spend.py | 119 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 1 deletion(-) diff --git a/tests/test_spend.py b/tests/test_spend.py index 4458f6a0..2d9959ff 100644 --- a/tests/test_spend.py +++ b/tests/test_spend.py @@ -1,6 +1,6 @@ from fixtures import * from test_framework.serializations import PSBT -from test_framework.utils import wait_for +from test_framework.utils import wait_for, COIN def test_spend_change(lianad, bitcoind): @@ -53,3 +53,120 @@ def test_spend_change(lianad, bitcoind): spend_txid = signed_psbt.tx.txid().hex() lianad.rpc.broadcastspend(spend_txid) bitcoind.generate_block(1, wait_for_mempool=spend_txid) + + +def test_coin_marked_spent(lianad, bitcoind): + """Test a spent coin is marked as such under various conditions.""" + # Receive a coin in a single transaction + addr = lianad.rpc.getnewaddress()["address"] + deposit_a = bitcoind.rpc.sendtoaddress(addr, 0.01) + bitcoind.generate_block(1, wait_for_mempool=deposit_a) + wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 1) + + # Receive another coin on the same address + deposit_b = bitcoind.rpc.sendtoaddress(addr, 0.02) + bitcoind.generate_block(1, wait_for_mempool=deposit_b) + wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 2) + + # Receive three coins in a single deposit transaction + destinations = { + lianad.rpc.getnewaddress()["address"]: 0.03, + lianad.rpc.getnewaddress()["address"]: 0.04, + lianad.rpc.getnewaddress()["address"]: 0.05, + } + deposit_c = bitcoind.rpc.sendmany("", destinations) + bitcoind.generate_block(1, wait_for_mempool=deposit_c) + wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 5) + + # Receive a coin in an unconfirmed deposit transaction + addr = lianad.rpc.getnewaddress()["address"] + deposit_d = bitcoind.rpc.sendtoaddress(addr, 0.06) + wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 5) + + def sign_and_broadcast(psbt): + txid = psbt.tx.txid().hex() + psbt = lianad.sign_psbt(psbt) + lianad.rpc.updatespend(psbt.to_base64()) + lianad.rpc.broadcastspend(txid) + return txid + + # Spend the first coin with a change output + outpoint = next( + c["outpoint"] + for c in lianad.rpc.listcoins()["coins"] + if deposit_a in c["outpoint"] + ) + destinations = { + bitcoind.rpc.getnewaddress(): 500_000, + } + res = lianad.rpc.createspend(destinations, [outpoint], 6) + psbt = PSBT.from_base64(res["psbt"]) + sign_and_broadcast(psbt) + + # Spend the second coin without a change output + outpoint = next( + c["outpoint"] + for c in lianad.rpc.listcoins()["coins"] + if deposit_b in c["outpoint"] + ) + destinations = { + bitcoind.rpc.getnewaddress(): int(0.02 * COIN) - 1_000, + } + res = lianad.rpc.createspend(destinations, [outpoint], 1) + psbt = PSBT.from_base64(res["psbt"]) + sign_and_broadcast(psbt) + + # Spend the third coin to an address of ours, no change + outpoints = [ + c["outpoint"] + for c in lianad.rpc.listcoins()["coins"] + if deposit_c in c["outpoint"] + ] + destinations = { + lianad.rpc.getnewaddress()["address"]: int(0.03 * COIN) - 1_000, + } + res = lianad.rpc.createspend(destinations, [outpoints[0]], 1) + psbt = PSBT.from_base64(res["psbt"]) + sign_and_broadcast(psbt) + + # Spend the fourth coin to an address of ours, with change + destinations = { + lianad.rpc.getnewaddress()["address"]: int(0.04 * COIN / 2), + } + res = lianad.rpc.createspend(destinations, [outpoints[1]], 18) + psbt = PSBT.from_base64(res["psbt"]) + sign_and_broadcast(psbt) + + # Batch spend the fourth and fifth coins + outpoint = next( + c["outpoint"] + for c in lianad.rpc.listcoins()["coins"] + if deposit_d in c["outpoint"] + ) + destinations = { + lianad.rpc.getnewaddress()["address"]: int(0.01 * COIN), + lianad.rpc.getnewaddress()["address"]: int(0.01 * COIN), + bitcoind.rpc.getnewaddress(): int(0.01 * COIN), + } + res = lianad.rpc.createspend(destinations, [outpoints[2], outpoint], 2) + psbt = PSBT.from_base64(res["psbt"]) + sign_and_broadcast(psbt) + + # All the spent coins must have been detected as such + all_deposits = (deposit_a, deposit_b, deposit_c, deposit_d) + + def deposited_coins(): + return ( + c + for c in lianad.rpc.listcoins()["coins"] + if c["outpoint"][:-2] in all_deposits + ) + + def is_spent(coin): + if coin["spend_info"] is None: + return False + if coin["spend_info"]["txid"] is None: + return False + return True + + wait_for(lambda: all(is_spent(c) for c in deposited_coins()))