Skip to content

Commit 22de0ae

Browse files
committed
Refactor max_mpp_path_count to max_path_count
Using this field just for MPP doesn't make sense when it could intuitively also be used to indicate single-path payments. We therefore rename `max_mpp_path_count` to `max_path_count` and make sure that a value of 1 ensures MPP is not even tried.
1 parent 3676a05 commit 22de0ae

File tree

1 file changed

+25
-23
lines changed

1 file changed

+25
-23
lines changed

lightning/src/routing/router.rs

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,10 @@ impl_writeable_tlv_based!(RouteParameters, {
176176
/// Maximum total CTLV difference we allow for a full payment path.
177177
pub const DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA: u32 = 1008;
178178

179-
/// Maximum number of paths we allow an MPP payment to have.
179+
/// Maximum number of paths we allow an (MPP) payment to have.
180180
// The default limit is currently set rather arbitrary - there aren't any real fundamental path-count
181181
// limits, but for now more than 10 paths likely carries too much one-path failure.
182-
pub const DEFAULT_MAX_MPP_PATH_COUNT: u8 = 10;
182+
pub const DEFAULT_MAX_PATH_COUNT: u8 = 10;
183183

184184
// The median hop CLTV expiry delta currently seen in the network.
185185
const MEDIAN_HOP_CLTV_EXPIRY_DELTA: u32 = 40;
@@ -222,16 +222,16 @@ pub struct PaymentParameters {
222222
/// Defaults to [`DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA`].
223223
pub max_total_cltv_expiry_delta: u32,
224224

225-
/// The maximum number of paths that may be used by MPP payments.
226-
/// Defaults to [`DEFAULT_MAX_MPP_PATH_COUNT`].
227-
pub max_mpp_path_count: u8,
225+
/// The maximum number of paths that may be used by (MPP) payments.
226+
/// Defaults to [`DEFAULT_MAX_PATH_COUNT`].
227+
pub max_path_count: u8,
228228
}
229229

230230
impl_writeable_tlv_based!(PaymentParameters, {
231231
(0, payee_pubkey, required),
232232
(1, max_total_cltv_expiry_delta, (default_value, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA)),
233233
(2, features, option),
234-
(3, max_mpp_path_count, (default_value, DEFAULT_MAX_MPP_PATH_COUNT)),
234+
(3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)),
235235
(4, route_hints, vec_type),
236236
(6, expiry_time, option),
237237
});
@@ -245,7 +245,7 @@ impl PaymentParameters {
245245
route_hints: vec![],
246246
expiry_time: None,
247247
max_total_cltv_expiry_delta: DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA,
248-
max_mpp_path_count: DEFAULT_MAX_MPP_PATH_COUNT,
248+
max_path_count: DEFAULT_MAX_PATH_COUNT,
249249
}
250250
}
251251

@@ -282,11 +282,11 @@ impl PaymentParameters {
282282
Self { max_total_cltv_expiry_delta, ..self }
283283
}
284284

285-
/// Includes a limit for the maximum number of payment paths that may be used by MPP.
285+
/// Includes a limit for the maximum number of payment paths that may be used.
286286
///
287287
/// (C-not exported) since bindings don't support move semantics
288-
pub fn with_max_mpp_path_count(self, max_mpp_path_count: u8) -> Self {
289-
Self { max_mpp_path_count, ..self }
288+
pub fn with_max_path_count(self, max_path_count: u8) -> Self {
289+
Self { max_path_count, ..self }
290290
}
291291
}
292292

