Abide by max path length param in router.

We now only limit the number of unblinded intermediate hops. Previously we
limited the total number of hops, including the final hop and any blinded hops.

Also adds some testing by augmenting existing tests.

Author: valentinewallace

lightning/src/routing/router.rs

@@ -1119,7 +1119,7 @@ struct RouteGraphNode {
 	//   considering their capacity and fees
 	value_contribution_msat: u64,
 	total_cltv_delta: u32,
-	/// The number of hops walked up to this node.
+	/// The number of intermediate hops walked up to this node.
 	path_length_to_node: u8,
 	is_intro_node: bool,
 }
@@ -1466,6 +1466,17 @@ impl<'a> CandidateRouteHop<'a> {
 			CandidateRouteHop::OneHopBlinded(_) => None,
 		}
 	}
+	/// Whether this candidate is an unblinded forwarding hop along the path, i.e. not the final
+	/// hop or part of a blinded path.
+	pub fn is_intermediate_hop(&self, payee_node_id_opt: &Option<NodeId>) -> bool {
+		match self {
+			CandidateRouteHop::FirstHop(h) => !h.targets_blinded_path && &self.target() != payee_node_id_opt,
+			CandidateRouteHop::PublicHop(h) => !h.targets_blinded_path && &self.target() != payee_node_id_opt,
+			CandidateRouteHop::PrivateHop(_) => &self.target() != payee_node_id_opt,
+			CandidateRouteHop::Blinded(_) => false,
+			CandidateRouteHop::OneHopBlinded(_) => false,
+		}
+	}
 }
 
 /// A unique(ish) identifier for a specific [`CandidateRouteHop`].
@@ -1873,6 +1884,7 @@ pub(crate) fn get_route<L: Deref, S: ScoreLookUp>(
 where L::Target: Logger {
 
 	let payment_params = &route_params.payment_params;
+	let max_intermediate_hops = core::cmp::min(payment_params.max_intermediate_hops, MAX_PATH_LENGTH_ESTIMATE);
 	let final_value_msat = route_params.final_value_msat;
 	// If we're routing to a blinded recipient, we won't have their node id. Therefore, keep the
 	// unblinded payee id as an option. We also need a non-optional "payee id" for path construction,
@@ -2168,8 +2180,9 @@ where L::Target: Logger {
 					// 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 + 1;
-					let exceeds_max_path_length = path_length_to_node > MAX_PATH_LENGTH_ESTIMATE;
+					let intermediate_path_length_to_node = $next_hops_path_length
+						+ if $candidate.is_intermediate_hop(&payee_node_id_opt) { 1 } else { 0 };
+					let exceeds_max_path_length = intermediate_path_length_to_node > max_intermediate_hops;
 
 					// 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
@@ -2380,7 +2393,7 @@ where L::Target: Logger {
 										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,
+										path_length_to_node: intermediate_path_length_to_node,
 										is_intro_node: $candidate.blinded_hint_idx().is_some(),
 									};
 									targets.push(new_graph_node);
@@ -2625,9 +2638,8 @@ 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(),
-						candidate.blinded_path().map_or(1, |bp| bp.blinded_hops.len() as u8));
+					add_entry!(&first_hop_candidate, blinded_path_fee, path_contribution_msat, path_min,
+						0_u64, candidate.cltv_expiry_delta(), 0);
 				}
 			}
 		}
@@ -2653,7 +2665,7 @@ where L::Target: Logger {
 				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_next_hops_intermediate_path_len: u8 = 0;
 				let mut aggregate_path_contribution_msat = path_value_msat;
 
 				for (idx, (hop, prev_hop_id)) in hop_iter.zip(prev_hop_iter).enumerate() {
@@ -2680,7 +2692,7 @@ where L::Target: Logger {
 					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_next_hops_cltv_delta, aggregate_next_hops_intermediate_path_len)
 					{
 						aggregate_path_contribution_msat = hop_used_msat;
 					} else {
@@ -2707,8 +2719,12 @@ where L::Target: Logger {
 					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
-						.saturating_add(1);
+					if idx > 0 {
+						// The final hop doesn't count towards the number of intermediate hops, so only start
+						// incrementing here after it's been processed.
+						aggregate_next_hops_intermediate_path_len = aggregate_next_hops_intermediate_path_len
+							.saturating_add(1);
+					}
 
 					// Searching for a direct channel between last checked hop and first_hop_targets
 					if let Some(first_channels) = first_hop_targets.get(target) {
@@ -2723,7 +2739,7 @@ where L::Target: Logger {
 							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);
+								aggregate_next_hops_cltv_delta, aggregate_next_hops_intermediate_path_len);
 						}
 					}
 
@@ -2775,7 +2791,7 @@ where L::Target: Logger {
 									aggregate_next_hops_path_htlc_minimum_msat,
 									aggregate_next_hops_path_penalty_msat,
 									aggregate_next_hops_cltv_delta,
-									aggregate_next_hops_path_length);
+									aggregate_next_hops_intermediate_path_len);
 							}
 						}
 					}
@@ -3390,7 +3406,8 @@ mod tests {
 	fn simple_route_test() {
 		let (secp_ctx, network_graph, _, _, logger) = build_graph();
 		let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
+		let mut payment_params = PaymentParameters::from_node_id(nodes[2], 42);
+		payment_params.max_intermediate_hops = 1;
 		let scorer = ln_test_utils::TestScorer::new();
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -3405,7 +3422,7 @@ mod tests {
 				assert_eq!(err, "Cannot send a payment of 0 msat");
 		} else { panic!(); }
 
-		let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
+		let mut route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
 		let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
 			Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
 		assert_eq!(route.paths[0].hops.len(), 2);
@@ -3423,6 +3440,10 @@ mod tests {
 		assert_eq!(route.paths[0].hops[1].cltv_expiry_delta, 42);
 		assert_eq!(route.paths[0].hops[1].node_features.le_flags(), &id_to_feature_flags(3));
 		assert_eq!(route.paths[0].hops[1].channel_features.le_flags(), &id_to_feature_flags(4));
+
+		route_params.payment_params.max_intermediate_hops = 0;
+		get_route(&our_id, &route_params, &network_graph.read_only(), None,
+			Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap_err();
 	}
 
 	#[test]
@@ -3827,7 +3848,8 @@ mod tests {
 		});
 
 		// If all the channels require some features we don't understand, route should fail
-		let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
+		let mut route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
+		route_params.payment_params.max_intermediate_hops = 1;
 		if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
 			&route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer,
 			&Default::default(), &random_seed_bytes) {
@@ -4083,8 +4105,9 @@ mod tests {
 			} else { panic!(); }
 		}
 
-		let payment_params = PaymentParameters::from_node_id(nodes[6], 42)
+		let mut payment_params = PaymentParameters::from_node_id(nodes[6], 42)
 			.with_route_hints(last_hops_multi_private_channels(&nodes)).unwrap();
+		payment_params.max_intermediate_hops = 4;
 		let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
 		let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
 			Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
@@ -4242,7 +4265,8 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		// Test through channels 2, 3, 0xff00, 0xff01.
-		// Test shows that multiple hop hints are considered.
+		// Test shows that multi-hop route hints are considered and factored correctly into the
+		// max path length.
 
 		// Disabling channels 6 & 7 by flags=2
 		update_channel(&gossip_sync, &secp_ctx, &privkeys[2], UnsignedChannelUpdate {
@@ -4270,7 +4294,8 @@ mod tests {
 			excess_data: Vec::new()
 		});
 
-		let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
+		let mut route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
+		route_params.payment_params.max_intermediate_hops = 3;
 		let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
 			Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
 		assert_eq!(route.paths[0].hops.len(), 4);
@@ -4302,6 +4327,9 @@ mod tests {
 		assert_eq!(route.paths[0].hops[3].cltv_expiry_delta, 42);
 		assert_eq!(route.paths[0].hops[3].node_features.le_flags(), default_node_features().le_flags()); // We dont pass flags in from invoices yet
 		assert_eq!(route.paths[0].hops[3].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
+		route_params.payment_params.max_intermediate_hops = 2;
+		get_route(&our_id, &route_params, &network_graph.read_only(), None,
+			Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap_err();
 	}
 
 	#[test]
@@ -6843,16 +6871,20 @@ mod tests {
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
 		// First check we can actually create a long route on this graph.
-		let feasible_payment_params = PaymentParameters::from_node_id(nodes[18], 0);
+		let mut feasible_payment_params = PaymentParameters::from_node_id(nodes[18], 0);
+		// Check that we default to MAX_PATH_LENGTH_ESTIMATE for the path length limit.
+		assert_eq!(feasible_payment_params.max_intermediate_hops, MAX_PATH_LENGTH_ESTIMATE);
+		feasible_payment_params.max_intermediate_hops = 18;
 		let route_params = RouteParameters::from_payment_params_and_value(
 			feasible_payment_params, 100);
 		let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger),
 			&scorer, &Default::default(), &random_seed_bytes).unwrap();
 		let path = route.paths[0].hops.iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
-		assert!(path.len() == MAX_PATH_LENGTH_ESTIMATE.into());
+		assert!(path.len() == route_params.payment_params.max_intermediate_hops as usize + 1);
 
-		// But we can't create a path surpassing the MAX_PATH_LENGTH_ESTIMATE limit.
-		let fail_payment_params = PaymentParameters::from_node_id(nodes[19], 0);
+		// But we can't create a path surpassing the parameterized path length limit.
+		let mut fail_payment_params = PaymentParameters::from_node_id(nodes[19], 0);
+		fail_payment_params.max_intermediate_hops = 18;
 		let route_params = RouteParameters::from_payment_params_and_value(
 			fail_payment_params, 100);
 		match get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer,