From a801adc8b5cf382b2457a45a8d00a0ca76fd9742 Mon Sep 17 00:00:00 2001 From: Velnbur Date: Thu, 11 Jul 2024 18:13:30 +0300 Subject: [PATCH 1/4] Move code out of `add_entry!` in `get_route` Move all of the code out of `add_entry!` macro inside `get_route` to facilitate development experience inside it and reduce enourmous `get_route` function size. --- lightning/src/routing/router.rs | 676 ++++++++++++++++++-------------- 1 file changed, 375 insertions(+), 301 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index fd1344ff031..eca29854129 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -2362,307 +2362,37 @@ where L::Target: Logger { ( $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 - // this channel, via this: - let mut hop_contribution_amt_msat = None; - // Channels to self should not be used. This is more of belt-and-suspenders, because in - // practice these cases should be caught earlier: - // - for regular channels at channel announcement (TODO) - // - for first and last hops early in get_route - let src_node_id = $candidate.source(); - 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); - - // It is tricky to subtract $next_hops_fee_msat from available liquidity here. - // It may be misleading because we might later choose to reduce the value transferred - // over these channels, and the channel which was insufficient might become sufficient. - // Worst case: we drop a good channel here because it can't cover the high following - // fees caused by one expensive channel, but then this channel could have been used - // if the amount being transferred over this path is lower. - // 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()) - .map_or(0, |used_liquidity_msat| { - available_value_contribution_msat = available_value_contribution_msat - .saturating_sub(*used_liquidity_msat); - *used_liquidity_msat - }); - - // Verify the liquidity offered by this channel complies to the minimal contribution. - let contributes_sufficient_value = available_value_contribution_msat >= minimal_value_contribution_msat; - // Do not consider candidate hops that would exceed the maximum path length. - let path_length_to_node = $next_hops_path_length - + if $candidate.blinded_hint_idx().is_some() { 0 } else { 1 }; - let exceeds_max_path_length = path_length_to_node > max_path_length; - - // Do not consider candidates that exceed the maximum total cltv expiry limit. - // In order to already account for some of the privacy enhancing random CLTV - // expiry delta offset we add on top later, we subtract a rough estimate - // (2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) here. - let max_total_cltv_expiry_delta = (payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta) - .checked_sub(2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) - .unwrap_or(payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta); - let hop_total_cltv_delta = ($next_hops_cltv_delta as u32) - .saturating_add($candidate.cltv_expiry_delta()); - let exceeds_cltv_delta_limit = hop_total_cltv_delta > max_total_cltv_expiry_delta; - - let value_contribution_msat = cmp::min(available_value_contribution_msat, $next_hops_value_contribution); - // Includes paying fees for the use of the following channels. - let amount_to_transfer_over_msat: u64 = match value_contribution_msat.checked_add($next_hops_fee_msat) { - Some(result) => result, - // Can't overflow due to how the values were computed right above. - None => unreachable!(), - }; - #[allow(unused_comparisons)] // $next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains - let over_path_minimum_msat = amount_to_transfer_over_msat >= $candidate.htlc_minimum_msat() && - amount_to_transfer_over_msat >= $next_hops_path_htlc_minimum_msat; - - #[allow(unused_comparisons)] // $next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains - let may_overpay_to_meet_path_minimum_msat = - ((amount_to_transfer_over_msat < $candidate.htlc_minimum_msat() && - recommended_value_msat >= $candidate.htlc_minimum_msat()) || - (amount_to_transfer_over_msat < $next_hops_path_htlc_minimum_msat && - recommended_value_msat >= $next_hops_path_htlc_minimum_msat)); - - let payment_failed_on_this_channel = match scid_opt { - Some(scid) => payment_params.previously_failed_channels.contains(&scid), - None => match $candidate.blinded_hint_idx() { - Some(idx) => { - payment_params.previously_failed_blinded_path_idxs.contains(&(idx as u64)) - }, - None => false, - }, - }; - - let (should_log_candidate, first_hop_details) = match $candidate { - CandidateRouteHop::FirstHop(hop) => (true, Some(hop.details)), - CandidateRouteHop::PrivateHop(_) => (true, None), - CandidateRouteHop::Blinded(_) => (true, None), - CandidateRouteHop::OneHopBlinded(_) => (true, None), - _ => (false, None), - }; - - // If HTLC minimum is larger than the amount we're going to transfer, we shouldn't - // bother considering this channel. If retrying with recommended_value_msat may - // allow us to hit the HTLC minimum limit, set htlc_minimum_limit so that we go - // around again with a higher amount. - if !contributes_sufficient_value { - if should_log_candidate { - log_trace!(logger, "Ignoring {} due to insufficient value contribution (channel max {:?}).", - LoggedCandidateHop(&$candidate), - effective_capacity); - } - num_ignored_value_contribution += 1; - } else if exceeds_max_path_length { - if should_log_candidate { - log_trace!(logger, "Ignoring {} due to exceeding maximum path length limit.", LoggedCandidateHop(&$candidate)); - } - num_ignored_path_length_limit += 1; - } else if exceeds_cltv_delta_limit { - if should_log_candidate { - log_trace!(logger, "Ignoring {} due to exceeding CLTV delta limit.", LoggedCandidateHop(&$candidate)); - - if let Some(_) = first_hop_details { - log_trace!(logger, - "First hop candidate cltv_expiry_delta: {}. Limit: {}", - hop_total_cltv_delta, - max_total_cltv_expiry_delta, - ); - } - } - num_ignored_cltv_delta_limit += 1; - } else if payment_failed_on_this_channel { - if should_log_candidate { - log_trace!(logger, "Ignoring {} due to a failed previous payment attempt.", LoggedCandidateHop(&$candidate)); - } - num_ignored_previously_failed += 1; - } else if may_overpay_to_meet_path_minimum_msat { - if should_log_candidate { - log_trace!(logger, - "Ignoring {} to avoid overpaying to meet htlc_minimum_msat limit ({}).", - LoggedCandidateHop(&$candidate), $candidate.htlc_minimum_msat()); - } - num_ignored_avoid_overpayment += 1; - hit_minimum_limit = true; - } else if over_path_minimum_msat { - // Note that low contribution here (limited by available_liquidity_msat) - // might violate htlc_minimum_msat on the hops which are next along the - // payment path (upstream to the payee). To avoid that, we recompute - // path fees knowing the final path contribution after constructing it. - let curr_min = cmp::max( - $next_hops_path_htlc_minimum_msat, $candidate.htlc_minimum_msat() - ); - let path_htlc_minimum_msat = compute_fees_saturating(curr_min, $candidate.fees()) - .saturating_add(curr_min); - - let dist_entry = &mut dist[$candidate.src_node_counter() as usize]; - let old_entry = if let Some(hop) = dist_entry { - hop - } else { - // 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 $candidate.target() node. - *dist_entry = Some(PathBuildingHop { - candidate: $candidate.clone(), - 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, - path_penalty_msat: u64::max_value(), - was_processed: false, - #[cfg(all(not(ldk_bench), any(test, fuzzing)))] - value_contribution_msat, - }); - dist_entry.as_mut().unwrap() - }; - - #[allow(unused_mut)] // We only use the mut in cfg(test) - let mut should_process = !old_entry.was_processed; - #[cfg(all(not(ldk_bench), any(test, fuzzing)))] - { - // In test/fuzzing builds, we do extra checks to make sure the skipping - // of already-seen nodes only happens in cases we expect (see below). - if !should_process { should_process = true; } - } - - if should_process { - let mut hop_use_fee_msat = 0; - let mut total_fee_msat: u64 = $next_hops_fee_msat; - - // 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 { - // 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()); - total_fee_msat = total_fee_msat.saturating_add(hop_use_fee_msat); - } - - // Ignore hops if augmenting the current path to them would put us over `max_total_routing_fee_msat` - if total_fee_msat > max_total_routing_fee_msat { - if should_log_candidate { - log_trace!(logger, "Ignoring {} due to exceeding max total routing fee limit.", LoggedCandidateHop(&$candidate)); - - if let Some(_) = first_hop_details { - log_trace!(logger, - "First hop candidate routing fee: {}. Limit: {}", - total_fee_msat, - max_total_routing_fee_msat, - ); - } - } - num_ignored_total_fee_limit += 1; - } else { - let channel_usage = ChannelUsage { - amount_msat: amount_to_transfer_over_msat, - inflight_htlc_msat: used_liquidity_msat, - effective_capacity, - }; - 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); - - // 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, - // and the cost of routing to the source node of this channel). - // Also, consider that htlc_minimum_msat_difference, because we might end up - // paying it. Consider the following exploit: - // we use 2 paths to transfer 1.5 BTC. One of them is 0-fee normal 1 BTC path, - // and for the other one we picked a 1sat-fee path with htlc_minimum_msat of - // 1 BTC. Now, since the latter is more expensive, we gonna try to cut it - // by 0.5 BTC, but then match htlc_minimum_msat by paying a fee of 0.5 BTC - // to this channel. - // Ideally the scoring could be smarter (e.g. 0.5*htlc_minimum_msat here), - // but it may require additional tracking - we don't want to double-count - // the fees included in $next_hops_path_htlc_minimum_msat, but also - // can't use something that may decrease on future hops. - let old_cost = cmp::max(old_entry.total_fee_msat, old_entry.path_htlc_minimum_msat) - .saturating_add(old_entry.path_penalty_msat); - let new_cost = cmp::max(total_fee_msat, path_htlc_minimum_msat) - .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; - old_entry.total_fee_msat = total_fee_msat; - 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; - old_entry.path_penalty_msat = path_penalty_msat; - #[cfg(all(not(ldk_bench), any(test, fuzzing)))] - { - old_entry.value_contribution_msat = value_contribution_msat; - } - hop_contribution_amt_msat = Some(value_contribution_msat); - } else if old_entry.was_processed && new_cost < old_cost { - #[cfg(all(not(ldk_bench), any(test, fuzzing)))] - { - // If we're skipping processing a node which was previously - // processed even though we found another path to it with a - // cheaper fee, check that it was because the second path we - // found (which we are processing now) has a lower value - // contribution due to an HTLC minimum limit. - // - // e.g. take a graph with two paths from node 1 to node 2, one - // through channel A, and one through channel B. Channel A and - // B are both in the to-process heap, with their scores set by - // a higher htlc_minimum than fee. - // Channel A is processed first, and the channels onwards from - // node 1 are added to the to-process heap. Thereafter, we pop - // Channel B off of the heap, note that it has a much more - // restrictive htlc_maximum_msat, and recalculate the fees for - // all of node 1's channels using the new, reduced, amount. - // - // This would be bogus - we'd be selecting a higher-fee path - // with a lower htlc_maximum_msat instead of the one we'd - // already decided to use. - debug_assert!(path_htlc_minimum_msat < old_entry.path_htlc_minimum_msat); - debug_assert!( - value_contribution_msat + path_penalty_msat < - old_entry.value_contribution_msat + old_entry.path_penalty_msat - ); - } - } - } - } - } else { - if should_log_candidate { - log_trace!(logger, - "Ignoring {} due to its htlc_minimum_msat limit.", - LoggedCandidateHop(&$candidate)); - - if let Some(details) = first_hop_details { - log_trace!(logger, - "First hop candidate next_outbound_htlc_minimum_msat: {}", - details.next_outbound_htlc_minimum_msat, - ); - } - } - num_ignored_htlc_minimum_msat_limit += 1; - } - } - } - hop_contribution_amt_msat + add_entry_internal( + channel_saturation_pow_half, + &used_liquidities, + minimal_value_contribution_msat, + &payment_params, + final_cltv_expiry_delta, + recommended_value_msat, + &logger, + &mut num_ignored_value_contribution, + &mut num_ignored_path_length_limit, + &mut num_ignored_cltv_delta_limit, + &mut num_ignored_previously_failed, + &mut num_ignored_total_fee_limit, + &mut num_ignored_avoid_overpayment, + &mut num_ignored_htlc_minimum_msat_limit, + &mut hit_minimum_limit, + &mut dist, + our_node_id, + max_total_routing_fee_msat, + &mut targets, + scorer, + score_params, + max_path_length, + $candidate, + $next_hops_fee_msat, + $next_hops_value_contribution, + $next_hops_path_htlc_minimum_msat, + $next_hops_path_penalty_msat, + $next_hops_cltv_delta, + $next_hops_path_length, + ) } } } @@ -3364,6 +3094,350 @@ where L::Target: Logger { Ok(route) } + +// 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`. +fn add_entry_internal<'a, L: Deref, S: ScoreLookUp>( + // parameters that were captured from the original macro add_entry: + channel_saturation_pow_half: u8, + used_liquidities: &HashMap, + minimal_value_contribution_msat: u64, + payment_params: &PaymentParameters, + final_cltv_expiry_delta: u32, + recommended_value_msat: u64, + logger: &L, + num_ignored_value_contribution: &mut u32, + num_ignored_path_length_limit: &mut u32, + num_ignored_cltv_delta_limit: &mut u32, + num_ignored_previously_failed: &mut u32, + num_ignored_total_fee_limit: &mut u32, + num_ignored_avoid_overpayment: &mut u32, + num_ignored_htlc_minimum_msat_limit: &mut u32, + hit_minimum_limit: &mut bool, + dist: &mut Vec>>, + our_node_id: NodeId, + max_total_routing_fee_msat: u64, + targets: &mut BinaryHeap, + scorer: &S, + score_params: &S::ScoreParams, + max_path_length: u8, + // original add_entry params: + candidate: &CandidateRouteHop<'a>, + next_hops_fee_msat: u64, + next_hops_value_contribution: u64, + next_hops_path_htlc_minimum_msat: u64, + next_hops_path_penalty_msat: u64, + next_hops_cltv_delta: u32, + next_hops_path_length: u8, +) -> Option +where + L::Target: Logger, +{ + // We "return" whether we updated the path at the end, and how much we can route via + // this channel, via this: + let mut hop_contribution_amt_msat = None; + // Channels to self should not be used. This is more of belt-and-suspenders, because in + // practice these cases should be caught earlier: + // - for regular channels at channel announcement (TODO) + // - for first and last hops early in get_route + let src_node_id = candidate.source(); + 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); + + // It is tricky to subtract $next_hops_fee_msat from available liquidity here. + // It may be misleading because we might later choose to reduce the value transferred + // over these channels, and the channel which was insufficient might become sufficient. + // Worst case: we drop a good channel here because it can't cover the high following + // fees caused by one expensive channel, but then this channel could have been used + // if the amount being transferred over this path is lower. + // 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()) + .map_or(0, |used_liquidity_msat| { + available_value_contribution_msat = available_value_contribution_msat + .saturating_sub(*used_liquidity_msat); + *used_liquidity_msat + }); + + // Verify the liquidity offered by this channel complies to the minimal contribution. + let contributes_sufficient_value = available_value_contribution_msat >= minimal_value_contribution_msat; + // Do not consider candidate hops that would exceed the maximum path length. + let path_length_to_node = next_hops_path_length + + if candidate.blinded_hint_idx().is_some() { 0 } else { 1 }; + let exceeds_max_path_length = path_length_to_node > max_path_length; + + // Do not consider candidates that exceed the maximum total cltv expiry limit. + // In order to already account for some of the privacy enhancing random CLTV + // expiry delta offset we add on top later, we subtract a rough estimate + // (2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) here. + let max_total_cltv_expiry_delta = (payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta) + .checked_sub(2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) + .unwrap_or(payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta); + let hop_total_cltv_delta = (next_hops_cltv_delta as u32) + .saturating_add(candidate.cltv_expiry_delta()); + let exceeds_cltv_delta_limit = hop_total_cltv_delta > max_total_cltv_expiry_delta; + + let value_contribution_msat = cmp::min(available_value_contribution_msat, next_hops_value_contribution); + // Includes paying fees for the use of the following channels. + let amount_to_transfer_over_msat: u64 = match value_contribution_msat.checked_add(next_hops_fee_msat) { + Some(result) => result, + // Can't overflow due to how the values were computed right above. + None => unreachable!(), + }; + #[allow(unused_comparisons)] // next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains + let over_path_minimum_msat = amount_to_transfer_over_msat >= candidate.htlc_minimum_msat() && + amount_to_transfer_over_msat >= next_hops_path_htlc_minimum_msat; + + #[allow(unused_comparisons)] // next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains + let may_overpay_to_meet_path_minimum_msat = + (amount_to_transfer_over_msat < candidate.htlc_minimum_msat() && + recommended_value_msat >= candidate.htlc_minimum_msat()) || + (amount_to_transfer_over_msat < next_hops_path_htlc_minimum_msat && + recommended_value_msat >= next_hops_path_htlc_minimum_msat); + + let payment_failed_on_this_channel = match scid_opt { + Some(scid) => payment_params.previously_failed_channels.contains(&scid), + None => match candidate.blinded_hint_idx() { + Some(idx) => { + payment_params.previously_failed_blinded_path_idxs.contains(&(idx as u64)) + }, + None => false, + }, + }; + + let (should_log_candidate, first_hop_details) = match candidate { + CandidateRouteHop::FirstHop(hop) => (true, Some(hop.details)), + CandidateRouteHop::PrivateHop(_) => (true, None), + CandidateRouteHop::Blinded(_) => (true, None), + CandidateRouteHop::OneHopBlinded(_) => (true, None), + _ => (false, None), + }; + + // If HTLC minimum is larger than the amount we're going to transfer, we shouldn't + // bother considering this channel. If retrying with recommended_value_msat may + // allow us to hit the HTLC minimum limit, set htlc_minimum_limit so that we go + // around again with a higher amount. + if !contributes_sufficient_value { + if should_log_candidate { + log_trace!(logger, "Ignoring {} due to insufficient value contribution (channel max {:?}).", + LoggedCandidateHop(&candidate), + effective_capacity); + } + *num_ignored_value_contribution += 1; + } else if exceeds_max_path_length { + if should_log_candidate { + log_trace!(logger, "Ignoring {} due to exceeding maximum path length limit.", LoggedCandidateHop(&candidate)); + } + *num_ignored_path_length_limit += 1; + } else if exceeds_cltv_delta_limit { + if should_log_candidate { + log_trace!(logger, "Ignoring {} due to exceeding CLTV delta limit.", LoggedCandidateHop(&candidate)); + + if let Some(_) = first_hop_details { + log_trace!(logger, + "First hop candidate cltv_expiry_delta: {}. Limit: {}", + hop_total_cltv_delta, + max_total_cltv_expiry_delta, + ); + } + } + *num_ignored_cltv_delta_limit += 1; + } else if payment_failed_on_this_channel { + if should_log_candidate { + log_trace!(logger, "Ignoring {} due to a failed previous payment attempt.", LoggedCandidateHop(&candidate)); + } + *num_ignored_previously_failed += 1; + } else if may_overpay_to_meet_path_minimum_msat { + if should_log_candidate { + log_trace!(logger, + "Ignoring {} to avoid overpaying to meet htlc_minimum_msat limit ({}).", + LoggedCandidateHop(&candidate), candidate.htlc_minimum_msat()); + } + *num_ignored_avoid_overpayment += 1; + *hit_minimum_limit = true; + } else if over_path_minimum_msat { + // Note that low contribution here (limited by available_liquidity_msat) + // might violate htlc_minimum_msat on the hops which are next along the + // payment path (upstream to the payee). To avoid that, we recompute + // path fees knowing the final path contribution after constructing it. + let curr_min = cmp::max( + next_hops_path_htlc_minimum_msat, candidate.htlc_minimum_msat() + ); + let path_htlc_minimum_msat = compute_fees_saturating(curr_min, candidate.fees()) + .saturating_add(curr_min); + + let dist_entry = &mut dist[candidate.src_node_counter() as usize]; + let old_entry = if let Some(hop) = dist_entry { + hop + } else { + // 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 candidate.target() node. + *dist_entry = Some(PathBuildingHop { + candidate: candidate.clone(), + 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, + path_penalty_msat: u64::max_value(), + was_processed: false, + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] + value_contribution_msat, + }); + dist_entry.as_mut().unwrap() + }; + + #[allow(unused_mut)] // We only use the mut in cfg(test) + let mut should_process = !old_entry.was_processed; + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] + { + // In test/fuzzing builds, we do extra checks to make sure the skipping + // of already-seen nodes only happens in cases we expect (see below). + if !should_process { should_process = true; } + } + + if should_process { + let mut hop_use_fee_msat = 0; + let mut total_fee_msat: u64 = next_hops_fee_msat; + + // 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 { + // 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()); + total_fee_msat = total_fee_msat.saturating_add(hop_use_fee_msat); + } + + // Ignore hops if augmenting the current path to them would put us over `max_total_routing_fee_msat` + if total_fee_msat > max_total_routing_fee_msat { + if should_log_candidate { + log_trace!(logger, "Ignoring {} due to exceeding max total routing fee limit.", LoggedCandidateHop(&candidate)); + + if let Some(_) = first_hop_details { + log_trace!(logger, + "First hop candidate routing fee: {}. Limit: {}", + total_fee_msat, + max_total_routing_fee_msat, + ); + } + } + *num_ignored_total_fee_limit += 1; + } else { + let channel_usage = ChannelUsage { + amount_msat: amount_to_transfer_over_msat, + inflight_htlc_msat: used_liquidity_msat, + effective_capacity, + }; + 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); + + // 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, + // and the cost of routing to the source node of this channel). + // Also, consider that htlc_minimum_msat_difference, because we might end up + // paying it. Consider the following exploit: + // we use 2 paths to transfer 1.5 BTC. One of them is 0-fee normal 1 BTC path, + // and for the other one we picked a 1sat-fee path with htlc_minimum_msat of + // 1 BTC. Now, since the latter is more expensive, we gonna try to cut it + // by 0.5 BTC, but then match htlc_minimum_msat by paying a fee of 0.5 BTC + // to this channel. + // Ideally the scoring could be smarter (e.g. 0.5*htlc_minimum_msat here), + // but it may require additional tracking - we don't want to double-count + // the fees included in next_hops_path_htlc_minimum_msat, but also + // can't use something that may decrease on future hops. + let old_cost = cmp::max(old_entry.total_fee_msat, old_entry.path_htlc_minimum_msat) + .saturating_add(old_entry.path_penalty_msat); + let new_cost = cmp::max(total_fee_msat, path_htlc_minimum_msat) + .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; + old_entry.total_fee_msat = total_fee_msat; + 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; + old_entry.path_penalty_msat = path_penalty_msat; + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] + { + old_entry.value_contribution_msat = value_contribution_msat; + } + hop_contribution_amt_msat = Some(value_contribution_msat); + } else if old_entry.was_processed && new_cost < old_cost { + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] + { + // If we're skipping processing a node which was previously + // processed even though we found another path to it with a + // cheaper fee, check that it was because the second path we + // found (which we are processing now) has a lower value + // contribution due to an HTLC minimum limit. + // + // e.g. take a graph with two paths from node 1 to node 2, one + // through channel A, and one through channel B. Channel A and + // B are both in the to-process heap, with their scores set by + // a higher htlc_minimum than fee. + // Channel A is processed first, and the channels onwards from + // node 1 are added to the to-process heap. Thereafter, we pop + // Channel B off of the heap, note that it has a much more + // restrictive htlc_maximum_msat, and recalculate the fees for + // all of node 1's channels using the new, reduced, amount. + // + // This would be bogus - we'd be selecting a higher-fee path + // with a lower htlc_maximum_msat instead of the one we'd + // already decided to use. + debug_assert!(path_htlc_minimum_msat < old_entry.path_htlc_minimum_msat); + debug_assert!( + value_contribution_msat + path_penalty_msat < + old_entry.value_contribution_msat + old_entry.path_penalty_msat + ); + } + } + } + } + } else { + if should_log_candidate { + log_trace!(logger, + "Ignoring {} due to its htlc_minimum_msat limit.", + LoggedCandidateHop(&candidate)); + + if let Some(details) = first_hop_details { + log_trace!(logger, + "First hop candidate next_outbound_htlc_minimum_msat: {}", + details.next_outbound_htlc_minimum_msat, + ); + } + } + *num_ignored_htlc_minimum_msat_limit += 1; + } + } + } + hop_contribution_amt_msat +} + // When an adversarial intermediary node observes a payment, it may be able to infer its // destination, if the remaining CLTV expiry delta exactly matches a feasible path in the network // graph. In order to improve privacy, this method obfuscates the CLTV expiry deltas along the From 612f17cd78827f2e494e4c849a8433a7cf61407f Mon Sep 17 00:00:00 2001 From: Velnbur Date: Thu, 11 Jul 2024 18:43:59 +0300 Subject: [PATCH 2/4] Group counters of ignored route candidates to struct --- lightning/src/routing/router.rs | 78 ++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index eca29854129..2288e3c83d9 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -2024,6 +2024,32 @@ impl<'a> fmt::Display for LoggedCandidateHop<'a> { } } + +// Remember how many candidates we ignored to allow for some logging afterwards. +#[derive(Default)] +struct IgnoredCandidatesStats { + value_contribution: u32, + cltv_delta_limit: u32, + path_length_limit: u32, + previously_failed: u32, + total_fee_limit: u32, + avoid_overpayment: u32, + htlc_minimum_msat_limit: u32, +} + +impl IgnoredCandidatesStats { + // Return total number of ignored cantotal number of ignored candidates. + const fn ignored_total(&self) -> u32 { + self.value_contribution + + self.cltv_delta_limit + + self.path_length_limit + + self.previously_failed + + self.total_fee_limit + + self.avoid_overpayment + + self.htlc_minimum_msat_limit + } +} + #[inline] fn sort_first_hop_channels( channels: &mut Vec<&ChannelDetails>, used_liquidities: &HashMap, @@ -2345,14 +2371,9 @@ where L::Target: Logger { log_trace!(logger, "Building path from {} to payer {} for value {} msat.", LoggedPayeePubkey(payment_params.payee.node_id()), our_node_pubkey, final_value_msat); + // Remember how many candidates we ignored to allow for some logging afterwards. - let mut num_ignored_value_contribution: u32 = 0; - let mut num_ignored_path_length_limit: u32 = 0; - let mut num_ignored_cltv_delta_limit: u32 = 0; - let mut num_ignored_previously_failed: u32 = 0; - let mut num_ignored_total_fee_limit: u32 = 0; - let mut num_ignored_avoid_overpayment: u32 = 0; - let mut num_ignored_htlc_minimum_msat_limit: u32 = 0; + let mut ignored_stats = IgnoredCandidatesStats::default(); macro_rules! add_entry { // Adds entry which goes from $candidate.source() to $candidate.target() over the $candidate hop. @@ -2370,13 +2391,7 @@ where L::Target: Logger { final_cltv_expiry_delta, recommended_value_msat, &logger, - &mut num_ignored_value_contribution, - &mut num_ignored_path_length_limit, - &mut num_ignored_cltv_delta_limit, - &mut num_ignored_previously_failed, - &mut num_ignored_total_fee_limit, - &mut num_ignored_avoid_overpayment, - &mut num_ignored_htlc_minimum_msat_limit, + &mut ignored_stats, &mut hit_minimum_limit, &mut dist, our_node_id, @@ -2924,17 +2939,14 @@ where L::Target: Logger { } } - let num_ignored_total = num_ignored_value_contribution + num_ignored_path_length_limit + - num_ignored_cltv_delta_limit + num_ignored_previously_failed + - num_ignored_avoid_overpayment + num_ignored_htlc_minimum_msat_limit + - num_ignored_total_fee_limit; + let num_ignored_total = ignored_stats.ignored_total(); if num_ignored_total > 0 { log_trace!(logger, "Ignored {} candidate hops due to insufficient value contribution, {} due to path length limit, {} due to CLTV delta limit, {} due to previous payment failure, {} due to htlc_minimum_msat limit, {} to avoid overpaying, {} due to maximum total fee limit. Total: {} ignored candidates.", - num_ignored_value_contribution, num_ignored_path_length_limit, - num_ignored_cltv_delta_limit, num_ignored_previously_failed, - num_ignored_htlc_minimum_msat_limit, num_ignored_avoid_overpayment, - num_ignored_total_fee_limit, num_ignored_total); + ignored_stats.value_contribution, ignored_stats.path_length_limit, + ignored_stats.cltv_delta_limit, ignored_stats.previously_failed, + ignored_stats.htlc_minimum_msat_limit, ignored_stats.avoid_overpayment, + ignored_stats.total_fee_limit, num_ignored_total); } // Step (5). @@ -3108,13 +3120,7 @@ fn add_entry_internal<'a, L: Deref, S: ScoreLookUp>( final_cltv_expiry_delta: u32, recommended_value_msat: u64, logger: &L, - num_ignored_value_contribution: &mut u32, - num_ignored_path_length_limit: &mut u32, - num_ignored_cltv_delta_limit: &mut u32, - num_ignored_previously_failed: &mut u32, - num_ignored_total_fee_limit: &mut u32, - num_ignored_avoid_overpayment: &mut u32, - num_ignored_htlc_minimum_msat_limit: &mut u32, + ignored_stats: &mut IgnoredCandidatesStats, hit_minimum_limit: &mut bool, dist: &mut Vec>>, our_node_id: NodeId, @@ -3228,12 +3234,12 @@ where LoggedCandidateHop(&candidate), effective_capacity); } - *num_ignored_value_contribution += 1; + ignored_stats.value_contribution += 1; } else if exceeds_max_path_length { if should_log_candidate { log_trace!(logger, "Ignoring {} due to exceeding maximum path length limit.", LoggedCandidateHop(&candidate)); } - *num_ignored_path_length_limit += 1; + ignored_stats.path_length_limit += 1; } else if exceeds_cltv_delta_limit { if should_log_candidate { log_trace!(logger, "Ignoring {} due to exceeding CLTV delta limit.", LoggedCandidateHop(&candidate)); @@ -3246,19 +3252,19 @@ where ); } } - *num_ignored_cltv_delta_limit += 1; + ignored_stats.cltv_delta_limit += 1; } else if payment_failed_on_this_channel { if should_log_candidate { log_trace!(logger, "Ignoring {} due to a failed previous payment attempt.", LoggedCandidateHop(&candidate)); } - *num_ignored_previously_failed += 1; + ignored_stats.previously_failed += 1; } else if may_overpay_to_meet_path_minimum_msat { if should_log_candidate { log_trace!(logger, "Ignoring {} to avoid overpaying to meet htlc_minimum_msat limit ({}).", LoggedCandidateHop(&candidate), candidate.htlc_minimum_msat()); } - *num_ignored_avoid_overpayment += 1; + ignored_stats.avoid_overpayment += 1; *hit_minimum_limit = true; } else if over_path_minimum_msat { // Note that low contribution here (limited by available_liquidity_msat) @@ -3330,7 +3336,7 @@ where ); } } - *num_ignored_total_fee_limit += 1; + ignored_stats.total_fee_limit += 1; } else { let channel_usage = ChannelUsage { amount_msat: amount_to_transfer_over_msat, @@ -3431,7 +3437,7 @@ where ); } } - *num_ignored_htlc_minimum_msat_limit += 1; + ignored_stats.htlc_minimum_msat_limit += 1; } } } From 69eba79398dc77c375b5ec804d220c83d17bd499 Mon Sep 17 00:00:00 2001 From: Velnbur Date: Thu, 11 Jul 2024 23:22:29 +0300 Subject: [PATCH 3/4] Add `GetRouteParameters` struct for `get_route` Group const after initialization values used across the `get_route` method into single `GetRouteParameters` struct. --- lightning/src/routing/router.rs | 54 ++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 2288e3c83d9..aff68a1ac2c 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -2123,6 +2123,17 @@ where L::Target: Logger, GL::Target: Logger { Ok(route) } +// Const after initialization params which are used across the [`get_route`]. +struct GetRouteParameters<'a> { + our_node_id: NodeId, + payment: &'a PaymentParameters, + max_total_routing_fee_msat: u64, + minimal_value_contribution_msat: u64, + final_cltv_expiry_delta: u32, + recommended_value_msat: u64, + max_path_length: u8, +} + pub(crate) fn get_route( our_node_pubkey: &PublicKey, route_params: &RouteParameters, network_graph: &ReadOnlyNetworkGraph, first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S, score_params: &S::ScoreParams, @@ -2375,6 +2386,11 @@ where L::Target: Logger { // Remember how many candidates we ignored to allow for some logging afterwards. let mut ignored_stats = IgnoredCandidatesStats::default(); + // Common parameters used across this function. + let params = GetRouteParameters { max_total_routing_fee_msat, + minimal_value_contribution_msat, final_cltv_expiry_delta, our_node_id, + recommended_value_msat, max_path_length, payment: payment_params }; + macro_rules! add_entry { // 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, @@ -2384,22 +2400,16 @@ where L::Target: Logger { $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 ) => { { add_entry_internal( + ¶ms, channel_saturation_pow_half, &used_liquidities, - minimal_value_contribution_msat, - &payment_params, - final_cltv_expiry_delta, - recommended_value_msat, &logger, &mut ignored_stats, &mut hit_minimum_limit, &mut dist, - our_node_id, - max_total_routing_fee_msat, &mut targets, scorer, score_params, - max_path_length, $candidate, $next_hops_fee_msat, $next_hops_value_contribution, @@ -3113,22 +3123,16 @@ where L::Target: Logger { // Returns the contribution amount of candidate if the channel caused an update to `targets`. fn add_entry_internal<'a, L: Deref, S: ScoreLookUp>( // parameters that were captured from the original macro add_entry: + params: &GetRouteParameters, channel_saturation_pow_half: u8, used_liquidities: &HashMap, - minimal_value_contribution_msat: u64, - payment_params: &PaymentParameters, - final_cltv_expiry_delta: u32, - recommended_value_msat: u64, logger: &L, ignored_stats: &mut IgnoredCandidatesStats, hit_minimum_limit: &mut bool, dist: &mut Vec>>, - our_node_id: NodeId, - max_total_routing_fee_msat: u64, targets: &mut BinaryHeap, scorer: &S, score_params: &S::ScoreParams, - max_path_length: u8, // original add_entry params: candidate: &CandidateRouteHop<'a>, next_hops_fee_msat: u64, @@ -3171,19 +3175,19 @@ where }); // Verify the liquidity offered by this channel complies to the minimal contribution. - let contributes_sufficient_value = available_value_contribution_msat >= minimal_value_contribution_msat; + let contributes_sufficient_value = available_value_contribution_msat >= params.minimal_value_contribution_msat; // Do not consider candidate hops that would exceed the maximum path length. let path_length_to_node = next_hops_path_length + if candidate.blinded_hint_idx().is_some() { 0 } else { 1 }; - let exceeds_max_path_length = path_length_to_node > max_path_length; + let exceeds_max_path_length = path_length_to_node > params.max_path_length; // Do not consider candidates that exceed the maximum total cltv expiry limit. // In order to already account for some of the privacy enhancing random CLTV // expiry delta offset we add on top later, we subtract a rough estimate // (2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) here. - let max_total_cltv_expiry_delta = (payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta) + let max_total_cltv_expiry_delta = (params.payment.max_total_cltv_expiry_delta - params.final_cltv_expiry_delta) .checked_sub(2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) - .unwrap_or(payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta); + .unwrap_or(params.payment.max_total_cltv_expiry_delta - params.final_cltv_expiry_delta); let hop_total_cltv_delta = (next_hops_cltv_delta as u32) .saturating_add(candidate.cltv_expiry_delta()); let exceeds_cltv_delta_limit = hop_total_cltv_delta > max_total_cltv_expiry_delta; @@ -3202,15 +3206,15 @@ where #[allow(unused_comparisons)] // next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains let may_overpay_to_meet_path_minimum_msat = (amount_to_transfer_over_msat < candidate.htlc_minimum_msat() && - recommended_value_msat >= candidate.htlc_minimum_msat()) || + params.recommended_value_msat >= candidate.htlc_minimum_msat()) || (amount_to_transfer_over_msat < next_hops_path_htlc_minimum_msat && - recommended_value_msat >= next_hops_path_htlc_minimum_msat); + params.recommended_value_msat >= next_hops_path_htlc_minimum_msat); let payment_failed_on_this_channel = match scid_opt { - Some(scid) => payment_params.previously_failed_channels.contains(&scid), + Some(scid) => params.payment.previously_failed_channels.contains(&scid), None => match candidate.blinded_hint_idx() { Some(idx) => { - payment_params.previously_failed_blinded_path_idxs.contains(&(idx as u64)) + params.payment.previously_failed_blinded_path_idxs.contains(&(idx as u64)) }, None => false, }, @@ -3316,7 +3320,7 @@ where // 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 != params.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()); @@ -3324,7 +3328,7 @@ where } // Ignore hops if augmenting the current path to them would put us over `max_total_routing_fee_msat` - if total_fee_msat > max_total_routing_fee_msat { + if total_fee_msat > params.max_total_routing_fee_msat { if should_log_candidate { log_trace!(logger, "Ignoring {} due to exceeding max total routing fee limit.", LoggedCandidateHop(&candidate)); @@ -3332,7 +3336,7 @@ where log_trace!(logger, "First hop candidate routing fee: {}. Limit: {}", total_fee_msat, - max_total_routing_fee_msat, + params.max_total_routing_fee_msat, ); } } From b4c62dc9d7b9b0f7200724258f57c2e31e51e6ca Mon Sep 17 00:00:00 2001 From: Velnbur Date: Thu, 11 Jul 2024 23:26:09 +0300 Subject: [PATCH 4/4] Group `next_hops_*` parameters in `add_entry` To `NextHopsData` and use it instead everywhere possible --- lightning/src/routing/router.rs | 150 ++++++++++++++++---------------- 1 file changed, 75 insertions(+), 75 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index aff68a1ac2c..a77c3703db5 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1824,6 +1824,28 @@ impl<'a> core::fmt::Debug for PathBuildingHop<'a> { } } + +#[derive(Default)] +struct NextHopsData { + fee_msat: u64, + value_contribution: u64, + path_htlc_minimum_msat: u64, + path_penalty_msat: u64, + cltv_delta: u32, + path_length: u8, +} + +impl NextHopsData { + /// Return [`NextHopsData`] with all default values (zeros), except a + /// `value_contribution`. + fn with_value_contribution(contribution: u64) -> Self { + Self { + value_contribution: contribution, + ..Default::default() + } + } +} + // Instantiated with a list of hops with correct data in them collected during path finding, // an instance of this struct should be further modified only via given methods. #[derive(Clone)] @@ -2396,9 +2418,7 @@ where L::Target: Logger { // $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, $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 ) => { { + ( $candidate: expr, $next_hops_data: expr ) => { { add_entry_internal( ¶ms, channel_saturation_pow_half, @@ -2411,12 +2431,7 @@ where L::Target: Logger { scorer, score_params, $candidate, - $next_hops_fee_msat, - $next_hops_value_contribution, - $next_hops_path_htlc_minimum_msat, - $next_hops_path_penalty_msat, - $next_hops_cltv_delta, - $next_hops_path_length, + $next_hops_data, ) } } } @@ -2461,10 +2476,13 @@ where L::Target: Logger { 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); + add_entry!(&candidate, &NextHopsData { + fee_msat: fee_to_target_msat, + value_contribution: $next_hops_value_contribution, + path_htlc_minimum_msat: next_hops_path_htlc_minimum_msat, + path_penalty_msat: next_hops_path_penalty_msat, + cltv_delta: $next_hops_cltv_delta, + path_length: $next_hops_path_length } ); } } @@ -2485,12 +2503,13 @@ where L::Target: Logger { info: directed_channel, short_channel_id: *chan_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); + add_entry!(&candidate, &NextHopsData { + fee_msat: fee_to_target_msat, + value_contribution: $next_hops_value_contribution, + path_htlc_minimum_msat: next_hops_path_htlc_minimum_msat, + path_penalty_msat: next_hops_path_penalty_msat, + cltv_delta: $next_hops_cltv_delta, + path_length: $next_hops_path_length }); } } } @@ -2522,8 +2541,8 @@ where L::Target: Logger { details, payer_node_id: &our_node_id, payer_node_counter, target_node_counter: payee_node_counter, }); - let added = add_entry!(&candidate, 0, path_value_msat, - 0, 0u64, 0, 0).is_some(); + let added = add_entry!(&candidate, + &NextHopsData::with_value_contribution(path_value_msat)).is_some(); log_trace!(logger, "{} direct route to payee via {}", if added { "Added" } else { "Skipped" }, LoggedCandidateHop(&candidate)); } @@ -2566,8 +2585,7 @@ where L::Target: Logger { CandidateRouteHop::Blinded(BlindedPathCandidate { source_node_counter, source_node_id, hint, hint_idx }) }; let mut path_contribution_msat = path_value_msat; - if let Some(hop_used_msat) = add_entry!(&candidate, - 0, path_contribution_msat, 0, 0_u64, 0, 0) + if let Some(hop_used_msat) = add_entry!(&candidate, &NextHopsData::with_value_contribution(path_contribution_msat)) { path_contribution_msat = hop_used_msat; } else { continue } @@ -2586,8 +2604,13 @@ where L::Target: Logger { }; let path_min = candidate.htlc_minimum_msat().saturating_add( compute_fees_saturating(candidate.htlc_minimum_msat(), candidate.fees())); - add_entry!(&first_hop_candidate, blinded_path_fee, path_contribution_msat, path_min, - 0_u64, candidate.cltv_expiry_delta(), 0); + add_entry!(&first_hop_candidate, &NextHopsData { + fee_msat: blinded_path_fee, + value_contribution: path_contribution_msat, + path_htlc_minimum_msat: path_min, + path_penalty_msat: 0, + cltv_delta: candidate.cltv_expiry_delta(), + path_length: 0 }); } } } @@ -2609,12 +2632,7 @@ where L::Target: Logger { let prev_hop_iter = core::iter::once(&maybe_dummy_payee_pk).chain( route.0.iter().skip(1).rev().map(|hop| &hop.src_node_id)); let mut hop_used = true; - let mut aggregate_next_hops_fee_msat: u64 = 0; - let mut aggregate_next_hops_path_htlc_minimum_msat: u64 = 0; - let mut aggregate_next_hops_path_penalty_msat: u64 = 0; - let mut aggregate_next_hops_cltv_delta: u32 = 0; - let mut aggregate_next_hops_path_length: u8 = 0; - let mut aggregate_path_contribution_msat = path_value_msat; + let mut aggregate_next_hops = NextHopsData::with_value_contribution(path_value_msat); for (idx, (hop, prev_hop_id)) in hop_iter.zip(prev_hop_iter).enumerate() { let (target, private_target_node_counter) = @@ -2645,12 +2663,8 @@ where L::Target: Logger { target_node_counter: *private_target_node_counter, })); - 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) - { - aggregate_path_contribution_msat = hop_used_msat; + if let Some(hop_used_msat) = add_entry!(&candidate, &aggregate_next_hops) { + aggregate_next_hops.value_contribution = hop_used_msat; } else { // If this hop was not used then there is no use checking the preceding // hops in the RouteHint. We can break by just searching for a direct @@ -2662,20 +2676,20 @@ where L::Target: Logger { .get(&candidate.id()).copied() .unwrap_or(0); let channel_usage = ChannelUsage { - amount_msat: final_value_msat + aggregate_next_hops_fee_msat, + amount_msat: final_value_msat + aggregate_next_hops.fee_msat, inflight_htlc_msat: used_liquidity_msat, effective_capacity: candidate.effective_capacity(), }; let channel_penalty_msat = scorer.channel_penalty_msat( &candidate, channel_usage, score_params ); - aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat + aggregate_next_hops.path_penalty_msat = aggregate_next_hops.path_penalty_msat .saturating_add(channel_penalty_msat); - aggregate_next_hops_cltv_delta = aggregate_next_hops_cltv_delta + aggregate_next_hops.cltv_delta = aggregate_next_hops.cltv_delta .saturating_add(hop.cltv_expiry_delta as u32); - aggregate_next_hops_path_length = aggregate_next_hops_path_length + aggregate_next_hops.path_length = aggregate_next_hops.path_length .saturating_add(1); // Searching for a direct channel between last checked hop and first_hop_targets @@ -2688,10 +2702,7 @@ where L::Target: Logger { details, payer_node_id: &our_node_id, payer_node_counter, target_node_counter: *peer_node_counter, }); - 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); + add_entry!(&first_hop_candidate, &aggregate_next_hops); } } @@ -2704,15 +2715,15 @@ where L::Target: Logger { // for the last node in the RouteHint. We need to just add the fees to // route through the current node so that the preceding node (next iteration) // can use it. - let hops_fee = compute_fees(aggregate_next_hops_fee_msat + final_value_msat, hop.fees) - .map_or(None, |inc| inc.checked_add(aggregate_next_hops_fee_msat)); - aggregate_next_hops_fee_msat = if let Some(val) = hops_fee { val } else { break; }; + let hops_fee = compute_fees(aggregate_next_hops.fee_msat + final_value_msat, hop.fees) + .map_or(None, |inc| inc.checked_add(aggregate_next_hops.fee_msat)); + aggregate_next_hops.fee_msat = if let Some(val) = hops_fee { val } else { break; }; // The next channel will need to relay this channel's min_htlc *plus* the fees taken by // this route hint's source node to forward said min over this channel. - aggregate_next_hops_path_htlc_minimum_msat = { + aggregate_next_hops.path_htlc_minimum_msat = { let curr_htlc_min = cmp::max( - candidate.htlc_minimum_msat(), aggregate_next_hops_path_htlc_minimum_msat + candidate.htlc_minimum_msat(), aggregate_next_hops.path_htlc_minimum_msat ); let curr_htlc_min_fee = if let Some(val) = compute_fees(curr_htlc_min, hop.fees) { val } else { break }; if let Some(min) = curr_htlc_min.checked_add(curr_htlc_min_fee) { min } else { break } @@ -2737,13 +2748,7 @@ where L::Target: Logger { details, payer_node_id: &our_node_id, payer_node_counter, target_node_counter: *peer_node_counter, }); - 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); + add_entry!(&first_hop_candidate, &aggregate_next_hops); } } } @@ -3135,12 +3140,7 @@ fn add_entry_internal<'a, L: Deref, S: ScoreLookUp>( score_params: &S::ScoreParams, // original add_entry params: candidate: &CandidateRouteHop<'a>, - next_hops_fee_msat: u64, - next_hops_value_contribution: u64, - next_hops_path_htlc_minimum_msat: u64, - next_hops_path_penalty_msat: u64, - next_hops_cltv_delta: u32, - next_hops_path_length: u8, + next_hops: &NextHopsData, ) -> Option where L::Target: Logger, @@ -3165,7 +3165,7 @@ where // fees caused by one expensive channel, but then this channel could have been used // if the amount being transferred over this path is lower. // 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) { + 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()) .map_or(0, |used_liquidity_msat| { @@ -3177,7 +3177,7 @@ where // Verify the liquidity offered by this channel complies to the minimal contribution. let contributes_sufficient_value = available_value_contribution_msat >= params.minimal_value_contribution_msat; // Do not consider candidate hops that would exceed the maximum path length. - let path_length_to_node = next_hops_path_length + let path_length_to_node = next_hops.path_length + if candidate.blinded_hint_idx().is_some() { 0 } else { 1 }; let exceeds_max_path_length = path_length_to_node > params.max_path_length; @@ -3188,27 +3188,27 @@ where let max_total_cltv_expiry_delta = (params.payment.max_total_cltv_expiry_delta - params.final_cltv_expiry_delta) .checked_sub(2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) .unwrap_or(params.payment.max_total_cltv_expiry_delta - params.final_cltv_expiry_delta); - let hop_total_cltv_delta = (next_hops_cltv_delta as u32) + let hop_total_cltv_delta = (next_hops.cltv_delta as u32) .saturating_add(candidate.cltv_expiry_delta()); let exceeds_cltv_delta_limit = hop_total_cltv_delta > max_total_cltv_expiry_delta; - let value_contribution_msat = cmp::min(available_value_contribution_msat, next_hops_value_contribution); + let value_contribution_msat = cmp::min(available_value_contribution_msat, next_hops.value_contribution); // Includes paying fees for the use of the following channels. - let amount_to_transfer_over_msat: u64 = match value_contribution_msat.checked_add(next_hops_fee_msat) { + let amount_to_transfer_over_msat: u64 = match value_contribution_msat.checked_add(next_hops.fee_msat) { Some(result) => result, // Can't overflow due to how the values were computed right above. None => unreachable!(), }; #[allow(unused_comparisons)] // next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains let over_path_minimum_msat = amount_to_transfer_over_msat >= candidate.htlc_minimum_msat() && - amount_to_transfer_over_msat >= next_hops_path_htlc_minimum_msat; + amount_to_transfer_over_msat >= next_hops.path_htlc_minimum_msat; #[allow(unused_comparisons)] // next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains let may_overpay_to_meet_path_minimum_msat = (amount_to_transfer_over_msat < candidate.htlc_minimum_msat() && params.recommended_value_msat >= candidate.htlc_minimum_msat()) || - (amount_to_transfer_over_msat < next_hops_path_htlc_minimum_msat && - params.recommended_value_msat >= next_hops_path_htlc_minimum_msat); + (amount_to_transfer_over_msat < next_hops.path_htlc_minimum_msat && + params.recommended_value_msat >= next_hops.path_htlc_minimum_msat); let payment_failed_on_this_channel = match scid_opt { Some(scid) => params.payment.previously_failed_channels.contains(&scid), @@ -3276,7 +3276,7 @@ where // payment path (upstream to the payee). To avoid that, we recompute // path fees knowing the final path contribution after constructing it. let curr_min = cmp::max( - next_hops_path_htlc_minimum_msat, candidate.htlc_minimum_msat() + next_hops.path_htlc_minimum_msat, candidate.htlc_minimum_msat() ); let path_htlc_minimum_msat = compute_fees_saturating(curr_min, candidate.fees()) .saturating_add(curr_min); @@ -3316,7 +3316,7 @@ where if should_process { let mut hop_use_fee_msat = 0; - let mut total_fee_msat: u64 = next_hops_fee_msat; + let mut total_fee_msat: u64 = next_hops.fee_msat; // Ignore hop_use_fee_msat for channel-from-us as we assume all channels-from-us // will have the same effective-fee @@ -3351,7 +3351,7 @@ where scorer.channel_penalty_msat(candidate, channel_usage, score_params); - let path_penalty_msat = next_hops_path_penalty_msat + let path_penalty_msat = next_hops.path_penalty_msat .saturating_add(channel_penalty_msat); // Update the way of reaching candidate.source() @@ -3385,7 +3385,7 @@ where path_length_to_node, }; targets.push(new_graph_node); - old_entry.next_hops_fee_msat = next_hops_fee_msat; + 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.candidate = candidate.clone();