Don't fail read b/c invalid chan upd. and node ann

lightning/src/ln/msgs.rs

@@ -2149,8 +2149,6 @@ mod tests {
 		do_encoding_channel_update(true, false, true);
 		do_encoding_channel_update(false, true, false);
 		do_encoding_channel_update(false, true, true);
-		do_encoding_channel_update(false, false, false);
-		do_encoding_channel_update(false, false, true);
 		do_encoding_channel_update(true, true, false);
 		do_encoding_channel_update(true, true, true);
 	}

lightning/src/routing/gossip.rs

@@ -28,7 +28,7 @@ use ln::msgs::{DecodeError, ErrorAction, Init, LightningError, RoutingMessageHan
 use ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, GossipTimestampFilter};
 use ln::msgs::{QueryChannelRange, ReplyChannelRange, QueryShortChannelIds, ReplyShortChannelIdsEnd};
 use ln::msgs;
-use util::ser::{Readable, ReadableArgs, Writeable, Writer};
+use util::ser::{Readable, ReadableArgs, Writeable, Writer, MaybeReadable};
 use util::logger::{Logger, Level};
 use util::events::{Event, EventHandler, MessageSendEvent, MessageSendEventsProvider};
 use util::scid_utils::{block_from_scid, scid_from_parts, MAX_SCID_BLOCK};
@@ -628,15 +628,56 @@ impl fmt::Display for ChannelUpdateInfo {
 	}
 }
 
-impl_writeable_tlv_based!(ChannelUpdateInfo, {
-	(0, last_update, required),
-	(2, enabled, required),
-	(4, cltv_expiry_delta, required),
-	(6, htlc_minimum_msat, required),
-	(8, htlc_maximum_msat, required),
-	(10, fees, required),
-	(12, last_update_message, required),
-});
+impl Writeable for ChannelUpdateInfo {
+	fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+		write_tlv_fields!(writer, {
+			(0, self.last_update, required),
+			(2, self.enabled, required),
+			(4, self.cltv_expiry_delta, required),
+			(6, self.htlc_minimum_msat, required),
+			(8, Some(self.htlc_maximum_msat), required),
+			(10, self.fees, required),
+			(12, self.last_update_message, required),
+		});
+		Ok(())
+	}
+}
+
+impl Readable for ChannelUpdateInfo {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		init_tlv_field_var!(last_update, required);
+		init_tlv_field_var!(enabled, required);
+		init_tlv_field_var!(cltv_expiry_delta, required);
+		init_tlv_field_var!(htlc_minimum_msat, required);
+		init_tlv_field_var!(htlc_maximum_msat, option);
+		init_tlv_field_var!(fees, required);
+		init_tlv_field_var!(last_update_message, required);
+
+		read_tlv_fields!(reader, {
+			(0, last_update, required),
+			(2, enabled, required),
+			(4, cltv_expiry_delta, required),
+			(6, htlc_minimum_msat, required),
+			(8, htlc_maximum_msat, required),
+			(10, fees, required),
+			(12, last_update_message, required)
+		});
+
+		if let Some(htlc_maximum_msat) = htlc_maximum_msat {
+			Ok(ChannelUpdateInfo {
+				last_update: init_tlv_based_struct_field!(last_update, required),
+				enabled: init_tlv_based_struct_field!(enabled, required),
+				cltv_expiry_delta: init_tlv_based_struct_field!(cltv_expiry_delta, required),
+				htlc_minimum_msat: init_tlv_based_struct_field!(htlc_minimum_msat, required),
+				htlc_maximum_msat,
+				fees: init_tlv_based_struct_field!(fees, required),
+				last_update_message: init_tlv_based_struct_field!(last_update_message, required),
+			})
+		} else {
+			Err(DecodeError::InvalidValue)
+		}
+	}
+}
 
 #[derive(Clone, Debug, PartialEq)]
 /// Details about a channel (both directions).
@@ -715,16 +756,72 @@ impl fmt::Display for ChannelInfo {
 	}
 }
 
