diff options
| -rw-r--r-- | src/rpc/mining.cpp | 2 | ||||
| -rw-r--r-- | src/txmempool.cpp | 14 | ||||
| -rw-r--r-- | src/txmempool.h | 27 | ||||
| -rw-r--r-- | src/validation.cpp | 10 | ||||
| -rw-r--r-- | src/validation.h | 7 |
5 files changed, 26 insertions, 34 deletions
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index ba5e0cdf64..86b0845738 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -469,7 +469,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) nTransactionsUpdatedLastLP = nTransactionsUpdatedLast; } - // Release the wallet and main lock while waiting + // Release lock while waiting LEAVE_CRITICAL_SECTION(cs_main); { checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 2f4e5fee4b..9257cff718 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -104,7 +104,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan // for each such descendant, also update the ancestor state to include the parent. void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate) { - LOCK(cs); + AssertLockHeld(cs); // For each entry in vHashesToUpdate, store the set of in-mempool, but not // in-vHashesToUpdate transactions, so that we don't have to recalculate // descendants when we come across a previously seen entry. @@ -457,8 +457,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries& setDescendants void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason) { // Remove transaction from memory pool - { - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; txiter origit = mapTx.find(origTx.GetHash()); if (origit != mapTx.end()) { @@ -483,13 +482,12 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso } RemoveStaged(setAllRemoves, false, reason); - } } void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, 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++) { const CTransaction& tx = it->GetTx(); @@ -545,7 +543,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) */ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) { - LOCK(cs); + AssertLockHeld(cs); std::vector<const CTxMemPoolEntry*> entries; for (const auto& tx : vtx) { @@ -920,7 +918,7 @@ void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPool } int CTxMemPool::Expire(int64_t time) { - LOCK(cs); + AssertLockHeld(cs); indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin(); setEntries toremove; while (it != mapTx.get<entry_time>().end() && it->GetTime() < time) { @@ -1013,7 +1011,7 @@ void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) { } void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining) { - LOCK(cs); + AssertLockHeld(cs); unsigned nTxnRemoved = 0; CFeeRate maxFeeRateRemoved(0); diff --git a/src/txmempool.h b/src/txmempool.h index 59514a945b..565dd61f0f 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -514,21 +514,12 @@ public: * `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; - std::vector<std::pair<uint256, txiter> > vTxHashes; //!< All tx witness hashes/entries in mapTx, in random order + std::vector<std::pair<uint256, txiter>> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order struct CompareIteratorByHash { bool operator()(const txiter &a, const txiter &b) const { @@ -583,10 +574,10 @@ public: void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); - void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); - void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight); + void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); void clear(); void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free @@ -599,7 +590,7 @@ public: * Check that none of this transactions inputs are in the mempool, and thus * 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 uint256& hash, const CAmount& nFeeDelta); @@ -633,7 +624,7 @@ public: * for). Note: vHashesToUpdate should be the set of transactions from the * disconnected block that have been accepted back into the mempool. */ - void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); /** Try to calculate all in-mempool ancestors of entry. * (these are all calculated including the tx itself) @@ -664,10 +655,10 @@ public: * pvNoSpendsRemaining, if set, will be populated with the list of outpoints * which are not in mempool which no longer have any spends in this mempool. */ - void TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining=nullptr); + void TrimToSize(size_t sizelimit, std::vector<COutPoint>* 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(int64_t time); + int Expire(int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); /** * Calculate the ancestor and descendant count for the given transaction. diff --git a/src/validation.cpp b/src/validation.cpp index b2925efe32..85bb2103f5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -305,7 +305,8 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag // Returns the script flags which should be checked for a given block static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams); -static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) { +static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) +{ int expired = pool.Expire(GetTime() - age); if (expired != 0) { LogPrint(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired); @@ -342,7 +343,7 @@ static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) * and instead just erase from the mempool as needed. */ -static void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs) { AssertLockHeld(cs_main); std::vector<uint256> vHashUpdate; @@ -2549,7 +2550,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& LimitValidationInterfaceQueue(); { - LOCK(cs_main); + LOCK2(cs_main, ::mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed CBlockIndex* starting_tip = m_chain.Tip(); bool blocks_connected = false; do { @@ -2669,6 +2670,7 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c LimitValidationInterfaceQueue(); LOCK(cs_main); + LOCK(::mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between if (!m_chain.Contains(pindex)) break; pindex_was_in_chain = true; CBlockIndex *invalid_walk_tip = m_chain.Tip(); @@ -4102,7 +4104,7 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // Loop until the tip is below nHeight, or we reach a pruned block. while (!ShutdownRequested()) { { - LOCK(cs_main); + LOCK2(cs_main, ::mempool.cs); // Make sure nothing changed from under us (this won't happen because RewindBlockIndex runs before importing/network are active) assert(tip == m_chain.Tip()); if (tip == nullptr || tip->nHeight < nHeight) break; diff --git a/src/validation.h b/src/validation.h index 963439d35b..9b301a4bcc 100644 --- a/src/validation.h +++ b/src/validation.h @@ -18,6 +18,7 @@ #include <protocol.h> // For CMessageHeader::MessageStartChars #include <script/script_error.h> #include <sync.h> +#include <txmempool.h> // For CTxMemPool::cs #include <versionbits.h> #include <algorithm> @@ -556,7 +557,7 @@ public: CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Block disconnection on our pcoinsTip: - bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); // Manual block validity manipulation: bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); @@ -575,8 +576,8 @@ public: bool IsInitialBlockDownload() const; private: - bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Create a new block index entry for a given block hash */ |