Skip to content

Commit ccfc9b7

Browse files
jbesraajbesraa
committed
Add CandidateRouteHop to channel_penalty_msat inputs
1 parent 362b79b commit ccfc9b7

File tree

4 files changed

+503
-192
lines changed

4 files changed

+503
-192
lines changed

‎lightning-background-processor/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,7 @@ mod tests {
863863
use lightning::ln::msgs::{ChannelMessageHandler, Init};
864864
use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler};
865865
use lightning::routing::gossip::{NetworkGraph, NodeId, P2PGossipSync};
866-
use lightning::routing::router::{DefaultRouter, Path, RouteHop};
866+
use lightning::routing::router::{DefaultRouter, Path, RouteHop, CandidateRouteHop};
867867
use lightning::routing::scoring::{ChannelUsage, ScoreUpdate, ScoreLookUp};
868868
use lightning::util::config::UserConfig;
869869
use lightning::util::ser::Writeable;
@@ -1066,7 +1066,7 @@ mod tests {
10661066
impl ScoreLookUp for TestScorer {
10671067
type ScoreParams = ();
10681068
fn channel_penalty_msat(
1069-
&self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage, _score_params: &Self::ScoreParams
1069+
&self, _candidate: &CandidateRouteHop, _usage: ChannelUsage, _score_params: &Self::ScoreParams
10701070
) -> u64 { unimplemented!(); }
10711071
}
10721072

‎lightning/src/routing/router.rs

Lines changed: 60 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,27 @@ impl<'a, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>, S: Deref<Target = Sc>> Wr
134134

135135
impl<'a, SP: Sized, Sc: 'a + ScoreLookUp<ScoreParams = SP>, S: Deref<Target = Sc>> ScoreLookUp for ScorerAccountingForInFlightHtlcs<'a, SP, Sc, S> {
136136
type ScoreParams = Sc::ScoreParams;
137-
fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 {
137+
fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 {
138+
let target = match candidate.target() {
139+
Some(target) => target,
140+
None => return self.scorer.channel_penalty_msat(candidate, usage, score_params),
141+
};
142+
let short_channel_id = match candidate.short_channel_id() {
143+
Some(short_channel_id) => short_channel_id,
144+
None => return self.scorer.channel_penalty_msat(candidate, usage, score_params),
145+
};
146+
let source = candidate.source();
138147
if let Some(used_liquidity) = self.inflight_htlcs.used_liquidity_msat(
139-
source, target, short_channel_id
148+
&source, &target, short_channel_id
140149
) {
141150
let usage = ChannelUsage {
142151
inflight_htlc_msat: usage.inflight_htlc_msat.saturating_add(used_liquidity),
143152
..usage
144153
};
145154

146-
self.scorer.channel_penalty_msat(short_channel_id, source, target, usage, score_params)
155+
self.scorer.channel_penalty_msat(candidate, usage, score_params)
147156
} else {
148-
self.scorer.channel_penalty_msat(short_channel_id, source, target, usage, score_params)
157+
self.scorer.channel_penalty_msat(candidate, usage, score_params)
149158
}
150159
}
151160
}
@@ -1961,9 +1970,10 @@ where L::Target: Logger {
19611970
inflight_htlc_msat: used_liquidity_msat,
19621971
effective_capacity,
19631972
};
1964-
let channel_penalty_msat = scid_opt.map_or(0,
1965-
|scid| scorer.channel_penalty_msat(scid, &$src_node_id, &$dest_node_id,
1966-
channel_usage, score_params));
1973+
let channel_penalty_msat =
1974+
scorer.channel_penalty_msat($candidate,
1975+
channel_usage,
1976+
score_params);
19671977
let path_penalty_msat = $next_hops_path_penalty_msat
19681978
.saturating_add(channel_penalty_msat);
19691979
let new_graph_node = RouteGraphNode {
@@ -2084,7 +2094,7 @@ where L::Target: Logger {
20842094
if let Some(first_channels) = first_hop_targets.get(&$node_id) {
20852095
for details in first_channels {
20862096
let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id };
2087-
add_entry!(candidate, our_node_id, $node_id, $fee_to_target_msat,
2097+
add_entry!(&candidate, our_node_id, $node_id, $fee_to_target_msat,
20882098
$next_hops_value_contribution,
20892099
$next_hops_path_htlc_minimum_msat, $next_hops_path_penalty_msat,
20902100
$next_hops_cltv_delta, $next_hops_path_length);
@@ -2110,7 +2120,7 @@ where L::Target: Logger {
21102120
source_node_id: *source,
21112121
target_node_id: $node_id,
21122122
};
2113-
add_entry!(candidate, *source, $node_id,
2123+
add_entry!(&candidate, *source, $node_id,
21142124
$fee_to_target_msat,
21152125
$next_hops_value_contribution,
21162126
$next_hops_path_htlc_minimum_msat,
@@ -2141,7 +2151,7 @@ where L::Target: Logger {
21412151
payee_node_id_opt.map(|payee| first_hop_targets.get(&payee).map(|first_channels| {
21422152
for details in first_channels {
21432153
let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id };
2144-
let added = add_entry!(candidate, our_node_id, payee, 0, path_value_msat,
2154+
let added = add_entry!(&candidate, our_node_id, payee, 0, path_value_msat,
21452155
0, 0u64, 0, 0).is_some();
21462156
log_trace!(logger, "{} direct route to payee via {}",
21472157
if added { "Added" } else { "Skipped" }, LoggedCandidateHop(&candidate));
@@ -2178,7 +2188,7 @@ where L::Target: Logger {
21782188
CandidateRouteHop::OneHopBlinded { hint, hint_idx }
21792189
} else { CandidateRouteHop::Blinded { hint, hint_idx } };
21802190
let mut path_contribution_msat = path_value_msat;
2181-
if let Some(hop_used_msat) = add_entry!(candidate, intro_node_id, maybe_dummy_payee_node_id,
2191+
if let Some(hop_used_msat) = add_entry!(&candidate, intro_node_id, maybe_dummy_payee_node_id,
21822192
0, path_contribution_msat, 0, 0_u64, 0, 0)
21832193
{
21842194
path_contribution_msat = hop_used_msat;
@@ -2194,7 +2204,7 @@ where L::Target: Logger {
21942204
};
21952205
let path_min = candidate.htlc_minimum_msat().saturating_add(
21962206
compute_fees_saturating(candidate.htlc_minimum_msat(), candidate.fees()));
2197-
add_entry!(first_hop_candidate, our_node_id, intro_node_id, blinded_path_fee,
2207+
add_entry!(&first_hop_candidate, our_node_id, intro_node_id, blinded_path_fee,
21982208
path_contribution_msat, path_min, 0_u64, candidate.cltv_expiry_delta(),
21992209
candidate.blinded_path().map_or(1, |bp| bp.blinded_hops.len() as u8));
22002210
}
@@ -2248,7 +2258,7 @@ where L::Target: Logger {
22482258
})
22492259
.unwrap_or_else(|| CandidateRouteHop::PrivateHop { hint: hop, target_node_id: target });
22502260

2251-
if let Some(hop_used_msat) = add_entry!(candidate, source, target,
2261+
if let Some(hop_used_msat) = add_entry!(&candidate, source, target,
22522262
aggregate_next_hops_fee_msat, aggregate_path_contribution_msat,
22532263
aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat,
22542264
aggregate_next_hops_cltv_delta, aggregate_next_hops_path_length)
@@ -2270,7 +2280,7 @@ where L::Target: Logger {
22702280
effective_capacity: candidate.effective_capacity(),
22712281
};
22722282
let channel_penalty_msat = scorer.channel_penalty_msat(
2273-
hop.short_channel_id, &source, &target, channel_usage, score_params
2283+
&candidate, channel_usage, score_params
22742284
);
22752285
aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat
22762286
.saturating_add(channel_penalty_msat);
@@ -2287,7 +2297,7 @@ where L::Target: Logger {
22872297
recommended_value_msat, our_node_pubkey);
22882298
for details in first_channels {
22892299
let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id};
2290-
add_entry!(first_hop_candidate, our_node_id, NodeId::from_pubkey(&prev_hop_id),
2300+
add_entry!(&first_hop_candidate, our_node_id, NodeId::from_pubkey(&prev_hop_id),
22912301
aggregate_next_hops_fee_msat, aggregate_path_contribution_msat,
22922302
aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat,
22932303
aggregate_next_hops_cltv_delta, aggregate_next_hops_path_length);
@@ -2332,7 +2342,7 @@ where L::Target: Logger {
23322342
recommended_value_msat, our_node_pubkey);
23332343
for details in first_channels {
23342344
let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id};
2335-
add_entry!(first_hop_candidate, our_node_id,
2345+
add_entry!(&first_hop_candidate, our_node_id,
23362346
NodeId::from_pubkey(&hop.src_node_id),
23372347
aggregate_next_hops_fee_msat,
23382348
aggregate_path_contribution_msat,
@@ -2829,13 +2839,18 @@ fn build_route_from_hops_internal<L: Deref>(
28292839

28302840
impl ScoreLookUp for HopScorer {
28312841
type ScoreParams = ();
2832-
fn channel_penalty_msat(&self, _short_channel_id: u64, source: &NodeId, target: &NodeId,
2842+
fn channel_penalty_msat(&self, candidate: &CandidateRouteHop,
28332843
_usage: ChannelUsage, _score_params: &Self::ScoreParams) -> u64
28342844
{
2845+
let target = match candidate.target() {
2846+
Some(target) => target,
2847+
None => return 0,
2848+
};
2849+
let source = candidate.source();
28352850
let mut cur_id = self.our_node_id;
28362851
for i in 0..self.hop_ids.len() {
28372852
if let Some(next_id) = self.hop_ids[i] {
2838-
if cur_id == *source && next_id == *target {
2853+
if cur_id == source && next_id == target {
28392854
return 0;
28402855
}
28412856
cur_id = next_id;
@@ -2911,6 +2926,8 @@ mod tests {
29112926

29122927
use core::convert::TryInto;
29132928

2929+
use super::CandidateRouteHop;
2930+
29142931
fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
29152932
features: InitFeatures, outbound_capacity_msat: u64) -> channelmanager::ChannelDetails {
29162933
channelmanager::ChannelDetails {
@@ -6164,7 +6181,11 @@ mod tests {
61646181
}
61656182
impl ScoreLookUp for BadChannelScorer {
61666183
type ScoreParams = ();
6167-
fn channel_penalty_msat(&self, short_channel_id: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
6184+
fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
6185+
let short_channel_id = match candidate.short_channel_id() {
6186+
Some(id) => id,
6187+
None => return 0,
6188+
};
61686189
if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
61696190
}
61706191
}
@@ -6180,8 +6201,12 @@ mod tests {
61806201

61816202
impl ScoreLookUp for BadNodeScorer {
61826203
type ScoreParams = ();
6183-
fn channel_penalty_msat(&self, _: u64, _: &NodeId, target: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
6184-
if *target == self.node_id { u64::max_value() } else { 0 }
6204+
fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
6205+
let target = match candidate.target() {
6206+
Some(target) => target,
6207+
None => return 0,
6208+
};
6209+
if target == self.node_id { u64::max_value() } else { 0 }
61856210
}
61866211
}
61876212

@@ -6667,26 +6692,34 @@ mod tests {
66676692
};
66686693
scorer_params.set_manual_penalty(&NodeId::from_pubkey(&nodes[3]), 123);
66696694
scorer_params.set_manual_penalty(&NodeId::from_pubkey(&nodes[4]), 456);
6670-
assert_eq!(scorer.channel_penalty_msat(42, &NodeId::from_pubkey(&nodes[3]), &NodeId::from_pubkey(&nodes[4]), usage, &scorer_params), 456);
6695+
let network_graph = network_graph.read_only();
6696+
let channels = network_graph.channels();
6697+
let channel = channels.get(&5).unwrap();
6698+
let info = channel.as_directed_from(&NodeId::from_pubkey(&nodes[3])).unwrap();
6699+
let candidate: CandidateRouteHop = CandidateRouteHop::PublicHop {
6700+
info: info.0,
6701+
short_channel_id: 5,
6702+
source_node_id: NodeId::from_pubkey(&nodes[3]),
6703+
target_node_id: NodeId::from_pubkey(&nodes[4]),
6704+
};
6705+
assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &scorer_params), 456);
66716706

66726707
// Then check we can get a normal route
66736708
let payment_params = PaymentParameters::from_node_id(nodes[10], 42);
66746709
let route_params = RouteParameters::from_payment_params_and_value(
66756710
payment_params, 100);
6676-
let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
6711+
let route = get_route(&our_id, &route_params, &network_graph, None,
66776712
Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes);
66786713
assert!(route.is_ok());
66796714

66806715
// Then check that we can't get a route if we ban an intermediate node.
66816716
scorer_params.add_banned(&NodeId::from_pubkey(&nodes[3]));
6682-
let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
6683-
Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes);
6717+
let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
66846718
assert!(route.is_err());
66856719

66866720
// Finally make sure we can route again, when we remove the ban.
66876721
scorer_params.remove_banned(&NodeId::from_pubkey(&nodes[3]));
6688-
let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
6689-
Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes);
6722+
let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
66906723
assert!(route.is_ok());
66916724
}
66926725

0 commit comments

Comments
 (0)