Skip to content

Commit 39dae9e

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 7e8fe34 commit 39dae9e

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 {
@@ -245,6 +248,9 @@ impl PaymentPath {
245248
assert!(value_msat < self.hops.last().unwrap().route_hop.fee_msat);
246249

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

@@ -433,7 +439,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
433439
// $next_hops_fee_msat represents the fees paid for using all the channel *after* this one,
434440
// since that value has to be transferred over this channel.
435441
( $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,
436-
$next_hops_value_contribution: expr ) => {
442+
$next_hops_value_contribution: expr, $incl_fee_next_hops_htlc_minimum_msat: expr ) => {
437443
// Channels to self should not be used. This is more of belt-and-suspenders, because in
438444
// practice these cases should be caught earlier:
439445
// - for regular channels at channel announcement (TODO)
@@ -495,14 +501,16 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
495501
// Can't overflow due to how the values were computed right above.
496502
None => unreachable!(),
497503
};
504+
let over_path_minimum_msat = amount_to_transfer_over_msat >= $directional_info.htlc_minimum_msat &&
505+
amount_to_transfer_over_msat >= $incl_fee_next_hops_htlc_minimum_msat;
498506

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

579591
// Update the way of reaching $src_node_id with the given $chan_id (from $dest_node_id),
@@ -633,10 +645,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
633645
// meaning how much will be paid in fees after this node (to the best of our knowledge).
634646
// This data can later be helpful to optimize routing (pay lower fees).
635647
macro_rules! add_entries_to_cheapest_to_target_node {
636-
( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr ) => {
648+
( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr, $incl_fee_next_hops_htlc_minimum_msat: expr ) => {
637649
if first_hops.is_some() {
638650
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat)) = first_hop_targets.get(&$node_id) {
639-
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);
651+
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);
640652
}
641653
}
642654

@@ -656,15 +668,15 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
656668
if first_hops.is_none() || chan.node_two != *our_node_id {
657669
if let Some(two_to_one) = chan.two_to_one.as_ref() {
658670
if two_to_one.enabled {
659-
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);
671+
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);
660672
}
661673
}
662674
}
663675
} else {
664676
if first_hops.is_none() || chan.node_one != *our_node_id {
665677
if let Some(one_to_two) = chan.one_to_two.as_ref() {
666678
if one_to_two.enabled {
667-
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);
679+
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);
668680
}
669681
}
670682

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

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

@@ -722,7 +734,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
722734
// bit lazy here. In the future, we should pull them out via our
723735
// ChannelManager, but there's no reason to waste the space until we
724736
// need them.
725-
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);
737+
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);
726738
true
727739
} else {
728740
// In any other case, only add the hop if the source is in the regular network
@@ -732,17 +744,17 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
732744
if have_hop_src_in_graph {
733745
// BOLT 11 doesn't allow inclusion of features for the last hop hints, which
734746
// really sucks, cause we're gonna need that eventually.
735-
let last_hop_htlc_minimum_msat: u64 = match hop.htlc_minimum_msat {
747+
let last_path_htlc_minimum_msat: u64 = match hop.htlc_minimum_msat {
736748
Some(htlc_minimum_msat) => htlc_minimum_msat,
737749
None => 0
738750
};
739751
let directional_info = DummyDirectionalChannelInfo {
740752
cltv_expiry_delta: hop.cltv_expiry_delta as u32,
741-
htlc_minimum_msat: last_hop_htlc_minimum_msat,
753+
htlc_minimum_msat: last_path_htlc_minimum_msat,
742754
htlc_maximum_msat: hop.htlc_maximum_msat,
743755
fees: hop.fees,
744756
};
745-
add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, ChannelFeatures::empty(), 0, recommended_value_msat);
757+
add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, ChannelFeatures::empty(), 0, recommended_value_msat, 0);
746758
}
747759
}
748760

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

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

0 commit comments

Comments
 (0)