diff options
-rw-r--r-- | src/node/interfaces.cpp | 5 | ||||
-rw-r--r-- | src/node/miner.cpp | 4 | ||||
-rw-r--r-- | src/policy/rbf.cpp | 8 | ||||
-rw-r--r-- | src/rpc/mempool.cpp | 10 | ||||
-rw-r--r-- | src/test/mempool_tests.cpp | 13 | ||||
-rw-r--r-- | src/txmempool.cpp | 103 | ||||
-rw-r--r-- | src/txmempool.h | 49 | ||||
-rw-r--r-- | src/validation.cpp | 40 |
8 files changed, 121 insertions, 111 deletions
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 7fd5ffd6a6..980506f8dc 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -672,12 +672,9 @@ public: if (!m_node.mempool) return true; LockPoints lp; CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); - CTxMemPool::setEntries ancestors; const CTxMemPool::Limits& limits{m_node.mempool->m_limits}; - std::string unused_error_string; LOCK(m_node.mempool->cs); - return m_node.mempool->CalculateMemPoolAncestors( - entry, ancestors, limits, unused_error_string); + return m_node.mempool->CalculateMemPoolAncestors(entry, limits).has_value(); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override { diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 7179d1a341..dc6849e0d2 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -394,9 +394,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele continue; } - CTxMemPool::setEntries ancestors; - std::string dummy; - mempool.CalculateMemPoolAncestors(*iter, ancestors, CTxMemPool::Limits::NoLimits(), dummy, false); + auto ancestors{mempool.AssumeCalculateMemPoolAncestors(__func__, *iter, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; onlyUnconfirmed(ancestors); ancestors.insert(iter); diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 10f85b9510..d032b74008 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -22,8 +22,6 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) { AssertLockHeld(pool.cs); - CTxMemPool::setEntries ancestors; - // First check the transaction itself. if (SignalsOptInRBF(tx)) { return RBFTransactionState::REPLACEABLE_BIP125; @@ -37,9 +35,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. - std::string dummy; - CTxMemPoolEntry entry = *pool.mapTx.find(tx.GetHash()); - pool.CalculateMemPoolAncestors(entry, ancestors, CTxMemPool::Limits::NoLimits(), dummy, false); + const CTxMemPoolEntry entry{*pool.mapTx.find(tx.GetHash())}; + auto ancestors{pool.AssumeCalculateMemPoolAncestors(__func__, entry, CTxMemPool::Limits::NoLimits(), + /*fSearchForParents=*/false)}; for (CTxMemPool::txiter it : ancestors) { if (SignalsOptInRBF(it->GetTx())) { diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 7a0c361ae0..58e71a0604 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -23,6 +23,8 @@ #include <util/moneystr.h> #include <util/time.h> +#include <utility> + using kernel::DumpMempool; using node::DEFAULT_MAX_RAW_TX_FEE_RATE; @@ -449,19 +451,17 @@ static RPCHelpMan getmempoolancestors() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } - CTxMemPool::setEntries setAncestors; - std::string dummy; - mempool.CalculateMemPoolAncestors(*it, setAncestors, CTxMemPool::Limits::NoLimits(), dummy, false); + auto ancestors{mempool.AssumeCalculateMemPoolAncestors(__func__, *it, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; if (!fVerbose) { UniValue o(UniValue::VARR); - for (CTxMemPool::txiter ancestorIt : setAncestors) { + for (CTxMemPool::txiter ancestorIt : ancestors) { o.push_back(ancestorIt->GetTx().GetHash().ToString()); } return o; } else { UniValue o(UniValue::VOBJ); - for (CTxMemPool::txiter ancestorIt : setAncestors) { + for (CTxMemPool::txiter ancestorIt : ancestors) { const CTxMemPoolEntry &e = *ancestorIt; const uint256& _hash = e.GetTx().GetHash(); UniValue info(UniValue::VOBJ); diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 1a24448569..7e9079743e 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -202,10 +202,9 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx7.vout[1].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx7.vout[1].nValue = 1 * COIN; - CTxMemPool::setEntries setAncestorsCalculated; - std::string dummy; - BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(2'000'000LL).FromTx(tx7), setAncestorsCalculated, CTxMemPool::Limits::NoLimits(), dummy), true); - BOOST_CHECK(setAncestorsCalculated == setAncestors); + auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), CTxMemPool::Limits::NoLimits())}; + BOOST_REQUIRE(ancestors_calculated.has_value()); + BOOST_CHECK(*ancestors_calculated == setAncestors); pool.addUnchecked(entry.FromTx(tx7), setAncestors); BOOST_CHECK_EQUAL(pool.size(), 7U); @@ -261,9 +260,9 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx10.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx10.vout[0].nValue = 10 * COIN; - setAncestorsCalculated.clear(); - BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(200'000LL).Time(NodeSeconds{4s}).FromTx(tx10), setAncestorsCalculated, CTxMemPool::Limits::NoLimits(), dummy), true); - BOOST_CHECK(setAncestorsCalculated == setAncestors); + ancestors_calculated = pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(NodeSeconds{4s}).FromTx(tx10), CTxMemPool::Limits::NoLimits()); + BOOST_REQUIRE(ancestors_calculated); + BOOST_CHECK(*ancestors_calculated == setAncestors); pool.addUnchecked(entry.FromTx(tx10), setAncestors); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 83020117a7..50cd34dde4 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -17,12 +17,16 @@ #include <util/check.h> #include <util/moneystr.h> #include <util/overflow.h> +#include <util/result.h> #include <util/system.h> #include <util/time.h> +#include <util/translation.h> #include <validationinterface.h> #include <cmath> #include <optional> +#include <string_view> +#include <utility> bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp) { @@ -147,32 +151,29 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashes } } -bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, - size_t entry_count, - setEntries& setAncestors, - CTxMemPoolEntry::Parents& staged_ancestors, - const Limits& limits, - std::string &errString) const +util::Result<CTxMemPool::setEntries> CTxMemPool::CalculateAncestorsAndCheckLimits( + size_t entry_size, + size_t entry_count, + CTxMemPoolEntry::Parents& staged_ancestors, + const Limits& limits) const { size_t totalSizeWithAncestors = entry_size; + setEntries ancestors; while (!staged_ancestors.empty()) { const CTxMemPoolEntry& stage = staged_ancestors.begin()->get(); txiter stageit = mapTx.iterator_to(stage); - setAncestors.insert(stageit); + ancestors.insert(stageit); staged_ancestors.erase(stage); totalSizeWithAncestors += stageit->GetTxSize(); if (stageit->GetSizeWithDescendants() + entry_size > static_cast<uint64_t>(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; + return util::Error{Untranslated(strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_size_vbytes))}; } else if (stageit->GetCountWithDescendants() + entry_count > static_cast<uint64_t>(limits.descendant_count)) { - errString = strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_count); - return false; + return util::Error{Untranslated(strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_count))}; } else if (totalSizeWithAncestors > static_cast<uint64_t>(limits.ancestor_size_vbytes)) { - errString = strprintf("exceeds ancestor size limit [limit: %u]", limits.ancestor_size_vbytes); - return false; + return util::Error{Untranslated(strprintf("exceeds ancestor size limit [limit: %u]", limits.ancestor_size_vbytes))}; } const CTxMemPoolEntry::Parents& parents = stageit->GetMemPoolParentsConst(); @@ -180,17 +181,16 @@ bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, txiter parent_it = mapTx.iterator_to(parent); // If this is a new ancestor, add it. - if (setAncestors.count(parent_it) == 0) { + if (ancestors.count(parent_it) == 0) { staged_ancestors.insert(parent); } - if (staged_ancestors.size() + setAncestors.size() + entry_count > static_cast<uint64_t>(limits.ancestor_count)) { - errString = strprintf("too many unconfirmed ancestors [limit: %u]", limits.ancestor_count); - return false; + if (staged_ancestors.size() + ancestors.size() + entry_count > static_cast<uint64_t>(limits.ancestor_count)) { + return util::Error{Untranslated(strprintf("too many unconfirmed ancestors [limit: %u]", limits.ancestor_count))}; } } } - return true; + return ancestors; } bool CTxMemPool::CheckPackageLimits(const Package& package, @@ -215,20 +215,17 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, // When multiple transactions are passed in, the ancestors and descendants of all transactions // considered together must be within limits even if they are not interdependent. This may be // stricter than the limits for each individual transaction. - setEntries setAncestors; - const auto ret = CalculateAncestorsAndCheckLimits(total_size, package.size(), - setAncestors, staged_ancestors, - limits, errString); + const auto ancestors{CalculateAncestorsAndCheckLimits(total_size, package.size(), + staged_ancestors, limits)}; // It's possible to overestimate the ancestor/descendant totals. - if (!ret) errString.insert(0, "possibly "); - return ret; + if (!ancestors.has_value()) errString = "possibly " + util::ErrorString(ancestors).original; + return ancestors.has_value(); } -bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, - setEntries &setAncestors, - const Limits& limits, - std::string &errString, - bool fSearchForParents /* = true */) const +util::Result<CTxMemPool::setEntries> CTxMemPool::CalculateMemPoolAncestors( + const CTxMemPoolEntry &entry, + const Limits& limits, + bool fSearchForParents /* = true */) const { CTxMemPoolEntry::Parents staged_ancestors; const CTransaction &tx = entry.GetTx(); @@ -242,8 +239,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, if (piter) { staged_ancestors.insert(**piter); if (staged_ancestors.size() + 1 > static_cast<uint64_t>(limits.ancestor_count)) { - errString = strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count); - return false; + return util::Error{Untranslated(strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count))}; } } } @@ -254,9 +250,22 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, staged_ancestors = it->GetMemPoolParentsConst(); } - return CalculateAncestorsAndCheckLimits(entry.GetTxSize(), /*entry_count=*/1, - setAncestors, staged_ancestors, - limits, errString); + return CalculateAncestorsAndCheckLimits(entry.GetTxSize(), /*entry_count=*/1, staged_ancestors, + limits); +} + +CTxMemPool::setEntries CTxMemPool::AssumeCalculateMemPoolAncestors( + std::string_view calling_fn_name, + const CTxMemPoolEntry &entry, + const Limits& limits, + bool fSearchForParents /* = true */) const +{ + auto result{Assume(CalculateMemPoolAncestors(entry, limits, fSearchForParents))}; + if (!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, setEntries &setAncestors) @@ -320,9 +329,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b } } for (txiter removeIt : entriesToRemove) { - setEntries setAncestors; const CTxMemPoolEntry &entry = *removeIt; - std::string dummy; // Since this is a tx that is already in the mempool, we can call CMPA // with fSearchForParents = false. If the mempool is in a consistent // state, then using true or false should both be correct, though false @@ -342,10 +349,10 @@ 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, Limits::NoLimits(), dummy, false); + auto ancestors{AssumeCalculateMemPoolAncestors(__func__, entry, Limits::NoLimits(), /*fSearchForParents=*/false)}; // Note that UpdateAncestorsOf severs the child links that point to // removeIt in the entries for the parents of removeIt. - UpdateAncestorsOf(false, removeIt, setAncestors); + UpdateAncestorsOf(false, removeIt, ancestors); } // After updating all the ancestor sizes, we can now sever the link between each // transaction being removed and any mempool children (ie, update CTxMemPoolEntry::m_parents @@ -695,15 +702,13 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei assert(setParentCheck.size() == it->GetMemPoolParentsConst().size()); assert(std::equal(setParentCheck.begin(), setParentCheck.end(), it->GetMemPoolParentsConst().begin(), comp)); // Verify ancestor state is correct. - setEntries setAncestors; - std::string dummy; - CalculateMemPoolAncestors(*it, setAncestors, Limits::NoLimits(), dummy); - uint64_t nCountCheck = setAncestors.size() + 1; + auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits())}; + uint64_t nCountCheck = ancestors.size() + 1; uint64_t nSizeCheck = it->GetTxSize(); CAmount nFeesCheck = it->GetModifiedFee(); int64_t nSigOpCheck = it->GetSigOpCost(); - for (txiter ancestorIt : setAncestors) { + for (txiter ancestorIt : ancestors) { nSizeCheck += ancestorIt->GetTxSize(); nFeesCheck += ancestorIt->GetModifiedFee(); nSigOpCheck += ancestorIt->GetSigOpCost(); @@ -858,10 +863,8 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD if (it != mapTx.end()) { mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); }); // Now update all ancestors' modified fees with descendants - setEntries setAncestors; - std::string dummy; - CalculateMemPoolAncestors(*it, setAncestors, Limits::NoLimits(), dummy, false); - for (txiter ancestorIt : setAncestors) { + auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits(), /*fSearchForParents=*/false)}; + for (txiter ancestorIt : ancestors) { mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e){ e.UpdateDescendantState(0, nFeeDelta, 0);}); } // Now update all descendants' modified fees with ancestors @@ -998,10 +1001,8 @@ int CTxMemPool::Expire(std::chrono::seconds time) void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, bool validFeeEstimate) { - setEntries setAncestors; - std::string dummy; - CalculateMemPoolAncestors(entry, setAncestors, Limits::NoLimits(), dummy); - return addUnchecked(entry, setAncestors, validFeeEstimate); + auto ancestors{AssumeCalculateMemPoolAncestors(__func__, entry, Limits::NoLimits())}; + return addUnchecked(entry, ancestors, validFeeEstimate); } void CTxMemPool::UpdateChild(txiter entry, txiter child, bool add) diff --git a/src/txmempool.h b/src/txmempool.h index bd62a09a4c..d275710cea 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -11,6 +11,7 @@ #include <optional> #include <set> #include <string> +#include <string_view> #include <utility> #include <vector> @@ -28,6 +29,7 @@ #include <sync.h> #include <util/epochguard.h> #include <util/hasher.h> +#include <util/result.h> #include <boost/multi_index/hashed_index.hpp> #include <boost/multi_index/ordered_index.hpp> @@ -428,24 +430,20 @@ 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). + * and descendant limits (including staged_ancestors themselves, 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[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 + * @return all in-mempool ancestors, or an error if any ancestor or descendant limits were hit */ - bool CalculateAncestorsAndCheckLimits(size_t entry_size, - size_t entry_count, - setEntries& setAncestors, - CTxMemPoolEntry::Parents &staged_ancestors, - const Limits& limits, - std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); + util::Result<setEntries> CalculateAncestorsAndCheckLimits(size_t entry_size, + size_t entry_count, + CTxMemPoolEntry::Parents &staged_ancestors, + const Limits& limits + ) const EXCLUSIVE_LOCKS_REQUIRED(cs); public: indirectmap<COutPoint, const CTransaction*> mapNextTx GUARDED_BY(cs); @@ -558,22 +556,37 @@ public: * (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 + * @return all in-mempool ancestors, or an error if any ancestor or descendant limits were hit */ - bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, - setEntries& setAncestors, + util::Result<setEntries> CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, const Limits& limits, - std::string& errString, 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, + const Limits& limits, + 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 diff --git a/src/validation.cpp b/src/validation.cpp index 33b0cf3f7a..e24d39170e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -64,6 +64,7 @@ #include <numeric> #include <optional> #include <string> +#include <utility> using kernel::CCoinsStats; using kernel::CoinStatsHashType; @@ -867,11 +868,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) m_limits.descendant_size_vbytes += conflict->GetSizeWithDescendants(); } - std::string errString; - if (!m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, m_limits, errString)) { - ws.m_ancestors.clear(); + auto ancestors{m_pool.CalculateMemPoolAncestors(*entry, m_limits)}; + if (!ancestors) { // If CalculateMemPoolAncestors fails second time, we want the original error string. - std::string dummy_err_string; // Contracting/payment channels CPFP carve-out: // If the new transaction is relatively small (up to 40k weight) // and has at most one ancestor (ie ancestor limit of 2, including @@ -889,14 +888,16 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) .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, - cpfp_carve_out_limits, - dummy_err_string)) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", errString); + const auto error_message{util::ErrorString(ancestors).original}; + if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); } + ancestors = m_pool.CalculateMemPoolAncestors(*entry, cpfp_carve_out_limits); + if (!ancestors) return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); } + ws.m_ancestors = *ancestors; + // A transaction that spends outputs that would be replaced by it is invalid. Now // that we have the set of all ancestors we can detect this // pathological case by making sure ws.m_conflicts and ws.m_ancestors don't @@ -1111,15 +1112,18 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& // 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_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); - all_submitted = false; - package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR, - strprintf("BUG! Mempool ancestors or descendants were underestimated: %s", - ws.m_ptx->GetHash().ToString())); + { + auto ancestors{m_pool.CalculateMemPoolAncestors(*ws.m_entry, m_limits)}; + if(!ancestors) { + results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); + // Since PreChecks() and PackageMempoolChecks() both enforce limits, this should never fail. + Assume(false); + all_submitted = false; + package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR, + strprintf("BUG! Mempool ancestors or descendants were underestimated: %s", + ws.m_ptx->GetHash().ToString())); + } + ws.m_ancestors = std::move(ancestors).value_or(ws.m_ancestors); } // If we call LimitMempoolSize() for each individual Finalize(), the mempool will not take // the transaction's descendant feerate into account because it hasn't seen them yet. Also, |