Merge bitcoin/bitcoin#28047: [25.x] Further backports for 25.1

494f1afa5a6eab4f243b19ab1691cd231a855b34 depends: xcb-proto 1.15.2 (fanquake)
513ca0a71173691fb030973612189ac6d064aedd test: wallet, add coverage for watch-only raw sh script migration (furszy)
6d5a510dcdbed7b53f73b8422cea331dd6e9589e descriptor: InferScript, do not return top-level only func as sub descriptor (furszy)
37d9cc657cf5b8126a0faef5237bc57e7453abb8 test: wallet, add coverage for addressbook migration (furszy)
4b16650c10b028485ece7b438b89236a52322b89 wallet: migration bugfix, persist empty labels (furszy)
59b06b696a2fd730ff73bb45e4d1161517bd4562 wallet: migration bugfix, clone 'send' record label to all wallets (furszy)

Pull request description:

  Currently backports:
  * #28038
  * #28067 2nd & 3rd commits.
  * #28097

ACKs for top commit:
  stickies-v:
    ACK 494f1afa5a6eab4f243b19ab1691cd231a855b34

Tree-SHA512: cea134cfa72950d8428191adce79c0881302ca54488f64d3d4a5f9070bb2445d8074e58fa31a482481c4eabb74c852a025f53597540fc646dc20f33b21cf0a06

6 files changed, 198 insertions, 10 deletions

diff --git a/depends/packages/xcb_proto.mk b/depends/packages/xcb_proto.mk
index 9be822506d..6e1c5a10a8 100644
--- a/depends/packages/xcb_proto.mk
+++ b/depends/packages/xcb_proto.mk
@@ -1,8 +1,8 @@
 package=xcb_proto
-$(package)_version=1.14.1
+$(package)_version=1.15.2
 $(package)_download_path=https://xorg.freedesktop.org/archive/individual/proto
 $(package)_file_name=xcb-proto-$($(package)_version).tar.xz
-$(package)_sha256_hash=f04add9a972ac334ea11d9d7eb4fc7f8883835da3e4859c9afa971efdf57fcc3
+$(package)_sha256_hash=7072beb1f680a2fe3f9e535b797c146d22528990c72f63ddb49d2f350a3653ed
 
 define $(package)_config_cmds
   $($(package)_autoconf)
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index b4ad1e3f10..d1a06c4f04 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -1678,6 +1678,10 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo
         }
     }
 
+    // The following descriptors are all top-level only descriptors.
+    // So if we are not at the top level, return early.
+    if (ctx != ParseScriptContext::TOP) return nullptr;
+
     CTxDestination dest;
     if (ExtractDestination(script, dest)) {
         if (GetScriptForDestination(dest) == script) {
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 4e8c0c0e5e..208b97bf07 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4009,7 +4009,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
                 return false;
             }
         } else {
-            // Labels for everything else (send) should be cloned to all
+            // Labels for everything else ("send") should be cloned to all
             if (data.watchonly_wallet) {
                 LOCK(data.watchonly_wallet->cs_wallet);
                 // Add to the watchonly. Preserve the labels, purpose, and change-ness
@@ -4018,7 +4018,6 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
                 if (!addr_pair.second.IsChange()) {
                     data.watchonly_wallet->m_address_book[addr_pair.first].SetLabel(label);
                 }
-                continue;
             }
             if (data.solvable_wallet) {
                 LOCK(data.solvable_wallet->cs_wallet);
@@ -4028,7 +4027,6 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
                 if (!addr_pair.second.IsChange()) {
                     data.solvable_wallet->m_address_book[addr_pair.first].SetLabel(label);
                 }
-                continue;
             }
         }
     }
@@ -4039,10 +4037,10 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
         WalletBatch batch{wallet.GetDatabase()};
         for (const auto& [destination, addr_book_data] : wallet.m_address_book) {
             auto address{EncodeDestination(destination)};
-            auto label{addr_book_data.GetLabel()};
-            // don't bother writing default values (unknown purpose, empty label)
+            std::optional<std::string> label = addr_book_data.IsChange() ? std::nullopt : std::make_optional(addr_book_data.GetLabel());
+            // don't bother writing default values (unknown purpose)
             if (addr_book_data.purpose) batch.WritePurpose(address, PurposeToString(*addr_book_data.purpose));
-            if (!label.empty()) batch.WriteName(address, label);
+            if (label) batch.WriteName(address, *label);
         }
     };
     if (data.watchonly_wallet) persist_address_book(*data.watchonly_wallet);
