Skip to content

Commit

Permalink
refactor: Use util::Result class for wallet loading
Browse files Browse the repository at this point in the history 
Wallet loading functions up and down the stack have lots of error and warning
parameters, and return error information in different ways. This PR makes them
uniformly return util::Result, without changing behavior.
  • Loading branch information
@ryanofsky
ryanofsky committed Oct 14, 2022
1 parent 222c510 commit 7bd578a
Show file tree
Hide file tree
Showing 24 changed files with 401 additions and 577 deletions.
8 changes: 4 additions & 4 deletions src/bench/wallet_loading.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ static const std::shared_ptr<CWallet> BenchLoadWallet(std::unique_ptr<WalletData
{
bilingual_str error;
std::vector<bilingual_str> warnings;
auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings);
NotifyWalletLoaded(context, wallet);
auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags);
NotifyWalletLoaded(context, *wallet);
if (context.chain) {
wallet->postInitProcess();
(*wallet)->postInitProcess();
}
return wallet;
return *wallet;
}

static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet)
Expand Down
6 changes: 3 additions & 3 deletions src/interfaces/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,16 +320,16 @@ class WalletLoader : public ChainClient
{
public:
//! Create new wallet.
virtual util::Result<std::unique_ptr<Wallet>> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, std::vector<bilingual_str>& warnings) = 0;
virtual util::Result<std::unique_ptr<Wallet>> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags) = 0;

//! Load existing wallet.
virtual util::Result<std::unique_ptr<Wallet>> loadWallet(const std::string& name, std::vector<bilingual_str>& warnings) = 0;
virtual util::Result<std::unique_ptr<Wallet>> loadWallet(const std::string& name) = 0;

//! Return default wallet directory.
virtual std::string getWalletDir() = 0;

//! Restore backup wallet
virtual util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0;
virtual util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name) = 0;

//! Return available wallets in wallet directory.
virtual std::vector<std::string> listWalletDir() = 0;
Expand Down
21 changes: 9 additions & 12 deletions src/qt/walletcontroller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,11 @@ void CreateWalletActivity::createWallet()
}

