f don't wrap ChanMan router, use one-shot router

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -55,7 +55,7 @@ use crate::ln::channel_state::ChannelDetails;
 use crate::types::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
 #[cfg(any(feature = "_test_utils", test))]
 use crate::types::features::Bolt11InvoiceFeatures;
-use crate::routing::router::{BlindedTail, CachingRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router};
+use crate::routing::router::{BlindedTail, FixedRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router};
 use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, InboundHTLCErr, NextPacketDetails};
 use crate::ln::msgs;
 use crate::ln::onion_utils;
@@ -2391,7 +2391,7 @@ where
 	fee_estimator: LowerBoundedFeeEstimator<F>,
 	chain_monitor: M,
 	tx_broadcaster: T,
-	router: CachingRouter<R>,
+	router: R,
 	message_router: MR,
 
 	/// See `ChannelManager` struct-level documentation for lock order requirements.
@@ -3498,7 +3498,7 @@ where
 			fee_estimator: LowerBoundedFeeEstimator::new(fee_est),
 			chain_monitor,
 			tx_broadcaster,
-			router: CachingRouter::new(router),
+			router,
 			message_router,
 
 			best_block: RwLock::new(params.best_block),
@@ -4616,12 +4616,12 @@ where
 			RouteParameters::from_payment_params_and_value(payment_params, route.get_total_amount())
 		});
 		if route.route_params.is_none() { route.route_params = Some(route_params.clone()); }
-		self.router.route_cache.lock().unwrap().insert(payment_id, route);
+		let router = FixedRouter::new(route);
 		self.pending_outbound_payments
 			.send_payment(payment_hash, recipient_onion, payment_id, Retry::Attempts(0),
-				route_params, &self.router, self.list_usable_channels(), ||
-				self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
-				&self.logger, &self.pending_events, |args| self.send_payment_along_path(args))
+				route_params, &router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
+				&self.entropy_source, &self.node_signer, best_block_height, &self.logger,
+				&self.pending_events, |args| self.send_payment_along_path(args))
 	}
 
 	/// Sends a payment to the route found using the provided [`RouteParameters`], retrying failed
@@ -4666,7 +4666,7 @@ where
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 		self.pending_outbound_payments
 			.send_payment(payment_hash, recipient_onion, payment_id, retry_strategy, route_params,
-				&self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
+				&*self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
 				&self.entropy_source, &self.node_signer, best_block_height, &self.logger,
 				&self.pending_events, |args| self.send_payment_along_path(args))
 	}
@@ -4743,7 +4743,7 @@ where
 		let features = self.bolt12_invoice_features();
 		self.pending_outbound_payments
 			.send_payment_for_bolt12_invoice(
-				invoice, payment_id, &self.router, self.list_usable_channels(), features,
+				invoice, payment_id, &*self.router, self.list_usable_channels(), features,
 				|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, &self,
 				&self.secp_ctx, best_block_height, &self.logger, &self.pending_events,
 				|args| self.send_payment_along_path(args)
@@ -4818,7 +4818,7 @@ where
 		let mut res = Ok(());
 		PersistenceNotifierGuard::optionally_notify(self, || {
 			let outbound_pmts_res = self.pending_outbound_payments.send_payment_for_static_invoice(
-				payment_id, &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
+				payment_id, &*self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
 				&self.entropy_source, &self.node_signer, &self, &self.secp_ctx, best_block_height,
 				&self.logger, &self.pending_events, |args| self.send_payment_along_path(args)
 			);
@@ -4894,7 +4894,7 @@ where
 		let best_block_height = self.best_block.read().unwrap().height;
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 		self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, recipient_onion,
-			payment_id, retry_strategy, route_params, &self.router, self.list_usable_channels(),
+			payment_id, retry_strategy, route_params, &*self.router, self.list_usable_channels(),
 			|| self.compute_inflight_htlcs(),  &self.entropy_source, &self.node_signer, best_block_height,
 			&self.logger, &self.pending_events, |args| self.send_payment_along_path(args))
 	}
@@ -6273,7 +6273,7 @@ where
 		}
 
 		let best_block_height = self.best_block.read().unwrap().height;
