Merge bitcoin/bitcoin#32618: wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs

b1a8ac07e9 doc: Release note for removed watchonly parameters and results (Ava Chow)
15710869e1 wallet: Remove ISMINE_WATCH_ONLY (Ava Chow)
4439bf4b41 wallet, spend: Remove fWatchOnly from CCoinControl (Ava Chow)
1337c72198 wallet, rpc: Remove watchonly from RPCs (Ava Chow)
e81d95d435 wallet: Remove watchonly balances (Ava Chow)
d20dc9c6aa wallet: Wallets without private keys cannot grind R (Ava Chow)
9991f49c38 test: Watchonly wallets should estimate larger size (Ava Chow)

Pull request description:

  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

ACKs for top commit:
  Eunovo:
    ACK https://github.com/bitcoin/bitcoin/pull/32618/commits/b1a8ac07e91dd1d305fcbc16ea931d60e46c0055
  maflcko:
    re-ACK b1a8ac07e9 🌈
  rkrux:
    ACK b1a8ac07e9
  furszy:
    light code review ACK b1a8ac07e9

Tree-SHA512: bc87f37a13294f7208991be8f93899b49e5bdf87c70e0f66d9c4cb09c03be6c202320406f27e9a35aa2f57319d19a3f0c07d5e5ddbc97c7edab165b1656d6612

test/functional/rpc_psbt.py

@@ -827,7 +827,7 @@ class PSBTTest(BitcoinTestFramework):
 
         # Test that psbts with p2pkh outputs are created properly
         p2pkh = self.nodes[0].getnewaddress(address_type='legacy')
-        psbt = self.nodes[1].walletcreatefundedpsbt([], [{p2pkh : 1}], 0, {"includeWatching" : True}, True)
+        psbt = self.nodes[1].walletcreatefundedpsbt(inputs=[], outputs=[{p2pkh : 1}], bip32derivs=True)
         self.nodes[0].decodepsbt(psbt['psbt'])
 
         # Test decoding error: invalid base64

test/functional/wallet_balance.py

@@ -83,9 +83,9 @@ class WalletTest(BitcoinTestFramework):
         assert_equal(self.nodes[0].getbalance("*"), 50)
         assert_equal(self.nodes[0].getbalance("*", 1), 50)
         assert_equal(self.nodes[0].getbalance(minconf=1), 50)
-        assert_equal(self.nodes[0].getbalance(minconf=0, include_watchonly=True), 50)
+        assert_equal(self.nodes[0].getbalance(minconf=0), 50)
         assert_equal(self.nodes[0].getbalance("*", 1, True), 50)
-        assert_equal(self.nodes[1].getbalance(minconf=0, include_watchonly=True), 50)
+        assert_equal(self.nodes[1].getbalance(minconf=0), 50)
 
         # Send 40 BTC from 0 to 1 and 60 BTC from 1 to 0.
         txs = create_transactions(self.nodes[0], self.nodes[1].getnewaddress(), 40, [Decimal('0.01')])
@@ -145,14 +145,10 @@ class WalletTest(BitcoinTestFramework):
             # getbalances
             expected_balances_0 = {'mine':      {'immature':          Decimal('0E-8'),
                                                  'trusted':           Decimal('9.99'),  # change from node 0's send
-                                                 'untrusted_pending': Decimal('60.0')},
-                                   'watchonly': {'immature':          Decimal('5000'),
-                                                 'trusted':           Decimal('50.0'),
-                                                 'untrusted_pending': Decimal('0E-8')}}
+                                                 'untrusted_pending': Decimal('60.0')}}
             expected_balances_1 = {'mine':      {'immature':          Decimal('0E-8'),
                                                  'trusted':           Decimal('0E-8'),  # node 1's send had an unsafe input
                                                  'untrusted_pending': Decimal('30.0') - fee_node_1}}  # Doesn't include output of node 0's send since it was spent
-            del expected_balances_0["watchonly"]
             balances_0 = self.nodes[0].getbalances()
             balances_1 = self.nodes[1].getbalances()
             # remove lastprocessedblock keys (they will be tested later)

