From cbf1a47d6062ec2c2c4a788636e8c950a0271997 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 11:54:28 -0500 Subject: [PATCH 01/29] CheckEphemeralSpends: only compute txid of tx when needed --- src/policy/ephemeral_policy.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index 6854822e35129..e0b96d32bb6c9 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -33,7 +33,6 @@ std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_r } for (const auto& tx : package) { - Txid txid = tx->GetHash(); std::unordered_set processed_parent_set; std::unordered_set unspent_parent_dust; @@ -70,7 +69,7 @@ std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_r } if (!unspent_parent_dust.empty()) { - return txid; + return tx->GetHash(); } } From 04a614bf9a7bb6abad150a3edf8938358f54d55b Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 11:56:32 -0500 Subject: [PATCH 02/29] Rename CheckValidEphemeralTx to PreCheckEphemeralTx --- src/policy/ephemeral_policy.cpp | 2 +- src/policy/ephemeral_policy.h | 4 ++-- src/validation.cpp | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index e0b96d32bb6c9..eadc1d24752ef 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -10,7 +10,7 @@ bool HasDust(const CTransactionRef& tx, CFeeRate dust_relay_rate) return std::any_of(tx->vout.cbegin(), tx->vout.cend(), [&](const auto& output) { return IsDust(output, dust_relay_rate); }); } -bool CheckValidEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state) +bool PreCheckEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state) { // We never want to give incentives to mine this transaction alone if ((base_fee != 0 || mod_fee != 0) && HasDust(tx, dust_relay_rate)) { diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index 26140f9a020b9..65dbab48e2525 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -15,7 +15,7 @@ * set. * This is ensured by requiring: - * - CheckValidEphemeralTx checks are respected + * - PreCheckEphemeralTx checks are respected * - The parent has no child (and 0-fee as implied above to disincentivize mining) * - OR the parent transaction has exactly one child, and the dust is spent by that child * @@ -43,7 +43,7 @@ bool HasDust(const CTransactionRef& tx, CFeeRate dust_relay_rate); * Does context-less checks about a single transaction. * Returns false if the fee is non-zero and dust exists, populating state. True otherwise. */ -bool CheckValidEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state); +bool PreCheckEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state); /** 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, diff --git a/src/validation.cpp b/src/validation.cpp index 62cae2cfb5046..1510200abf073 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -915,8 +915,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Enforces 0-fee for dust transactions, no incentive to be mined alone if (m_pool.m_opts.require_standard) { - if (!CheckValidEphemeralTx(ptx, m_pool.m_opts.dust_relay_feerate, ws.m_base_fees, ws.m_modified_fees, state)) { - return false; // state filled in by CheckValidEphemeralTx + if (!PreCheckEphemeralTx(ptx, m_pool.m_opts.dust_relay_feerate, ws.m_base_fees, ws.m_modified_fees, state)) { + return false; // state filled in by PreCheckEphemeralTx } } From 3ed930a1f41f7d7160c6ede5dcf3d4d5f1cfa876 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 11:59:42 -0500 Subject: [PATCH 03/29] Have HasDust and PreCheckValidEphemeralTx take CTransaction --- src/policy/ephemeral_policy.cpp | 6 +++--- src/policy/ephemeral_policy.h | 4 ++-- src/rpc/mining.cpp | 2 +- src/validation.cpp | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index eadc1d24752ef..67421683ab989 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -5,12 +5,12 @@ #include #include -bool HasDust(const CTransactionRef& tx, CFeeRate dust_relay_rate) +bool HasDust(const CTransaction& tx, CFeeRate dust_relay_rate) { - return std::any_of(tx->vout.cbegin(), tx->vout.cend(), [&](const auto& output) { return IsDust(output, dust_relay_rate); }); + return std::any_of(tx.vout.cbegin(), tx.vout.cend(), [&](const auto& output) { return IsDust(output, dust_relay_rate); }); } -bool PreCheckEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state) +bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state) { // We never want to give incentives to mine this transaction alone if ((base_fee != 0 || mod_fee != 0) && HasDust(tx, dust_relay_rate)) { diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index 65dbab48e2525..0ed4a54eb70c6 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -35,7 +35,7 @@ */ /** Returns true if transaction contains dust */ -bool HasDust(const CTransactionRef& tx, CFeeRate dust_relay_rate); +bool HasDust(const CTransaction& tx, CFeeRate dust_relay_rate); /* All the following checks are only called if standardness rules are being applied. */ @@ -43,7 +43,7 @@ bool HasDust(const CTransactionRef& tx, CFeeRate dust_relay_rate); * Does context-less checks about a single transaction. * Returns false if the fee is non-zero and dust exists, populating state. True otherwise. */ -bool PreCheckEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state); +bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state); /** 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, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 006ec3c78c741..d4491ab1080c4 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -496,7 +496,7 @@ static RPCHelpMan prioritisetransaction() // Non-0 fee dust transactions are not allowed for entry, and modification not allowed afterwards const auto& tx = mempool.get(hash); - if (tx && HasDust(tx, mempool.m_opts.dust_relay_feerate)) { + if (tx && HasDust(*tx, mempool.m_opts.dust_relay_feerate)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is not supported for transactions with dust outputs."); } diff --git a/src/validation.cpp b/src/validation.cpp index 1510200abf073..91a9d02611ed8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -915,7 +915,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Enforces 0-fee for dust transactions, no incentive to be mined alone if (m_pool.m_opts.require_standard) { - if (!PreCheckEphemeralTx(ptx, m_pool.m_opts.dust_relay_feerate, ws.m_base_fees, ws.m_modified_fees, state)) { + if (!PreCheckEphemeralTx(*ptx, m_pool.m_opts.dust_relay_feerate, ws.m_base_fees, ws.m_modified_fees, state)) { return false; // state filled in by PreCheckEphemeralTx } } From 62016b32300123a44599e649b4f35a3a0f32565f Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:04:29 -0500 Subject: [PATCH 04/29] Use std::ranges for ephemeral policy checks --- src/policy/ephemeral_policy.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index 67421683ab989..08fb2626096f3 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -7,7 +7,7 @@ bool HasDust(const CTransaction& tx, CFeeRate dust_relay_rate) { - return std::any_of(tx.vout.cbegin(), tx.vout.cend(), [&](const auto& output) { return IsDust(output, dust_relay_rate); }); + return std::ranges::any_of(tx.vout, [&](const auto& output) { return IsDust(output, dust_relay_rate); }); } bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state) @@ -22,7 +22,7 @@ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmou std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool) { - if (!Assume(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;}))) { + 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 return std::nullopt; } From c6859ce2de7531e42fc304b69d74ca0d8e99ea29 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:14:32 -0500 Subject: [PATCH 05/29] Move+rename GetDustIndexes -> GetDust Use to replace HasDust and where appropraite --- src/policy/ephemeral_policy.cpp | 7 +------ src/policy/ephemeral_policy.h | 3 --- src/policy/policy.cpp | 14 ++++++++++---- src/policy/policy.h | 2 ++ src/rpc/mining.cpp | 2 +- src/test/fuzz/package_eval.cpp | 4 ++-- src/test/util/txmempool.cpp | 13 +------------ src/test/util/txmempool.h | 5 ----- 8 files changed, 17 insertions(+), 33 deletions(-) diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index 08fb2626096f3..c1e184c81f5b3 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -5,15 +5,10 @@ #include #include -bool HasDust(const CTransaction& tx, CFeeRate dust_relay_rate) -{ - return std::ranges::any_of(tx.vout, [&](const auto& output) { return IsDust(output, dust_relay_rate); }); -} - bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state) { // We never want to give incentives to mine this transaction alone - if ((base_fee != 0 || mod_fee != 0) && HasDust(tx, dust_relay_rate)) { + if ((base_fee != 0 || mod_fee != 0) && !GetDust(tx, dust_relay_rate).empty()) { return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust", "tx with dust output must be 0-fee"); } diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index 0ed4a54eb70c6..ac889e8234ea0 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -34,9 +34,6 @@ * are the only way to bring fees. */ -/** Returns true if transaction contains dust */ -bool HasDust(const CTransaction& tx, CFeeRate dust_relay_rate); - /* All the following checks are only called if standardness rules are being applied. */ /** Must be called for each transaction once transaction fees are known. diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 21c35af5ccbca..ed33692823517 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -67,6 +67,15 @@ bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFeeIn) return (txout.nValue < GetDustThreshold(txout, dustRelayFeeIn)); } +std::vector GetDust(const CTransaction& tx, CFeeRate dust_relay_rate) +{ + std::vector dust_outputs; + for (uint32_t i{0}; i < tx.vout.size(); ++i) { + if (IsDust(tx.vout[i], dust_relay_rate)) dust_outputs.push_back(i); + } + return dust_outputs; +} + bool IsStandard(const CScript& scriptPubKey, const std::optional& max_datacarrier_bytes, TxoutType& whichType) { std::vector > vSolutions; @@ -129,7 +138,6 @@ bool IsStandardTx(const CTransaction& tx, const std::optional& max_dat } unsigned int nDataOut = 0; - unsigned int num_dust_outputs{0}; TxoutType whichType; for (const CTxOut& txout : tx.vout) { if (!::IsStandard(txout.scriptPubKey, max_datacarrier_bytes, whichType)) { @@ -142,13 +150,11 @@ bool IsStandardTx(const CTransaction& tx, const std::optional& max_dat else if ((whichType == TxoutType::MULTISIG) && (!permit_bare_multisig)) { reason = "bare-multisig"; return false; - } else if (IsDust(txout, dust_relay_fee)) { - num_dust_outputs++; } } // Only MAX_DUST_OUTPUTS_PER_TX dust is permitted(on otherwise valid ephemeral dust) - if (num_dust_outputs > MAX_DUST_OUTPUTS_PER_TX) { + if (GetDust(tx, dust_relay_fee).size() > MAX_DUST_OUTPUTS_PER_TX) { reason = "dust"; return false; } diff --git a/src/policy/policy.h b/src/policy/policy.h index 0488f8dbee86c..4412f2db87ab1 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -131,6 +131,8 @@ bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFee); bool IsStandard(const CScript& scriptPubKey, const std::optional& max_datacarrier_bytes, TxoutType& whichType); +/** Get the vout index numbers of all dust outputs */ +std::vector GetDust(const CTransaction& tx, CFeeRate dust_relay_rate); // Changing the default transaction version requires a two step process: first // adapting relay policy by bumping TX_MAX_STANDARD_VERSION, and then later diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d4491ab1080c4..ca88fad61ef1b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -496,7 +496,7 @@ static RPCHelpMan prioritisetransaction() // Non-0 fee dust transactions are not allowed for entry, and modification not allowed afterwards const auto& tx = mempool.get(hash); - if (tx && HasDust(*tx, mempool.m_opts.dust_relay_feerate)) { + if (tx && !GetDust(*tx, mempool.m_opts.dust_relay_feerate).empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is not supported for transactions with dust outputs."); } diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index f944170474577..56ec52f852edf 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -167,7 +167,7 @@ std::optional GetChildEvictingPrevout(const CTxMemPool& tx_pool) LOCK(tx_pool.cs); for (const auto& tx_info : tx_pool.infoAll()) { const auto& entry = *Assert(tx_pool.GetEntry(tx_info.tx->GetHash())); - std::vector dust_indexes{GetDustIndexes(tx_info.tx, tx_pool.m_opts.dust_relay_feerate)}; + std::vector dust_indexes{GetDust(*tx_info.tx, tx_pool.m_opts.dust_relay_feerate)}; if (!dust_indexes.empty()) { const auto& children = entry.GetMemPoolChildrenConst(); if (!children.empty()) { @@ -311,7 +311,7 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) // filter for ephemeral dust GetEntry if (tx_pool.exists(GenTxid::Txid(txid))) { const auto tx_info{tx_pool.info(GenTxid::Txid(txid))}; - if (GetDustIndexes(tx_info.tx, tx_pool.m_opts.dust_relay_feerate).empty()) { + if (GetDust(*tx_info.tx, tx_pool.m_opts.dust_relay_feerate).empty()) { tx_pool.PrioritiseTransaction(txid.ToUint256(), delta); } } diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index 0191653ff7a9f..e393804f72c34 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -141,24 +141,13 @@ std::optional CheckPackageMempoolAcceptResult(const Package& txns, return std::nullopt; } -std::vector GetDustIndexes(const CTransactionRef& tx_ref, CFeeRate dust_relay_rate) -{ - std::vector dust_indexes; - for (size_t i = 0; i < tx_ref->vout.size(); ++i) { - const auto& output = tx_ref->vout[i]; - if (IsDust(output, dust_relay_rate)) dust_indexes.push_back(i); - } - - return dust_indexes; -} - void CheckMempoolEphemeralInvariants(const CTxMemPool& tx_pool) { LOCK(tx_pool.cs); for (const auto& tx_info : tx_pool.infoAll()) { const auto& entry = *Assert(tx_pool.GetEntry(tx_info.tx->GetHash())); - std::vector dust_indexes = GetDustIndexes(tx_info.tx, tx_pool.m_opts.dust_relay_feerate); + std::vector dust_indexes = GetDust(*tx_info.tx, tx_pool.m_opts.dust_relay_feerate); Assert(dust_indexes.size() < 2); diff --git a/src/test/util/txmempool.h b/src/test/util/txmempool.h index dbbd8e7665a28..932474a80c374 100644 --- a/src/test/util/txmempool.h +++ b/src/test/util/txmempool.h @@ -54,11 +54,6 @@ std::optional CheckPackageMempoolAcceptResult(const Package& txns, */ void CheckMempoolEphemeralInvariants(const CTxMemPool& tx_pool); -/** Return indexes of the transaction's outputs that are considered dust - * at given dust_relay_rate. -*/ -std::vector GetDustIndexes(const CTransactionRef& tx_ref, CFeeRate dust_relay_rate); - /** For every transaction in tx_pool, check TRUC invariants: * - a TRUC tx's ancestor count must be within TRUC_ANCESTOR_LIMIT * - a TRUC tx's descendant count must be within TRUC_DESCENDANT_LIMIT From dd9044b8d4624fb7ffd432b6b89ab99290957a3e Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:20:30 -0500 Subject: [PATCH 06/29] ephemeral policy: IWYU --- src/policy/ephemeral_policy.cpp | 15 +++++++++++++++ src/policy/ephemeral_policy.h | 9 +++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index c1e184c81f5b3..2d13a7e007b68 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -2,8 +2,23 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include +#include +#include #include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state) { diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index ac889e8234ea0..90ac21b2d496c 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -5,10 +5,15 @@ #ifndef BITCOIN_POLICY_EPHEMERAL_POLICY_H #define BITCOIN_POLICY_EPHEMERAL_POLICY_H +#include #include -#include #include -#include + +#include + +class CFeeRate; +class CTxMemPool; +class TxValidationState; /** These utility functions ensure that ephemeral dust is safely * created and spent without unduly risking them entering the utxo From c5c10fd317c6b4c033f3001757e6975b8b9a4942 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:22:51 -0500 Subject: [PATCH 07/29] ephemeral policy doxygen cleanup --- src/policy/ephemeral_policy.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index 90ac21b2d496c..015c0770c12cf 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -43,14 +43,14 @@ class TxValidationState; /** Must be called for each transaction once transaction fees are known. * Does context-less checks about a single transaction. - * Returns false if the fee is non-zero and dust exists, populating state. True otherwise. + * @returns false if the fee is non-zero and dust exists, populating state. True otherwise. */ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmount base_fee, CAmount mod_fee, TxValidationState& state); /** 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. - * The function returns std::nullopt if all dust is properly spent, or the txid of the violating child spend. + * @returns std::nullopt if all dust is properly spent, or the txid of the violating child spend. */ std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool); From ef94d84b4e469d8dbd63e63598d3b8d53595c695 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:24:22 -0500 Subject: [PATCH 08/29] bench: remove unnecessary CMTxn constructors --- src/bench/mempool_ephemeral_spends.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bench/mempool_ephemeral_spends.cpp b/src/bench/mempool_ephemeral_spends.cpp index e867c61752c1e..78f869b9cde42 100644 --- a/src/bench/mempool_ephemeral_spends.cpp +++ b/src/bench/mempool_ephemeral_spends.cpp @@ -43,7 +43,7 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench) } // Tx with many outputs - CMutableTransaction tx1 = CMutableTransaction(); + CMutableTransaction tx1; tx1.vin.resize(1); tx1.vout.resize(number_outputs); for (size_t i = 0; i < tx1.vout.size(); i++) { @@ -55,7 +55,7 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench) const auto& parent_txid = tx1.GetHash(); // Spends all outputs of tx1, other details don't matter - CMutableTransaction tx2 = CMutableTransaction(); + CMutableTransaction tx2; tx2.vin.resize(tx1.vout.size()); for (size_t i = 0; i < tx2.vin.size(); i++) { tx2.vin[0].prevout.hash = parent_txid; From 5fbcfd12b8f508c87740883435800b6260fa308b Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:30:49 -0500 Subject: [PATCH 09/29] unit test: assert txid returned on CheckEphemeralSpends failures Simplify nullopt checks --- src/test/txvalidation_tests.cpp | 47 ++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 808e085917c88..732f2fd86d22e 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -128,85 +128,88 @@ 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}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({dust_spend}, minrelay, pool)); // Now with dust, ok because the tx has no dusty parents - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, minrelay, pool)); // Dust checks pass - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool).has_value()); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, minrelay, pool)); auto dust_non_spend = make_tx({COutPoint{dust_txid, dust_index - 1}}, /*version=*/2); // Child spending non-dust only from parent should be disallowed even if dust otherwise spent - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, minrelay, pool).has_value()); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, minrelay, pool).has_value()); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, minrelay, pool).has_value()); + const auto dust_non_spend_txid{dust_non_spend->GetHash()}; + const Txid null_txid; + assert(dust_non_spend_txid != null_txid); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, minrelay, pool).value_or(null_txid), dust_non_spend_txid); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, minrelay, pool).value_or(null_txid), dust_non_spend_txid); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, minrelay, pool).value_or(null_txid), dust_non_spend_txid); 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}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, minrelay, pool)); auto dust_non_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, 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}, minrelay, pool).has_value()); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, minrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); auto dust_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, minrelay, pool)); // 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}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, minrelay, pool)); // 2 grandparents with dust <- 1 dust-spending parent with dust <- child with no dust auto parent_with_dust = make_ephemeral_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2); // Ok for parent to have dust - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, minrelay, pool)); auto child_no_dust = make_tx({COutPoint{parent_with_dust->GetHash(), dust_index}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, minrelay, pool)); // 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(), dust_index}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, minrelay, pool)); // Tests with parents in mempool // Nothing in mempool, this should pass for any transaction - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, minrelay, pool)); // Add first grandparent to mempool and fetch entry pool.addUnchecked(entry.FromTx(grandparent_tx_1)); // Ignores ancestors that aren't direct parents - BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, minrelay, pool)); // Valid spend of dust with grandparent in mempool - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, minrelay, pool)); // Second grandparent in same package - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, minrelay, pool)); // Order in package doesn't matter - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, minrelay, pool)); // Add second grandparent to mempool pool.addUnchecked(entry.FromTx(grandparent_tx_2)); // Only spends single dust out of two direct parents - BOOST_CHECK(CheckEphemeralSpends({dust_non_spend_both_parents}, minrelay, pool).has_value()); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({dust_non_spend_both_parents}, minrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); // Spends both parents' dust - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, minrelay, pool)); // Now add dusty parent to mempool pool.addUnchecked(entry.FromTx(parent_with_dust)); // Passes dust checks even with non-parent ancestors - BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, minrelay, pool).has_value()); + BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, minrelay, pool)); } BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) From 15b6cbf07f5c3db650a0a8cccf46d3fbe031aef0 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:39:19 -0500 Subject: [PATCH 10/29] unit test: make dust index less magical --- src/test/txvalidation_tests.cpp | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 732f2fd86d22e..5937b6a639564 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -90,19 +90,22 @@ static inline CTransactionRef make_tx(const std::vector& inputs, int3 return MakeTransactionRef(mtx); } +static constexpr auto NUM_EPHEMERAL_TX_OUTPUTS = 3; +static constexpr auto EPHEMERAL_DUST_INDEX = NUM_EPHEMERAL_TX_OUTPUTS - 1; + // Same as make_tx but adds 2 normal outputs and 0-value dust to end of vout static inline CTransactionRef make_ephemeral_tx(const std::vector& inputs, int32_t version) { CMutableTransaction mtx = CMutableTransaction{}; mtx.version = version; mtx.vin.resize(inputs.size()); - mtx.vout.resize(3); for (size_t i{0}; i < inputs.size(); ++i) { mtx.vin[i].prevout = inputs[i]; } - for (auto i{0}; i < 3; ++i) { + mtx.vout.resize(NUM_EPHEMERAL_TX_OUTPUTS); + for (auto i{0}; i < NUM_EPHEMERAL_TX_OUTPUTS; ++i) { mtx.vout[i].scriptPubKey = CScript() << OP_TRUE; - mtx.vout[i].nValue = (i == 2) ? 0 : 10000; + mtx.vout[i].nValue = (i == EPHEMERAL_DUST_INDEX) ? 0 : 10000; } return MakeTransactionRef(mtx); } @@ -120,10 +123,8 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) auto grandparent_tx_1 = make_ephemeral_tx(random_outpoints(1), /*version=*/2); const auto dust_txid = grandparent_tx_1->GetHash(); - uint32_t dust_index = 2; - // Child transaction spending dust - auto dust_spend = make_tx({COutPoint{dust_txid, dust_index}}, /*version=*/2); + auto dust_spend = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}}, /*version=*/2); // We first start with nothing "in the mempool", using package checks @@ -137,7 +138,7 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool)); BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, minrelay, pool)); - auto dust_non_spend = make_tx({COutPoint{dust_txid, dust_index - 1}}, /*version=*/2); + 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()}; @@ -153,11 +154,11 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) // 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}, minrelay, pool)); - auto dust_non_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index - 1}}, /*version=*/2); + 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_EQUAL(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, minrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); - auto dust_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2); + 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}, minrelay, pool)); // Spending other outputs is also correct, as long as the dusty one is spent @@ -167,14 +168,14 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, minrelay, pool)); // 2 grandparents with dust <- 1 dust-spending parent with dust <- child with no dust - auto parent_with_dust = make_ephemeral_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2); + 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}, minrelay, pool)); - auto child_no_dust = make_tx({COutPoint{parent_with_dust->GetHash(), dust_index}}, /*version=*/2); + 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}, minrelay, pool)); // 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(), dust_index}}, /*version=*/2); + 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}, minrelay, pool)); // Tests with parents in mempool From bc0d98ea6126ea95526c2b70721131764c6ff3a7 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:40:43 -0500 Subject: [PATCH 11/29] fuzz: remove dangling reference to GetEntry --- src/test/fuzz/package_eval.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index 56ec52f852edf..fe20452f01c42 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -308,7 +308,7 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) PickValue(fuzzed_data_provider, mempool_outpoints).hash; const auto delta = fuzzed_data_provider.ConsumeIntegralInRange(-50 * COIN, +50 * COIN); // We only prioritise out of mempool transactions since PrioritiseTransaction doesn't - // filter for ephemeral dust GetEntry + // filter for ephemeral dust if (tx_pool.exists(GenTxid::Txid(txid))) { const auto tx_info{tx_pool.info(GenTxid::Txid(txid))}; if (GetDust(*tx_info.tx, tx_pool.m_opts.dust_relay_feerate).empty()) { From c041ad6eccb5aae87648cf510257a06f711b1bc3 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:42:09 -0500 Subject: [PATCH 12/29] fuzz: explain package eval coin tracking better --- src/test/fuzz/package_eval.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index fe20452f01c42..f3c5b0ab44646 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -248,7 +248,8 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) CAmount amount_in{0}; for (int i = 0; i < num_in; ++i) { - // Pop random outpoint + // Pop random outpoint. We erase them to avoid double-spending + // while in this loop, but later add them back (unless last_tx). auto pop = outpoints.begin(); std::advance(pop, fuzzed_data_provider.ConsumeIntegralInRange(0, outpoints.size() - 1)); auto outpoint = *pop; @@ -405,7 +406,8 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool) CAmount amount_in{0}; for (size_t i = 0; i < num_in; ++i) { - // Pop random outpoint + // Pop random outpoint. We erase them to avoid double-spending + // while in this loop, but later add them back (unless last_tx). auto pop = outpoints.begin(); std::advance(pop, fuzzed_data_provider.ConsumeIntegralInRange(0, outpoints.size() - 1)); const auto outpoint = *pop; From bedca1cb6633f4b9a5f8f532f27e084f23f04a2e Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:47:56 -0500 Subject: [PATCH 13/29] fuzz: Directly place transactions in vector --- src/test/fuzz/package_eval.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index f3c5b0ab44646..88eea3bf909fd 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -232,7 +232,7 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) bool last_tx = num_txs > 1 && txs.size() == num_txs - 1; // Create transaction to add to the mempool - const CTransactionRef tx = [&] { + txs.emplace_back([&] { CMutableTransaction tx_mut; tx_mut.version = CTransaction::CURRENT_VERSION; tx_mut.nLockTime = 0; @@ -299,8 +299,7 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) outpoints_value[COutPoint(tx->GetHash(), i)] = tx->vout[i].nValue; } return tx; - }(); - txs.push_back(tx); + }()); } if (fuzzed_data_provider.ConsumeBool()) { @@ -392,7 +391,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool) bool last_tx = num_txs > 1 && txs.size() == num_txs - 1; // Create transaction to add to the mempool - const CTransactionRef tx = [&] { + txs.emplace_back([&] { CMutableTransaction tx_mut; tx_mut.version = fuzzed_data_provider.ConsumeBool() ? TRUC_VERSION : CTransaction::CURRENT_VERSION; tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral(); @@ -470,8 +469,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool) outpoints_value[COutPoint(tx->GetHash(), i)] = tx->vout[i].nValue; } return tx; - }(); - txs.push_back(tx); + }()); } if (fuzzed_data_provider.ConsumeBool()) { From 768a0c1889e57ae8bb3596ac7aa9fd2b1ecab9fa Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:57:39 -0500 Subject: [PATCH 14/29] func: cleanup test_dustrelay comments --- test/functional/mempool_dust.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/mempool_dust.py b/test/functional/mempool_dust.py index bc9a12fb603e4..937e77fbd41ba 100755 --- a/test/functional/mempool_dust.py +++ b/test/functional/mempool_dust.py @@ -78,8 +78,8 @@ def test_dustrelay(self): assert_equal(self.nodes[0].getrawmempool(), []) - # Double dust, both unspent, with fees. Would have failed individual checks. - # Dust is 1 satoshi create_self_transfer_multi disallows 0 + # Create two dust outputs. Transaction has zero fees. both dust outputs are unspent, and would have failed individual checks. + # The amount is 1 satoshi because create_self_transfer_multi disallows 0. dusty_tx = self.wallet.create_self_transfer_multi(fee_per_output=1000, amount_per_output=1, num_outputs=2) dust_txid = self.nodes[0].sendrawtransaction(hexstring=dusty_tx["hex"], maxfeerate=0) From 09ce926e4a14f183cfab387d2531519e000ea176 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 12:59:34 -0500 Subject: [PATCH 15/29] func: cleanup reorg test comment --- test/functional/mempool_ephemeral_dust.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/mempool_ephemeral_dust.py b/test/functional/mempool_ephemeral_dust.py index 75c98ce0f89c7..5d769269a149c 100755 --- a/test/functional/mempool_ephemeral_dust.py +++ b/test/functional/mempool_ephemeral_dust.py @@ -363,7 +363,7 @@ def test_reorgs(self): self.nodes[0].invalidateblock(block_res["hash"]) assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"]], sync=False) - # Also should happen if dust is swept + # Should re-enter if dust is swept sweep_tx_2 = self.wallet.create_self_transfer_multi(fee_per_output=0, utxos_to_spend=dusty_tx["new_utxos"], version=3) self.add_output_to_create_multi_result(sweep_tx_2) assert_raises_rpc_error(-26, "min relay fee not met", self.nodes[0].sendrawtransaction, sweep_tx_2["hex"]) From 4dfdf615b9dbdc2204347029bea1db974a88e392 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:02:24 -0500 Subject: [PATCH 16/29] fuzz: remove unused TransactionsDelta validation interface --- src/test/fuzz/package_eval.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index 88eea3bf909fd..a805b43e33e6c 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -317,11 +317,6 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) } } - // Remember all added transactions - std::set added; - auto txr = std::make_shared(added); - node.validation_signals->RegisterSharedValidationInterface(txr); - auto single_submit = txs.size() == 1; const auto result_package = WITH_LOCK(::cs_main, @@ -339,7 +334,6 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) } node.validation_signals->SyncWithValidationInterfaceQueue(); - node.validation_signals->UnregisterSharedValidationInterface(txr); CheckMempoolEphemeralInvariants(tx_pool); } From 445eaed182a714e65ee2fe679ecdf7a86055313b Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:06:50 -0500 Subject: [PATCH 17/29] fuzz: use optional status instead of should_rbf_eph_spend --- src/test/fuzz/package_eval.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index a805b43e33e6c..69a7e44da2636 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -217,11 +217,10 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) std::vector txs; // Find something we may want to double-spend with two input single tx - std::optional outpoint_to_rbf{GetChildEvictingPrevout(tx_pool)}; - bool should_rbf_eph_spend = outpoint_to_rbf && fuzzed_data_provider.ConsumeBool(); + std::optional outpoint_to_rbf{fuzzed_data_provider.ConsumeBool() ? GetChildEvictingPrevout(tx_pool) : std::nullopt}; // Make small packages - const auto num_txs = should_rbf_eph_spend ? 1 : (size_t) fuzzed_data_provider.ConsumeIntegralInRange(1, 4); + const auto num_txs = outpoint_to_rbf ? 1 : (size_t) fuzzed_data_provider.ConsumeIntegralInRange(1, 4); std::set package_outpoints; while (txs.size() < num_txs) { @@ -237,10 +236,10 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) tx_mut.version = CTransaction::CURRENT_VERSION; tx_mut.nLockTime = 0; // Last tx will sweep half or more of all outpoints from package - const auto num_in = should_rbf_eph_spend ? 2 : + const auto num_in = outpoint_to_rbf ? 2 : last_tx ? fuzzed_data_provider.ConsumeIntegralInRange(package_outpoints.size()/2 + 1, package_outpoints.size()) : fuzzed_data_provider.ConsumeIntegralInRange(1, 4); - auto num_out = should_rbf_eph_spend ? 1 : fuzzed_data_provider.ConsumeIntegralInRange(1, 4); + const auto num_out = outpoint_to_rbf ? 1 : fuzzed_data_provider.ConsumeIntegralInRange(1, 4); auto& outpoints = last_tx ? package_outpoints : mempool_outpoints; @@ -254,7 +253,7 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) std::advance(pop, fuzzed_data_provider.ConsumeIntegralInRange(0, outpoints.size() - 1)); auto outpoint = *pop; - if (i == 0 && should_rbf_eph_spend) { + if (i == 0 && outpoint_to_rbf) { outpoint = *outpoint_to_rbf; outpoints.erase(outpoint); } else { @@ -278,7 +277,7 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) } // Note output amounts can naturally drop to dust on their own. - if (!should_rbf_eph_spend && fuzzed_data_provider.ConsumeBool()) { + if (!outpoint_to_rbf && fuzzed_data_provider.ConsumeBool()) { uint32_t dust_index = fuzzed_data_provider.ConsumeIntegralInRange(0, num_out); tx_mut.vout.insert(tx_mut.vout.begin() + dust_index, CTxOut(0, P2WSH_EMPTY)); } From 7c3490169c9e20375d3f525f81798fcced01a30a Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:09:40 -0500 Subject: [PATCH 18/29] fuzz: package_eval: move last_tx inside txn ctor --- src/test/fuzz/package_eval.cpp | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index 69a7e44da2636..0174852f56350 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -224,18 +224,15 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) std::set package_outpoints; while (txs.size() < num_txs) { - - // Last transaction in a package needs to be a child of parents to get further in validation - // so the last transaction to be generated(in a >1 package) must spend all package-made outputs - // Note that this test currently only spends package outputs in last transaction. - bool last_tx = num_txs > 1 && txs.size() == num_txs - 1; - // Create transaction to add to the mempool txs.emplace_back([&] { CMutableTransaction tx_mut; tx_mut.version = CTransaction::CURRENT_VERSION; tx_mut.nLockTime = 0; - // Last tx will sweep half or more of all outpoints from package + // Last transaction in a package needs to be a child of parents to get further in validation + // so the last transaction to be generated(in a >1 package) must spend all package-made outputs + // Note that this test currently only spends package outputs in last transaction. + bool last_tx = num_txs > 1 && txs.size() == num_txs - 1; const auto num_in = outpoint_to_rbf ? 2 : last_tx ? fuzzed_data_provider.ConsumeIntegralInRange(package_outpoints.size()/2 + 1, package_outpoints.size()) : fuzzed_data_provider.ConsumeIntegralInRange(1, 4); @@ -377,18 +374,15 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool) const auto num_txs = (size_t) fuzzed_data_provider.ConsumeIntegralInRange(1, 26); std::set package_outpoints; while (txs.size() < num_txs) { - - // Last transaction in a package needs to be a child of parents to get further in validation - // so the last transaction to be generated(in a >1 package) must spend all package-made outputs - // Note that this test currently only spends package outputs in last transaction. - bool last_tx = num_txs > 1 && txs.size() == num_txs - 1; - // Create transaction to add to the mempool txs.emplace_back([&] { CMutableTransaction tx_mut; tx_mut.version = fuzzed_data_provider.ConsumeBool() ? TRUC_VERSION : CTransaction::CURRENT_VERSION; tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral(); - // Last tx will sweep all outpoints in package + // Last transaction in a package needs to be a child of parents to get further in validation + // so the last transaction to be generated(in a >1 package) must spend all package-made outputs + // Note that this test currently only spends package outputs in last transaction. + bool last_tx = num_txs > 1 && txs.size() == num_txs - 1; const auto num_in = last_tx ? package_outpoints.size() : fuzzed_data_provider.ConsumeIntegralInRange(1, mempool_outpoints.size()); auto num_out = fuzzed_data_provider.ConsumeIntegralInRange(1, mempool_outpoints.size() * 2); From ca050d12e76f61af7e60fa564dd04db08f2b8f38 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:15:09 -0500 Subject: [PATCH 19/29] unit test: adapt to changing MAX_DUST_OUTPUTS_PER_TX --- src/test/transaction_tests.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 3e4c085c0edba..f38c015f6d7ab 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -814,9 +814,11 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) CAmount nDustThreshold = 182 * g_dust.GetFeePerK() / 1000; BOOST_CHECK_EQUAL(nDustThreshold, 546); - // Add dust output to take dust slot, still standard! - t.vout.emplace_back(0, t.vout[0].scriptPubKey); - CheckIsStandard(t); + // Add dust outputs up to allowed maximum, still standard! + for (size_t i{0}; i < MAX_DUST_OUTPUTS_PER_TX; ++i) { + t.vout.emplace_back(0, t.vout[0].scriptPubKey); + CheckIsStandard(t); + } // dust: t.vout[0].nValue = nDustThreshold - 1; @@ -974,9 +976,9 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) CheckIsNotStandard(t, "bare-multisig"); g_bare_multi = DEFAULT_PERMIT_BAREMULTISIG; - // Add dust output to take dust slot + // Add dust outputs up to allowed maximum assert(t.vout.size() == 1); - t.vout.emplace_back(0, t.vout[0].scriptPubKey); + t.vout.insert(t.vout.end(), MAX_DUST_OUTPUTS_PER_TX, {0, t.vout[0].scriptPubKey}); // Check compressed P2PK outputs dust threshold (must have leading 02 or 03) t.vout[0].scriptPubKey = CScript() << std::vector(33, 0x02) << OP_CHECKSIG; From 08e969bd1076c99e0b43ecd01dd790b9ebd04d0a Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:19:40 -0500 Subject: [PATCH 20/29] RPC: only enforce dust rules on priority when standardness active --- src/rpc/mining.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index ca88fad61ef1b..3019789d09b0e 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -496,7 +496,7 @@ static RPCHelpMan prioritisetransaction() // Non-0 fee dust transactions are not allowed for entry, and modification not allowed afterwards const auto& tx = mempool.get(hash); - if (tx && !GetDust(*tx, mempool.m_opts.dust_relay_feerate).empty()) { + if (mempool.m_opts.require_standard && tx && !GetDust(*tx, mempool.m_opts.dust_relay_feerate).empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is not supported for transactions with dust outputs."); } From e5709a4a41ecd8c7b1e695871c1a6153864e76ae Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:22:09 -0500 Subject: [PATCH 21/29] func: slight elaboration on submitpackage restriction --- test/functional/mempool_ephemeral_dust.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/mempool_ephemeral_dust.py b/test/functional/mempool_ephemeral_dust.py index 5d769269a149c..813ca65512be0 100755 --- a/test/functional/mempool_ephemeral_dust.py +++ b/test/functional/mempool_ephemeral_dust.py @@ -470,7 +470,7 @@ def test_free_relay(self): self.wallet.rescan_utxos() assert_equal(self.nodes[0].getrawmempool(), []) - # Other topology tests require relaxation of submitpackage topology + # Other topology tests (e.g., grandparents and parents both with dust) require relaxation of submitpackage topology self.restart_node(0, extra_args=[]) self.restart_node(1, extra_args=[]) From 87b26e3dc07b283cb05064ccde179c6777397ce8 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:22:59 -0500 Subject: [PATCH 22/29] func: rename test_free_relay to test_no_minrelay_fee --- test/functional/mempool_ephemeral_dust.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/mempool_ephemeral_dust.py b/test/functional/mempool_ephemeral_dust.py index 813ca65512be0..58c84b37305ac 100755 --- a/test/functional/mempool_ephemeral_dust.py +++ b/test/functional/mempool_ephemeral_dust.py @@ -61,7 +61,7 @@ def run_test(self): self.test_non_truc() self.test_unspent_ephemeral() self.test_reorgs() - self.test_free_relay() + self.test_no_minrelay_fee() def test_normal_dust(self): self.log.info("Create 0-value dusty output, show that it works inside truc when spent in package") @@ -401,7 +401,7 @@ def test_reorgs(self): self.sync_all() # N.B. this extra_args can be removed post cluster mempool - def test_free_relay(self): + def test_no_minrelay_fee(self): self.log.info("Test that ephemeral dust works in non-TRUC contexts when there's no minrelay requirement") # Note: since minrelay is 0, it is not testing 1P1C relay From d9cfa5fc4eb03fb425fd5d46d3b72db72fbc3243 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:26:34 -0500 Subject: [PATCH 23/29] CheckEphemeralSpends: no need to iterate inputs if no parent dust --- src/policy/ephemeral_policy.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index 2d13a7e007b68..84863edd594e5 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -72,6 +72,10 @@ std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_r processed_parent_set.insert(parent_txid); } + if (unspent_parent_dust.empty()) { + continue; + } + // Now that we have gathered parents' dust, make sure it's spent // by the child for (const auto& tx_input : tx->vin) { From 84242903043bb14fca917790c9381c411817c9f7 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:30:08 -0500 Subject: [PATCH 24/29] unit test: ephemeral_tests is using a dust relay rate, not minrelay --- src/test/txvalidation_tests.cpp | 45 +++++++++++++++++---------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 5937b6a639564..63876616d3df3 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -117,7 +117,8 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) TestMemPoolEntryHelper entry; CTxMemPool::setEntries empty_ancestors; - CFeeRate minrelay(1000); + // Arbitrary non-0 feerate for these tests + CFeeRate dustrelay(DUST_RELAY_TX_FEE); // Basic transaction with dust auto grandparent_tx_1 = make_ephemeral_tx(random_outpoints(1), /*version=*/2); @@ -129,14 +130,14 @@ 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}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({dust_spend}, dustrelay, pool)); // Now with dust, ok because the tx has no dusty parents - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool)); // Dust checks pass BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool)); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, dustrelay, pool)); auto dust_non_spend = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX - 1}}, /*version=*/2); @@ -144,73 +145,73 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) const auto dust_non_spend_txid{dust_non_spend->GetHash()}; const Txid null_txid; assert(dust_non_spend_txid != null_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, minrelay, pool).value_or(null_txid), dust_non_spend_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, minrelay, pool).value_or(null_txid), dust_non_spend_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, minrelay, pool).value_or(null_txid), dust_non_spend_txid); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); 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}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool)); 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_EQUAL(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, minrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); 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}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool)); // 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}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool)); // 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}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool)); 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}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool)); // 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}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool)); // Tests with parents in mempool // Nothing in mempool, this should pass for any transaction - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool)); // Add first grandparent to mempool and fetch entry pool.addUnchecked(entry.FromTx(grandparent_tx_1)); // Ignores ancestors that aren't direct parents - BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, dustrelay, pool)); // Valid spend of dust with grandparent in mempool - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool)); // Second grandparent in same package - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool)); // Order in package doesn't matter - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool)); // Add second grandparent to mempool pool.addUnchecked(entry.FromTx(grandparent_tx_2)); // Only spends single dust out of two direct parents - BOOST_CHECK_EQUAL(CheckEphemeralSpends({dust_non_spend_both_parents}, minrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); + BOOST_CHECK_EQUAL(CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); // Spends both parents' dust - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool)); // Now add dusty parent to mempool pool.addUnchecked(entry.FromTx(parent_with_dust)); // Passes dust checks even with non-parent ancestors - BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, minrelay, pool)); + BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, dustrelay, pool)); } BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) From cf0cee1617c0bf065b295a9807a4c7de0558393d Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 12 Nov 2024 13:32:11 -0500 Subject: [PATCH 25/29] func: add note about lack of 1P1C propagation in tree submitpackage --- test/functional/mempool_ephemeral_dust.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/mempool_ephemeral_dust.py b/test/functional/mempool_ephemeral_dust.py index 58c84b37305ac..a7a799f544abd 100755 --- a/test/functional/mempool_ephemeral_dust.py +++ b/test/functional/mempool_ephemeral_dust.py @@ -462,6 +462,8 @@ def test_no_minrelay_fee(self): # Sweeps all dust, where all dusty txs are already in-mempool sweep_tx = self.wallet.create_self_transfer_multi(fee_per_output=25000, utxos_to_spend=all_parent_utxos, version=2) + # N.B. Since we have multiple parents these are not propagating via 1P1C relay. + # minrelay being zero allows them to propagate on their own. res = self.nodes[0].submitpackage([dusty_tx["hex"] for dusty_tx in dusty_txs] + [sweep_tx["hex"]]) assert_equal(res['package_msg'], "success") assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"] for dusty_tx in dusty_txs] + [sweep_tx["tx"], cancel_sweep["tx"]]) From ba35a570c5d4ade342cb32630ffaa5f5bdd5e826 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Wed, 13 Nov 2024 11:19:14 -0500 Subject: [PATCH 26/29] CheckEphemeralSpends: return boolean, and set child state and txid outparams --- src/bench/mempool_ephemeral_spends.cpp | 5 +- src/policy/ephemeral_policy.cpp | 11 ++- src/policy/ephemeral_policy.h | 5 +- src/test/txvalidation_tests.cpp | 106 +++++++++++++++++++------ src/validation.cpp | 15 ++-- 5 files changed, 101 insertions(+), 41 deletions(-) diff --git a/src/bench/mempool_ephemeral_spends.cpp b/src/bench/mempool_ephemeral_spends.cpp index 78f869b9cde42..600eb6d7cb1df 100644 --- a/src/bench/mempool_ephemeral_spends.cpp +++ b/src/bench/mempool_ephemeral_spends.cpp @@ -73,9 +73,12 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench) uint32_t iteration{0}; + TxValidationState dummy_state; + Txid dummy_txid; + bench.run([&]() NO_THREAD_SAFETY_ANALYSIS { - CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool); + CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool, dummy_state, dummy_txid); iteration++; }); } diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index 84863edd594e5..b7d254dd63045 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -30,11 +30,11 @@ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmou return true; } -std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool) +bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Txid& out_child_txid) { 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 - return std::nullopt; + return true; } std::map map_txid_ref; @@ -83,9 +83,12 @@ std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_r } if (!unspent_parent_dust.empty()) { - return tx->GetHash(); + out_child_txid = tx->GetHash(); + 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())); + return false; } } - return std::nullopt; + return true; } diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index 015c0770c12cf..f9c6e4363e733 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -50,8 +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. - * @returns std::nullopt if all dust is properly spent, or the txid of the violating child spend. + * Sets out_child_state and out_child_txid on failure. + * @returns true if all dust is properly spent. */ -std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool); +bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Txid& out_child_txid); #endif // BITCOIN_POLICY_EPHEMERAL_POLICY_H diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 63876616d3df3..1e036aa508731 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -117,6 +117,9 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) TestMemPoolEntryHelper entry; CTxMemPool::setEntries empty_ancestors; + TxValidationState child_state; + Txid child_txid; + // Arbitrary non-0 feerate for these tests CFeeRate dustrelay(DUST_RELAY_TX_FEE); @@ -130,88 +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)); + BOOST_CHECK(CheckEphemeralSpends({dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Now with dust, ok because the tx has no dusty parents - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Dust checks pass - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool)); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool, child_state, child_txid)); + 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(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); 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()}; - const Txid null_txid; - assert(dust_non_spend_txid != null_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + child_state = TxValidationState(); + child_txid = Txid(); + + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + child_state = TxValidationState(); + child_txid = Txid(); + + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + child_state = TxValidationState(); + child_txid = Txid(); 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)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); 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_EQUAL(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash()); + child_state = TxValidationState(); + child_txid = Txid(); 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)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // 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)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // 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)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); 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)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // 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)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Tests with parents in mempool // Nothing in mempool, this should pass for any transaction - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Add first grandparent to mempool and fetch entry pool.addUnchecked(entry.FromTx(grandparent_tx_1)); // Ignores ancestors that aren't direct parents - BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Valid spend of dust with grandparent in mempool - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Second grandparent in same package - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); + // Order in package doesn't matter - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Add second grandparent to mempool pool.addUnchecked(entry.FromTx(grandparent_tx_2)); // Only spends single dust out of two direct parents - BOOST_CHECK_EQUAL(CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); + BOOST_CHECK(!CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash()); + child_state = TxValidationState(); + child_txid = Txid(); // Spends both parents' dust - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Now add dusty parent to mempool pool.addUnchecked(entry.FromTx(parent_with_dust)); // Passes dust checks even with non-parent ancestors - BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); } BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) diff --git a/src/validation.cpp b/src/validation.cpp index 91a9d02611ed8..7ab857e58a53a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1441,11 +1441,8 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef } if (m_pool.m_opts.require_standard) { - if (const auto ephemeral_violation{CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool)}) { - const Txid& txid = ephemeral_violation.value(); - Assume(txid == ptx->GetHash()); - ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends", - strprintf("tx %s did not spend parent's ephemeral dust", txid.ToString())); + Txid dummy_txid; + if (!CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool, ws.m_state, dummy_txid)) { return MempoolAcceptResult::Failure(ws.m_state); } } @@ -1590,11 +1587,9 @@ 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) { - if (const auto ephemeral_violation{CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate, m_pool)}) { - const Txid& child_txid = ephemeral_violation.value(); - TxValidationState child_state; - child_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends", - strprintf("tx %s did not spend parent's ephemeral dust", child_txid.ToString())); + TxValidationState child_state; + Txid child_txid; + if (!CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate, m_pool, child_state, child_txid)) { package_state.Invalid(PackageValidationResult::PCKG_TX, "unspent-dust"); results.emplace(child_txid, MempoolAcceptResult::Failure(child_state)); return PackageMempoolAcceptResult(package_state, std::move(results)); From d033acb608391f3ba95864cdaa7025cc00888ea2 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Wed, 13 Nov 2024 11:36:08 -0500 Subject: [PATCH 27/29] fuzz: package_eval: let fuzzer run out input in main tx creation loop --- src/test/fuzz/package_eval.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index 0174852f56350..0373d2493fa5f 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -210,7 +210,7 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) chainstate.SetMempool(&tx_pool); - LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) + LIMITED_WHILE(fuzzed_data_provider.remaining_bytes() > 0, 300) { Assert(!mempool_outpoints.empty()); @@ -364,7 +364,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool) chainstate.SetMempool(&tx_pool); - LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) + LIMITED_WHILE(fuzzed_data_provider.remaining_bytes() > 0, 300) { Assert(!mempool_outpoints.empty()); From ea5db2f26920bce7caf85e5c1b70a527cc3b82c2 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 19 Nov 2024 09:55:35 -0500 Subject: [PATCH 28/29] functional: only generate required blocks for test --- test/functional/mempool_ephemeral_dust.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/mempool_ephemeral_dust.py b/test/functional/mempool_ephemeral_dust.py index a7a799f544abd..3e0cbabeec021 100755 --- a/test/functional/mempool_ephemeral_dust.py +++ b/test/functional/mempool_ephemeral_dust.py @@ -468,7 +468,7 @@ def test_no_minrelay_fee(self): assert_equal(res['package_msg'], "success") assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"] for dusty_tx in dusty_txs] + [sweep_tx["tx"], cancel_sweep["tx"]]) - self.generate(self.nodes[0], 25) + self.generate(self.nodes[0], 1) self.wallet.rescan_utxos() assert_equal(self.nodes[0].getrawmempool(), []) From 466e4df3fb83ef82b6add22e202e3a70dbf83a12 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Wed, 20 Nov 2024 12:57:29 -0500 Subject: [PATCH 29/29] assert_mempool_contents: assert not duplicates expected --- test/functional/test_framework/mempool_util.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index e04ae914ccfd9..0e9c821e2ead9 100644 --- a/test/functional/test_framework/mempool_util.py +++ b/test/functional/test_framework/mempool_util.py @@ -30,6 +30,7 @@ def assert_mempool_contents(test_framework, node, expected=None, sync=True): test_framework.sync_mempools() if not expected: expected = [] + assert_equal(len(expected), len(set(expected))) mempool = node.getrawmempool(verbose=False) assert_equal(len(mempool), len(expected)) for tx in expected: