From 00330a4827ab740d28387bffa2320c298eae57e5 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Thu, 14 Dec 2023 13:47:36 -0500 Subject: [PATCH] Remove unused CheckV3Inheritance --- src/policy/v3_policy.cpp | 41 --------------------------------- src/policy/v3_policy.h | 11 --------- src/test/txvalidation_tests.cpp | 26 --------------------- 3 files changed, 78 deletions(-) diff --git a/src/policy/v3_policy.cpp b/src/policy/v3_policy.cpp index 054336259c088..1207b264f823c 100644 --- a/src/policy/v3_policy.cpp +++ b/src/policy/v3_policy.cpp @@ -14,47 +14,6 @@ #include #include -std::optional> CheckV3Inheritance(const Package& package) -{ - assert(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;})); - // If all transactions are V3, we can stop here. - if (std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx->nVersion == 3;})) { - return std::nullopt; - } - // If all transactions are non-V3, we can stop here. - if (std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx->nVersion != 3;})) { - return std::nullopt; - } - // Look for a V3 transaction spending a non-V3 or vice versa. - std::unordered_map v3_txid_to_wtxid; - std::unordered_map non_v3_txid_to_wtxid; - for (const auto& tx : package) { - if (tx->nVersion == 3) { - // If duplicate txids exist, this function will still detect violations, but it - // will return the earlier transaction's wtxid. - Assume(v3_txid_to_wtxid.emplace(tx->GetHash(), tx->GetWitnessHash()).second); - } else { - Assume(non_v3_txid_to_wtxid.emplace(tx->GetHash(), tx->GetWitnessHash()).second); - } - } - for (const auto& tx : package) { - if (tx->nVersion == 3) { - for (const auto& input : tx->vin) { - if (auto it = non_v3_txid_to_wtxid.find(input.prevout.hash); it != non_v3_txid_to_wtxid.end()) { - return std::make_tuple(it->second, tx->GetWitnessHash(), true); - } - } - } else { - for (const auto& input : tx->vin) { - if (auto it = v3_txid_to_wtxid.find(input.prevout.hash); it != v3_txid_to_wtxid.end()) { - return std::make_tuple(it->second, tx->GetWitnessHash(), false); - } - } - } - } - return std::nullopt; -} - std::optional PackageV3SanityChecks(const Package& package) { // This should only be called in scenarios where the topo of the package diff --git a/src/policy/v3_policy.h b/src/policy/v3_policy.h index a076eaf857925..c390bc65f6897 100644 --- a/src/policy/v3_policy.h +++ b/src/policy/v3_policy.h @@ -60,17 +60,6 @@ std::optional ApplyV3Rules(const CTransactionRef& ptx, const std::set& direct_conflicts, int64_t vsize); -/** Helper function for PackageV3SanityChecks below. - * Any two unconfirmed transactions with a dependency relationship must either both be v3 or both - * be non-v3. Check this rule for any list of unconfirmed transactions (no topology requirements). - * - * @returns a tuple (parent wtxid, child wtxid, bool) where one is V3 but the other is not, if at - * least one such pair exists. The bool represents whether the child is v3 or not. There may be - * other such pairs that are not returned. - * Otherwise std::nullopt. - */ -std::optional> CheckV3Inheritance(const Package& package); - /** Optimization for early package rejection. Should not be called for non-v3 packages. * * Check the following rules for transactions within the package: diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 08b10ce847133..0f20257f570ee 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -101,8 +101,6 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) pool.addUnchecked(entry.FromTx(mempool_tx_v3)); auto mempool_tx_v2 = make_tx(random_outpoints(1), /*version=*/2); pool.addUnchecked(entry.FromTx(mempool_tx_v2)); - // These two transactions are unrelated, so CheckV3Inheritance should pass. - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v2, mempool_tx_v3}) == std::nullopt); // Default values. CTxMemPool::Limits m_limits{}; @@ -113,7 +111,6 @@ 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), m_limits)}; - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v3, tx_v2_from_v3}).has_value()); const auto expected_error_str{strprintf("non-v3 tx %s cannot spend from v3 tx %s", tx_v2_from_v3->GetWitnessHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())}; BOOST_CHECK(*ApplyV3Rules(tx_v2_from_v3, *ancestors_v2_from_v3, /*num_other_ancestors=*/0, false, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3)) == expected_error_str); BOOST_CHECK(*PackageV3SanityChecks({mempool_tx_v3, tx_v2_from_v3}) == "txs in package are not all v3"); @@ -125,8 +122,6 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto ancestors_v2_from_both{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v2_from_v2_and_v3), m_limits)}; const auto expected_error_str_2{strprintf("non-v3 tx %s cannot spend from v3 tx %s", tx_v2_from_v2_and_v3->GetWitnessHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())}; - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v2, mempool_tx_v3, tx_v2_from_v2_and_v3}).value() == - std::make_tuple(mempool_tx_v3->GetWitnessHash(), tx_v2_from_v2_and_v3->GetWitnessHash(), false)); BOOST_CHECK(*ApplyV3Rules(tx_v2_from_v2_and_v3, *ancestors_v2_from_both, /*num_other_ancestors=*/0, false, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3)) == expected_error_str_2); BOOST_CHECK(*PackageV3SanityChecks({mempool_tx_v2, mempool_tx_v3, tx_v2_from_v2_and_v3}) == "txs in package are not all v3"); @@ -139,8 +134,6 @@ 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), m_limits)}; - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v2, tx_v3_from_v2}).value() == - std::make_tuple(mempool_tx_v2->GetWitnessHash(), tx_v3_from_v2->GetWitnessHash(), true)); const auto expected_error_str{strprintf("v3 tx %s cannot spend from non-v3 tx %s", tx_v3_from_v2->GetWitnessHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())}; BOOST_CHECK(*ApplyV3Rules(tx_v3_from_v2, *ancestors_v3_from_v2, /*num_other_ancestors=*/0, false, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2)) == expected_error_str); @@ -153,8 +146,6 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto ancestors_v3_from_both{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_from_v2_and_v3), m_limits)}; const auto expected_error_str_2{strprintf("v3 tx %s cannot spend from non-v3 tx %s", tx_v3_from_v2_and_v3->GetWitnessHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())}; - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v2, mempool_tx_v3, tx_v3_from_v2_and_v3}).value() == - std::make_tuple(mempool_tx_v2->GetWitnessHash(), tx_v3_from_v2_and_v3->GetWitnessHash(), true)); BOOST_CHECK(*ApplyV3Rules(tx_v3_from_v2_and_v3, *ancestors_v3_from_both, /*num_other_ancestors=*/0, false, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3)) == expected_error_str_2); BOOST_CHECK(*PackageV3SanityChecks({mempool_tx_v2, mempool_tx_v3, tx_v3_from_v2_and_v3}) == "txs in package are not all v3"); @@ -166,34 +157,17 @@ 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(CheckV3Inheritance({mempool_tx_v3, tx_v3_from_v3}) == 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(CheckV3Inheritance({tx_v2_from_v2, mempool_tx_v2}) == std::nullopt); - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v2, tx_v2_from_v2}) == std::nullopt); } - // Empty: no violations detected - BOOST_CHECK(CheckV3Inheritance({}) == std::nullopt); - - // Just 1 transaction: no violations detected. - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v2}) == std::nullopt); - - // Duplicate transactions: no violations detected because no spending relationships between - // these transactions. - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v3, mempool_tx_v3}) == std::nullopt); - - // Unrelated transactions: no violations detected because no spending relationships between - // these transactions. - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v2, mempool_tx_v3}) == std::nullopt); // Putting them all together is fine, as spending relationships are checked. auto tx_v3_child_1 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}}, /*version=*/3); auto tx_v2_child_1 = make_tx({COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/2); - BOOST_CHECK(CheckV3Inheritance({mempool_tx_v2, tx_v2_child_1, mempool_tx_v3, tx_v3_child_1}) == std::nullopt); // Tx spending v3 cannot have too many mempool ancestors // Configuration where the tx has multiple direct parents.