Skip to content
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

tx_signatures is missing tlvs field, needed for Splicing #2556

Closed
optout21 opened this issue Sep 6, 2023 · 5 comments
Closed

tx_signatures is missing tlvs field, needed for Splicing #2556

optout21 opened this issue Sep 6, 2023 · 5 comments
Assignees

Comments

@optout21
Copy link
Contributor

optout21 commented Sep 6, 2023

Splicing uses the interactive TX construction from Dual Funding, but it has a special issue:
the previous funding output is an input, that must be added by the initiator, but it has to be signed by both parties (as it's a 2-of-2 multisig).
To accommodate this use case, the tx_signatures message has been extended by a tlvs field.

The spec can be seen in this PR (draft PR against draft dual-funding PR):
https://github.com/lightning/bolts/pull/1009/files#diff-ed04ca2c673fd6aabde69389511fa9ee60cb44d6b2ef6c88b549ffaa753d6afeR387

Note: CLN already has this field (can be seen here https://github.com/ElementsProject/lightning/blob/master/wire/peer_wire.csv#L69 )

@optout21
Copy link
Contributor Author

optout21 commented Sep 6, 2023

I plan to create a PR to address this
As interactive TX negotiation is not yet used, this can wait; it could be addressed with (first part of) Dual Funded or Splicing support.

@optout21
Copy link
Contributor Author

optout21 commented Sep 8, 2023

Change can be found in branch: https://github.com/optout21/rust-lightning/tree/tx-sigs-field
Relevant diff:

--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -511,6 +511,8 @@ pub struct TxSignatures {
        pub tx_hash: Txid,
        /// The list of witnesses
        pub witnesses: Vec<Witness>,
+       /// Optional signature for shared funding outpoint input (signed by both parties)
+       pub tlvs: Option<Signature>,
 }
 
 /// A tx_init_rbf message which initiates a replacement of the transaction after it's been
@@ -1747,7 +1749,9 @@ impl_writeable_msg!(TxSignatures, {
        channel_id,
        tx_hash,
        witnesses,
-}, {});
+}, {
+       (0, tlvs, option),
+});
 
 impl_writeable_msg!(TxInitRbf, {
        channel_id,

@wpaulino
Copy link
Contributor

wpaulino commented Sep 8, 2023

Thanks! I think this was an ongoing change at the time we added the messages initially. It looks like we may be close to having the spec PRs merged, so let's wait a bit longer before opening a PR to see if anything else comes up?

@optout21
Copy link
Contributor Author

optout21 commented Sep 8, 2023

Unfortunately the spec PRs are a bit outdated, I think at this moment the existing implementations are a better source of truth (the impl. authors and spec authors are mostly the same).
I think this small change is not important now, but should be done whenever the first (part) of dual funding or splicing logic is being added to LDK, regardless of the state of the spec.

@dunxen dunxen self-assigned this Nov 21, 2023
@optout21
Copy link
Contributor Author

optout21 commented Sep 5, 2024

Update: this has been resolved in the meantime, see field shared_input_signature in https://github.com/lightningdevkit/rust-lightning/blob/main/lightning/src/ln/msgs.rs
Some splicing-specific changes are still outstanding, related: #3293
Closing.

@optout21 optout21 closed this as completed Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants