Skip to content

Commit d20f10a

Browse files
committed
Merge bitcoin/bitcoin#33268: wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet
113a422 wallet: Add m_cached_from_me to cache "from me" status (Ava Chow) 609d265 test: Add a test for anchor outputs in the wallet (Ava Chow) c40dc82 wallet: Throw an error in sendall if the tx size cannot be calculated (Ava Chow) 39a7dbd wallet: Determine IsFromMe by checking for TXOs of inputs (Ava Chow) e76c2f7 test: Test wallet 'from me' status change (Ava Chow) Pull request description: One of the ways that the wallet would determine if a transaction was sent from the wallet was by checking if the total amount being spent by a transaction from outputs known to the wallet was greater than 0. This has worked fine until recently since there was no reason for 0-value outputs to be created. However, with ephemeral dust and P2A, it is possible to create standard 0-value outputs, and the wallet was not correctly identifying the spends of such outputs. This PR updates `IsFromMe` to only check whether the wallet knows any of the inputs, rather than checking the debit amount of a transaction. Additionally, a new functional test is added to test for this case, as well as a few other anchor output related scenarios. This also revealed a bug in `sendall` which would cause an assertion error when trying to spend all of the outputs in a wallet that has anchor outputs. Fixes #33265 ACKs for top commit: rkrux: lgtm ACK 113a422 enirox001: Tested ACK 113a422. Ran the full functional test suite including `wallet_anchor.py`; all tests passed. Fix for 0 value anchor detection and sendall size errors looks good. LGTM. furszy: ACK 113a422 Tree-SHA512: df2ce4b258d1875ad0b4f27a5b9b4437137a5889a7d5ed7fbca65f904615e9572d232a8b8d070760f75ac168c1a49b7981f6b5052308575866dc610d191ca964
2 parents 9a5ba15 + 113a422 commit d20f10a

File tree

8 files changed

+182
-3
lines changed

8 files changed

+182
-3
lines changed

src/wallet/receive.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,10 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
195195

196196
bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx)
197197
{
198-
return (CachedTxGetDebit(wallet, wtx, /*avoid_reuse=*/false) > 0);
198+
if (!wtx.m_cached_from_me.has_value()) {
199+
wtx.m_cached_from_me = wallet.IsFromMe(*wtx.tx);
200+
}
201+
return wtx.m_cached_from_me.value();
199202
}
200203

201204
// NOLINTNEXTLINE(misc-no-recursion)

src/wallet/rpc/spend.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1521,7 +1521,6 @@ RPCHelpMan sendall()
15211521
CoinFilterParams coins_params;
15221522
coins_params.min_amount = 0;
15231523
for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, coins_params).All()) {
1524-
CHECK_NONFATAL(output.input_bytes > 0);
15251524
if (send_max && fee_rate.GetFee(output.input_bytes) > output.txout.nValue) {
15261525
continue;
15271526
}
@@ -1544,6 +1543,9 @@ RPCHelpMan sendall()
15441543

15451544
// estimate final size of tx
15461545
const TxSize tx_size{CalculateMaximumSignedTxSize(CTransaction(rawTx), pwallet.get())};
1546+
if (tx_size.vsize == -1) {
1547+
throw JSONRPCError(RPC_WALLET_ERROR, "Unable to determine the size of the transaction, the wallet contains unsolvable descriptors");
1548+
}
15471549
const CAmount fee_from_size{fee_rate.GetFee(tx_size.vsize)};
15481550
const std::optional<CAmount> total_bump_fees{pwallet->chain().calculateCombinedBumpFee(outpoints_spent, fee_rate)};
15491551
CAmount effective_value = total_input_value - fee_from_size - total_bump_fees.value_or(0);

src/wallet/transaction.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,8 @@ class CWalletTx
232232
* CWallet::ComputeTimeSmart().
233233
*/
234234
unsigned int nTimeSmart;
235+
// Cached value for whether the transaction spends any inputs known to the wallet
236+
mutable std::optional<bool> m_cached_from_me{std::nullopt};
235237
int64_t nOrderPos; //!< position in ordered transaction list
236238
std::multimap<int64_t, CWalletTx*>::const_iterator m_it_wtxOrdered;
237239

@@ -339,6 +341,7 @@ class CWalletTx
339341
m_amounts[CREDIT].Reset();
340342
fChangeCached = false;
341343
m_is_cache_empty = true;
344+
m_cached_from_me = std::nullopt;
342345
}
343346

