Default to ProbabilisticScorer with a base penalty

ProbabilisticScorer tends to prefer longer routes to shorter ones. Make
the default scoring behavior include a customizable base penalty to
avoid longer routes. A new DefaultScorer struct is used such that the
default implementation could vary without requiring users to explicitly
upgrade.

Author: jkczyz

lightning/src/routing/scoring.rs

@@ -9,8 +9,8 @@
 
 //! Utilities for scoring payment channels.
 //!
-//! [`ProbabilisticScorer`] may be given to [`find_route`] to score payment channels during path
-//! finding when a custom [`Score`] implementation is not needed.
+//! [`DefaultScorer`] may be given to [`find_route`] to score payment channels during path finding
+//! when a custom [`Score`] implementation is not needed.
 //!
 //! # Example
 //!
@@ -19,7 +19,7 @@
 //! #
 //! # use lightning::routing::network_graph::NetworkGraph;
 //! # use lightning::routing::router::{RouteParameters, find_route};
-//! # use lightning::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringParameters, Scorer, ScoringParameters};
+//! # use lightning::routing::scoring::{DefaultScorer, DefaultScoringParameters, ProbabilisticScoringParameters};
 //! # use lightning::chain::keysinterface::{KeysManager, KeysInterface};
 //! # use lightning::util::logger::{Logger, Record};
 //! # use secp256k1::key::PublicKey;
@@ -32,15 +32,18 @@
 //! # let logger = FakeLogger {};
 //! #
 //! // Use the default channel penalties.
-//! let params = ProbabilisticScoringParameters::default();
-//! let scorer = ProbabilisticScorer::new(params, &network_graph);
+//! let params = DefaultScoringParameters::default();
+//! let scorer = DefaultScorer::new(params, &network_graph);
 //!
 //! // Or use custom channel penalties.
-//! let params = ProbabilisticScoringParameters {
-//!     liquidity_penalty_multiplier_msat: 2 * 1000,
-//!     ..ProbabilisticScoringParameters::default()
+//! let params = DefaultScoringParameters {
+//!     base_penalty_msat: 1000,
+//!     probabilistic_params: ProbabilisticScoringParameters {
+//!         liquidity_penalty_multiplier_msat: 2 * 1000,
+//!         ..Default::default()
+//!     },
 //! };
-//! let scorer = ProbabilisticScorer::new(params, &network_graph);
+//! let scorer = DefaultScorer::new(params, &network_graph);
 //! # let random_seed_bytes = [42u8; 32];
 //!
 //! let route = find_route(&payer, &route_params, &network_graph, None, &logger, &scorer, &random_seed_bytes);
@@ -191,6 +194,93 @@ impl<'a, S: Writeable> Writeable for MutexGuard<'a, S> {
 	}
 }
 
