Skip to content
Merged
251 changes: 240 additions & 11 deletions lightning/src/routing/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1489,6 +1489,9 @@ where L::Target: Logger {
if payee_node_id_opt.map_or(false, |payee| payee == our_node_id) {
return Err(LightningError{err: "Cannot generate a route to ourselves".to_owned(), action: ErrorAction::IgnoreError});
}
if our_node_id == maybe_dummy_payee_node_id {
return Err(LightningError{err: "Invalid origin node id provided, use a different one".to_owned(), action: ErrorAction::IgnoreError});
}

if final_value_msat > MAX_VALUE_MSAT {
return Err(LightningError{err: "Cannot generate a route of more value than all existing satoshis".to_owned(), action: ErrorAction::IgnoreError});
Expand Down Expand Up @@ -1707,13 +1710,13 @@ where L::Target: Logger {
// Adds entry which goes from $src_node_id to $dest_node_id over the $candidate hop.
// $next_hops_fee_msat represents the fees paid for using all the channels *after* this one,
// since that value has to be transferred over this channel.
// Returns whether this channel caused an update to `targets`.
// Returns the contribution amount of $candidate if the channel caused an update to `targets`.
( $candidate: expr, $src_node_id: expr, $dest_node_id: expr, $next_hops_fee_msat: expr,
$next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr,
$next_hops_path_penalty_msat: expr, $next_hops_cltv_delta: expr, $next_hops_path_length: expr ) => { {
// We "return" whether we updated the path at the end, and how much we can route via
// this channel, via this:
let mut did_add_update_path_to_src_node = None;
let mut hop_contribution_amt_msat = None;
// 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)
Expand Down Expand Up @@ -1781,6 +1784,7 @@ where L::Target: Logger {
CandidateRouteHop::FirstHop { .. } => true,
CandidateRouteHop::PrivateHop { .. } => true,
CandidateRouteHop::Blinded { .. } => true,
CandidateRouteHop::OneHopBlinded { .. } => true,
_ => false,
};

Expand Down Expand Up @@ -1926,7 +1930,7 @@ where L::Target: Logger {
{
old_entry.value_contribution_msat = value_contribution_msat;
}
did_add_update_path_to_src_node = Some(value_contribution_msat);
hop_contribution_amt_msat = Some(value_contribution_msat);
} else if old_entry.was_processed && new_cost < old_cost {
#[cfg(all(not(ldk_bench), any(test, fuzzing)))]
{
Expand Down Expand Up @@ -1961,7 +1965,7 @@ where L::Target: Logger {
}
}
}
did_add_update_path_to_src_node
hop_contribution_amt_msat
} }
}

Expand Down Expand Up @@ -2079,7 +2083,7 @@ where L::Target: Logger {
// in the regular network graph.
first_hop_targets.get(&intro_node_id).is_some() ||
network_nodes.get(&intro_node_id).is_some();
if !have_intro_node_in_graph { continue }
if !have_intro_node_in_graph || our_node_id == intro_node_id { continue }
let candidate = if hint.1.blinded_hops.len() == 1 {
CandidateRouteHop::OneHopBlinded { hint, hint_idx }
} else { CandidateRouteHop::Blinded { hint, hint_idx } };
Expand Down Expand Up @@ -2200,11 +2204,15 @@ where L::Target: Logger {
.map_or(None, |inc| inc.checked_add(aggregate_next_hops_fee_msat));
aggregate_next_hops_fee_msat = if let Some(val) = hops_fee { val } else { break; };

let hop_htlc_minimum_msat = candidate.htlc_minimum_msat();
let hop_htlc_minimum_msat_inc = if let Some(val) = compute_fees(aggregate_next_hops_path_htlc_minimum_msat, hop.fees) { val } else { break; };
let hops_path_htlc_minimum = aggregate_next_hops_path_htlc_minimum_msat
.checked_add(hop_htlc_minimum_msat_inc);
aggregate_next_hops_path_htlc_minimum_msat = if let Some(val) = hops_path_htlc_minimum { cmp::max(hop_htlc_minimum_msat, val) } else { break; };
// The next channel will need to relay this channel's min_htlc *plus* the fees taken by
// this route hint's source node to forward said min over this channel.
aggregate_next_hops_path_htlc_minimum_msat = {
let curr_htlc_min = cmp::max(
candidate.htlc_minimum_msat(), aggregate_next_hops_path_htlc_minimum_msat
);
let curr_htlc_min_fee = if let Some(val) = compute_fees(curr_htlc_min, hop.fees) { val } else { break };
if let Some(min) = curr_htlc_min.checked_add(curr_htlc_min_fee) { min } else { break }
};

if idx == route.0.len() - 1 {
// The last hop in this iterator is the first hop in
Expand Down Expand Up @@ -2884,7 +2892,7 @@ mod tests {
Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes) {
assert_eq!(err, "First hop cannot have our_node_pubkey as a destination.");
} else { panic!(); }

let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
assert_eq!(route.paths[0].hops.len(), 2);
Expand Down Expand Up @@ -7166,6 +7174,227 @@ mod tests {
assert_eq!(route.get_total_fees(), blinded_payinfo.fee_base_msat as u64);
assert_eq!(route.get_total_amount(), amt_msat);
}

#[test]
fn we_are_intro_node_candidate_hops() {
// This previously led to a panic in the router because we'd generate a Path with only a
// BlindedTail and 0 unblinded hops, due to the only candidate hops being blinded route hints
// where the origin node is the intro node. We now fully disallow considering candidate hops
// where the origin node is the intro node.
let (secp_ctx, network_graph, _, _, logger) = build_graph();
let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
let scorer = ln_test_utils::TestScorer::new();
let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
let random_seed_bytes = keys_manager.get_secure_random_bytes();
let config = UserConfig::default();

// Values are taken from the fuzz input that uncovered this panic.
let amt_msat = 21_7020_5185_1423_0019;

let blinded_path = BlindedPath {
introduction_node_id: our_id,
blinding_point: ln_test_utils::pubkey(42),
blinded_hops: vec![
BlindedHop { blinded_node_id: ln_test_utils::pubkey(42 as u8), encrypted_payload: Vec::new() },
BlindedHop { blinded_node_id: ln_test_utils::pubkey(42 as u8), encrypted_payload: Vec::new() }
],
};
let blinded_payinfo = BlindedPayInfo {
fee_base_msat: 5052_9027,
fee_proportional_millionths: 0,
htlc_minimum_msat: 21_7020_5185_1423_0019,
htlc_maximum_msat: 1844_6744_0737_0955_1615,
cltv_expiry_delta: 0,
features: BlindedHopFeatures::empty(),
};
let mut blinded_hints = vec![
(blinded_payinfo.clone(), blinded_path.clone()),
(blinded_payinfo.clone(), blinded_path.clone()),
];
blinded_hints[1].1.introduction_node_id = nodes[6];

let bolt12_features: Bolt12InvoiceFeatures = channelmanager::provided_invoice_features(&config).to_context();
let payment_params = PaymentParameters::blinded(blinded_hints.clone())
.with_bolt12_features(bolt12_features.clone()).unwrap();

let netgraph = network_graph.read_only();
let route_params = RouteParameters::from_payment_params_and_value(
payment_params, amt_msat);
if let Err(LightningError { err, .. }) = get_route(
&our_id, &route_params, &netgraph, None, Arc::clone(&logger), &scorer, &(), &random_seed_bytes
) {
assert_eq!(err, "Failed to find a path to the given destination");
} else { panic!() }
}

#[test]
fn we_are_intro_node_bp_in_final_path_fee_calc() {
// This previously led to a debug panic in the router because we'd find an invalid Path with
// 0 unblinded hops and a blinded tail, leading to the generation of a final
// PaymentPathHop::fee_msat that included both the blinded path fees and the final value of
// the payment, when it was intended to only include the final value of the payment.
let (secp_ctx, network_graph, _, _, logger) = build_graph();
let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
let scorer = ln_test_utils::TestScorer::new();
let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
let random_seed_bytes = keys_manager.get_secure_random_bytes();
let config = UserConfig::default();

// Values are taken from the fuzz input that uncovered this panic.
let amt_msat = 21_7020_5185_1423_0019;

let blinded_path = BlindedPath {
introduction_node_id: our_id,
blinding_point: ln_test_utils::pubkey(42),
blinded_hops: vec![
BlindedHop { blinded_node_id: ln_test_utils::pubkey(42 as u8), encrypted_payload: Vec::new() },
BlindedHop { blinded_node_id: ln_test_utils::pubkey(42 as u8), encrypted_payload: Vec::new() }
],
};
let blinded_payinfo = BlindedPayInfo {
fee_base_msat: 10_4425_1395,
fee_proportional_millionths: 0,
htlc_minimum_msat: 21_7301_9934_9094_0931,
htlc_maximum_msat: 1844_6744_0737_0955_1615,
cltv_expiry_delta: 0,
features: BlindedHopFeatures::empty(),
};
let mut blinded_hints = vec![
(blinded_payinfo.clone(), blinded_path.clone()),
(blinded_payinfo.clone(), blinded_path.clone()),
(blinded_payinfo.clone(), blinded_path.clone()),
];
blinded_hints[1].0.fee_base_msat = 5052_9027;
blinded_hints[1].0.htlc_minimum_msat = 21_7020_5185_1423_0019;
blinded_hints[1].0.htlc_maximum_msat = 1844_6744_0737_0955_1615;

blinded_hints[2].1.introduction_node_id = nodes[6];

let bolt12_features: Bolt12InvoiceFeatures = channelmanager::provided_invoice_features(&config).to_context();
let payment_params = PaymentParameters::blinded(blinded_hints.clone())
.with_bolt12_features(bolt12_features.clone()).unwrap();

let netgraph = network_graph.read_only();
let route_params = RouteParameters::from_payment_params_and_value(
payment_params, amt_msat);
if let Err(LightningError { err, .. }) = get_route(
&our_id, &route_params, &netgraph, None, Arc::clone(&logger), &scorer, &(), &random_seed_bytes
) {
assert_eq!(err, "Failed to find a path to the given destination");
} else { panic!() }
}

#[test]
fn min_htlc_overpay_violates_max_htlc() {
// Test that if overpaying to meet a later hop's min_htlc and causes us to violate an earlier
// hop's max_htlc, we don't consider that candidate hop valid. Previously we would add this hop
// to `targets` and build an invalid path with it, and subsquently hit a debug panic asserting
// that the used liquidity for a hop was less than its available liquidity limit.
let secp_ctx = Secp256k1::new();
let logger = Arc::new(ln_test_utils::TestLogger::new());
let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, Arc::clone(&logger)));
let scorer = ln_test_utils::TestScorer::new();
let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
let random_seed_bytes = keys_manager.get_secure_random_bytes();
let config = UserConfig::default();

// Values are taken from the fuzz input that uncovered this panic.
let amt_msat = 7_4009_8048;
let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
let first_hop_outbound_capacity = 2_7345_2000;
Copy link
Contributor Author

@valentinewallace valentinewallace Sep 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheBlueMatt this was an existing issue before routing to blinded paths was supported. ISTM the router fuzzer should've caught this previously?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that's my impression too, not sure why I wasn't able to repro prior to the changes in 117.

let first_hops = vec![get_channel_details(
Some(200), nodes[0], channelmanager::provided_init_features(&config),
first_hop_outbound_capacity
)];

let base_fee = 1_6778_3453;
let htlc_min = 2_5165_8240;
let payment_params = {
let route_hint = RouteHint(vec![RouteHintHop {
src_node_id: nodes[0],
short_channel_id: 42,
fees: RoutingFees {
base_msat: base_fee,
proportional_millionths: 0,
},
cltv_expiry_delta: 10,
htlc_minimum_msat: Some(htlc_min),
htlc_maximum_msat: None,
}]);

PaymentParameters::from_node_id(nodes[1], 42)
.with_route_hints(vec![route_hint]).unwrap()
.with_bolt11_features(channelmanager::provided_invoice_features(&config)).unwrap()
};

let netgraph = network_graph.read_only();
let route_params = RouteParameters::from_payment_params_and_value(
payment_params, amt_msat);
if let Err(LightningError { err, .. }) = get_route(
&our_id, &route_params, &netgraph, Some(&first_hops.iter().collect::<Vec<_>>()),
Arc::clone(&logger), &scorer, &(), &random_seed_bytes
) {
assert_eq!(err, "Failed to find a path to the given destination");
} else { panic!() }
}

#[test]
fn previously_used_liquidity_violates_max_htlc() {
// Test that if a candidate first_hop<>route_hint_src_node channel does not have enough
// contribution amount to cover the next hop's min_htlc plus fees, we will not consider that
// candidate. In this case, the candidate does not have enough due to a previous path taking up
// some of its liquidity. Previously we would construct an invalid path and hit a debug panic
// asserting that the used liquidity for a hop was less than its available liquidity limit.
let secp_ctx = Secp256k1::new();
let logger = Arc::new(ln_test_utils::TestLogger::new());
let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, Arc::clone(&logger)));
let scorer = ln_test_utils::TestScorer::new();
let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
let random_seed_bytes = keys_manager.get_secure_random_bytes();
let config = UserConfig::default();

// Values are taken from the fuzz input that uncovered this panic.
let amt_msat = 52_4288;
let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
let first_hops = vec![get_channel_details(
Some(161), nodes[0], channelmanager::provided_init_features(&config), 486_4000
), get_channel_details(
Some(122), nodes[0], channelmanager::provided_init_features(&config), 179_5000
)];

let base_fees = [0, 425_9840, 0, 0];
let htlc_mins = [1_4392, 19_7401, 1027, 6_5535];
let payment_params = {
let mut route_hints = Vec::new();
for (idx, (base_fee, htlc_min)) in base_fees.iter().zip(htlc_mins.iter()).enumerate() {
route_hints.push(RouteHint(vec![RouteHintHop {
src_node_id: nodes[0],
short_channel_id: 42 + idx as u64,
fees: RoutingFees {
base_msat: *base_fee,
proportional_millionths: 0,
},
cltv_expiry_delta: 10,
htlc_minimum_msat: Some(*htlc_min),
htlc_maximum_msat: Some(htlc_min * 100),
}]));
}
PaymentParameters::from_node_id(nodes[1], 42)
.with_route_hints(route_hints).unwrap()
.with_bolt11_features(channelmanager::provided_invoice_features(&config)).unwrap()
};

let netgraph = network_graph.read_only();
let route_params = RouteParameters::from_payment_params_and_value(
payment_params, amt_msat);

let route = get_route(
&our_id, &route_params, &netgraph, Some(&first_hops.iter().collect::<Vec<_>>()),
Arc::clone(&logger), &scorer, &(), &random_seed_bytes
).unwrap();
assert_eq!(route.paths.len(), 1);
assert_eq!(route.get_total_amount(), amt_msat);
}
}

#[cfg(all(any(test, ldk_bench), not(feature = "no-std")))]
Expand Down