Move Scorer requirement away from Router trait

We do this to enable users to create routers that do not need a scorer.
This can be useful if they are running a node the delegates pathfinding.

We replace the `scorer` parameter in `find_route` with `inflight_map`,
so `InvoicePayer` can still hand off a map of inflight HTLCs. It can
also be left empty.

Then, we move `AccountsForInflightHtlcs` to `DefaultRouter`, which we
will use to wrap the new `scorer` field, so scoring only happens in
`DefaultRouter` explicitly.

Author: jurvis

lightning-invoice/src/payment.rs

@@ -145,7 +145,7 @@ use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
 use lightning::ln::channelmanager::{ChannelDetails, PaymentId, PaymentSendFailure};
 use lightning::ln::msgs::LightningError;
 use lightning::routing::gossip::NodeId;
-use lightning::routing::scoring::{ChannelUsage, LockableScore, Score};
+use lightning::routing::scoring::LockableScore;
 use lightning::routing::router::{PaymentParameters, Route, RouteHop, RouteParameters};
 use lightning::util::errors::APIError;
 use lightning::util::events::{Event, EventHandler};
@@ -209,49 +209,6 @@ impl<T: Time> PaymentInfo<T> {
 	}
 }
 
-/// Used to store information about all the HTLCs that are inflight across all payment attempts
-struct AccountForInFlightHtlcs<'a, S: Score> {
-	scorer: &'a mut S,
-	/// Maps a channel's short channel id and its direction to the liquidity used up.
-	inflight_htlcs: HashMap<(u64, bool), u64>,
-}
-
-#[cfg(c_bindings)]
-impl<'a, S:Score> lightning::util::ser::Writeable for AccountForInFlightHtlcs<'a, S> {
-	fn write<W: lightning::util::ser::Writer>(&self, writer: &mut W) -> Result<(), std::io::Error> { self.scorer.write(writer) }
-}
-
-impl<'a, S: Score> Score for AccountForInFlightHtlcs<'a, S> {
-	fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage) -> u64 {
-		if let Some(used_liqudity) = self.inflight_htlcs.get(&(short_channel_id, source < target)) {
-			let usage = ChannelUsage {
-				inflight_htlc_msat: usage.inflight_htlc_msat + used_liqudity,
-				..usage
-			};
-
-			self.scorer.channel_penalty_msat(short_channel_id, source, target, usage)
-		} else {
-			self.scorer.channel_penalty_msat(short_channel_id, source, target, usage)
-		}
-	}
-
-	fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
-		self.scorer.payment_path_failed(path, short_channel_id)
-	}
-
-	fn payment_path_successful(&mut self, path: &[&RouteHop]) {
-		self.scorer.payment_path_successful(path)
-	}
-
-	fn probe_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
-		self.scorer.probe_failed(path, short_channel_id)
-	}
-
-	fn probe_successful(&mut self, path: &[&RouteHop]) {
-		self.scorer.probe_successful(path)
-	}
-}
-
 /// Storing minimal payment attempts information required for determining if a outbound payment can
 /// be retried.
 #[derive(Clone, Copy)]
