Multi-hop blinded paths in ChannelManager
[rust-lightning] / lightning / src / routing / router.rs
index 6138b87bc9a8a72a349af84d7711681043b4c408..42eeb6cb136ee6a1747ece85b3f8fd929572f0c0 100644 (file)
@@ -9,7 +9,7 @@
 
 //! The router finds paths within a [`NetworkGraph`] for a payment.
 
-use bitcoin::secp256k1::PublicKey;
+use bitcoin::secp256k1::{PublicKey, Secp256k1, self};
 use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
 
@@ -19,8 +19,10 @@ use crate::ln::channelmanager::{ChannelDetails, PaymentId};
 use crate::ln::features::{Bolt11InvoiceFeatures, Bolt12InvoiceFeatures, ChannelFeatures, NodeFeatures};
 use crate::ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
 use crate::offers::invoice::{BlindedPayInfo, Bolt12Invoice};
+use crate::onion_message::{DefaultMessageRouter, Destination, MessageRouter, OnionMessagePath};
 use crate::routing::gossip::{DirectedChannelInfo, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId, RoutingFees};
 use crate::routing::scoring::{ChannelUsage, LockableScore, ScoreLookUp};
+use crate::sign::EntropySource;
 use crate::util::ser::{Writeable, Readable, ReadableArgs, Writer};
 use crate::util::logger::{Level, Logger};
 use crate::util::chacha20::ChaCha20;
@@ -33,7 +35,7 @@ use core::{cmp, fmt};
 use core::ops::Deref;
 
 /// A [`Router`] implemented using [`find_route`].
-pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> where
+pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>> + Clone, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> where
        L::Target: Logger,
        S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>,
 {
@@ -41,21 +43,23 @@ pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref,
        logger: L,
        random_seed_bytes: Mutex<[u8; 32]>,
        scorer: S,
-       score_params: SP
+       score_params: SP,
+       message_router: DefaultMessageRouter<G, L>,
 }
 
-impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> DefaultRouter<G, L, S, SP, Sc> where
+impl<G: Deref<Target = NetworkGraph<L>> + Clone, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> DefaultRouter<G, L, S, SP, Sc> where
        L::Target: Logger,
        S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>,
 {
        /// Creates a new router.
        pub fn new(network_graph: G, logger: L, random_seed_bytes: [u8; 32], scorer: S, score_params: SP) -> Self {
                let random_seed_bytes = Mutex::new(random_seed_bytes);
-               Self { network_graph, logger, random_seed_bytes, scorer, score_params }
+               let message_router = DefaultMessageRouter::new(network_graph.clone());
+               Self { network_graph, logger, random_seed_bytes, scorer, score_params, message_router }
        }
 }
 
-impl< G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> Router for DefaultRouter<G, L, S, SP, Sc> where
+impl<G: Deref<Target = NetworkGraph<L>> + Clone, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> Router for DefaultRouter<G, L, S, SP, Sc> where
        L::Target: Logger,
        S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>,
 {
@@ -80,8 +84,28 @@ impl< G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: Sco
        }
 }
 
+impl< G: Deref<Target = NetworkGraph<L>> + Clone, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> MessageRouter for DefaultRouter<G, L, S, SP, Sc> where
+       L::Target: Logger,
+       S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>,
+{
+       fn find_path(
+               &self, sender: PublicKey, peers: Vec<PublicKey>, destination: Destination
+       ) -> Result<OnionMessagePath, ()> {
+               self.message_router.find_path(sender, peers, destination)
+       }
+
+       fn create_blinded_paths<
+               ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification
+       >(
+               &self, recipient: PublicKey, peers: Vec<PublicKey>, entropy_source: &ES,
+               secp_ctx: &Secp256k1<T>
+       ) -> Result<Vec<BlindedPath>, ()> {
+               self.message_router.create_blinded_paths(recipient, peers, entropy_source, secp_ctx)
+       }
+}
+
 /// A trait defining behavior for routing a payment.
