merge bitcoin#19325: Refactor BerkeleyDatabase to introduce DatabaseBatch abstract class

src/wallet/bdb.cpp

@@ -849,3 +849,8 @@ bool BerkeleyBatch::HasKey(CDataStream&& key)
     int ret = pdb->exists(activeTxn, datKey, 0);
     return ret == 0;
 }
+
+std::unique_ptr<BerkeleyBatch> BerkeleyDatabase::MakeBatch(const char* mode, bool flush_on_close)
+{
+    return MakeUnique<BerkeleyBatch>(*this, mode, flush_on_close);
+}

src/wallet/bdb.h

@@ -91,6 +91,8 @@ std::shared_ptr<BerkeleyEnvironment> GetWalletEnv(const fs::path& wallet_path, s
 /** Return whether a wallet database is currently loaded. */
 bool IsBDBWalletLoaded(const fs::path& wallet_path);
 
+class BerkeleyBatch;
+
 /** An instance of this class represents one database.
  * For BerkeleyDB this is just a (env, strFile) tuple.
  **/
@@ -159,6 +161,9 @@ public:
     /** Database pointer. This is initialized lazily and reset during flushes, so it can be null. */
     std::unique_ptr<Db> m_db;
 
+    /** Make a BerkeleyBatch connected to this database */
+    std::unique_ptr<BerkeleyBatch> MakeBatch(const char* mode, bool flush_on_close);
+
 private:
     std::string strFile;
 
@@ -170,7 +175,7 @@ private:
 };
 
 /** RAII class that provides access to a Berkeley database */
-class BerkeleyBatch
+class BerkeleyBatch : public DatabaseBatch
 {
     /** RAII class that automatically cleanses its data on destruction */
     class SafeDbt final
@@ -193,10 +198,10 @@ class BerkeleyBatch
     };
 
 private:
-    bool ReadKey(CDataStream&& key, CDataStream& value);
+    bool ReadKey(CDataStream&& key, CDataStream& value) override;
-    bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true);
+    bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true) override;
-    bool EraseKey(CDataStream&& key);
+    bool EraseKey(CDataStream&& key) override;
-    bool HasKey(CDataStream&& key);
+    bool HasKey(CDataStream&& key) override;
 
 protected:
     Db* pdb;
@@ -209,71 +214,20 @@ protected:
 
 public:
     explicit BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode = "r+", bool fFlushOnCloseIn=true);
-    ~BerkeleyBatch() { Close(); }
+    ~BerkeleyBatch() override { Close(); }
 
     BerkeleyBatch(const BerkeleyBatch&) = delete;
     BerkeleyBatch& operator=(const BerkeleyBatch&) = delete;
 
-    void Flush();
+    void Flush() override;
-    void Close();
+    void Close() override;
 
-    template <typename K, typename T>
+    bool StartCursor() override;
-    bool Read(const K& key, T& value)
+    bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override;
-    {
+    void CloseCursor() override;
-        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
+    bool TxnBegin() override;
-        ssKey.reserve(1000);
+    bool TxnCommit() override;
-        ssKey << key;
+    bool TxnAbort() override;
-
-        CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-        if (!ReadKey(std::move(ssKey), ssValue)) return false;
-        try {
-            ssValue >> value;
-            return true;
-        } catch (const std::exception&) {
-            return false;
-        }
-    }
-
-    template <typename K, typename T>
-    bool Write(const K& key, const T& value, bool fOverwrite = true)
-    {
-        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
-        ssKey.reserve(1000);
-        ssKey << key;
-
-        CDataStream ssValue(SER_DISK, CLIENT_VERSION);
-        ssValue.reserve(10000);
-        ssValue << value;
-
-        return WriteKey(std::move(ssKey), std::move(ssValue), fOverwrite);
-    }
-
-    template <typename K>
-    bool Erase(const K& key)
-    {
-        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
-        ssKey.reserve(1000);
-        ssKey << key;
-
-        return EraseKey(std::move(ssKey));
-    }
-
-    template <typename K>
-    bool Exists(const K& key)
-    {
-        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
-        ssKey.reserve(1000);
-        ssKey << key;
-
-        return HasKey(std::move(ssKey));
-    }
-
-    bool StartCursor();
-    bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete);
-    void CloseCursor();
-    bool TxnBegin();
-    bool TxnCommit();
-    bool TxnAbort();
 };
 
 std::string BerkeleyDatabaseVersion();

src/wallet/db.h

@@ -6,7 +6,9 @@
 #ifndef BITCOIN_WALLET_DB_H
 #define BITCOIN_WALLET_DB_H
 
+#include <clientversion.h>
 #include <fs.h>
+#include <streams.h>
 
 #include <string>
 
@@ -14,4 +16,82 @@
 fs::path WalletDataFilePath(const fs::path& wallet_path);
 void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename);
 