@@ -314,9 +271,9 @@ pub trait Payer {
 /// A trait defining behavior for routing an [`Invoice`] payment.
 pub trait Router {
 	/// Finds a [`Route`] between `payer` and `payee` for a payment with the given values.
-	fn find_route<S: Score>(
+	fn find_route(
 		&self, payer: &PublicKey, route_params: &RouteParameters, payment_hash: &PaymentHash,
-		first_hops: Option<&[&ChannelDetails]>, scorer: &S
+		first_hops: Option<&[&ChannelDetails]>, inflight_htlc_map: HashMap<(u64, bool), u64>
 	) -> Result<Route, LightningError>;
 }
 
@@ -487,7 +444,7 @@ where
 		let inflight_htlcs = self.create_inflight_map();
 		let route = self.router.find_route(
 			&payer, &params, &payment_hash, Some(&first_hops.iter().collect::<Vec<_>>()),
-			&AccountForInFlightHtlcs { scorer: &mut self.scorer.lock(), inflight_htlcs }
+			inflight_htlcs
 		).map_err(|e| PaymentError::Routing(e))?;
 
 		match send_payment(&route) {
@@ -592,7 +549,7 @@ where
 
 		let route = self.router.find_route(
 			&payer, &params, &payment_hash, Some(&first_hops.iter().collect::<Vec<_>>()),
-			&AccountForInFlightHtlcs { scorer: &mut self.scorer.lock(), inflight_htlcs }
+			inflight_htlcs
 		);
 
 		if route.is_err() {

lightning-invoice/src/utils.rs

@@ -15,9 +15,9 @@ use lightning::ln::channelmanager::{ChannelDetails, ChannelManager, PaymentId, P
 use lightning::ln::channelmanager::{PhantomRouteHints, MIN_CLTV_EXPIRY_DELTA};
 use lightning::ln::inbound_payment::{create, create_from_hash, ExpandedKey};
 use lightning::ln::msgs::LightningError;
-use lightning::routing::gossip::{NetworkGraph, RoutingFees};
-use lightning::routing::router::{Route, RouteHint, RouteHintHop, RouteParameters, find_route};
-use lightning::routing::scoring::Score;
+use lightning::routing::gossip::{NetworkGraph, NodeId, RoutingFees};
+use lightning::routing::router::{Route, RouteHint, RouteHintHop, RouteParameters, find_route, RouteHop};
+use lightning::routing::scoring::{ChannelUsage, LockableScore, Score};
 use lightning::util::logger::Logger;
 use secp256k1::PublicKey;
 use core::ops::Deref;
@@ -440,33 +440,47 @@ fn filter_channels(channels: Vec<ChannelDetails>, min_inbound_capacity_msat: Opt
 }
 
 /// A [`Router`] implemented using [`find_route`].
-pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref> where L::Target: Logger {
+pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> where
+	L::Target: Logger,
+	S::Target: for <'a> LockableScore<'a>,
+{
 	network_graph: G,
 	logger: L,
 	random_seed_bytes: Mutex<[u8; 32]>,
+	scorer: S
 }
 
-impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> DefaultRouter<G, L> where L::Target: Logger {
+impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> DefaultRouter<G, L, S> where
+	L::Target: Logger,
+	S::Target: for <'a> LockableScore<'a>,
+{
 	/// Creates a new router using the given [`NetworkGraph`], a [`Logger`], and a randomness source
 	/// `random_seed_bytes`.
-	pub fn new(network_graph: G, logger: L, random_seed_bytes: [u8; 32]) -> Self {
+	pub fn new(network_graph: G, logger: L, random_seed_bytes: [u8; 32], scorer: S) -> Self {
 		let random_seed_bytes = Mutex::new(random_seed_bytes);
-		Self { network_graph, logger, random_seed_bytes }
+		Self { network_graph, logger, random_seed_bytes, scorer }
 	}
 }
 
-impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> Router for DefaultRouter<G, L>
-where L::Target: Logger {
-	fn find_route<S: Score>(
+impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> Router for DefaultRouter<G, L, S> where
+	L::Target: Logger,
+	S::Target: for <'a> LockableScore<'a>,
+{
+	fn find_route(
 		&self, payer: &PublicKey, params: &RouteParameters, _payment_hash: &PaymentHash,
-		first_hops: Option<&[&ChannelDetails]>, scorer: &S
+		first_hops: Option<&[&ChannelDetails]>, inflight_htlc_map: HashMap<(u64, bool), u64>
 	) -> Result<Route, LightningError> {
 		let random_seed_bytes = {
 			let mut locked_random_seed_bytes = self.random_seed_bytes.lock().unwrap();
 			*locked_random_seed_bytes = sha256::Hash::hash(&*locked_random_seed_bytes).into_inner();
 			*locked_random_seed_bytes
 		};
-		find_route(payer, params, &self.network_graph, first_hops, &*self.logger, scorer, &random_seed_bytes)
+
+		find_route(
+			payer, params, &self.network_graph, first_hops, &*self.logger,
+			&AccountForInFlightHtlcs { scorer: &mut self.scorer.lock(), inflight_htlcs: inflight_htlc_map },
+				   &random_seed_bytes
+		)
 	}
 }
 
@@ -510,6 +524,51 @@ where
 	}
 }
 
+
+/// Used to store information about all the HTLCs that are inflight across all payment attempts
+struct AccountForInFlightHtlcs<'a, S: Score> {
+	scorer: &'a mut S,
+	/// Maps a channel's short channel id and its direction to the liquidity used up.
+	inflight_htlcs: HashMap<(u64, bool), u64>,
+}
+
+#[cfg(c_bindings)]
+impl<'a, S:Score> lightning::util::ser::Writeable for AccountForInFlightHtlcs<'a, S> {
+	fn write<W: lightning::util::ser::Writer>(&self, writer: &mut W) -> Result<(), std::io::Error> { self.scorer.write(writer) }
+}
+
+impl<'a, S: Score> Score for AccountForInFlightHtlcs<'a, S> {
+	fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage) -> u64 {
+		if let Some(used_liqudity) = self.inflight_htlcs.get(&(short_channel_id, source < target)) {
+			let usage = ChannelUsage {
+				inflight_htlc_msat: usage.inflight_htlc_msat + used_liqudity,
+				..usage
+			};
+
+			self.scorer.channel_penalty_msat(short_channel_id, source, target, usage)
+		} else {
+			self.scorer.channel_penalty_msat(short_channel_id, source, target, usage)
+		}
+	}
+
+	fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
+		self.scorer.payment_path_failed(path, short_channel_id)
+	}
+
+	fn payment_path_successful(&mut self, path: &[&RouteHop]) {
+		self.scorer.payment_path_successful(path)
+	}
+
+	fn probe_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
+		self.scorer.probe_failed(path, short_channel_id)
+	}
+
+	fn probe_successful(&mut self, path: &[&RouteHop]) {
+		self.scorer.probe_successful(path)
+	}
+}
+
+
 #[cfg(test)]
 mod test {
 	use core::time::Duration;