@@ -799,21 +799,23 @@ where L::Target: Logger {
799799
let network_channels = network_graph.channels();
800800
let network_nodes = network_graph.nodes();
801801

802+
if payment_params.max_path_count == 0 {
803+
return Err(LightningError{err: "Can't find a route with no paths allowed.".to_owned(), action: ErrorAction::IgnoreError});
804+
}
805+
802806
// Allow MPP only if we have a features set from somewhere that indicates the payee supports
803807
// it. If the payee supports it they're supposed to include it in the invoice, so that should
804808
// work reliably.
805-
let allow_mpp = if let Some(features) = &payment_params.features {
809+
let allow_mpp = if payment_params.max_path_count == 1 {
810+
false
811+
} else if let Some(features) = &payment_params.features {
806812
features.supports_basic_mpp()
807813
} else if let Some(node) = network_nodes.get(&payee_node_id) {
808814
if let Some(node_info) = node.announcement_info.as_ref() {
809815
node_info.features.supports_basic_mpp()
810816
} else { false }
811817
} else { false };
812818

813-
if allow_mpp && payment_params.max_mpp_path_count == 0 {
814-
return Err(LightningError{err: "Can't find an MPP route with no paths allowed.".to_owned(), action: ErrorAction::IgnoreError});
815-
}
816-
817819
log_trace!(logger, "Searching for a route from payer {} to payee {} {} MPP and {} first hops {}overriding the network graph", our_node_pubkey,
818820
payment_params.payee_pubkey, if allow_mpp { "with" } else { "without" },
819821
first_hops.map(|hops| hops.len()).unwrap_or(0), if first_hops.is_some() { "" } else { "not " });
@@ -871,10 +873,10 @@ where L::Target: Logger {
871873
// Taking too many smaller paths also increases the chance of payment failure.
872874
// Thus to avoid this effect, we require from our collected links to provide
873875
// at least a minimal contribution to the recommended value yet-to-be-fulfilled.
874-
// This requirement is currently set to be 1/max_mpp_path_count of the payment
876+
// This requirement is currently set to be 1/max_path_count of the payment
875877
// value to ensure we only ever return routes that do not violate this limit.
876878
let minimal_value_contribution_msat: u64 = if allow_mpp {
877-
(final_value_msat + (payment_params.max_mpp_path_count as u64 - 1)) / payment_params.max_mpp_path_count as u64
879+
(final_value_msat + (payment_params.max_path_count as u64 - 1)) / payment_params.max_path_count as u64
878880
} else {
879881
final_value_msat
880882
};
@@ -1693,7 +1695,7 @@ where L::Target: Logger {
16931695
selected_paths.push(path);
16941696
}
16951697
// Make sure we would never create a route with more paths than we allow.
1696-
debug_assert!(selected_paths.len() <= payment_params.max_mpp_path_count.into());
1698+
debug_assert!(selected_paths.len() <= payment_params.max_path_count.into());
16971699

16981700
if let Some(features) = &payment_params.features {
16991701
for path in selected_paths.iter_mut() {
@@ -4117,20 +4119,20 @@ mod tests {
41174119
}
41184120

41194121
{
4120-
// Attempt to route while setting max_mpp_path_count to 0 results in a failure.
4121-
let zero_payment_params = payment_params.clone().with_max_mpp_path_count(0);
4122+
// Attempt to route while setting max_path_count to 0 results in a failure.
4123+
let zero_payment_params = payment_params.clone().with_max_path_count(0);
41224124
if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
41234125
&our_id, &zero_payment_params, &network_graph.read_only(), None, 100, 42,
41244126
Arc::clone(&logger), &scorer, &random_seed_bytes) {
4125-
assert_eq!(err, "Can't find an MPP route with no paths allowed.");
4127+
assert_eq!(err, "Can't find a route with no paths allowed.");
41264128
} else { panic!(); }
41274129
}
41284130

41294131
{
4130-
// Attempt to route while setting max_mpp_path_count to 3 results in a failure.
4132+
// Attempt to route while setting max_path_count to 3 results in a failure.
41314133
// This is the case because the minimal_value_contribution_msat would require each path
41324134
// to account for 1/3 of the total value, which is violated by 2 out of 3 paths.
4133-
let fail_payment_params = payment_params.clone().with_max_mpp_path_count(3);
4135+
let fail_payment_params = payment_params.clone().with_max_path_count(3);
41344136
if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
41354137
&our_id, &fail_payment_params, &network_graph.read_only(), None, 250_000, 42,
41364138
Arc::clone(&logger), &scorer, &random_seed_bytes) {

0 commit comments

Comments
 (0)