From b85af25f8770974bae4ef3fae64e75ef6dd2d3c2 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Thu, 15 Sep 2022 18:26:21 +0100 Subject: [PATCH 1/4] refactor: mempool: add MemPoolLimits::NoLimits() There are quite a few places in the codebase that require us to construct a CTxMemPool without limits on ancestors and descendants. This helper function allows us to get rid of all that duplication. --- src/kernel/mempool_limits.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/kernel/mempool_limits.h b/src/kernel/mempool_limits.h index e192e7e6cd..8d4495c3cb 100644 --- a/src/kernel/mempool_limits.h +++ b/src/kernel/mempool_limits.h @@ -24,6 +24,15 @@ struct MemPoolLimits { int64_t descendant_count{DEFAULT_DESCENDANT_LIMIT}; //! The maximum allowed size in virtual bytes of an entry and its descendants within a package. int64_t descendant_size_vbytes{DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1'000}; + + /** + * @return MemPoolLimits with all the limits set to the maximum + */ + static constexpr MemPoolLimits NoLimits() + { + int64_t no_limit{std::numeric_limits::max()}; + return {no_limit, no_limit, no_limit, no_limit}; + } }; } // namespace kernel From 3a86f24a4c1e4e985b1d90eddc135b8dd17049a4 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Thu, 15 Sep 2022 18:11:32 +0100 Subject: [PATCH 2/4] refactor: mempool: use CTxMempool::Limits Simplifies function signatures by removing repetition of all the ancestor/descendant limits, and increases readability by being more verbose by naming the limits, while still reducing the LoC. --- src/node/interfaces.cpp | 3 +-- src/node/miner.cpp | 3 +-- src/policy/rbf.cpp | 3 +-- src/rpc/mempool.cpp | 3 +-- src/txmempool.cpp | 57 +++++++++++++++-------------------------- src/txmempool.h | 32 +++++++++-------------- src/validation.cpp | 43 ++++++++++++++++--------------- 7 files changed, 59 insertions(+), 85 deletions(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index aa7ddec770..8a0011a629 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -660,8 +660,7 @@ public: std::string unused_error_string; LOCK(m_node.mempool->cs); return m_node.mempool->CalculateMemPoolAncestors( - entry, ancestors, limits.ancestor_count, limits.ancestor_size_vbytes, - limits.descendant_count, limits.descendant_size_vbytes, unused_error_string); + entry, ancestors, limits, unused_error_string); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override { diff --git a/src/node/miner.cpp b/src/node/miner.cpp index b277188c1f..ef8946773f 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -396,9 +396,8 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele } CTxMemPool::setEntries ancestors; - uint64_t nNoLimit = std::numeric_limits::max(); std::string dummy; - mempool.CalculateMemPoolAncestors(*iter, ancestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); + mempool.CalculateMemPoolAncestors(*iter, ancestors, CTxMemPool::Limits::NoLimits(), dummy, false); onlyUnconfirmed(ancestors); ancestors.insert(iter); diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 6098caced9..55f47f485b 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -36,10 +36,9 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) // If all the inputs have nSequence >= maxint-1, it still might be // signaled for RBF if any unconfirmed parents have signaled. - uint64_t noLimit = std::numeric_limits::max(); std::string dummy; CTxMemPoolEntry entry = *pool.mapTx.find(tx.GetHash()); - pool.CalculateMemPoolAncestors(entry, ancestors, noLimit, noLimit, noLimit, noLimit, dummy, false); + pool.CalculateMemPoolAncestors(entry, ancestors, CTxMemPool::Limits::NoLimits(), dummy, false); for (CTxMemPool::txiter it : ancestors) { if (SignalsOptInRBF(it->GetTx())) { diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index e390a7c15c..706d783942 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -449,9 +449,8 @@ static RPCHelpMan getmempoolancestors() } CTxMemPool::setEntries setAncestors; - uint64_t noLimit = std::numeric_limits::max(); std::string dummy; - mempool.CalculateMemPoolAncestors(*it, setAncestors, noLimit, noLimit, noLimit, noLimit, dummy, false); + mempool.CalculateMemPoolAncestors(*it, setAncestors, CTxMemPool::Limits::NoLimits(), dummy, false); if (!fVerbose) { UniValue o(UniValue::VARR); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index e1288b7346..97315a05cd 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -187,10 +187,7 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, size_t entry_count, setEntries& setAncestors, CTxMemPoolEntry::Parents& staged_ancestors, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString) const { size_t totalSizeWithAncestors = entry_size; @@ -203,14 +200,14 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, staged_ancestors.erase(stage); totalSizeWithAncestors += stageit->GetTxSize(); - if (stageit->GetSizeWithDescendants() + entry_size > limitDescendantSize) { - errString = strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantSize); + if (stageit->GetSizeWithDescendants() + entry_size > static_cast(limits.descendant_size_vbytes)) { + errString = strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_size_vbytes); return false; - } else if (stageit->GetCountWithDescendants() + entry_count > limitDescendantCount) { - errString = strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantCount); + } else if (stageit->GetCountWithDescendants() + entry_count > static_cast(limits.descendant_count)) { + errString = strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_count); return false; - } else if (totalSizeWithAncestors > limitAncestorSize) { - errString = strprintf("exceeds ancestor size limit [limit: %u]", limitAncestorSize); + } else if (totalSizeWithAncestors > static_cast(limits.ancestor_size_vbytes)) { + errString = strprintf("exceeds ancestor size limit [limit: %u]", limits.ancestor_size_vbytes); return false; } @@ -222,8 +219,8 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, if (setAncestors.count(parent_it) == 0) { staged_ancestors.insert(parent); } - if (staged_ancestors.size() + setAncestors.size() + entry_count > limitAncestorCount) { - errString = strprintf("too many unconfirmed ancestors [limit: %u]", limitAncestorCount); + if (staged_ancestors.size() + setAncestors.size() + entry_count > static_cast(limits.ancestor_count)) { + errString = strprintf("too many unconfirmed ancestors [limit: %u]", limits.ancestor_count); return false; } } @@ -233,10 +230,7 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, } bool CTxMemPool::CheckPackageLimits(const Package& package, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString) const { CTxMemPoolEntry::Parents staged_ancestors; @@ -247,8 +241,8 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, std::optional piter = GetIter(input.prevout.hash); if (piter) { staged_ancestors.insert(**piter); - if (staged_ancestors.size() + package.size() > limitAncestorCount) { - errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + if (staged_ancestors.size() + package.size() > static_cast(limits.ancestor_count)) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count); return false; } } @@ -260,8 +254,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, setEntries setAncestors; const auto ret = CalculateAncestorsAndCheckLimits(total_size, package.size(), setAncestors, staged_ancestors, - limitAncestorCount, limitAncestorSize, - limitDescendantCount, limitDescendantSize, errString); + limits, errString); // It's possible to overestimate the ancestor/descendant totals. if (!ret) errString.insert(0, "possibly "); return ret; @@ -269,10 +262,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString, bool fSearchForParents /* = true */) const { @@ -287,8 +277,8 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, std::optional piter = GetIter(tx.vin[i].prevout.hash); if (piter) { staged_ancestors.insert(**piter); - if (staged_ancestors.size() + 1 > limitAncestorCount) { - errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + if (staged_ancestors.size() + 1 > static_cast(limits.ancestor_count)) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count); return false; } } @@ -302,8 +292,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, return CalculateAncestorsAndCheckLimits(entry.GetTxSize(), /*entry_count=*/1, setAncestors, staged_ancestors, - limitAncestorCount, limitAncestorSize, - limitDescendantCount, limitDescendantSize, errString); + limits, errString); } void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors) @@ -347,7 +336,6 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b { // For each entry, walk back all ancestors and decrement size associated with this // transaction - const uint64_t nNoLimit = std::numeric_limits::max(); if (updateDescendants) { // updateDescendants should be true whenever we're not recursively // removing a tx and all its descendants, eg when a transaction is @@ -390,7 +378,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b // mempool parents we'd calculate by searching, and it's important that // we use the cached notion of ancestor transactions as the set of // things to update for removal. - CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); + CalculateMemPoolAncestors(entry, setAncestors, Limits::NoLimits(), dummy, false); // Note that UpdateAncestorsOf severs the child links that point to // removeIt in the entries for the parents of removeIt. UpdateAncestorsOf(false, removeIt, setAncestors); @@ -744,9 +732,8 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei assert(std::equal(setParentCheck.begin(), setParentCheck.end(), it->GetMemPoolParentsConst().begin(), comp)); // Verify ancestor state is correct. setEntries setAncestors; - uint64_t nNoLimit = std::numeric_limits::max(); std::string dummy; - CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); + CalculateMemPoolAncestors(*it, setAncestors, Limits::NoLimits(), dummy); uint64_t nCountCheck = setAncestors.size() + 1; uint64_t nSizeCheck = it->GetTxSize(); CAmount nFeesCheck = it->GetModifiedFee(); @@ -908,9 +895,8 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); }); // Now update all ancestors' modified fees with descendants setEntries setAncestors; - uint64_t nNoLimit = std::numeric_limits::max(); std::string dummy; - CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); + CalculateMemPoolAncestors(*it, setAncestors, Limits::NoLimits(), dummy, false); for (txiter ancestorIt : setAncestors) { mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e){ e.UpdateDescendantState(0, nFeeDelta, 0);}); } @@ -1049,9 +1035,8 @@ int CTxMemPool::Expire(std::chrono::seconds time) void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, bool validFeeEstimate) { setEntries setAncestors; - uint64_t nNoLimit = std::numeric_limits::max(); std::string dummy; - CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); + CalculateMemPoolAncestors(entry, setAncestors, Limits::NoLimits(), dummy); return addUnchecked(entry, setAncestors, validFeeEstimate); } diff --git a/src/txmempool.h b/src/txmempool.h index cd15d069b1..8597b21417 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -526,6 +526,8 @@ public: typedef std::set setEntries; + using Limits = kernel::MemPoolLimits; + uint64_t CalculateDescendantMaximum(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); private: typedef std::map cacheMap; @@ -554,10 +556,7 @@ private: size_t entry_count, setEntries& setAncestors, CTxMemPoolEntry::Parents &staged_ancestors, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); public: @@ -576,8 +575,6 @@ public: const bool m_require_standard; const bool m_full_rbf; - using Limits = kernel::MemPoolLimits; - const Limits m_limits; /** Create a new CTxMemPool. @@ -670,36 +667,31 @@ public: /** Try to calculate all in-mempool ancestors of entry. * (these are all calculated including the tx itself) - * limitAncestorCount = max number of ancestors - * limitAncestorSize = max size of ancestors - * limitDescendantCount = max number of descendants any ancestor can have - * limitDescendantSize = max size of descendants any ancestor can have + * limits = maximum number and size of ancestors and descendants * errString = populated with error reason if any limits are hit * fSearchForParents = whether to search a tx's vin for in-mempool parents, or * look up parents from mapLinks. Must be true for entries not in the mempool */ - bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, setEntries& setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string& errString, bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); + bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, + setEntries& setAncestors, + const Limits& limits, + std::string& errString, + bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Calculate all in-mempool ancestors of a set of transactions not already in the mempool and * check ancestor and descendant limits. Heuristics are used to estimate the ancestor and * descendant count of all entries if the package were to be added to the mempool. The limits * are applied to the union of all package transactions. For example, if the package has 3 - * transactions and limitAncestorCount = 25, the union of all 3 sets of ancestors (including the + * transactions and limits.ancestor_count = 25, the union of all 3 sets of ancestors (including the * transactions themselves) must be <= 22. * @param[in] package Transaction package being evaluated for acceptance * to mempool. The transactions need not be direct * ancestors/descendants of each other. - * @param[in] limitAncestorCount Max number of txns including ancestors. - * @param[in] limitAncestorSize Max virtual size including ancestors. - * @param[in] limitDescendantCount Max number of txns including descendants. - * @param[in] limitDescendantSize Max virtual size including descendants. + * @param[in] limits Maximum number and size of ancestors and descendants * @param[out] errString Populated with error reason if a limit is hit. */ bool CheckPackageLimits(const Package& package, - uint64_t limitAncestorCount, - uint64_t limitAncestorSize, - uint64_t limitDescendantCount, - uint64_t limitDescendantSize, + const Limits& limits, std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Populate setDescendants with all in-mempool descendants of hash. diff --git a/src/validation.cpp b/src/validation.cpp index fb29ca3ae7..895e7eb61c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -424,11 +424,13 @@ namespace { class MemPoolAccept { public: - explicit MemPoolAccept(CTxMemPool& mempool, Chainstate& active_chainstate) : m_pool(mempool), m_view(&m_dummy), m_viewmempool(&active_chainstate.CoinsTip(), m_pool), m_active_chainstate(active_chainstate), - m_limit_ancestors(m_pool.m_limits.ancestor_count), - m_limit_ancestor_size(m_pool.m_limits.ancestor_size_vbytes), - m_limit_descendants(m_pool.m_limits.descendant_count), - m_limit_descendant_size(m_pool.m_limits.descendant_size_vbytes) { + explicit MemPoolAccept(CTxMemPool& mempool, Chainstate& active_chainstate) : + m_pool(mempool), + m_view(&m_dummy), + m_viewmempool(&active_chainstate.CoinsTip(), m_pool), + m_active_chainstate(active_chainstate), + m_limits{m_pool.m_limits} + { } // We put the arguments we're handed into a struct, so we can pass them @@ -660,13 +662,7 @@ private: Chainstate& m_active_chainstate; - // The package limits in effect at the time of invocation. - const size_t m_limit_ancestors; - const size_t m_limit_ancestor_size; - // These may be modified while evaluating a transaction (eg to account for - // in-mempool conflicts; see below). - size_t m_limit_descendants; - size_t m_limit_descendant_size; + CTxMemPool::Limits m_limits; /** Whether the transaction(s) would replace any mempool transactions. If so, RBF rules apply. */ bool m_rbf{false}; @@ -879,12 +875,12 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) assert(ws.m_iters_conflicting.size() == 1); CTxMemPool::txiter conflict = *ws.m_iters_conflicting.begin(); - m_limit_descendants += 1; - m_limit_descendant_size += conflict->GetSizeWithDescendants(); + m_limits.descendant_count += 1; + m_limits.descendant_size_vbytes += conflict->GetSizeWithDescendants(); } std::string errString; - if (!m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, errString)) { + if (!m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, m_limits, errString)) { ws.m_ancestors.clear(); // If CalculateMemPoolAncestors fails second time, we want the original error string. std::string dummy_err_string; @@ -899,8 +895,16 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // to be secure by simply only having two immediately-spendable // outputs - one for each counterparty. For more info on the uses for // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html + CTxMemPool::Limits cpfp_carve_out_limits{ + .ancestor_count = 2, + .ancestor_size_vbytes = m_limits.ancestor_size_vbytes, + .descendant_count = m_limits.descendant_count + 1, + .descendant_size_vbytes = m_limits.descendant_size_vbytes + EXTRA_DESCENDANT_TX_SIZE_LIMIT, + }; if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || - !m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { + !m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, + cpfp_carve_out_limits, + dummy_err_string)) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", errString); } } @@ -976,8 +980,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& txn { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));})); std::string err_string; - if (!m_pool.CheckPackageLimits(txns, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, - m_limit_descendant_size, err_string)) { + if (!m_pool.CheckPackageLimits(txns, m_limits, err_string)) { // This is a package-wide error, separate from an individual transaction error. return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); } @@ -1121,9 +1124,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& // Re-calculate mempool ancestors to call addUnchecked(). They may have changed since the // last calculation done in PreChecks, since package ancestors have already been submitted. std::string unused_err_string; - if(!m_pool.CalculateMemPoolAncestors(*ws.m_entry, ws.m_ancestors, m_limit_ancestors, - m_limit_ancestor_size, m_limit_descendants, - m_limit_descendant_size, unused_err_string)) { + if(!m_pool.CalculateMemPoolAncestors(*ws.m_entry, ws.m_ancestors, m_limits, unused_err_string)) { results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); // Since PreChecks() and PackageMempoolChecks() both enforce limits, this should never fail. Assume(false); From 6945853c0bf3b1941bfd18b5ffbd5a81be0e56da Mon Sep 17 00:00:00 2001 From: stickies-v Date: Wed, 5 Oct 2022 12:05:04 +0100 Subject: [PATCH 3/4] test: use NoLimits() in MempoolIndexingTest The (100, 1000000, 1000, 1000000) limits are arbitrarily high and don't restrict anything, they are just meant to calculate ancestors properly. Using NoLimits() makes this intent more clear and simplifies the code. --- src/test/mempool_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 8c745b07b9..19f457b8dd 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -203,7 +203,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) CTxMemPool::setEntries setAncestorsCalculated; std::string dummy; - BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), setAncestorsCalculated, 100, 1000000, 1000, 1000000, dummy), true); + BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(2'000'000LL).FromTx(tx7), setAncestorsCalculated, CTxMemPool::Limits::NoLimits(), dummy), true); BOOST_CHECK(setAncestorsCalculated == setAncestors); pool.addUnchecked(entry.FromTx(tx7), setAncestors); @@ -261,7 +261,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx10.vout[0].nValue = 10 * COIN; setAncestorsCalculated.clear(); - BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(4).FromTx(tx10), setAncestorsCalculated, 100, 1000000, 1000, 1000000, dummy), true); + BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(200'000LL).Time(4).FromTx(tx10), setAncestorsCalculated, CTxMemPool::Limits::NoLimits(), dummy), true); BOOST_CHECK(setAncestorsCalculated == setAncestors); pool.addUnchecked(entry.FromTx(tx10), setAncestors); From 33b12e5df6aa14344dd084e0c6f2d34158fca383 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Wed, 5 Oct 2022 13:06:11 +0100 Subject: [PATCH 4/4] docs: improve docs where MemPoolLimits is used --- src/txmempool.h | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/txmempool.h b/src/txmempool.h index 8597b21417..760af78500 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -547,10 +547,16 @@ private: /** * Helper function to calculate all in-mempool ancestors of staged_ancestors and apply ancestor * and descendant limits (including staged_ancestors thsemselves, entry_size and entry_count). - * param@[in] entry_size Virtual size to include in the limits. - * param@[in] entry_count How many entries to include in the limits. - * param@[in] staged_ancestors Should contain entries in the mempool. - * param@[out] setAncestors Will be populated with all mempool ancestors. + * + * @param[in] entry_size Virtual size to include in the limits. + * @param[in] entry_count How many entries to include in the limits. + * @param[out] setAncestors Will be populated with all mempool ancestors. + * @param[in] staged_ancestors Should contain entries in the mempool. + * @param[in] limits Maximum number and size of ancestors and descendants + * @param[out] errString Populated with error reason if any limits are hit + * + * @return true if no limits were hit and all in-mempool ancestors were calculated, false + * otherwise */ bool CalculateAncestorsAndCheckLimits(size_t entry_size, size_t entry_count, @@ -665,12 +671,20 @@ public: */ void UpdateTransactionsFromBlock(const std::vector& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch); - /** Try to calculate all in-mempool ancestors of entry. - * (these are all calculated including the tx itself) - * limits = maximum number and size of ancestors and descendants - * errString = populated with error reason if any limits are hit - * fSearchForParents = whether to search a tx's vin for in-mempool parents, or - * look up parents from mapLinks. Must be true for entries not in the mempool + /** + * Try to calculate all in-mempool ancestors of entry. + * (these are all calculated including the tx itself) + * + * @param[in] entry CTxMemPoolEntry of which all in-mempool ancestors are calculated + * @param[out] setAncestors Will be populated with all mempool ancestors. + * @param[in] limits Maximum number and size of ancestors and descendants + * @param[out] errString Populated with error reason if any limits are hit + * @param[in] fSearchForParents Whether to search a tx's vin for in-mempool parents, or look + * up parents from mapLinks. Must be true for entries not in + * the mempool + * + * @return true if no limits were hit and all in-mempool ancestors were calculated, false + * otherwise */ bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, setEntries& setAncestors,