Require WithContext log wrappers on OutboundPayments calls

In much of LDK we pass around `Logger` objects both to avoid having
to `Clone` `Logger` `Deref`s (soon to only be `Logger`s) and to
allow us to set context with a wrapper such that any log calls on
that wrapper get additional useful metadata in them.

Sadly, when we added a `Logger` type to `OutboundPayments` we broke
the ability to do the second thing - payment information logged
directly or indirectly via logic in the `OutboundPayments` has no
context making log-searching rather challenging.

Here we fix this by retunring to passing loggers explicitly to
`OutboundPayments` methods that need them, specifically requiring
`WithContext` wrappers to ensure the callsite sets appropriate
context on the logger.

Fixes #4307

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -2672,7 +2672,7 @@ pub struct ChannelManager<
 	/// after reloading from disk while replaying blocks against ChannelMonitors.
 	///
 	/// See `PendingOutboundPayment` documentation for more info.
-	pending_outbound_payments: OutboundPayments<L>,
+	pending_outbound_payments: OutboundPayments,
 
 	/// SCID/SCID Alias -> forward infos. Key of 0 means payments received.
 	///
@@ -3485,7 +3485,7 @@ where
 			best_block: RwLock::new(params.best_block),
 
 			outbound_scid_aliases: Mutex::new(new_hash_set()),
-			pending_outbound_payments: OutboundPayments::new(new_hash_map(), logger.clone()),
+			pending_outbound_payments: OutboundPayments::new(new_hash_map()),
 			forward_htlcs: Mutex::new(new_hash_map()),
 			decode_update_add_htlcs: Mutex::new(new_hash_map()),
 			claimable_payments: Mutex::new(ClaimablePayments { claimable_payments: new_hash_map(), pending_claiming_payments: new_hash_map() }),
@@ -5354,11 +5354,12 @@ where
 		});
 		if route.route_params.is_none() { route.route_params = Some(route_params.clone()); }
 		let router = FixedRouter::new(route);
+		let logger = WithContext::from(&self.logger, None, None, Some(payment_hash));
 		self.pending_outbound_payments
 			.send_payment(payment_hash, recipient_onion, payment_id, Retry::Attempts(0),
 				route_params, &&router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
 				&self.entropy_source, &self.node_signer, best_block_height,
-				&self.pending_events, |args| self.send_payment_along_path(args))
+				&self.pending_events, |args| self.send_payment_along_path(args), &logger)
 	}
 
 	/// Sends a payment to the route found using the provided [`RouteParameters`], retrying failed
@@ -5418,6 +5419,7 @@ where
 			best_block_height,
 			&self.pending_events,
 			|args| self.send_payment_along_path(args),
+			&WithContext::from(&self.logger, None, None, Some(payment_hash)),
 		)
 	}
 
@@ -5516,6 +5518,7 @@ where
 			best_block_height,
 			&self.pending_events,
 			|args| self.send_payment_along_path(args),
+			&WithContext::from(&self.logger, None, None, Some(invoice.payment_hash())),
 		)
 	}
 
@@ -5568,6 +5571,7 @@ where
 			best_block_height,
 			&self.pending_events,
 			|args| self.send_payment_along_path(args),
+			&WithContext::from(&self.logger, None, None, None),
 		)
 	}
 
@@ -5748,6 +5752,7 @@ where
 			best_block_height,
 			&self.pending_events,
 			|args| self.send_payment_along_path(args),
+			&WithContext::from(&self.logger, None, None, None),
 		)
 	}
 
@@ -5826,6 +5831,7 @@ where
 	) -> Result<PaymentHash, RetryableSendFailure> {
 		let best_block_height = self.best_block.read().unwrap().height;
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
+		let payment_hash = payment_preimage.map(|preimage| preimage.into());
 		self.pending_outbound_payments.send_spontaneous_payment(
 			payment_preimage,
 			recipient_onion,
@@ -5840,6 +5846,7 @@ where
 			best_block_height,
 			&self.pending_events,
 			|args| self.send_payment_along_path(args),
+			&WithContext::from(&self.logger, None, None, payment_hash),
 		)
 	}
 
@@ -7252,6 +7259,7 @@ where
 			best_block_height,
 			&self.pending_events,
 			|args| self.send_payment_along_path(args),