-		self.pending_outbound_payments.check_retry_payments(&self.router, || self.list_usable_channels(),
+		self.pending_outbound_payments.check_retry_payments(&*self.router, || self.list_usable_channels(),
 			|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
 			&self.pending_events, &self.logger, |args| self.send_payment_along_path(args));
 
@@ -14184,7 +14184,7 @@ where
 			fee_estimator: bounded_fee_estimator,
 			chain_monitor: args.chain_monitor,
 			tx_broadcaster: args.tx_broadcaster,
-			router: CachingRouter::new(args.router),
+			router: args.router,
 			message_router: args.message_router,
 
 			best_block: RwLock::new(BestBlock::new(best_block_hash, best_block_height)),

lightning/src/routing/router.rs

@@ -186,50 +186,49 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, ES: Deref, S: Deref, SP: Size
 	}
 }
 
-/// A wrapper around a `Router` that can cache routes intended to be used with specific
-/// `PaymentId`s. Useful for `ChannelManager::send_payment_with_route` to support sending to
-/// specific routes without requiring a custom `Router` implementation to be used.
-pub(crate) struct CachingRouter<R: Deref> where R::Target: Router {
-	pub(crate) route_cache: Mutex<HashMap<PaymentId, Route>>,
-	router: R,
+/// A `Router` that returns a fixed route one time, erroring otherwise. Useful for
+/// `ChannelManager::send_payment_with_route` to support sending to specific routes without
+/// requiring a custom `Router` implementation.
+pub(crate) struct FixedRouter {
+	// Use an `Option` to avoid needing to clone the route when `find_route` is called.
+	route: Mutex<Option<Route>>,
 }
 
-impl<R: Deref> CachingRouter<R> where R::Target: Router {
-	pub(crate) fn new(router: R) -> Self {
-		Self { route_cache: Mutex::new(new_hash_map()), router }
+impl FixedRouter {
+	pub(crate) fn new(route: Route) -> Self {
+		Self { route: Mutex::new(Some(route)) }
 	}
 }
 
-impl<R: Deref> Router for CachingRouter<R> where R::Target: Router {
+impl Router for FixedRouter {
 	fn find_route(
-		&self, payer: &PublicKey, route_params: &RouteParameters,
-		first_hops: Option<&[&ChannelDetails]>, inflight_htlcs: InFlightHtlcs
+		&self, _payer: &PublicKey, _route_params: &RouteParameters,
+		_first_hops: Option<&[&ChannelDetails]>, _inflight_htlcs: InFlightHtlcs
 	) -> Result<Route, LightningError> {
-		self.router.find_route(payer, route_params, first_hops, inflight_htlcs)
+		self.route.lock().unwrap().take().ok_or_else(|| {
+			LightningError {
+				err: "Can't use this router to return multiple routes".to_owned(),
+				action: ErrorAction::IgnoreError,
+			}
+		})
 	}
 
 	fn find_route_with_id(
 		&self, payer: &PublicKey, route_params: &RouteParameters,
 		first_hops: Option<&[&ChannelDetails]>, inflight_htlcs: InFlightHtlcs,
-		payment_hash: PaymentHash, payment_id: PaymentId
+		_payment_hash: PaymentHash, _payment_id: PaymentId
 	) -> Result<Route, LightningError> {
-		let mut cache = self.route_cache.lock().unwrap();
-		if let Some(route) = cache.remove(&payment_id) {
-			return Ok(route)
-		}
-		core::mem::drop(cache);
-		self.router.find_route_with_id(
-			payer, route_params, first_hops, inflight_htlcs, payment_hash, payment_id
-		)
+		self.find_route(payer, route_params, first_hops, inflight_htlcs)
 	}
 
 	fn create_blinded_payment_paths<
 		T: secp256k1::Signing + secp256k1::Verification
 	> (
-		&self, recipient: PublicKey, first_hops: Vec<ChannelDetails>, tlvs: ReceiveTlvs,
-		amount_msats: u64, secp_ctx: &Secp256k1<T>
+		&self, _recipient: PublicKey, _first_hops: Vec<ChannelDetails>, _tlvs: ReceiveTlvs,
+		_amount_msats: Option<u64>, _secp_ctx: &Secp256k1<T>
 	) -> Result<Vec<BlindedPaymentPath>, ()> {
-		self.router.create_blinded_payment_paths(recipient, first_hops, tlvs, amount_msats, secp_ctx)
+		debug_assert!(false);
+		Err(())
 	}
 }