-pub trait Router {
+pub trait Router: MessageRouter {
        /// Finds a [`Route`] for a payment between the given `payer` and a payee.
        ///
        /// The `payee` and the payment's value are given in [`RouteParameters::payment_params`]
@@ -968,33 +992,24 @@ impl_writeable_tlv_based!(RouteHintHop, {
 });
 
 #[derive(Eq, PartialEq)]
+#[repr(align(64))] // Force the size to 64 bytes
 struct RouteGraphNode {
        node_id: NodeId,
-       lowest_fee_to_node: u64,
-       total_cltv_delta: u32,
+       score: u64,
        // The maximum value a yet-to-be-constructed payment path might flow through this node.
        // This value is upper-bounded by us by:
        // - how much is needed for a path being constructed
        // - how much value can channels following this node (up to the destination) can contribute,
        //   considering their capacity and fees
        value_contribution_msat: u64,
-       /// The effective htlc_minimum_msat at this hop. If a later hop on the path had a higher HTLC
-       /// minimum, we use it, plus the fees required at each earlier hop to meet it.
-       path_htlc_minimum_msat: u64,
-       /// All penalties incurred from this hop on the way to the destination, as calculated using
-       /// channel scoring.
-       path_penalty_msat: u64,
+       total_cltv_delta: u32,
        /// The number of hops walked up to this node.
        path_length_to_node: u8,
 }
 
 impl cmp::Ord for RouteGraphNode {
        fn cmp(&self, other: &RouteGraphNode) -> cmp::Ordering {
-               let other_score = cmp::max(other.lowest_fee_to_node, other.path_htlc_minimum_msat)
-                       .saturating_add(other.path_penalty_msat);
-               let self_score = cmp::max(self.lowest_fee_to_node, self.path_htlc_minimum_msat)
-                       .saturating_add(self.path_penalty_msat);
-               other_score.cmp(&self_score).then_with(|| other.node_id.cmp(&self.node_id))
+               other.score.cmp(&self.score).then_with(|| other.node_id.cmp(&self.node_id))
        }
 }
 
@@ -1004,6 +1019,16 @@ impl cmp::PartialOrd for RouteGraphNode {
        }
 }
 
+// While RouteGraphNode can be laid out with fewer bytes, performance appears to be improved
+// substantially when it is laid out at exactly 64 bytes.
+//
+// Thus, we use `#[repr(C)]` on the struct to force a suboptimal layout and check that it stays 64
+// bytes here.
+#[cfg(any(ldk_bench, not(any(test, fuzzing))))]
+const _GRAPH_NODE_SMALL: usize = 64 - core::mem::size_of::<RouteGraphNode>();
+#[cfg(any(ldk_bench, not(any(test, fuzzing))))]
+const _GRAPH_NODE_FIXED_SIZE: usize = core::mem::size_of::<RouteGraphNode>() - 64;
+
 /// A wrapper around the various hop representations.
 ///
 /// Can be used to examine the properties of a hop,
@@ -1020,7 +1045,7 @@ pub enum CandidateRouteHop<'a> {
                /// [`find_route`] validates this prior to constructing a [`CandidateRouteHop`].
                details: &'a ChannelDetails,
                /// The node id of the payer, which is also the source side of this candidate route hop.
-               node_id: NodeId,
+               payer_node_id: &'a NodeId,
        },
        /// A hop found in the [`ReadOnlyNetworkGraph`].
        PublicHop {
@@ -1039,7 +1064,7 @@ pub enum CandidateRouteHop<'a> {
                /// Information about the private hop communicated via BOLT 11.
                hint: &'a RouteHintHop,
                /// Node id of the next hop in BOLT 11 route hint.
-               target_node_id: NodeId
+               target_node_id: &'a NodeId
        },
        /// A blinded path which starts with an introduction point and ultimately terminates with the
        /// payee.
@@ -1138,7 +1163,11 @@ impl<'a> CandidateRouteHop<'a> {
                }
        }
 