diff --git a/test/functional/data/rpc_decodescript.json b/test/functional/data/rpc_decodescript.json
index 5f3e725d4c..4a15ae8792 100644
--- a/test/functional/data/rpc_decodescript.json
+++ b/test/functional/data/rpc_decodescript.json
@@ -69,7 +69,7 @@
             "p2sh": "2N34iiGoUUkVSPiaaTFpJjB1FR9TXQu3PGM",
             "segwit": {
                 "asm": "0 96c2368fc30514a438a8bd909f93c49a1549d77198ccbdb792043b666cb24f42",
-                "desc": "wsh(raw(02eeee))#gtay4y0z",
+                "desc": "addr(bcrt1qjmprdr7rq522gw9ghkgfly7yng25n4m3nrxtmdujqsakvm9jfapqk795l5)#5akkdska",
                 "hex": "002096c2368fc30514a438a8bd909f93c49a1549d77198ccbdb792043b666cb24f42",
                 "address": "bcrt1qjmprdr7rq522gw9ghkgfly7yng25n4m3nrxtmdujqsakvm9jfapqk795l5",
                 "type": "witness_v0_scripthash",
diff --git a/test/functional/rpc_decodescript.py b/test/functional/rpc_decodescript.py
index 673836bd04..f37e61ab50 100755
--- a/test/functional/rpc_decodescript.py
+++ b/test/functional/rpc_decodescript.py
@@ -271,7 +271,7 @@ class DecodeScriptTest(BitcoinTestFramework):
         assert res["segwit"]["desc"] == "wsh(and_v(and_v(v:hash160(ffffffffffffffffffffffffffffffffffffffff),v:pk(0250929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0)),older(1)))#gm8xz4fl"
         # Miniscript-incompatible offered HTLC
         res = self.nodes[0].decodescript("82012088a914ffffffffffffffffffffffffffffffffffffffff882102ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffacb2")
-        assert res["segwit"]["desc"] == "wsh(raw(82012088a914ffffffffffffffffffffffffffffffffffffffff882102ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffacb2))#ra6w2xa7"
+        assert res["segwit"]["desc"] == "addr(bcrt1q73qyfypp47hvgnkjqnav0j3k2lq3v76wg22dk8tmwuz5sfgv66xsvxg6uu)#9p3q328s"
         # Miniscript-compatible multisig bigger than 520 byte P2SH limit.
         res = self.nodes[0].decodescript("5b21020e0338c96a8870479f2396c373cc7696ba124e8635d41b0ea581112b678172612102675333a4e4b8fb51d9d4e22fa5a8eaced3fdac8a8cbf9be8c030f75712e6af992102896807d54bc55c24981f24a453c60ad3e8993d693732288068a23df3d9f50d4821029e51a5ef5db3137051de8323b001749932f2ff0d34c82e96a2c2461de96ae56c2102a4e1a9638d46923272c266631d94d36bdb03a64ee0e14c7518e49d2f29bc401021031c41fdbcebe17bec8d49816e00ca1b5ac34766b91c9f2ac37d39c63e5e008afb2103079e252e85abffd3c401a69b087e590a9b86f33f574f08129ccbd3521ecf516b2103111cf405b627e22135b3b3733a4a34aa5723fb0f58379a16d32861bf576b0ec2210318f331b3e5d38156da6633b31929c5b220349859cc9ca3d33fb4e68aa08401742103230dae6b4ac93480aeab26d000841298e3b8f6157028e47b0897c1e025165de121035abff4281ff00660f99ab27bb53e6b33689c2cd8dcd364bc3c90ca5aea0d71a62103bd45cddfacf2083b14310ae4a84e25de61e451637346325222747b157446614c2103cc297026b06c71cbfa52089149157b5ff23de027ac5ab781800a578192d175462103d3bde5d63bdb3a6379b461be64dad45eabff42f758543a9645afd42f6d4248282103ed1e8d5109c9ed66f7941bc53cc71137baa76d50d274bda8d5e8ffbd6e61fe9a5fae736402c00fb269522103aab896d53a8e7d6433137bbba940f9c521e085dd07e60994579b64a6d992cf79210291b7d0b1b692f8f524516ed950872e5da10fb1b808b5a526dedc6fed1cf29807210386aa9372fbab374593466bc5451dc59954e90787f08060964d95c87ef34ca5bb53ae68")
         assert_equal(res["segwit"]["desc"], "wsh(or_d(multi(11,020e0338c96a8870479f2396c373cc7696ba124e8635d41b0ea581112b67817261,02675333a4e4b8fb51d9d4e22fa5a8eaced3fdac8a8cbf9be8c030f75712e6af99,02896807d54bc55c24981f24a453c60ad3e8993d693732288068a23df3d9f50d48,029e51a5ef5db3137051de8323b001749932f2ff0d34c82e96a2c2461de96ae56c,02a4e1a9638d46923272c266631d94d36bdb03a64ee0e14c7518e49d2f29bc4010,031c41fdbcebe17bec8d49816e00ca1b5ac34766b91c9f2ac37d39c63e5e008afb,03079e252e85abffd3c401a69b087e590a9b86f33f574f08129ccbd3521ecf516b,03111cf405b627e22135b3b3733a4a34aa5723fb0f58379a16d32861bf576b0ec2,0318f331b3e5d38156da6633b31929c5b220349859cc9ca3d33fb4e68aa0840174,03230dae6b4ac93480aeab26d000841298e3b8f6157028e47b0897c1e025165de1,035abff4281ff00660f99ab27bb53e6b33689c2cd8dcd364bc3c90ca5aea0d71a6,03bd45cddfacf2083b14310ae4a84e25de61e451637346325222747b157446614c,03cc297026b06c71cbfa52089149157b5ff23de027ac5ab781800a578192d17546,03d3bde5d63bdb3a6379b461be64dad45eabff42f758543a9645afd42f6d424828,03ed1e8d5109c9ed66f7941bc53cc71137baa76d50d274bda8d5e8ffbd6e61fe9a),and_v(v:older(4032),multi(2,03aab896d53a8e7d6433137bbba940f9c521e085dd07e60994579b64a6d992cf79,0291b7d0b1b692f8f524516ed950872e5da10fb1b808b5a526dedc6fed1cf29807,0386aa9372fbab374593466bc5451dc59954e90787f08060964d95c87ef34ca5bb))))#7jwwklk4")
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index 7c2959bb89..157d279385 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -8,6 +8,8 @@ import os
 import random
 from test_framework.descriptors import descsum_create
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.messages import COIN, CTransaction, CTxOut
+from test_framework.script_util import key_to_p2pkh_script, script_to_p2sh_script, script_to_p2wsh_script
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
@@ -67,6 +69,15 @@ class WalletMigrationTest(BitcoinTestFramework):
                 del d["parent_descs"]
         assert_equal(received_list_txs, expected_list_txs)
 
+    def check_address(self, wallet, addr, is_mine, is_change, label):
+        addr_info = wallet.getaddressinfo(addr)
+        assert_equal(addr_info['ismine'], is_mine)
+        assert_equal(addr_info['ischange'], is_change)
+        if label is not None:
+            assert_equal(addr_info['labels'], [label]),
+        else:
+            assert_equal(addr_info['labels'], []),
+
     def test_basic(self):
         default = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
 
@@ -470,6 +481,179 @@ class WalletMigrationTest(BitcoinTestFramework):
 
         assert_equal(bals, wallet.getbalances())
 
+    def test_addressbook(self):
+        df_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        self.log.info("Test migration of address book data")
+        wallet = self.create_legacy_wallet("legacy_addrbook")
+        df_wallet.sendtoaddress(wallet.getnewaddress(), 3)
+
+        # Import watch-only script to create a watch-only wallet after migration
+        watch_addr = df_wallet.getnewaddress()
+        wallet.importaddress(watch_addr)
+        df_wallet.sendtoaddress(watch_addr, 2)
+
+        # Import solvable script
+        multi_addr1 = wallet.getnewaddress()
+        multi_addr2 = wallet.getnewaddress()
+        multi_addr3 = df_wallet.getnewaddress()
+        wallet.importpubkey(df_wallet.getaddressinfo(multi_addr3)["pubkey"])
+        ms_addr_info = wallet.addmultisigaddress(2, [multi_addr1, multi_addr2, multi_addr3])
+
+        self.generate(self.nodes[0], 1)
+
+        # Test vectors
+        addr_external = {
+            "addr": df_wallet.getnewaddress(),
+            "is_mine": False,
+            "is_change": False,
+            "label": ""
+        }
+        addr_external_with_label = {
+            "addr": df_wallet.getnewaddress(),
+            "is_mine": False,
+            "is_change": False,
+            "label": "external"
+        }
+        addr_internal = {
+            "addr": wallet.getnewaddress(),
+            "is_mine": True,
+            "is_change": False,
+            "label": ""
+        }
+        addr_internal_with_label = {
+            "addr": wallet.getnewaddress(),
+            "is_mine": True,
+            "is_change": False,
+            "label": "internal"
+        }
+        change_address = {
+            "addr": wallet.getrawchangeaddress(),
+            "is_mine": True,
+            "is_change": True,
+            "label": None
+        }
+        watch_only_addr = {
+            "addr": watch_addr,
+            "is_mine": False,
+            "is_change": False,
+            "label": "imported"
+        }
+        ms_addr = {
+            "addr": ms_addr_info['address'],
+            "is_mine": False,
+            "is_change": False,
+            "label": "multisig"
+        }
+
+        # To store the change address in the addressbook need to send coins to it
+        wallet.send(outputs=[{wallet.getnewaddress(): 2}], options={"change_address": change_address['addr']})
+        self.generate(self.nodes[0], 1)
+
+        # Util wrapper func for 'addr_info'
+        def check(info, node):
+            self.check_address(node, info['addr'], info['is_mine'], info['is_change'], info["label"])
+
+        # Pre-migration: set label and perform initial checks
+        for addr_info in [addr_external, addr_external_with_label, addr_internal, addr_internal_with_label, change_address, watch_only_addr, ms_addr]:
+            if not addr_info['is_change']:
+                wallet.setlabel(addr_info['addr'], addr_info["label"])
+            check(addr_info, wallet)
+
+        # Migrate wallet
+        info_migration = wallet.migratewallet()
+        wallet_wo = self.nodes[0].get_wallet_rpc(info_migration["watchonly_name"])
+        wallet_solvables = self.nodes[0].get_wallet_rpc(info_migration["solvables_name"])
+
+        #########################
+        # Post migration checks #
+        #########################
+
+        # First check the main wallet
+        for addr_info in [addr_external, addr_external_with_label, addr_internal, addr_internal_with_label, change_address, ms_addr]:
+            check(addr_info, wallet)
+
+        # Watch-only wallet will contain the watch-only entry (with 'is_mine=True') and all external addresses ('send')
+        self.check_address(wallet_wo, watch_only_addr['addr'], is_mine=True, is_change=watch_only_addr['is_change'], label=watch_only_addr["label"])
+        for addr_info in [addr_external, addr_external_with_label, ms_addr]:
+            check(addr_info, wallet_wo)
+
+        # Solvables wallet will contain the multisig entry (with 'is_mine=True') and all external addresses ('send')
+        self.check_address(wallet_solvables, ms_addr['addr'], is_mine=True, is_change=ms_addr['is_change'], label=ms_addr["label"])
+        for addr_info in [addr_external, addr_external_with_label]:
+            check(addr_info, wallet_solvables)
+
+        ########################################################################################
+        # Now restart migrated wallets and verify that the addressbook entries are still there #
+        ########################################################################################
+
+        # First the main wallet
+        self.nodes[0].unloadwallet("legacy_addrbook")
+        self.nodes[0].loadwallet("legacy_addrbook")
+        for addr_info in [addr_external, addr_external_with_label, addr_internal, addr_internal_with_label, change_address, ms_addr]:
+            check(addr_info, wallet)
+
+        # Watch-only wallet
+        self.nodes[0].unloadwallet(info_migration["watchonly_name"])
+        self.nodes[0].loadwallet(info_migration["watchonly_name"])
+        self.check_address(wallet_wo, watch_only_addr['addr'], is_mine=True, is_change=watch_only_addr['is_change'], label=watch_only_addr["label"])
+        for addr_info in [addr_external, addr_external_with_label, ms_addr]:
+            check(addr_info, wallet_wo)
+
+        # Solvables wallet
+        self.nodes[0].unloadwallet(info_migration["solvables_name"])
+        self.nodes[0].loadwallet(info_migration["solvables_name"])
+        self.check_address(wallet_solvables, ms_addr['addr'], is_mine=True, is_change=ms_addr['is_change'], label=ms_addr["label"])
+        for addr_info in [addr_external, addr_external_with_label]:
+            check(addr_info, wallet_solvables)
+
+    def test_migrate_raw_p2sh(self):
+        self.log.info("Test migration of watch-only raw p2sh script")
+        df_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+        wallet = self.create_legacy_wallet("raw_p2sh")
+
+        def send_to_script(script, amount):
+            tx = CTransaction()
+            tx.vout.append(CTxOut(nValue=amount*COIN, scriptPubKey=script))
+
+            hex_tx = df_wallet.fundrawtransaction(tx.serialize().hex())['hex']
+            signed_tx = df_wallet.signrawtransactionwithwallet(hex_tx)
+            df_wallet.sendrawtransaction(signed_tx['hex'])
+            self.generate(self.nodes[0], 1)
+
+        # Craft sh(pkh(key)) script and send coins to it
+        pubkey = df_wallet.getaddressinfo(df_wallet.getnewaddress())["pubkey"]
+        script_pkh = key_to_p2pkh_script(pubkey)
+        script_sh_pkh = script_to_p2sh_script(script_pkh)
+        send_to_script(script=script_sh_pkh, amount=2)
+
+        # Import script and check balance
+        wallet.rpc.importaddress(address=script_pkh.hex(), label="raw_spk", rescan=True, p2sh=True)
+        assert_equal(wallet.getbalances()['watchonly']['trusted'], 2)
+
+        # Craft wsh(pkh(key)) and send coins to it
+        pubkey = df_wallet.getaddressinfo(df_wallet.getnewaddress())["pubkey"]
+        script_wsh_pkh = script_to_p2wsh_script(key_to_p2pkh_script(pubkey))
+        send_to_script(script=script_wsh_pkh, amount=3)
+
+        # Import script and check balance
+        wallet.rpc.importaddress(address=script_wsh_pkh.hex(), label="raw_spk2", rescan=True, p2sh=False)
+        assert_equal(wallet.getbalances()['watchonly']['trusted'], 5)
+
+        # Migrate wallet and re-check balance
+        info_migration = wallet.migratewallet()
+        wallet_wo = self.nodes[0].get_wallet_rpc(info_migration["watchonly_name"])
+
+        # Watch-only balance is under "mine".
+        assert_equal(wallet_wo.getbalances()['mine']['trusted'], 5)
+        # The watch-only scripts are no longer part of the main wallet
+        assert_equal(wallet.getbalances()['mine']['trusted'], 0)
+
+        # Just in case, also verify wallet restart
+        self.nodes[0].unloadwallet(info_migration["watchonly_name"])
+        self.nodes[0].loadwallet(info_migration["watchonly_name"])
+        assert_equal(wallet_wo.getbalances()['mine']['trusted'], 5)
+
     def run_test(self):
         self.generate(self.nodes[0], 101)
 
@@ -482,6 +666,8 @@ class WalletMigrationTest(BitcoinTestFramework):
         self.test_encrypted()
         self.test_unloaded()
         self.test_unloaded_by_path()
+        self.test_addressbook()
+        self.test_migrate_raw_p2sh()
 
 if __name__ == '__main__':
     WalletMigrationTest().main()