Skip to content

Commit 208e443

Browse files
jbesraajbesraa
committed
Add CandidateRouteHop to channel_penalty_msat inputs
1 parent af6fbaa commit 208e443

File tree

4 files changed

+425
-186
lines changed

4 files changed

+425
-186
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,7 @@ mod tests {
861861
use lightning::ln::msgs::{ChannelMessageHandler, Init};
862862
use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler};
863863
use lightning::routing::gossip::{NetworkGraph, NodeId, P2PGossipSync};
864-
use lightning::routing::router::{DefaultRouter, Path, RouteHop};
864+
use lightning::routing::router::{DefaultRouter, Path, RouteHop, CandidateRouteHop};
865865
use lightning::routing::scoring::{ChannelUsage, ScoreUpdate, ScoreLookUp};
866866
use lightning::util::config::UserConfig;
867867
use lightning::util::ser::Writeable;
@@ -1040,7 +1040,7 @@ mod tests {
10401040
impl ScoreLookUp for TestScorer {
10411041
type ScoreParams = ();
10421042
fn channel_penalty_msat(
1043-
&self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage, _score_params: &Self::ScoreParams
1043+
&self, _candidate: &CandidateRouteHop, _usage: ChannelUsage, _score_params: &Self::ScoreParams
10441044
) -> u64 { unimplemented!(); }
10451045
}
10461046

‎lightning/src/routing/router.rs‎

Lines changed: 55 additions & 23 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 0,
141+
};
142+
let short_channel_id = match candidate.short_channel_id() {
143+
Some(short_channel_id) => short_channel_id,
144+
None => return 0,
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 + 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
}
@@ -1837,7 +1846,7 @@ where L::Target: Logger {
18371846
effective_capacity,
18381847
};
18391848
let channel_penalty_msat = scid_opt.map_or(0,
1840-
|scid| scorer.channel_penalty_msat(scid, &$src_node_id, &$dest_node_id,
1849+
|scid| scorer.channel_penalty_msat($candidate,
18411850
channel_usage, score_params));
18421851
let path_penalty_msat = $next_hops_path_penalty_msat
18431852
.saturating_add(channel_penalty_msat);
@@ -1951,7 +1960,7 @@ where L::Target: Logger {
19511960
if let Some(first_channels) = first_hop_targets.get(&$node_id) {
19521961
for details in first_channels {
19531962
let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id };
1954-
add_entry!(candidate, our_node_id, $node_id, $fee_to_target_msat,
1963+
add_entry!(&candidate, our_node_id, $node_id, $fee_to_target_msat,
19551964
$next_hops_value_contribution,
19561965
$next_hops_path_htlc_minimum_msat, $next_hops_path_penalty_msat,
19571966
$next_hops_cltv_delta, $next_hops_path_length);
@@ -1975,7 +1984,7 @@ where L::Target: Logger {
19751984
info: directed_channel,
19761985
short_channel_id: *chan_id,
19771986
};
1978-
add_entry!(candidate, *source, $node_id,
1987+
add_entry!(&candidate, *source, $node_id,
19791988
$fee_to_target_msat,
19801989
$next_hops_value_contribution,
19811990
$next_hops_path_htlc_minimum_msat,
@@ -2006,7 +2015,7 @@ where L::Target: Logger {
20062015
payee_node_id_opt.map(|payee| first_hop_targets.get(&payee).map(|first_channels| {
20072016
for details in first_channels {
20082017
let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id };
2009-
let added = add_entry!(candidate, our_node_id, payee, 0, path_value_msat,
2018+
let added = add_entry!(&candidate, our_node_id, payee, 0, path_value_msat,
20102019
0, 0u64, 0, 0).is_some();
20112020
log_trace!(logger, "{} direct route to payee via {}",
20122021
if added { "Added" } else { "Skipped" }, LoggedCandidateHop(&candidate));
@@ -2043,7 +2052,7 @@ where L::Target: Logger {
20432052
CandidateRouteHop::OneHopBlinded { hint, hint_idx }
20442053
} else { CandidateRouteHop::Blinded { hint, hint_idx } };
20452054
let mut path_contribution_msat = path_value_msat;
2046-
if let Some(hop_used_msat) = add_entry!(candidate, intro_node_id, maybe_dummy_payee_node_id,
2055+
if let Some(hop_used_msat) = add_entry!(&candidate, intro_node_id, maybe_dummy_payee_node_id,
20472056
0, path_contribution_msat, 0, 0_u64, 0, 0)
20482057
{
20492058
path_contribution_msat = hop_used_msat;
@@ -2057,7 +2066,7 @@ where L::Target: Logger {
20572066
Some(fee) => fee,
20582067
None => continue
20592068
};
2060-
add_entry!(first_hop_candidate, our_node_id, intro_node_id, blinded_path_fee,
2069+
add_entry!(&first_hop_candidate, our_node_id, intro_node_id, blinded_path_fee,
20612070
path_contribution_msat, candidate.htlc_minimum_msat(), 0_u64,
20622071
candidate.cltv_expiry_delta(),
20632072
candidate.blinded_path().map_or(1, |bp| bp.blinded_hops.len() as u8));
@@ -2100,7 +2109,7 @@ where L::Target: Logger {
21002109
})
21012110
.unwrap_or_else(|| CandidateRouteHop::PrivateHop { hint: hop, target_node_id: target });
21022111

2103-
if let Some(hop_used_msat) = add_entry!(candidate, source, target,
2112+
if let Some(hop_used_msat) = add_entry!(&candidate, source, target,
21042113
aggregate_next_hops_fee_msat, aggregate_path_contribution_msat,
21052114
aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat,
21062115
aggregate_next_hops_cltv_delta, aggregate_next_hops_path_length)
@@ -2122,7 +2131,7 @@ where L::Target: Logger {
21222131
effective_capacity: candidate.effective_capacity(),
21232132
};
21242133
let channel_penalty_msat = scorer.channel_penalty_msat(
2125-
hop.short_channel_id, &source, &target, channel_usage, score_params
2134+
&candidate, channel_usage, score_params
21262135
);
21272136
aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat
21282137
.saturating_add(channel_penalty_msat);
@@ -2139,7 +2148,7 @@ where L::Target: Logger {
21392148
recommended_value_msat, our_node_pubkey);
21402149
for details in first_channels {
21412150
let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id};
2142-
add_entry!(first_hop_candidate, our_node_id, NodeId::from_pubkey(&prev_hop_id),
2151+
add_entry!(&first_hop_candidate, our_node_id, NodeId::from_pubkey(&prev_hop_id),
21432152
aggregate_next_hops_fee_msat, aggregate_path_contribution_msat,
21442153
aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat,
21452154
aggregate_next_hops_cltv_delta, aggregate_next_hops_path_length);
@@ -2180,7 +2189,7 @@ where L::Target: Logger {
21802189
recommended_value_msat, our_node_pubkey);
21812190
for details in first_channels {
21822191
let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id};
2183-
add_entry!(first_hop_candidate, our_node_id,
2192+
add_entry!(&first_hop_candidate, our_node_id,
21842193
NodeId::from_pubkey(&hop.src_node_id),
21852194
aggregate_next_hops_fee_msat,
21862195
aggregate_path_contribution_msat,
@@ -2638,13 +2647,18 @@ fn build_route_from_hops_internal<L: Deref>(
26382647

26392648
impl ScoreLookUp for HopScorer {
26402649
type ScoreParams = ();
2641-
fn channel_penalty_msat(&self, _short_channel_id: u64, source: &NodeId, target: &NodeId,
2650+
fn channel_penalty_msat(&self, candidate: &CandidateRouteHop,
26422651
_usage: ChannelUsage, _score_params: &Self::ScoreParams) -> u64
26432652
{
2653+
let target = match candidate.target() {
2654+
Some(target) => target,
2655+
None => return 0,
2656+
};
2657+
let source = candidate.source();
26442658
let mut cur_id = self.our_node_id;
26452659
for i in 0..self.hop_ids.len() {
26462660
if let Some(next_id) = self.hop_ids[i] {
2647-
if cur_id == *source && next_id == *target {
2661+
if cur_id == source && next_id == target {
26482662
return 0;
26492663
}
26502664
cur_id = next_id;
@@ -2721,6 +2735,8 @@ mod tests {
27212735

27222736
use core::convert::TryInto;
27232737

2738+
use super::CandidateRouteHop;
2739+
27242740
fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
27252741
features: InitFeatures, outbound_capacity_msat: u64) -> channelmanager::ChannelDetails {
27262742
channelmanager::ChannelDetails {
@@ -5737,7 +5753,11 @@ mod tests {
57375753
}
57385754
impl ScoreLookUp for BadChannelScorer {
57395755
type ScoreParams = ();
5740-
fn channel_penalty_msat(&self, short_channel_id: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
5756+
fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
5757+
let short_channel_id = match candidate.short_channel_id() {
5758+
Some(id) => id,
5759+
None => return 0,
5760+
};
57415761
if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
57425762
}
57435763
}
@@ -5753,8 +5773,12 @@ mod tests {
57535773

57545774
impl ScoreLookUp for BadNodeScorer {
57555775
type ScoreParams = ();
5756-
fn channel_penalty_msat(&self, _: u64, _: &NodeId, target: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
5757-
if *target == self.node_id { u64::max_value() } else { 0 }
5776+
fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
5777+
let target = match candidate.target() {
5778+
Some(target) => target,
5779+
None => return 0,
5780+
};
5781+
if target == self.node_id { u64::max_value() } else { 0 }
57585782
}
57595783
}
57605784

@@ -6218,21 +6242,29 @@ mod tests {
62186242
};
62196243
scorer_params.set_manual_penalty(&NodeId::from_pubkey(&nodes[3]), 123);
62206244
scorer_params.set_manual_penalty(&NodeId::from_pubkey(&nodes[4]), 456);
6221-
assert_eq!(scorer.channel_penalty_msat(42, &NodeId::from_pubkey(&nodes[3]), &NodeId::from_pubkey(&nodes[4]), usage, &scorer_params), 456);
6245+
let network_graph = network_graph.read_only();
6246+
let channels = network_graph.channels();
6247+
let channel = channels.get(&5).unwrap();
6248+
let info = channel.as_directed_from(&NodeId::from_pubkey(&nodes[3])).unwrap();
6249+
let candidate: CandidateRouteHop = CandidateRouteHop::PublicHop {
6250+
info: info.0,
6251+
short_channel_id: 5,
6252+
};
6253+
assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &scorer_params), 456);
62226254

62236255
// Then check we can get a normal route
62246256
let payment_params = PaymentParameters::from_node_id(nodes[10], 42);
6225-
let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
6257+
let route = get_route(&our_id, &payment_params, &network_graph, None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
62266258
assert!(route.is_ok());
62276259

62286260
// Then check that we can't get a route if we ban an intermediate node.
62296261
scorer_params.add_banned(&NodeId::from_pubkey(&nodes[3]));
6230-
let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
6262+
let route = get_route(&our_id, &payment_params, &network_graph, None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
62316263
assert!(route.is_err());
62326264

62336265
// Finally make sure we can route again, when we remove the ban.
62346266
scorer_params.remove_banned(&NodeId::from_pubkey(&nodes[3]));
6235-
let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
6267+
let route = get_route(&our_id, &payment_params, &network_graph, None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
62366268
assert!(route.is_ok());
62376269
}
62386270

0 commit comments

Comments
 (0)