From 16726f5029bad5457db1f7c5c02a6146aa49577d Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Tue, 26 Nov 2024 09:31:55 +0000 Subject: [PATCH] commands: add is_from_self to listcoins response --- doc/API.md | 1 + liana-gui/src/app/state/coins.rs | 4 ++ liana-gui/src/app/state/psbt.rs | 1 + liana-gui/src/lianalite/client/backend/mod.rs | 3 + lianad/src/commands/mod.rs | 6 ++ tests/test_chain.py | 60 +++++++++++++++---- tests/test_misc.py | 20 +++++-- tests/test_rpc.py | 50 ++++++++++++++++ 8 files changed, 130 insertions(+), 15 deletions(-) diff --git a/doc/API.md b/doc/API.md index c89d206a5..0bdadcc46 100644 --- a/doc/API.md +++ b/doc/API.md @@ -135,6 +135,7 @@ A coin may have one of the following four statuses: | `spend_info` | object | Information about the transaction spending this coin. See [Spending transaction info](#spending_transaction_info). | | `is_immature` | bool | Whether this coin was created by a coinbase transaction that is still immature. | | `is_change` | bool | Whether the coin deposit address was derived from the change descriptor. | +| `is_from_self` | bool | Whether the coin and all its unconfirmed ancestors, if any, are outputs of transactions from this wallet. | ##### Spending transaction info diff --git a/liana-gui/src/app/state/coins.rs b/liana-gui/src/app/state/coins.rs index c7efb89ea..054241d16 100644 --- a/liana-gui/src/app/state/coins.rs +++ b/liana-gui/src/app/state/coins.rs @@ -226,6 +226,7 @@ mod tests { address: dummy_address.clone(), derivation_index: 0.into(), is_change: false, + is_from_self: false, }, Coin { outpoint: bitcoin::OutPoint { txid, vout: 3 }, @@ -236,6 +237,7 @@ mod tests { address: dummy_address.clone(), derivation_index: 1.into(), is_change: false, + is_from_self: false, }, Coin { outpoint: bitcoin::OutPoint { txid, vout: 0 }, @@ -246,6 +248,7 @@ mod tests { address: dummy_address.clone(), derivation_index: 2.into(), is_change: false, + is_from_self: false, }, Coin { outpoint: bitcoin::OutPoint { txid, vout: 1 }, @@ -256,6 +259,7 @@ mod tests { address: dummy_address, derivation_index: 3.into(), is_change: false, + is_from_self: false, }, ]); diff --git a/liana-gui/src/app/state/psbt.rs b/liana-gui/src/app/state/psbt.rs index b736219b9..c133865c9 100644 --- a/liana-gui/src/app/state/psbt.rs +++ b/liana-gui/src/app/state/psbt.rs @@ -793,6 +793,7 @@ mod tests { "derivation_index": 0, "is_immature": false, "is_change": false, + "is_from_self": false, }]})), ), diff --git a/liana-gui/src/lianalite/client/backend/mod.rs b/liana-gui/src/lianalite/client/backend/mod.rs index 554df5548..1118c6862 100644 --- a/liana-gui/src/lianalite/client/backend/mod.rs +++ b/liana-gui/src/lianalite/client/backend/mod.rs @@ -649,6 +649,7 @@ impl Daemon for BackendWalletClient { txid: info.txid, height: info.height, }), + is_from_self: false, // FIXME: use value from backend }) .collect(), }) @@ -1133,6 +1134,7 @@ fn history_tx_from_api(value: api::Transaction, network: Network) -> HistoryTran txid: info.txid, height: info.height, }), + is_from_self: false, // FIXME: use value from backend }); } } @@ -1188,6 +1190,7 @@ fn spend_tx_from_api( txid: info.txid, height: info.height, }), + is_from_self: false, // FIXME: use value from backend }); } } diff --git a/lianad/src/commands/mod.rs b/lianad/src/commands/mod.rs index 9afa5c2cb..1669da533 100644 --- a/lianad/src/commands/mod.rs +++ b/lianad/src/commands/mod.rs @@ -425,6 +425,7 @@ impl DaemonControl { spend_block, is_immature, is_change, + is_from_self, derivation_index, .. } = coin; @@ -445,6 +446,7 @@ impl DaemonControl { spend_info, is_immature, is_change, + is_from_self, } }) .collect(); @@ -1229,6 +1231,10 @@ pub struct ListCoinsEntry { pub is_immature: bool, /// Whether the coin deposit address was derived from the change descriptor. pub is_change: bool, + /// Whether the coin is the output of a transaction whose inputs are all from + /// this same wallet. If the coin is unconfirmed, it also means that all its + /// unconfirmed ancestors, if any, are also from self. + pub is_from_self: bool, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/tests/test_chain.py b/tests/test_chain.py index 30027bbbc..87d010828 100644 --- a/tests/test_chain.py +++ b/tests/test_chain.py @@ -75,14 +75,36 @@ def test_reorg_exclusion(lianad, bitcoind): coin_b = get_coin(lianad, txid) b_spend_tx = spend_coins(lianad, bitcoind, [coin_b]) + # These are external deposits so not from self. + assert coin_a["is_from_self"] is False + assert coin_b["is_from_self"] is False + # A confirmed and spent coin addr = lianad.rpc.getnewaddress()["address"] - txid = bitcoind.rpc.sendtoaddress(addr, 3) - bitcoind.generate_block(1, wait_for_mempool=txid) + txid_c = bitcoind.rpc.sendtoaddress(addr, 3) wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 3) - coin_c = get_coin(lianad, txid) - c_spend_tx = spend_coins(lianad, bitcoind, [coin_c]) - bitcoind.generate_block(1, wait_for_mempool=1) + # Now refresh this coin while it is unconfirmed. + res = lianad.rpc.createspend({}, [get_coin(lianad, txid_c)["outpoint"]], 1) + c_spend_psbt = PSBT.from_base64(res["psbt"]) + txid_d = sign_and_broadcast_psbt(lianad, c_spend_psbt) + wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 4) + coin_c = get_coin(lianad, txid_c) + coin_d = get_coin(lianad, txid_d) + assert coin_c["is_from_self"] is False + assert coin_c["block_height"] is None + # Even though coin_d is from a self-send, coin_c is still unconfirmed + # and is not from self. Therefore, coin_d is not from self either. + assert coin_d["is_from_self"] is False + + bitcoind.generate_block(1) + # Wait for confirmation to be detected. + wait_for(lambda: get_coin(lianad, txid_d)["block_height"] is not None) + coin_c = get_coin(lianad, txid_c) + coin_d = get_coin(lianad, txid_d) + assert coin_c["is_from_self"] is False + assert coin_c["block_height"] is not None + assert coin_d["is_from_self"] is True + assert coin_d["block_height"] is not None # Make sure the transaction were confirmed >10 blocks ago, so bitcoind won't update the # mempool during the reorg to the initial height. @@ -108,7 +130,7 @@ def test_reorg_exclusion(lianad, bitcoind): tx = bitcoind.rpc.gettransaction(txid)["hex"] bitcoind.rpc.sendrawtransaction(tx) bitcoind.rpc.sendrawtransaction(b_spend_tx) - bitcoind.rpc.sendrawtransaction(c_spend_tx) + sign_and_broadcast_psbt(lianad, c_spend_psbt) bitcoind.generate_block(1, wait_for_mempool=5) new_height = bitcoind.rpc.getblockcount() wait_for(lambda: lianad.rpc.getinfo()["block_height"] == new_height) @@ -128,9 +150,11 @@ def test_reorg_exclusion(lianad, bitcoind): for c in lianad.rpc.listcoins()["coins"] if coin_c["outpoint"] == c["outpoint"] ) - c_spend_txid = get_txid(c_spend_tx) - assert new_coin_c["spend_info"]["txid"] == c_spend_txid + assert new_coin_c["spend_info"]["txid"] == txid_d assert new_coin_c["spend_info"]["height"] == new_height + new_coin_d = get_coin(lianad, txid_d) + assert new_coin_d["is_from_self"] is True + assert new_coin_d["block_height"] == new_height # TODO: maybe test with some malleation for the deposit and spending txs? @@ -162,17 +186,26 @@ def test_reorg_status_recovery(lianad, bitcoind): assert initial_height > 100 wait_for(lambda: lianad.rpc.getinfo()["block_height"] == initial_height) - # Both coins are confirmed. Spend the second one then get their infos. + # Both coins are confirmed. Refresh the second one then get their infos. wait_for(lambda: len(list_coins()) == 2) wait_for(lambda: all(c["block_height"] is not None for c in list_coins())) coin_b = get_coin(lianad, txids[1]) - tx = spend_coins(lianad, bitcoind, [coin_b]) - locktime = bitcoind.rpc.decoderawtransaction(tx)["locktime"] + # Refresh coin_b. + res = lianad.rpc.createspend({}, [coin_b["outpoint"]], 1) + b_spend_psbt = PSBT.from_base64(res["psbt"]) + txid = sign_and_broadcast_psbt(lianad, b_spend_psbt) + coin_c = get_coin(lianad, txid) + # coin_c is unconfirmed and marked as from self as its parent is confirmed. + assert coin_c["block_height"] is None + assert coin_c["is_from_self"] is True + + locktime = b_spend_psbt.tx.nLockTime assert initial_height - 100 <= locktime <= initial_height bitcoind.generate_block(1, wait_for_mempool=1) wait_for(lambda: spend_confirmed_noticed(lianad, coin_b["outpoint"])) coin_a = get_coin(lianad, txids[0]) coin_b = get_coin(lianad, txids[1]) + coin_c = get_coin(lianad, txid) # Reorg the chain down to the initial height without shifting nor malleating # any transaction. The coin info should be identical (except the spend info @@ -187,9 +220,14 @@ def test_reorg_status_recovery(lianad, bitcoind): if locktime == initial_height: # Cannot be mined until next block (initial_height + 1). coin_b["spend_info"] = None + # coin_c no longer exists. + with pytest.raises(StopIteration): + get_coin(lianad, coin_c["outpoint"]) else: # Otherwise, the tx will be mined at the height the reorg happened. coin_b["spend_info"]["height"] = initial_height + new_coin_c = get_coin(lianad, coin_c["outpoint"]) + assert new_coin_c["is_from_self"] is True assert new_coin_b == coin_b diff --git a/tests/test_misc.py b/tests/test_misc.py index 7cd1f95bb..52f5e7a37 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -213,14 +213,22 @@ def test_coinbase_deposit(lianad, bitcoind): wait_for_sync() coins = lianad.rpc.listcoins()["coins"] assert ( - len(coins) == 1 and coins[0]["is_immature"] and coins[0]["spend_info"] is None + len(coins) == 1 + and coins[0]["is_immature"] + and coins[0]["spend_info"] is None + and not coins[0]["is_from_self"] ) # Generate 100 blocks to make the coinbase mature. We should detect it as such. + # It remains as not from self. bitcoind.generate_block(100) wait_for_sync() coin = lianad.rpc.listcoins()["coins"][0] - assert not coin["is_immature"] and coin["block_height"] is not None + assert ( + not coin["is_immature"] + and coin["block_height"] is not None + and not coins[0]["is_from_self"] + ) # We must be able to spend the mature coin. destinations = {bitcoind.rpc.getnewaddress(): int(0.999999 * COIN)} @@ -249,13 +257,17 @@ def test_coinbase_deposit(lianad, bitcoind): bitcoind.rpc.generatetoaddress(1, change_addr) wait_for(lambda: any(c["is_immature"] for c in lianad.rpc.listcoins()["coins"])) coin = next(c for c in lianad.rpc.listcoins()["coins"] if c["is_immature"]) - assert coin["is_change"] + assert coin["is_change"] and not coin["is_from_self"] bitcoind.generate_block(100) wait_for_sync() coin = next( c for c in lianad.rpc.listcoins()["coins"] if c["outpoint"] == coin["outpoint"] ) - assert not coin["is_immature"] and coin["block_height"] is not None + assert ( + not coin["is_immature"] + and coin["block_height"] is not None + and not coin["is_from_self"] + ) @pytest.mark.skipif( diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 0a118506c..92084c270 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -100,6 +100,7 @@ def test_listcoins(lianad, bitcoind): assert res[0]["is_change"] == False assert res[0]["block_height"] is None assert res[0]["spend_info"] is None + assert res[0]["is_from_self"] is False assert len(lianad.rpc.listcoins(["confirmed", "spent", "spending"])["coins"]) == 0 assert ( @@ -131,6 +132,8 @@ def test_listcoins(lianad, bitcoind): == lianad.rpc.listcoins(["spent", "unconfirmed", "confirmed"], [outpoint_a]) ) + assert lianad.rpc.listcoins()["coins"][0]["is_from_self"] is False + # Same if the coin gets spent. spend_tx = spend_coins(lianad, bitcoind, (res[0],)) spend_txid = get_txid(spend_tx) @@ -140,6 +143,7 @@ def test_listcoins(lianad, bitcoind): assert spend_info["height"] is None assert len(lianad.rpc.listcoins(["spent"])["coins"]) == 0 assert len(lianad.rpc.listcoins(["spending"])["coins"]) == 1 + assert lianad.rpc.listcoins(["spending"])["coins"][0]["is_from_self"] is False # And if this spending tx gets confirmed. bitcoind.generate_block(1, wait_for_mempool=spend_txid) @@ -154,6 +158,8 @@ def test_listcoins(lianad, bitcoind): == lianad.rpc.listcoins(["spent"]) == lianad.rpc.listcoins(["spent", "unconfirmed", "confirmed"]) ) + assert len(lianad.rpc.listcoins()["coins"]) == 1 + assert lianad.rpc.listcoins()["coins"][0]["is_from_self"] is False # Add a second coin. addr_b = lianad.rpc.getnewaddress()["address"] @@ -161,6 +167,7 @@ def test_listcoins(lianad, bitcoind): wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 2) res = lianad.rpc.listcoins(["unconfirmed"], [])["coins"] outpoint_b = res[0]["outpoint"] + assert res[0]["is_from_self"] is False # We have one unconfirmed coin and one spent coin. assert ( @@ -182,6 +189,7 @@ def test_listcoins(lianad, bitcoind): == lianad.rpc.listcoins(["spending", "unconfirmed", "confirmed"]) == lianad.rpc.listcoins(["spending", "unconfirmed", "confirmed"], [outpoint_b]) ) + assert lianad.rpc.listcoins([], [outpoint_b])["coins"][0]["is_from_self"] is False # Now confirm the second coin. bitcoind.generate_block(1, wait_for_mempool=txid_b) @@ -190,6 +198,7 @@ def test_listcoins(lianad, bitcoind): lambda: lianad.rpc.listcoins([], [outpoint_b])["coins"][0]["block_height"] == block_height ) + assert lianad.rpc.listcoins([], [outpoint_b])["coins"][0]["is_from_self"] is False # We have one confirmed coin and one spent coin. assert ( @@ -1079,6 +1088,7 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind): bitcoind.generate_block(1, wait_for_mempool=txid) wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 3) coins = lianad.rpc.listcoins(["confirmed"])["coins"] + assert all(c["is_from_self"] is False for c in coins) # Create a spend that will later be replaced. first_outpoints = [c["outpoint"] for c in coins[:2]] @@ -1113,6 +1123,13 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind): for c in lianad.rpc.listcoins([], first_outpoints)["coins"] ) ) + # The change output is from self as its parent is confirmed. + lc_res = lianad.rpc.listcoins(["unconfirmed"], [])["coins"] + assert ( + len(lc_res) == 1 + and first_txid in lc_res[0]["outpoint"] + and lc_res[0]["is_from_self"] is True + ) # We can now use RBF, but the feerate must be higher than that of the first transaction. with pytest.raises(RpcError, match=f"Feerate 1 too low for minimum feerate 2."): lianad.rpc.rbfpsbt(first_txid, False, 1) @@ -1145,6 +1162,13 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind): for c in lianad.rpc.listcoins([], first_outpoints)["coins"] ) ) + # The change output of the replacement is also from self as its parent is confirmed. + lc_res = lianad.rpc.listcoins(["unconfirmed"], [])["coins"] + assert ( + len(lc_res) == 1 + and rbf_1_txid in lc_res[0]["outpoint"] + and lc_res[0]["is_from_self"] is True + ) mempool_rbf_1 = bitcoind.rpc.getmempoolentry(rbf_1_txid) # Note that in the mempool entry, "ancestor" includes rbf_1_txid itself. rbf_1_feerate = ( @@ -1178,6 +1202,12 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind): for c in lianad.rpc.listcoins([], desc_1_outpoints)["coins"] ) ) + lc_res = [c for c in lianad.rpc.listcoins(["unconfirmed"], [])["coins"]] + assert ( + len(lc_res) == 1 + and desc_1_txid in lc_res[0]["outpoint"] + and lc_res[0]["is_from_self"] is True + ) # Add a new transaction spending the change from the first descendant. desc_2_destinations = { bitcoind.rpc.getnewaddress(): 25_000, @@ -1194,6 +1224,12 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind): for c in lianad.rpc.listcoins([], desc_2_outpoints)["coins"] ) ) + lc_res = [c for c in lianad.rpc.listcoins(["unconfirmed"], [])["coins"]] + assert ( + len(lc_res) == 1 + and desc_2_txid in lc_res[0]["outpoint"] + and lc_res[0]["is_from_self"] is True + ) # Now replace the first RBF, which will also remove its descendants. rbf_2_res = lianad.rpc.rbfpsbt(rbf_1_txid, False, feerate) rbf_2_psbt = PSBT.from_base64(rbf_2_res["psbt"]) @@ -1216,6 +1252,12 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind): for c in lianad.rpc.listcoins([], first_outpoints)["coins"] ) ) + lc_res = [c for c in lianad.rpc.listcoins(["unconfirmed"], [])["coins"]] + assert ( + len(lc_res) == 1 + and rbf_2_txid in lc_res[0]["outpoint"] + and lc_res[0]["is_from_self"] is True + ) # The unconfirmed coins used in the descendant transactions have been removed so that # only one of the input coins remains, and its spend info has been wiped so that it is as before. assert lianad.rpc.listcoins([], desc_1_outpoints + desc_2_outpoints)["coins"] == [ @@ -1230,6 +1272,14 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind): for c in lianad.rpc.listcoins([], first_outpoints)["coins"] ) ) + final_coins = lianad.rpc.listcoins()["coins"] + # We have the three original coins plus the change output from the last RBF. + assert len(final_coins) == 4 + assert len(coins + lc_res) == 4 + for fc in final_coins: + assert fc["outpoint"] in [c["outpoint"] for c in coins + lc_res] + # Original coins are not from self, but RBF change output is. + assert fc["is_from_self"] is (fc["outpoint"] in [c["outpoint"] for c in lc_res]) def test_rbfpsbt_insufficient_funds(lianad, bitcoind):