From 868d041622e2f589ab4535c30ce683534b6d4f71 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 11 Nov 2014 16:41:44 -0800 Subject: Remove coinbase-dependant transactions during reorg. This still leaves transactions in mempool that are potentially invalid if the maturity period has been reorged out of, but at least they're not missing inputs entirely. --- src/main.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 70e3973e6..3a2c167e8 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1892,9 +1892,8 @@ bool static DisconnectTip(CValidationState &state) { // ignore validation errors in resurrected transactions list removed; CValidationState stateDummy; - if (!tx.IsCoinBase()) - if (!AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL)) - mempool.remove(tx, removed, true); + if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL)) + mempool.remove(tx, removed, true); } mempool.check(pcoinsTip); // Update chainActive and related variables. -- cgit v1.2.3 From 723d12c098456e7682e641076e76468a9fb0cec0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 11 Nov 2014 20:57:54 -0800 Subject: Remove txn which are invalidated by coinbase maturity during reorg --- src/main.cpp | 1 + src/txmempool.cpp | 26 ++++++++++++++++++++++++++ src/txmempool.h | 1 + 3 files changed, 28 insertions(+) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 3a2c167e8..9e1c41ada 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1895,6 +1895,7 @@ bool static DisconnectTip(CValidationState &state) { if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL)) mempool.remove(tx, removed, true); } + mempool.removeCoinbaseSpends(pcoinsTip, pindexDelete->nHeight); mempool.check(pcoinsTip); // Update chainActive and related variables. UpdateTip(pindexDelete->pprev); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index e13f1cc35..ff04a8ad9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -6,6 +6,7 @@ #include "txmempool.h" #include "clientversion.h" +#include "main.h" // for COINBASE_MATURITY #include "streams.h" #include "util.h" #include "utilmoneystr.h" @@ -453,6 +454,31 @@ void CTxMemPool::remove(const CTransaction &tx, std::list& removed } } +void CTxMemPool::removeCoinbaseSpends(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight) +{ + // Remove transactions spending a coinbase which are now immature + LOCK(cs); + list transactionsToRemove; + for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + const CTransaction& tx = it->second.GetTx(); + BOOST_FOREACH(const CTxIn& txin, tx.vin) { + std::map::const_iterator it2 = mapTx.find(txin.prevout.hash); + if (it2 != mapTx.end()) + continue; + const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash); + if (fSanityCheck) assert(coins); + if (!coins || (coins->IsCoinBase() && nMemPoolHeight - coins->nHeight < COINBASE_MATURITY)) { + transactionsToRemove.push_back(tx); + break; + } + } + } + BOOST_FOREACH(const CTransaction& tx, transactionsToRemove) { + list removed; + remove(tx, removed, true); + } +} + void CTxMemPool::removeConflicts(const CTransaction &tx, std::list& removed) { // Remove transactions which depend on inputs of tx, recursively diff --git a/src/txmempool.h b/src/txmempool.h index d00bdd061..f671352b5 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -113,6 +113,7 @@ public: bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry); void remove(const CTransaction &tx, std::list& removed, bool fRecursive = false); + void removeCoinbaseSpends(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight); void removeConflicts(const CTransaction &tx, std::list& removed); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, std::list& conflicts); -- cgit v1.2.3 From b7b4318f3a65d6e55d44bff5da1091ec0b3a26d2 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 11 Nov 2014 21:06:15 -0800 Subject: Make CTxMemPool::check more thourough by using CheckInputs --- src/txmempool.cpp | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ff04a8ad9..6ecd64636 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -6,7 +6,7 @@ #include "txmempool.h" #include "clientversion.h" -#include "main.h" // for COINBASE_MATURITY +#include "main.h" #include "streams.h" #include "util.h" #include "utilmoneystr.h" @@ -539,17 +539,22 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const uint64_t checkTotal = 0; + CCoinsViewCache mempoolDuplicate(const_cast(pcoins)); + LOCK(cs); + list waitingOnDependants; for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { unsigned int i = 0; checkTotal += it->second.GetTxSize(); const CTransaction& tx = it->second.GetTx(); + bool fDependsWait = false; BOOST_FOREACH(const CTxIn &txin, tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. std::map::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) { const CTransaction& tx2 = it2->second.GetTx(); assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); + fDependsWait = true; } else { const CCoins* coins = pcoins->AccessCoins(txin.prevout.hash); assert(coins && coins->IsAvailable(txin.prevout.n)); @@ -561,6 +566,29 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const assert(it3->second.n == i); i++; } + if (fDependsWait) + waitingOnDependants.push_back(&it->second); + else { + CValidationState state; CTxUndo undo; + assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, NULL)); + UpdateCoins(tx, state, mempoolDuplicate, undo, 1000000); + } + } + unsigned int stepsSinceLastRemove = 0; + while (!waitingOnDependants.empty()) { + const CTxMemPoolEntry* entry = waitingOnDependants.front(); + waitingOnDependants.pop_front(); + CValidationState state; + if (!mempoolDuplicate.HaveInputs(entry->GetTx())) { + waitingOnDependants.push_back(entry); + stepsSinceLastRemove++; + assert(stepsSinceLastRemove < waitingOnDependants.size()); + } else { + assert(CheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, NULL)); + CTxUndo undo; + UpdateCoins(entry->GetTx(), state, mempoolDuplicate, undo, 1000000); + stepsSinceLastRemove = 0; + } } for (std::map::const_iterator it = mapNextTx.begin(); it != mapNextTx.end(); it++) { uint256 hash = it->second.ptx->GetHash(); -- cgit v1.2.3 From 7fd6219af7006fddc4b675236bcd43dd7a9eb553 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 18 Nov 2014 14:57:20 -0800 Subject: Make CTxMemPool::remove more effecient by avoiding recursion --- src/txmempool.cpp | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 6ecd64636..840eb536b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -427,26 +427,32 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry) } -void CTxMemPool::remove(const CTransaction &tx, std::list& removed, bool fRecursive) +void CTxMemPool::remove(const CTransaction &origTx, std::list& removed, bool fRecursive) { // Remove transaction from memory pool { LOCK(cs); - uint256 hash = tx.GetHash(); - if (fRecursive) { - for (unsigned int i = 0; i < tx.vout.size(); i++) { - std::map::iterator it = mapNextTx.find(COutPoint(hash, i)); - if (it == mapNextTx.end()) - continue; - remove(*it->second.ptx, removed, true); - } - } - if (mapTx.count(hash)) + std::deque txToRemove; + txToRemove.push_back(origTx.GetHash()); + while (!txToRemove.empty()) { - removed.push_front(tx); + uint256 hash = txToRemove.front(); + txToRemove.pop_front(); + if (!mapTx.count(hash)) + continue; + const CTransaction& tx = mapTx[hash].GetTx(); + if (fRecursive) { + for (unsigned int i = 0; i < tx.vout.size(); i++) { + std::map::iterator it = mapNextTx.find(COutPoint(hash, i)); + if (it == mapNextTx.end()) + continue; + txToRemove.push_back(it->second.ptx->GetHash()); + } + } BOOST_FOREACH(const CTxIn& txin, tx.vin) mapNextTx.erase(txin.prevout); + removed.push_back(tx); totalTxSize -= mapTx[hash].GetTxSize(); mapTx.erase(hash); nTransactionsUpdated++; -- cgit v1.2.3