test/functional/wallet_basic.py

@@ -532,7 +532,6 @@ class WalletTest(BitcoinTestFramework):
         assert_equal(address_info['address'], "mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ")
         assert_equal(address_info["scriptPubKey"], "76a9144e3854046c7bd1594ac904e4793b6a45b36dea0988ac")
         assert not address_info["ismine"]
-        assert not address_info["iswatchonly"]
         assert not address_info["isscript"]
         assert not address_info["ischange"]
 

test/functional/wallet_fundrawtransaction.py

@@ -153,6 +153,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.test_feerate_rounding()
         self.test_input_confs_control()
         self.test_duplicate_outputs()
+        self.test_watchonly_cannot_grind_r()
 
     def test_duplicate_outputs(self):
         self.log.info("Test deserializing and funding a transaction with duplicate outputs")
@@ -781,7 +782,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.nodes[3].loadwallet('wwatch')
         wwatch = self.nodes[3].get_wallet_rpc('wwatch')
         w3 = self.nodes[3].get_wallet_rpc(self.default_wallet_name)
-        result = wwatch.fundrawtransaction(rawtx, includeWatching=True, changeAddress=w3.getrawchangeaddress(), subtractFeeFromOutputs=[0])
+        result = wwatch.fundrawtransaction(rawtx, changeAddress=w3.getrawchangeaddress(), subtractFeeFromOutputs=[0])
         res_dec = self.nodes[0].decoderawtransaction(result["hex"])
         assert_equal(len(res_dec["vin"]), 1)
         assert res_dec["vin"][0]["txid"] == self.watchonly_utxo['txid']
@@ -1517,5 +1518,28 @@ class RawTransactionsTest(BitcoinTestFramework):
 
         wallet.unloadwallet()
 
+    def test_watchonly_cannot_grind_r(self):
+        self.log.info("Test that a watchonly wallet will estimate higher fees for a tx than the wallet with private keys")
+        self.nodes[0].createwallet("grind")
+        wallet = self.nodes[0].get_wallet_rpc("grind")
+        default_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        self.nodes[0].createwallet(wallet_name="grind_watchonly", disable_private_keys=True)
+        watchonly = self.nodes[0].get_wallet_rpc("grind_watchonly")
+        assert_equal(watchonly.importdescriptors(wallet.listdescriptors()["descriptors"])[0]["success"], True)
+
+        # Send to legacy address type so that we will have an ecdsa signature with a measurable effect on the feerate
+        default_wallet.sendtoaddress(wallet.getnewaddress(address_type="legacy"), 10)
+        self.generate(self.nodes[0], 1)
+
+        assert_equal(wallet.listunspent(), watchonly.listunspent())
+
+        ret_addr = default_wallet.getnewaddress()
+        tx = wallet.createrawtransaction([], [{ret_addr: 5}])
+        funded = wallet.fundrawtransaction(hexstring=tx, fee_rate=10)
+
+        watchonly_funded = watchonly.fundrawtransaction(hexstring=tx, fee_rate=10)
+        assert_greater_than(watchonly_funded["fee"], funded["fee"])
+
 if __name__ == '__main__':
     RawTransactionsTest(__file__).main()

test/functional/wallet_importprunedfunds.py

@@ -52,15 +52,12 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
 
         # Address Test - before import
         address_info = self.nodes[1].getaddressinfo(address1)
-        assert_equal(address_info['iswatchonly'], False)
         assert_equal(address_info['ismine'], False)
 
         address_info = self.nodes[1].getaddressinfo(address2)
-        assert_equal(address_info['iswatchonly'], False)
         assert_equal(address_info['ismine'], False)
 
         address_info = self.nodes[1].getaddressinfo(address3)
-        assert_equal(address_info['iswatchonly'], False)
         assert_equal(address_info['ismine'], False)
 
         # Send funds to self
