Fix scoring methods to score Paths instead of Vec<RouteHop>s

Author: valentinewallace

lightning-background-processor/src/lib.rs

@@ -236,25 +236,20 @@ fn update_scorer<'a, S: 'static + Deref<Target = SC> + Send + Sync, SC: 'a + Wri
 	let mut score = scorer.lock();
 	match event {
 		Event::PaymentPathFailed { ref path, short_channel_id: Some(scid), .. } => {
-			let path = path.iter().collect::<Vec<_>>();
 			score.payment_path_failed(&path, *scid);
 		},
 		Event::PaymentPathFailed { ref path, payment_failed_permanently: true, .. } => {
 			// Reached if the destination explicitly failed it back. We treat this as a successful probe
 			// because the payment made it all the way to the destination with sufficient liquidity.
-			let path = path.iter().collect::<Vec<_>>();
 			score.probe_successful(&path);
 		},
 		Event::PaymentPathSuccessful { path, .. } => {
-			let path = path.iter().collect::<Vec<_>>();
 			score.payment_path_successful(&path);
 		},
 		Event::ProbeSuccessful { path, .. } => {
-			let path = path.iter().collect::<Vec<_>>();
 			score.probe_successful(&path);
 		},
 		Event::ProbeFailed { path, short_channel_id: Some(scid), .. } => {
-			let path = path.iter().collect::<Vec<_>>();
 			score.probe_failed(&path, *scid);
 		},
 		_ => {},
@@ -748,7 +743,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, RouteHop};
+	use lightning::routing::router::{DefaultRouter, Path, RouteHop};
 	use lightning::routing::scoring::{ChannelUsage, Score};
 	use lightning::util::config::UserConfig;
 	use lightning::util::ser::Writeable;