+/** RAII class that provides access to a WalletDatabase */
+class DatabaseBatch
+{
+private:
+    virtual bool ReadKey(CDataStream&& key, CDataStream& value) = 0;
+    virtual bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite=true) = 0;
+    virtual bool EraseKey(CDataStream&& key) = 0;
+    virtual bool HasKey(CDataStream&& key) = 0;
+
+public:
+    explicit DatabaseBatch() {}
+    virtual ~DatabaseBatch() {}
+
+    DatabaseBatch(const DatabaseBatch&) = delete;
+    DatabaseBatch& operator=(const DatabaseBatch&) = delete;
+
+    virtual void Flush() = 0;
+    virtual void Close() = 0;
+
+    template <typename K, typename T>
+    bool Read(const K& key, T& value)
+    {
+        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
+        ssKey.reserve(1000);
+        ssKey << key;
+
+        CDataStream ssValue(SER_DISK, CLIENT_VERSION);
+        if (!ReadKey(std::move(ssKey), ssValue)) return false;
+        try {
+            ssValue >> value;
+            return true;
+        } catch (const std::exception&) {
+            return false;
+        }
+    }
+
+    template <typename K, typename T>
+    bool Write(const K& key, const T& value, bool fOverwrite = true)
+    {
+        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
+        ssKey.reserve(1000);
+        ssKey << key;
+
+        CDataStream ssValue(SER_DISK, CLIENT_VERSION);
+        ssValue.reserve(10000);
+        ssValue << value;
+
+        return WriteKey(std::move(ssKey), std::move(ssValue), fOverwrite);
+    }
+
+    template <typename K>
+    bool Erase(const K& key)
+    {
+        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
+        ssKey.reserve(1000);
+        ssKey << key;
+
+        return EraseKey(std::move(ssKey));
+    }
+
+    template <typename K>
+    bool Exists(const K& key)
+    {
+        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
+        ssKey.reserve(1000);
+        ssKey << key;
+
+        return HasKey(std::move(ssKey));
+    }
+
+    virtual bool StartCursor() = 0;
+    virtual bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) = 0;
+    virtual void CloseCursor() = 0;
+    virtual bool TxnBegin() = 0;
+    virtual bool TxnCommit() = 0;
+    virtual bool TxnAbort() = 0;
+};
+
 #endif // BITCOIN_WALLET_DB_H

src/wallet/walletdb.cpp

@@ -158,8 +158,8 @@ bool WalletBatch::WriteBestBlock(const CBlockLocator& locator)
 
 bool WalletBatch::ReadBestBlock(CBlockLocator& locator)
 {
-    if (m_batch.Read(DBKeys::BESTBLOCK, locator) && !locator.vHave.empty()) return true;
+    if (m_batch->Read(DBKeys::BESTBLOCK, locator) && !locator.vHave.empty()) return true;
-    return m_batch.Read(DBKeys::BESTBLOCK_NOMERKLE, locator);
+    return m_batch->Read(DBKeys::BESTBLOCK_NOMERKLE, locator);
 }
 
 bool WalletBatch::WriteOrderPosNext(int64_t nOrderPosNext)
@@ -169,7 +169,7 @@ bool WalletBatch::WriteOrderPosNext(int64_t nOrderPosNext)
 
 bool WalletBatch::ReadPool(int64_t nPool, CKeyPool& keypool)
 {
-    return m_batch.Read(std::make_pair(DBKeys::POOL, nPool), keypool);
+    return m_batch->Read(std::make_pair(DBKeys::POOL, nPool), keypool);
 }
 
 bool WalletBatch::WritePool(int64_t nPool, const CKeyPool& keypool)
@@ -190,7 +190,7 @@ bool WalletBatch::WriteMinVersion(int nVersion)
 bool WalletBatch::ReadCoinJoinSalt(uint256& salt, bool fLegacy)
 {
     // TODO: Remove legacy checks after few major releases
-    return m_batch.Read(std::string(fLegacy ? DBKeys::PRIVATESEND_SALT : DBKeys::COINJOIN_SALT), salt);
+    return m_batch->Read(std::string(fLegacy ? DBKeys::PRIVATESEND_SALT : DBKeys::COINJOIN_SALT), salt);
 }
 
 bool WalletBatch::WriteCoinJoinSalt(const uint256& salt)
