Implement quiescence protocol #3588
Open
+1,238
−123
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3588 +/- ##
==========================================
- Coverage 88.55% 88.46% -0.10%
==========================================
Files 149 150 +1
Lines 114944 115873 +929
Branches 114944 115873 +929
==========================================
+ Hits 101794 102507 +713
- Misses 10663 10854 +191
- Partials 2487 2512 +25 ☔ View full report in Codecov by Sentry. |
Quiescence is a new protocol feature that allows for channels to undergo "fundamental" changes (i.e., protocol upgrade) while there are no pending updates on either side. Its first use case will be to carry out channel splices, to ensure new HTLC/fee updates are not made while a splice is being negotiated. Each side of the channel is allowed to send a `stfu` message if any of their outbound updates are not pending for either side (i.e., irrevocably committed on both commitment transactions). Once both sides exchange `stfu`, the channel becomes quiescent. A message timeout is enforced during the quiescence handshake to ensure we can eventually re-establish the channel and propose new HTLC/fee updates again. Several new state flags have been added to `ChannelState::ChannelReady` to track the progress of the quiescence handshake. Once the channel becomes quiescent, we transition a new `ChannelState::ChannelQuiescent` state. Since quiescence is not a persistent protocol (it implicitly terminates upon peer disconnection), and updates cannot be made, we chose to introduce a new state completely once quiescent, as several of the `ChannelState::ChannelReady` flags (e.g., `AWAITING_REMOTE_REVOKE`, `MONITOR_UPDATE_IN_PROGRESS`, etc.) are not relevant.
We previously would avoid freeing our holding cells upon a `revoke_and_ack` if a monitor update was in progress, which we checked explicitly. With quiescence, if we've already sent `stfu`, we're not allowed to make further commitment updates, so we must also avoid freeing our holding cells in such cases.
With the introduction of `has_pending_channel_update`, we can now determine whether any messages are owed to irrevocably commit HTLC updates based on the current channel state. We prefer using the channel state, over manually tracking as previously done, to have a single source of truth. We also gain the ability to expect to receive multiple messages at once, which will become relevant with the quiescence protocol, where we may be waiting on a counterparty `revoke_and_ack` and `stfu`.
|
This no longer depends on #3573. I've included an alternative approach here that @TheBlueMatt and I discussed offline. |
Since new updates are not allowed during quiescence (local updates enter the holding cell), we want to ensure quiescence eventually terminates if the handshake takes too long or our counterparty is uncooperative. Disconnecting implicitly terminates quiescence, so the holding cell can be freed upon re-establishing the channel (assuming quiescence is not requested again).
| let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| { | ||
| debug_assert!(false); | ||
| MsgHandleErrInternal::send_err_msg_no_close( | ||
| format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), |
There was a problem hiding this comment.
probably just my reading preference, but we could use format!("Can't find a peer matching the passed counterparty node_id { counterparty_node_id}"),
| // Cleared upon receiving `stfu`. | ||
| || self.context.channel_state.is_local_stfu_sent() | ||
| // Cleared upon receiving a message that triggers the end of quiescence. | ||
| || matches!(self.context.channel_state, ChannelState::ChannelQuiescent) |
There was a problem hiding this comment.
Would it be much more readable to have self.context.channel_state.is_stfu()?
vincenzopalazzo
approved these changes
Feb 7, 2025
| { | ||
| // This is the first tick we've seen after expecting to make forward progress. | ||
| self.context.sent_message_awaiting_response = Some(1); | ||
| debug_assert!(DISCONNECT_PEER_AWAITING_RESPONSE_TICKS > 1); |
There was a problem hiding this comment.
ops I forget the comment here!
This could be removed right? or we need to explicitly silent le linter
error: `debug_assert!(true)` will be optimized out by the compiler
--> lightning/src/ln/channel.rs:7126:4
|
7126 | debug_assert!(DISCONNECT_PEER_AWAITING_RESPONSE_TICKS > 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: remove it
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
= note: `-D clippy::assertions-on-constants` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::assertions_on_constants)]`
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.
Quiescence is a new protocol feature that allows for channels to undergo "fundamental" changes (i.e., protocol upgrade) while there are no pending updates on either side. Its first use case will be to carry out channel splices, to ensure new HTLC/fee updates are not made while a splice is being negotiated.
Each side of the channel is allowed to send a
stfumessage if any of their outbound updates are not pending for either side (i.e., irrevocably committed on both commitment transactions). Once both sides exchangestfu, the channel becomes quiescent. A message timeout is enforced during the quiescence handshake to ensure we can eventually re-establish the channel and propose new HTLC/fee updates again.Several new state flags have been added to
ChannelState::ChannelReadyto track the progress of the quiescence handshake. Once the channel becomes quiescent, we transition a newChannelState::ChannelQuiescentstate. Since quiescence is not a persistent protocol (it implicitly terminates upon peer disconnection), and updates cannot be made, we chose to introduce a new state completely once quiescent, as several of theChannelState::ChannelReadyflags (e.g.,AWAITING_REMOTE_REVOKE,MONITOR_UPDATE_IN_PROGRESS, etc.) are not relevant.