From 83d4fb71260f268abd41d083fb3458476aed83ce Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 17 Dec 2021 14:06:16 +0000 Subject: [PATCH 1/6] [packages] return DIFFERENT_WITNESS for same-txid-different-witness tx The previous interface required callers to guess that the tx had been swapped and look up the tx again by txid to find a `MEMPOOL_ENTRY` result. This is a confusing interface. Instead, explicitly tell the caller that this transaction was `DIFFERENT_WITNESS` in the result linked to the mempool entry's wtxid. This gives the caller all the information they need in 1 lookup, and they can query the mempool for the other transaction if needed. --- src/validation.cpp | 5 +++-- src/validation.h | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index a98ffe006d9..616873057c5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1274,9 +1274,10 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, // transaction for the mempool one. Note that we are ignoring the validity of the // package transaction passed in. // TODO: allow witness replacement in packages. - auto iter = m_pool.GetIter(wtxid); + auto iter = m_pool.GetIter(txid); assert(iter != std::nullopt); - results.emplace(txid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee())); + // Provide the wtxid of the mempool tx so that the caller can look it up in the mempool. + results.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(iter.value()->GetTx().GetWitnessHash())); } else { // Transaction does not already exist in the mempool. txns_new.push_back(tx); diff --git a/src/validation.h b/src/validation.h index 16f4bfe7419..cdd32438dab 100644 --- a/src/validation.h +++ b/src/validation.h @@ -161,8 +161,12 @@ struct MempoolAcceptResult { VALID, //!> Fully validated, valid. INVALID, //!> Invalid. MEMPOOL_ENTRY, //!> Valid, transaction was already in the mempool. + DIFFERENT_WITNESS, //!> Not validated. A same-txid-different-witness tx (see m_other_wtxid) already exists in the mempool and was not replaced. }; + /** Result type. Present in all MempoolAcceptResults. */ const ResultType m_result_type; + + /** 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 @@ -173,6 +177,10 @@ struct MempoolAcceptResult { /** Raw base fees in satoshis. */ const std::optional m_base_fees; + // 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; + static MempoolAcceptResult Failure(TxValidationState state) { return MempoolAcceptResult(state); } @@ -185,6 +193,10 @@ struct MempoolAcceptResult { return MempoolAcceptResult(vsize, fees); } + static MempoolAcceptResult MempoolTxDifferentWitness(const uint256& other_wtxid) { + return MempoolAcceptResult(other_wtxid); + } + // Private constructors. Use static methods MempoolAcceptResult::Success, etc. to construct. private: /** Constructor for failure case */ @@ -201,6 +213,10 @@ private: /** 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) {} + + /** Constructor for witness-swapped case. */ + explicit MempoolAcceptResult(const uint256& other_wtxid) + : m_result_type(ResultType::DIFFERENT_WITNESS), m_other_wtxid(other_wtxid) {} }; /** @@ -210,7 +226,7 @@ struct PackageMempoolAcceptResult { const PackageValidationState m_state; /** - * Map from (w)txid to finished MempoolAcceptResults. The client is responsible + * Map from wtxid to finished MempoolAcceptResults. The client is responsible * for keeping track of the transaction objects themselves. If a result is not * present, it means validation was unfinished for that transaction. If there * was a package-wide error (see result in m_state), m_tx_results will be empty. From 9ad211c5753dbd148ba6f0ed56854f6364362ca8 Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 17 Dec 2021 14:18:02 +0000 Subject: [PATCH 2/6] [doc] more detailed explanation for deduplication --- src/validation.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 616873057c5..b48eb6ac3c5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1250,10 +1250,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, LOCK(m_pool.cs); std::map results; - // As node operators are free to set their mempool policies however they please, it's possible - // for package transaction(s) to already be in the mempool, and we don't want to reject the - // entire package in that case (as that could be a censorship vector). Filter the transactions - // that are already in mempool and add their information to results, since we already have them. + // Node operators are free to set their mempool policies however they please, nodes may receive + // transactions in different orders, and malicious counterparties may try to take advantage of + // policy differences to pin or delay propagation of transactions. As such, it's possible for + // some package transaction(s) to already be in the mempool, and we don't want to reject the + // entire package in that case (as that could be a censorship vector). De-duplicate the + // transactions that are already in the mempool, and only call AcceptMultipleTransactions() with + // the new transactions. This ensures we don't double-count transaction counts and sizes when + // checking ancestor/descendant limits, or double-count transaction fees for fee-related policy. std::vector txns_new; for (const auto& tx : package) { const auto& wtxid = tx->GetWitnessHash(); From 2db77cd3b835d052de678755bcdde5a645ce2d65 Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 17 Dec 2021 15:02:49 +0000 Subject: [PATCH 3/6] [unit test] different witness in package submission If/when witness replacement is implemented in the future, this test case can be easily replaced with witness replacement tests. --- src/test/txpackage_tests.cpp | 120 +++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 6f78b438266..022510d19ef 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -327,4 +327,124 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash()))); } } + +// Tests for packages containing transactions that have same-txid-different-witness equivalents in +// the mempool. +BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) +{ + LOCK(cs_main); + + // Transactions with a same-txid-different-witness transaction in the mempool should be ignored, + // and the mempool entry's wtxid returned. + CScript witnessScript = CScript() << OP_DROP << OP_TRUE; + CScript scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessScript)); + auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[0], /*vout=*/ 0, + /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey, + /*output_destination=*/ scriptPubKey, + /*output_amount=*/ CAmount(49 * COIN), /*submit=*/ false); + CTransactionRef ptx_parent = MakeTransactionRef(mtx_parent); + + // Make two children with the same txid but different witnesses. + CScriptWitness witness1; + witness1.stack.push_back(std::vector(1)); + witness1.stack.push_back(std::vector(witnessScript.begin(), witnessScript.end())); + + CScriptWitness witness2(witness1); + witness2.stack.push_back(std::vector(2)); + witness2.stack.push_back(std::vector(witnessScript.begin(), witnessScript.end())); + + CKey child_key; + child_key.MakeNewKey(true); + CScript child_locking_script = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey())); + CMutableTransaction mtx_child1; + mtx_child1.nVersion = 1; + mtx_child1.vin.resize(1); + mtx_child1.vin[0].prevout.hash = ptx_parent->GetHash(); + mtx_child1.vin[0].prevout.n = 0; + mtx_child1.vin[0].scriptSig = CScript(); + mtx_child1.vin[0].scriptWitness = witness1; + mtx_child1.vout.resize(1); + mtx_child1.vout[0].nValue = CAmount(48 * COIN); + mtx_child1.vout[0].scriptPubKey = child_locking_script; + + CMutableTransaction mtx_child2{mtx_child1}; + mtx_child2.vin[0].scriptWitness = witness2; + + CTransactionRef ptx_child1 = MakeTransactionRef(mtx_child1); + CTransactionRef ptx_child2 = MakeTransactionRef(mtx_child2); + + // child1 and child2 have the same txid + BOOST_CHECK_EQUAL(ptx_child1->GetHash(), ptx_child2->GetHash()); + // child1 and child2 have different wtxids + BOOST_CHECK(ptx_child1->GetWitnessHash() != ptx_child2->GetWitnessHash()); + + // Try submitting Package1{parent, child1} and Package2{parent, child2} where the children are + // same-txid-different-witness. + { + 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()); + 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()))); + + 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()); + 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()))); + } + + // Try submitting Package1{child2, grandchild} where child2 is same-txid-different-witness as + // the in-mempool transaction, child1. Since child1 exists in the mempool and its outputs are + // available, child2 should be ignored and grandchild should be accepted. + // + // This tests a potential censorship vector in which an attacker broadcasts a competing package + // where a parent's witness is mutated. The honest package should be accepted despite the fact + // that we don't allow witness replacement. + CKey grandchild_key; + grandchild_key.MakeNewKey(true); + CScript grandchild_locking_script = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey())); + auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/ ptx_child2, /* vout=*/ 0, + /*input_height=*/ 0, /*input_signing_key=*/ child_key, + /*output_destination=*/ grandchild_locking_script, + /*output_amount=*/ CAmount(47 * COIN), /*submit=*/ false); + CTransactionRef ptx_grandchild = MakeTransactionRef(mtx_grandchild); + + // We already submitted child1 above. + { + 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()); + 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()))); + } +} BOOST_AUTO_TEST_SUITE_END() From 9d88853e0c85f765f7d982b15e8122ede50110ed Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 7 Jan 2022 15:45:23 +0000 Subject: [PATCH 4/6] AcceptPackage fixups No behavior changes, just clarifications. --- src/validation.cpp | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index b48eb6ac3c5..8c142d5908e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -596,10 +596,10 @@ private: // Submit all transactions to the mempool and call ConsensusScriptChecks to add to the script // cache - should only be called after successful validation of all transactions in the package. - // The package may end up partially-submitted after size limitting; returns true if all + // The package may end up partially-submitted after size limiting; returns true if all // transactions are successfully added to the mempool, false otherwise. - bool FinalizePackage(const ATMPArgs& args, std::vector& workspaces, PackageValidationState& package_state, - std::map& results) + bool SubmitPackage(const ATMPArgs& args, std::vector& workspaces, PackageValidationState& package_state, + std::map& results) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Compare a package's feerate against minimum allowed. @@ -1038,12 +1038,17 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) return true; } -bool MemPoolAccept::FinalizePackage(const ATMPArgs& args, std::vector& workspaces, - PackageValidationState& package_state, - std::map& results) +bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& workspaces, + PackageValidationState& package_state, + std::map& results) { AssertLockHeld(cs_main); AssertLockHeld(m_pool.cs); + // Sanity check: none of the transactions should be in the mempool, and none of the transactions + // should have a same-txid-different-witness equivalent in the mempool. + assert(std::all_of(workspaces.cbegin(), workspaces.cend(), [this](const auto& ws){ + return !m_pool.exists(GenTxid::Txid(ws.m_ptx->GetHash())); })); + bool all_submitted = true; // ConsensusScriptChecks adds to the script cache and is therefore consensus-critical; // CheckInputsFromMempoolAndCache asserts that transactions only spend coins available from the @@ -1058,10 +1063,10 @@ bool MemPoolAccept::FinalizePackage(const ATMPArgs& args, std::vector // Re-calculate mempool ancestors to call addUnchecked(). They may have changed since the // last calculation done in PreChecks, since package ancestors have already been submitted. - std::string err_string; + std::string unused_err_string; if(!m_pool.CalculateMemPoolAncestors(*ws.m_entry, ws.m_ancestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, - m_limit_descendant_size, err_string)) { + m_limit_descendant_size, unused_err_string)) { results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); // Since PreChecks() and PackageMempoolChecks() both enforce limits, this should never fail. all_submitted = Assume(false); @@ -1188,7 +1193,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: if (args.m_test_accept) return PackageMempoolAcceptResult(package_state, std::move(results)); - if (!FinalizePackage(args, workspaces, package_state, results)) { + if (!SubmitPackage(args, workspaces, package_state, results)) { package_state.Invalid(PackageValidationResult::PCKG_TX, "submission failed"); return PackageMempoolAcceptResult(package_state, std::move(results)); } @@ -1214,11 +1219,13 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, return PackageMempoolAcceptResult(package_state, {}); } - const auto& child = package[package.size() - 1]; + // IsChildWithParents() guarantees the package is > 1 transactions. + assert(package.size() > 1); // The package must be 1 child with all of its unconfirmed parents. The package is expected to // be sorted, so the last transaction is the child. + const auto& child = package.back(); std::unordered_set unconfirmed_parent_txids; - std::transform(package.cbegin(), package.end() - 1, + std::transform(package.cbegin(), package.cend() - 1, std::inserter(unconfirmed_parent_txids, unconfirmed_parent_txids.end()), [](const auto& tx) { return tx->GetHash(); }); @@ -1348,12 +1355,12 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx }(); // Uncache coins pertaining to transactions that were not submitted to the mempool. - // Ensure the coins cache is still within limits. if (test_accept || result.m_state.IsInvalid()) { for (const COutPoint& hashTx : coins_to_uncache) { active_chainstate.CoinsTip().Uncache(hashTx); } } + // Ensure the coins cache is still within limits. BlockValidationState state_dummy; active_chainstate.FlushStateToDisk(state_dummy, FlushStateMode::PERIODIC); return result; From de075a98eaf0b3f7676c5c78b50b66902202b34c Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 7 Jan 2022 16:55:53 +0000 Subject: [PATCH 5/6] [validation] better handle errors in SubmitPackage Behavior change: don't quit right after LimitMempoolSize() when a package is partially submitted. We should still send TransactionAddedToMempool notifications for transactions that were submitted. Not behavior change: add a new package validation result for mempool logic errors. --- src/policy/packages.h | 1 + src/validation.cpp | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/policy/packages.h b/src/policy/packages.h index d2744f1265a..9f274f6b7d1 100644 --- a/src/policy/packages.h +++ b/src/policy/packages.h @@ -25,6 +25,7 @@ enum class PackageValidationResult { PCKG_RESULT_UNSET = 0, //!< Initial value. The package has not yet been rejected. PCKG_POLICY, //!< The package itself is invalid (e.g. too many transactions). PCKG_TX, //!< At least one tx is invalid. + PCKG_MEMPOOL_ERROR, //!< Mempool logic error. }; /** A package is an ordered list of transactions. The transactions cannot conflict with (spend the diff --git a/src/validation.cpp b/src/validation.cpp index 8c142d5908e..ff3d891b989 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1058,7 +1058,10 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& if (!ConsensusScriptChecks(args, ws)) { results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); // Since PolicyScriptChecks() passed, this should never fail. - all_submitted = Assume(false); + all_submitted = false; + package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR, + strprintf("BUG! PolicyScriptChecks succeeded but ConsensusScriptChecks failed: %s", + ws.m_ptx->GetHash().ToString())); } // Re-calculate mempool ancestors to call addUnchecked(). They may have changed since the @@ -1069,7 +1072,10 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& m_limit_descendant_size, unused_err_string)) { results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); // Since PreChecks() and PackageMempoolChecks() both enforce limits, this should never fail. - all_submitted = Assume(false); + all_submitted = false; + package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR, + strprintf("BUG! Mempool ancestors or descendants were underestimated: %s", + ws.m_ptx->GetHash().ToString())); } // If we call LimitMempoolSize() for each individual Finalize(), the mempool will not take // the transaction's descendant feerate into account because it hasn't seen them yet. Also, @@ -1079,7 +1085,9 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& if (!Finalize(args, ws)) { results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); // Since LimitMempoolSize() won't be called, this should never fail. - all_submitted = Assume(false); + all_submitted = false; + package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR, + strprintf("BUG! Adding to mempool failed: %s", ws.m_ptx->GetHash().ToString())); } } @@ -1088,7 +1096,6 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip(), gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetIntArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); - if (!all_submitted) return false; // Find the wtxids of the transactions that made it into the mempool. Allow partial submission, // but don't report success unless they all made it into the mempool. @@ -1194,7 +1201,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: if (args.m_test_accept) return PackageMempoolAcceptResult(package_state, std::move(results)); if (!SubmitPackage(args, workspaces, package_state, results)) { - package_state.Invalid(PackageValidationResult::PCKG_TX, "submission failed"); + // PackageValidationState filled in by SubmitPackage(). return PackageMempoolAcceptResult(package_state, std::move(results)); } From 3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 17 Jan 2022 11:57:48 +0000 Subject: [PATCH 6/6] [unit test] package parents are a mix --- src/test/txpackage_tests.cpp | 112 +++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 022510d19ef..560efb6b420 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -332,6 +332,8 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) // the mempool. BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) { + // Mine blocks to mature coinbases. + mineBlocks(5); LOCK(cs_main); // Transactions with a same-txid-different-witness transaction in the mempool should be ignored, @@ -446,5 +448,115 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_child2->GetWitnessHash()))); BOOST_CHECK(m_node.mempool->exists(GenTxid::Wtxid(ptx_grandchild->GetWitnessHash()))); } + + // A package Package{parent1, parent2, parent3, child} where the parents are a mixture of + // identical-tx-in-mempool, same-txid-different-witness-in-mempool, and new transactions. + Package package_mixed; + + // Give all the parents anyone-can-spend scripts so we don't have to deal with signing the child. + CScript acs_script = CScript() << OP_TRUE; + CScript acs_spk = GetScriptForDestination(WitnessV0ScriptHash(acs_script)); + CScriptWitness acs_witness; + acs_witness.stack.push_back(std::vector(acs_script.begin(), acs_script.end())); + + // parent1 will already be in the mempool + auto mtx_parent1 = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[1], /*vout=*/ 0, + /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey, + /*output_destination=*/ acs_spk, + /*output_amount=*/ CAmount(49 * COIN), /*submit=*/ true); + CTransactionRef ptx_parent1 = MakeTransactionRef(mtx_parent1); + package_mixed.push_back(ptx_parent1); + + // parent2 will have a same-txid-different-witness tx already in the mempool + CScript grandparent2_script = CScript() << OP_DROP << OP_TRUE; + CScript grandparent2_spk = GetScriptForDestination(WitnessV0ScriptHash(grandparent2_script)); + CScriptWitness parent2_witness1; + parent2_witness1.stack.push_back(std::vector(1)); + parent2_witness1.stack.push_back(std::vector(grandparent2_script.begin(), grandparent2_script.end())); + CScriptWitness parent2_witness2; + parent2_witness2.stack.push_back(std::vector(2)); + parent2_witness2.stack.push_back(std::vector(grandparent2_script.begin(), grandparent2_script.end())); + + // Create grandparent2 creating an output with multiple spending paths. Submit to mempool. + auto mtx_grandparent2 = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[2], /* vout=*/ 0, + /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey, + /*output_destination=*/ grandparent2_spk, + /*output_amount=*/ CAmount(49 * COIN), /*submit=*/ true); + CTransactionRef ptx_grandparent2 = MakeTransactionRef(mtx_grandparent2); + + CMutableTransaction mtx_parent2_v1; + mtx_parent2_v1.nVersion = 1; + mtx_parent2_v1.vin.resize(1); + mtx_parent2_v1.vin[0].prevout.hash = ptx_grandparent2->GetHash(); + mtx_parent2_v1.vin[0].prevout.n = 0; + mtx_parent2_v1.vin[0].scriptSig = CScript(); + mtx_parent2_v1.vin[0].scriptWitness = parent2_witness1; + mtx_parent2_v1.vout.resize(1); + mtx_parent2_v1.vout[0].nValue = CAmount(48 * COIN); + mtx_parent2_v1.vout[0].scriptPubKey = acs_spk; + + CMutableTransaction mtx_parent2_v2{mtx_parent2_v1}; + mtx_parent2_v2.vin[0].scriptWitness = parent2_witness2; + + CTransactionRef ptx_parent2_v1 = MakeTransactionRef(mtx_parent2_v1); + CTransactionRef ptx_parent2_v2 = MakeTransactionRef(mtx_parent2_v2); + // Put parent2_v1 in the package, submit parent2_v2 to the mempool. + const MempoolAcceptResult parent2_v2_result = m_node.chainman->ProcessTransaction(ptx_parent2_v2); + BOOST_CHECK(parent2_v2_result.m_result_type == MempoolAcceptResult::ResultType::VALID); + package_mixed.push_back(ptx_parent2_v1); + + // parent3 will be a new transaction + auto mtx_parent3 = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[3], /*vout=*/ 0, + /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey, + /*output_destination=*/ acs_spk, + /*output_amount=*/ CAmount(49 * COIN), /*submit=*/ false); + CTransactionRef ptx_parent3 = MakeTransactionRef(mtx_parent3); + package_mixed.push_back(ptx_parent3); + + // child spends parent1, parent2, and parent3 + CKey mixed_grandchild_key; + mixed_grandchild_key.MakeNewKey(true); + CScript mixed_child_spk = GetScriptForDestination(WitnessV0KeyHash(mixed_grandchild_key.GetPubKey())); + + CMutableTransaction mtx_mixed_child; + mtx_mixed_child.vin.push_back(CTxIn(COutPoint(ptx_parent1->GetHash(), 0))); + mtx_mixed_child.vin.push_back(CTxIn(COutPoint(ptx_parent2_v1->GetHash(), 0))); + mtx_mixed_child.vin.push_back(CTxIn(COutPoint(ptx_parent3->GetHash(), 0))); + mtx_mixed_child.vin[0].scriptWitness = acs_witness; + mtx_mixed_child.vin[1].scriptWitness = acs_witness; + mtx_mixed_child.vin[2].scriptWitness = acs_witness; + mtx_mixed_child.vout.push_back(CTxOut(145 * COIN, mixed_child_spk)); + CTransactionRef ptx_mixed_child = MakeTransactionRef(mtx_mixed_child); + package_mixed.push_back(ptx_mixed_child); + + // Submit package: + // parent1 should be ignored + // parent2_v1 should be ignored (and v2 wtxid returned) + // parent3 should be accepted + // 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()); + 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()); + + 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()))); + } } BOOST_AUTO_TEST_SUITE_END()