wallet: Make Mockable{Database,Batch} subclasses of SQLite classes
What changed, and why it matters
This commit is a test-only refactoring change. It replaces a custom in-memory fake database used in Bitcoin Core's automated tests with a real SQLite database running in memory. There is no change to how actual user wallets work, no bug fix for live code, and no security issue introduced or fixed.
No security action needed. Review as normal code-quality/test-coverage refactoring if desired.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch removes the hand-rolled MockableDatabase/MockableBatch/MockableCursor classes (which stored records in a std::map and simulated database behavior) and makes the test helpers thin subclasses of the real SQLiteDatabase/SQLiteBatch classes using SQLite’s in-memory/mock mode. It adjusts one assertion in wallet.cpp to accept the new ‘sqlite-mock’ format string. The change is confined to test utilities and a format-string check; no production wallet logic is altered.
Changed components
src/wallet/test/util.cppsrc/wallet/test/util.hsrc/wallet/sqlite.hsrc/wallet/wallet.cppInspect captured patch +18 / −151
diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h
index c34597e6..f467996d 100644
--- a/src/wallet/sqlite.h
+++ b/src/wallet/sqlite.h
@@ -75,6 +75,7 @@ private:
void SetupSQLStatements();
bool ExecStatement(sqlite3_stmt* stmt, std::span<const std::byte> blob);
+protected:
bool ReadKey(DataStream&& key, DataStream& value) override;
bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) override;
bool EraseKey(DataStream&& key) override;
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index 9c101d7e..c799721c 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -108,9 +108,9 @@ std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database)
std::unique_ptr<DatabaseBatch> batch_orig = database.MakeBatch();
std::unique_ptr<DatabaseCursor> cursor_orig = batch_orig->GetNewCursor();
- std::unique_ptr<WalletDatabase> new_db = std::make_unique<MockableDatabase>();
+ std::unique_ptr<WalletDatabase> new_db = CreateMockableWalletDatabase();
std::unique_ptr<DatabaseBatch> new_db_batch = new_db->MakeBatch();
- MockableBatch* batch_new = dynamic_cast<MockableBatch*>(new_db_batch.get());
+ MockableSQLiteBatch* batch_new = dynamic_cast<MockableSQLiteBatch*>(new_db_batch.get());
Assert(batch_new);
while (true) {
@@ -135,103 +135,13 @@ CTxDestination getNewDestination(CWallet& w, OutputType output_type)
return *Assert(w.GetNewDestination(output_type, ""));
}
-MockableCursor::MockableCursor(const MockableData& records, bool pass, std::span<const std::byte> prefix)
-{
- m_pass = pass;
- std::tie(m_cursor, m_cursor_end) = records.equal_range(BytePrefix{prefix});
-}
-
-DatabaseCursor::Status MockableCursor::Next(DataStream& key, DataStream& value)
-{
- if (!m_pass) {
- return Status::FAIL;
- }
- if (m_cursor == m_cursor_end) {
- return Status::DONE;
- }
- key.clear();
- value.clear();
- const auto& [key_data, value_data] = *m_cursor;
- key.write(key_data);
- value.write(value_data);
- m_cursor++;
- return Status::MORE;
-}
-
-bool MockableBatch::ReadKey(DataStream&& key, DataStream& value)
-{
- if (!m_pass) {
- return false;
- }
- SerializeData key_data{key.begin(), key.end()};
- const auto& it = m_records.find(key_data);
- if (it == m_records.end()) {
- return false;
- }
- value.clear();
- value.write(it->second);
- return true;
-}
-
-bool MockableBatch::WriteKey(DataStream&& key, DataStream&& value, bool overwrite)
-{
- if (!m_pass) {
- return false;
- }
- SerializeData key_data{key.begin(), key.end()};
- SerializeData value_data{value.begin(), value.end()};
- auto [it, inserted] = m_records.emplace(key_data, value_data);
- if (!inserted && overwrite) { // Overwrite if requested
- it->second = value_data;
- inserted = true;
- }
- return inserted;
-}
-
-bool MockableBatch::EraseKey(DataStream&& key)
-{
- if (!m_pass) {
- return false;
- }
- SerializeData key_data{key.begin(), key.end()};
- m_records.erase(key_data);
- return true;
-}
-
-bool MockableBatch::HasKey(DataStream&& key)
-{
- if (!m_pass) {
- return false;
- }
- SerializeData key_data{key.begin(), key.end()};
- return m_records.contains(key_data);
-}
-
-bool MockableBatch::ErasePrefix(std::span<const std::byte> prefix)
-{
- if (!m_pass) {
- return false;
- }
- auto it = m_records.begin();
- while (it != m_records.end()) {
- auto& key = it->first;
- if (key.size() < prefix.size() || std::search(key.begin(), key.end(), prefix.begin(), prefix.end()) != key.begin()) {
- it++;
- continue;
- }
- it = m_records.erase(it);
- }
- return true;
-}
+MockableSQLiteDatabase::MockableSQLiteDatabase()
+ : SQLiteDatabase(fs::PathFromString("mock/"), fs::PathFromString("mock/wallet.dat"), DatabaseOptions(), /*mock=*/true)
+{}
std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase()
{
- return std::make_unique<MockableDatabase>();
-}
-
-MockableDatabase& GetMockableDatabase(CWallet& wallet)
-{
- return dynamic_cast<MockableDatabase&>(wallet.GetDatabase());
+ return std::make_unique<MockableSQLiteDatabase>();
}
wallet::DescriptorScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success)
diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
index f812b0cc..fbc5188f 100644
--- a/src/wallet/test/util.h
+++ b/src/wallet/test/util.h
@@ -8,6 +8,7 @@
#include <addresstype.h>
#include <wallet/db.h>
#include <wallet/scriptpubkeyman.h>
+#include <wallet/sqlite.h>
#include <memory>
@@ -48,76 +49,31 @@ CTxDestination getNewDestination(CWallet& w, OutputType output_type);
using MockableData = std::map<SerializeData, SerializeData, std::less<>>;
-class MockableCursor: public DatabaseCursor
-{
-public:
- MockableData::const_iterator m_cursor;
- MockableData::const_iterator m_cursor_end;
- bool m_pass;
-
- explicit MockableCursor(const MockableData& records, bool pass) : m_cursor(records.begin()), m_cursor_end(records.end()), m_pass(pass) {}
- MockableCursor(const MockableData& records, bool pass, std::span<const std::byte> prefix);
- ~MockableCursor() = default;
-
- Status Next(DataStream& key, DataStream& value) override;
-};
-class MockableBatch : public DatabaseBatch
+class MockableSQLiteBatch : public SQLiteBatch
{
-private:
- MockableData& m_records;
- bool m_pass;
-
public:
- explicit MockableBatch(MockableData& records, bool pass) : m_records(records), m_pass(pass) {}
- ~MockableBatch() = default;
-
- void Close() override {}
-
- bool ReadKey(DataStream&& key, DataStream& value) override;
- bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite=true) override;
- bool EraseKey(DataStream&& key) override;
- bool HasKey(DataStream&& key) override;
- bool ErasePrefix(std::span<const std::byte> prefix) override;
-
- std::unique_ptr<DatabaseCursor> GetNewCursor() override
- {
- return std::make_unique<MockableCursor>(m_records, m_pass);
- }
- std::unique_ptr<DatabaseCursor> GetNewPrefixCursor(std::span<const std::byte> prefix) override {
- return std::make_unique<MockableCursor>(m_records, m_pass, prefix);
- }
- bool TxnBegin() override { return m_pass; }
- bool TxnCommit() override { return m_pass; }
- bool TxnAbort() override { return m_pass; }
- bool HasActiveTxn() override { return false; }
+ using SQLiteBatch::SQLiteBatch;
+ using SQLiteBatch::WriteKey;
};
/** A WalletDatabase whose contents and return values can be modified as needed for testing
**/
-class MockableDatabase : public WalletDatabase
+class MockableSQLiteDatabase : public SQLiteDatabase
{
public:
- MockableData m_records;
- bool m_pass{true};
-
- MockableDatabase() : WalletDatabase() {}
- ~MockableDatabase() = default;
-
- void Open() override {}
+ MockableSQLiteDatabase();
- bool Rewrite() override { return m_pass; }
- bool Backup(const std::string& strDest) const override { return m_pass; }
- void Close() override {}
+ bool Backup(const std::string& strDest) const override { return true; }
std::string Filename() override { return "mockable"; }
std::vector<fs::path> Files() override { return {}; }
- std::string Format() override { return "mock"; }
- std::unique_ptr<DatabaseBatch> MakeBatch() override { return std::make_unique<MockableBatch>(m_records, m_pass); }
+ std::string Format() override { return "sqlite-mock"; }
+ std::unique_ptr<DatabaseBatch> MakeBatch() override { return std::make_unique<MockableSQLiteBatch>(*this); }
};
std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase();
-MockableDatabase& GetMockableDatabase(CWallet& wallet);
+MockableSQLiteDatabase& GetMockableDatabase(CWallet& wallet);
DescriptorScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, bool success);
} // namespace wallet
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 3724c3f2..027f1d40 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3521,7 +3521,7 @@ void CWallet::SetupLegacyScriptPubKeyMan()
return;
}
- Assert(m_database->Format() == "bdb_ro" || m_database->Format() == "mock");
+ Assert(m_database->Format() == "bdb_ro" || m_database->Format() == "sqlite-mock");
std::unique_ptr<ScriptPubKeyMan> spk_manager = std::make_unique<LegacyDataSPKM>(*this);
for (const auto& type : LEGACY_OUTPUT_TYPES) {
Why this scored 15/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.