Skip to content
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 #69

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ BITCOIN_CORE_H = \
node/warnings.h \
noui.h \
outputtype.h \
policy/ephemeral_policy.h \
policy/feerate.h \
policy/fees.h \
policy/fees_args.h \
Expand Down Expand Up @@ -445,6 +446,7 @@ libbitcoin_node_a_SOURCES = \
node/utxo_snapshot.cpp \
node/warnings.cpp \
noui.cpp \
policy/ephemeral_policy.cpp \
policy/fees.cpp \
policy/fees_args.cpp \
policy/packages.cpp \
Expand Down Expand Up @@ -951,6 +953,7 @@ libbitcoinkernel_la_SOURCES = \
node/blockstorage.cpp \
node/chainstate.cpp \
node/utxo_snapshot.cpp \
policy/ephemeral_policy.cpp \
policy/feerate.cpp \
policy/packages.cpp \
policy/policy.cpp \
Expand Down
1 change: 1 addition & 0 deletions src/Makefile.bench.include
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ bench_bench_bitcoin_SOURCES = \
bench/load_external.cpp \
bench/lockedpool.cpp \
bench/logging.cpp \
bench/mempool_ephemeral_spends.cpp \
bench/mempool_eviction.cpp \
bench/mempool_stress.cpp \
bench/merkle_root.cpp \
Expand Down
83 changes: 83 additions & 0 deletions src/bench/mempool_ephemeral_spends.cpp
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);
78 changes: 78 additions & 0 deletions src/policy/ephemeral_policy.cpp
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)) {
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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skimming this, two thoughts come to mind:

  • why call it "check ephemeral spends" when "check dust is swept" is right there?
  • I'm surprised this check isn't the other way around, something like:
std::set<COutPoint> dust_created;
for (tx : package) {
     for (tx_input : tx->vin) {
        dust_created.erase(tx_input.prevout);
     }
     Txid txid = tx->GetHash();
     for (int out = 0; out < tx->vout.size(); ++out) {
          const auto* tx_output = tx->vout[out];
          if (IsDust(tx_output)) {
             dust_created.insert(COutPoint{txid, out});
          }
    }
}
if (!dust_created.empty()) {
    ...
}

If the idea is that all dust created by a tx should be swept by the immediate next tx (so that the package can't be split and leave dust in the utxo set), you could move the empty() check to the middle of the main loop. Reporting the txid of the transaction that created the dust seems more logical than the one that should have spent it but didn't to me, fwiw.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why call it "check ephemeral spends" when "check dust is swept" is right there?

Er, dunno, I chose the name 2 years ago I think. Could amend the name change I have in the follow-up!

I'm surprised this check isn't the other way around, something like:

There are two interlocking rules:

  1. If a transaction has dust, it must be 0-fee (CheckValidEphemeralTx)
  2. If a transaction is spending from a transaction that has unconfirmed dust, the dust must be spent (CheckEphemeralSpends)

The snippet above is different in at least a couple of ways, the more important being that it would not detect the second case where there are in-mempool ancestors to the package.

If the idea is that all dust created by a tx should be swept by the immediate next tx

p2p relay questions aside, it's not really the case. Batched CPFP is possible for one, and if/when submitpackage is generalized further, could allow arbitrary chains of transactions with single dust outputs.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snippet above is different in at least a couple of ways, the more important being that it would not detect the second case where there are in-mempool ancestors to the package.

Sure, I was handwaving that any in-mempool ancestors were already part of the package.

Mostly, I think I was misled by:

 * 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

(missing close quote after "legal btw)

But we don't consider (2) legal, and can't because our current mining algorithm isn't smart enough to be able to call/satisfy CheckEphemeralSpends(). Cluster mempool could conceivably be smart enough to preserve that property, though, by ensuring that txs are never split from a chunk if it would cause a function like this to be unhappy... (In that case, the chunk would likely be [TxA, TxC, TxB] anyway...) In any event, relaying TxB prior to TxA being confirmed probably increases bad incentives for leaving dust in the utxo set anyway...

(This rationale would be better in a BIP than in code comments... /grumble)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we don't consider (2) legal

Right this could have been phrased better, "legal" here means a block template may be created where there is no dust entering into the utxo set. ie If TxA is mined, we want TxC mined, not that the implementation would allow that config to happen currently.

Cluster mempool could conceivably be smart enough to preserve that property, though, by ensuring that txs are never split from a chunk if it would cause a function like this to be unhappy...

I noodled on stuff like this for a while half a year ago and came away unsatisfied. I think it's hard to get something more advanced that acts natural and non-pinny from a wallet perspective but I don't have notes in front of me.

(This rationale would be better in a BIP than in code comments... /grumble)

Enjoy the aborted BIP attempt bitcoin/bips#1524

{
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};
// Skip parents we've already checked dust for
if (processed_parent_set.contains(parent_txid)) continue;

// We look for an in-package or in-mempool dependency
CTransactionRef parent_ref = nullptr;
if (auto it = map_txid_ref.find(parent_txid); it != map_txid_ref.end()) {
parent_ref = it->second;
} else {
parent_ref = tx_pool.get(parent_txid);
}

// Check for dust on parents
if (parent_ref) {
for (uint32_t out_index = 0; out_index < parent_ref->vout.size(); out_index++) {
const auto& tx_output = parent_ref->vout[out_index];
if (IsDust(tx_output, dust_relay_rate)) {
unspent_parent_dust.insert(COutPoint(parent_txid, out_index));
}
}
}

processed_parent_set.insert(parent_txid);
}

// 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;
}
}

return std::nullopt;
}
55 changes: 55 additions & 0 deletions src/policy/ephemeral_policy.h
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.
*/
std::optional<Txid> CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool);

#endif // BITCOIN_POLICY_EPHEMERAL_POLICY_H
10 changes: 8 additions & 2 deletions src/policy/policy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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++;
}
}

// 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";
Expand Down
4 changes: 4 additions & 0 deletions src/policy/policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion src/rpc/mining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -492,7 +493,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;
},
};
Expand Down
Loading
Loading