From bd6b710328ec6d116d43162ac6922e5b007e3e3c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 8 Jul 2022 18:26:06 +0000 Subject: [PATCH] Avoid saturating channels before we split payments Currently we only opt to split a payment into an MPP if we have completely and totally used a channel's available capacity (up to the announced htlc_max or on-chain capacity, whichever is lower). This is obviously destined to fail as channels are unlikely to have their full capacity available. Here we do the minimum viable fix by simply limiting channels to only using up to a configurable power-of-1/2. We default this new configuration knob to 1 (1/2 of the channel) so as to avoid a substantial change but in the future we may consider changing this to 2 (1/4) or even 3 (1/8). --- lightning/src/routing/gossip.rs | 4 --- lightning/src/routing/router.rs | 55 ++++++++++++++++++++++++--------- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 716ca2b30..55887623c 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -816,10 +816,6 @@ impl<'a> DirectedChannelInfoWithUpdate<'a> { /// Returns the [`EffectiveCapacity`] of the channel in the direction. #[inline] pub(super) fn effective_capacity(&self) -> EffectiveCapacity { self.inner.effective_capacity() } - - /// Returns the maximum HTLC amount allowed over the channel in the direction. - #[inline] - pub(super) fn htlc_maximum_msat(&self) -> u64 { self.inner.htlc_maximum_msat() } } impl<'a> fmt::Debug for DirectedChannelInfoWithUpdate<'a> { diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 92a78829e..ed424a2ac 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -225,6 +225,17 @@ pub struct PaymentParameters { /// The maximum number of paths that may be used by (MPP) payments. /// Defaults to [`DEFAULT_MAX_PATH_COUNT`]. pub max_path_count: u8, + + /// Selects the maximum share of a channel's total capacity which will be sent over a channel, + /// as a power of 1/2. A higher value prefers to send the payment using more MPP parts whereas + /// a lower value prefers to send larger MPP parts, potentially saturating channels and + /// increasing failure probability for those paths. + /// + /// A value of 0 will allow payments up to and including a channel's total announced usable + /// capacity, a value of one will only use up to half its capacity, two 1/4, etc. + /// + /// Default value: 1 + pub max_channel_saturation_power_of_half: u8, } impl_writeable_tlv_based!(PaymentParameters, { @@ -233,6 +244,7 @@ impl_writeable_tlv_based!(PaymentParameters, { (2, features, option), (3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)), (4, route_hints, vec_type), + (5, max_channel_saturation_power_of_half, (default_value, 1)), (6, expiry_time, option), }); @@ -246,6 +258,11 @@ impl PaymentParameters { expiry_time: None, max_total_cltv_expiry_delta: DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, max_path_count: DEFAULT_MAX_PATH_COUNT, + #[cfg(test)] // Many tests were written prior to the introduction of this parameter, so + // we leave it as 0 by default in tests, and change it for a few. + max_channel_saturation_power_of_half: 0, + #[cfg(not(test))] + max_channel_saturation_power_of_half: 1, } } @@ -433,16 +450,6 @@ impl<'a> CandidateRouteHop<'a> { } } - fn htlc_maximum_msat(&self) -> u64 { - match self { - CandidateRouteHop::FirstHop { details } => details.next_outbound_htlc_limit_msat, - CandidateRouteHop::PublicHop { info, .. } => info.htlc_maximum_msat(), - CandidateRouteHop::PrivateHop { hint } => { - hint.htlc_maximum_msat.unwrap_or(u64::max_value()) - }, - } - } - fn fees(&self) -> RoutingFees { match self { CandidateRouteHop::FirstHop { .. } => RoutingFees { @@ -464,6 +471,22 @@ impl<'a> CandidateRouteHop<'a> { } } +#[inline] +fn max_htlc_from_capacity(capacity: EffectiveCapacity, max_channel_saturation_power_of_half: u8) -> u64 { + let saturation_shift: u32 = max_channel_saturation_power_of_half as u32; + match capacity { + EffectiveCapacity::ExactLiquidity { liquidity_msat } => liquidity_msat, + EffectiveCapacity::Infinite => u64::max_value(), + EffectiveCapacity::Unknown => EffectiveCapacity::Unknown.as_msat(), + EffectiveCapacity::MaximumHTLC { amount_msat } => + amount_msat.checked_shr(saturation_shift).unwrap_or(0), + EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: None } => + capacity_msat.checked_shr(saturation_shift).unwrap_or(0), + EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: Some(htlc_max) } => + cmp::min(capacity_msat.checked_shr(saturation_shift).unwrap_or(0), htlc_max), + } +} + /// It's useful to keep track of the hops associated with the fees required to use them, /// so that we can choose cheaper paths (as per Dijkstra's algorithm). /// Fee values should be updated only in the context of the whole path, see update_value_and_recompute_fees. @@ -934,7 +957,8 @@ where L::Target: Logger { // - for first and last hops early in get_route if $src_node_id != $dest_node_id { let short_channel_id = $candidate.short_channel_id(); - let htlc_maximum_msat = $candidate.htlc_maximum_msat(); + let effective_capacity = $candidate.effective_capacity(); + let htlc_maximum_msat = max_htlc_from_capacity(effective_capacity, payment_params.max_channel_saturation_power_of_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 @@ -1084,7 +1108,7 @@ where L::Target: Logger { let channel_usage = ChannelUsage { amount_msat: amount_to_transfer_over_msat, inflight_htlc_msat: used_liquidity_msat, - effective_capacity: $candidate.effective_capacity(), + effective_capacity, }; let channel_penalty_msat = scorer.channel_penalty_msat( short_channel_id, &$src_node_id, &$dest_node_id, channel_usage @@ -1505,12 +1529,15 @@ where L::Target: Logger { .entry((hop.candidate.short_channel_id(), *prev_hop < hop.node_id)) .and_modify(|used_liquidity_msat| *used_liquidity_msat += spent_on_hop_msat) .or_insert(spent_on_hop_msat); - if *used_liquidity_msat == hop.candidate.htlc_maximum_msat() { + let hop_capacity = hop.candidate.effective_capacity(); + let hop_max_msat = max_htlc_from_capacity(hop_capacity, + payment_params.max_channel_saturation_power_of_half); + if *used_liquidity_msat == hop_max_msat { // If this path used all of this channel's available liquidity, we know // this path will not be selected again in the next loop iteration. prevented_redundant_path_selection = true; } - debug_assert!(*used_liquidity_msat <= hop.candidate.htlc_maximum_msat()); + debug_assert!(*used_liquidity_msat <= hop_max_msat); } if !prevented_redundant_path_selection { // If we weren't capped by hitting a liquidity limit on a channel in the path, -- 2.39.5