Consider `RouteParameters::max_total_routing_fee_msat` in `get_route`
authorElias Rohrer <dev@tnull.de>
Fri, 14 Jul 2023 11:25:33 +0000 (13:25 +0200)
committerElias Rohrer <dev@tnull.de>
Tue, 26 Sep 2023 07:44:05 +0000 (09:44 +0200)
We exclude any candidate hops if we find that using them would let the
aggregated path routing fees exceed `max_total_routing_fee_msat`.

Moreover, we return an error if the aggregated fees over all paths of
the selected route would surpass `max_total_routing_fee_msat`.

lightning/src/routing/router.rs

index dacc137ae8f536b6282f52576c7427b53908d825..340d5e5c1078d50f24feedfe10abf5c9c124392b 100644 (file)
@@ -1693,6 +1693,7 @@ where L::Target: Logger {
        let mut num_ignored_path_length_limit = 0;
        let mut num_ignored_cltv_delta_limit = 0;
        let mut num_ignored_previously_failed = 0;
+       let mut num_ignored_total_fee_limit = 0;
 
        macro_rules! add_entry {
                // Adds entry which goes from $src_node_id to $dest_node_id over the $candidate hop.
@@ -1854,89 +1855,98 @@ where L::Target: Logger {
                                                                total_fee_msat = total_fee_msat.saturating_add(hop_use_fee_msat);
                                                        }
 
-                                                       let channel_usage = ChannelUsage {
-                                                               amount_msat: amount_to_transfer_over_msat,
-                                                               inflight_htlc_msat: used_liquidity_msat,
-                                                               effective_capacity,
-                                                       };
-                                                       let channel_penalty_msat = scid_opt.map_or(0,
-                                                               |scid| scorer.channel_penalty_msat(scid, &$src_node_id, &$dest_node_id,
-                                                                       channel_usage, score_params));
-                                                       let path_penalty_msat = $next_hops_path_penalty_msat
-                                                               .saturating_add(channel_penalty_msat);
-                                                       let new_graph_node = RouteGraphNode {
-                                                               node_id: $src_node_id,
-                                                               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 $src_node_id with the given short_channel_id (from $dest_node_id),
-                                                       // 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 {
-                                                               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.node_id = $dest_node_id.clone();
-                                                               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;
+                                                       // Ignore hops if augmenting the current path to them would put us over `max_total_routing_fee_msat`
+                                                       let max_total_routing_fee_msat = route_params.max_total_routing_fee_msat.unwrap_or(u64::max_value());
+                                                       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));
                                                                }
-                                                               did_add_update_path_to_src_node = 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
-                                                                       );
+                                                               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 = scid_opt.map_or(0,
+                                                                       |scid| scorer.channel_penalty_msat(scid, &$src_node_id, &$dest_node_id,
+                                                                               channel_usage, score_params));
+                                                               let path_penalty_msat = $next_hops_path_penalty_msat
+                                                                       .saturating_add(channel_penalty_msat);
+                                                               let new_graph_node = RouteGraphNode {
+                                                                       node_id: $src_node_id,
+                                                                       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 $src_node_id with the given short_channel_id (from $dest_node_id),
+                                                               // 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 {
+                                                                       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.node_id = $dest_node_id.clone();
+                                                                       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;
+                                                                       }
+                                                                       did_add_update_path_to_src_node = 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
+                                                                               );
+                                                                       }
                                                                }
                                                        }
                                                }
@@ -2407,9 +2417,9 @@ 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_cltv_delta_limit + num_ignored_previously_failed + num_ignored_total_fee_limit;
        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. Total: {} ignored candidates.", num_ignored_value_contribution, num_ignored_path_length_limit, num_ignored_cltv_delta_limit, num_ignored_previously_failed, num_ignored_total);
+               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 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_total_fee_limit, num_ignored_total);
        }
 
        // Step (5).
@@ -2551,6 +2561,14 @@ where L::Target: Logger {
        // Make sure we would never create a route with more paths than we allow.
        debug_assert!(paths.len() <= payment_params.max_path_count.into());
 
+       // Make sure we would never create a route whose total fees exceed max_total_routing_fee_msat.
+       if let Some(max_total_routing_fee_msat) = route_params.max_total_routing_fee_msat {
+               if paths.iter().map(|p| p.fee_msat()).sum::<u64>() > max_total_routing_fee_msat {
+                       return Err(LightningError{err: format!("Failed to find route that adheres to the maximum total fee limit of {}msat",
+                               max_total_routing_fee_msat), action: ErrorAction::IgnoreError});
+               }
+       }
+
        if let Some(node_features) = payment_params.payee.node_features() {
                for path in paths.iter_mut() {
                        path.hops.last_mut().unwrap().node_features = node_features.clone();