wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs

achow101 commented at 7:33 pm on May 26, 2025: member

Descriptor wallets do not use the watchonly behavior as it is not possible to mix watchonly and non-watchonly in a descriptor wallet. With legacy wallets now removed, all of the watchonly handling and reporting code is no longer needed. This PR removes watchonly options and results from the RPCs and the handling of watchonly things from the wallet’s internals.

With all of the watchonly things removed, ISMINE_WATCH_ONLY is removed as well.

Split from #32523

Depends on #32594 for tests that are easier to read

DrahtBot commented at 7:33 pm on May 26, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32618.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Stale ACK	maflcko, BrandonOdiwuor

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30972 (Wallet: “listreceivedby*” fix by BrandonOdiwuor)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
#25269 (wallet: re-activate “AmountWithFeeExceedsBalance” error by furszy)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

In coincontrol.h: “while requires all selected inputs be used” -> “while requiring all selected inputs be used” [corrects the gerund for grammatical parallelism]

No other typos were found.

drahtbot_id_4_m

DrahtBot added the label Wallet on May 26, 2025

achow101 force-pushed on May 26, 2025

achow101 force-pushed on May 27, 2025

DrahtBot added the label Needs rebase on Jun 4, 2025

achow101 force-pushed on Jun 4, 2025

DrahtBot removed the label Needs rebase on Jun 5, 2025

in src/wallet/rpc/coins.cpp:174 in 69c6a2abc3 outdated

170@@ -171,7 +171,7 @@ RPCHelpMan getbalance()
171                 {
172                     {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Remains for backward compatibility. Must be excluded or set to \"*\"."},
173                     {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "Only include transactions confirmed at least this many times."},
174-                    {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also include balance in watch-only addresses (see 'importaddress')"},
175+                    {"include_watchonly", RPCArg::Type::BOOL, RPCArg::Default{false}, "No longer used"},

rkrux commented at 11:35 am on June 12, 2025:

Does this argument include_watchonly not require the DEPRECATED flag now in the RPCResult description?

achow101 commented at 6:47 pm on June 12, 2025:

I suppose so, added to this and all others in the RPC docs.

achow101 force-pushed on Jun 12, 2025

achow101 force-pushed on Jun 14, 2025

achow101 marked this as ready for review on Jun 14, 2025

achow101 commented at 0:56 am on June 14, 2025: member

Ready for review

DrahtBot added the label Needs rebase on Jun 19, 2025

achow101 force-pushed on Jun 20, 2025

DrahtBot removed the label Needs rebase on Jun 20, 2025

maflcko commented at 3:55 pm on June 24, 2025: member

thx for the test

review ACK 003a3cdb29dcc1e3e1a53f8227de73389071fbd1 📝

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 003a3cdb29dcc1e3e1a53f8227de73389071fbd1 📝
3W/H75G0Njq3xUMblb1LYeqKLIsDTB7AbxYK6n8ESm5k84ss0X35FSu3GEVPcDpVGWOancqnM5G43AeA9J97pCw==

BrandonOdiwuor commented at 8:18 am on July 1, 2025: contributor

Tested ACK 003a3cdb29dcc1e3e1a53f8227de73389071fbd1

Confirmed that all watch-only related functionality has been removed from the wallet and relevant RPCs, in line with the deprecation of legacy wallets.

Verified that watch only has been removed from balances and getbalance RPC.
Tested that include_watchonly option and iswatchonly flag was removed from all RPCs including getaddressinfo, fundrawtransaction, send, sendall, walletcreatefundedpsbt, listreceivedbyaddress, listreceivedbylabel, listtransactions, listsinceblock, simulaterawtransaction

Eunovo commented at 9:26 am on July 1, 2025: contributor

Consider cleaning up wallet_balance.py too, like this:

 0diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py
 1index 4efc0169eb..ac0374c3fa 100755
 2--- a/test/functional/wallet_balance.py
 3+++ b/test/functional/wallet_balance.py
 4@@ -80,9 +80,9 @@ class WalletTest(BitcoinTestFramework):
 5         assert_equal(self.nodes[0].getbalance("*"), 50)
 6         assert_equal(self.nodes[0].getbalance("*", 1), 50)
 7         assert_equal(self.nodes[0].getbalance(minconf=1), 50)
 8-        assert_equal(self.nodes[0].getbalance(minconf=0, include_watchonly=True), 50)
 9-        assert_equal(self.nodes[0].getbalance("*", 1, True), 50)
10-        assert_equal(self.nodes[1].getbalance(minconf=0, include_watchonly=True), 50)
11+        assert_equal(self.nodes[0].getbalance(minconf=0), 50)
12+        assert_equal(self.nodes[0].getbalance("*", 1), 50)
13+        assert_equal(self.nodes[1].getbalance(minconf=0), 50)
14 
15         # Send 40 BTC from 0 to 1 and 60 BTC from 1 to 0.
16         txs = create_transactions(self.nodes[0], self.nodes[1].getnewaddress(), 40, [Decimal('0.01')])
17@@ -142,14 +142,10 @@ class WalletTest(BitcoinTestFramework):
18             # getbalances
19             expected_balances_0 = {'mine':      {'immature':          Decimal('0E-8'),
20                                                  'trusted':           Decimal('9.99'),  # change from node 0's send
21-                                                 'untrusted_pending': Decimal('60.0')},
22-                                   'watchonly': {'immature':          Decimal('5000'),
23-                                                 'trusted':           Decimal('50.0'),
24-                                                 'untrusted_pending': Decimal('0E-8')}}
25+                                                 'untrusted_pending': Decimal('60.0')}}
26             expected_balances_1 = {'mine':      {'immature':          Decimal('0E-8'),
27                                                  'trusted':           Decimal('0E-8'),  # node 1's send had an unsafe input
28                                                  'untrusted_pending': Decimal('30.0') - fee_node_1}}  # Doesn't include output of node 0's send since it was spent
29-            del expected_balances_0["watchonly"]
30             balances_0 = self.nodes[0].getbalances()
31             balances_1 = self.nodes[1].getbalances()
32             # remove lastprocessedblock keys (they will be tested later)

