Remove now-unused NetworkUpdate::ChannelUpdateMessage.

See previous commit.

Author: valentinewallace

lightning/src/ln/functional_test_utils.rs

@@ -2453,13 +2453,6 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
 			if let Some(chan_closed) = conditions.expected_blamed_chan_closed {
 				if let PathFailure::OnPath { network_update: Some(upd) } = failure {
 					match upd {
-						NetworkUpdate::ChannelUpdateMessage { ref msg } if !chan_closed => {
-							if let Some(scid) = conditions.expected_blamed_scid {
-								assert_eq!(msg.contents.short_channel_id, scid);
-							}
-							const CHAN_DISABLED_FLAG: u8 = 2;
-							assert_eq!(msg.contents.flags & CHAN_DISABLED_FLAG, 0);
-						},
 						NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 							if let Some(scid) = conditions.expected_blamed_scid {
 								assert_eq!(*short_channel_id, scid);

lightning/src/ln/onion_route_tests.rs

@@ -180,11 +180,6 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(
 		if expected_channel_update.is_some() {
 			match network_update {
 				Some(update) => match update {
-					&NetworkUpdate::ChannelUpdateMessage { .. } => {
-						if let NetworkUpdate::ChannelUpdateMessage { .. } = expected_channel_update.unwrap() {} else {
-							panic!("channel_update not found!");
-						}
-					},
 					&NetworkUpdate::ChannelFailure { ref short_channel_id, ref is_permanent } => {
 						if let NetworkUpdate::ChannelFailure { short_channel_id: ref expected_short_channel_id, is_permanent: ref expected_is_permanent } = expected_channel_update.unwrap() {
 							assert!(*short_channel_id == *expected_short_channel_id);

lightning/src/routing/gossip.rs

@@ -31,7 +31,7 @@ use crate::routing::utxo::{self, UtxoLookup, UtxoResolver};
 use crate::util::indexed_map::{Entry as IndexedMapEntry, IndexedMap};
 use crate::util::logger::{Level, Logger};
 use crate::util::scid_utils::{block_from_scid, scid_from_parts, MAX_SCID_BLOCK};
-use crate::util::ser::{MaybeReadable, Readable, ReadableArgs, Writeable, Writer};
+use crate::util::ser::{MaybeReadable, Readable, ReadableArgs, RequiredWrapper, Writeable, Writer};
 use crate::util::string::PrintableString;
 
 use crate::io;
@@ -218,12 +218,6 @@ pub struct ReadOnlyNetworkGraph<'a> {
 /// [BOLT #4]: https://github.com/lightning/bolts/blob/master/04-onion-routing.md
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum NetworkUpdate {
-	/// An error indicating a `channel_update` messages should be applied via
-	/// [`NetworkGraph::update_channel`].
-	ChannelUpdateMessage {
-		/// The update to apply via [`NetworkGraph::update_channel`].
-		msg: ChannelUpdate,
-	},
 	/// An error indicating that a channel failed to route a payment, which should be applied via
 	/// [`NetworkGraph::channel_failed_permanent`] if permanent.
 	ChannelFailure {
@@ -244,19 +238,69 @@ pub enum NetworkUpdate {
 	}
 }
 
-impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate,
-	(0, ChannelUpdateMessage) => {
-		(0, msg, required),
-	},
-	(2, ChannelFailure) => {
-		(0, short_channel_id, required),
-		(2, is_permanent, required),
-	},
-	(4, NodeFailure) => {
-		(0, node_id, required),
-		(2, is_permanent, required),
-	},
-);
+impl Writeable for NetworkUpdate {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+		match self {
+			Self::ChannelFailure { short_channel_id, is_permanent } => {
+				2u8.write(writer)?;
+				write_tlv_fields!(writer, {
+					(0, short_channel_id, required),
+					(2, is_permanent, required),
+				});
+			},
+			Self::NodeFailure { node_id, is_permanent } => {
+				4u8.write(writer)?;
+				write_tlv_fields!(writer, {
+					(0, node_id, required),
+					(2, is_permanent, required),
+				});
+			}
+		}
+		Ok(())
+	}
+}
+
+impl MaybeReadable for NetworkUpdate {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
+		let id: u8 = Readable::read(reader)?;
+		match id {
+			0 => {
+				// 0 was previously used for network updates containing a channel update, subsequently
+				// removed in LDK version 0.0.124.
+				let mut msg: RequiredWrapper<ChannelUpdate> = RequiredWrapper(None);
+				read_tlv_fields!(reader, {
+					(0, msg, required),
+				});
+				Ok(Some(Self::ChannelFailure {
+					short_channel_id: msg.0.unwrap().contents.short_channel_id,
+					is_permanent: false
+				}))
+			},
+			2 => {
+				_init_and_read_len_prefixed_tlv_fields!(reader, {
+					(0, short_channel_id, required),
+					(2, is_permanent, required),
+				});
+				Ok(Some(Self::ChannelFailure {
+					short_channel_id: short_channel_id.0.unwrap(),
+					is_permanent: is_permanent.0.unwrap(),
+				}))
+			},
+			4 => {
+				_init_and_read_len_prefixed_tlv_fields!(reader, {
+					(0, node_id, required),
+					(2, is_permanent, required),
+				});
+				Ok(Some(Self::NodeFailure {
+					node_id: node_id.0.unwrap(),
+					is_permanent: is_permanent.0.unwrap(),
+				}))
+			}
+			t if t % 2 == 0 => Err(DecodeError::UnknownRequiredFeature),
+			_ => Ok(None),
+		}
+	}
+}
 
 /// Receives and validates network updates from peers,
 /// stores authentic and relevant data as a network graph.
@@ -353,19 +397,10 @@ where U::Target: UtxoLookup, L::Target: Logger
 
 impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 	/// Handles any network updates originating from [`Event`]s.
-	//
-	/// Note that this will skip applying any [`NetworkUpdate::ChannelUpdateMessage`] to avoid
-	/// leaking possibly identifying information of the sender to the public network.
 	///
 	/// [`Event`]: crate::events::Event
 	pub fn handle_network_update(&self, network_update: &NetworkUpdate) {
 		match *network_update {
-			NetworkUpdate::ChannelUpdateMessage { ref msg } => {
-				let short_channel_id = msg.contents.short_channel_id;
-				let is_enabled = msg.contents.flags & (1 << 1) != (1 << 1);
-				let status = if is_enabled { "enabled" } else { "disabled" };
-				log_debug!(self.logger, "Skipping application of a channel update from a payment failure. Channel {} is {}.", short_channel_id, status);
-			},
 			NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 				if is_permanent {
 					log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
@@ -2575,23 +2610,18 @@ pub(crate) mod tests {
 
 		let short_channel_id;
 		{
-			// Check we won't apply an update via `handle_network_update` for privacy reasons, but
-			// can continue fine if we manually apply it.
+			// Check that we can manually apply a channel update.
 			let valid_channel_announcement = get_signed_channel_announcement(|_| {}, node_1_privkey, node_2_privkey, &secp_ctx);
 			short_channel_id = valid_channel_announcement.contents.short_channel_id;
 			let chain_source: Option<&test_utils::TestChainSource> = None;
 			assert!(network_graph.update_channel_from_announcement(&valid_channel_announcement, &chain_source).is_ok());
 			assert!(network_graph.read_only().channels().get(&short_channel_id).is_some());
 
 			let valid_channel_update = get_signed_channel_update(|_| {}, node_1_privkey, &secp_ctx);
-			assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none());
-
-			network_graph.handle_network_update(&NetworkUpdate::ChannelUpdateMessage {
-				msg: valid_channel_update.clone(),
-			});
 
 			assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none());
 			network_graph.update_channel(&valid_channel_update).unwrap();
+			assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
 		}
 
 		// Non-permanent failure doesn't touch the channel at all