From 55297ef433e71a998d9dfc44c47e217cbf99574c Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Fri, 5 Jan 2024 18:42:28 -0500 Subject: [PATCH] wallet: Remove unused db functions SOme db functions were for BDB, these are no longer needed. --- src/wallet/db.h | 26 +++----------------------- src/wallet/interfaces.cpp | 2 +- src/wallet/load.cpp | 7 ------- src/wallet/load.h | 3 --- src/wallet/migrate.cpp | 2 +- src/wallet/migrate.h | 18 +----------------- src/wallet/sqlite.cpp | 2 +- src/wallet/sqlite.h | 23 +---------------------- src/wallet/test/util.h | 9 +-------- src/wallet/test/walletload_tests.cpp | 4 ++-- src/wallet/wallet.cpp | 24 +++++------------------- src/wallet/wallet.h | 5 +---- src/wallet/walletdb.cpp | 27 --------------------------- src/wallet/walletdb.h | 17 ++--------------- 14 files changed, 19 insertions(+), 150 deletions(-) diff --git a/src/wallet/db.h b/src/wallet/db.h index d85ae4597f0f76..9eb22417d83641 100644 --- a/src/wallet/db.h +++ b/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 @@ -130,18 +129,14 @@ class WalletDatabase { public: /** Create dummy DB handle */ - WalletDatabase() : nUpdateCounter(0) {} - virtual ~WalletDatabase() {}; + 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 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 nUpdateCounter; - unsigned int nLastSeen{0}; - unsigned int nLastFlushed{0}; - int64_t nLastWalletUpdate{0}; - /** Make a DatabaseBatch connected to this database */ - virtual std::unique_ptr MakeBatch(bool flush_on_close = true) = 0; + virtual std::unique_ptr MakeBatch() = 0; }; enum class DatabaseFormat { diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 15c1780a3b0af1..4acc44af1d0d69 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -585,7 +585,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); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 49b30d7495bfe8..51529b8446fbc1 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -150,13 +150,6 @@ void StartWallets(WalletContext& context) context.scheduler->scheduleEvery([&context] { MaybeResendWalletTxs(context); }, 1min); } -void FlushWallets(WalletContext& context) -{ - for (const std::shared_ptr& pwallet : GetWallets(context)) { - pwallet->Flush(); - } -} - void StopWallets(WalletContext& context) { for (const std::shared_ptr& pwallet : GetWallets(context)) { diff --git a/src/wallet/load.h b/src/wallet/load.h index c079cad955b447..e7224c27ee976d 100644 --- a/src/wallet/load.h +++ b/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); diff --git a/src/wallet/migrate.cpp b/src/wallet/migrate.cpp index 09254a76ad89a8..2917e9ea02b1e4 100644 --- a/src/wallet/migrate.cpp +++ b/src/wallet/migrate.cpp @@ -699,7 +699,7 @@ void BerkeleyRODatabase::Open() } } -std::unique_ptr BerkeleyRODatabase::MakeBatch(bool flush_on_close) +std::unique_ptr BerkeleyRODatabase::MakeBatch() { return std::make_unique(*this); } diff --git a/src/wallet/migrate.h b/src/wallet/migrate.h index e4826450af41fa..0e87e0046e78d3 100644 --- a/src/wallet/migrate.h +++ b/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,20 +43,10 @@ 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); } @@ -69,7 +54,7 @@ class BerkeleyRODatabase : public WalletDatabase std::string Format() override { return "bdb_ro"; } /** Make a DatabaseBatch connected to this database */ - std::unique_ptr MakeBatch(bool flush_on_close = true) override; + std::unique_ptr 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 GetNewCursor() override { return std::make_unique(m_database); } diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 5e3a8179a272c2..ce516c923b6ed1 100644 --- a/src/wallet/sqlite.cpp +++ b/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 SQLiteDatabase::MakeBatch(bool flush_on_close) +std::unique_ptr SQLiteDatabase::MakeBatch() { // We ignore flush_on_close because we don't do manual flushing for SQLite return std::make_unique(*this); diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index 0a3243fe19c5fd..e7c8838a22c808 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -85,9 +85,6 @@ class SQLiteBatch : public DatabaseBatch void SetExecHandler(std::unique_ptr&& handler) { m_exec_handler = std::move(handler); } - /* No-op. See comment on SQLiteDatabase::Flush */ - void Flush() override {} - void Close() override; std::unique_ptr GetNewCursor() override; @@ -139,10 +136,6 @@ 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; @@ -150,25 +143,11 @@ class SQLiteDatabase : public WalletDatabase */ 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 MakeBatch(bool flush_on_close = true) override; + std::unique_ptr MakeBatch() override; /** Return true if there is an on-going txn in this connection */ bool HasActiveTxn(); diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index b6af4ec81700bd..9b5c999607e050 100644 --- a/src/wallet/test/util.h +++ b/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() {} - void Flush() override {} void Close() override {} std::unique_ptr GetNewCursor() override @@ -102,20 +101,14 @@ class MockableDatabase : public WalletDatabase ~MockableDatabase() {}; 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 MakeBatch(bool flush_on_close = true) override { return std::make_unique(m_records, m_pass); } + std::unique_ptr MakeBatch() override { return std::make_unique(m_records, m_pass); } }; std::unique_ptr CreateMockableWalletDatabase(MockableData records = {}); diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index bd4885eb6b9593..0c69849d0b6462 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -42,7 +42,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup) std::unique_ptr 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(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(desc), 0, 0, 0, 0); BOOST_CHECK(batch.WriteDescriptor(uint256::ONE, wallet_descriptor)); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8e51be1f29b535..619d87da0ace7a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -221,7 +221,6 @@ static void ReleaseWallet(CWallet* wallet) { const std::string name = wallet->GetName(); wallet->WalletLogPrintf("Releasing wallet\n"); - wallet->Flush(); delete wallet; // Wallet is now released, notify UnloadWallet, if any. { @@ -670,11 +669,6 @@ bool CWallet::HasWalletSpend(const CTransactionRef& tx) const return false; } -void CWallet::Flush() -{ - GetDatabase().Flush(); -} - void CWallet::Close() { GetDatabase().Close(); @@ -847,15 +841,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); @@ -1004,11 +992,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(); @@ -1218,7 +1206,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. @@ -1313,8 +1301,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c } void CWallet::RecursiveUpdateTxState(const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { - // Do not flush the wallet here for performance reasons - WalletBatch batch(GetDatabase(), false); + WalletBatch batch(GetDatabase()); RecursiveUpdateTxState(&batch, tx_hash, try_updating_state); } @@ -3167,7 +3154,6 @@ bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interf } walletInstance->m_attaching_chain = false; walletInstance->chainStateFlushed(ChainstateRole::NORMAL, chain.getTipLocator()); - walletInstance->GetDatabase().IncrementUpdateCounter(); } walletInstance->m_attaching_chain = false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 576126917f5f8c..b693477ada4ec3 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -601,7 +601,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; @@ -815,9 +815,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(); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index e6e8dbd756a91c..511077bad54a97 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1245,33 +1245,6 @@ bool RunWithinTxn(WalletDatabase& database, std::string_view process_desc, const return RunWithinTxn(batch, process_desc, func); } -void MaybeCompactWalletDB(WalletContext& context) -{ - static std::atomic fOneThread(false); - if (fOneThread.exchange(true)) { - return; - } - - for (const std::shared_ptr& 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")))}; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index b00480b63be10e..52a463455363d9 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -197,10 +197,6 @@ class WalletBatch if (!m_batch->Write(key, value, fOverwrite)) { return false; } - m_database.IncrementUpdateCounter(); - if (m_database.nUpdateCounter % 1000 == 0) { - m_batch->Flush(); - } return true; } @@ -210,17 +206,12 @@ class WalletBatch if (!m_batch->Erase(key)) { return false; } - m_database.IncrementUpdateCounter(); - if (m_database.nUpdateCounter % 1000 == 0) { - m_batch->Flush(); - } return true; } public: - explicit WalletBatch(WalletDatabase &database, bool _fFlushOnClose = true) : - m_batch(database.MakeBatch(_fFlushOnClose)), - m_database(database) + explicit WalletBatch(WalletDatabase &database) : + m_batch(database.MakeBatch()) { } WalletBatch(const WalletBatch&) = delete; @@ -292,7 +283,6 @@ class WalletBatch bool TxnAbort(); private: std::unique_ptr m_batch; - WalletDatabase& m_database; }; /** @@ -309,9 +299,6 @@ class WalletBatch */ bool RunWithinTxn(WalletDatabase& database, std::string_view process_desc, const std::function& func); -//! Compacts BDB state so that wallet.dat is self-contained (if there are changes) -void MaybeCompactWalletDB(WalletContext& context); - bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); bool LoadCryptedKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); bool LoadEncryptionKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr);