From 0103d6434ea9d155259b40575008239a3762d6f7 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 28 May 2020 17:26:18 -0400 Subject: [PATCH 1/3] Introduce DummyDatabase and use it in the tests --- src/wallet/db.h | 41 +++++++++++++++++++++++++++++++++++++++++ src/wallet/walletdb.cpp | 2 +- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/wallet/db.h b/src/wallet/db.h index 12dc1cc96b..0afaba5fd1 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -154,4 +155,44 @@ public: virtual std::unique_ptr MakeBatch(const char* mode = "r+", bool flush_on_close = true) = 0; }; +/** RAII class that provides access to a DummyDatabase. Never fails. */ +class DummyBatch : public DatabaseBatch +{ +private: + bool ReadKey(CDataStream&& key, CDataStream& value) override { return true; } + bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite=true) override { return true; } + bool EraseKey(CDataStream&& key) override { return true; } + bool HasKey(CDataStream&& key) override { return true; } + +public: + void Flush() override {} + void Close() override {} + + bool StartCursor() override { return true; } + bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return true; } + void CloseCursor() override {} + bool TxnBegin() override { return true; } + bool TxnCommit() override { return true; } + bool TxnAbort() override { return true; } +}; + +/** A dummy WalletDatabase that does nothing and never fails. Only used by unit tests. + **/ +class DummyDatabase : public WalletDatabase +{ +public: + void Open(const char* mode) override {}; + void AddRef() override {} + void RemoveRef() override {} + bool Rewrite(const char* pszSkip=nullptr) override { return true; } + bool Backup(const std::string& strDest) const override { return true; } + void Close() override {} + void Flush() override {} + bool PeriodicFlush() override { return true; } + void IncrementUpdateCounter() override { ++nUpdateCounter; } + void ReloadDbEnv() override {} + bool Verify(bilingual_str& errorStr) override { return true; } + std::unique_ptr MakeBatch(const char* mode = "r+", bool flush_on_close = true) override { return MakeUnique(); } +}; + #endif // BITCOIN_WALLET_DB_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 8c409b40cd..a6d327994b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1021,7 +1021,7 @@ std::unique_ptr CreateWalletDatabase(const fs::path& path) /** Return object for accessing dummy database with no read/write capabilities. */ std::unique_ptr CreateDummyWalletDatabase() { - return MakeUnique(); + return MakeUnique(); } /** Return object for accessing temporary in-memory database. */ From da039d2a915097c23f2b46e063042409bdc3c4f4 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 28 May 2020 17:30:50 -0400 Subject: [PATCH 2/3] Remove BDB dummy databases --- src/wallet/bdb.cpp | 25 +++---------------------- src/wallet/bdb.h | 13 +------------ 2 files changed, 4 insertions(+), 34 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index a8719806ab..3178a7b47a 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -337,10 +337,6 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bo void BerkeleyDatabase::Open(const char* pszMode) { - if (IsDummy()){ - return; - } - bool fCreate = strchr(pszMode, 'c') != nullptr; unsigned int nFlags = DB_THREAD; if (fCreate) @@ -472,9 +468,6 @@ void BerkeleyEnvironment::ReloadDbEnv() bool BerkeleyDatabase::Rewrite(const char* pszSkip) { - if (IsDummy()) { - return true; - } while (true) { { LOCK(cs_db); @@ -602,9 +595,6 @@ void BerkeleyEnvironment::Flush(bool fShutdown) bool BerkeleyDatabase::PeriodicFlush() { - // There's nothing to do for dummy databases. Return true. - if (IsDummy()) return true; - // Don't flush if we can't acquire the lock. TRY_LOCK(cs_db, lockDb); if (!lockDb) return false; @@ -632,9 +622,6 @@ bool BerkeleyDatabase::PeriodicFlush() bool BerkeleyDatabase::Backup(const std::string& strDest) const { - if (IsDummy()) { - return false; - } while (true) { { @@ -672,23 +659,17 @@ bool BerkeleyDatabase::Backup(const std::string& strDest) const void BerkeleyDatabase::Flush() { - if (!IsDummy()) { - env->Flush(false); - } + env->Flush(false); } void BerkeleyDatabase::Close() { - if (!IsDummy()) { - env->Flush(true); - } + env->Flush(true); } void BerkeleyDatabase::ReloadDbEnv() { - if (!IsDummy()) { - env->ReloadDbEnv(); - } + env->ReloadDbEnv(); } bool BerkeleyBatch::StartCursor() diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 982423f00e..75546924e8 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -98,10 +98,7 @@ class BerkeleyBatch; class BerkeleyDatabase : public WalletDatabase { public: - /** Create dummy DB handle */ - BerkeleyDatabase() : WalletDatabase(), env(nullptr) - { - } + BerkeleyDatabase() = delete; /** Create DB handle to real database */ BerkeleyDatabase(std::shared_ptr env, std::string filename) : @@ -166,14 +163,6 @@ public: /** Make a BerkeleyBatch connected to this database */ std::unique_ptr MakeBatch(const char* mode = "r+", bool flush_on_close = true) override; - -private: - - /** Return whether this database handle is a dummy for testing. - * Only to be used at a low level, application should ideally not care - * about this. - */ - bool IsDummy() const { return env == nullptr; } }; /** RAII class that provides access to a Berkeley database */ From 0fcff547d5b47822c13104978fda0c486e596526 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 18 Jun 2020 11:28:39 -0400 Subject: [PATCH 3/3] walletdb: Ensure that having no database handle is a failure Previously having no database handle could still be considered a success when BerkeleyDatabase and BerkeleyBatch were used for dummy database things. With dedicated DummyDatabase and DummyBatch classes now, these should fail. --- src/wallet/bdb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 3178a7b47a..a04311fdf5 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -767,7 +767,7 @@ bool BerkeleyBatch::ReadKey(CDataStream&& key, CDataStream& value) bool BerkeleyBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite) { if (!pdb) - return true; + return false; if (fReadOnly) assert(!"Write called on database in read-only mode");