From 5d35b4a7dee95ae70bfdcbe79bc39fe488641b23 Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 5 Dec 2022 14:32:35 +0000 Subject: [test] package validation quits early due to non-policy, non-missing-inputs failure --- src/test/txpackage_tests.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'src/test') diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index b290a664f9..e77fcecf37 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -278,6 +278,24 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK(result_3gen_submit.m_package_feerate == std::nullopt); } + // Parent and child package where transactions are invalid for reasons other than fee and + // missing inputs, so the package validation isn't expected to happen. + { + CScriptWitness bad_witness; + bad_witness.stack.push_back(std::vector(1)); + CMutableTransaction mtx_parent_invalid{mtx_parent}; + mtx_parent_invalid.vin[0].scriptWitness = bad_witness; + CTransactionRef tx_parent_invalid = MakeTransactionRef(mtx_parent_invalid); + auto result_quit_early = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, + {tx_parent_invalid, tx_child}, /*test_accept=*/ false); + BOOST_CHECK(result_quit_early.m_state.IsInvalid()); + BOOST_CHECK_EQUAL(result_quit_early.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK(!result_quit_early.m_tx_results.empty()); + auto it_parent = result_quit_early.m_tx_results.find(tx_parent_invalid->GetWitnessHash()); + BOOST_CHECK(it_parent != result_quit_early.m_tx_results.end()); + BOOST_CHECK_EQUAL(it_parent->second.m_state.GetResult(), TxValidationResult::TX_WITNESS_MUTATED); + } + // Child with missing parent. mtx_child.vin.push_back(CTxIn(COutPoint(package_unrelated[0]->GetHash(), 0))); Package package_missing_parent; -- cgit v1.2.3 From 1605886380e4d3ff2e1236739fb800fa07322c49 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 28 Sep 2022 11:23:55 +0100 Subject: [validation] return effective feerate from mempool validation --- src/test/txpackage_tests.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'src/test') diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index e77fcecf37..244afb9161 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -95,12 +95,14 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end()); BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(), "Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason()); + BOOST_CHECK(it_parent->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_parent)) == COIN); BOOST_CHECK(it_child != result_parent_child.m_tx_results.end()); BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(), "Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason()); BOOST_CHECK(result_parent_child.m_package_feerate.has_value()); BOOST_CHECK(result_parent_child.m_package_feerate.value() == CFeeRate(2 * COIN, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child))); + BOOST_CHECK(it_child->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_child)) == COIN); // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy. CTransactionRef giant_ptx = create_placeholder_tx(999, 999); @@ -323,8 +325,10 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) auto it_child = submit_parent_child.m_tx_results.find(tx_child->GetWitnessHash()); BOOST_CHECK(it_parent != submit_parent_child.m_tx_results.end()); BOOST_CHECK(it_parent->second.m_state.IsValid()); + BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_parent))); BOOST_CHECK(it_child != submit_parent_child.m_tx_results.end()); BOOST_CHECK(it_child->second.m_state.IsValid()); + BOOST_CHECK(it_child->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_child))); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash()))); @@ -613,6 +617,8 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK_MESSAGE(mixed_result.m_package_feerate.value() == expected_feerate, strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), mixed_result.m_package_feerate.value().ToString())); + BOOST_CHECK(it_parent3->second.m_effective_feerate.value() == expected_feerate); + BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); } } @@ -695,6 +701,8 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) const CFeeRate expected_feerate(coinbase_value - child_value, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)); + BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); + BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); BOOST_CHECK(expected_feerate.GetFeePerK() > 1000); BOOST_CHECK(submit_cpfp.m_package_feerate.has_value()); BOOST_CHECK_MESSAGE(submit_cpfp.m_package_feerate.value() == expected_feerate, @@ -756,6 +764,16 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK_MESSAGE(submit_prioritised_package.m_package_feerate.value() == expected_feerate, strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), submit_prioritised_package.m_package_feerate.value().ToString())); + auto it_parent = submit_prioritised_package.m_tx_results.find(tx_parent_cheap->GetWitnessHash()); + auto it_child = submit_prioritised_package.m_tx_results.find(tx_child_cheap->GetWitnessHash()); + BOOST_CHECK(it_parent != submit_prioritised_package.m_tx_results.end()); + BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_parent->second.m_base_fees.value() == 0); + BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); + BOOST_CHECK(it_child != submit_prioritised_package.m_tx_results.end()); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_base_fees.value() == 200); + BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); } // Package feerate is calculated without topology in mind; it's just aggregating fees and sizes. @@ -801,6 +819,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK(it_parent->second.m_state.GetRejectReason() == ""); BOOST_CHECK_MESSAGE(it_parent->second.m_base_fees.value() == high_parent_fee, strprintf("rich parent: expected fee %s, got %s", high_parent_fee, it_parent->second.m_base_fees.value())); + BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(high_parent_fee, GetVirtualTransactionSize(*tx_parent_rich))); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_rich->GetHash()))); -- cgit v1.2.3 From d6c7b78ef2924af72f677ce2a7472c2447141e18 Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 9 Dec 2022 13:13:49 +0000 Subject: [validation] return wtxids of other transactions whose fees were used --- src/test/txpackage_tests.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'src/test') diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 244afb9161..0be843d5d9 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -96,6 +96,8 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(), "Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason()); BOOST_CHECK(it_parent->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_parent)) == COIN); + BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().size(), 1); + BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().front(), tx_parent->GetWitnessHash()); BOOST_CHECK(it_child != result_parent_child.m_tx_results.end()); BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(), "Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason()); @@ -103,6 +105,8 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) BOOST_CHECK(result_parent_child.m_package_feerate.value() == CFeeRate(2 * COIN, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child))); BOOST_CHECK(it_child->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_child)) == COIN); + BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().size(), 1); + BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().front(), tx_child->GetWitnessHash()); // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy. CTransactionRef giant_ptx = create_placeholder_tx(999, 999); @@ -326,9 +330,13 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK(it_parent != submit_parent_child.m_tx_results.end()); BOOST_CHECK(it_parent->second.m_state.IsValid()); BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_parent))); + BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().size(), 1); + BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().front(), tx_parent->GetWitnessHash()); BOOST_CHECK(it_child != submit_parent_child.m_tx_results.end()); BOOST_CHECK(it_child->second.m_state.IsValid()); BOOST_CHECK(it_child->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_child))); + BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().size(), 1); + BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().front(), tx_child->GetWitnessHash()); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash()))); @@ -619,6 +627,9 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) mixed_result.m_package_feerate.value().ToString())); BOOST_CHECK(it_parent3->second.m_effective_feerate.value() == expected_feerate); BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); + std::vector expected_wtxids({ptx_parent3->GetWitnessHash(), ptx_mixed_child->GetWitnessHash()}); + BOOST_CHECK(it_parent3->second.m_wtxids_fee_calculations.value() == expected_wtxids); + BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); } } @@ -703,6 +714,9 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)); BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); + std::vector expected_wtxids({tx_parent->GetWitnessHash(), tx_child->GetWitnessHash()}); + BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); + BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); BOOST_CHECK(expected_feerate.GetFeePerK() > 1000); BOOST_CHECK(submit_cpfp.m_package_feerate.has_value()); BOOST_CHECK_MESSAGE(submit_cpfp.m_package_feerate.value() == expected_feerate, @@ -774,6 +788,9 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); BOOST_CHECK(it_child->second.m_base_fees.value() == 200); BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); + std::vector expected_wtxids({tx_parent_cheap->GetWitnessHash(), tx_child_cheap->GetWitnessHash()}); + BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); + BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); } // Package feerate is calculated without topology in mind; it's just aggregating fees and sizes. -- cgit v1.2.3 From 5eab397b9840de5a4729bea723794b529e5fcbb4 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 28 Sep 2022 14:31:20 +0100 Subject: [validation] remove PackageMempoolAcceptResult::m_package_feerate This value creates an extremely confusing interface as its existence is dependent upon implementation details (whether something was submitted on its own, etc). MempoolAcceptResult::m_effective_feerate is much more helpful, as it always exists for submitted transactions. --- src/test/txpackage_tests.cpp | 46 -------------------------------------------- 1 file changed, 46 deletions(-) (limited to 'src/test') diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 0be843d5d9..6a7d2c122d 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -101,9 +101,6 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) BOOST_CHECK(it_child != result_parent_child.m_tx_results.end()); BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(), "Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason()); - BOOST_CHECK(result_parent_child.m_package_feerate.has_value()); - BOOST_CHECK(result_parent_child.m_package_feerate.value() == - CFeeRate(2 * COIN, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child))); BOOST_CHECK(it_child->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_child)) == COIN); BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().size(), 1); BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().front(), tx_child->GetWitnessHash()); @@ -118,7 +115,6 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash()); BOOST_CHECK(it_giant_tx != result_single_large.m_tx_results.end()); BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size"); - BOOST_CHECK(result_single_large.m_package_feerate == std::nullopt); // Check that mempool size hasn't changed. BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize); @@ -239,7 +235,6 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK_EQUAL(result_unrelated_submit.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(result_unrelated_submit.m_state.GetRejectReason(), "package-not-child-with-parents"); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - BOOST_CHECK(result_unrelated_submit.m_package_feerate == std::nullopt); // Parent and Child (and Grandchild) Package Package package_parent_child; @@ -281,7 +276,6 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK_EQUAL(result_3gen_submit.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(result_3gen_submit.m_state.GetRejectReason(), "package-not-child-with-parents"); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - BOOST_CHECK(result_3gen_submit.m_package_feerate == std::nullopt); } // Parent and child package where transactions are invalid for reasons other than fee and @@ -314,8 +308,6 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK_EQUAL(result_missing_parent.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(result_missing_parent.m_state.GetRejectReason(), "package-not-child-with-unconfirmed-parents"); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - - BOOST_CHECK(result_missing_parent.m_package_feerate == std::nullopt); } // Submit package with parent + child. @@ -341,10 +333,6 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash()))); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash()))); - - // Since both transactions have high feerates, they each passed validation individually. - // Package validation was unnecessary, so there is no package feerate. - BOOST_CHECK(submit_parent_child.m_package_feerate == std::nullopt); } // Already-in-mempool transactions should be detected and de-duplicated. @@ -365,8 +353,6 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash()))); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash()))); - - BOOST_CHECK(submit_deduped.m_package_feerate == std::nullopt); } } @@ -442,11 +428,8 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child1->GetHash()))); // Child2 would have been validated individually. - BOOST_CHECK(submit_witness1.m_package_feerate == std::nullopt); - const auto submit_witness2 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {ptx_parent, ptx_child2}, /*test_accept=*/false); - BOOST_CHECK(submit_witness2.m_package_feerate == std::nullopt); BOOST_CHECK_MESSAGE(submit_witness2.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_witness2.m_state.GetRejectReason()); auto it_parent2_deduped = submit_witness2.m_tx_results.find(ptx_parent->GetWitnessHash()); @@ -470,7 +453,6 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) auto it_child_dup = submit_segwit_dedup.m_tx_results.find(ptx_child1->GetWitnessHash()); BOOST_CHECK(it_parent_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); BOOST_CHECK(it_child_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - BOOST_CHECK(submit_witness2.m_package_feerate == std::nullopt); } // Try submitting Package1{child2, grandchild} where child2 is same-txid-different-witness as @@ -505,9 +487,6 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child2->GetHash()))); BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_child2->GetWitnessHash()))); BOOST_CHECK(m_node.mempool->exists(GenTxid::Wtxid(ptx_grandchild->GetWitnessHash()))); - - // Since child2 is ignored, grandchild would be validated individually. - BOOST_CHECK(submit_spend_ignored.m_package_feerate == std::nullopt); } // A package Package{parent1, parent2, parent3, child} where the parents are a mixture of @@ -620,11 +599,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_mixed_child->GetHash()))); // package feerate should include parent3 and child. It should not include parent1 or parent2_v1. - BOOST_CHECK(mixed_result.m_package_feerate.has_value()); const CFeeRate expected_feerate(1 * COIN, GetVirtualTransactionSize(*ptx_parent3) + GetVirtualTransactionSize(*ptx_mixed_child)); - BOOST_CHECK_MESSAGE(mixed_result.m_package_feerate.value() == expected_feerate, - strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), - mixed_result.m_package_feerate.value().ToString())); BOOST_CHECK(it_parent3->second.m_effective_feerate.value() == expected_feerate); BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); std::vector expected_wtxids({ptx_parent3->GetWitnessHash(), ptx_mixed_child->GetWitnessHash()}); @@ -678,11 +653,6 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK(submit_cpfp_deprio.m_tx_results.empty()); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); const CFeeRate expected_feerate(0, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)); - BOOST_CHECK(submit_cpfp_deprio.m_package_feerate.has_value()); - BOOST_CHECK(submit_cpfp_deprio.m_package_feerate.value() == CFeeRate{0}); - BOOST_CHECK_MESSAGE(submit_cpfp_deprio.m_package_feerate.value() == expected_feerate, - strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), - submit_cpfp_deprio.m_package_feerate.value().ToString())); } // Clear the prioritisation of the parent transaction. @@ -718,10 +688,6 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); BOOST_CHECK(expected_feerate.GetFeePerK() > 1000); - BOOST_CHECK(submit_cpfp.m_package_feerate.has_value()); - BOOST_CHECK_MESSAGE(submit_cpfp.m_package_feerate.value() == expected_feerate, - strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), - submit_cpfp.m_package_feerate.value().ToString())); } // Just because we allow low-fee parents doesn't mean we allow low-feerate packages. @@ -756,10 +722,6 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) const CFeeRate expected_feerate(200, GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)); BOOST_CHECK(expected_feerate.GetFeePerK() < 1000); - BOOST_CHECK(submit_package_too_low.m_package_feerate.has_value()); - BOOST_CHECK_MESSAGE(submit_package_too_low.m_package_feerate.value() == expected_feerate, - strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), - submit_package_too_low.m_package_feerate.value().ToString())); } // Package feerate includes the modified fees of the transactions. @@ -774,10 +736,6 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) "Package validation unexpectedly failed" << submit_prioritised_package.m_state.GetRejectReason()); const CFeeRate expected_feerate(1 * COIN + 200, GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)); - BOOST_CHECK(submit_prioritised_package.m_package_feerate.has_value()); - BOOST_CHECK_MESSAGE(submit_prioritised_package.m_package_feerate.value() == expected_feerate, - strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), - submit_prioritised_package.m_package_feerate.value().ToString())); auto it_parent = submit_prioritised_package.m_tx_results.find(tx_parent_cheap->GetWitnessHash()); auto it_child = submit_prioritised_package.m_tx_results.find(tx_child_cheap->GetWitnessHash()); BOOST_CHECK(it_parent != submit_prioritised_package.m_tx_results.end()); @@ -823,10 +781,6 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK_MESSAGE(submit_rich_parent.m_state.IsInvalid(), "Package validation unexpectedly succeeded"); // The child would have been validated on its own and failed, then submitted as a "package" of 1. - // The package feerate is just the child's feerate, which is 0sat/vb. - BOOST_CHECK(submit_rich_parent.m_package_feerate.has_value()); - BOOST_CHECK_MESSAGE(submit_rich_parent.m_package_feerate.value() == CFeeRate(), - "expected 0, got " << submit_rich_parent.m_package_feerate.value().ToString()); BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetRejectReason(), "package-fee-too-low"); -- cgit v1.2.3 From 264f9ef17f650035882d24083fb419845942a3ac Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 5 Dec 2022 14:55:52 +0000 Subject: [validation] return MempoolAcceptResult for every tx on PCKG_TX failure This makes the interface more predictable and useful. The caller understands one or more transactions failed, and can learn what happened with each transaction. We already have this information, so we might as well return it. It doesn't make sense to do this for other PackageValidationResult values because: - PCKG_RESULT_UNSET: this means everything succeeded, so the individual failures are no longer accurate. - PCKG_MEMPOOL_ERROR: something went wrong with the mempool logic; transaction failures might not be meaningful. - PCKG_POLICY: this means something was wrong with the package as a whole. The caller should use the PackageValidationState to find the error, rather than looking at individual MempoolAcceptResults. --- src/test/txpackage_tests.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'src/test') diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 6a7d2c122d..e438867d15 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -90,6 +90,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {tx_parent, tx_child}, /*test_accept=*/true); BOOST_CHECK_MESSAGE(result_parent_child.m_state.IsValid(), "Package validation unexpectedly failed: " << result_parent_child.m_state.GetRejectReason()); + BOOST_CHECK(result_parent_child.m_tx_results.size() == 2); auto it_parent = result_parent_child.m_tx_results.find(tx_parent->GetWitnessHash()); auto it_child = result_parent_child.m_tx_results.find(tx_child->GetWitnessHash()); BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end()); @@ -112,6 +113,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) BOOST_CHECK(result_single_large.m_state.IsInvalid()); BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX); BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed"); + BOOST_CHECK(result_single_large.m_tx_results.size() == 1); auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash()); BOOST_CHECK(it_giant_tx != result_single_large.m_tx_results.end()); BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size"); @@ -291,9 +293,15 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK(result_quit_early.m_state.IsInvalid()); BOOST_CHECK_EQUAL(result_quit_early.m_state.GetResult(), PackageValidationResult::PCKG_TX); BOOST_CHECK(!result_quit_early.m_tx_results.empty()); + BOOST_CHECK_EQUAL(result_quit_early.m_tx_results.size(), 2); auto it_parent = result_quit_early.m_tx_results.find(tx_parent_invalid->GetWitnessHash()); + auto it_child = result_quit_early.m_tx_results.find(tx_child->GetWitnessHash()); BOOST_CHECK(it_parent != result_quit_early.m_tx_results.end()); + BOOST_CHECK(it_child != result_quit_early.m_tx_results.end()); BOOST_CHECK_EQUAL(it_parent->second.m_state.GetResult(), TxValidationResult::TX_WITNESS_MUTATED); + BOOST_CHECK_EQUAL(it_parent->second.m_state.GetRejectReason(), "bad-witness-nonstandard"); + BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MISSING_INPUTS); + BOOST_CHECK_EQUAL(it_child->second.m_state.GetRejectReason(), "bad-txns-inputs-missingorspent"); } // Child with missing parent. @@ -317,6 +325,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) expected_pool_size += 2; BOOST_CHECK_MESSAGE(submit_parent_child.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_parent_child.m_state.GetRejectReason()); + BOOST_CHECK_EQUAL(submit_parent_child.m_tx_results.size(), package_parent_child.size()); auto it_parent = submit_parent_child.m_tx_results.find(tx_parent->GetWitnessHash()); auto it_child = submit_parent_child.m_tx_results.find(tx_child->GetWitnessHash()); BOOST_CHECK(it_parent != submit_parent_child.m_tx_results.end()); @@ -341,6 +350,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) package_parent_child, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_deduped.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_deduped.m_state.GetRejectReason()); + BOOST_CHECK_EQUAL(submit_deduped.m_tx_results.size(), package_parent_child.size()); auto it_parent_deduped = submit_deduped.m_tx_results.find(tx_parent->GetWitnessHash()); auto it_child_deduped = submit_deduped.m_tx_results.find(tx_child->GetWitnessHash()); BOOST_CHECK(it_parent_deduped != submit_deduped.m_tx_results.end()); @@ -415,6 +425,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) {ptx_parent, ptx_child1}, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_witness1.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_witness1.m_state.GetRejectReason()); + BOOST_CHECK_EQUAL(submit_witness1.m_tx_results.size(), 2); auto it_parent1 = submit_witness1.m_tx_results.find(ptx_parent->GetWitnessHash()); auto it_child1 = submit_witness1.m_tx_results.find(ptx_child1->GetWitnessHash()); BOOST_CHECK(it_parent1 != submit_witness1.m_tx_results.end()); @@ -432,6 +443,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) {ptx_parent, ptx_child2}, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_witness2.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_witness2.m_state.GetRejectReason()); + BOOST_CHECK_EQUAL(submit_witness2.m_tx_results.size(), 2); auto it_parent2_deduped = submit_witness2.m_tx_results.find(ptx_parent->GetWitnessHash()); auto it_child2 = submit_witness2.m_tx_results.find(ptx_child2->GetWitnessHash()); BOOST_CHECK(it_parent2_deduped != submit_witness2.m_tx_results.end()); @@ -449,6 +461,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) {ptx_parent, ptx_child1}, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_segwit_dedup.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_segwit_dedup.m_state.GetRejectReason()); + BOOST_CHECK_EQUAL(submit_segwit_dedup.m_tx_results.size(), 2); auto it_parent_dup = submit_segwit_dedup.m_tx_results.find(ptx_parent->GetWitnessHash()); auto it_child_dup = submit_segwit_dedup.m_tx_results.find(ptx_child1->GetWitnessHash()); BOOST_CHECK(it_parent_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); @@ -477,6 +490,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) {ptx_child2, ptx_grandchild}, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_spend_ignored.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_spend_ignored.m_state.GetRejectReason()); + BOOST_CHECK_EQUAL(submit_spend_ignored.m_tx_results.size(), 2); auto it_child2_ignored = submit_spend_ignored.m_tx_results.find(ptx_child2->GetWitnessHash()); auto it_grandchild = submit_spend_ignored.m_tx_results.find(ptx_grandchild->GetWitnessHash()); BOOST_CHECK(it_child2_ignored != submit_spend_ignored.m_tx_results.end()); @@ -577,6 +591,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) { const auto mixed_result = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_mixed, false); BOOST_CHECK_MESSAGE(mixed_result.m_state.IsValid(), mixed_result.m_state.GetRejectReason()); + BOOST_CHECK_EQUAL(mixed_result.m_tx_results.size(), package_mixed.size()); auto it_parent1 = mixed_result.m_tx_results.find(ptx_parent1->GetWitnessHash()); auto it_parent2 = mixed_result.m_tx_results.find(ptx_parent2_v1->GetWitnessHash()); auto it_parent3 = mixed_result.m_tx_results.find(ptx_parent3->GetWitnessHash()); @@ -648,6 +663,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); const auto submit_cpfp_deprio = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_cpfp, /*test_accept=*/ false); + BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_MESSAGE(submit_cpfp_deprio.m_state.IsInvalid(), "Package validation unexpectedly succeeded: " << submit_cpfp_deprio.m_state.GetRejectReason()); BOOST_CHECK(submit_cpfp_deprio.m_tx_results.empty()); @@ -667,6 +683,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) expected_pool_size += 2; BOOST_CHECK_MESSAGE(submit_cpfp.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_cpfp.m_state.GetRejectReason()); + BOOST_CHECK_EQUAL(submit_cpfp.m_tx_results.size(), package_cpfp.size()); auto it_parent = submit_cpfp.m_tx_results.find(tx_parent->GetWitnessHash()); auto it_child = submit_cpfp.m_tx_results.find(tx_child->GetWitnessHash()); BOOST_CHECK(it_parent != submit_cpfp.m_tx_results.end()); @@ -736,6 +753,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) "Package validation unexpectedly failed" << submit_prioritised_package.m_state.GetRejectReason()); const CFeeRate expected_feerate(1 * COIN + 200, GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)); + BOOST_CHECK_EQUAL(submit_prioritised_package.m_tx_results.size(), package_still_too_low.size()); auto it_parent = submit_prioritised_package.m_tx_results.find(tx_parent_cheap->GetWitnessHash()); auto it_child = submit_prioritised_package.m_tx_results.find(tx_child_cheap->GetWitnessHash()); BOOST_CHECK(it_parent != submit_prioritised_package.m_tx_results.end()); -- cgit v1.2.3