-
Notifications
You must be signed in to change notification settings - Fork 492
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
Closing tx detailed specification #201
Conversation
9afd244
to
5d27a11
Compare
…heck fee The recipient should check the fee is valid, otherwise we might get fun overflow games. So might as well check that it's better than the previous offer (as the sender is already required to do). Changing BOLT #3 to define the mutual close tx is next patch. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The only surprise here (maybe?) is that we use the commitment number encoding. I think that makes sense, but it was unspecified before. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Version fix as pointed out by @Roasbeef
5d27a11
to
04d318a
Compare
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 find it much cleaner this way, thanks!
The part about fee is still a little unclear though. I am guessing the funder has to be able to pay the fee since it had to be able to pay the last commitment tx as required here and reminded here, and weight(closingTx) <= weight(commitTx). Maybe this deserves to be mentioned in the Rationale?
I think we should also do a bit of reorganization and group "Commitment Transaction", "HTLC-Timeout and HTLC-Success Transactions", "Commitment Transaction construction" and "Fees". Also, "Commitment Transaction Construction" is missing in the ToC.
|
||
Note that there are two possible variants for each node. | ||
|
||
* version: 1 |
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.
Isn't that version 2?
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.
@Roasbeef said version 1, since version 2 only needed for CLTV, which we don't need here. I don't really mind...
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.
Using version 1 here allow us to blend in a bit more with regular (non contract) traffic (assuming an eventual saturation of transactions spending segwit inputs).
02-peer-protocol.md
Outdated
recipient MUST fail the connection if `fee_satoshis` is greater than | ||
the base fee of the final commitment transaction as calculated in | ||
[BOLT #3](03-transactions.md#fee-calculation). Otherwise the | ||
recipient SHOULD fail the connection if `fee_satoshis` is not strictly |
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 this doesn't work in case of a reconnection, where both parties re-send their last closing_signed
?
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.
That's why the sentence ends:
... unless it is equal to the previously-received fee_satoshis
and it has reconnected since then.
It's a bit awkward, I agree :(
03-transactions.md
Outdated
|
||
### Requirements | ||
|
||
Each node offering MUST subtract the fee given by `fee_satoshis` from |
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.
How about:
Each node offering a signature MUST subtract the fee given by
fee_satoshis
from
the output to the funder; it MUST then remove any output below its owndust_limit_satoshis
, and MAY also eliminate its own output.
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.
Yes, that's better,
03-transactions.md
Outdated
explicitly allowed. The signature will indicate which variant | ||
has been used. | ||
|
||
It is assumed that `dust_limit_satoshis` is greater than the funding |
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.
Something is missing here?
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.
Oops! yep....
03-transactions.md
Outdated
|
||
Note that there are two possible variants for each node. | ||
|
||
* version: 2 |
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.
Why a version of 2? We're just spending a plain multi-sig. In the future we can use more "covert" outputs with functionality equivalent to the multi-sig output. With such outputs (and if this is just a version 1), then channel opening and closing transactions gain a greater anonymity set, as they look like regular bitcoin transactions.
02-peer-protocol.md
Outdated
recipient SHOULD fail the connection if `fee_satoshis` is not strictly | ||
between its last-sent `fee_satoshis` and its previously-received | ||
`fee_satoshis`, unless it is equal to the previously-received | ||
`fee_satoshis` and it has reconnected since then. |
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.
In line with my comment on #199 (which is related to this PR as well), why are we requiring nodes remember the prior close negotiation state between re-connects. It may be the case that fees have gone down significantly since the prior negotiation, and both nodes would be massively overpaying (or the other way around). If we instead allow nodes to forget any negotiation upon re-connect then this allows them to have more completely control over the fees that they're paying.
From another angle, it seems that auto-attempt-to-re-close-on-re-connect may not even be a behavior that we desire. Days (even weeks) may pass between re-connects, it may not necessarily be the case that if a user wished to cooperatively close out the channel 3 days ago, that they still wish to do so upon re-connect (whenever that is).
|
||
Note that there are two possible variants for each node. | ||
|
||
* version: 1 |
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.
Using version 1 here allow us to blend in a bit more with regular (non contract) traffic (assuming an eventual saturation of transactions spending segwit inputs).
* `txin[0]` sequence: 0xFFFFFFFF | ||
* `txin[0]` script bytes: 0 | ||
* `txin[0]` witness: `0 <signature_for_key1> <signature_for_key2>` | ||
* txout count: 0, 1 or 2 |
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.
In what case would having no transaction outputs be warranted or valid?
* txout count: 0, 1 or 2 | ||
* `txout` amount: final balance to be paid to one node (minus `fee_satoshis` from `closing_signed` if this peer funded the channel) | ||
* `txout` script: as specified in that node's `scriptpubkey` in its `shutdown` message. | ||
|
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.
BIP 69 should be used here as well. I'm assuming y'all already do so?
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.
yes
Olaoluwa Osuntokun <notifications@github.com> writes:
Roasbeef requested changes on this pull request.
> @@ -234,6 +235,45 @@ The witness script for the output is:
To spend this via penalty, the remote node uses a witness stack `<revocationsig> 1` and to collect the output the local node uses an input with nSequence `to_self_delay` and a witness stack `<local_delayedsig> 0`
+## Closing Transaction
+
+Note that there are two possible variants for each node.
+
+* version: 2
Why a version of 2? We're just spending a plain multi-sig. In the future we can use more "covert" outputs with functionality equivalent to the multi-sig output. With such outputs (and if this is just a version 1), then channel opening and closing transactions gain a greater anonymity set, as they look like regular bitcoin transactions.
Agreed, it was fixed in the fixup to v1.
> +The receiver MUST check `signature` is valid for either variant of close
+transaction specified in [BOLT #3](03-transactions.md#closing-transaction),
+and MUST fail the connection if it is not.
+
+If `fee_satoshis` is equal to its previously sent `fee_satoshis`, the receiver
+SHOULD close the connection and SHOULD sign and broadcast the final closing
+transaction.
+
+Otherwise, if this is the first `closing_signed` received, the
+recipient MUST fail the connection if `fee_satoshis` is greater than
+the base fee of the final commitment transaction as calculated in
+[BOLT #3](03-transactions.md#fee-calculation). Otherwise the
+recipient SHOULD fail the connection if `fee_satoshis` is not strictly
+between its last-sent `fee_satoshis` and its previously-received
+`fee_satoshis`, unless it is equal to the previously-received
+`fee_satoshis` and it has reconnected since then.
In line with my comment on #199 (which is related to this PR as well), why are we requiring nodes remember the _prior_ close negotiation state _between_ re-connects. It may be the case that fees have gone down significantly since the prior negotiation, and both nodes would be massively overpaying (or the other way around). If we instead allow nodes to forget any negotiation upon re-connect then this allows them to have more completely control over the fees that they're paying.
Hmm, I actually like this, if only because it's stateless. If there's
consensus on this, let's loosen the spec.
>From another angle, it seems that auto-attempt-to-re-close-on-re-connect may not even be a behavior that we desire. Days (even weeks) may pass between re-connects, it may not necessarily be the case that if a user wished to cooperatively close out the channel 3 days ago, that they _still_ wish to do so upon re-connect (whenever that is).
Someone wants to close, they're probably not going to wait 3 days for
it. You can't *unoffer*, since the close isn't revocable.
+Note that there are two possible variants for each node.
+
+* version: 1
+* locktime: 0
+* txin count: 1
+ * `txin[0]` outpoint: `txid` and `output_index` from `funding_created` message
+ * `txin[0]` sequence: 0xFFFFFFFF
+ * `txin[0]` script bytes: 0
+ * `txin[0]` witness: `0 <signature_for_key1> <signature_for_key2>`
+* txout count: 0, 1 or 2
In what case would having _no_ transaction outputs be warranted or valid?
It's theoretically possible if your dust limits and fees are large
enough. I'd put it under *won't happen*, except if it does, it's better
than leaving unspent UTXOs.
+ * `txin[0]` sequence: 0xFFFFFFFF
+ * `txin[0]` script bytes: 0
+ * `txin[0]` witness: `0 <signature_for_key1> <signature_for_key2>`
+* txout count: 0, 1 or 2
+ * `txout` amount: final balance to be paid to one node (minus `fee_satoshis` from `closing_signed` if this peer funded the channel)
+ * `txout` script: as specified in that node's `scriptpubkey` in its `shutdown` message.
+
BIP 69 should be used here as well. I'm assuming y'all already do so?
Yep, and I just checked: that's specified in the top of BOLT #3 for all
transactions.
|
Suggested-by: Roasbeef Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Change descriptions of closing tx construction to references to BOLT 3. 2. Recipient *should* check the fee offer has improved in closing_signed. 3. Therefore, sender *must* improve closing offer. 4. Offers do not persist across reconnection, so no state req'd, and also helps if fee has changed. 5. You don't need to re-send `shutdown` if you received `closing_signed` (implicit acknowledgement). 6. You don't have to accept a `channel_reestablish` which requests the last revoke_and_ack be retransmitted if you've already received `closing_signed` (which is an implicit acknowledgement). Closes: lightning#201 Closes: lightning#199 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Change descriptions of closing tx construction to references to BOLT 3. 2. Recipient *should* check the fee offer has improved in closing_signed. 3. Therefore, sender *must* improve closing offer. 4. Offers do not persist across reconnection, so no state req'd, and also helps if fee has changed. 5. You don't need to re-send `shutdown` if you received `closing_signed` (implicit acknowledgement). 6. You don't have to accept a `channel_reestablish` which requests the last revoke_and_ack be retransmitted if you've already received `closing_signed` (which is an implicit acknowledgement). Closes: lightning#201 Closes: lightning#199 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Change descriptions of closing tx construction to references to BOLT 3. 2. Recipient *should* check the fee offer has improved in closing_signed. 3. Therefore, sender *must* improve closing offer. 4. Offers do not persist across reconnection, so no state req'd, and also helps if fee has changed. 5. You don't need to re-send `shutdown` if you received `closing_signed` (implicit acknowledgement). 6. You don't have to accept a `channel_reestablish` which requests the last revoke_and_ack be retransmitted if you've already received `closing_signed` (which is an implicit acknowledgement). Closes: lightning#201 Closes: lightning#199 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Spec changes:
The "forward progress" check is a bit nastier than it should be due to reconnect, where we have to allow re-transmissions.