diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index b620c6beb02..04287581f5a 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -597,7 +597,7 @@ std::unique_ptr LegacyDataSPKM::GetSolvingProvider(const CScrip return std::make_unique(*this); } -bool LegacyScriptPubKeyMan::CanProvide(const CScript& script, SignatureData& sigdata) +bool LegacyDataSPKM::CanProvide(const CScript& script, SignatureData& sigdata) { IsMineResult ismine = IsMineInner(*this, script, IsMineSigVersion::TOP, /* recurse_scripthash= */ false); if (ismine == IsMineResult::SPENDABLE || ismine == IsMineResult::WATCH_ONLY) { @@ -1706,61 +1706,64 @@ std::set LegacyScriptPubKeyMan::GetKeys() const return set_address; } -std::unordered_set LegacyDataSPKM::GetScriptPubKeys() const +std::unordered_set LegacyDataSPKM::GetCandidateScriptPubKeys() const { LOCK(cs_KeyStore); + std::unordered_set candidate_spks; + + // For every private key in the wallet, there should be a P2PK, P2PKH, P2WPKH, and P2SH-P2WPKH + const auto& add_pubkey = [&candidate_spks](const CPubKey& pub) -> void { + candidate_spks.insert(GetScriptForRawPubKey(pub)); + candidate_spks.insert(GetScriptForDestination(PKHash(pub))); + + CScript wpkh = GetScriptForDestination(WitnessV0KeyHash(pub)); + candidate_spks.insert(wpkh); + candidate_spks.insert(GetScriptForDestination(ScriptHash(wpkh))); + }; + for (const auto& [_, key] : mapKeys) { + add_pubkey(key.GetPubKey()); + } + for (const auto& [_, ckeypair] : mapCryptedKeys) { + add_pubkey(ckeypair.first); + } + + // mapScripts contains all redeemScripts and witnessScripts. Therefore each script in it has + // itself, P2SH, P2WSH, and P2SH-P2WSH as a candidate. + // Invalid scripts such as P2SH-P2SH and P2WSH-P2SH, among others, will be added as candidates. + // Callers of this function will need to remove such scripts. + const auto& add_script = [&candidate_spks](const CScript& script) -> void { + candidate_spks.insert(script); + candidate_spks.insert(GetScriptForDestination(ScriptHash(script))); + + CScript wsh = GetScriptForDestination(WitnessV0ScriptHash(script)); + candidate_spks.insert(wsh); + candidate_spks.insert(GetScriptForDestination(ScriptHash(wsh))); + }; + for (const auto& [_, script] : mapScripts) { + add_script(script); + } + + // Although setWatchOnly should only contain output scripts, we will also include each script's + // P2SH, P2WSH, and P2SH-P2WSH as a precaution. + for (const auto& script : setWatchOnly) { + add_script(script); + } + + return candidate_spks; +} + +std::unordered_set LegacyDataSPKM::GetScriptPubKeys() const +{ + // Run IsMine() on each candidate output script. Any script that is not ISMINE_NO is an output + // script to return. + // This both filters out things that are not watched by the wallet, and things that are invalid. std::unordered_set spks; - - // All keys have at least P2PK and P2PKH - for (const auto& key_pair : mapKeys) { - const CPubKey& pub = key_pair.second.GetPubKey(); - spks.insert(GetScriptForRawPubKey(pub)); - spks.insert(GetScriptForDestination(PKHash(pub))); - } - for (const auto& key_pair : mapCryptedKeys) { - const CPubKey& pub = key_pair.second.first; - spks.insert(GetScriptForRawPubKey(pub)); - spks.insert(GetScriptForDestination(PKHash(pub))); - } - - // For every script in mapScript, only the ISMINE_SPENDABLE ones are being tracked. - // The watchonly ones will be in setWatchOnly which we deal with later - // For all keys, if they have segwit scripts, those scripts will end up in mapScripts - for (const auto& script_pair : mapScripts) { - const CScript& script = script_pair.second; - if (IsMine(script) == ISMINE_SPENDABLE) { - // Add ScriptHash for scripts that are not already P2SH - if (!script.IsPayToScriptHash()) { - spks.insert(GetScriptForDestination(ScriptHash(script))); - } - // For segwit scripts, we only consider them spendable if we have the segwit spk - int wit_ver = -1; - std::vector witprog; - if (script.IsWitnessProgram(wit_ver, witprog) && wit_ver == 0) { - spks.insert(script); - } - } else { - // Multisigs are special. They don't show up as ISMINE_SPENDABLE unless they are in a P2SH - // So check the P2SH of a multisig to see if we should insert it - std::vector> sols; - TxoutType type = Solver(script, sols); - if (type == TxoutType::MULTISIG) { - CScript ms_spk = GetScriptForDestination(ScriptHash(script)); - if (IsMine(ms_spk) != ISMINE_NO) { - spks.insert(ms_spk); - } - } + for (const CScript& script : GetCandidateScriptPubKeys()) { + if (IsMine(script) != ISMINE_NO) { + spks.insert(script); } } - // All watchonly scripts are raw - for (const CScript& script : setWatchOnly) { - // As the legacy wallet allowed to import any script, we need to verify the validity here. - // LegacyScriptPubKeyMan::IsMine() return 'ISMINE_NO' for invalid or not watched scripts (IsMineResult::INVALID or IsMineResult::NO). - // e.g. a "sh(sh(pkh()))" which legacy wallets allowed to import!. - if (IsMine(script) != ISMINE_NO) spks.insert(script); - } - return spks; } @@ -1931,6 +1934,21 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() // InferDescriptor as that will get us all the solving info if it is there std::unique_ptr desc = InferDescriptor(spk, *GetSolvingProvider(spk)); + + // Past bugs in InferDescriptor have caused it to create descriptors which cannot be re-parsed. + // Re-parse the descriptors to detect that, and skip any that do not parse. + { + std::string desc_str = desc->ToString(); + FlatSigningProvider parsed_keys; + std::string parse_error; + std::vector> parsed_descs = Parse(desc_str, parsed_keys, parse_error); + if (parsed_descs.empty()) { + // Remove this scriptPubKey from the set + it = spks.erase(it); + continue; + } + } + // Get the private keys for this descriptor std::vector scripts; FlatSigningProvider keys; @@ -1980,10 +1998,29 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() } } - // Multisigs are special. They don't show up as ISMINE_SPENDABLE unless they are in a P2SH - // So we have to check if any of our scripts are a multisig and if so, add the P2SH - for (const auto& script_pair : mapScripts) { - const CScript script = script_pair.second; + // Make sure that we have accounted for all scriptPubKeys + if (!Assume(spks.empty())) { + LogPrintf("%s\n", STR_INTERNAL_BUG("Error: Some output scripts were not migrated.\n")); + return std::nullopt; + } + + // Legacy wallets can also contain scripts whose P2SH, P2WSH, or P2SH-P2WSH it is not watching for + // but can provide script data to a PSBT spending them. These "solvable" output scripts will need to + // be put into the separate "solvables" wallet. + // These can be detected by going through the entire candidate output scripts, finding the ISMINE_NO scripts, + // and checking CanProvide() which will dummy sign. + for (const CScript& script : GetCandidateScriptPubKeys()) { + // Since we only care about P2SH, P2WSH, and P2SH-P2WSH, filter out any scripts that are not those + if (!script.IsPayToScriptHash() && !script.IsPayToWitnessScriptHash()) { + continue; + } + if (IsMine(script) != ISMINE_NO) { + continue; + } + SignatureData dummy_sigdata; + if (!CanProvide(script, dummy_sigdata)) { + continue; + } // Get birthdate from script meta uint64_t creation_time = 0; @@ -1992,45 +2029,28 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() creation_time = it->second.nCreateTime; } - std::vector> sols; - TxoutType type = Solver(script, sols); - if (type == TxoutType::MULTISIG) { - CScript sh_spk = GetScriptForDestination(ScriptHash(script)); - CTxDestination witdest = WitnessV0ScriptHash(script); - CScript witprog = GetScriptForDestination(witdest); - CScript sh_wsh_spk = GetScriptForDestination(ScriptHash(witprog)); + // InferDescriptor as that will get us all the solving info if it is there + std::unique_ptr desc = InferDescriptor(script, *GetSolvingProvider(script)); + if (!desc->IsSolvable()) { + // The wallet was able to provide some information, but not enough to make a descriptor that actually + // contains anything useful. This is probably because the script itself is actually unsignable (e.g. P2WSH-P2WSH). + continue; + } - // We only want the multisigs that we have not already seen, i.e. they are not watchonly and not spendable - // For P2SH, a multisig is not ISMINE_NO when: - // * All keys are in the wallet - // * The multisig itself is watch only - // * The P2SH is watch only - // For P2SH-P2WSH, if the script is in the wallet, then it will have the same conditions as P2SH. - // For P2WSH, a multisig is not ISMINE_NO when, other than the P2SH conditions: - // * The P2WSH script is in the wallet and it is being watched - std::vector> keys(sols.begin() + 1, sols.begin() + sols.size() - 1); - if (HaveWatchOnly(sh_spk) || HaveWatchOnly(script) || HaveKeys(keys, *this) || (HaveCScript(CScriptID(witprog)) && HaveWatchOnly(witprog))) { - // The above emulates IsMine for these 3 scriptPubKeys, so double check that by running IsMine - assert(IsMine(sh_spk) != ISMINE_NO || IsMine(witprog) != ISMINE_NO || IsMine(sh_wsh_spk) != ISMINE_NO); + // Past bugs in InferDescriptor have caused it to create descriptors which cannot be re-parsed + // Re-parse the descriptors to detect that, and skip any that do not parse. + { + std::string desc_str = desc->ToString(); + FlatSigningProvider parsed_keys; + std::string parse_error; + std::vector> parsed_descs = Parse(desc_str, parsed_keys, parse_error, false); + if (parsed_descs.empty()) { continue; } - assert(IsMine(sh_spk) == ISMINE_NO && IsMine(witprog) == ISMINE_NO && IsMine(sh_wsh_spk) == ISMINE_NO); - - std::unique_ptr sh_desc = InferDescriptor(sh_spk, *GetSolvingProvider(sh_spk)); - out.solvable_descs.emplace_back(sh_desc->ToString(), creation_time); - - const auto desc = InferDescriptor(witprog, *this); - if (desc->IsSolvable()) { - std::unique_ptr wsh_desc = InferDescriptor(witprog, *GetSolvingProvider(witprog)); - out.solvable_descs.emplace_back(wsh_desc->ToString(), creation_time); - std::unique_ptr sh_wsh_desc = InferDescriptor(sh_wsh_spk, *GetSolvingProvider(sh_wsh_spk)); - out.solvable_descs.emplace_back(sh_wsh_desc->ToString(), creation_time); - } } - } - // Make sure that we have accounted for all scriptPubKeys - assert(spks.size() == 0); + out.solvable_descs.emplace_back(desc->ToString(), creation_time); + } // Finalize transaction if (!batch.TxnCommit()) { diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 0d19eb92229..4dbb2838d71 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -303,6 +303,10 @@ protected: virtual bool AddKeyPubKeyInner(const CKey& key, const CPubKey &pubkey); bool AddCryptedKeyInner(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret); + // Helper function to retrieve a conservative superset of all output scripts that may be relevant to this LegacyDataSPKM. + // It may include scripts that are invalid or not actually watched by this LegacyDataSPKM. + // Used only in migration. + std::unordered_set GetCandidateScriptPubKeys() const; public: using ScriptPubKeyMan::ScriptPubKeyMan; @@ -319,6 +323,7 @@ public: uint256 GetID() const override { return uint256::ONE; } // TODO: Remove IsMine when deleting LegacyScriptPubKeyMan isminetype IsMine(const CScript& script) const override; + bool CanProvide(const CScript& script, SignatureData& sigdata) override; // FillableSigningProvider overrides bool HaveKey(const CKeyID &address) const override; @@ -488,8 +493,6 @@ public: bool CanGetAddresses(bool internal = false) const override; - bool CanProvide(const CScript& script, SignatureData& sigdata) override; - bool SignTransaction(CMutableTransaction& tx, const std::map& coins, int sighash, std::map& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; std::optional FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override; diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 5be56cec291..ce8dc19460d 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -10,9 +10,10 @@ import struct import time from test_framework.address import ( - script_to_p2sh, key_to_p2pkh, key_to_p2wpkh, + script_to_p2sh, + script_to_p2wsh, ) from test_framework.bdb import BTREE_MAGIC from test_framework.descriptors import descsum_create @@ -24,6 +25,7 @@ from test_framework.script_util import key_to_p2pkh_script, key_to_p2pk_script, from test_framework.util import ( assert_equal, assert_raises_rpc_error, + find_vout_for_address, sha256sum_file, ) from test_framework.wallet_util import ( @@ -102,6 +104,7 @@ class WalletMigrationTest(BitcoinTestFramework): # Reload to force write that record self.old_node.unloadwallet(wallet_name) self.old_node.loadwallet(wallet_name) + assert_equal(self.old_node.get_wallet_rpc(wallet_name).getwalletinfo()["descriptors"], False) # Now unload so we can copy it to the master node for the migration test self.old_node.unloadwallet(wallet_name) if wallet_name == "": @@ -111,7 +114,10 @@ class WalletMigrationTest(BitcoinTestFramework): # Migrate, checking that rescan does not occur with self.master_node.assert_debug_log(expected_msgs=[], unexpected_msgs=["Rescanning"]): migrate_info = self.master_node.migratewallet(wallet_name=wallet_name, **kwargs) - return migrate_info, self.master_node.get_wallet_rpc(wallet_name) + wallet = self.master_node.get_wallet_rpc(wallet_name) + assert_equal(wallet.getwalletinfo()["descriptors"], True) + self.assert_is_sqlite(wallet_name) + return migrate_info, wallet def test_basic(self): default = self.master_node.get_wallet_rpc(self.default_wallet_name) @@ -132,10 +138,6 @@ class WalletMigrationTest(BitcoinTestFramework): # Note: migration could take a while. _, basic0 = self.migrate_and_get_rpc("basic0") - # Verify created descriptors - assert_equal(basic0.getwalletinfo()["descriptors"], True) - self.assert_is_sqlite("basic0") - # The wallet should create the following descriptors: # * BIP32 descriptors in the form of "0h/0h/*" and "0h/1h/*" (2 descriptors) # * BIP44 descriptors in the form of "44h/1h/0h/0/*" and "44h/1h/0h/1/*" (2 descriptors) @@ -172,8 +174,6 @@ class WalletMigrationTest(BitcoinTestFramework): addr_gps = basic1.listaddressgroupings() basic1_migrate, basic1 = self.migrate_and_get_rpc("basic1") - assert_equal(basic1.getwalletinfo()["descriptors"], True) - self.assert_is_sqlite("basic1") assert_equal(basic1.getbalance(), bal) self.assert_list_txs_equal(basic1.listtransactions(), txs) @@ -191,8 +191,6 @@ class WalletMigrationTest(BitcoinTestFramework): default = self.master_node.get_wallet_rpc(self.default_wallet_name) self.master_node.loadwallet("basic1") basic1 = self.master_node.get_wallet_rpc("basic1") - assert_equal(basic1.getwalletinfo()["descriptors"], True) - self.assert_is_sqlite("basic1") assert_equal(basic1.getbalance(), bal) self.assert_list_txs_equal(basic1.listtransactions(), txs) @@ -214,8 +212,6 @@ class WalletMigrationTest(BitcoinTestFramework): # Now migrate and test that we still have the same balance/transactions _, basic2 = self.migrate_and_get_rpc("basic2") - assert_equal(basic2.getwalletinfo()["descriptors"], True) - self.assert_is_sqlite("basic2") assert_equal(basic2.getbalance(), basic2_balance) self.assert_list_txs_equal(basic2.listtransactions(), basic2_txs) @@ -240,8 +236,6 @@ class WalletMigrationTest(BitcoinTestFramework): ms_info = multisig0.addmultisigaddress(2, [addr1, addr2, addr3]) _, multisig0 = self.migrate_and_get_rpc("multisig0") - assert_equal(multisig0.getwalletinfo()["descriptors"], True) - self.assert_is_sqlite("multisig0") ms_addr_info = multisig0.getaddressinfo(ms_info["address"]) assert_equal(ms_addr_info["ismine"], True) assert_equal(ms_addr_info["desc"], ms_info["descriptor"]) @@ -275,8 +269,6 @@ class WalletMigrationTest(BitcoinTestFramework): # A new wallet multisig1_watchonly is created which has the multisig address # Transaction to multisig is in multisig1_watchonly and not multisig1 _, multisig1 = self.migrate_and_get_rpc("multisig1") - assert_equal(multisig1.getwalletinfo()["descriptors"], True) - self.assert_is_sqlite("multisig1") assert_equal(multisig1.getaddressinfo(addr1)["ismine"], False) assert_equal(multisig1.getaddressinfo(addr1)["iswatchonly"], False) assert_equal(multisig1.getaddressinfo(addr1)["solvable"], False) @@ -361,8 +353,6 @@ class WalletMigrationTest(BitcoinTestFramework): # Migrate _, imports0 = self.migrate_and_get_rpc("imports0") - assert_equal(imports0.getwalletinfo()["descriptors"], True) - self.assert_is_sqlite("imports0") 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) @@ -932,7 +922,6 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(wallet.getwalletinfo()["blank"], True) _, wallet = self.migrate_and_get_rpc("blank") assert_equal(wallet.getwalletinfo()["blank"], True) - assert_equal(wallet.getwalletinfo()["descriptors"], True) def test_avoidreuse(self): self.log.info("Test that avoidreuse persists after migration") @@ -1074,6 +1063,300 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(expected_descs, migrated_desc) wo_wallet.unloadwallet() + def test_p2wsh(self): + self.log.info("Test that non-multisig P2WSH output scripts are migrated") + def_wallet = self.master_node.get_wallet_rpc(self.default_wallet_name) + + wallet = self.create_legacy_wallet("p2wsh") + + # Craft wsh(pkh(key)) + pubkey = wallet.getaddressinfo(wallet.getnewaddress())["pubkey"] + pkh_script = key_to_p2pkh_script(pubkey).hex() + wsh_pkh_script = script_to_p2wsh_script(pkh_script).hex() + wsh_pkh_addr = script_to_p2wsh(pkh_script) + + # Legacy single key scripts (i.e. pkh(key) and pk(key)) are not inserted into mapScripts + # automatically, they need to be imported directly if we want to receive to P2WSH (or P2SH) + # wrappings of such scripts. + wallet.importaddress(address=pkh_script, p2sh=False) + wallet.importaddress(address=wsh_pkh_script, p2sh=False) + + def_wallet.sendtoaddress(wsh_pkh_addr, 5) + self.generate(self.nodes[0], 6) + assert_equal(wallet.getbalances()['mine']['trusted'], 5) + + _, wallet = self.migrate_and_get_rpc("p2wsh") + + 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() + + def test_disallowed_p2wsh(self): + self.log.info("Test that P2WSH output scripts with invalid witnessScripts are not migrated and do not cause migration failure") + def_wallet = self.master_node.get_wallet_rpc(self.default_wallet_name) + + wallet = self.create_legacy_wallet("invalid_p2wsh") + + invalid_addrs = [] + + # For a P2WSH output script stored in the legacy wallet's mapScripts, both the native P2WSH + # and the P2SH-P2WSH are detected by IsMine. We need to verify that descriptors for both + # output scripts are added to the resulting descriptor wallet. + # However, this cannot be done using a multisig as wallet migration treats multisigs specially. + # Instead, this is tested by importing a wsh(pkh()) script. But importing this directly will + # insert the wsh() into setWatchOnly which means that the setWatchOnly migration ends up handling + # this case, which we do not want. + # In order to get the wsh(pkh()) into only mapScripts and not setWatchOnly, we need to utilize + # importmulti and wrap the wsh(pkh()) inside of a sh(). This will insert the sh(wsh(pkh())) into + # setWatchOnly but not the wsh(pkh()). + # Furthermore, migration should not migrate the wsh(pkh()) if the key is uncompressed. + comp_wif, comp_pubkey = generate_keypair(compressed=True, wif=True) + comp_pkh_script = key_to_p2pkh_script(comp_pubkey).hex() + comp_wsh_pkh_script = script_to_p2wsh_script(comp_pkh_script).hex() + comp_sh_wsh_pkh_script = script_to_p2sh_script(comp_wsh_pkh_script).hex() + comp_wsh_pkh_addr = script_to_p2wsh(comp_pkh_script) + + uncomp_wif, uncomp_pubkey = generate_keypair(compressed=False, wif=True) + uncomp_pkh_script = key_to_p2pkh_script(uncomp_pubkey).hex() + uncomp_wsh_pkh_script = script_to_p2wsh_script(uncomp_pkh_script).hex() + uncomp_sh_wsh_pkh_script = script_to_p2sh_script(uncomp_wsh_pkh_script).hex() + uncomp_wsh_pkh_addr = script_to_p2wsh(uncomp_pkh_script) + invalid_addrs.append(uncomp_wsh_pkh_addr) + + import_res = wallet.importmulti([ + { + "scriptPubKey": comp_sh_wsh_pkh_script, + "timestamp": "now", + "redeemscript": comp_wsh_pkh_script, + "witnessscript": comp_pkh_script, + "keys": [ + comp_wif, + ], + }, + { + "scriptPubKey": uncomp_sh_wsh_pkh_script, + "timestamp": "now", + "redeemscript": uncomp_wsh_pkh_script, + "witnessscript": uncomp_pkh_script, + "keys": [ + uncomp_wif, + ], + }, + ]) + assert_equal(import_res[0]["success"], True) + assert_equal(import_res[1]["success"], True) + + # Create a wsh(sh(pkh())) - P2SH inside of P2WSH is invalid + comp_sh_pkh_script = script_to_p2sh_script(comp_pkh_script).hex() + wsh_sh_pkh_script = script_to_p2wsh_script(comp_sh_pkh_script).hex() + wsh_sh_pkh_addr = script_to_p2wsh(comp_sh_pkh_script) + invalid_addrs.append(wsh_sh_pkh_addr) + + # Import wsh(sh(pkh())) + wallet.importaddress(address=comp_sh_pkh_script, p2sh=False) + wallet.importaddress(address=wsh_sh_pkh_script, p2sh=False) + + # Create a wsh(wsh(pkh())) - P2WSH inside of P2WSH is invalid + wsh_wsh_pkh_script = script_to_p2wsh_script(comp_wsh_pkh_script).hex() + wsh_wsh_pkh_addr = script_to_p2wsh(comp_wsh_pkh_script) + invalid_addrs.append(wsh_wsh_pkh_addr) + + # Import wsh(wsh(pkh())) + wallet.importaddress(address=wsh_wsh_pkh_script, p2sh=False) + + # The wsh(pkh()) with a compressed key is always valid, so we should see that the wallet detects it as ismine, not + # watchonly, and can provide us information about the witnessScript via "embedded" + 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 + + # The invalid addresses are invalid, so the legacy wallet should not detect them as ismine, + # nor consider them watchonly. However, because the legacy wallet has the witnessScripts/redeemScripts, + # we should see information about those in "embedded" + for addr in invalid_addrs: + addr_info = wallet.getaddressinfo(addr) + assert_equal(addr_info["ismine"], False) + assert_equal(addr_info["iswatchonly"], False) + assert "embedded" in addr_info + + # Fund those output scripts, although the invalid addresses will not have any balance. + # This behavior follows as the addresses are not ismine. + def_wallet.send([{comp_wsh_pkh_addr: 1}] + [{k: i + 1} for i, k in enumerate(invalid_addrs)]) + self.generate(self.nodes[0], 6) + bal = wallet.getbalances() + assert_equal(bal["mine"]["trusted"], 1) + assert_equal(bal["watchonly"]["trusted"], 0) + + res, wallet = self.migrate_and_get_rpc("invalid_p2wsh") + assert "watchonly_name" not in res + assert "solvables_name" not in res + + assert_equal(wallet.getbalances()["mine"]["trusted"], 1) + + # After migration, the wsh(pkh()) with a compressed key is still valid and the descriptor wallet will have + # 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. + # The descriptor wallet should not have migrated these at all, so there should additionally be no + # information in "embedded" about the witnessScripts/redeemScripts. + 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() + + def test_miniscript(self): + # It turns out that due to how signing logic works, legacy wallets that have valid miniscript witnessScripts + # and the private keys for them can still sign and spend them, even though output scripts involving them + # as a witnessScript would not be detected as ISMINE_SPENDABLE. + self.log.info("Test migration of a legacy wallet containing miniscript") + def_wallet = self.master_node.get_wallet_rpc(self.default_wallet_name) + wallet = self.create_legacy_wallet("miniscript") + + privkey, _ = generate_keypair(compressed=True, wif=True) + + # Make a descriptor where we only have some of the keys. This will be migrated to the watchonly wallet. + some_keys_priv_desc = descsum_create(f"wsh(or_b(pk({privkey}),s:pk(029ffbe722b147f3035c87cb1c60b9a5947dd49c774cc31e94773478711a929ac0)))") + some_keys_addr = self.master_node.deriveaddresses(some_keys_priv_desc)[0] + + # Make a descriptor where we have all of the keys. This will stay in the migrated wallet + all_keys_priv_desc = descsum_create(f"wsh(and_v(v:pk({privkey}),1))") + all_keys_addr = self.master_node.deriveaddresses(all_keys_priv_desc)[0] + + imp = wallet.importmulti([ + { + "desc": some_keys_priv_desc, + "timestamp": "now", + }, + { + "desc": all_keys_priv_desc, + "timestamp": "now", + } + ]) + assert_equal(imp[0]["success"], True) + assert_equal(imp[1]["success"], True) + + def_wallet.sendtoaddress(some_keys_addr, 1) + def_wallet.sendtoaddress(all_keys_addr, 1) + self.generate(self.master_node, 6) + # Check that the miniscript can be spent by the legacy wallet + send_res = wallet.send(outputs=[{some_keys_addr: 1},{all_keys_addr: 0.75}], include_watching=True, change_address=def_wallet.getnewaddress()) + assert_equal(send_res["complete"], True) + self.generate(self.old_node, 6) + assert_equal(wallet.getbalances()["watchonly"]["trusted"], 1.75) + + _, wallet = self.migrate_and_get_rpc("miniscript") + + # The miniscript with all keys should be in the migrated wallet + assert_equal(wallet.getbalances()["mine"], {"trusted": 0.75, "untrusted_pending": 0, "immature": 0}) + assert_equal(wallet.getaddressinfo(all_keys_addr)["ismine"], True) + assert_equal(wallet.getaddressinfo(some_keys_addr)["ismine"], False) + + # The miniscript with some keys should be in the watchonly wallet + assert "miniscript_watchonly" in self.master_node.listwallets() + watchonly = self.master_node.get_wallet_rpc("miniscript_watchonly") + assert_equal(watchonly.getbalances()["mine"], {"trusted": 1, "untrusted_pending": 0, "immature": 0}) + assert_equal(watchonly.getaddressinfo(some_keys_addr)["ismine"], True) + assert_equal(watchonly.getaddressinfo(all_keys_addr)["ismine"], False) + + def test_taproot(self): + # It turns out that due to how signing logic works, legacy wallets that have the private key for a Taproot + # output key will be able to sign and spend those scripts, even though they would not be detected as ISMINE_SPENDABLE. + self.log.info("Test migration of Taproot scripts") + def_wallet = self.master_node.get_wallet_rpc(self.default_wallet_name) + wallet = self.create_legacy_wallet("taproot") + + privkey, _ = generate_keypair(compressed=True, wif=True) + + rawtr_desc = descsum_create(f"rawtr({privkey})") + rawtr_addr = self.master_node.deriveaddresses(rawtr_desc)[0] + rawtr_spk = self.master_node.validateaddress(rawtr_addr)["scriptPubKey"] + tr_desc = descsum_create(f"tr({privkey})") + tr_addr = self.master_node.deriveaddresses(tr_desc)[0] + tr_spk = self.master_node.validateaddress(tr_addr)["scriptPubKey"] + tr_script_desc = descsum_create(f"tr(9ffbe722b147f3035c87cb1c60b9a5947dd49c774cc31e94773478711a929ac0,pk({privkey}))") + tr_script_addr = self.master_node.deriveaddresses(tr_script_desc)[0] + tr_script_spk = self.master_node.validateaddress(tr_script_addr)["scriptPubKey"] + + wallet.importaddress(rawtr_spk) + wallet.importaddress(tr_spk) + wallet.importaddress(tr_script_spk) + wallet.importprivkey(privkey) + + txid = def_wallet.send([{rawtr_addr: 1},{tr_addr: 2}, {tr_script_addr: 3}])["txid"] + rawtr_vout = find_vout_for_address(self.master_node, txid, rawtr_addr) + tr_vout = find_vout_for_address(self.master_node, txid, tr_addr) + tr_script_vout = find_vout_for_address(self.master_node, txid, tr_script_addr) + self.generate(self.master_node, 6) + assert_equal(wallet.getbalances()["watchonly"]["trusted"], 6) + + # Check that the rawtr can be spent by the legacy wallet + send_res = wallet.send(outputs=[{rawtr_addr: 0.5}], include_watching=True, change_address=def_wallet.getnewaddress(), inputs=[{"txid": txid, "vout": rawtr_vout}]) + assert_equal(send_res["complete"], True) + self.generate(self.old_node, 6) + assert_equal(wallet.getbalances()["watchonly"]["trusted"], 5.5) + assert_equal(wallet.getbalances()["mine"]["trusted"], 0) + + # Check that the tr() cannot be spent by the legacy wallet + send_res = wallet.send(outputs=[{def_wallet.getnewaddress(): 4}], include_watching=True, inputs=[{"txid": txid, "vout": tr_vout}, {"txid": txid, "vout": tr_script_vout}]) + assert_equal(send_res["complete"], False) + + res, wallet = self.migrate_and_get_rpc("taproot") + + # The rawtr should be migrated + assert_equal(wallet.getbalances()["mine"], {"trusted": 0.5, "untrusted_pending": 0, "immature": 0}) + assert_equal(wallet.getaddressinfo(rawtr_addr)["ismine"], True) + assert_equal(wallet.getaddressinfo(tr_addr)["ismine"], False) + assert_equal(wallet.getaddressinfo(tr_script_addr)["ismine"], False) + + # The tr() with some keys should be in the watchonly wallet + assert "taproot_watchonly" in self.master_node.listwallets() + watchonly = self.master_node.get_wallet_rpc("taproot_watchonly") + assert_equal(watchonly.getbalances()["mine"], {"trusted": 5, "untrusted_pending": 0, "immature": 0}) + assert_equal(watchonly.getaddressinfo(rawtr_addr)["ismine"], False) + assert_equal(watchonly.getaddressinfo(tr_addr)["ismine"], True) + assert_equal(watchonly.getaddressinfo(tr_script_addr)["ismine"], True) + + def test_solvable_no_privs(self): + self.log.info("Test migrating a multisig that we do not have any private keys for") + wallet = self.create_legacy_wallet("multisig_noprivs") + + _, pubkey = generate_keypair(compressed=True, wif=True) + + add_ms_res = wallet.addmultisigaddress(nrequired=1, keys=[pubkey.hex()]) + addr = add_ms_res["address"] + + # The multisig address should be ISMINE_NO but we should have the script info + addr_info = wallet.getaddressinfo(addr) + assert_equal(addr_info["ismine"], False) + assert "hex" in addr_info + + migrate_res, wallet = self.migrate_and_get_rpc("multisig_noprivs") + assert_equal(migrate_res["solvables_name"], "multisig_noprivs_solvables") + solvables = self.master_node.get_wallet_rpc(migrate_res["solvables_name"]) + + # The multisig should not be in the spendable wallet + addr_info = wallet.getaddressinfo(addr) + assert_equal(addr_info["ismine"], False) + assert "hex" not in addr_info + + # The multisig address should be in the solvables wallet + addr_info = solvables.getaddressinfo(addr) + assert_equal(addr_info["ismine"], True) + assert_equal(addr_info["solvable"], True) + assert "hex" in addr_info + def run_test(self): self.master_node = self.nodes[0] self.old_node = self.nodes[1] @@ -1101,7 +1384,11 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_blank() self.test_migrate_simple_watch_only() self.test_manual_keys_import() - + self.test_p2wsh() + self.test_disallowed_p2wsh() + self.test_miniscript() + self.test_taproot() + self.test_solvable_no_privs() if __name__ == '__main__': WalletMigrationTest(__file__).main()