Track message type for messages we are expecting to receive

[wpaulino]

This was previously not considered because we only ever expected to receive a specific message at a time.

This will change with quiescence, as we may be waiting for the counterparty's stfu in response to ours, while at the same time waiting for a counterparty's revoke_and_ack to irrevocably commit any pending updates. If this were unchanged, we could potentially overwrite our expectation to receive stfu with the revoke_and_ack and the channel would become stuck (until a disconnect) if the counterparty chooses to never send the stfu.

[codecov]

Codecov Report

Attention: Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.50%. Comparing base (569f906) to head (4d23c84).
Report is 69 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	94.28%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3573      +/-   ##
==========================================
- Coverage   88.54%   88.50%   -0.04%     
==========================================
  Files         149      149              
  Lines      114459   114491      +32     
  Branches   114459   114491      +32     
==========================================
- Hits       101345   101331      -14     
- Misses      10618    10651      +33     
- Partials     2496     2509      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

Wait, I'm confused, how does the protocol work? So we send a stfu, and the counterparty might send something in response (eg later an RAA). That somehow cancels the stfu request we'd sent and returns the channel to normal operation, but only if we weren't expecting it? That just sounds like a horribly broken protocol, there's no way we're not gonna end up with bugs here.

[wpaulino]

This is only a shortcoming because of the current implementation of expecting messages, not because of the protocol. We're only allowed to send stfu once our outgoing updates have been irrevocably committed on both sides. When we send stfu, we want to make sure we receive theirs as well to enter quiescence in a timely manner, otherwise we can't propose any new updates without reestablishing the channel. While we're waiting to receive theirs, we may also be waiting on a revoke_and_ack due to an update they've made, such that they can send stfu eventually. If we're waiting on a stfu, then the revoke_and_ack (which overwrites waiting for the stfu), AND the counterparty chooses to ignore our stfu for whatever reason without sending theirs, LDK wouldn't enforce the disconnect without these changes.

<- update_add
<- commit_sig
-> stfu
// LDK waiting for counterparty stfu
-> revoke_and_ack
-> commit_sig
// LDK waiting for counterparty revoke_and_ack
<- revoke_and_ack
// counterparty is allowed to send stfu now, but chooses not to
// LDK is no longer waiting for any other messages because it received the revoke_and_ack, and now can't make any new updates without channel reestablishment because it already sent stfu

[TheBlueMatt]

This is only a shortcoming because of the current implementation of expecting messages, not because of the protocol.

I mean this is still introducing a major change to the protocol, no? Up until now there's been no requirement that we track whether we're expecting a message at all, let alone what type. Now the protocol requires that we do so.

the revoke_and_ack (which overwrites waiting for the stfu)

So I'm still confused by this - if we send CS followed by stfu, then they respond with RAA, are we allowed to send new updates (because the RAA "overwrote" the stfu) or not (because the peer was waiting for a CS+RAA from us when they received the stfu)?

the counterparty chooses to ignore our stfu for whatever reason without sending theirs, LDK wouldn't enforce the disconnect without these changes.

That just sounds like a bug on the counterparty's end? Can we just time out a stfu by disconnecting with a warning instead of tracking here?