-
Notifications
You must be signed in to change notification settings - Fork 525
Trim closing output below network dust threshold #896
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,8 @@ This details the exact format of on-chain transactions, which both sides need to | |
| * [Fees](#fees) | ||
| * [Fee Calculation](#fee-calculation) | ||
| * [Fee Payment](#fee-payment) | ||
| * [Dust Limits](#dust-limits) | ||
| * [Commitment Transaction Construction](#commitment-transaction-construction) | ||
| * [Keys](#keys) | ||
| * [Key Derivation](#key-derivation) | ||
| * [`localpubkey`, `remotepubkey`, `local_htlcpubkey`, `remote_htlcpubkey`, `local_delayedpubkey`, and `remote_delayedpubkey` Derivation](#localpubkey-remotepubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation) | ||
|
|
@@ -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)): | ||
| - MUST eliminate its own output. | ||
| - otherwise: | ||
| - MAY eliminate its own output. | ||
|
|
||
| ### Rationale | ||
|
|
||
|
|
@@ -381,9 +386,6 @@ reason for the other side to fail the closing protocol; so this is | |
| explicitly allowed. The signature indicates which variant | ||
| has been used. | ||
|
|
||
| There will be at least one output, if the funding amount is greater | ||
| than twice `dust_limit_satoshis`. | ||
|
|
||
| ## Fees | ||
|
|
||
| ### Fee Calculation | ||
|
|
@@ -480,6 +482,70 @@ A node: | |
| - if the resulting fee rate is too low: | ||
| - MAY fail the channel. | ||
|
|
||
| ## Dust Limits | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1000 for this derivation here! |
||
|
|
||
| The `dust_limit_satoshis` parameter is used to configure the threshold below | ||
| which nodes will not produce on-chain transaction outputs. | ||
|
|
||
| There is no consensus rule in Bitcoin that makes outputs below dust thresholds | ||
| invalid or unspendable, but policy rules in popular implementations will prevent | ||
| relaying transactions that contain such outputs. | ||
|
|
||
| Bitcoin Core defines the following dust thresholds: | ||
|
|
||
| - pay to pubkey hash (p2pkh): 546 satoshis | ||
| - pay to script hash (p2sh): 540 satoshis | ||
| - pay to witness pubkey hash (p2wpkh): 294 satoshis | ||
| - pay to witness script hash (p2wsh): 330 satoshis | ||
|
|
||
| Details of this calculation (implemented [here](https://github.com/bitcoin/bitcoin/blob/0.21/src/policy/policy.cpp)): | ||
|
|
||
| - the feerate is set to 3000 sat/kB | ||
| - a p2wpkh output is 31 bytes: | ||
| - 8 bytes for the output amount | ||
| - 1 byte for the script length | ||
| - 22 bytes for the script (`OP_0` `20` 20-bytes) | ||
| - a p2wpkh input is at least 67 bytes (depending on the signature length): | ||
| - 36 bytes for the previous output (32 bytes hash + 4 bytes index) | ||
| - 1 byte for the script sig length | ||
| - 4 bytes for the sequence | ||
| - 26 bytes for the witness (with the 75% segwit discount applied): | ||
| - 1 byte for the items count | ||
| - 1 byte for the signature length | ||
| - 71 bytes for the signature | ||
| - 1 byte for the public key length | ||
| - 33 bytes for the public key | ||
| - the p2wpkh dust threshold is then `(31 + 67) * 3000 / 1000 = 294 satoshis` | ||
| - a p2wsh output is 43 bytes: | ||
| - 8 bytes for the output amount | ||
| - 1 byte for the script length | ||
| - 34 bytes for the script (`OP_0` `32` 32-bytes) | ||
| - a p2wsh input doesn't have a fixed size, since it depends on the underlying | ||
| script, so we use 67 bytes as a lower bound | ||
| - the p2wsh dust threshold is then `(43 + 67) * 3000 / 1000 = 330 satoshis` | ||
| - a p2pkh output is 34 bytes: | ||
| - 8 bytes for the output amount | ||
| - 1 byte for the script length | ||
| - 25 bytes for the script (`OP_DUP` `OP_HASH160` `20` 20-bytes `OP_EQUALVERIFY` `OP_CHECKSIG`) | ||
| - a p2pkh input is at least 148 bytes: | ||
| - 36 bytes for the previous output (32 bytes hash + 4 bytes index) | ||
| - 1 byte for the script sig length | ||
| - 4 bytes for the sequence | ||
| - 107 bytes for the script sig: | ||
| - 1 byte for the items count | ||
| - 1 byte for the signature length | ||
| - 71 bytes for the signature | ||
| - 1 byte for the public key length | ||
| - 33 bytes for the public key | ||
| - the p2pkh dust threshold is then `(34 + 148) * 3000 / 1000 = 546 satoshis` | ||
| - a p2sh output is 32 bytes: | ||
| - 8 bytes for the output amount | ||
| - 1 byte for the script length | ||
| - 23 bytes for the script (`OP_HASH160` `20` 20-bytes `OP_EQUAL`) | ||
| - 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` | ||
|
|
||
This comment was marked as abuse.
Sorry, something went wrong. |
||
| ## Commitment Transaction Construction | ||
|
|
||
| This section ties the previous sections together to detail the | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 it would be cleaner to also add a tlv to
closing_signedto advertise "I removed my own output" to make this explicit instead of having to check the sig against two distinct txs. Thoughts?This comment was marked as abuse.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 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_signedcan 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 theclosing_txhas been signed.Here is a concrete example. Let's imagine that Alice and Bob both set
dust_limitto 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_signedis 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_signedmessage 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.This comment was marked as abuse.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
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.