Repository: bitcoin/bitcoin (language: cpp)

Issue:
Unknown descriptor in wallet crashes
Attempting to load a wallet with an unknown descriptor causes a fatal error:

```
2022-09-06T01:51:06Z init message: Loading wallet…
2022-09-06T01:51:06Z [descriptor] Invalid descriptor: Can only have tr at top level: iostream error
2022-09-06T01:51:06Z [descriptor] Invalid descriptor: Can only have tr at top level: iostream error
2022-09-06T01:51:06Z [descriptor] Setting spkMan to active: id = 50e6032b4d1d62c020ebb6a4d0c8fafc6f01ea80f4d2a41c1a256086611636e9, type = legacy, internal = false
2022-09-06T01:51:06Z [descriptor] Setting spkMan to active: id = 7f709abbde61e8c253798375692bf41202e625ed74a65f9474d464be34fb2870, type = p2sh-segwit, internal = false
2022-09-06T01:51:06Z [descriptor] Setting spkMan to active: id = 2dc6c34099e0ca1235fc990a51042967ecbc8c8ac57062f964739cba2f451be8, type = bech32, internal = false
2022-09-06T01:51:06Z [descriptor] Setting spkMan to active: id = 8d3c63ea1ac9936e789713dc8bae857d328ff029ab9392e67c716752e9b11b0c, type = bech32m, internal = false
2022-09-06T01:51:06Z [descriptor] Releasing wallet
2022-09-06T01:51:06Z 

************************
EXCEPTION: St12out_of_range       
map::at       
bitcoin in Runaway exception       



************************
EXCEPTION: St12out_of_range       
map::at       
bitcoin in Runaway exception       

bitcoin-qt: ./checkqueue.h:204: CCheckQueue<CScriptCheck>::~CCheckQueue() [T = CScriptCheck]: Assertion `m_worker_threads.empty()' failed.
Aborted
```

> A fatal error occurred. Bitcoin Core can no longer continue safely and will quit.

To reproduce, a quick hack:

```c++
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -1408,7 +1408,7 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
         error = "Can only have addr() at top level";
         return nullptr;
     }
