← Watch feed
Eliminate use of cached ancestor data in miniminer_tests and truc_policy
Public commit record
50/100 · Thin
What the developer wrote
Authored by Suhas Daftuar
Eliminate use of cached ancestor data in miniminer_tests and truc_policy
✓ Specific, descriptive subject✓ Names a concrete action or component! No meaningful explanatory body
What changed, and why it matters
This commit refactors how Bitcoin Core's mempool counts transaction ancestors and descendants. It replaces some uses of internally cached counts with freshly calculated counts from a newer transaction graph structure. The change is mostly cleanup and test adaptation, but it touches the TRUC (v3 transaction) policy rules that limit how many related transactions can be in the mempool. There is no clear security bug fixed here, but the change is in a security-sensitive area (mempool policy), so it deserves careful review.
Recommended action
Review the new CalculateAncestorData/CalculateDescendantData implementations and the TxGraph methods they call to ensure they produce identical results to the previous cached values under all edge cases, especially around reorgs, replacement, and package acceptance. Run the updated tests and consider additional fuzzing of TRUC topology limits.
Security signals we found
01
Changes mempool ancestor/descendant counting logic used by TRUC policy
02
TRUC policy enforces transaction topology limits (descendant/ancestor counts and sizes)
03
Replaces cached values with graph-derived calculations
04
No explicit vulnerability description in commit message or diff
05
No advisory or CVE references present in supplied materials
Evidence from the diff
The commit removes direct use of CTxMemPoolEntry cached ancestor/descendant statistics (GetCountWithDescendants, GetCountWithAncestors, GetSizeWithAncestors, GetModFeesWithAncestors) in the TRUC policy implementation and in tests. Instead it introduces CTxMemPool::GetDescendantCount, GetAncestorCount, and uses CalculateAncestorData/CalculateDescendantData which compute values from m_txgraph. The TRUC policy functions SingleTRUCChecks and PackageTRUCChecks now take a CTxMemPool reference. Tests and validation callers are updated accordingly. The stated goal is to eliminate use of cached ancestor data in miniminer_tests and truc_policy.
Changed components
src/policy/truc_policy.cppsrc/policy/truc_policy.hsrc/validation.cppsrc/txmempool.hsrc/test/miniminer_tests.cppsrc/test/txvalidation_tests.cppsrc/test/util/txmempool.cppInspect captured patch +66 / −52
diff --git a/src/policy/truc_policy.cpp b/src/policy/truc_policy.cpp
index 69e8d5ed..96fbebb7 100644
--- a/src/policy/truc_policy.cpp
+++ b/src/policy/truc_policy.cpp
@@ -55,7 +55,7 @@ struct ParentInfo {
{}
};
-std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize,
+std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx, int64_t vsize,
const Package& package,
const CTxMemPool::setEntries& mempool_ancestors)
{
@@ -94,7 +94,7 @@ std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t
return ParentInfo{mempool_parent->GetTx().GetHash(),
mempool_parent->GetTx().GetWitnessHash(),
mempool_parent->GetTx().version,
- /*has_mempool_descendant=*/mempool_parent->GetCountWithDescendants() > 1};
+ /*has_mempool_descendant=*/pool.GetDescendantCount(mempool_parent) > 1};
} else {
auto& parent_index = in_package_parents.front();
auto& package_parent = package.at(parent_index);
@@ -161,7 +161,7 @@ std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t
return std::nullopt;
}
-std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTransactionRef& ptx,
+std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx,
const CTxMemPool::setEntries& mempool_ancestors,
const std::set<Txid>& direct_conflicts,
int64_t vsize)
@@ -222,14 +222,14 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
const bool child_will_be_replaced = !children.empty() &&
std::any_of(children.cbegin(), children.cend(),
[&direct_conflicts](const CTxMemPoolEntry& child){return direct_conflicts.count(child.GetTx().GetHash()) > 0;});
- if (parent_entry->GetCountWithDescendants() + 1 > TRUC_DESCENDANT_LIMIT && !child_will_be_replaced) {
+ if (pool.GetDescendantCount(parent_entry) + 1 > TRUC_DESCENDANT_LIMIT && !child_will_be_replaced) {
// Allow sibling eviction for TRUC transaction: if another child already exists, even if
// we don't conflict inputs with it, consider evicting it under RBF rules. We rely on TRUC rules
// only permitting 1 descendant, as otherwise we would need to have logic for deciding
// which descendant to evict. Skip if this isn't true, e.g. if the transaction has
// multiple children or the sibling also has descendants due to a reorg.
- const bool consider_sibling_eviction{parent_entry->GetCountWithDescendants() == 2 &&
- children.begin()->get().GetCountWithAncestors() == 2};
+ const bool consider_sibling_eviction{pool.GetDescendantCount(parent_entry) == 2 &&
+ pool.GetAncestorCount(children.begin()->get()) == 2};
// 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.
diff --git a/src/policy/truc_policy.h b/src/policy/truc_policy.h
index acd8ae56..abe52958 100644
--- a/src/policy/truc_policy.h
+++ b/src/policy/truc_policy.h
@@ -49,6 +49,7 @@ static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_DESCENDANT_SIZE_L
* 6. A TRUC tx must be within TRUC_MAX_VSIZE.
*
*
+ * @param[in] pool A reference to the mempool.
* @param[in] mempool_ancestors 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
@@ -63,7 +64,7 @@ static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_DESCENDANT_SIZE_L
* - debug string + nullptr if this transaction violates some TRUC rule and sibling eviction is not
* applicable.
*/
-std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTransactionRef& ptx,
+std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx,
const CTxMemPool::setEntries& mempool_ancestors,
const std::set<Txid>& direct_conflicts,
int64_t vsize);
@@ -89,7 +90,7 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CT
*
* @returns debug string if an error occurs, std::nullopt otherwise.
* */
-std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize,
+std::optional<std::string> PackageTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx, int64_t vsize,
const Package& package,
const CTxMemPool::setEntries& mempool_ancestors);
diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp
index 4d4cf940..28f467a4 100644
--- a/src/test/miniminer_tests.cpp
+++ b/src/test/miniminer_tests.cpp
@@ -448,14 +448,22 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup)
BOOST_CHECK(tx2_feerate > tx3_feerate);
const auto tx3_anc_feerate = CFeeRate(low_fee + med_fee + high_fee + high_fee, tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]);
const auto& tx3_entry{*Assert(pool.GetEntry(tx3->GetHash()))};
- BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_entry.GetModFeesWithAncestors(), tx3_entry.GetSizeWithAncestors()));
+
+ // Check that ancestor feerate is calculated correctly.
+ auto [dummy_count, ancestor_vsize, mod_fees] = pool.CalculateAncestorData(tx3_entry);
+ BOOST_CHECK(tx3_anc_feerate == CFeeRate(mod_fees, ancestor_vsize));
+
const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[4]);
const auto tx6_anc_feerate = CFeeRate(high_fee + low_fee + med_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]);
const auto& tx6_entry{*Assert(pool.GetEntry(tx6->GetHash()))};
- BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_entry.GetModFeesWithAncestors(), tx6_entry.GetSizeWithAncestors()));
+
+ std::tie(std::ignore, ancestor_vsize, mod_fees) = pool.CalculateAncestorData(tx6_entry);
+ BOOST_CHECK(tx6_anc_feerate == CFeeRate(mod_fees, ancestor_vsize));
const auto tx7_anc_feerate = CFeeRate(high_fee + low_fee + high_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]);
const auto& tx7_entry{*Assert(pool.GetEntry(tx7->GetHash()))};
- BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_entry.GetModFeesWithAncestors(), tx7_entry.GetSizeWithAncestors()));
+
+ std::tie(std::ignore, ancestor_vsize, mod_fees) = pool.CalculateAncestorData(tx7_entry);
+ BOOST_CHECK(tx7_anc_feerate == CFeeRate(mod_fees, ancestor_vsize));
BOOST_CHECK(tx4_feerate > tx6_anc_feerate);
BOOST_CHECK(tx4_feerate > tx7_anc_feerate);
diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp
index d7f78443..89cf2f85 100644
--- a/src/test/txvalidation_tests.cpp
+++ b/src/test/txvalidation_tests.cpp
@@ -298,14 +298,14 @@ 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(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);
Package package_v3_v2{mempool_tx_v3, tx_v2_from_v3};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_ancestors), expected_error_str);
+ 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(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), {tx_v2_from_v3}, entries_mempool_v3), expected_error_str);
// mempool_tx_v3 mempool_tx_v2
// ^ ^
@@ -315,12 +315,12 @@ 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(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);
Package package_v3_v2_v2{mempool_tx_v3, mempool_tx_v2, tx_v2_from_v2_and_v3};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(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_ancestors), expected_error_str_2);
}
// TRUC cannot spend from an unconfirmed non-TRUC transaction.
@@ -333,14 +333,14 @@ 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(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);
Package package_v2_v3{mempool_tx_v2, tx_v3_from_v2};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), package_v2_v3, empty_ancestors), expected_error_str);
+ 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(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), {tx_v3_from_v2}, entries_mempool_v2), expected_error_str);
// mempool_tx_v3 mempool_tx_v2
// ^ ^
@@ -350,7 +350,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(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);
@@ -358,7 +358,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(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_ancestors), expected_error_str_3);
}
// V3 from V3 is ok, and non-V3 from non-V3 is ok.
{
@@ -367,22 +367,22 @@ 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), m_limits)};
- BOOST_CHECK(SingleTRUCChecks(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};
- BOOST_CHECK(PackageTRUCChecks(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_ancestors) == 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), m_limits)};
- BOOST_CHECK(SingleTRUCChecks(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};
- BOOST_CHECK(PackageTRUCChecks(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_ancestors) == std::nullopt);
}
// Tx spending TRUC cannot have too many mempool ancestors
@@ -404,11 +404,11 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
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(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);
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(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_ancestors),
expected_error_str);
}
@@ -429,13 +429,13 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_multi_gen), m_limits)};
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(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);
// Middle tx is what triggers a failure for the grandchild:
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(middle_tx, GetVirtualTransactionSize(*middle_tx), package_multi_gen, empty_ancestors), expected_error_str);
- BOOST_CHECK(PackageTRUCChecks(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_ancestors), expected_error_str);
+ BOOST_CHECK(PackageTRUCChecks(pool, tx_v3_multi_gen, GetVirtualTransactionSize(*tx_v3_multi_gen), package_multi_gen, empty_ancestors) == std::nullopt);
}
// Tx spending TRUC cannot be too large in virtual size.
@@ -447,12 +447,12 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child_big), m_limits)};
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(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);
Package package_child_big{mempool_tx_v3, tx_v3_child_big};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(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_ancestors),
expected_error_str);
}
@@ -485,18 +485,18 @@ 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(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(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);
Package package_child_sigops{mempool_tx_v3, tx_many_sigops};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(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_ancestors),
expected_error_str);
}
@@ -505,11 +505,11 @@ 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), m_limits)};
- BOOST_CHECK(SingleTRUCChecks(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};
- BOOST_CHECK(PackageTRUCChecks(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_ancestors) == std::nullopt);
}
// A TRUC transaction cannot have more than 1 descendant. Sibling is returned when exactly 1 exists.
@@ -520,27 +520,27 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
auto ancestors_1sibling{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child2), m_limits)};
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(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(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};
- BOOST_CHECK_EQUAL(*PackageTRUCChecks(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_ancestors),
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.
AddToMempool(pool, entry.FromTx(tx_v3_child2));
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(entry_mempool_parent->GetCountWithDescendants(), 3);
+ BOOST_CHECK_EQUAL(pool.GetDescendantCount(entry_mempool_parent), 3);
auto ancestors_2siblings{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child3), m_limits)};
- auto result_2children{SingleTRUCChecks(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);
@@ -560,7 +560,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
auto ancestors_3gen{pool.CalculateMemPoolAncestors(entry.FromTx(tx_to_submit), m_limits)};
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(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/test/util/txmempool.cpp b/src/test/util/txmempool.cpp
index 0cb15b69..ffa21a41 100644
--- a/src/test/util/txmempool.cpp
+++ b/src/test/util/txmempool.cpp
@@ -183,24 +183,26 @@ void CheckMempoolTRUCInvariants(const CTxMemPool& tx_pool)
LOCK(tx_pool.cs);
for (const auto& tx_info : tx_pool.infoAll()) {
const auto& entry = *Assert(tx_pool.GetEntry(tx_info.tx->GetHash()));
+ auto [desc_count, desc_size, desc_fees] = tx_pool.CalculateDescendantData(entry);
+ auto [anc_count, anc_size, anc_fees] = tx_pool.CalculateAncestorData(entry);
+
if (tx_info.tx->version == TRUC_VERSION) {
// Check that special maximum virtual size is respected
Assert(entry.GetTxSize() <= TRUC_MAX_VSIZE);
// Check that special TRUC ancestor/descendant limits and rules are always respected
- Assert(entry.GetCountWithDescendants() <= TRUC_DESCENDANT_LIMIT);
- Assert(entry.GetCountWithAncestors() <= TRUC_ANCESTOR_LIMIT);
- Assert(entry.GetSizeWithDescendants() <= TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE);
- Assert(entry.GetSizeWithAncestors() <= TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE);
-
+ Assert(desc_count <= TRUC_DESCENDANT_LIMIT);
+ Assert(anc_count <= TRUC_ANCESTOR_LIMIT);
+ Assert(desc_size <= TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE);
+ Assert(anc_size <= TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE);
// If this transaction has at least 1 ancestor, it's a "child" and has restricted weight.
- if (entry.GetCountWithAncestors() > 1) {
+ if (anc_count > 1) {
Assert(entry.GetTxSize() <= TRUC_CHILD_MAX_VSIZE);
// All TRUC transactions must only have TRUC unconfirmed parents.
const auto& parents = entry.GetMemPoolParentsConst();
Assert(parents.begin()->get().GetSharedTx()->version == TRUC_VERSION);
}
- } else if (entry.GetCountWithAncestors() > 1) {
+ } else if (anc_count > 1) {
// All non-TRUC transactions must only have non-TRUC unconfirmed parents.
for (const auto& parent : entry.GetMemPoolParentsConst()) {
Assert(parent.get().GetSharedTx()->version != TRUC_VERSION);
diff --git a/src/txmempool.h b/src/txmempool.h
index 054d9e12..9a4389c3 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -286,6 +286,9 @@ public:
std::tuple<size_t, size_t, CAmount> CalculateAncestorData(const CTxMemPoolEntry& entry) const EXCLUSIVE_LOCKS_REQUIRED(cs);
std::tuple<size_t, size_t, CAmount> CalculateDescendantData(const CTxMemPoolEntry& entry) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+ int64_t GetDescendantCount(txiter it) const { LOCK(cs); return m_txgraph->GetDescendants(*it, TxGraph::Level::MAIN).size(); }
+ int64_t GetDescendantCount(const CTxMemPoolEntry &e) const { LOCK(cs); return m_txgraph->GetDescendants(e, TxGraph::Level::MAIN).size(); }
+ int64_t GetAncestorCount(const CTxMemPoolEntry &e) const { LOCK(cs); return m_txgraph->GetAncestors(e, TxGraph::Level::MAIN).size(); }
private:
typedef std::map<txiter, setEntries, CompareIteratorByHash> cacheMap;
diff --git a/src/validation.cpp b/src/validation.cpp
index ac820552..348e6102 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -972,7 +972,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// 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(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
+ if (const auto err{SingleTRUCChecks(m_pool, ws.m_ptx, ws.m_ancestors, 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.
@@ -1498,7 +1498,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactionsInternal(con
// At this point we have all in-mempool ancestors, and we know every transaction's vsize.
// Run the TRUC checks on the package.
for (Workspace& ws : workspaces) {
- if (auto err{PackageTRUCChecks(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_ancestors)}) {
package_state.Invalid(PackageValidationResult::PCKG_POLICY, "TRUC-violation", err.value());
return PackageMempoolAcceptResult(package_state, {});
}
Risk score
Our methodology →Why this scored 28/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.