Merge bitcoin#35437: migrate: Handle HD chains that have identical seeds but different IDs

fanquake · fanquake · commit 0e95e1abdb47 · 2026-06-18T16:14:36.000+02:00
de92208 migrate: Handle HD chains that have identical seeds but different IDs (Ava Chow) Pull request description: The seed ID is calculated from a pubkey produced by treating the seed as a private key. This calculation includes a pubkey compression parameter, even thought that compression is completely irrelevant for the usage of the seed as a BIP 32 seed. Thus migration should detect if a seed has been used multiple times by checking if the computed master key was already processed. The spkm_migration fuzzer needs to have it's added descriptors accounting updated for this fix. It should not be possible for users to actually run into this problem as all HD chains use seeds with the pubkey compression option set. Fixes bitcoin#35434 ACKs for top commit: kevkevinpal: crACK [de92208](bitcoin@de92208) marcofleon: crACK de92208 rkrux: code review ACK de92208 Tree-SHA512: c420a24722fd6a94bf6656f195bad3432ba54c38b3c49a02750577281d0864988fd6d44cd9594b57cfaf33061a1e250e21378e3637b4e9a45f2d7aad6045884d
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
@@ -628,19 +628,28 @@ std::optional<MigrationData> LegacyDataSPKM::MigrateToDescriptor()
 
     bool can_support_hd_split_feature = m_hd_chain.nVersion >= CHDChain::VERSION_HD_CHAIN_SPLIT;
 
+    std::set<CExtPubKey> master_xpubs;
     for (const CHDChain& chain : chains) {
+        if (chain.seed_id.IsNull()) continue;
+
+        // Get the master xprv
+        CKey seed_key;
+        if (!GetKey(chain.seed_id, seed_key)) {
+            assert(false);
+        }
+        CExtKey master_key;
+        master_key.SetSeed(seed_key);
+
+        // Get the xpub and verify that we haven't already seen this xpub before
+        CExtPubKey master_xpub = master_key.Neuter();
+        const auto& [_, inserted] = master_xpubs.insert(master_xpub);
+        if (!inserted) continue;
+
         for (int i = 0; i < 2; ++i) {
             // Skip if doing internal chain and split chain is not supported
-            if (chain.seed_id.IsNull() || (i == 1 && !can_support_hd_split_feature)) {
+            if (i == 1 && !can_support_hd_split_feature) {
                 continue;
             }
-            // Get the master xprv
-            CKey seed_key;
-            if (!GetKey(chain.seed_id, seed_key)) {
-                assert(false);
-            }
-            CExtKey master_key;
-            master_key.SetSeed(seed_key);
 
             // Make the combo descriptor
             std::string xpub = EncodeExtPubKey(master_key.Neuter());
diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp
@@ -257,10 +257,10 @@ FUZZ_TARGET(spkm_migration, .init = initialize_spkm_migration)
 
     bool add_inactive_hd_chain{fuzzed_data_provider.ConsumeBool() && !keys.empty()};
     if (add_inactive_hd_chain) {
-        hd_key = PickValue(fuzzed_data_provider, keys);
+        CKey inactive_hd_key = PickValue(fuzzed_data_provider, keys);
         hd_chain.nVersion = fuzzed_data_provider.ConsumeBool() ? CHDChain::VERSION_HD_CHAIN_SPLIT : CHDChain::VERSION_HD_BASE;
-        bool dup_chain = hd_chain.seed_id == hd_key.GetPubKey().GetID();
-        hd_chain.seed_id = hd_key.GetPubKey().GetID();
+        bool dup_chain = hd_key.IsValid() && std::equal(hd_key.begin(), hd_key.end(), inactive_hd_key.begin());
+        hd_chain.seed_id = inactive_hd_key.GetPubKey().GetID();
         legacy_data.AddInactiveHDChain(hd_chain);
         if (!dup_chain) added_chains++;
     }
