From ec4525ccc1c1b66bc99f064348a9842e163c1324 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 15 Oct 2016 17:34:02 -0400 Subject: [PATCH 1/3] Move orphan processing to ActivateBestChain This further decouples "main" and "net" processing logic by moving orphan processing out of the chain-connecting cs_main lock and into its own cs_main lock, beside all of the other chain callbacks. Once further decoupling of net and main processing logic occurs, orphan handing should move to its own lock, out of cs_main. Note that this will introduce a race if there are any cases where we assume the orphan map to be consistent with the current chain tip, however I am confident there is no such case (ATMP will fail without DoS score in all such cases). --- src/main.cpp | 47 +++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 263421aea..d587b39e6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2461,7 +2461,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin CCheckQueueControl control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL); - std::vector vOrphanErase; std::vector prevheights; CAmount nFees = 0; int nInputs = 0; @@ -2492,17 +2491,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin prevheights[j] = view.AccessCoins(tx.vin[j].prevout.hash)->nHeight; } - // Which orphan pool entries must we evict? - for (size_t j = 0; j < tx.vin.size(); j++) { - auto itByPrev = mapOrphanTransactionsByPrev.find(tx.vin[j].prevout); - if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; - for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { - const CTransaction& orphanTx = (*mi)->second.tx; - const uint256& orphanHash = orphanTx.GetHash(); - vOrphanErase.push_back(orphanHash); - } - } - if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { return state.DoS(100, error("%s: contains a non-BIP68-final transaction", __func__), REJECT_INVALID, "bad-txns-nonfinal"); @@ -2592,14 +2580,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin GetMainSignals().UpdatedTransaction(hashPrevBestCoinBase); hashPrevBestCoinBase = block.vtx[0].GetHash(); - // Erase orphan transactions include or precluded by this block - if (vOrphanErase.size()) { - int nErased = 0; - BOOST_FOREACH(uint256 &orphanHash, vOrphanErase) { - nErased += EraseOrphanTx(orphanHash); - } - LogPrint("mempool", "Erased %d orphan tx included or conflicted by block\n", nErased); - } int64_t nTime6 = GetTimeMicros(); nTimeCallbacks += nTime6 - nTime5; LogPrint("bench", " - Callbacks: %.2fms [%.2fs]\n", 0.001 * (nTime6 - nTime5), nTimeCallbacks * 0.000001); @@ -3105,6 +3085,33 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } // When we reach this point, we switched to a new tip (stored in pindexNewTip). + // Remove orphan transactions with cs_main + { + LOCK(cs_main); + std::vector vOrphanErase; + for(unsigned int i = 0; i < txChanged.size(); i++) { + const CTransaction& tx = std::get<0>(txChanged[i]); + // Which orphan pool entries must we evict? + for (size_t j = 0; j < tx.vin.size(); j++) { + auto itByPrev = mapOrphanTransactionsByPrev.find(tx.vin[j].prevout); + if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; + for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { + const CTransaction& orphanTx = (*mi)->second.tx; + const uint256& orphanHash = orphanTx.GetHash(); + vOrphanErase.push_back(orphanHash); + } + } + } + // Erase orphan transactions include or precluded by this block + if (vOrphanErase.size()) { + int nErased = 0; + BOOST_FOREACH(uint256 &orphanHash, vOrphanErase) { + nErased += EraseOrphanTx(orphanHash); + } + LogPrint("mempool", "Erased %d orphan tx included or conflicted by block\n", nErased); + } + } + // Notifications/callbacks that can run without cs_main // throw all transactions though the signal-interface From 97e28029c94bc187721242a8c59468cebd73441b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 3 Nov 2016 14:53:12 -0400 Subject: [PATCH 2/3] Erase orphans per-transaction instead of per-block --- src/main.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index d587b39e6..a5b008136 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3088,8 +3088,8 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // Remove orphan transactions with cs_main { LOCK(cs_main); - std::vector vOrphanErase; for(unsigned int i = 0; i < txChanged.size(); i++) { + std::vector vOrphanErase; const CTransaction& tx = std::get<0>(txChanged[i]); // Which orphan pool entries must we evict? for (size_t j = 0; j < tx.vin.size(); j++) { @@ -3101,14 +3101,15 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, vOrphanErase.push_back(orphanHash); } } - } - // Erase orphan transactions include or precluded by this block - if (vOrphanErase.size()) { - int nErased = 0; - BOOST_FOREACH(uint256 &orphanHash, vOrphanErase) { - nErased += EraseOrphanTx(orphanHash); + + // Erase orphan transactions include or precluded by this block + if (vOrphanErase.size()) { + int nErased = 0; + BOOST_FOREACH(uint256 &orphanHash, vOrphanErase) { + nErased += EraseOrphanTx(orphanHash); + } + LogPrint("mempool", "Erased %d orphan tx included or conflicted by block\n", nErased); } - LogPrint("mempool", "Erased %d orphan tx included or conflicted by block\n", nErased); } } From d2b88f97a1235057290f9e8ca0bf11437ba919f8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 3 Nov 2016 14:58:29 -0400 Subject: [PATCH 3/3] Move orphan-conflict removal from main logic into a callback This makes the orphan map a part of net-processing logic instead of main logic. --- src/main.cpp | 56 ++++++++++++++++++++++++++-------------------------- src/main.h | 1 + 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a5b008136..025449ad6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3085,34 +3085,6 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } // When we reach this point, we switched to a new tip (stored in pindexNewTip). - // Remove orphan transactions with cs_main - { - LOCK(cs_main); - for(unsigned int i = 0; i < txChanged.size(); i++) { - std::vector vOrphanErase; - const CTransaction& tx = std::get<0>(txChanged[i]); - // Which orphan pool entries must we evict? - for (size_t j = 0; j < tx.vin.size(); j++) { - auto itByPrev = mapOrphanTransactionsByPrev.find(tx.vin[j].prevout); - if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; - for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { - const CTransaction& orphanTx = (*mi)->second.tx; - const uint256& orphanHash = orphanTx.GetHash(); - vOrphanErase.push_back(orphanHash); - } - } - - // Erase orphan transactions include or precluded by this block - if (vOrphanErase.size()) { - int nErased = 0; - BOOST_FOREACH(uint256 &orphanHash, vOrphanErase) { - nErased += EraseOrphanTx(orphanHash); - } - LogPrint("mempool", "Erased %d orphan tx included or conflicted by block\n", nErased); - } - } - } - // Notifications/callbacks that can run without cs_main // throw all transactions though the signal-interface @@ -4752,6 +4724,34 @@ PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn) : connman(connmanI recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); } +void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int nPosInBlock) { + if (nPosInBlock == CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK) + return; + + LOCK(cs_main); + + std::vector vOrphanErase; + // Which orphan pool entries must we evict? + for (size_t j = 0; j < tx.vin.size(); j++) { + auto itByPrev = mapOrphanTransactionsByPrev.find(tx.vin[j].prevout); + if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; + for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { + const CTransaction& orphanTx = (*mi)->second.tx; + const uint256& orphanHash = orphanTx.GetHash(); + vOrphanErase.push_back(orphanHash); + } + } + + // Erase orphan transactions include or precluded by this block + if (vOrphanErase.size()) { + int nErased = 0; + BOOST_FOREACH(uint256 &orphanHash, vOrphanErase) { + nErased += EraseOrphanTx(orphanHash); + } + LogPrint("mempool", "Erased %d orphan tx included or conflicted by block\n", nErased); + } +} + void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { const int nNewHeight = pindexNew->nHeight; connman->SetBestHeight(nNewHeight); diff --git a/src/main.h b/src/main.h index 678f0fd99..6d4240c26 100644 --- a/src/main.h +++ b/src/main.h @@ -560,6 +560,7 @@ private: public: PeerLogicValidation(CConnman* connmanIn); + virtual void SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int nPosInBlock); virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload); virtual void BlockChecked(const CBlock& block, const CValidationState& state); };