Move banned_nodes to params struct

Author: tnull

lightning/src/routing/router.rs

@@ -5721,21 +5721,23 @@ mod tests {
 		let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
-		let scorer_params = ProbabilisticScoringParameters::default();
-		let mut scorer = ProbabilisticScorer::new(scorer_params.clone(), Arc::clone(&network_graph), Arc::clone(&logger));
+		let payment_params = PaymentParameters::from_node_id(nodes[10]);
+		let mut scorer_params = ProbabilisticScoringParameters::default();
 
 		// First check we can get a route.
-		let payment_params = PaymentParameters::from_node_id(nodes[10]);
+		let scorer = ProbabilisticScorer::new(scorer_params.clone(), Arc::clone(&network_graph), Arc::clone(&logger));
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes);
 		assert!(route.is_ok());
 
 		// Then check that we can't get a route if we ban an intermediate node.
-		scorer.add_banned(&NodeId::from_pubkey(&nodes[3]));
+		scorer_params.add_banned(&NodeId::from_pubkey(&nodes[3]));
+		let scorer = ProbabilisticScorer::new(scorer_params.clone(), Arc::clone(&network_graph), Arc::clone(&logger));
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes);
 		assert!(route.is_err());
 
 		// Finally make sure we can route again, when we remove the ban.
-		scorer.remove_banned(&NodeId::from_pubkey(&nodes[3]));
+		scorer_params.remove_banned(&NodeId::from_pubkey(&nodes[3]));
+		let scorer = ProbabilisticScorer::new(scorer_params.clone(), Arc::clone(&network_graph), Arc::clone(&logger));
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes);
 		assert!(route.is_ok());
 	}

lightning/src/routing/scoring.rs

@@ -300,14 +300,13 @@ where L::Target: Logger {
 	logger: L,
 	// TODO: Remove entries of closed channels.
 	channel_liquidities: HashMap<u64, ChannelLiquidity<T>>,
-	banned_nodes: HashSet<NodeId>,
 }
 
 /// Parameters for configuring [`ProbabilisticScorer`].
 ///
 /// Used to configure base, liquidity, and amount penalties, the sum of which comprises the channel
 /// penalty (i.e., the amount in msats willing to be paid to avoid routing through the channel).
-#[derive(Clone, Copy)]
+#[derive(Clone)]
 pub struct ProbabilisticScoringParameters {
 	/// A fixed penalty in msats to apply to each channel.
 	///
@@ -362,6 +361,11 @@ pub struct ProbabilisticScoringParameters {
 	///
 	/// Default value: 256 msat
 	pub amount_penalty_multiplier_msat: u64,
+
+	/// A list of nodes that won't be considered during path finding.
+	///
+	/// (C-not exported)
+	pub banned_nodes: HashSet<NodeId>,
 }
 
 /// Accounting for channel liquidity balance uncertainty.
@@ -400,7 +404,6 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
 			network_graph,
 			logger,
 			channel_liquidities: HashMap::new(),
-			banned_nodes: HashSet::new(),
 		}
 	}
 
@@ -410,22 +413,6 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
 		self
 	}
 
-	/// Marks the node with the given `node_id` as banned, i.e.,
-	/// it will be avoided during path finding.
-	pub fn add_banned(&mut self, node_id: &NodeId) {
-		self.banned_nodes.insert(*node_id);
-	}
-
-	/// Removes the node with the given `node_id` from the list of nodes to avoid.
-	pub fn remove_banned(&mut self, node_id: &NodeId) {
-		self.banned_nodes.remove(node_id);
-	}
-
-	/// Clears the list of nodes that are avoided during path finding.
-	pub fn clear_banned(&mut self) {
-		self.banned_nodes = HashSet::new();
-	}
-
 	/// Dump the contents of this scorer into the configured logger.
 	///
 	/// Note that this writes roughly one line per channel for which we have a liquidity estimate,
