-
Notifications
You must be signed in to change notification settings - Fork 521
Explicit commit_sig retransmission for interactive-tx
#1289
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
base: master
Are you sure you want to change the base?
Explicit commit_sig retransmission for interactive-tx
#1289
Conversation
When disconnecting in the middle of the signing steps of an `interactive-tx` transaction, we must retransmit signatures on reconnection to complete the `interactive-tx` protocol. Nodes first exchange `commitment_signed`, followed by `tx_signatures` once they have both sent and received `commitment_signed`. We previously always retransmitted `commitment_signed`, even when our peer had already received it. We now include an explicit bitfield that lets nodes request `commitment_signed` if they haven't received it. Note that this is a breaking change, and we are thus changing the TLV type to make it easier to detect. In practice it should be fine, since only `eclair` and `cln` have shipped support for dual funding, and those two implementations are already incompatible on reconnection because `eclair` implements lightning#1214 but `cln` doesn't. This edge case only creates an issue when nodes disconnect after exchanging `tx_complete` but before receiving signatures, which should happen very infrequently. Replaces lightning#1214.
Can you remind me again why we can't just retransmit the |
Because it's hacky, why retransmit a message that doesn't need retransmission? Also, it's trivial without taproot, because the sender could simply store their
If we can avoid an unnecessary musig2 round, I think it's worth it, especially since it makes the protocol conceptually cleaner. |
Well, without a signal you don't know if it doesn't need retransmission. You can leave the signal out and retransmit the message and the protocol will work. It's simpler and requires less if statements/state checks to take the same action on every reconnection.
It's only "unnecessary" in some fraction of cases; in others it will be necessary. You can easily remove an if switch by taking the same action on every reconnection, which makes the protocol simpler to write and verify. |
Updating splice related reestablish code to lightning/bolts#1289 and lightning/bolts#1160 Changelog-Changed: Breaking change -- if you have splicing enabled on a channel both nodes must upgrade in unison due to updating `channel_reestablish` for to new splice specifications
Updating splice related reestablish code to lightning/bolts#1289 and lightning/bolts#1160 Changelog-Changed: Breaking change -- if you have splicing enabled on a channel both nodes must upgrade in unison due to updating `channel_reestablish` for to new splice specifications
Updating splice related reestablish code to lightning/bolts#1289 and lightning/bolts#1160 Changelog-Changed: Breaking change -- if you have splicing enabled on a channel both nodes must upgrade in unison due to updating `channel_reestablish` for to new splice specifications
Updating splice related reestablish code to lightning/bolts#1289 and lightning/bolts#1160 Changelog-Changed: Breaking change -- if you have splicing enabled on a channel both nodes must upgrade in unison due to updating `channel_reestablish` for to new splice specifications
Updating splice related reestablish code to lightning/bolts#1289 and lightning/bolts#1160 Changelog-Changed: Breaking change -- if you have splicing enabled on a channel both nodes must upgrade in unison due to updating `channel_reestablish` for to new splice specifications
Updating splice related reestablish code to lightning/bolts#1289 and lightning/bolts#1160 Changelog-Changed: Breaking change -- if you have splicing enabled on a channel both nodes must upgrade in unison due to updating `channel_reestablish` for to new splice specifications
jkczyz
left a comment
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.
ACK 1d25dd1 modulo a minor wording issue.
|
@niftynei @ddustin can you please take a look and ACK this PR? We'd really like to move forward and get it merged since we have cross-compat with LDK and it works with This PR is a pre-requisite for the splice PR and is blocking its progress. |
jkczyz
left a comment
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.
ACK 7646fc2
When disconnecting in the middle of the signing steps of an
interactive-txtransaction, we must retransmit signatures on reconnection to complete theinteractive-txprotocol.Nodes first exchange
commitment_signed, followed bytx_signaturesonce they have both sent and receivedcommitment_signed.We previously always retransmitted
commitment_signed, even when our peer had already received it. We now include an explicit bitfield that lets nodes requestcommitment_signedif they haven't received it.Note that this is a breaking change, and we are thus changing the TLV type to make it easier to detect. In practice it should be fine, since only
eclairandclnhave shipped support for dual funding, and those two implementations are already incompatible on reconnection becauseeclairimplements #1214 butclndoesn't. This edge case only creates an issue when nodes disconnect after exchangingtx_completebut before receiving signatures, which should happen very infrequently.Replaces #1214.