-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Ephemeral Dust #30239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ephemeral Dust #30239
Changes from all commits
04b2714
e1d3e81
4e68f90
e2e30e8
127719f
21d28b2
71a6ab4
3f6559f
5c2e291
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| P2P and network changes | ||
| ----------------------- | ||
|
|
||
| Ephemeral dust is a new concept that allows a single | ||
| dust output in a transaction, provided the transaction | ||
| is zero fee. In order to spend any unconfirmed outputs | ||
| from this transaction, the spender must also spend | ||
| this dust in addition to any other outputs. | ||
|
|
||
| In other words, this type of transaction | ||
| should be created in a transaction package where | ||
| the dust is both created and spent simultaneously. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| // Copyright (c) 2011-2022 The Bitcoin Core developers | ||
| // Distributed under the MIT software license, see the accompanying | ||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
||
| #include <bench/bench.h> | ||
| #include <consensus/amount.h> | ||
| #include <kernel/cs_main.h> | ||
| #include <policy/ephemeral_policy.h> | ||
| #include <policy/policy.h> | ||
| #include <primitives/transaction.h> | ||
| #include <script/script.h> | ||
| #include <sync.h> | ||
| #include <test/util/setup_common.h> | ||
| #include <txmempool.h> | ||
| #include <util/check.h> | ||
|
|
||
| #include <cstdint> | ||
| #include <memory> | ||
| #include <vector> | ||
|
|
||
|
|
||
| static void AddTx(const CTransactionRef& tx, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) | ||
| { | ||
| int64_t nTime{0}; | ||
| unsigned int nHeight{1}; | ||
| uint64_t sequence{0}; | ||
| bool spendsCoinbase{false}; | ||
| unsigned int sigOpCost{4}; | ||
| uint64_t fee{0}; | ||
| LockPoints lp; | ||
| pool.addUnchecked(CTxMemPoolEntry( | ||
| tx, fee, nTime, nHeight, sequence, | ||
| spendsCoinbase, sigOpCost, lp)); | ||
| } | ||
|
|
||
| static void MempoolCheckEphemeralSpends(benchmark::Bench& bench) | ||
| { | ||
| const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(); | ||
|
|
||
| int number_outputs{1000}; | ||
| if (bench.complexityN() > 1) { | ||
| number_outputs = static_cast<int>(bench.complexityN()); | ||
| } | ||
|
|
||
| // Tx with many outputs | ||
| CMutableTransaction tx1 = CMutableTransaction(); | ||
| tx1.vin.resize(1); | ||
| tx1.vout.resize(number_outputs); | ||
| for (size_t i = 0; i < tx1.vout.size(); i++) { | ||
| tx1.vout[i].scriptPubKey = CScript(); | ||
| // Each output progressively larger | ||
| tx1.vout[i].nValue = i * CENT; | ||
| } | ||
|
|
||
| const auto& parent_txid = tx1.GetHash(); | ||
|
|
||
| // Spends all outputs of tx1, other details don't matter | ||
| CMutableTransaction tx2 = CMutableTransaction(); | ||
| tx2.vin.resize(tx1.vout.size()); | ||
| for (size_t i = 0; i < tx2.vin.size(); i++) { | ||
| tx2.vin[0].prevout.hash = parent_txid; | ||
| tx2.vin[0].prevout.n = i; | ||
| } | ||
| tx2.vout.resize(1); | ||
|
|
||
| CTxMemPool& pool = *Assert(testing_setup->m_node.mempool); | ||
| LOCK2(cs_main, pool.cs); | ||
| // Create transaction references outside the "hot loop" | ||
| const CTransactionRef tx1_r{MakeTransactionRef(tx1)}; | ||
| const CTransactionRef tx2_r{MakeTransactionRef(tx2)}; | ||
|
|
||
| AddTx(tx1_r, pool); | ||
|
|
||
| uint32_t iteration{0}; | ||
|
|
||
| bench.run([&]() NO_THREAD_SAFETY_ANALYSIS { | ||
|
|
||
| CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool); | ||
|
||
| iteration++; | ||
| }); | ||
| } | ||
|
|
||
| BENCHMARK(MempoolCheckEphemeralSpends, benchmark::PriorityLevel::HIGH); | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,78 @@ | ||||||
| // Copyright (c) 2024-present The Bitcoin Core developers | ||||||
| // Distributed under the MIT software license, see the accompanying | ||||||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||||||
|
|
||||||
| #include <policy/ephemeral_policy.h> | ||||||
|
||||||
| #include <policy/policy.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); }); | ||||||
|
||||||
| } | ||||||
|
|
||||||
| bool CheckValidEphemeralTx(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)) { | ||||||
instagibbs marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| 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) | ||||||
|
||||||
| { | ||||||
| if (!Assume(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;}))) { | ||||||
| // Bail out of spend checks if caller gave us an invalid package | ||||||
| return std::nullopt; | ||||||
| } | ||||||
|
|
||||||
| std::map<Txid, CTransactionRef> map_txid_ref; | ||||||
| for (const auto& tx : package) { | ||||||
| map_txid_ref[tx->GetHash()] = tx; | ||||||
| } | ||||||
|
|
||||||
| 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; | ||||||
|
|
||||||
| for (const auto& tx_input : tx->vin) { | ||||||
| const Txid& parent_txid{tx_input.prevout.hash}; | ||||||
glozow marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| // Skip parents we've already checked dust for | ||||||
| if (processed_parent_set.contains(parent_txid)) continue; | ||||||
|
||||||
| if (processed_parent_set.contains(parent_txid)) continue; | |
| if (!processed_parent_set.insert(parent_txid).second) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be heretical but I find as-is easier to read? Can add to follow-up if demand is there.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although it seems correct to run the loop for the outputs of the parent transaction but as per the definition of Ephemeral Dust, we can break (and end) the loop here after the dust is found because only one is allowed per parent transaction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would make the logic simpler or significantly more performant, and would rather the IsStandardTx check not be so tightly bound.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm missing something simple, but could this line insert the parent txid into processed_parent_set even when the parent couldn't be checked for dust (parent_ref is nullptr)? If so, would we want to handle this as an error condition?
Maybe it's not an issue currently (e.g. reliance on 1P1C), but could be defensive to check to protect in the event of future change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's not in mempool or the package, then it can't have unconfirmed dust. Is there a scenario you're concerned about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was just focused on handling edge cases (and preventing harder to notice issues later). Not sure if there's a scenario involving orphaning that might play out (haven't thought through all scenarios).
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I think this is much easier to follow with the codepaths merged, thanks. However on further review, I am concerned about how much work we might do in this function.
As written, if a transaction had 1000 inputs, which all spend from a single parent with 1000 outputs, then we'd do 1M loop iterations, because we're not deduplicating parents for each input of a transaction (so for each input of the child, we'd be looking at all 1000 outputs of the parent). If we deduplicate the parents first, then the number of parent outputs we might possibly have to look at is bounded by the ancestor size limit (or the cluster size limit, in the future), to something on the order of a few thousand outputs.
If we are relying on the ancestor size limits for a work bound, then we should make sure that all those limits are checked prior to this function being invoked. I think in the package acceptance case, that would involve moving this check to come after the CheckPackageLimits() check that happens in AcceptMultipleTransactions.
I think this might be enough, but we could consider going further -- what if we were to cache how many dust outputs a transaction has in the CTxMemPoolEntry itself? Then we would be reduced to just looping over the inputs of the incoming transactions and counting, for each distinct parent, how many of the dust outputs are spent. This is probably not simple to do at the moment (since in the package case, we don't have CTxMemPoolEntry objects to work on yet), but I have some refactors in mind for mempool acceptance which would make this easier to consider in the future.
Anyway, it might be nice to add a micro-benchmark for transaction validation of a single transaction with 1000 inputs, which all come from a single parent with 1000 outputs, just to make sure we're not slowing that case down inordinately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't have CTxMemPoolEntry objects to work on yet
I don't think that's true? PreChecks has built a mempool entry for each package tx. We could track the exact outpoint that is dust in the mempool entry. I'll give that a shot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sdaftuar this should do it if you think something like this is reasonable: instagibbs@6007146
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I I think that approach uses an extra 24 bytes per mempool entry, which strikes me as a bit heavy for what we need. Let's see if this is fast enough as-is and if so, call it a day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative is to store a bool, and use that to filter 99.9% of transactions that won't have dust, and then eat the O(num_outputs) cost to find the dust when at spending time.
With 1000 inputs/outputs and a small number of dust outputs, I get:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 60,600.53 | 16,501.51 | 0.7% | 0.01 | `MempoolCheckEphemeralSpends`
When I make all the outputs dust(which can't happen currently due to IsStandard):
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 75,160.69 | 13,304.83 | 0.6% | 1.07 | `MempoolCheckEphemeralSpends`
number of outputs/inputs can be varied in the benchmark via -asymptote=X
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be fine for performance. Large transactions take orders of magnitude more time to validate than what is being added here (and for small transactions this is extremely fast).
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could speed up the common case a bit by checking unspent_parent_dust.empty() here, but perhaps not significantly
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| // Copyright (c) 2024-present The Bitcoin Core developers | ||
| // Distributed under the MIT software license, see the accompanying | ||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
||
| #ifndef BITCOIN_POLICY_EPHEMERAL_POLICY_H | ||
| #define BITCOIN_POLICY_EPHEMERAL_POLICY_H | ||
|
|
||
| #include <policy/packages.h> | ||
| #include <policy/policy.h> | ||
| #include <primitives/transaction.h> | ||
| #include <txmempool.h> | ||
|
|
||
| /** 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 | ||
| * - 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 | ||
| * | ||
| * Imagine three transactions: | ||
| * TxA, 0-fee with two outputs, one non-dust, one dust | ||
| * TxB, spends TxA's non-dust | ||
| * TxC, spends TxA's dust | ||
| * | ||
| * All the dust is spent if TxA+TxB+TxC is accepted, but the mining template may just pick | ||
| * up TxA+TxB rather than the three "legal configurations: | ||
|
||
| * 1) None | ||
| * 2) TxA+TxB+TxC | ||
| * 3) TxA+TxC | ||
| * By requiring the child transaction to sweep any dust from the parent txn, we ensure that | ||
| * there is a single child only, and this child, or the child's descendants, | ||
| * 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. | ||
| */ | ||
| bool CheckValidEphemeralTx(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, | ||
| * 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. | ||
instagibbs marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| */ | ||
| std::optional<Txid> CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool); | ||
|
||
|
|
||
| #endif // BITCOIN_POLICY_EPHEMERAL_POLICY_H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -129,6 +129,7 @@ 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,11 +143,16 @@ bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_dat | |
| reason = "bare-multisig"; | ||
| return false; | ||
| } else if (IsDust(txout, dust_relay_fee)) { | ||
| reason = "dust"; | ||
| return false; | ||
| num_dust_outputs++; | ||
instagibbs marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| // Only MAX_DUST_OUTPUTS_PER_TX dust is permitted(on otherwise valid ephemeral dust) | ||
| if (num_dust_outputs > MAX_DUST_OUTPUTS_PER_TX) { | ||
| reason = "dust"; | ||
| return false; | ||
| } | ||
|
||
|
|
||
| // only one OP_RETURN txout is permitted | ||
| if (nDataOut > 1) { | ||
| reason = "multi-op-return"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,10 @@ static const unsigned int MAX_OP_RETURN_RELAY = 83; | |
| */ | ||
| static constexpr unsigned int EXTRA_DESCENDANT_TX_SIZE_LIMIT{10000}; | ||
|
|
||
| /** | ||
| * Maximum number of ephemeral dust outputs allowed. | ||
| */ | ||
| static constexpr unsigned int MAX_DUST_OUTPUTS_PER_TX{1}; | ||
|
||
|
|
||
| /** | ||
| * Mandatory script verification flags that all new transactions must comply with for | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| #include <node/context.h> | ||
| #include <node/miner.h> | ||
| #include <node/warnings.h> | ||
| #include <policy/ephemeral_policy.h> | ||
| #include <pow.h> | ||
| #include <rpc/blockchain.h> | ||
| #include <rpc/mining.h> | ||
|
|
@@ -491,7 +492,15 @@ static RPCHelpMan prioritisetransaction() | |
| throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is no longer supported, dummy argument to prioritisetransaction must be 0."); | ||
| } | ||
|
|
||
| EnsureAnyMemPool(request.context).PrioritiseTransaction(hash, nAmount); | ||
| CTxMemPool& mempool = EnsureAnyMemPool(request.context); | ||
|
|
||
| // 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)) { | ||
| throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is not supported for transactions with dust outputs."); | ||
|
||
| } | ||
|
||
|
|
||
| mempool.PrioritiseTransaction(hash, nAmount); | ||
| return true; | ||
| }, | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and for
tx2:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe touch in follow-up