Skip unnecessary persists in process_pending_htlc_forwards

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

Author: tnull

lightning/src/ln/channelmanager.rs

@@ -6332,7 +6332,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();
@@ -6342,6 +6342,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 {
 				self.process_forward_htlcs(
 					short_chan_id,
@@ -6359,7 +6360,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(),
@@ -6370,6 +6371,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(
@@ -6385,13 +6389,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,12 +1226,14 @@ impl OutboundPayments {
 		)
 	}
 
+	// Returns whether the data changed and needs to be repersisted.
 	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,
-	) where
+	) -> bool
+	where
 		R::Target: Router,
 		ES::Target: EntropySource,
 		NS::Target: NodeSigner,
@@ -1241,6 +1243,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;
@@ -1288,7 +1291,8 @@ impl OutboundPayments {
 					logger,
 					pending_events,
 					&send_payment_along_path,
-				)
+				);
+				should_persist = true;
 			} else {
 				break;
 			}
@@ -1312,10 +1316,12 @@ impl OutboundPayments {
 						None,
 					));
 					retain = false;
+					should_persist = true;
 				}
 			}
 			retain
 		});
+		should_persist
 	}
 
 	pub(super) fn needs_abandon_or_retry(&self) -> bool {