lightningdevkit
Support async fetching of commitment point during channel reestablish
HolderCommitmentPoint currently tracks the current and next point used on counterparty commitments, which are unrevoked. When we reestablish a channel, the counterparty sends us the commitment height, along with the corresponding secret, for the state they believe to be the latest. We compare said secret to the derived point we fetch from the signer to know if the peer is being honest.
Since the protocol does not allow peers (assuming no data loss) to be behind the current state by more than one update, we can cache the two latest revoked commitment points alongside HolderCommitmentPoint, such that we no longer need to reach the signer asynchronously when handling channel_reestablish messages throughout the happy path. By doing so, we avoid complexity in needing to pause the state machine (which may also result in needing to stash any update messages from the counterparty) while the signer response is pending.
The only remaining case left to handle is when the counterparty presents a channel_reestablish with a state later than what we know. This can only result in two terminal cases: either they provided a valid commitment secret proving we are behind and we need to panic, or they lied and we force close the channel. This is the only case we choose to handle asynchronously as it's relatively trivial to handle.
👋 Thanks for assigning @TheBlueMatt as a reviewer! I'll wait for their review and will help manage the review process. Once they submit their review, I'll check if a second reviewer would be helpful.
Codecov Report
:x: Patch coverage is 86.39053% with 23 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 89.34%. Comparing base (6749bc6) to head (1f7b249).
:warning: Report is 2 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #4197 +/- ##
==========================================
- Coverage 89.34% 89.34% -0.01%
==========================================
Files 180 180
Lines 138480 138620 +140
Branches 138480 138620 +140
==========================================
+ Hits 123730 123846 +116
- Misses 12129 12149 +20
- Partials 2621 2625 +4
| Flag | Coverage Δ | |
|---|---|---|
| fuzzing | 35.96% <31.81%> (-0.01%) |
:arrow_down: |
| tests | 88.70% <86.39%> (-0.01%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
🔔 1st Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
👋 The first review has been submitted!
Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.
ln::async_signer_tests::test_async_force_close_on_invalid_secret_for_stale_state is failing in CI.
🔔 1st Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 2nd Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 3rd Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 1st Reminder
Hey @valentinewallace! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.