-       /// Returns cltv_expiry_delta for this hop.
+       /// Returns the required difference in HTLC CLTV expiry between the [`Self::source`] and the
+       /// next-hop for an HTLC taking this hop.
+       ///
+       /// This is the time that the node(s) in this hop have to claim the HTLC on-chain if the
+       /// next-hop goes on chain with a payment preimage.
        #[inline]
        pub fn cltv_expiry_delta(&self) -> u32 {
                match self {
@@ -1150,7 +1179,7 @@ impl<'a> CandidateRouteHop<'a> {
                }
        }
 
-       /// Returns the htlc_minimum_msat for this hop.
+       /// Returns the minimum amount that can be sent over this hop, in millisatoshis.
        #[inline]
        pub fn htlc_minimum_msat(&self) -> u64 {
                match self {
@@ -1162,7 +1191,7 @@ impl<'a> CandidateRouteHop<'a> {
                }
        }
 
-       /// Returns the fees for this hop.
+       /// Returns the fees that must be paid to route an HTLC over this channel.
        #[inline]
        pub fn fees(&self) -> RoutingFees {
                match self {
@@ -1182,6 +1211,10 @@ impl<'a> CandidateRouteHop<'a> {
                }
        }
 
+       /// Fetch the effective capacity of this hop.
+       ///
+       /// Note that this may be somewhat expensive, so calls to this should be limited and results
+       /// cached!
        fn effective_capacity(&self) -> EffectiveCapacity {
                match self {
                        CandidateRouteHop::FirstHop { details, .. } => EffectiveCapacity::ExactLiquidity {
@@ -1219,13 +1252,13 @@ impl<'a> CandidateRouteHop<'a> {
        }
        /// Returns the source node id of current hop.
        ///
-       /// Source node id refers to the hop forwarding the payment.
+       /// Source node id refers to the node forwarding the HTLC through this hop.
        ///
-       /// For `FirstHop` we return payer's node id.
+       /// For [`Self::FirstHop`] we return payer's node id.
        #[inline]
        pub fn source(&self) -> NodeId {
                match self {
-                       CandidateRouteHop::FirstHop { node_id, .. } => *node_id,
+                       CandidateRouteHop::FirstHop { payer_node_id, .. } => **payer_node_id,
                        CandidateRouteHop::PublicHop { info, .. } => *info.source(),
                        CandidateRouteHop::PrivateHop { hint, .. } => hint.src_node_id.into(),
                        CandidateRouteHop::Blinded { hint, .. } => hint.1.introduction_node_id.into(),
@@ -1234,15 +1267,19 @@ impl<'a> CandidateRouteHop<'a> {
        }
        /// Returns the target node id of this hop, if known.
        ///
-       /// Target node id refers to the hop receiving the payment.
+       /// Target node id refers to the node receiving the HTLC after this hop.
        ///
-       /// For `Blinded` and `OneHopBlinded` we return `None` because next hop is blinded.
+       /// For [`Self::Blinded`] we return `None` because the ultimate destination after the blinded
+       /// path is unknown.
+       ///
+       /// For [`Self::OneHopBlinded`] we return `None` because the target is the same as the source,
+       /// and such a return value would be somewhat nonsensical.
        #[inline]
        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::PrivateHop { target_node_id, .. } => Some(**target_node_id),
                        CandidateRouteHop::Blinded { .. } => None,
                        CandidateRouteHop::OneHopBlinded { .. } => None,
                }
@@ -1299,15 +1336,15 @@ fn iter_equal<I1: Iterator, I2: Iterator>(mut iter_a: I1, mut iter_b: I2)
 /// Fee values should be updated only in the context of the whole path, see update_value_and_recompute_fees.
 /// These fee values are useful to choose hops as we traverse the graph "payee-to-payer".
 #[derive(Clone)]
+#[repr(C)] // Force fields to appear in the order we define them.
 struct PathBuildingHop<'a> {
        candidate: CandidateRouteHop<'a>,
-       fee_msat: u64,
-
-       /// All the fees paid *after* this channel on the way to the destination
-       next_hops_fee_msat: u64,
-       /// Fee paid for the use of the current channel (see candidate.fees()).
-       /// The value will be actually deducted from the counterparty balance on the previous link.
-       hop_use_fee_msat: u64,
+       /// If we've already processed a node as the best node, we shouldn't process it again. Normally
+       /// we'd just ignore it if we did as all channels would have a higher new fee, but because we
+       /// may decrease the amounts in use as we walk the graph, the actual calculated fee may
+       /// decrease as well. Thus, we have to explicitly track which nodes have been processed and
+       /// avoid processing them again.
+       was_processed: bool,
        /// Used to compare channels when choosing the for routing.
        /// Includes paying for the use of a hop and the following hops, as well as
        /// an estimated cost of reaching this hop.
@@ -1319,12 +1356,20 @@ struct PathBuildingHop<'a> {
        /// All penalties incurred from this channel on the way to the destination, as calculated using
        /// channel scoring.
        path_penalty_msat: u64,
-       /// If we've already processed a node as the best node, we shouldn't process it again. Normally
-       /// we'd just ignore it if we did as all channels would have a higher new fee, but because we
-       /// may decrease the amounts in use as we walk the graph, the actual calculated fee may
-       /// decrease as well. Thus, we have to explicitly track which nodes have been processed and
-       /// avoid processing them again.
-       was_processed: bool,
+
+       // The last 16 bytes are on the next cache line by default in glibc's malloc. Thus, we should
+       // only place fields which are not hot there. Luckily, the next three fields are only read if
+       // we end up on the selected path, and only in the final path layout phase, so we don't care
+       // too much if reading them is slow.
+
+       fee_msat: u64,
+
+       /// All the fees paid *after* this channel on the way to the destination
+       next_hops_fee_msat: u64,
+       /// Fee paid for the use of the current channel (see candidate.fees()).
+       /// The value will be actually deducted from the counterparty balance on the previous link.
+       hop_use_fee_msat: u64,
+
        #[cfg(all(not(ldk_bench), any(test, fuzzing)))]
        // In tests, we apply further sanity checks on cases where we skip nodes we already processed
        // to ensure it is specifically in cases where the fee has gone down because of a decrease in
@@ -1333,6 +1378,18 @@ struct PathBuildingHop<'a> {
        value_contribution_msat: u64,
 }
 
+// Checks that the entries in the `find_route` `dist` map fit in (exactly) two standard x86-64
+// cache lines. Sadly, they're not guaranteed to actually lie on a cache line (and in fact,
+// generally won't, because at least glibc's malloc will align to a nice, big, round
+// boundary...plus 16), but at least it will reduce the amount of data we'll need to load.
+//
+// Note that these assertions only pass on somewhat recent rustc, and thus are gated on the
+// ldk_bench flag.
+#[cfg(ldk_bench)]
+const _NODE_MAP_SIZE_TWO_CACHE_LINES: usize = 128 - core::mem::size_of::<(NodeId, PathBuildingHop)>();
+#[cfg(ldk_bench)]
+const _NODE_MAP_SIZE_EXACTLY_CACHE_LINES: usize = core::mem::size_of::<(NodeId, PathBuildingHop)>() - 128;
+
 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");
@@ -1787,6 +1844,20 @@ where L::Target: Logger {
                }
        }
 
+       let mut private_hop_key_cache = HashMap::with_capacity(
+               payment_params.payee.unblinded_route_hints().iter().map(|path| path.0.len()).sum()
+       );
+
+       // Because we store references to private hop node_ids in `dist`, below, we need them to exist
+       // (as `NodeId`, not `PublicKey`) for the lifetime of `dist`. Thus, we calculate all the keys
+       // we'll need here and simply fetch them when routing.
+       private_hop_key_cache.insert(maybe_dummy_payee_pk, NodeId::from_pubkey(&maybe_dummy_payee_pk));
+       for route in payment_params.payee.unblinded_route_hints().iter() {
+               for hop in route.0.iter() {
+                       private_hop_key_cache.insert(hop.src_node_id, NodeId::from_pubkey(&hop.src_node_id));
+               }
+       }
+
        // The main heap containing all candidate next-hops sorted by their score (max(fee,
        // htlc_minimum)). Ideally this would be a heap which allowed cheap score reduction instead of
        // adding duplicate entries when we find a better path to a given node.
@@ -2074,15 +2145,6 @@ where L::Target: Logger {
                                                                                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,
-                                                                       lowest_fee_to_node: total_fee_msat,
-                                                                       total_cltv_delta: hop_total_cltv_delta,
-                                                                       value_contribution_msat,
-                                                                       path_htlc_minimum_msat,
-                                                                       path_penalty_msat,
-                                                                       path_length_to_node,
-                                                               };
 
                                                                // Update the way of reaching $candidate.source()
                                                                // with the given short_channel_id (from $candidate.target()),
@@ -2107,6 +2169,13 @@ where L::Target: Logger {
                                                                        .saturating_add(path_penalty_msat);
 
                                                                if !old_entry.was_processed && new_cost < old_cost {
+                                                                       let new_graph_node = RouteGraphNode {
+                                                                               node_id: src_node_id,
+                                                                               score: cmp::max(total_fee_msat, path_htlc_minimum_msat).saturating_add(path_penalty_msat),
+                                                                               total_cltv_delta: hop_total_cltv_delta,
+                                                                               value_contribution_msat,
+                                                                               path_length_to_node,
+                                                                       };
                                                                        targets.push(new_graph_node);
                                                                        old_entry.next_hops_fee_msat = $next_hops_fee_msat;
                                                                        old_entry.hop_use_fee_msat = hop_use_fee_msat;
@@ -2180,28 +2249,38 @@ where L::Target: Logger {
        // meaning how much will be paid in fees after this node (to the best of our knowledge).
        // This data can later be helpful to optimize routing (pay lower fees).
        macro_rules! add_entries_to_cheapest_to_target_node {
-               ( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr,
-                 $next_hops_path_htlc_minimum_msat: expr, $next_hops_path_penalty_msat: expr,
+               ( $node: expr, $node_id: expr, $next_hops_value_contribution: expr,
                  $next_hops_cltv_delta: expr, $next_hops_path_length: expr ) => {
+                       let fee_to_target_msat;
+                       let next_hops_path_htlc_minimum_msat;
+                       let next_hops_path_penalty_msat;
                        let skip_node = if let Some(elem) = dist.get_mut(&$node_id) {
                                let was_processed = elem.was_processed;
                                elem.was_processed = true;
+                               fee_to_target_msat = elem.total_fee_msat;
+                               next_hops_path_htlc_minimum_msat = elem.path_htlc_minimum_msat;
+                               next_hops_path_penalty_msat = elem.path_penalty_msat;
                                was_processed
                        } else {
                                // Entries are added to dist in add_entry!() when there is a channel from a node.
                                // Because there are no channels from payee, it will not have a dist entry at this point.
                                // If we're processing any other node, it is always be the result of a channel from it.
                                debug_assert_eq!($node_id, maybe_dummy_payee_node_id);
+                               fee_to_target_msat = 0;
+                               next_hops_path_htlc_minimum_msat = 0;
+                               next_hops_path_penalty_msat = 0;
                                false
                        };
 
                        if !skip_node {
                                if let Some(first_channels) = first_hop_targets.get(&$node_id) {
                                        for details in first_channels {
-                                               let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id };
-                                               add_entry!(&candidate, $fee_to_target_msat,
+                                               let candidate = CandidateRouteHop::FirstHop {
+                                                       details, payer_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_path_htlc_minimum_msat, next_hops_path_penalty_msat,
                                                        $next_hops_cltv_delta, $next_hops_path_length);
                                        }
                                }
@@ -2224,10 +2303,10 @@ where L::Target: Logger {
                                                                                        short_channel_id: *chan_id,
                                                                                };
                                                                                add_entry!(&candidate,
-                                                                                       $fee_to_target_msat,
+                                                                                       fee_to_target_msat,
                                                                                        $next_hops_value_contribution,
-                                                                                       $next_hops_path_htlc_minimum_msat,
-                                                                                       $next_hops_path_penalty_msat,
+                                                                                       next_hops_path_htlc_minimum_msat,
+                                                                                       next_hops_path_penalty_msat,
                                                                                        $next_hops_cltv_delta, $next_hops_path_length);
                                                                        }
                                                                }
@@ -2253,7 +2332,9 @@ 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, node_id: our_node_id };
+                               let candidate = CandidateRouteHop::FirstHop {
+                                       details, payer_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 {}",
@@ -2270,7 +2351,7 @@ where L::Target: Logger {
                        // If not, targets.pop() will not even let us enter the loop in step 2.
                        None => {},
                        Some(node) => {
-                               add_entries_to_cheapest_to_target_node!(node, payee, 0, path_value_msat, 0, 0u64, 0, 0);
+                               add_entries_to_cheapest_to_target_node!(node, payee, path_value_msat, 0, 0);
                        },
                });
 
@@ -2300,7 +2381,9 @@ 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, node_id: our_node_id};
+                                       let first_hop_candidate = CandidateRouteHop::FirstHop {
+                                               details, payer_node_id: &our_node_id,
+                                       };
                                        let blinded_path_fee = match compute_fees(path_contribution_msat, candidate.fees()) {
                                                Some(fee) => fee,
                                                None => continue
@@ -2339,8 +2422,7 @@ where L::Target: Logger {
                                let mut aggregate_path_contribution_msat = path_value_msat;
 
                                for (idx, (hop, prev_hop_id)) in hop_iter.zip(prev_hop_iter).enumerate() {
-                                       let source = NodeId::from_pubkey(&hop.src_node_id);
-                                       let target = NodeId::from_pubkey(&prev_hop_id);
+                                       let target = private_hop_key_cache.get(&prev_hop_id).unwrap();
 
                                        if let Some(first_channels) = first_hop_targets.get(&target) {
                                                if first_channels.iter().any(|d| d.outbound_scid_alias == Some(hop.short_channel_id)) {
@@ -2397,7 +2479,9 @@ 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, node_id: our_node_id};
+                                                       let first_hop_candidate = CandidateRouteHop::FirstHop {
+                                                               details, payer_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,
@@ -2442,7 +2526,9 @@ 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, node_id: our_node_id};
+                                                               let first_hop_candidate = CandidateRouteHop::FirstHop {
+                                                                       details, payer_node_id: &our_node_id,
+                                                               };
                                                                add_entry!(&first_hop_candidate,
                                                                        aggregate_next_hops_fee_msat,
                                                                        aggregate_path_contribution_msat,
@@ -2472,7 +2558,7 @@ where L::Target: Logger {
                // Both these cases (and other cases except reaching recommended_value_msat) mean that
                // paths_collection will be stopped because found_new_path==false.
                // This is not necessarily a routing failure.
-               'path_construction: while let Some(RouteGraphNode { node_id, lowest_fee_to_node, total_cltv_delta, mut value_contribution_msat, path_htlc_minimum_msat, path_penalty_msat, path_length_to_node, .. }) = targets.pop() {
+               'path_construction: while let Some(RouteGraphNode { node_id, total_cltv_delta, mut value_contribution_msat, path_length_to_node, .. }) = targets.pop() {
 
                        // Since we're going payee-to-payer, hitting our node as a target means we should stop
                        // traversing the graph and arrange the path out of what we found.
@@ -2607,8 +2693,8 @@ where L::Target: Logger {
                        match network_nodes.get(&node_id) {
                                None => {},
                                Some(node) => {
-                                       add_entries_to_cheapest_to_target_node!(node, node_id, lowest_fee_to_node,
-                                               value_contribution_msat, path_htlc_minimum_msat, path_penalty_msat,
+                                       add_entries_to_cheapest_to_target_node!(node, node_id,
+                                               value_contribution_msat,
                                                total_cltv_delta, path_length_to_node);
                                },
                        }
@@ -2737,8 +2823,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.candidate.target())),
-                                                                       selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.id(), h.0.candidate.target()))) {
+               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);
@@ -8098,6 +8184,7 @@ mod tests {
 pub(crate) mod bench_utils {
        use super::*;
        use std::fs::File;
+       use std::time::Duration;
 
        use bitcoin::hashes::Hash;
        use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
@@ -8246,10 +8333,10 @@ pub(crate) mod bench_utils {
                                                if let Ok(route) = route_res {
                                                        for path in route.paths {
                                                                if seed & 0x80 == 0 {
-                                                                       scorer.payment_path_successful(&path);
+                                                                       scorer.payment_path_successful(&path, Duration::ZERO);
                                                                } else {
                                                                        let short_channel_id = path.hops[path.hops.len() / 2].short_channel_id;
-                                                                       scorer.payment_path_failed(&path, short_channel_id);
+                                                                       scorer.payment_path_failed(&path, short_channel_id, Duration::ZERO);
                                                                }
                                                                seed = seed.overflowing_mul(6364136223846793005).0.overflowing_add(1).0;
                                                        }