From 59b06b696a2fd730ff73bb45e4d1161517bd4562 Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 26 Feb 2023 21:02:34 -0300 Subject: wallet: migration bugfix, clone 'send' record label to all wallets Github-Pull: #28038 Rebased-From: 1b64f6498c394a143df196172a14204fe3b8a744 --- src/wallet/wallet.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4e8c0c0e5e..45de555803 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; } } } -- cgit v1.2.3 From 4b16650c10b028485ece7b438b89236a52322b89 Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 6 Jul 2023 16:10:58 -0300 Subject: wallet: migration bugfix, persist empty labels addressbook records with no associated label could be treated as change. And we don't want that for external addresses. Github-Pull: #28038 Rebased-From: a277f8357ad8b0eb26f33fc36f919d868c06847b --- src/wallet/wallet.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 45de555803..208b97bf07 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4037,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 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); -- cgit v1.2.3 From 37d9cc657cf5b8126a0faef5237bc57e7453abb8 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 12 Apr 2023 12:27:20 -0300 Subject: test: wallet, add coverage for addressbook migration Github-Pull: #28038 Rebased-From: 7ecc29a0b7a23d8f5d3c1e6a0dad29b3ad839eb9 --- test/functional/wallet_migration.py | 136 ++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 7c2959bb89..6cb4abd5e0 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -67,6 +67,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 +479,132 @@ 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 run_test(self): self.generate(self.nodes[0], 101) @@ -482,6 +617,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_encrypted() self.test_unloaded() self.test_unloaded_by_path() + self.test_addressbook() if __name__ == '__main__': WalletMigrationTest().main() -- cgit v1.2.3 From 6d5a510dcdbed7b53f73b8422cea331dd6e9589e Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 11 Jul 2023 11:26:22 -0300 Subject: descriptor: InferScript, do not return top-level only func as sub descriptor e.g. sh(addr(ADDR)) or sh(raw(HEX)) are invalid descriptors. Making sh and wsh top level functions to return addr/raw descriptors when the subscript inference fails. Github-Pull: #28067 Rebased-From: cc781a21800a6ce13875feefd0cb14ab0a84524c --- src/script/descriptor.cpp | 4 ++++ test/functional/data/rpc_decodescript.json | 2 +- test/functional/rpc_decodescript.py | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) 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 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/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") -- cgit v1.2.3 From 513ca0a71173691fb030973612189ac6d064aedd Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 11 Jul 2023 11:31:49 -0300 Subject: test: wallet, add coverage for watch-only raw sh script migration Github-Pull: #28067 Rebased-From: dd9633b516d6936ac4e23a40f9b0bea120117d35 --- test/functional/wallet_migration.py | 50 +++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 6cb4abd5e0..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, @@ -605,6 +607,53 @@ class WalletMigrationTest(BitcoinTestFramework): 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) @@ -618,6 +667,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_unloaded() self.test_unloaded_by_path() self.test_addressbook() + self.test_migrate_raw_p2sh() if __name__ == '__main__': WalletMigrationTest().main() -- cgit v1.2.3 From 494f1afa5a6eab4f243b19ab1691cd231a855b34 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 18 Jul 2023 11:27:09 +0100 Subject: depends: xcb-proto 1.15.2 Resolves build failures under Python 3.12, i.e building on rawhide: ```bash make[3]: Nothing to be done for 'install-exec-am'. /usr/bin/mkdir -p '/bitcoin/depends/work/staging/aarch64-unknown-linux-gnu/xcb_proto/1.14.1-4a91ac9dc41/bitcoin/depends/aarch64-unknown-linux-gnu/lib/python3.12/site-packages/xcbgen' /usr/bin/install -c -m 644 __init__.py error.py expr.py align.py matcher.py state.py xtypes.py '/bitcoin/depends/work/staging/aarch64-unknown-linux-gnu/xcb_proto/1.14.1-4a91ac9dc41/bitcoin/depends/aarch64-unknown-linux-gnu/lib/python3.12/site-packages/xcbgen' Traceback (most recent call last): File "", line 2, in ModuleNotFoundError: No module named 'imp' make[3]: *** [Makefile:271: install-pkgpythonPYTHON] Error 1 ``` `imp` was removed in 3.12: https://docs.python.org/3/library/imp.html. Github-Pull: #28097 Rebased-From: 7cb88c8b46723d306b96953a6a60c90a4ab211e3 --- depends/packages/xcb_proto.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 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) -- cgit v1.2.3