diff --git a/src/consensus/validation.h b/src/consensus/validation.h index d5bf08cd61a..bd3a5913c36 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -54,6 +54,8 @@ enum class TxValidationResult { TX_CONFLICT, TX_MEMPOOL_POLICY, //!< violated mempool's fee/size/descendant/RBF/etc limits TX_NO_MEMPOOL, //!< this node does not have a mempool so can't validate the transaction + TX_RECONSIDERABLE, //!< fails some policy, but might be acceptable if submitted in a (different) package + TX_UNKNOWN, //!< transaction was not validated because package failed }; /** A "reason" why a block was invalid, suitable for determining whether the diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2f41eb2b1d7..c9596a36b8f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1820,6 +1820,8 @@ bool PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationStat case TxValidationResult::TX_CONFLICT: case TxValidationResult::TX_MEMPOOL_POLICY: case TxValidationResult::TX_NO_MEMPOOL: + case TxValidationResult::TX_RECONSIDERABLE: + case TxValidationResult::TX_UNKNOWN: break; } return false; diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 66e537a57ba..96095539ece 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -154,12 +154,15 @@ void CheckATMPInvariants(const MempoolAcceptResult& res, bool txid_in_mempool, b // It may be already in the mempool since in ATMP cases we don't set MEMPOOL_ENTRY or DIFFERENT_WITNESS Assert(!res.m_state.IsValid()); Assert(res.m_state.IsInvalid()); + + const bool is_reconsiderable{res.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE}; Assert(!res.m_replaced_transactions); Assert(!res.m_vsize); Assert(!res.m_base_fees); - // Unable or unwilling to calculate fees - Assert(!res.m_effective_feerate); - Assert(!res.m_wtxids_fee_calculations); + // Fee information is provided if the failure is TX_RECONSIDERABLE. + // In other cases, validation may be unable or unwilling to calculate the fees. + Assert(res.m_effective_feerate.has_value() == is_reconsiderable); + Assert(res.m_wtxids_fee_calculations.has_value() == is_reconsiderable); Assert(!res.m_other_wtxid); break; } diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index debefa7f93c..84c9ecc3d1e 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -132,36 +133,35 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) /*output_destination=*/child_locking_script, /*output_amount=*/CAmount(48 * COIN), /*submit=*/false); CTransactionRef tx_child = MakeTransactionRef(mtx_child); - 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()); - 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()); - 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()); + Package package_parent_child{tx_parent, tx_child}; + const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_parent_child, /*test_accept=*/true); + if (auto err_parent_child{CheckPackageMempoolAcceptResult(package_parent_child, result_parent_child, /*expect_valid=*/true, nullptr)}) { + BOOST_ERROR(err_parent_child.value()); + } else { + 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->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->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); BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1000); - auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {giant_ptx}, /*test_accept=*/true); - 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"); + Package package_single_giant{giant_ptx}; + auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_single_giant, /*test_accept=*/true); + if (auto err_single_large{CheckPackageMempoolAcceptResult(package_single_giant, result_single_large, /*expect_valid=*/false, nullptr)}) { + BOOST_ERROR(err_single_large.value()); + } else { + BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed"); + auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash()); + BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size"); + } // Check that mempool size hasn't changed. BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize); @@ -281,6 +281,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) } auto result_unrelated_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_unrelated, /*test_accept=*/false); + // We don't expect m_tx_results for each transaction when basic sanity checks haven't passed. BOOST_CHECK(result_unrelated_submit.m_state.IsInvalid()); 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"); @@ -336,20 +337,20 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) CMutableTransaction mtx_parent_invalid{mtx_parent}; mtx_parent_invalid.vin[0].scriptWitness = bad_witness; CTransactionRef tx_parent_invalid = MakeTransactionRef(mtx_parent_invalid); + Package package_invalid_parent{tx_parent_invalid, tx_child}; 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()); + package_invalid_parent, /*test_accept=*/ false); + if (auto err_parent_invalid{CheckPackageMempoolAcceptResult(package_invalid_parent, result_quit_early, /*expect_valid=*/false, m_node.mempool.get())}) { + BOOST_ERROR(err_parent_invalid.value()); + } else { + 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_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"); + } 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. @@ -381,36 +382,27 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) 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()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash()))); } // Already-in-mempool transactions should be detected and de-duplicated. { const auto submit_deduped = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, 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()); - BOOST_CHECK(it_parent_deduped->second.m_state.IsValid()); - BOOST_CHECK(it_parent_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - BOOST_CHECK(it_child_deduped != submit_deduped.m_tx_results.end()); - BOOST_CHECK(it_child_deduped->second.m_state.IsValid()); - BOOST_CHECK(it_child_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + if (auto err_deduped{CheckPackageMempoolAcceptResult(package_parent_child, submit_deduped, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_deduped.value()); + } else { + 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->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + BOOST_CHECK(it_child_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + } 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()))); } } @@ -470,51 +462,39 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // Try submitting Package1{parent, child1} and Package2{parent, child2} where the children are // same-txid-different-witness. { + Package package_parent_child1{ptx_parent, ptx_child1}; const auto submit_witness1 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {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()); - BOOST_CHECK_MESSAGE(it_parent1->second.m_state.IsValid(), - "Transaction unexpectedly failed: " << it_parent1->second.m_state.GetRejectReason()); - BOOST_CHECK(it_child1 != submit_witness1.m_tx_results.end()); - BOOST_CHECK_MESSAGE(it_child1->second.m_state.IsValid(), - "Transaction unexpectedly failed: " << it_child1->second.m_state.GetRejectReason()); - - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent->GetHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child1->GetHash()))); + package_parent_child1, /*test_accept=*/false); + if (auto err_witness1{CheckPackageMempoolAcceptResult(package_parent_child1, submit_witness1, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_witness1.value()); + } // Child2 would have been validated individually. + Package package_parent_child2{ptx_parent, ptx_child2}; const auto submit_witness2 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {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()); - BOOST_CHECK(it_parent2_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - BOOST_CHECK(it_child2 != submit_witness2.m_tx_results.end()); - BOOST_CHECK(it_child2->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); - BOOST_CHECK_EQUAL(ptx_child1->GetWitnessHash(), it_child2->second.m_other_wtxid.value()); - - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child2->GetHash()))); - BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_child2->GetWitnessHash()))); + package_parent_child2, /*test_accept=*/false); + if (auto err_witness2{CheckPackageMempoolAcceptResult(package_parent_child2, submit_witness2, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_witness2.value()); + } else { + 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->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + BOOST_CHECK(it_child2->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); + BOOST_CHECK_EQUAL(ptx_child1->GetWitnessHash(), it_child2->second.m_other_wtxid.value()); + } // Deduplication should work when wtxid != txid. Submit package with the already-in-mempool // transactions again, which should not fail. const auto submit_segwit_dedup = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {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); - BOOST_CHECK(it_child_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + package_parent_child1, /*test_accept=*/false); + if (auto err_segwit_dedup{CheckPackageMempoolAcceptResult(package_parent_child1, submit_segwit_dedup, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_segwit_dedup.value()); + } else { + 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); + BOOST_CHECK(it_child_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + } } // Try submitting Package1{child2, grandchild} where child2 is same-txid-different-witness as @@ -535,21 +515,17 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // We already submitted child1 above. { + Package package_child2_grandchild{ptx_child2, ptx_grandchild}; const auto submit_spend_ignored = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {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()); - BOOST_CHECK(it_child2_ignored->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); - BOOST_CHECK(it_grandchild != submit_spend_ignored.m_tx_results.end()); - BOOST_CHECK(it_grandchild->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - - 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()))); + package_child2_grandchild, /*test_accept=*/false); + if (auto err_spend_ignored{CheckPackageMempoolAcceptResult(package_child2_grandchild, submit_spend_ignored, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_spend_ignored.value()); + } else { + 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->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); + BOOST_CHECK(it_grandchild->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + } } // A package Package{parent1, parent2, parent3, child} where the parents are a mixture of @@ -641,36 +617,28 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // child should be accepted { 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()); - auto it_child = mixed_result.m_tx_results.find(ptx_mixed_child->GetWitnessHash()); - BOOST_CHECK(it_parent1 != mixed_result.m_tx_results.end()); - BOOST_CHECK(it_parent2 != mixed_result.m_tx_results.end()); - BOOST_CHECK(it_parent3 != mixed_result.m_tx_results.end()); - BOOST_CHECK(it_child != mixed_result.m_tx_results.end()); + if (auto err_mixed{CheckPackageMempoolAcceptResult(package_mixed, mixed_result, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_mixed.value()); + } else { + 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()); + auto it_child = mixed_result.m_tx_results.find(ptx_mixed_child->GetWitnessHash()); - BOOST_CHECK(it_parent1->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - BOOST_CHECK(it_parent2->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); - BOOST_CHECK(it_parent3->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK_EQUAL(ptx_parent2_v2->GetWitnessHash(), it_parent2->second.m_other_wtxid.value()); + BOOST_CHECK(it_parent1->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + BOOST_CHECK(it_parent2->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); + BOOST_CHECK(it_parent3->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK_EQUAL(ptx_parent2_v2->GetWitnessHash(), it_parent2->second.m_other_wtxid.value()); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent1->GetHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent2_v1->GetHash()))); - BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_parent2_v1->GetWitnessHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent3->GetHash()))); - 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. - const CFeeRate expected_feerate(1 * COIN, GetVirtualTransactionSize(*ptx_parent3) + GetVirtualTransactionSize(*ptx_mixed_child)); - 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); + // package feerate should include parent3 and child. It should not include parent1 or parent2_v1. + const CFeeRate expected_feerate(1 * COIN, GetVirtualTransactionSize(*ptx_parent3) + GetVirtualTransactionSize(*ptx_mixed_child)); + 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); + } } } @@ -715,15 +683,17 @@ 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_TX); - BOOST_CHECK(submit_cpfp_deprio.m_state.IsInvalid()); - BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetResult(), - TxValidationResult::TX_MEMPOOL_POLICY); - BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_child->GetWitnessHash())->second.m_state.GetResult(), - TxValidationResult::TX_MISSING_INPUTS); - BOOST_CHECK(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetRejectReason() == "min relay fee not met"); - BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - const CFeeRate expected_feerate(0, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)); + if (auto err_cpfp_deprio{CheckPackageMempoolAcceptResult(package_cpfp, submit_cpfp_deprio, /*expect_valid=*/false, m_node.mempool.get())}) { + BOOST_ERROR(err_cpfp_deprio.value()); + } else { + BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetResult(), + TxValidationResult::TX_MEMPOOL_POLICY); + BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_child->GetWitnessHash())->second.m_state.GetResult(), + TxValidationResult::TX_MISSING_INPUTS); + BOOST_CHECK(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetRejectReason() == "min relay fee not met"); + BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); + } } // Clear the prioritisation of the parent transaction. @@ -735,31 +705,27 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); const auto submit_cpfp = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_cpfp, /*test_accept=*/ false); + if (auto err_cpfp{CheckPackageMempoolAcceptResult(package_cpfp, submit_cpfp, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_cpfp.value()); + } else { + 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->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_parent->second.m_base_fees.value() == coinbase_value - parent_value); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_base_fees.value() == COIN); + + 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); + 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); + } 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()); - BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_parent->second.m_base_fees.value() == coinbase_value - parent_value); - BOOST_CHECK(it_child != submit_cpfp.m_tx_results.end()); - BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_child->second.m_base_fees.value() == COIN); - 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()))); - - 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); - 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); } // Just because we allow low-fee parents doesn't mean we allow low-feerate packages. @@ -785,15 +751,28 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) package_still_too_low.push_back(tx_child_cheap); BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*tx_child_cheap)) <= child_fee); BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)) > parent_fee + child_fee); + BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - // Cheap package should fail with package-fee-too-low. + // Cheap package should fail for being too low fee. { - BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); const auto submit_package_too_low = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_still_too_low, /*test_accept=*/false); - BOOST_CHECK_MESSAGE(submit_package_too_low.m_state.IsInvalid(), "Package validation unexpectedly succeeded"); - BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); - BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetRejectReason(), "package-fee-too-low"); + if (auto err_package_too_low{CheckPackageMempoolAcceptResult(package_still_too_low, submit_package_too_low, /*expect_valid=*/false, m_node.mempool.get())}) { + BOOST_ERROR(err_package_too_low.value()); + } else { + // Individual feerate of parent is too low. + BOOST_CHECK_EQUAL(submit_package_too_low.m_tx_results.at(tx_parent_cheap->GetWitnessHash()).m_state.GetResult(), + TxValidationResult::TX_RECONSIDERABLE); + BOOST_CHECK(submit_package_too_low.m_tx_results.at(tx_parent_cheap->GetWitnessHash()).m_effective_feerate.value() == + CFeeRate(parent_fee, GetVirtualTransactionSize(*tx_parent_cheap))); + // Package feerate of parent + child is too low. + BOOST_CHECK_EQUAL(submit_package_too_low.m_tx_results.at(tx_child_cheap->GetWitnessHash()).m_state.GetResult(), + TxValidationResult::TX_RECONSIDERABLE); + BOOST_CHECK(submit_package_too_low.m_tx_results.at(tx_child_cheap->GetWitnessHash()).m_effective_feerate.value() == + CFeeRate(parent_fee + child_fee, GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap))); + } + BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetRejectReason(), "transaction failed"); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); } @@ -804,25 +783,26 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) { const auto submit_prioritised_package = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_still_too_low, /*test_accept=*/false); + if (auto err_prioritised{CheckPackageMempoolAcceptResult(package_still_too_low, submit_prioritised_package, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_prioritised.value()); + } else { + const CFeeRate expected_feerate(1 * COIN + parent_fee + child_fee, + 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->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_parent->second.m_base_fees.value() == parent_fee); + BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_base_fees.value() == child_fee); + 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); + } expected_pool_size += 2; - BOOST_CHECK_MESSAGE(submit_prioritised_package.m_state.IsValid(), - "Package validation unexpectedly failed" << submit_prioritised_package.m_state.GetRejectReason()); - const CFeeRate expected_feerate(1 * COIN + parent_fee + child_fee, - 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()); - BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_parent->second.m_base_fees.value() == parent_fee); - 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() == child_fee); - 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); + BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); } // Package feerate is calculated without topology in mind; it's just aggregating fees and sizes. @@ -851,31 +831,27 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); const auto submit_rich_parent = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_rich_parent, /*test_accept=*/false); + if (auto err_rich_parent{CheckPackageMempoolAcceptResult(package_rich_parent, submit_rich_parent, /*expect_valid=*/false, m_node.mempool.get())}) { + BOOST_ERROR(err_rich_parent.value()); + } else { + // The child would have been validated on its own and failed. + BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetRejectReason(), "transaction failed"); + + auto it_parent = submit_rich_parent.m_tx_results.find(tx_parent_rich->GetWitnessHash()); + auto it_child = submit_rich_parent.m_tx_results.find(tx_child_poor->GetWitnessHash()); + BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::INVALID); + 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(it_child->second.m_result_type, MempoolAcceptResult::ResultType::INVALID); + BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MEMPOOL_POLICY); + BOOST_CHECK(it_child->second.m_state.GetRejectReason() == "min relay fee not met"); + } expected_pool_size += 1; - BOOST_CHECK_MESSAGE(submit_rich_parent.m_state.IsInvalid(), "Package validation unexpectedly succeeded"); - - // The child would have been validated on its own and failed. - BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetResult(), PackageValidationResult::PCKG_TX); - BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetRejectReason(), "transaction failed"); - - auto it_parent = submit_rich_parent.m_tx_results.find(tx_parent_rich->GetWitnessHash()); - auto it_child = submit_rich_parent.m_tx_results.find(tx_child_poor->GetWitnessHash()); - BOOST_CHECK(it_parent != submit_rich_parent.m_tx_results.end()); - BOOST_CHECK(it_child != submit_rich_parent.m_tx_results.end()); - BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::INVALID); - 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(it_child != submit_rich_parent.m_tx_results.end()); - BOOST_CHECK_EQUAL(it_child->second.m_result_type, MempoolAcceptResult::ResultType::INVALID); - BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MEMPOOL_POLICY); - BOOST_CHECK(it_child->second.m_state.GetRejectReason() == "min relay fee not met"); - BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_rich->GetHash()))); - BOOST_CHECK(!m_node.mempool->exists(GenTxid::Txid(tx_child_poor->GetHash()))); } } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index 147e589debf..6d3bb01be86 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -89,11 +89,14 @@ std::optional CheckPackageMempoolAcceptResult(const Package& txns, } // m_effective_feerate and m_wtxids_fee_calculations should exist iff the result was valid - if (atmp_result.m_effective_feerate.has_value() != valid) { + // or if the failure was TX_RECONSIDERABLE + const bool valid_or_reconsiderable{atmp_result.m_result_type == MempoolAcceptResult::ResultType::VALID || + atmp_result.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE}; + if (atmp_result.m_effective_feerate.has_value() != valid_or_reconsiderable) { return strprintf("tx %s result should %shave m_effective_feerate", wtxid.ToString(), valid ? "" : "not "); } - if (atmp_result.m_wtxids_fee_calculations.has_value() != valid) { + if (atmp_result.m_wtxids_fee_calculations.has_value() != valid_or_reconsiderable) { return strprintf("tx %s result should %shave m_effective_feerate", wtxid.ToString(), valid ? "" : "not "); } diff --git a/src/validation.cpp b/src/validation.cpp index 8d7e3661254..018566fa9c3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -670,11 +670,11 @@ private: AssertLockHeld(m_pool.cs); CAmount mempoolRejectFee = m_pool.GetMinFee().GetFee(package_size); if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); + return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); } if (package_fee < m_pool.m_min_relay_feerate.GetFee(package_size)) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", + return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "min relay fee not met", strprintf("%d < %d", package_fee, m_pool.m_min_relay_feerate.GetFee(package_size))); } return true; @@ -867,6 +867,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // method of ensuring the tx remains bumped. For example, the fee-bumping child could disappear // due to a replacement. if (!bypass_limits && ws.m_modified_fees < m_pool.m_min_relay_feerate.GetFee(ws.m_vsize)) { + // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not + // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", strprintf("%d < %d", ws.m_modified_fees, m_pool.m_min_relay_feerate.GetFee(ws.m_vsize))); } @@ -981,6 +983,9 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) // descendant transaction of a direct conflict to pay a higher feerate than the transaction that // might replace them, under these rules. if (const auto err_string{PaysMoreThanConflicts(ws.m_iters_conflicting, newFeeRate, hash)}) { + // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not + // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. + // This must be changed if package RBF is enabled. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } @@ -1002,6 +1007,9 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) } if (const auto err_string{PaysForRBF(ws.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize, m_pool.m_incremental_relay_feerate, hash)}) { + // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not + // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. + // This must be changed if package RBF is enabled. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } return true; @@ -1139,7 +1147,8 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) if (!args.m_package_submission && !bypass_limits) { LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip()); if (!m_pool.exists(GenTxid::Txid(hash))) - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full"); + // The tx no longer meets our (new) mempool minimum feerate but could be reconsidered in a package. + return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool full"); } return true; } @@ -1201,7 +1210,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& } } - std::vector all_package_wtxids; + std::vector all_package_wtxids; all_package_wtxids.reserve(workspaces.size()); std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); @@ -1211,7 +1220,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate : CFeeRate{ws.m_modified_fees, static_cast(ws.m_vsize)}; const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids : - std::vector({ws.m_ptx->GetWitnessHash()}); + std::vector{ws.m_ptx->GetWitnessHash()}; results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, effective_feerate_wtxids)); @@ -1226,8 +1235,15 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef LOCK(m_pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool()) Workspace ws(ptx); + const std::vector single_wtxid{ws.m_ptx->GetWitnessHash()}; - if (!PreChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); + if (!PreChecks(args, ws)) { + if (ws.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) { + // Failed for fee reasons. Provide the effective feerate and which tx was included. + return MempoolAcceptResult::FeeFailure(ws.m_state, CFeeRate(ws.m_modified_fees, ws.m_vsize), single_wtxid); + } + return MempoolAcceptResult::Failure(ws.m_state); + } if (m_rbf && !ReplacementChecks(ws)) return MempoolAcceptResult::Failure(ws.m_state); @@ -1238,14 +1254,18 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef if (!ConsensusScriptChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); const CFeeRate effective_feerate{ws.m_modified_fees, static_cast(ws.m_vsize)}; - const std::vector single_wtxid{ws.m_ptx->GetWitnessHash()}; // Tx was accepted, but not added if (args.m_test_accept) { return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, single_wtxid); } - if (!Finalize(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); + if (!Finalize(args, ws)) { + // The only possible failure reason is fee-related (mempool full). + // Failed for fee reasons. Provide the effective feerate and which txns were included. + Assume(ws.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE); + return MempoolAcceptResult::FeeFailure(ws.m_state, CFeeRate(ws.m_modified_fees, ws.m_vsize), {ws.m_ptx->GetWitnessHash()}); + } GetMainSignals().TransactionAddedToMempool(ptx, m_pool.GetAndIncrementSequence()); @@ -1299,11 +1319,16 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: const auto m_total_modified_fees = std::accumulate(workspaces.cbegin(), workspaces.cend(), CAmount{0}, [](CAmount sum, auto& ws) { return sum + ws.m_modified_fees; }); const CFeeRate package_feerate(m_total_modified_fees, m_total_vsize); + std::vector all_package_wtxids; + all_package_wtxids.reserve(workspaces.size()); + std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), + [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); TxValidationState placeholder_state; if (args.m_package_feerates && !CheckFeeRate(m_total_vsize, m_total_modified_fees, placeholder_state)) { - package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-fee-too-low"); - return PackageMempoolAcceptResult(package_state, {}); + package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); + return PackageMempoolAcceptResult(package_state, {{workspaces.back().m_ptx->GetWitnessHash(), + MempoolAcceptResult::FeeFailure(placeholder_state, CFeeRate(m_total_modified_fees, m_total_vsize), all_package_wtxids)}}); } // Apply package mempool ancestor/descendant limits. Skip if there is only one transaction, @@ -1314,10 +1339,6 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: return PackageMempoolAcceptResult(package_state, std::move(results)); } - std::vector all_package_wtxids; - all_package_wtxids.reserve(workspaces.size()); - std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), - [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); for (Workspace& ws : workspaces) { ws.m_package_feerate = package_feerate; if (!PolicyScriptChecks(args, ws)) { @@ -1330,7 +1351,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate : CFeeRate{ws.m_modified_fees, static_cast(ws.m_vsize)}; const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids : - std::vector{ws.m_ptx->GetWitnessHash()}; + std::vector{ws.m_ptx->GetWitnessHash()}; results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, @@ -1510,7 +1531,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, // in package validation, because its fees should only be "used" once. assert(m_pool.exists(GenTxid::Wtxid(wtxid))); results_final.emplace(wtxid, single_res); - } else if (single_res.m_state.GetResult() != TxValidationResult::TX_MEMPOOL_POLICY && + } else if (single_res.m_state.GetResult() != TxValidationResult::TX_RECONSIDERABLE && single_res.m_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) { // Package validation policy only differs from individual policy in its evaluation // of feerate. For example, if a transaction fails here due to violation of a diff --git a/src/validation.h b/src/validation.h index 7ce60da6340..e669ec46d5d 100644 --- a/src/validation.h +++ b/src/validation.h @@ -114,7 +114,27 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex* pin void PruneBlockFilesManual(Chainstate& active_chainstate, int nManualPruneHeight); /** -* Validation result for a single transaction mempool acceptance. +* Validation result for a transaction evaluated by MemPoolAccept (single or package). +* Here are the expected fields and properties of a result depending on its ResultType, applicable to +* results returned from package evaluation: +*+---------------------------+----------------+-------------------+------------------+----------------+-------------------+ +*| Field or property | VALID | INVALID | MEMPOOL_ENTRY | DIFFERENT_WITNESS | +*| | |--------------------------------------| | | +*| | | TX_RECONSIDERABLE | Other | | | +*+---------------------------+----------------+-------------------+------------------+----------------+-------------------+ +*| txid in mempool? | yes | no | no* | yes | yes | +*| wtxid in mempool? | yes | no | no* | yes | no | +*| m_state | yes, IsValid() | yes, IsInvalid() | yes, IsInvalid() | yes, IsValid() | yes, IsValid() | +*| m_replaced_transactions | yes | no | no | no | no | +*| m_vsize | yes | no | no | yes | no | +*| m_base_fees | yes | no | no | yes | no | +*| m_effective_feerate | yes | yes | no | no | no | +*| m_wtxids_fee_calculations | yes | yes | no | no | no | +*| m_other_wtxid | no | no | no | no | yes | +*+---------------------------+----------------+-------------------+------------------+----------------+-------------------+ +* (*) Individual transaction acceptance doesn't return MEMPOOL_ENTRY and DIFFERENT_WITNESS. It returns +* INVALID, with the errors txn-already-in-mempool and txn-same-nonwitness-data-in-mempool +* respectively. In those cases, the txid or wtxid may be in the mempool for a TX_CONFLICT. */ struct MempoolAcceptResult { /** Used to indicate the results of mempool validation. */ @@ -130,7 +150,6 @@ struct MempoolAcceptResult { /** Contains information about why the transaction failed. */ const TxValidationState m_state; - // The following fields are only present when m_result_type = ResultType::VALID or MEMPOOL_ENTRY /** Mempool transactions replaced by the tx. */ const std::optional> m_replaced_transactions; /** Virtual size as used by the mempool, calculated using serialized size and sigops. */ @@ -141,7 +160,6 @@ struct MempoolAcceptResult { * using prioritisetransaction (i.e. modified fees). If this transaction was submitted as a * package, this is the package feerate, which may also include its descendants and/or * ancestors (see m_wtxids_fee_calculations below). - * Only present when m_result_type = ResultType::VALID. */ const std::optional m_effective_feerate; /** Contains the wtxids of the transactions used for fee-related checks. Includes this @@ -149,9 +167,8 @@ struct MempoolAcceptResult { * package. This is not necessarily equivalent to the list of transactions passed to * ProcessNewPackage(). * Only present when m_result_type = ResultType::VALID. */ - const std::optional> m_wtxids_fee_calculations; + const std::optional> m_wtxids_fee_calculations; - // The following field is only present when m_result_type = ResultType::DIFFERENT_WITNESS /** The wtxid of the transaction in the mempool which has the same txid but different witness. */ const std::optional m_other_wtxid; @@ -159,11 +176,17 @@ struct MempoolAcceptResult { return MempoolAcceptResult(state); } + static MempoolAcceptResult FeeFailure(TxValidationState state, + CFeeRate effective_feerate, + const std::vector& wtxids_fee_calculations) { + return MempoolAcceptResult(state, effective_feerate, wtxids_fee_calculations); + } + static MempoolAcceptResult Success(std::list&& replaced_txns, int64_t vsize, CAmount fees, CFeeRate effective_feerate, - const std::vector& wtxids_fee_calculations) { + const std::vector& wtxids_fee_calculations) { return MempoolAcceptResult(std::move(replaced_txns), vsize, fees, effective_feerate, wtxids_fee_calculations); } @@ -189,7 +212,7 @@ private: int64_t vsize, CAmount fees, CFeeRate effective_feerate, - const std::vector& wtxids_fee_calculations) + const std::vector& wtxids_fee_calculations) : m_result_type(ResultType::VALID), m_replaced_transactions(std::move(replaced_txns)), m_vsize{vsize}, @@ -197,6 +220,15 @@ private: m_effective_feerate(effective_feerate), m_wtxids_fee_calculations(wtxids_fee_calculations) {} + /** Constructor for fee-related failure case */ + explicit MempoolAcceptResult(TxValidationState state, + CFeeRate effective_feerate, + const std::vector& wtxids_fee_calculations) + : m_result_type(ResultType::INVALID), + m_state(state), + m_effective_feerate(effective_feerate), + m_wtxids_fee_calculations(wtxids_fee_calculations) {} + /** Constructor for already-in-mempool case. It wouldn't replace any transactions. */ explicit MempoolAcceptResult(int64_t vsize, CAmount fees) : m_result_type(ResultType::MEMPOOL_ENTRY), m_vsize{vsize}, m_base_fees(fees) {}