From bf663f8e93454fdc7c77e298586cefbb4488ee4c Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 29 Nov 2016 17:15:12 -0500 Subject: [PATCH 1/2] remove external usage of mempool conflict tracking --- src/test/mempool_tests.cpp | 2 +- src/txmempool.cpp | 4 ++-- src/txmempool.h | 2 +- src/validation.cpp | 12 +++--------- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 1faf8b6aeb..2fb56e172f 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -412,7 +412,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) /* after tx6 is mined, tx7 should move up in the sort */ std::vector vtx; vtx.push_back(MakeTransactionRef(tx6)); - pool.removeForBlock(vtx, 1, NULL, false); + pool.removeForBlock(vtx, 1); sortedOrder.erase(sortedOrder.begin()+1); sortedOrder.pop_back(); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index c035a84db5..3bea0e4fbd 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -597,7 +597,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::vector& vtx, unsigned int nBlockHeight, - std::vector* conflicts, bool fCurrentEstimate) + bool fCurrentEstimate) { LOCK(cs); std::vector entries; @@ -617,7 +617,7 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne stage.insert(it); RemoveStaged(stage, true); } - removeConflicts(*tx, conflicts); + removeConflicts(*tx); ClearPrioritisation(tx->GetHash()); } // After the txs in the new block have been removed from the mempool, update policy estimates diff --git a/src/txmempool.h b/src/txmempool.h index 23fe5a7abe..2f40981455 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -531,7 +531,7 @@ public: void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); void removeConflicts(const CTransaction &tx, std::vector* removed = NULL); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, - std::vector* conflicts = NULL, bool fCurrentEstimate = true); + bool fCurrentEstimate = true); void clear(); void _clear(); //lock free bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); diff --git a/src/validation.cpp b/src/validation.cpp index a541978c85..a6377f0315 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2153,11 +2153,10 @@ static int64_t nTimeChainState = 0; static int64_t nTimePostConnect = 0; /** - * Used to track conflicted transactions removed from mempool and transactions - * applied to the UTXO state as a part of a single ActivateBestChainStep call. + * Used to track blocks whose transactions were applied to the UTXO state as a + * part of a single ActivateBestChainStep call. */ struct ConnectTrace { - std::vector txConflicted; std::vector > > blocksConnected; }; @@ -2208,7 +2207,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4; LogPrint("bench", " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); // Remove conflicting transactions from the mempool.; - mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, &connectTrace.txConflicted, !IsInitialBlockDownload()); + mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, !IsInitialBlockDownload()); // Update chainActive & related variables. UpdateTip(pindexNew, chainparams); @@ -2433,11 +2432,6 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // throw all transactions though the signal-interface // while _not_ holding the cs_main lock - for (const auto& tx : connectTrace.txConflicted) - { - GetMainSignals().SyncTransaction(*tx, pindexNewTip, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); - } - // ... and about transactions that got confirmed: for (const auto& pair : connectTrace.blocksConnected) { assert(pair.second); const CBlock& block = *(pair.second); From a874ab5ccf7839edb445830f81591fa608d85fa6 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 29 Nov 2016 17:51:26 -0500 Subject: [PATCH 2/2] remove internal tracking of mempool conflicts for reporting to wallet --- src/test/blockencodings_tests.cpp | 6 ++--- src/test/mempool_tests.cpp | 40 ++++++++++++++++--------------- src/txmempool.cpp | 11 +++------ src/txmempool.h | 4 ++-- 4 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index b013cda6d7..7478758f73 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -80,9 +80,9 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); - std::vector removed; - pool.removeRecursive(*block.vtx[2], &removed); - BOOST_CHECK_EQUAL(removed.size(), 1); + size_t poolSize = pool.size(); + pool.removeRecursive(*block.vtx[2]); + BOOST_CHECK_EQUAL(pool.size(), poolSize - 1); CBlock block2; std::vector vtx_missing; diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 2fb56e172f..bdd6eaf538 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -55,17 +55,17 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) CTxMemPool testPool(CFeeRate(0)); - std::vector removed; // Nothing in pool, remove should do nothing: - testPool.removeRecursive(txParent, &removed); - BOOST_CHECK_EQUAL(removed.size(), 0); + unsigned int poolSize = testPool.size(); + testPool.removeRecursive(txParent); + BOOST_CHECK_EQUAL(testPool.size(), poolSize); // Just the parent: testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); - testPool.removeRecursive(txParent, &removed); - BOOST_CHECK_EQUAL(removed.size(), 1); - removed.clear(); + poolSize = testPool.size(); + testPool.removeRecursive(txParent); + BOOST_CHECK_EQUAL(testPool.size(), poolSize - 1); // Parent, children, grandchildren: testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); @@ -75,19 +75,21 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); } // Remove Child[0], GrandChild[0] should be removed: - testPool.removeRecursive(txChild[0], &removed); - BOOST_CHECK_EQUAL(removed.size(), 2); - removed.clear(); + poolSize = testPool.size(); + testPool.removeRecursive(txChild[0]); + BOOST_CHECK_EQUAL(testPool.size(), poolSize - 2); // ... make sure grandchild and child are gone: - testPool.removeRecursive(txGrandChild[0], &removed); - BOOST_CHECK_EQUAL(removed.size(), 0); - testPool.removeRecursive(txChild[0], &removed); - BOOST_CHECK_EQUAL(removed.size(), 0); + poolSize = testPool.size(); + testPool.removeRecursive(txGrandChild[0]); + BOOST_CHECK_EQUAL(testPool.size(), poolSize); + poolSize = testPool.size(); + testPool.removeRecursive(txChild[0]); + BOOST_CHECK_EQUAL(testPool.size(), poolSize); // Remove parent, all children/grandchildren should go: - testPool.removeRecursive(txParent, &removed); - BOOST_CHECK_EQUAL(removed.size(), 5); + poolSize = testPool.size(); + testPool.removeRecursive(txParent); + BOOST_CHECK_EQUAL(testPool.size(), poolSize - 5); BOOST_CHECK_EQUAL(testPool.size(), 0); - removed.clear(); // Add children and grandchildren, but NOT the parent (simulate the parent being in a block) for (int i = 0; i < 3; i++) @@ -97,10 +99,10 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) } // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be // put into the mempool (maybe because it is non-standard): - testPool.removeRecursive(txParent, &removed); - BOOST_CHECK_EQUAL(removed.size(), 6); + poolSize = testPool.size(); + testPool.removeRecursive(txParent); + BOOST_CHECK_EQUAL(testPool.size(), poolSize - 6); BOOST_CHECK_EQUAL(testPool.size(), 0); - removed.clear(); } template diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 3bea0e4fbd..4334ebde6d 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -503,7 +503,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants } } -void CTxMemPool::removeRecursive(const CTransaction &origTx, std::vector* removed) +void CTxMemPool::removeRecursive(const CTransaction &origTx) { // Remove transaction from memory pool { @@ -530,11 +530,6 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, std::vectoremplace_back(it->GetSharedTx()); - } - } RemoveStaged(setAllRemoves, false); } } @@ -576,7 +571,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem RemoveStaged(setAllRemoves, false); } -void CTxMemPool::removeConflicts(const CTransaction &tx, std::vector* removed) +void CTxMemPool::removeConflicts(const CTransaction &tx) { // Remove transactions which depend on inputs of tx, recursively LOCK(cs); @@ -586,7 +581,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::vectorsecond; if (txConflict != tx) { - removeRecursive(txConflict, removed); + removeRecursive(txConflict); ClearPrioritisation(txConflict.GetHash()); } } diff --git a/src/txmempool.h b/src/txmempool.h index 2f40981455..8a935391de 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -527,9 +527,9 @@ public: bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate = true); bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true); - void removeRecursive(const CTransaction &tx, std::vector* removed = NULL); + void removeRecursive(const CTransaction &tx); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); - void removeConflicts(const CTransaction &tx, std::vector* removed = NULL); + void removeConflicts(const CTransaction &tx); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, bool fCurrentEstimate = true); void clear();