344347
/** True if only scriptSigs are different */

src/wallet/wallet.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1634,7 +1634,11 @@ bool CWallet::IsMine(const COutPoint& outpoint) const
16341634

16351635
bool CWallet::IsFromMe(const CTransaction& tx) const
16361636
{
1637-
return (GetDebit(tx) > 0);
1637+
LOCK(cs_wallet);
1638+
for (const CTxIn& txin : tx.vin) {
1639+
if (GetTXO(txin.prevout)) return true;
1640+
}
1641+
return false;
16381642
}
16391643

16401644
CAmount CWallet::GetDebit(const CTransaction& tx) const

test/functional/test_framework/script_util.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
assert len(DUMMY_MIN_OP_RETURN_SCRIPT) == MIN_PADDING
6767

6868
PAY_TO_ANCHOR = CScript([OP_1, bytes.fromhex("4e73")])
69+
ANCHOR_ADDRESS = "bcrt1pfeesnyr2tx"
6970

7071
def key_to_p2pk_script(key):
7172
key = check_key(key)

test/functional/test_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@
152152
'rpc_orphans.py',
153153
'wallet_listreceivedby.py',
154154
'wallet_abandonconflict.py',
155+
'wallet_anchor.py',
155156
'feature_reindex.py',
156157
'feature_reindex_readonly.py',
157158
'wallet_labels.py',

