// - how much is needed for a path being constructed
// - how much value can channels following this node (up to the destination) can contribute,
// considering their capacity and fees
- value_contribution_msat: u64
+ value_contribution_msat: u64,
+ /// The effective htlc_minimum_msat at this hop. If a later hop on the path had a higher HTLC
+ /// minimum, we use it, plus the fees required at each earlier hop to meet it.
+ path_htlc_minimum_msat: u64,
}
impl cmp::Ord for RouteGraphNode {
// $next_hops_fee_msat represents the fees paid for using all the channel *after* this one,
// since that value has to be transferred over this channel.
( $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,
- $next_hops_value_contribution: expr ) => {
+ $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr ) => {
// Channels to self should not be used. This is more of belt-and-suspenders, because in
// practice these cases should be caught earlier:
// - for regular channels at channel announcement (TODO)
// Can't overflow due to how the values were computed right above.
None => unreachable!(),
};
+ #[allow(unused_comparisons)] // $next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains
+ let over_path_minimum_msat = amount_to_transfer_over_msat >= $directional_info.htlc_minimum_msat &&
+ amount_to_transfer_over_msat >= $next_hops_path_htlc_minimum_msat;
// If HTLC minimum is larger than the amount we're going to transfer, we shouldn't
// bother considering this channel.
// Since we're choosing amount_to_transfer_over_msat as maximum possible, it can
// be only reduced later (not increased), so this channel should just be skipped
// as not sufficient.
- if amount_to_transfer_over_msat < $directional_info.htlc_minimum_msat {
+ if !over_path_minimum_msat {
hit_minimum_limit = true;
} else if contributes_sufficient_value {
// Note that low contribution here (limited by available_liquidity_msat)
// might violate htlc_minimum_msat on the hops which are next along the
// payment path (upstream to the payee). To avoid that, we recompute path
// path fees knowing the final path contribution after constructing it.
+ let path_htlc_minimum_msat = match compute_fees($next_hops_path_htlc_minimum_msat, $directional_info.fees)
+ .map(|fee_msat| fee_msat.checked_add($next_hops_path_htlc_minimum_msat)) {
+ Some(Some(value_msat)) => cmp::max(value_msat, $directional_info.htlc_minimum_msat),
+ _ => u64::max_value()
+ };
let hm_entry = dist.entry(&$src_node_id);
let old_entry = hm_entry.or_insert_with(|| {
// If there was previously no known way to access
lowest_fee_to_peer_through_node: total_fee_msat,
lowest_fee_to_node: $next_hops_fee_msat as u64 + hop_use_fee_msat,
value_contribution_msat: value_contribution_msat,
+ path_htlc_minimum_msat,
};
// Update the way of reaching $src_node_id with the given $chan_id (from $dest_node_id),
// meaning how much will be paid in fees after this node (to the best of our knowledge).
// This data can later be helpful to optimize routing (pay lower fees).
macro_rules! add_entries_to_cheapest_to_target_node {
- ( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr ) => {
+ ( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr ) => {
if first_hops.is_some() {
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat)) = first_hop_targets.get(&$node_id) {
- 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);
+ 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, $next_hops_path_htlc_minimum_msat);
}
}
if first_hops.is_none() || chan.node_two != *our_node_id {
if let Some(two_to_one) = chan.two_to_one.as_ref() {
if two_to_one.enabled {
- 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);
+ 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, $next_hops_path_htlc_minimum_msat);
}
}
}
if first_hops.is_none() || chan.node_one != *our_node_id {
if let Some(one_to_two) = chan.one_to_two.as_ref() {
if one_to_two.enabled {
- 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);
+ 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, $next_hops_path_htlc_minimum_msat);
}
}
// place where it could be added.
if first_hops.is_some() {
if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat)) = first_hop_targets.get(&payee) {
- add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, path_value_msat);
+ add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, path_value_msat, 0);
}
}
// If not, targets.pop() will not even let us enter the loop in step 2.
None => {},
Some(node) => {
- add_entries_to_cheapest_to_target_node!(node, payee, 0, path_value_msat);
+ add_entries_to_cheapest_to_target_node!(node, payee, 0, path_value_msat, 0);
},
}
// bit lazy here. In the future, we should pull them out via our
// ChannelManager, but there's no reason to waste the space until we
// need them.
- add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, path_value_msat);
+ add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, path_value_msat, 0);
true
} else {
// In any other case, only add the hop if the source is in the regular network
htlc_maximum_msat: hop.htlc_maximum_msat,
fees: hop.fees,
};
- add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, ChannelFeatures::empty(), 0, path_value_msat);
+ add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, ChannelFeatures::empty(), 0, path_value_msat, 0);
}
}
// Both these cases (and other cases except reaching recommended_value_msat) mean that
// paths_collection will be stopped because found_new_path==false.
// This is not necessarily a routing failure.
- 'path_construction: while let Some(RouteGraphNode { pubkey, lowest_fee_to_node, value_contribution_msat, .. }) = targets.pop() {
+ 'path_construction: while let Some(RouteGraphNode { pubkey, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat, .. }) = targets.pop() {
// Since we're going payee-to-payer, hitting our node as a target means we should stop
// traversing the graph and arrange the path out of what we found.
match network.get_nodes().get(&pubkey) {
None => {},
Some(node) => {
- add_entries_to_cheapest_to_target_node!(node, &pubkey, lowest_fee_to_node, value_contribution_msat);
+ add_entries_to_cheapest_to_target_node!(node, &pubkey, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat);
},
}
}
assert_eq!(route.paths[0][1].channel_features.le_flags(), &id_to_feature_flags(13));
}
}
+
+ #[test]
+ fn htlc_max_reduction_below_min() {
+ // Test that if, while walking the graph, we reduce the value being sent to meet an
+ // htlc_maximum_msat, we don't end up undershooting a later htlc_minimum_msat. In the
+ // initial version of MPP we'd accept such routes but reject them while recalculating fees,
+ // resulting in us thinking there is no possible path, even if other paths exist.
+ let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
+ let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
+
+ // We modify the graph to set the htlc_minimum of channel 2 and 4 as needed - channel 2
+ // gets an htlc_maximum_msat of 80_000 and channel 4 an htlc_minimum_msat of 90_000. We
+ // then try to send 90_000.
+ update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
+ chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+ short_channel_id: 2,
+ timestamp: 2,
+ flags: 0,
+ cltv_expiry_delta: 0,
+ htlc_minimum_msat: 0,
+ htlc_maximum_msat: OptionalField::Present(80_000),
+ fee_base_msat: 0,
+ fee_proportional_millionths: 0,
+ excess_data: Vec::new()
+ });
+ update_channel(&net_graph_msg_handler, &secp_ctx, &privkeys[1], UnsignedChannelUpdate {
+ chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+ short_channel_id: 4,
+ timestamp: 2,
+ flags: 0,
+ cltv_expiry_delta: (4 << 8) | 1,
+ htlc_minimum_msat: 90_000,
+ htlc_maximum_msat: OptionalField::Absent,
+ fee_base_msat: 0,
+ fee_proportional_millionths: 0,
+ excess_data: Vec::new()
+ });
+
+ {
+ // Now, attempt to route 90 sats, hitting the htlc_minimum on channel 4, but
+ // overshooting the htlc_maximum on channel 2. Thus, we should pick the (absurdly
+ // expensive) channels 12-13 path.
+ let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], Some(InvoiceFeatures::known()), None, &Vec::new(), 90_000, 42, Arc::clone(&logger)).unwrap();
+ assert_eq!(route.paths.len(), 1);
+ assert_eq!(route.paths[0].len(), 2);
+
+ assert_eq!(route.paths[0][0].pubkey, nodes[7]);
+ assert_eq!(route.paths[0][0].short_channel_id, 12);
+ assert_eq!(route.paths[0][0].fee_msat, 90_000*2);
+ assert_eq!(route.paths[0][0].cltv_expiry_delta, (13 << 8) | 1);
+ assert_eq!(route.paths[0][0].node_features.le_flags(), &id_to_feature_flags(8));
+ assert_eq!(route.paths[0][0].channel_features.le_flags(), &id_to_feature_flags(12));
+
+ assert_eq!(route.paths[0][1].pubkey, nodes[2]);
+ assert_eq!(route.paths[0][1].short_channel_id, 13);
+ assert_eq!(route.paths[0][1].fee_msat, 90_000);
+ assert_eq!(route.paths[0][1].cltv_expiry_delta, 42);
+ assert_eq!(route.paths[0][1].node_features.le_flags(), InvoiceFeatures::known().le_flags());
+ assert_eq!(route.paths[0][1].channel_features.le_flags(), &id_to_feature_flags(13));
+ }
+ }
}
#[cfg(all(test, feature = "unstable"))]
projects / rust-lightning / commitdiff