From 03574b956a274207ba90591781e0914609225136 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 21 Mar 2022 21:22:19 -0400 Subject: [PATCH 1/6] tree-wide: clang-format CTxMemPool references [META] Do this so that we can more easily grep for all actual instances of CTxMemPool construction. --- src/net_processing.cpp | 2 +- src/test/mempool_tests.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 502bbb5d99e..eb4903f597b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4628,7 +4628,7 @@ void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::mi namespace { class CompareInvMempoolOrder { - CTxMemPool *mp; + CTxMemPool* mp; bool m_wtxid_relay; public: explicit CompareInvMempoolOrder(CTxMemPool *_mempool, bool use_wtxid) diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 89424a0cd2a..5f7c4470fd1 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -108,12 +108,12 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) BOOST_CHECK_EQUAL(testPool.size(), 0U); } -template -static void CheckSort(CTxMemPool &pool, std::vector &sortedOrder) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) +template +static void CheckSort(CTxMemPool& pool, std::vector& sortedOrder) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) { BOOST_CHECK_EQUAL(pool.size(), sortedOrder.size()); typename CTxMemPool::indexed_transaction_set::index::type::iterator it = pool.mapTx.get().begin(); - int count=0; + int count = 0; for (; it != pool.mapTx.get().end(); ++it, ++count) { BOOST_CHECK_EQUAL(it->GetTx().GetHash().ToString(), sortedOrder[count]); } From 319f0ceeeb25f28e027fc41be2755092dc5365b4 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 23 May 2022 16:19:44 -0400 Subject: [PATCH 2/6] rest/getutxos: Don't construct empty mempool ...just don't try to consult it at all when fCheckMemPool is false --- src/rest.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index 1b90baaf95e..43c248b03b4 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -799,10 +799,10 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: if (!maybe_chainman) return false; ChainstateManager& chainman = *maybe_chainman; { - auto process_utxos = [&vOutPoints, &outs, &hits](const CCoinsView& view, const CTxMemPool& mempool) { + auto process_utxos = [&vOutPoints, &outs, &hits](const CCoinsView& view, const CTxMemPool* mempool) { for (const COutPoint& vOutPoint : vOutPoints) { Coin coin; - bool hit = !mempool.isSpent(vOutPoint) && view.GetCoin(vOutPoint, coin); + bool hit = (!mempool || !mempool->isSpent(vOutPoint)) && view.GetCoin(vOutPoint, coin); hits.push_back(hit); if (hit) outs.emplace_back(std::move(coin)); } @@ -815,10 +815,10 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: LOCK2(cs_main, mempool->cs); CCoinsViewCache& viewChain = chainman.ActiveChainstate().CoinsTip(); CCoinsViewMemPool viewMempool(&viewChain, *mempool); - process_utxos(viewMempool, *mempool); + process_utxos(viewMempool, mempool); } else { - LOCK(cs_main); // no need to lock mempool! - process_utxos(chainman.ActiveChainstate().CoinsTip(), CTxMemPool()); + LOCK(cs_main); + process_utxos(chainman.ActiveChainstate().CoinsTip(), nullptr); } for (size_t i = 0; i < hits.size(); ++i) { From 213457e170ce41a0b26c644aa010111df36414a6 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 21 Mar 2022 21:56:03 -0400 Subject: [PATCH 3/6] test/policyestimator: Use ChainTestingSetup's CTxMemPool --- src/test/policyestimator_tests.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 06877898a4d..3f66a8fc46c 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -12,12 +12,12 @@ #include -BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, BasicTestingSetup) +BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, ChainTestingSetup) BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) { - CBlockPolicyEstimator feeEst; - CTxMemPool mpool(&feeEst); + CBlockPolicyEstimator& feeEst = *Assert(m_node.fee_estimator); + CTxMemPool& mpool = *Assert(m_node.mempool); LOCK2(cs_main, mpool.cs); TestMemPoolEntryHelper entry; CAmount basefee(2000); From 86e732def3983f99ec84b59375615bedcdc0e664 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 21 Mar 2022 21:42:40 -0400 Subject: [PATCH 4/6] scripted-diff: test: Use CTxMemPool in TestingSetup After this commit, there should be no explicit instantiation of CTxMemPool in src/test other than those in fuzz/ and setup_common -BEGIN VERIFY SCRIPT- find_regex="CTxMemPool\s+([^;({]+)(|\(\)|\{\});" \ && git grep -l -E "$find_regex" -- src/test \ | grep -v -e "^src/test/util/setup_common.cpp$" \ -e "^src/test/fuzz/" \ | xargs sed -i -E "s@$find_regex@CTxMemPool\& \1 = *Assert(m_node.mempool);@g" -END VERIFY SCRIPT- --- src/test/blockencodings_tests.cpp | 8 ++++---- src/test/mempool_tests.cpp | 12 ++++++------ src/test/validation_chainstate_tests.cpp | 2 +- src/test/validation_flush_tests.cpp | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 875241094dc..78b82b9b202 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -54,7 +54,7 @@ constexpr long SHARED_TX_OFFSET{3}; BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) { - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); TestMemPoolEntryHelper entry; CBlock block(BuildBlockTestCase()); @@ -137,7 +137,7 @@ public: BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) { - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); TestMemPoolEntryHelper entry; CBlock block(BuildBlockTestCase()); @@ -207,7 +207,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) { - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); TestMemPoolEntryHelper entry; CBlock block(BuildBlockTestCase()); @@ -258,7 +258,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) { - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); CMutableTransaction coinbase; coinbase.vin.resize(1); coinbase.vin[0].scriptSig.resize(10); diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 5f7c4470fd1..bc631220251 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -56,7 +56,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) } - CTxMemPool testPool; + CTxMemPool& testPool = *Assert(m_node.mempool); LOCK2(cs_main, testPool.cs); // Nothing in pool, remove should do nothing: @@ -121,7 +121,7 @@ static void CheckSort(CTxMemPool& pool, std::vector& sortedOrder) E BOOST_AUTO_TEST_CASE(MempoolIndexingTest) { - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); LOCK2(cs_main, pool.cs); TestMemPoolEntryHelper entry; @@ -294,7 +294,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) { - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); LOCK2(cs_main, pool.cs); TestMemPoolEntryHelper entry; @@ -423,7 +423,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) { - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); LOCK2(cs_main, pool.cs); TestMemPoolEntryHelper entry; @@ -594,7 +594,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) { size_t ancestors, descendants; - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); LOCK2(cs_main, pool.cs); TestMemPoolEntryHelper entry; @@ -753,7 +753,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTestsDiamond) { size_t ancestors, descendants; - CTxMemPool pool; + CTxMemPool& pool = *Assert(m_node.mempool); LOCK2(::cs_main, pool.cs); TestMemPoolEntryHelper entry; diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 98cb713a815..102de74389f 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -30,7 +30,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) ChainstateManager manager{chainman_opts}; WITH_LOCK(::cs_main, manager.m_blockman.m_block_tree_db = std::make_unique(1 << 20, true)); - CTxMemPool mempool; + CTxMemPool& mempool = *Assert(m_node.mempool); //! Create and add a Coin with DynamicMemoryUsage of 80 bytes to the given view. auto add_coin = [](CCoinsViewCache& coins_view) -> COutPoint { diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index a34895d4aed..012169b17e2 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -20,7 +20,7 @@ BOOST_FIXTURE_TEST_SUITE(validation_flush_tests, ChainTestingSetup) //! BOOST_AUTO_TEST_CASE(getcoinscachesizestate) { - CTxMemPool mempool; + CTxMemPool& mempool = *Assert(m_node.mempool); BlockManager blockman{}; CChainState chainstate{&mempool, blockman, *Assert(m_node.chainman)}; chainstate.InitCoinsDB(/*cache_size_bytes=*/1 << 10, /*in_memory=*/true, /*should_wipe=*/false); From 020caba3df727ae8ede50eace86ae76971c0fea1 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 21 Mar 2022 22:01:51 -0400 Subject: [PATCH 5/6] bench: Use existing CTxMemPool in TestingSetup --- src/bench/mempool_eviction.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index e80b9e1ac2b..60d991fab9a 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -108,7 +108,7 @@ static void MempoolEviction(benchmark::Bench& bench) tx7.vout[1].scriptPubKey = CScript() << OP_7 << OP_EQUAL; tx7.vout[1].nValue = 10 * COIN; - CTxMemPool pool; + CTxMemPool& pool = *Assert(testing_setup->m_node.mempool); LOCK2(cs_main, pool.cs); // Create transaction references outside the "hot loop" const CTransactionRef tx1_r{MakeTransactionRef(tx1)}; From d273e53b6e2cabd91a83f0ff0f9b6cfe1815b637 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 22 Mar 2022 15:03:13 -0400 Subject: [PATCH 6/6] bench/rpc_mempool: Create ChainTestingSetup, use its CTxMemPool This is correct because: - The ChainTestingSetup is constructed before the call to bench.run(...) - All the runs are performed on the same mempool --- src/bench/rpc_mempool.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp index 6e322ba6aab..0e6fdae3d70 100644 --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -3,7 +3,9 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include +#include #include #include @@ -17,7 +19,8 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& poo static void RpcMempool(benchmark::Bench& bench) { - CTxMemPool pool; + const auto testing_setup = MakeNoLogFileContext(CBaseChainParams::MAIN); + CTxMemPool& pool = *Assert(testing_setup->m_node.mempool); LOCK2(cs_main, pool.cs); for (int i = 0; i < 1000; ++i) {