-impl_writeable_tlv_based!(ChannelInfo, {
-	(0, features, required),
-	(1, announcement_received_time, (default_value, 0)),
-	(2, node_one, required),
-	(4, one_to_two, required),
-	(6, node_two, required),
-	(8, two_to_one, required),
-	(10, capacity_sats, required),
-	(12, announcement_message, required),
-});
+impl Writeable for ChannelInfo {
+	fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+		write_tlv_fields!(writer, {
+			(0, self.features, required),
+			(1, self.announcement_received_time, (default_value, 0)),
+			(2, self.node_one, required),
+			(4, self.one_to_two, required),
+			(6, self.node_two, required),
+			(8, self.two_to_one, required),
+			(10, self.capacity_sats, required),
+			(12, self.announcement_message, required),
+		});
+		Ok(())
+	}
+}
+
+// A wrapper allowing for the optional deseralization of ChannelUpdateInfo. Utilizing this is
+// necessary to maintain backwards compatibility with previous serializations of `ChannelUpdateInfo`
+// that may have no `htlc_maximum_msat` field set. In case the field is absent, we simply ignore
+// the error and continue reading the `ChannelInfo`. Hopefully, we'll then eventually receive newer
+// channel updates via the gossip network.
+struct ChannelUpdateInfoDeserWrapper(Option<ChannelUpdateInfo>);
+
+impl MaybeReadable for ChannelUpdateInfoDeserWrapper {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
+		match ::util::ser::Readable::read(reader) {
+			Ok(channel_update_option) => Ok(Some(Self(channel_update_option))),
+			Err(DecodeError::ShortRead) => Ok(None),
+			Err(err) => Err(err),
+		}
+	}
+}
+
+impl Readable for ChannelInfo {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		init_tlv_field_var!(features, required);
+		init_tlv_field_var!(announcement_received_time, (default_value, 0));
+		init_tlv_field_var!(node_one, required);
+		let mut one_to_two_wrap: Option<ChannelUpdateInfoDeserWrapper> = None;
+		init_tlv_field_var!(node_two, required);
+		let mut two_to_one_wrap: Option<ChannelUpdateInfoDeserWrapper> = None;
+		init_tlv_field_var!(capacity_sats, required);
+		init_tlv_field_var!(announcement_message, required);
+		read_tlv_fields!(reader, {
+			(0, features, required),
+			(1, announcement_received_time, (default_value, 0)),
+			(2, node_one, required),
+			(4, one_to_two_wrap, ignorable),
+			(6, node_two, required),
+			(8, two_to_one_wrap, ignorable),
+			(10, capacity_sats, required),
+			(12, announcement_message, required),
+		});
+
+		Ok(ChannelInfo {
+			features: init_tlv_based_struct_field!(features, required),
+			node_one: init_tlv_based_struct_field!(node_one, required),
+			one_to_two: one_to_two_wrap.map(|w| w.0).unwrap_or(None),
+			node_two: init_tlv_based_struct_field!(node_two, required),
+			two_to_one: two_to_one_wrap.map(|w| w.0).unwrap_or(None),
+			capacity_sats: init_tlv_based_struct_field!(capacity_sats, required),
+			announcement_message: init_tlv_based_struct_field!(announcement_message, required),
+			announcement_received_time: init_tlv_based_struct_field!(announcement_received_time, (default_value, 0)),
+		})
+	}
+}
 
 /// A wrapper around [`ChannelInfo`] representing information about the channel as directed from a
 /// source node to a target node.
@@ -993,11 +1090,51 @@ impl fmt::Display for NodeInfo {
 	}
 }
 
-impl_writeable_tlv_based!(NodeInfo, {
-	(0, lowest_inbound_channel_fees, option),
-	(2, announcement_info, option),
-	(4, channels, vec_type),
-});
+impl Writeable for NodeInfo {
+	fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+		write_tlv_fields!(writer, {
+			(0, self.lowest_inbound_channel_fees, option),
+			(2, self.announcement_info, option),
+			(4, self.channels, vec_type),
+		});
+		Ok(())
+	}
+}
+
+// A wrapper allowing for the optional deseralization of `NodeAnnouncementInfo`. Utilizing this is
+// necessary to maintain compatibility with previous serializations of `NodeAnnouncementInfo` that have an
+// invalid hostname set. In this case, we simply ignore the error and continue reading the `NodeInfo`.
+struct NodeAnnouncementInfoDeserWrapper(NodeAnnouncementInfo);
+
+impl MaybeReadable for NodeAnnouncementInfoDeserWrapper {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
+		match ::util::ser::Readable::read(reader) {
+			Ok(node_announcement) => Ok(Some(Self(node_announcement))),
+			Err(DecodeError::ShortRead) => Ok(None),
+			Err(err) => Err(err),
+		}
+	}
+}
+
+impl Readable for NodeInfo {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		init_tlv_field_var!(lowest_inbound_channel_fees, option);
+		let mut announcement_info_wrap: Option<NodeAnnouncementInfoDeserWrapper> = None;
+		init_tlv_field_var!(channels, vec_type);
+
+		read_tlv_fields!(reader, {
+			(0, lowest_inbound_channel_fees, option),
+			(2, announcement_info_wrap, ignorable),
+			(4, channels, vec_type),
+		});
+
+		Ok(NodeInfo {
+			lowest_inbound_channel_fees: init_tlv_based_struct_field!(lowest_inbound_channel_fees, option),
+			announcement_info: announcement_info_wrap.map(|w| w.0),
+			channels: init_tlv_based_struct_field!(channels, vec_type),
+		})
+	}
+}
 
 const SERIALIZATION_VERSION: u8 = 1;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
