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

Author: valentinewallace

lightning-background-processor/src/lib.rs

@@ -236,26 +236,21 @@ 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.hops.iter().collect::<Vec<_>>();
-			score.payment_path_failed(&path, *scid);
+			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.hops.iter().collect::<Vec<_>>();
-			score.probe_successful(&path);
+			score.probe_successful(path);
 		},
 		Event::PaymentPathSuccessful { path, .. } => {
-			let path = path.hops.iter().collect::<Vec<_>>();
-			score.payment_path_successful(&path);
+			score.payment_path_successful(path);
 		},
 		Event::ProbeSuccessful { path, .. } => {
-			let path = path.hops.iter().collect::<Vec<_>>();
-			score.probe_successful(&path);
+			score.probe_successful(path);
 		},
 		Event::ProbeFailed { path, short_channel_id: Some(scid), .. } => {
-			let path = path.hops.iter().collect::<Vec<_>>();
-			score.probe_failed(&path, *scid);
+			score.probe_failed(path, *scid);
 		},
 		_ => {},
 	}
@@ -916,11 +911,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.hops.iter().collect::<Vec<_>>()[..]);
+						assert_eq!(actual_path, &path);
 						assert_eq!(actual_short_channel_id, short_channel_id);
 					},
 					TestResult::PaymentSuccess { path } => {
@@ -936,14 +931,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.hops.iter().collect::<Vec<_>>()[..]);
+						assert_eq!(actual_path, &path);
 					},
 					TestResult::ProbeFailure { path } => {
 						panic!("Unexpected probe failure: {:?}", path)
@@ -955,7 +950,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, .. } => {
@@ -965,15 +960,15 @@ mod tests {
 						panic!("Unexpected payment path success: {:?}", path)
 					},
 					TestResult::ProbeFailure { path } => {
-						assert_eq!(actual_path, &path.hops.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, .. } => {
@@ -986,7 +981,7 @@ mod tests {
 						panic!("Unexpected probe failure: {:?}", path)
 					},
 					TestResult::ProbeSuccess { path } => {
-						assert_eq!(actual_path, &path.hops.iter().collect::<Vec<_>>()[..]);
+						assert_eq!(actual_path, &path);
 					}
 				}
 			}

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 {
@@ -5299,10 +5299,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 {
@@ -5319,10 +5319,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]
@@ -5986,14 +5986,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);
 				}
 			}
 		}