wallet: Remove unused db functions

SOme db functions were for BDB, these are no longer needed.

Author: achow101

src/wallet/db.h

@@ -62,7 +62,6 @@ class DatabaseBatch
     DatabaseBatch(const DatabaseBatch&) = delete;
     DatabaseBatch& operator=(const DatabaseBatch&) = delete;
 
-    virtual void Flush() = 0;
     virtual void Close() = 0;
 
     template <typename K, typename T>
@@ -130,18 +129,14 @@ class WalletDatabase
 {
 public:
     /** Create dummy DB handle */
-    WalletDatabase() : nUpdateCounter(0) {}
+    WalletDatabase() = default;
     virtual ~WalletDatabase() = default;
 
     /** Open the database if it is not already opened. */
     virtual void Open() = 0;
 
     //! Counts the number of active database users to be sure that the database is not closed while someone is using it
     std::atomic<int> m_refcount{0};
-    /** Indicate the a new database user has began using the database. Increments m_refcount */
-    virtual void AddRef() = 0;
-    /** Indicate that database user has stopped using the database and that it could be flushed or closed. Decrement m_refcount */
-    virtual void RemoveRef() = 0;
 
     /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero
      */
@@ -151,33 +146,18 @@ class WalletDatabase
      */
     virtual bool Backup(const std::string& strDest) const = 0;
 
-    /** Make sure all changes are flushed to database file.
-     */
-    virtual void Flush() = 0;
     /** Flush to the database file and close the database.
      *  Also close the environment if no other databases are open in it.
      */
     virtual void Close() = 0;
-    /* flush the wallet passively (TRY_LOCK)
-       ideal to be called periodically */
-    virtual bool PeriodicFlush() = 0;
-
-    virtual void IncrementUpdateCounter() = 0;
-
-    virtual void ReloadDbEnv() = 0;
 
     /** Return path to main database file for logs and error messages. */
     virtual std::string Filename() = 0;
 
     virtual std::string Format() = 0;
 
-    std::atomic<unsigned int> nUpdateCounter;
-    unsigned int nLastSeen{0};
-    unsigned int nLastFlushed{0};
-    int64_t nLastWalletUpdate{0};
-
     /** Make a DatabaseBatch connected to this database */
-    virtual std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) = 0;
+    virtual std::unique_ptr<DatabaseBatch> MakeBatch() = 0;
 };
 
 enum class DatabaseFormat {

src/wallet/interfaces.cpp

@@ -587,7 +587,7 @@ class WalletLoaderImpl : public WalletLoader
         m_context.scheduler = &scheduler;
         return StartWallets(m_context);
     }
-    void flush() override { return FlushWallets(m_context); }
+    void flush() override {}
     void stop() override { return StopWallets(m_context); }
     void setMockTime(int64_t time) override { return SetMockTime(time); }
     void schedulerMockForward(std::chrono::seconds delta) override { Assert(m_context.scheduler)->MockForward(delta); }

src/wallet/load.cpp

@@ -152,13 +152,6 @@ void StartWallets(WalletContext& context)
     context.scheduler->scheduleEvery([&context] { MaybeResendWalletTxs(context); }, 1min);
 }
 
-void FlushWallets(WalletContext& context)
-{
-    for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) {
-        pwallet->Flush();
-    }
-}
-
 void StopWallets(WalletContext& context)
 {
     for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) {

src/wallet/load.h

@@ -28,9 +28,6 @@ bool LoadWallets(WalletContext& context);
 //! Complete startup of wallets.
 void StartWallets(WalletContext& context);
 
-//! Flush all wallets in preparation for shutdown.
-void FlushWallets(WalletContext& context);
-
 //! Stop all wallets. Wallets will be flushed first.
 void StopWallets(WalletContext& context);
 

src/wallet/migrate.cpp

@@ -699,7 +699,7 @@ void BerkeleyRODatabase::Open()
     }
 }
 
-std::unique_ptr<DatabaseBatch> BerkeleyRODatabase::MakeBatch(bool flush_on_close)
+std::unique_ptr<DatabaseBatch> BerkeleyRODatabase::MakeBatch()
 {
     return std::make_unique<BerkeleyROBatch>(*this);
 }

