From 6445aa7d97551ec5d501d91f6829071c67169122 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 21 Sep 2023 14:45:20 -0400 Subject: [PATCH] Remove the ancestor and descendant indices from the mempool --- src/test/mempool_tests.cpp | 316 ------------------------------------- src/txmempool.cpp | 4 +- src/txmempool.h | 77 --------- 3 files changed, 2 insertions(+), 395 deletions(-) diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index ec3bbd158fe..2271e7b88db 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -115,322 +115,6 @@ 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) -{ - BOOST_CHECK_EQUAL(pool.size(), sortedOrder.size()); - typename CTxMemPool::indexed_transaction_set::index::type::iterator it = pool.mapTx.get().begin(); - int count = 0; - for (; it != pool.mapTx.get().end(); ++it, ++count) { - BOOST_CHECK_EQUAL(it->GetTx().GetHash().ToString(), sortedOrder[count]); - } -} - -BOOST_AUTO_TEST_CASE(MempoolIndexingTest) -{ - CTxMemPool& pool = *Assert(m_node.mempool); - LOCK2(cs_main, pool.cs); - TestMemPoolEntryHelper entry; - - /* 3rd highest fee */ - CMutableTransaction tx1 = CMutableTransaction(); - tx1.vout.resize(1); - tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx1.vout[0].nValue = 10 * COIN; - AddToMempool(pool, entry.Fee(10000LL).FromTx(tx1)); - - /* highest fee */ - CMutableTransaction tx2 = CMutableTransaction(); - tx2.vout.resize(1); - tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx2.vout[0].nValue = 2 * COIN; - AddToMempool(pool, entry.Fee(20000LL).FromTx(tx2)); - - /* lowest fee */ - CMutableTransaction tx3 = CMutableTransaction(); - tx3.vout.resize(1); - tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx3.vout[0].nValue = 5 * COIN; - AddToMempool(pool, entry.Fee(0LL).FromTx(tx3)); - - /* 2nd highest fee */ - CMutableTransaction tx4 = CMutableTransaction(); - tx4.vout.resize(1); - tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx4.vout[0].nValue = 6 * COIN; - AddToMempool(pool, entry.Fee(15000LL).FromTx(tx4)); - - /* equal fee rate to tx1, but newer */ - CMutableTransaction tx5 = CMutableTransaction(); - tx5.vout.resize(1); - tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx5.vout[0].nValue = 11 * COIN; - entry.time = NodeSeconds{1s}; - AddToMempool(pool, entry.Fee(10000LL).FromTx(tx5)); - BOOST_CHECK_EQUAL(pool.size(), 5U); - - std::vector sortedOrder; - sortedOrder.resize(5); - sortedOrder[0] = tx3.GetHash().ToString(); // 0 - sortedOrder[1] = tx5.GetHash().ToString(); // 10000 - sortedOrder[2] = tx1.GetHash().ToString(); // 10000 - sortedOrder[3] = tx4.GetHash().ToString(); // 15000 - sortedOrder[4] = tx2.GetHash().ToString(); // 20000 - CheckSort(pool, sortedOrder); - - /* low fee but with high fee child */ - /* tx6 -> tx7 -> tx8, tx9 -> tx10 */ - CMutableTransaction tx6 = CMutableTransaction(); - tx6.vout.resize(1); - tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx6.vout[0].nValue = 20 * COIN; - AddToMempool(pool, entry.Fee(0LL).FromTx(tx6)); - BOOST_CHECK_EQUAL(pool.size(), 6U); - // Check that at this point, tx6 is sorted low - sortedOrder.insert(sortedOrder.begin(), tx6.GetHash().ToString()); - CheckSort(pool, sortedOrder); - - CTxMemPool::setEntries setAncestors; - setAncestors.insert(pool.GetIter(tx6.GetHash()).value()); - CMutableTransaction tx7 = CMutableTransaction(); - tx7.vin.resize(1); - tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0); - tx7.vin[0].scriptSig = CScript() << OP_11; - tx7.vout.resize(2); - tx7.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx7.vout[0].nValue = 10 * COIN; - tx7.vout[1].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx7.vout[1].nValue = 1 * COIN; - - { - auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), CTxMemPool::Limits::NoLimits())}; - BOOST_REQUIRE(ancestors_calculated.has_value()); - BOOST_CHECK(*ancestors_calculated == setAncestors); - } - - AddToMempool(pool, entry.FromTx(tx7)); - BOOST_CHECK_EQUAL(pool.size(), 7U); - - // Now tx6 should be sorted higher (high fee child): tx7, tx6, tx2, ... - sortedOrder.erase(sortedOrder.begin()); - sortedOrder.push_back(tx6.GetHash().ToString()); - sortedOrder.push_back(tx7.GetHash().ToString()); - CheckSort(pool, sortedOrder); - - /* low fee child of tx7 */ - CMutableTransaction tx8 = CMutableTransaction(); - tx8.vin.resize(1); - tx8.vin[0].prevout = COutPoint(tx7.GetHash(), 0); - tx8.vin[0].scriptSig = CScript() << OP_11; - tx8.vout.resize(1); - tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx8.vout[0].nValue = 10 * COIN; - setAncestors.insert(pool.GetIter(tx7.GetHash()).value()); - AddToMempool(pool, entry.Fee(0LL).Time(NodeSeconds{2s}).FromTx(tx8)); - - // Now tx8 should be sorted low, but tx6/tx both high - sortedOrder.insert(sortedOrder.begin(), tx8.GetHash().ToString()); - CheckSort(pool, sortedOrder); - - /* low fee child of tx7 */ - CMutableTransaction tx9 = CMutableTransaction(); - tx9.vin.resize(1); - tx9.vin[0].prevout = COutPoint(tx7.GetHash(), 1); - tx9.vin[0].scriptSig = CScript() << OP_11; - tx9.vout.resize(1); - tx9.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx9.vout[0].nValue = 1 * COIN; - AddToMempool(pool, entry.Fee(0LL).Time(NodeSeconds{3s}).FromTx(tx9)); - - // tx9 should be sorted low - BOOST_CHECK_EQUAL(pool.size(), 9U); - sortedOrder.insert(sortedOrder.begin(), tx9.GetHash().ToString()); - CheckSort(pool, sortedOrder); - - std::vector snapshotOrder = sortedOrder; - - setAncestors.insert(pool.GetIter(tx8.GetHash()).value()); - setAncestors.insert(pool.GetIter(tx9.GetHash()).value()); - /* tx10 depends on tx8 and tx9 and has a high fee*/ - CMutableTransaction tx10 = CMutableTransaction(); - tx10.vin.resize(2); - tx10.vin[0].prevout = COutPoint(tx8.GetHash(), 0); - tx10.vin[0].scriptSig = CScript() << OP_11; - tx10.vin[1].prevout = COutPoint(tx9.GetHash(), 0); - tx10.vin[1].scriptSig = CScript() << OP_11; - tx10.vout.resize(1); - tx10.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx10.vout[0].nValue = 10 * COIN; - - { - auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(NodeSeconds{4s}).FromTx(tx10), CTxMemPool::Limits::NoLimits())}; - BOOST_REQUIRE(ancestors_calculated); - BOOST_CHECK(*ancestors_calculated == setAncestors); - } - - AddToMempool(pool, entry.FromTx(tx10)); - - /** - * tx8 and tx9 should both now be sorted higher - * Final order after tx10 is added: - * - * tx3 = 0 (1) - * tx5 = 10000 (1) - * tx1 = 10000 (1) - * tx4 = 15000 (1) - * tx2 = 20000 (1) - * tx9 = 200k (2 txs) - * tx8 = 200k (2 txs) - * tx10 = 200k (1 tx) - * tx6 = 2.2M (5 txs) - * tx7 = 2.2M (4 txs) - */ - sortedOrder.erase(sortedOrder.begin(), sortedOrder.begin()+2); // take out tx9, tx8 from the beginning - sortedOrder.insert(sortedOrder.begin()+5, tx9.GetHash().ToString()); - sortedOrder.insert(sortedOrder.begin()+6, tx8.GetHash().ToString()); - sortedOrder.insert(sortedOrder.begin()+7, tx10.GetHash().ToString()); // tx10 is just before tx6 - CheckSort(pool, sortedOrder); - - // there should be 10 transactions in the mempool - BOOST_CHECK_EQUAL(pool.size(), 10U); - - // Now try removing tx10 and verify the sort order returns to normal - pool.removeRecursive(*Assert(pool.get(tx10.GetHash())), REMOVAL_REASON_DUMMY); - CheckSort(pool, snapshotOrder); - - pool.removeRecursive(*Assert(pool.get(tx9.GetHash())), REMOVAL_REASON_DUMMY); - pool.removeRecursive(*Assert(pool.get(tx8.GetHash())), REMOVAL_REASON_DUMMY); -} - -BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) -{ - CTxMemPool& pool = *Assert(m_node.mempool); - LOCK2(cs_main, pool.cs); - TestMemPoolEntryHelper entry; - - /* 3rd highest fee */ - CMutableTransaction tx1 = CMutableTransaction(); - tx1.vout.resize(1); - tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx1.vout[0].nValue = 10 * COIN; - AddToMempool(pool, entry.Fee(10000LL).FromTx(tx1)); - - /* highest fee */ - CMutableTransaction tx2 = CMutableTransaction(); - tx2.vout.resize(1); - tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx2.vout[0].nValue = 2 * COIN; - AddToMempool(pool, entry.Fee(20000LL).FromTx(tx2)); - uint64_t tx2Size = GetVirtualTransactionSize(CTransaction(tx2)); - - /* lowest fee */ - CMutableTransaction tx3 = CMutableTransaction(); - tx3.vout.resize(1); - tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx3.vout[0].nValue = 5 * COIN; - AddToMempool(pool, entry.Fee(0LL).FromTx(tx3)); - - /* 2nd highest fee */ - CMutableTransaction tx4 = CMutableTransaction(); - tx4.vout.resize(1); - tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx4.vout[0].nValue = 6 * COIN; - AddToMempool(pool, entry.Fee(15000LL).FromTx(tx4)); - - /* equal fee rate to tx1, but newer */ - CMutableTransaction tx5 = CMutableTransaction(); - tx5.vout.resize(1); - tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx5.vout[0].nValue = 11 * COIN; - AddToMempool(pool, entry.Fee(10000LL).FromTx(tx5)); - BOOST_CHECK_EQUAL(pool.size(), 5U); - - std::vector sortedOrder; - sortedOrder.resize(5); - sortedOrder[0] = tx2.GetHash().ToString(); // 20000 - sortedOrder[1] = tx4.GetHash().ToString(); // 15000 - // tx1 and tx5 are both 10000 - // Ties are broken by hash, not timestamp, so determine which - // hash comes first. - if (tx1.GetHash() < tx5.GetHash()) { - sortedOrder[2] = tx1.GetHash().ToString(); - sortedOrder[3] = tx5.GetHash().ToString(); - } else { - sortedOrder[2] = tx5.GetHash().ToString(); - sortedOrder[3] = tx1.GetHash().ToString(); - } - sortedOrder[4] = tx3.GetHash().ToString(); // 0 - - CheckSort(pool, sortedOrder); - - /* low fee parent with high fee child */ - /* tx6 (0) -> tx7 (high) */ - CMutableTransaction tx6 = CMutableTransaction(); - tx6.vout.resize(1); - tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx6.vout[0].nValue = 20 * COIN; - uint64_t tx6Size = GetVirtualTransactionSize(CTransaction(tx6)); - - AddToMempool(pool, entry.Fee(0LL).FromTx(tx6)); - BOOST_CHECK_EQUAL(pool.size(), 6U); - // Ties are broken by hash - if (tx3.GetHash() < tx6.GetHash()) - sortedOrder.push_back(tx6.GetHash().ToString()); - else - sortedOrder.insert(sortedOrder.end()-1,tx6.GetHash().ToString()); - - CheckSort(pool, sortedOrder); - - CMutableTransaction tx7 = CMutableTransaction(); - tx7.vin.resize(1); - tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0); - tx7.vin[0].scriptSig = CScript() << OP_11; - tx7.vout.resize(1); - tx7.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx7.vout[0].nValue = 10 * COIN; - uint64_t tx7Size = GetVirtualTransactionSize(CTransaction(tx7)); - - /* set the fee to just below tx2's feerate when including ancestor */ - CAmount fee = (20000/tx2Size)*(tx7Size + tx6Size) - 1; - - AddToMempool(pool, entry.Fee(fee).FromTx(tx7)); - BOOST_CHECK_EQUAL(pool.size(), 7U); - sortedOrder.insert(sortedOrder.begin()+1, tx7.GetHash().ToString()); - CheckSort(pool, sortedOrder); - - /* after tx6 is mined, tx7 should move up in the sort */ - std::vector vtx; - vtx.push_back(MakeTransactionRef(tx6)); - pool.removeForBlock(vtx, 1); - - sortedOrder.erase(sortedOrder.begin()+1); - // Ties are broken by hash - if (tx3.GetHash() < tx6.GetHash()) - sortedOrder.pop_back(); - else - sortedOrder.erase(sortedOrder.end()-2); - sortedOrder.insert(sortedOrder.begin(), tx7.GetHash().ToString()); - CheckSort(pool, sortedOrder); - - // High-fee parent, low-fee child - // tx7 -> tx8 - CMutableTransaction tx8 = CMutableTransaction(); - tx8.vin.resize(1); - tx8.vin[0].prevout = COutPoint(tx7.GetHash(), 0); - tx8.vin[0].scriptSig = CScript() << OP_11; - tx8.vout.resize(1); - tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; - tx8.vout[0].nValue = 10*COIN; - - // Check that we sort by min(feerate, ancestor_feerate): - // set the fee so that the ancestor feerate is above tx1/5, - // but the transaction's own feerate is lower - AddToMempool(pool, entry.Fee(5000LL).FromTx(tx8)); - sortedOrder.insert(sortedOrder.end()-1, tx8.GetHash().ToString()); - CheckSort(pool, sortedOrder); -} - - BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) { auto& pool = static_cast(*Assert(m_node.mempool)); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 15b2508119c..f2a63c26d26 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1057,8 +1057,8 @@ void CCoinsViewMemPool::Reset() size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); - // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. - return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(txns_randomized) + m_txgraph->GetMainMemoryUsage() + cachedInnerUsage; + // Estimate the overhead of mapTx to be 9 pointers (3 pointers per index) + an allocation, as no exact formula for boost::multi_index_contained is implemented. + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 9 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(txns_randomized) + m_txgraph->GetMainMemoryUsage() + cachedInnerUsage; } void CTxMemPool::RemoveUnbroadcastTx(const Txid& txid, const bool unchecked) { diff --git a/src/txmempool.h b/src/txmempool.h index 1750596c0cf..5834cd0065a 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -90,37 +90,6 @@ struct mempoolentry_wtxid } }; - -/** \class CompareTxMemPoolEntryByDescendantScore - * - * Sort an entry by max(score/size of entry's tx, score/size with all descendants). - */ -class CompareTxMemPoolEntryByDescendantScore -{ -public: - bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const - { - FeeFrac f1 = GetModFeeAndSize(a); - FeeFrac f2 = GetModFeeAndSize(b); - - if (FeeRateCompare(f1, f2) == 0) { - return a.GetTime() >= b.GetTime(); - } - return f1 < f2; - } - - // Return the fee/size we're using for sorting this entry. - FeeFrac GetModFeeAndSize(const CTxMemPoolEntry &a) const - { - // Compare feerate with descendants to feerate of the transaction, and - // return the fee/size for the max. - return std::max( - FeeFrac(a.GetModFeesWithDescendants(), a.GetSizeWithDescendants()), - FeeFrac(a.GetModifiedFee(), a.GetTxSize()) - ); - } -}; - /** \class CompareTxMemPoolEntryByScore * * Sort by feerate of entry (fee/size) in descending order @@ -151,42 +120,8 @@ public: } }; -/** \class CompareTxMemPoolEntryByAncestorScore - * - * Sort an entry by min(score/size of entry's tx, score/size with all ancestors). - */ -class CompareTxMemPoolEntryByAncestorFee -{ -public: - template - bool operator()(const T& a, const T& b) const - { - FeeFrac f1 = GetModFeeAndSize(a); - FeeFrac f2 = GetModFeeAndSize(b); - - if (FeeRateCompare(f1, f2) == 0) { - return a.GetTx().GetHash() < b.GetTx().GetHash(); - } - return f1 > f2; - } - - // Return the fee/size we're using for sorting this entry. - template - FeeFrac GetModFeeAndSize(const T &a) const - { - // Compare feerate with ancestors to feerate of the transaction, and - // return the fee/size for the min. - return std::min( - FeeFrac(a.GetModFeesWithAncestors(), a.GetSizeWithAncestors()), - FeeFrac(a.GetModifiedFee(), a.GetTxSize()) - ); - } -}; - // Multi_index tag names -struct descendant_score {}; struct entry_time {}; -struct ancestor_score {}; struct index_by_wtxid {}; /** @@ -321,23 +256,11 @@ public: mempoolentry_wtxid, SaltedWtxidHasher >, - // sorted by fee rate - boost::multi_index::ordered_non_unique< - boost::multi_index::tag, - boost::multi_index::identity, - CompareTxMemPoolEntryByDescendantScore - >, // sorted by entry time boost::multi_index::ordered_non_unique< boost::multi_index::tag, boost::multi_index::identity, CompareTxMemPoolEntryByEntryTime - >, - // sorted by fee rate with ancestors - boost::multi_index::ordered_non_unique< - boost::multi_index::tag, - boost::multi_index::identity, - CompareTxMemPoolEntryByAncestorFee > > {};