Trim closing output below network dust threshold #896
Conversation
Since HTLCs below this amount will not appear in the commitment tx, they are effectively converted to miner fees. The peer could use this to grief you by broadcasting its commitment once it contains a lot of dust HTLCs. Fixes #696
As implemented in Bitcoin Core's default relay policy.
When `dust_limit_satoshis` are below 546 sats, we may create closing txs that won't propagate (e.g. if one of the closing scripts is not using segwit). This isn't a critical issue since the channel can be force-closed and the commit should always propagate, but we can do better. Signers should remove their output if it causes the transaction to fail propagation. This is backwards-compatible because they already had the option to remove their own output (and receivers should check the signature against both potential closing transactions).
| @@ -364,7 +366,10 @@ Each node offering a signature: | |||
| - MUST round each output down to whole satoshis. | |||
| - MUST subtract the fee given by `fee_satoshis` from the output to the funder. | |||
| - MUST remove any output below its own `dust_limit_satoshis`. | |||
| - MAY eliminate its own output. | |||
| - if its own output is below the network's dust threshold (see the [dust limits section](#dust-limits)): | |||
There was a problem hiding this comment.
I think it would be cleaner to also add a tlv to closing_signed to advertise "I removed my own output" to make this explicit instead of having to check the sig against two distinct txs. Thoughts?
There was a problem hiding this comment.
Because you don't know if your counteparty has the default policy of pruning under their dust_limit_satoshis at closing? Not even sure a tlv signaling in closing_signed works as the scriptpubkey could be unknown and smaller and as such the Core's GetDustThreshold lower than the announced dust_limit_satoshis for p2wsh.
So maybe the pruning should be opt-in at shutdown to remove the signature equivocation ?
There was a problem hiding this comment.
I think you may be misunderstanding my comment (it's probably misplaced, it's not related to this specific line but is more general).
As this paragraph previously mentioned, the sender of closing_signed can always chose to remove its own output from the tx it's signing (for whatever reason they want) which means the receiver must test the signature against potentially two versions of the closing tx.
I'm suggesting to add a tlv to make this explicit: when a node signs a closing_tx, if its own output has been removed from that tx, it says so in that tlv. This way the receiver doesn't need to potentially check two distinct txs against the signature, it knows what "version" of the closing_tx has been signed.
Here is a concrete example. Let's imagine that Alice and Bob both set dust_limit to 546 sats.
Alice's main output is 1 000 sats and Bob's main output is 10 000 sats.
She decides to be nice to miners and trims her output: the signature she sends in her closing_signed is for a tx with only Bob's output.
According to the spec, Bob should check the signature against two potential txs: one with Alice and Bob's outputs, and one with Bob's output only. I'm suggesting that Alice adds a tlv in her closing_signed message that says "I removed my own output, the tx I signed contains only yours". This way Bob doesn't need to check two txs, he knows exactly what tx the signature should be for.
There was a problem hiding this comment.
BOLT3
MAY eliminate its own output.
Right, I forgot that point! Okay I agree with the proposal part to explicitly signal one's own output removal from closing_tx inclusion through a new tlv and that way having the receiver knowing exactly what tx the signature is for.
There was a problem hiding this comment.
Yeah seems we need a new TLV here as @t-bast mentions, as otherwise the node receiving the signature may just reject it as being invalid if it doesn't know to drop the output. One hacky way to handle this would be to have the node just try the diff combinations of the outputs being there (or not). IMO it's better to make it explicit though (along with a feature bit), as otherwise we'll see co-op close attempts break down in practice.
There was a problem hiding this comment.
Concept ACK
I'm curious to know whether other implementations allow this or not.
We prune to_remote/to_local outputs if they're under our holder_dust_limit_satoshis, see https://github.com/rust-bitcoin/rust-lightning/blob/2ced708b71600e99e53ef526d5b4508c784208f9/lightning/src/ln/channel.rs#L1308
| @@ -364,7 +366,10 @@ Each node offering a signature: | |||
| - MUST round each output down to whole satoshis. | |||
| - MUST subtract the fee given by `fee_satoshis` from the output to the funder. | |||
| - MUST remove any output below its own `dust_limit_satoshis`. | |||
| - MAY eliminate its own output. | |||
| - if its own output is below the network's dust threshold (see the [dust limits section](#dust-limits)): | |||
There was a problem hiding this comment.
Because you don't know if your counteparty has the default policy of pruning under their dust_limit_satoshis at closing? Not even sure a tlv signaling in closing_signed works as the scriptpubkey could be unknown and smaller and as such the Core's GetDustThreshold lower than the announced dust_limit_satoshis for p2wsh.
So maybe the pruning should be opt-in at shutdown to remove the signature equivocation ?
| - a p2sh input doesn't have a fixed size, since it depends on the underlying | ||
| script, so we use 148 bytes as a lower bound | ||
| - the p2sh dust threshold is then `(32 + 148) * 3000 / 1000 = 540 satoshis` | ||
|
|
| @@ -480,6 +482,70 @@ A node: | |||
| - if the resulting fee rate is too low: | |||
| - MAY fail the channel. | |||
|
|
|||
| ## Dust Limits | |||
There was a problem hiding this comment.
+1000 for this derivation here!
| @@ -364,7 +366,10 @@ Each node offering a signature: | |||
| - MUST round each output down to whole satoshis. | |||
| - MUST subtract the fee given by `fee_satoshis` from the output to the funder. | |||
| - MUST remove any output below its own `dust_limit_satoshis`. | |||
| - MAY eliminate its own output. | |||
| - if its own output is below the network's dust threshold (see the [dust limits section](#dust-limits)): | |||
There was a problem hiding this comment.
Yeah seems we need a new TLV here as @t-bast mentions, as otherwise the node receiving the signature may just reject it as being invalid if it doesn't know to drop the output. One hacky way to handle this would be to have the node just try the diff combinations of the outputs being there (or not). IMO it's better to make it explicit though (along with a feature bit), as otherwise we'll see co-op close attempts break down in practice.
|
There seems to be agreement in #894 to require the dust threshold be over the network dust threshold, which means this PR is redundant. I believe it can be closed. |
|
Agreed, closing this PR for now as I think we'll instead deprecate non-segwit scripts in |
When
dust_limit_satoshisare below 546 sats, we may create closing txs that won't propagate (e.g. if one of the closing scripts is not using segwit). This isn't a critical issue since the channel can be force-closed and the commit tx should always propagate, but we can do better.Signers should remove their output if it causes the transaction to fail propagation. This is backwards-compatible because they already had the option to remove their own output (and receivers should check the signature against both potential closing transactions).
To be fair, I just discovered this requirement to check both potential transactions, and eclair currently doesn't implement it 😱
I'm curious to know whether other implementations allow this or not.
@ariard @TheBlueMatt
NB: this builds on #894