-    if (ctx == ParseScriptContext::TOP && Func("tr", expr)) {
+    if (ctx == ParseScriptContext::TOP && Func("trx", expr)) {
         auto arg = Expr(expr);
         auto internal_key = ParsePubkey(key_exp_index, arg, ParseScriptContext::P2TR, out, error);
         if (!internal_key) {
```


Discussion / hints:

Fix patch:
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 0949045ff01b0..784ea24b9875e 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2819,8 +2819,12 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
             warnings.push_back(strprintf(_("Error reading %s! Transaction data may be missing or incorrect."
                                            " Rescanning wallet."), walletFile));
             rescan_required = true;
-        }
-        else {
+        } else if (nLoadWalletRet == DBErrors::UNKNOWN_DESCRIPTOR) {
+            error = strprintf(_("Unrecognized descriptor found. Loading wallet %s\n\n"
+                                "The wallet might had been created on a newer version.\n"
+                                "Please try running the latest software version.\n"), walletFile);
+            return nullptr;
+        } else {
             error = strprintf(_("Error loading %s"), walletFile);
             return nullptr;
         }
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index 30406a22f928b..6a8f0d24817e9 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -314,6 +314,7 @@ class CWalletScanState {
     std::map<std::pair<uint256, CKeyID>, std::pair<CPubKey, std::vector<unsigned char>>> m_descriptor_crypt_keys;
     std::map<uint160, CHDChain> m_hd_chains;
     bool tx_corrupt{false};
+    bool descriptor_unknown{false};
 
     CWalletScanState() = default;
 };
@@ -627,7 +628,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
             uint256 id;
             ssKey >> id;
             WalletDescriptor desc;
-            ssValue >> desc;
+            try {
+                ssValue >> desc;
+            } catch (const std::ios_base::failure& e) {
+                strErr = e.what();
+                wss.descriptor_unknown = true;
+                return false;
+            }
             if (wss.m_descriptor_caches.count(id) == 0) {
                 wss.m_descriptor_caches[id] = DescriptorCache();
             }
@@ -767,6 +774,12 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
     DBErrors result = DBErrors::LOAD_OK;
 
     LOCK(pwallet->cs_wallet);
+
+    // Last client version to open this wallet
+    int last_client = CLIENT_VERSION;
+    bool has_last_client = m_batch->Read(DBKeys::VERSION, last_client);
+    pwallet->WalletLogPrintf("Wallet file version = %d, last client version = %d\n", pwallet->GetVersion(), last_client);
+
     try {
         int nMinVersion = 0;
         if (m_batch->Read(DBKeys::MINVERSION, nMinVersion)) {
@@ -832,6 +845,13 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
                     // Set tx_corrupt back to false so that the error is only printed once (per corrupt tx)
                     wss.tx_corrupt = false;
                     result = DBErrors::CORRUPT;
+                } else if (wss.descriptor_unknown) {
+                    strErr = strprintf("Error: Unrecognized descriptor found in wallet %s. ", pwallet->GetName());
+                    strErr += (last_client > CLIENT_VERSION) ? "The wallet might had been created on a newer version. " :
+                            "The database might be corrupted or the software version is not compatible with one of your wallet descriptors. ";
+                    strErr += "Please try running the latest software version";
+                    pwallet->WalletLogPrintf("%s\n", strErr);
+                    return DBErrors::UNKNOWN_DESCRIPTOR;
                 } else {
                     // Leave other errors alone, if we try to fix them we might make things worse.
                     fNoncriticalErrors = true; // ... but do warn the user there is something wrong.
@@ -884,11 +904,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
     if (result != DBErrors::LOAD_OK)
         return result;
 
-    // Last client version to open this wallet
-    int last_client = CLIENT_VERSION;
-    bool has_last_client = m_batch->Read(DBKeys::VERSION, last_client);
-    pwallet->WalletLogPrintf("Wallet file version = %d, last client version = %d\n", pwallet->GetVersion(), last_client);
-
     pwallet->WalletLogPrintf("Keys: %u plaintext, %u encrypted, %u w/ metadata, %u total. Unknown wallet records: %u\n",
            wss.nKeys, wss.nCKeys, wss.nKeyMeta, wss.nKeys + wss.nCKeys, wss.m_unknown_records);
 
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
index 6aa25fae038df..da6efe534b565 100644
--- a/src/wallet/walletdb.h
+++ b/src/wallet/walletdb.h
@@ -51,7 +51,8 @@ enum class DBErrors
     EXTERNAL_SIGNER_SUPPORT_REQUIRED,
     LOAD_FAIL,
     NEED_REWRITE,
-    NEED_RESCAN
+    NEED_RESCAN,
+    UNKNOWN_DESCRIPTOR
 };
 
 namespace DBKeys {


Test patch:
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 8a2386a2b4bc6..987f30251c20c 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -174,7 +174,8 @@ BITCOIN_TESTS += \
   wallet/test/availablecoins_tests.cpp \
   wallet/test/init_tests.cpp \
   wallet/test/ismine_tests.cpp \
-  wallet/test/scriptpubkeyman_tests.cpp
+  wallet/test/scriptpubkeyman_tests.cpp \
+  wallet/test/walletload_tests.cpp
 
 FUZZ_SUITE_LD_COMMON +=\
  $(SQLITE_LIBS) \
diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
new file mode 100644
index 0000000000000..f45b69a418038
--- /dev/null
+++ b/src/wallet/test/walletload_tests.cpp
@@ -0,0 +1,54 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
+
+#include <wallet/wallet.h>
+#include <test/util/setup_common.h>
+
+#include <boost/test/unit_test.hpp>
+
+namespace wallet {
+
+BOOST_AUTO_TEST_SUITE(walletload_tests)
+
+class DummyDescriptor final : public Descriptor {
+private:
+    std::string desc;
+public:
+    explicit DummyDescriptor(const std::string& descriptor) : desc(descriptor) {};
+    ~DummyDescriptor() = default;
+
+    std::string ToString() const override { return desc; }
+    std::optional<OutputType> GetOutputType() const override { return OutputType::UNKNOWN; }
+
+    bool IsRange() const override { return false; }
+    bool IsSolvable() const override { return false; }
+    bool IsSingleType() const override { return true; }
+    bool ToPrivateString(const SigningProvider& provider, std::string& out) const override { return false; }
+    bool ToNormalizedString(const SigningProvider& provider, std::string& out, const DescriptorCache* cache = nullptr) const override { return false; }
+    bool Expand(int pos, const SigningProvider& provider, std::vector<CScript>& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache = nullptr) const override { return false; };
+    bool ExpandFromCache(int pos, const DescriptorCache& read_cache, std::vector<CScript>& output_scripts, FlatSigningProvider& out) const override { return false; }
+    void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const override {}
+};
+
+BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup)
+{
+    std::unique_ptr<WalletDatabase> database = CreateMockWalletDatabase();
+    {
+        // Write unknown active descriptor
+        WalletBatch batch(*database, false);
+        std::string unknown_desc = "trx(tpubD6NzVbkrYhZ4Y4S7m6Y5s9GD8FqEMBy56AGphZXuagajudVZEnYyBahZMgHNCTJc2at82YX6s8JiL1Lohu5A3v1Ur76qguNH4QVQ7qYrBQx/86'/1'/0'/0/*)#8pn8tzdt";
+        WalletDescriptor wallet_descriptor(std::make_shared<DummyDescriptor>(unknown_desc), 0, 0, 0, 0);
+        BOOST_CHECK(batch.WriteDescriptor(uint256(), wallet_descriptor));
+        BOOST_CHECK(batch.WriteActiveScriptPubKeyMan(static_cast<uint8_t>(OutputType::UNKNOWN), uint256(), false));
+    }
+
+    {
+        // Now try to load the wallet and verify the error.
+        const std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, std::move(database)));
+        BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::UNKNOWN_DESCRIPTOR);
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+} // namespace wallet