salvage: Remove use of ReadKeyValue in salvage

To prepare to remove ReadKeyValue, change salvage to not use it

Author: achow101

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/walletdb.cpp

@@ -470,17 +470,13 @@ bool LoadHDChain(CWallet* pwallet, DataStream& ssValue, std::string& strErr)
 
 static bool
 ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue,
-             CWalletScanState &wss, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
+             CWalletScanState &wss, std::string& strType, std::string& strErr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
 {
     try {
         // Unserialize
         // Taking advantage of the fact that pair serialization
         // is just the two items serialized one after the other
         ssKey >> strType;
-        // If we have a filter, check if this matches the filter
-        if (filter_fn && !filter_fn(strType)) {
-            return true;
-        }
         // Legacy entries in descriptor wallets are not allowed, abort immediately
         if (pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS) && DBKeys::LEGACY_TYPES.count(strType) > 0) {
             wss.unexpected_legacy_entry = true;
@@ -834,19 +830,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue,
     return true;
 }
 
-bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn)
-{
-    CWalletScanState dummy_wss;
-    LOCK(pwallet->cs_wallet);
-    return ReadKeyValue(pwallet, ssKey, ssValue, dummy_wss, strType, strErr, filter_fn);
-}
-
-bool WalletBatch::IsKeyType(const std::string& strType)
-{
-    return (strType == DBKeys::KEY ||
-            strType == DBKeys::MASTER_KEY || strType == DBKeys::CRYPTED_KEY);
-}
-
 static DBErrors LoadMinVersion(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
 {
     AssertLockHeld(pwallet->cs_wallet);
@@ -934,7 +917,10 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
                 }
                 // losing keys is considered a catastrophic error, anything else
                 // we assume the user can live with:
-                if (IsKeyType(strType) || strType == DBKeys::DEFAULTKEY) {
+                if (strType == DBKeys::KEY ||
+                    strType == DBKeys::MASTER_KEY ||
+                    strType == DBKeys::CRYPTED_KEY ||
+                    strType == DBKeys::DEFAULTKEY) {
                     result = DBErrors::CORRUPT;
                 } else if (wss.tx_corrupt) {
                     pwallet->WalletLogPrintf("Error: Corrupt transaction found. This can be fixed by removing transactions from wallet and rescanning.\n");

src/wallet/walletdb.h

@@ -276,8 +276,6 @@ class WalletBatch
     DBErrors LoadWallet(CWallet* pwallet);
     DBErrors FindWalletTxHashes(std::vector<uint256>& tx_hashes);
     DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut);
-    /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */
-    static bool IsKeyType(const std::string& strType);
 
     //! write the hdchain model (external chain child index counter)
     bool WriteHDChain(const CHDChain& chain);
@@ -300,12 +298,6 @@ class WalletBatch
 //! Compacts BDB state so that wallet.dat is self-contained (if there are changes)
 void MaybeCompactWalletDB(WalletContext& context);
 
-//! Callback for filtering key types to deserialize in ReadKeyValue
-using KeyFilterFn = std::function<bool(const std::string&)>;
-
-//! Unserialize a given Key-Value pair and load it into the wallet
-bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr);
-
 bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr);
 bool LoadCryptedKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr);
 bool LoadEncryptionKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr);