Merge pull request #1862 from valentinewallace/2022-11-chanman-retries-prep

Prepare for Payment Retries in `ChannelManager`

Author: TheBlueMatt

lightning-background-processor/src/lib.rs

@@ -578,13 +578,13 @@ mod tests {
 	use lightning::ln::msgs::{ChannelMessageHandler, Init};
 	use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler};
 	use lightning::routing::gossip::{NetworkGraph, P2PGossipSync};
+	use lightning::routing::router::DefaultRouter;
 	use lightning::util::config::UserConfig;
 	use lightning::util::events::{Event, MessageSendEventsProvider, MessageSendEvent};
 	use lightning::util::ser::Writeable;
 	use lightning::util::test_utils;
 	use lightning::util::persist::KVStorePersister;
 	use lightning_invoice::payment::{InvoicePayer, Retry};
-	use lightning_invoice::utils::DefaultRouter;
 	use lightning_persister::FilesystemPersister;
 	use std::fs;
 	use std::path::PathBuf;

lightning-invoice/src/payment.rs

@@ -44,7 +44,7 @@
 //! # use lightning::util::logger::{Logger, Record};
 //! # use lightning::util::ser::{Writeable, Writer};
 //! # use lightning_invoice::Invoice;
-//! # use lightning_invoice::payment::{InvoicePayer, Payer, Retry, ScoringRouter};
+//! # use lightning_invoice::payment::{InvoicePayer, Payer, Retry};
 //! # use secp256k1::PublicKey;
 //! # use std::cell::RefCell;
 //! # use std::ops::Deref;
@@ -78,8 +78,6 @@
 //! #         &self, payer: &PublicKey, params: &RouteParameters,
 //! #         first_hops: Option<&[&ChannelDetails]>, _inflight_htlcs: InFlightHtlcs
 //! #     ) -> Result<Route, LightningError> { unimplemented!() }
-//! # }
-//! # impl ScoringRouter for FakeRouter {
 //! #     fn notify_payment_path_failed(&self, path: &[&RouteHop], short_channel_id: u64) {  unimplemented!() }
 //! #     fn notify_payment_path_successful(&self, path: &[&RouteHop]) {  unimplemented!() }
 //! #     fn notify_payment_probe_successful(&self, path: &[&RouteHop]) {  unimplemented!() }
@@ -146,7 +144,7 @@ use crate::prelude::*;
 use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
 use lightning::ln::channelmanager::{ChannelDetails, PaymentId, PaymentSendFailure};
 use lightning::ln::msgs::LightningError;
-use lightning::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, Router};
+use lightning::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters, Router};
 use lightning::util::events::{Event, EventHandler};
 use lightning::util::logger::Logger;
 use crate::time_utils::Time;
@@ -186,7 +184,7 @@ mod sealed {
 /// (C-not exported) generally all users should use the [`InvoicePayer`] type alias.
 pub struct InvoicePayerUsingTime<
 	P: Deref,
-	R: ScoringRouter,
+	R: Router,
 	L: Deref,
 	E: sealed::BaseEventHandler,
 	T: Time
@@ -279,30 +277,6 @@ pub trait Payer {
 	fn inflight_htlcs(&self) -> InFlightHtlcs;
 }
 
-/// A trait defining behavior for a [`Router`] implementation that also supports scoring channels
-/// based on payment and probe success/failure.
-///
-/// [`Router`]: lightning::routing::router::Router
-pub trait ScoringRouter: Router {
-	/// Finds a [`Route`] between `payer` and `payee` for a payment with the given values. Includes
-	/// `PaymentHash` and `PaymentId` to be able to correlate the request with a specific payment.
-	fn find_route_with_id(
-		&self, payer: &PublicKey, route_params: &RouteParameters,
-		first_hops: Option<&[&ChannelDetails]>, inflight_htlcs: InFlightHtlcs,
-		_payment_hash: PaymentHash, _payment_id: PaymentId
-	) -> Result<Route, LightningError> {
-		self.find_route(payer, route_params, first_hops, inflight_htlcs)
-	}
-	/// Lets the router know that payment through a specific path has failed.
-	fn notify_payment_path_failed(&self, path: &[&RouteHop], short_channel_id: u64);
-	/// Lets the router know that payment through a specific path was successful.
-	fn notify_payment_path_successful(&self, path: &[&RouteHop]);
-	/// Lets the router know that a payment probe was successful.
-	fn notify_payment_probe_successful(&self, path: &[&RouteHop]);
-	/// Lets the router know that a payment probe failed.
-	fn notify_payment_probe_failed(&self, path: &[&RouteHop], short_channel_id: u64);
-}
-
 /// Strategies available to retry payment path failures for an [`Invoice`].
 ///
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
@@ -342,7 +316,7 @@ pub enum PaymentError {
 	Sending(PaymentSendFailure),
 }
 
-impl<P: Deref, R: ScoringRouter, L: Deref, E: sealed::BaseEventHandler, T: Time>
+impl<P: Deref, R: Router, L: Deref, E: sealed::BaseEventHandler, T: Time>
 	InvoicePayerUsingTime<P, R, L, E, T>
 where
 	P::Target: Payer,
@@ -656,7 +630,7 @@ fn has_expired(route_params: &RouteParameters) -> bool {
 	} else { false }
 }
 
-impl<P: Deref, R: ScoringRouter, L: Deref, E: sealed::BaseEventHandler, T: Time>
+impl<P: Deref, R: Router, L: Deref, E: sealed::BaseEventHandler, T: Time>
 	InvoicePayerUsingTime<P, R, L, E, T>
 where
 	P::Target: Payer,
@@ -723,7 +697,7 @@ where
 	}
 }
 
-impl<P: Deref, R: ScoringRouter, L: Deref, E: EventHandler, T: Time>
+impl<P: Deref, R: Router, L: Deref, E: EventHandler, T: Time>
 	EventHandler for InvoicePayerUsingTime<P, R, L, E, T>
 where
 	P::Target: Payer,
@@ -737,7 +711,7 @@ where
 	}
 }
 
-impl<P: Deref, R: ScoringRouter, L: Deref, T: Time, F: Future, H: Fn(Event) -> F>
+impl<P: Deref, R: Router, L: Deref, T: Time, F: Future, H: Fn(Event) -> F>
 	InvoicePayerUsingTime<P, R, L, H, T>
 where
 	P::Target: Payer,
@@ -757,15 +731,15 @@ where
 mod tests {
 	use super::*;
 	use crate::{InvoiceBuilder, Currency};
-	use crate::utils::{ScorerAccountingForInFlightHtlcs, create_invoice_from_channelmanager_and_duration_since_epoch};
+	use crate::utils::create_invoice_from_channelmanager_and_duration_since_epoch;
 	use bitcoin_hashes::sha256::Hash as Sha256;
 	use lightning::ln::PaymentPreimage;
 	use lightning::ln::channelmanager;
 	use lightning::ln::features::{ChannelFeatures, NodeFeatures};
 	use lightning::ln::functional_test_utils::*;
 	use lightning::ln::msgs::{ChannelMessageHandler, ErrorAction, LightningError};
 	use lightning::routing::gossip::{EffectiveCapacity, NodeId};
-	use lightning::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, Router};
+	use lightning::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, Router, ScorerAccountingForInFlightHtlcs};
 	use lightning::routing::scoring::{ChannelUsage, LockableScore, Score};
 	use lightning::util::test_utils::TestLogger;
 	use lightning::util::errors::APIError;
@@ -1726,9 +1700,7 @@ mod tests {
 				payment_params: Some(route_params.payment_params.clone()), ..Self::route_for_value(route_params.final_value_msat)
 			})
 		}
-	}
 
-	impl ScoringRouter for TestRouter {
 		fn notify_payment_path_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
 			self.scorer.lock().payment_path_failed(path, short_channel_id);
 		}
@@ -1755,9 +1727,7 @@ mod tests {
 		) -> Result<Route, LightningError> {
 			Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError })
 		}
-	}
 
