mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-25 07:17:40 +01:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #1100 from TheBlueMatt/2021-09-multihop-route-hint-fix

Browse source 
Consider many first-hop paths to the same counterparty in routing
This commit is contained in:
Matt Corallo 2021-10-05 18:43:39 +00:00 • committed by GitHub
commit 17d75a845a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 94 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

100
lightning/src/routing/router.rs
View file

@ -487,12 +487,14 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
node_info.features.supports_basic_mpp() node_info.features.supports_basic_mpp()
} else { false } } else { false }
} else { false }; } else { false };
log_trace!(logger, "Searching for a route from payer {} to payee {} {} MPP", our_node_id, payee,
if allow_mpp { "with" } else { "without" });
// Step (1). // Step (1).
// Prepare the data we'll use for payee-to-payer search by // Prepare the data we'll use for payee-to-payer search by
// inserting first hops suggested by the caller as targets. // inserting first hops suggested by the caller as targets.
// Our search will then attempt to reach them while traversing from the payee node. // Our search will then attempt to reach them while traversing from the payee node.
let mut first_hop_targets: HashMap<_, (_, ChannelFeatures, _, NodeFeatures)> = let mut first_hop_targets: HashMap<_, Vec<(_, ChannelFeatures, _, NodeFeatures)>> =
HashMap::with_capacity(if first_hops.is_some() { first_hops.as_ref().unwrap().len() } else { 0 }); HashMap::with_capacity(if first_hops.is_some() { first_hops.as_ref().unwrap().len() } else { 0 });
if let Some(hops) = first_hops { if let Some(hops) = first_hops {
for chan in hops { for chan in hops {
@ -500,7 +502,8 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
if chan.counterparty.node_id == *our_node_id { if chan.counterparty.node_id == *our_node_id {
return Err(LightningError{err: "First hop cannot have our_node_id as a destination.".to_owned(), action: ErrorAction::IgnoreError}); return Err(LightningError{err: "First hop cannot have our_node_id as a destination.".to_owned(), action: ErrorAction::IgnoreError});
} }
first_hop_targets.insert(chan.counterparty.node_id, (short_channel_id, chan.counterparty.features.to_context(), chan.outbound_capacity_msat, chan.counterparty.features.to_context())); first_hop_targets.entry(chan.counterparty.node_id).or_insert(Vec::new())
.push((short_channel_id, chan.counterparty.features.to_context(), chan.outbound_capacity_msat, chan.counterparty.features.to_context()));
} }
if first_hop_targets.is_empty() { if first_hop_targets.is_empty() {
return Err(LightningError{err: "Cannot route when there are no outbound routes away from us".to_owned(), action: ErrorAction::IgnoreError}); return Err(LightningError{err: "Cannot route when there are no outbound routes away from us".to_owned(), action: ErrorAction::IgnoreError});
@ -824,8 +827,8 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
}; };
if !skip_node { if !skip_node {
if first_hops.is_some() { if let Some(first_channels) = first_hop_targets.get(&$node_id) {
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&$node_id) { for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
add_entry!(first_hop, *our_node_id, $node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat); add_entry!(first_hop, *our_node_id, $node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
} }
} }
@ -878,9 +881,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
// If first hop is a private channel and the only way to reach the payee, this is the only // If first hop is a private channel and the only way to reach the payee, this is the only
// place where it could be added. // place where it could be added.
if first_hops.is_some() { if let Some(first_channels) = first_hop_targets.get(&payee) {
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&payee) { for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0); let added = add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0);
log_trace!(logger, "{} direct route to payee via SCID {}", if added { "Added" } else { "Skipped" }, first_hop);
} }
} }
@ -897,7 +901,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
}, },
} }
// Step (1). // Step (2).
// If a caller provided us with last hops, add them to routing targets. Since this happens // If a caller provided us with last hops, add them to routing targets. Since this happens
// earlier than general path finding, they will be somewhat prioritized, although currently // earlier than general path finding, they will be somewhat prioritized, although currently
// it matters only if the fees are exactly the same. // it matters only if the fees are exactly the same.
@ -949,9 +953,11 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
} }
// Searching for a direct channel between last checked hop and first_hop_targets // Searching for a direct channel between last checked hop and first_hop_targets
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&prev_hop_id) { if let Some(first_channels) = first_hop_targets.get(&prev_hop_id) {
for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
add_entry!(first_hop, *our_node_id , prev_hop_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat); add_entry!(first_hop, *our_node_id , prev_hop_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat);
} }
}
if !hop_used { if !hop_used {
break; break;
@ -981,13 +987,15 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
// Note that we *must* check if the last hop was added as `add_entry` // Note that we *must* check if the last hop was added as `add_entry`
// always assumes that the third argument is a node to which we have a // always assumes that the third argument is a node to which we have a
// path. // path.
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&hop.src_node_id) { if let Some(first_channels) = first_hop_targets.get(&hop.src_node_id) {
for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat); add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat);
} }
} }
} }
} }
} }
}
log_trace!(logger, "Starting main path collection loop with {} nodes pre-filled from first/last hops.", targets.len()); log_trace!(logger, "Starting main path collection loop with {} nodes pre-filled from first/last hops.", targets.len());
@ -995,7 +1003,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
// last hops communicated by the caller, and the payment receiver. // last hops communicated by the caller, and the payment receiver.
let mut found_new_path = false; let mut found_new_path = false;
// Step (2). // Step (3).
// If this loop terminates due the exhaustion of targets, two situations are possible: // If this loop terminates due the exhaustion of targets, two situations are possible:
// - not enough outgoing liquidity: // - not enough outgoing liquidity:
// 0 < already_collected_value_msat < final_value_msat // 0 < already_collected_value_msat < final_value_msat
@ -1013,9 +1021,18 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
let mut ordered_hops = vec!((new_entry.clone(), NodeFeatures::empty())); let mut ordered_hops = vec!((new_entry.clone(), NodeFeatures::empty()));
'path_walk: loop { 'path_walk: loop {
if let Some(&(_, _, _, ref features)) = first_hop_targets.get(&ordered_hops.last().unwrap().0.pubkey) { let mut features_set = false;
if let Some(first_channels) = first_hop_targets.get(&ordered_hops.last().unwrap().0.pubkey) {
for (scid, _, _, ref features) in first_channels {
if *scid == ordered_hops.last().unwrap().0.short_channel_id {
ordered_hops.last_mut().unwrap().1 = features.clone(); ordered_hops.last_mut().unwrap().1 = features.clone();
} else if let Some(node) = network_nodes.get(&ordered_hops.last().unwrap().0.pubkey) { features_set = true;
break;
}
}
}
if !features_set {
if let Some(node) = network_nodes.get(&ordered_hops.last().unwrap().0.pubkey) {
if let Some(node_info) = node.announcement_info.as_ref() { if let Some(node_info) = node.announcement_info.as_ref() {
ordered_hops.last_mut().unwrap().1 = node_info.features.clone(); ordered_hops.last_mut().unwrap().1 = node_info.features.clone();
} else { } else {
@ -1028,6 +1045,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
// flags for the last hop node itself). // flags for the last hop node itself).
assert!(ordered_hops.last().unwrap().0.pubkey == *payee); assert!(ordered_hops.last().unwrap().0.pubkey == *payee);
} }
}
// Means we succesfully traversed from the payer to the payee, now // Means we succesfully traversed from the payer to the payee, now
// save this path for the payment route. Also, update the liquidity // save this path for the payment route. Also, update the liquidity
@ -1130,7 +1148,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
break 'paths_collection; break 'paths_collection;
} }
// Step (3). // Step (4).
// Stop either when the recommended value is reached or if no new path was found in this // Stop either when the recommended value is reached or if no new path was found in this
// iteration. // iteration.
// In the latter case, making another path finding attempt won't help, // In the latter case, making another path finding attempt won't help,
@ -1154,7 +1172,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
} }
} }
// Step (4). // Step (5).
if payment_paths.len() == 0 { if payment_paths.len() == 0 {
return Err(LightningError{err: "Failed to find a path to the given destination".to_owned(), action: ErrorAction::IgnoreError}); return Err(LightningError{err: "Failed to find a path to the given destination".to_owned(), action: ErrorAction::IgnoreError});
} }
@ -1175,12 +1193,12 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
let mut cur_route = Vec::<PaymentPath>::new(); let mut cur_route = Vec::<PaymentPath>::new();
let mut aggregate_route_value_msat = 0; let mut aggregate_route_value_msat = 0;
// Step (5). // Step (6).
// TODO: real random shuffle // TODO: real random shuffle
// Currently just starts with i_th and goes up to i-1_th in a looped way. // 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(); let cur_payment_paths = [&payment_paths[i..], &payment_paths[..i]].concat();
// Step (6). // Step (7).
for payment_path in cur_payment_paths { for payment_path in cur_payment_paths {
cur_route.push(payment_path.clone()); cur_route.push(payment_path.clone());
aggregate_route_value_msat += payment_path.get_value_msat(); aggregate_route_value_msat += payment_path.get_value_msat();
@ -1219,7 +1237,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
assert!(cur_route.len() > 0); assert!(cur_route.len() > 0);
// Step (7). // Step (8).
// Now, substract the overpaid value from the most-expensive path. // Now, substract the overpaid value from the most-expensive path.
// TODO: this could also be optimized by also sorting by feerate_per_sat_routed, // 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. // so that the sender pays less fees overall. And also htlc_minimum_msat.
@ -1236,7 +1254,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
drawn_routes.push(cur_route); drawn_routes.push(cur_route);
} }
// Step (8). // Step (9).
// Select the best route by lowest total fee. // Select the best route by lowest total fee.
drawn_routes.sort_by_key(|paths| paths.iter().map(|path| path.get_total_fee_paid_msat()).sum::<u64>()); drawn_routes.sort_by_key(|paths| paths.iter().map(|path| path.get_total_fee_paid_msat()).sum::<u64>());
let mut selected_paths = Vec::<Vec<RouteHop>>::new(); let mut selected_paths = Vec::<Vec<RouteHop>>::new();
@ -4220,6 +4238,50 @@ mod tests {
} }
} }
#[test]
fn multiple_direct_first_hops() {
// Previously we'd only ever considered one first hop path per counterparty.
// However, as we don't restrict users to one channel per peer, we really need to support
// looking at all first hop paths.
// Here we test that we do not ignore all-but-the-last first hop paths per counterparty (as
// we used to do by overwriting the `first_hop_targets` hashmap entry) and that we can MPP
// route over multiple channels with the same first hop.
let secp_ctx = Secp256k1::new();
let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
let logger = Arc::new(test_utils::TestLogger::new());
let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
{
let route = get_route(&our_id, &network_graph, &nodes[0], Some(InvoiceFeatures::known()), Some(&[
&get_channel_details(Some(3), nodes[0], InitFeatures::known(), 200_000),
&get_channel_details(Some(2), nodes[0], InitFeatures::known(), 10_000),
]), &[], 100_000, 42, Arc::clone(&logger)).unwrap();
assert_eq!(route.paths.len(), 1);
assert_eq!(route.paths[0].len(), 1);
assert_eq!(route.paths[0][0].pubkey, nodes[0]);
assert_eq!(route.paths[0][0].short_channel_id, 3);
assert_eq!(route.paths[0][0].fee_msat, 100_000);
}
{
let route = get_route(&our_id, &network_graph, &nodes[0], Some(InvoiceFeatures::known()), Some(&[
&get_channel_details(Some(3), nodes[0], InitFeatures::known(), 50_000),
&get_channel_details(Some(2), nodes[0], InitFeatures::known(), 50_000),
]), &[], 100_000, 42, Arc::clone(&logger)).unwrap();
assert_eq!(route.paths.len(), 2);
assert_eq!(route.paths[0].len(), 1);
assert_eq!(route.paths[1].len(), 1);
assert_eq!(route.paths[0][0].pubkey, nodes[0]);
assert_eq!(route.paths[0][0].short_channel_id, 3);
assert_eq!(route.paths[0][0].fee_msat, 50_000);
assert_eq!(route.paths[1][0].pubkey, nodes[0]);
assert_eq!(route.paths[1][0].short_channel_id, 2);
assert_eq!(route.paths[1][0].fee_msat, 50_000);
}
}
#[test] #[test]
fn total_fees_single_path() { fn total_fees_single_path() {
let route = Route { let route = Route {