Skip to content

Commit 8896492

Browse files
TheBlueMattnaumenkogs
authored andcommitted
Track full-path htlc-minimum-msat while routing
Previously, we'd happily send funds through a path where, while generating the path, in some middle hope we reduce the value being sent to meet an htlc_maximum, making a later hop invalid due to it no longer meeting its htlc_minimum. Instead, we need to track the path's htlc-minimum while we're transiting the graph.
1 parent 7699b13 commit 8896492

File tree

1 file changed

+27
-15
lines changed

1 file changed

+27
-15
lines changed

lightning/src/routing/router.rs

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,10 @@ struct RouteGraphNode {
141141
// - how much is needed for a path being constructed
142142
// - how much value can channels following this node (up to the destination) can contribute,
143143
// considering their capacity and fees
144-
value_contribution_msat: u64
144+
value_contribution_msat: u64,
145+
/// The maximum htlc_minimum_msat along the path, taking into consideration the fees required
146+
/// to meet the minimum over the hops required to get there.
147+
path_htlc_minimum_msat: u64,
145148
}
146149

147150
impl cmp::Ord for RouteGraphNode {
@@ -244,6 +247,9 @@ impl PaymentPath {
244247
assert!(value_msat < self.hops.last().unwrap().route_hop.fee_msat);
245248

246249
let mut total_fee_paid_msat = 0 as u64;
250+
// TODO: while reducing the transferred value on hop N, we could cause some N+M hop to fall
251+
// below its htlc_minimum_msat in terms of amount being transferred that. We should handle
252+
// it here similarly to how we handle it in add_entry!.
247253
for i in (0..self.hops.len()).rev() {
248254
let last_hop = i == self.hops.len() - 1;
249255

@@ -432,7 +438,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
432438
// $next_hops_fee_msat represents the fees paid for using all the channel *after* this one,
433439
// since that value has to be transferred over this channel.
434440
( $chan_id: expr, $src_node_id: expr, $dest_node_id: expr, $directional_info: expr, $capacity_sats: expr, $chan_features: expr, $next_hops_fee_msat: expr,
435-
$next_hops_value_contribution: expr ) => {
441+
$next_hops_value_contribution: expr, $incl_fee_next_hops_htlc_minimum_msat: expr ) => {
436442
// Channels to self should not be used. This is more of belt-and-suspenders, because in
437443
// practice these cases should be caught earlier:
438444
// - for regular channels at channel announcement (TODO)
@@ -494,14 +500,16 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
494500
// Can't overflow due to how the values were computed right above.
495501
None => unreachable!(),
496502
};
503+
let over_path_minimum_msat = amount_to_transfer_over_msat >= $directional_info.htlc_minimum_msat &&
504+
amount_to_transfer_over_msat >= $incl_fee_next_hops_htlc_minimum_msat;
497505

498506
// If HTLC minimum is larger than the amount we're going to transfer, we shouldn't
499507
// bother considering this channel.
500508
// Since we're choosing amount_to_transfer_over_msat as maximum possible, it can
501509
// be only reduced later (not increased), so this channel should just be skipped
502510
// as not sufficient.
503511
// TODO: Explore simply adding fee to hit htlc_minimum_msat
504-
if contributes_sufficient_value && amount_to_transfer_over_msat >= $directional_info.htlc_minimum_msat {
512+
if contributes_sufficient_value && over_path_minimum_msat {
505513
let hm_entry = dist.entry(&$src_node_id);
506514
let old_entry = hm_entry.or_insert_with(|| {
507515
// If there was previously no known way to access
@@ -573,6 +581,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
573581
lowest_fee_to_peer_through_node: total_fee_msat,
574582
lowest_fee_to_node: $next_hops_fee_msat as u64 + hop_use_fee_msat,
575583
value_contribution_msat: value_contribution_msat,
584+
path_htlc_minimum_msat: match compute_fees($incl_fee_next_hops_htlc_minimum_msat, $directional_info.fees) {
585+
Some(fee_msat) => cmp::max(fee_msat, $directional_info.htlc_minimum_msat),
586+
None => u64::max_value(),
587+
},
576588
};
577589

578590
// Update the way of reaching $src_node_id with the given $chan_id (from $dest_node_id),
@@ -632,10 +644,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
632644
// meaning how much will be paid in fees after this node (to the best of our knowledge).
633645
// This data can later be helpful to optimize routing (pay lower fees).
634646
macro_rules! add_entries_to_cheapest_to_target_node {
635-
( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr ) => {
647+
( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr, $incl_fee_next_hops_htlc_minimum_msat: expr ) => {
636648
if first_hops.is_some() {
637649
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat)) = first_hop_targets.get(&$node_id) {
638-
add_entry!(first_hop, *our_node_id, $node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), $fee_to_target_msat, $next_hops_value_contribution);
650+
add_entry!(first_hop, *our_node_id, $node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), $fee_to_target_msat, $next_hops_value_contribution, $incl_fee_next_hops_htlc_minimum_msat);
639651
}
640652
}
641653

@@ -655,15 +667,15 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
655667
if first_hops.is_none() || chan.node_two != *our_node_id {
656668
if let Some(two_to_one) = chan.two_to_one.as_ref() {
657669
if two_to_one.enabled {
658-
add_entry!(chan_id, chan.node_two, chan.node_one, two_to_one, chan.capacity_sats, chan.features, $fee_to_target_msat, $next_hops_value_contribution);
670+
add_entry!(chan_id, chan.node_two, chan.node_one, two_to_one, chan.capacity_sats, chan.features, $fee_to_target_msat, $next_hops_value_contribution, $incl_fee_next_hops_htlc_minimum_msat);
659671
}
660672
}
661673
}
662674
} else {
663675
if first_hops.is_none() || chan.node_one != *our_node_id {
664676
if let Some(one_to_two) = chan.one_to_two.as_ref() {
665677
if one_to_two.enabled {
666-
add_entry!(chan_id, chan.node_one, chan.node_two, one_to_two, chan.capacity_sats, chan.features, $fee_to_target_msat, $next_hops_value_contribution);
678+
add_entry!(chan_id, chan.node_one, chan.node_two, one_to_two, chan.capacity_sats, chan.features, $fee_to_target_msat, $next_hops_value_contribution, $incl_fee_next_hops_htlc_minimum_msat);
667679
}
668680
}
669681

@@ -689,7 +701,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
689701
// place where it could be added.
690702
if first_hops.is_some() {
691703
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat)) = first_hop_targets.get(&payee) {
692-
add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, recommended_value_msat);
704+
add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, recommended_value_msat, 0);
693705
}
694706
}
695707

@@ -702,7 +714,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
702714
// If not, targets.pop() will not even let us enter the loop in step 2.
703715
None => {},
704716
Some(node) => {
705-
add_entries_to_cheapest_to_target_node!(node, payee, 0, recommended_value_msat);
717+
add_entries_to_cheapest_to_target_node!(node, payee, 0, recommended_value_msat, 0);
706718
},
707719
}
708720

@@ -721,7 +733,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
721733
// bit lazy here. In the future, we should pull them out via our
722734
// ChannelManager, but there's no reason to waste the space until we
723735
// need them.
724-
add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, recommended_value_msat);
736+
add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, recommended_value_msat, 0);
725737
true
726738
} else {
727739
// In any other case, only add the hop if the source is in the regular network
@@ -731,17 +743,17 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
731743
if have_hop_src_in_graph {
732744
// BOLT 11 doesn't allow inclusion of features for the last hop hints, which
733745
// really sucks, cause we're gonna need that eventually.
734-
let last_hop_htlc_minimum_msat: u64 = match hop.htlc_minimum_msat {
746+
let last_path_htlc_minimum_msat: u64 = match hop.htlc_minimum_msat {
735747
Some(htlc_minimum_msat) => htlc_minimum_msat,
736748
None => 0
737749
};
738750
let directional_info = DummyDirectionalChannelInfo {
739751
cltv_expiry_delta: hop.cltv_expiry_delta as u32,
740-
htlc_minimum_msat: last_hop_htlc_minimum_msat,
752+
htlc_minimum_msat: last_path_htlc_minimum_msat,
741753
htlc_maximum_msat: hop.htlc_maximum_msat,
742754
fees: hop.fees,
743755
};
744-
add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, ChannelFeatures::empty(), 0, recommended_value_msat);
756+
add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, ChannelFeatures::empty(), 0, recommended_value_msat, 0);
745757
}
746758
}
747759

@@ -758,7 +770,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
758770
// Both these cases (and other cases except reaching recommended_value_msat) mean that
759771
// paths_collection will be stopped because found_new_path==false.
760772
// This is not necessarily a routing failure.
761-
'path_construction: while let Some(RouteGraphNode { pubkey, lowest_fee_to_node, value_contribution_msat, .. }) = targets.pop() {
773+
'path_construction: while let Some(RouteGraphNode { pubkey, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat, .. }) = targets.pop() {
762774

763775
// Since we're going payee-to-payer, hitting our node as a target means we should stop
764776
// traversing the graph and arrange the path out of what we found.
@@ -855,7 +867,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
855867
match network.get_nodes().get(&pubkey) {
856868
None => {},
857869
Some(node) => {
858-
add_entries_to_cheapest_to_target_node!(node, &pubkey, lowest_fee_to_node, value_contribution_msat);
870+
add_entries_to_cheapest_to_target_node!(node, &pubkey, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat);
859871
},
860872
}
861873
}

0 commit comments

Comments
 (0)