@@ -92,7 +89,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
         wwatch = self.nodes[1].get_wallet_rpc('wwatch')
         wwatch.importdescriptors([{"desc": self.nodes[0].getaddressinfo(address2)["desc"], "timestamp": "now"}])
         wwatch.importprunedfunds(rawtransaction=rawtxn2, txoutproof=proof2)
-        assert [tx for tx in wwatch.listtransactions(include_watchonly=True) if tx['txid'] == txnid2]
+        assert [tx for tx in wwatch.listtransactions() if tx['txid'] == txnid2]
 
         # Import with private key with no rescan
         w1 = self.nodes[1].get_wallet_rpc(self.default_wallet_name)
@@ -104,24 +101,21 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
 
         # Addresses Test - after import
         address_info = w1.getaddressinfo(address1)
-        assert_equal(address_info['iswatchonly'], False)
         assert_equal(address_info['ismine'], False)
         address_info = wwatch.getaddressinfo(address2)
-        assert_equal(address_info['iswatchonly'], False)
         assert_equal(address_info['ismine'], True)
         address_info = w1.getaddressinfo(address3)
-        assert_equal(address_info['iswatchonly'], False)
         assert_equal(address_info['ismine'], True)
 
         # Remove transactions
         assert_raises_rpc_error(-4, f'Transaction {txnid1} does not belong to this wallet', w1.removeprunedfunds, txnid1)
-        assert not [tx for tx in w1.listtransactions(include_watchonly=True) if tx['txid'] == txnid1]
+        assert not [tx for tx in w1.listtransactions() if tx['txid'] == txnid1]
 
         wwatch.removeprunedfunds(txnid2)
-        assert not [tx for tx in wwatch.listtransactions(include_watchonly=True) if tx['txid'] == txnid2]
+        assert not [tx for tx in wwatch.listtransactions() if tx['txid'] == txnid2]
 
         w1.removeprunedfunds(txnid3)
-        assert not [tx for tx in w1.listtransactions(include_watchonly=True) if tx['txid'] == txnid3]
+        assert not [tx for tx in w1.listtransactions() if tx['txid'] == txnid3]
 
         # Check various RPC parameter validation errors
         assert_raises_rpc_error(-22, "TX decode failed", w1.importprunedfunds, b'invalid tx'.hex(), proof1)

test/functional/wallet_listreceivedby.py

@@ -27,7 +27,7 @@ class ReceivedByTest(BitcoinTestFramework):
 
     def run_test(self):
         # save the number of coinbase reward addresses so far
-        num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True))
+        num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True))
 
         self.log.info("listreceivedbyaddress Test")
 
@@ -67,7 +67,7 @@ class ReceivedByTest(BitcoinTestFramework):
         # Test Address filtering
         # Only on addr
         expected = {"address": addr, "label": "", "amount": Decimal("0.1"), "confirmations": 10, "txids": [txid, ]}
-        res = self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True, address_filter=addr)
+        res = self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, address_filter=addr)
         assert_array_result(res, {"address": addr}, expected)
         assert_equal(len(res), 1)
         # Test for regression on CLI calls with address string (#14173)
@@ -75,7 +75,7 @@ class ReceivedByTest(BitcoinTestFramework):
         assert_array_result(cli_res, {"address": addr}, expected)
         assert_equal(len(cli_res), 1)
         # Error on invalid address
-        assert_raises_rpc_error(-4, "address_filter parameter was invalid", self.nodes[1].listreceivedbyaddress, minconf=0, include_empty=True, include_watchonly=True, address_filter="bamboozling")
+        assert_raises_rpc_error(-4, "address_filter parameter was invalid", self.nodes[1].listreceivedbyaddress, minconf=0, include_empty=True, address_filter="bamboozling")
         # Another address receive money
         res = self.nodes[1].listreceivedbyaddress(0, True, True)
         assert_equal(len(res), 2 + num_cb_reward_addresses)  # Right now 2 entries

test/functional/wallet_migration.py