+			&WithContext::from(&self.logger, None, None, None),
 		);
 		if needs_persist {
 			should_persist = NotifyOption::DoPersist;
@@ -8644,6 +8652,7 @@ where
 		// being fully configured. See the docs for `ChannelManagerReadArgs` for more.
 		match source {
 			HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, .. } => {
+				let logger = WithContext::from(&self.logger, None, None, Some(*payment_hash));
 				self.pending_outbound_payments.fail_htlc(
 					source,
 					payment_hash,
@@ -8655,6 +8664,7 @@ where
 					&self.secp_ctx,
 					&self.pending_events,
 					&mut from_monitor_update_completion,
+					&logger,
 				);
 				if let Some(update) = from_monitor_update_completion {
 					// If `fail_htlc` didn't `take` the post-event action, we should go ahead and
@@ -9345,6 +9355,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 					from_onchain,
 					&mut ev_completion_action,
 					&self.pending_events,
+					&WithContext::from(&self.logger, None, None, Some(payment_preimage.into())),
 				);
 				// If an event was generated, `claim_htlc` set `ev_completion_action` to None, if
 				// not, we should go ahead and run it now (as the claim was duplicative), at least
@@ -18064,8 +18075,7 @@ where
 			}
 			pending_outbound_payments = Some(outbounds);
 		}
-		let pending_outbounds =
-			OutboundPayments::new(pending_outbound_payments.unwrap(), args.logger.clone());
+		let pending_outbounds = OutboundPayments::new(pending_outbound_payments.unwrap());
 
 		for (peer_pubkey, peer_storage) in peer_storage_dir {
 			if let Some(peer_state) = per_peer_state.get_mut(&peer_pubkey) {
@@ -18418,6 +18428,7 @@ where
 								session_priv_bytes,
 								&path,
 								best_block_height,
+								&logger,
 							);
 						}
 					}
@@ -18452,7 +18463,7 @@ where
 									&mut decode_update_add_htlcs,
 									&prev_hop_data,
 									"HTLC already forwarded to the outbound edge",
-									&args.logger,
+									&&logger,
 								);
 							}
 
@@ -18469,7 +18480,7 @@ where
 								&mut decode_update_add_htlcs_legacy,
 								&prev_hop_data,
 								"HTLC was forwarded to the closed channel",
-								&args.logger,
+								&&logger,
 							);
 							forward_htlcs_legacy.retain(|_, forwards| {
 								forwards.retain(|forward| {
@@ -18526,6 +18537,7 @@ where
 									true,
 									&mut compl_action,
 									&pending_events,
+									&logger,
 								);
 								// If the completion action was not consumed, then there was no
 								// payment to claim, and we need to tell the `ChannelMonitor`
@@ -18579,8 +18591,10 @@ where
 					}
 				}
 				for (htlc_source, payment_hash) in monitor.get_onchain_failed_outbound_htlcs() {
+					let logger =
+						WithChannelMonitor::from(&args.logger, monitor, Some(payment_hash));
 					log_info!(
-						args.logger,
+						logger,
 						"Failing HTLC with payment hash {} as it was resolved on-chain.",
 						payment_hash
 					);
@@ -18648,6 +18662,11 @@ where
 									// inbound edge of the payment's monitor has already claimed
 									// the HTLC) we skip trying to replay the claim.
 									let htlc_payment_hash: PaymentHash = payment_preimage.into();
+									let logger = WithChannelMonitor::from(
+										&args.logger,
+										monitor,
+										Some(htlc_payment_hash),
+									);
 									let balance_could_incl_htlc = |bal| match bal {
 										&Balance::ClaimableOnChannelClose { .. } => {
 											// The channel is still open, assume we can still
@@ -18670,7 +18689,7 @@ where
 									// edge monitor but the channel is closed (and thus we'll
 									// immediately panic if we call claim_funds_from_hop).
 									if short_to_chan_info.get(&prev_hop.prev_outbound_scid_alias).is_none() {
-										log_error!(args.logger,
+										log_error!(logger,
 											"We need to replay the HTLC claim for payment_hash {} (preimage {}) but cannot do so as the HTLC was forwarded prior to LDK 0.0.124.\
 											All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1",
 											htlc_payment_hash,
@@ -18685,7 +18704,7 @@ where
 									// of panicking at runtime. The user ideally should have read
 									// the release notes and we wouldn't be here, but we go ahead
 									// and let things run in the hope that it'll all just work out.
-									log_error!(args.logger,
+									log_error!(logger,
 										"We need to replay the HTLC claim for payment_hash {} (preimage {}) but don't have all the required information to do so reliably.\
 										As long as the channel for the inbound edge of the forward remains open, this may work okay, but we may panic at runtime!\
 										All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1\