Merge bitcoin#20153: wallet: do not import a descriptor with hardened derivations into a watch-only wallet

laanwj · knst · commit b18351e41541 · 2024-03-07T01:23:19.000+07:00
538be42 wallet: fix importdescriptor silent fail (Ivan Metlushko) Pull request description: Currently `importdescriptor` command will successfully import a descriptor with hardened derivations into a watch-only wallet while silently failing to expand the descriptor to fill the cache. This leads to a broken wallet state and failure to load such wallet due to missing cache on subsequent restart. ACKs for top commit: laanwj: Code review ACK 538be42 achow101: ACK 538be42 meshcollider: utACK 538be42 Tree-SHA512: 4bdd0ab4437d55b3f1a79c3a300a0b186089155c020fe220a73d0cce274de47d90371d88918d39fd795f9fccf8db328f1e322d29a6062f9ce94a1c254398f004
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
@@ -1692,7 +1692,9 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue&
         // Need to ExpandPrivate to check if private keys are available for all pubkeys
         FlatSigningProvider expand_keys;
         std::vector<CScript> scripts;
-        parsed_desc->Expand(0, keys, scripts, expand_keys);
+        if (!parsed_desc->Expand(0, keys, scripts, expand_keys)) {
+            throw JSONRPCError(RPC_WALLET_ERROR, "Cannot expand descriptor. Probably because of hardened derivations without private keys provided");
+        }
         parsed_desc->ExpandPrivate(0, keys, expand_keys);
 
         // Check if all private keys are provided
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -5637,7 +5637,10 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat
     }
 
     // Top up key pool, the manager will generate new scriptPubKeys internally
-    new_spk_man->TopUp();
+    if (!new_spk_man->TopUp()) {
+        WalletLogPrintf("Could not top up scriptPubKeys\n");
+        return nullptr;
+    }
 
     // Apply the label if necessary
     // Note: we disable labels for ranged descriptors
diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py
@@ -205,6 +205,15 @@ def run_test(self):
                              success=False,
                              error_code=-4,
                              error_message='Cannot import private keys to a wallet with private keys disabled')
+
+        self.log.info("Should not import a descriptor with hardened derivations when private keys are disabled")
+        self.test_importdesc({"desc": descsum_create("pkh(" + xpub + "/1h/*)"),
+                              "timestamp": "now",
+                              "range": 1},
+                             success=False,
+                             error_code=-4,
+                             error_message='Cannot expand descriptor. Probably because of hardened derivations without private keys provided')
+
         for address in addresses:
             test_address(w1,
                          address,
