bitcoin · glozow · Nov 25, 2024 · Nov 12, 2024 · Nov 12, 2024 · Nov 12, 2024
diff --git 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;
@@ -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
@@ -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 <consensus/validation.h>
 #include <policy/ephemeral_policy.h>
+#include <policy/feerate.h>
+#include <policy/packages.h>
 #include <policy/policy.h>
+#include <primitives/transaction.h>
+#include <txmempool.h>
+#include <util/check.h>
+#include <util/hasher.h>
 
-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 <algorithm>
+#include <cstdint>
+#include <map>
+#include <memory>
+#include <unordered_set>
+#include <utility>
+#include <vector>
 
-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<Txid> 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<Txid, CTransactionRef> map_txid_ref;
@@ -33,7 +43,6 @@ std::optional<Txid> CheckEphemeralSpends(const Package& package, CFeeRate dust_r
     }
 
     for (const auto& tx : package) {
-        Txid txid = tx->GetHash();
         std::unordered_set<Txid, SaltedTxidHasher> processed_parent_set;
         std::unordered_set<COutPoint, SaltedOutpointHasher> unspent_parent_dust;
 
@@ -63,16 +72,23 @@ std::optional<Txid> 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) {
             unspent_parent_dust.erase(tx_input.prevout);
         }
 
         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
@@ -5,17 +5,22 @@
 #ifndef BITCOIN_POLICY_EPHEMERAL_POLICY_H
 #define BITCOIN_POLICY_EPHEMERAL_POLICY_H
 
+#include <consensus/amount.h>
 #include <policy/packages.h>
-#include <policy/policy.h>
 #include <primitives/transaction.h>
-#include <txmempool.h>
+
+#include <optional>
+
+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<Txid> 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
@@ -67,6 +67,15 @@ bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFeeIn)
     return (txout.nValue < GetDustThreshold(txout, dustRelayFeeIn));
 }
 
+std::vector<uint32_t> GetDust(const CTransaction& tx, CFeeRate dust_relay_rate)
+{
+    std::vector<uint32_t> 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<unsigned>& max_datacarrier_bytes, TxoutType& whichType)
 {
     std::vector<std::vector<unsigned char> > vSolutions;
@@ -129,7 +138,6 @@ bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& 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<unsigned>& 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
@@ -131,6 +131,8 @@ bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFee);
 
 bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_datacarrier_bytes, TxoutType& whichType);
 
+/** Get the vout index numbers of all dust outputs */
+std::vector<uint32_t> 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
@@ -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.");
     }