Skip to content

Commit 0d5ac88

Browse files
Account for used liquidity in first hops when processing route hints
.. in get_route.
1 parent 677161b commit 0d5ac88

File tree

1 file changed

+44
-26
lines changed

1 file changed

+44
-26
lines changed

lightning/src/routing/router.rs

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,41 @@ impl fmt::Display for LoggedPayeePubkey {
11981198
}
11991199
}
12001200

1201+
#[inline]
1202+
fn sort_first_hop_channels(
1203+
channels: &mut Vec<&ChannelDetails>, used_channel_liquidities: &HashMap<(u64, bool), u64>,
1204+
recommended_value_msat: u64, our_node_pubkey: &PublicKey
1205+
) {
1206+
// Sort the first_hops channels to the same node(s) in priority order of which channel we'd
1207+
// most like to use.
1208+
//
1209+
// First, if channels are below `recommended_value_msat`, sort them in descending order,
1210+
// preferring larger channels to avoid splitting the payment into more MPP parts than is
1211+
// required.
1212+
//
1213+
// Second, because simply always sorting in descending order would always use our largest
1214+
// available outbound capacity, needlessly fragmenting our available channel capacities,
1215+
// sort channels above `recommended_value_msat` in ascending order, preferring channels
1216+
// which have enough, but not too much, capacity for the payment.
1217+
//
1218+
// Available outbound balances factor in liquidity already reserved for previously found paths.
1219+
channels.sort_unstable_by(|chan_a, chan_b| {
1220+
let chan_a_outbound_limit_msat = chan_a.next_outbound_htlc_limit_msat
1221+
.saturating_sub(*used_channel_liquidities.get(&(chan_a.get_outbound_payment_scid().unwrap(),
1222+
our_node_pubkey < &chan_a.counterparty.node_id)).unwrap_or(&0));
1223+
let chan_b_outbound_limit_msat = chan_b.next_outbound_htlc_limit_msat
1224+
.saturating_sub(*used_channel_liquidities.get(&(chan_b.get_outbound_payment_scid().unwrap(),
1225+
our_node_pubkey < &chan_b.counterparty.node_id)).unwrap_or(&0));
1226+
if chan_b_outbound_limit_msat < recommended_value_msat || chan_a_outbound_limit_msat < recommended_value_msat {
1227+
// Sort in descending order
1228+
chan_b_outbound_limit_msat.cmp(&chan_a_outbound_limit_msat)
1229+
} else {
1230+
// Sort in ascending order
1231+
chan_a_outbound_limit_msat.cmp(&chan_b_outbound_limit_msat)
1232+
}
1233+
});
1234+
}
1235+
12011236
/// Finds a route from us (payer) to the given target node (payee).
12021237
///
12031238
/// If the payee provided features in their invoice, they should be provided via `params.payee`.
@@ -1443,26 +1478,8 @@ where L::Target: Logger {
14431478
let mut already_collected_value_msat = 0;
14441479

14451480
for (_, channels) in first_hop_targets.iter_mut() {
1446-
// Sort the first_hops channels to the same node(s) in priority order of which channel we'd
1447-
// most like to use.
1448-
//
1449-
// First, if channels are below `recommended_value_msat`, sort them in descending order,
1450-
// preferring larger channels to avoid splitting the payment into more MPP parts than is
1451-
// required.
1452-
//
1453-
// Second, because simply always sorting in descending order would always use our largest
1454-
// available outbound capacity, needlessly fragmenting our available channel capacities,
1455-
// sort channels above `recommended_value_msat` in ascending order, preferring channels
1456-
// which have enough, but not too much, capacity for the payment.
1457-
channels.sort_unstable_by(|chan_a, chan_b| {
1458-
if chan_b.next_outbound_htlc_limit_msat < recommended_value_msat || chan_a.next_outbound_htlc_limit_msat < recommended_value_msat {
1459-
// Sort in descending order
1460-
chan_b.next_outbound_htlc_limit_msat.cmp(&chan_a.next_outbound_htlc_limit_msat)
1461-
} else {
1462-
// Sort in ascending order
1463-
chan_a.next_outbound_htlc_limit_msat.cmp(&chan_b.next_outbound_htlc_limit_msat)
1464-
}
1465-
});
1481+
sort_first_hop_channels(channels, &used_channel_liquidities, recommended_value_msat,
1482+
our_node_pubkey);
14661483
}
14671484

14681485
log_trace!(logger, "Building path from {} to payer {} for value {} msat.",
@@ -1871,7 +1888,9 @@ where L::Target: Logger {
18711888
// Searching for a direct channel between last checked hop and first_hop_targets
18721889
let hint_candidate_contribution_msat = cmp::min(path_value_msat,
18731890
candidate.effective_capacity().as_msat().saturating_sub(used_liquidity_msat));
1874-
if let Some(first_channels) = first_hop_targets.get(&NodeId::from_pubkey(&prev_hop_id)) {
1891+
if let Some(first_channels) = first_hop_targets.get_mut(&NodeId::from_pubkey(&prev_hop_id)) {
1892+
sort_first_hop_channels(first_channels, &used_channel_liquidities,
1893+
recommended_value_msat, our_node_pubkey);
18751894
for details in first_channels {
18761895
let first_hop_candidate = CandidateRouteHop::FirstHop { details };
18771896
add_entry!(first_hop_candidate, our_node_id, NodeId::from_pubkey(&prev_hop_id),
@@ -1910,7 +1929,9 @@ where L::Target: Logger {
19101929
// Note that we *must* check if the last hop was added as `add_entry`
19111930
// always assumes that the third argument is a node to which we have a
19121931
// path.
1913-
if let Some(first_channels) = first_hop_targets.get(&NodeId::from_pubkey(&hop.src_node_id)) {
1932+
if let Some(first_channels) = first_hop_targets.get_mut(&NodeId::from_pubkey(&hop.src_node_id)) {
1933+
sort_first_hop_channels(first_channels, &used_channel_liquidities,
1934+
recommended_value_msat, our_node_pubkey);
19141935
for details in first_channels {
19151936
let first_hop_candidate = CandidateRouteHop::FirstHop { details };
19161937
add_entry!(first_hop_candidate, our_node_id,
@@ -6018,12 +6039,9 @@ mod tests {
60186039
let route = get_route(&our_node_id, &payment_params, &network_graph.read_only(),
60196040
Some(&first_hops.iter().collect::<Vec<_>>()), amt_msat, Arc::clone(&logger), &scorer, &(),
60206041
&random_seed_bytes).unwrap();
6021-
// TODO: `get_route` returns a suboptimal route here because first hop channels are not
6022-
// resorted on the fly when processing route hints.
6023-
assert_eq!(route.paths.len(), 3);
6042+
assert_eq!(route.paths.len(), 2);
60246043
assert!(route.paths[0].hops.last().unwrap().fee_msat <= max_htlc_msat);
60256044
assert!(route.paths[1].hops.last().unwrap().fee_msat <= max_htlc_msat);
6026-
assert!(route.paths[2].hops.last().unwrap().fee_msat <= max_htlc_msat);
60276045
assert_eq!(route.get_total_amount(), amt_msat);
60286046
}
60296047

0 commit comments

Comments
 (0)