-	impl ScoringRouter for FailingRouter {
 		fn notify_payment_path_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
 
 		fn notify_payment_path_successful(&self, _path: &[&RouteHop]) {}
@@ -2045,8 +2015,7 @@ mod tests {
 		) -> Result<Route, LightningError> {
 			self.0.borrow_mut().pop_front().unwrap()
 		}
-	}
-	impl ScoringRouter for ManualRouter {
+
 		fn notify_payment_path_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
 
 		fn notify_payment_path_successful(&self, _path: &[&RouteHop]) {}

lightning-invoice/src/utils.rs

@@ -1,11 +1,11 @@
 //! Convenient utilities to create an invoice.
 
 use crate::{CreationError, Currency, Invoice, InvoiceBuilder, SignOrCreationError};
-use crate::payment::{Payer, ScoringRouter};
+use crate::payment::Payer;
 
 use crate::{prelude::*, Description, InvoiceDescription, Sha256};
 use bech32::ToBase32;
-use bitcoin_hashes::{Hash, sha256};
+use bitcoin_hashes::Hash;
 use lightning::chain;
 use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use lightning::chain::keysinterface::{Recipient, KeysInterface};
@@ -14,15 +14,12 @@ use lightning::ln::channelmanager::{ChannelDetails, ChannelManager, PaymentId, P
 #[cfg(feature = "std")]
 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, NodeId, RoutingFees};
-use lightning::routing::router::{InFlightHtlcs, Route, RouteHint, RouteHintHop, RouteParameters, find_route, RouteHop, Router};
-use lightning::routing::scoring::{ChannelUsage, LockableScore, Score};
+use lightning::routing::gossip::RoutingFees;
+use lightning::routing::router::{InFlightHtlcs, Route, RouteHint, RouteHintHop};
 use lightning::util::logger::Logger;
 use secp256k1::PublicKey;
 use core::ops::Deref;
 use core::time::Duration;
-use crate::sync::Mutex;
 
 #[cfg(feature = "std")]
 /// Utility to create an invoice that can be paid to one of multiple nodes, or a "phantom invoice."
@@ -524,72 +521,6 @@ fn filter_channels<L: Deref>(
 		.collect::<Vec<RouteHint>>()
 }
 
-/// A [`Router`] implemented using [`find_route`].
-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, 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], scorer: S) -> Self {
-		let random_seed_bytes = Mutex::new(random_seed_bytes);
-		Self { network_graph, logger, random_seed_bytes, scorer }
-	}
-}
-
-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, first_hops: Option<&[&ChannelDetails]>,
-		inflight_htlcs: InFlightHtlcs
-	) -> 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,
-			&ScorerAccountingForInFlightHtlcs::new(&mut self.scorer.lock(), inflight_htlcs),
-			&random_seed_bytes
-		)
-	}
-}
-
-impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> ScoringRouter for DefaultRouter<G, L, S> where
-	L::Target: Logger,
-	S::Target: for <'a> LockableScore<'a>,
-{
-	fn notify_payment_path_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
-		self.scorer.lock().payment_path_failed(path, short_channel_id);
-	}
-
-	fn notify_payment_path_successful(&self, path: &[&RouteHop]) {
-		self.scorer.lock().payment_path_successful(path);
-	}
-
-	fn notify_payment_probe_successful(&self, path: &[&RouteHop]) {
-		self.scorer.lock().probe_successful(path);
-	}
-
-	fn notify_payment_probe_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
-		self.scorer.lock().probe_failed(path, short_channel_id);
-	}
-}
-
 impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Payer for ChannelManager<M, T, K, F, L>
 where
 	M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
@@ -632,54 +563,6 @@ where
 	fn inflight_htlcs(&self) -> InFlightHtlcs { self.compute_inflight_htlcs() }
 }
 
-
-/// Used to store information about all the HTLCs that are inflight across all payment attempts.
-pub(crate) struct ScorerAccountingForInFlightHtlcs<'a, S: Score> {
-	scorer: &'a mut S,
-	/// Maps a channel's short channel id and its direction to the liquidity used up.
-	inflight_htlcs: InFlightHtlcs,
-}
-
-impl<'a, S: Score> ScorerAccountingForInFlightHtlcs<'a, S> {
-	pub(crate) fn new(scorer: &'a mut S, inflight_htlcs: InFlightHtlcs) -> Self {
-		ScorerAccountingForInFlightHtlcs {
-			scorer,
-			inflight_htlcs
-		}
-	}
-}
-
-#[cfg(c_bindings)]
-impl<'a, S:Score> lightning::util::ser::Writeable for ScorerAccountingForInFlightHtlcs<'a, S> {
-	fn write<W: lightning::util::ser::Writer>(&self, writer: &mut W) -> Result<(), lightning::io::Error> { self.scorer.write(writer) }
-}
-
-impl<'a, S: Score> Score for ScorerAccountingForInFlightHtlcs<'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.used_liquidity_msat(
-			source, target, short_channel_id
-		) {
-			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) { unreachable!() }
-
-	fn payment_path_successful(&mut self, _path: &[&RouteHop]) { unreachable!() }
-
-	fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) { unreachable!() }
-
-	fn probe_successful(&mut self, _path: &[&RouteHop]) { unreachable!() }
-}
-
-
 #[cfg(test)]
 mod test {
 	use core::time::Duration;