@@ -888,10 +883,10 @@ mod tests {
 
 	#[derive(Debug)]
 	enum TestResult {
-		PaymentFailure { path: Vec<RouteHop>, short_channel_id: u64 },
-		PaymentSuccess { path: Vec<RouteHop> },
-		ProbeFailure { path: Vec<RouteHop> },
-		ProbeSuccess { path: Vec<RouteHop> },
+		PaymentFailure { path: Path, short_channel_id: u64 },
+		PaymentSuccess { path: Path },
+		ProbeFailure { path: Path },
+		ProbeSuccess { path: Path },
 	}
 
 	impl TestScorer {
@@ -913,11 +908,11 @@ mod tests {
 			&self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage
 		) -> u64 { unimplemented!(); }
 
-		fn payment_path_failed(&mut self, actual_path: &[&RouteHop], actual_short_channel_id: u64) {
+		fn payment_path_failed(&mut self, actual_path: &Path, actual_short_channel_id: u64) {
 			if let Some(expectations) = &mut self.event_expectations {
 				match expectations.pop_front().unwrap() {
 					TestResult::PaymentFailure { path, short_channel_id } => {
-						assert_eq!(actual_path, &path.iter().collect::<Vec<_>>()[..]);
+						assert_eq!(actual_path, &path);
 						assert_eq!(actual_short_channel_id, short_channel_id);
 					},
 					TestResult::PaymentSuccess { path } => {
@@ -933,14 +928,14 @@ mod tests {
 			}
 		}
 
-		fn payment_path_successful(&mut self, actual_path: &[&RouteHop]) {
+		fn payment_path_successful(&mut self, actual_path: &Path) {
 			if let Some(expectations) = &mut self.event_expectations {
 				match expectations.pop_front().unwrap() {
 					TestResult::PaymentFailure { path, .. } => {
 						panic!("Unexpected payment path failure: {:?}", path)
 					},
 					TestResult::PaymentSuccess { path } => {
-						assert_eq!(actual_path, &path.iter().collect::<Vec<_>>()[..]);
+						assert_eq!(actual_path, &path);
 					},
 					TestResult::ProbeFailure { path } => {
 						panic!("Unexpected probe failure: {:?}", path)
@@ -952,7 +947,7 @@ mod tests {
 			}
 		}
 
-		fn probe_failed(&mut self, actual_path: &[&RouteHop], _: u64) {
+		fn probe_failed(&mut self, actual_path: &Path, _: u64) {
 			if let Some(expectations) = &mut self.event_expectations {
 				match expectations.pop_front().unwrap() {
 					TestResult::PaymentFailure { path, .. } => {
@@ -962,15 +957,15 @@ mod tests {
 						panic!("Unexpected payment path success: {:?}", path)
 					},
 					TestResult::ProbeFailure { path } => {
-						assert_eq!(actual_path, &path.iter().collect::<Vec<_>>()[..]);
+						assert_eq!(actual_path, &path);
 					},
 					TestResult::ProbeSuccess { path } => {
 						panic!("Unexpected probe success: {:?}", path)
 					}
 				}
 			}
 		}
-		fn probe_successful(&mut self, actual_path: &[&RouteHop]) {
+		fn probe_successful(&mut self, actual_path: &Path) {
 			if let Some(expectations) = &mut self.event_expectations {
 				match expectations.pop_front().unwrap() {
 					TestResult::PaymentFailure { path, .. } => {
@@ -983,7 +978,7 @@ mod tests {
 						panic!("Unexpected probe failure: {:?}", path)
 					},
 					TestResult::ProbeSuccess { path } => {
-						assert_eq!(actual_path, &path.iter().collect::<Vec<_>>()[..]);
+						assert_eq!(actual_path, &path);
 					}
 				}
 			}
@@ -1431,14 +1426,14 @@ mod tests {
 		let node_1_privkey = SecretKey::from_slice(&[42; 32]).unwrap();
 		let node_1_id = PublicKey::from_secret_key(&secp_ctx, &node_1_privkey);
 
-		let path = vec![RouteHop {
+		let path = Path { hops: vec![RouteHop {
 			pubkey: node_1_id,
 			node_features: NodeFeatures::empty(),
 			short_channel_id: scored_scid,
 			channel_features: ChannelFeatures::empty(),
 			fee_msat: 0,
 			cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA as u32,
-		}];
+		}], blinded_tail: None };
 
 		nodes[0].scorer.lock().unwrap().expect(TestResult::PaymentFailure { path: path.clone(), short_channel_id: scored_scid });
 		nodes[0].node.push_pending_event(Event::PaymentPathFailed {

lightning/src/routing/router.rs

@@ -137,19 +137,19 @@ impl<'a, S: Score> Score for ScorerAccountingForInFlightHtlcs<'a, S> {
 		}
 	}
 
-	fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
+	fn payment_path_failed(&mut self, path: &Path, short_channel_id: u64) {
 		self.scorer.payment_path_failed(path, short_channel_id)
 	}
 
-	fn payment_path_successful(&mut self, path: &[&RouteHop]) {
+	fn payment_path_successful(&mut self, path: &Path) {
 		self.scorer.payment_path_successful(path)
 	}
 
-	fn probe_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
+	fn probe_failed(&mut self, path: &Path, short_channel_id: u64) {
 		self.scorer.probe_failed(path, short_channel_id)
 	}
 
-	fn probe_successful(&mut self, path: &[&RouteHop]) {
+	fn probe_successful(&mut self, path: &Path) {
 		self.scorer.probe_successful(path)
 	}
 }
@@ -2251,13 +2251,13 @@ fn build_route_from_hops_internal<L: Deref>(
 			u64::max_value()
 		}
 
-		fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
+		fn payment_path_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
 
-		fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
+		fn payment_path_successful(&mut self, _path: &Path) {}
 
-		fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
+		fn probe_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
 
-		fn probe_successful(&mut self, _path: &[&RouteHop]) {}
+		fn probe_successful(&mut self, _path: &Path) {}
 	}
 
 	impl<'a> Writeable for HopScorer {
@@ -5268,10 +5268,10 @@ mod tests {
 			if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
 		}
 
-		fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
-		fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
-		fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
-		fn probe_successful(&mut self, _path: &[&RouteHop]) {}
+		fn payment_path_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
+		fn payment_path_successful(&mut self, _path: &Path) {}
+		fn probe_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
+		fn probe_successful(&mut self, _path: &Path) {}
 	}
 
 	struct BadNodeScorer {
@@ -5288,10 +5288,10 @@ mod tests {
 			if *target == self.node_id { u64::max_value() } else { 0 }
 		}
 
-		fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
-		fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
-		fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
-		fn probe_successful(&mut self, _path: &[&RouteHop]) {}
+		fn payment_path_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
+		fn payment_path_successful(&mut self, _path: &Path) {}
+		fn probe_failed(&mut self, _path: &Path, _short_channel_id: u64) {}
+		fn probe_successful(&mut self, _path: &Path) {}
 	}
 
 	#[test]
@@ -5955,14 +5955,12 @@ mod benches {
 			let amount = route.get_total_amount();
 			if amount < 250_000 {
 				for path in route.paths {
-					// fixed to use the whole Path in an upcoming commit
-					scorer.payment_path_successful(&path.hops.iter().collect::<Vec<_>>());
+					scorer.payment_path_successful(&path);
 				}
 			} else if amount > 750_000 {
 				for path in route.paths {
 					let short_channel_id = path.hops[path.hops.len() / 2].short_channel_id;
-					// fixed to use the whole Path in an upcoming commit
-					scorer.payment_path_failed(&path.hops.iter().collect::<Vec<_>>(), short_channel_id);
+					scorer.payment_path_failed(&path, short_channel_id);
 				}
 			}
 		}