Cache whether a node is a first-hop target in the per-node state
authorMatt Corallo <git@bluematt.me>
Sun, 10 Dec 2023 03:28:37 +0000 (03:28 +0000)
committerMatt Corallo <git@bluematt.me>
Thu, 21 Dec 2023 05:59:53 +0000 (05:59 +0000)
When processing the main loop during routefinding, for each node,
we check whether it happens to be our peer in one of our channels.
This ensures we never fail to find a route that takes a hop through
a private channel of ours, to a private node, then through
invoice-provided route hints to reach the ultimate payee.

Because this is incredibly hot code, doing a full `HashMap` lookup
to check if each node is a first-hop target ends up eating a good
chunk of time during routing. Luckily, we can trivially avoid this
cost.

Because we're already looking up the per-node state in the `dist`
map, we can store a bool in each first-hop target's state, avoiding
the lookup unless we know its going to succeed.

This requires storing a dummy entry in `dist`, which feels somewhat
strange, but is ultimately fine as we should never be looking at
per-node state unless we've already found a path to that node,
updating the fields in doign so.

lightning/src/routing/router.rs

index 7c78a7bad896e33b5777baf3efac84ecda088415..28270baa1f57fa00c9298b652c77d8dff220ca18 100644 (file)
@@ -1501,6 +1501,14 @@ struct PathBuildingHop<'a> {
        /// decrease as well. Thus, we have to explicitly track which nodes have been processed and
        /// avoid processing them again.
        was_processed: bool,
+       /// When processing a node as the next best-score candidate, we want to quickly check if it is
+       /// a direct counterparty of ours, using our local channel information immediately if we can.
+       ///
+       /// In order to do so efficiently, we cache whether a node is a direct counterparty here at the
+       /// start of a route-finding pass. Unlike all other fields in this struct, this field is never
+       /// updated after being initialized - it is set at the start of a route-finding pass and only
+       /// read thereafter.
+       is_first_hop_target: 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.
@@ -2270,7 +2278,7 @@ where L::Target: Logger {
                                                        .saturating_add(curr_min);
 
                                                let dist_entry = &mut dist[$candidate.src_node_counter() as usize];
-                                               let mut old_entry = if let Some(hop) = dist_entry {
+                                               let old_entry = if let Some(hop) = dist_entry {
                                                        hop
                                                } else {
                                                        // If there was previously no known way to access the source node
@@ -2288,6 +2296,7 @@ where L::Target: Logger {
                                                                path_htlc_minimum_msat,
                                                                path_penalty_msat: u64::max_value(),
                                                                was_processed: false,
+                                                               is_first_hop_target: false,
                                                                #[cfg(all(not(ldk_bench), any(test, fuzzing)))]
                                                                value_contribution_msat,
                                                        });
@@ -2452,12 +2461,14 @@ where L::Target: Logger {
                        let fee_to_target_msat;
                        let next_hops_path_htlc_minimum_msat;
                        let next_hops_path_penalty_msat;
+                       let is_first_hop_target;
                        let skip_node = if let Some(elem) = &mut dist[$node.node_counter as usize] {
                                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;
+                               is_first_hop_target = elem.is_first_hop_target;
                                was_processed
                        } else {
                                // Entries are added to dist in add_entry!() when there is a channel from a node.
@@ -2468,21 +2479,24 @@ where L::Target: Logger {
                                fee_to_target_msat = 0;
                                next_hops_path_htlc_minimum_msat = 0;
                                next_hops_path_penalty_msat = 0;
+                               is_first_hop_target = false;
                                false
                        };
 
                        if !skip_node {
-                               if let Some((first_channels, peer_node_counter)) = first_hop_targets.get(&$node_id) {
-                                       for details in first_channels {
-                                               debug_assert_eq!(*peer_node_counter, $node.node_counter);
-                                               let candidate = CandidateRouteHop::FirstHop(CandidateFirstHop {
-                                                       details, payer_node_id: &our_node_id, payer_node_counter,
-                                                       target_node_counter: $node.node_counter,
-                                               });
-                                               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);
+                               if is_first_hop_target {
+                                       if let Some((first_channels, peer_node_counter)) = first_hop_targets.get(&$node_id) {
+                                               for details in first_channels {
+                                                       debug_assert_eq!(*peer_node_counter, $node.node_counter);
+                                                       let candidate = CandidateRouteHop::FirstHop(CandidateFirstHop {
+                                                               details, payer_node_id: &our_node_id, payer_node_counter,
+                                                               target_node_counter: $node.node_counter,
+                                                       });
+                                                       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);
+                                               }
                                        }
                                }
 
@@ -2529,6 +2543,33 @@ where L::Target: Logger {
                for e in dist.iter_mut() {
                        *e = None;
                }
+               for (node_id, (chans, peer_node_counter)) in first_hop_targets.iter() {
+                       // In order to avoid looking up whether each node is a first-hop target, we store a
+                       // dummy entry in dist for each first-hop target, allowing us to do this lookup for
+                       // free since we're already looking at the `was_processed` flag.
+                       //
+                       // Note that all the fields (except `is_first_hop_target`) will be overwritten whenever
+                       // we find a path to the target, so are left as dummies here.
+                       dist[*peer_node_counter as usize] = Some(PathBuildingHop {
+                               candidate: CandidateRouteHop::FirstHop {
+                                       details: &chans[0],
+                                       payer_node_id: &our_node_id,
+                                       target_node_counter: u32::max_value(),
+                                       payer_node_counter: u32::max_value(),
+                               },
+                               target_node_counter: None,
+                               fee_msat: 0,
+                               next_hops_fee_msat: u64::max_value(),
+                               hop_use_fee_msat: u64::max_value(),
+                               total_fee_msat: u64::max_value(),
+                               path_htlc_minimum_msat: u64::max_value(),
+                               path_penalty_msat: u64::max_value(),
+                               was_processed: false,
+                               is_first_hop_target: true,
+                               #[cfg(all(not(ldk_bench), any(test, fuzzing)))]
+                               value_contribution_msat: 0,
+                       });
+               }
                hit_minimum_limit = false;
 
                // If first hop is a private channel and the only way to reach the payee, this is the only