Resume reading after processing unknown messages

[pm47]

If the current chunk of data read from the TCP connection only contains unknown messages (in particular, could be only one isolated unknown message on an otherwise idle connection), we never resumed reading on the connection.

This means all subsequent messages, including pings/pongs, won't be read, which is why the most visible effect is disconnecting due to no response to ping.

Related to ElementsProject/lightning#5347.

Reported by @wtogami.

[codecov-commenter]

Codecov Report

Merging #2332 (f784690) into master (af79f44) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2332      +/-   ##
==========================================
+ Coverage   84.71%   84.75%   +0.03%     
==========================================
  Files         194      194              
  Lines       14632    14637       +5     
  Branches      621      612       -9     
==========================================
+ Hits        12395    12405      +10     
+ Misses       2237     2232       -5     

Impacted Files	Coverage Δ
...cala/fr/acinq/eclair/crypto/TransportHandler.scala	91.00% <100.00%> (+0.78%)	⬆️
...cinq/eclair/channel/publish/MempoolTxMonitor.scala	88.23% <0.00%> (-2.36%)	⬇️
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala	88.08% <0.00%> (+0.11%)	⬆️
...main/scala/fr/acinq/eclair/io/PeerConnection.scala	86.34% <0.00%> (+0.36%)	⬆️
...clair/channel/publish/ReplaceableTxPublisher.scala	75.48% <0.00%> (+1.29%)	⬆️
...cinq/eclair/payment/receive/MultiPartHandler.scala	95.03% <0.00%> (+1.41%)	⬆️

[t-bast]

Good catch! What's surprising is that we use a lightningMessageCodecWithFallback so the codec should always succeed even for messages we don't understand, unless they're not using the lightning message format at all - which should also be a bug on the sender side as well, shouldn't it (but we need to fix the bug on our side of course)?

As detailed in the linked issue, this is because cln sends us PeerSwap messages, which are currently not spec-compliant...I'm surprised they would send us such messages since we don't have a feature bit activated for it?

[pm47]

What's surprising is that we use a lightningMessageCodecWithFallback so the codec should always succeed even for messages we don't understand, unless they're not using the lightning message format at all

Thanks, I get now why the error I got when reproducing locally (unknown discriminator) was different from the one on the node (cannot acquire 252176 bits from a vector that contains 440 bits): I was using lightningMessageCodec, not lightningMessageCodecWithFallback.

But there is no "standard lightning message format" though, the UnknownMessage has been introduced by Anton for his plugins. We are actually defining our own standard there, and will never get clean errors for all other unknown messages...

[t-bast]

It does make more sense like this, LGTM