From f28eb8020e3bb341e7f16c6bf93da7e92c82ec94 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 9 Mar 2017 20:29:01 +0000 Subject: Bugfix: wallet: Increment "update counter" only after actually making the applicable db changes to avoid potential races Also does all "update counter" access via IncrementUpdateCounter --- src/wallet/walletdb.cpp | 78 ++++++++++++++++++------------------------------- 1 file changed, 29 insertions(+), 49 deletions(-) (limited to 'src/wallet/walletdb.cpp') diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 342c797dd3..3015d6b963 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -32,47 +32,39 @@ static std::atomic nWalletDBUpdateCounter; bool CWalletDB::WriteName(const std::string& strAddress, const std::string& strName) { - nWalletDBUpdateCounter++; - return batch.Write(std::make_pair(std::string("name"), strAddress), strName); + return WriteIC(std::make_pair(std::string("name"), strAddress), strName); } bool CWalletDB::EraseName(const std::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. - nWalletDBUpdateCounter++; - return batch.Erase(std::make_pair(std::string("name"), strAddress)); + return EraseIC(std::make_pair(std::string("name"), strAddress)); } bool CWalletDB::WritePurpose(const std::string& strAddress, const std::string& strPurpose) { - nWalletDBUpdateCounter++; - return batch.Write(std::make_pair(std::string("purpose"), strAddress), strPurpose); + return WriteIC(std::make_pair(std::string("purpose"), strAddress), strPurpose); } bool CWalletDB::ErasePurpose(const std::string& strPurpose) { - nWalletDBUpdateCounter++; - return batch.Erase(std::make_pair(std::string("purpose"), strPurpose)); + return EraseIC(std::make_pair(std::string("purpose"), strPurpose)); } bool CWalletDB::WriteTx(const CWalletTx& wtx) { - nWalletDBUpdateCounter++; - return batch.Write(std::make_pair(std::string("tx"), wtx.GetHash()), wtx); + return WriteIC(std::make_pair(std::string("tx"), wtx.GetHash()), wtx); } bool CWalletDB::EraseTx(uint256 hash) { - nWalletDBUpdateCounter++; - return batch.Erase(std::make_pair(std::string("tx"), hash)); + return EraseIC(std::make_pair(std::string("tx"), hash)); } bool CWalletDB::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata& keyMeta) { - nWalletDBUpdateCounter++; - - if (!batch.Write(std::make_pair(std::string("keymeta"), vchPubKey), + if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta, false)) return false; @@ -82,7 +74,7 @@ bool CWalletDB::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, c vchKey.insert(vchKey.end(), vchPubKey.begin(), vchPubKey.end()); vchKey.insert(vchKey.end(), vchPrivKey.begin(), vchPrivKey.end()); - return batch.Write(std::make_pair(std::string("key"), vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false); + return WriteIC(std::make_pair(std::string("key"), vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false); } bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, @@ -90,55 +82,50 @@ bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, const CKeyMetadata &keyMeta) { const bool fEraseUnencryptedKey = true; - nWalletDBUpdateCounter++; - if (!batch.Write(std::make_pair(std::string("keymeta"), vchPubKey), + if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta)) return false; - if (!batch.Write(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false)) + if (!WriteIC(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false)) return false; if (fEraseUnencryptedKey) { - batch.Erase(std::make_pair(std::string("key"), vchPubKey)); - batch.Erase(std::make_pair(std::string("wkey"), vchPubKey)); + EraseIC(std::make_pair(std::string("key"), vchPubKey)); + EraseIC(std::make_pair(std::string("wkey"), vchPubKey)); } + return true; } bool CWalletDB::WriteMasterKey(unsigned int nID, const CMasterKey& kMasterKey) { - nWalletDBUpdateCounter++; - return batch.Write(std::make_pair(std::string("mkey"), nID), kMasterKey, true); + return WriteIC(std::make_pair(std::string("mkey"), nID), kMasterKey, true); } bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript) { - nWalletDBUpdateCounter++; - return batch.Write(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false); + return WriteIC(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false); } bool CWalletDB::WriteWatchOnly(const CScript &dest, const CKeyMetadata& keyMeta) { - nWalletDBUpdateCounter++; - if (!batch.Write(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta)) + if (!WriteIC(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta)) return false; - return batch.Write(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1'); + return WriteIC(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1'); } bool CWalletDB::EraseWatchOnly(const CScript &dest) { - nWalletDBUpdateCounter++; - if (!batch.Erase(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)))) + if (!EraseIC(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)))) return false; - return batch.Erase(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest))); + return EraseIC(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest))); } bool CWalletDB::WriteBestBlock(const CBlockLocator& locator) { - nWalletDBUpdateCounter++; - batch.Write(std::string("bestblock"), CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan - return batch.Write(std::string("bestblock_nomerkle"), locator); + WriteIC(std::string("bestblock"), CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan + return WriteIC(std::string("bestblock_nomerkle"), locator); } bool CWalletDB::ReadBestBlock(CBlockLocator& locator) @@ -149,14 +136,12 @@ bool CWalletDB::ReadBestBlock(CBlockLocator& locator) bool CWalletDB::WriteOrderPosNext(int64_t nOrderPosNext) { - nWalletDBUpdateCounter++; - return batch.Write(std::string("orderposnext"), nOrderPosNext); + return WriteIC(std::string("orderposnext"), nOrderPosNext); } bool CWalletDB::WriteDefaultKey(const CPubKey& vchPubKey) { - nWalletDBUpdateCounter++; - return batch.Write(std::string("defaultkey"), vchPubKey); + return WriteIC(std::string("defaultkey"), vchPubKey); } bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool) @@ -166,19 +151,17 @@ bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool) bool CWalletDB::WritePool(int64_t nPool, const CKeyPool& keypool) { - nWalletDBUpdateCounter++; - return batch.Write(std::make_pair(std::string("pool"), nPool), keypool); + return WriteIC(std::make_pair(std::string("pool"), nPool), keypool); } bool CWalletDB::ErasePool(int64_t nPool) { - nWalletDBUpdateCounter++; - return batch.Erase(std::make_pair(std::string("pool"), nPool)); + return EraseIC(std::make_pair(std::string("pool"), nPool)); } bool CWalletDB::WriteMinVersion(int nVersion) { - return batch.Write(std::string("minversion"), nVersion); + return WriteIC(std::string("minversion"), nVersion); } bool CWalletDB::ReadAccount(const std::string& strAccount, CAccount& account) @@ -854,21 +837,18 @@ bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path bool CWalletDB::WriteDestData(const std::string &address, const std::string &key, const std::string &value) { - nWalletDBUpdateCounter++; - return batch.Write(std::make_pair(std::string("destdata"), std::make_pair(address, key)), value); + return WriteIC(std::make_pair(std::string("destdata"), std::make_pair(address, key)), value); } bool CWalletDB::EraseDestData(const std::string &address, const std::string &key) { - nWalletDBUpdateCounter++; - return batch.Erase(std::make_pair(std::string("destdata"), std::make_pair(address, key))); + return EraseIC(std::make_pair(std::string("destdata"), std::make_pair(address, key))); } bool CWalletDB::WriteHDChain(const CHDChain& chain) { - nWalletDBUpdateCounter++; - return batch.Write(std::string("hdchain"), chain); + return WriteIC(std::string("hdchain"), chain); } void CWalletDB::IncrementUpdateCounter() -- cgit v1.2.3 From 9d15d5548d375e7841e0505cdcf4ddcf11591994 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 9 Mar 2017 20:30:31 +0000 Subject: Bugfix: wallet: Increment "update counter" when modifying account stuff --- src/wallet/walletdb.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/wallet/walletdb.cpp') diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 3015d6b963..f0d7f44ec0 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -172,12 +172,12 @@ bool CWalletDB::ReadAccount(const std::string& strAccount, CAccount& account) bool CWalletDB::WriteAccount(const std::string& strAccount, const CAccount& account) { - return batch.Write(std::make_pair(std::string("acc"), strAccount), account); + return WriteIC(std::make_pair(std::string("acc"), strAccount), account); } bool CWalletDB::WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry) { - return batch.Write(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry); + return WriteIC(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry); } bool CWalletDB::WriteAccountingEntry_Backend(const CAccountingEntry& acentry) -- cgit v1.2.3 From 23fb9adaea01c7baece6e1269e65713571088c5f Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 9 Mar 2017 20:11:36 +0000 Subject: wallet: Move nAccountingEntryNumber from static/global to CWallet --- src/wallet/walletdb.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'src/wallet/walletdb.cpp') diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index f0d7f44ec0..75dbb2edff 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -22,8 +22,6 @@ #include #include -static uint64_t nAccountingEntryNumber = 0; - static std::atomic nWalletDBUpdateCounter; // @@ -180,11 +178,6 @@ bool CWalletDB::WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccount return WriteIC(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry); } -bool CWalletDB::WriteAccountingEntry_Backend(const CAccountingEntry& acentry) -{ - return WriteAccountingEntry(++nAccountingEntryNumber, acentry); -} - CAmount CWalletDB::GetAccountCreditDebit(const std::string& strAccount) { std::list entries; @@ -321,8 +314,9 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, ssKey >> strAccount; uint64_t nNumber; ssKey >> nNumber; - if (nNumber > nAccountingEntryNumber) - nAccountingEntryNumber = nNumber; + if (nNumber > pwallet->nAccountingEntryNumber) { + pwallet->nAccountingEntryNumber = nNumber; + } if (!wss.fAnyUnordered) { -- cgit v1.2.3 From 19b3648bb52d27c3a9674159d71726b73fe532d9 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 9 Mar 2017 20:56:58 +0000 Subject: CWalletDB: Store the update counter per wallet --- src/wallet/walletdb.cpp | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) (limited to 'src/wallet/walletdb.cpp') diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 75dbb2edff..149d0e0c20 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -22,8 +22,6 @@ #include #include -static std::atomic nWalletDBUpdateCounter; - // // CWalletDB // @@ -762,20 +760,22 @@ void MaybeCompactWalletDB() return; } - static unsigned int nLastSeen = CWalletDB::GetUpdateCounter(); - static unsigned int nLastFlushed = CWalletDB::GetUpdateCounter(); + CWalletDBWrapper& dbh = pwalletMain->GetDBHandle(); + + static unsigned int nLastSeen = dbh.GetUpdateCounter(); + static unsigned int nLastFlushed = dbh.GetUpdateCounter(); static int64_t nLastWalletUpdate = GetTime(); - if (nLastSeen != CWalletDB::GetUpdateCounter()) + if (nLastSeen != dbh.GetUpdateCounter()) { - nLastSeen = CWalletDB::GetUpdateCounter(); + nLastSeen = dbh.GetUpdateCounter(); nLastWalletUpdate = GetTime(); } - if (nLastFlushed != CWalletDB::GetUpdateCounter() && GetTime() - nLastWalletUpdate >= 2) + if (nLastFlushed != dbh.GetUpdateCounter() && GetTime() - nLastWalletUpdate >= 2) { - if (CDB::PeriodicFlush(pwalletMain->GetDBHandle())) { - nLastFlushed = CWalletDB::GetUpdateCounter(); + if (CDB::PeriodicFlush(dbh)) { + nLastFlushed = dbh.GetUpdateCounter(); } } fOneThread = false; @@ -845,16 +845,6 @@ bool CWalletDB::WriteHDChain(const CHDChain& chain) return WriteIC(std::string("hdchain"), chain); } -void CWalletDB::IncrementUpdateCounter() -{ - nWalletDBUpdateCounter++; -} - -unsigned int CWalletDB::GetUpdateCounter() -{ - return nWalletDBUpdateCounter; -} - bool CWalletDB::TxnBegin() { return batch.TxnBegin(); -- cgit v1.2.3 From b124cf04ea47e1eda60bbc26a9690a9715c6b23f Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Fri, 9 Sep 2016 08:42:30 +0000 Subject: Wallet: Replace pwalletMain with a vector of wallet pointers --- src/wallet/walletdb.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'src/wallet/walletdb.cpp') diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 149d0e0c20..6f5ddece9a 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -760,24 +760,23 @@ void MaybeCompactWalletDB() return; } - CWalletDBWrapper& dbh = pwalletMain->GetDBHandle(); + for (CWalletRef pwallet : vpwallets) { + CWalletDBWrapper& dbh = pwallet->GetDBHandle(); - static unsigned int nLastSeen = dbh.GetUpdateCounter(); - static unsigned int nLastFlushed = dbh.GetUpdateCounter(); - static int64_t nLastWalletUpdate = GetTime(); + unsigned int nUpdateCounter = dbh.nUpdateCounter; - if (nLastSeen != dbh.GetUpdateCounter()) - { - nLastSeen = dbh.GetUpdateCounter(); - nLastWalletUpdate = GetTime(); - } + if (dbh.nLastSeen != nUpdateCounter) { + dbh.nLastSeen = nUpdateCounter; + dbh.nLastWalletUpdate = GetTime(); + } - if (nLastFlushed != dbh.GetUpdateCounter() && GetTime() - nLastWalletUpdate >= 2) - { - if (CDB::PeriodicFlush(dbh)) { - nLastFlushed = dbh.GetUpdateCounter(); + if (dbh.nLastFlushed != nUpdateCounter && GetTime() - dbh.nLastWalletUpdate >= 2) { + if (CDB::PeriodicFlush(dbh)) { + dbh.nLastFlushed = nUpdateCounter; + } } } + fOneThread = false; } -- cgit v1.2.3 From 84dcb45017a85e358fed274c49b342a40a1d1233 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 5 Jun 2017 21:36:02 +0000 Subject: Bugfix: wallet: Fix warningStr, errorStr argument order --- src/wallet/walletdb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/wallet/walletdb.cpp') diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 6f5ddece9a..a1fc933a37 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -825,7 +825,7 @@ bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr) { - return CDB::VerifyDatabaseFile(walletFile, dataDir, errorStr, warningStr, CWalletDB::Recover); + return CDB::VerifyDatabaseFile(walletFile, dataDir, warningStr, errorStr, CWalletDB::Recover); } bool CWalletDB::WriteDestData(const std::string &address, const std::string &key, const std::string &value) -- cgit v1.2.3 From b823a4c9f6836c802803dbd265cb7451fb93b8e7 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 5 Jun 2017 22:01:48 +0000 Subject: wallet: Include actual backup filename in recovery warning message --- src/wallet/walletdb.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/wallet/walletdb.cpp') diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index a1fc933a37..4dd1ebf677 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -783,16 +783,16 @@ void MaybeCompactWalletDB() // // Try to (very carefully!) recover wallet file if there is a problem. // -bool CWalletDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue)) +bool CWalletDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename) { - return CDB::Recover(filename, callbackDataIn, recoverKVcallback); + return CDB::Recover(filename, callbackDataIn, recoverKVcallback, out_backup_filename); } -bool CWalletDB::Recover(const std::string& filename) +bool CWalletDB::Recover(const std::string& filename, std::string& out_backup_filename) { // recover without a key filter callback // results in recovering all record types - return CWalletDB::Recover(filename, NULL, NULL); + return CWalletDB::Recover(filename, NULL, NULL, out_backup_filename); } bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) -- cgit v1.2.3 From c237bd750e11472f0f9639891036b975cf7faf15 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 5 Jun 2017 22:23:20 +0000 Subject: wallet: Update formatting --- src/wallet/walletdb.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'src/wallet/walletdb.cpp') diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 4dd1ebf677..4e2a963b71 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -60,9 +60,9 @@ bool CWalletDB::EraseTx(uint256 hash) bool CWalletDB::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata& keyMeta) { - if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey), - keyMeta, false)) + if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta, false)) { return false; + } // hash pubkey/privkey to accelerate wallet load std::vector vchKey; @@ -79,12 +79,13 @@ bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, { const bool fEraseUnencryptedKey = true; - if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey), - keyMeta)) + if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta)) { return false; + } - if (!WriteIC(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false)) + if (!WriteIC(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false)) { return false; + } if (fEraseUnencryptedKey) { EraseIC(std::make_pair(std::string("key"), vchPubKey)); @@ -106,15 +107,17 @@ bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript) bool CWalletDB::WriteWatchOnly(const CScript &dest, const CKeyMetadata& keyMeta) { - if (!WriteIC(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta)) + if (!WriteIC(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta)) { return false; + } return WriteIC(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1'); } bool CWalletDB::EraseWatchOnly(const CScript &dest) { - if (!EraseIC(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)))) + if (!EraseIC(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)))) { return false; + } return EraseIC(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest))); } -- cgit v1.2.3