Require directional updates for a DirectionalChannelInfo

We currently construct `DirectedChannelInfo`s for routing before
checking if the given direction has its directional info filled in.
We then always check for directional info before actually deciding
to route over a channel, as otherwise we assume the channel is not
online.

This makes for somewhat redundant checks, and `DirectedCHannelInfo`
isn't, by itself, a very useful API. Because fetching the HTLC-max
or effective channel capacity gives spurious data if no directional
info is available, there's little reason to have that data
available, and so we here check for directional info first. This
effectively merges `DirectionalChannelInfo` and
`DirectionalChannelInfoWithUpdate`.

Author: TheBlueMatt

lightning/src/routing/gossip.rs

@@ -750,7 +750,7 @@ impl ChannelInfo {
 				return None;
 			}
 		};
-		Some((DirectedChannelInfo::new(self, direction), source))
+		direction.map(|dir| (DirectedChannelInfo::new(self, dir), source))
 	}
 
 	/// Returns a [`DirectedChannelInfo`] for the channel directed from the given `source` to a
@@ -765,7 +765,7 @@ impl ChannelInfo {
 				return None;
 			}
 		};
-		Some((DirectedChannelInfo::new(self, direction), target))
+		direction.map(|dir| (DirectedChannelInfo::new(self, dir), target))
 	}
 
 	/// Returns a [`ChannelUpdateInfo`] based on the direction implied by the channel_flag.
@@ -860,29 +860,23 @@ impl Readable for ChannelInfo {
 #[derive(Clone)]
 pub struct DirectedChannelInfo<'a> {
 	channel: &'a ChannelInfo,
-	direction: Option<&'a ChannelUpdateInfo>,
+	direction: &'a ChannelUpdateInfo,
 	htlc_maximum_msat: u64,
 	effective_capacity: EffectiveCapacity,
 }
 
 impl<'a> DirectedChannelInfo<'a> {
 	#[inline]
-	fn new(channel: &'a ChannelInfo, direction: Option<&'a ChannelUpdateInfo>) -> Self {
-		let htlc_maximum_msat = direction.map(|direction| direction.htlc_maximum_msat);
+	fn new(channel: &'a ChannelInfo, direction: &'a ChannelUpdateInfo) -> Self {
+		let mut htlc_maximum_msat = direction.htlc_maximum_msat;
 		let capacity_msat = channel.capacity_sats.map(|capacity_sats| capacity_sats * 1000);
 
-		let (htlc_maximum_msat, effective_capacity) = match (htlc_maximum_msat, capacity_msat) {
-			(Some(amount_msat), Some(capacity_msat)) => {
-				let htlc_maximum_msat = cmp::min(amount_msat, capacity_msat);
-				(htlc_maximum_msat, EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: Some(htlc_maximum_msat) })
+		let effective_capacity = match capacity_msat {
+			Some(capacity_msat) => {
+				htlc_maximum_msat = cmp::min(htlc_maximum_msat, capacity_msat);
+				EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: Some(htlc_maximum_msat) }
 			},
-			(Some(amount_msat), None) => {
-				(amount_msat, EffectiveCapacity::MaximumHTLC { amount_msat })
-			},
-			(None, Some(capacity_msat)) => {
-				(capacity_msat, EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: None })
-			},
-			(None, None) => (EffectiveCapacity::Unknown.as_msat(), EffectiveCapacity::Unknown),
+			None => EffectiveCapacity::MaximumHTLC { amount_msat: htlc_maximum_msat },
 		};
 
 		Self {
@@ -891,12 +885,11 @@ impl<'a> DirectedChannelInfo<'a> {
 	}
 
 	/// Returns information for the channel.
+	#[inline]
 	pub fn channel(&self) -> &'a ChannelInfo { self.channel }
 
-	/// Returns information for the direction.
-	pub fn direction(&self) -> Option<&'a ChannelUpdateInfo> { self.direction }
-
 	/// Returns the maximum HTLC amount allowed over the channel in the direction.
+	#[inline]
 	pub fn htlc_maximum_msat(&self) -> u64 {
 		self.htlc_maximum_msat
 	}
@@ -910,13 +903,9 @@ impl<'a> DirectedChannelInfo<'a> {
 		self.effective_capacity
 	}
 
-	/// Returns `Some` if [`ChannelUpdateInfo`] is available in the direction.
-	pub(super) fn with_update(self) -> Option<DirectedChannelInfoWithUpdate<'a>> {
-		match self.direction {
-			Some(_) => Some(DirectedChannelInfoWithUpdate { inner: self }),
-			None => None,
-		}
-	}
+	/// Returns information for the direction.
+	#[inline]
+	pub(super) fn direction(&self) -> &'a ChannelUpdateInfo { self.direction }
 }
 
 impl<'a> fmt::Debug for DirectedChannelInfo<'a> {
@@ -927,32 +916,6 @@ impl<'a> fmt::Debug for DirectedChannelInfo<'a> {
 	}
 }
 
-/// A [`DirectedChannelInfo`] with [`ChannelUpdateInfo`] available in its direction.
-#[derive(Clone)]
-pub(super) struct DirectedChannelInfoWithUpdate<'a> {
-	inner: DirectedChannelInfo<'a>,
-}
-
-impl<'a> DirectedChannelInfoWithUpdate<'a> {
-	/// Returns information for the channel.
-	#[inline]
-	pub(super) fn channel(&self) -> &'a ChannelInfo { &self.inner.channel }
-
-	/// Returns information for the direction.
-	#[inline]
-	pub(super) fn direction(&self) -> &'a ChannelUpdateInfo { self.inner.direction.unwrap() }
-
-	/// Returns the [`EffectiveCapacity`] of the channel in the direction.
-	#[inline]
-	pub(super) fn effective_capacity(&self) -> EffectiveCapacity { self.inner.effective_capacity() }
-}
-
-impl<'a> fmt::Debug for DirectedChannelInfoWithUpdate<'a> {
-	fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-		self.inner.fmt(f)
-	}
-}
-
 /// The effective capacity of a channel for routing purposes.
 ///
 /// While this may be smaller than the actual channel capacity, amounts greater than