test/functional/wallet_anchor.py

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2025-present The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or https://www.opensource.org/licenses/mit-license.php.
5+
6+
import time
7+
8+
from test_framework.blocktools import MAX_FUTURE_BLOCK_TIME
9+
from test_framework.descriptors import descsum_create
10+
from test_framework.messages import (
11+
COutPoint,
12+
CTxIn,
13+
CTxInWitness,
14+
CTxOut,
15+
)
16+
from test_framework.script_util import (
17+
ANCHOR_ADDRESS,
18+
PAY_TO_ANCHOR,
19+
)
20+
from test_framework.test_framework import BitcoinTestFramework
21+
from test_framework.util import (
22+
assert_equal,
23+
assert_raises_rpc_error,
24+
)
25+
from test_framework.wallet import MiniWallet
26+
27+
class WalletAnchorTest(BitcoinTestFramework):
28+
def set_test_params(self):
29+
self.num_nodes = 1
30+
31+
def skip_test_if_missing_module(self):
32+
self.skip_if_no_wallet()
33+
34+
def test_0_value_anchor_listunspent(self):
35+
self.log.info("Test that 0-value anchor outputs are detected as UTXOs")
36+
37+
# Create an anchor output, and spend it
38+
sender = MiniWallet(self.nodes[0])
39+
anchor_tx = sender.create_self_transfer(fee_rate=0, version=3)["tx"]
40+
anchor_tx.vout.append(CTxOut(0, PAY_TO_ANCHOR))
41+
anchor_spend = sender.create_self_transfer(version=3)["tx"]
42+
anchor_spend.vin.append(CTxIn(COutPoint(anchor_tx.txid_int, 1), b""))
43+
anchor_spend.wit.vtxinwit.append(CTxInWitness())
44+
submit_res = self.nodes[0].submitpackage([anchor_tx.serialize().hex(), anchor_spend.serialize().hex()])
45+
assert_equal(submit_res["package_msg"], "success")
46+
anchor_txid = anchor_tx.txid_hex
47+
anchor_spend_txid = anchor_spend.txid_hex
48+
49+
# Mine each tx in separate blocks
50+
self.generateblock(self.nodes[0], sender.get_address(), [anchor_tx.serialize().hex()])
51+
anchor_tx_height = self.nodes[0].getblockcount()
52+
self.generateblock(self.nodes[0], sender.get_address(), [anchor_spend.serialize().hex()])
53+
54+
# Mock time forward and generate some blocks to avoid rescanning of latest blocks
55+
self.nodes[0].setmocktime(int(time.time()) + MAX_FUTURE_BLOCK_TIME + 1)
56+
self.generate(self.nodes[0], 10)
57+
58+
self.nodes[0].createwallet(wallet_name="anchor", disable_private_keys=True)
59+
wallet = self.nodes[0].get_wallet_rpc("anchor")
60+
import_res = wallet.importdescriptors([{"desc": descsum_create(f"addr({ANCHOR_ADDRESS})"), "timestamp": "now"}])
61+
assert_equal(import_res[0]["success"], True)
62+
63+
# The wallet should have no UTXOs, and not know of the anchor tx or its spend
64+
assert_equal(wallet.listunspent(), [])
65+
assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", wallet.gettransaction, anchor_txid)
66+
assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", wallet.gettransaction, anchor_spend_txid)
67+
68+
# Rescanning the block containing the anchor so that listunspent will list the output
69+
wallet.rescanblockchain(0, anchor_tx_height)
70+
utxos = wallet.listunspent()
71+
assert_equal(len(utxos), 1)
72+
assert_equal(utxos[0]["txid"], anchor_txid)
73+
assert_equal(utxos[0]["address"], ANCHOR_ADDRESS)
74+
assert_equal(utxos[0]["amount"], 0)
75+
wallet.gettransaction(anchor_txid)
76+
assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", wallet.gettransaction, anchor_spend_txid)
77+
78+
# Rescan the rest of the blockchain to see the anchor was spent
79+
wallet.rescanblockchain()
80+
assert_equal(wallet.listunspent(), [])
81+
wallet.gettransaction(anchor_spend_txid)
82+
83+
def test_cannot_sign_anchors(self):
84+
self.log.info("Test that the wallet cannot spend anchor outputs")
85+
for disable_privkeys in [False, True]:
86+
self.nodes[0].createwallet(wallet_name=f"anchor_spend_{disable_privkeys}", disable_private_keys=disable_privkeys)
87+
wallet = self.nodes[0].get_wallet_rpc(f"anchor_spend_{disable_privkeys}")
88+
import_res = wallet.importdescriptors([
89+
{"desc": descsum_create(f"addr({ANCHOR_ADDRESS})"), "timestamp": "now"},
90+
{"desc": descsum_create(f"raw({PAY_TO_ANCHOR.hex()})"), "timestamp": "now"}
91+
])
92+
assert_equal(import_res[0]["success"], disable_privkeys)
93+
assert_equal(import_res[1]["success"], disable_privkeys)
94+
95+
anchor_txid = self.default_wallet.sendtoaddress(ANCHOR_ADDRESS, 1)
96+
self.generate(self.nodes[0], 1)
97+
98+
wallet = self.nodes[0].get_wallet_rpc("anchor_spend_True")
99+
utxos = wallet.listunspent()
100+
assert_equal(len(utxos), 1)
101+
assert_equal(utxos[0]["txid"], anchor_txid)
102+
assert_equal(utxos[0]["address"], ANCHOR_ADDRESS)
103+
assert_equal(utxos[0]["amount"], 1)
104+
105+
assert_raises_rpc_error(-4, "Missing solving data for estimating transaction size", wallet.send, [{self.default_wallet.getnewaddress(): 0.9999}])
106+
assert_raises_rpc_error(-4, "Error: Private keys are disabled for this wallet", wallet.sendtoaddress, self.default_wallet.getnewaddress(), 0.9999)
107+
assert_raises_rpc_error(-4, "Unable to determine the size of the transaction, the wallet contains unsolvable descriptors", wallet.sendall, recipients=[self.default_wallet.getnewaddress()], inputs=utxos)
108+
assert_raises_rpc_error(-4, "Unable to determine the size of the transaction, the wallet contains unsolvable descriptors", wallet.sendall, recipients=[self.default_wallet.getnewaddress()])
109+
110+
def run_test(self):
111+
self.default_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
112+
self.test_0_value_anchor_listunspent()
113+
self.test_cannot_sign_anchors()
114+
115+
if __name__ == '__main__':
116+
WalletAnchorTest(__file__).main()