@@ -1676,7 +1813,7 @@ mod tests {
 	use chain;
 	use ln::PaymentHash;
 	use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
-	use routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate, NodeAlias, MAX_EXCESS_BYTES_FOR_RELAY};
+	use routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate, NodeAlias, MAX_EXCESS_BYTES_FOR_RELAY, NodeId, RoutingFees, ChannelUpdateInfo, ChannelInfo};
 	use ln::msgs::{Init, RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement,
 		UnsignedChannelAnnouncement, ChannelAnnouncement, UnsignedChannelUpdate, ChannelUpdate,
 		ReplyChannelRange, QueryChannelRange, QueryShortChannelIds, MAX_VALUE_MSAT};
@@ -2802,6 +2939,71 @@ mod tests {
 		assert_eq!(format_bytes_alias(b"\xFFI <heart>\0LDK!"), "\u{FFFD}I <heart>");
 		assert_eq!(format_bytes_alias(b"\xFFI <heart>\tLDK!"), "\u{FFFD}I <heart>\u{FFFD}LDK!");
 	}
+
+	#[test]
+	fn written_channel_info_is_readable() {
+		let chanmon_cfgs = ::ln::functional_test_utils::create_chanmon_cfgs(2);
+		let node_cfgs = ::ln::functional_test_utils::create_node_cfgs(2, &chanmon_cfgs);
+		let node_chanmgrs = ::ln::functional_test_utils::create_node_chanmgrs(2, &node_cfgs, &[None, None, None, None]);
+		let nodes = ::ln::functional_test_utils::create_network(2, &node_cfgs, &node_chanmgrs);
+
+		// First make sure we can encode/decode ChannelUpdateInfo.
+		let chan_update_info = ChannelUpdateInfo {
+			last_update: 23,
+			enabled: true,
+			cltv_expiry_delta: 42,
+			htlc_minimum_msat: 1234,
+			htlc_maximum_msat: 5678,
+			fees: RoutingFees { base_msat: 9, proportional_millionths: 10 },
+			last_update_message: None,
+		};
+
+		let mut buf: Vec<u8> = Vec::new();
+		assert!(chan_update_info.write(&mut buf).is_ok());
+
+		let read_chan_update_info_result: Option<ChannelUpdateInfo> = ::util::ser::MaybeReadable::read(&mut buf.as_slice()).unwrap();
+		if let Some(read_chan_update_info) = read_chan_update_info_result {
+			assert_eq!(chan_update_info, read_chan_update_info);
+		} else {
+			panic!();
+		}
+
+		// Then check we can encode/decode ChannelInfo without ChannelUpdateInfo fields present.
+		let chan_info_none_updates = ChannelInfo {
+			features: ChannelFeatures::known(),
+			node_one: NodeId::from_pubkey(&nodes[0].node.get_our_node_id()),
+			one_to_two: None,
+			node_two: NodeId::from_pubkey(&nodes[1].node.get_our_node_id()),
+			two_to_one: None,
+			capacity_sats: None,
+			announcement_message: None,
+			announcement_received_time: 87654,
+		};
+
+		let mut buf: Vec<u8> = Vec::new();
+		assert!(chan_info_none_updates.write(&mut buf).is_ok());
+
+		let read_chan_info: ChannelInfo = ::util::ser::Readable::read(&mut buf.as_slice()).unwrap();
+		assert_eq!(chan_info_none_updates, read_chan_info);
+
+		// Finally check we can encode/decode ChannelInfo with ChannelUpdateInfo fields present.
+		let chan_info_some_updates = ChannelInfo {
+			features: ChannelFeatures::known(),
+			node_one: NodeId::from_pubkey(&nodes[0].node.get_our_node_id()),
+			one_to_two: Some(chan_update_info.clone()),
+			node_two: NodeId::from_pubkey(&nodes[1].node.get_our_node_id()),
+			two_to_one: Some(chan_update_info.clone()),
+			capacity_sats: None,
+			announcement_message: None,
+			announcement_received_time: 87654,
+		};
+
+		let mut buf: Vec<u8> = Vec::new();
+		assert!(chan_info_some_updates.write(&mut buf).is_ok());
+
+		let read_chan_info: ChannelInfo = ::util::ser::Readable::read(&mut buf.as_slice()).unwrap();
+		assert_eq!(chan_info_some_updates, read_chan_info);
+	}
 }
 
 #[cfg(all(test, feature = "_bench_unstable"))]