diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index e6a1825758c..e674d9379fb 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1198,6 +1198,41 @@ impl fmt::Display for LoggedPayeePubkey { } } +#[inline] +fn sort_first_hop_channels( + channels: &mut Vec<&ChannelDetails>, used_channel_liquidities: &HashMap<(u64, bool), u64>, + recommended_value_msat: u64, our_node_pubkey: &PublicKey +) { + // Sort the first_hops channels to the same node(s) in priority order of which channel we'd + // most like to use. + // + // First, if channels are below `recommended_value_msat`, sort them in descending order, + // preferring larger channels to avoid splitting the payment into more MPP parts than is + // required. + // + // Second, because simply always sorting in descending order would always use our largest + // available outbound capacity, needlessly fragmenting our available channel capacities, + // sort channels above `recommended_value_msat` in ascending order, preferring channels + // which have enough, but not too much, capacity for the payment. + // + // Available outbound balances factor in liquidity already reserved for previously found paths. + channels.sort_unstable_by(|chan_a, chan_b| { + let chan_a_outbound_limit_msat = chan_a.next_outbound_htlc_limit_msat + .saturating_sub(*used_channel_liquidities.get(&(chan_a.get_outbound_payment_scid().unwrap(), + our_node_pubkey < &chan_a.counterparty.node_id)).unwrap_or(&0)); + let chan_b_outbound_limit_msat = chan_b.next_outbound_htlc_limit_msat + .saturating_sub(*used_channel_liquidities.get(&(chan_b.get_outbound_payment_scid().unwrap(), + our_node_pubkey < &chan_b.counterparty.node_id)).unwrap_or(&0)); + if chan_b_outbound_limit_msat < recommended_value_msat || chan_a_outbound_limit_msat < recommended_value_msat { + // Sort in descending order + chan_b_outbound_limit_msat.cmp(&chan_a_outbound_limit_msat) + } else { + // Sort in ascending order + chan_a_outbound_limit_msat.cmp(&chan_b_outbound_limit_msat) + } + }); +} + /// Finds a route from us (payer) to the given target node (payee). /// /// If the payee provided features in their invoice, they should be provided via `params.payee`. @@ -1442,38 +1477,9 @@ where L::Target: Logger { // when we want to stop looking for new paths. let mut already_collected_value_msat = 0; - macro_rules! sort_first_hop_channels { - ($channels: expr) => { - // Sort the first_hops channels to the same node(s) in priority order of which channel we'd - // most like to use. - // - // First, if channels are below `recommended_value_msat`, sort them in descending order, - // preferring larger channels to avoid splitting the payment into more MPP parts than is - // required. - // - // Second, because simply always sorting in descending order would always use our largest - // available outbound capacity, needlessly fragmenting our available channel capacities, - // sort channels above `recommended_value_msat` in ascending order, preferring channels - // which have enough, but not too much, capacity for the payment. - $channels.sort_unstable_by(|chan_a, chan_b| { - let chan_a_outbound_limit_msat = chan_a.next_outbound_htlc_limit_msat - .saturating_sub(*used_channel_liquidities.get(&(chan_a.get_outbound_payment_scid().unwrap(), - our_node_pubkey < &chan_a.counterparty.node_id)).unwrap_or(&0)); - let chan_b_outbound_limit_msat = chan_b.next_outbound_htlc_limit_msat - .saturating_sub(*used_channel_liquidities.get(&(chan_b.get_outbound_payment_scid().unwrap(), - our_node_pubkey < &chan_b.counterparty.node_id)).unwrap_or(&0)); - if chan_b_outbound_limit_msat < recommended_value_msat || chan_a_outbound_limit_msat < recommended_value_msat { - // Sort in descending order - chan_b_outbound_limit_msat.cmp(&chan_a_outbound_limit_msat) - } else { - // Sort in ascending order - chan_a_outbound_limit_msat.cmp(&chan_b_outbound_limit_msat) - } - }); - } - } for (_, channels) in first_hop_targets.iter_mut() { - sort_first_hop_channels!(channels); + sort_first_hop_channels(channels, &used_channel_liquidities, recommended_value_msat, + our_node_pubkey); } log_trace!(logger, "Building path from {} to payer {} for value {} msat.", @@ -1883,7 +1889,8 @@ where L::Target: Logger { let hint_candidate_contribution_msat = cmp::min(path_value_msat, candidate.effective_capacity().as_msat().saturating_sub(used_liquidity_msat)); if let Some(first_channels) = first_hop_targets.get_mut(&NodeId::from_pubkey(&prev_hop_id)) { - sort_first_hop_channels!(first_channels); + sort_first_hop_channels(first_channels, &used_channel_liquidities, + recommended_value_msat, our_node_pubkey); for details in first_channels { let first_hop_candidate = CandidateRouteHop::FirstHop { details }; add_entry!(first_hop_candidate, our_node_id, NodeId::from_pubkey(&prev_hop_id), @@ -1923,7 +1930,8 @@ where L::Target: Logger { // always assumes that the third argument is a node to which we have a // path. if let Some(first_channels) = first_hop_targets.get_mut(&NodeId::from_pubkey(&hop.src_node_id)) { - sort_first_hop_channels!(first_channels); + sort_first_hop_channels(first_channels, &used_channel_liquidities, + recommended_value_msat, our_node_pubkey); for details in first_channels { let first_hop_candidate = CandidateRouteHop::FirstHop { details }; add_entry!(first_hop_candidate, our_node_id,