Don't FC when a channel_update has a bogus htlc_minimum_msat
#2611
Conversation
If a peer sends us a `channel_update` for our own channel with an `htlc_minimum_msat` which is more than the channel's amount, that's dumb, but there's no reason to force-close the channel. We don't even use the field. Here we simply drop the unnecessary check.
Codecov ReportAll modified lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2611 +/- ##
==========================================
+ Coverage 88.98% 89.01% +0.02%
==========================================
Files 113 113
Lines 86291 86288 -3
Branches 86291 86288 -3
==========================================
+ Hits 76787 76805 +18
+ Misses 7267 7250 -17
+ Partials 2237 2233 -4
☔ View full report in Codecov by Sentry. |
| @@ -5538,9 +5538,6 @@ impl<SP: Deref> Channel<SP> where | |||
| } | |||
|
|
|||
| pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> { | |||
| if msg.contents.htlc_minimum_msat >= self.context.channel_value_satoshis * 1000 { | |||
| return Err(ChannelError::Close("Minimum htlc value is greater than channel value".to_string())); | |||
There was a problem hiding this comment.
Maybe should log it still?
There was a problem hiding this comment.
Even better, I'll just log the full message.
There was a problem hiding this comment.
We log the update, but don't log that it was bogus htlc_minimum_msat?
There was a problem hiding this comment.
True, but we also don't do anything with it so its not really an error that's interesting. We should maybe consider not ignoring it, but the spec isn't super clear on if we have to honor it anyway. In any case, the full message with all its fields is probably more interesting than something specific to one field.
If a peer sends us a
channel_updatefor our own channel with anhtlc_minimum_msatwhich is more than the channel's amount, that's dumb, but there's no reason to force-close the channel. We don't even use the field.Here we simply drop the unnecessary check.
I had a channel that hit this. Waiting to hear back from the peer as to why or how they generated such nonsense, but there's no reason for the FC.