diff options
author | fanquake <fanquake@gmail.com> | 2020-12-02 07:42:34 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2020-12-02 08:23:00 +0800 |
commit | 80d4231e1638969b01c999028eaf3419bf3af23b (patch) | |
tree | d9f4aedb0c7f235ad78c2962dafa4de26409ac61 /src | |
parent | f17e8ba3a17b6516a1b1fb7f45d506a339e99f90 (diff) | |
parent | 9b74461fa293453a9eb0b1717b30b3f7fa778d91 (diff) | |
download | bitcoin-80d4231e1638969b01c999028eaf3419bf3af23b.tar.xz |
Merge #19980: refactor: Some wallet cleanups
9b74461fa293453a9eb0b1717b30b3f7fa778d91 refactor: Assert before dereference in CWallet::GetDatabase (João Barbosa)
021feb3187b207d511561c1f0ffd7f9e5e0c9c1d refactor: Drop redudant CWallet::GetDBHandle (João Barbosa)
Pull request description:
ACKs for top commit:
achow101:
Code Review ACK 9b74461fa293453a9eb0b1717b30b3f7fa778d91
meshcollider:
utACK 9b74461fa293453a9eb0b1717b30b3f7fa778d91
ryanofsky:
Code review ACK 9b74461fa293453a9eb0b1717b30b3f7fa778d91. Changes since last review: rebasing due to conflict, dropping wallet path commit c6a5cd7a64c78b162f545a3467d0fea7dcaadfcc as suggested in discussion, making GetDatabase() const in the earlier commit. Giving more descriptive title like
Tree-SHA512: 68cf3b5e9fe0acb3a5cd081086629989f213f1904cc344e5775767b56759a7d905b1e1c303afbe40f172ff81bf07f3719b59d8f6ec2de3fdd53cd0e2d220fb25
Diffstat (limited to 'src')
-rw-r--r-- | src/wallet/wallet.cpp | 56 | ||||
-rw-r--r-- | src/wallet/wallet.h | 13 | ||||
-rw-r--r-- | src/wallet/walletdb.cpp | 2 |
3 files changed, 34 insertions, 37 deletions
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3019d53c6c..65b54f39b4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -419,7 +419,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, return false; if (!crypter.Encrypt(_vMasterKey, pMasterKey.second.vchCryptedKey)) return false; - WalletBatch(*database).WriteMasterKey(pMasterKey.first, pMasterKey.second); + WalletBatch(GetDatabase()).WriteMasterKey(pMasterKey.first, pMasterKey.second); if (fWasLocked) Lock(); return true; @@ -432,7 +432,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, void CWallet::chainStateFlushed(const CBlockLocator& loc) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); batch.WriteBestBlock(loc); } @@ -444,7 +444,7 @@ void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in) nWalletVersion = nVersion; { - WalletBatch* batch = batch_in ? batch_in : new WalletBatch(*database); + WalletBatch* batch = batch_in ? batch_in : new WalletBatch(GetDatabase()); if (nWalletVersion > 40000) batch->WriteMinVersion(nWalletVersion); if (!batch_in) @@ -484,12 +484,12 @@ bool CWallet::HasWalletSpend(const uint256& txid) const void CWallet::Flush() { - database->Flush(); + GetDatabase().Flush(); } void CWallet::Close() { - database->Close(); + GetDatabase().Close(); } void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> range) @@ -615,7 +615,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) { LOCK(cs_wallet); mapMasterKeys[++nMasterKeyMaxID] = kMasterKey; - WalletBatch* encrypted_batch = new WalletBatch(*database); + WalletBatch* encrypted_batch = new WalletBatch(GetDatabase()); if (!encrypted_batch->TxnBegin()) { delete encrypted_batch; encrypted_batch = nullptr; @@ -667,12 +667,12 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) // Need to completely rewrite the wallet file; if we don't, bdb might keep // bits of the unencrypted private key in slack space in the database file. - database->Rewrite(); + 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: - database->ReloadDbEnv(); + GetDatabase().ReloadDbEnv(); } NotifyStatusChanged(this); @@ -683,7 +683,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) DBErrors CWallet::ReorderTransactions() { LOCK(cs_wallet); - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); // Old wallets didn't have any defined order for transactions // Probably a bad idea to change the output of this @@ -744,7 +744,7 @@ int64_t CWallet::IncOrderPosNext(WalletBatch* batch) if (batch) { batch->WriteOrderPosNext(nOrderPosNext); } else { - WalletBatch(*database).WriteOrderPosNext(nOrderPosNext); + WalletBatch(GetDatabase()).WriteOrderPosNext(nOrderPosNext); } return nRet; } @@ -774,7 +774,7 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash) wtx.mapValue["replaced_by_txid"] = newHash.ToString(); - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); bool success = true; if (!batch.WriteTx(wtx)) { @@ -846,7 +846,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio { LOCK(cs_wallet); - WalletBatch batch(*database, fFlushOnClose); + WalletBatch batch(GetDatabase(), fFlushOnClose); uint256 hash = tx->GetHash(); @@ -1045,7 +1045,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) { LOCK(cs_wallet); - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); std::set<uint256> todo; std::set<uint256> done; @@ -1108,7 +1108,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c return; // Do not flush the wallet here for performance reasons - WalletBatch batch(*database, false); + WalletBatch batch(GetDatabase(), false); std::set<uint256> todo; std::set<uint256> done; @@ -1446,13 +1446,13 @@ void CWallet::SetWalletFlag(uint64_t flags) { LOCK(cs_wallet); m_wallet_flags |= flags; - if (!WalletBatch(*database).WriteWalletFlags(m_wallet_flags)) + if (!WalletBatch(GetDatabase()).WriteWalletFlags(m_wallet_flags)) throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } void CWallet::UnsetWalletFlag(uint64_t flag) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); UnsetWalletFlagWithDB(batch, flag); } @@ -1491,7 +1491,7 @@ bool CWallet::AddWalletFlags(uint64_t flags) LOCK(cs_wallet); // We should never be writing unknown non-tolerable wallet flags assert(((flags & KNOWN_WALLET_FLAGS) >> 32) == (flags >> 32)); - if (!WalletBatch(*database).WriteWalletFlags(flags)) { + if (!WalletBatch(GetDatabase()).WriteWalletFlags(flags)) { throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } @@ -1582,7 +1582,7 @@ bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set<CScri return false; } if (apply_label) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); for (const CScript& script : script_pub_keys) { CTxDestination dest; ExtractDestination(script, dest); @@ -3177,10 +3177,10 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) LOCK(cs_wallet); fFirstRunRet = false; - DBErrors nLoadWalletRet = WalletBatch(*database).LoadWallet(this); + DBErrors nLoadWalletRet = WalletBatch(GetDatabase()).LoadWallet(this); if (nLoadWalletRet == DBErrors::NEED_REWRITE) { - if (database->Rewrite("\x04pool")) + if (GetDatabase().Rewrite("\x04pool")) { for (const auto& spk_man_pair : m_spk_managers) { spk_man_pair.second->RewriteDB(); @@ -3204,7 +3204,7 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut) { AssertLockHeld(cs_wallet); - DBErrors nZapSelectTxRet = WalletBatch(*database).ZapSelectTx(vHashIn, vHashOut); + DBErrors nZapSelectTxRet = WalletBatch(GetDatabase()).ZapSelectTx(vHashIn, vHashOut); for (const uint256& hash : vHashOut) { const auto& it = mapWallet.find(hash); wtxOrdered.erase(it->second.m_it_wtxOrdered); @@ -3216,7 +3216,7 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 if (nZapSelectTxRet == DBErrors::NEED_REWRITE) { - if (database->Rewrite("\x04pool")) + if (GetDatabase().Rewrite("\x04pool")) { for (const auto& spk_man_pair : m_spk_managers) { spk_man_pair.second->RewriteDB(); @@ -3254,14 +3254,14 @@ bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& add bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& strPurpose) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); return SetAddressBookWithDB(batch, address, strName, strPurpose); } bool CWallet::DelAddressBook(const CTxDestination& address) { bool is_mine; - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); { LOCK(cs_wallet); // If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted) @@ -4008,7 +4008,7 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st int rescan_height = 0; if (!gArgs.GetBoolArg("-rescan", false)) { - WalletBatch batch(*walletInstance->database); + WalletBatch batch(walletInstance->GetDatabase()); CBlockLocator locator; if (batch.ReadBestBlock(locator)) { if (const Optional<int> fork_height = chain.findLocatorFork(locator)) { @@ -4071,7 +4071,7 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st } } walletInstance->chainStateFlushed(chain.getTipLocator()); - walletInstance->database->IncrementUpdateCounter(); + walletInstance->GetDatabase().IncrementUpdateCounter(); } { @@ -4149,7 +4149,7 @@ void CWallet::postInitProcess() bool CWallet::BackupWallet(const std::string& strDest) const { - return database->Backup(strDest); + return GetDatabase().Backup(strDest); } CKeyPool::CKeyPool() @@ -4452,7 +4452,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() void CWallet::AddActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); if (!batch.WriteActiveScriptPubKeyMan(static_cast<uint8_t>(type), id, internal)) { throw std::runtime_error(std::string(__func__) + ": writing active ScriptPubKeyMan id failed"); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 69cf6b66a4..e6beb111fb 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -695,7 +695,7 @@ private: std::string m_name; /** Internal database handle. */ - std::unique_ptr<WalletDatabase> database; + std::unique_ptr<WalletDatabase> const m_database; /** * The following is used to keep track of how far behind the wallet is @@ -729,14 +729,11 @@ public: */ mutable RecursiveMutex cs_wallet; - /** Get database handle used by this wallet. Ideally this function would - * not be necessary. - */ - WalletDatabase& GetDBHandle() + WalletDatabase& GetDatabase() const override { - return *database; + assert(static_cast<bool>(m_database)); + return *m_database; } - WalletDatabase& GetDatabase() const override { return *database; } /** * Select a set of coins such that nValueRet >= nTargetValue and at least @@ -758,7 +755,7 @@ public: CWallet(interfaces::Chain* chain, const std::string& name, std::unique_ptr<WalletDatabase> database) : m_chain(chain), m_name(name), - database(std::move(database)) + m_database(std::move(database)) { } diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 45807ae6fd..c0521d3386 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -945,7 +945,7 @@ void MaybeCompactWalletDB() } for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { - WalletDatabase& dbh = pwallet->GetDBHandle(); + WalletDatabase& dbh = pwallet->GetDatabase(); unsigned int nUpdateCounter = dbh.nUpdateCounter; |