test/functional/wallet_listtransactions.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@
55
"""Test the listtransactions API."""
66

77
from decimal import Decimal
8+
import time
89
import os
910
import shutil
1011

12+
from test_framework.blocktools import MAX_FUTURE_BLOCK_TIME
13+
from test_framework.descriptors import descsum_create
1114
from test_framework.messages import (
1215
COIN,
1316
tx_from_hex,
@@ -18,7 +21,9 @@
1821
assert_array_result,
1922
assert_equal,
2023
assert_raises_rpc_error,
24+
find_vout_for_address,
2125
)
26+
from test_framework.wallet_util import get_generate_key
2227

2328

2429
class ListTransactionsTest(BitcoinTestFramework):
@@ -97,6 +102,7 @@ def run_test(self):
97102
self.run_coinjoin_test()
98103
self.run_invalid_parameters_test()
99104
self.test_op_return()
105+
self.test_from_me_status_change()
100106

101107
def run_rbf_opt_in_test(self):
102108
"""Test the opt-in-rbf flag for sent and received transactions."""
@@ -311,6 +317,49 @@ def test_op_return(self):
311317

312318
assert 'address' not in op_ret_tx
313319

320+
def test_from_me_status_change(self):
321+
self.log.info("Test gettransaction after changing a transaction's 'from me' status")
322+
self.nodes[0].createwallet("fromme")
323+
default_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
324+
wallet = self.nodes[0].get_wallet_rpc("fromme")
325+
326+
# The 'fee' field of gettransaction is only added when the transaction is 'from me'
327+
# Run twice, once for a transaction in the mempool, again when it confirms
328+
for confirm in [False, True]:
329+
key = get_generate_key()
330+
descriptor = descsum_create(f"wpkh({key.privkey})")
331+
default_wallet.importdescriptors([{"desc": descriptor, "timestamp": "now"}])
332+
333+
send_res = default_wallet.send(outputs=[{key.p2wpkh_addr: 1}, {wallet.getnewaddress(): 1}])
334+
assert_equal(send_res["complete"], True)
335+
vout = find_vout_for_address(self.nodes[0], send_res["txid"], key.p2wpkh_addr)
336+
utxos = [{"txid": send_res["txid"], "vout": vout}]
337+
self.generate(self.nodes[0], 1, sync_fun=self.no_op)
338+
339+
# Send to the test wallet, ensuring that one input is for the descriptor we will import,
340+
# and that there are other inputs belonging to only the sending wallet
341+
send_res = default_wallet.send(outputs=[{wallet.getnewaddress(): 1.5}], inputs=utxos, add_inputs=True)
342+
assert_equal(send_res["complete"], True)
343+
txid = send_res["txid"]
344+
self.nodes[0].syncwithvalidationinterfacequeue()
345+
tx_info = wallet.gettransaction(txid)
346+
assert "fee" not in tx_info
347+
assert_equal(any(detail["category"] == "send" for detail in tx_info["details"]), False)
348+
349+
if confirm:
350+
self.generate(self.nodes[0], 1, sync_fun=self.no_op)
351+
# Mock time forward and generate blocks so that the import does not rescan the transaction
352+
self.nodes[0].setmocktime(int(time.time()) + MAX_FUTURE_BLOCK_TIME + 1)
353+
self.generate(self.nodes[0], 10, sync_fun=self.no_op)
354+
355+
import_res = wallet.importdescriptors([{"desc": descriptor, "timestamp": "now"}])
356+
assert_equal(import_res[0]["success"], True)
357+
# TODO: We should check that the fee matches, but since the transaction spends inputs
358+
# not known to the wallet, it is incorrectly calculating the fee.
359+
# assert_equal(wallet.gettransaction(txid)["fee"], fee)
360+
tx_info = wallet.gettransaction(txid)
361+
assert "fee" in tx_info
362+
assert_equal(any(detail["category"] == "send" for detail in tx_info["details"]), True)
314363

315364
if __name__ == '__main__':
316365
ListTransactionsTest(__file__).main()

0 commit comments

Comments
 (0)