Skip to content

Commit beffe75

Browse files
committed
Handle overflowing route-hint fee aggregates
Crafted route hints can overflow aggregate downstream proportional fees when the payer disables the routing fee cap. Treat such paths as unusable so route finding fails cleanly instead of panicking. Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>
1 parent c8f9833 commit beffe75

1 file changed

Lines changed: 90 additions & 4 deletions

File tree

lightning/src/routing/router.rs

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2421,10 +2421,12 @@ impl<'a> PaymentPath<'a> {
24212421
/// contribution this path can make to the final value of the payment.
24222422
/// May be slightly lower than the actual max due to rounding errors when aggregating fees
24232423
/// along the path.
2424+
/// Returns an error with the index of a later hop to discard if the following hops' aggregate
2425+
/// fees overflow.
24242426
#[rustfmt::skip]
24252427
fn max_final_value_msat(
24262428
&self, used_liquidities: &HashMap<CandidateHopId, u64>, channel_saturation_pow_half: u8
2427-
) -> (usize, u64) {
2429+
) -> Result<(usize, u64), usize> {
24282430
let mut max_path_contribution = (0, u64::MAX);
24292431
for (idx, (hop, _)) in self.hops.iter().enumerate() {
24302432
let hop_effective_capacity_msat = hop.candidate.effective_capacity();
@@ -2440,7 +2442,8 @@ impl<'a> PaymentPath<'a> {
24402442
// Aggregate the fees of the hops that come after this one, and use those fees to compute the
24412443
// maximum amount that this hop can contribute to the final value received by the payee.
24422444
let (next_hops_aggregated_base, next_hops_aggregated_prop) =
2443-
crate::blinded_path::payment::compute_aggregated_base_prop_fee(next_hops_feerates_iter).unwrap();
2445+
crate::blinded_path::payment::compute_aggregated_base_prop_fee(next_hops_feerates_iter)
2446+
.map_err(|_| idx + 1)?;
24442447

24452448
// floor(((hop_max_msat - agg_base) * 1_000_000) / (1_000_000 + agg_prop))
24462449
let hop_max_final_value_contribution = (hop_max_msat as u128)
@@ -2457,7 +2460,19 @@ impl<'a> PaymentPath<'a> {
24572460
} else { debug_assert!(false); }
24582461
}
24592462

2460-
max_path_contribution
2463+
Ok(max_path_contribution)
2464+
}
2465+
}
2466+
2467+
fn mark_candidate_liquidity_exhausted(
2468+
used_liquidities: &mut HashMap<CandidateHopId, u64>, candidate: &CandidateRouteHop,
2469+
) {
2470+
let exhausted = u64::max_value();
2471+
if let Some(scid) = candidate.short_channel_id() {
2472+
*used_liquidities.entry(CandidateHopId::Clear((scid, false))).or_default() = exhausted;
2473+
*used_liquidities.entry(CandidateHopId::Clear((scid, true))).or_default() = exhausted;
2474+
} else {
2475+
*used_liquidities.entry(candidate.id()).or_default() = exhausted;
24612476
}
24622477
}
24632478

@@ -3637,7 +3652,17 @@ pub(crate) fn get_route<L: Logger, S: ScoreLookUp>(
36373652
// underpaid htlc_minimum_msat with fees.
36383653
debug_assert_eq!(payment_path.get_value_msat(), value_contribution_msat);
36393654
let (lowest_value_contrib_hop, max_path_contribution_msat) =
3640-
payment_path.max_final_value_msat(&used_liquidities, channel_saturation_pow_half);
3655+
match payment_path.max_final_value_msat(&used_liquidities, channel_saturation_pow_half) {
3656+
Ok(contribution) => contribution,
3657+
Err(candidate_idx_to_skip) => {
3658+
let candidate = &payment_path.hops[candidate_idx_to_skip].0.candidate;
3659+
log_trace!(logger,
3660+
"Ignoring path because aggregate fees including hop {} overflow.",
3661+
LoggedCandidateHop(candidate));
3662+
mark_candidate_liquidity_exhausted(&mut used_liquidities, candidate);
3663+
continue 'paths_collection;
3664+
}
3665+
};
36413666
let desired_value_contribution = cmp::min(max_path_contribution_msat, final_value_msat);
36423667
value_contribution_msat = payment_path.update_value_and_recompute_fees(desired_value_contribution);
36433668

@@ -9332,6 +9357,67 @@ mod tests {
93329357
assert_eq!(route.paths[0].hops[0].short_channel_id, 44);
93339358
}
93349359

9360+
#[test]
9361+
fn aggregated_prop_fee_overflow_fails_route() {
9362+
// If the fee cap is disabled, we may consider invoice hints with very large
9363+
// proportional fees. Aggregating those fees can overflow, in which case we should fail
9364+
// routing cleanly rather than panic.
9365+
let secp_ctx = Secp256k1::new();
9366+
let logger = Arc::new(ln_test_utils::TestLogger::new());
9367+
let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, Arc::clone(&logger)));
9368+
let scorer = ln_test_utils::TestScorer::new();
9369+
let random_seed_bytes = [42; 32];
9370+
let config = UserConfig::default();
9371+
9372+
let (_, our_node_id, _, nodes) = get_nodes(&secp_ctx);
9373+
let route_hint = RouteHint(vec![
9374+
RouteHintHop {
9375+
src_node_id: nodes[0],
9376+
short_channel_id: 100,
9377+
fees: RoutingFees { base_msat: 0, proportional_millionths: u32::MAX },
9378+
cltv_expiry_delta: 10,
9379+
htlc_minimum_msat: None,
9380+
htlc_maximum_msat: None,
9381+
},
9382+
RouteHintHop {
9383+
src_node_id: nodes[1],
9384+
short_channel_id: 101,
9385+
fees: RoutingFees { base_msat: 0, proportional_millionths: u32::MAX },
9386+
cltv_expiry_delta: 10,
9387+
htlc_minimum_msat: None,
9388+
htlc_maximum_msat: None,
9389+
},
9390+
]);
9391+
9392+
let payment_params = PaymentParameters::from_node_id(nodes[2], 42)
9393+
.with_route_hints(vec![route_hint])
9394+
.unwrap()
9395+
.with_bolt11_features(channelmanager::provided_bolt11_invoice_features(&config))
9396+
.unwrap();
9397+
let first_hops = [get_channel_details(
9398+
Some(1),
9399+
nodes[0],
9400+
channelmanager::provided_init_features(&config),
9401+
100_000_000,
9402+
)];
9403+
let route_params = RouteParameters {
9404+
payment_params,
9405+
final_value_msat: 1,
9406+
max_total_routing_fee_msat: None,
9407+
};
9408+
let route = get_route(
9409+
&our_node_id,
9410+
&route_params,
9411+
&network_graph.read_only(),
9412+
Some(&first_hops.iter().collect::<Vec<_>>()),
9413+
Arc::clone(&logger),
9414+
&scorer,
9415+
&Default::default(),
9416+
&random_seed_bytes,
9417+
);
9418+
assert!(route.is_err());
9419+
}
9420+
93359421
#[test]
93369422
fn prefers_paths_by_cost_amt_ratio() {
93379423
// Previously, we preferred paths during MPP selection based on their absolute cost, rather

0 commit comments

Comments
 (0)