From 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Mon, 24 Oct 2022 18:03:07 +0100 Subject: [PATCH] mempool: log/halt when CalculateMemPoolAncestors fails unexpectedly When CalculateMemPoolAncestors fails unexpectedly (e.g. it exceeds ancestor/descendant limits even though we expect no limits to be applied), add an error log entry for increased visibility. For debug builds, the application will even halt completely since this is not supposed to happen. --- src/node/miner.cpp | 3 +-- src/policy/rbf.cpp | 7 +++---- src/rpc/mempool.cpp | 3 +-- src/txmempool.cpp | 12 ++++-------- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/node/miner.cpp b/src/node/miner.cpp index e507c1381cf..ddb304f2106 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -394,8 +394,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele continue; } - auto ancestors_result{mempool.CalculateMemPoolAncestors(*iter, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; - auto ancestors{std::move(ancestors_result).value_or(CTxMemPool::setEntries{})}; + 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 bdaf2d0a308..5211c18743d 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -16,7 +16,6 @@ #include #include -#include #include RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) @@ -36,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. - CTxMemPoolEntry entry = *pool.mapTx.find(tx.GetHash()); - auto ancestors_result{pool.CalculateMemPoolAncestors(entry, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; - auto ancestors{std::move(ancestors_result).value_or(CTxMemPool::setEntries{})}; + 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 af980de6b91..7584c9c772a 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -451,8 +451,7 @@ static RPCHelpMan getmempoolancestors() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } - auto ancestors_result{mempool.CalculateMemPoolAncestors(*it, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; - auto ancestors{std::move(ancestors_result).value_or(CTxMemPool::setEntries{})}; + auto ancestors{mempool.AssumeCalculateMemPoolAncestors(__func__, *it, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; if (!fVerbose) { UniValue o(UniValue::VARR); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4232c6d305c..c83f89179bd 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -349,8 +349,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. - auto ancestors_result{CalculateMemPoolAncestors(entry, Limits::NoLimits(), /*fSearchForParents=*/false)}; - auto ancestors{std::move(ancestors_result).value_or(setEntries{})}; + 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, ancestors); @@ -703,8 +702,7 @@ 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. - auto ancestors_result{CalculateMemPoolAncestors(*it, Limits::NoLimits())}; - auto ancestors{std::move(ancestors_result).value_or(setEntries{})}; + auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits())}; uint64_t nCountCheck = ancestors.size() + 1; uint64_t nSizeCheck = it->GetTxSize(); CAmount nFeesCheck = it->GetModifiedFee(); @@ -865,8 +863,7 @@ 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 - auto ancestors_result{CalculateMemPoolAncestors(*it, Limits::NoLimits(), /*fSearchForParents=*/false)}; - auto ancestors{std::move(ancestors_result).value_or(setEntries{})}; + auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits(), /*fSearchForParents=*/false)}; for (txiter ancestorIt : ancestors) { mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e){ e.UpdateDescendantState(0, nFeeDelta, 0);}); } @@ -1004,8 +1001,7 @@ int CTxMemPool::Expire(std::chrono::seconds time) void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, bool validFeeEstimate) { - auto ancestors_result{CalculateMemPoolAncestors(entry, Limits::NoLimits())}; - auto ancestors{std::move(ancestors_result).value_or(CTxMemPool::setEntries{})}; + auto ancestors{AssumeCalculateMemPoolAncestors(__func__, entry, Limits::NoLimits())}; return addUnchecked(entry, ancestors, validFeeEstimate); }