Add CandidateRouteHop to channel_penalty_msat inputs

Author: jbesraa

lightning-background-processor/src/lib.rs

@@ -863,7 +863,7 @@ mod tests {
 	use lightning::ln::msgs::{ChannelMessageHandler, Init};
 	use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler};
 	use lightning::routing::gossip::{NetworkGraph, NodeId, P2PGossipSync};
-	use lightning::routing::router::{DefaultRouter, Path, RouteHop};
+	use lightning::routing::router::{DefaultRouter, Path, RouteHop, CandidateRouteHop};
 	use lightning::routing::scoring::{ChannelUsage, ScoreUpdate, ScoreLookUp};
 	use lightning::util::config::UserConfig;
 	use lightning::util::ser::Writeable;
@@ -1063,7 +1063,7 @@ mod tests {
 	impl ScoreLookUp for TestScorer {
 		type ScoreParams = ();
 		fn channel_penalty_msat(
-			&self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage, _score_params: &Self::ScoreParams
+			&self, _candidate: &CandidateRouteHop, _usage: ChannelUsage, _score_params: &Self::ScoreParams
 		) -> u64 { unimplemented!(); }
 	}
 

lightning/src/routing/router.rs

@@ -134,18 +134,27 @@ impl<'a, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>, S: Deref<Target = Sc>> Wr
 
 impl<'a, SP: Sized, Sc: 'a + ScoreLookUp<ScoreParams = SP>, S: Deref<Target = Sc>> ScoreLookUp for ScorerAccountingForInFlightHtlcs<'a, SP, Sc, S> {
 	type ScoreParams = Sc::ScoreParams;
-	fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 {
+	fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 {
+		let target = match candidate.target() {
+			Some(target) => target,
+			None => return 0,
+		};
+		let short_channel_id = match candidate.short_channel_id() {
+			Some(short_channel_id) => short_channel_id,
+			None => return 0,
+		};
+		let source = candidate.source();
 		if let Some(used_liquidity) = self.inflight_htlcs.used_liquidity_msat(
-			source, target, short_channel_id
+			&source, &target, short_channel_id
 		) {
 			let usage = ChannelUsage {
 				inflight_htlc_msat: usage.inflight_htlc_msat + used_liquidity,
 				..usage
 			};
 
-			self.scorer.channel_penalty_msat(short_channel_id, source, target, usage, score_params)
+			self.scorer.channel_penalty_msat(candidate, usage, score_params)
 		} else {
-			self.scorer.channel_penalty_msat(short_channel_id, source, target, usage, score_params)
+			self.scorer.channel_penalty_msat(candidate, usage, score_params)
 		}
 	}
 }
@@ -1886,7 +1895,7 @@ where L::Target: Logger {
 								effective_capacity,
 							};
 							let channel_penalty_msat = scid_opt.map_or(0,
-								|scid| scorer.channel_penalty_msat(scid, &$src_node_id, &$dest_node_id,
+								|scid| scorer.channel_penalty_msat($candidate,
 									channel_usage, score_params));
 							let path_penalty_msat = $next_hops_path_penalty_msat
 								.saturating_add(channel_penalty_msat);
@@ -2000,7 +2009,7 @@ where L::Target: Logger {
 				if let Some(first_channels) = first_hop_targets.get(&$node_id) {
 					for details in first_channels {
 						let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id };
-						add_entry!(candidate, our_node_id, $node_id, $fee_to_target_msat,
+						add_entry!(&candidate, our_node_id, $node_id, $fee_to_target_msat,
 							$next_hops_value_contribution,
 							$next_hops_path_htlc_minimum_msat, $next_hops_path_penalty_msat,
 							$next_hops_cltv_delta, $next_hops_path_length);
@@ -2024,7 +2033,7 @@ where L::Target: Logger {
 											info: directed_channel,
 											short_channel_id: *chan_id,
 										};
-										add_entry!(candidate, *source, $node_id,
+										add_entry!(&candidate, *source, $node_id,
 											$fee_to_target_msat,
 											$next_hops_value_contribution,
 											$next_hops_path_htlc_minimum_msat,
@@ -2055,7 +2064,7 @@ where L::Target: Logger {
 		payee_node_id_opt.map(|payee| first_hop_targets.get(&payee).map(|first_channels| {
 			for details in first_channels {
 				let candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id };
-				let added = add_entry!(candidate, our_node_id, payee, 0, path_value_msat,
+				let added = add_entry!(&candidate, our_node_id, payee, 0, path_value_msat,
 									0, 0u64, 0, 0).is_some();
 				log_trace!(logger, "{} direct route to payee via {}",
 						if added { "Added" } else { "Skipped" }, LoggedCandidateHop(&candidate));
@@ -2092,7 +2101,7 @@ where L::Target: Logger {
 				CandidateRouteHop::OneHopBlinded { hint, hint_idx }
 			} else { CandidateRouteHop::Blinded { hint, hint_idx } };
 			let mut path_contribution_msat = path_value_msat;
-			if let Some(hop_used_msat) = add_entry!(candidate, intro_node_id, maybe_dummy_payee_node_id,
+			if let Some(hop_used_msat) = add_entry!(&candidate, intro_node_id, maybe_dummy_payee_node_id,
 				0, path_contribution_msat, 0, 0_u64, 0, 0)
 			{
 				path_contribution_msat = hop_used_msat;
@@ -2106,7 +2115,7 @@ where L::Target: Logger {
 						Some(fee) => fee,
 						None => continue
 					};
-					add_entry!(first_hop_candidate, our_node_id, intro_node_id, blinded_path_fee,
+					add_entry!(&first_hop_candidate, our_node_id, intro_node_id, blinded_path_fee,
 						path_contribution_msat, candidate.htlc_minimum_msat(), 0_u64,
 						candidate.cltv_expiry_delta(),
 						candidate.blinded_path().map_or(1, |bp| bp.blinded_hops.len() as u8));
@@ -2149,7 +2158,7 @@ where L::Target: Logger {
 						})
 						.unwrap_or_else(|| CandidateRouteHop::PrivateHop { hint: hop, target_node_id: target });
 
-					if let Some(hop_used_msat) = add_entry!(candidate, source, target,
+					if let Some(hop_used_msat) = add_entry!(&candidate, source, target,
 						aggregate_next_hops_fee_msat, aggregate_path_contribution_msat,
 						aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat,
 						aggregate_next_hops_cltv_delta, aggregate_next_hops_path_length)
@@ -2171,7 +2180,7 @@ where L::Target: Logger {
 						effective_capacity: candidate.effective_capacity(),
 					};
 					let channel_penalty_msat = scorer.channel_penalty_msat(
-						hop.short_channel_id, &source, &target, channel_usage, score_params
+						&candidate, channel_usage, score_params
 					);
 					aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat
 						.saturating_add(channel_penalty_msat);
@@ -2188,7 +2197,7 @@ where L::Target: Logger {
 							recommended_value_msat, our_node_pubkey);
 						for details in first_channels {
 							let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id};
-							add_entry!(first_hop_candidate, our_node_id, NodeId::from_pubkey(&prev_hop_id),
+							add_entry!(&first_hop_candidate, our_node_id, NodeId::from_pubkey(&prev_hop_id),
 								aggregate_next_hops_fee_msat, aggregate_path_contribution_msat,
 								aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat,
 								aggregate_next_hops_cltv_delta, aggregate_next_hops_path_length);
@@ -2229,7 +2238,7 @@ where L::Target: Logger {
 								recommended_value_msat, our_node_pubkey);
 							for details in first_channels {
 								let first_hop_candidate = CandidateRouteHop::FirstHop { details, node_id: our_node_id};
-								add_entry!(first_hop_candidate, our_node_id,
+								add_entry!(&first_hop_candidate, our_node_id,
 									NodeId::from_pubkey(&hop.src_node_id),
 									aggregate_next_hops_fee_msat,
 									aggregate_path_contribution_msat,
@@ -2685,13 +2694,18 @@ fn build_route_from_hops_internal<L: Deref>(
 
 	impl ScoreLookUp for HopScorer {
 		type ScoreParams = ();
-		fn channel_penalty_msat(&self, _short_channel_id: u64, source: &NodeId, target: &NodeId,
+		fn channel_penalty_msat(&self, candidate: &CandidateRouteHop,
 			_usage: ChannelUsage, _score_params: &Self::ScoreParams) -> u64
 		{
+			let target = match candidate.target() {
+				Some(target) => target,
+				None => return 0,
+			};
+			let source = candidate.source();
 			let mut cur_id = self.our_node_id;
 			for i in 0..self.hop_ids.len() {
 				if let Some(next_id) = self.hop_ids[i] {
-					if cur_id == *source && next_id == *target {
+					if cur_id == source && next_id == target {
 						return 0;
 					}
 					cur_id = next_id;
@@ -2767,6 +2781,8 @@ mod tests {
 
 	use core::convert::TryInto;
 
+	use super::CandidateRouteHop;
+
 	fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
 			features: InitFeatures, outbound_capacity_msat: u64) -> channelmanager::ChannelDetails {
 		channelmanager::ChannelDetails {
@@ -2862,7 +2878,7 @@ mod tests {
 			Arc::clone(&logger), &scorer, &(), &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, &(), &random_seed_bytes).unwrap();
 		assert_eq!(route.paths[0].hops.len(), 2);
@@ -5943,7 +5959,11 @@ mod tests {
 	}
 	impl ScoreLookUp for BadChannelScorer {
 		type ScoreParams = ();
-		fn channel_penalty_msat(&self, short_channel_id: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
+		fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
+			let short_channel_id = match candidate.short_channel_id() {
+				Some(id) => id,
+				None => return 0,
+			};
 			if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
 		}
 	}
@@ -5959,8 +5979,12 @@ mod tests {
 
 	impl ScoreLookUp for BadNodeScorer {
 		type ScoreParams = ();
-		fn channel_penalty_msat(&self, _: u64, _: &NodeId, target: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
-			if *target == self.node_id { u64::max_value() } else { 0 }
+		fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
+			let target = match candidate.target() {
+				Some(target) => target,
+				None => return 0,
+			};
+			if target == self.node_id { u64::max_value() } else { 0 }
 		}
 	}
 
@@ -6446,26 +6470,33 @@ mod tests {
 		};
 		scorer_params.set_manual_penalty(&NodeId::from_pubkey(&nodes[3]), 123);
 		scorer_params.set_manual_penalty(&NodeId::from_pubkey(&nodes[4]), 456);
-		assert_eq!(scorer.channel_penalty_msat(42, &NodeId::from_pubkey(&nodes[3]), &NodeId::from_pubkey(&nodes[4]), usage, &scorer_params), 456);
+		let network_graph = network_graph.read_only();
+		let channels = network_graph.channels();
+		let channel = channels.get(&5).unwrap();
+		let info = channel.as_directed_from(&NodeId::from_pubkey(&nodes[3])).unwrap();
+		let candidate: CandidateRouteHop = CandidateRouteHop::PublicHop {
+			info: info.0,
+			short_channel_id: 5,
+		};
+		assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &scorer_params), 456);
 
 		// Then check we can get a normal route
 		let payment_params = PaymentParameters::from_node_id(nodes[10], 42);
 		let route_params = RouteParameters::from_payment_params_and_value(
 			payment_params, 100);
-		let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-			Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes);
+		// let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
+			// Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes);
+		let route = get_route(&our_id, &payment_params, &network_graph, None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
 		assert!(route.is_ok());
 
 		// Then check that we can't get a route if we ban an intermediate node.
 		scorer_params.add_banned(&NodeId::from_pubkey(&nodes[3]));
-		let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-			Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes);
+		let route = get_route(&our_id, &payment_params, &network_graph, None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
 		assert!(route.is_err());
 
 		// Finally make sure we can route again, when we remove the ban.
 		scorer_params.remove_banned(&NodeId::from_pubkey(&nodes[3]));
-		let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-			Arc::clone(&logger), &scorer, &scorer_params, &random_seed_bytes);
+		let route = get_route(&our_id, &payment_params, &network_graph, None, 100, Arc::clone(&logger), &scorer, &scorer_params,&random_seed_bytes);
 		assert!(route.is_ok());
 	}