Merge #18918: wallet: Move salvagewallet into wallettool

84ae0578b6c68dda145ca65fef510ce0fdac0d7b Add release notes about salvage changes (Andrew Chow)
ea337f2d0318a860f695698cfb3aa91c03ded858 Move RecoverKeysOnlyFilter into RecoverDataBaseFile (Andrew Chow)
9ea2d258b46e8a9776100633585ed0feede5c2a4 Move RecoverDatabaseFile and RecoverKeysOnlyFilter into salvage.{cpp/h} (Andrew Chow)
b426c7764d26e280e1f814cf36e050743c45cd12 Make BerkeleyBatch::Recover and WalletBatch::RecoverKeysOnlyFilter standalone (Andrew Chow)
2741774214168eb287c7066d6823afe5e570381d Expose a version of ReadKeyValue and use it in RecoverKeysOnlyFilter (Andrew Chow)
ced95d0e43389fe62b5d30fcc7c42dbca0e88242 Move BerkeleyEnvironment::Salvage into BerkeleyBatch::Recover (Andrew Chow)
07250b8dcebe2b97ed0fd900ad35cba4091b8ecf walletdb: remove fAggressive from Salvage (Andrew Chow)
8ebcbc85c652665b78dcfd2ad55fa67cafd42c73 walletdb: don't automatically salvage when corruption is detected (Andrew Chow)
d321046f4bb4887742699c586755a21f3a2edbe1 wallet: remove -salvagewallet (Andrew Chow)
cdd955e580dff99f3fa440494ed2b348f7f094af Add basic test for bitcoin-wallet salvage (Andrew Chow)
c87770915b88d195d264b58111c64142b1965cfa wallettool: Add a salvage command (Andrew Chow)

Pull request description:

  Removes the `-salvagewallet` startup option and adds a `salvage` command to the `bitcoin-wallet` tool. As such, `-salvagewallet` is removed. Additionally, the automatic salvage that is done if the wallet file fails to load is removed.

  Lastly the salvage code entirely is moved out entirely into `bitcoin-wallet` from `walletdb.{cpp/h}` and `db.{cpp/h}`.

ACKs for top commit:
  jonatack:
    ACK 84ae0578b6c68dda145ca65fef510ce0fdac0d7b feedback taken, and compared to my previous review, the bitcoin-wallet salvage command now seems to run and it exits without raising. The new test passes at both 9454105 and 84ae057 so as a sanity check I'd agree there is room for improvement, if possible.
  MarcoFalke:
    re-ACK 84ae0578b6 🏉
  Empact:
    Code Review ACK https://github.com/bitcoin/bitcoin/pull/18918/commits/84ae0578b6c68dda145ca65fef510ce0fdac0d7b
  ryanofsky:
    Code review ACK 84ae0578b6c68dda145ca65fef510ce0fdac0d7b. Lot of small changes since previous review: added verify step before salvage, added basic test in new commit, removed unused scanstate variable and warnings parameter, tweaked various comments and strings, moved fsuccess variable declaration
  meshcollider:
    Concept / light code review ACK 84ae0578b6c68dda145ca65fef510ce0fdac0d7b

Tree-SHA512: 05be116b56ecade1c58faca1728c8fe4b78f0a082dbc2544a3f7507dd155f1f4f39070bd1fe90053444384337bc48b97149df5c1010230d78f8ecc08e69d93af

1 files changed, 4 insertions, 7 deletions

diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
index a2788ed6c4..b95ed24d12 100644
--- a/src/wallet/walletdb.h
+++ b/src/wallet/walletdb.h
@@ -261,18 +261,12 @@ public:
     DBErrors FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWalletTx>& vWtx);
     DBErrors ZapWalletTx(std::list<CWalletTx>& vWtx);
     DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut);
-    /* Try to (very carefully!) recover wallet database (with a possible key type filter) */
-    static bool Recover(const fs::path& wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename);
-    /* Recover convenience-function to bypass the key filter callback, called when verify fails, recovers everything */
-    static bool Recover(const fs::path& wallet_path, std::string& out_backup_filename);
-    /* Recover filter (used as callback), will only let keys (cryptographical keys) as KV/key-type pass through */
-    static bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue);
     /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */
     static bool IsKeyType(const std::string& strType);
     /* verifies the database environment */
     static bool VerifyEnvironment(const fs::path& wallet_path, bilingual_str& errorStr);
     /* verifies the database file */
-    static bool VerifyDatabaseFile(const fs::path& wallet_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr);
+    static bool VerifyDatabaseFile(const fs::path& wallet_path, bilingual_str& errorStr);
 
     //! write the hdchain model (external chain child index counter)
     bool WriteHDChain(const CHDChain& chain);
@@ -292,4 +286,7 @@ private:
 //! Compacts BDB state so that wallet.dat is self-contained (if there are changes)
 void MaybeCompactWalletDB();
 
+//! Unserialize a given Key-Value pair and load it into the wallet
+bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr);
+
 #endif // BITCOIN_WALLET_WALLETDB_H