From: Matt Corallo Date: Sun, 10 Dec 2023 03:28:37 +0000 (+0000) Subject: Cache whether a node is a first-hop target in the per-node state X-Git-Tag: v0.0.124-beta~45^2~5 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=df9c15de75aca8dd6368e4ef8f475237f83ea632;p=rust-lightning Cache whether a node is a first-hop target in the per-node state 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. --- diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index fd1344ff0..66b2d87e5 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1772,6 +1772,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. @@ -1810,6 +1818,7 @@ impl<'a> core::fmt::Debug for PathBuildingHop<'a> { .field("source_node_id", &self.candidate.source()) .field("target_node_id", &self.candidate.target()) .field("short_channel_id", &self.candidate.short_channel_id()) + .field("is_first_hop_target", &self.is_first_hop_target) .field("total_fee_msat", &self.total_fee_msat) .field("next_hops_fee_msat", &self.next_hops_fee_msat) .field("hop_use_fee_msat", &self.hop_use_fee_msat) @@ -2516,6 +2525,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, }); @@ -2679,12 +2689,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. @@ -2695,21 +2707,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(FirstHopCandidate { - 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(FirstHopCandidate { + 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); + } } } @@ -2756,6 +2771,32 @@ where L::Target: Logger { for e in dist.iter_mut() { *e = None; } + for (_, (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(FirstHopCandidate { + details: &chans[0], + payer_node_id: &our_node_id, + target_node_counter: u32::max_value(), + payer_node_counter: u32::max_value(), + }), + 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