Merge 12645d1dee into 65714c162c

fadf12a56c test: Add missing check for empty stderr in util tester (MarcoFalke)

Pull request description:

  Now that wine support was removed from the CI in 25b56fd9b4, it can probably be removed from the util tester as well.

  If someone really needs this, they can comment the new check out, or submit a patch to add an option/env var to silence the new check.

ACKs for top commit:
  achow101:
    ACK fadf12a56c
  i-am-yuvi:
    tACK fadf12a56c
  BrandonOdiwuor:
    Code Review ACK fadf12a56c
  ismaelsadeeq:
    Tested ACK fadf12a56c

Tree-SHA512: d9e4d7a7f724e114391070ea7f17b585a7e4c4f3221c3bf510eeb11df6ccd089b881ab5654adfef8d3a1f8fa7ec6bf5e3a3feeb0cdfe724a8f3e5a146c388e66

src/validation.cpp

@@ -1970,7 +1970,7 @@ Chainstate::Chainstate(
       m_chainman(chainman),
       m_from_snapshot_blockhash(from_snapshot_blockhash) {}
 
-const CBlockIndex* Chainstate::SnapshotBase()
+const CBlockIndex* Chainstate::SnapshotBase() const
 {
     if (!m_from_snapshot_blockhash) return nullptr;
     if (!m_cached_snapshot_base) m_cached_snapshot_base = Assert(m_chainman.m_blockman.LookupBlockIndex(*m_from_snapshot_blockhash));
@@ -5257,7 +5257,7 @@ bool ChainstateManager::ShouldCheckBlockIndex() const
     return true;
 }
 
-void ChainstateManager::CheckBlockIndex()
+void ChainstateManager::CheckBlockIndex() const
 {
     if (!ShouldCheckBlockIndex()) {
         return;
@@ -5282,7 +5282,7 @@ void ChainstateManager::CheckBlockIndex()
     assert(m_best_header);
     best_hdr_chain.SetTip(*m_best_header);
 
-    std::multimap<CBlockIndex*,CBlockIndex*> forward;
+    std::multimap<const CBlockIndex*, const CBlockIndex*> forward;
     for (auto& [_, block_index] : m_blockman.m_block_index) {
         // Only save indexes in forward that are not part of the best header chain.
         if (!best_hdr_chain.Contains(&block_index)) {
@@ -5293,27 +5293,27 @@ void ChainstateManager::CheckBlockIndex()
     }
     assert(forward.size() + best_hdr_chain.Height() + 1 == m_blockman.m_block_index.size());
 
-    CBlockIndex* pindex = best_hdr_chain[0];
+    const CBlockIndex* pindex = best_hdr_chain[0];
     assert(pindex);
     // Iterate over the entire block tree, using depth-first search.
     // Along the way, remember whether there are blocks on the path from genesis
     // block being explored which are the first to have certain properties.
     size_t nNodes = 0;
     int nHeight = 0;
-    CBlockIndex* pindexFirstInvalid = nullptr; // Oldest ancestor of pindex which is invalid.
+    const CBlockIndex* pindexFirstInvalid = nullptr;              // Oldest ancestor of pindex which is invalid.
-    CBlockIndex* pindexFirstMissing = nullptr; // Oldest ancestor of pindex which does not have BLOCK_HAVE_DATA, since assumeutxo snapshot if used.
+    const CBlockIndex* pindexFirstMissing = nullptr;              // Oldest ancestor of pindex which does not have BLOCK_HAVE_DATA, since assumeutxo snapshot if used.
-    CBlockIndex* pindexFirstNeverProcessed = nullptr; // Oldest ancestor of pindex for which nTx == 0, since assumeutxo snapshot if used.
+    const CBlockIndex* pindexFirstNeverProcessed = nullptr;       // Oldest ancestor of pindex for which nTx == 0, since assumeutxo snapshot if used.
-    CBlockIndex* pindexFirstNotTreeValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_TREE (regardless of being valid or not).
+    const CBlockIndex* pindexFirstNotTreeValid = nullptr;         // Oldest ancestor of pindex which does not have BLOCK_VALID_TREE (regardless of being valid or not).
-    CBlockIndex* pindexFirstNotTransactionsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_TRANSACTIONS (regardless of being valid or not), since assumeutxo snapshot if used.
+    const CBlockIndex* pindexFirstNotTransactionsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_TRANSACTIONS (regardless of being valid or not), since assumeutxo snapshot if used.
-    CBlockIndex* pindexFirstNotChainValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_CHAIN (regardless of being valid or not), since assumeutxo snapshot if used.
+    const CBlockIndex* pindexFirstNotChainValid = nullptr;        // Oldest ancestor of pindex which does not have BLOCK_VALID_CHAIN (regardless of being valid or not), since assumeutxo snapshot if used.
-    CBlockIndex* pindexFirstNotScriptsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_SCRIPTS (regardless of being valid or not), since assumeutxo snapshot if used.
+    const CBlockIndex* pindexFirstNotScriptsValid = nullptr;      // Oldest ancestor of pindex which does not have BLOCK_VALID_SCRIPTS (regardless of being valid or not), since assumeutxo snapshot if used.
 
     // After checking an assumeutxo snapshot block, reset pindexFirst pointers
     // to earlier blocks that have not been downloaded or validated yet, so
     // checks for later blocks can assume the earlier blocks were validated and
     // be stricter, testing for more requirements.
     const CBlockIndex* snap_base{GetSnapshotBaseBlock()};
-    CBlockIndex *snap_first_missing{}, *snap_first_notx{}, *snap_first_notv{}, *snap_first_nocv{}, *snap_first_nosv{};
+    const CBlockIndex *snap_first_missing{}, *snap_first_notx{}, *snap_first_notv{}, *snap_first_nocv{}, *snap_first_nosv{};
     auto snap_update_firsts = [&] {
         if (pindex == snap_base) {
             std::swap(snap_first_missing, pindexFirstMissing);
@@ -5453,7 +5453,7 @@ void ChainstateManager::CheckBlockIndex()
                         // pindex only needs to be added if it is an ancestor of
                         // the snapshot that is being validated.
                         if (c == &ActiveChainstate() || snap_base->GetAncestor(pindex->nHeight) == pindex) {
-                            assert(c->setBlockIndexCandidates.count(pindex));
+                            assert(c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex)));
                         }
                     }
                     // If some parent is missing, then it could be that this block was in
@@ -5461,11 +5461,11 @@ void ChainstateManager::CheckBlockIndex()
                     // In this case it must be in m_blocks_unlinked -- see test below.
                 }
             } else { // If this block sorts worse than the current tip or some ancestor's block has never been seen, it cannot be in setBlockIndexCandidates.
-                assert(c->setBlockIndexCandidates.count(pindex) == 0);
+                assert(!c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex)));
             }
         }
         // Check whether this block is in m_blocks_unlinked.