@@ -282,7 +282,6 @@ class WalletMigrationTest(BitcoinTestFramework):
         # Transaction to multisig is in multisig1_watchonly and not multisig1
         _, multisig1 = self.migrate_and_get_rpc("multisig1")
         assert_equal(multisig1.getaddressinfo(addr1)["ismine"], False)
-        assert_equal(multisig1.getaddressinfo(addr1)["iswatchonly"], False)
         assert_equal(multisig1.getaddressinfo(addr1)["solvable"], False)
         assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", multisig1.gettransaction, txid)
         assert_equal(multisig1.getbalance(), 0)
@@ -368,7 +367,7 @@ class WalletMigrationTest(BitcoinTestFramework):
         assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_watchonly_txid)
         assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_sent_watchonly_utxo['txid'])
         assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, sent_watchonly_txid)
-        assert_equal(len(imports0.listtransactions(include_watchonly=True)), 2)
+        assert_equal(len(imports0.listtransactions()), 2)
         imports0.gettransaction(received_txid)
         imports0.gettransaction(watchonly_spendable_txid)
         assert_equal(imports0.getbalance(), spendable_bal)
@@ -869,18 +868,14 @@ class WalletMigrationTest(BitcoinTestFramework):
 
         # Both addresses should only appear in the watchonly wallet
         p2pkh_addr_info = wallet.getaddressinfo(p2pkh_addr)
-        assert_equal(p2pkh_addr_info["iswatchonly"], False)
         assert_equal(p2pkh_addr_info["ismine"], False)
         p2wpkh_addr_info = wallet.getaddressinfo(p2wpkh_addr)
-        assert_equal(p2wpkh_addr_info["iswatchonly"], False)
         assert_equal(p2wpkh_addr_info["ismine"], False)
 
         watchonly_wallet = self.master_node.get_wallet_rpc(migrate_info["watchonly_name"])
         watchonly_p2pkh_addr_info = watchonly_wallet.getaddressinfo(p2pkh_addr)
-        assert_equal(watchonly_p2pkh_addr_info["iswatchonly"], False)
         assert_equal(watchonly_p2pkh_addr_info["ismine"], True)
         watchonly_p2wpkh_addr_info = watchonly_wallet.getaddressinfo(p2wpkh_addr)
-        assert_equal(watchonly_p2wpkh_addr_info["iswatchonly"], False)
         assert_equal(watchonly_p2wpkh_addr_info["ismine"], True)
 
         # There should only be raw or addr descriptors
@@ -1113,7 +1108,6 @@ class WalletMigrationTest(BitcoinTestFramework):
         assert_equal(wallet.getbalances()['mine']['trusted'], 5)
         addr_info = wallet.getaddressinfo(wsh_pkh_addr)
         assert_equal(addr_info["ismine"], True)
-        assert_equal(addr_info["iswatchonly"], False)
         assert_equal(addr_info["solvable"], True)
 
         wallet.unloadwallet()
@@ -1225,7 +1219,6 @@ class WalletMigrationTest(BitcoinTestFramework):
         # information about the witnessScript
         comp_wsh_pkh_addr_info = wallet.getaddressinfo(comp_wsh_pkh_addr)
         assert_equal(comp_wsh_pkh_addr_info["ismine"], True)
-        assert_equal(comp_wsh_pkh_addr_info["iswatchonly"], False)
         assert "embedded" in comp_wsh_pkh_addr_info
 
         # After migration, the invalid addresses should still not be detected as ismine and not watchonly.
@@ -1234,7 +1227,6 @@ class WalletMigrationTest(BitcoinTestFramework):
         for addr in invalid_addrs:
             addr_info = wallet.getaddressinfo(addr)
             assert_equal(addr_info["ismine"], False)
-            assert_equal(addr_info["iswatchonly"], False)
             assert "embedded" not in addr_info
 
         wallet.unloadwallet()

test/functional/wallet_send.py

