diff --git a/src/policy/v3_policy.cpp b/src/policy/v3_policy.cpp index f838dc6c0f..c98262c364 100644 --- a/src/policy/v3_policy.cpp +++ b/src/policy/v3_policy.cpp @@ -158,7 +158,7 @@ std::optional PackageV3Checks(const CTransactionRef& ptx, int64_t v return std::nullopt; } -std::optional SingleV3Checks(const CTransactionRef& ptx, +std::optional> SingleV3Checks(const CTransactionRef& ptx, const CTxMemPool::setEntries& mempool_ancestors, const std::set& direct_conflicts, int64_t vsize) @@ -166,13 +166,15 @@ std::optional SingleV3Checks(const CTransactionRef& ptx, // Check v3 and non-v3 inheritance. for (const auto& entry : mempool_ancestors) { if (ptx->nVersion != 3 && entry->GetTx().nVersion == 3) { - return strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)", + return std::make_pair(strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)", ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), - entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()); + entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()), + nullptr); } else if (ptx->nVersion == 3 && entry->GetTx().nVersion != 3) { - return strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)", + return std::make_pair(strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)", ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), - entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()); + entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()), + nullptr); } } @@ -185,16 +187,18 @@ std::optional SingleV3Checks(const CTransactionRef& ptx, // Check that V3_ANCESTOR_LIMIT would not be violated. if (mempool_ancestors.size() + 1 > V3_ANCESTOR_LIMIT) { - return strprintf("tx %s (wtxid=%s) would have too many ancestors", - ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()); + 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 this transaction spends V3 parents, it cannot be too large. if (vsize > V3_CHILD_MAX_VSIZE) { - return strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes", - ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE); + return std::make_pair(strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes", + ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE), + nullptr); } // Check the descendant counts of in-mempool ancestors. @@ -210,9 +214,10 @@ std::optional SingleV3Checks(const CTransactionRef& ptx, 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 > V3_DESCENDANT_LIMIT && !child_will_be_replaced) { - return strprintf("tx %u (wtxid=%s) would exceed descendant count limit", + 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()->GetWitnessHash().ToString()), + nullptr); } } return std::nullopt; diff --git a/src/policy/v3_policy.h b/src/policy/v3_policy.h index 9e871915e5..c61d8ac4cc 100644 --- a/src/policy/v3_policy.h +++ b/src/policy/v3_policy.h @@ -50,7 +50,7 @@ static_assert(V3_CHILD_MAX_VSIZE + MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR * * @returns debug string if an error occurs, std::nullopt otherwise. */ -std::optional SingleV3Checks(const CTransactionRef& ptx, +std::optional> SingleV3Checks(const CTransactionRef& ptx, const CTxMemPool::setEntries& mempool_ancestors, const std::set& direct_conflicts, int64_t vsize); diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index e045949b43..04bb7c4828 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -115,7 +115,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str{strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 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())}; - BOOST_CHECK(*SingleV3Checks(tx_v2_from_v3, *ancestors_v2_from_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3)) == expected_error_str); + auto result_v2_from_v3{SingleV3Checks(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(*PackageV3Checks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_ancestors), expected_error_str); @@ -130,8 +132,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str_2{strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 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())}; - BOOST_CHECK(*SingleV3Checks(tx_v2_from_v2_and_v3, *ancestors_v2_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3)) - == expected_error_str_2); + auto result_v2_from_both{SingleV3Checks(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(*PackageV3Checks(tx_v2_from_v2_and_v3, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3), package_v3_v2_v2, empty_ancestors), expected_error_str_2); @@ -147,7 +150,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str{strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 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())}; - BOOST_CHECK(*SingleV3Checks(tx_v3_from_v2, *ancestors_v3_from_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2)) == expected_error_str); + auto result_v3_from_v2{SingleV3Checks(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(*PackageV3Checks(tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), package_v2_v3, empty_ancestors), expected_error_str); @@ -162,8 +167,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str_2{strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 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())}; - BOOST_CHECK(*SingleV3Checks(tx_v3_from_v2_and_v3, *ancestors_v3_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3)) - == expected_error_str_2); + auto result_v3_from_both{SingleV3Checks(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); // tx_v3_from_v2_and_v3 also violates V3_ANCESTOR_LIMIT. const auto expected_error_str_3{strprintf("tx %s (wtxid=%s) would have too many ancestors", @@ -215,8 +221,9 @@ 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())}; - BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_multi_parent, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent)), - expected_error_str); + auto result{SingleV3Checks(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(*PackageV3Checks(tx_v3_multi_parent, GetVirtualTransactionSize(*tx_v3_multi_parent), package_multi_parents, empty_ancestors), expected_error_str); @@ -239,8 +246,9 @@ 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())}; - BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_multi_gen, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen)), - expected_error_str); + auto result{SingleV3Checks(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(*PackageV3Checks(middle_tx, GetVirtualTransactionSize(*middle_tx), package_multi_gen, empty_ancestors), expected_error_str); @@ -256,8 +264,9 @@ 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("v3 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, V3_CHILD_MAX_VSIZE)}; - BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_child_big, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big)), - expected_error_str); + auto result{SingleV3Checks(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(*PackageV3Checks(tx_v3_child_big, GetVirtualTransactionSize(*tx_v3_child_big), package_child_big, empty_ancestors), @@ -298,9 +307,10 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) const auto expected_error_str{strprintf("v3 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, V3_CHILD_MAX_VSIZE)}; - BOOST_CHECK_EQUAL(*SingleV3Checks(tx_many_sigops, *ancestors, empty_conflicts_set, - GetVirtualTransactionSize(*tx_many_sigops, /*nSigOpCost=*/total_sigops, /*bytes_per_sigop=*/ DEFAULT_BYTES_PER_SIGOP)), - expected_error_str); + auto result{SingleV3Checks(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(*PackageV3Checks(tx_many_sigops, total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, package_child_sigops, empty_ancestors), @@ -326,8 +336,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto ancestors{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())}; - BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_child2, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2)), - expected_error_str); + auto result{SingleV3Checks(tx_v3_child2, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2))}; + BOOST_CHECK_EQUAL(result->first, expected_error_str); + BOOST_CHECK_EQUAL(result->second, nullptr); // If replacing the child, make sure there is no double-counting. BOOST_CHECK(SingleV3Checks(tx_v3_child2, *ancestors, {tx_mempool_v3_child->GetHash()}, GetVirtualTransactionSize(*tx_v3_child2)) == std::nullopt); diff --git a/src/validation.cpp b/src/validation.cpp index 0552bde665..2c36f0d00d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -954,8 +954,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } ws.m_ancestors = *ancestors; - if (const auto err_string{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "v3-rule-violation", *err_string); + if (const auto err{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "v3-rule-violation", err->first); } // A transaction that spends outputs that would be replaced by it is invalid. Now