+/// [`Score`] implementation providing reasonable default behavior.
+pub struct DefaultScorer<G: Deref<Target = NetworkGraph>> {
+	base_penalty_scorer: FixedPenaltyScorer,
+	probabilistic_scorer: ProbabilisticScorer<G>,
+}
+
+/// Parameters for configuring [`DefaultScorer`].
+pub struct DefaultScoringParameters {
+	/// A fixed penalty in msats to apply to each channel.
+	///
+	/// Default value: 500 msat
+	pub base_penalty_msat: u64,
+
+	/// Parameters for calculating a penalty for routing a payment through a channel using a
+	/// probabilistic model of success.
+	pub probabilistic_params: ProbabilisticScoringParameters,
+}
+
+impl<G: Deref<Target = NetworkGraph>> DefaultScorer<G> {
+	/// Creates a new scorer using the given scoring parameters for sending payments from a node
+	/// through a network graph.
+	pub fn new(params: DefaultScoringParameters, network_graph: G) -> Self {
+		Self {
+			base_penalty_scorer: FixedPenaltyScorer::with_penalty(params.base_penalty_msat),
+			probabilistic_scorer:
+				ProbabilisticScorer::new(params.probabilistic_params, network_graph),
+		}
+	}
+}
+
+impl Default for DefaultScoringParameters {
+	fn default() -> Self {
+		Self {
+			base_penalty_msat: 500,
+			probabilistic_params: Default::default(),
+		}
+	}
+}
+
+impl<G: Deref<Target = NetworkGraph>> Score for DefaultScorer<G> {
+	fn channel_penalty_msat(
+		&self, short_channel_id: u64, amount_msat: u64, capacity_msat: u64, source: &NodeId,
+		target: &NodeId
+	) -> u64 {
+		let base_penalty_msat = self.base_penalty_scorer.channel_penalty_msat(
+			short_channel_id, amount_msat, capacity_msat, source, target
+		);
+		let probabilistic_penalty_msat = self.probabilistic_scorer.channel_penalty_msat(
+			short_channel_id, amount_msat, capacity_msat, source, target
+		);
+		base_penalty_msat.saturating_add(probabilistic_penalty_msat)
+	}
+
+	fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
+		self.base_penalty_scorer.payment_path_failed(path, short_channel_id);
+		self.probabilistic_scorer.payment_path_failed(path, short_channel_id);
+	}
+
+	fn payment_path_successful(&mut self, path: &[&RouteHop]) {
+		self.base_penalty_scorer.payment_path_successful(path);
+		self.probabilistic_scorer.payment_path_successful(path);
+	}
+}
+
+impl<G: Deref<Target = NetworkGraph>> Writeable for DefaultScorer<G> {
+	#[inline]
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+		self.base_penalty_scorer.write(w)?;
+		self.probabilistic_scorer.write(w)
+	}
+}
+
+impl<G> ReadableArgs<(DefaultScoringParameters, G)> for DefaultScorer<G>
+where
+	G: Deref<Target = NetworkGraph>,
+{
+	#[inline]
+	fn read<R: Read>(r: &mut R, args: (DefaultScoringParameters, G)) -> Result<Self, DecodeError> {
+		let (params, network_graph) = args;
+		Ok(Self {
+			base_penalty_scorer: ReadableArgs::read(r, params.base_penalty_msat)?,
+			probabilistic_scorer:
+				ReadableArgs::read(r, (params.probabilistic_params, network_graph))?,
+		})
+	}
+}
+
 #[derive(Clone)]
 /// [`Score`] implementation that uses a fixed penalty.
 pub struct FixedPenaltyScorer {
@@ -235,7 +325,7 @@ impl ReadableArgs<u64> for FixedPenaltyScorer {
 /// Used to apply a fixed penalty to each channel, thus avoiding long paths when shorter paths with
 /// slightly higher fees are available. Will further penalize channels that fail to relay payments.
 ///
-/// See [module-level documentation] for usage and [`ScoringParameters`] for customization.
+/// See [`ScoringParameters`] for customization.
 ///
 /// # Note
 ///
@@ -245,7 +335,7 @@ impl ReadableArgs<u64> for FixedPenaltyScorer {
 /// [module-level documentation]: crate::routing::scoring
 #[deprecated(
 	since = "0.0.105",
-	note = "ProbabilisticScorer should be used instead of Scorer.",
+	note = "DefaultScorer should be used instead of Scorer.",
 )]
 pub type Scorer = ScorerUsingTime::<ConfiguredTime>;
 
@@ -1100,7 +1190,7 @@ pub(crate) use self::time::Time;
 
 #[cfg(test)]
 mod tests {
-	use super::{ChannelLiquidity, ProbabilisticScoringCostFunction, ProbabilisticScoringParameters, ProbabilisticScorerUsingTime, ScoringParameters, ScorerUsingTime, Time};
+	use super::{ChannelLiquidity, DefaultScorer, DefaultScoringParameters, ProbabilisticScoringCostFunction, ProbabilisticScoringParameters, ProbabilisticScorerUsingTime, ScoringParameters, ScorerUsingTime, Time};
 	use super::time::Eternity;
 
 	use ln::features::{ChannelFeatures, NodeFeatures};
@@ -1187,6 +1277,27 @@ mod tests {
 		assert_eq!(later - elapsed, now);
 	}
 
+	// `DefaultScorer` tests
+
+	#[test]
+	fn adds_base_penalty_to_scorer_penalty() {
+		let network_graph = network_graph();
+		let source = source_node_id();
+		let target = target_node_id();
+
+		let params = DefaultScoringParameters {
+			base_penalty_msat: 0, ..Default::default()
+		};
+		let scorer = DefaultScorer::new(params, &network_graph);
+		assert_eq!(scorer.channel_penalty_msat(42, 128, 1_024, &source, &target), 585);
+
+		let params = DefaultScoringParameters {
+			base_penalty_msat: 500, ..Default::default()
+		};
+		let scorer = DefaultScorer::new(params, &network_graph);
+		assert_eq!(scorer.channel_penalty_msat(42, 128, 1_024, &source, &target), 1085);
+	}
+
 	// `Scorer` tests
 
 	/// A scorer for testing with time that can be manually advanced.