QTimer::singleShot(500ms, worker(), [this, name, flags] {
auto wallet{node().walletLoader().createWallet(name, m_passphrase, flags, m_warning_message)};

auto wallet{node().walletLoader().createWallet(name, m_passphrase, flags)};
m_error_message = Join(wallet.GetErrors(), Untranslated(" "));
m_warning_message = wallet.GetWarnings();
if (wallet) {
m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
} else {
m_error_message = util::ErrorString(wallet);
}

QTimer::singleShot(500ms, this, &CreateWalletActivity::finish);
Expand Down Expand Up @@ -351,12 +350,11 @@ void OpenWalletActivity::open(const std::string& path)
tr("Opening Wallet <b>%1</b>…").arg(name.toHtmlEscaped()));

QTimer::singleShot(0, worker(), [this, path] {
auto wallet{node().walletLoader().loadWallet(path, m_warning_message)};

auto wallet{node().walletLoader().loadWallet(path)};
m_error_message = Join(wallet.GetErrors(), Untranslated(" "));
m_warning_message = wallet.GetWarnings();
if (wallet) {
m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
} else {
m_error_message = util::ErrorString(wallet);
}

QTimer::singleShot(0, this, &OpenWalletActivity::finish);
Expand Down Expand Up @@ -403,12 +401,11 @@ void RestoreWalletActivity::restore(const fs::path& backup_file, const std::stri
tr("Restoring Wallet <b>%1</b>…").arg(name.toHtmlEscaped()));

QTimer::singleShot(0, worker(), [this, backup_file, wallet_name] {
auto wallet{node().walletLoader().restoreWallet(backup_file, wallet_name, m_warning_message)};

auto wallet{node().walletLoader().restoreWallet(backup_file, wallet_name)};
m_error_message = Join(wallet.GetErrors(), Untranslated(" "));
m_warning_message = wallet.GetWarnings();
if (wallet) {
m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
} else {
m_error_message = util::ErrorString(wallet);
}

QTimer::singleShot(0, this, &RestoreWalletActivity::finish);
Expand Down
52 changes: 24 additions & 28 deletions src/wallet/bdb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,17 @@ BerkeleyEnvironment::~BerkeleyEnvironment()
Close();
}

bool BerkeleyEnvironment::Open(bilingual_str& err)
util::Result<void> BerkeleyEnvironment::Open()
{
if (fDbEnvInit) {
return true;
return {};
}

fs::path pathIn = fs::PathFromString(strPath);
TryCreateDirectories(pathIn);
if (!LockDirectory(pathIn, ".walletlock")) {
LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance may be using it.\n", strPath);
err = strprintf(_("Error initializing wallet database environment %s!"), fs::quoted(fs::PathToString(Directory())));
return false;
return {util::Error{strprintf(_("Error initializing wallet database environment %s!"), fs::quoted(fs::PathToString(Directory())))}};
}

fs::path pathLogDir = pathIn / "database";
Expand Down Expand Up @@ -177,16 +176,16 @@ bool BerkeleyEnvironment::Open(bilingual_str& err)
LogPrintf("BerkeleyEnvironment::Open: Error %d closing failed database environment: %s\n", ret2, DbEnv::strerror(ret2));
}
Reset();
err = strprintf(_("Error initializing wallet database environment %s!"), fs::quoted(fs::PathToString(Directory())));
auto err = strprintf(_("Error initializing wallet database environment %s!"), fs::quoted(fs::PathToString(Directory())));
if (ret == DB_RUNRECOVERY) {
err += Untranslated(" ") + _("This error could occur if this wallet was not shutdown cleanly and was last loaded using a build with a newer version of Berkeley DB. If so, please use the software that last loaded this wallet");
}
return false;
return {util::Error{std::move(err)}};
}

fDbEnvInit = true;
fMockDb = false;
return true;
return {};
}

//! Construct an in-memory mock Berkeley environment for testing
Expand Down Expand Up @@ -259,17 +258,16 @@ BerkeleyBatch::SafeDbt::operator Dbt*()
return &m_dbt;
}

bool BerkeleyDatabase::Verify(bilingual_str& errorStr)
util::Result<void> BerkeleyDatabase::Verify()
{
fs::path walletDir = env->Directory();
fs::path file_path = walletDir / m_filename;

LogPrintf("Using BerkeleyDB version %s\n", BerkeleyDatabaseVersion());
LogPrintf("Using wallet %s\n", fs::PathToString(file_path));

if (!env->Open(errorStr)) {
return false;
}
util::Result<void> opened = env->Open();
if (!opened) return opened;

if (fs::exists(file_path))
{
Expand All @@ -279,12 +277,11 @@ bool BerkeleyDatabase::Verify(bilingual_str& errorStr)
const std::string strFile = fs::PathToString(m_filename);
int result = db.verify(strFile.c_str(), nullptr, nullptr, 0);
if (result != 0) {
errorStr = strprintf(_("%s corrupt. Try using the wallet tool bitcoin-wallet to salvage or restoring a backup."), fs::quoted(fs::PathToString(file_path)));
return false;
return {util::Error{strprintf(_("%s corrupt. Try using the wallet tool bitcoin-wallet to salvage or restoring a backup."), fs::quoted(fs::PathToString(file_path)))}};
}
}
// also return true if files does not exists
return true;
return {};
}

void BerkeleyEnvironment::CheckpointLSN(const std::string& strFile)
Expand Down Expand Up @@ -324,9 +321,10 @@ void BerkeleyDatabase::Open()

{
LOCK(cs_db);
bilingual_str open_err;
if (!env->Open(open_err))
throw std::runtime_error("BerkeleyDatabase: Failed to open database environment.");
auto opened = env->Open();
if (!opened) {
throw std::runtime_error("BerkeleyDatabase: Failed to open database environment. " + util::ErrorString(opened).original);
}

if (m_db == nullptr) {
int ret;
Expand Down Expand Up @@ -442,8 +440,8 @@ void BerkeleyEnvironment::ReloadDbEnv()
// Reset the environment
Flush(true); // This will flush and close the environment
Reset();
bilingual_str open_err;
Open(open_err);
auto opened = Open();
if (!opened) LogPrintf("BerkeleyEnvironment::ReloadDbEnv Open failed: %s\n", util::ErrorString(opened).original);
}

bool BerkeleyDatabase::Rewrite(const char* pszSkip)
Expand Down Expand Up @@ -826,7 +824,7 @@ std::unique_ptr<DatabaseBatch> BerkeleyDatabase::MakeBatch(bool flush_on_close)
return std::make_unique<BerkeleyBatch>(*this, false, flush_on_close);
}

std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error)
util::Result<std::unique_ptr<BerkeleyDatabase>, DatabaseError> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options)
{
fs::path data_file = BDBDataFile(path);
std::unique_ptr<BerkeleyDatabase> db;
Expand All @@ -835,19 +833,17 @@ std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, con
fs::path data_filename = data_file.filename();
std::shared_ptr<BerkeleyEnvironment> env = GetBerkeleyEnv(data_file.parent_path(), options.use_shared_memory);
if (env->m_databases.count(data_filename)) {
error = Untranslated(strprintf("Refusing to load database. Data file '%s' is already loaded.", fs::PathToString(env->Directory() / data_filename)));
status = DatabaseStatus::FAILED_ALREADY_LOADED;
return nullptr;
return {util::Error{Untranslated(strprintf("Refusing to load database. Data file '%s' is already loaded.", fs::PathToString(env->Directory() / data_filename)))},
DatabaseError::FAILED_ALREADY_LOADED};
}
db = std::make_unique<BerkeleyDatabase>(std::move(env), std::move(data_filename), options);
}

