forked from ElementsProject/lightning
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Splicing support #95
Draft
ksedgwic
wants to merge
13
commits into
2023-08-remote-hsmd-v23.08
Choose a base branch
from
2023-08-explore-splicing
base: 2023-08-remote-hsmd-v23.08
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Splicing support #95
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
861d26e
hsmd: Rename hsmd_ready_channel to hsmd_setup_channel
ksedgwic 4cc80e4
splice: signer must be informed of splice params
ksedgwic 08ba2fc
Only run splicing tests when EXPERIMENTAL_SPLICING
ksedgwic fdcd9bc
THROWAWAY: Add print debugging to test_splice
ksedgwic c68d8cb
tests: Skip unsupported tests
ksedgwic f38cc1d
THROWAWAY: avoided a unit test link issue
ksedgwic 4df626b
WIP: relax psbt_has_required_fields, psbt debugging
ksedgwic 19027e5
WORKAROUND: disable psbt_contribs_changed check because it deletes ou…
devrandom a3652ef
tests: skip sign_option_will_fund_offer because (VLS:[#399])
ksedgwic 37ad963
splice: Add hsmd_next_funding_pubkey
ksedgwic 7904073
splice: Add hsmd_check_outpoint and hsmd_lock_outpoint ([#6722])
ksedgwic 9064b14
splice: Add call to hsmd_{check,lock}_outpoint on mutual channel_ready
ksedgwic b2dcf3b
splice: Add call to hsmd_{check,lock}_outpoint on mutual splice_locked
ksedgwic 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 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 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
Oops, something went wrong.
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.
the funding pubkey is allowed to vary in the splice, and we need to be involved in that. I'm wondering where that happens and how we will be informed of the remote one (and asked about the local one).
(note that the funding keys will be the same for all splice candidates)
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.
Are you saying that roundtrips/protocol is missing?
I think we are being informed with this call. Am I missing something?
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.
https://github.com/lightning-signer/c-lightning/blob/2023-08-explore-splicing/channeld/channeld.c#L3536-L3539
That seems to imply it is not changing?
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.
This is where we are telling the other side our key:
https://github.com/lightning-signer/c-lightning/blob/2023-08-explore-splicing/channeld/channeld.c#L3548-L3552
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.
each of the prior is one side, presume there are similar in the other "direction"
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.
the spec allows for it to change (for the whole splice, not per RBF candidate). we should ask the CLN team about this.
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 just scoured the spec looking for where it is allowed to change ... can you point me to it?
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.
https://github.com/lightning/bolts/pull/863/files#diff-ed04ca2c673fd6aabde69389511fa9ee60cb44d6b2ef6c88b549ffaa753d6afeR522
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.
looks like CLN explicitly doesn't support changing them (they send an error to the peer if the key changes), but I assume that will be fixed before release. need to ask them.