walletdb: Remove m_mock from SQLiteDatabase
What the developer wrote
Authored by Ava Chow
walletdb: Remove m_mock from SQLiteDatabase
What changed, and why it matters
This commit is a small internal code cleanup in Bitcoin Core's wallet database code. It removes a special 'mock' flag used only for testing and replaces it with a more general way to pass extra SQLite database options. The change only affects test helper code and how mock (in-memory) wallets are created during tests. There is no indication it fixes a security bug or introduces a security risk.
No security action required. Treat as normal code maintenance. Reviewers may verify that mock wallet tests still pass and that production SQLiteDatabase construction still uses file-backed databases by default.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors SQLiteDatabase construction: it removes the boolean m_mock member and the corresponding constructor parameter, replacing it with an int additional_flags parameter used only by a protected constructor. The public constructor no longer accepts a mock flag; test code now creates mock databases by calling the protected constructor with SQLITE_OPEN_MEMORY directly. A CMake dependency on SQLite3 is added to test_util only when wallets are enabled. This is a structural/testability refactor with no functional change to production wallet behavior.
Changed components
src/wallet/sqlite.cppsrc/wallet/sqlite.hsrc/wallet/test/util.cppsrc/test/util/CMakeLists.txtInspect captured patch +24 / −12
diff --git a/src/test/util/CMakeLists.txt b/src/test/util/CMakeLists.txt
index 32396c41..4cb00cec 100644
--- a/src/test/util/CMakeLists.txt
+++ b/src/test/util/CMakeLists.txt
@@ -25,6 +25,7 @@ target_link_libraries(test_util
PRIVATE
core_interface
Boost::headers
+ $<$<BOOL:${ENABLE_WALLET}>:SQLite3::SQLite3>
PUBLIC
univalue
)
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index e72455a3..3d6583bb 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -111,8 +111,12 @@ static void SetPragma(sqlite3* db, const std::string& key, const std::string& va
Mutex SQLiteDatabase::g_sqlite_mutex;
int SQLiteDatabase::g_sqlite_count = 0;
-SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock)
- : WalletDatabase(), m_mock(mock), m_dir_path(dir_path), m_file_path(fs::PathToString(file_path)), m_write_semaphore(1), m_use_unsafe_sync(options.use_unsafe_sync)
+SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options)
+ : SQLiteDatabase(dir_path, file_path, options, /*additional_flags=*/0)
+{}
+
+SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, int additional_flags)
+ : WalletDatabase(), m_dir_path(dir_path), m_file_path(fs::PathToString(file_path)), m_write_semaphore(1), m_use_unsafe_sync(options.use_unsafe_sync)
{
{
LOCK(g_sqlite_mutex);
@@ -135,7 +139,7 @@ SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_pa
}
try {
- Open();
+ Open(additional_flags);
} catch (const std::runtime_error&) {
// If open fails, cleanup this object and rethrow the exception
Cleanup();
@@ -243,13 +247,15 @@ bool SQLiteDatabase::Verify(bilingual_str& error)
void SQLiteDatabase::Open()
{
- int flags = SQLITE_OPEN_FULLMUTEX | SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
- if (m_mock) {
- flags |= SQLITE_OPEN_MEMORY; // In memory database for mock db
- }
+ Open(/*additional_flags*/0);
+}
+
+void SQLiteDatabase::Open(int additional_flags)
+{
+ int flags = SQLITE_OPEN_FULLMUTEX | SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | additional_flags;
if (m_db == nullptr) {
- if (!m_mock) {
+ if (!(flags & SQLITE_OPEN_MEMORY)) {
TryCreateDirectories(m_dir_path);
}
int ret = sqlite3_open_v2(m_file_path.c_str(), &m_db, flags, nullptr);
diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h
index f467996d..fb0fa39c 100644
--- a/src/wallet/sqlite.h
+++ b/src/wallet/sqlite.h
@@ -103,8 +103,6 @@ public:
class SQLiteDatabase : public WalletDatabase
{
private:
- const bool m_mock{false};
-
const fs::path m_dir_path;
const std::string m_file_path;
@@ -120,11 +118,16 @@ private:
void Cleanup() noexcept EXCLUSIVE_LOCKS_REQUIRED(!g_sqlite_mutex);
+ void Open(int additional_flags);
+
+protected:
+ SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, int additional_flags);
+
public:
SQLiteDatabase() = delete;
/** Create DB handle to real database */
- SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock = false);
+ SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options);
~SQLiteDatabase();
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index c799721c..b21c96f1 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -14,6 +14,8 @@
#include <wallet/wallet.h>
#include <wallet/walletdb.h>
+#include <sqlite3.h>
+
#include <memory>
namespace wallet {
@@ -136,7 +138,7 @@ CTxDestination getNewDestination(CWallet& w, OutputType output_type)
}
MockableSQLiteDatabase::MockableSQLiteDatabase()
- : SQLiteDatabase(fs::PathFromString("mock/"), fs::PathFromString("mock/wallet.dat"), DatabaseOptions(), /*mock=*/true)
+ : SQLiteDatabase(fs::PathFromString("mock/"), fs::PathFromString("mock/wallet.dat"), DatabaseOptions(), SQLITE_OPEN_MEMORY)
{}
std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase()
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.