Merge bitcoin/bitcoin#24914: wallet: Load database records in a particular order

3c83b1d doc: Add release note for wallet loading changes (Andrew Chow)
2636844 walletdb: Remove loading code where the database is iterated (Andrew Chow)
cd211b3 walletdb: refactor decryption key loading (Andrew Chow)
31c033e walletdb: refactor defaultkey and wkey loading (Andrew Chow)
c978c6d walletdb: refactor active spkm loading (Andrew Chow)
6fabb7f walletdb: refactor tx loading (Andrew Chow)
abcc13d walletdb: refactor address book loading (Andrew Chow)
405b4d9 walletdb: Refactor descriptor wallet records loading (Andrew Chow)
30ab11c walletdb: Refactor legacy wallet record loading into its own function (Andrew Chow)
9e077d9 salvage: Remove use of ReadKeyValue in salvage (Andrew Chow)
ad779e9 walletdb: Refactor hd chain loading to its own function (Andrew Chow)
72c2a54 walletdb: Refactor encryption key loading to its own function (Andrew Chow)
3ccde45 walletdb: Refactor crypted key loading to its own function (Andrew Chow)
7be10ad walletdb: Refactor key reading and loading to its own function (Andrew Chow)
52932c5 walletdb: Refactor wallet flags loading (Andrew Chow)
01b35b5 walletdb: Refactor minversion loading (Andrew Chow)

Pull request description:

  Currently when we load a wallet, we just iterate through all of the records in the database and add them completely statelessly. However we have some records which do rely on other records being loaded before they are. To deal with this, we use `CWalletScanState` to hold things temporarily until all of the records have been read and then we load the stateful things.

  However this can be slow, and with some future improvements, can cause some pretty drastic slowdowns to retain this pattern. So this PR changes the way we load records by choosing to load the records in a particular order. This lets us do things such as loading a descriptor record, then finding and loading that descriptor's cache and key records. In the future, this will also let us use `IsMine` when loading transactions as then `IsMine` will actually be working as we now always load keys and descriptors before transactions.

  In order to get records of a specific type, this PR includes some refactors to how we do database cursors. Functionality is also added to retrieve a cursor that will give us records beginning with a specified prefix.

  Lastly, one thing that iterating the entire database let us do was to find unknown records. However even if unknown records were found, we would not do anything with this information except output a number in a log line. With this PR, we would no longer be aware of any unknown records. This does not change functionality as we don't do anything with unknown records, and having unknown records is not an error. Now we would just not be aware that unknown records even exist.

ACKs for top commit:
  MarcoFalke:
    re-ACK 3c83b1d 🍤
  furszy:
    reACK 3c83b1d
  ryanofsky:
    Code review ACK 3c83b1d. Just Marco's suggested error handling fixes since last review

Tree-SHA512: 15fa56332fb2ce4371db468a0c674ee7a3a8889c8cee9f428d06a7d1385d17a9bf54bcb0ba885c87736841fe6a5c934594bcf4476a473616510ee47862ef30b4

Author: ryanofsky

doc/release-notes-24914.md

@@ -0,0 +1,9 @@
+Wallet
+------
+
+- Wallet loading has changed in this release. Wallets with some corrupted records that could be
+  previously loaded (with warnings) may no longer load. For example, wallets with corrupted
+  address book entries may no longer load. If this happens, it is recommended
+  load the wallet in a previous version of Bitcoin Core and import the data into a new wallet.
+  Please also report an issue to help improve the software and make wallet loading more robust
+  in these cases.

src/wallet/salvage.cpp

@@ -18,11 +18,6 @@ static const char *HEADER_END = "HEADER=END";
 static const char *DATA_END = "DATA=END";
 typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair;
 
-static bool KeyFilter(const std::string& type)
-{
-    return WalletBatch::IsKeyType(type) || type == DBKeys::HDCHAIN;
-}
-
 class DummyCursor : public DatabaseCursor
 {
     Status Next(DataStream& key, DataStream& value) override { return Status::FAIL; }
@@ -186,17 +181,24 @@ bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bil
     {
         /* Filter for only private key type KV pairs to be added to the salvaged wallet */
         DataStream ssKey{row.first};
-        CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION);
+        DataStream ssValue(row.second);
         std::string strType, strErr;
-        bool fReadOK;
-        {
-            // Required in LoadKeyMetadata():
-            LOCK(dummyWallet.cs_wallet);
-            fReadOK = ReadKeyValue(&dummyWallet, ssKey, ssValue, strType, strErr, KeyFilter);
-        }
-        if (!KeyFilter(strType)) {
+
+        // We only care about KEY, MASTER_KEY, CRYPTED_KEY, and HDCHAIN types
+        ssKey >> strType;
+        bool fReadOK = false;
+        if (strType == DBKeys::KEY) {
+            fReadOK = LoadKey(&dummyWallet, ssKey, ssValue, strErr);
+        } else if (strType == DBKeys::CRYPTED_KEY) {
+            fReadOK = LoadCryptedKey(&dummyWallet, ssKey, ssValue, strErr);
+        } else if (strType == DBKeys::MASTER_KEY) {
+            fReadOK = LoadEncryptionKey(&dummyWallet, ssKey, ssValue, strErr);
+        } else if (strType == DBKeys::HDCHAIN) {
+            fReadOK = LoadHDChain(&dummyWallet, ssValue, strErr);
+        } else {
             continue;
         }
+
         if (!fReadOK)
         {
             warnings.push_back(strprintf(Untranslated("WARNING: WalletBatch::Recover skipping %s: %s"), strType, strErr));

src/wallet/wallet.cpp

@@ -2929,7 +2929,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
         else if (nLoadWalletRet == DBErrors::NONCRITICAL_ERROR)
         {
             warnings.push_back(strprintf(_("Error reading %s! All keys read correctly, but transaction data"
-                                           " or address book entries might be missing or incorrect."),
+                                           " or address metadata may be missing or incorrect."),
                 walletFile));
         }
         else if (nLoadWalletRet == DBErrors::TOO_NEW) {