-        std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> rangeUnlinked = m_blockman.m_blocks_unlinked.equal_range(pindex->pprev);
+        auto rangeUnlinked{m_blockman.m_blocks_unlinked.equal_range(pindex->pprev)};
         bool foundInUnlinked = false;
         while (rangeUnlinked.first != rangeUnlinked.second) {
             assert(rangeUnlinked.first->first == pindex->pprev);
@@ -5494,7 +5494,7 @@ void ChainstateManager::CheckBlockIndex()
             // setBlockIndexCandidates, then it must be in m_blocks_unlinked.
             for (auto c : GetAll()) {
                 const bool is_active = c == &ActiveChainstate();
-                if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && c->setBlockIndexCandidates.count(pindex) == 0) {
+                if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && !c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex))) {
                     if (pindexFirstInvalid == nullptr) {
                         if (is_active || snap_base->GetAncestor(pindex->nHeight) == pindex) {
                             assert(foundInUnlinked);
@@ -5509,7 +5509,7 @@ void ChainstateManager::CheckBlockIndex()
 
         // Try descending into the first subnode. Always process forks first and the best header chain after.
         snap_update_firsts();
-        std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> range = forward.equal_range(pindex);
+        auto range{forward.equal_range(pindex)};
         if (range.first != range.second) {
             // A subnode not part of the best header chain was found.
             pindex = range.first->second;
@@ -5538,7 +5538,7 @@ void ChainstateManager::CheckBlockIndex()
             // Find our parent.
             CBlockIndex* pindexPar = pindex->pprev;
             // Find which child we just visited.
-            std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> rangePar = forward.equal_range(pindexPar);
+            auto rangePar{forward.equal_range(pindexPar)};
             while (rangePar.first->second != pindex) {
                 assert(rangePar.first != rangePar.second); // Our parent must have at least the node we're coming from as child.
                 rangePar.first++;
@@ -5657,6 +5657,18 @@ std::vector<Chainstate*> ChainstateManager::GetAll()
     return out;
 }
 
+std::vector<const Chainstate*> ChainstateManager::GetAll() const
+{
+    LOCK(::cs_main);
+    std::vector<const Chainstate*> out;
+
+    for (const Chainstate* cs : {m_ibd_chainstate.get(), m_snapshot_chainstate.get()}) {
+        if (this->IsUsable(cs)) out.push_back(cs);
+    }
+
+    return out;
+}
+
 Chainstate& ChainstateManager::InitializeChainstate(CTxMemPool* mempool)
 {
     AssertLockHeld(::cs_main);

src/validation.h

@@ -532,7 +532,7 @@ protected:
     bool m_disabled GUARDED_BY(::cs_main) {false};
 
     //! Cached result of LookupBlockIndex(*m_from_snapshot_blockhash)
-    const CBlockIndex* m_cached_snapshot_base GUARDED_BY(::cs_main) {nullptr};
+    mutable const CBlockIndex* m_cached_snapshot_base GUARDED_BY(::cs_main){nullptr};
 
 public:
     //! Reference to a BlockManager instance which itself is shared across all
@@ -596,7 +596,7 @@ public:
      *
      * nullptr if this chainstate was not created from a snapshot.
      */
-    const CBlockIndex* SnapshotBase() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    const CBlockIndex* SnapshotBase() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     /**
      * The set of all CBlockIndex entries that have as much work as our current
@@ -985,7 +985,7 @@ public:
      *
      * By default this only executes fully when using the Regtest chain; see: m_options.check_block_index.
      */
-    void CheckBlockIndex();
+    void CheckBlockIndex() const;
 
     /**
      * Alias for ::cs_main.
@@ -1078,6 +1078,7 @@ public:
 
     //! Get all chainstates currently being used.
     std::vector<Chainstate*> GetAll();
+    std::vector<const Chainstate*> GetAll() const;
 
     //! Construct and activate a Chainstate on the basis of UTXO snapshot data.
     //!

test/functional/rpc_psbt.py

@@ -111,7 +111,7 @@ class PSBTTest(BitcoinTestFramework):
         # Mine a transaction that credits the offline address
         offline_addr = offline_node.getnewaddress(address_type="bech32m")
         online_addr = w2.getnewaddress(address_type="bech32m")
-        wonline.importaddress(offline_addr, "", False)
+        wonline.importaddress(offline_addr, label="", rescan=False)
         mining_wallet = mining_node.get_wallet_rpc(self.default_wallet_name)
         mining_wallet.sendtoaddress(address=offline_addr, amount=1.0)
         self.generate(mining_node, nblocks=1, sync_fun=lambda: self.sync_all([online_node, mining_node]))
@@ -312,9 +312,9 @@ class PSBTTest(BitcoinTestFramework):
         wmulti = self.nodes[2].get_wallet_rpc('wmulti')
 
         # Create all the addresses
-        p2sh = wmulti.addmultisigaddress(2, [pubkey0, pubkey1, pubkey2], "", "legacy")['address']
+        p2sh = wmulti.addmultisigaddress(2, [pubkey0, pubkey1, pubkey2], label="", address_type="legacy")["address"]
-        p2wsh = wmulti.addmultisigaddress(2, [pubkey0, pubkey1, pubkey2], "", "bech32")['address']
+        p2wsh = wmulti.addmultisigaddress(2, [pubkey0, pubkey1, pubkey2], label="", address_type="bech32")["address"]
-        p2sh_p2wsh = wmulti.addmultisigaddress(2, [pubkey0, pubkey1, pubkey2], "", "p2sh-segwit")['address']
+        p2sh_p2wsh = wmulti.addmultisigaddress(2, [pubkey0, pubkey1, pubkey2], label="", address_type="p2sh-segwit")["address"]
         p2wpkh = self.nodes[1].getnewaddress("", "bech32")
         p2pkh = self.nodes[1].getnewaddress("", "legacy")
         p2sh_p2wpkh = self.nodes[1].getnewaddress("", "p2sh-segwit")

test/functional/test_framework/test_node.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-# Copyright (c) 2017-2022 The Bitcoin Core developers
+# Copyright (c) 2017-present The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Class for bitcoind node under test"""
@@ -209,7 +209,7 @@ class TestNode():
     def __getattr__(self, name):
         """Dispatches any unrecognised messages to the RPC connection or a CLI instance."""
         if self.use_cli:
-            return getattr(RPCOverloadWrapper(self.cli, True), name)
+            return getattr(RPCOverloadWrapper(self.cli), name)
         else:
             assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
             return getattr(RPCOverloadWrapper(self.rpc), name)
@@ -374,7 +374,7 @@ class TestNode():
 
     def get_wallet_rpc(self, wallet_name):
         if self.use_cli:
-            return RPCOverloadWrapper(self.cli("-rpcwallet={}".format(wallet_name)), True)
+            return RPCOverloadWrapper(self.cli("-rpcwallet={}".format(wallet_name)))
         else:
             assert self.rpc_connected and self.rpc, self._node_msg("RPC not connected")
             wallet_path = "wallet/{}".format(urllib.parse.quote(wallet_name))
@@ -925,17 +925,13 @@ class TestNodeCLI():
             return cli_stdout.rstrip("\n")
 
 class RPCOverloadWrapper():
-    def __init__(self, rpc, cli=False):
+    def __init__(self, rpc):
         self.rpc = rpc
-        self.is_cli = cli
 
     def __getattr__(self, name):
         return getattr(self.rpc, name)
 
-    def createwallet_passthrough(self, *args, **kwargs):
+    def importprivkey(self, privkey, *, label=None, rescan=None):
-        return self.__getattr__("createwallet")(*args, **kwargs)
-
-    def importprivkey(self, privkey, label=None, rescan=None):
         wallet_info = self.getwalletinfo()
         if 'descriptors' not in wallet_info or ('descriptors' in wallet_info and not wallet_info['descriptors']):
             return self.__getattr__('importprivkey')(privkey, label, rescan)
@@ -943,13 +939,13 @@ class RPCOverloadWrapper():
         req = [{
             'desc': desc,
             'timestamp': 0 if rescan else 'now',
-            'label': label if label else ''
+            'label': label if label else '',
         }]
         import_res = self.importdescriptors(req)
         if not import_res[0]['success']:
             raise JSONRPCException(import_res[0]['error'])
 
-    def addmultisigaddress(self, nrequired, keys, label=None, address_type=None):
+    def addmultisigaddress(self, nrequired, keys, *, label=None, address_type=None):
         wallet_info = self.getwalletinfo()
         if 'descriptors' not in wallet_info or ('descriptors' in wallet_info and not wallet_info['descriptors']):
             return self.__getattr__('addmultisigaddress')(nrequired, keys, label, address_type)
@@ -957,14 +953,14 @@ class RPCOverloadWrapper():
         req = [{
             'desc': cms['descriptor'],
             'timestamp': 0,
-            'label': label if label else ''
+            'label': label if label else '',
         }]
         import_res = self.importdescriptors(req)
         if not import_res[0]['success']:
             raise JSONRPCException(import_res[0]['error'])
         return cms
 
-    def importpubkey(self, pubkey, label=None, rescan=None):
+    def importpubkey(self, pubkey, *, label=None, rescan=None):
         wallet_info = self.getwalletinfo()
         if 'descriptors' not in wallet_info or ('descriptors' in wallet_info and not wallet_info['descriptors']):
             return self.__getattr__('importpubkey')(pubkey, label, rescan)
@@ -972,13 +968,13 @@ class RPCOverloadWrapper():
         req = [{
             'desc': desc,
             'timestamp': 0 if rescan else 'now',
-            'label': label if label else ''
+            'label': label if label else '',
         }]
         import_res = self.importdescriptors(req)
         if not import_res[0]['success']:
             raise JSONRPCException(import_res[0]['error'])
 
-    def importaddress(self, address, label=None, rescan=None, p2sh=None):
+    def importaddress(self, address, *, label=None, rescan=None, p2sh=None):
         wallet_info = self.getwalletinfo()
         if 'descriptors' not in wallet_info or ('descriptors' in wallet_info and not wallet_info['descriptors']):
             return self.__getattr__('importaddress')(address, label, rescan, p2sh)
@@ -992,13 +988,13 @@ class RPCOverloadWrapper():
         reqs = [{
             'desc': desc,
             'timestamp': 0 if rescan else 'now',
-            'label': label if label else ''
+            'label': label if label else '',
         }]
         if is_hex and p2sh:
             reqs.append({
                 'desc': descsum_create('p2sh(raw(' + address + '))'),
                 'timestamp': 0 if rescan else 'now',
-                'label': label if label else ''
+                'label': label if label else '',
             })
         import_res = self.importdescriptors(reqs)
         for res in import_res:

test/functional/wallet_address_types.py

@@ -344,7 +344,7 @@ class AddressTypeTest(BitcoinTestFramework):
         self.test_address(3, self.nodes[3].getrawchangeaddress(), multisig=False, typ='bech32')
 
         self.log.info('test invalid address type arguments')
-        assert_raises_rpc_error(-5, "Unknown address type ''", self.nodes[3].addmultisigaddress, 2, [compressed_1, compressed_2], None, '')
+        assert_raises_rpc_error(-5, "Unknown address type ''", self.nodes[3].addmultisigaddress, 2, [compressed_1, compressed_2], address_type="")
         assert_raises_rpc_error(-5, "Unknown address type ''", self.nodes[3].getnewaddress, None, '')
         assert_raises_rpc_error(-5, "Unknown address type ''", self.nodes[3].getrawchangeaddress, '')
         assert_raises_rpc_error(-5, "Unknown address type 'bech23'", self.nodes[3].getrawchangeaddress, 'bech23')

test/functional/wallet_backwards_compatibility.py

@@ -26,11 +26,12 @@ from test_framework.util import (
     assert_raises_rpc_error,
 )
 
+LAST_KEYPOOL_INDEX = 9 # Index of the last derived address with the keypool size of 10
 
 class BackwardsCompatibilityTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
-        self.num_nodes = 10
+        self.num_nodes = 8
         # Add new version after each release:
         self.extra_args = [
             ["-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # Pre-release: use to mine blocks. noban for immediate tx relay
@@ -38,11 +39,9 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v25.0
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v24.0.1
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v23.0
-            ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v22.0
+            ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1", f"-keypool={LAST_KEYPOOL_INDEX + 1}"], # v22.0
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.21.0
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.20.1
-            ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.19.1
-            ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1"], # v0.18.1
         ]
         self.wallet_names = [self.default_wallet_name]
 
@@ -60,8 +59,6 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
             220000,
             210000,
             200100,
-            190100,
-            180100,
         ])
 
         self.start_nodes()
@@ -85,21 +82,98 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         node_major, _, _ = self.split_version(node)
         return node_major >= major
 
+    def test_v22_inactivehdchain_path(self):
+        self.log.info("Testing inactive hd chain bad derivation path cleanup")
+        # 0.21.x and 22.x would both produce bad derivation paths when topping up an inactive hd chain
+        # Make sure that this is being automatically cleaned up by migration
+        node_master = self.nodes[1]
+        node_v22 = self.nodes[self.num_nodes - 5]
+        wallet_name = "bad_deriv_path"
+        node_v22.createwallet(wallet_name=wallet_name, descriptors=False)
+        bad_deriv_wallet = node_v22.get_wallet_rpc(wallet_name)
+
+        # Make a dump of the wallet to get an unused address
+        dump_path = node_v22.wallets_path / f"{wallet_name}.dump"
+        bad_deriv_wallet.dumpwallet(dump_path)
+        addr = None
+        seed = None
+        with open(dump_path, encoding="utf8") as f:
+            for line in f:
+                if f"hdkeypath=m/0'/0'/{LAST_KEYPOOL_INDEX}'" in line:
+                    addr = line.split(" ")[4].split("=")[1]
+                elif " hdseed=1 " in line:
+                    seed = line.split(" ")[0]
+        assert addr is not None
+        assert seed is not None
+        # Rotate seed and unload
+        bad_deriv_wallet.sethdseed()
+        bad_deriv_wallet.unloadwallet()
+        # Receive at addr to trigger inactive chain topup on next load
+        self.nodes[0].sendtoaddress(addr, 1)
+        self.generate(self.nodes[0], 1, sync_fun=self.no_op)
+        self.sync_all(nodes=[self.nodes[0], node_master, node_v22])
+        node_v22.loadwallet(wallet_name)
+
+        # Dump again to find bad hd keypath
+        bad_deriv_path = f"m/0'/0'/{LAST_KEYPOOL_INDEX}'/0'/0'/{LAST_KEYPOOL_INDEX + 1}'"
+        good_deriv_path = f"m/0h/0h/{LAST_KEYPOOL_INDEX + 1}h"
+        os.unlink(dump_path)
+        bad_deriv_wallet.dumpwallet(dump_path)
+        bad_path_addr = None
+        with open(dump_path, encoding="utf8") as f:
+            for line in f:
+                if f"hdkeypath={bad_deriv_path}" in line:
+                    bad_path_addr = line.split(" ")[4].split("=")[1]
+        assert bad_path_addr is not None
+        assert_equal(bad_deriv_wallet.getaddressinfo(bad_path_addr)["hdkeypath"], bad_deriv_path)
+
+        # Verify that this bad derivation path addr is actually at m/0'/0'/10' by making a new wallet with the same seed but larger keypool
+        node_v22.createwallet(wallet_name="path_verify", descriptors=False, blank=True)
+        verify_wallet = node_v22.get_wallet_rpc("path_verify")
+        verify_wallet.sethdseed(True, seed)
+        # Bad addr is after keypool, so need to generate it by refilling
+        verify_wallet.keypoolrefill(LAST_KEYPOOL_INDEX + 2)
+        assert_equal(verify_wallet.getaddressinfo(bad_path_addr)["hdkeypath"], good_deriv_path.replace("h", "'"))
+
+        # Migrate with master
+        # Since all keymeta records are now deleted after migration, the derivation path
+        # should now be correct as it is derived on-the-fly from the inactive hd chain's descriptor
+        backup_path = node_v22.wallets_path / f"{wallet_name}.bak"
+        bad_deriv_wallet.backupwallet(backup_path)
+        wallet_dir_master = os.path.join(node_master.wallets_path, wallet_name)
+        os.makedirs(wallet_dir_master, exist_ok=True)
+        shutil.copy(backup_path, os.path.join(wallet_dir_master, "wallet.dat"))
+        node_master.migratewallet(wallet_name)
+        bad_deriv_wallet_master = node_master.get_wallet_rpc(wallet_name)
+        assert_equal(bad_deriv_wallet_master.getaddressinfo(bad_path_addr)["hdkeypath"], good_deriv_path)
+        bad_deriv_wallet_master.unloadwallet()
+
+        # If we have sqlite3, verify that there are no keymeta records
+        try:
+            import sqlite3
+            wallet_db = node_master.wallets_path / wallet_name / "wallet.dat"
+            conn = sqlite3.connect(wallet_db)
+            with conn:
+                # Retrieve all records that have the "keymeta" prefix. The remaining key data varies for each record.
+                keymeta_rec = conn.execute("SELECT value FROM main where key >= x'076b65796d657461' AND key < x'076b65796d657462'").fetchone()
+                assert_equal(keymeta_rec, None)
+            conn.close()
+        except ImportError:
+            self.log.warning("sqlite3 module not available, skipping lack of keymeta records check")
+
     def run_test(self):
         node_miner = self.nodes[0]
         node_master = self.nodes[1]
-        node_v21 = self.nodes[self.num_nodes - 4]
+        node_v21 = self.nodes[self.num_nodes - 2]
-        node_v18 = self.nodes[self.num_nodes - 1]
+        node_v20 = self.nodes[self.num_nodes - 1] # bdb only
 
         legacy_nodes = self.nodes[2:] # Nodes that support legacy wallets
-        legacy_only_nodes = self.nodes[-3:] # Nodes that only support legacy wallets
+        descriptors_nodes = self.nodes[2:-1] # Nodes that support descriptor wallets
-        descriptors_nodes = self.nodes[2:-3] # Nodes that support descriptor wallets
 
         self.generatetoaddress(node_miner, COINBASE_MATURITY + 1, node_miner.getnewaddress())
 
         # Sanity check the test framework:
-        res = node_v18.getblockchaininfo()
+        assert_equal(node_v20.getblockchaininfo()["blocks"], COINBASE_MATURITY + 1)
-        assert_equal(res['blocks'], COINBASE_MATURITY + 1)
 
         self.log.info("Test wallet backwards compatibility...")
         # Create a number of wallets and open them in older versions:
@@ -206,13 +280,11 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
                         )
 
         # Check that descriptor wallets don't work on legacy only nodes
-        self.log.info("Test descriptor wallet incompatibility on:")
+        self.log.info("Test descriptor wallet incompatibility on v0.20")
-        for node in legacy_only_nodes:
+        # Descriptor wallets appear to be corrupted wallets to old software
-            self.log.info(f"- {node.version}")
+        assert self.major_version_equals(node_v20, 20)
-            # Descriptor wallets appear to be corrupted wallets to old software
+        for wallet_name in ["w1", "w2", "w3"]:
-            assert self.major_version_less_than(node, 21)
+            assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node_v20.loadwallet, wallet_name)
-            for wallet_name in ["w1", "w2", "w3"]:
-                assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node.loadwallet, wallet_name)
 
         # w1 cannot be opened by 0.21 since it contains a taproot descriptor
         self.log.info("Test that 0.21 cannot open wallet containing tr() descriptors")
@@ -308,5 +380,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
             # Legacy wallets are no longer supported. Trying to load these should result in an error
             assert_raises_rpc_error(-18, "The wallet appears to be a Legacy wallet, please use the wallet migration tool (migratewallet RPC)", node_master.restorewallet, wallet_name, backup_path)
 
+        self.test_v22_inactivehdchain_path()
+
 if __name__ == '__main__':
     BackwardsCompatibilityTest(__file__).main()

test/functional/wallet_fundrawtransaction.py

@@ -192,7 +192,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         watchonly_address = self.nodes[0].getnewaddress()
         watchonly_pubkey = self.nodes[0].getaddressinfo(watchonly_address)["pubkey"]
         self.watchonly_amount = Decimal(200)
-        wwatch.importpubkey(watchonly_pubkey, "", True)
+        wwatch.importpubkey(watchonly_pubkey, label="", rescan=True)
         self.watchonly_utxo = self.create_outpoints(self.nodes[0], outputs=[{watchonly_address: self.watchonly_amount}])[0]
 
         # Lock UTXO so nodes[0] doesn't accidentally spend it

test/functional/wallet_labels.py

@@ -49,7 +49,7 @@ class WalletLabelsTest(BitcoinTestFramework):
         assert_equal(response[0]['error']['message'], "Invalid label name")
 
         for rpc_call in rpc_calls:
-            assert_raises_rpc_error(-11, "Invalid label name", *rpc_call, "*")
+            assert_raises_rpc_error(-11, "Invalid label name", *rpc_call, label="*")
 
     def run_test(self):
         # Check that there's no UTXO on the node

test/get_previous_releases.py

@@ -24,34 +24,7 @@ SHA256_SUMS = {
     "d86fc90824a85c38b25c8488115178d5785dbc975f5ff674f9f5716bc8ad6e65": {"tag": "v0.14.3", "tarball": "bitcoin-0.14.3-arm-linux-gnueabihf.tar.gz"},
     "1b0a7408c050e3d09a8be8e21e183ef7ee570385dc41216698cc3ab392a484e7": {"tag": "v0.14.3", "tarball": "bitcoin-0.14.3-osx64.tar.gz"},
     "706e0472dbc933ed2757650d54cbcd780fd3829ebf8f609b32780c7eedebdbc9": {"tag": "v0.14.3", "tarball": "bitcoin-0.14.3-x86_64-linux-gnu.tar.gz"},
-    #
+
-    "d40f18b4e43c6e6370ef7db9131f584fbb137276ec2e3dba67a4b267f81cb644": {"tag": "v0.15.2", "tarball": "bitcoin-0.15.2-aarch64-linux-gnu.tar.gz"},
-    "54fb877a148a6ad189a1e1ab1ff8b11181e58ff2aaf430da55b3fd46ae549a6b": {"tag": "v0.15.2", "tarball": "bitcoin-0.15.2-arm-linux-gnueabihf.tar.gz"},
-    "87e9340ff3d382d543b2b69112376077f0c8b4f7450d372e83b68f5a1e22b2df": {"tag": "v0.15.2", "tarball": "bitcoin-0.15.2-osx64.tar.gz"},
-    "566be44190fd76daa01f13d428939dadfb8e3daacefc8fa17f433cad28f73bd5": {"tag": "v0.15.2", "tarball": "bitcoin-0.15.2-x86_64-linux-gnu.tar.gz"},
-    #
-    "0768c6c15caffbaca6524824c9563b42c24f70633c681c2744649158aa3fd484": {"tag": "v0.16.3", "tarball": "bitcoin-0.16.3-aarch64-linux-gnu.tar.gz"},
-    "fb2818069854a6ad20ea03b28b55dbd35d8b1f7d453e90b83eace5d0098a2a87": {"tag": "v0.16.3", "tarball": "bitcoin-0.16.3-arm-linux-gnueabihf.tar.gz"},
-    "78c3bff3b619a19aed575961ea43cc9e142959218835cf51aede7f0b764fc25d": {"tag": "v0.16.3", "tarball": "bitcoin-0.16.3-osx64.tar.gz"},
-    "5d422a9d544742bc0df12427383f9c2517433ce7b58cf672b9a9b17c2ef51e4f": {"tag": "v0.16.3", "tarball": "bitcoin-0.16.3-x86_64-linux-gnu.tar.gz"},
-    #
-    "5a6b35d1a348a402f2d2d6ab5aed653a1a1f13bc63aaaf51605e3501b0733b7a": {"tag": "v0.17.2", "tarball": "bitcoin-0.17.2-aarch64-linux-gnu.tar.gz"},
-    "d1913a5d19c8e8da4a67d1bd5205d03c8614dfd2e02bba2fe3087476643a729e": {"tag": "v0.17.2", "tarball": "bitcoin-0.17.2-arm-linux-gnueabihf.tar.gz"},
-    "a783ba20706dbfd5b47fbedf42165fce70fbbc7d78003305d964f6b3da14887f": {"tag": "v0.17.2", "tarball": "bitcoin-0.17.2-osx64.tar.gz"},
-    "943f9362b9f11130177839116f48f809d83478b4c28591d486ee9a7e35179da6": {"tag": "v0.17.2", "tarball": "bitcoin-0.17.2-x86_64-linux-gnu.tar.gz"},
-    #
-    "88f343af72803b851c7da13874cc5525026b0b55e63e1b5e1298390c4688adc6": {"tag": "v0.18.1", "tarball": "bitcoin-0.18.1-aarch64-linux-gnu.tar.gz"},
-    "cc7d483e4b20c5dabd4dcaf304965214cf4934bcc029ca99cbc9af00d3771a1f": {"tag": "v0.18.1", "tarball": "bitcoin-0.18.1-arm-linux-gnueabihf.tar.gz"},
-    "b7bbcee7a7540f711b171d6981f939ca8482005fde22689bc016596d80548bb1": {"tag": "v0.18.1", "tarball": "bitcoin-0.18.1-osx64.tar.gz"},
-    "425ee5ec631ae8da71ebc1c3f5c0269c627cf459379b9b030f047107a28e3ef8": {"tag": "v0.18.1", "tarball": "bitcoin-0.18.1-riscv64-linux-gnu.tar.gz"},
-    "600d1db5e751fa85903e935a01a74f5cc57e1e7473c15fd3e17ed21e202cfe5a": {"tag": "v0.18.1", "tarball": "bitcoin-0.18.1-x86_64-linux-gnu.tar.gz"},
-    #
-    "3a80431717842672df682bdb619e66523b59541483297772a7969413be3502ff": {"tag": "v0.19.1", "tarball": "bitcoin-0.19.1-aarch64-linux-gnu.tar.gz"},
-    "657f28213823d240dd3324d14829702f9ad6f0710f8bdd1c379cb3c447197f48": {"tag": "v0.19.1", "tarball": "bitcoin-0.19.1-arm-linux-gnueabihf.tar.gz"},
-    "1ae1b87de26487075cd2fd22e0d4ead87d969bd55c44f2f1d873ecdc6147ebb3": {"tag": "v0.19.1", "tarball": "bitcoin-0.19.1-osx64.tar.gz"},
-    "aa7a9563b48aa79252c8e7b6a41c07a5441bd9f14c5e4562cc72720ea6cb0ee5": {"tag": "v0.19.1", "tarball": "bitcoin-0.19.1-riscv64-linux-gnu.tar.gz"},
-    "5fcac9416e486d4960e1a946145566350ca670f9aaba99de6542080851122e4c": {"tag": "v0.19.1", "tarball": "bitcoin-0.19.1-x86_64-linux-gnu.tar.gz"},
-    #
     "60c93e3462c303eb080be7cf623f1a7684b37fd47a018ad3848bc23e13c84e1c": {"tag": "v0.20.1", "tarball": "bitcoin-0.20.1-aarch64-linux-gnu.tar.gz"},
     "55b577e0fb306fb429d4be6c9316607753e8543e5946b542d75d876a2f08654c": {"tag": "v0.20.1", "tarball": "bitcoin-0.20.1-arm-linux-gnueabihf.tar.gz"},
     "b9024dde373ea7dad707363e07ec7e265383204127539ae0c234bff3a61da0d1": {"tag": "v0.20.1", "tarball": "bitcoin-0.20.1-osx64.tar.gz"},

test/util/test_runner.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 # Copyright 2014 BitPay Inc.
-# Copyright 2016-2017 The Bitcoin Core developers
+# Copyright 2016-present The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test framework for bitcoin utils.
@@ -155,15 +155,16 @@ def bctest(testDir, testObj, buildenv):
 
     if "error_txt" in testObj:
         want_error = testObj["error_txt"]
-        # Compare error text
+        # A partial match instead of an exact match makes writing tests easier
-        # TODO: ideally, we'd compare the strings exactly and also assert
+        # and should be sufficient.
-        # That stderr is empty if no errors are expected. However, bitcoin-tx
-        # emits DISPLAY errors when running as a windows application on
-        # linux through wine. Just assert that the expected error text appears
-        # somewhere in stderr.
         if want_error not in res.stderr:
             logging.error(f"Error mismatch:\nExpected: {want_error}\nReceived: {res.stderr.rstrip()}\nres: {str(res)}")
             raise Exception
+    else:
+        if res.stderr:
+            logging.error(f"Unexpected error received: {res.stderr.rstrip()}\nres: {str(res)}")
+            raise Exception
+
 
 def parse_output(a, fmt):
     """Parse the output according to specified format.