Skip unnecessary persists in process_pending_htlc_forwards

We skip repersisting `ChannelManager` when nothing is actually
processed.

Author: tnull

lightning/src/ln/channelmanager.rs

@@ -6341,7 +6341,7 @@ where
 
 	// Returns whether or not we need to re-persist.
 	fn internal_process_pending_htlc_forwards(&self) -> NotifyOption {
-		let should_persist = NotifyOption::DoPersist;
+		let mut should_persist = NotifyOption::SkipPersistNoEvents;
 		self.process_pending_update_add_htlcs();
 
 		let mut new_events = VecDeque::new();
@@ -6352,6 +6352,7 @@ where
 			mem::swap(&mut forward_htlcs, &mut self.forward_htlcs.lock().unwrap());
 
 			for (short_chan_id, mut pending_forwards) in forward_htlcs {
+				should_persist = NotifyOption::DoPersist;
 				if short_chan_id != 0 {
 					let mut forwarding_counterparty = None;
 					macro_rules! forwarding_channel_not_found {
@@ -7112,7 +7113,7 @@ where
 		}
 
 		let best_block_height = self.best_block.read().unwrap().height;
-		self.pending_outbound_payments.check_retry_payments(
+		let needs_persist = self.pending_outbound_payments.check_retry_payments(
 			&self.router,
 			|| self.list_usable_channels(),
 			|| self.compute_inflight_htlcs(),
@@ -7123,6 +7124,9 @@ where
 			&self.logger,
 			|args| self.send_payment_along_path(args),
 		);
+		if needs_persist {
+			should_persist = NotifyOption::DoPersist;
+		}
 
 		for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) {
 			self.fail_htlc_backwards_internal(
@@ -7138,13 +7142,17 @@ where
 		// next get a `get_and_clear_pending_msg_events` call, but some tests rely on it, and it's
 		// nice to do the work now if we can rather than while we're trying to get messages in the
 		// network stack.
-		self.check_free_holding_cells();
+		if self.check_free_holding_cells() {
+			should_persist = NotifyOption::DoPersist;
+		}
 
 		if new_events.is_empty() {
 			return should_persist;
 		}
 		let mut events = self.pending_events.lock().unwrap();
 		events.append(&mut new_events);
+		should_persist = NotifyOption::DoPersist;
+
 		should_persist
 	}
 

lightning/src/ln/outbound_payment.rs

@@ -1226,13 +1226,14 @@ impl OutboundPayments {
 		)
 	}
 
+	// Returns whether the data changed and needs to be repersisted.
 	#[rustfmt::skip]
 	pub(super) fn check_retry_payments<R: Deref, ES: Deref, NS: Deref, SP, IH, FH, L: Deref>(
 		&self, router: &R, first_hops: FH, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS,
 		best_block_height: u32,
 		pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>, logger: &L,
 		send_payment_along_path: SP,
-	)
+	) -> bool
 	where
 		R::Target: Router,
 		ES::Target: EntropySource,
@@ -1243,6 +1244,7 @@ impl OutboundPayments {
 		L::Target: Logger,
 	{
 		let _single_thread = self.retry_lock.lock().unwrap();
+		let mut should_persist = false;
 		loop {
 			let mut outbounds = self.pending_outbound_payments.lock().unwrap();
 			let mut retry_id_route_params = None;
@@ -1262,7 +1264,8 @@ impl OutboundPayments {
 			}
 			core::mem::drop(outbounds);
 			if let Some((payment_hash, payment_id, route_params)) = retry_id_route_params {
-				self.find_route_and_send_payment(payment_hash, payment_id, route_params, router, first_hops(), &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, &send_payment_along_path)
+				self.find_route_and_send_payment(payment_hash, payment_id, route_params, router, first_hops(), &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, &send_payment_along_path);
+				should_persist = true;
 			} else { break }
 		}
 
@@ -1278,10 +1281,12 @@ impl OutboundPayments {
 						reason: *reason,
 					}, None));
 					retain = false;
+					should_persist = true;
 				}
 			}
 			retain
 		});
+		should_persist
 	}
 
 	#[rustfmt::skip]