Add `source` and `target` fn's to `CandidateRouteHop`
authorjbesraa <jbesraa@gmail.com>
Sat, 26 Aug 2023 15:06:45 +0000 (18:06 +0300)
committerjbesraa <jbesraa@gmail.com>
Tue, 5 Dec 2023 19:07:46 +0000 (21:07 +0200)
    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

index 395731502cb06b2b8dce9667674514703f126b16..361d5793edcc5e27980b34b93737ce608d6f2103 100644 (file)
@@ -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<u64> {
                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<NodeId> {
+               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, _)| {