Conversation
We use a full pattern matching on the `when` to prevent future omissions.
Use exhaustive match.
This is inspired from eclair, but changes are kept minimal. We don't support the `RemoteLate` case. Eclair had a full rework in ACINQ/eclair#2036, that separated concerns better and got completely rid of exceptions.
This is the equivalent of ACINQ/eclair#2036. Notably we remove the `RevocationSyncError` exception, and in case the peer has an outdated commitment we stay in stand by, waiting for their `error` before force-closing.
pm47
commented
Apr 16, 2024
t-bast
previously approved these changes
Apr 17, 2024
Member
t-bast
left a comment
t-bast
left a comment
There was a problem hiding this comment.
I've compared this to both eclair and the previous version of lightning-kmp, it looks good 👍
t-bast
approved these changes
Apr 17, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR does two things:
channel_reestablishin stateSyncing(Shutdown)The two middle commits are intermediate steps and may not be necessarily useful to review. I recommend checking the first commit, then the last 3 commits.
The full reestablish rework is sensitive stuff (a bug here could cause mass force close of phoenix channels). However, the code is a litteral copy paste of eclair, ported to Kotlin. Tests have not been modified, except two of them, which is due to a change in behavior: we are not pro-actively force closing channels when we detect that our peer is behind. Instead, we stay in stand-by and force-close when we receive their error.