From a3baead7cb8376e3b09f1726b8c466648d187524 Mon Sep 17 00:00:00 2001 From: marcofleon Date: Mon, 10 Mar 2025 14:14:38 +0000 Subject: [PATCH 1/2] validation: use wtxid instead of txid in CheckEphemeralSpends --- src/bench/mempool_ephemeral_spends.cpp | 4 +- src/policy/ephemeral_policy.cpp | 7 +- src/policy/ephemeral_policy.h | 4 +- src/test/txvalidation_tests.cpp | 102 +++++++++++----------- src/validation.cpp | 10 +-- test/functional/mempool_ephemeral_dust.py | 8 +- 6 files changed, 68 insertions(+), 67 deletions(-) diff --git a/src/bench/mempool_ephemeral_spends.cpp b/src/bench/mempool_ephemeral_spends.cpp index a8f2efecb93..8f294113815 100644 --- a/src/bench/mempool_ephemeral_spends.cpp +++ b/src/bench/mempool_ephemeral_spends.cpp @@ -75,11 +75,11 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench) uint32_t iteration{0}; TxValidationState dummy_state; - Txid dummy_txid; + Wtxid dummy_wtxid; bench.run([&]() NO_THREAD_SAFETY_ANALYSIS { - CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool, dummy_state, dummy_txid); + CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool, dummy_state, dummy_wtxid); iteration++; }); } diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index b7d254dd630..0b35d13271e 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -30,7 +30,7 @@ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmou return true; } -bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Txid& out_child_txid) +bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Wtxid& out_child_wtxid) { if (!Assume(std::ranges::all_of(package, [](const auto& tx){return tx != nullptr;}))) { // Bail out of spend checks if caller gave us an invalid package @@ -83,9 +83,10 @@ bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, cons } if (!unspent_parent_dust.empty()) { - out_child_txid = tx->GetHash(); + const Txid& out_child_txid = tx->GetHash(); + out_child_wtxid = tx->GetWitnessHash(); out_child_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends", - strprintf("tx %s did not spend parent's ephemeral dust", out_child_txid.ToString())); + strprintf("tx %s (wtxid=%s) did not spend parent's ephemeral dust", out_child_txid.ToString(), out_child_wtxid.ToString())); return false; } } diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index 4667f25b5af..33876b49e78 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -50,9 +50,9 @@ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmou /** Must be called for each transaction(package) if any dust is in the package. * Checks that each transaction's parents have their dust spent by the child, * where parents are either in the mempool or in the package itself. - * Sets out_child_state and out_child_txid on failure. + * Sets out_child_state and out_child_wtxid on failure. * @returns true if all dust is properly spent. */ -bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Txid& out_child_txid); +bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Wtxid& out_child_wtxid); #endif // BITCOIN_POLICY_EPHEMERAL_POLICY_H diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 54d94b3ce4a..83d6c246849 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -118,7 +118,7 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) CTxMemPool::setEntries empty_ancestors; TxValidationState child_state; - Txid child_txid; + Wtxid child_wtxid; // Arbitrary non-0 feerate for these tests CFeeRate dustrelay(DUST_RELAY_TX_FEE); @@ -133,143 +133,143 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) // We first start with nothing "in the mempool", using package checks // Trivial single transaction with no dust - BOOST_CHECK(CheckEphemeralSpends({dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({dust_spend}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Now with dust, ok because the tx has no dusty parents - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Dust checks pass - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); auto dust_non_spend = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX - 1}}, /*version=*/2); // Child spending non-dust only from parent should be disallowed even if dust otherwise spent - const auto dust_non_spend_txid{dust_non_spend->GetHash()}; - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool, child_state, child_txid)); + const auto dust_non_spend_wtxid{dust_non_spend->GetWitnessHash()}; + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(!child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + BOOST_CHECK_EQUAL(child_wtxid, dust_non_spend_wtxid); child_state = TxValidationState(); - child_txid = Txid(); + child_wtxid = Wtxid(); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(!child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + BOOST_CHECK_EQUAL(child_wtxid, dust_non_spend_wtxid); child_state = TxValidationState(); - child_txid = Txid(); + child_wtxid = Wtxid(); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(!child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + BOOST_CHECK_EQUAL(child_wtxid, dust_non_spend_wtxid); child_state = TxValidationState(); - child_txid = Txid(); + child_wtxid = Wtxid(); auto grandparent_tx_2 = make_ephemeral_tx(random_outpoints(1), /*version=*/2); const auto dust_txid_2 = grandparent_tx_2->GetHash(); // Spend dust from one but not another is ok, as long as second grandparent has no child - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); auto dust_non_spend_both_parents = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX - 1}}, /*version=*/2); // But if we spend from the parent, it must spend dust - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(!child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash()); + BOOST_CHECK_EQUAL(child_wtxid, dust_non_spend_both_parents->GetWitnessHash()); child_state = TxValidationState(); - child_txid = Txid(); + child_wtxid = Wtxid(); auto dust_spend_both_parents = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX}}, /*version=*/2); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Spending other outputs is also correct, as long as the dusty one is spent const std::vector all_outpoints{COutPoint(dust_txid, 0), COutPoint(dust_txid, 1), COutPoint(dust_txid, 2), COutPoint(dust_txid_2, 0), COutPoint(dust_txid_2, 1), COutPoint(dust_txid_2, 2)}; auto dust_spend_all_outpoints = make_tx(all_outpoints, /*version=*/2); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // 2 grandparents with dust <- 1 dust-spending parent with dust <- child with no dust auto parent_with_dust = make_ephemeral_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX}}, /*version=*/2); // Ok for parent to have dust - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); auto child_no_dust = make_tx({COutPoint{parent_with_dust->GetHash(), EPHEMERAL_DUST_INDEX}}, /*version=*/2); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // 2 grandparents with dust <- 1 dust-spending parent with dust <- child with dust auto child_with_dust = make_ephemeral_tx({COutPoint{parent_with_dust->GetHash(), EPHEMERAL_DUST_INDEX}}, /*version=*/2); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Tests with parents in mempool // Nothing in mempool, this should pass for any transaction - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Add first grandparent to mempool and fetch entry AddToMempool(pool, entry.FromTx(grandparent_tx_1)); // Ignores ancestors that aren't direct parents - BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Valid spend of dust with grandparent in mempool - BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Second grandparent in same package - BOOST_CHECK(CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Order in package doesn't matter - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Add second grandparent to mempool AddToMempool(pool, entry.FromTx(grandparent_tx_2)); // Only spends single dust out of two direct parents - BOOST_CHECK(!CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(!child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash()); + BOOST_CHECK_EQUAL(child_wtxid, dust_non_spend_both_parents->GetWitnessHash()); child_state = TxValidationState(); - child_txid = Txid(); + child_wtxid = Wtxid(); // Spends both parents' dust - BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Now add dusty parent to mempool AddToMempool(pool, entry.FromTx(parent_with_dust)); // Passes dust checks even with non-parent ancestors - BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_wtxid)); BOOST_CHECK(child_state.IsValid()); - BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); } BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) diff --git a/src/validation.cpp b/src/validation.cpp index 93da4f326d4..9514e6c0d72 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1438,8 +1438,8 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef } if (m_pool.m_opts.require_standard) { - Txid dummy_txid; - if (!CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool, ws.m_state, dummy_txid)) { + Wtxid dummy_wtxid; + if (!CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool, ws.m_state, dummy_wtxid)) { return MempoolAcceptResult::Failure(ws.m_state); } } @@ -1592,10 +1592,10 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // Now that we've bounded the resulting possible ancestry count, check package for dust spends if (m_pool.m_opts.require_standard) { TxValidationState child_state; - Txid child_txid; - if (!CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate, m_pool, child_state, child_txid)) { + Wtxid child_wtxid; + if (!CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate, m_pool, child_state, child_wtxid)) { package_state.Invalid(PackageValidationResult::PCKG_TX, "unspent-dust"); - results.emplace(child_txid, MempoolAcceptResult::Failure(child_state)); + results.emplace(child_wtxid, MempoolAcceptResult::Failure(child_state)); return PackageMempoolAcceptResult(package_state, std::move(results)); } } diff --git a/test/functional/mempool_ephemeral_dust.py b/test/functional/mempool_ephemeral_dust.py index e614a9e6074..004db3656ed 100755 --- a/test/functional/mempool_ephemeral_dust.py +++ b/test/functional/mempool_ephemeral_dust.py @@ -233,8 +233,8 @@ class EphemeralDustTest(BitcoinTestFramework): unspent_sweep_tx = self.wallet.create_self_transfer_multi(fee_per_output=2000, utxos_to_spend=[dusty_tx["new_utxos"][0]], version=3) assert_greater_than(unspent_sweep_tx["fee"], sweep_tx["fee"]) res = self.nodes[0].submitpackage([dusty_tx["hex"], unspent_sweep_tx["hex"]]) - assert_equal(res["tx-results"][unspent_sweep_tx["wtxid"]]["error"], f"missing-ephemeral-spends, tx {unspent_sweep_tx['txid']} did not spend parent's ephemeral dust") - assert_raises_rpc_error(-26, f"missing-ephemeral-spends, tx {unspent_sweep_tx['txid']} did not spend parent's ephemeral dust", self.nodes[0].sendrawtransaction, unspent_sweep_tx["hex"]) + assert_equal(res["tx-results"][unspent_sweep_tx["wtxid"]]["error"], f"missing-ephemeral-spends, tx {unspent_sweep_tx['txid']} (wtxid={unspent_sweep_tx['wtxid']}) did not spend parent's ephemeral dust") + assert_raises_rpc_error(-26, f"missing-ephemeral-spends, tx {unspent_sweep_tx['txid']} (wtxid={unspent_sweep_tx['wtxid']}) did not spend parent's ephemeral dust", self.nodes[0].sendrawtransaction, unspent_sweep_tx["hex"]) assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]]) # Spend works with dust spent @@ -405,7 +405,7 @@ class EphemeralDustTest(BitcoinTestFramework): res = self.nodes[0].submitpackage([dusty_tx["hex"] for dusty_tx in dusty_txs] + [insufficient_sweep_tx["hex"]]) assert_equal(res['package_msg'], "transaction failed") - assert_equal(res['tx-results'][insufficient_sweep_tx['wtxid']]['error'], f"missing-ephemeral-spends, tx {insufficient_sweep_tx['txid']} did not spend parent's ephemeral dust") + assert_equal(res['tx-results'][insufficient_sweep_tx['wtxid']]['error'], f"missing-ephemeral-spends, tx {insufficient_sweep_tx['txid']} (wtxid={insufficient_sweep_tx['wtxid']}) did not spend parent's ephemeral dust") # Everything got in except for insufficient spend assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"] for dusty_tx in dusty_txs]) @@ -418,7 +418,7 @@ class EphemeralDustTest(BitcoinTestFramework): res = self.nodes[0].submitpackage([dusty_tx["hex"] for dusty_tx in dusty_txs] + [insufficient_sweep_tx["hex"]]) assert_equal(res['package_msg'], "transaction failed") - assert_equal(res['tx-results'][insufficient_sweep_tx["wtxid"]]["error"], f"missing-ephemeral-spends, tx {insufficient_sweep_tx['txid']} did not spend parent's ephemeral dust") + assert_equal(res['tx-results'][insufficient_sweep_tx["wtxid"]]["error"], f"missing-ephemeral-spends, tx {insufficient_sweep_tx['txid']} (wtxid={insufficient_sweep_tx['wtxid']}) did not spend parent's ephemeral dust") assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"] for dusty_tx in dusty_txs] + [sweep_all_but_one_tx["tx"]]) # Cycle out the partial sweep to avoid triggering package RBF behavior which limits package to no in-mempool ancestors From e637dc2c01c3b566e6c51c911c5881a8d206c924 Mon Sep 17 00:00:00 2001 From: marcofleon Date: Mon, 10 Mar 2025 14:38:24 +0000 Subject: [PATCH 2/2] refactor: Replace uint256 type with Wtxid in PackageMempoolAcceptResult struct --- src/validation.cpp | 10 +++++----- src/validation.h | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 9514e6c0d72..a1ac4e1e145 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -696,7 +696,7 @@ private: // cache - should only be called after successful validation of all transactions in the package. // Does not call LimitMempoolSize(), so mempool max_size_bytes may be temporarily exceeded. bool SubmitPackage(const ATMPArgs& args, std::vector& workspaces, PackageValidationState& package_state, - std::map& results) + std::map& results) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Compare a package's feerate against minimum allowed. @@ -1338,7 +1338,7 @@ void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args) bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& workspaces, PackageValidationState& package_state, - std::map& results) + std::map& results) { AssertLockHeld(cs_main); AssertLockHeld(m_pool.cs); @@ -1512,7 +1512,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: workspaces.reserve(txns.size()); std::transform(txns.cbegin(), txns.cend(), std::back_inserter(workspaces), [](const auto& tx) { return Workspace(tx); }); - std::map results; + std::map results; LOCK(m_pool.cs); @@ -1751,11 +1751,11 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, LOCK(m_pool.cs); // Stores results from which we will create the returned PackageMempoolAcceptResult. // A result may be changed if a mempool transaction is evicted later due to LimitMempoolSize(). - std::map results_final; + std::map results_final; // Results from individual validation which will be returned if no other result is available for // this transaction. "Nonfinal" because if a transaction fails by itself but succeeds later // (i.e. when evaluated with a fee-bumping child), the result in this map may be discarded. - std::map individual_results_nonfinal; + std::map individual_results_nonfinal; // Tracks whether we think package submission could result in successful entry to the mempool bool quit_early{false}; std::vector txns_package_eval; diff --git a/src/validation.h b/src/validation.h index 9e4fdbe6809..f6cbee28fc5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -154,7 +154,7 @@ struct MempoolAcceptResult { const std::optional> m_wtxids_fee_calculations; /** The wtxid of the transaction in the mempool which has the same txid but different witness. */ - const std::optional m_other_wtxid; + const std::optional m_other_wtxid; static MempoolAcceptResult Failure(TxValidationState state) { return MempoolAcceptResult(state); @@ -179,7 +179,7 @@ struct MempoolAcceptResult { return MempoolAcceptResult(vsize, fees); } - static MempoolAcceptResult MempoolTxDifferentWitness(const uint256& other_wtxid) { + static MempoolAcceptResult MempoolTxDifferentWitness(const Wtxid& other_wtxid) { return MempoolAcceptResult(other_wtxid); } @@ -218,7 +218,7 @@ private: : m_result_type(ResultType::MEMPOOL_ENTRY), m_vsize{vsize}, m_base_fees(fees) {} /** Constructor for witness-swapped case. */ - explicit MempoolAcceptResult(const uint256& other_wtxid) + explicit MempoolAcceptResult(const Wtxid& other_wtxid) : m_result_type(ResultType::DIFFERENT_WITNESS), m_other_wtxid(other_wtxid) {} }; @@ -234,18 +234,18 @@ struct PackageMempoolAcceptResult * 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. */ - std::map m_tx_results; + std::map m_tx_results; explicit PackageMempoolAcceptResult(PackageValidationState state, - std::map&& results) + std::map&& results) : m_state{state}, m_tx_results(std::move(results)) {} explicit PackageMempoolAcceptResult(PackageValidationState state, CFeeRate feerate, - std::map&& results) + std::map&& results) : m_state{state}, m_tx_results(std::move(results)) {} /** Constructor to create a PackageMempoolAcceptResult from a single MempoolAcceptResult */ - explicit PackageMempoolAcceptResult(const uint256& wtxid, const MempoolAcceptResult& result) + explicit PackageMempoolAcceptResult(const Wtxid& wtxid, const MempoolAcceptResult& result) : m_tx_results{ {wtxid, result} } {} };