Eunovo commented at 9:35 am on July 1, 2025: contributor

Here is a diff removing include_watchonly=True from other functional tests, except wallet_migration.py

 0diff --git a/test/functional/wallet_importprunedfunds.py b/test/functional/wallet_importprunedfunds.py
 1index cb052e4511..d45be4aa18 100755
 2--- a/test/functional/wallet_importprunedfunds.py
 3+++ b/test/functional/wallet_importprunedfunds.py
 4@@ -89,7 +89,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
 5         wwatch = self.nodes[1].get_wallet_rpc('wwatch')
 6         wwatch.importdescriptors([{"desc": self.nodes[0].getaddressinfo(address2)["desc"], "timestamp": "now"}])
 7         wwatch.importprunedfunds(rawtransaction=rawtxn2, txoutproof=proof2)
 8-        assert [tx for tx in wwatch.listtransactions(include_watchonly=True) if tx['txid'] == txnid2]
 9+        assert [tx for tx in wwatch.listtransactions() if tx['txid'] == txnid2]
10 
11         # Import with private key with no rescan
12         w1 = self.nodes[1].get_wallet_rpc(self.default_wallet_name)
13@@ -109,13 +109,13 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
14 
15         # Remove transactions
16         assert_raises_rpc_error(-4, f'Transaction {txnid1} does not belong to this wallet', w1.removeprunedfunds, txnid1)
17-        assert not [tx for tx in w1.listtransactions(include_watchonly=True) if tx['txid'] == txnid1]
18+        assert not [tx for tx in w1.listtransactions() if tx['txid'] == txnid1]
19 
20         wwatch.removeprunedfunds(txnid2)
21-        assert not [tx for tx in wwatch.listtransactions(include_watchonly=True) if tx['txid'] == txnid2]
22+        assert not [tx for tx in wwatch.listtransactions() if tx['txid'] == txnid2]
23 
24         w1.removeprunedfunds(txnid3)
25-        assert not [tx for tx in w1.listtransactions(include_watchonly=True) if tx['txid'] == txnid3]
26+        assert not [tx for tx in w1.listtransactions() if tx['txid'] == txnid3]
27 
28         # Check various RPC parameter validation errors
29         assert_raises_rpc_error(-22, "TX decode failed", w1.importprunedfunds, b'invalid tx'.hex(), proof1)
30diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py
31index d6219af17c..91dc1a9528 100755
32--- a/test/functional/wallet_listreceivedby.py
33+++ b/test/functional/wallet_listreceivedby.py
34@@ -27,7 +27,7 @@ class ReceivedByTest(BitcoinTestFramework):
35 
36     def run_test(self):
37         # save the number of coinbase reward addresses so far
38-        num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True))
39+        num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True))
40 
41         self.log.info("listreceivedbyaddress Test")
42 
43@@ -67,7 +67,7 @@ class ReceivedByTest(BitcoinTestFramework):
44         # Test Address filtering
45         # Only on addr
46         expected = {"address": addr, "label": "", "amount": Decimal("0.1"), "confirmations": 10, "txids": [txid, ]}
47-        res = self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True, address_filter=addr)
48+        res = self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, address_filter=addr)
49         assert_array_result(res, {"address": addr}, expected)
50         assert_equal(len(res), 1)
51         # Test for regression on CLI calls with address string (#14173)
52@@ -75,7 +75,7 @@ class ReceivedByTest(BitcoinTestFramework):
53         assert_array_result(cli_res, {"address": addr}, expected)
54         assert_equal(len(cli_res), 1)
55         # Error on invalid address
56-        assert_raises_rpc_error(-4, "address_filter parameter was invalid", self.nodes[1].listreceivedbyaddress, minconf=0, include_empty=True, include_watchonl
57y=True, address_filter="bamboozling")
58+        assert_raises_rpc_error(-4, "address_filter parameter was invalid", self.nodes[1].listreceivedbyaddress, minconf=0, include_empty=True, address_filter="
59bamboozling")
60         # Another address receive money
61         res = self.nodes[1].listreceivedbyaddress(0, True, True)
62         assert_equal(len(res), 2 + num_cb_reward_addresses)  # Right now 2 entries

DrahtBot added the label Needs rebase on Jul 1, 2025

test: Watchonly wallets should estimate larger size

Test that when a watchonly wallet and the wallet with private keys fund
the same tx, the watchonly wallet should use a higher fee since it
should be estimating the size to be larger as it assumes the signer
cannot grind the R value.

9991f49c38

wallet: Wallets without private keys cannot grind R d20dc9c6aa

achow101 force-pushed on Jul 1, 2025

wallet: Remove watchonly balances

Descriptor wallets do not have mixed mine and watchonly, so there is no
need to report a watchonly balance.

e81d95d435

wallet, rpc: Remove watchonly from RPCs

Descriptor wallets don't have a conception of watchonly within a wallet,
so remove all of these options and results from the RPCs

1337c72198

wallet, spend: Remove fWatchOnly from CCoinControl

Descriptor wallets do not have a concept of watchonly within a wallet,
so remove the watchonly option from coin control.

4439bf4b41

wallet: Remove ISMINE_WATCH_ONLY

ISMINE_WATCH_ONLY has been removed from all places it was being used,
and migration does not need ISMINE_WATCH_ONLY, so remove the enum.

15710869e1

doc: Release note for removed watchonly parameters and results b1a8ac07e9

achow101 force-pushed on Jul 1, 2025

achow101 commented at 6:27 pm on July 1, 2025: member

Removed the include_watchonly=True.

DrahtBot removed the label Needs rebase on Jul 1, 2025

wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs #32618

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)