Merge #19054: wallet: Skip hdKeypath of 'm' when determining inactive hd seeds

951bca6 tests: feature_backwards_compatibility.py test 0.16 up/downgrade (Andrew Chow)
3a03a11 Skip hdKeypath of 'm' (Andrew Chow)

Pull request description:

  Previously the seed was stored with keypath 'm' so we need to skip this as well when determining inactive seeds.

  Fixes #19051

ACKs for top commit:
  Sjors:
    ACK 951bca6
  instagibbs:
    re-utACK bitcoin/bitcoin@951bca6
  ryanofsky:
    Code review ACK 951bca6. No significant changes since last review, just updated comment and some test tweaks

Tree-SHA512: 930f77e7097c9cf4f1012e540bd2b1a72fd279262517f10c1531b2ad48c632ef95e0dd4edea81bcc3b3db306479d34e5e79e5d6c4ed31dfa4b77a4231436436e

Author: MarcoFalke

src/wallet/walletdb.cpp

@@ -439,10 +439,11 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
             // Extract some CHDChain info from this metadata if it has any
             if (keyMeta.nVersion >= CKeyMetadata::VERSION_WITH_HDDATA && !keyMeta.hd_seed_id.IsNull() && keyMeta.hdKeypath.size() > 0) {
                 // Get the path from the key origin or from the path string
-                // Not applicable when path is "s" as that indicates a seed
+                // Not applicable when path is "s" or "m" as those indicate a seed
+                // See https://github.com/bitcoin/bitcoin/pull/12924
                 bool internal = false;
                 uint32_t index = 0;
-                if (keyMeta.hdKeypath != "s") {
+                if (keyMeta.hdKeypath != "s" && keyMeta.hdKeypath != "m") {
                     std::vector<uint32_t> path;
                     if (keyMeta.has_key_origin) {
                         // We have a key origin, so pull it from its path vector

test/functional/feature_backwards_compatibility.py

@@ -319,6 +319,15 @@ def run_test(self):
         info = wallet.getwalletinfo()
         assert info['keypoolsize'] == 1
 
+        # Create upgrade wallet in v0.16
+        self.stop_node(-1)
+        self.start_node(-1, extra_args=["-wallet=u1_v16"])
+        wallet = node_v16.get_wallet_rpc("u1_v16")
+        v16_addr = wallet.getnewaddress('', "bech32")
+        v16_info = wallet.validateaddress(v16_addr)
+        v16_pubkey = v16_info['pubkey']
+        self.stop_node(-1)
+
         self.log.info("Test wallet upgrade path...")
         # u1: regular wallet, created with v0.17
         node_v17.rpc.createwallet(wallet_name="u1_v17")
@@ -328,6 +337,30 @@ def run_test(self):
         hdkeypath = v17_info["hdkeypath"]
         pubkey = v17_info["pubkey"]
 
+        # Copy the 0.16 wallet to the last Bitcoin Core version and open it:
+        shutil.copyfile(
+            os.path.join(node_v16_wallets_dir, "wallets/u1_v16"),
+            os.path.join(node_master_wallets_dir, "u1_v16")
+        )
+        load_res = node_master.loadwallet("u1_v16")
+        # Make sure this wallet opens without warnings. See https://github.com/bitcoin/bitcoin/pull/19054
+        assert_equal(load_res['warning'], '')
+        wallet = node_master.get_wallet_rpc("u1_v16")
+        info = wallet.getaddressinfo(v16_addr)
+        descriptor = "wpkh([" + info["hdmasterfingerprint"] + hdkeypath[1:] + "]" + v16_pubkey + ")"
+        assert_equal(info["desc"], descsum_create(descriptor))
+
+        # Now copy that same wallet back to 0.16 to make sure no automatic upgrade breaks it
+        os.remove(os.path.join(node_v16_wallets_dir, "wallets/u1_v16"))
+        shutil.copyfile(
+            os.path.join(node_master_wallets_dir, "u1_v16"),
+            os.path.join(node_v16_wallets_dir, "wallets/u1_v16")
+        )
+        self.start_node(-1, extra_args=["-wallet=u1_v16"])
+        wallet = node_v16.get_wallet_rpc("u1_v16")
+        info = wallet.validateaddress(v16_addr)
+        assert_equal(info, v16_info)
+
         # Copy the 0.17 wallet to the last Bitcoin Core version and open it:
         node_v17.unloadwallet("u1_v17")
         shutil.copytree(