Skip to content

Commit

Permalink
policy, rpc: Disallow non-0 mod fees
Browse files Browse the repository at this point in the history
Reject ephemeral dust entry into mempool
if base or modified fees are non-0.

prioritisetransaction throws an error
if called on a transaction in its mempool
that has dust.

This has the effect that unspent dust should
never be mined unless minrelay et al are changed
to 0 fee, or the operator simply redefines that dust
values using `-dustrelayfee` to make dust threshholds
lower.
  • Loading branch information
instagibbs committed Sep 13, 2024
1 parent e2eecf6 commit 289811a
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 27 deletions.
5 changes: 3 additions & 2 deletions src/policy/ephemeral_policy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
#include<policy/ephemeral_policy.h>
#include<policy/policy.h>

bool CheckValidEphemeralTx(const CTransactionRef tx, CFeeRate dust_relay_fee, CAmount txfee, TxValidationState& state)
bool CheckValidEphemeralTx(const CTransactionRef tx, CFeeRate dust_relay_fee, CAmount base_fee, CAmount mod_fee, TxValidationState& state)
{
// We never want to give incentives to mine this transaction alone
if (txfee != 0 && std::any_of(tx->vout.cbegin(), tx->vout.cend(), [&](const auto& output) { return IsDust(output, dust_relay_fee); })) {
if ((base_fee != 0 || mod_fee != 0) &&
std::any_of(tx->vout.cbegin(), tx->vout.cend(), [&](const auto& output) { return IsDust(output, dust_relay_fee); })) {
return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust", "tx with dust output must be 0-fee");
}

Expand Down
2 changes: 1 addition & 1 deletion src/policy/ephemeral_policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* 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_fee, CAmount txfee, TxValidationState& state);
bool CheckValidEphemeralTx(const CTransactionRef tx, CFeeRate dust_relay_fee, CAmount base_fee, CAmount mod_fee, TxValidationState& state);

/** Must be called for each transaction package if any dust is in the package.
* Checks that all dust in a package ends up spent by an only-child or has no child.
Expand Down
10 changes: 9 additions & 1 deletion src/rpc/mining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -493,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 fees are not allowed for entry, and modification not allowed afterwards
const auto& tx = mempool.get(hash);
if (tx && std::any_of(tx->vout.cbegin(), tx->vout.cend(), [&](const auto& output) { return IsDust(output, mempool.m_opts.dust_relay_feerate); })) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is not supported for transactions with ephemeral dust.");
}

mempool.PrioritiseTransaction(hash, nAmount);
return true;
},
};
Expand Down
4 changes: 2 additions & 2 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -935,9 +935,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
fSpendsCoinbase, nSigOpsCost, lock_points.value()));
ws.m_vsize = entry->GetTxSize();

// Enforces 0 base fee for dust transactions, no incentive to be mined alone
// 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, state)) {
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
}
}
Expand Down
50 changes: 29 additions & 21 deletions test/functional/mempool_ephemeral_dust.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,23 @@ def test_normal_dust(self):
# And doesn't work on its own
assert_raises_rpc_error(-26, "min relay fee not met", self.nodes[0].sendrawtransaction, dusty_tx["hex"])

# But if we add modified fees, it would succeed
# If we add modified fees, it is still not allowed due to dust check
self.nodes[0].prioritisetransaction(txid=dusty_tx["txid"], dummy=0, fee_delta=COIN)
test_res = self.nodes[0].testmempoolaccept([dusty_tx["hex"]])
assert test_res[0]["allowed"]
assert not test_res[0]["allowed"]
assert_equal(test_res[0]["reject-reason"], "dust")
# Reset priority
self.nodes[0].prioritisetransaction(txid=dusty_tx["txid"], dummy=0, fee_delta=-COIN)

# And package evaluation succeeds
# Package evaluation succeeds
res = self.nodes[0].submitpackage([dusty_tx["hex"], sweep_tx["hex"]])
assert_equal(res["package_msg"], "success")
assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]])

# Entry is denied when non-0-fee, either base or unmodified.
# If in-mempool, we're not allowed to prioritise due to detected dust output
assert_raises_rpc_error(-8, "Priority is not supported for transactions with ephemeral dust.", self.nodes[0].prioritisetransaction, dusty_tx["txid"], 0, 1)

self.generate(self.nodes[0], 1)
assert_equal(self.nodes[0].getrawmempool(), [])

Expand Down Expand Up @@ -144,7 +150,7 @@ def test_fee_having_parent(self):
assert_equal(res["package_msg"], "transaction failed")
assert_equal(res["tx-results"][dusty_tx["wtxid"]]["error"], "dust, tx with dust output must be 0-fee")

# Will be accepted if base fee is 0 even if modified fee is non-0
# Will not be accepted if base fee is 0 with modified fee of non-0
dusty_tx = self.wallet.create_self_transfer_multi(fee_per_output=0, version=3)
self.add_output_to_create_multi_result(dusty_tx)

Expand All @@ -153,18 +159,17 @@ def test_fee_having_parent(self):
self.nodes[0].prioritisetransaction(txid=dusty_tx["txid"], dummy=0, fee_delta=1000)
self.nodes[1].prioritisetransaction(txid=dusty_tx["txid"], dummy=0, fee_delta=1000)

# Submitted alone
# It's rejected submitted alone
test_res = self.nodes[0].testmempoolaccept([dusty_tx["hex"]])
assert test_res[0]["allowed"]
assert not test_res[0]["allowed"]
assert_equal(test_res[0]["reject-reason"], "dust")

# Or as a package
res = self.nodes[0].submitpackage([dusty_tx["hex"], sweep_tx["hex"]])
assert_equal(res["package_msg"], "success")
assert_equal(len(self.nodes[0].getrawmempool()), 2)
assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]])
assert_equal(res["package_msg"], "transaction failed")
assert_equal(res["tx-results"][dusty_tx["wtxid"]]["error"], "dust, tx with dust output must be 0-fee")

self.generate(self.nodes[0], 1)
assert_equal(self.nodes[0].getrawmempool(), [])
assert_mempool_contents(self, self.nodes[0], expected=[])

def test_multidust(self):
self.log.info("Test that a transaction with multiple ephemeral dusts is not allowed")
Expand All @@ -185,23 +190,26 @@ def test_multidust(self):
def test_nonzero_dust(self):
self.log.info("Test that a single output of any size is allowed, not checking spending")

# We aren't checking spending, allow it in with no fee
self.restart_node(0, extra_args=["-minrelaytxfee=0"])
self.restart_node(1, extra_args=["-minrelaytxfee=0"])
self.connect_nodes(0, 1)

# 330 is dust threshold for taproot outputs
for value in [1, 329, 330]:
assert_equal(self.nodes[0].getrawmempool(), [])

dusty_tx = self.wallet.create_self_transfer_multi(fee_per_output=0, version=3)
self.add_output_to_create_multi_result(dusty_tx, value)

self.nodes[0].prioritisetransaction(txid=dusty_tx["txid"], dummy=0, fee_delta=1000)
self.nodes[1].prioritisetransaction(txid=dusty_tx["txid"], dummy=0, fee_delta=1000)

test_res = self.nodes[0].testmempoolaccept([dusty_tx["hex"]])
assert test_res[0]["allowed"]

self.generate(self.nodes[0], 1)
self.restart_node(0, extra_args=[])
self.restart_node(1, extra_args=[])
self.connect_nodes(0, 1)
assert_mempool_contents(self, self.nodes[0], expected=[])


# N.B. If individual minrelay requirement is dropped, this test can be dropped
def test_non_truc(self):
self.log.info("Test that v2 dust-having transaction is rejected even if spent, because of min relay requirement")
Expand All @@ -227,16 +235,16 @@ def test_unspent_ephemeral(self):
dusty_tx = self.wallet.create_self_transfer_multi(fee_per_output=0, version=3)
self.add_output_to_create_multi_result(dusty_tx, 329)

# First time, enter dust tx by itself into mempool and test that dust must be spent
self.nodes[0].prioritisetransaction(dusty_tx["txid"], 0, 1000)
self.nodes[1].prioritisetransaction(dusty_tx["txid"], 0, 1000)

self.nodes[0].sendrawtransaction(dusty_tx["hex"])
# Valid sweep we will RBF incorrectly by not spending dust as well
sweep_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=dusty_tx["new_utxos"], version=3)
self.nodes[0].submitpackage([dusty_tx["hex"], sweep_tx["hex"]])
assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]])

# Doesn't spend in-mempool dust output from parent
unspent_sweep_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=[dusty_tx["new_utxos"][0]], version=3)
res = self.nodes[0].submitpackage([dusty_tx["hex"], unspent_sweep_tx["hex"]])
assert_equal(res["tx-results"][unspent_sweep_tx["wtxid"]]["error"], "ephemeral-dust-unspent, tx does not spend parent ephemeral dust")
assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]])

# Spend works with dust spent
sweep_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=dusty_tx["new_utxos"], version=3)
Expand Down

0 comments on commit 289811a

Please sign in to comment.