Ephemeral Dust #69
Ephemeral Dust #69
Conversation
This test would catch regressions where ephemeral dust checks are being erroneously applied on outputs that are not actually dust.
Also known as Ephemeral Dust.
We try to ensure that dust is spent in blocks by requiring:
- ephemeral dust tx is 0-fee
- ephemeral dust tx only has one dust output
- If the ephemeral dust transaction has a child,
the dust is spent by by that child.
0-fee requirement means there is no incentive to mine
a transaction which doesn't have a child bringing its
own fees for the transaction package.
Checks that transactions in mempool with dust follow expected invariants.
Works a bit harder to get ephemeral dust transactions into the mempool.
| return true; | ||
| } | ||
|
|
||
| std::optional<Txid> CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- If a transaction has dust, it must be 0-fee (CheckValidEphemeralTx)
- 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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
|
port-only-review-ACK efa9ff7 |
|
Merged; but probably will defer cutting a release and updating miner/nodes until upstream 28.1, unless there's a reason to go faster? |
Only differences from bitcoin#30239 are a couple lines in the functional test to use a clean chain, switch to autotools, and I did not port the release notes.
Was a trivial port otherwise.