Add support for splices #430
Merged
Conversation
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
t-bast
reviewed
Mar 31, 2023
src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelData.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/wire/LightningMessages.kt
Outdated
Show resolved
Hide resolved
t-bast
reviewed
Apr 3, 2023
There was a problem hiding this comment.
I focused on the first commits that implement the splice protocol, without looking at the interaction with payments: can we separate this to another PR? I believe that all the commits starting at a15eac1 can be extracted to another future PR to allow merging the splice protocol to master early, as it doesn't have any negative impact on the rest of the codebase.
src/commonMain/kotlin/fr/acinq/lightning/channel/Commitments.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/channel/Commitments.kt
Outdated
Show resolved
Hide resolved
src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt
Outdated
Show resolved
Hide resolved
Replace `makeFirstCommitTxs` by `makeCommitTxsWithoutHtlcs` which generates a pair of commitments for arbitrary `fundingTxIndex` and `commitmentIndex`.
We introduce a new model for channel commands, that allows for better typing, and is suitable with `replyTo` pattern.
An important side-effect is that we do not eagerly prune initial funding txs as soon as one of the rbf confirms. We could, but it is better to be consistent with the splice case, where we must wait for the counterparty to send their `splice_locked` before pruning.
We still use the `fundingTxId` internally.
- if we have a channel in `Normal`, we use it - if we have no channels, or if none of them will ever be suitable for splice, we request a new channel - in all other cases we do nothing and wait
To compute the remote reserve, we discard the local contribution. It's okay if they go below reserve because we added capacity to the channel with a splice-in.
- add comments in complex parts of the code - match whole outpoint instead of just txid when possible - rename `Splice.Response.Success` to `Splice.Response.Created` - send `splice_locked` in the non-zero-conf case - correctly overwrite splice status when disconnecting - add tests
t-bast
approved these changes
Apr 11, 2023
|
🥁 🥁 🥁 |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Most commits are preparatory or intermediate steps, most of the work happens in:
Tests based on flows in https://gist.github.com/t-bast/1ac31f4e27734a10c5b9847d06db8d86 need to be added.