@@ -462,8 +449,33 @@ impl ProbabilisticScoringParameters {
 			liquidity_penalty_multiplier_msat: 0,
 			liquidity_offset_half_life: Duration::from_secs(3600),
 			amount_penalty_multiplier_msat: 0,
+			banned_nodes: HashSet::new(),
+		}
+	}
+
+	/// Marks the node with the given `node_id` as banned, i.e.,
+	/// it will be avoided during path finding.
+	pub fn add_banned(&mut self, node_id: &NodeId) {
+		self.banned_nodes.insert(*node_id);
+	}
+
+	/// Marks all nodes in the given list as banned, i.e.,
+	/// they will be avoided during path finding.
+	pub fn add_banned_from_list(&mut self, node_ids: Vec<NodeId>) {
+		for id in node_ids {
+			self.banned_nodes.insert(id);
 		}
 	}
+
+	/// Removes the node with the given `node_id` from the list of nodes to avoid.
+	pub fn remove_banned(&mut self, node_id: &NodeId) {
+		self.banned_nodes.remove(node_id);
+	}
+
+	/// Clears the list of nodes that are avoided during path finding.
+	pub fn clear_banned(&mut self) {
+		self.banned_nodes = HashSet::new();
+	}
 }
 
 impl Default for ProbabilisticScoringParameters {
@@ -473,6 +485,7 @@ impl Default for ProbabilisticScoringParameters {
 			liquidity_penalty_multiplier_msat: 40_000,
 			liquidity_offset_half_life: Duration::from_secs(3600),
 			amount_penalty_multiplier_msat: 256,
+			banned_nodes: HashSet::new(),
 		}
 	}
 }
@@ -673,7 +686,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> Score for Probabilis
 	fn channel_penalty_msat(
 		&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage
 	) -> u64 {
-		if self.banned_nodes.contains(source) || self.banned_nodes.contains(target) {
+		if self.params.banned_nodes.contains(source) || self.params.banned_nodes.contains(target) {
 			return u64::max_value();
 		}
 
@@ -693,7 +706,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> Score for Probabilis
 			.get(&short_channel_id)
 			.unwrap_or(&ChannelLiquidity::new())
 			.as_directed(source, target, capacity_msat, liquidity_offset_half_life)
-			.penalty_msat(amount_msat, self.params)
+			.penalty_msat(amount_msat, self.params.clone())
 	}
 
 	fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
@@ -1099,7 +1112,6 @@ ReadableArgs<(ProbabilisticScoringParameters, G, L)> for ProbabilisticScorerUsin
 			network_graph,
 			logger,
 			channel_liquidities,
-			banned_nodes: HashSet::new(),
 		})
 	}
 }
@@ -1868,7 +1880,7 @@ mod tests {
 			liquidity_offset_half_life: Duration::from_secs(10),
 			..ProbabilisticScoringParameters::zero_penalty()
 		};
-		let mut scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
+		let mut scorer = ProbabilisticScorer::new(params.clone(), &network_graph, &logger);
 		let source = source_node_id();
 		let target = target_node_id();
 		let usage = ChannelUsage {
@@ -1904,7 +1916,7 @@ mod tests {
 			liquidity_offset_half_life: Duration::from_secs(10),
 			..ProbabilisticScoringParameters::zero_penalty()
 		};
-		let mut scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
+		let mut scorer = ProbabilisticScorer::new(params.clone(), &network_graph, &logger);
 		let source = source_node_id();
 		let target = target_node_id();
 		let usage = ChannelUsage {
@@ -2092,7 +2104,7 @@ mod tests {
 		let logger = TestLogger::new();
 		let network_graph = network_graph(&logger);
 		let params = ProbabilisticScoringParameters::default();
-		let scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
+		let scorer = ProbabilisticScorer::new(params.clone(), &network_graph, &logger);
 		let source = source_node_id();
 		let target = target_node_id();