LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

validation: Add missing mempool locks

Browse source
This commit is contained in:
MarcoFalke 2018-12-22 16:33:54 +01:00
parent fa0c9dbf91
commit fabeb1f613
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
5 changed files with 26 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/rpc/mining.cpp
View file

@ -469,7 +469,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
nTransactionsUpdatedLastLP = nTransactionsUpdatedLast; nTransactionsUpdatedLastLP = nTransactionsUpdatedLast;
} }
// Release the wallet and main lock while waiting // Release lock while waiting
LEAVE_CRITICAL_SECTION(cs_main); LEAVE_CRITICAL_SECTION(cs_main);
{ {
checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1);

14
src/txmempool.cpp
View file

@ -104,7 +104,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan
// for each such descendant, also update the ancestor state to include the parent. // for each such descendant, also update the ancestor state to include the parent.
void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate) 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 // For each entry in vHashesToUpdate, store the set of in-mempool, but not
// in-vHashesToUpdate transactions, so that we don't have to recalculate // in-vHashesToUpdate transactions, so that we don't have to recalculate
// descendants when we come across a previously seen entry. // 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) void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason)
{ {
// Remove transaction from memory pool // Remove transaction from memory pool
{ AssertLockHeld(cs);
LOCK(cs);
setEntries txToRemove; setEntries txToRemove;
txiter origit = mapTx.find(origTx.GetHash()); txiter origit = mapTx.find(origTx.GetHash());
if (origit != mapTx.end()) { if (origit != mapTx.end()) {
@ -484,12 +483,11 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso
RemoveStaged(setAllRemoves, false, reason); RemoveStaged(setAllRemoves, false, reason);
} }
}
void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) 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 // Remove transactions spending a coinbase which are now immature and no-longer-final transactions
LOCK(cs); AssertLockHeld(cs);
setEntries txToRemove; setEntries txToRemove;
for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
const CTransaction& tx = it->GetTx(); 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) void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight)
{ {
LOCK(cs); AssertLockHeld(cs);
std::vector<const CTxMemPoolEntry*> entries; std::vector<const CTxMemPoolEntry*> entries;
for (const auto& tx : vtx) for (const auto& tx : vtx)
{ {
@ -920,7 +918,7 @@ void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPool
} }
int CTxMemPool::Expire(int64_t time) { int CTxMemPool::Expire(int64_t time) {
LOCK(cs); AssertLockHeld(cs);
indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin(); indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin();
setEntries toremove; setEntries toremove;
while (it != mapTx.get<entry_time>().end() && it->GetTime() < time) { 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) { void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining) {
LOCK(cs); AssertLockHeld(cs);
unsigned nTxnRemoved = 0; unsigned nTxnRemoved = 0;
CFeeRate maxFeeRateRemoved(0); CFeeRate maxFeeRateRemoved(0);

25
src/txmempool.h
View file

@ -514,21 +514,12 @@ public:
* `mempool.cs` whenever adding transactions to the mempool and whenever * `mempool.cs` whenever adding transactions to the mempool and whenever
* changing the chain tip. It's necessary to keep both mutexes locked until * 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. * 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; mutable RecursiveMutex cs;
indexed_transaction_set mapTx GUARDED_BY(cs); indexed_transaction_set mapTx GUARDED_BY(cs);
using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; 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 { struct CompareIteratorByHash {
bool operator()(const txiter &a, const txiter &b) const { 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, 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 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 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_main); 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 removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight); void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
void clear(); void clear();
void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free 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 * 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. * 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 */ /** Affect CreateNewBlock prioritisation of transactions */
void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta); void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta);
@ -633,7 +624,7 @@ public:
* for). Note: vHashesToUpdate should be the set of transactions from the * for). Note: vHashesToUpdate should be the set of transactions from the
* disconnected block that have been accepted back into the mempool. * 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. /** Try to calculate all in-mempool ancestors of entry.
* (these are all calculated including the tx itself) * (these are all calculated including the tx itself)
@ -664,10 +655,10 @@ public:
* pvNoSpendsRemaining, if set, will be populated with the list of outpoints * 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. * 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. */ /** 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. * Calculate the ancestor and descendant count for the given transaction.

10
src/validation.cpp
View file

@ -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 // Returns the script flags which should be checked for a given block
static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams); 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); int expired = pool.Expire(GetTime() - age);
if (expired != 0) { if (expired != 0) {
LogPrint(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired); 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. * 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); AssertLockHeld(cs_main);
std::vector<uint256> vHashUpdate; std::vector<uint256> vHashUpdate;
@ -2549,7 +2550,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams&
LimitValidationInterfaceQueue(); 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(); CBlockIndex* starting_tip = m_chain.Tip();
bool blocks_connected = false; bool blocks_connected = false;
do { do {
@ -2669,6 +2670,7 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c
LimitValidationInterfaceQueue(); LimitValidationInterfaceQueue();
LOCK(cs_main); 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; if (!m_chain.Contains(pindex)) break;
pindex_was_in_chain = true; pindex_was_in_chain = true;
CBlockIndex *invalid_walk_tip = m_chain.Tip(); 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. // Loop until the tip is below nHeight, or we reach a pruned block.
while (!ShutdownRequested()) { 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) // Make sure nothing changed from under us (this won't happen because RewindBlockIndex runs before importing/network are active)
assert(tip == m_chain.Tip()); assert(tip == m_chain.Tip());
if (tip == nullptr || tip->nHeight < nHeight) break; if (tip == nullptr || tip->nHeight < nHeight) break;

7
src/validation.h
View file

@ -18,6 +18,7 @@
#include <protocol.h> // For CMessageHeader::MessageStartChars #include <protocol.h> // For CMessageHeader::MessageStartChars
#include <script/script_error.h> #include <script/script_error.h>
#include <sync.h> #include <sync.h>
#include <txmempool.h> // For CTxMemPool::cs
#include <versionbits.h> #include <versionbits.h>
#include <algorithm> #include <algorithm>
@ -556,7 +557,7 @@ public:
CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
// Block disconnection on our pcoinsTip: // 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: // Manual block validity manipulation:
bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main);
@ -575,8 +576,8 @@ public:
bool IsInitialBlockDownload() const; bool IsInitialBlockDownload() const;
private: 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 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); 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); CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Create a new block index entry for a given block hash */ /** Create a new block index entry for a given block hash */