Public commit record
90/100 · Strong
What the developer wrote
Authored by Suhas Daftuar
Rework RBF and TRUC validation
Calculating mempool ancestors for a new transaction should not be done until
after cluster size limits have been enforced, to limit CPU DoS potential.
Achieve this by reworking TRUC and RBF validation logic:
- TRUC policy enforcement is now done using only mempool parents of
new transactions, not all mempool ancestors (note that it's fine to calculate
ancestors of in-mempool transactions, if the number of such calls is
reasonably bounded).
- RBF replacement checks are performed earlier (which allows for checking
cluster size limits earlier, because cluster size checks cannot happen until
after all conflicts are staged for removal).
- Verifying that a new transaction doesn't conflict with an ancestor now
happens later, in AcceptSingleTransaction() rather than in PreChecks(). This
means that the test is not performed at all in AcceptMultipleTransactions(),
but in package acceptance we already disallow RBF in situations where a
package transaction has in-mempool parents.
Also to ensure that all RBF validation logic is applied in both the single
transaction and multiple transaction cases, remove the optimization that skips
the PackageMempoolChecks() in the case of a single transaction being validated
in AcceptMultipleTransactions().
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification
What changed, and why it matters
This Bitcoin Core commit reorders and reworks how two new transaction-handling features—RBF (Replace-By-Fee) and TRUC (a restricted version-3 transaction policy)—are validated. The main goal is to avoid doing expensive 'ancestor' calculations on the whole mempool until cheaper cluster-size checks have already run, so a maliciously crafted transaction can't waste a node's CPU as easily. It also makes sure the same RBF checks apply whether one transaction or a whole package is being accepted, and moves a 'spends a transaction it is replacing' check to a later, safer point.
Recommended action
Treat this as a hardening/DoS-prevention patch. Reviewers should verify that the new parent-only TRUC checks still enforce the intended 2-ancestor limit in all cases, that moving the conflict-with-ancestor check later does not create a window for inconsistent state, and that package RBF behavior is unchanged for legitimate users. Node operators should upgrade when the release containing this commit is available.
Security signals we found
01
CPU DoS mitigation: expensive ancestor calculation deferred until after cluster size limits
02
TRUC validation narrowed from full ancestors to direct parents
03
RBF checks moved earlier and unified across single/package acceptance
04
Conflict-with-ancestor check relocated to after cluster limit enforcement
05
Removal of single-tx shortcut in package RBF checks
Evidence from the diff
The patch changes mempool validation so that TRUC policy is enforced using only direct mempool parents instead of the full ancestor set, and RBF replacement checks are performed earlier. It moves the ‘tx spends a conflicting ancestor’ check from PreChecks() into AcceptSingleTransactionInternal(), after cluster-size limits are verified. It also removes the optimization that skipped PackageMempoolChecks() for single-transaction package acceptance, ensuring RBF validation is consistent across single and multiple transaction paths. New helper CTxMemPool::GetParents() is used, and CalculateDescendants() gains an overload taking a const CTxMemPoolEntry&.
Changed components
src/policy/truc_policy.cppsrc/policy/truc_policy.hsrc/validation.cppsrc/txmempool.cppsrc/txmempool.hsrc/test/txvalidation_tests.cppInspect captured patch +137 / −111
diff --git a/src/policy/truc_policy.cpp b/src/policy/truc_policy.cpp
index 853ccd58..c5ba866a 100644
--- a/src/policy/truc_policy.cpp
+++ b/src/policy/truc_policy.cpp
@@ -57,7 +57,7 @@ struct ParentInfo {
std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx, int64_t vsize,
const Package& package,
- const CTxMemPool::setEntries& mempool_ancestors)
+ const std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef>& mempool_parents)
{
// This function is specialized for these limits, and must be reimplemented if they ever change.
static_assert(TRUC_ANCESTOR_LIMIT == 2);
@@ -65,7 +65,7 @@ std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTran
const auto in_package_parents{FindInPackageParents(package, ptx)};
- // Now we have all ancestors, so we can start checking TRUC rules.
+ // Now we have all parents, so we can start checking TRUC rules.
if (ptx->version == TRUC_VERSION) {
// SingleTRUCChecks should have checked this already.
if (!Assume(vsize <= TRUC_MAX_VSIZE)) {
@@ -73,12 +73,19 @@ std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTran
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, TRUC_MAX_VSIZE);
}
- if (mempool_ancestors.size() + in_package_parents.size() + 1 > TRUC_ANCESTOR_LIMIT) {
+ if (mempool_parents.size() + in_package_parents.size() + 1 > TRUC_ANCESTOR_LIMIT) {
return strprintf("tx %s (wtxid=%s) would have too many ancestors",
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
}
- const bool has_parent{mempool_ancestors.size() + in_package_parents.size() > 0};
+ if (mempool_parents.size()) {
+ if (pool.GetAncestorCount(mempool_parents[0]) + in_package_parents.size() + 1 > TRUC_ANCESTOR_LIMIT) {
+ return strprintf("tx %s (wtxid=%s) would have too many ancestors",
+ ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
+ }
+ }
+
+ const bool has_parent{mempool_parents.size() + in_package_parents.size() > 0};
if (has_parent) {
// A TRUC child cannot be too large.
if (vsize > TRUC_CHILD_MAX_VSIZE) {
@@ -89,12 +96,12 @@ std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTran
// Exactly 1 parent exists, either in mempool or package. Find it.
const auto parent_info = [&] {
- if (mempool_ancestors.size() > 0) {
- auto& mempool_parent = *mempool_ancestors.begin();
+ if (mempool_parents.size() > 0) {
+ const auto& mempool_parent = &mempool_parents[0].get();
return ParentInfo{mempool_parent->GetTx().GetHash(),
mempool_parent->GetTx().GetWitnessHash(),
mempool_parent->GetTx().version,
- /*has_mempool_descendant=*/pool.GetDescendantCount(mempool_parent) > 1};
+ /*has_mempool_descendant=*/pool.GetDescendantCount(*mempool_parent) > 1};
} else {
auto& parent_index = in_package_parents.front();
auto& package_parent = package.at(parent_index);
@@ -141,11 +148,11 @@ std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTran
}
} else {
// Non-TRUC transactions cannot have TRUC parents.
- for (auto it : mempool_ancestors) {
- if (it->GetTx().version == TRUC_VERSION) {
+ for (auto it : mempool_parents) {
+ if (it.get().GetTx().version == TRUC_VERSION) {
return strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)",
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
- it->GetSharedTx()->GetHash().ToString(), it->GetSharedTx()->GetWitnessHash().ToString());
+ it.get().GetSharedTx()->GetHash().ToString(), it.get().GetSharedTx()->GetWitnessHash().ToString());
}
}
for (const auto& index: in_package_parents) {
@@ -162,13 +169,14 @@ std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTran
}
std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx,
- const CTxMemPool::setEntries& mempool_ancestors,
+ const std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef>& mempool_parents,
const std::set<Txid>& direct_conflicts,
int64_t vsize)
{
LOCK(pool.cs);
// Check TRUC and non-TRUC inheritance.
- for (const auto& entry : mempool_ancestors) {
+ for (const auto& entry_ref : mempool_parents) {
+ const auto& entry = &entry_ref.get();
if (ptx->version != TRUC_VERSION && entry->GetTx().version == TRUC_VERSION) {
return std::make_pair(strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)",
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
@@ -196,14 +204,21 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
}
// Check that TRUC_ANCESTOR_LIMIT would not be violated.
- if (mempool_ancestors.size() + 1 > TRUC_ANCESTOR_LIMIT) {
+ if (mempool_parents.size() + 1 > TRUC_ANCESTOR_LIMIT) {
return std::make_pair(strprintf("tx %s (wtxid=%s) would have too many ancestors",
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()),
nullptr);
}
// Remaining checks only pertain to transactions with unconfirmed ancestors.
- if (mempool_ancestors.size() > 0) {
+ if (mempool_parents.size() > 0) {
+ // Ensure that the in-mempool parent doesn't have any additional
+ // ancestors, as that would also be a violation.
+ if (pool.GetAncestorCount(mempool_parents[0]) + 1 > TRUC_ANCESTOR_LIMIT) {
+ return std::make_pair(strprintf("tx %s (wtxid=%s) would have too many ancestors",
+ ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()),
+ nullptr);
+ }
// If this transaction spends TRUC parents, it cannot be too large.
if (vsize > TRUC_CHILD_MAX_VSIZE) {
return std::make_pair(strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
@@ -211,14 +226,14 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
nullptr);
}
- // Check the descendant counts of in-mempool ancestors.
- const auto& parent_entry = *mempool_ancestors.begin();
- // If there are any ancestors, this is the only child allowed. The parent cannot have any
+ // Check the descendant counts of in-mempool parents.
+ const auto& parent_entry = mempool_parents[0].get();
+ // If there are any parents, this is the only child allowed. The parent cannot have any
// other descendants. We handle the possibility of multiple children as that case is
// possible through a reorg.
CTxMemPool::setEntries descendants;
- pool.CalculateDescendants(parent_entry, descendants);
- descendants.erase(parent_entry);
+ auto parent_it = pool.CalculateDescendants(parent_entry, descendants);
+ descendants.erase(parent_it);
// Don't double-count a transaction that is going to be replaced. This logic assumes that
// any descendant of the TRUC transaction is a direct child, which makes sense because a
// TRUC transaction can only have 1 descendant.
@@ -237,8 +252,8 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
// Return the sibling if its eviction can be considered. Provide the "descendant count
// limit" string either way, as the caller may decide not to do sibling eviction.
return std::make_pair(strprintf("tx %u (wtxid=%s) would exceed descendant count limit",
- parent_entry->GetSharedTx()->GetHash().ToString(),
- parent_entry->GetSharedTx()->GetWitnessHash().ToString()),
+ parent_entry.GetSharedTx()->GetHash().ToString(),
+ parent_entry.GetSharedTx()->GetWitnessHash().ToString()),
consider_sibling_eviction ? (*descendants.begin())->GetSharedTx() : nullptr);
}
}
diff --git a/src/policy/truc_policy.h b/src/policy/truc_policy.h
index abe52958..0ffd0751 100644
--- a/src/policy/truc_policy.h
+++ b/src/policy/truc_policy.h
@@ -50,7 +50,7 @@ static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_DESCENDANT_SIZE_L
*
*
* @param[in] pool A reference to the mempool.
- * @param[in] mempool_ancestors The in-mempool ancestors of ptx.
+ * @param[in] mempool_parents The in-mempool ancestors of ptx.
* @param[in] direct_conflicts In-mempool transactions this tx conflicts with. These conflicts
* are used to more accurately calculate the resulting descendant
* count of in-mempool ancestors.
@@ -65,7 +65,7 @@ static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_DESCENDANT_SIZE_L
* applicable.
*/
std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx,
- const CTxMemPool::setEntries& mempool_ancestors,
+ const std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef>& mempool_parents,
const std::set<Txid>& direct_conflicts,
int64_t vsize);
@@ -92,6 +92,6 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
* */
std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx, int64_t vsize,
const Package& package,
- const CTxMemPool::setEntries& mempool_ancestors);
+ const std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef>& mempool_parents);
#endif // BITCOIN_POLICY_TRUC_POLICY_H
diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp
index c0c9cc9c..3271838e 100644
--- a/src/test/txvalidation_tests.cpp
+++ b/src/test/txvalidation_tests.cpp
@@ -115,7 +115,6 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup)
CTxMemPool& pool = *Assert(m_node.mempool);
LOCK2(cs_main, pool.cs);
TestMemPoolEntryHelper entry;
- CTxMemPool::setEntries empty_ancestors;
TxValidationState child_state;
Wtxid child_wtxid;
@@ -279,7 +278,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
LOCK2(cs_main, pool.cs);
TestMemPoolEntryHelper entry;
std::set<Txid> empty_conflicts_set;
- CTxMemPool::setEntries empty_ancestors;
+ std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef> empty_parents;
auto mempool_tx_v3 = make_tx(random_outpoints(1), /*version=*/3);
AddToMempool(pool, entry.FromTx(mempool_tx_v3));
@@ -292,33 +291,32 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
// ^
// tx_v2_from_v3
auto tx_v2_from_v3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}}, /*version=*/2);
- auto ancestors_v2_from_v3{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v2_from_v3))};
+ auto parents_v2_from_v3 = pool.GetParents(entry.FromTx(tx_v2_from_v3));
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, parents_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);
Package package_v3_v2{mempool_tx_v3, tx_v2_from_v3};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_ancestors), expected_error_str);
- CTxMemPool::setEntries entries_mempool_v3{pool.GetIter(mempool_tx_v3->GetHash()).value()};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), {tx_v2_from_v3}, entries_mempool_v3), expected_error_str);
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_parents), expected_error_str);
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), {tx_v2_from_v3}, parents_v2_from_v3), expected_error_str);
// mempool_tx_v3 mempool_tx_v2
// ^ ^
// tx_v2_from_v2_and_v3
auto tx_v2_from_v2_and_v3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}, COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/2);
- auto ancestors_v2_from_both{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v2_from_v2_and_v3))};
+ auto parents_2_from_both{pool.GetParents(entry.FromTx(tx_v2_from_v2_and_v3))};
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, parents_2_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);
Package package_v3_v2_v2{mempool_tx_v3, mempool_tx_v2, tx_v2_from_v2_and_v3};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v2_from_v2_and_v3, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3), package_v3_v2_v2, empty_ancestors), expected_error_str_2);
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v2_from_v2_and_v3, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3), package_v3_v2_v2, empty_parents), expected_error_str_2);
}
// TRUC cannot spend from an unconfirmed non-TRUC transaction.
@@ -327,28 +325,27 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
// ^
// tx_v3_from_v2
auto tx_v3_from_v2 = make_tx({COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/3);
- auto ancestors_v3_from_v2{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_from_v2))};
+ auto parents_v3_from_v2{pool.GetParents(entry.FromTx(tx_v3_from_v2))};
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, parents_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);
Package package_v2_v3{mempool_tx_v2, tx_v3_from_v2};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), package_v2_v3, empty_ancestors), expected_error_str);
- CTxMemPool::setEntries entries_mempool_v2{pool.GetIter(mempool_tx_v2->GetHash()).value()};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), {tx_v3_from_v2}, entries_mempool_v2), expected_error_str);
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), package_v2_v3, empty_parents), expected_error_str);
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), {tx_v3_from_v2}, parents_v3_from_v2), expected_error_str);
// mempool_tx_v3 mempool_tx_v2
// ^ ^
// tx_v3_from_v2_and_v3
auto tx_v3_from_v2_and_v3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}, COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/3);
- auto ancestors_v3_from_both{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_from_v2_and_v3))};
+ auto parents_v3_from_both{pool.GetParents(entry.FromTx(tx_v3_from_v2_and_v3))};
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, parents_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);
@@ -356,7 +353,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
const auto expected_error_str_3{strprintf("tx %s (wtxid=%s) would have too many ancestors",
tx_v3_from_v2_and_v3->GetHash().ToString(), tx_v3_from_v2_and_v3->GetWitnessHash().ToString())};
Package package_v3_v2_v3{mempool_tx_v3, mempool_tx_v2, tx_v3_from_v2_and_v3};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_from_v2_and_v3, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3), package_v3_v2_v3, empty_ancestors), expected_error_str_3);
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_from_v2_and_v3, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3), package_v3_v2_v3, empty_parents), expected_error_str_3);
}
// V3 from V3 is ok, and non-V3 from non-V3 is ok.
{
@@ -364,23 +361,23 @@ 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))
+ auto parents_v3{pool.GetParents(entry.FromTx(tx_v3_from_v3))};
+ BOOST_CHECK(SingleTRUCChecks(pool, tx_v3_from_v3, parents_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v3))
== std::nullopt);
Package package_v3_v3{mempool_tx_v3, tx_v3_from_v3};
- BOOST_CHECK(PackageTRUCChecks(pool, tx_v3_from_v3, GetVirtualTransactionSize(*tx_v3_from_v3), package_v3_v3, empty_ancestors) == std::nullopt);
+ BOOST_CHECK(PackageTRUCChecks(pool, tx_v3_from_v3, GetVirtualTransactionSize(*tx_v3_from_v3), package_v3_v3, empty_parents) == std::nullopt);
// mempool_tx_v2
// ^
// 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))
+ auto parents_v2{pool.GetParents(entry.FromTx(tx_v2_from_v2))};
+ BOOST_CHECK(SingleTRUCChecks(pool, tx_v2_from_v2, parents_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2))
== std::nullopt);
Package package_v2_v2{mempool_tx_v2, tx_v2_from_v2};
- BOOST_CHECK(PackageTRUCChecks(pool, tx_v2_from_v2, GetVirtualTransactionSize(*tx_v2_from_v2), package_v2_v2, empty_ancestors) == std::nullopt);
+ BOOST_CHECK(PackageTRUCChecks(pool, tx_v2_from_v2, GetVirtualTransactionSize(*tx_v2_from_v2), package_v2_v2, empty_parents) == std::nullopt);
}
// Tx spending TRUC cannot have too many mempool ancestors
@@ -398,15 +395,15 @@ 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);
+ auto parents{pool.GetParents(entry.FromTx(tx_v3_multi_parent))};
+ BOOST_CHECK_EQUAL(parents.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, parents, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent))};
BOOST_CHECK_EQUAL(result->first, expected_error_str);
BOOST_CHECK_EQUAL(result->second, nullptr);
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_multi_parent, GetVirtualTransactionSize(*tx_v3_multi_parent), package_multi_parents, empty_ancestors),
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_multi_parent, GetVirtualTransactionSize(*tx_v3_multi_parent), package_multi_parents, empty_parents),
expected_error_str);
}
@@ -424,16 +421,16 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
}
auto tx_v3_multi_gen = make_tx({last_outpoint}, /*version=*/3);
package_multi_gen.emplace_back(tx_v3_multi_gen);
- auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_multi_gen))};
+ auto parents{pool.GetParents(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, parents, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen))};
BOOST_CHECK_EQUAL(result->first, expected_error_str);
BOOST_CHECK_EQUAL(result->second, nullptr);
// Middle tx is what triggers a failure for the grandchild:
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, middle_tx, GetVirtualTransactionSize(*middle_tx), package_multi_gen, empty_ancestors), expected_error_str);
- BOOST_CHECK(PackageTRUCChecks(pool, tx_v3_multi_gen, GetVirtualTransactionSize(*tx_v3_multi_gen), package_multi_gen, empty_ancestors) == std::nullopt);
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, middle_tx, GetVirtualTransactionSize(*middle_tx), package_multi_gen, empty_parents), expected_error_str);
+ BOOST_CHECK(PackageTRUCChecks(pool, tx_v3_multi_gen, GetVirtualTransactionSize(*tx_v3_multi_gen), package_multi_gen, empty_parents) == std::nullopt);
}
// Tx spending TRUC cannot be too large in virtual size.
@@ -442,15 +439,15 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
{
auto tx_v3_child_big = make_tx(many_inputs, /*version=*/3);
const auto vsize{GetVirtualTransactionSize(*tx_v3_child_big)};
- auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child_big))};
+ auto parents{pool.GetParents(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, parents, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big))};
BOOST_CHECK_EQUAL(result->first, expected_error_str);
BOOST_CHECK_EQUAL(result->second, nullptr);
Package package_child_big{mempool_tx_v3, tx_v3_child_big};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_child_big, GetVirtualTransactionSize(*tx_v3_child_big), package_child_big, empty_ancestors),
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_child_big, GetVirtualTransactionSize(*tx_v3_child_big), package_child_big, empty_parents),
expected_error_str);
}
@@ -477,24 +474,24 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
mtx_many_sigops.vout.back().nValue = 10000;
auto tx_many_sigops{MakeTransactionRef(mtx_many_sigops)};
- auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_many_sigops))};
+ auto parents{pool.GetParents(entry.FromTx(tx_many_sigops))};
// legacy uses fAccurate = false, and the maximum number of multisig keys is used
const int64_t total_sigops{static_cast<int64_t>(tx_many_sigops->vin.size()) * static_cast<int64_t>(script_multisig.GetSigOpCount(/*fAccurate=*/false))};
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, parents, 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, parents, 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);
Package package_child_sigops{mempool_tx_v3, tx_many_sigops};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_many_sigops, total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, package_child_sigops, empty_ancestors),
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_many_sigops, total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, package_child_sigops, empty_parents),
expected_error_str);
}
@@ -502,32 +499,32 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
auto tx_mempool_v3_child = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}}, /*version=*/3);
{
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);
+ auto parents{pool.GetParents(entry.FromTx(tx_mempool_v3_child))};
+ BOOST_CHECK(SingleTRUCChecks(pool, tx_mempool_v3_child, parents, 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};
- BOOST_CHECK(PackageTRUCChecks(pool, tx_mempool_v3_child, GetVirtualTransactionSize(*tx_mempool_v3_child), package_v3_1p1c, empty_ancestors) == std::nullopt);
+ BOOST_CHECK(PackageTRUCChecks(pool, tx_mempool_v3_child, GetVirtualTransactionSize(*tx_mempool_v3_child), package_v3_1p1c, empty_parents) == std::nullopt);
}
// A TRUC transaction cannot have more than 1 descendant. Sibling is returned when exactly 1 exists.
{
auto tx_v3_child2 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 1}}, /*version=*/3);
// Configuration where parent already has 1 other child in mempool
- auto ancestors_1sibling{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child2))};
+ auto parents_1sibling{pool.GetParents(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, parents_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, parents_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};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_child2, GetVirtualTransactionSize(*tx_v3_child2), package_v3_1p2c, empty_ancestors),
+ BOOST_CHECK_EQUAL(*PackageTRUCChecks(pool, tx_v3_child2, GetVirtualTransactionSize(*tx_v3_child2), package_v3_1p2c, empty_parents),
expected_error_str);
// Configuration where parent already has 2 other children in mempool (no sibling eviction allowed). This may happen as the result of a reorg.
@@ -535,9 +532,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
auto tx_v3_child3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 24}}, /*version=*/3);
auto entry_mempool_parent = pool.GetIter(mempool_tx_v3->GetHash()).value();
BOOST_CHECK_EQUAL(pool.GetDescendantCount(entry_mempool_parent), 3);
- auto ancestors_2siblings{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child3))};
+ auto parents_2siblings{pool.GetParents(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, parents_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);
@@ -554,10 +551,10 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
AddToMempool(pool, entry.FromTx(tx_mempool_sibling));
AddToMempool(pool, entry.FromTx(tx_mempool_nibling));
- auto ancestors_3gen{pool.CalculateMemPoolAncestors(entry.FromTx(tx_to_submit))};
+ auto parents_3gen{pool.GetParents(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, parents_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 fb835b1f..99b7d3ef 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -358,9 +358,16 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
// Calculates descendants of given entry and adds to setDescendants.
void CTxMemPool::CalculateDescendants(txiter entryit, setEntries& setDescendants) const
{
- for (auto tx : m_txgraph->GetDescendants(*entryit, TxGraph::Level::MAIN)) {
+ (void)CalculateDescendants(*entryit, setDescendants);
+ return;
+}
+
+CTxMemPool::txiter CTxMemPool::CalculateDescendants(const CTxMemPoolEntry& entry, setEntries& setDescendants) const
+{
+ for (auto tx : m_txgraph->GetDescendants(entry, TxGraph::Level::MAIN)) {
setDescendants.insert(mapTx.iterator_to(static_cast<const CTxMemPoolEntry&>(*tx)));
}
+ return mapTx.iterator_to(entry);
}
void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason)
diff --git a/src/txmempool.h b/src/txmempool.h
index addf3e13..437bdf4e 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -451,10 +451,11 @@ public:
util::Result<void> CheckPackageLimits(const Package& package,
int64_t total_vsize) const EXCLUSIVE_LOCKS_REQUIRED(cs);
- /** Populate setDescendants with all in-mempool descendants of hash.
+ /** Populate setDescendants with all in-mempool descendants of given transaction.
* Assumes that setDescendants includes all in-mempool descendants of anything
* already in it. */
void CalculateDescendants(txiter it, setEntries& setDescendants) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+ CTxMemPool::txiter CalculateDescendants(const CTxMemPoolEntry& entry, setEntries& setDescendants) const EXCLUSIVE_LOCKS_REQUIRED(cs);
/** The minimum fee to get into the mempool, which may itself not be enough
* for larger-sized transactions.
diff --git a/src/validation.cpp b/src/validation.cpp
index 6a02acd5..62b629d4 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -632,8 +632,8 @@ private:
/** Iterators to mempool entries that this transaction directly conflicts with or may
* replace via sibling eviction. */
CTxMemPool::setEntries m_iters_conflicting;
- /** All mempool ancestors of this transaction. */
- CTxMemPool::setEntries m_ancestors;
+ /** All mempool parents of this transaction. */
+ std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef> m_parents;
/* Handle to the tx in the changeset */
CTxMemPool::ChangeSet::TxHandle m_tx_handle;
/** Whether RBF-related data structures (m_conflicts, m_iters_conflicting,
@@ -961,18 +961,15 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
ws.m_iters_conflicting = m_pool.GetIterSet(ws.m_conflicts);
- ws.m_ancestors = m_subpackage.m_changeset->CalculateMemPoolAncestors(ws.m_tx_handle);
+ ws.m_parents = m_pool.GetParents(*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
- // already calculated.
if (!args.m_bypass_limits) {
- if (const auto err{SingleTRUCChecks(m_pool, ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
+ // Perform the TRUC checks, using the in-mempool parents.
+ if (const auto err{SingleTRUCChecks(m_pool, ws.m_ptx, ws.m_parents, ws.m_conflicts, ws.m_vsize)}) {
// Single transaction contexts only.
if (args.m_allow_sibling_eviction && err->second != nullptr) {
// We should only be considering where replacement is considered valid as well.
Assume(args.m_allow_replacement);
-
// Potential sibling eviction. Add the sibling to our list of mempool conflicts to be
// included in RBF checks.
ws.m_conflicts.insert(err->second->GetHash());
@@ -991,16 +988,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
}
}
- // 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
- // intersect.
- if (const auto err_string{EntriesAndTxidsDisjoint(ws.m_ancestors, ws.m_conflicts, hash)}) {
- // We classify this as a consensus error because a transaction depending on something it
- // conflicts with would be inconsistent.
- return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string);
- }
-
// We want to detect conflicts in any tx in a package to trigger package RBF logic
m_subpackage.m_rbf |= !ws.m_conflicts.empty();
return true;
@@ -1086,12 +1073,16 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: package must be 1-parent-1-child");
}
- // If the package has in-mempool ancestors, we won't consider a package RBF
+ // If the package has in-mempool parents, we won't consider a package RBF
// since it would result in a cluster larger than 2.
// N.B. To relax this constraint we will need to revisit how CCoinsViewMemPool::PackageAddTransaction
// is being used inside AcceptMultipleTransactions to track available inputs while processing a package.
+ // Specifically we would need to check that the ancestors of the new
+ // transactions don't intersect with the set of transactions to be removed
+ // due to RBF, which is not checked at all in the package acceptance
+ // context.
for (const auto& ws : workspaces) {
- if (!ws.m_ancestors.empty()) {
+ if (!ws.m_parents.empty()) {
return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: new transaction cannot have mempool ancestors");
}
}
@@ -1115,7 +1106,6 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
"package RBF failed: too many potential replacements", *err_string);
}
-
for (CTxMemPool::txiter it : all_conflicts) {
m_subpackage.m_changeset->StageRemoval(it);
m_subpackage.m_conflicting_fees += it->GetModifiedFee();
@@ -1361,15 +1351,6 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransactionInternal(const CTransa
return MempoolAcceptResult::Failure(ws.m_state);
}
- m_subpackage.m_total_vsize = ws.m_vsize;
- m_subpackage.m_total_modified_fees = ws.m_modified_fees;
-
- // Individual modified feerate exceeded caller-defined max; abort
- if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) {
- ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "max feerate exceeded", "");
- return MempoolAcceptResult::Failure(ws.m_state);
- }
-
if (m_subpackage.m_rbf && !ReplacementChecks(ws)) {
if (ws.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) {
// Failed for incentives-based fee reasons. Provide the effective feerate and which tx was included.
@@ -1384,6 +1365,32 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransactionInternal(const CTransa
return MempoolAcceptResult::Failure(ws.m_state);
}
+ // Now that we've verified the cluster limit is respected, we can perform
+ // calculations involving the full ancestors of the tx.
+ if (ws.m_conflicts.size()) {
+ auto ancestors = m_subpackage.m_changeset->CalculateMemPoolAncestors(ws.m_tx_handle);
+
+ // 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 this tx's ancestors don't
+ // intersect.
+ if (const auto err_string{EntriesAndTxidsDisjoint(ancestors, ws.m_conflicts, ptx->GetHash())}) {
+ // We classify this as a consensus error because a transaction depending on something it
+ // conflicts with would be inconsistent.
+ ws.m_state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string);
+ return MempoolAcceptResult::Failure(ws.m_state);
+ }
+ }
+
+ m_subpackage.m_total_vsize = ws.m_vsize;
+ m_subpackage.m_total_modified_fees = ws.m_modified_fees;
+
+ // Individual modified feerate exceeded caller-defined max; abort
+ if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) {
+ ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "max feerate exceeded", "");
+ return MempoolAcceptResult::Failure(ws.m_state);
+ }
+
if (m_pool.m_opts.require_standard) {
Wtxid dummy_wtxid;
if (!CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool, ws.m_state, dummy_wtxid)) {
@@ -1490,10 +1497,10 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactionsInternal(con
m_viewmempool.PackageAddTransaction(ws.m_ptx);
}
- // At this point we have all in-mempool ancestors, and we know every transaction's vsize.
+ // At this point we have all in-mempool parents, and we know every transaction's vsize.
// Run the TRUC checks on the package.
for (Workspace& ws : workspaces) {
- if (auto err{PackageTRUCChecks(m_pool, ws.m_ptx, ws.m_vsize, txns, ws.m_ancestors)}) {
+ if (auto err{PackageTRUCChecks(m_pool, ws.m_ptx, ws.m_vsize, txns, ws.m_parents)}) {
package_state.Invalid(PackageValidationResult::PCKG_POLICY, "TRUC-violation", err.value());
return PackageMempoolAcceptResult(package_state, {});
}
@@ -1525,9 +1532,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactionsInternal(con
MempoolAcceptResult::FeeFailure(placeholder_state, CFeeRate(m_subpackage.m_total_modified_fees, m_subpackage.m_total_vsize), all_package_wtxids)}});
}
- // Apply package mempool ancestor/descendant limits. Skip if there is only one transaction,
- // because it's unnecessary.
- if (txns.size() > 1 && !PackageMempoolChecks(txns, workspaces, m_subpackage.m_total_vsize, package_state)) {
+ // Apply package mempool RBF checks.
+ if (!PackageMempoolChecks(txns, workspaces, m_subpackage.m_total_vsize, package_state)) {
return PackageMempoolAcceptResult(package_state, std::move(results));
}
Risk score
Our methodology →Why this scored 63/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.