Repository: bitcoin/bitcoin (language: cpp)

Issue:
Calling `upgradewallet` on a legacy wallet causes Bitcoin Core to crash
I have 2 legacy wallets and on both if I run `upgradewallet` it will cause bitcoin core to crash. This should just return me an error string

Currenl result from `getwalletinfo`:
```
{
  "walletname": "Wallet",
  "walletversion": 169900,
  "format": "bdb",
  "balance": 0.00000000,
  "unconfirmed_balance": 0.00000000,
  "immature_balance": 0.00000000,
  "txcount": 80,
  "keypoololdest": 1613078825,
  "keypoolsize": 941,
  "keypoolsize_hd_internal": 941,
  "paytxfee": 0.00000000,
  "private_keys_enabled": false,
  "avoid_reuse": true,
  "scanning": false,
  "descriptors": false
}
```


Discussion / hints:
Can you run this in valgrind or gdb to get a stack trace? Also, which version of Bitcoin Core?
> Can you run this in valgrind or gdb to get a stack trace? Also, which version of Bitcoin Core?

version is 22.0
Do you have a backup of the wallets? If so, can you load them and do `getwalletinfo`?

Is there anything in the debug.log that contains an error message?

***

Oh, this is probably because the wallet has private keys disabled but `upgradewallet` isn't checking that so it tries to generate keys, but hits an assertion error.
Only logs I get is
```
2021-12-21T16:13:24Z [Wallet] Performing wallet upgrade to 169900
2021-12-21T16:13:24Z [Wallet] Upgrading wallet to HD
```

this is a watch only wallet so that may be the case

Fix patch:
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 7218ed11dcb6c..53f7b773b4735 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -469,6 +469,12 @@ bool LegacyScriptPubKeyMan::CanGetAddresses(bool internal) const
 bool LegacyScriptPubKeyMan::Upgrade(int prev_version, int new_version, bilingual_str& error)
 {
     LOCK(cs_KeyStore);
+
+    if (m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
+        // Nothing to do here if private keys are not enabled
+        return true;
+    }
+
     bool hd_upgrade = false;
     bool split_upgrade = false;
     if (IsFeatureSupported(new_version, FEATURE_HD) && !IsHDEnabled()) {


Test patch:
diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py
index 36e72f2dd92db..c452e1eafd82c 100755
--- a/test/functional/wallet_upgradewallet.py
+++ b/test/functional/wallet_upgradewallet.py
@@ -345,5 +345,16 @@ def copy_split_hd():
             desc_wallet = self.nodes[0].get_wallet_rpc("desc_upgrade")
             self.test_upgradewallet(desc_wallet, previous_version=169900, expected_version=169900)
 
+            self.log.info("Checking that descriptor wallets without privkeys do nothing, successfully")
+            self.nodes[0].createwallet(wallet_name="desc_upgrade_nopriv", descriptors=True, disable_private_keys=True)
+            desc_wallet = self.nodes[0].get_wallet_rpc("desc_upgrade_nopriv")
+            self.test_upgradewallet(desc_wallet, previous_version=169900, expected_version=169900)
+
+        if self.is_bdb_compiled():
+            self.log.info("Upgrading a wallet with private keys disabled")
+            self.nodes[0].createwallet(wallet_name="privkeys_disabled_upgrade", disable_private_keys=True, descriptors=False)
+            disabled_wallet = self.nodes[0].get_wallet_rpc("privkeys_disabled_upgrade")
+            self.test_upgradewallet(disabled_wallet, previous_version=169900, expected_version=169900)
+
 if __name__ == '__main__':
     UpgradeWalletTest().main()