src/wallet/migrate.h

@@ -35,11 +35,6 @@ class BerkeleyRODatabase : public WalletDatabase
     /** Open the database if it is not already opened. */
     void Open() override;
 
-    /** Indicate the a new database user has began using the database. Increments m_refcount */
-    void AddRef() override {}
-    /** Indicate that database user has stopped using the database and that it could be flushed or closed. Decrement m_refcount */
-    void RemoveRef() override {}
-
     /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero
      */
     bool Rewrite(const char* pszSkip = nullptr) override { return false; }
@@ -48,28 +43,18 @@ class BerkeleyRODatabase : public WalletDatabase
      */
     bool Backup(const std::string& strDest) const override;
 
-    /** Make sure all changes are flushed to database file.
-     */
-    void Flush() override {}
     /** Flush to the database file and close the database.
      *  Also close the environment if no other databases are open in it.
      */
     void Close() override {}
-    /* flush the wallet passively (TRY_LOCK)
-       ideal to be called periodically */
-    bool PeriodicFlush() override { return false; }
-
-    void IncrementUpdateCounter() override {}
-
-    void ReloadDbEnv() override {}
 
     /** Return path to main database file for logs and error messages. */
     std::string Filename() override { return fs::PathToString(m_filepath); }
 
     std::string Format() override { return "bdb_ro"; }
 
     /** Make a DatabaseBatch connected to this database */
-    std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override;
+    std::unique_ptr<DatabaseBatch> MakeBatch() override;
 };
 
 class BerkeleyROCursor : public DatabaseCursor
@@ -107,7 +92,6 @@ class BerkeleyROBatch : public DatabaseBatch
     BerkeleyROBatch(const BerkeleyROBatch&) = delete;
     BerkeleyROBatch& operator=(const BerkeleyROBatch&) = delete;
 
-    void Flush() override {}
     void Close() override {}
 
     std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<BerkeleyROCursor>(m_database); }

src/wallet/sqlite.cpp

@@ -390,7 +390,7 @@ int SQliteExecHandler::Exec(SQLiteDatabase& database, const std::string& stateme
     return sqlite3_exec(database.m_db, statement.data(), nullptr, nullptr, nullptr);
 }
 
-std::unique_ptr<DatabaseBatch> SQLiteDatabase::MakeBatch(bool flush_on_close)
+std::unique_ptr<DatabaseBatch> SQLiteDatabase::MakeBatch()
 {
     // We ignore flush_on_close because we don't do manual flushing for SQLite
     return std::make_unique<SQLiteBatch>(*this);

src/wallet/sqlite.h

@@ -85,9 +85,6 @@ class SQLiteBatch : public DatabaseBatch
 
     void SetExecHandler(std::unique_ptr<SQliteExecHandler>&& handler) { m_exec_handler = std::move(handler); }
 
-    /* No-op. See comment on SQLiteDatabase::Flush */
-    void Flush() override {}
-
     void Close() override;
 
     std::unique_ptr<DatabaseCursor> GetNewCursor() override;
@@ -139,36 +136,18 @@ class SQLiteDatabase : public WalletDatabase
     /** Close the database */
     void Close() override;
 
-    /* These functions are unused */
-    void AddRef() override { assert(false); }
-    void RemoveRef() override { assert(false); }
-
     /** Rewrite the entire database on disk */
     bool Rewrite(const char* skip = nullptr) override;
 
     /** Back up the entire database to a file.
      */
     bool Backup(const std::string& dest) const override;
 
-    /** No-ops
-     *
-     * SQLite always flushes everything to the database file after each transaction
-     * (each Read/Write/Erase that we do is its own transaction unless we called
-     * TxnBegin) so there is no need to have Flush or Periodic Flush.
-     *
-     * There is no DB env to reload, so ReloadDbEnv has nothing to do
-     */
-    void Flush() override {}
-    bool PeriodicFlush() override { return false; }
-    void ReloadDbEnv() override {}
-
-    void IncrementUpdateCounter() override { ++nUpdateCounter; }
-
     std::string Filename() override { return m_file_path; }
     std::string Format() override { return "sqlite"; }
 
     /** Make a SQLiteBatch connected to this database */
-    std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override;
+    std::unique_ptr<DatabaseBatch> MakeBatch() override;
 
     /** Return true if there is an on-going txn in this connection */
     bool HasActiveTxn();

src/wallet/test/util.h

@@ -75,7 +75,6 @@ class MockableBatch : public DatabaseBatch
     explicit MockableBatch(MockableData& records, bool pass) : m_records(records), m_pass(pass) {}
     ~MockableBatch() = default;
 
-    void Flush() override {}
     void Close() override {}
 
     std::unique_ptr<DatabaseCursor> GetNewCursor() override
@@ -102,20 +101,14 @@ class MockableDatabase : public WalletDatabase
     ~MockableDatabase() = default;
 
     void Open() override {}
-    void AddRef() override {}
-    void RemoveRef() override {}
 
     bool Rewrite(const char* pszSkip=nullptr) override { return m_pass; }
     bool Backup(const std::string& strDest) const override { return m_pass; }
-    void Flush() override {}
     void Close() override {}
-    bool PeriodicFlush() override { return m_pass; }
-    void IncrementUpdateCounter() override {}
-    void ReloadDbEnv() override {}
 
     std::string Filename() override { return "mockable"; }
     std::string Format() override { return "mock"; }
-    std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<MockableBatch>(m_records, m_pass); }
+    std::unique_ptr<DatabaseBatch> MakeBatch() override { return std::make_unique<MockableBatch>(m_records, m_pass); }
 };
 
 std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(MockableData records = {});

