diff options
author | MarcoFalke <falke.marco@gmail.com> | 2020-07-30 17:01:01 +0200 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2020-07-30 17:01:11 +0200 |
commit | 37b765b962385df35aa820b737ab938d4212a128 (patch) | |
tree | ca338273fbd6da8a5932387416f84f312430379b | |
parent | 17de75b02814a8f746ad4c827c34d693f3477d40 (diff) | |
parent | 0fcff547d5b47822c13104978fda0c486e596526 (diff) | |
download | bitcoin-37b765b962385df35aa820b737ab938d4212a128.tar.xz |
Merge #19102: wallet: Introduce and use DummyDatabase instead of dummy BerkeleyDatabase
0fcff547d5b47822c13104978fda0c486e596526 walletdb: Ensure that having no database handle is a failure (Andrew Chow)
da039d2a915097c23f2b46e063042409bdc3c4f4 Remove BDB dummy databases (Andrew Chow)
0103d6434ea9d155259b40575008239a3762d6f7 Introduce DummyDatabase and use it in the tests (Andrew Chow)
Pull request description:
In the unit tests, we use a dummy `WalletDatabase` which does nothing and always returns true. This is currently implemented by creating a `BerkeleyDatabase` in dummy mode. This PR instead adds a `DummyDatabase` class which does nothing and never fails for use in the tests. `CreateDummyWalletDatabase` is changed to return this `DummyDatabase` and `BerkeleyDatabase` is cleaned up to remove all of the checks for `IsDummy`.
Based on `WalletDatabase` abstract class introduced in #19334
ACKs for top commit:
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/19102/commits/0fcff547d5b47822c13104978fda0c486e596526
MarcoFalke:
crACK 0fcff547d5b47822c13104978fda0c486e596526 🚈
Tree-SHA512: 05fbf32e078753e9a55a05f4c080b6d365b909a2a3a8e571b7e64b59ebbe53da49394f70419cc793192ade79f312f5e0422ca7c261ba81bae5912671c5ff6402
-rw-r--r-- | src/wallet/bdb.cpp | 27 | ||||
-rw-r--r-- | src/wallet/bdb.h | 13 | ||||
-rw-r--r-- | src/wallet/db.h | 41 | ||||
-rw-r--r-- | src/wallet/walletdb.cpp | 2 |
4 files changed, 47 insertions, 36 deletions
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index a8719806ab..a04311fdf5 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() @@ -786,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"); 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<BerkeleyEnvironment> env, std::string filename) : @@ -166,14 +163,6 @@ public: /** Make a BerkeleyBatch connected to this database */ std::unique_ptr<DatabaseBatch> 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 */ 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 <clientversion.h> #include <fs.h> #include <streams.h> +#include <util/memory.h> #include <atomic> #include <memory> @@ -154,4 +155,44 @@ public: virtual std::unique_ptr<DatabaseBatch> 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<DatabaseBatch> MakeBatch(const char* mode = "r+", bool flush_on_close = true) override { return MakeUnique<DummyBatch>(); } +}; + #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<WalletDatabase> CreateWalletDatabase(const fs::path& path) /** Return object for accessing dummy database with no read/write capabilities. */ std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase() { - return MakeUnique<BerkeleyDatabase>(); + return MakeUnique<DummyDatabase>(); } /** Return object for accessing temporary in-memory database. */ |