lightning/src/routing/router.rs

@@ -17,7 +17,7 @@ use bitcoin::secp256k1::PublicKey;
 use crate::ln::channelmanager::ChannelDetails;
 use crate::ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures};
 use crate::ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
-use crate::routing::gossip::{DirectedChannelInfoWithUpdate, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId, RoutingFees};
+use crate::routing::gossip::{DirectedChannelInfo, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId, RoutingFees};
 use crate::routing::scoring::{ChannelUsage, Score};
 use crate::util::ser::{Writeable, Readable, Writer};
 use crate::util::logger::{Level, Logger};
@@ -421,7 +421,7 @@ enum CandidateRouteHop<'a> {
 	},
 	/// A hop found in the [`ReadOnlyNetworkGraph`], where the channel capacity may be unknown.
 	PublicHop {
-		info: DirectedChannelInfoWithUpdate<'a>,
+		info: DirectedChannelInfo<'a>,
 		short_channel_id: u64,
 	},
 	/// A hop to the payee found in the payment invoice, though not necessarily a direct channel.
@@ -1276,13 +1276,11 @@ where L::Target: Logger {
 					for chan_id in $node.channels.iter() {
 						let chan = network_channels.get(chan_id).unwrap();
 						if !chan.features.requires_unknown_bits() {
-							let (directed_channel, source) =
-								chan.as_directed_to(&$node_id).expect("inconsistent NetworkGraph");
-							if first_hops.is_none() || *source != our_node_id {
-								if let Some(direction) = directed_channel.direction() {
-									if direction.enabled {
+							if let Some((directed_channel, source)) = chan.as_directed_to(&$node_id) {
+								if first_hops.is_none() || *source != our_node_id {
+									if directed_channel.direction().enabled {
 										let candidate = CandidateRouteHop::PublicHop {
-											info: directed_channel.with_update().unwrap(),
+											info: directed_channel,
 											short_channel_id: *chan_id,
 										};
 										add_entry!(candidate, *source, $node_id,
@@ -1367,8 +1365,7 @@ where L::Target: Logger {
 					let candidate = network_channels
 						.get(&hop.short_channel_id)
 						.and_then(|channel| channel.as_directed_to(&target))
-						.and_then(|(channel, _)| channel.with_update())
-						.map(|info| CandidateRouteHop::PublicHop {
+						.map(|(info, _)| CandidateRouteHop::PublicHop {
 							info,
 							short_channel_id: hop.short_channel_id,
 						})
@@ -1816,10 +1813,8 @@ fn add_random_cltv_offset(route: &mut Route, payment_params: &PaymentParameters,
 							random_channel.as_directed_from(&cur_node_id).map(|(dir_info, next_id)| {
 								if !nodes_to_avoid.iter().any(|x| x == next_id) {
 									nodes_to_avoid[random_hop] = *next_id;
-									dir_info.direction().map(|channel_update_info| {
-										random_hop_offset = channel_update_info.cltv_expiry_delta.into();
-										cur_hop = Some(*next_id);
-									});
+									random_hop_offset = dir_info.direction().cltv_expiry_delta.into();
+									cur_hop = Some(*next_id);
 								}
 							});
 						}
@@ -5214,14 +5209,12 @@ mod tests {
 					for channel_id in &cur_node.channels {
 						if let Some(channel_info) = network_channels.get(&channel_id) {
 							if let Some((dir_info, next_id)) = channel_info.as_directed_from(&cur_node_id) {
-								if let Some(channel_update_info) = dir_info.direction() {
-									let next_cltv_expiry_delta = channel_update_info.cltv_expiry_delta as u32;
-									if cur_path_cltv_deltas.iter().sum::<u32>()
-										.saturating_add(next_cltv_expiry_delta) <= observed_cltv_expiry_delta {
-										let mut new_path_cltv_deltas = cur_path_cltv_deltas.clone();
-										new_path_cltv_deltas.push(next_cltv_expiry_delta);
-										candidates.push_back((*next_id, new_path_cltv_deltas));
-									}
+								let next_cltv_expiry_delta = dir_info.direction().cltv_expiry_delta as u32;
+								if cur_path_cltv_deltas.iter().sum::<u32>()
+									.saturating_add(next_cltv_expiry_delta) <= observed_cltv_expiry_delta {
+									let mut new_path_cltv_deltas = cur_path_cltv_deltas.clone();
+									new_path_cltv_deltas.push(next_cltv_expiry_delta);
+									candidates.push_back((*next_id, new_path_cltv_deltas));
 								}
 							}
 						}