src/wallet/test/walletload_tests.cpp

@@ -42,7 +42,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup)
     std::unique_ptr<WalletDatabase> database = CreateMockableWalletDatabase();
     {
         // Write unknown active descriptor
-        WalletBatch batch(*database, false);
+        WalletBatch batch(*database);
         std::string unknown_desc = "trx(tpubD6NzVbkrYhZ4Y4S7m6Y5s9GD8FqEMBy56AGphZXuagajudVZEnYyBahZMgHNCTJc2at82YX6s8JiL1Lohu5A3v1Ur76qguNH4QVQ7qYrBQx/86'/1'/0'/0/*)#8pn8tzdt";
         WalletDescriptor wallet_descriptor(std::make_shared<DummyDescriptor>(unknown_desc), 0, 0, 0, 0);
         BOOST_CHECK(batch.WriteDescriptor(uint256(), wallet_descriptor));
@@ -69,7 +69,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup)
 
     {
         // Write valid descriptor with invalid ID
-        WalletBatch batch(*database, false);
+        WalletBatch batch(*database);
         std::string desc = "wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#cjjspncu";
         WalletDescriptor wallet_descriptor(std::make_shared<DummyDescriptor>(desc), 0, 0, 0, 0);
         BOOST_CHECK(batch.WriteDescriptor(uint256::ONE, wallet_descriptor));

src/wallet/wallet.cpp

@@ -235,8 +235,7 @@ static std::set<std::string> g_unloading_wallet_set GUARDED_BY(g_wallet_release_
 static void FlushAndDeleteWallet(CWallet* wallet)
 {
     const std::string name = wallet->GetName();
-    wallet->WalletLogPrintf("Releasing wallet %s..\n", name);
-    wallet->Flush();
+    wallet->WalletLogPrintf("Releasing wallet\n");
     delete wallet;
     // Wallet is now released, notify WaitForDeleteWallet, if any.
     {
@@ -682,11 +681,6 @@ bool CWallet::HasWalletSpend(const CTransactionRef& tx) const
     return false;
 }
 
-void CWallet::Flush()
-{
-    GetDatabase().Flush();
-}
-
 void CWallet::Close()
 {
     GetDatabase().Close();
@@ -859,15 +853,9 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase)
         }
         Lock();
 
-        // Need to completely rewrite the wallet file; if we don't, bdb might keep
+        // Need to completely rewrite the wallet file; if we don't, the database might keep
         // bits of the unencrypted private key in slack space in the database file.
         GetDatabase().Rewrite();
-
-        // BDB seems to have a bad habit of writing old data into
-        // slack space in .dat files; that is bad if the old data is
-        // unencrypted private keys. So:
-        GetDatabase().ReloadDbEnv();
-
     }
     NotifyStatusChanged(this);
 
@@ -1016,11 +1004,11 @@ bool CWallet::IsSpentKey(const CScript& scriptPubKey) const
     return false;
 }
 
-CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose, bool rescanning_old_block)
+CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx, bool rescanning_old_block)
 {
     LOCK(cs_wallet);
 
-    WalletBatch batch(GetDatabase(), fFlushOnClose);
+    WalletBatch batch(GetDatabase());
 
     uint256 hash = tx->GetHash();
 
@@ -1230,7 +1218,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
             // Block disconnection override an abandoned tx as unconfirmed
             // which means user may have to call abandontransaction again
             TxState tx_state = std::visit([](auto&& s) -> TxState { return s; }, state);
-            CWalletTx* wtx = AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, rescanning_old_block);
+            CWalletTx* wtx = AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, rescanning_old_block);
             if (!wtx) {
                 // Can only be nullptr if there was a db write error (missing db, read-only db or a db engine internal writing error).
                 // As we only store arriving transaction in this process, and we don't want an inconsistent state, let's throw an error.
@@ -1325,8 +1313,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
 }
 
 void CWallet::RecursiveUpdateTxState(const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) {
-    // Do not flush the wallet here for performance reasons
-    WalletBatch batch(GetDatabase(), false);
+    WalletBatch batch(GetDatabase());
     RecursiveUpdateTxState(&batch, tx_hash, try_updating_state);
 }
 
@@ -3179,7 +3166,6 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
         }
         walletInstance->m_attaching_chain = false;
         walletInstance->chainStateFlushed(ChainstateRole::NORMAL, chain.getTipLocator());
-        walletInstance->GetDatabase().IncrementUpdateCounter();
     }
     walletInstance->m_attaching_chain = false;
 

src/wallet/wallet.h

@@ -600,7 +600,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
      * Add the transaction to the wallet, wrapping it up inside a CWalletTx
      * @return the recently added wtx pointer or nullptr if there was a db write error.
      */
-    CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false);
+    CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool rescanning_old_block = false);
     bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     void transactionAddedToMempool(const CTransactionRef& tx) override;
     void blockConnected(ChainstateRole role, const interfaces::BlockInfo& block) override;
@@ -814,9 +814,6 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     //! Check if a given transaction has any of its outputs spent by another transaction in the wallet
     bool HasWalletSpend(const CTransactionRef& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
-    //! Flush wallet (bitdb flush)
-    void Flush();
-
     //! Close wallet database
     void Close();
 

src/wallet/walletdb.cpp

@@ -1256,33 +1256,6 @@ bool RunWithinTxn(WalletDatabase& database, std::string_view process_desc, const
     return RunWithinTxn(batch, process_desc, func);
 }
 
-void MaybeCompactWalletDB(WalletContext& context)
-{
-    static std::atomic<bool> fOneThread(false);
-    if (fOneThread.exchange(true)) {
-        return;
-    }
-
-    for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) {
-        WalletDatabase& dbh = pwallet->GetDatabase();
-
-        unsigned int nUpdateCounter = dbh.nUpdateCounter;
-
-        if (dbh.nLastSeen != nUpdateCounter) {
-            dbh.nLastSeen = nUpdateCounter;
-            dbh.nLastWalletUpdate = GetTime();
-        }
-
-        if (dbh.nLastFlushed != nUpdateCounter && GetTime() - dbh.nLastWalletUpdate >= 2) {
-            if (dbh.PeriodicFlush()) {
-                dbh.nLastFlushed = nUpdateCounter;
-            }
-        }
-    }
-
-    fOneThread = false;
-}
-
 bool WalletBatch::WriteAddressPreviouslySpent(const CTxDestination& dest, bool previously_spent)
 {
     auto key{std::make_pair(DBKeys::DESTDATA, std::make_pair(EncodeDestination(dest), std::string("used")))};