From c87770915b88d195d264b58111c64142b1965cfa Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 8 May 2020 17:06:16 -0400 Subject: [PATCH 01/11] wallettool: Add a salvage command --- src/bitcoin-wallet.cpp | 1 + src/wallet/wallettool.cpp | 38 +++++++++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index 7f9439788ac..b420463c007 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -31,6 +31,7 @@ static void SetupWalletToolArgs() gArgs.AddArg("info", "Get wallet info", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); gArgs.AddArg("create", "Create new wallet file", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); + gArgs.AddArg("salvage", "Attempt to recover private keys from a corrupt wallet", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); } static bool WalletAppInit(int argc, char* argv[]) diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 522efaa8844..89645fb5cd7 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -103,6 +103,29 @@ static void WalletShowInfo(CWallet* wallet_instance) tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->m_address_book.size()); } +static bool SalvageWallet(const fs::path& path) +{ + // Create a Database handle to allow for the db to be initialized before recovery + std::unique_ptr database = WalletDatabase::Create(path); + + // Initialize the environment before recovery + bilingual_str error_string; + try { + WalletBatch::VerifyEnvironment(path, error_string); + } catch (const fs::filesystem_error& e) { + error_string = Untranslated(strprintf("Error loading wallet. %s", fsbridge::get_filesystem_error_message(e))); + } + if (!error_string.original.empty()) { + tfm::format(std::cerr, "Failed to open wallet for salvage :%s\n", error_string.original); + return false; + } + + // Perform the recovery + CWallet dummy_wallet(nullptr, WalletLocation(), WalletDatabase::CreateDummy()); + std::string backup_filename; + return WalletBatch::Recover(path, (void*)&dummy_wallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename); +} + bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) { fs::path path = fs::absolute(name, GetWalletDir()); @@ -113,7 +136,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) WalletShowInfo(wallet_instance.get()); wallet_instance->Flush(true); } - } else if (command == "info") { + } else if (command == "info" || command == "salvage") { if (!fs::exists(path)) { tfm::format(std::cerr, "Error: no wallet file at %s\n", name); return false; @@ -123,10 +146,15 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) tfm::format(std::cerr, "%s\nError loading %s. Is wallet being used by other process?\n", error.original, name); return false; } - std::shared_ptr wallet_instance = LoadWallet(name, path); - if (!wallet_instance) return false; - WalletShowInfo(wallet_instance.get()); - wallet_instance->Flush(true); + + if (command == "info") { + std::shared_ptr wallet_instance = LoadWallet(name, path); + if (!wallet_instance) return false; + WalletShowInfo(wallet_instance.get()); + wallet_instance->Flush(true); + } else if (command == "salvage") { + return SalvageWallet(path); + } } else { tfm::format(std::cerr, "Invalid command: %s\n", command); return false; From cdd955e580dff99f3fa440494ed2b348f7f094af Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 25 May 2020 12:23:26 -0400 Subject: [PATCH 02/11] Add basic test for bitcoin-wallet salvage --- test/functional/tool_wallet.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 039ce7daee2..524e1593bae 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -203,6 +203,14 @@ class ToolWalletTest(BitcoinTestFramework): assert_equal(shasum_after, shasum_before) self.log.debug('Wallet file shasum unchanged\n') + def test_salvage(self): + # TODO: Check salvage actually salvages and doesn't break things. https://github.com/bitcoin/bitcoin/issues/7463 + self.log.info('Check salvage') + self.start_node(0, ['-wallet=salvage']) + self.stop_node(0) + + self.assert_tool_output('', '-wallet=salvage', 'salvage') + def run_test(self): self.wallet_path = os.path.join(self.nodes[0].datadir, self.chain, 'wallets', 'wallet.dat') self.test_invalid_tool_commands_and_args() @@ -211,7 +219,7 @@ class ToolWalletTest(BitcoinTestFramework): self.test_tool_wallet_info_after_transaction() self.test_tool_wallet_create_on_existing_wallet() self.test_getwalletinfo_on_different_wallet() - + self.test_salvage() if __name__ == '__main__': ToolWalletTest().main() From d321046f4bb4887742699c586755a21f3a2edbe1 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 8 May 2020 17:10:59 -0400 Subject: [PATCH 03/11] wallet: remove -salvagewallet --- src/wallet/init.cpp | 11 ----------- src/wallet/load.cpp | 7 +------ src/wallet/load.h | 2 -- src/wallet/wallet.cpp | 15 +++------------ src/wallet/wallet.h | 2 +- test/functional/wallet_basic.py | 2 -- test/functional/wallet_multiwallet.py | 4 ---- 7 files changed, 5 insertions(+), 38 deletions(-) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 6f973aab1cc..3885eb61854 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -54,7 +54,6 @@ void WalletInit::AddWalletOptions() const gArgs.AddArg("-paytxfee=", strprintf("Fee (in %s/kB) to add to transactions you send (default: %s)", CURRENCY_UNIT, FormatMoney(CFeeRate{DEFAULT_PAY_TX_FEE}.GetFeePerK())), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-rescan", "Rescan the block chain for missing wallet transactions on startup", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - gArgs.AddArg("-salvagewallet", "Attempt to recover private keys from a corrupt wallet on startup", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-spendzeroconfchange", strprintf("Spend unconfirmed change when sending transactions (default: %u)", DEFAULT_SPEND_ZEROCONF_CHANGE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-txconfirmtarget=", strprintf("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)", DEFAULT_TX_CONFIRM_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-wallet=", "Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to if it is not absolute, and will be created if it does not exist (as a directory containing a wallet.dat file and log files). For backwards compatibility this will also accept names of existing data files in .)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET); @@ -89,16 +88,6 @@ bool WalletInit::ParameterInteraction() const LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); } - if (gArgs.GetBoolArg("-salvagewallet", false)) { - if (is_multiwallet) { - return InitError(strprintf(Untranslated("%s is only allowed with a single wallet file"), "-salvagewallet")); - } - // Rewrite just private keys: rescan to find transactions - if (gArgs.SoftSetBoolArg("-rescan", true)) { - LogPrintf("%s: parameter interaction: -salvagewallet=1 -> setting -rescan=1\n", __func__); - } - } - bool zapwallettxes = gArgs.GetBoolArg("-zapwallettxes", false); // -zapwallettxes implies dropping the mempool on startup if (zapwallettxes && gArgs.SoftSetBoolArg("-persistmempool", false)) { diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 16f3699d377..8df3e78215c 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -37,11 +37,6 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal chain.initMessage(_("Verifying wallet(s)...").translated); - // Parameter interaction code should have thrown an error if -salvagewallet - // was enabled with more than wallet file, so the wallet_files size check - // here should have no effect. - bool salvage_wallet = gArgs.GetBoolArg("-salvagewallet", false) && wallet_files.size() <= 1; - // Keep track of each wallet absolute path to detect duplicates. std::set wallet_paths; @@ -55,7 +50,7 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal bilingual_str error_string; std::vector warnings; - bool verify_success = CWallet::Verify(chain, location, salvage_wallet, error_string, warnings); + bool verify_success = CWallet::Verify(chain, location, error_string, warnings); if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!verify_success) { chain.initError(error_string); diff --git a/src/wallet/load.h b/src/wallet/load.h index 5a62e293035..e24b1f2e692 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -16,8 +16,6 @@ class Chain; } // namespace interfaces //! Responsible for reading and validating the -wallet arguments and verifying the wallet database. -//! This function will perform salvage on the wallet if requested, as long as only one wallet is -//! being loaded (WalletInit::ParameterInteraction() forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet). bool VerifyWallets(interfaces::Chain& chain, const std::vector& wallet_files); //! Load wallet databases. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2b45c6a536e..3c968abea07 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -153,7 +153,7 @@ void UnloadWallet(std::shared_ptr&& wallet) std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings) { try { - if (!CWallet::Verify(chain, location, false, error, warnings)) { + if (!CWallet::Verify(chain, location, error, warnings)) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; return nullptr; } @@ -195,7 +195,7 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& } // Wallet::Verify will check if we're trying to create a wallet with a duplicate name. - if (!CWallet::Verify(chain, location, false, error, warnings)) { + if (!CWallet::Verify(chain, location, error, warnings)) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; return WalletCreationStatus::CREATION_FAILED; } @@ -3654,7 +3654,7 @@ std::vector CWallet::GetDestValues(const std::string& prefix) const return values; } -bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, bool salvage_wallet, bilingual_str& error_string, std::vector& warnings) +bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error_string, std::vector& warnings) { // Do some checking on wallet path. It should be either a: // @@ -3694,15 +3694,6 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b return false; } - if (salvage_wallet) { - // Recover readable keypairs: - CWallet dummyWallet(&chain, WalletLocation(), WalletDatabase::CreateDummy()); - std::string backup_filename; - if (!WalletBatch::Recover(wallet_path, (void *)&dummyWallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename)) { - return false; - } - } - return WalletBatch::VerifyDatabaseFile(wallet_path, warnings, error_string); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a29fa222079..fc4cc9495ca 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1137,7 +1137,7 @@ public: bool MarkReplaced(const uint256& originalHash, const uint256& newHash); //! Verify wallet naming and perform salvage on the wallet if required - static bool Verify(interfaces::Chain& chain, const WalletLocation& location, bool salvage_wallet, bilingual_str& error_string, std::vector& warnings); + static bool Verify(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error_string, std::vector& warnings); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ static std::shared_ptr CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings, uint64_t wallet_creation_flags = 0); diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 9e295af3306..797c903dd34 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -404,8 +404,6 @@ class WalletTest(BitcoinTestFramework): '-reindex', '-zapwallettxes=1', '-zapwallettxes=2', - # disabled until issue is fixed: https://github.com/bitcoin/bitcoin/issues/7463 - # '-salvagewallet', ] chainlimit = 6 for m in maintenance: diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 580a61f9f3a..ff9ff341853 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -122,10 +122,6 @@ class MultiWalletTest(BitcoinTestFramework): self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=1', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=2', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") - self.log.info("Do not allow -salvagewallet with multiwallet") - self.nodes[0].assert_start_raises_init_error(['-salvagewallet', '-wallet=w1', '-wallet=w2'], "Error: -salvagewallet is only allowed with a single wallet file") - self.nodes[0].assert_start_raises_init_error(['-salvagewallet=1', '-wallet=w1', '-wallet=w2'], "Error: -salvagewallet is only allowed with a single wallet file") - # if wallets/ doesn't exist, datadir should be the default wallet dir wallet_dir2 = data_dir('walletdir') os.rename(wallet_dir(), wallet_dir2) From 8ebcbc85c652665b78dcfd2ad55fa67cafd42c73 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 8 May 2020 18:44:21 -0400 Subject: [PATCH 04/11] walletdb: don't automatically salvage when corruption is detected --- src/wallet/db.cpp | 28 +++++----------------------- src/wallet/db.h | 14 ++------------ src/wallet/wallet.cpp | 2 +- src/wallet/walletdb.cpp | 4 ++-- src/wallet/walletdb.h | 2 +- 5 files changed, 11 insertions(+), 39 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 1b2bd83a4ce..34babd26817 100644 --- 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,7 +403,7 @@ bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, bilingual_str& return true; } -bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector& 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 env = GetWalletEnv(file_path, walletFile); @@ -418,19 +411,8 @@ bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vectorVerify(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 index 37f96a1a962..1151124da3e 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -66,17 +66,7 @@ public: 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 @@ public: /* 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& warnings, bilingual_str& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc); + static bool VerifyDatabaseFile(const fs::path& file_path, bilingual_str& errorStr); template bool Read(const K& key, T& value) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3c968abea07..6826782073b 100644 --- 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::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings, uint64_t wallet_creation_flags) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 98597bdb0f3..c0c408e2ecb 100644 --- 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& 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 index e2bf229c687..26c7ae4360f 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -274,7 +274,7 @@ public: /* 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& 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); From 07250b8dcebe2b97ed0fd900ad35cba4091b8ecf Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 15 May 2020 18:48:33 -0400 Subject: [PATCH 05/11] walletdb: remove fAggressive from Salvage The only call to Salvage set fAggressive = true so remove that parameter and always use DB_AGGRESSIVE --- src/wallet/db.cpp | 14 +++----------- src/wallet/db.h | 2 +- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 34babd26817..c51401c1029 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -343,7 +343,7 @@ bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, boo } std::vector salvagedData; - bool fSuccess = env->Salvage(newFilename, true, salvagedData); + bool fSuccess = env->Salvage(newFilename, salvagedData); if (salvagedData.empty()) { LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); @@ -425,25 +425,17 @@ static const char *HEADER_END = "HEADER=END"; /* End of key/value data */ static const char *DATA_END = "DATA=END"; -bool BerkeleyEnvironment::Salvage(const std::string& strFile, bool fAggressive, std::vector& vResult) +bool BerkeleyEnvironment::Salvage(const std::string& strFile, std::vector& vResult) { LOCK(cs_db); assert(mapFileUseCount.count(strFile) == 0); - u_int32_t flags = DB_SALVAGE; - if (fAggressive) - flags |= DB_AGGRESSIVE; - std::stringstream strDump; Db db(dbenv.get(), 0); - int result = db.verify(strFile.c_str(), nullptr, &strDump, flags); + int result = db.verify(strFile.c_str(), nullptr, &strDump, DB_SALVAGE | DB_AGGRESSIVE); if (result == DB_VERIFY_BAD) { LogPrintf("BerkeleyEnvironment::Salvage: Database salvage found errors, all data may not be recoverable.\n"); - if (!fAggressive) { - LogPrintf("BerkeleyEnvironment::Salvage: Rerun with aggressive mode to ignore errors and continue.\n"); - return false; - } } if (result != 0 && result != DB_VERIFY_BAD) { LogPrintf("BerkeleyEnvironment::Salvage: Database salvage failed with result %d.\n", result); diff --git a/src/wallet/db.h b/src/wallet/db.h index 1151124da3e..4543c8c7e0a 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -75,7 +75,7 @@ public: * for huge databases. */ typedef std::pair, std::vector > KeyValPair; - bool Salvage(const std::string& strFile, bool fAggressive, std::vector& vResult); + bool Salvage(const std::string& strFile, std::vector& vResult); bool Open(bool retry); void Close(); From ced95d0e43389fe62b5d30fcc7c42dbca0e88242 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 15 May 2020 18:54:18 -0400 Subject: [PATCH 06/11] Move BerkeleyEnvironment::Salvage into BerkeleyBatch::Recover --- src/wallet/db.cpp | 123 +++++++++++++++++++++++----------------------- src/wallet/db.h | 9 ---- 2 files changed, 62 insertions(+), 70 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index c51401c1029..c115786b7f4 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -317,6 +317,12 @@ BerkeleyBatch::SafeDbt::operator Dbt*() return &m_dbt; } +/* End of headers, beginning of key/value data */ +static const char *HEADER_END = "HEADER=END"; +/* End of key/value data */ +static const char *DATA_END = "DATA=END"; +typedef std::pair, std::vector > KeyValPair; + bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) { std::string filename; @@ -342,8 +348,61 @@ bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, boo return false; } - std::vector salvagedData; - bool fSuccess = env->Salvage(newFilename, salvagedData); + /** + * Salvage data from a file. The DB_AGGRESSIVE flag is being used (see berkeley DB->verify() method documentation). + * key/value pairs are appended to salvagedData which are then written out to a new wallet file. + * NOTE: reads the entire database into memory, so cannot be used + * for huge databases. + */ + std::vector salvagedData; + + std::stringstream strDump; + + Db db(env->dbenv.get(), 0); + result = db.verify(newFilename.c_str(), nullptr, &strDump, DB_SALVAGE | DB_AGGRESSIVE); + if (result == DB_VERIFY_BAD) { + LogPrintf("Salvage: Database salvage found errors, all data may not be recoverable.\n"); + } + if (result != 0 && result != DB_VERIFY_BAD) { + LogPrintf("Salvage: Database salvage failed with result %d.\n", result); + return false; + } + + // Format of bdb dump is ascii lines: + // header lines... + // HEADER=END + // hexadecimal key + // hexadecimal value + // ... repeated + // DATA=END + + std::string strLine; + while (!strDump.eof() && strLine != HEADER_END) + getline(strDump, strLine); // Skip past header + + std::string keyHex, valueHex; + while (!strDump.eof() && keyHex != DATA_END) { + getline(strDump, keyHex); + if (keyHex != DATA_END) { + if (strDump.eof()) + break; + getline(strDump, valueHex); + if (valueHex == DATA_END) { + LogPrintf("Salvage: WARNING: Number of keys in data does not match number of values.\n"); + break; + } + salvagedData.push_back(make_pair(ParseHex(keyHex), ParseHex(valueHex))); + } + } + + bool fSuccess; + if (keyHex != DATA_END) { + LogPrintf("Salvage: WARNING: Unexpected end of file while reading salvage output.\n"); + fSuccess = false; + } else { + fSuccess = (result == 0); + } + if (salvagedData.empty()) { LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); @@ -365,7 +424,7 @@ bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, boo } DbTxn* ptxn = env->TxnBegin(); - for (BerkeleyEnvironment::KeyValPair& row : salvagedData) + for (KeyValPair& row : salvagedData) { if (recoverKVcallback) { @@ -420,64 +479,6 @@ bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, bilingual_str& return true; } -/* End of headers, beginning of key/value data */ -static const char *HEADER_END = "HEADER=END"; -/* End of key/value data */ -static const char *DATA_END = "DATA=END"; - -bool BerkeleyEnvironment::Salvage(const std::string& strFile, std::vector& vResult) -{ - LOCK(cs_db); - assert(mapFileUseCount.count(strFile) == 0); - - std::stringstream strDump; - - Db db(dbenv.get(), 0); - int result = db.verify(strFile.c_str(), nullptr, &strDump, DB_SALVAGE | DB_AGGRESSIVE); - if (result == DB_VERIFY_BAD) { - LogPrintf("BerkeleyEnvironment::Salvage: Database salvage found errors, all data may not be recoverable.\n"); - } - if (result != 0 && result != DB_VERIFY_BAD) { - LogPrintf("BerkeleyEnvironment::Salvage: Database salvage failed with result %d.\n", result); - return false; - } - - // Format of bdb dump is ascii lines: - // header lines... - // HEADER=END - // hexadecimal key - // hexadecimal value - // ... repeated - // DATA=END - - std::string strLine; - while (!strDump.eof() && strLine != HEADER_END) - getline(strDump, strLine); // Skip past header - - std::string keyHex, valueHex; - while (!strDump.eof() && keyHex != DATA_END) { - getline(strDump, keyHex); - if (keyHex != DATA_END) { - if (strDump.eof()) - break; - getline(strDump, valueHex); - if (valueHex == DATA_END) { - LogPrintf("BerkeleyEnvironment::Salvage: WARNING: Number of keys in data does not match number of values.\n"); - break; - } - vResult.push_back(make_pair(ParseHex(keyHex), ParseHex(valueHex))); - } - } - - if (keyHex != DATA_END) { - LogPrintf("BerkeleyEnvironment::Salvage: WARNING: Unexpected end of file while reading salvage output.\n"); - return false; - } - - return (result == 0); -} - - void BerkeleyEnvironment::CheckpointLSN(const std::string& strFile) { dbenv->txn_checkpoint(0, 0, 0); diff --git a/src/wallet/db.h b/src/wallet/db.h index 4543c8c7e0a..4acb414a5b5 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -67,15 +67,6 @@ public: fs::path Directory() const { return strPath; } 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). - * Appends binary key/value pairs to vResult, returns true if successful. - * NOTE: reads the entire database into memory, so cannot be used - * for huge databases. - */ - typedef std::pair, std::vector > KeyValPair; - bool Salvage(const std::string& strFile, std::vector& vResult); bool Open(bool retry); void Close(); From 2741774214168eb287c7066d6823afe5e570381d Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 15 May 2020 19:15:50 -0400 Subject: [PATCH 07/11] Expose a version of ReadKeyValue and use it in RecoverKeysOnlyFilter We need this exposed for BerkeleyBatch::Recover to be moved out. --- src/wallet/walletdb.cpp | 11 ++++++++--- src/wallet/walletdb.h | 3 +++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c0c408e2ecb..c04637934dd 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -588,6 +588,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return true; } +bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr) +{ + CWalletScanState dummy_wss; + LOCK(pwallet->cs_wallet); + return ReadKeyValue(pwallet, ssKey, ssValue, dummy_wss, strType, strErr); +} + bool WalletBatch::IsKeyType(const std::string& strType) { return (strType == DBKeys::KEY || @@ -896,14 +903,12 @@ bool WalletBatch::Recover(const fs::path& wallet_path, std::string& out_backup_f bool WalletBatch::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) { CWallet *dummyWallet = reinterpret_cast(callbackData); - CWalletScanState dummyWss; std::string strType, strErr; bool fReadOK; { // Required in LoadKeyMetadata(): LOCK(dummyWallet->cs_wallet); - fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue, - dummyWss, strType, strErr); + fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue, strType, strErr); } if (!IsKeyType(strType) && strType != DBKeys::HDCHAIN) { return false; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 26c7ae4360f..155046dfba1 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -294,4 +294,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 From b426c7764d26e280e1f814cf36e050743c45cd12 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 15 May 2020 19:24:26 -0400 Subject: [PATCH 08/11] Make BerkeleyBatch::Recover and WalletBatch::RecoverKeysOnlyFilter standalone Instead of having these be class static functions, just make them be standalone. Also removes WalletBatch::Recover which just passed through to BerkeleyBatch::Recover. --- src/wallet/db.cpp | 2 +- src/wallet/db.h | 3 ++- src/wallet/walletdb.cpp | 19 ++----------------- src/wallet/walletdb.h | 9 +++------ src/wallet/wallettool.cpp | 2 +- 5 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index c115786b7f4..cff83e3d011 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -323,7 +323,7 @@ static const char *HEADER_END = "HEADER=END"; static const char *DATA_END = "DATA=END"; typedef std::pair, std::vector > KeyValPair; -bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) +bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) { std::string filename; std::shared_ptr env = GetWalletEnv(file_path, filename); diff --git a/src/wallet/db.h b/src/wallet/db.h index 4acb414a5b5..c8024700456 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -226,7 +226,6 @@ public: void Flush(); void Close(); - static bool Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); /* flush the wallet passively (TRY_LOCK) ideal to be called periodically */ @@ -390,6 +389,8 @@ public: bool static Rewrite(BerkeleyDatabase& database, const char* pszSkip = nullptr); }; +bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); + std::string BerkeleyDatabaseVersion(); #endif // BITCOIN_WALLET_DB_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c04637934dd..2a3daf59b4f 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -885,22 +885,7 @@ void MaybeCompactWalletDB() fOneThread = false; } -// -// Try to (very carefully!) recover wallet file if there is a problem. -// -bool WalletBatch::Recover(const fs::path& wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename) -{ - return BerkeleyBatch::Recover(wallet_path, callbackDataIn, recoverKVcallback, out_backup_filename); -} - -bool WalletBatch::Recover(const fs::path& wallet_path, std::string& out_backup_filename) -{ - // recover without a key filter callback - // results in recovering all record types - return WalletBatch::Recover(wallet_path, nullptr, nullptr, out_backup_filename); -} - -bool WalletBatch::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) +bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) { CWallet *dummyWallet = reinterpret_cast(callbackData); std::string strType, strErr; @@ -910,7 +895,7 @@ bool WalletBatch::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, C LOCK(dummyWallet->cs_wallet); fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue, strType, strErr); } - if (!IsKeyType(strType) && strType != DBKeys::HDCHAIN) { + if (!WalletBatch::IsKeyType(strType) && strType != DBKeys::HDCHAIN) { return false; } if (!fReadOK) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 155046dfba1..389b96189fb 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -263,12 +263,6 @@ public: DBErrors FindWalletTx(std::vector& vTxHash, std::list& vWtx); DBErrors ZapWalletTx(std::list& vWtx); DBErrors ZapSelectTx(std::vector& vHashIn, std::vector& 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 */ @@ -297,4 +291,7 @@ 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); +/* Recover filter (used as callback), will only let keys (cryptographical keys) as KV/key-type pass through */ +bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue); + #endif // BITCOIN_WALLET_WALLETDB_H diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 89645fb5cd7..2b46eef0e00 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -123,7 +123,7 @@ static bool SalvageWallet(const fs::path& path) // Perform the recovery CWallet dummy_wallet(nullptr, WalletLocation(), WalletDatabase::CreateDummy()); std::string backup_filename; - return WalletBatch::Recover(path, (void*)&dummy_wallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename); + return RecoverDatabaseFile(path, (void*)&dummy_wallet, RecoverKeysOnlyFilter, backup_filename); } bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) From 9ea2d258b46e8a9776100633585ed0feede5c2a4 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 15 May 2020 19:37:55 -0400 Subject: [PATCH 09/11] Move RecoverDatabaseFile and RecoverKeysOnlyFilter into salvage.{cpp/h} --- src/Makefile.am | 2 + src/wallet/db.cpp | 128 ------------------------------ src/wallet/db.h | 2 - src/wallet/salvage.cpp | 160 ++++++++++++++++++++++++++++++++++++++ src/wallet/salvage.h | 17 ++++ src/wallet/walletdb.cpp | 22 ------ src/wallet/walletdb.h | 3 - src/wallet/wallettool.cpp | 1 + 8 files changed, 180 insertions(+), 155 deletions(-) create mode 100644 src/wallet/salvage.cpp create mode 100644 src/wallet/salvage.h diff --git a/src/Makefile.am b/src/Makefile.am index 13812de0e7d..0f562433ded 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -243,6 +243,7 @@ BITCOIN_CORE_H = \ wallet/ismine.h \ wallet/load.h \ wallet/rpcwallet.h \ + wallet/salvage.h \ wallet/scriptpubkeyman.h \ wallet/wallet.h \ wallet/walletdb.h \ @@ -351,6 +352,7 @@ libbitcoin_wallet_a_SOURCES = \ wallet/load.cpp \ wallet/rpcdump.cpp \ wallet/rpcwallet.cpp \ + wallet/salvage.cpp \ wallet/scriptpubkeyman.cpp \ wallet/wallet.cpp \ wallet/walletdb.cpp \ diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index cff83e3d011..4ed28b06230 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -317,134 +317,6 @@ BerkeleyBatch::SafeDbt::operator Dbt*() return &m_dbt; } -/* End of headers, beginning of key/value data */ -static const char *HEADER_END = "HEADER=END"; -/* End of key/value data */ -static const char *DATA_END = "DATA=END"; -typedef std::pair, std::vector > KeyValPair; - -bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) -{ - std::string filename; - std::shared_ptr env = GetWalletEnv(file_path, filename); - - // Recovery procedure: - // move wallet file to walletfilename.timestamp.bak - // Call Salvage with fAggressive=true to - // get as much data as possible. - // Rewrite salvaged data to fresh wallet file - // Set -rescan so any missing transactions will be - // found. - int64_t now = GetTime(); - newFilename = strprintf("%s.%d.bak", filename, now); - - int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, - newFilename.c_str(), DB_AUTO_COMMIT); - if (result == 0) - LogPrintf("Renamed %s to %s\n", filename, newFilename); - else - { - LogPrintf("Failed to rename %s to %s\n", filename, newFilename); - return false; - } - - /** - * Salvage data from a file. The DB_AGGRESSIVE flag is being used (see berkeley DB->verify() method documentation). - * key/value pairs are appended to salvagedData which are then written out to a new wallet file. - * NOTE: reads the entire database into memory, so cannot be used - * for huge databases. - */ - std::vector salvagedData; - - std::stringstream strDump; - - Db db(env->dbenv.get(), 0); - result = db.verify(newFilename.c_str(), nullptr, &strDump, DB_SALVAGE | DB_AGGRESSIVE); - if (result == DB_VERIFY_BAD) { - LogPrintf("Salvage: Database salvage found errors, all data may not be recoverable.\n"); - } - if (result != 0 && result != DB_VERIFY_BAD) { - LogPrintf("Salvage: Database salvage failed with result %d.\n", result); - return false; - } - - // Format of bdb dump is ascii lines: - // header lines... - // HEADER=END - // hexadecimal key - // hexadecimal value - // ... repeated - // DATA=END - - std::string strLine; - while (!strDump.eof() && strLine != HEADER_END) - getline(strDump, strLine); // Skip past header - - std::string keyHex, valueHex; - while (!strDump.eof() && keyHex != DATA_END) { - getline(strDump, keyHex); - if (keyHex != DATA_END) { - if (strDump.eof()) - break; - getline(strDump, valueHex); - if (valueHex == DATA_END) { - LogPrintf("Salvage: WARNING: Number of keys in data does not match number of values.\n"); - break; - } - salvagedData.push_back(make_pair(ParseHex(keyHex), ParseHex(valueHex))); - } - } - - bool fSuccess; - if (keyHex != DATA_END) { - LogPrintf("Salvage: WARNING: Unexpected end of file while reading salvage output.\n"); - fSuccess = false; - } else { - fSuccess = (result == 0); - } - - if (salvagedData.empty()) - { - LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); - return false; - } - LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); - - std::unique_ptr pdbCopy = MakeUnique(env->dbenv.get(), 0); - int ret = pdbCopy->open(nullptr, // Txn pointer - filename.c_str(), // Filename - "main", // Logical db name - DB_BTREE, // Database type - DB_CREATE, // Flags - 0); - if (ret > 0) { - LogPrintf("Cannot create database file %s\n", filename); - pdbCopy->close(0); - return false; - } - - DbTxn* ptxn = env->TxnBegin(); - for (KeyValPair& row : salvagedData) - { - if (recoverKVcallback) - { - CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); - CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); - if (!(*recoverKVcallback)(callbackDataIn, ssKey, ssValue)) - continue; - } - Dbt datKey(&row.first[0], row.first.size()); - Dbt datValue(&row.second[0], row.second.size()); - int ret2 = pdbCopy->put(ptxn, &datKey, &datValue, DB_NOOVERWRITE); - if (ret2 > 0) - fSuccess = false; - } - ptxn->commit(0); - pdbCopy->close(0); - - return fSuccess; -} - bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, bilingual_str& errorStr) { std::string walletFile; diff --git a/src/wallet/db.h b/src/wallet/db.h index c8024700456..54ce144ffc4 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -389,8 +389,6 @@ public: bool static Rewrite(BerkeleyDatabase& database, const char* pszSkip = nullptr); }; -bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); - std::string BerkeleyDatabaseVersion(); #endif // BITCOIN_WALLET_DB_H diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp new file mode 100644 index 00000000000..99d62384903 --- /dev/null +++ b/src/wallet/salvage.cpp @@ -0,0 +1,160 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include + +/* End of headers, beginning of key/value data */ +static const char *HEADER_END = "HEADER=END"; +/* End of key/value data */ +static const char *DATA_END = "DATA=END"; +typedef std::pair, std::vector > KeyValPair; + +bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) +{ + std::string filename; + std::shared_ptr env = GetWalletEnv(file_path, filename); + + // Recovery procedure: + // move wallet file to walletfilename.timestamp.bak + // Call Salvage with fAggressive=true to + // get as much data as possible. + // Rewrite salvaged data to fresh wallet file + // Set -rescan so any missing transactions will be + // found. + int64_t now = GetTime(); + newFilename = strprintf("%s.%d.bak", filename, now); + + int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, + newFilename.c_str(), DB_AUTO_COMMIT); + if (result == 0) + LogPrintf("Renamed %s to %s\n", filename, newFilename); + else + { + LogPrintf("Failed to rename %s to %s\n", filename, newFilename); + return false; + } + + /** + * Salvage data from a file. The DB_AGGRESSIVE flag is being used (see berkeley DB->verify() method documentation). + * key/value pairs are appended to salvagedData which are then written out to a new wallet file. + * NOTE: reads the entire database into memory, so cannot be used + * for huge databases. + */ + std::vector salvagedData; + + std::stringstream strDump; + + Db db(env->dbenv.get(), 0); + result = db.verify(newFilename.c_str(), nullptr, &strDump, DB_SALVAGE | DB_AGGRESSIVE); + if (result == DB_VERIFY_BAD) { + LogPrintf("Salvage: Database salvage found errors, all data may not be recoverable.\n"); + } + if (result != 0 && result != DB_VERIFY_BAD) { + LogPrintf("Salvage: Database salvage failed with result %d.\n", result); + return false; + } + + // Format of bdb dump is ascii lines: + // header lines... + // HEADER=END + // hexadecimal key + // hexadecimal value + // ... repeated + // DATA=END + + std::string strLine; + while (!strDump.eof() && strLine != HEADER_END) + getline(strDump, strLine); // Skip past header + + std::string keyHex, valueHex; + while (!strDump.eof() && keyHex != DATA_END) { + getline(strDump, keyHex); + if (keyHex != DATA_END) { + if (strDump.eof()) + break; + getline(strDump, valueHex); + if (valueHex == DATA_END) { + LogPrintf("Salvage: WARNING: Number of keys in data does not match number of values.\n"); + break; + } + salvagedData.push_back(make_pair(ParseHex(keyHex), ParseHex(valueHex))); + } + } + + bool fSuccess; + if (keyHex != DATA_END) { + LogPrintf("Salvage: WARNING: Unexpected end of file while reading salvage output.\n"); + fSuccess = false; + } else { + fSuccess = (result == 0); + } + + if (salvagedData.empty()) + { + LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); + return false; + } + LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); + + std::unique_ptr pdbCopy = MakeUnique(env->dbenv.get(), 0); + int ret = pdbCopy->open(nullptr, // Txn pointer + filename.c_str(), // Filename + "main", // Logical db name + DB_BTREE, // Database type + DB_CREATE, // Flags + 0); + if (ret > 0) { + LogPrintf("Cannot create database file %s\n", filename); + pdbCopy->close(0); + return false; + } + + DbTxn* ptxn = env->TxnBegin(); + for (KeyValPair& row : salvagedData) + { + if (recoverKVcallback) + { + CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); + CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); + if (!(*recoverKVcallback)(callbackDataIn, ssKey, ssValue)) + continue; + } + Dbt datKey(&row.first[0], row.first.size()); + Dbt datValue(&row.second[0], row.second.size()); + int ret2 = pdbCopy->put(ptxn, &datKey, &datValue, DB_NOOVERWRITE); + if (ret2 > 0) + fSuccess = false; + } + ptxn->commit(0); + pdbCopy->close(0); + + return fSuccess; +} + +bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) +{ + CWallet *dummyWallet = reinterpret_cast(callbackData); + std::string strType, strErr; + bool fReadOK; + { + // Required in LoadKeyMetadata(): + LOCK(dummyWallet->cs_wallet); + fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue, strType, strErr); + } + if (!WalletBatch::IsKeyType(strType) && strType != DBKeys::HDCHAIN) { + return false; + } + if (!fReadOK) + { + LogPrintf("WARNING: WalletBatch::Recover skipping %s: %s\n", strType, strErr); + return false; + } + + return true; +} diff --git a/src/wallet/salvage.h b/src/wallet/salvage.h new file mode 100644 index 00000000000..e501c54456e --- /dev/null +++ b/src/wallet/salvage.h @@ -0,0 +1,17 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_WALLET_SALVAGE_H +#define BITCOIN_WALLET_SALVAGE_H + +#include +#include + +bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); + +/* Recover filter (used as callback), will only let keys (cryptographical keys) as KV/key-type pass through */ +bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue); + +#endif // BITCOIN_WALLET_SALVAGE_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 2a3daf59b4f..32fc0026601 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -885,28 +885,6 @@ void MaybeCompactWalletDB() fOneThread = false; } -bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) -{ - CWallet *dummyWallet = reinterpret_cast(callbackData); - std::string strType, strErr; - bool fReadOK; - { - // Required in LoadKeyMetadata(): - LOCK(dummyWallet->cs_wallet); - fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue, strType, strErr); - } - if (!WalletBatch::IsKeyType(strType) && strType != DBKeys::HDCHAIN) { - return false; - } - if (!fReadOK) - { - LogPrintf("WARNING: WalletBatch::Recover skipping %s: %s\n", strType, strErr); - return false; - } - - return true; -} - bool WalletBatch::VerifyEnvironment(const fs::path& wallet_path, bilingual_str& errorStr) { return BerkeleyBatch::VerifyEnvironment(wallet_path, errorStr); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 389b96189fb..bcd1f9303d7 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -291,7 +291,4 @@ 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); -/* Recover filter (used as callback), will only let keys (cryptographical keys) as KV/key-type pass through */ -bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue); - #endif // BITCOIN_WALLET_WALLETDB_H diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 2b46eef0e00..ab5cf0061c6 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include From ea337f2d0318a860f695698cfb3aa91c03ded858 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 15 May 2020 19:45:19 -0400 Subject: [PATCH 10/11] Move RecoverKeysOnlyFilter into RecoverDataBaseFile --- src/wallet/salvage.cpp | 48 ++++++++++++++++----------------------- src/wallet/salvage.h | 5 +--- src/wallet/wallettool.cpp | 4 +--- 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index 99d62384903..70067ebef04 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -15,7 +15,7 @@ static const char *HEADER_END = "HEADER=END"; static const char *DATA_END = "DATA=END"; typedef std::pair, std::vector > KeyValPair; -bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) +bool RecoverDatabaseFile(const fs::path& file_path) { std::string filename; std::shared_ptr env = GetWalletEnv(file_path, filename); @@ -28,7 +28,7 @@ bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool ( // Set -rescan so any missing transactions will be // found. int64_t now = GetTime(); - newFilename = strprintf("%s.%d.bak", filename, now); + std::string newFilename = strprintf("%s.%d.bak", filename, now); int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, newFilename.c_str(), DB_AUTO_COMMIT); @@ -116,14 +116,26 @@ bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool ( } DbTxn* ptxn = env->TxnBegin(); + CWallet dummyWallet(nullptr, WalletLocation(), WalletDatabase::CreateDummy()); for (KeyValPair& row : salvagedData) { - if (recoverKVcallback) + /* Filter for only private key type KV pairs to be added to the salvaged wallet */ + CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); + CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); + std::string strType, strErr; + bool fReadOK; { - CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); - CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); - if (!(*recoverKVcallback)(callbackDataIn, ssKey, ssValue)) - continue; + // Required in LoadKeyMetadata(): + LOCK(dummyWallet.cs_wallet); + fReadOK = ReadKeyValue(&dummyWallet, ssKey, ssValue, strType, strErr); + } + if (!WalletBatch::IsKeyType(strType) && strType != DBKeys::HDCHAIN) { + continue; + } + if (!fReadOK) + { + LogPrintf("WARNING: WalletBatch::Recover skipping %s: %s\n", strType, strErr); + continue; } Dbt datKey(&row.first[0], row.first.size()); Dbt datValue(&row.second[0], row.second.size()); @@ -136,25 +148,3 @@ bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool ( return fSuccess; } - -bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) -{ - CWallet *dummyWallet = reinterpret_cast(callbackData); - std::string strType, strErr; - bool fReadOK; - { - // Required in LoadKeyMetadata(): - LOCK(dummyWallet->cs_wallet); - fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue, strType, strErr); - } - if (!WalletBatch::IsKeyType(strType) && strType != DBKeys::HDCHAIN) { - return false; - } - if (!fReadOK) - { - LogPrintf("WARNING: WalletBatch::Recover skipping %s: %s\n", strType, strErr); - return false; - } - - return true; -} diff --git a/src/wallet/salvage.h b/src/wallet/salvage.h index e501c54456e..e361930f5ed 100644 --- a/src/wallet/salvage.h +++ b/src/wallet/salvage.h @@ -9,9 +9,6 @@ #include #include -bool RecoverDatabaseFile(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); - -/* Recover filter (used as callback), will only let keys (cryptographical keys) as KV/key-type pass through */ -bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue); +bool RecoverDatabaseFile(const fs::path& file_path); #endif // BITCOIN_WALLET_SALVAGE_H diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index ab5cf0061c6..be07c285030 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -122,9 +122,7 @@ static bool SalvageWallet(const fs::path& path) } // Perform the recovery - CWallet dummy_wallet(nullptr, WalletLocation(), WalletDatabase::CreateDummy()); - std::string backup_filename; - return RecoverDatabaseFile(path, (void*)&dummy_wallet, RecoverKeysOnlyFilter, backup_filename); + return RecoverDatabaseFile(path); } bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) From 84ae0578b6c68dda145ca65fef510ce0fdac0d7b Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 21 May 2020 14:33:54 -0400 Subject: [PATCH 11/11] Add release notes about salvage changes --- doc/release-notes-18918.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 doc/release-notes-18918.md diff --git a/doc/release-notes-18918.md b/doc/release-notes-18918.md new file mode 100644 index 00000000000..f57a62eeb7d --- /dev/null +++ b/doc/release-notes-18918.md @@ -0,0 +1,3 @@ +# Wallet + +The `-salvagewallet` startup option has been removed. A new `salvage` command has been added to the `bitcoin-wallet` tool which performs the salvage operations that `-salvagewallet` did.