From b9a2039f51226dce2c4e38ce5f26eefee171744b Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Sun, 14 Apr 2024 13:49:03 -0400 Subject: [PATCH] Eliminate use of cached ancestor data in miniminer_tests and truc_policy --- src/policy/truc_policy.cpp | 12 +++---- src/policy/truc_policy.h | 5 +-- src/test/miniminer_tests.cpp | 14 ++++++-- src/test/txvalidation_tests.cpp | 64 ++++++++++++++++----------------- src/test/util/txmempool.cpp | 16 +++++---- src/txmempool.h | 3 ++ src/validation.cpp | 4 +-- 7 files changed, 66 insertions(+), 52 deletions(-) diff --git a/src/policy/truc_policy.cpp b/src/policy/truc_policy.cpp index 69e8d5ed1d2..96fbebb7220 100644 --- a/src/policy/truc_policy.cpp +++ b/src/policy/truc_policy.cpp @@ -55,7 +55,7 @@ struct ParentInfo { {} }; -std::optional PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize, +std::optional PackageTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx, int64_t vsize, const Package& package, const CTxMemPool::setEntries& mempool_ancestors) { @@ -94,7 +94,7 @@ std::optional 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 PackageTRUCChecks(const CTransactionRef& ptx, int64_t return std::nullopt; } -std::optional> SingleTRUCChecks(const CTransactionRef& ptx, +std::optional> SingleTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx, const CTxMemPool::setEntries& mempool_ancestors, const std::set& direct_conflicts, int64_t vsize) @@ -222,14 +222,14 @@ std::optional> 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 acd8ae56b1c..abe5295876d 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> SingleTRUCChecks(const CTransactionRef& ptx, +std::optional> SingleTRUCChecks(const CTxMemPool& pool, const CTransactionRef& ptx, const CTxMemPool::setEntries& mempool_ancestors, const std::set& direct_conflicts, int64_t vsize); @@ -89,7 +90,7 @@ std::optional> SingleTRUCChecks(const CT * * @returns debug string if an error occurs, std::nullopt otherwise. * */ -std::optional PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize, +std::optional 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 4d4cf940e37..28f467a416e 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 d7f78443a88..89cf2f85626 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 0cb15b69420..ffa21a41889 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 054d9e12891..9a4389c366f 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -286,6 +286,9 @@ public: std::tuple CalculateAncestorData(const CTxMemPoolEntry& entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); std::tuple 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 cacheMap; diff --git a/src/validation.cpp b/src/validation.cpp index ac8205528a5..348e610238e 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, {}); }