@@ -43,7 +43,7 @@ class WalletSendTest(BitcoinTestFramework):
                   arg_conf_target=None, arg_estimate_mode=None, arg_fee_rate=None,
                   conf_target=None, estimate_mode=None, fee_rate=None, add_to_wallet=None, psbt=None,
                   inputs=None, add_inputs=None, include_unsafe=None, change_address=None, change_position=None, change_type=None,
-                  include_watching=None, locktime=None, lock_unspents=None, replaceable=None, subtract_fee_from_outputs=None,
+                  locktime=None, lock_unspents=None, replaceable=None, subtract_fee_from_outputs=None,
                   expect_error=None, solving_data=None, minconf=None):
         assert_not_equal((amount is None), (data is None))
 
@@ -91,8 +91,6 @@ class WalletSendTest(BitcoinTestFramework):
             options["change_position"] = change_position
         if change_type is not None:
             options["change_type"] = change_type
-        if include_watching is not None:
-            options["include_watching"] = include_watching
         if locktime is not None:
             options["locktime"] = locktime
         if lock_unspents is not None:
@@ -111,6 +109,11 @@ class WalletSendTest(BitcoinTestFramework):
         if len(options.keys()) == 0:
             options = None
 
+        expect_sign = from_wallet.getwalletinfo()["private_keys_enabled"]
+        expect_sign = expect_sign and solving_data is None
+        if inputs is not None:
+            expect_sign = expect_sign and all(["weight" not in i for i in inputs])
+
         if expect_error is None:
             res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, fee_rate=arg_fee_rate, options=options)
         else:
@@ -143,7 +146,7 @@ class WalletSendTest(BitcoinTestFramework):
         if locktime:
             return res
 
-        if from_wallet.getwalletinfo()["private_keys_enabled"] and not include_watching:
+        if expect_sign:
             assert_equal(res["complete"], True)
             assert "txid" in res
         else:
@@ -155,7 +158,7 @@ class WalletSendTest(BitcoinTestFramework):
         if include_unsafe:
             from_balance += from_wallet.getbalances()["mine"]["untrusted_pending"]
 
-        if add_to_wallet and not include_watching:
+        if add_to_wallet:
             # Ensure transaction exists in the wallet:
             tx = from_wallet.gettransaction(res["txid"])
             assert tx
@@ -216,17 +219,13 @@ class WalletSendTest(BitcoinTestFramework):
             "desc": descsum_create("wpkh(" + xpub + "/0/0/*)"),
             "timestamp": "now",
             "range": [0, 100],
-            "keypool": True,
             "active": True,
-            "watchonly": True
         },{
             "desc": descsum_create("wpkh(" + xpub + "/0/1/*)"),
             "timestamp": "now",
             "range": [0, 100],
-            "keypool": True,
             "active": True,
             "internal": True,
-            "watchonly": True
         }])
         assert_equal(res, [{"success": True}, {"success": True}])
 
@@ -470,15 +469,15 @@ class WalletSendTest(BitcoinTestFramework):
         ext_utxo = ext_fund.listunspent(addresses=[addr])[0]
 
         # An external input without solving data should result in an error
-        self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, expect_error=(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"])))
+        self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, expect_error=(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"])))
 
         # But funding should work when the solving data is provided
-        res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"], addr_info["embedded"]["embedded"]["scriptPubKey"]]})
+        res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"], addr_info["embedded"]["embedded"]["scriptPubKey"]]})
         signed = ext_wallet.walletprocesspsbt(res["psbt"])
         signed = ext_fund.walletprocesspsbt(res["psbt"])
         assert signed["complete"]
 
-        res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"descriptors": [desc]})
+        res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, solving_data={"descriptors": [desc]})
         signed = ext_wallet.walletprocesspsbt(res["psbt"])
         signed = ext_fund.walletprocesspsbt(res["psbt"])
         assert signed["complete"]
@@ -511,7 +510,6 @@ class WalletSendTest(BitcoinTestFramework):
             inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": input_weight}],
             add_inputs=True,
             psbt=True,
-            include_watching=True,
             fee_rate=target_fee_rate_sat_vb
         )
         signed = ext_wallet.walletprocesspsbt(res["psbt"])