diff --git a/configure.ac b/configure.ac index c88a77d3c7..c26c8b6b97 100755 --- a/configure.ac +++ b/configure.ac @@ -179,6 +179,12 @@ AC_ARG_ENABLE([glibc-back-compat], [use_glibc_compat=$enableval], [use_glibc_compat=no]) +AC_ARG_ENABLE([threadlocal], + [AS_HELP_STRING([--enable-threadlocal], + [enable features that depend on the c++ thread_local keyword (currently just thread names in debug logs). (default is to enabled if there is platform support and glibc-back-compat is not enabled)])], + [use_thread_local=$enableval], + [use_thread_local=auto]) + AC_ARG_ENABLE([asm], [AS_HELP_STRING([--disable-asm], [disable assembly routines (enabled by default)])], @@ -229,7 +235,6 @@ if test "x$enable_werror" = "xyes"; then if test "x$CXXFLAG_WERROR" = "x"; then AC_MSG_ERROR("enable-werror set but -Werror is not usable") fi - AX_CHECK_COMPILE_FLAG([-Werror=gnu],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=gnu"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=vla],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=vla"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=shadow-field],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=shadow-field"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=switch],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=switch"],,[[$CXXFLAG_WERROR]]) @@ -246,7 +251,6 @@ fi if test "x$CXXFLAGS_overridden" = "xno"; then AX_CHECK_COMPILE_FLAG([-Wall],[CXXFLAGS="$CXXFLAGS -Wall"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wextra],[CXXFLAGS="$CXXFLAGS -Wextra"],,[[$CXXFLAG_WERROR]]) - AX_CHECK_COMPILE_FLAG([-Wgnu],[CXXFLAGS="$CXXFLAGS -Wgnu"],,[[$CXXFLAG_WERROR]]) dnl some compilers will ignore -Wformat-security without -Wformat, so just combine the two here. AX_CHECK_COMPILE_FLAG([-Wformat -Wformat-security],[CXXFLAGS="$CXXFLAGS -Wformat -Wformat-security"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wvla],[CXXFLAGS="$CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]]) @@ -782,6 +786,48 @@ AC_LINK_IFELSE([AC_LANG_SOURCE([ ] ) +dnl thread_local is currently disabled when building with glibc back compat. +dnl Our minimum supported glibc is 2.17, however support for thread_local +dnl did not arrive in glibc until 2.18. +if test "x$use_thread_local" = xyes || { test "x$use_thread_local" = xauto && test "x$use_glibc_compat" = xno; }; then + TEMP_LDFLAGS="$LDFLAGS" + LDFLAGS="$TEMP_LDFLAGS $PTHREAD_CFLAGS" + AC_MSG_CHECKING([for thread_local support]) + AC_LINK_IFELSE([AC_LANG_SOURCE([ + #include + static thread_local int foo = 0; + static void run_thread() { foo++;} + int main(){ + for(int i = 0; i < 10; i++) { std::thread(run_thread).detach();} + return foo; + } + ])], + [ + case $host in + *mingw*) + dnl mingw32's implementation of thread_local has also been shown to behave + dnl erroneously under concurrent usage; see: + dnl https://gist.github.com/jamesob/fe9a872051a88b2025b1aa37bfa98605 + AC_MSG_RESULT(no) + ;; + *freebsd*) + dnl FreeBSD's implementation of thread_local is also buggy (per + dnl https://groups.google.com/d/msg/bsdmailinglist/22ncTZAbDp4/Dii_pII5AwAJ) + AC_MSG_RESULT(no) + ;; + *) + AC_DEFINE(HAVE_THREAD_LOCAL,1,[Define if thread_local is supported.]) + AC_MSG_RESULT(yes) + ;; + esac + ], + [ + AC_MSG_RESULT(no) + ] + ) + LDFLAGS="$TEMP_LDFLAGS" +fi + dnl check for gmtime_r(), fallback to gmtime_s() if that is unavailable dnl fail if neither are available. AC_MSG_CHECKING(for gmtime_r) diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 648e72376b..423f5eb9ad 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -6,7 +6,7 @@ #include -bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) +bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); // Time based nLockTime implemented in 0.1.6 diff --git a/src/crypter.cpp b/src/crypter.cpp index 8c74ecb19e..14e057c0ca 100644 --- a/src/crypter.cpp +++ b/src/crypter.cpp @@ -132,3 +132,17 @@ bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector& vchCryptedSecret, const CPubKey& vchPubKey, CKey& key) +{ + CKeyingMaterial vchSecret; + if(!DecryptSecret(vMasterKey, vchCryptedSecret, vchPubKey.GetHash(), vchSecret)) + return false; + + if (vchSecret.size() != 32) + return false; + + key.SetPubKey(vchPubKey); + key.SetSecret(vchSecret); + return (key.GetPubKey() == vchPubKey); +} diff --git a/src/crypter.h b/src/crypter.h index bf00712569..abd4cdd7b7 100644 --- a/src/crypter.h +++ b/src/crypter.h @@ -121,4 +121,5 @@ class CCrypter bool EncryptSecret(CKeyingMaterial& vMasterKey, const CSecret &vchPlaintext, const uint256& nIV, std::vector &vchCiphertext); bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector &vchCiphertext, const uint256& nIV, CSecret &vchPlaintext); +bool DecryptKey(const CKeyingMaterial& vMasterKey, const std::vector& vchCryptedSecret, const CPubKey& vchPubKey, CKey& key); #endif diff --git a/src/gridcoin/gridcoin.cpp b/src/gridcoin/gridcoin.cpp index 5e7e17d987..709cde1e5a 100644 --- a/src/gridcoin/gridcoin.cpp +++ b/src/gridcoin/gridcoin.cpp @@ -4,6 +4,7 @@ #include "chainparams.h" #include "main.h" +#include "util/threadnames.h" #include "gridcoin/backup.h" #include "gridcoin/contract/contract.h" #include "gridcoin/gridcoin.h" @@ -218,6 +219,9 @@ void InitializeResearcherContext() //! void ThreadScraper(void* parg) { + RenameThread("grc-scraper"); + util::ThreadSetInternalName("grc-scraper"); + LogPrint(BCLog::LogFlags::NOISY, "ThreadSraper starting"); try { @@ -246,6 +250,9 @@ void ThreadScraper(void* parg) //! void ThreadScraperSubscriber(void* parg) { + RenameThread("grc-scrapersubscriber"); + util::ThreadSetInternalName("grc-scrapersubscriber"); + LogPrint(BCLog::LogFlags::NOISY, "ThreadScraperSubscriber starting"); try { diff --git a/src/gridcoin/gridcoin.h b/src/gridcoin/gridcoin.h index f4b2991738..b99de55688 100644 --- a/src/gridcoin/gridcoin.h +++ b/src/gridcoin/gridcoin.h @@ -4,7 +4,8 @@ #pragma once -class CBlockIndex; +#include "fwd.h" + class CScheduler; namespace GRC { diff --git a/src/gridcoin/magnitude.h b/src/gridcoin/magnitude.h index 4df3970697..ebb56eebed 100644 --- a/src/gridcoin/magnitude.h +++ b/src/gridcoin/magnitude.h @@ -150,7 +150,7 @@ class Magnitude bool operator==(const int64_t other) const { - return static_cast(m_scaled) == other * SCALE_FACTOR; + return static_cast(m_scaled) == other * static_cast(SCALE_FACTOR); } bool operator!=(const int64_t other) const diff --git a/src/gridcoin/researcher.cpp b/src/gridcoin/researcher.cpp index 6f60c32713..aa0202d227 100644 --- a/src/gridcoin/researcher.cpp +++ b/src/gridcoin/researcher.cpp @@ -475,7 +475,7 @@ class RecentBeacons //! //! \param beacons Contains the set of pending beacons to import from. //! - void ImportRegistry(const BeaconRegistry& beacons) + void ImportRegistry(const BeaconRegistry& beacons) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pwalletMain->cs_wallet) { AssertLockHeld(cs_main); AssertLockHeld(pwalletMain->cs_wallet); @@ -498,7 +498,7 @@ class RecentBeacons //! //! \return A pointer to the pending beacon if one exists for the CPID. //! - const PendingBeacon* Try(const Cpid cpid) + const PendingBeacon* Try(const Cpid cpid) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); @@ -531,7 +531,7 @@ class RecentBeacons //! \param cpid CPID that the beacon was advertised for. //! \param result Contains the public key if the transaction succeeded. //! - void Remember(const Cpid cpid, const AdvertiseBeaconResult& result) + void Remember(const Cpid cpid, const AdvertiseBeaconResult& result) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pwalletMain->cs_wallet) { AssertLockHeld(cs_main); diff --git a/src/gridcoin/scraper/scraper.cpp b/src/gridcoin/scraper/scraper.cpp index f568e03887..15aa872e00 100755 --- a/src/gridcoin/scraper/scraper.cpp +++ b/src/gridcoin/scraper/scraper.cpp @@ -44,6 +44,13 @@ fs::path pathScraper = {}; extern bool fShutdown; extern CWallet* pwalletMain; + +CCriticalSection cs_Scraper; +CCriticalSection cs_StructScraperFileManifest; +CCriticalSection cs_ConvergedScraperStatsCache; +CCriticalSection cs_TeamIDMap; +CCriticalSection cs_VerifiedBeacons; + bool fScraperActive = false; std::vector> vuserpass; std::vector> vprojectteamids; @@ -84,20 +91,20 @@ struct ScraperFileManifest // Both TeamIDMap and ProjTeamETags are protected by cs_TeamIDMap. // --------------- project -------------team name -- teamID typedef std::map> mTeamIDs; -mTeamIDs TeamIDMap; +mTeamIDs TeamIDMap GUARDED_BY(cs_TeamIDMap); // ProjTeamETags is not persisted to disk. There would be little to be gained by doing so. The scrapers are restarted very // rarely, and on restart, this would only save downloading team files for those projects that have one or TeamIDs missing AND // an ETag had NOT changed since the last pull. Not worth the complexity. // --------------- project ---- eTag typedef std::map mProjectTeamETags; -mProjectTeamETags ProjTeamETags; +mProjectTeamETags ProjTeamETags GUARDED_BY(cs_TeamIDMap); std::vector GetTeamWhiteList(); std::string urlsanity(const std::string& s, const std::string& type); -ScraperFileManifest StructScraperFileManifest = {}; +ScraperFileManifest StructScraperFileManifest GUARDED_BY(cs_StructScraperFileManifest) = {}; // Although scraper_net.h declares these maps, we define them here instead of // in scraper_net.cpp to ensure that the executable destroys these objects in @@ -109,13 +116,7 @@ std::map CSplitBlob::mapParts; std::map> CScraperManifest::mapManifest; // Global cache for converged scraper stats. Access must be with the lock cs_ConvergedScraperStatsCache taken. -ConvergedScraperStats ConvergedScraperStatsCache = {}; - -CCriticalSection cs_Scraper; -CCriticalSection cs_StructScraperFileManifest; -CCriticalSection cs_ConvergedScraperStatsCache; -CCriticalSection cs_TeamIDMap; -CCriticalSection cs_VerifiedBeacons; +ConvergedScraperStats ConvergedScraperStatsCache GUARDED_BY(cs_ConvergedScraperStatsCache) = {}; void _log(logattribute eType, const std::string& sCall, const std::string& sMessage); @@ -176,17 +177,17 @@ extern UniValue SuperblockToJson(const Superblock& superblock); namespace { //! //! \brief Get the block index for the height to consider beacons eligible for -//! rewards. +//! rewards. A lock must be held on cs_main before calling this function. //! //! \return Block index for the height of a block about 1 hour below the chain //! tip. //! -const CBlockIndex* GetBeaconConsensusHeight() +const CBlockIndex* GetBeaconConsensusHeight() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - static BlockFinder block_finder; - AssertLockHeld(cs_main); + static BlockFinder block_finder; + // Use 4 times the BLOCK_GRANULARITY which moves the consensus block every hour. // TODO: Make the mod a function of SCRAPER_CMANIFEST_RETENTION_TIME in scraper.h. return block_finder.FindByHeight( @@ -280,7 +281,7 @@ BeaconConsensus GetConsensusBeaconList() // the only one to survive, which is fine. We only need one. // This map has to be global because the scraper function is reentrant. -ScraperVerifiedBeacons g_verified_beacons; +ScraperVerifiedBeacons g_verified_beacons GUARDED_BY(cs_VerifiedBeacons); // Use of this global should be protected by a lock on cs_VerifiedBeacons ScraperVerifiedBeacons& GetVerifiedBeacons() @@ -1863,7 +1864,7 @@ bool DownloadProjectTeamFiles(const WhitelistSnapshot& projectWhitelist) // Note this should be called with a lock held on cs_TeamIDMap, which is intended to protect both // TeamIDMap and ProjTeamETags. -bool ProcessProjectTeamFile(const std::string& project, const fs::path& file, const std::string& etag) +bool ProcessProjectTeamFile(const std::string& project, const fs::path& file, const std::string& etag) EXCLUSIVE_LOCKS_REQUIRED(cs_TeamIDMap) { std::map mTeamIdsForProject; @@ -2448,7 +2449,7 @@ uint256 GetFileHash(const fs::path& inputfile) // Note that cs_StructScraperFileManifest needs to be taken before calling. -uint256 GetmScraperFileManifestHash() +uint256 GetmScraperFileManifestHash() EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest) { uint256 nHash; CDataStream ss(SER_NETWORK, 1); @@ -2752,7 +2753,7 @@ bool StoreTeamIDList(const fs::path& file) // Insert entry into Manifest. Note that cs_StructScraperFileManifest needs to be taken before calling. -bool InsertScraperFileManifestEntry(ScraperFileManifestEntry& entry) +bool InsertScraperFileManifestEntry(ScraperFileManifestEntry& entry) EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest) { // This less readable form is so we know whether the element already existed or not. std::pair ret; @@ -2773,7 +2774,7 @@ bool InsertScraperFileManifestEntry(ScraperFileManifestEntry& entry) } // Delete entry from Manifest and corresponding file if it exists. Note that cs_StructScraperFileManifest needs to be taken before calling. -unsigned int DeleteScraperFileManifestEntry(ScraperFileManifestEntry& entry) +unsigned int DeleteScraperFileManifestEntry(ScraperFileManifestEntry& entry) EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest) { unsigned int ret; @@ -2800,7 +2801,7 @@ unsigned int DeleteScraperFileManifestEntry(ScraperFileManifestEntry& entry) // Mark manifest entry non-current. The reason this is encapsulated in a function is // to ensure the rehash is done. Note that cs_StructScraperFileManifest needs to be // taken before calling. -bool MarkScraperFileManifestEntryNonCurrent(ScraperFileManifestEntry& entry) +bool MarkScraperFileManifestEntryNonCurrent(ScraperFileManifestEntry& entry) EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest) { entry.current = false; @@ -4063,7 +4064,7 @@ unsigned int ScraperDeleteUnauthorizedCScraperManifests() // A lock needs to be taken on cs_StructScraperFileManifest for this function. // The sCManifestName is the public key of the scraper in address form. -bool ScraperSendFileManifestContents(CBitcoinAddress& Address, CKey& Key) +bool ScraperSendFileManifestContents(CBitcoinAddress& Address, CKey& Key) EXCLUSIVE_LOCKS_REQUIRED(cs_StructScraperFileManifest) { // This "broadcasts" the current ScraperFileManifest contents to the network. @@ -5303,8 +5304,13 @@ UniValue sendscraperfilemanifest(const UniValue& params, bool fHelp) bool ret; if (IsScraperAuthorizedToBroadcastManifests(AddressOut, KeyOut)) { + LOCK(cs_StructScraperFileManifest); + _log(logattribute::INFO, "LOCK", "cs_StructScraperFileManifest"); + ret = ScraperSendFileManifestContents(AddressOut, KeyOut); uiInterface.NotifyScraperEvent(scrapereventtypes::Manifest, CT_NEW, {}); + + _log(logattribute::INFO, "ENDLOCK", "cs_StructScraperFileManifest"); } else ret = false; @@ -5525,6 +5531,8 @@ UniValue testnewsb(const UniValue& params, bool fHelp) if (params.size() == 1) nReducedCacheBits = params[0].get_int(); + UniValue res(UniValue::VOBJ); + { LOCK(cs_ConvergedScraperStatsCache); @@ -5535,12 +5543,10 @@ UniValue testnewsb(const UniValue& params, bool fHelp) return error; } - } - UniValue res(UniValue::VOBJ); - - _log(logattribute::INFO, "testnewsb", "Size of the PastConvergences map = " + ToString(ConvergedScraperStatsCache.PastConvergences.size())); - res.pushKV("Size of the PastConvergences map", ToString(ConvergedScraperStatsCache.PastConvergences.size())); + _log(logattribute::INFO, "testnewsb", "Size of the PastConvergences map = " + ToString(ConvergedScraperStatsCache.PastConvergences.size())); + res.pushKV("Size of the PastConvergences map", ToString(ConvergedScraperStatsCache.PastConvergences.size())); + } // Contract binary pack/unpack check... _log(logattribute::INFO, "testnewsb", "Checking compatibility with binary SB pack/unpack by packing then unpacking, then comparing to the original"); diff --git a/src/gridcoin/superblock.h b/src/gridcoin/superblock.h index cb18911a77..c7cc32a988 100644 --- a/src/gridcoin/superblock.h +++ b/src/gridcoin/superblock.h @@ -22,7 +22,7 @@ extern std::vector GetVerifiedBeaconIDs(const ScraperPendingBeaconMap& extern ScraperStatsAndVerifiedBeacons GetScraperStatsByConvergedManifest(const ConvergedManifest& StructConvergedManifest); class CBlockIndex; -class ConvergedScraperStats; // Forward for Superblock +struct ConvergedScraperStats; // Forward for Superblock namespace GRC { class Superblock; // Forward for QuorumHash diff --git a/src/gridcoinresearchd.cpp b/src/gridcoinresearchd.cpp index d5ef2f3481..6e53d95b3e 100644 --- a/src/gridcoinresearchd.cpp +++ b/src/gridcoinresearchd.cpp @@ -10,6 +10,7 @@ #include "chainparams.h" #include "chainparamsbase.h" #include "util.h" +#include "util/threadnames.h" #include "net.h" #include "txdb.h" #include "wallet/walletdb.h" @@ -37,6 +38,8 @@ bool AppInit(int argc, char* argv[]) #endif SetupEnvironment(); + util::ThreadSetInternalName("gridcoinresearchd-main"); + SetupServerArgs(); // Note every function above the InitLogging() call must use tfm::format or similar. diff --git a/src/init.cpp b/src/init.cpp index 6c6d64478e..8a0c4571d5 100755 --- a/src/init.cpp +++ b/src/init.cpp @@ -6,6 +6,7 @@ #include "chainparams.h" #include "util.h" +#include "util/threadnames.h" #include "net.h" #include "txdb.h" #include "wallet/walletdb.h" @@ -702,6 +703,7 @@ void ThreadAppInit2(ThreadHandlerPtr th) { // Make this thread recognisable RenameThread("grc-appinit2"); + util::ThreadSetInternalName("grc-appinit2"); bGridcoinCoreInitComplete = false; @@ -1253,6 +1255,9 @@ bool AppInit2(ThreadHandlerPtr threads) // Create new keyUser and set as default key RandAddSeedPerfmon(); + // So Clang doesn't complain, even though we are really essentially single-threaded here. + LOCK(pwalletMain->cs_wallet); + CPubKey newDefaultKey; if (pwalletMain->GetKeyFromPool(newDefaultKey, false)) { pwalletMain->SetDefaultKey(newDefaultKey); @@ -1375,6 +1380,10 @@ bool AppInit2(ThreadHandlerPtr threads) { LogPrintf("mapBlockIndex.size() = %" PRIszu, mapBlockIndex.size()); LogPrintf("nBestHeight = %d", nBestHeight); + + // So Clang doesn't complian, even though we are essentially single-threaded here. + LOCK(pwalletMain->cs_wallet); + LogPrintf("setKeyPool.size() = %" PRIszu, pwalletMain->setKeyPool.size()); LogPrintf("mapWallet.size() = %" PRIszu, pwalletMain->mapWallet.size()); LogPrintf("mapAddressBook.size() = %" PRIszu, pwalletMain->mapAddressBook.size()); diff --git a/src/keystore.cpp b/src/keystore.cpp index c75baa4307..35b00ebb9c 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -97,24 +97,33 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) if (!SetCrypted()) return false; + bool keyPass = false; + bool keyFail = false; + CryptedKeyMap::const_iterator mi = mapCryptedKeys.begin(); for (; mi != mapCryptedKeys.end(); ++mi) { - const CPubKey& vchPubKey = mi->second.first; - const std::vector& vchCryptedSecret = mi->second.second; - CSecret vchSecret; - if(!DecryptSecret(vMasterKeyIn, vchCryptedSecret, vchPubKey.GetHash(), vchSecret)) - return false; - if (vchSecret.size() != 32) - return false; + const CPubKey &vchPubKey = (*mi).second.first; + const std::vector &vchCryptedSecret = (*mi).second.second; CKey key; - key.SetPubKey(vchPubKey); - key.SetSecret(vchSecret); - if (key.GetPubKey() == vchPubKey) + if (!DecryptKey(vMasterKeyIn, vchCryptedSecret, vchPubKey, key)) + { + keyFail = true; + break; + } + keyPass = true; + if (fDecryptionThoroughlyChecked) break; - return false; } + if (keyPass && keyFail) + { + error("%s: The wallet is probably corrupted: Some keys decrypt but not all.", __func__); + assert(false); + } + if (keyFail || !keyPass) + return false; vMasterKey = vMasterKeyIn; + fDecryptionThoroughlyChecked = true; } NotifyStatusChanged(this); return true; diff --git a/src/keystore.h b/src/keystore.h index bd78d7a90b..4802a4fc49 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -113,6 +113,8 @@ class CCryptoKeyStore : public CBasicKeyStore // if fUseCrypto is false, vMasterKey must be empty bool fUseCrypto; + bool fDecryptionThoroughlyChecked; + protected: bool SetCrypted(); diff --git a/src/main.cpp b/src/main.cpp index 5d84c1b50f..8a688f2232 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -325,7 +325,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) // -int CMerkleTx::SetMerkleBranch(const CBlock* pblock) +int CMerkleTx::SetMerkleBranch(const CBlock* pblock) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); @@ -366,7 +366,7 @@ int CMerkleTx::SetMerkleBranch(const CBlock* pblock) } -bool AcceptToMemoryPool(CTxMemPool& pool, CTransaction &tx, bool* pfMissingInputs) +bool AcceptToMemoryPool(CTxMemPool& pool, CTransaction &tx, bool* pfMissingInputs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); if (pfMissingInputs) @@ -604,7 +604,7 @@ void CTxMemPool::queryHashes(std::vector& vtxid) vtxid.push_back(mi->first); } -int CMerkleTx::GetDepthInMainChainINTERNAL(CBlockIndex* &pindexRet) const +int CMerkleTx::GetDepthInMainChainINTERNAL(CBlockIndex* &pindexRet) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) { if (hashBlock.IsNull() || nIndex == -1) return 0; @@ -622,7 +622,7 @@ int CMerkleTx::GetDepthInMainChainINTERNAL(CBlockIndex* &pindexRet) const return pindexBest->nHeight - pindex->nHeight + 1; } -int CMerkleTx::GetDepthInMainChain(CBlockIndex* &pindexRet) const +int CMerkleTx::GetDepthInMainChain(CBlockIndex* &pindexRet) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); int nResult = GetDepthInMainChainINTERNAL(pindexRet); @@ -640,14 +640,14 @@ int CMerkleTx::GetBlocksToMaturity() const } -bool CMerkleTx::AcceptToMemoryPool() +bool CMerkleTx::AcceptToMemoryPool() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return ::AcceptToMemoryPool(mempool, *this, nullptr); } -bool CWalletTx::AcceptWalletTransaction(CTxDB& txdb) +bool CWalletTx::AcceptWalletTransaction(CTxDB& txdb) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { { @@ -666,7 +666,7 @@ bool CWalletTx::AcceptWalletTransaction(CTxDB& txdb) return false; } -bool CWalletTx::AcceptWalletTransaction() +bool CWalletTx::AcceptWalletTransaction() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { CTxDB txdb("r"); return AcceptWalletTransaction(txdb); @@ -1561,7 +1561,7 @@ bool ForceReorganizeToHash(uint256 NewHash) return true; } -bool DisconnectBlocksBatch(CTxDB& txdb, list& vResurrect, unsigned& cnt_dis, CBlockIndex* pcommon) +bool DisconnectBlocksBatch(CTxDB& txdb, list& vResurrect, unsigned& cnt_dis, CBlockIndex* pcommon) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { set vRereadCPIDs; GRC::BeaconRegistry& beacons = GRC::GetBeaconRegistry(); @@ -1676,7 +1676,7 @@ bool DisconnectBlocksBatch(CTxDB& txdb, list& vResurrect, unsigned return true; } -bool ReorganizeChain(CTxDB& txdb, unsigned &cnt_dis, unsigned &cnt_con, CBlock &blockNew, CBlockIndex* pindexNew) +bool ReorganizeChain(CTxDB& txdb, unsigned &cnt_dis, unsigned &cnt_con, CBlock &blockNew, CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { assert(pindexNew); //assert(!pindexNew->pnext); @@ -1844,7 +1844,7 @@ bool ReorganizeChain(CTxDB& txdb, unsigned &cnt_dis, unsigned &cnt_con, CBlock & return true; } -bool SetBestChain(CTxDB& txdb, CBlock &blockNew, CBlockIndex* pindexNew) +bool SetBestChain(CTxDB& txdb, CBlock &blockNew, CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { unsigned cnt_dis=0; unsigned cnt_con=0; @@ -2116,7 +2116,7 @@ bool CBlock::CheckBlock(int height1, bool fCheckPOW, bool fCheckMerkleRoot, bool return true; } -bool CBlock::AcceptBlock(bool generated_by_me) +bool CBlock::AcceptBlock(bool generated_by_me) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); @@ -2384,7 +2384,7 @@ bool AskForOutstandingBlocks(uint256 hashStart) return true; } -bool ProcessBlock(CNode* pfrom, CBlock* pblock, bool generated_by_me) +bool ProcessBlock(CNode* pfrom, CBlock* pblock, bool generated_by_me) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); @@ -2716,7 +2716,7 @@ bool LoadBlockIndex(bool fAllowNew) return true; } -void PrintBlockTree() +void PrintBlockTree() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); // pre-compute tree structure @@ -4025,7 +4025,7 @@ GRC::ClaimOption GetClaimByIndex(const CBlockIndex* const pblockindex) return block.PullClaim(); } -GRC::MintSummary CBlock::GetMint() const +GRC::MintSummary CBlock::GetMint() const EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); diff --git a/src/miner.cpp b/src/miner.cpp index 3708b9ebf3..e76b52204c 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -78,7 +78,7 @@ bool TrySignClaim( CWallet* pwallet, GRC::Claim& claim, const CBlock& block, - const bool dry_run = false) + const bool dry_run = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); @@ -142,7 +142,7 @@ bool TrySignClaim( CWallet* pwallet, const GRC::Claim& claim, const CBlock& block, - const bool dry_run = false) + const bool dry_run = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return TrySignClaim(pwallet, const_cast(claim), block, dry_run); } @@ -459,7 +459,7 @@ bool CreateRestOfTheBlock(CBlock &block, CBlockIndex* pindexPrev) bool CreateCoinStake(CBlock &blocknew, CKey &key, vector &StakeInputs, - CWallet &wallet, CBlockIndex* pindexPrev) + CWallet &wallet, CBlockIndex* pindexPrev) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::string function = __func__; function += ": "; @@ -869,7 +869,7 @@ unsigned int GetNumberOfStakeOutputs(int64_t &nValue, int64_t &nMinStakeSplitVal return nStakeOutputs; } -bool SignStakeBlock(CBlock &block, CKey &key, vector &StakeInputs, CWallet *pwallet) +bool SignStakeBlock(CBlock &block, CKey &key, vector &StakeInputs, CWallet *pwallet) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { //Sign the coinstake transaction unsigned nIn = 0; @@ -932,7 +932,7 @@ bool CreateGridcoinReward( CBlock &blocknew, CBlockIndex* pindexPrev, int64_t &nReward, - CWallet* pwallet) + CWallet* pwallet) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { // Remove fees from coinbase: int64_t nFees = blocknew.vtx[0].vout[0].nValue; diff --git a/src/net.cpp b/src/net.cpp index 859b32e627..b347501901 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -13,6 +13,7 @@ #include "init.h" #include "ui_interface.h" #include "util.h" +#include "util/threadnames.h" #include #include @@ -746,6 +747,7 @@ void ThreadSocketHandler(void* parg) { // Make this thread recognisable as the networking thread RenameThread("grc-net"); + util::ThreadSetInternalName("grc-net"); try { @@ -1118,6 +1120,7 @@ void ThreadMapPort(void* parg) { // Make this thread recognisable as the UPnP thread RenameThread("grc-UPnP"); + util::ThreadSetInternalName("grc-UPnP"); try { @@ -1268,6 +1271,7 @@ void ThreadDNSAddressSeed(void* parg) { // Make this thread recognisable as the DNS seeding thread RenameThread("grc-dnsseed"); + util::ThreadSetInternalName("grc-dnsseed"); try { @@ -1365,6 +1369,7 @@ void ThreadDumpAddress(void* parg) { // Make this thread recognisable as the address dumping thread RenameThread("grc-adrdump"); + util::ThreadSetInternalName("grc-adrdump"); try { @@ -1390,6 +1395,7 @@ void ThreadOpenConnections(void* parg) { // Make this thread recognisable as the connection opening thread RenameThread("grc-opencon"); + util::ThreadSetInternalName("grc-opencon"); try { @@ -1431,6 +1437,8 @@ void static ProcessOneShot() void static ThreadStakeMiner(void* parg) { + RenameThread("grc-stakeminer"); + util::ThreadSetInternalName("grc-stakeminer"); LogPrint(BCLog::LogFlags::NET, "ThreadStakeMiner started"); CWallet* pwallet = (CWallet*)parg; @@ -1594,7 +1602,8 @@ void ThreadOpenConnections2(void* parg) void ThreadOpenAddedConnections(void* parg) { // Make this thread recognisable as the connection opening thread - RenameThread("grc-opencon"); + RenameThread("grc-openaddedcon"); + util::ThreadSetInternalName("grc-openaddedcon"); try { @@ -1719,6 +1728,7 @@ void ThreadMessageHandler(void* parg) { // Make this thread recognisable as the message handling thread RenameThread("grc-msghand"); + util::ThreadSetInternalName("grc-msghand"); try { @@ -1982,7 +1992,9 @@ void static Discover() void StartNode(void* parg) { // Make this thread recognisable as the startup thread - RenameThread("grc-start"); + RenameThread("grc-nodestart"); + util::ThreadSetInternalName("grc-nodestart"); + fShutdown = false; MAX_OUTBOUND_CONNECTIONS = (int)gArgs.GetArg("-maxoutboundconnections", 8); int nMaxOutbound = 0; diff --git a/src/policy/fees.h b/src/policy/fees.h index 4dbbc6342a..0bc185ee2e 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -8,6 +8,7 @@ #include "amount.h" #include "primitives/transaction.h" +#include "consensus/consensus.h" enum GetMinFee_mode { @@ -19,7 +20,19 @@ enum GetMinFee_mode inline CAmount GetBaseFee(const CTransaction& tx, enum GetMinFee_mode mode = GMF_BLOCK) { // Base fee is either MIN_TX_FEE or MIN_RELAY_TX_FEE - CAmount nBaseFee = (mode == GMF_RELAY) ? MIN_RELAY_TX_FEE : MIN_TX_FEE; + + CAmount nBaseFee = 0; + + // Written this way to silence the duplicate branch warning on advanced compilers while MIN_RELAY_TX_FEE + // and MIN_TX_FEE are equal. + if (mode != GMF_RELAY) + { + nBaseFee = MIN_TX_FEE; + } + else if (mode == GMF_RELAY) + { + nBaseFee = MIN_RELAY_TX_FEE; + } // For block version 11 onwards, which corresponds to CTransaction::CURRENT_VERSION 2, // a multiplier is used on top of MIN_TX_FEE and MIN_RELAY_TX_FEE diff --git a/src/prevector.h b/src/prevector.h index 81e0155d6f..d4865bfcee 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -1,6 +1,6 @@ -// Copyright (c) 2015-2018 The Bitcoin Core developers +// Copyright (c) 2015-2020 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying -// file COPYING or https://opensource.org/licenses/mit-license.php. +// file COPYING or https://www.opensource.org/licenses/mit-license.php. #ifndef BITCOIN_PREVECTOR_H #define BITCOIN_PREVECTOR_H @@ -12,10 +12,9 @@ #include #include -#include #include +#include -#pragma pack(push, 1) /** Implements a drop-in replacement for std::vector which stores up to N * elements directly (without heap allocation). The types Size and Diff are * used to store element counts, and can be any unsigned + signed type. @@ -147,19 +146,25 @@ class prevector { }; private: - size_type _size = 0; +#pragma pack(push, 1) union direct_or_indirect { char direct[sizeof(T) * N]; struct { - size_type capacity; char* indirect; - }; - } _union = {}; + size_type capacity; + } indirect_contents; + }; +#pragma pack(pop) + alignas(char*) direct_or_indirect _union = {}; + size_type _size = 0; + + static_assert(alignof(char*) % alignof(size_type) == 0 && sizeof(char*) % alignof(size_type) == 0, "size_type cannot have more restrictive alignment requirement than pointer"); + static_assert(alignof(char*) % alignof(T) == 0, "value_type T cannot have more restrictive alignment requirement than pointer"); T* direct_ptr(difference_type pos) { return reinterpret_cast(_union.direct) + pos; } const T* direct_ptr(difference_type pos) const { return reinterpret_cast(_union.direct) + pos; } - T* indirect_ptr(difference_type pos) { return reinterpret_cast(_union.indirect) + pos; } - const T* indirect_ptr(difference_type pos) const { return reinterpret_cast(_union.indirect) + pos; } + T* indirect_ptr(difference_type pos) { return reinterpret_cast(_union.indirect_contents.indirect) + pos; } + const T* indirect_ptr(difference_type pos) const { return reinterpret_cast(_union.indirect_contents.indirect) + pos; } bool is_direct() const { return _size <= N; } void change_capacity(size_type new_capacity) { @@ -177,17 +182,17 @@ class prevector { /* FIXME: Because malloc/realloc here won't call new_handler if allocation fails, assert success. These should instead use an allocator or new/delete so that handlers are called as necessary, but performance would be slightly degraded by doing so. */ - _union.indirect = static_cast(realloc(_union.indirect, ((size_t)sizeof(T)) * new_capacity)); - assert(_union.indirect); - _union.capacity = new_capacity; + _union.indirect_contents.indirect = static_cast(realloc(_union.indirect_contents.indirect, ((size_t)sizeof(T)) * new_capacity)); + assert(_union.indirect_contents.indirect); + _union.indirect_contents.capacity = new_capacity; } else { char* new_indirect = static_cast(malloc(((size_t)sizeof(T)) * new_capacity)); assert(new_indirect); T* src = direct_ptr(0); T* dst = reinterpret_cast(new_indirect); memcpy(dst, src, size() * sizeof(T)); - _union.indirect = new_indirect; - _union.capacity = new_capacity; + _union.indirect_contents.indirect = new_indirect; + _union.indirect_contents.capacity = new_capacity; _size += N + 1; } } @@ -296,7 +301,7 @@ class prevector { if (is_direct()) { return N; } else { - return _union.capacity; + return _union.indirect_contents.capacity; } } @@ -408,7 +413,7 @@ class prevector { char* endp = (char*)&(*end()); if (!std::is_trivially_destructible::value) { while (p != last) { - p->~T(); + (*p).~T(); _size--; ++p; } @@ -419,15 +424,20 @@ class prevector { return first; } - void push_back(const T& value) { + template + void emplace_back(Args&&... args) { size_type new_size = size() + 1; if (capacity() < new_size) { change_capacity(new_size + (new_size >> 1)); } - new(item_ptr(size())) T(value); + new(item_ptr(size())) T(std::forward(args)...); _size++; } + void push_back(const T& value) { + emplace_back(value); + } + void pop_back() { erase(end() - 1, end()); } @@ -458,8 +468,8 @@ class prevector { clear(); } if (!is_direct()) { - free(_union.indirect); - _union.indirect = nullptr; + free(_union.indirect_contents.indirect); + _union.indirect_contents.indirect = nullptr; } } @@ -511,7 +521,7 @@ class prevector { if (is_direct()) { return 0; } else { - return ((size_t)(sizeof(T))) * _union.capacity; + return ((size_t)(sizeof(T))) * _union.indirect_contents.capacity; } } @@ -523,6 +533,5 @@ class prevector { return item_ptr(0); } }; -#pragma pack(pop) #endif // BITCOIN_PREVECTOR_H diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index faecf34e68..16e34af75d 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -222,7 +222,13 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value, editStatus = OK; - if(role == Qt::EditRole) + auto address_count = [this](const QVariant &value) { + LOCK(wallet->cs_wallet); + + return wallet->mapAddressBook.count(CBitcoinAddress(value.toString().toStdString()).Get()); + }; + + if (role == Qt::EditRole) { switch(index.column()) { @@ -250,7 +256,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value, } // Check for duplicate addresses to prevent accidental deletion of addresses, if you try // to paste an existing address over another address (with a different label) - else if(wallet->mapAddressBook.count(CBitcoinAddress(value.toString().toStdString()).Get())) + else if(address_count(value)) { editStatus = DUPLICATE_ADDRESS; return false; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 87aa352457..77888c9714 100755 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -22,6 +22,7 @@ #include "qtipcserver.h" #include "txdb.h" #include "util.h" +#include "util/threadnames.h" #include "winshutdownmonitor.h" #include "gridcoin/upgrade.h" #include "gridcoin/gridcoin.h" @@ -248,6 +249,8 @@ int main(int argc, char *argv[]) g_timer.InitTimer("default", false); SetupEnvironment(); + util::ThreadSetInternalName("gridcoinresearch-main"); + SetupServerArgs(); SetupUIArgs(gArgs); diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index ecf9083fa8..f4f493478a 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -348,50 +348,37 @@ static void MinerStatusChanged(ClientModel *clientmodel, bool staking, double co Q_ARG(double, coin_weight)); } -// This is ugly but is the easiest way to support the wide range of boost versions and deal with the -// boost placeholders global namespace pollution fix for later versions (>= 1.73) without breaking earlier ones. -#if BOOST_VERSION >= 107300 void ClientModel::subscribeToCoreSignals() { // Connect signals to client - uiInterface.NotifyBlocksChanged.connect(boost::bind(NotifyBlocksChanged, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3, boost::placeholders::_4)); + uiInterface.NotifyBlocksChanged.connect(boost::bind(NotifyBlocksChanged, this, + boost::placeholders::_1, boost::placeholders::_2, + boost::placeholders::_3, boost::placeholders::_4)); uiInterface.BannedListChanged.connect(boost::bind(BannedListChanged, this)); - uiInterface.NotifyNumConnectionsChanged.connect(boost::bind(NotifyNumConnectionsChanged, this, boost::placeholders::_1)); - uiInterface.NotifyAlertChanged.connect(boost::bind(NotifyAlertChanged, this, boost::placeholders::_1, boost::placeholders::_2)); - uiInterface.NotifyScraperEvent.connect(boost::bind(NotifyScraperEvent, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3)); - uiInterface.MinerStatusChanged.connect(boost::bind(MinerStatusChanged, this, boost::placeholders::_1, boost::placeholders::_2)); + uiInterface.NotifyNumConnectionsChanged.connect(boost::bind(NotifyNumConnectionsChanged, this, + boost::placeholders::_1)); + uiInterface.NotifyAlertChanged.connect(boost::bind(NotifyAlertChanged, this, + boost::placeholders::_1, boost::placeholders::_2)); + uiInterface.NotifyScraperEvent.connect(boost::bind(NotifyScraperEvent, this, + boost::placeholders::_1, boost::placeholders::_2, + boost::placeholders::_3)); + uiInterface.MinerStatusChanged.connect(boost::bind(MinerStatusChanged, this, + boost::placeholders::_1, boost::placeholders::_2)); } void ClientModel::unsubscribeFromCoreSignals() { // Disconnect signals from client - uiInterface.NotifyBlocksChanged.disconnect(boost::bind(NotifyBlocksChanged, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3, boost::placeholders::_4)); + uiInterface.NotifyBlocksChanged.disconnect(boost::bind(NotifyBlocksChanged, this, + boost::placeholders::_1, boost::placeholders::_2, + boost::placeholders::_3, boost::placeholders::_4)); uiInterface.BannedListChanged.disconnect(boost::bind(BannedListChanged, this)); - uiInterface.NotifyNumConnectionsChanged.disconnect(boost::bind(NotifyNumConnectionsChanged, this, boost::placeholders::_1)); - uiInterface.NotifyAlertChanged.disconnect(boost::bind(NotifyAlertChanged, this, boost::placeholders::_1, boost::placeholders::_2)); - uiInterface.NotifyScraperEvent.disconnect(boost::bind(NotifyScraperEvent, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3)); - uiInterface.MinerStatusChanged.disconnect(boost::bind(MinerStatusChanged, this, boost::placeholders::_1, boost::placeholders::_2)); + uiInterface.NotifyNumConnectionsChanged.disconnect(boost::bind(NotifyNumConnectionsChanged, this, + boost::placeholders::_1)); + uiInterface.NotifyAlertChanged.disconnect(boost::bind(NotifyAlertChanged, this, + boost::placeholders::_1, boost::placeholders::_2)); + uiInterface.NotifyScraperEvent.disconnect(boost::bind(NotifyScraperEvent, this, + boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3)); + uiInterface.MinerStatusChanged.disconnect(boost::bind(MinerStatusChanged, this, + boost::placeholders::_1, boost::placeholders::_2)); } -#else -void ClientModel::subscribeToCoreSignals() -{ - // Connect signals to client - uiInterface.NotifyBlocksChanged.connect(boost::bind(NotifyBlocksChanged, this, _1, _2, _3, _4)); - uiInterface.BannedListChanged.connect(boost::bind(BannedListChanged, this)); - uiInterface.NotifyNumConnectionsChanged.connect(boost::bind(NotifyNumConnectionsChanged, this, _1)); - uiInterface.NotifyAlertChanged.connect(boost::bind(NotifyAlertChanged, this, _1, _2)); - uiInterface.NotifyScraperEvent.connect(boost::bind(NotifyScraperEvent, this, _1, _2, _3)); - uiInterface.MinerStatusChanged.connect(boost::bind(MinerStatusChanged, this, _1, _2)); -} - -void ClientModel::unsubscribeFromCoreSignals() -{ - // Disconnect signals from client - uiInterface.NotifyBlocksChanged.disconnect(boost::bind(NotifyBlocksChanged, this, _1, _2, _3, _4)); - uiInterface.BannedListChanged.disconnect(boost::bind(BannedListChanged, this)); - uiInterface.NotifyNumConnectionsChanged.disconnect(boost::bind(NotifyNumConnectionsChanged, this, _1)); - uiInterface.NotifyAlertChanged.disconnect(boost::bind(NotifyAlertChanged, this, _1, _2)); - uiInterface.NotifyScraperEvent.disconnect(boost::bind(NotifyScraperEvent, this, _1, _2, _3)); - uiInterface.MinerStatusChanged.disconnect(boost::bind(MinerStatusChanged, this, _1, _2)); -} -#endif diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index 320ca6431a..a85282e755 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -11,7 +11,7 @@ class TransactionTableModel; class BanTableModel; class PeerTableModel; -class ConvergedScraperStats; +struct ConvergedScraperStats; class CWallet; QT_BEGIN_NAMESPACE diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index c1f7a191ce..96a6aefc86 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -26,7 +26,7 @@ QString ToQString(std::string s) return str1; } -QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx) +QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 8d4a1df13b..d02b8feff9 100755 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -347,7 +347,7 @@ QList TransactionRecord::decomposeTransaction(const CWallet * return parts; } -void TransactionRecord::updateStatus(const CWalletTx &wtx) +void TransactionRecord::updateStatus(const CWalletTx &wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); // Determine transaction status @@ -434,7 +434,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) } } -bool TransactionRecord::statusUpdateNeeded() +bool TransactionRecord::statusUpdateNeeded() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); return status.cur_num_blocks != nBestHeight; diff --git a/src/qt/voting/votingmodel.cpp b/src/qt/voting/votingmodel.cpp index ba55c87874..9e9986bf03 100644 --- a/src/qt/voting/votingmodel.cpp +++ b/src/qt/voting/votingmodel.cpp @@ -112,7 +112,7 @@ VotingModel::VotingModel( { LOCK(cs_main); - for (const auto iter : m_registry.Polls().OnlyActive()) { + for (const auto& iter : m_registry.Polls().OnlyActive()) { m_last_poll_time = std::max(m_last_poll_time, iter->Ref().Time()); } } @@ -202,7 +202,7 @@ std::vector VotingModel::buildPollTable(const PollFilterFlag flags) co LOCK(cs_main); - for (const auto iter : m_registry.Polls().Where(flags)) { + for (const auto& iter : m_registry.Polls().Where(flags)) { if (std::optional item = BuildPollItem(iter)) { items.push_back(std::move(*item)); } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 78af87f143..e19b8f5844 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -378,41 +378,33 @@ static void NotifyTransactionChanged(WalletModel *walletmodel, CWallet *wallet, Q_ARG(int, status)); } -// This is ugly but is the easiest way to support the wide range of boost versions and deal with the -// boost placeholders global namespace pollution fix for later versions (>= 1.73) without breaking earlier ones. -#if BOOST_VERSION >= 107300 void WalletModel::subscribeToCoreSignals() { // Connect signals to wallet - wallet->NotifyStatusChanged.connect(boost::bind(&NotifyKeyStoreStatusChanged, this, boost::placeholders::_1)); - wallet->NotifyAddressBookChanged.connect(boost::bind(NotifyAddressBookChanged, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3, boost::placeholders::_4, boost::placeholders::_5)); - wallet->NotifyTransactionChanged.connect(boost::bind(NotifyTransactionChanged, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3)); + wallet->NotifyStatusChanged.connect(boost::bind(&NotifyKeyStoreStatusChanged, this, + boost::placeholders::_1)); + wallet->NotifyAddressBookChanged.connect(boost::bind(NotifyAddressBookChanged, this, + boost::placeholders::_1, boost::placeholders::_2, + boost::placeholders::_3, boost::placeholders::_4, + boost::placeholders::_5)); + wallet->NotifyTransactionChanged.connect(boost::bind(NotifyTransactionChanged, this, + boost::placeholders::_1, boost::placeholders::_2, + boost::placeholders::_3)); } void WalletModel::unsubscribeFromCoreSignals() { // Disconnect signals from wallet - wallet->NotifyStatusChanged.disconnect(boost::bind(&NotifyKeyStoreStatusChanged, this, boost::placeholders::_1)); - wallet->NotifyAddressBookChanged.disconnect(boost::bind(NotifyAddressBookChanged, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3, boost::placeholders::_4, boost::placeholders::_5)); - wallet->NotifyTransactionChanged.disconnect(boost::bind(NotifyTransactionChanged, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3)); + wallet->NotifyStatusChanged.disconnect(boost::bind(&NotifyKeyStoreStatusChanged, this, + boost::placeholders::_1)); + wallet->NotifyAddressBookChanged.disconnect(boost::bind(NotifyAddressBookChanged, this, + boost::placeholders::_1, boost::placeholders::_2, + boost::placeholders::_3, boost::placeholders::_4, + boost::placeholders::_5)); + wallet->NotifyTransactionChanged.disconnect(boost::bind(NotifyTransactionChanged, this, + boost::placeholders::_1, boost::placeholders::_2, + boost::placeholders::_3)); } -#else -void WalletModel::subscribeToCoreSignals() -{ - // Connect signals to wallet - wallet->NotifyStatusChanged.connect(boost::bind(&NotifyKeyStoreStatusChanged, this, _1)); - wallet->NotifyAddressBookChanged.connect(boost::bind(NotifyAddressBookChanged, this, _1, _2, _3, _4, _5)); - wallet->NotifyTransactionChanged.connect(boost::bind(NotifyTransactionChanged, this, _1, _2, _3)); -} - -void WalletModel::unsubscribeFromCoreSignals() -{ - // Disconnect signals from wallet - wallet->NotifyStatusChanged.disconnect(boost::bind(&NotifyKeyStoreStatusChanged, this, _1)); - wallet->NotifyAddressBookChanged.disconnect(boost::bind(NotifyAddressBookChanged, this, _1, _2, _3, _4, _5)); - wallet->NotifyTransactionChanged.disconnect(boost::bind(NotifyTransactionChanged, this, _1, _2, _3)); -} -#endif // WalletModel::UnlockContext implementation WalletModel::UnlockContext WalletModel::requestUnlock() diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 2838243320..796db8ff2a 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/rpc/protocol.cpp b/src/rpc/protocol.cpp index 8a9cf4b1c5..b7a4da4890 100644 --- a/src/rpc/protocol.cpp +++ b/src/rpc/protocol.cpp @@ -13,7 +13,7 @@ #include #include -#include +#include #include #include #include diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 7ef750531b..f1b78d9513 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -16,7 +16,7 @@ #include #include -#include +#include #include #include #include @@ -917,7 +917,7 @@ std::vector CRPCTable::listCommands() const std::transform( mapCommands.begin(), mapCommands.end(), std::back_inserter(commandList), - boost::bind(&commandMap::value_type::first,_1) ); + boost::bind(&commandMap::value_type::first,boost::placeholders::_1) ); // remove deprecated commands from autocomplete for(auto &command: DEPRECATED_RPCS) { std::remove(commandList.begin(), commandList.end(), command); diff --git a/src/rpc/voting.cpp b/src/rpc/voting.cpp index 556fcf9c5a..202a65bb69 100644 --- a/src/rpc/voting.cpp +++ b/src/rpc/voting.cpp @@ -347,7 +347,7 @@ UniValue listpolls(const UniValue& params, bool fHelp) LOCK(cs_main); - for (const auto iter : GetPollRegistry().Polls().OnlyActive(active)) { + for (const auto& iter : GetPollRegistry().Polls().OnlyActive(active)) { if (const PollOption poll = iter->TryPollFromDisk()) { json.push_back(PollToJson(*poll, iter.Ref())); } diff --git a/src/script.h b/src/script.h index de19cfa158..34e6637d1b 100644 --- a/src/script.h +++ b/src/script.h @@ -452,7 +452,7 @@ class CScript : public CScriptBase // Immediate operand if (opcode <= OP_PUSHDATA4) { - unsigned int nSize; + unsigned int nSize = 0; if (opcode < OP_PUSHDATA1) { nSize = opcode; diff --git a/src/sync.cpp b/src/sync.cpp index 94a067c99a..4728f7c1e2 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -1,19 +1,23 @@ -// Copyright (c) 2011-2016 The Bitcoin Core developers +// Copyright (c) 2011-2020 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or https://opensource.org/licenses/mit-license.php. #include "sync.h" #include "util.h" +#include #include #include #include #ifdef DEBUG_LOCKCONTENTION +#if !defined(HAVE_THREAD_LOCAL) +static_assert(false, "thread_local is not supported"); +#endif void PrintLockContention(const char* pszName, const char* pszFile, int nLine) { - LogPrintf("LOCKCONTENTION: %s", pszName); - LogPrintf("Locker: %s:%d", pszFile, nLine); + LogPrintf("LOCKCONTENTION: %s\n", pszName); + LogPrintf("Locker: %s:%d\n", pszFile, nLine); } #endif /* DEBUG_LOCKCONTENTION */ @@ -30,104 +34,165 @@ void PrintLockContention(const char* pszName, const char* pszFile, int nLine) // struct CLockLocation { - CLockLocation(const char* pszName, const char* pszFile, int nLine, bool fTryIn) + CLockLocation( + const char* pszName, + const char* pszFile, + int nLine, + bool fTryIn, + const std::string& thread_name + ) + : fTry(fTryIn), + mutexName(pszName), + sourceFile(pszFile), + m_thread_name(thread_name), + sourceLine(nLine) {} + + std::string ToString() const { - mutexName = pszName; - sourceFile = pszFile; - sourceLine = nLine; - fTry = fTryIn; + return strprintf( + "'%s' in %s:%s%s (in thread '%s')", + mutexName, sourceFile, sourceLine, (fTry ? " (TRY)" : "") , m_thread_name); } - std::string ToString() const + std::string Name() const { - return mutexName + " " + sourceFile + ":" + ::ToString(sourceLine) + (fTry ? " (TRY)" : ""); + return mutexName; } private: bool fTry; std::string mutexName; std::string sourceFile; + const std::string& m_thread_name; int sourceLine; }; -typedef std::vector > LockStack; -typedef std::map, LockStack> LockOrders; -typedef std::set > InvLockOrders; +using LockStackItem = std::pair; +using LockStack = std::vector; +using LockStacks = std::unordered_map; -struct LockData { - // Very ugly hack: as the global constructs and destructors run single - // threaded, we use this boolean to know whether LockData still exists, - // as DeleteLock can get called by global CCriticalSection destructors - // after LockData disappears. - bool available; - LockData() : available(true) {} - ~LockData() { available = false; } +using LockPair = std::pair; +using LockOrders = std::map; +using InvLockOrders = std::set; +struct LockData { + LockStacks m_lock_stacks; LockOrders lockorders; InvLockOrders invlockorders; std::mutex dd_mutex; -} static lockdata; +}; -static thread_local std::unique_ptr lockstack; +LockData& GetLockData() { + // This approach guarantees that the object is not destroyed until after its last use. + // The operating system automatically reclaims all the memory in a program's heap when that program exits. + // Since the ~LockData() destructor is never called, the LockData class and all + // its subclasses must have implicitly-defined destructors. + static LockData& lock_data = *new LockData(); + return lock_data; +} -static void potential_deadlock_detected(const std::pair& mismatch, const LockStack& s1, const LockStack& s2) +static void potential_deadlock_detected(const LockPair& mismatch, const LockStack& s1, const LockStack& s2) { - LogPrintf("POTENTIAL DEADLOCK DETECTED"); - LogPrintf("Previous lock order was:"); - for (const std::pair & i : s2) { + LogPrintf("POTENTIAL DEADLOCK DETECTED\n"); + LogPrintf("Previous lock order was:\n"); + for (const LockStackItem& i : s1) { if (i.first == mismatch.first) { - LogPrintf(" (1)"); + LogPrintf(" (1)"); /* Continued */ } if (i.first == mismatch.second) { - LogPrintf(" (2)"); + LogPrintf(" (2)"); /* Continued */ } - LogPrintf(" %s", i.second.ToString()); + LogPrintf(" %s\n", i.second.ToString()); } - LogPrintf("Current lock order is:"); - for (const std::pair & i : s1) { + + std::string mutex_a, mutex_b; + LogPrintf("Current lock order is:\n"); + for (const LockStackItem& i : s2) { if (i.first == mismatch.first) { - LogPrintf(" (1)"); + LogPrintf(" (1)"); /* Continued */ + mutex_a = i.second.Name(); } if (i.first == mismatch.second) { - LogPrintf(" (2)"); + LogPrintf(" (2)"); /* Continued */ + mutex_b = i.second.Name(); } - LogPrintf(" %s", i.second.ToString()); + LogPrintf(" %s\n", i.second.ToString()); + } + if (g_debug_lockorder_abort) { + tfm::format(std::cerr, "Assertion failed: detected inconsistent lock order for %s, details in debug log.\n", s2.back().second.ToString()); + abort(); + } + + if (g_debug_lockorder_throw_exception) { + throw std::logic_error(strprintf("potential deadlock detected: %s -> %s -> %s", mutex_b, mutex_a, mutex_b)); + } else { + LogPrintf("potential deadlock detected: %s -> %s -> %s", mutex_b, mutex_a, mutex_b); } } -static void push_lock(void* c, const CLockLocation& locklocation, bool fTry) +static void push_lock(void* c, const CLockLocation& locklocation) { - if (!lockstack) - lockstack.reset(new LockStack); - + LockData& lockdata = GetLockData(); std::lock_guard lock(lockdata.dd_mutex); - lockstack->push_back(std::make_pair(c, locklocation)); - - for (const std::pair & i : (*lockstack)) { + LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + lock_stack.emplace_back(c, locklocation); + for (const LockStackItem& i : lock_stack) { if (i.first == c) break; - std::pair p1 = std::make_pair(i.first, c); + const LockPair p1 = std::make_pair(i.first, c); if (lockdata.lockorders.count(p1)) continue; - lockdata.lockorders[p1] = (*lockstack); - std::pair p2 = std::make_pair(c, i.first); + const LockPair p2 = std::make_pair(c, i.first); + if (lockdata.lockorders.count(p2)) { + auto lock_stack_copy = lock_stack; + // Only pop_back if not continuing program operation + if (g_debug_lockorder_abort || g_debug_lockorder_throw_exception) lock_stack.pop_back(); + potential_deadlock_detected(p1, lockdata.lockorders[p2], lock_stack_copy); + // potential_deadlock_detected() does not return unless both g_debug_lockorder_abort and + // g_debug_lockorder_throw_exception are false. + } + + lockdata.lockorders.emplace(p1, lock_stack); lockdata.invlockorders.insert(p2); - if (lockdata.lockorders.count(p2)) - potential_deadlock_detected(p1, lockdata.lockorders[p2], lockdata.lockorders[p1]); } } static void pop_lock() { - lockstack->pop_back(); + LockData& lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + + LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + lock_stack.pop_back(); + if (lock_stack.empty()) { + lockdata.m_lock_stacks.erase(std::this_thread::get_id()); + } } void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry) { - push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry), fTry); + push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry, util::ThreadGetInternalName())); +} + +void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) +{ + { + LockData& lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + + const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + if (!lock_stack.empty()) { + const auto& lastlock = lock_stack.back(); + if (lastlock.first == cs) { + lockname = lastlock.second.Name(); + return; + } + } + } + throw std::system_error(EPERM, std::generic_category(), strprintf("%s:%s %s was not most recent critical section locked", file, line, guardname)); } void LeaveCritical() @@ -137,41 +202,80 @@ void LeaveCritical() std::string LocksHeld() { + LockData& lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + + const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; std::string result; - for (const std::pair & i : *lockstack) + for (const LockStackItem& i : lock_stack) result += i.second.ToString() + std::string("\n"); return result; } -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) +static bool LockHeld(void* mutex) +{ + LockData& lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + + const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + for (const LockStackItem& i : lock_stack) { + if (i.first == mutex) return true; + } + + return false; +} + +template +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) { - for (const std::pair & i : *lockstack) - if (i.first == cs) - return; - tfm::format(std::cerr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld().c_str()); + if (LockHeld(cs)) return; + tfm::format(std::cerr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); abort(); } +template void AssertLockHeldInternal(const char*, const char*, int, Mutex*); +template void AssertLockHeldInternal(const char*, const char*, int, RecursiveMutex*); + +template +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) +{ + if (!LockHeld(cs)) return; + tfm::format(std::cerr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); + abort(); +} +template void AssertLockNotHeldInternal(const char*, const char*, int, Mutex*); +template void AssertLockNotHeldInternal(const char*, const char*, int, RecursiveMutex*); void DeleteLock(void* cs) { - if (!lockdata.available) { - // We're already shutting down. - return; - } + LockData& lockdata = GetLockData(); std::lock_guard lock(lockdata.dd_mutex); - std::pair item = std::make_pair(cs, (void*)0); + const LockPair item = std::make_pair(cs, nullptr); LockOrders::iterator it = lockdata.lockorders.lower_bound(item); while (it != lockdata.lockorders.end() && it->first.first == cs) { - std::pair invitem = std::make_pair(it->first.second, it->first.first); + const LockPair invitem = std::make_pair(it->first.second, it->first.first); lockdata.invlockorders.erase(invitem); lockdata.lockorders.erase(it++); } InvLockOrders::iterator invit = lockdata.invlockorders.lower_bound(item); while (invit != lockdata.invlockorders.end() && invit->first == cs) { - std::pair invinvitem = std::make_pair(invit->second, invit->first); + const LockPair invinvitem = std::make_pair(invit->second, invit->first); lockdata.lockorders.erase(invinvitem); lockdata.invlockorders.erase(invit++); } } +bool LockStackEmpty() +{ + LockData& lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + const auto it = lockdata.m_lock_stacks.find(std::this_thread::get_id()); + if (it == lockdata.m_lock_stacks.end()) { + return true; + } + return it->second.empty(); +} + +bool g_debug_lockorder_abort = false; +bool g_debug_lockorder_throw_exception = false; + #endif /* DEBUG_LOCKORDER */ diff --git a/src/sync.h b/src/sync.h index af70878739..62f57365fd 100644 --- a/src/sync.h +++ b/src/sync.h @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2009-2020 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or https://opensource.org/licenses/mit-license.php. @@ -9,16 +9,20 @@ #include "threadsafety.h" #include -#include #include +#include +#include -//////////////////////////////////////////////// -// // +///////////////////////////////////////////////// +// // // THE SIMPLE DEFINITION, EXCLUDING DEBUG CODE // -// // -//////////////////////////////////////////////// +// // +///////////////////////////////////////////////// /* +RecursiveMutex mutex; + std::recursive_mutex mutex; + CCriticalSection mutex; std::recursive_mutex mutex; @@ -45,17 +49,15 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII // // /////////////////////////////// - #ifdef DEBUG_LOCKORDER -template -void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false); +void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); void LeaveCritical(); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); std::string LocksHeld(); template void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs); template -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) LOCKS_EXCLUDED(cs); +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs); void DeleteLock(void* cs); bool LockStackEmpty(); @@ -65,15 +67,15 @@ bool LockStackEmpty(); * set to false in DEBUG_LOCKORDER unit tests. */ extern bool g_debug_lockorder_abort; +extern bool g_debug_lockorder_throw_exception; #else -template -inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false) {} +inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} inline void LeaveCritical() {} inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} template inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs) {} template -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) LOCKS_EXCLUDED(cs) {} +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) {} inline void DeleteLock(void* cs) {} inline bool LockStackEmpty() { return true; } #endif @@ -137,66 +139,6 @@ typedef std::condition_variable CConditionVariable; void PrintLockContention(const char* pszName, const char* pszFile, int nLine); #endif -/** Wrapper around std::unique_lock */ -class SCOPED_LOCKABLE CCriticalBlock -{ -private: - std::unique_lock lock; - - void Enter(const char* pszName, const char* pszFile, int nLine) - { - EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex())); -#ifdef DEBUG_LOCKCONTENTION - if (!lock.try_lock()) { - PrintLockContention(pszName, pszFile, nLine); -#endif - lock.lock(); -#ifdef DEBUG_LOCKCONTENTION - } -#endif - } - - bool TryEnter(const char* pszName, const char* pszFile, int nLine) - { - EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true); - lock.try_lock(); - if (!lock.owns_lock()) - LeaveCritical(); - return lock.owns_lock(); - } - -public: - CCriticalBlock(CCriticalSection& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, std::defer_lock) - { - if (fTry) - TryEnter(pszName, pszFile, nLine); - else - Enter(pszName, pszFile, nLine); - } - - CCriticalBlock(CCriticalSection* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) - { - if (!pmutexIn) return; - - lock = std::unique_lock(*pmutexIn, std::defer_lock); - if (fTry) - TryEnter(pszName, pszFile, nLine); - else - Enter(pszName, pszFile, nLine); - } - - ~CCriticalBlock() UNLOCK_FUNCTION() - { - if (lock.owns_lock()) - LeaveCritical(); - } - - operator bool() - { - return lock.owns_lock(); - } -}; - /** Wrapper around std::unique_lock style lock for Mutex. */ template class SCOPED_LOCKABLE UniqueLock : public Base @@ -204,7 +146,7 @@ class SCOPED_LOCKABLE UniqueLock : public Base private: void Enter(const char* pszName, const char* pszFile, int nLine) { - EnterCritical(pszName, pszFile, nLine, Base::mutex()); + EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex())); #ifdef DEBUG_LOCKCONTENTION if (!Base::try_lock()) { PrintLockContention(pszName, pszFile, nLine); @@ -217,7 +159,7 @@ class SCOPED_LOCKABLE UniqueLock : public Base bool TryEnter(const char* pszName, const char* pszFile, int nLine) { - EnterCritical(pszName, pszFile, nLine, Base::mutex(), true); + EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true); Base::try_lock(); if (!Base::owns_lock()) LeaveCritical(); @@ -274,7 +216,7 @@ class SCOPED_LOCKABLE UniqueLock : public Base ~reverse_lock() { templock.swap(lock); - EnterCritical(lockname.c_str(), file.c_str(), line, lock.mutex()); + EnterCritical(lockname.c_str(), file.c_str(), line, (void*)lock.mutex()); lock.lock(); } @@ -291,12 +233,11 @@ class SCOPED_LOCKABLE UniqueLock : public Base friend class reverse_lock; }; -#define REVERSE_LOCK(g) typename std::decay::type::reverse_lock PASTE2(revlock, __COUNTER__)(g, #g, __FILE__, __LINE__) - - #define PASTE(x, y) x ## y #define PASTE2(x, y) PASTE(x, y) +#define REVERSE_LOCK(g) typename std::decay::type::reverse_lock PASTE2(revlock, __COUNTER__)(g, #g, __FILE__, __LINE__) + template using DebugLock = UniqueLock::type>::type>; @@ -309,18 +250,26 @@ using DebugLock = UniqueLockcs_wallet); + pwalletMain->AddToWallet(wtx, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[0]->nTimeReceived = (unsigned int)1333333335; diff --git a/src/test/gridcoin/beacon_tests.cpp b/src/test/gridcoin/beacon_tests.cpp index c3044885a6..91cde024a2 100644 --- a/src/test/gridcoin/beacon_tests.cpp +++ b/src/test/gridcoin/beacon_tests.cpp @@ -919,7 +919,7 @@ BOOST_AUTO_TEST_CASE(it_parses_a_payload_from_a_legacy_contract_key_and_value) const GRC::BeaconPayload payload = GRC::BeaconPayload::Parse(key, value); // Legacy beacon payloads always parse to version 1: - BOOST_CHECK_EQUAL(payload.m_version, 1); + BOOST_CHECK_EQUAL(payload.m_version, (uint32_t) 1); BOOST_CHECK(payload.m_cpid == cpid); BOOST_CHECK(payload.m_beacon.m_public_key == TestKey::Public()); BOOST_CHECK_EQUAL(payload.m_beacon.m_timestamp, 0); diff --git a/src/test/gridcoin/cpid_tests.cpp b/src/test/gridcoin/cpid_tests.cpp index e659950f53..95973dd65e 100644 --- a/src/test/gridcoin/cpid_tests.cpp +++ b/src/test/gridcoin/cpid_tests.cpp @@ -249,7 +249,7 @@ BOOST_AUTO_TEST_CASE(it_is_hashable_to_key_a_lookup_map) std::hash hasher; // CPID halves, little endian - const size_t expected = 0x0706050403020100ull + 0x1514131211100908ull; + const size_t expected = static_cast(0x0706050403020100ull + 0x1514131211100908ull); BOOST_CHECK_EQUAL(hasher(cpid), expected); } diff --git a/src/test/gridcoin/magnitude_tests.cpp b/src/test/gridcoin/magnitude_tests.cpp index 62ea1f0e46..43e58c1f4f 100644 --- a/src/test/gridcoin/magnitude_tests.cpp +++ b/src/test/gridcoin/magnitude_tests.cpp @@ -18,49 +18,49 @@ BOOST_AUTO_TEST_CASE(it_initializes_to_zero_magnitude) { const GRC::Magnitude mag = GRC::Magnitude::Zero(); - BOOST_CHECK_EQUAL(mag.Scaled(), 0); + BOOST_CHECK_EQUAL(mag.Scaled(), (uint32_t) 0); } BOOST_AUTO_TEST_CASE(it_initializes_to_zero_when_rounding_invalid_magnitudes) { // Negative const GRC::Magnitude negative = GRC::Magnitude::RoundFrom(-1.234); - BOOST_CHECK_EQUAL(negative.Scaled(), 0); + BOOST_CHECK_EQUAL(negative.Scaled(), (uint32_t) 0); // Rounds-down to zero const GRC::Magnitude effectively_zero = GRC::Magnitude::RoundFrom(0.005); - BOOST_CHECK_EQUAL(effectively_zero.Scaled(), 0); + BOOST_CHECK_EQUAL(effectively_zero.Scaled(), (uint32_t) 0); // Exceeded maximum const GRC::Magnitude overflow = GRC::Magnitude::RoundFrom(32767.123); - BOOST_CHECK_EQUAL(overflow.Scaled(), 0); + BOOST_CHECK_EQUAL(overflow.Scaled(), (uint32_t) 0); } BOOST_AUTO_TEST_CASE(it_rounds_a_small_magnitude_to_two_places) { const GRC::Magnitude min = GRC::Magnitude::RoundFrom(0.0051); - BOOST_CHECK_EQUAL(min.Scaled(), 1); + BOOST_CHECK_EQUAL(min.Scaled(), (uint32_t) 1); const GRC::Magnitude max = GRC::Magnitude::RoundFrom(0.994); - BOOST_CHECK_EQUAL(max.Scaled(), 99); + BOOST_CHECK_EQUAL(max.Scaled(), (uint32_t) 99); } BOOST_AUTO_TEST_CASE(it_rounds_a_medium_magnitude_to_one_place) { const GRC::Magnitude min = GRC::Magnitude::RoundFrom(0.995); - BOOST_CHECK_EQUAL(min.Scaled(), 100); + BOOST_CHECK_EQUAL(min.Scaled(), (uint32_t) 100); const GRC::Magnitude max = GRC::Magnitude::RoundFrom(9.94); - BOOST_CHECK_EQUAL(max.Scaled(), 990); + BOOST_CHECK_EQUAL(max.Scaled(), (uint32_t) 990); } BOOST_AUTO_TEST_CASE(it_rounds_a_large_magnitude_to_a_whole) { const GRC::Magnitude min = GRC::Magnitude::RoundFrom(9.95); - BOOST_CHECK_EQUAL(min.Scaled(), 1000); + BOOST_CHECK_EQUAL(min.Scaled(), (uint32_t) 1000); const GRC::Magnitude max = GRC::Magnitude::RoundFrom(32767.0); - BOOST_CHECK_EQUAL(max.Scaled(), 3276700); + BOOST_CHECK_EQUAL(max.Scaled(), (uint32_t) 3276700); } BOOST_AUTO_TEST_CASE(it_compares_another_magnitude_for_equality) @@ -124,25 +124,25 @@ BOOST_AUTO_TEST_CASE(it_reports_its_size_category) BOOST_AUTO_TEST_CASE(it_presents_the_scaled_magnitude_representation) { const GRC::Magnitude small = GRC::Magnitude::RoundFrom(0.11); - BOOST_CHECK_EQUAL(small.Scaled(), 11); + BOOST_CHECK_EQUAL(small.Scaled(), (uint32_t) 11); const GRC::Magnitude medium = GRC::Magnitude::RoundFrom(1.1); - BOOST_CHECK_EQUAL(medium.Scaled(), 110); + BOOST_CHECK_EQUAL(medium.Scaled(), (uint32_t) 110); const GRC::Magnitude large = GRC::Magnitude::RoundFrom(11.0); - BOOST_CHECK_EQUAL(large.Scaled(), 1100); + BOOST_CHECK_EQUAL(large.Scaled(), (uint32_t) 1100); } BOOST_AUTO_TEST_CASE(it_presents_the_compact_magnitude_representation) { const GRC::Magnitude small = GRC::Magnitude::RoundFrom(0.11); - BOOST_CHECK_EQUAL(small.Compact(), 11); + BOOST_CHECK_EQUAL(small.Compact(), (uint16_t) 11); const GRC::Magnitude medium = GRC::Magnitude::RoundFrom(1.1); - BOOST_CHECK_EQUAL(medium.Compact(), 11); + BOOST_CHECK_EQUAL(medium.Compact(), (uint16_t) 11); const GRC::Magnitude large = GRC::Magnitude::RoundFrom(11.0); - BOOST_CHECK_EQUAL(large.Compact(), 11); + BOOST_CHECK_EQUAL(large.Compact(), (uint16_t) 11); } BOOST_AUTO_TEST_CASE(it_presents_the_floating_point_magnitude_representation) diff --git a/src/test/gridcoin/superblock_tests.cpp b/src/test/gridcoin/superblock_tests.cpp index b647c239bd..348261fec4 100644 --- a/src/test/gridcoin/superblock_tests.cpp +++ b/src/test/gridcoin/superblock_tests.cpp @@ -1087,7 +1087,7 @@ BOOST_AUTO_TEST_CASE(it_deserializes_from_a_stream) } const auto& beacon_ids = superblock.m_verified_beacons; - BOOST_CHECK_EQUAL(beacon_ids.m_verified.size(), 2); + BOOST_CHECK_EQUAL(beacon_ids.m_verified.size(), (size_t) 2); BOOST_CHECK(beacon_ids.m_verified[0] == meta.beacon_id_1); BOOST_CHECK(beacon_ids.m_verified[1] == meta.beacon_id_2); } @@ -1232,7 +1232,7 @@ BOOST_AUTO_TEST_CASE(it_deserializes_from_a_stream_for_fallback_convergence) } const auto& beacon_ids = superblock.m_verified_beacons; - BOOST_CHECK_EQUAL(beacon_ids.m_verified.size(), 2); + BOOST_CHECK_EQUAL(beacon_ids.m_verified.size(), (size_t) 2); BOOST_CHECK(beacon_ids.m_verified[0] == meta.beacon_id_1); BOOST_CHECK(beacon_ids.m_verified[1] == meta.beacon_id_2); } @@ -1954,7 +1954,7 @@ BOOST_AUTO_TEST_CASE(it_replaces_the_collection_from_scraper_statistics) beacon_ids.Reset(stats_and_verified_beacons.mVerifiedMap); - BOOST_CHECK_EQUAL(beacon_ids.m_verified.size(), 2); + BOOST_CHECK_EQUAL(beacon_ids.m_verified.size(), (size_t) 2); BOOST_CHECK(beacon_ids.m_verified[0] == meta.beacon_id_1); BOOST_CHECK(beacon_ids.m_verified[1] == meta.beacon_id_2); } @@ -1997,7 +1997,7 @@ BOOST_AUTO_TEST_CASE(it_deserializes_from_a_stream) GRC::Superblock::VerifiedBeacons beacon_ids; stream >> beacon_ids; - BOOST_CHECK_EQUAL(beacon_ids.m_verified.size(), 2); + BOOST_CHECK_EQUAL(beacon_ids.m_verified.size(), (size_t) 2); BOOST_CHECK(beacon_ids.m_verified[0] == meta.beacon_id_1); BOOST_CHECK(beacon_ids.m_verified[1] == meta.beacon_id_2); } @@ -2363,7 +2363,7 @@ BOOST_AUTO_TEST_CASE(it_is_hashable_to_key_a_lookup_map) }); // MD5 halves, little endian - const size_t expected = 0x0706050403020100ull + 0x1514131211100908ull; + const size_t expected = static_cast(0x0706050403020100ull + 0x1514131211100908ull); BOOST_CHECK_EQUAL(hasher(hash_md5), expected); } diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 87bbae7963..0cc4c9539c 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -190,7 +190,7 @@ BOOST_AUTO_TEST_CASE(multisig_Solver1) CScript s; s << key[0].GetPubKey() << OP_CHECKSIG; BOOST_CHECK(Solver(s, whichType, solutions)); - BOOST_CHECK(solutions.size() == 1); + BOOST_CHECK(solutions.size() == (size_t) 1); CTxDestination addr; BOOST_CHECK(ExtractDestination(s, addr)); BOOST_CHECK(addr == keyaddr[0]); @@ -203,7 +203,7 @@ BOOST_AUTO_TEST_CASE(multisig_Solver1) CScript s; s << OP_DUP << OP_HASH160 << key[0].GetPubKey().GetID() << OP_EQUALVERIFY << OP_CHECKSIG; BOOST_CHECK(Solver(s, whichType, solutions)); - BOOST_CHECK(solutions.size() == 1); + BOOST_CHECK(solutions.size() == (size_t) 1); CTxDestination addr; BOOST_CHECK(ExtractDestination(s, addr)); BOOST_CHECK(addr == keyaddr[0]); @@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(multisig_Solver1) CScript s; s << OP_2 << key[0].GetPubKey() << key[1].GetPubKey() << OP_2 << OP_CHECKMULTISIG; BOOST_CHECK(Solver(s, whichType, solutions)); - BOOST_CHECK_EQUAL(solutions.size(), 4); + BOOST_CHECK_EQUAL(solutions.size(), (size_t) 4); CTxDestination addr; BOOST_CHECK(!ExtractDestination(s, addr)); BOOST_CHECK(IsMine(keystore, s) != ISMINE_NO); @@ -229,7 +229,7 @@ BOOST_AUTO_TEST_CASE(multisig_Solver1) CScript s; s << OP_1 << key[0].GetPubKey() << key[1].GetPubKey() << OP_2 << OP_CHECKMULTISIG; BOOST_CHECK(Solver(s, whichType, solutions)); - BOOST_CHECK_EQUAL(solutions.size(), 4); + BOOST_CHECK_EQUAL(solutions.size(), (size_t) 4); vector addrs; int nRequired; BOOST_CHECK(ExtractDestinations(s, whichType, addrs, nRequired)); @@ -246,7 +246,7 @@ BOOST_AUTO_TEST_CASE(multisig_Solver1) CScript s; s << OP_2 << key[0].GetPubKey() << key[1].GetPubKey() << key[2].GetPubKey() << OP_3 << OP_CHECKMULTISIG; BOOST_CHECK(Solver(s, whichType, solutions)); - BOOST_CHECK(solutions.size() == 5); + BOOST_CHECK(solutions.size() == (size_t) 5); } } diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index 8cd7573841..6c6b943e12 100755 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -295,7 +295,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 2)); BOOST_CHECK(::AreInputsStandard(txTo, mapInputs)); - BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txTo, mapInputs), 1); + BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txTo, mapInputs), (unsigned int) 1); // Make sure adding crap to the scriptSigs makes them non-standard: for (int i = 0; i < 3; i++) @@ -318,7 +318,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) txToNonStd.vin[1].scriptSig << OP_0 << Serialize(oneOfEleven); BOOST_CHECK(!::AreInputsStandard(txToNonStd, mapInputs)); - BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txToNonStd, mapInputs), 11); + BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txToNonStd, mapInputs), (unsigned int) 11); txToNonStd.vin[0].scriptSig.clear(); BOOST_CHECK(!::AreInputsStandard(txToNonStd, mapInputs)); diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index 79c119fbb5..9b3913a597 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -20,21 +20,21 @@ BOOST_AUTO_TEST_CASE(GetSigOpCount) { // Test CScript::GetSigOpCount() CScript s1; - BOOST_CHECK_EQUAL(s1.GetSigOpCount(false), 0); - BOOST_CHECK_EQUAL(s1.GetSigOpCount(true), 0); + BOOST_CHECK_EQUAL(s1.GetSigOpCount(false), (unsigned int) 0); + BOOST_CHECK_EQUAL(s1.GetSigOpCount(true), (unsigned int) 0); uint160 dummy; s1 << OP_1 << dummy << dummy << OP_2 << OP_CHECKMULTISIG; - BOOST_CHECK_EQUAL(s1.GetSigOpCount(true), 2); + BOOST_CHECK_EQUAL(s1.GetSigOpCount(true), (unsigned int) 2); s1 << OP_IF << OP_CHECKSIG << OP_ENDIF; - BOOST_CHECK_EQUAL(s1.GetSigOpCount(true), 3); - BOOST_CHECK_EQUAL(s1.GetSigOpCount(false), 21); + BOOST_CHECK_EQUAL(s1.GetSigOpCount(true), (unsigned int) 3); + BOOST_CHECK_EQUAL(s1.GetSigOpCount(false), (unsigned int) 21); CScript p2sh; p2sh.SetDestination(s1.GetID()); CScript scriptSig; scriptSig << OP_0 << Serialize(s1); - BOOST_CHECK_EQUAL(p2sh.GetSigOpCount(scriptSig), 3); + BOOST_CHECK_EQUAL(p2sh.GetSigOpCount(scriptSig), (unsigned int) 3); std::vector keys; for (int i = 0; i < 3; i++) @@ -45,15 +45,15 @@ BOOST_AUTO_TEST_CASE(GetSigOpCount) } CScript s2; s2.SetMultisig(1, keys); - BOOST_CHECK_EQUAL(s2.GetSigOpCount(true), 3); - BOOST_CHECK_EQUAL(s2.GetSigOpCount(false), 20); + BOOST_CHECK_EQUAL(s2.GetSigOpCount(true), (unsigned int) 3); + BOOST_CHECK_EQUAL(s2.GetSigOpCount(false), (unsigned int) 20); p2sh.SetDestination(s2.GetID()); - BOOST_CHECK_EQUAL(p2sh.GetSigOpCount(true), 0); - BOOST_CHECK_EQUAL(p2sh.GetSigOpCount(false), 0); + BOOST_CHECK_EQUAL(p2sh.GetSigOpCount(true), (unsigned int) 0); + BOOST_CHECK_EQUAL(p2sh.GetSigOpCount(false), (unsigned int) 0); CScript scriptSig2; scriptSig2 << OP_1 << dummy << dummy << Serialize(s2); - BOOST_CHECK_EQUAL(p2sh.GetSigOpCount(scriptSig2), 3); + BOOST_CHECK_EQUAL(p2sh.GetSigOpCount(scriptSig2), (unsigned int) 3); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/wallet_tests.cpp b/src/test/wallet_tests.cpp index d23b2d4c49..caaf2b2f13 100755 --- a/src/test/wallet_tests.cpp +++ b/src/test/wallet_tests.cpp @@ -124,22 +124,22 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) // try making 34 cents from 1,2,5,10,20 - we can't do it exactly BOOST_CHECK( wallet.SelectCoinsMinConf(34 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_GT(nValueRet, 34 * CENT); // but should get more than 34 cents - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 3); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 BOOST_CHECK( wallet.SelectCoinsMinConf( 7 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 2); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. BOOST_CHECK( wallet.SelectCoinsMinConf( 8 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK(nValueRet == 8 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 3); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) BOOST_CHECK( wallet.SelectCoinsMinConf( 9 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 1); // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin empty_wallet(); @@ -157,26 +157,26 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 BOOST_CHECK( wallet.SelectCoinsMinConf(16 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 1); add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 BOOST_CHECK( wallet.SelectCoinsMinConf(16 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 3); add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 BOOST_CHECK( wallet.SelectCoinsMinConf(16 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1); // because in the event of a tie, the biggest coin wins + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 1); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 BOOST_CHECK( wallet.SelectCoinsMinConf(11 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 2); // check that the smallest bigger coin is used add_coin( 1*COIN); @@ -185,11 +185,11 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents BOOST_CHECK( wallet.SelectCoinsMinConf(95 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 1); BOOST_CHECK( wallet.SelectCoinsMinConf(195 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 1); // empty the wallet and start again, now with fractions of a cent, to test sub-cent change avoidance empty_wallet(); @@ -227,7 +227,7 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) BOOST_CHECK( wallet.SelectCoinsMinConf(500000 * COIN, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 10); // in ten coins + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 10); // in ten coins // if there's not enough in the smaller coins to make at least 1 cent change (0.5+0.6+0.7 < 1.0+1.0), // we need to try finding an exact subset anyway @@ -240,7 +240,7 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) add_coin(1111 * CENT); BOOST_CHECK( wallet.SelectCoinsMinConf(1 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1111 * CENT); // we get the bigger coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 1); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) empty_wallet(); @@ -250,7 +250,7 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) add_coin(1111 * CENT); BOOST_CHECK( wallet.SelectCoinsMinConf(1 * CENT, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2); // in two coins 0.4+0.6 + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 2); // in two coins 0.4+0.6 // test avoiding sub-cent change empty_wallet(); @@ -261,12 +261,12 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) // trying to make 1.0001 from these three coins BOOST_CHECK( wallet.SelectCoinsMinConf(1.0001 * COIN, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1.0105 * COIN); // we should get all coins - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 3); // but if we try to make 0.999, we should take the bigger of the two small coins to avoid sub-cent change BOOST_CHECK( wallet.SelectCoinsMinConf(0.999 * COIN, spendTime, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1.01 * COIN); // we should get 1 + 0.01 - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2); + BOOST_CHECK_EQUAL(setCoinsRet.size(), (size_t) 2); // test randomness { diff --git a/src/util.h b/src/util.h index 3e98f758cb..3753075184 100644 --- a/src/util.h +++ b/src/util.h @@ -289,7 +289,7 @@ class ThreadHandler void removeByName(const std::string tname); private: boost::thread_group threadGroup; - std::map threadMap; + std::map threadMap; }; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c9c7ecacab..4992ad96cf 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -226,7 +226,7 @@ UniValue getnewaddress(const UniValue& params, bool fHelp) } -CBitcoinAddress GetAccountAddress(string strAccount, bool bForceNew=false) +CBitcoinAddress GetAccountAddress(string strAccount, bool bForceNew=false) EXCLUSIVE_LOCKS_REQUIRED(pwalletMain->cs_wallet) { CWalletDB walletdb(pwalletMain->strWalletFile); @@ -566,7 +566,7 @@ UniValue getreceivedbyaddress(const UniValue& params, bool fHelp) } -void GetAccountAddresses(string strAccount, set& setAddress) +void GetAccountAddresses(string strAccount, set& setAddress) EXCLUSIVE_LOCKS_REQUIRED(pwalletMain->cs_wallet) { for (auto const& item : pwalletMain->mapAddressBook) { @@ -622,7 +622,7 @@ UniValue getreceivedbyaccount(const UniValue& params, bool fHelp) return ValueFromAmount(nAmount); } -int64_t GetAccountBalance(CWalletDB& walletdb, const string& strAccount, int nMinDepth, const isminefilter& filter = ISMINE_SPENDABLE) +int64_t GetAccountBalance(CWalletDB& walletdb, const string& strAccount, int nMinDepth, const isminefilter& filter = ISMINE_SPENDABLE) EXCLUSIVE_LOCKS_REQUIRED(pwalletMain->cs_wallet) { int64_t nBalance = 0; @@ -648,7 +648,7 @@ int64_t GetAccountBalance(CWalletDB& walletdb, const string& strAccount, int nMi return nBalance; } -int64_t GetAccountBalance(const string& strAccount, int nMinDepth, const isminefilter& filter = ISMINE_SPENDABLE) +int64_t GetAccountBalance(const string& strAccount, int nMinDepth, const isminefilter& filter = ISMINE_SPENDABLE) EXCLUSIVE_LOCKS_REQUIRED(pwalletMain->cs_wallet) { CWalletDB walletdb(pwalletMain->strWalletFile); return GetAccountBalance(walletdb, strAccount, nMinDepth, filter); @@ -1212,7 +1212,7 @@ struct tallyitem } }; -UniValue ListReceived(const UniValue& params, bool fByAccounts) +UniValue ListReceived(const UniValue& params, bool fByAccounts) EXCLUSIVE_LOCKS_REQUIRED(pwalletMain->cs_wallet) { // Minimum confirmations int nMinDepth = 1; @@ -1387,7 +1387,7 @@ UniValue listreceivedbyaccount(const UniValue& params, bool fHelp) void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter = ISMINE_SPENDABLE, - bool stakes_only = false) + bool stakes_only = false) EXCLUSIVE_LOCKS_REQUIRED(pwalletMain->cs_wallet) { int64_t nFee; string strSentAccount; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 52ceb30164..cb31fbc6a5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -86,7 +86,7 @@ CKey CWallet::MasterPrivateKey() const return key_out; } -CPubKey CWallet::GenerateNewKey() +CPubKey CWallet::GenerateNewKey() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); // mapKeyMetadata bool fCompressed = CanSupportFeature(FEATURE_COMPRPUBKEY); // default to compressed public keys if we want 0.6.0 wallets @@ -112,7 +112,7 @@ CPubKey CWallet::GenerateNewKey() return key.GetPubKey(); } -bool CWallet::AddKey(const CKey& key) +bool CWallet::AddKey(const CKey& key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); // mapKeyMetadata @@ -143,7 +143,7 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey, const vector& acentries, std::string strAccount) +CWallet::TxItems CWallet::OrderedTxItems(std::list& acentries, std::string strAccount) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); // mapWallet CWalletDB walletdb(strWalletFile); @@ -1310,7 +1310,7 @@ void CWallet::AvailableCoins(vector& vCoins, bool fOnlyConfirmed, const } // A lock must be taken on cs_main before calling this function. -void CWallet::AvailableCoinsForStaking(vector& vCoins, unsigned int nSpendTime, int64_t& balance_out) const +void CWallet::AvailableCoinsForStaking(vector& vCoins, unsigned int nSpendTime, int64_t& balance_out) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) { vCoins.clear(); @@ -1675,7 +1675,7 @@ bool CWallet::SelectCoins(int64_t nTargetValue, unsigned int nSpendTime, set >& vCoinsRet, GRC::MinerStatus::ErrorFlags& not_staking_error, int64_t& balance_out, - bool fMiner) const + bool fMiner) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::string function = __func__; function += ": "; @@ -2204,7 +2204,11 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) if (nLoadWalletRet != DB_LOAD_OK) return nLoadWalletRet; - fFirstRunRet = !vchDefaultKey.IsValid(); + { + LOCK(cs_wallet); + + fFirstRunRet = !vchDefaultKey.IsValid(); + } NewThread(ThreadFlushWalletDB, &strWalletFile); @@ -2305,7 +2309,7 @@ bool CWallet::GetTransaction(const uint256 &hashTx, CWalletTx& wtx) return false; } -bool CWallet::SetDefaultKey(const CPubKey &vchPubKey) +bool CWallet::SetDefaultKey(const CPubKey &vchPubKey) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { if (fFileBacked) { @@ -2514,7 +2518,7 @@ std::map CWallet::GetAddressBalances() return balances; } -set< set > CWallet::GetAddressGroupings() +set< set > CWallet::GetAddressGroupings() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); // mapWallet set< set > groupings; @@ -2690,7 +2694,7 @@ void CWallet::DisableTransaction(const CTransaction &tx) } } -bool CReserveKey::GetReservedKey(CPubKey& pubkey) +bool CReserveKey::GetReservedKey(CPubKey& pubkey) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { if (nIndex == -1) { @@ -2831,7 +2835,8 @@ void CWallet::UpdatedTransaction(const uint256 &hashTx) } } -void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) const { +void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) +{ AssertLockHeld(cs_wallet); // mapKeyMetadata mapKeyBirth.clear(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0cfdd49911..1ece81d9ed 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -86,13 +86,13 @@ class CWallet : public CCryptoKeyStore std::set>& setCoinsRet, int64_t& nValueRet, const CCoinControl* coinControl = nullptr, bool contract = false) const; - CWalletDB *pwalletdbEncryption; + CWalletDB *pwalletdbEncryption GUARDED_BY(cs_wallet); // the current wallet version: clients below this version are not able to load the wallet - int nWalletVersion; + int nWalletVersion GUARDED_BY(cs_wallet); // the maximum wallet format version: memory-only variable that specifies to what version this wallet may be upgraded - int nWalletMaxVersion; + int nWalletMaxVersion GUARDED_BY(cs_wallet); public: /// Main wallet lock. @@ -105,21 +105,25 @@ class CWallet : public CCryptoKeyStore bool fFileBacked; std::string strWalletFile; - std::set setKeyPool; - std::map mapKeyMetadata; + std::set setKeyPool GUARDED_BY(cs_wallet); + std::map mapKeyMetadata GUARDED_BY(cs_wallet); typedef std::map MasterKeyMap; - MasterKeyMap mapMasterKeys; - unsigned int nMasterKeyMaxID; + MasterKeyMap mapMasterKeys GUARDED_BY(cs_wallet); + unsigned int nMasterKeyMaxID GUARDED_BY(cs_wallet); CWallet() { + LOCK(cs_wallet); + SetNull(); } CWallet(std::string strWalletFileIn) { + LOCK(cs_wallet); + SetNull(); strWalletFile = strWalletFileIn; @@ -168,7 +172,7 @@ class CWallet : public CCryptoKeyStore //! CKey MasterPrivateKey() const; - void SetNull() + void SetNull() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { nWalletVersion = FEATURE_BASE; nWalletMaxVersion = FEATURE_BASE; @@ -179,17 +183,21 @@ class CWallet : public CCryptoKeyStore nTimeFirstKey = 0; } - std::map mapWallet; - int64_t nOrderPosNext; - std::map mapRequestCount; + std::map mapWallet GUARDED_BY(cs_wallet); + int64_t nOrderPosNext GUARDED_BY(cs_wallet); + std::map mapRequestCount GUARDED_BY(cs_wallet); - std::map mapAddressBook; + std::map mapAddressBook GUARDED_BY(cs_wallet); - CPubKey vchDefaultKey; - int64_t nTimeFirstKey; + CPubKey vchDefaultKey GUARDED_BY(cs_wallet); + int64_t nTimeFirstKey GUARDED_BY(cs_wallet); // check whether we are allowed to upgrade (or already support) to the named feature - bool CanSupportFeature(enum WalletFeature wf) { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; } + bool CanSupportFeature(enum WalletFeature wf) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) + { + AssertLockHeld(cs_wallet); + return nWalletMaxVersion >= wf; + } void AvailableCoinsForStaking(std::vector& vCoins, unsigned int nSpendTime, int64_t& nBalanceOut) const; bool SelectCoinsForStaking(unsigned int nSpendTime, std::vector >& vCoinsRet, @@ -211,7 +219,13 @@ class CWallet : public CCryptoKeyStore // Load metadata (used by LoadWallet) bool LoadKeyMetadata(const CPubKey &pubkey, const CKeyMetadata &metadata); - bool LoadMinVersion(int nVersion) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); return true; } + bool LoadMinVersion(int nVersion) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) + { + AssertLockHeld(cs_wallet); + nWalletVersion = nVersion; + nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); + return true; + } // Adds an encrypted key to the store, and saves it to disk. bool AddCryptedKey(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret); @@ -368,7 +382,7 @@ class CWallet : public CCryptoKeyStore } } - unsigned int GetKeyPoolSize() + unsigned int GetKeyPoolSize() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); // setKeyPool return setKeyPool.size(); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c584e953b6..bb6b9243a5 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -215,7 +215,7 @@ class CWalletScanState { bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, - CWalletScanState &wss, string& strType, string& strErr) + CWalletScanState &wss, string& strType, string& strErr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { try { // Unserialize @@ -437,13 +437,14 @@ static bool IsKeyType(string strType) DBErrors CWalletDB::LoadWallet(CWallet* pwallet) { + LOCK(pwallet->cs_wallet); + pwallet->vchDefaultKey = CPubKey(); CWalletScanState wss; bool fNoncriticalErrors = false; DBErrors result = DB_LOAD_OK; try { - LOCK(pwallet->cs_wallet); int nMinVersion = 0; if (Read((string)"minversion", nMinVersion)) { @@ -538,11 +539,12 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) DBErrors CWalletDB::FindWalletTx(CWallet* pwallet, vector& vTxHash, vector& vWtx) { + LOCK(pwallet->cs_wallet); + pwallet->vchDefaultKey = CPubKey(); DBErrors result = DB_LOAD_OK; try { - LOCK(pwallet->cs_wallet); int nMinVersion = 0; if (Read((string)"minversion", nMinVersion)) { @@ -732,6 +734,10 @@ bool CWalletDB::Recover(CDBEnv& dbenv, std::string filename, bool fOnlyKeys) return false; } CWallet dummyWallet; + + // Lock to prevent Clang from complaining, although this is most certainly single-threaded access. + LOCK(dummyWallet.cs_wallet); + CWalletScanState wss; DbTxn* ptxn = dbenv.TxnBegin();