@@ -514,14 +514,14 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
     LOCK(pwallet->cs_wallet);
     try {
         int nMinVersion = 0;
-        if (m_batch.Read(DBKeys::MINVERSION, nMinVersion)) {
+        if (m_batch->Read(DBKeys::MINVERSION, nMinVersion)) {
             if (nMinVersion > FEATURE_LATEST)
                 return DBErrors::TOO_NEW;
             pwallet->LoadMinVersion(nMinVersion);
         }
 
         // Get cursor
-        if (!m_batch.StartCursor())
+        if (!m_batch->StartCursor())
         {
             pwallet->WalletLogPrintf("Error getting wallet database cursor\n");
             return DBErrors::CORRUPT;
@@ -533,13 +533,13 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
             CDataStream ssKey(SER_DISK, CLIENT_VERSION);
             CDataStream ssValue(SER_DISK, CLIENT_VERSION);
             bool complete;
-            bool ret = m_batch.ReadAtCursor(ssKey, ssValue, complete);
+            bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete);
             if (complete) {
                 break;
             }
             else if (!ret)
             {
-                m_batch.CloseCursor();
+                m_batch->CloseCursor();
                 pwallet->WalletLogPrintf("Error reading next record from wallet database\n");
                 return DBErrors::CORRUPT;
             }
@@ -573,7 +573,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
     } catch (...) {
         result = DBErrors::CORRUPT;
     }
-    m_batch.CloseCursor();
+    m_batch->CloseCursor();
 
     if (fNoncriticalErrors && result == DBErrors::LOAD_OK)
         result = DBErrors::NONCRITICAL_ERROR;
@@ -585,7 +585,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
 
     // Last client version to open this wallet, was previously the file version number
     int last_client = CLIENT_VERSION;
-    m_batch.Read(DBKeys::VERSION, last_client);
+    m_batch->Read(DBKeys::VERSION, last_client);
 
     int wallet_version = pwallet->GetVersion();
     pwallet->WalletLogPrintf("Wallet File Version = %d\n", wallet_version > 0 ? wallet_version : last_client);
@@ -606,7 +606,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
         return DBErrors::NEED_REWRITE;
 
     if (last_client < CLIENT_VERSION) // Update
-        m_batch.Write(DBKeys::VERSION, CLIENT_VERSION);
+        m_batch->Write(DBKeys::VERSION, CLIENT_VERSION);
 
     if (wss.fAnyUnordered)
         result = pwallet->ReorderTransactions();
@@ -628,13 +628,13 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CW
 
     try {
         int nMinVersion = 0;
-        if (m_batch.Read(DBKeys::MINVERSION, nMinVersion)) {
+        if (m_batch->Read(DBKeys::MINVERSION, nMinVersion)) {
             if (nMinVersion > FEATURE_LATEST)
                 return DBErrors::TOO_NEW;
         }
 
         // Get cursor
-        if (!m_batch.StartCursor())
+        if (!m_batch->StartCursor())
         {
             LogPrintf("Error getting wallet database cursor\n");
             return DBErrors::CORRUPT;
@@ -646,11 +646,11 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CW
             CDataStream ssKey(SER_DISK, CLIENT_VERSION);
             CDataStream ssValue(SER_DISK, CLIENT_VERSION);
             bool complete;
-            bool ret = m_batch.ReadAtCursor(ssKey, ssValue, complete);
+            bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete);
             if (complete) {
                 break;
             } else if (!ret) {
-                m_batch.CloseCursor();
+                m_batch->CloseCursor();
                 LogPrintf("Error reading next record from wallet database\n");
                 return DBErrors::CORRUPT;
             }
@@ -671,7 +671,7 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CW
     } catch (...) {
         result = DBErrors::CORRUPT;
     }
-    m_batch.CloseCursor();
+    m_batch->CloseCursor();
 
     return result;
 }
@@ -801,17 +801,17 @@ bool WalletBatch::WriteWalletFlags(const uint64_t flags)
 
 bool WalletBatch::TxnBegin()
 {
-    return m_batch.TxnBegin();
+    return m_batch->TxnBegin();
 }
 
 bool WalletBatch::TxnCommit()
 {
-    return m_batch.TxnCommit();
+    return m_batch->TxnCommit();
 }
 
 bool WalletBatch::TxnAbort()
 {
-    return m_batch.TxnAbort();
+    return m_batch->TxnAbort();
 }
 
 bool IsWalletLoaded(const fs::path& wallet_path)

src/wallet/walletdb.h

@@ -137,12 +137,12 @@ private:
     template <typename K, typename T>
     bool WriteIC(const K& key, const T& value, bool fOverwrite = true)
     {
-        if (!m_batch.Write(key, value, fOverwrite)) {
+        if (!m_batch->Write(key, value, fOverwrite)) {
             return false;
         }
         m_database.IncrementUpdateCounter();
         if (m_database.nUpdateCounter % 1000 == 0) {
-            m_batch.Flush();
+            m_batch->Flush();
         }
         return true;
     }
@@ -150,19 +150,19 @@ private:
     template <typename K>
     bool EraseIC(const K& key)
     {
-        if (!m_batch.Erase(key)) {
+        if (!m_batch->Erase(key)) {
             return false;
         }
         m_database.IncrementUpdateCounter();
         if (m_database.nUpdateCounter % 1000 == 0) {
-            m_batch.Flush();
+            m_batch->Flush();
         }
         return true;
     }
 
 public:
     explicit WalletBatch(WalletDatabase& database, const char* pszMode = "r+", bool _fFlushOnClose = true) :
-        m_batch(database, pszMode, _fFlushOnClose),
+        m_batch(database.MakeBatch(pszMode, _fFlushOnClose)),
         m_database(database)
     {
     }
@@ -230,7 +230,7 @@ public:
     //! Abort current transaction
     bool TxnAbort();
 private:
-    BerkeleyBatch m_batch;
+    std::unique_ptr<BerkeleyBatch> m_batch;
     WalletDatabase& m_database;
 };