walletdb: don't automatically salvage when corruption is detected

achow101 · achow101 · commit 8ebcbc85c652 · 2020-05-25T12:59:22.000-04:00
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
@@ -268,21 +268,14 @@ BerkeleyEnvironment::BerkeleyEnvironment()
     fMockDb = true;
 }
 
-BerkeleyEnvironment::VerifyResult BerkeleyEnvironment::Verify(const std::string& strFile, recoverFunc_type recoverFunc, std::string& out_backup_filename)
+bool BerkeleyEnvironment::Verify(const std::string& strFile)
 {
     LOCK(cs_db);
     assert(mapFileUseCount.count(strFile) == 0);
 
     Db db(dbenv.get(), 0);
     int result = db.verify(strFile.c_str(), nullptr, nullptr, 0);
-    if (result == 0)
-        return VerifyResult::VERIFY_OK;
-    else if (recoverFunc == nullptr)
-        return VerifyResult::RECOVER_FAIL;
-
-    // Try to recover:
-    bool fRecovered = (*recoverFunc)(fs::path(strPath) / strFile, out_backup_filename);
-    return (fRecovered ? VerifyResult::RECOVER_OK : VerifyResult::RECOVER_FAIL);
+    return result == 0;
 }
 
 BerkeleyBatch::SafeDbt::SafeDbt()
@@ -410,27 +403,16 @@ bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, bilingual_str&
     return true;
 }
 
-bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc)
+bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, bilingual_str& errorStr)
 {
     std::string walletFile;
     std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, walletFile);
     fs::path walletDir = env->Directory();
 
     if (fs::exists(walletDir / walletFile))
     {
-        std::string backup_filename;
-        BerkeleyEnvironment::VerifyResult r = env->Verify(walletFile, recoverFunc, backup_filename);
-        if (r == BerkeleyEnvironment::VerifyResult::RECOVER_OK)
-        {
-            warnings.push_back(strprintf(_("Warning: Wallet file corrupt, data salvaged!"
-                                     " Original %s saved as %s in %s; if"
-                                     " your balance or transactions are incorrect you should"
-                                     " restore from a backup."),
-                walletFile, backup_filename, walletDir));
-        }
-        if (r == BerkeleyEnvironment::VerifyResult::RECOVER_FAIL)
-        {
-            errorStr = strprintf(_("%s corrupt, salvage failed"), walletFile);
+        if (!env->Verify(walletFile)) {
+            errorStr = strprintf(_("%s corrupt. Try using the wallet tool bitcoin-wallet to salvage or restoring a backup."), walletFile);
             return false;
         }
     }
diff --git a/src/wallet/db.h b/src/wallet/db.h
@@ -66,17 +66,7 @@ class BerkeleyEnvironment
     bool IsDatabaseLoaded(const std::string& db_filename) const { return m_databases.find(db_filename) != m_databases.end(); }
     fs::path Directory() const { return strPath; }
 
-    /**
-     * Verify that database file strFile is OK. If it is not,
-     * call the callback to try to recover.
-     * This must be called BEFORE strFile is opened.
-     * Returns true if strFile is OK.
-     */
-    enum class VerifyResult { VERIFY_OK,
-                        RECOVER_OK,
-                        RECOVER_FAIL };
-    typedef bool (*recoverFunc_type)(const fs::path& file_path, std::string& out_backup_filename);
-    VerifyResult Verify(const std::string& strFile, recoverFunc_type recoverFunc, std::string& out_backup_filename);
+    bool Verify(const std::string& strFile);
     /**
      * Salvage data from a file that Verify says is bad.
      * fAggressive sets the DB_AGGRESSIVE flag (see berkeley DB->verify() method documentation).
@@ -253,7 +243,7 @@ class BerkeleyBatch
     /* verifies the database environment */
     static bool VerifyEnvironment(const fs::path& file_path, bilingual_str& errorStr);
     /* verifies the database file */
-    static bool VerifyDatabaseFile(const fs::path& file_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc);
+    static bool VerifyDatabaseFile(const fs::path& file_path, bilingual_str& errorStr);
 
     template <typename K, typename T>
     bool Read(const K& key, T& value)
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -3694,7 +3694,7 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b
         return false;
     }
 
-    return WalletBatch::VerifyDatabaseFile(wallet_path, warnings, error_string);
+    return WalletBatch::VerifyDatabaseFile(wallet_path, error_string);
 }
 
 std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings, uint64_t wallet_creation_flags)
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
@@ -922,9 +922,9 @@ bool WalletBatch::VerifyEnvironment(const fs::path& wallet_path, bilingual_str&
     return BerkeleyBatch::VerifyEnvironment(wallet_path, errorStr);
 }
 
-bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr)
+bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, bilingual_str& errorStr)
 {
-    return BerkeleyBatch::VerifyDatabaseFile(wallet_path, warnings, errorStr, WalletBatch::Recover);
+    return BerkeleyBatch::VerifyDatabaseFile(wallet_path, errorStr);
 }
 
 bool WalletBatch::WriteDestData(const std::string &address, const std::string &key, const std::string &value)
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
@@ -274,7 +274,7 @@ class WalletBatch
     /* 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);

Original file line number	Diff line number	Diff line change
`@@ -3694,7 +3694,7 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b`
`3694`	`3694`	`return false;`
`3695`	`3695`	`}`
`3696`	`3696`
`3697`		`- return WalletBatch::VerifyDatabaseFile(wallet_path, warnings, error_string);`
	`3697`	`+ return WalletBatch::VerifyDatabaseFile(wallet_path, error_string);`
`3698`	`3698`	`}`
`3699`	`3699`
`3700`	`3700`	`std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings, uint64_t wallet_creation_flags)`
Original file line number	Diff line number	Diff line change
`@@ -922,9 +922,9 @@ bool WalletBatch::VerifyEnvironment(const fs::path& wallet_path, bilingual_str&`
`922`	`922`	`return BerkeleyBatch::VerifyEnvironment(wallet_path, errorStr);`
`923`	`923`	`}`
`924`	`924`
`925`		`-bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr)`
	`925`	`+bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, bilingual_str& errorStr)`
`926`	`926`	`{`
`927`		`- return BerkeleyBatch::VerifyDatabaseFile(wallet_path, warnings, errorStr, WalletBatch::Recover);`
	`927`	`+ return BerkeleyBatch::VerifyDatabaseFile(wallet_path, errorStr);`
`928`	`928`	`}`
`929`	`929`
`930`	`930`	`bool WalletBatch::WriteDestData(const std::string &address, const std::string &key, const std::string &value)`