Skip to content

Commit 8effd86

Browse files
committed
Include any recipient overpayment amounts in the route fee limit
If the user told us to limit their total fee exposure, we should do so including any potential overpayment to the recipient, which is ultimately a part of the "fee" as far as the user is concerned.
1 parent f3e33f4 commit 8effd86

File tree

1 file changed

+22
-10
lines changed

1 file changed

+22
-10
lines changed

lightning/src/routing/router.rs

+22-10
Original file line numberDiff line numberDiff line change
@@ -2628,21 +2628,22 @@ where L::Target: Logger {
26282628
// Make sure we would never create a route with more paths than we allow.
26292629
debug_assert!(paths.len() <= payment_params.max_path_count.into());
26302630

2631-
// Make sure we would never create a route whose total fees exceed max_total_routing_fee_msat.
2632-
if let Some(max_total_routing_fee_msat) = route_params.max_total_routing_fee_msat {
2633-
if paths.iter().map(|p| p.fee_msat()).sum::<u64>() > max_total_routing_fee_msat {
2634-
return Err(LightningError{err: format!("Failed to find route that adheres to the maximum total fee limit of {}msat",
2635-
max_total_routing_fee_msat), action: ErrorAction::IgnoreError});
2636-
}
2637-
}
2638-
26392631
if let Some(node_features) = payment_params.payee.node_features() {
26402632
for path in paths.iter_mut() {
26412633
path.hops.last_mut().unwrap().node_features = node_features.clone();
26422634
}
26432635
}
26442636

26452637
let route = Route { paths, route_params: Some(route_params.clone()) };
2638+
2639+
// Make sure we would never create a route whose total fees exceed max_total_routing_fee_msat.
2640+
if let Some(max_total_routing_fee_msat) = route_params.max_total_routing_fee_msat {
2641+
if route.get_total_fees() > max_total_routing_fee_msat {
2642+
return Err(LightningError{err: format!("Failed to find route that adheres to the maximum total fee limit of {}msat",
2643+
max_total_routing_fee_msat), action: ErrorAction::IgnoreError});
2644+
}
2645+
}
2646+
26462647
log_info!(logger, "Got route: {}", log_route!(route));
26472648
Ok(route)
26482649
}
@@ -3266,11 +3267,22 @@ mod tests {
32663267
excess_data: Vec::new()
32673268
});
32683269

3269-
// Now check that we'll find a path if the htlc_minimum is overrun substantially.
3270+
// Now check that we'll fail to find a path if we fail to find a path if the htlc_minimum
3271+
// is overrun. Note that the fees are actually calculated on 3*payment amount as that's
3272+
// what we try to find a route for, so this test only just happens to work out to exactly
3273+
// the fee limit.
32703274
let mut route_params = RouteParameters::from_payment_params_and_value(
32713275
payment_params.clone(), 5_000);
3272-
// TODO: This can even overrun the fee limit set by the recipient!
32733276
route_params.max_total_routing_fee_msat = Some(9_999);
3277+
if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
3278+
&route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer,
3279+
&Default::default(), &random_seed_bytes) {
3280+
assert_eq!(err, "Failed to find route that adheres to the maximum total fee limit of 9999msat");
3281+
} else { panic!(); }
3282+
3283+
let mut route_params = RouteParameters::from_payment_params_and_value(
3284+
payment_params.clone(), 5_000);
3285+
route_params.max_total_routing_fee_msat = Some(10_000);
32743286
let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
32753287
Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
32763288
assert_eq!(route.get_total_fees(), 10_000);

0 commit comments

Comments
 (0)