(Re-)add handling for ChannelUpdate::message_flags

[TheBlueMatt]

When the htlc_maximum_msat field was made mandatory in
ChannelUpdate (in b0e8b73) we
started ignoring the message_flags field entirely and always
writing 1. The comment updates indicated that the message_flags
field was deprecated, but this is not true - only the
htlc_maximum_msat indicator bit was deprecated, requiring it to
be 1.

If a node creates a channel_update with message_flags bits set
other than the low bit, this will cause us to spuriously reject
the message with an invalid signature error as we will check the
message against the wrong hash.

With today's current spec this is totally fine - the only other bit
defined for message_flags is the dont_forward bit, which when
set indicates we shouldn't accept the message into our gossip store
anyway (though this may lead to spurious warning messages being
sent to peers). However, in the future this may mean we start
rejecting valid channel_updates if new bits are defiend.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 97.76358% with 7 lines in your changes missing coverage. Please review.

Project coverage is 90.23%. Comparing base (88e1b56) to head (162b81f).
Report is 11 commits behind head on main.

Files	Patch %	Lines
lightning/src/ln/msgs.rs	46.15%	0 Missing and 7 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3144      +/-   ##
==========================================
+ Coverage   89.83%   90.23%   +0.39%     
==========================================
  Files         121      121              
  Lines       98900   103329    +4429     
  Branches    98900   103329    +4429     
==========================================
+ Hits        88847    93237    +4390     
- Misses       7457     7462       +5     
- Partials     2596     2630      +34     

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

[tnull]

Hmm, good catch! Not sure why I was under the impression the whole field has been deprecated back then.

LGTM

[TheBlueMatt]

Address @tnull's nit and squashed:

$ git diff-tree -U1 0fa0c2dc 5b48f3ff
diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 6701a1806..0cc8f1323 100644
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -3505,3 +3505,3 @@ mod tests {
 			timestamp: 20190119,
-			message_flags: 0,
+			message_flags: 1, // Only must_be_one
 			channel_flags: if direction { 1 } else { 0 } | if disable { 1 << 1 } else { 0 },

[valentinewallace]

Fuzz is failing

[TheBlueMatt]

Ugh, had to update the log regex:

$ git diff-tree -U1 5b48f3ff 162b81fb
diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index a2ae07a21..86f4ef4de 100644
--- a/fuzz/src/full_stack.rs
+++ b/fuzz/src/full_stack.rs
@@ -1781,3 +1781,3 @@ mod tests {
 		assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Sending message to all peers except Some(PublicKey(0000000000000000000000000000000000000000000000000000000000000002ff00000000000000000000000000000000000000000000000000000000000002)) or the announced channel's counterparties: ChannelAnnouncement { node_signature_1: 3026020200b202200303030303030303030303030303030303030303030303030303030303030303, node_signature_2: 3026020200b202200202020202020202020202020202020202020202020202020202020202020202, bitcoin_signature_1: 3026020200b202200303030303030303030303030303030303030303030303030303030303030303, bitcoin_signature_2: 3026020200b202200202020202020202020202020202020202020202020202020202020202020202, contents: UnsignedChannelAnnouncement { features: [], chain_hash: 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000, short_channel_id: 42, node_id_1: NodeId(030303030303030303030303030303030303030303030303030303030303030303), node_id_2: NodeId(020202020202020202020202020202020202020202020202020202020202020202), bitcoin_key_1: NodeId(030303030303030303030303030303030303030303030303030303030303030303), bitcoin_key_2: NodeId(020202020202020202020202020202020202020202020202020202020202020202), excess_data: [] } }".to_string())), Some(&1));
-		assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Sending message to all peers except Some(PublicKey(0000000000000000000000000000000000000000000000000000000000000002ff00000000000000000000000000000000000000000000000000000000000002)): ChannelUpdate { signature: 3026020200a602200303030303030303030303030303030303030303030303030303030303030303, contents: UnsignedChannelUpdate { chain_hash: 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000, short_channel_id: 42, timestamp: 44, flags: 0, cltv_expiry_delta: 40, htlc_minimum_msat: 0, htlc_maximum_msat: 100000000, fee_base_msat: 0, fee_proportional_millionths: 0, excess_data: [] } }".to_string())), Some(&1));
+		assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Sending message to all peers except Some(PublicKey(0000000000000000000000000000000000000000000000000000000000000002ff00000000000000000000000000000000000000000000000000000000000002)): ChannelUpdate { signature: 3026020200a602200303030303030303030303030303030303030303030303030303030303030303, contents: UnsignedChannelUpdate { chain_hash: 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000, short_channel_id: 42, timestamp: 44, message_flags: 1, channel_flags: 0, cltv_expiry_delta: 40, htlc_minimum_msat: 0, htlc_maximum_msat: 100000000, fee_base_msat: 0, fee_proportional_millionths: 0, excess_data: [] } }".to_string())), Some(&1));
 		assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Sending message to all peers except Some(PublicKey(0000000000000000000000000000000000000000000000000000000000000002ff00000000000000000000000000000000000000000000000000000000000002)) or the announced node: NodeAnnouncement { signature: 302502012802200303030303030303030303030303030303030303030303030303030303030303, contents: UnsignedNodeAnnouncement { features: [], timestamp: 43, node_id: NodeId(030303030303030303030303030303030303030303030303030303030303030303), rgb: [0, 0, 0], alias: NodeAlias([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]), addresses: [], excess_address_data: [], excess_data: [] } }".to_string())), Some(&1));

[valentinewallace]

nit: s/Thw/The

[valentinewallace]

FYI mutants is failing

[TheBlueMatt]

Yea, mutants is gonna be somewhat informational for now - tells us where our coverage is short but we don't really need to hold up a PR on it.