if (options.verify && !db->Verify(error)) {
status = DatabaseStatus::FAILED_VERIFY;
return nullptr;
util::Result<void> verified;
if (options.verify && !(verified = db->Verify())) {
return {util::Error{}, std::move(verified), DatabaseError::FAILED_VERIFY};
}

status = DatabaseStatus::SUCCESS;
return db;
return {std::move(db)};
}
} // namespace wallet
6 changes: 3 additions & 3 deletions src/wallet/bdb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class BerkeleyEnvironment
bool IsInitialized() const { return fDbEnvInit; }
fs::path Directory() const { return fs::PathFromString(strPath); }

bool Open(bilingual_str& error);
util::Result<void> Open();
void Close();
void Flush(bool fShutdown);
void CheckpointLSN(const std::string& strFile);
Expand Down Expand Up @@ -138,7 +138,7 @@ class BerkeleyDatabase : public WalletDatabase
void ReloadDbEnv() override;

/** Verifies the environment and database file */
bool Verify(bilingual_str& error);
util::Result<void> Verify();

/** Return path to main database filename */
std::string Filename() override { return fs::PathToString(env->Directory() / m_filename); }
Expand Down Expand Up @@ -229,7 +229,7 @@ std::string BerkeleyDatabaseVersion();
bool BerkeleyDatabaseSanityCheck();

//! Return object giving access to Berkeley database at specified path.
std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
util::Result<std::unique_ptr<BerkeleyDatabase>, DatabaseError> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options);
} // namespace wallet

#endif // BITCOIN_WALLET_BDB_H
6 changes: 3 additions & 3 deletions src/wallet/db.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <fs.h>
#include <streams.h>
#include <support/allocators/secure.h>
#include <util/result.h>

#include <atomic>
#include <memory>
Expand Down Expand Up @@ -216,8 +217,7 @@ struct DatabaseOptions {
int64_t max_log_mb = 100; //!< Max log size to allow before consolidating.
};

enum class DatabaseStatus {
SUCCESS,
enum class DatabaseError {
FAILED_BAD_PATH,
FAILED_BAD_FORMAT,
FAILED_ALREADY_LOADED,
Expand All @@ -234,7 +234,7 @@ enum class DatabaseStatus {
std::vector<fs::path> ListDatabases(const fs::path& path);

void ReadDatabaseArgs(const ArgsManager& args, DatabaseOptions& options);
std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
util::Result<std::unique_ptr<WalletDatabase>, DatabaseError> MakeDatabase(const fs::path& path, const DatabaseOptions& options);

fs::path BDBDataFile(const fs::path& path);
fs::path SQLiteDataFile(const fs::path& path);
Expand Down

0 comments on commit 7bd578a

Please sign in to comment.