From 0f4a95243704a91f6a5dfd9d6cd60644e54df7e5 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 | 24 ++---------------------- 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 | 26 ++++++-------------------- 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 b26c48cd3d1e4e..75908b3f886964 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 @@ -131,7 +130,7 @@ class WalletDatabase { public: /** Create dummy DB handle */ - WalletDatabase() : nUpdateCounter(0) {} + WalletDatabase() = default; virtual ~WalletDatabase() = default; /** Open the database if it is not already opened. */ @@ -139,10 +138,6 @@ class WalletDatabase //! 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 */ @@ -152,33 +147,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 79efaab8a8dba7..a561a8d8c92679 100644 --- a/src/wallet/interfaces.cpp +++ b/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); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 274c52d16fb468..c1ac9ab3d1d937 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -162,13 +162,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 d7d85773743469..369fc601443f4c 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 16eadeb019d56e..4f352f73270a75 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 a8c9f8a8ab6ee2..a9ee60f755672c 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 78a3accf890e7a..285736427e58e4 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; @@ -140,10 +137,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; @@ -151,25 +144,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 32a4afe12fee79..fc23e53b0b7277 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() = default; - void Flush() override {} void Close() override {} std::unique_ptr GetNewCursor() override @@ -103,20 +102,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 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 cb8cf15d4a9178..0369897b569b22 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -235,8 +235,7 @@ static std::set 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); } @@ -3190,7 +3177,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 4a813885428dee..f89e7adf18c7f3 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -603,7 +603,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; @@ -818,9 +818,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 afd2902622c735..bab36b4cb1ae26 100644 --- a/src/wallet/walletdb.cpp +++ b/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 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 f2cc527fb08a42..01d6933291c3af 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -202,10 +202,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; } @@ -215,17 +211,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; @@ -305,7 +296,6 @@ class WalletBatch private: std::unique_ptr m_batch; - WalletDatabase& m_database; // External functions listening to the current db txn outcome. // Listeners are cleared at the end of the transaction. @@ -326,9 +316,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);