Randomize candidate paths during route selection.

Author: tnull

lightning/src/routing/router.rs

@@ -645,7 +645,7 @@ where L::Target: Logger {
 pub(crate) fn get_route<L: Deref, S: Score>(
 	our_node_pubkey: &PublicKey, payment_params: &PaymentParameters, network_graph: &ReadOnlyNetworkGraph,
 	first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv_expiry_delta: u32,
-	logger: L, scorer: &S, _random_seed_bytes: &[u8; 32]
+	logger: L, scorer: &S, random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
 where L::Target: Logger {
 	let payee_node_id = NodeId::from_pubkey(&payment_params.payee_pubkey);
@@ -833,7 +833,7 @@ where L::Target: Logger {
 					.entry(short_channel_id)
 					.or_insert_with(|| $candidate.effective_capacity().as_msat());
 
-				// It is tricky to substract $next_hops_fee_msat from available liquidity here.
+				// 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
@@ -1449,14 +1449,31 @@ where L::Target: Logger {
 
 	// Draw multiple sufficient routes by randomly combining the selected paths.
 	let mut drawn_routes = Vec::new();
-	for i in 0..payment_paths.len() {
+	let mut prng = ChaCha20::new(random_seed_bytes, &[0u8; 12]);
+	let mut random_index_bytes = [0u8; ::core::mem::size_of::<usize>()];
+
+	let num_permutations = payment_paths.len();
+	let mut seen_permutation_scids: HashSet<Vec<Vec<u64>>> = HashSet::new();
+	for _ in 0..num_permutations {
 		let mut cur_route = Vec::<PaymentPath>::new();
 		let mut aggregate_route_value_msat = 0;
 
 		// Step (6).
-		// TODO: real random shuffle
-		// Currently just starts with i_th and goes up to i-1_th in a looped way.
-		let cur_payment_paths = [&payment_paths[i..], &payment_paths[..i]].concat();
+		// Do a Fisher-Yates shuffle to create a unique permutation of the payment paths
+		let cur_payment_paths = loop {
+			let mut cur_perm = payment_paths.clone();
+			for cur_index in (1..cur_perm.len()).rev() {
+				prng.process_in_place(&mut random_index_bytes);
+				let random_index = usize::from_be_bytes(random_index_bytes).wrapping_rem(cur_index+1);
+				cur_perm.swap(cur_index, random_index);
+			}
+			let cur_perm_scids = cur_perm.iter().map(|path|
+				path.hops.iter().map(|(hop,_)| hop.candidate.short_channel_id()).collect::<Vec<u64>>()
+				).collect::<Vec<Vec<u64>>>();
+			if seen_permutation_scids.insert(cur_perm_scids) {
+				break cur_perm;
+			}
+		};
 
 		// Step (7).
 		for payment_path in cur_payment_paths {
@@ -1469,12 +1486,17 @@ where L::Target: Logger {
 				// also makes routing more reliable.
 				let mut overpaid_value_msat = aggregate_route_value_msat - final_value_msat;
 
-				// First, drop some expensive low-value paths entirely if possible.
-				// Sort by value so that we drop many really-low values first, since
+				// First, we drop some expensive low-value paths entirely if possible.
+				// In order to do so, we pre-sort by total fees paid, so that in case of equal
+				// values we prefer lower cost paths.
+				// (Descending order, so we drop higher-fee paths first)
+				cur_route.sort_by_key(|path| path.get_total_fee_paid_msat());
+				cur_route.reverse();
+
+				// Then sort by value so that we drop many really-low values first, since
 				// fewer paths is better: the payment is less likely to fail.
-				// TODO: this could also be optimized by also sorting by feerate_per_sat_routed,
-				// so that the sender pays less fees overall. And also htlc_minimum_msat.
 				cur_route.sort_by_key(|path| path.get_value_msat());
+
 				// We should make sure that at least 1 path left.
 				let mut paths_left = cur_route.len();
 				cur_route.retain(|path| {
@@ -1498,13 +1520,14 @@ where L::Target: Logger {
 				assert!(cur_route.len() > 0);
 
 				// Step (8).
-				// Now, substract the overpaid value from the most-expensive path.
+				// Now, subtract the overpaid value from the most-expensive path.
 				// TODO: this could also be optimized by also sorting by feerate_per_sat_routed,
 				// so that the sender pays less fees overall. And also htlc_minimum_msat.
 				cur_route.sort_by_key(|path| { path.hops.iter().map(|hop| hop.0.candidate.fees().proportional_millionths as u64).sum::<u64>() });
 				let expensive_payment_path = cur_route.first_mut().unwrap();
-				// We already dropped all the small channels above, meaning all the
-				// remaining channels are larger than remaining overpaid_value_msat.
+
+				// We already dropped all the small value paths above, meaning all the
+				// remaining paths are larger than remaining overpaid_value_msat.
 				// Thus, this can't be negative.
 				let expensive_path_new_value_msat = expensive_payment_path.get_value_msat() - overpaid_value_msat;
 				expensive_payment_path.update_value_and_recompute_fees(expensive_path_new_value_msat);