diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -564,7 +564,7 @@ nTransactionsUpdatedLastLP = nTransactionsUpdatedLast; } - // Release the wallet and main lock while waiting + // Release lock while waiting LEAVE_CRITICAL_SECTION(cs_main); { checktxtime = diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -882,7 +882,10 @@ vtx.push_back(MakeTransactionRef(*tx)); } DisconnectedBlockTransactions disconnectPool; - disconnectPool.addForBlock(vtx); + { + LOCK(::g_mempool.cs); + disconnectPool.addForBlock(vtx); + } CheckDisconnectPoolOrder(disconnectPool, correctlyOrderedIds, disconnectedTxns.size()); diff --git a/src/txmempool.h b/src/txmempool.h --- a/src/txmempool.h +++ b/src/txmempool.h @@ -54,6 +54,7 @@ }; class CTxMemPool; +extern CTxMemPool g_mempool; /** \class CTxMemPoolEntry * @@ -544,22 +545,13 @@ * `mempool.cs` whenever adding transactions to the mempool and whenever * changing the chain tip. It's necessary to keep both mutexes locked until * the mempool is consistent with the new chain tip and fully populated. - * - * @par Consistency bug - * - * The second guarantee above is not currently enforced, but - * https://github.com/bitcoin/bitcoin/pull/14193 will fix it. No known code - * in bitcoin currently depends on second guarantee, but it is important to - * fix for third party code that needs be able to frequently poll the - * mempool without locking `cs_main` and without encountering missing - * transactions during reorgs. */ mutable RecursiveMutex cs; indexed_transaction_set mapTx GUARDED_BY(cs); using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; //! All tx hashes/entries in mapTx, in random order - std::vector> vTxHashes; + std::vector> vTxHashes GUARDED_BY(cs); struct CompareIteratorById { bool operator()(const txiter &a, const txiter &b) const { @@ -626,13 +618,14 @@ void addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); - void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason); + void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason) + EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForReorg(const Config &config, const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); + EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector &vtx, - unsigned int nBlockHeight); + unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); void clear(); // lock free @@ -647,7 +640,8 @@ * the tx is not dependent on other mempool transactions to be included in a * block. */ - bool HasNoInputsOf(const CTransaction &tx) const; + bool HasNoInputsOf(const CTransaction &tx) const + EXCLUSIVE_LOCKS_REQUIRED(cs); /** Affect CreateNewBlock prioritisation of transactions */ void PrioritiseTransaction(const TxId &txid, const Amount nFeeDelta); @@ -691,7 +685,7 @@ * disconnected block that have been accepted back into the mempool. */ void UpdateTransactionsFromBlock(const std::vector &txidsToUpdate) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); + EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); /** * Try to calculate all in-mempool ancestors of entry. @@ -736,18 +730,20 @@ * this mempool. */ void TrimToSize(size_t sizelimit, - std::vector *pvNoSpendsRemaining = nullptr); + std::vector *pvNoSpendsRemaining = nullptr) + EXCLUSIVE_LOCKS_REQUIRED(cs); /** * Expire all transaction (and their dependencies) in the mempool older than * time. Return the number of removed transactions. */ - int Expire(std::chrono::seconds time); + int Expire(std::chrono::seconds time) EXCLUSIVE_LOCKS_REQUIRED(cs); /** * Reduce the size of the mempool by expiring and then trimming the mempool. */ - void LimitSize(size_t limit, std::chrono::seconds age); + void LimitSize(size_t limit, std::chrono::seconds age) + EXCLUSIVE_LOCKS_REQUIRED(cs); /** * Calculate the ancestor and descendant count for the given transaction. @@ -932,7 +928,8 @@ // Add entries for a block while reconstructing the topological ordering so // they can be added back to the mempool simply. - void addForBlock(const std::vector &vtx); + void addForBlock(const std::vector &vtx) + EXCLUSIVE_LOCKS_REQUIRED(::g_mempool.cs); // Remove entries based on txid_index, and update memory usage. void removeForBlock(const std::vector &vtx) { @@ -976,7 +973,8 @@ * Passing fAddToMempool=false will skip trying to add the transactions * back, and instead just erase from the mempool as needed. */ - void updateMempoolForReorg(const Config &config, bool fAddToMempool); + void updateMempoolForReorg(const Config &config, bool fAddToMempool) + EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::g_mempool.cs); }; #endif // BITCOIN_TXMEMPOOL_H diff --git a/src/txmempool.cpp b/src/txmempool.cpp --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -134,7 +134,7 @@ // state to include the parent. void CTxMemPool::UpdateTransactionsFromBlock( const std::vector &txidsToUpdate) { - LOCK(cs); + AssertLockHeld(cs); // For each entry in txidsToUpdate, store the set of in-mempool, but not // in-txidsToUpdate transactions, so that we don't have to recalculate // descendants when we come across a previously seen entry. @@ -526,7 +526,7 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason) { // Remove transaction from memory pool. - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; txiter origit = mapTx.find(origTx.GetId()); if (origit != mapTx.end()) { @@ -561,7 +561,7 @@ unsigned int nMemPoolHeight, int flags) { // Remove transactions spending a coinbase which are now immature and // no-longer-final transactions. - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { @@ -631,7 +631,7 @@ */ void CTxMemPool::removeForBlock(const std::vector &vtx, unsigned int nBlockHeight) { - LOCK(cs); + AssertLockHeld(cs); DisconnectedBlockTransactions disconnectpool; disconnectpool.addForBlock(vtx); @@ -1073,7 +1073,7 @@ } int CTxMemPool::Expire(std::chrono::seconds time) { - LOCK(cs); + AssertLockHeld(cs); indexed_transaction_set::index::type::iterator it = mapTx.get().begin(); setEntries toremove; @@ -1091,7 +1091,8 @@ return stage.size(); } -void CTxMemPool::LimitSize(size_t limit, std::chrono::seconds age) { +void CTxMemPool::LimitSize(size_t limit, std::chrono::seconds age) + EXCLUSIVE_LOCKS_REQUIRED(cs) { int expired = Expire(GetTime() - age); if (expired != 0) { LogPrint(BCLog::MEMPOOL, @@ -1181,7 +1182,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector *pvNoSpendsRemaining) { - LOCK(cs); + AssertLockHeld(cs); unsigned nTxnRemoved = 0; CFeeRate maxFeeRateRemoved(Amount::zero()); @@ -1286,6 +1287,7 @@ void DisconnectedBlockTransactions::addForBlock( const std::vector &vtx) { + AssertLockHeld(::g_mempool.cs); for (const auto &tx : reverse_iterate(vtx)) { // If we already added it, just skip. auto it = queuedTx.find(tx->GetId()); @@ -1360,16 +1362,16 @@ vtx.push_back(e.GetSharedTx()); } pool.clear(); - } - // Use addForBlocks to sort the transactions and then splice them in front - // of queuedTx - DisconnectedBlockTransactions orderedTxnPool; - orderedTxnPool.addForBlock(vtx); - cachedInnerUsage += orderedTxnPool.cachedInnerUsage; - queuedTx.get().splice( - queuedTx.get().begin(), - orderedTxnPool.queuedTx.get()); + // Use addForBlocks to sort the transactions and then splice them in + // front of queuedTx + DisconnectedBlockTransactions orderedTxnPool; + orderedTxnPool.addForBlock(vtx); + cachedInnerUsage += orderedTxnPool.cachedInnerUsage; + queuedTx.get().splice( + queuedTx.get().begin(), + orderedTxnPool.queuedTx.get()); + } // We limit memory usage because we can't know if more blocks will be // disconnected @@ -1380,7 +1382,8 @@ } void DisconnectedBlockTransactions::updateMempoolForReorg(const Config &config, - bool fAddToMempool) { + bool fAddToMempool) + EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::g_mempool.cs) { AssertLockHeld(cs_main); std::vector txidsUpdate; diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -23,6 +23,7 @@ #include