From 241a3e666b59abb695c9d0a13d7458a763c2c5a0 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Sat, 14 Oct 2023 21:31:51 -0400 Subject: [PATCH] Simplify ancestor calculation functions Now that ancestor calculation never fails (due to ancestor/descendant limits being eliminated), we can eliminate the error handling from CalculateMemPoolAncestors. --- src/policy/rbf.cpp | 3 +- src/rpc/mempool.cpp | 2 +- src/test/txvalidation_tests.cpp | 34 ++++++++++---------- src/txmempool.cpp | 55 +++------------------------------ src/txmempool.h | 34 +++++--------------- src/validation.cpp | 7 +---- 6 files changed, 31 insertions(+), 104 deletions(-) diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 19823464768..fdf6f53cf40 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -39,8 +39,7 @@ 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. const auto& entry{*Assert(pool.GetEntry(tx.GetHash()))}; - auto ancestors{pool.AssumeCalculateMemPoolAncestors(__func__, entry, - /*fSearchForParents=*/false)}; + auto ancestors{pool.CalculateMemPoolAncestors(entry, /*fSearchForParents=*/false)}; for (CTxMemPool::txiter it : ancestors) { if (SignalsOptInRBF(it->GetTx())) { diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index d558a8cb58e..01870ef3577 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -474,7 +474,7 @@ static RPCHelpMan getmempoolancestors() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } - auto ancestors{mempool.AssumeCalculateMemPoolAncestors(self.m_name, *entry, /*fSearchForParents=*/false)}; + auto ancestors{mempool.CalculateMemPoolAncestors(*entry, /*fSearchForParents=*/false)}; if (!fVerbose) { UniValue o(UniValue::VARR); diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 27f5d959841..c0c9cc9c586 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -296,7 +296,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str{strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)", tx_v2_from_v3->GetHash().ToString(), tx_v2_from_v3->GetWitnessHash().ToString(), mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())}; - auto result_v2_from_v3{SingleTRUCChecks(pool, tx_v2_from_v3, *ancestors_v2_from_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3))}; + auto result_v2_from_v3{SingleTRUCChecks(pool, tx_v2_from_v3, ancestors_v2_from_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3))}; BOOST_CHECK_EQUAL(result_v2_from_v3->first, expected_error_str); BOOST_CHECK_EQUAL(result_v2_from_v3->second, nullptr); @@ -313,7 +313,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str_2{strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)", tx_v2_from_v2_and_v3->GetHash().ToString(), tx_v2_from_v2_and_v3->GetWitnessHash().ToString(), mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())}; - auto result_v2_from_both{SingleTRUCChecks(pool, tx_v2_from_v2_and_v3, *ancestors_v2_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3))}; + auto result_v2_from_both{SingleTRUCChecks(pool, tx_v2_from_v2_and_v3, ancestors_v2_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3))}; BOOST_CHECK_EQUAL(result_v2_from_both->first, expected_error_str_2); BOOST_CHECK_EQUAL(result_v2_from_both->second, nullptr); @@ -331,7 +331,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str{strprintf("version=3 tx %s (wtxid=%s) cannot spend from non-version=3 tx %s (wtxid=%s)", tx_v3_from_v2->GetHash().ToString(), tx_v3_from_v2->GetWitnessHash().ToString(), mempool_tx_v2->GetHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())}; - auto result_v3_from_v2{SingleTRUCChecks(pool, tx_v3_from_v2, *ancestors_v3_from_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2))}; + auto result_v3_from_v2{SingleTRUCChecks(pool, tx_v3_from_v2, ancestors_v3_from_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2))}; BOOST_CHECK_EQUAL(result_v3_from_v2->first, expected_error_str); BOOST_CHECK_EQUAL(result_v3_from_v2->second, nullptr); @@ -348,7 +348,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str_2{strprintf("version=3 tx %s (wtxid=%s) cannot spend from non-version=3 tx %s (wtxid=%s)", tx_v3_from_v2_and_v3->GetHash().ToString(), tx_v3_from_v2_and_v3->GetWitnessHash().ToString(), mempool_tx_v2->GetHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())}; - auto result_v3_from_both{SingleTRUCChecks(pool, tx_v3_from_v2_and_v3, *ancestors_v3_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3))}; + auto result_v3_from_both{SingleTRUCChecks(pool, tx_v3_from_v2_and_v3, ancestors_v3_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3))}; BOOST_CHECK_EQUAL(result_v3_from_both->first, expected_error_str_2); BOOST_CHECK_EQUAL(result_v3_from_both->second, nullptr); @@ -365,7 +365,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) // tx_v3_from_v3 auto tx_v3_from_v3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}}, /*version=*/3); auto ancestors_v3{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_from_v3))}; - BOOST_CHECK(SingleTRUCChecks(pool, tx_v3_from_v3, *ancestors_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v3)) + BOOST_CHECK(SingleTRUCChecks(pool, tx_v3_from_v3, ancestors_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v3)) == std::nullopt); Package package_v3_v3{mempool_tx_v3, tx_v3_from_v3}; @@ -376,7 +376,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) // tx_v2_from_v2 auto tx_v2_from_v2 = make_tx({COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/2); auto ancestors_v2{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v2_from_v2))}; - BOOST_CHECK(SingleTRUCChecks(pool, tx_v2_from_v2, *ancestors_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2)) + BOOST_CHECK(SingleTRUCChecks(pool, tx_v2_from_v2, ancestors_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2)) == std::nullopt); Package package_v2_v2{mempool_tx_v2, tx_v2_from_v2}; @@ -399,10 +399,10 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto tx_v3_multi_parent = make_tx(mempool_outpoints, /*version=*/3); package_multi_parents.emplace_back(tx_v3_multi_parent); auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_multi_parent))}; - BOOST_CHECK_EQUAL(ancestors->size(), 3); + BOOST_CHECK_EQUAL(ancestors.size(), 3); const auto expected_error_str{strprintf("tx %s (wtxid=%s) would have too many ancestors", tx_v3_multi_parent->GetHash().ToString(), tx_v3_multi_parent->GetWitnessHash().ToString())}; - auto result{SingleTRUCChecks(pool, tx_v3_multi_parent, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent))}; + auto result{SingleTRUCChecks(pool, tx_v3_multi_parent, ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent))}; BOOST_CHECK_EQUAL(result->first, expected_error_str); BOOST_CHECK_EQUAL(result->second, nullptr); @@ -427,7 +427,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_multi_gen))}; const auto expected_error_str{strprintf("tx %s (wtxid=%s) would have too many ancestors", tx_v3_multi_gen->GetHash().ToString(), tx_v3_multi_gen->GetWitnessHash().ToString())}; - auto result{SingleTRUCChecks(pool, tx_v3_multi_gen, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen))}; + auto result{SingleTRUCChecks(pool, tx_v3_multi_gen, ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen))}; BOOST_CHECK_EQUAL(result->first, expected_error_str); BOOST_CHECK_EQUAL(result->second, nullptr); @@ -445,7 +445,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child_big))}; const auto expected_error_str{strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes", tx_v3_child_big->GetHash().ToString(), tx_v3_child_big->GetWitnessHash().ToString(), vsize, TRUC_CHILD_MAX_VSIZE)}; - auto result{SingleTRUCChecks(pool, tx_v3_child_big, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big))}; + auto result{SingleTRUCChecks(pool, tx_v3_child_big, ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big))}; BOOST_CHECK_EQUAL(result->first, expected_error_str); BOOST_CHECK_EQUAL(result->second, nullptr); @@ -483,12 +483,12 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) BOOST_CHECK_EQUAL(total_sigops, tx_many_sigops->vin.size() * MAX_PUBKEYS_PER_MULTISIG); const int64_t bip141_vsize{GetVirtualTransactionSize(*tx_many_sigops)}; // Weight limit is not reached... - BOOST_CHECK(SingleTRUCChecks(pool, tx_many_sigops, *ancestors, empty_conflicts_set, bip141_vsize) == std::nullopt); + BOOST_CHECK(SingleTRUCChecks(pool, tx_many_sigops, ancestors, empty_conflicts_set, bip141_vsize) == std::nullopt); // ...but sigop limit is. const auto expected_error_str{strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes", tx_many_sigops->GetHash().ToString(), tx_many_sigops->GetWitnessHash().ToString(), total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, TRUC_CHILD_MAX_VSIZE)}; - auto result{SingleTRUCChecks(pool, tx_many_sigops, *ancestors, empty_conflicts_set, + auto result{SingleTRUCChecks(pool, tx_many_sigops, ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_many_sigops, /*nSigOpCost=*/total_sigops, /*bytes_per_sigop=*/ DEFAULT_BYTES_PER_SIGOP))}; BOOST_CHECK_EQUAL(result->first, expected_error_str); BOOST_CHECK_EQUAL(result->second, nullptr); @@ -503,7 +503,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) { BOOST_CHECK(GetTransactionWeight(*tx_mempool_v3_child) <= TRUC_CHILD_MAX_VSIZE * WITNESS_SCALE_FACTOR); auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_mempool_v3_child))}; - BOOST_CHECK(SingleTRUCChecks(pool, tx_mempool_v3_child, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_mempool_v3_child)) == std::nullopt); + BOOST_CHECK(SingleTRUCChecks(pool, tx_mempool_v3_child, ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_mempool_v3_child)) == std::nullopt); AddToMempool(pool, entry.FromTx(tx_mempool_v3_child)); Package package_v3_1p1c{mempool_tx_v3, tx_mempool_v3_child}; @@ -517,13 +517,13 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto ancestors_1sibling{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child2))}; const auto expected_error_str{strprintf("tx %s (wtxid=%s) would exceed descendant count limit", mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())}; - auto result_with_sibling_eviction{SingleTRUCChecks(pool, tx_v3_child2, *ancestors_1sibling, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2))}; + auto result_with_sibling_eviction{SingleTRUCChecks(pool, tx_v3_child2, ancestors_1sibling, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2))}; BOOST_CHECK_EQUAL(result_with_sibling_eviction->first, expected_error_str); // The other mempool child is returned to allow for sibling eviction. BOOST_CHECK_EQUAL(result_with_sibling_eviction->second, tx_mempool_v3_child); // If directly replacing the child, make sure there is no double-counting. - BOOST_CHECK(SingleTRUCChecks(pool, tx_v3_child2, *ancestors_1sibling, {tx_mempool_v3_child->GetHash()}, GetVirtualTransactionSize(*tx_v3_child2)) + BOOST_CHECK(SingleTRUCChecks(pool, tx_v3_child2, ancestors_1sibling, {tx_mempool_v3_child->GetHash()}, GetVirtualTransactionSize(*tx_v3_child2)) == std::nullopt); Package package_v3_1p2c{mempool_tx_v3, tx_mempool_v3_child, tx_v3_child2}; @@ -537,7 +537,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) BOOST_CHECK_EQUAL(pool.GetDescendantCount(entry_mempool_parent), 3); auto ancestors_2siblings{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child3))}; - auto result_2children{SingleTRUCChecks(pool, tx_v3_child3, *ancestors_2siblings, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child3))}; + auto result_2children{SingleTRUCChecks(pool, tx_v3_child3, ancestors_2siblings, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child3))}; BOOST_CHECK_EQUAL(result_2children->first, expected_error_str); // The other mempool child is not returned because sibling eviction is not allowed. BOOST_CHECK_EQUAL(result_2children->second, nullptr); @@ -557,7 +557,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto ancestors_3gen{pool.CalculateMemPoolAncestors(entry.FromTx(tx_to_submit))}; const auto expected_error_str{strprintf("tx %s (wtxid=%s) would exceed descendant count limit", tx_mempool_grandparent->GetHash().ToString(), tx_mempool_grandparent->GetWitnessHash().ToString())}; - auto result_3gen{SingleTRUCChecks(pool, tx_to_submit, *ancestors_3gen, empty_conflicts_set, GetVirtualTransactionSize(*tx_to_submit))}; + auto result_3gen{SingleTRUCChecks(pool, tx_to_submit, ancestors_3gen, empty_conflicts_set, GetVirtualTransactionSize(*tx_to_submit))}; BOOST_CHECK_EQUAL(result_3gen->first, expected_error_str); // The other mempool child is not returned because sibling eviction is not allowed. BOOST_CHECK_EQUAL(result_3gen->second, nullptr); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index d16a5efc4d6..f83b4544f40 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -99,7 +99,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector& vHashesToU } } -util::Result CTxMemPool::CalculateAncestors(CTxMemPoolEntry::Parents& staged_ancestors) const +CTxMemPool::setEntries CTxMemPool::CalculateAncestors(CTxMemPoolEntry::Parents& staged_ancestors) const { setEntries ancestors; @@ -127,19 +127,6 @@ util::Result CTxMemPool::CalculateAncestors(CTxMemPoolEn util::Result CTxMemPool::CheckPackageLimits(const Package& package, const int64_t total_vsize) const { - CTxMemPoolEntry::Parents staged_ancestors; - for (const auto& tx : package) { - for (const auto& input : tx->vin) { - std::optional piter = GetIter(input.prevout.hash); - if (piter) { - staged_ancestors.insert(**piter); - } - } - } - - const auto ancestors{CalculateAncestors(staged_ancestors)}; - // It's possible to overestimate the ancestor/descendant totals. - if (!ancestors.has_value()) return util::Error{Untranslated("possibly " + util::ErrorString(ancestors).original)}; return {}; } @@ -151,7 +138,7 @@ bool CTxMemPool::HasDescendants(const Txid& txid) const return m_txgraph->GetDescendants(*entry, TxGraph::Level::MAIN).size() > 1; } -util::Result CTxMemPool::CalculateMemPoolAncestors( +CTxMemPool::setEntries CTxMemPool::CalculateMemPoolAncestors( const CTxMemPoolEntry &entry, bool fSearchForParents /* = true */) const { @@ -178,19 +165,6 @@ util::Result CTxMemPool::CalculateMemPoolAncestors( return CalculateAncestors(staged_ancestors); } -CTxMemPool::setEntries CTxMemPool::AssumeCalculateMemPoolAncestors( - std::string_view calling_fn_name, - const CTxMemPoolEntry &entry, - bool fSearchForParents /* = true */) const -{ - auto result{CalculateMemPoolAncestors(entry, fSearchForParents)}; - if (!Assume(result)) { - LogPrintLevel(BCLog::MEMPOOL, BCLog::Level::Error, "%s: CalculateMemPoolAncestors failed unexpectedly, continuing with empty ancestor set (%s)\n", - calling_fn_name, util::ErrorString(result).original); - } - return std::move(result).value_or(CTxMemPool::setEntries{}); -} - void CTxMemPool::UpdateAncestorsOf(bool add, txiter it) { const CTxMemPoolEntry::Parents& parents = it->GetMemPoolParentsConst(); @@ -264,16 +238,6 @@ void CTxMemPool::Apply(ChangeSet* changeset) for (size_t i=0; im_entry_vec.size(); ++i) { auto tx_entry = changeset->m_entry_vec[i]; - std::optional ancestors; - if (i == 0) { - // Note: ChangeSet::CalculateMemPoolAncestors() will return a - // cached value if mempool ancestors for this transaction were - // previously calculated. - // We can only use a cached ancestor calculation for the first - // transaction in a package, because in-package parents won't be - // present in the cached ancestor sets of in-package children. - ancestors = *Assume(changeset->CalculateMemPoolAncestors(tx_entry)); - } // First splice this entry into mapTx. auto node_handle = changeset->m_to_add.extract(tx_entry); auto result = mapTx.insert(std::move(node_handle)); @@ -281,22 +245,11 @@ void CTxMemPool::Apply(ChangeSet* changeset) Assume(result.inserted); txiter it = result.position; - // Now update the entry for ancestors/descendants. - if (ancestors.has_value()) { - addNewTransaction(it, *ancestors); - } else { - addNewTransaction(it); - } + addNewTransaction(it); } } -void CTxMemPool::addNewTransaction(CTxMemPool::txiter it) -{ - auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it)}; - return addNewTransaction(it, ancestors); -} - -void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit, CTxMemPool::setEntries& setAncestors) +void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit) { const CTxMemPoolEntry& entry = *newit; diff --git a/src/txmempool.h b/src/txmempool.h index cdf9bcc9f19..d2b7f232b94 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -310,9 +310,9 @@ private: * * @param[in] staged_ancestors Should contain entries in the mempool. * - * @return all in-mempool ancestors, or an error if any ancestor or descendant limits were hit + * @return all in-mempool ancestors */ - util::Result CalculateAncestors(CTxMemPoolEntry::Parents &staged_ancestors) + setEntries CalculateAncestors(CTxMemPoolEntry::Parents &staged_ancestors) const EXCLUSIVE_LOCKS_REQUIRED(cs); static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it) @@ -429,7 +429,7 @@ public: } /** - * Try to calculate all in-mempool ancestors of entry. + * 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 @@ -439,28 +439,9 @@ public: * * @return all in-mempool ancestors */ - util::Result CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, + setEntries CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); - /** - * Same as CalculateMemPoolAncestors, but always returns a (non-optional) setEntries. - * Should only be used when it is assumed CalculateMemPoolAncestors would not fail. If - * CalculateMemPoolAncestors does unexpectedly fail, an empty setEntries is returned and the - * error is logged to BCLog::MEMPOOL with level BCLog::Level::Error. In debug builds, failure - * of CalculateMemPoolAncestors will lead to shutdown due to assertion failure. - * - * @param[in] calling_fn_name Name of calling function so we can properly log the call site - * - * @return a setEntries corresponding to the result of CalculateMemPoolAncestors or an empty - * setEntries if it failed - * - * @see CTXMemPool::CalculateMemPoolAncestors() - */ - setEntries AssumeCalculateMemPoolAncestors( - std::string_view calling_fn_name, - const CTxMemPoolEntry &entry, - bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); - bool HasDescendants(const Txid& txid) const; /** Collect the entire cluster of connected transactions for each transaction in txids. @@ -722,7 +703,7 @@ public: /** Check if any cluster limits are exceeded. Returns true if pass, false if fail. */ bool CheckMemPoolPolicyLimits(); - util::Result CalculateMemPoolAncestors(TxHandle tx) + CTxMemPool::setEntries CalculateMemPoolAncestors(TxHandle tx) { // Look up transaction in our cache first auto it = m_ancestors.find(tx); @@ -731,8 +712,8 @@ public: // If not found, try to have the mempool calculate it, and cache // for later. LOCK(m_pool->cs); - auto ret{m_pool->CalculateMemPoolAncestors(*tx)}; - if (ret) m_ancestors.try_emplace(tx, *ret); + auto ret = m_pool->CalculateMemPoolAncestors(*tx); + m_ancestors.try_emplace(tx, ret); return ret; } @@ -796,7 +777,6 @@ private: // tracking (due to lack of CValidationInterface::TransactionAddedToMempool // callbacks). void addNewTransaction(CTxMemPool::txiter it) EXCLUSIVE_LOCKS_REQUIRED(cs); - void addNewTransaction(CTxMemPool::txiter it, CTxMemPool::setEntries& setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs); public: void StartBlockBuilding() const EXCLUSIVE_LOCKS_REQUIRED(cs) { assert(!m_builder); m_builder = m_txgraph->GetBlockBuilder(); } FeePerWeight GetBlockBuilderChunk(std::vector& entries) const EXCLUSIVE_LOCKS_REQUIRED(cs) diff --git a/src/validation.cpp b/src/validation.cpp index 49f8af3ec94..6a02acd5861 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -961,12 +961,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) ws.m_iters_conflicting = m_pool.GetIterSet(ws.m_conflicts); - // Calculate in-mempool ancestors, up to a limit. - if (auto ancestors{m_subpackage.m_changeset->CalculateMemPoolAncestors(ws.m_tx_handle)}) { - ws.m_ancestors = std::move(*ancestors); - } else { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", util::ErrorString(ancestors).original); - } + ws.m_ancestors = m_subpackage.m_changeset->CalculateMemPoolAncestors(ws.m_tx_handle); // Even though just checking direct mempool parents for inheritance would be sufficient, we // check using the full ancestor set here because it's more convenient to use what we have