From 51430680ecb722e1d4ee4a26dac5724050f41c9e Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 9 Jan 2025 10:28:57 -0500 Subject: [PATCH 01/65] Allow moving an Epoch::Marker --- src/util/epochguard.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/epochguard.h b/src/util/epochguard.h index 145f4dc132b..25ae3ac798f 100644 --- a/src/util/epochguard.h +++ b/src/util/epochguard.h @@ -59,8 +59,8 @@ public: public: Marker() = default; Marker(const Marker&) = default; - Marker(Marker&&) = delete; - Marker& operator=(Marker&&) = delete; + Marker(Marker&&) = default; + Marker& operator=(Marker&&) = default; ~Marker() = default; }; From 6c73e4744837a7dc138a9177df3a48f30a1ba6c1 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Wed, 5 Feb 2025 18:39:07 -0500 Subject: [PATCH 02/65] mempool: Store iterators into mapTx in mapNextTx This takes the same amount of space as CTransaction pointers, and saves a map lookup in many common uses. --- src/txmempool.cpp | 25 +++++++++++-------------- src/txmempool.h | 2 +- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index cae16a675bd..a695fbffb86 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -137,12 +137,11 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector& vHashesToU { WITH_FRESH_EPOCH(m_epoch); for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) { - const Txid &childHash = iter->second->GetHash(); - txiter childIter = mapTx.find(childHash); + txiter childIter = iter->second; assert(childIter != mapTx.end()); // We can skip updating entries we've encountered before or that // are in the block (which are already accounted for). - if (!visited(childIter) && !setAlreadyIncluded.count(childHash)) { + if (!visited(childIter) && !setAlreadyIncluded.count(iter->second->GetTx().GetHash())) { UpdateChild(it, childIter, true); UpdateParent(childIter, it, true); } @@ -486,7 +485,7 @@ void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit, CTxMemPool::setEntr const CTransaction& tx = newit->GetTx(); std::set setParentTransactions; for (unsigned int i = 0; i < tx.vin.size(); i++) { - mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, &tx)); + mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, newit)); setParentTransactions.insert(tx.vin[i].prevout.hash); } // Don't bother worrying about child transactions of this one. @@ -611,7 +610,7 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso auto it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); if (it == mapNextTx.end()) continue; - txiter nextit = mapTx.find(it->second->GetHash()); + txiter nextit = it->second; assert(nextit != mapTx.end()); txToRemove.insert(nextit); } @@ -652,7 +651,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) for (const CTxIn &txin : tx.vin) { auto it = mapNextTx.find(txin.prevout); if (it != mapNextTx.end()) { - const CTransaction &txConflict = *it->second; + const CTransaction &txConflict = it->second->GetTx(); if (Assume(txConflict.GetHash() != tx.GetHash())) { ClearPrioritisation(txConflict.GetHash()); @@ -729,7 +728,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei auto it3 = mapNextTx.find(txin.prevout); assert(it3 != mapNextTx.end()); assert(it3->first == &txin.prevout); - assert(it3->second == &tx); + assert(&it3->second->GetTx() == &tx); } auto comp = [](const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) -> bool { return a.GetTx().GetHash() == b.GetTx().GetHash(); @@ -762,7 +761,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); int32_t child_sizes{0}; for (; iter != mapNextTx.end() && iter->first->hash == it->GetTx().GetHash(); ++iter) { - txiter childit = mapTx.find(iter->second->GetHash()); + txiter childit = iter->second; assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions if (setChildrenCheck.insert(*childit).second) { child_sizes += childit->GetTxSize(); @@ -781,11 +780,9 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei for (const auto& input: tx.vin) mempoolDuplicate.SpendCoin(input.prevout); AddCoins(mempoolDuplicate, tx, std::numeric_limits::max()); } - for (const auto& [_, next_tx] : mapNextTx) { - auto it = mapTx.find(next_tx->GetHash()); - const CTransaction& tx = it->GetTx(); - assert(it != mapTx.end()); - assert(&tx == next_tx); + for (auto it = mapNextTx.cbegin(); it != mapNextTx.cend(); it++) { + indexed_transaction_set::const_iterator it2 = it->second; + assert(it2 != mapTx.end()); } assert(totalTxSize == checkTotal); @@ -956,7 +953,7 @@ std::vector CTxMemPool::GetPrioritisedTransactions() con const CTransaction* CTxMemPool::GetConflictTx(const COutPoint& prevout) const { const auto it = mapNextTx.find(prevout); - return it == mapNextTx.end() ? nullptr : it->second; + return it == mapNextTx.end() ? nullptr : &(it->second->GetTx()); } std::optional CTxMemPool::GetIter(const Txid& txid) const diff --git a/src/txmempool.h b/src/txmempool.h index a36754810f7..90da90da7db 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -413,7 +413,7 @@ private: } public: - indirectmap mapNextTx GUARDED_BY(cs); + indirectmap mapNextTx GUARDED_BY(cs); std::map mapDeltas GUARDED_BY(cs); using Options = kernel::MemPoolOptions; From 92b0079fe3863b20b71282aa82341d4b6ee4b337 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 9 Jan 2025 10:30:12 -0500 Subject: [PATCH 03/65] Allow moving CTxMemPoolEntry objects, disallow copying --- src/kernel/mempool_entry.h | 10 ++-------- src/test/fuzz/policy_estimator.cpp | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 51a1f26642f..40aa77b5769 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -71,10 +71,7 @@ public: typedef std::set Children; private: - CTxMemPoolEntry(const CTxMemPoolEntry&) = default; - struct ExplicitCopyTag { - explicit ExplicitCopyTag() = default; - }; + CTxMemPoolEntry(const CTxMemPoolEntry&) = delete; const CTransactionRef tx; mutable Parents m_parents; @@ -127,13 +124,10 @@ public: nModFeesWithAncestors{nFee}, nSigOpCostWithAncestors{sigOpCost} {} - CTxMemPoolEntry(ExplicitCopyTag, const CTxMemPoolEntry& entry) : CTxMemPoolEntry(entry) {} CTxMemPoolEntry& operator=(const CTxMemPoolEntry&) = delete; - CTxMemPoolEntry(CTxMemPoolEntry&&) = delete; + CTxMemPoolEntry(CTxMemPoolEntry&&) = default; CTxMemPoolEntry& operator=(CTxMemPoolEntry&&) = delete; - static constexpr ExplicitCopyTag ExplicitCopy{}; - const CTransaction& GetTx() const { return *this->tx; } CTransactionRef GetSharedTx() const { return this->tx; } const CAmount& GetFee() const { return nFee; } diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 8455a23284d..cfd50cf80d9 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -74,7 +74,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) break; } const CTransaction tx{*mtx}; - mempool_entries.emplace_back(CTxMemPoolEntry::ExplicitCopy, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx, current_height)); + mempool_entries.push_back(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx, current_height)); } std::vector txs; txs.reserve(mempool_entries.size()); From 29a94d5b2f26a4a8b7464894e4db944ea67241b7 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 9 Jan 2025 10:36:34 -0500 Subject: [PATCH 04/65] Make CTxMemPoolEntry derive from TxGraph::Ref --- src/bench/blockencodings.cpp | 2 +- src/bench/mempool_ephemeral_spends.cpp | 2 +- src/bench/mempool_eviction.cpp | 2 +- src/bench/mempool_stress.cpp | 2 +- src/bench/rpc_mempool.cpp | 2 +- src/kernel/CMakeLists.txt | 1 + src/kernel/mempool_entry.h | 9 ++++++--- src/node/interfaces.cpp | 2 +- src/test/fuzz/util/mempool.cpp | 2 +- src/test/util/txmempool.cpp | 2 +- src/txmempool.cpp | 2 +- 11 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/bench/blockencodings.cpp b/src/bench/blockencodings.cpp index 8f6659919c6..c92ade60d2d 100644 --- a/src/bench/blockencodings.cpp +++ b/src/bench/blockencodings.cpp @@ -22,7 +22,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - AddToMempool(pool, CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); } namespace { diff --git a/src/bench/mempool_ephemeral_spends.cpp b/src/bench/mempool_ephemeral_spends.cpp index 8f294113815..ce17650ee4b 100644 --- a/src/bench/mempool_ephemeral_spends.cpp +++ b/src/bench/mempool_ephemeral_spends.cpp @@ -29,7 +29,7 @@ static void AddTx(const CTransactionRef& tx, CTxMemPool& pool) EXCLUSIVE_LOCKS_R unsigned int sigOpCost{4}; uint64_t fee{0}; LockPoints lp; - AddToMempool(pool, CTxMemPoolEntry( + AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, fee, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index aa2e8682e93..72d2356a0b0 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -27,7 +27,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& po bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; - AddToMempool(pool, CTxMemPoolEntry( + AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, nFee, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index fbac25db5f5..5095438cd20 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -29,7 +29,7 @@ static void AddTx(const CTransactionRef& tx, CTxMemPool& pool) EXCLUSIVE_LOCKS_R bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; - AddToMempool(pool, CTxMemPoolEntry(tx, 1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); + AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, 1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } struct Available { diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp index a61c6609bd9..4ad1f0b28a0 100644 --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -22,7 +22,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - AddToMempool(pool, CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); } static void RpcMempool(benchmark::Bench& bench) diff --git a/src/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt index c45ce7819e9..9ab2d67272c 100644 --- a/src/kernel/CMakeLists.txt +++ b/src/kernel/CMakeLists.txt @@ -58,6 +58,7 @@ add_library(bitcoinkernel ../support/lockedpool.cpp ../sync.cpp ../txdb.cpp + ../txgraph.cpp ../txmempool.cpp ../uint256.cpp ../util/chaintype.cpp diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 40aa77b5769..cd86f36698b 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -62,7 +63,7 @@ struct CompareIteratorByHash { * */ -class CTxMemPoolEntry +class CTxMemPoolEntry : public TxGraph::Ref { public: typedef std::reference_wrapper CTxMemPoolEntryRef; @@ -103,11 +104,13 @@ private: int64_t nSigOpCostWithAncestors; public: - CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, + virtual ~CTxMemPoolEntry() = default; + CTxMemPoolEntry(TxGraph::Ref&& ref, const CTransactionRef& tx, CAmount fee, int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, int64_t sigops_cost, LockPoints lp) - : tx{tx}, + : TxGraph::Ref(std::move(ref)), + tx{tx}, nFee{fee}, nTxWeight{GetTransactionWeight(*tx)}, nUsageSize{RecursiveDynamicUsage(tx)}, diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index fd3fa226cae..0626e2c6837 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -720,7 +720,7 @@ public: { if (!m_node.mempool) return {}; LockPoints lp; - CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(TxGraph::Ref(), tx, 0, 0, 0, 0, false, 0, lp); LOCK(m_node.mempool->cs); return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize()); } diff --git a/src/test/fuzz/util/mempool.cpp b/src/test/fuzz/util/mempool.cpp index a6a28f94006..241a4d559c7 100644 --- a/src/test/fuzz/util/mempool.cpp +++ b/src/test/fuzz/util/mempool.cpp @@ -27,5 +27,5 @@ CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const auto entry_height{fuzzed_data_provider.ConsumeIntegralInRange(0, max_height)}; const bool spends_coinbase = fuzzed_data_provider.ConsumeBool(); const unsigned int sig_op_cost = fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_BLOCK_SIGOPS_COST); - return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, entry_sequence, spends_coinbase, sig_op_cost, {}}; + return CTxMemPoolEntry{TxGraph::Ref(), MakeTransactionRef(tx), fee, time, entry_height, entry_sequence, spends_coinbase, sig_op_cost, {}}; } diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index 5febb6791c6..0cb15b69420 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -37,7 +37,7 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) co CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) const { - return CTxMemPoolEntry{tx, nFee, TicksSinceEpoch(time), nHeight, m_sequence, spendsCoinbase, sigOpCost, lp}; + return CTxMemPoolEntry{TxGraph::Ref(), tx, nFee, TicksSinceEpoch(time), nHeight, m_sequence, spendsCoinbase, sigOpCost, lp}; } std::optional CheckPackageMempoolAcceptResult(const Package& txns, diff --git a/src/txmempool.cpp b/src/txmempool.cpp index a695fbffb86..e6e2042e494 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1386,7 +1386,7 @@ CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::StageAddition(const CTran { LOCK(m_pool->cs); Assume(m_to_add.find(tx->GetHash()) == m_to_add.end()); - auto newit = m_to_add.emplace(tx, fee, time, entry_height, entry_sequence, spends_coinbase, sigops_cost, lp).first; + auto newit = m_to_add.emplace(TxGraph::Ref(), tx, fee, time, entry_height, entry_sequence, spends_coinbase, sigops_cost, lp).first; CAmount delta{0}; m_pool->ApplyDelta(tx->GetHash(), delta); if (delta) m_to_add.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(delta); }); From c18c68a950d3a17e80ad0bc11ac7ee3de1a87f6c Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 9 Jan 2025 11:04:14 -0500 Subject: [PATCH 05/65] Create a txgraph inside CTxMemPool --- src/txmempool.cpp | 3 ++- src/txmempool.h | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index e6e2042e494..c931b361d47 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -413,6 +413,7 @@ static CTxMemPool::Options&& Flatten(CTxMemPool::Options&& opts, bilingual_str& CTxMemPool::CTxMemPool(Options opts, bilingual_str& error) : m_opts{Flatten(std::move(opts), error)} { + m_txgraph = MakeTxGraph(64, 101'000, ACCEPTABLE_ITERS); } bool CTxMemPool::isSpent(const COutPoint& outpoint) const @@ -1042,7 +1043,7 @@ 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) + cachedInnerUsage; + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * 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 90da90da7db..bc6eb36c841 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -49,6 +50,11 @@ struct bilingual_str; /** Fake height value used in Coin to signify they are only in the memory pool (since 0.8) */ static const uint32_t MEMPOOL_HEIGHT = 0x7FFFFFFF; +/** How many linearization iterations required for TxGraph clusters to have + * "acceptable" quality, if they cannot be optimally linearized with fewer + * iterations. */ +static constexpr uint64_t ACCEPTABLE_ITERS = 1'700; + /** * Test whether the LockPoints height and time are still valid on the current chain */ @@ -376,6 +382,7 @@ public: uint64_t CalculateDescendantMaximum(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); private: + std::unique_ptr m_txgraph GUARDED_BY(cs); typedef std::map cacheMap; From 1bf3b513966e34b45ea359cbe7576383437f5d93 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 30 Jan 2025 13:16:21 -0500 Subject: [PATCH 06/65] Add sigops adjusted weight calculator --- src/policy/policy.cpp | 7 ++++++- src/policy/policy.h | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 3da6cb7489b..b159d616021 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -373,9 +373,14 @@ bool SpendsNonAnchorWitnessProg(const CTransaction& tx, const CCoinsViewCache& p return false; } +int64_t GetSigOpsAdjustedWeight(int64_t weight, int64_t sigop_cost, unsigned int bytes_per_sigop) +{ + return std::max(weight, sigop_cost * bytes_per_sigop); +} + int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost, unsigned int bytes_per_sigop) { - return (std::max(nWeight, nSigOpCost * bytes_per_sigop) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; + return (GetSigOpsAdjustedWeight(nWeight, nSigOpCost, bytes_per_sigop) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; } int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop) diff --git a/src/policy/policy.h b/src/policy/policy.h index 0131b56b031..0e4314ea9d5 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -11,6 +11,7 @@ #include #include