From ec95e589ba250dae16e4a33c72f8f0ef0437ffeb Mon Sep 17 00:00:00 2001 From: jbesraa Date: Tue, 5 Dec 2023 20:56:51 +0200 Subject: [PATCH 1/6] Add `outbound` flag to `DirectedChannelInfo` If `outband` flag is set to true then `ChannelInfo::node_one` is forwarding a payment to target `ChannelInfo::node_two`. If `outband` flag is set to false then `ChannelInfo::node_two` is forwarding a payment to target `ChannelInfo::node_one`. --- lightning/src/routing/gossip.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index f556fabad..68bc7945a 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -870,31 +870,31 @@ impl ChannelInfo { /// Returns a [`DirectedChannelInfo`] for the channel directed to the given `target` from a /// returned `source`, or `None` if `target` is not one of the channel's counterparties. pub fn as_directed_to(&self, target: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> { - let (direction, source) = { + let (direction, source, outbound) = { if target == &self.node_one { - (self.two_to_one.as_ref(), &self.node_two) + (self.two_to_one.as_ref(), &self.node_two, false) } else if target == &self.node_two { - (self.one_to_two.as_ref(), &self.node_one) + (self.one_to_two.as_ref(), &self.node_one, true) } else { return None; } }; - direction.map(|dir| (DirectedChannelInfo::new(self, dir), source)) + direction.map(|dir| (DirectedChannelInfo::new(self, dir, outbound), source)) } /// Returns a [`DirectedChannelInfo`] for the channel directed from the given `source` to a /// returned `target`, or `None` if `source` is not one of the channel's counterparties. pub fn as_directed_from(&self, source: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> { - let (direction, target) = { + let (direction, target, outbound) = { if source == &self.node_one { - (self.one_to_two.as_ref(), &self.node_two) + (self.one_to_two.as_ref(), &self.node_two, true) } else if source == &self.node_two { - (self.two_to_one.as_ref(), &self.node_one) + (self.two_to_one.as_ref(), &self.node_one, false) } else { return None; } }; - direction.map(|dir| (DirectedChannelInfo::new(self, dir), target)) + direction.map(|dir| (DirectedChannelInfo::new(self, dir, outbound), target)) } /// Returns a [`ChannelUpdateInfo`] based on the direction implied by the channel_flag. @@ -992,24 +992,32 @@ pub struct DirectedChannelInfo<'a> { direction: &'a ChannelUpdateInfo, htlc_maximum_msat: u64, effective_capacity: EffectiveCapacity, + /// Outbound from the perspective of `node_one`. + /// + /// If true, the channel is considered to be outbound from `node_one` perspective. + /// If false, the channel is considered to be outbound from `node_two` perspective. + /// + /// [`ChannelInfo::node_one`] + /// [`ChannelInfo::node_two`] + outbound: bool, } impl<'a> DirectedChannelInfo<'a> { #[inline] - fn new(channel: &'a ChannelInfo, direction: &'a ChannelUpdateInfo) -> Self { + fn new(channel: &'a ChannelInfo, direction: &'a ChannelUpdateInfo, outbound: bool) -> Self { let mut htlc_maximum_msat = direction.htlc_maximum_msat; let capacity_msat = channel.capacity_sats.map(|capacity_sats| capacity_sats * 1000); 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: htlc_maximum_msat } + EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat } }, None => EffectiveCapacity::AdvertisedMaxHTLC { amount_msat: htlc_maximum_msat }, }; Self { - channel, direction, htlc_maximum_msat, effective_capacity + channel, direction, htlc_maximum_msat, effective_capacity, outbound } } From 366d68808734bab379072e2a069bd72da51f4421 Mon Sep 17 00:00:00 2001 From: jbesraa Date: Tue, 5 Dec 2023 21:00:56 +0200 Subject: [PATCH 2/6] Add `source` and `target` to `DirectedChannelInfo` `DirectedChannelInfo::source` return the `node_id` of the forwarding hop. `DirectedChannelInfo::target` return the `node_id` of the destination hop. --- lightning/src/routing/gossip.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 68bc7945a..edb372616 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -1043,6 +1043,16 @@ impl<'a> DirectedChannelInfo<'a> { /// Returns information for the direction. #[inline] pub(super) fn direction(&self) -> &'a ChannelUpdateInfo { self.direction } + + /// Returns the `node_id` of the source hop. + /// + /// Refers to the `node_id` forwarding the payment to the next hop. + pub(super) fn source(&self) -> &'a NodeId { if self.outbound { &self.channel.node_one } else { &self.channel.node_two } } + + /// Returns the `node_id` of the target hop. + /// + /// Refers to the `node_id` receiving the payment from the previous hop. + pub(super) fn target(&self) -> &'a NodeId { if self.outbound { &self.channel.node_two } else { &self.channel.node_one } } } impl<'a> fmt::Debug for DirectedChannelInfo<'a> { From a1d15ac1926f70aa5ab4f6686fe12fb18383bb3b Mon Sep 17 00:00:00 2001 From: jbesraa Date: Sat, 26 Aug 2023 18:06:45 +0300 Subject: [PATCH 3/6] Add `source` and `target` fn's to `CandidateRouteHop` We add `CandidateRouteHop::source` and `CandidateRouteHop::source` functions to point to current and next hops in route respectively. As we have now `source` and `target` available in `CandidateRouteHop` we also remove `CandidateRouteHop::id` inputs so now they are consumed from `self.target` and `self.source` functions. In the `add_entry` macro we also remove `source` and `target` arguments in favor of `candidate` of type `CandidateRouteHop` that holds the needed info. --- lightning/src/routing/router.rs | 198 +++++++++++++++++++++----------- 1 file changed, 134 insertions(+), 64 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 395731502..361d5793e 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -993,34 +993,70 @@ impl cmp::PartialOrd for RouteGraphNode { /// A wrapper around the various hop representations. /// -/// Used to construct a [`PathBuildingHop`] and to estimate [`EffectiveCapacity`]. +/// Can be used to examine the properties of a hop, +/// potentially to decide whether to include it in a route. #[derive(Clone, Debug)] -enum CandidateRouteHop<'a> { +pub enum CandidateRouteHop<'a> { /// A hop from the payer, where the outbound liquidity is known. FirstHop { + /// Channel details of the first hop + /// [`ChannelDetails::get_outbound_payment_scid`] is assumed + /// to always return `Some(scid)` + /// this assumption is checked in [`find_route`] method. details: &'a ChannelDetails, + /// The node id of the payer. + /// + /// Can be accessed via `source` method. + node_id: NodeId }, - /// A hop found in the [`ReadOnlyNetworkGraph`], where the channel capacity may be unknown. + /// A hop found in the [`ReadOnlyNetworkGraph`], + /// where the channel capacity may be unknown. PublicHop { + /// channel info of the hop. info: DirectedChannelInfo<'a>, + /// short_channel_id of the channel. short_channel_id: u64, }, - /// A hop to the payee found in the BOLT 11 payment invoice, though not necessarily a direct + /// A hop to the payee found in the BOLT 11 payment invoice, + /// though not necessarily a direct /// channel. PrivateHop { + /// Hint provides information about a private hop, + /// needed while routing through a private + /// channel. hint: &'a RouteHintHop, + /// Node id of the next hop in route. + target_node_id: NodeId }, - /// The payee's identity is concealed behind blinded paths provided in a BOLT 12 invoice. + /// The payee's identity is concealed behind + /// blinded paths provided in a BOLT 12 invoice. Blinded { + /// Hint provides information about a blinded hop, + /// needed while routing through a blinded path. + /// `BlindedPayInfo` provides information needed about the + /// payment while routing through a blinded path. + /// `BlindedPath` is the blinded path to the destination. hint: &'a (BlindedPayInfo, BlindedPath), + /// Index of the hint in the original list of blinded hints. + /// Provided to uniquely identify a hop as we are + /// route building. hint_idx: usize, }, - /// Similar to [`Self::Blinded`], but the path here has 1 blinded hop. `BlindedPayInfo` provided - /// for 1-hop blinded paths is ignored because it is meant to apply to the hops *between* the - /// introduction node and the destination. Useful for tracking that we need to include a blinded + /// Similar to [`Self::Blinded`], but the path here + /// has 1 blinded hop. `BlindedPayInfo` provided + /// for 1-hop blinded paths is ignored + /// because it is meant to apply to the hops *between* the + /// introduction node and the destination. + /// Useful for tracking that we need to include a blinded /// path at the end of our [`Route`]. OneHopBlinded { + /// Hint provides information about a single blinded hop, + /// needed while routing through a one hop blinded path. + /// `BlindedPayInfo` is ignored here. + /// `BlindedPath` is the blinded path to the destination. hint: &'a (BlindedPayInfo, BlindedPath), + /// Index of the hint in the original list of blinded hints. + /// Provided to uniquely identify a hop as we are route building. hint_idx: usize, }, } @@ -1028,9 +1064,9 @@ enum CandidateRouteHop<'a> { impl<'a> CandidateRouteHop<'a> { fn short_channel_id(&self) -> Option { match self { - CandidateRouteHop::FirstHop { details } => Some(details.get_outbound_payment_scid().unwrap()), + CandidateRouteHop::FirstHop { details, .. } => Some(details.get_outbound_payment_scid().unwrap()), CandidateRouteHop::PublicHop { short_channel_id, .. } => Some(*short_channel_id), - CandidateRouteHop::PrivateHop { hint } => Some(hint.short_channel_id), + CandidateRouteHop::PrivateHop { hint, .. } => Some(hint.short_channel_id), CandidateRouteHop::Blinded { .. } => None, CandidateRouteHop::OneHopBlinded { .. } => None, } @@ -1039,7 +1075,7 @@ impl<'a> CandidateRouteHop<'a> { // NOTE: This may alloc memory so avoid calling it in a hot code path. fn features(&self) -> ChannelFeatures { match self { - CandidateRouteHop::FirstHop { details } => details.counterparty.features.to_context(), + CandidateRouteHop::FirstHop { details, .. } => details.counterparty.features.to_context(), CandidateRouteHop::PublicHop { info, .. } => info.channel().features.clone(), CandidateRouteHop::PrivateHop { .. } => ChannelFeatures::empty(), CandidateRouteHop::Blinded { .. } => ChannelFeatures::empty(), @@ -1051,7 +1087,7 @@ impl<'a> CandidateRouteHop<'a> { match self { CandidateRouteHop::FirstHop { .. } => 0, CandidateRouteHop::PublicHop { info, .. } => info.direction().cltv_expiry_delta as u32, - CandidateRouteHop::PrivateHop { hint } => hint.cltv_expiry_delta as u32, + CandidateRouteHop::PrivateHop { hint, .. } => hint.cltv_expiry_delta as u32, CandidateRouteHop::Blinded { hint, .. } => hint.0.cltv_expiry_delta as u32, CandidateRouteHop::OneHopBlinded { .. } => 0, } @@ -1059,9 +1095,9 @@ impl<'a> CandidateRouteHop<'a> { fn htlc_minimum_msat(&self) -> u64 { match self { - CandidateRouteHop::FirstHop { details } => details.next_outbound_htlc_minimum_msat, + CandidateRouteHop::FirstHop { details, .. } => details.next_outbound_htlc_minimum_msat, CandidateRouteHop::PublicHop { info, .. } => info.direction().htlc_minimum_msat, - CandidateRouteHop::PrivateHop { hint } => hint.htlc_minimum_msat.unwrap_or(0), + CandidateRouteHop::PrivateHop { hint, .. } => hint.htlc_minimum_msat.unwrap_or(0), CandidateRouteHop::Blinded { hint, .. } => hint.0.htlc_minimum_msat, CandidateRouteHop::OneHopBlinded { .. } => 0, } @@ -1073,7 +1109,7 @@ impl<'a> CandidateRouteHop<'a> { base_msat: 0, proportional_millionths: 0, }, CandidateRouteHop::PublicHop { info, .. } => info.direction().fees, - CandidateRouteHop::PrivateHop { hint } => hint.fees, + CandidateRouteHop::PrivateHop { hint, .. } => hint.fees, CandidateRouteHop::Blinded { hint, .. } => { RoutingFees { base_msat: hint.0.fee_base_msat, @@ -1087,13 +1123,13 @@ impl<'a> CandidateRouteHop<'a> { fn effective_capacity(&self) -> EffectiveCapacity { match self { - CandidateRouteHop::FirstHop { details } => EffectiveCapacity::ExactLiquidity { + CandidateRouteHop::FirstHop { details, .. } => EffectiveCapacity::ExactLiquidity { liquidity_msat: details.next_outbound_htlc_limit_msat, }, CandidateRouteHop::PublicHop { info, .. } => info.effective_capacity(), - CandidateRouteHop::PrivateHop { hint: RouteHintHop { htlc_maximum_msat: Some(max), .. }} => + CandidateRouteHop::PrivateHop { hint: RouteHintHop { htlc_maximum_msat: Some(max), .. }, .. } => EffectiveCapacity::HintMaxHTLC { amount_msat: *max }, - CandidateRouteHop::PrivateHop { hint: RouteHintHop { htlc_maximum_msat: None, .. }} => + CandidateRouteHop::PrivateHop { hint: RouteHintHop { htlc_maximum_msat: None, .. }, .. } => EffectiveCapacity::Infinite, CandidateRouteHop::Blinded { hint, .. } => EffectiveCapacity::HintMaxHTLC { amount_msat: hint.0.htlc_maximum_msat }, @@ -1101,11 +1137,14 @@ impl<'a> CandidateRouteHop<'a> { } } - fn id(&self, channel_direction: bool /* src_node_id < target_node_id */) -> CandidateHopId { + /// Returns the id of this hop. + /// For `Blinded` and `OneHopBlinded` we return `CandidateHopId::Blinded` with `hint_idx` because we don't know the channel id. + /// For any other option we return `CandidateHopId::Clear` because we know the channel id and the direction. + pub fn id(&self) -> CandidateHopId { match self { CandidateRouteHop::Blinded { hint_idx, .. } => CandidateHopId::Blinded(*hint_idx), CandidateRouteHop::OneHopBlinded { hint_idx, .. } => CandidateHopId::Blinded(*hint_idx), - _ => CandidateHopId::Clear((self.short_channel_id().unwrap(), channel_direction)), + _ => CandidateHopId::Clear((self.short_channel_id().unwrap(), self.source() < self.target().unwrap())), } } fn blinded_path(&self) -> Option<&'a BlindedPath> { @@ -1116,10 +1155,42 @@ impl<'a> CandidateRouteHop<'a> { _ => None, } } + /// Returns the source node id of current hop. + /// + /// Source node id refers to the hop forwarding the payment. + /// + /// For `FirstHop` we return payer's node id. + pub fn source(&self) -> NodeId { + match self { + CandidateRouteHop::FirstHop { node_id, .. } => *node_id, + CandidateRouteHop::PublicHop { info, .. } => *info.source(), + CandidateRouteHop::PrivateHop { hint, .. } => hint.src_node_id.into(), + CandidateRouteHop::Blinded { hint, .. } => hint.1.introduction_node_id.into(), + CandidateRouteHop::OneHopBlinded { hint, .. } => hint.1.introduction_node_id.into() + } + } + /// Returns the target node id of this hop, if known. + /// + /// Target node id refers to the hop receiving the payment. + /// + /// For `Blinded` and `OneHopBlinded` we return `None` because next hop is blinded. + pub fn target(&self) -> Option { + match self { + CandidateRouteHop::FirstHop { details, .. } => Some(details.counterparty.node_id.into()), + CandidateRouteHop::PublicHop { info, .. } => Some(*info.target()), + CandidateRouteHop::PrivateHop { target_node_id, .. } => Some(*target_node_id), + CandidateRouteHop::Blinded { .. } => None, + CandidateRouteHop::OneHopBlinded { .. } => None, + } + } } +/// A wrapper around the various hop id representations. +/// +/// `CandidateHopId::Clear` is used to identify a hop with a known short channel id and direction. +/// `CandidateHopId::Blinded` is used to identify a blinded hop `hint_idx`. #[derive(Clone, Copy, Eq, Hash, Ord, PartialOrd, PartialEq)] -enum CandidateHopId { +pub enum CandidateHopId { /// Contains (scid, src_node_id < target_node_id) Clear((u64, bool)), /// Index of the blinded route hint in [`Payee::Blinded::route_hints`]. @@ -1722,11 +1793,11 @@ where L::Target: Logger { let mut num_ignored_htlc_minimum_msat_limit: u32 = 0; macro_rules! add_entry { - // Adds entry which goes from $src_node_id to $dest_node_id over the $candidate hop. + // Adds entry which goes from $candidate.source() to $candidate.target() over the $candidate hop. // $next_hops_fee_msat represents the fees paid for using all the channels *after* this one, // since that value has to be transferred over this channel. // Returns the contribution amount of $candidate if the channel caused an update to `targets`. - ( $candidate: expr, $src_node_id: expr, $dest_node_id: expr, $next_hops_fee_msat: expr, + ( $candidate: expr, $next_hops_fee_msat: expr, $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr, $next_hops_path_penalty_msat: expr, $next_hops_cltv_delta: expr, $next_hops_path_length: expr ) => { { // We "return" whether we updated the path at the end, and how much we can route via @@ -1736,7 +1807,9 @@ where L::Target: Logger { // practice these cases should be caught earlier: // - for regular channels at channel announcement (TODO) // - for first and last hops early in get_route - if $src_node_id != $dest_node_id { + let src_node_id = $candidate.source(); + let dest_node_id = $candidate.target().unwrap_or(maybe_dummy_payee_node_id); + if src_node_id != dest_node_id { let scid_opt = $candidate.short_channel_id(); let effective_capacity = $candidate.effective_capacity(); let htlc_maximum_msat = max_htlc_from_capacity(effective_capacity, channel_saturation_pow_half); @@ -1750,7 +1823,7 @@ where L::Target: Logger { // We do this for now, but this is a subject for removal. if let Some(mut available_value_contribution_msat) = htlc_maximum_msat.checked_sub($next_hops_fee_msat) { let used_liquidity_msat = used_liquidities - .get(&$candidate.id($src_node_id < $dest_node_id)) + .get(&$candidate.id()) .map_or(0, |used_liquidity_msat| { available_value_contribution_msat = available_value_contribution_msat .saturating_sub(*used_liquidity_msat); @@ -1796,7 +1869,7 @@ where L::Target: Logger { |scid| payment_params.previously_failed_channels.contains(&scid)); let (should_log_candidate, first_hop_details) = match $candidate { - CandidateRouteHop::FirstHop { details } => (true, Some(details)), + CandidateRouteHop::FirstHop { details, .. } => (true, Some(details)), CandidateRouteHop::PrivateHop { .. } => (true, None), CandidateRouteHop::Blinded { .. } => (true, None), CandidateRouteHop::OneHopBlinded { .. } => (true, None), @@ -1867,15 +1940,15 @@ where L::Target: Logger { ); let path_htlc_minimum_msat = compute_fees_saturating(curr_min, $candidate.fees()) .saturating_add(curr_min); - let hm_entry = dist.entry($src_node_id); + let hm_entry = dist.entry(src_node_id); let old_entry = hm_entry.or_insert_with(|| { // If there was previously no known way to access the source node // (recall it goes payee-to-payer) of short_channel_id, first add a // semi-dummy record just to compute the fees to reach the source node. // This will affect our decision on selecting short_channel_id - // as a way to reach the $dest_node_id. + // as a way to reach the $candidate.target() node. PathBuildingHop { - node_id: $dest_node_id.clone(), + node_id: dest_node_id.clone(), candidate: $candidate.clone(), fee_msat: 0, next_hops_fee_msat: u64::max_value(), @@ -1904,7 +1977,7 @@ where L::Target: Logger { // Ignore hop_use_fee_msat for channel-from-us as we assume all channels-from-us // will have the same effective-fee - if $src_node_id != our_node_id { + if src_node_id != our_node_id { // Note that `u64::max_value` means we'll always fail the // `old_entry.total_fee_msat > total_fee_msat` check below hop_use_fee_msat = compute_fees_saturating(amount_to_transfer_over_msat, $candidate.fees()); @@ -1932,12 +2005,12 @@ where L::Target: Logger { effective_capacity, }; let channel_penalty_msat = scid_opt.map_or(0, - |scid| scorer.channel_penalty_msat(scid, &$src_node_id, &$dest_node_id, + |scid| scorer.channel_penalty_msat(scid, &src_node_id, &dest_node_id, channel_usage, score_params)); let path_penalty_msat = $next_hops_path_penalty_msat .saturating_add(channel_penalty_msat); let new_graph_node = RouteGraphNode { - node_id: $src_node_id, + node_id: src_node_id, lowest_fee_to_node: total_fee_msat, total_cltv_delta: hop_total_cltv_delta, value_contribution_msat, @@ -1946,7 +2019,8 @@ where L::Target: Logger { path_length_to_node, }; - // Update the way of reaching $src_node_id with the given short_channel_id (from $dest_node_id), + // Update the way of reaching $candidate.source() + // with the given short_channel_id (from $candidate.target()), // if this way is cheaper than the already known // (considering the cost to "reach" this channel from the route destination, // the cost of using this channel, @@ -1972,7 +2046,7 @@ where L::Target: Logger { old_entry.next_hops_fee_msat = $next_hops_fee_msat; old_entry.hop_use_fee_msat = hop_use_fee_msat; old_entry.total_fee_msat = total_fee_msat; - old_entry.node_id = $dest_node_id.clone(); + old_entry.node_id = dest_node_id; old_entry.candidate = $candidate.clone(); old_entry.fee_msat = 0; // This value will be later filled with hop_use_fee_msat of the following channel old_entry.path_htlc_minimum_msat = path_htlc_minimum_msat; @@ -2060,8 +2134,8 @@ where L::Target: Logger { if !skip_node { if let Some(first_channels) = first_hop_targets.get(&$node_id) { for details in first_channels { - let candidate = CandidateRouteHop::FirstHop { details }; - add_entry!(candidate, our_node_id, $node_id, $fee_to_target_msat, + let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id }; + add_entry!(&candidate, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat, $next_hops_path_penalty_msat, $next_hops_cltv_delta, $next_hops_path_length); @@ -2085,7 +2159,7 @@ where L::Target: Logger { info: directed_channel, short_channel_id: *chan_id, }; - add_entry!(candidate, *source, $node_id, + add_entry!(&candidate, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat, @@ -2115,8 +2189,8 @@ where L::Target: Logger { // place where it could be added. payee_node_id_opt.map(|payee| first_hop_targets.get(&payee).map(|first_channels| { for details in first_channels { - let candidate = CandidateRouteHop::FirstHop { details }; - let added = add_entry!(candidate, our_node_id, payee, 0, path_value_msat, + let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id }; + let added = add_entry!(&candidate, 0, path_value_msat, 0, 0u64, 0, 0).is_some(); log_trace!(logger, "{} direct route to payee via {}", if added { "Added" } else { "Skipped" }, LoggedCandidateHop(&candidate)); @@ -2153,7 +2227,7 @@ where L::Target: Logger { CandidateRouteHop::OneHopBlinded { hint, hint_idx } } else { CandidateRouteHop::Blinded { hint, hint_idx } }; let mut path_contribution_msat = path_value_msat; - if let Some(hop_used_msat) = add_entry!(candidate, intro_node_id, maybe_dummy_payee_node_id, + if let Some(hop_used_msat) = add_entry!(&candidate, 0, path_contribution_msat, 0, 0_u64, 0, 0) { path_contribution_msat = hop_used_msat; @@ -2162,14 +2236,14 @@ where L::Target: Logger { sort_first_hop_channels(first_channels, &used_liquidities, recommended_value_msat, our_node_pubkey); for details in first_channels { - let first_hop_candidate = CandidateRouteHop::FirstHop { details }; + let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id}; let blinded_path_fee = match compute_fees(path_contribution_msat, candidate.fees()) { Some(fee) => fee, None => continue }; let path_min = candidate.htlc_minimum_msat().saturating_add( compute_fees_saturating(candidate.htlc_minimum_msat(), candidate.fees())); - add_entry!(first_hop_candidate, our_node_id, intro_node_id, blinded_path_fee, + add_entry!(&first_hop_candidate, blinded_path_fee, path_contribution_msat, path_min, 0_u64, candidate.cltv_expiry_delta(), candidate.blinded_path().map_or(1, |bp| bp.blinded_hops.len() as u8)); } @@ -2219,9 +2293,9 @@ where L::Target: Logger { info, short_channel_id: hop.short_channel_id, }) - .unwrap_or_else(|| CandidateRouteHop::PrivateHop { hint: hop }); + .unwrap_or_else(|| CandidateRouteHop::PrivateHop { hint: hop, target_node_id: target }); - if let Some(hop_used_msat) = add_entry!(candidate, source, target, + if let Some(hop_used_msat) = add_entry!(&candidate, aggregate_next_hops_fee_msat, aggregate_path_contribution_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat, aggregate_next_hops_cltv_delta, aggregate_next_hops_path_length) @@ -2235,7 +2309,7 @@ where L::Target: Logger { } let used_liquidity_msat = used_liquidities - .get(&candidate.id(source < target)).copied() + .get(&candidate.id()).copied() .unwrap_or(0); let channel_usage = ChannelUsage { amount_msat: final_value_msat + aggregate_next_hops_fee_msat, @@ -2259,8 +2333,8 @@ where L::Target: Logger { sort_first_hop_channels(first_channels, &used_liquidities, recommended_value_msat, our_node_pubkey); for details in first_channels { - let first_hop_candidate = CandidateRouteHop::FirstHop { details }; - add_entry!(first_hop_candidate, our_node_id, target, + let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id}; + add_entry!(&first_hop_candidate, aggregate_next_hops_fee_msat, aggregate_path_contribution_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat, aggregate_next_hops_cltv_delta, aggregate_next_hops_path_length); @@ -2304,9 +2378,8 @@ where L::Target: Logger { sort_first_hop_channels(first_channels, &used_liquidities, recommended_value_msat, our_node_pubkey); for details in first_channels { - let first_hop_candidate = CandidateRouteHop::FirstHop { details }; - add_entry!(first_hop_candidate, our_node_id, - NodeId::from_pubkey(&hop.src_node_id), + let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id}; + add_entry!(&first_hop_candidate, aggregate_next_hops_fee_msat, aggregate_path_contribution_msat, aggregate_next_hops_path_htlc_minimum_msat, @@ -2417,12 +2490,10 @@ where L::Target: Logger { // Remember that we used these channels so that we don't rely // on the same liquidity in future paths. let mut prevented_redundant_path_selection = false; - let prev_hop_iter = core::iter::once(&our_node_id) - .chain(payment_path.hops.iter().map(|(hop, _)| &hop.node_id)); - for (prev_hop, (hop, _)) in prev_hop_iter.zip(payment_path.hops.iter()) { + for (hop, _) in payment_path.hops.iter() { let spent_on_hop_msat = value_contribution_msat + hop.next_hops_fee_msat; let used_liquidity_msat = used_liquidities - .entry(hop.candidate.id(*prev_hop < hop.node_id)) + .entry(hop.candidate.id()) .and_modify(|used_liquidity_msat| *used_liquidity_msat += spent_on_hop_msat) .or_insert(spent_on_hop_msat); let hop_capacity = hop.candidate.effective_capacity(); @@ -2443,8 +2514,10 @@ where L::Target: Logger { log_trace!(logger, "Disabling route candidate {} for future path building iterations to avoid duplicates.", LoggedCandidateHop(victim_candidate)); - *used_liquidities.entry(victim_candidate.id(false)).or_default() = exhausted; - *used_liquidities.entry(victim_candidate.id(true)).or_default() = exhausted; + if let Some(scid) = victim_candidate.short_channel_id() { + *used_liquidities.entry(CandidateHopId::Clear((scid, false))).or_default() = exhausted; + *used_liquidities.entry(CandidateHopId::Clear((scid, true))).or_default() = exhausted; + } } // Track the total amount all our collected paths allow to send so that we know @@ -2590,15 +2663,15 @@ where L::Target: Logger { selected_route.sort_unstable_by_key(|path| { let mut key = [CandidateHopId::Clear((42, true)) ; MAX_PATH_LENGTH_ESTIMATE as usize]; debug_assert!(path.hops.len() <= key.len()); - for (scid, key) in path.hops.iter() .map(|h| h.0.candidate.id(true)).zip(key.iter_mut()) { + for (scid, key) in path.hops.iter() .map(|h| h.0.candidate.id()).zip(key.iter_mut()) { *key = scid; } key }); for idx in 0..(selected_route.len() - 1) { if idx + 1 >= selected_route.len() { break; } - if iter_equal(selected_route[idx ].hops.iter().map(|h| (h.0.candidate.id(true), h.0.node_id)), - selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.id(true), h.0.node_id))) { + if iter_equal(selected_route[idx].hops.iter().map(|h| (h.0.candidate.id(), h.0.node_id)), + selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.id(), h.0.node_id))) { let new_value = selected_route[idx].get_value_msat() + selected_route[idx + 1].get_value_msat(); selected_route[idx].update_value_and_recompute_fees(new_value); selected_route.remove(idx + 1); @@ -2608,14 +2681,13 @@ where L::Target: Logger { let mut paths = Vec::new(); for payment_path in selected_route { let mut hops = Vec::with_capacity(payment_path.hops.len()); - let mut prev_hop_node_id = our_node_id; for (hop, node_features) in payment_path.hops.iter() .filter(|(h, _)| h.candidate.short_channel_id().is_some()) { let maybe_announced_channel = if let CandidateRouteHop::PublicHop { .. } = hop.candidate { // If we sourced the hop from the graph we're sure the target node is announced. true - } else if let CandidateRouteHop::FirstHop { details } = hop.candidate { + } else if let CandidateRouteHop::FirstHop { details, .. } = hop.candidate { // If this is a first hop we also know if it's announced. details.is_public } else { @@ -2625,7 +2697,7 @@ where L::Target: Logger { // an alias, in which case we don't take any chances here. network_graph.node(&hop.node_id).map_or(false, |hop_node| hop_node.channels.iter().any(|scid| network_graph.channel(*scid) - .map_or(false, |c| c.as_directed_from(&prev_hop_node_id).is_some())) + .map_or(false, |c| c.as_directed_from(&hop.candidate.source()).is_some())) ) }; @@ -2638,8 +2710,6 @@ where L::Target: Logger { cltv_expiry_delta: hop.candidate.cltv_expiry_delta(), maybe_announced_channel, }); - - prev_hop_node_id = hop.node_id; } let mut final_cltv_delta = final_cltv_expiry_delta; let blinded_tail = payment_path.hops.last().and_then(|(h, _)| { From 04e93fc887bf6db34971037f2f7deae303995a2f Mon Sep 17 00:00:00 2001 From: jbesraa Date: Mon, 20 Nov 2023 17:38:07 +0200 Subject: [PATCH 4/6] Change `CandidateRouteHop` functions visbility Change `short_channel_id`, `cltv_expiry_delta` `fees` and `htlc_minimum_msat` under `CandidateRouteHop` visibility to public and add documentation. --- lightning/src/routing/router.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 361d5793e..5e906878d 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1062,7 +1062,11 @@ pub enum CandidateRouteHop<'a> { } impl<'a> CandidateRouteHop<'a> { - fn short_channel_id(&self) -> Option { + /// Returns short_channel_id if known. + /// For `FirstHop` we assume [`ChannelDetails::get_outbound_payment_scid`] is always set, this assumption is checked in + /// [`find_route`] method. + /// For `Blinded` and `OneHopBlinded` we return `None` because next hop is not known. + pub fn short_channel_id(&self) -> Option { match self { CandidateRouteHop::FirstHop { details, .. } => Some(details.get_outbound_payment_scid().unwrap()), CandidateRouteHop::PublicHop { short_channel_id, .. } => Some(*short_channel_id), @@ -1083,7 +1087,8 @@ impl<'a> CandidateRouteHop<'a> { } } - fn cltv_expiry_delta(&self) -> u32 { + /// Returns cltv_expiry_delta for this hop. + pub fn cltv_expiry_delta(&self) -> u32 { match self { CandidateRouteHop::FirstHop { .. } => 0, CandidateRouteHop::PublicHop { info, .. } => info.direction().cltv_expiry_delta as u32, @@ -1093,7 +1098,8 @@ impl<'a> CandidateRouteHop<'a> { } } - fn htlc_minimum_msat(&self) -> u64 { + /// Returns the htlc_minimum_msat for this hop. + pub fn htlc_minimum_msat(&self) -> u64 { match self { CandidateRouteHop::FirstHop { details, .. } => details.next_outbound_htlc_minimum_msat, CandidateRouteHop::PublicHop { info, .. } => info.direction().htlc_minimum_msat, @@ -1103,7 +1109,8 @@ impl<'a> CandidateRouteHop<'a> { } } - fn fees(&self) -> RoutingFees { + /// Returns the fees for this hop. + pub fn fees(&self) -> RoutingFees { match self { CandidateRouteHop::FirstHop { .. } => RoutingFees { base_msat: 0, proportional_millionths: 0, From f0ecc3ec73dcdb9303b1bd5ac687a361decce2dd Mon Sep 17 00:00:00 2001 From: jbesraa Date: Wed, 6 Sep 2023 16:04:08 +0300 Subject: [PATCH 5/6] Use `CandidateRouteHop` as input for `channel_penalty_msat` We remove `source`, `target` and `scid` from `channel_penalty_msat` inputs to consume them from `candidate` of type `CandidateRouteHop` --- lightning-background-processor/src/lib.rs | 4 +- lightning/src/routing/router.rs | 62 ++- lightning/src/routing/scoring.rs | 490 ++++++++++++++-------- lightning/src/util/test_utils.rs | 37 +- 4 files changed, 399 insertions(+), 194 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 95796e860..1e6d447c8 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -863,8 +863,8 @@ mod tests { use lightning::ln::msgs::{ChannelMessageHandler, Init}; use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler}; use lightning::routing::gossip::{NetworkGraph, NodeId, P2PGossipSync}; - use lightning::routing::router::{DefaultRouter, Path, RouteHop}; use lightning::routing::scoring::{ChannelUsage, ScoreUpdate, ScoreLookUp, LockableScore}; + use lightning::routing::router::{DefaultRouter, Path, RouteHop, CandidateRouteHop}; use lightning::util::config::UserConfig; use lightning::util::ser::Writeable; use lightning::util::test_utils; @@ -1071,7 +1071,7 @@ mod tests { impl ScoreLookUp for TestScorer { type ScoreParams = (); fn channel_penalty_msat( - &self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage, _score_params: &Self::ScoreParams + &self, _candidate: &CandidateRouteHop, _usage: ChannelUsage, _score_params: &Self::ScoreParams ) -> u64 { unimplemented!(); } } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 5e906878d..c33a0d36d 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -130,18 +130,27 @@ impl<'a, S: Deref> ScorerAccountingForInFlightHtlcs<'a, S> where S::Target: Scor impl<'a, S: Deref> ScoreLookUp for ScorerAccountingForInFlightHtlcs<'a, S> where S::Target: ScoreLookUp { type ScoreParams = ::ScoreParams; - fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 { + fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 { + let target = match candidate.target() { + Some(target) => target, + None => return self.scorer.channel_penalty_msat(candidate, usage, score_params), + }; + let short_channel_id = match candidate.short_channel_id() { + Some(short_channel_id) => short_channel_id, + None => return self.scorer.channel_penalty_msat(candidate, usage, score_params), + }; + let source = candidate.source(); if let Some(used_liquidity) = self.inflight_htlcs.used_liquidity_msat( - source, target, short_channel_id + &source, &target, short_channel_id ) { let usage = ChannelUsage { inflight_htlc_msat: usage.inflight_htlc_msat.saturating_add(used_liquidity), ..usage }; - self.scorer.channel_penalty_msat(short_channel_id, source, target, usage, score_params) + self.scorer.channel_penalty_msat(candidate, usage, score_params) } else { - self.scorer.channel_penalty_msat(short_channel_id, source, target, usage, score_params) + self.scorer.channel_penalty_msat(candidate, usage, score_params) } } } @@ -1068,7 +1077,7 @@ impl<'a> CandidateRouteHop<'a> { /// For `Blinded` and `OneHopBlinded` we return `None` because next hop is not known. pub fn short_channel_id(&self) -> Option { match self { - CandidateRouteHop::FirstHop { details, .. } => Some(details.get_outbound_payment_scid().unwrap()), + CandidateRouteHop::FirstHop { details, .. } => details.get_outbound_payment_scid(), CandidateRouteHop::PublicHop { short_channel_id, .. } => Some(*short_channel_id), CandidateRouteHop::PrivateHop { hint, .. } => Some(hint.short_channel_id), CandidateRouteHop::Blinded { .. } => None, @@ -1173,7 +1182,7 @@ impl<'a> CandidateRouteHop<'a> { CandidateRouteHop::PublicHop { info, .. } => *info.source(), CandidateRouteHop::PrivateHop { hint, .. } => hint.src_node_id.into(), CandidateRouteHop::Blinded { hint, .. } => hint.1.introduction_node_id.into(), - CandidateRouteHop::OneHopBlinded { hint, .. } => hint.1.introduction_node_id.into() + CandidateRouteHop::OneHopBlinded { hint, .. } => hint.1.introduction_node_id.into(), } } /// Returns the target node id of this hop, if known. @@ -2011,9 +2020,10 @@ where L::Target: Logger { inflight_htlc_msat: used_liquidity_msat, effective_capacity, }; - let channel_penalty_msat = scid_opt.map_or(0, - |scid| scorer.channel_penalty_msat(scid, &src_node_id, &dest_node_id, - channel_usage, score_params)); + let channel_penalty_msat = + scorer.channel_penalty_msat($candidate, + channel_usage, + score_params); let path_penalty_msat = $next_hops_path_penalty_msat .saturating_add(channel_penalty_msat); let new_graph_node = RouteGraphNode { @@ -2324,7 +2334,7 @@ where L::Target: Logger { effective_capacity: candidate.effective_capacity(), }; let channel_penalty_msat = scorer.channel_penalty_msat( - hop.short_channel_id, &source, &target, channel_usage, score_params + &candidate, channel_usage, score_params ); aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat .saturating_add(channel_penalty_msat); @@ -2879,13 +2889,13 @@ fn build_route_from_hops_internal( impl ScoreLookUp for HopScorer { type ScoreParams = (); - fn channel_penalty_msat(&self, _short_channel_id: u64, source: &NodeId, target: &NodeId, + fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _usage: ChannelUsage, _score_params: &Self::ScoreParams) -> u64 { let mut cur_id = self.our_node_id; for i in 0..self.hop_ids.len() { if let Some(next_id) = self.hop_ids[i] { - if cur_id == *source && next_id == *target { + if cur_id == candidate.source() && Some(next_id) == candidate.target() { return 0; } cur_id = next_id; @@ -2926,7 +2936,7 @@ mod tests { use crate::routing::utxo::UtxoResult; use crate::routing::router::{get_route, build_route_from_hops_internal, add_random_cltv_offset, default_node_features, BlindedTail, InFlightHtlcs, Path, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees, - DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE, RouteParameters}; + DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE, RouteParameters, CandidateRouteHop}; use crate::routing::scoring::{ChannelUsage, FixedPenaltyScorer, ScoreLookUp, ProbabilisticScorer, ProbabilisticScoringFeeParameters, ProbabilisticScoringDecayParameters}; use crate::routing::test_utils::{add_channel, add_or_update_node, build_graph, build_line_graph, id_to_feature_flags, get_nodes, update_channel}; use crate::chain::transaction::OutPoint; @@ -6231,8 +6241,8 @@ mod tests { } impl ScoreLookUp for BadChannelScorer { type ScoreParams = (); - fn channel_penalty_msat(&self, short_channel_id: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 { - if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 } + fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 { + if candidate.short_channel_id() == Some(self.short_channel_id) { u64::max_value() } else { 0 } } } @@ -6247,8 +6257,8 @@ mod tests { impl ScoreLookUp for BadNodeScorer { type ScoreParams = (); - fn channel_penalty_msat(&self, _: u64, _: &NodeId, target: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 { - if *target == self.node_id { u64::max_value() } else { 0 } + fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 { + if candidate.target() == Some(self.node_id) { u64::max_value() } else { 0 } } } @@ -6736,26 +6746,32 @@ mod tests { }; scorer_params.set_manual_penalty(&NodeId::from_pubkey(&nodes[3]), 123); scorer_params.set_manual_penalty(&NodeId::from_pubkey(&nodes[4]), 456); - assert_eq!(scorer.channel_penalty_msat(42, &NodeId::from_pubkey(&nodes[3]), &NodeId::from_pubkey(&nodes[4]), usage, &scorer_params), 456); + let network_graph = network_graph.read_only(); + let channels = network_graph.channels(); + let channel = channels.get(&5).unwrap(); + let info = channel.as_directed_from(&NodeId::from_pubkey(&nodes[3])).unwrap(); + let candidate: CandidateRouteHop = CandidateRouteHop::PublicHop { + info: info.0, + short_channel_id: 5, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &scorer_params), 456); // Then check we can get a normal route let payment_params = PaymentParameters::from_node_id(nodes[10], 42); let route_params = RouteParameters::from_payment_params_and_value( payment_params, 100); - let route = get_route(&our_id, &route_params, &network_graph.read_only(), None, + let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes); assert!(route.is_ok()); // Then check that we can't get a route if we ban an intermediate node. scorer_params.add_banned(&NodeId::from_pubkey(&nodes[3])); - let route = get_route(&our_id, &route_params, &network_graph.read_only(), None, - Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes); + let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes); assert!(route.is_err()); // Finally make sure we can route again, when we remove the ban. scorer_params.remove_banned(&NodeId::from_pubkey(&nodes[3])); - let route = get_route(&our_id, &route_params, &network_graph.read_only(), None, - Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes); + let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes); assert!(route.is_ok()); } diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index de40b295e..5d637a859 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -58,7 +58,7 @@ use crate::ln::msgs::DecodeError; use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId}; -use crate::routing::router::Path; +use crate::routing::router::{Path, CandidateRouteHop}; use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer}; use crate::util::logger::Logger; use crate::util::time::Time; @@ -103,7 +103,7 @@ pub trait ScoreLookUp { /// [`u64::max_value`] is given to indicate sufficient capacity for the invoice's full amount. /// Thus, implementations should be overflow-safe. fn channel_penalty_msat( - &self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams + &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams ) -> u64; } @@ -137,9 +137,9 @@ impl Score for T {} impl> ScoreLookUp for T { type ScoreParams = S::ScoreParams; fn channel_penalty_msat( - &self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams + &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams ) -> u64 { - self.deref().channel_penalty_msat(short_channel_id, source, target, usage, score_params) + self.deref().channel_penalty_msat(candidate, usage, score_params) } } @@ -315,10 +315,9 @@ impl<'a, T: 'a + Score> Deref for MultiThreadedScoreLockRead<'a, T> { #[cfg(c_bindings)] impl<'a, T: Score> ScoreLookUp for MultiThreadedScoreLockRead<'a, T> { type ScoreParams = T::ScoreParams; - fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, - target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams + fn channel_penalty_msat(&self, candidate:&CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams ) -> u64 { - self.0.channel_penalty_msat(short_channel_id, source, target, usage, score_params) + self.0.channel_penalty_msat(candidate, usage, score_params) } } @@ -394,7 +393,7 @@ impl FixedPenaltyScorer { impl ScoreLookUp for FixedPenaltyScorer { type ScoreParams = (); - fn channel_penalty_msat(&self, _: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params: &Self::ScoreParams) -> u64 { + fn channel_penalty_msat(&self, _: &CandidateRouteHop, _: ChannelUsage, _score_params: &Self::ScoreParams) -> u64 { self.penalty_msat } } @@ -1343,9 +1342,18 @@ impl, BRT: DerefMut>, L: Deref, T: Time> ScoreLookUp for ProbabilisticScorerUsingTime where L::Target: Logger { type ScoreParams = ProbabilisticScoringFeeParameters; fn channel_penalty_msat( - &self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters + &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters ) -> u64 { - if let Some(penalty) = score_params.manual_node_penalties.get(target) { + let scid = match candidate.short_channel_id() { + Some(scid) => scid, + None => return 0, + }; + let target = match candidate.target() { + Some(target) => target, + None => return 0, + }; + let source = candidate.source(); + if let Some(penalty) = score_params.manual_node_penalties.get(&target) { return *penalty; } @@ -1375,9 +1383,9 @@ impl>, L: Deref, T: Time> ScoreLookUp for Prob let amount_msat = usage.amount_msat.saturating_add(usage.inflight_htlc_msat); let capacity_msat = usage.effective_capacity.as_msat(); self.channel_liquidities - .get(&short_channel_id) + .get(&scid) .unwrap_or(&ChannelLiquidity::new()) - .as_directed(source, target, capacity_msat, self.decay_params) + .as_directed(&source, &target, capacity_msat, self.decay_params) .penalty_msat(amount_msat, score_params) .saturating_add(anti_probing_penalty_msat) .saturating_add(base_penalty_msat) @@ -2174,7 +2182,7 @@ mod tests { use crate::ln::channelmanager; use crate::ln::msgs::{ChannelAnnouncement, ChannelUpdate, UnsignedChannelAnnouncement, UnsignedChannelUpdate}; use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId}; - use crate::routing::router::{BlindedTail, Path, RouteHop}; + use crate::routing::router::{BlindedTail, Path, RouteHop, CandidateRouteHop}; use crate::routing::scoring::{ChannelUsage, ScoreLookUp, ScoreUpdate}; use crate::util::ser::{ReadableArgs, Writeable}; use crate::util::test_utils::{self, TestLogger}; @@ -2541,39 +2549,45 @@ mod tests { let decay_params = ProbabilisticScoringDecayParameters::default(); let scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 1_024, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024_000, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + let network_graph = network_graph.read_only(); + let channel = network_graph.channel(42).unwrap(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 10_240, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 102_400, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 47); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 47); let usage = ChannelUsage { amount_msat: 1_023_999, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 2_000); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 2_000); let usage = ChannelUsage { amount_msat: 128, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 58); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 58); let usage = ChannelUsage { amount_msat: 256, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 125); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 125); let usage = ChannelUsage { amount_msat: 374, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 198); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 198); let usage = ChannelUsage { amount_msat: 512, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); let usage = ChannelUsage { amount_msat: 640, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 425); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 425); let usage = ChannelUsage { amount_msat: 768, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 602); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 602); let usage = ChannelUsage { amount_msat: 896, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 902); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 902); } #[test] @@ -2597,19 +2611,24 @@ mod tests { max_liquidity_offset_history: HistoricalBucketRangeTracker::new(), }); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 39, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 100, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 50, ..usage }; - assert_ne!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); - assert_ne!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_ne!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); + assert_ne!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); let usage = ChannelUsage { amount_msat: 61, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); } #[test] @@ -2621,7 +2640,6 @@ mod tests { ..ProbabilisticScoringFeeParameters::zero_penalty() }; let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - let sender = sender_node_id(); let source = source_node_id(); let usage = ChannelUsage { amount_msat: 500, @@ -2630,14 +2648,20 @@ mod tests { }; let failed_path = payment_path_for_amount(500); let successful_path = payment_path_for_amount(200); + let channel = &network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 41, + }; - assert_eq!(scorer.channel_penalty_msat(41, &sender, &source, usage, ¶ms), 301); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 301); scorer.payment_path_failed(&failed_path, 41); - assert_eq!(scorer.channel_penalty_msat(41, &sender, &source, usage, ¶ms), 301); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 301); scorer.payment_path_successful(&successful_path); - assert_eq!(scorer.channel_penalty_msat(41, &sender, &source, usage, ¶ms), 301); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 301); } #[test] @@ -2650,7 +2674,6 @@ mod tests { }; let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let path = payment_path_for_amount(500); let usage = ChannelUsage { @@ -2658,20 +2681,26 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_000, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 128); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 128); let usage = ChannelUsage { amount_msat: 500, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 301); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 301); let usage = ChannelUsage { amount_msat: 750, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 602); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 602); scorer.payment_path_failed(&path, 43); let usage = ChannelUsage { amount_msat: 250, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 500, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 750, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); } #[test] @@ -2685,7 +2714,6 @@ mod tests { }; let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let path = payment_path_for_amount(500); let usage = ChannelUsage { @@ -2693,20 +2721,26 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_000, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 128); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 128); let usage = ChannelUsage { amount_msat: 500, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 301); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 301); let usage = ChannelUsage { amount_msat: 750, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 602); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 602); scorer.payment_path_failed(&path, 42); let usage = ChannelUsage { amount_msat: 250, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); let usage = ChannelUsage { amount_msat: 500, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); let usage = ChannelUsage { amount_msat: 750, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); } #[test] @@ -2741,7 +2775,6 @@ mod tests { let node_a = NodeId::from_pubkey(&pub_a); let node_b = NodeId::from_pubkey(&pub_b); let node_c = NodeId::from_pubkey(&pub_c); - let node_d = NodeId::from_pubkey(&pub_d); let params = ProbabilisticScoringFeeParameters { liquidity_penalty_multiplier_msat: 1_000, @@ -2754,17 +2787,53 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_000, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &node_a, &node_b, usage, ¶ms), 128); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&node_a).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 128); // Note that a default liquidity bound is used for B -> C as no channel exists - assert_eq!(scorer.channel_penalty_msat(43, &node_b, &node_c, usage, ¶ms), 128); - assert_eq!(scorer.channel_penalty_msat(44, &node_c, &node_d, usage, ¶ms), 128); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&node_b).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 43, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 128); + let channel = network_graph.read_only().channel(44).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&node_c).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 44, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 128); scorer.payment_path_failed(&Path { hops: path, blinded_tail: None }, 43); - assert_eq!(scorer.channel_penalty_msat(42, &node_a, &node_b, usage, ¶ms), 80); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&node_a).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 80); // Note that a default liquidity bound is used for B -> C as no channel exists - assert_eq!(scorer.channel_penalty_msat(43, &node_b, &node_c, usage, ¶ms), 128); - assert_eq!(scorer.channel_penalty_msat(44, &node_c, &node_d, usage, ¶ms), 128); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&node_b).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 43, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 128); + let channel = network_graph.read_only().channel(44).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&node_c).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 44, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 128); } #[test] @@ -2776,25 +2845,39 @@ mod tests { ..ProbabilisticScoringFeeParameters::zero_penalty() }; let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - let sender = sender_node_id(); let source = source_node_id(); - let target = target_node_id(); - let recipient = recipient_node_id(); let usage = ChannelUsage { amount_msat: 250, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_000, htlc_maximum_msat: 1_000 }, }; - - assert_eq!(scorer.channel_penalty_msat(41, &sender, &source, usage, ¶ms), 128); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 128); - assert_eq!(scorer.channel_penalty_msat(43, &target, &recipient, usage, ¶ms), 128); + let network_graph = network_graph.read_only().channels().clone(); + let channel_42 = network_graph.get(&42).unwrap(); + let channel_43 = network_graph.get(&43).unwrap(); + let (info, _) = channel_42.as_directed_from(&source).unwrap(); + let candidate_41 = CandidateRouteHop::PublicHop { + info, + short_channel_id: 41, + }; + let (info, target) = channel_42.as_directed_from(&source).unwrap(); + let candidate_42 = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + let (info, _) = channel_43.as_directed_from(&target).unwrap(); + let candidate_43 = CandidateRouteHop::PublicHop { + info, + short_channel_id: 43, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate_41, usage, ¶ms), 128); + assert_eq!(scorer.channel_penalty_msat(&candidate_42, usage, ¶ms), 128); + assert_eq!(scorer.channel_penalty_msat(&candidate_43, usage, ¶ms), 128); scorer.payment_path_successful(&payment_path_for_amount(500)); - assert_eq!(scorer.channel_penalty_msat(41, &sender, &source, usage, ¶ms), 128); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); - assert_eq!(scorer.channel_penalty_msat(43, &target, &recipient, usage, ¶ms), 300); + assert_eq!(scorer.channel_penalty_msat(&candidate_41, usage, ¶ms), 128); + assert_eq!(scorer.channel_penalty_msat(&candidate_42, usage, ¶ms), 300); + assert_eq!(scorer.channel_penalty_msat(&candidate_43, usage, ¶ms), 300); } #[test] @@ -2812,86 +2895,91 @@ mod tests { }; let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 0, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_024 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, target) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 1_023, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 2_000); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 2_000); scorer.payment_path_failed(&payment_path_for_amount(768), 42); scorer.payment_path_failed(&payment_path_for_amount(128), 43); // Initial penalties let usage = ChannelUsage { amount_msat: 128, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 256, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 93); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 93); let usage = ChannelUsage { amount_msat: 768, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 1_479); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 1_479); let usage = ChannelUsage { amount_msat: 896, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); // No decay SinceEpoch::advance(Duration::from_secs(4)); let usage = ChannelUsage { amount_msat: 128, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 256, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 93); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 93); let usage = ChannelUsage { amount_msat: 768, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 1_479); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 1_479); let usage = ChannelUsage { amount_msat: 896, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); // Half decay (i.e., three-quarter life) SinceEpoch::advance(Duration::from_secs(1)); let usage = ChannelUsage { amount_msat: 128, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 22); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 22); let usage = ChannelUsage { amount_msat: 256, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 106); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 106); let usage = ChannelUsage { amount_msat: 768, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 921); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 921); let usage = ChannelUsage { amount_msat: 896, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); // One decay (i.e., half life) SinceEpoch::advance(Duration::from_secs(5)); let usage = ChannelUsage { amount_msat: 64, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 128, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 34); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 34); let usage = ChannelUsage { amount_msat: 896, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 1_970); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 1_970); let usage = ChannelUsage { amount_msat: 960, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); // Fully decay liquidity lower bound. SinceEpoch::advance(Duration::from_secs(10 * 7)); let usage = ChannelUsage { amount_msat: 0, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 1, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 1_023, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 2_000); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 2_000); let usage = ChannelUsage { amount_msat: 1_024, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); // Fully decay liquidity upper bound. SinceEpoch::advance(Duration::from_secs(10)); let usage = ChannelUsage { amount_msat: 0, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 1_024, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); SinceEpoch::advance(Duration::from_secs(10)); let usage = ChannelUsage { amount_msat: 0, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); let usage = ChannelUsage { amount_msat: 1_024, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); } #[test] @@ -2908,24 +2996,29 @@ mod tests { }; let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 256, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 125); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, target) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 125); scorer.payment_path_failed(&payment_path_for_amount(512), 42); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 281); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 281); // An unchecked right shift 64 bits or more in DirectedChannelLiquidity::decayed_offset_msat // would cause an overflow. SinceEpoch::advance(Duration::from_secs(10 * 64)); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 125); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 125); SinceEpoch::advance(Duration::from_secs(10)); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 125); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 125); } #[test] @@ -2942,37 +3035,42 @@ mod tests { }; let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 512, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_000 }, }; + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); // More knowledge gives higher confidence (256, 768), meaning a lower penalty. scorer.payment_path_failed(&payment_path_for_amount(768), 42); scorer.payment_path_failed(&payment_path_for_amount(256), 43); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 281); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 281); // Decaying knowledge gives less confidence (128, 896), meaning a higher penalty. SinceEpoch::advance(Duration::from_secs(10)); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 291); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 291); // Reducing the upper bound gives more confidence (128, 832) that the payment amount (512) // is closer to the upper bound, meaning a higher penalty. scorer.payment_path_successful(&payment_path_for_amount(64)); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 331); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 331); // Increasing the lower bound gives more confidence (256, 832) that the payment amount (512) // is closer to the lower bound, meaning a lower penalty. scorer.payment_path_failed(&payment_path_for_amount(256), 43); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 245); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 245); // Further decaying affects the lower bound more than the upper bound (128, 928). SinceEpoch::advance(Duration::from_secs(10)); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 280); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 280); } #[test] @@ -2990,7 +3088,6 @@ mod tests { }; let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 500, inflight_htlc_msat: 0, @@ -2998,13 +3095,19 @@ mod tests { }; scorer.payment_path_failed(&payment_path_for_amount(500), 42); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); SinceEpoch::advance(Duration::from_secs(10)); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 473); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 473); scorer.payment_path_failed(&payment_path_for_amount(250), 43); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); let mut serialized_scorer = Vec::new(); scorer.write(&mut serialized_scorer).unwrap(); @@ -3012,7 +3115,7 @@ mod tests { let mut serialized_scorer = io::Cursor::new(&serialized_scorer); let deserialized_scorer = ::read(&mut serialized_scorer, (decay_params, &network_graph, &logger)).unwrap(); - assert_eq!(deserialized_scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + assert_eq!(deserialized_scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); } #[test] @@ -3030,7 +3133,6 @@ mod tests { }; let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 500, inflight_htlc_msat: 0, @@ -3038,7 +3140,13 @@ mod tests { }; scorer.payment_path_failed(&payment_path_for_amount(500), 42); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); let mut serialized_scorer = Vec::new(); scorer.write(&mut serialized_scorer).unwrap(); @@ -3048,13 +3156,13 @@ mod tests { let mut serialized_scorer = io::Cursor::new(&serialized_scorer); let deserialized_scorer = ::read(&mut serialized_scorer, (decay_params, &network_graph, &logger)).unwrap(); - assert_eq!(deserialized_scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 473); + assert_eq!(deserialized_scorer.channel_penalty_msat(&candidate, usage, ¶ms), 473); scorer.payment_path_failed(&payment_path_for_amount(250), 43); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); SinceEpoch::advance(Duration::from_secs(10)); - assert_eq!(deserialized_scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 370); + assert_eq!(deserialized_scorer.channel_penalty_msat(&candidate, usage, ¶ms), 370); } #[test] @@ -3066,54 +3174,59 @@ mod tests { let params = ProbabilisticScoringFeeParameters::default(); let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 100_000_000, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 950_000_000, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 11497); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 11497); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 7408); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 7408); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 2_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 6151); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 6151); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 3_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 5427); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 5427); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 4_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 4955); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 4955); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 5_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 4736); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 4736); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 6_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 4484); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 4484); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 7_450_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 4484); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 4484); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 7_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 4263); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 4263); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 8_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 4263); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 4263); let usage = ChannelUsage { effective_capacity: EffectiveCapacity::Total { capacity_msat: 9_950_000_000, htlc_maximum_msat: 1_000 }, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 4044); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 4044); } #[test] @@ -3121,7 +3234,6 @@ mod tests { let logger = TestLogger::new(); let network_graph = network_graph(&logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 128, inflight_htlc_msat: 0, @@ -3133,14 +3245,20 @@ mod tests { ..ProbabilisticScoringFeeParameters::zero_penalty() }; let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 58); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 58); let params = ProbabilisticScoringFeeParameters { base_penalty_msat: 500, liquidity_penalty_multiplier_msat: 1_000, anti_probing_penalty_msat: 0, ..ProbabilisticScoringFeeParameters::zero_penalty() }; let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 558); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 558); let params = ProbabilisticScoringFeeParameters { base_penalty_msat: 500, liquidity_penalty_multiplier_msat: 1_000, @@ -3149,7 +3267,7 @@ mod tests { }; let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 558 + 128); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 558 + 128); } #[test] @@ -3157,7 +3275,6 @@ mod tests { let logger = TestLogger::new(); let network_graph = network_graph(&logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 512_000, inflight_htlc_msat: 0, @@ -3170,7 +3287,13 @@ mod tests { ..ProbabilisticScoringFeeParameters::zero_penalty() }; let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); let params = ProbabilisticScoringFeeParameters { liquidity_penalty_multiplier_msat: 1_000, @@ -3178,7 +3301,7 @@ mod tests { ..ProbabilisticScoringFeeParameters::zero_penalty() }; let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 337); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 337); } #[test] @@ -3186,20 +3309,24 @@ mod tests { let logger = TestLogger::new(); let network_graph = network_graph(&logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: u64::max_value(), inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Infinite, }; - let params = ProbabilisticScoringFeeParameters { liquidity_penalty_multiplier_msat: 40_000, ..ProbabilisticScoringFeeParameters::zero_penalty() }; let decay_params = ProbabilisticScoringDecayParameters::zero_penalty(); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; let scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 80_000); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 80_000); } #[test] @@ -3212,17 +3339,23 @@ mod tests { }; let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 750, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_000, htlc_maximum_msat: 1_000 }, }; - assert_ne!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + let network_graph = network_graph.read_only(); + let channel = network_graph.channel(42).unwrap(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_ne!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); let usage = ChannelUsage { inflight_htlc_msat: 251, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); } #[test] @@ -3232,7 +3365,6 @@ mod tests { let params = ProbabilisticScoringFeeParameters::default(); let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let base_penalty_msat = params.base_penalty_msat; let usage = ChannelUsage { @@ -3240,16 +3372,22 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::ExactLiquidity { liquidity_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), base_penalty_msat); + let network_graph = network_graph.read_only(); + let channel = network_graph.channel(42).unwrap(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), base_penalty_msat); let usage = ChannelUsage { amount_msat: 1_000, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), base_penalty_msat); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), base_penalty_msat); let usage = ChannelUsage { amount_msat: 1_001, ..usage }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), u64::max_value()); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), u64::max_value()); } - #[test] fn remembers_historical_failures() { let logger = TestLogger::new(); let network_graph = network_graph(&logger); @@ -3264,7 +3402,6 @@ mod tests { }; let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 100, @@ -3276,17 +3413,24 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_024 }, }; + let network_graph = network_graph.read_only(); + let channel = network_graph.channel(42).unwrap(); + let (info, target) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; // With no historical data the normal liquidity penalty calculation is used. - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 168); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 168); assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target), - None); + None); assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 42, ¶ms), - None); + None); scorer.payment_path_failed(&payment_path_for_amount(1), 42); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 2048); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage_1, ¶ms), 249); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 2048); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage_1, ¶ms), 249); // The "it failed" increment is 32, where the probability should lie several buckets into // the first octile. assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target), @@ -3300,7 +3444,7 @@ mod tests { // Even after we tell the scorer we definitely have enough available liquidity, it will // still remember that there was some failure in the past, and assign a non-0 penalty. scorer.payment_path_failed(&payment_path_for_amount(1000), 43); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 105); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 105); // The first points should be decayed just slightly and the last bucket has a new point. assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target), Some(([31, 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, 32, 0, 0, 0, 0, 0], @@ -3320,7 +3464,7 @@ mod tests { // Advance the time forward 16 half-lives (which the docs claim will ensure all data is // gone), and check that we're back to where we started. SinceEpoch::advance(Duration::from_secs(10 * 16)); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 168); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 168); // Once fully decayed we still have data, but its all-0s. In the future we may remove the // data entirely instead. assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target), @@ -3333,16 +3477,16 @@ mod tests { effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_024 }, }; scorer.payment_path_failed(&payment_path_for_amount(1), 42); - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 2050); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 2050); usage.inflight_htlc_msat = 0; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 866); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 866); let usage = ChannelUsage { amount_msat: 1, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::AdvertisedMaxHTLC { amount_msat: 0 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &target, &source, usage, ¶ms), 2048); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 2048); // Advance to decay all liquidity offsets to zero. SinceEpoch::advance(Duration::from_secs(60 * 60 * 10)); @@ -3362,7 +3506,6 @@ mod tests { let logger = TestLogger::new(); let network_graph = network_graph(&logger); let source = source_node_id(); - let target = target_node_id(); let params = ProbabilisticScoringFeeParameters { anti_probing_penalty_msat: 500, ..ProbabilisticScoringFeeParameters::zero_penalty() @@ -3375,7 +3518,14 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024_000, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + let network_graph = network_graph.read_only(); + let channel = network_graph.channel(42).unwrap(); + let (info, _) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); // Check we receive anti-probing penalty for htlc_maximum_msat == channel_capacity. let usage = ChannelUsage { @@ -3383,7 +3533,7 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024_000, htlc_maximum_msat: 1_024_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 500); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 500); // Check we receive anti-probing penalty for htlc_maximum_msat == channel_capacity/2. let usage = ChannelUsage { @@ -3391,7 +3541,7 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024_000, htlc_maximum_msat: 512_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 500); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 500); // Check we receive no anti-probing penalty for htlc_maximum_msat == channel_capacity/2 - 1. let usage = ChannelUsage { @@ -3399,7 +3549,7 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024_000, htlc_maximum_msat: 511_999 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 0); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 0); } #[test] @@ -3414,13 +3564,18 @@ mod tests { let decay_params = ProbabilisticScoringDecayParameters::default(); let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let usage = ChannelUsage { amount_msat: 512, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_000 }, }; - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 300); + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, target) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 300); let mut path = payment_path_for_amount(768); let recipient_hop = path.hops.pop().unwrap(); @@ -3476,7 +3631,6 @@ mod tests { let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger); let source = source_node_id(); - let target = target_node_id(); let mut amount_msat = 10_000_000; let usage = ChannelUsage { @@ -3484,9 +3638,15 @@ mod tests { inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: capacity_msat }, }; + let channel = network_graph.read_only().channel(42).unwrap().to_owned(); + let (info, target) = channel.as_directed_from(&source).unwrap(); + let candidate = CandidateRouteHop::PublicHop { + info, + short_channel_id: 42, + }; // With no historical data the normal liquidity penalty calculation is used, which results // in a success probability of ~75%. - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), 1269); + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 1269); assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target), None); assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 42, ¶ms), @@ -3496,7 +3656,7 @@ mod tests { scorer.payment_path_failed(&payment_path_for_amount(amount_msat), 42); // The penalty should be the maximum penalty, as the payment we're scoring is now in the // same bucket which is the only maximum datapoint. - assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, ¶ms), + assert_eq!(scorer.channel_penalty_msat(&candidate, usage, ¶ms), 2048 + 2048 * amount_msat / super::AMOUNT_PENALTY_DIVISOR); // The "it failed" increment is 32, which we should apply to the first upper-bound (between // 6k sats and 12k sats). diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index a6c2d7745..9bbbec0d7 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -17,6 +17,7 @@ use crate::chain::chainmonitor::{MonitorUpdateId, UpdateOrigin}; use crate::chain::channelmonitor; use crate::chain::channelmonitor::MonitorEvent; use crate::chain::transaction::OutPoint; +use crate::routing::router::CandidateRouteHop; use crate::sign; use crate::events; use crate::events::bump_transaction::{WalletSource, Utxo}; @@ -139,10 +140,34 @@ impl<'a> Router for TestRouter<'a> { // Since the path is reversed, the last element in our iteration is the first // hop. if idx == path.hops.len() - 1 { - scorer.channel_penalty_msat(hop.short_channel_id, &NodeId::from_pubkey(payer), &NodeId::from_pubkey(&hop.pubkey), usage, &Default::default()); + let first_hops = match first_hops { + Some(hops) => hops, + None => continue, + }; + if first_hops.len() == 0 { + continue; + } + let idx = if first_hops.len() > 1 { route.paths.iter().position(|p| p == path).unwrap_or(0) } else { 0 }; + let candidate = CandidateRouteHop::FirstHop { + details: first_hops[idx], + node_id: NodeId::from_pubkey(payer) + }; + scorer.channel_penalty_msat(&candidate, usage, &()); } else { - let curr_hop_path_idx = path.hops.len() - 1 - idx; - scorer.channel_penalty_msat(hop.short_channel_id, &NodeId::from_pubkey(&path.hops[curr_hop_path_idx - 1].pubkey), &NodeId::from_pubkey(&hop.pubkey), usage, &Default::default()); + let network_graph = self.network_graph.read_only(); + let channel = match network_graph.channel(hop.short_channel_id) { + Some(channel) => channel, + None => continue, + }; + let channel = match channel.as_directed_to(&NodeId::from_pubkey(&hop.pubkey)) { + Some(channel) => channel, + None => panic!("Channel directed to {} was not found", hop.pubkey), + }; + let candidate = CandidateRouteHop::PublicHop { + info: channel.0, + short_channel_id: hop.short_channel_id, + }; + scorer.channel_penalty_msat(&candidate, usage, &()); } } } @@ -1297,8 +1322,12 @@ impl crate::util::ser::Writeable for TestScorer { impl ScoreLookUp for TestScorer { type ScoreParams = (); fn channel_penalty_msat( - &self, short_channel_id: u64, _source: &NodeId, _target: &NodeId, usage: ChannelUsage, _score_params: &Self::ScoreParams + &self, candidate: &CandidateRouteHop, usage: ChannelUsage, _score_params: &Self::ScoreParams ) -> u64 { + let short_channel_id = match candidate.short_channel_id() { + Some(scid) => scid, + None => return 0, + }; if let Some(scorer_expectations) = self.scorer_expectations.borrow_mut().as_mut() { match scorer_expectations.pop_front() { Some((scid, expectation)) => { From cbde4a756e45939c5f5b18434d5480b6baf0c4ba Mon Sep 17 00:00:00 2001 From: jbesraa Date: Mon, 20 Nov 2023 15:44:50 +0200 Subject: [PATCH 6/6] Remove `node_id` from `PathBuildingHop` We remove `node_id` from `PathBuildingHop` as we can rely `CandidateRouteHop::target` instead. --- lightning/src/routing/router.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index c33a0d36d..b4cbf3cb2 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1247,9 +1247,6 @@ fn iter_equal(mut iter_a: I1, mut iter_b: I2) /// These fee values are useful to choose hops as we traverse the graph "payee-to-payer". #[derive(Clone)] struct PathBuildingHop<'a> { - // Note that this should be dropped in favor of loading it from CandidateRouteHop, but doing so - // is a larger refactor and will require careful performance analysis. - node_id: NodeId, candidate: CandidateRouteHop<'a>, fee_msat: u64, @@ -1287,7 +1284,7 @@ impl<'a> core::fmt::Debug for PathBuildingHop<'a> { fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { let mut debug_struct = f.debug_struct("PathBuildingHop"); debug_struct - .field("node_id", &self.node_id) + .field("node_id", &self.candidate.target()) .field("short_channel_id", &self.candidate.short_channel_id()) .field("total_fee_msat", &self.total_fee_msat) .field("next_hops_fee_msat", &self.next_hops_fee_msat) @@ -1824,8 +1821,7 @@ where L::Target: Logger { // - for regular channels at channel announcement (TODO) // - for first and last hops early in get_route let src_node_id = $candidate.source(); - let dest_node_id = $candidate.target().unwrap_or(maybe_dummy_payee_node_id); - if src_node_id != dest_node_id { + if Some(src_node_id) != $candidate.target() { let scid_opt = $candidate.short_channel_id(); let effective_capacity = $candidate.effective_capacity(); let htlc_maximum_msat = max_htlc_from_capacity(effective_capacity, channel_saturation_pow_half); @@ -1964,7 +1960,6 @@ where L::Target: Logger { // This will affect our decision on selecting short_channel_id // as a way to reach the $candidate.target() node. PathBuildingHop { - node_id: dest_node_id.clone(), candidate: $candidate.clone(), fee_msat: 0, next_hops_fee_msat: u64::max_value(), @@ -2063,7 +2058,6 @@ where L::Target: Logger { old_entry.next_hops_fee_msat = $next_hops_fee_msat; old_entry.hop_use_fee_msat = hop_use_fee_msat; old_entry.total_fee_msat = total_fee_msat; - old_entry.node_id = dest_node_id; old_entry.candidate = $candidate.clone(); old_entry.fee_msat = 0; // This value will be later filled with hop_use_fee_msat of the following channel old_entry.path_htlc_minimum_msat = path_htlc_minimum_msat; @@ -2435,7 +2429,8 @@ where L::Target: Logger { 'path_walk: loop { let mut features_set = false; - if let Some(first_channels) = first_hop_targets.get(&ordered_hops.last().unwrap().0.node_id) { + let target = ordered_hops.last().unwrap().0.candidate.target().unwrap_or(maybe_dummy_payee_node_id); + if let Some(first_channels) = first_hop_targets.get(&target) { for details in first_channels { if let Some(scid) = ordered_hops.last().unwrap().0.candidate.short_channel_id() { if details.get_outbound_payment_scid().unwrap() == scid { @@ -2447,7 +2442,7 @@ where L::Target: Logger { } } if !features_set { - if let Some(node) = network_nodes.get(&ordered_hops.last().unwrap().0.node_id) { + if let Some(node) = network_nodes.get(&target) { if let Some(node_info) = node.announcement_info.as_ref() { ordered_hops.last_mut().unwrap().1 = node_info.features.clone(); } else { @@ -2464,11 +2459,11 @@ where L::Target: Logger { // save this path for the payment route. Also, update the liquidity // remaining on the used hops, so that we take them into account // while looking for more paths. - if ordered_hops.last().unwrap().0.node_id == maybe_dummy_payee_node_id { + if target == maybe_dummy_payee_node_id { break 'path_walk; } - new_entry = match dist.remove(&ordered_hops.last().unwrap().0.node_id) { + new_entry = match dist.remove(&target) { Some(payment_hop) => payment_hop, // We can't arrive at None because, if we ever add an entry to targets, // we also fill in the entry in dist (see add_entry!). @@ -2687,8 +2682,8 @@ where L::Target: Logger { }); for idx in 0..(selected_route.len() - 1) { if idx + 1 >= selected_route.len() { break; } - if iter_equal(selected_route[idx].hops.iter().map(|h| (h.0.candidate.id(), h.0.node_id)), - selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.id(), h.0.node_id))) { + if iter_equal(selected_route[idx].hops.iter().map(|h| (h.0.candidate.id(), h.0.candidate.target())), + selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.id(), h.0.candidate.target()))) { let new_value = selected_route[idx].get_value_msat() + selected_route[idx + 1].get_value_msat(); selected_route[idx].update_value_and_recompute_fees(new_value); selected_route.remove(idx + 1); @@ -2701,6 +2696,7 @@ where L::Target: Logger { for (hop, node_features) in payment_path.hops.iter() .filter(|(h, _)| h.candidate.short_channel_id().is_some()) { + let target = hop.candidate.target().expect("target is defined when short_channel_id is defined"); let maybe_announced_channel = if let CandidateRouteHop::PublicHop { .. } = hop.candidate { // If we sourced the hop from the graph we're sure the target node is announced. true @@ -2712,14 +2708,14 @@ where L::Target: Logger { // there are announced channels between the endpoints. If so, the hop might be // referring to any of the announced channels, as its `short_channel_id` might be // an alias, in which case we don't take any chances here. - network_graph.node(&hop.node_id).map_or(false, |hop_node| + network_graph.node(&target).map_or(false, |hop_node| hop_node.channels.iter().any(|scid| network_graph.channel(*scid) .map_or(false, |c| c.as_directed_from(&hop.candidate.source()).is_some())) ) }; hops.push(RouteHop { - pubkey: PublicKey::from_slice(hop.node_id.as_slice()).map_err(|_| LightningError{err: format!("Public key {:?} is invalid", &hop.node_id), action: ErrorAction::IgnoreAndLog(Level::Trace)})?, + pubkey: PublicKey::from_slice(target.as_slice()).map_err(|_| LightningError{err: format!("Public key {:?} is invalid", &target), action: ErrorAction::IgnoreAndLog(Level::Trace)})?, node_features: node_features.clone(), short_channel_id: hop.candidate.short_channel_id().unwrap(), channel_features: hop.candidate.features(),