diff --git a/src/bench/mempool_ephemeral_spends.cpp b/src/bench/mempool_ephemeral_spends.cpp index f34664a736b..a8f2efecb93 100644 --- a/src/bench/mempool_ephemeral_spends.cpp +++ b/src/bench/mempool_ephemeral_spends.cpp @@ -44,7 +44,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++) { @@ -56,7 +56,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; @@ -74,9 +74,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 6854822e351..b7d254dd630 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -2,29 +2,39 @@ // 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 -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); }); -} +#include +#include +#include +#include +#include +#include +#include -bool CheckValidEphemeralTx(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)) { + 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"); } 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::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; + return true; } std::map map_txid_ref; @@ -33,7 +43,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; @@ -63,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) { @@ -70,9 +83,12 @@ std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_r } if (!unspent_parent_dust.empty()) { - return txid; + 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 26140f9a020..f9c6e4363e7 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -5,17 +5,22 @@ #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 * 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 * @@ -34,22 +39,20 @@ * are the only way to bring fees. */ -/** Returns true if transaction contains dust */ -bool HasDust(const CTransactionRef& 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. * 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 CheckValidEphemeralTx(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, * 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. + * 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/policy/policy.cpp b/src/policy/policy.cpp index 21c35af5ccb..ed336928235 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 0488f8dbee8..4412f2db87a 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 006ec3c78c7..3019789d09b 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 (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."); } diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index f9441704745..0373d2493fa 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()) { @@ -210,37 +210,33 @@ 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()); 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) { - - // 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 - const CTransactionRef tx = [&] { + 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 - const auto num_in = should_rbf_eph_spend ? 2 : + // 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); - 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; @@ -248,12 +244,13 @@ 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; - if (i == 0 && should_rbf_eph_spend) { + if (i == 0 && outpoint_to_rbf) { outpoint = *outpoint_to_rbf; outpoints.erase(outpoint); } else { @@ -277,7 +274,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)); } @@ -298,8 +295,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()) { @@ -308,20 +304,15 @@ 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 (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); } } } - // 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 +330,6 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool) } node.validation_signals->SyncWithValidationInterfaceQueue(); - node.validation_signals->UnregisterSharedValidationInterface(txr); CheckMempoolEphemeralInvariants(tx_pool); } @@ -374,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()); @@ -384,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 - 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(); - // 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); @@ -405,7 +392,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; @@ -468,8 +456,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()) { diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 3e4c085c0ed..f38c015f6d7 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; diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 28afac0248f..54d94b3ce4a 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); } @@ -114,99 +117,159 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) TestMemPoolEntryHelper entry; CTxMemPool::setEntries empty_ancestors; - CFeeRate minrelay(1000); + TxValidationState child_state; + Txid child_txid; + + // 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); 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 // Trivial single transaction with no dust - BOOST_CHECK(!CheckEphemeralSpends({dust_spend}, minrelay, pool).has_value()); + 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}, minrelay, pool).has_value()); + 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).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, 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, 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 - 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()}; + 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}, minrelay, pool).has_value()); + 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, 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(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, minrelay, pool).has_value()); + 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, 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()); + 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(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}, minrelay, pool).has_value()); + 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, 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).has_value()); - 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}, 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, 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(), dust_index}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, minrelay, pool).has_value()); + 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(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}, minrelay, pool).has_value()); + 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 AddToMempool(pool, 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}, 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}, minrelay, pool).has_value()); + 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}, minrelay, pool).has_value()); + 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}, minrelay, pool).has_value()); + 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 AddToMempool(pool, 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(!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}, minrelay, pool).has_value()); + 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 AddToMempool(pool, 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}, 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/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index ae0493467be..f1ca33bec78 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 3c2841db919..36caad2ae1a 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 diff --git a/src/validation.cpp b/src/validation.cpp index ed9758365ca..95f3bc58d7d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -913,8 +913,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 } } @@ -1436,11 +1436,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); } } @@ -1592,11 +1589,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)); diff --git a/test/functional/mempool_dust.py b/test/functional/mempool_dust.py index bc9a12fb603..937e77fbd41 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) diff --git a/test/functional/mempool_ephemeral_dust.py b/test/functional/mempool_ephemeral_dust.py index 75c98ce0f89..3e0cbabeec0 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") @@ -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"]) @@ -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 @@ -462,15 +462,17 @@ def test_free_relay(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"]]) - self.generate(self.nodes[0], 25) + self.generate(self.nodes[0], 1) 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=[]) diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index e04ae914ccf..0e9c821e2ea 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: