Merge bitcoin/bitcoin#29117: wallettool: Always be able to dump a wallet's database

d83bea42d1 wallettool: Don't create CWallet when dumping DB (Andrew Chow)
40c80e36b1 wallettool: Don't unilaterally reset wallet_instance if loading error (Ava Chow)

Pull request description:

  https://github.com/bitcoin/bitcoin/issues/29109#issuecomment-1863449058 reports that a wallet with noncritical errors cannot be dumped with `bitcoin-wallet dump`. This was caused by an erroneous reset of the wallet pointer when the loading the wallet returns something other than `LOAD_OK`. Not all errors are errors that require aborting, so unilaterally resetting the pointer at that time is incorrect. The first commit resolves this issue.

  Furthermore, if a wallet has loading errors, that should not prevent the wallet tool from dumping the wallet. The wallet application logic should not get in the way of performing such a low level database operation, especially when it's primary usage is for debugging potentially corrupted wallets. The 2nd commit is taken from #28710 and changes the `dump` to stop at making a `WalletDatabase` rather than making a `CWallet` only to retrieve the underlying `WalletDatabase`.

ACKs for top commit:
  furszy:
    Code review ACK d83bea42d1
  BrandonOdiwuor:
    Code Review ACK d83bea42d1

Tree-SHA512: 425d712dfff1002bd81272aca0bae1016f9126a3c89506f8cb7cf0a0ec9f33d0c03b8d03896394f3a45c2998e59047e19218dfd08dc8a5f40e8625134e886b0f
This commit is contained in:
fanquake
2024-01-05 17:35:18 +00:00
3 changed files with 13 additions and 11 deletions

View File

@@ -8,6 +8,7 @@
#include <util/fs.h>
#include <util/translation.h>
#include <wallet/wallet.h>
#include <wallet/walletdb.h>
#include <algorithm>
#include <fstream>
@@ -20,7 +21,7 @@ namespace wallet {
static const std::string DUMP_MAGIC = "BITCOIN_CORE_WALLET_DUMP";
uint32_t DUMP_VERSION = 1;
bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error)
bool DumpWallet(const ArgsManager& args, WalletDatabase& db, bilingual_str& error)
{
// Get the dumpfile
std::string dump_filename = args.GetArg("-dumpfile", "");
@@ -44,7 +45,6 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error)
HashWriter hasher{};
WalletDatabase& db = wallet.GetDatabase();
std::unique_ptr<DatabaseBatch> batch = db.MakeBatch();
bool ret = true;
@@ -90,9 +90,6 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error)
cursor.reset();
batch.reset();
// Close the wallet after we're done with it. The caller won't be doing this
wallet.Close();
if (ret) {
// Write the hash
tfm::format(dump_file, "checksum,%s\n", HexStr(hasher.GetHash()));

View File

@@ -14,8 +14,9 @@ struct bilingual_str;
class ArgsManager;
namespace wallet {
class CWallet;
bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error);
class WalletDatabase;
bool DumpWallet(const ArgsManager& args, WalletDatabase& db, bilingual_str& error);
bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs::path& wallet_path, bilingual_str& error, std::vector<bilingual_str>& warnings);
} // namespace wallet

View File

@@ -68,7 +68,6 @@ static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::pa
}
if (load_wallet_ret != DBErrors::LOAD_OK) {
wallet_instance = nullptr;
if (load_wallet_ret == DBErrors::CORRUPT) {
tfm::format(std::cerr, "Error loading %s: Wallet corrupted", name);
return nullptr;
@@ -194,10 +193,15 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
DatabaseOptions options;
ReadDatabaseArgs(args, options);
options.require_existing = true;
const std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options);
if (!wallet_instance) return false;
DatabaseStatus status;
bilingual_str error;
bool ret = DumpWallet(args, *wallet_instance, error);
std::unique_ptr<WalletDatabase> database = MakeDatabase(path, options, status, error);
if (!database) {
tfm::format(std::cerr, "%s\n", error.original);
return false;
}
bool ret = DumpWallet(args, *database, error);
if (!ret && !error.empty()) {
tfm::format(std::cerr, "%s\n", error.original);
return ret;