From 43369f37060a1b4c987672707c500d35c9a27c1d Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 6 Jul 2023 18:18:33 +0000 Subject: [PATCH 1/7] wallet: bdb: drop default parameter --- src/wallet/bdb.cpp | 2 +- src/wallet/bdb.h | 2 +- src/wallet/salvage.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 658500e4563..e17359f1508 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -742,7 +742,7 @@ bool BerkeleyBatch::TxnBegin() { if (!pdb || activeTxn) return false; - DbTxn* ptxn = env->TxnBegin(); + DbTxn* ptxn = env->TxnBegin(DB_TXN_WRITE_NOSYNC); if (!ptxn) return false; activeTxn = ptxn; diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 1073d32e0f2..e10ebbbdfd9 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -67,7 +67,7 @@ public: void CloseDb(const fs::path& filename); void ReloadDbEnv(); - DbTxn* TxnBegin(int flags = DB_TXN_WRITE_NOSYNC) + DbTxn* TxnBegin(int flags) { DbTxn* ptxn = nullptr; int ret = dbenv->txn_begin(nullptr, &ptxn, flags); diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index da16435f041..a9b84fbcabf 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -175,7 +175,7 @@ bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bil return false; } - DbTxn* ptxn = env->TxnBegin(); + DbTxn* ptxn = env->TxnBegin(DB_TXN_WRITE_NOSYNC); CWallet dummyWallet(nullptr, "", std::make_unique()); for (KeyValPair& row : salvagedData) { From 4216f69250937b1ca4650dc0c21678a8444c6650 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 6 Jul 2023 18:23:23 +0000 Subject: [PATCH 2/7] wallet: bdb: move TxnBegin to cpp file since it uses a bdb function --- src/wallet/bdb.cpp | 9 +++++++++ src/wallet/bdb.h | 9 +-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index e17359f1508..e08391547ad 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -462,6 +462,15 @@ void BerkeleyEnvironment::ReloadDbEnv() Open(open_err); } +DbTxn* BerkeleyEnvironment::TxnBegin(int flags) +{ + DbTxn* ptxn = nullptr; + int ret = dbenv->txn_begin(nullptr, &ptxn, flags); + if (!ptxn || ret != 0) + return nullptr; + return ptxn; +} + bool BerkeleyDatabase::Rewrite(const char* pszSkip) { while (true) { diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index e10ebbbdfd9..9552d8ce252 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -67,14 +67,7 @@ public: void CloseDb(const fs::path& filename); void ReloadDbEnv(); - DbTxn* TxnBegin(int flags) - { - DbTxn* ptxn = nullptr; - int ret = dbenv->txn_begin(nullptr, &ptxn, flags); - if (!ptxn || ret != 0) - return nullptr; - return ptxn; - } + DbTxn* TxnBegin(int flags); }; /** Get BerkeleyEnvironment given a directory path. */ From e5e5aa1da261633c8f73b97d5aefe5dc450a7db9 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 6 Jul 2023 18:32:24 +0000 Subject: [PATCH 3/7] wallet: bdb: move SpanFromDbt to below SafeDbt's implementation No functional change, just simplifies the code move in the next commit. --- src/wallet/bdb.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index e08391547ad..4d410fa39e5 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -31,10 +31,6 @@ namespace wallet { namespace { -Span SpanFromDbt(const SafeDbt& dbt) -{ - return {reinterpret_cast(dbt.get_data()), dbt.get_size()}; -} //! Make sure database has a unique fileid within the environment. If it //! doesn't, throw an error. BDB caches do not work properly when more than one @@ -275,6 +271,11 @@ SafeDbt::operator Dbt*() return &m_dbt; } +static Span SpanFromDbt(const SafeDbt& dbt) +{ + return {reinterpret_cast(dbt.get_data()), dbt.get_size()}; +} + bool BerkeleyDatabase::Verify(bilingual_str& errorStr) { fs::path walletDir = env->Directory(); From b3582baa3a2f84db7d2fb5a681121a5f2d6de3a1 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 6 Jul 2023 18:35:13 +0000 Subject: [PATCH 4/7] wallet: bdb: move SafeDbt to cpp file Dbt requires including bdb headers. --- src/wallet/bdb.cpp | 20 ++++++++++++++++++++ src/wallet/bdb.h | 20 -------------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 4d410fa39e5..26938e0a7d5 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -232,6 +232,26 @@ BerkeleyEnvironment::BerkeleyEnvironment() : m_use_shared_memory(false) fMockDb = true; } +/** RAII class that automatically cleanses its data on destruction */ +class SafeDbt final +{ + Dbt m_dbt; + +public: + // construct Dbt with internally-managed data + SafeDbt(); + // construct Dbt with provided data + SafeDbt(void* data, size_t size); + ~SafeDbt(); + + // delegate to Dbt + const void* get_data() const; + uint32_t get_size() const; + + // conversion operator to access the underlying Dbt + operator Dbt*(); +}; + SafeDbt::SafeDbt() { m_dbt.set_flags(DB_DBT_MALLOC); diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 9552d8ce252..da32195dc62 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -152,26 +152,6 @@ public: std::unique_ptr MakeBatch(bool flush_on_close = true) override; }; -/** RAII class that automatically cleanses its data on destruction */ -class SafeDbt final -{ - Dbt m_dbt; - -public: - // construct Dbt with internally-managed data - SafeDbt(); - // construct Dbt with provided data - SafeDbt(void* data, size_t size); - ~SafeDbt(); - - // delegate to Dbt - const void* get_data() const; - uint32_t get_size() const; - - // conversion operator to access the underlying Dbt - operator Dbt*(); -}; - class BerkeleyCursor : public DatabaseCursor { private: From 004b184b027520a4f9019d1432a816e6ec891fe3 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 6 Jul 2023 19:29:27 +0000 Subject: [PATCH 5/7] wallet: bdb: move BerkeleyDatabase constructor to cpp file Else some compilers/stdlibs may not be able to construct std::unique_ptr without Db defined. --- src/wallet/bdb.cpp | 7 +++++++ src/wallet/bdb.h | 7 +------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 26938e0a7d5..d74950c5b11 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -296,6 +296,13 @@ static Span SpanFromDbt(const SafeDbt& dbt) return {reinterpret_cast(dbt.get_data()), dbt.get_size()}; } +BerkeleyDatabase::BerkeleyDatabase(std::shared_ptr env, fs::path filename, const DatabaseOptions& options) : + WalletDatabase(), env(std::move(env)), m_filename(std::move(filename)), m_max_log_mb(options.max_log_mb) +{ + auto inserted = this->env->m_databases.emplace(m_filename, std::ref(*this)); + assert(inserted.second); +} + bool BerkeleyDatabase::Verify(bilingual_str& errorStr) { fs::path walletDir = env->Directory(); diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index da32195dc62..0470a3183af 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -84,12 +84,7 @@ public: BerkeleyDatabase() = delete; /** Create DB handle to real database */ - BerkeleyDatabase(std::shared_ptr env, fs::path filename, const DatabaseOptions& options) : - WalletDatabase(), env(std::move(env)), m_filename(std::move(filename)), m_max_log_mb(options.max_log_mb) - { - auto inserted = this->env->m_databases.emplace(m_filename, std::ref(*this)); - assert(inserted.second); - } + BerkeleyDatabase(std::shared_ptr env, fs::path filename, const DatabaseOptions& options); ~BerkeleyDatabase() override; From 6e010626af7ed51f1748323ece2f46335e145f2f Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 6 Jul 2023 19:09:50 +0000 Subject: [PATCH 6/7] wallet: bdb: don't use bdb define in header --- src/wallet/bdb.cpp | 2 ++ src/wallet/bdb.h | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index d74950c5b11..69a0201d4a4 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -29,6 +29,8 @@ #endif #endif +static_assert(BDB_DB_FILE_ID_LEN == DB_FILE_ID_LEN, "DB_FILE_ID_LEN should be 20."); + namespace wallet { namespace { diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 0470a3183af..f9b0b7601d4 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -25,10 +25,14 @@ struct bilingual_str; +// This constant was introduced in BDB 4.0.14 and has never changed, but there +// is a belt-and-suspenders check in the cpp file just in case. +#define BDB_DB_FILE_ID_LEN 20 /* Unique file ID length. */ + namespace wallet { struct WalletDatabaseFileId { - uint8_t value[DB_FILE_ID_LEN]; + uint8_t value[BDB_DB_FILE_ID_LEN]; bool operator==(const WalletDatabaseFileId& rhs) const; }; From 8b5397c00e821d7eaab22f512e9d71a1a0392ebf Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 6 Jul 2023 19:12:28 +0000 Subject: [PATCH 7/7] wallet: bdb: include bdb header from our implementation files only This way the dependency can't sneak into other files without being noticed. Forward-declare bdb classes as necessary. --- src/wallet/bdb.cpp | 1 + src/wallet/bdb.h | 7 +++++-- src/wallet/salvage.cpp | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 69a0201d4a4..9ea43ca67c5 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -18,6 +18,7 @@ #include +#include #include // Windows may not define S_IRUSR or S_IWUSR. We define both diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index f9b0b7601d4..630630ebe01 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -21,10 +21,13 @@ #include #include -#include - struct bilingual_str; +class DbEnv; +class DbTxn; +class Db; +class Dbc; + // This constant was introduced in BDB 4.0.14 and has never changed, but there // is a belt-and-suspenders check in the cpp file just in case. #define BDB_DB_FILE_ID_LEN 20 /* Unique file ID length. */ diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index a9b84fbcabf..0a0745b1c50 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -11,6 +11,8 @@ #include #include +#include + namespace wallet { /* End of headers, beginning of key/value data */ static const char *HEADER_END = "HEADER=END";