From 0658e2d0d393609ba01df525069aaef5b7ba5bdd Mon Sep 17 00:00:00 2001 From: Patrick Strateman Date: Sat, 26 Nov 2016 19:32:30 -0800 Subject: [PATCH] Make nWalletDBUpdated atomic to avoid a potential race. zcash: cherry picked from commit d63ff6265b0c6ae30efcbb9120d4db419606198a zcash: https://github.com/bitcoin/bitcoin/pull/9227 --- src/wallet/db.cpp | 3 -- src/wallet/db.h | 2 - src/wallet/wallet.cpp | 2 +- src/wallet/walletdb.cpp | 89 +++++++++++++++++++++++------------------ src/wallet/walletdb.h | 2 + 5 files changed, 54 insertions(+), 44 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index d45f3662f..088a74784 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -24,9 +24,6 @@ using namespace std; -unsigned int nWalletDBUpdated; - - // // CDB // diff --git a/src/wallet/db.h b/src/wallet/db.h index 1b0da7882..1aa88f90b 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -24,8 +24,6 @@ static const unsigned int DEFAULT_WALLET_DBLOGSIZE = 100; static const bool DEFAULT_WALLET_PRIVDB = true; -extern unsigned int nWalletDBUpdated; - class CDBEnv { private: diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ff0e30058..c8ce6117b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4835,7 +4835,7 @@ bool CWallet::InitLoadWallet(bool clearWitnessCaches) walletInstance->ScanForWalletTransactions(pindexRescan, true); LogPrintf(" rescan %15dms\n", GetTimeMillis() - nStart); walletInstance->SetBestChain(chainActive.GetLocator()); - nWalletDBUpdated++; + CWalletDB::IncrementUpdateCounter(); // Restore wallet transaction metadata after -zapwallettxes=1 if (GetBoolArg("-zapwallettxes", false) && GetArg("-zapwallettxes", "1") != "2") diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 4b8f0ee92..642dbf7a6 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -20,18 +20,21 @@ #include #include +#include using namespace std; static uint64_t nAccountingEntryNumber = 0; +static std::atomic nWalletDBUpdateCounter; + // // CWalletDB // bool CWalletDB::WriteName(const string& strAddress, const string& strName) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(make_pair(string("name"), strAddress), strName); } @@ -39,37 +42,37 @@ bool CWalletDB::EraseName(const string& strAddress) { // This should only be used for sending addresses, never for receiving addresses, // receiving addresses must always have an address book entry if they're not change return. - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Erase(make_pair(string("name"), strAddress)); } bool CWalletDB::WritePurpose(const string& strAddress, const string& strPurpose) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(make_pair(string("purpose"), strAddress), strPurpose); } bool CWalletDB::ErasePurpose(const string& strPurpose) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Erase(make_pair(string("purpose"), strPurpose)); } bool CWalletDB::WriteTx(uint256 hash, const CWalletTx& wtx) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("tx"), hash), wtx); } bool CWalletDB::EraseTx(uint256 hash) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Erase(std::make_pair(std::string("tx"), hash)); } bool CWalletDB::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata& keyMeta) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; if (!Write(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta, false)) @@ -89,7 +92,7 @@ bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, const CKeyMetadata &keyMeta) { const bool fEraseUnencryptedKey = true; - nWalletDBUpdated++; + nWalletDBUpdateCounter++; if (!Write(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta)) @@ -111,7 +114,7 @@ bool CWalletDB::WriteCryptedZKey(const libzcash::SproutPaymentAddress & addr, const CKeyMetadata &keyMeta) { const bool fEraseUnencryptedKey = true; - nWalletDBUpdated++; + nWalletDBUpdateCounter++; if (!Write(std::make_pair(std::string("zkeymeta"), addr), keyMeta)) return false; @@ -131,7 +134,7 @@ bool CWalletDB::WriteCryptedSaplingZKey( const CKeyMetadata &keyMeta) { const bool fEraseUnencryptedKey = true; - nWalletDBUpdated++; + nWalletDBUpdateCounter++; auto ivk = extfvk.fvk.in_viewing_key(); if (!Write(std::make_pair(std::string("sapzkeymeta"), ivk), keyMeta)) @@ -149,13 +152,13 @@ bool CWalletDB::WriteCryptedSaplingZKey( bool CWalletDB::WriteMasterKey(unsigned int nID, const CMasterKey& kMasterKey) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("mkey"), nID), kMasterKey, true); } bool CWalletDB::WriteZKey(const libzcash::SproutPaymentAddress& addr, const libzcash::SproutSpendingKey& key, const CKeyMetadata &keyMeta) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; if (!Write(std::make_pair(std::string("zkeymeta"), addr), keyMeta)) return false; @@ -167,7 +170,7 @@ bool CWalletDB::WriteSaplingZKey(const libzcash::SaplingIncomingViewingKey &ivk, const libzcash::SaplingExtendedSpendingKey &key, const CKeyMetadata &keyMeta) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; if (!Write(std::make_pair(std::string("sapzkeymeta"), ivk), keyMeta)) return false; @@ -179,58 +182,58 @@ bool CWalletDB::WriteSaplingPaymentAddress( const libzcash::SaplingPaymentAddress &addr, const libzcash::SaplingIncomingViewingKey &ivk) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("sapzaddr"), addr), ivk, false); } bool CWalletDB::WriteSproutViewingKey(const libzcash::SproutViewingKey &vk) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("vkey"), vk), '1'); } bool CWalletDB::EraseSproutViewingKey(const libzcash::SproutViewingKey &vk) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Erase(std::make_pair(std::string("vkey"), vk)); } bool CWalletDB::WriteSaplingExtendedFullViewingKey( const libzcash::SaplingExtendedFullViewingKey &extfvk) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("sapextfvk"), extfvk), '1'); } bool CWalletDB::EraseSaplingExtendedFullViewingKey( const libzcash::SaplingExtendedFullViewingKey &extfvk) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Erase(std::make_pair(std::string("sapextfvk"), extfvk)); } bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false); } bool CWalletDB::WriteWatchOnly(const CScript &dest) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1'); } bool CWalletDB::EraseWatchOnly(const CScript &dest) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Erase(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest))); } bool CWalletDB::WriteBestBlock(const CBlockLocator& locator) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::string("bestblock"), locator); } @@ -241,19 +244,19 @@ bool CWalletDB::ReadBestBlock(CBlockLocator& locator) bool CWalletDB::WriteOrderPosNext(int64_t nOrderPosNext) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::string("orderposnext"), nOrderPosNext); } bool CWalletDB::WriteDefaultKey(const CPubKey& vchPubKey) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::string("defaultkey"), vchPubKey); } bool CWalletDB::WriteWitnessCacheSize(int64_t nWitnessCacheSize) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::string("witnesscachesize"), nWitnessCacheSize); } @@ -264,13 +267,13 @@ bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool) bool CWalletDB::WritePool(int64_t nPool, const CKeyPool& keypool) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("pool"), nPool), keypool); } bool CWalletDB::ErasePool(int64_t nPool) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Erase(std::make_pair(std::string("pool"), nPool)); } @@ -1105,20 +1108,20 @@ void ThreadFlushWalletDB(const string& strFile) if (!GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) return; - unsigned int nLastSeen = nWalletDBUpdated; - unsigned int nLastFlushed = nWalletDBUpdated; + unsigned int nLastSeen = CWalletDB::GetUpdateCounter(); + unsigned int nLastFlushed = CWalletDB::GetUpdateCounter(); int64_t nLastWalletUpdate = GetTime(); while (true) { MilliSleep(500); - if (nLastSeen != nWalletDBUpdated) + if (nLastSeen != CWalletDB::GetUpdateCounter()) { - nLastSeen = nWalletDBUpdated; + nLastSeen = CWalletDB::GetUpdateCounter(); nLastWalletUpdate = GetTime(); } - if (nLastFlushed != nWalletDBUpdated && GetTime() - nLastWalletUpdate >= 2) + if (nLastFlushed != CWalletDB::GetUpdateCounter() && GetTime() - nLastWalletUpdate >= 2) { TRY_LOCK(bitdb.cs_db,lockDb); if (lockDb) @@ -1139,7 +1142,7 @@ void ThreadFlushWalletDB(const string& strFile) if (mi != bitdb.mapFileUseCount.end()) { LogPrint("db", "Flushing %s\n", strFile); - nLastFlushed = nWalletDBUpdated; + nLastFlushed = CWalletDB::GetUpdateCounter(); int64_t nStart = GetTimeMillis(); // Flush wallet file so it's self contained @@ -1282,31 +1285,41 @@ bool CWalletDB::Recover(CDBEnv& dbenv, const std::string& filename) bool CWalletDB::WriteDestData(const std::string &address, const std::string &key, const std::string &value) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("destdata"), std::make_pair(address, key)), value); } bool CWalletDB::EraseDestData(const std::string &address, const std::string &key) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Erase(std::make_pair(std::string("destdata"), std::make_pair(address, key))); } bool CWalletDB::WriteHDSeed(const HDSeed& seed) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("hdseed"), seed.Fingerprint()), seed.RawSeed()); } bool CWalletDB::WriteCryptedHDSeed(const uint256& seedFp, const std::vector& vchCryptedSecret) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::make_pair(std::string("chdseed"), seedFp), vchCryptedSecret); } bool CWalletDB::WriteHDChain(const CHDChain& chain) { - nWalletDBUpdated++; + nWalletDBUpdateCounter++; return Write(std::string("hdchain"), chain); } + +void CWalletDB::IncrementUpdateCounter() +{ + nWalletDBUpdateCounter++; +} + +unsigned int CWalletDB::GetUpdateCounter() +{ + return nWalletDBUpdateCounter; +} diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 087fda714..0a5281fce 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -203,6 +203,8 @@ public: bool WriteSaplingExtendedFullViewingKey(const libzcash::SaplingExtendedFullViewingKey &extfvk); bool EraseSaplingExtendedFullViewingKey(const libzcash::SaplingExtendedFullViewingKey &extfvk); + static void IncrementUpdateCounter(); + static unsigned int GetUpdateCounter(); private: CWalletDB(const CWalletDB&); void operator=(const CWalletDB&);