From 5fc6282c41862719769df85b6103ff1a94936be7 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 18 Sep 2025 13:19:18 +0200 Subject: [PATCH] Add struct-level logging to OutboundPayments Previously, OutboundPayments methods required borrowed logger arguments since Logger lacked a Clone bound. This prevented logging in all desired places, limiting trace statements for debugging. This change initializes OutboundPayments with a cloned Logger, simplifying method signatures and improving logging flexibility. --- lightning/src/ln/channelmanager.rs | 19 +-- lightning/src/ln/outbound_payment.rs | 233 +++++++++++++-------------- 2 files changed, 121 insertions(+), 131 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3110ed604a5..be3e05dd969 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2680,7 +2680,7 @@ pub struct ChannelManager< /// See `PendingOutboundPayment` documentation for more info. /// /// See `ChannelManager` struct-level documentation for lock order requirements. - pending_outbound_payments: OutboundPayments, + pending_outbound_payments: OutboundPayments, /// SCID/SCID Alias -> forward infos. Key of 0 means payments received. /// @@ -3893,7 +3893,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()), + pending_outbound_payments: OutboundPayments::new(new_hash_map(), logger.clone()), 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() }), @@ -5166,7 +5166,7 @@ where 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.logger, + &self.entropy_source, &self.node_signer, best_block_height, &self.pending_events, |args| self.send_payment_along_path(args)) } @@ -5225,7 +5225,6 @@ where &self.entropy_source, &self.node_signer, best_block_height, - &self.logger, &self.pending_events, |args| self.send_payment_along_path(args), ) @@ -5324,7 +5323,6 @@ where &self.entropy_source, &self.node_signer, best_block_height, - &self.logger, &self.pending_events, |args| self.send_payment_along_path(args), ) @@ -5394,7 +5392,6 @@ where &self, &self.secp_ctx, best_block_height, - &self.logger, &self.pending_events, |args| self.send_payment_along_path(args), ) @@ -5515,7 +5512,6 @@ where &self, &self.secp_ctx, best_block_height, - &self.logger, &self.pending_events, |args| self.send_payment_along_path(args), ); @@ -5615,7 +5611,6 @@ where &self.entropy_source, &self.node_signer, best_block_height, - &self.logger, &self.pending_events, |args| self.send_payment_along_path(args), ) @@ -6772,7 +6767,6 @@ where &self.node_signer, best_block_height, &self.pending_events, - &self.logger, |args| self.send_payment_along_path(args), ); if needs_persist { @@ -8156,7 +8150,6 @@ where self.probing_cookie_secret, &self.secp_ctx, &self.pending_events, - &self.logger, &mut from_monitor_update_completion, ); if let Some(update) = from_monitor_update_completion { @@ -8838,7 +8831,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ from_onchain, &mut ev_completion_action, &self.pending_events, - &self.logger, ); // 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 @@ -16552,7 +16544,8 @@ where } pending_outbound_payments = Some(outbounds); } - let pending_outbounds = OutboundPayments::new(pending_outbound_payments.unwrap()); + let pending_outbounds = + OutboundPayments::new(pending_outbound_payments.unwrap(), args.logger.clone()); for (peer_pubkey, peer_storage) in peer_storage_dir { if let Some(peer_state) = per_peer_state.get_mut(&peer_pubkey) { @@ -16858,7 +16851,6 @@ where session_priv_bytes, &path, best_block_height, - logger, ); } } @@ -16958,7 +16950,6 @@ 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` diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index f51d88f9cfd..465d2ee7eac 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -836,15 +836,22 @@ pub(super) struct SendAlongPathArgs<'a> { pub session_priv_bytes: [u8; 32], } -pub(super) struct OutboundPayments { +pub(super) struct OutboundPayments +where + L::Target: Logger, +{ pub(super) pending_outbound_payments: Mutex>, awaiting_invoice: AtomicBool, retry_lock: Mutex<()>, + logger: L, } -impl OutboundPayments { +impl OutboundPayments +where + L::Target: Logger, +{ pub(super) fn new( - pending_outbound_payments: HashMap, + pending_outbound_payments: HashMap, logger: L, ) -> Self { let has_invoice_requests = pending_outbound_payments.values().any(|payment| { matches!( @@ -859,43 +866,42 @@ impl OutboundPayments { pending_outbound_payments: Mutex::new(pending_outbound_payments), awaiting_invoice: AtomicBool::new(has_invoice_requests), retry_lock: Mutex::new(()), + logger, } } #[rustfmt::skip] - pub(super) fn send_payment( + pub(super) fn send_payment( &self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId, retry_strategy: Retry, route_params: RouteParameters, router: &R, first_hops: Vec, compute_inflight_htlcs: IH, entropy_source: &ES, - node_signer: &NS, best_block_height: u32, logger: &L, + node_signer: &NS, best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: SP, ) -> Result<(), RetryableSendFailure> where R::Target: Router, ES::Target: EntropySource, NS::Target: NodeSigner, - L::Target: Logger, IH: Fn() -> InFlightHtlcs, SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { self.send_payment_for_non_bolt12_invoice(payment_id, payment_hash, recipient_onion, None, retry_strategy, route_params, router, first_hops, &compute_inflight_htlcs, entropy_source, node_signer, - best_block_height, logger, pending_events, &send_payment_along_path) + best_block_height, pending_events, &send_payment_along_path) } #[rustfmt::skip] - pub(super) fn send_spontaneous_payment( + pub(super) fn send_spontaneous_payment( &self, payment_preimage: Option, recipient_onion: RecipientOnionFields, payment_id: PaymentId, retry_strategy: Retry, route_params: RouteParameters, router: &R, first_hops: Vec, inflight_htlcs: IH, entropy_source: &ES, - node_signer: &NS, best_block_height: u32, logger: &L, + node_signer: &NS, best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: SP ) -> Result where R::Target: Router, ES::Target: EntropySource, NS::Target: NodeSigner, - L::Target: Logger, IH: Fn() -> InFlightHtlcs, SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { @@ -904,26 +910,25 @@ impl OutboundPayments { let payment_hash = PaymentHash(Sha256::hash(&preimage.0).to_byte_array()); self.send_payment_for_non_bolt12_invoice(payment_id, payment_hash, recipient_onion, Some(preimage), retry_strategy, route_params, router, first_hops, inflight_htlcs, entropy_source, - node_signer, best_block_height, logger, pending_events, send_payment_along_path) + node_signer, best_block_height, pending_events, send_payment_along_path) .map(|()| payment_hash) } #[rustfmt::skip] - pub(super) fn pay_for_bolt11_invoice( + pub(super) fn pay_for_bolt11_invoice( &self, invoice: &Bolt11Invoice, payment_id: PaymentId, amount_msats: Option, route_params_config: RouteParametersConfig, retry_strategy: Retry, router: &R, first_hops: Vec, compute_inflight_htlcs: IH, entropy_source: &ES, - node_signer: &NS, best_block_height: u32, logger: &L, + node_signer: &NS, best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: SP, ) -> Result<(), Bolt11PaymentError> where R::Target: Router, ES::Target: EntropySource, NS::Target: NodeSigner, - L::Target: Logger, IH: Fn() -> InFlightHtlcs, SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { @@ -950,19 +955,19 @@ impl OutboundPayments { self.send_payment_for_non_bolt12_invoice(payment_id, payment_hash, recipient_onion, None, retry_strategy, route_params, router, first_hops, compute_inflight_htlcs, - entropy_source, node_signer, best_block_height, logger, + entropy_source, node_signer, best_block_height, pending_events, send_payment_along_path ).map_err(|err| Bolt11PaymentError::SendingFailed(err)) } #[rustfmt::skip] pub(super) fn send_payment_for_bolt12_invoice< - R: Deref, ES: Deref, NS: Deref, NL: Deref, IH, SP, L: Deref + R: Deref, ES: Deref, NS: Deref, NL: Deref, IH, SP >( &self, invoice: &Bolt12Invoice, payment_id: PaymentId, router: &R, first_hops: Vec, features: Bolt12InvoiceFeatures, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL, - secp_ctx: &Secp256k1, best_block_height: u32, logger: &L, + secp_ctx: &Secp256k1, best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: SP, ) -> Result<(), Bolt12PaymentError> @@ -971,7 +976,6 @@ impl OutboundPayments { ES::Target: EntropySource, NS::Target: NodeSigner, NL::Target: NodeIdLookUp, - L::Target: Logger, IH: Fn() -> InFlightHtlcs, SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { @@ -997,20 +1001,20 @@ impl OutboundPayments { self.send_payment_for_bolt12_invoice_internal( payment_id, payment_hash, None, None, invoice, route_params, retry_strategy, router, first_hops, inflight_htlcs, entropy_source, node_signer, node_id_lookup, secp_ctx, best_block_height, - logger, pending_events, send_payment_along_path + pending_events, send_payment_along_path ) } #[rustfmt::skip] fn send_payment_for_bolt12_invoice_internal< - R: Deref, ES: Deref, NS: Deref, NL: Deref, IH, SP, L: Deref + R: Deref, ES: Deref, NS: Deref, NL: Deref, IH, SP >( &self, payment_id: PaymentId, payment_hash: PaymentHash, keysend_preimage: Option, invoice_request: Option<&InvoiceRequest>, bolt12_invoice: PaidBolt12Invoice, mut route_params: RouteParameters, retry_strategy: Retry, router: &R, first_hops: Vec, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, - node_id_lookup: &NL, secp_ctx: &Secp256k1, best_block_height: u32, logger: &L, + node_id_lookup: &NL, secp_ctx: &Secp256k1, best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: SP, ) -> Result<(), Bolt12PaymentError> @@ -1019,7 +1023,6 @@ impl OutboundPayments { ES::Target: EntropySource, NS::Target: NodeSigner, NL::Target: NodeIdLookUp, - L::Target: Logger, IH: Fn() -> InFlightHtlcs, SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { @@ -1049,7 +1052,6 @@ impl OutboundPayments { let route = match self.find_initial_route( payment_id, payment_hash, &recipient_onion, keysend_preimage, invoice_request, &mut route_params, router, &first_hops, &inflight_htlcs, node_signer, best_block_height, - logger, ) { Ok(route) => route, Err(e) => { @@ -1099,13 +1101,13 @@ impl OutboundPayments { &send_payment_along_path ); log_info!( - logger, "Sending payment with id {} and hash {} returned {:?}", payment_id, + self.logger, "Sending payment with id {} and hash {} returned {:?}", payment_id, payment_hash, result ); if let Err(e) = result { self.handle_pay_route_err( e, payment_id, payment_hash, route, route_params, onion_session_privs, router, first_hops, - &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, + &inflight_htlcs, entropy_source, node_signer, best_block_height, pending_events, &send_payment_along_path ); } @@ -1228,11 +1230,10 @@ impl OutboundPayments { NL: Deref, IH, SP, - L: Deref, >( &self, payment_id: PaymentId, router: &R, first_hops: Vec, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL, - secp_ctx: &Secp256k1, best_block_height: u32, logger: &L, + secp_ctx: &Secp256k1, best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: SP, ) -> Result<(), Bolt12PaymentError> @@ -1241,7 +1242,6 @@ impl OutboundPayments { ES::Target: EntropySource, NS::Target: NodeSigner, NL::Target: NodeIdLookUp, - L::Target: Logger, IH: Fn() -> InFlightHtlcs, SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { @@ -1291,18 +1291,17 @@ impl OutboundPayments { node_id_lookup, secp_ctx, best_block_height, - logger, pending_events, send_payment_along_path, ) } // Returns whether the data changed and needs to be repersisted. - pub(super) fn check_retry_payments( + pub(super) fn check_retry_payments( &self, router: &R, first_hops: FH, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, best_block_height: u32, pending_events: &Mutex)>>, - logger: &L, send_payment_along_path: SP, + send_payment_along_path: SP, ) -> bool where R::Target: Router, @@ -1311,7 +1310,6 @@ impl OutboundPayments { SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, IH: Fn() -> InFlightHtlcs, FH: Fn() -> Vec, - L::Target: Logger, { let _single_thread = self.retry_lock.lock().unwrap(); let mut should_persist = false; @@ -1359,7 +1357,6 @@ impl OutboundPayments { entropy_source, node_signer, best_block_height, - logger, pending_events, &send_payment_along_path, ); @@ -1407,11 +1404,11 @@ impl OutboundPayments { } #[rustfmt::skip] - fn find_initial_route( + fn find_initial_route( &self, payment_id: PaymentId, payment_hash: PaymentHash, recipient_onion: &RecipientOnionFields, keysend_preimage: Option, invoice_request: Option<&InvoiceRequest>, route_params: &mut RouteParameters, router: &R, first_hops: &Vec, - inflight_htlcs: &IH, node_signer: &NS, best_block_height: u32, logger: &L, + inflight_htlcs: &IH, node_signer: &NS, best_block_height: u32, ) -> Result where R::Target: Router, @@ -1421,7 +1418,7 @@ impl OutboundPayments { { #[cfg(feature = "std")] { if has_expired(&route_params) { - log_error!(logger, "Payment with id {} and hash {} had expired before we started paying", + log_error!(self.logger, "Payment with id {} and hash {} had expired before we started paying", payment_id, payment_hash); return Err(RetryableSendFailure::PaymentExpired) } @@ -1431,7 +1428,7 @@ impl OutboundPayments { route_params, recipient_onion, keysend_preimage, invoice_request, best_block_height ) .map_err(|()| { - log_error!(logger, "Can't construct an onion packet without exceeding 1300-byte onion \ + log_error!(self.logger, "Can't construct an onion packet without exceeding 1300-byte onion \ hop_data length for payment with id {} and hash {}", payment_id, payment_hash); RetryableSendFailure::OnionPacketSizeExceeded })?; @@ -1441,7 +1438,7 @@ impl OutboundPayments { Some(&first_hops.iter().collect::>()), inflight_htlcs(), payment_hash, payment_id, ).map_err(|_| { - log_error!(logger, "Failed to find route for payment with id {} and hash {}", + log_error!(self.logger, "Failed to find route for payment with id {} and hash {}", payment_id, payment_hash); RetryableSendFailure::RouteNotFound })?; @@ -1462,11 +1459,11 @@ impl OutboundPayments { /// [`Event::PaymentPathFailed`]: crate::events::Event::PaymentPathFailed /// [`Event::PaymentFailed`]: crate::events::Event::PaymentFailed #[rustfmt::skip] - fn send_payment_for_non_bolt12_invoice( + fn send_payment_for_non_bolt12_invoice( &self, payment_id: PaymentId, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, keysend_preimage: Option, retry_strategy: Retry, mut route_params: RouteParameters, router: &R, first_hops: Vec, inflight_htlcs: IH, entropy_source: &ES, - node_signer: &NS, best_block_height: u32, logger: &L, + node_signer: &NS, best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: SP, ) -> Result<(), RetryableSendFailure> where @@ -1479,14 +1476,14 @@ impl OutboundPayments { { let route = self.find_initial_route( payment_id, payment_hash, &recipient_onion, keysend_preimage, None, &mut route_params, router, - &first_hops, &inflight_htlcs, node_signer, best_block_height, logger, + &first_hops, &inflight_htlcs, node_signer, best_block_height, )?; let onion_session_privs = self.add_new_pending_payment(payment_hash, recipient_onion.clone(), payment_id, keysend_preimage, &route, Some(retry_strategy), Some(route_params.payment_params.clone()), entropy_source, best_block_height, None) .map_err(|_| { - log_error!(logger, "Payment with id {} is already pending. New payment had payment hash {}", + log_error!(self.logger, "Payment with id {} is already pending. New payment had payment hash {}", payment_id, payment_hash); RetryableSendFailure::DuplicatePayment })?; @@ -1494,12 +1491,12 @@ impl OutboundPayments { let res = self.pay_route_internal(&route, payment_hash, &recipient_onion, keysend_preimage, None, None, payment_id, None, &onion_session_privs, node_signer, best_block_height, &send_payment_along_path); - log_info!(logger, "Sending payment with id {} and hash {} returned {:?}", + log_info!(self.logger, "Sending payment with id {} and hash {} returned {:?}", payment_id, payment_hash, res); if let Err(e) = res { self.handle_pay_route_err( e, payment_id, payment_hash, route, route_params, onion_session_privs, router, first_hops, - &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, + &inflight_htlcs, entropy_source, node_signer, best_block_height, pending_events, &send_payment_along_path ); } @@ -1507,10 +1504,10 @@ impl OutboundPayments { } #[rustfmt::skip] - fn find_route_and_send_payment( + fn find_route_and_send_payment( &self, payment_hash: PaymentHash, payment_id: PaymentId, route_params: RouteParameters, router: &R, first_hops: Vec, inflight_htlcs: &IH, entropy_source: &ES, - node_signer: &NS, best_block_height: u32, logger: &L, + node_signer: &NS, best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: &SP, ) where @@ -1523,7 +1520,7 @@ impl OutboundPayments { { #[cfg(feature = "std")] { if has_expired(&route_params) { - log_error!(logger, "Payment params expired on retry, abandoning payment {}", &payment_id); + log_error!(self.logger, "Payment params expired on retry, abandoning payment {}", &payment_id); self.abandon_payment(payment_id, PaymentFailureReason::PaymentExpired, pending_events); return } @@ -1536,7 +1533,7 @@ impl OutboundPayments { ) { Ok(route) => route, Err(e) => { - log_error!(logger, "Failed to find a route on retry, abandoning payment {}: {:#?}", &payment_id, e); + log_error!(self.logger, "Failed to find a route on retry, abandoning payment {}: {:#?}", &payment_id, e); self.abandon_payment(payment_id, PaymentFailureReason::RouteNotFound, pending_events); return } @@ -1550,7 +1547,7 @@ impl OutboundPayments { for path in route.paths.iter() { if path.hops.len() == 0 { - log_error!(logger, "Unusable path in route (path.hops.len() must be at least 1"); + log_error!(self.logger, "Unusable path in route (path.hops.len() must be at least 1"); self.abandon_payment(payment_id, PaymentFailureReason::UnexpectedError, pending_events); return } @@ -1583,13 +1580,13 @@ impl OutboundPayments { const RETRY_OVERFLOW_PERCENTAGE: u64 = 10; let retry_amt_msat = route.get_total_amount(); if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 { - log_error!(logger, "retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat); + log_error!(self.logger, "retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat); abandon_with_entry!(payment, PaymentFailureReason::UnexpectedError); return } if !payment.get().is_retryable_now() { - log_error!(logger, "Retries exhausted for payment id {}", &payment_id); + log_error!(self.logger, "Retries exhausted for payment id {}", &payment_id); abandon_with_entry!(payment, PaymentFailureReason::RetriesExhausted); return } @@ -1618,38 +1615,38 @@ impl OutboundPayments { (total_msat, recipient_onion, keysend_preimage, onion_session_privs, invoice_request, bolt12_invoice.cloned()) }, PendingOutboundPayment::Legacy { .. } => { - log_error!(logger, "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102"); + log_error!(self.logger, "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102"); return }, PendingOutboundPayment::AwaitingInvoice { .. } | PendingOutboundPayment::AwaitingOffer { .. } => { - log_error!(logger, "Payment not yet sent"); + log_error!(self.logger, "Payment not yet sent"); debug_assert!(false); return }, PendingOutboundPayment::InvoiceReceived { .. } => { - log_error!(logger, "Payment already initiating"); + log_error!(self.logger, "Payment already initiating"); debug_assert!(false); return }, PendingOutboundPayment::StaticInvoiceReceived { .. } => { - log_error!(logger, "Payment already initiating"); + log_error!(self.logger, "Payment already initiating"); debug_assert!(false); return }, PendingOutboundPayment::Fulfilled { .. } => { - log_error!(logger, "Payment already completed"); + log_error!(self.logger, "Payment already completed"); return }, PendingOutboundPayment::Abandoned { .. } => { - log_error!(logger, "Payment already abandoned (with some HTLCs still pending)"); + log_error!(self.logger, "Payment already abandoned (with some HTLCs still pending)"); return }, } }, hash_map::Entry::Vacant(_) => { - log_error!(logger, "Payment with ID {} not found", &payment_id); + log_error!(self.logger, "Payment with ID {} not found", &payment_id); return } } @@ -1657,22 +1654,22 @@ impl OutboundPayments { let res = self.pay_route_internal(&route, payment_hash, &recipient_onion, keysend_preimage, invoice_request.as_ref(), bolt12_invoice.as_ref(), payment_id, Some(total_msat), &onion_session_privs, node_signer, best_block_height, &send_payment_along_path); - log_info!(logger, "Result retrying payment id {}: {:?}", &payment_id, res); + log_info!(self.logger, "Result retrying payment id {}: {:?}", &payment_id, res); if let Err(e) = res { self.handle_pay_route_err( e, payment_id, payment_hash, route, route_params, onion_session_privs, router, first_hops, - inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, + inflight_htlcs, entropy_source, node_signer, best_block_height, pending_events, send_payment_along_path ); } } #[rustfmt::skip] - fn handle_pay_route_err( + fn handle_pay_route_err( &self, err: PaymentSendFailure, payment_id: PaymentId, payment_hash: PaymentHash, route: Route, mut route_params: RouteParameters, onion_session_privs: Vec<[u8; 32]>, router: &R, first_hops: Vec, inflight_htlcs: &IH, entropy_source: &ES, node_signer: &NS, - best_block_height: u32, logger: &L, + best_block_height: u32, pending_events: &Mutex)>>, send_payment_along_path: &SP, ) @@ -1680,15 +1677,14 @@ impl OutboundPayments { R::Target: Router, ES::Target: EntropySource, NS::Target: NodeSigner, - L::Target: Logger, IH: Fn() -> InFlightHtlcs, SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { match err { PaymentSendFailure::AllFailedResendSafe(errs) => { self.remove_session_privs(payment_id, route.paths.iter().zip(onion_session_privs.iter())); - Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut route_params, route.paths, errs.into_iter().map(|e| Err(e)), logger, pending_events); - 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::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut route_params, route.paths, errs.into_iter().map(|e| Err(e)), &self.logger, pending_events); + self.find_route_and_send_payment(payment_hash, payment_id, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, pending_events, send_payment_along_path); }, PaymentSendFailure::PartialFailure { failed_paths_retry: Some(mut retry), results, .. } => { debug_assert_eq!(results.len(), route.paths.len()); @@ -1704,11 +1700,11 @@ impl OutboundPayments { } }); self.remove_session_privs(payment_id, failed_paths); - Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut retry, route.paths, results.into_iter(), logger, pending_events); + Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut retry, route.paths, results.into_iter(), &self.logger, pending_events); // Some paths were sent, even if we failed to send the full MPP value our recipient may // misbehave and claim the funds, at which point we have to consider the payment sent, so // return `Ok()` here, ignoring any retry errors. - self.find_route_and_send_payment(payment_hash, payment_id, retry, 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, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, pending_events, send_payment_along_path); }, PaymentSendFailure::PartialFailure { failed_paths_retry: None, .. } => { // This may happen if we send a payment and some paths fail, but only due to a temporary @@ -1716,13 +1712,13 @@ impl OutboundPayments { // initial HTLC-Add messages yet. }, PaymentSendFailure::PathParameterError(results) => { - log_error!(logger, "Failed to send to route due to parameter error in a single path. Your router is buggy"); + log_error!(self.logger, "Failed to send to route due to parameter error in a single path. Your router is buggy"); self.remove_session_privs(payment_id, route.paths.iter().zip(onion_session_privs.iter())); - Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut route_params, route.paths, results.into_iter(), logger, pending_events); + Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut route_params, route.paths, results.into_iter(), &self.logger, pending_events); self.abandon_payment(payment_id, PaymentFailureReason::UnexpectedError, pending_events); }, PaymentSendFailure::ParameterError(e) => { - log_error!(logger, "Failed to send to route due to parameter error: {:?}. Your router is buggy", e); + log_error!(self.logger, "Failed to send to route due to parameter error: {:?}. Your router is buggy", e); self.remove_session_privs(payment_id, route.paths.iter().zip(onion_session_privs.iter())); self.abandon_payment(payment_id, PaymentFailureReason::UnexpectedError, pending_events); }, @@ -1732,14 +1728,11 @@ impl OutboundPayments { fn push_path_failed_evs_and_scids< I: ExactSizeIterator + Iterator>, - L: Deref, >( payment_id: PaymentId, payment_hash: PaymentHash, route_params: &mut RouteParameters, paths: Vec, path_results: I, logger: &L, pending_events: &Mutex)>>, - ) where - L::Target: Logger, - { + ) { let mut events = pending_events.lock().unwrap(); debug_assert_eq!(paths.len(), path_results.len()); for (path, path_res) in paths.into_iter().zip(path_results) { @@ -2213,12 +2206,11 @@ impl OutboundPayments { } #[rustfmt::skip] - pub(super) fn claim_htlc( + pub(super) fn claim_htlc( &self, payment_id: PaymentId, payment_preimage: PaymentPreimage, bolt12_invoice: Option, session_priv: SecretKey, path: Path, from_onchain: bool, ev_completion_action: &mut Option, pending_events: &Mutex)>>, - logger: &L, - ) where L::Target: Logger { + ) { let mut session_priv_bytes = [0; 32]; session_priv_bytes.copy_from_slice(&session_priv[..]); let mut outbounds = self.pending_outbound_payments.lock().unwrap(); @@ -2226,7 +2218,7 @@ impl OutboundPayments { if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) { if !payment.get().is_fulfilled() { let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).to_byte_array()); - log_info!(logger, "Payment with id {} and hash {} sent!", payment_id, payment_hash); + log_info!(self.logger, "Payment with id {} and hash {} sent!", payment_id, payment_hash); let fee_paid_msat = payment.get().get_pending_fee_msat(); let amount_msat = payment.get().total_msat(); pending_events.push_back((events::Event::PaymentSent { @@ -2256,7 +2248,7 @@ impl OutboundPayments { } } } else { - log_trace!(logger, "Received duplicative fulfill for HTLC with payment_preimage {}", &payment_preimage); + log_trace!(self.logger, "Received duplicative fulfill for HTLC with payment_preimage {}", &payment_preimage); } } @@ -2376,15 +2368,13 @@ impl OutboundPayments { }); } - pub(super) fn fail_htlc( + pub(super) fn fail_htlc( &self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, path: &Path, session_priv: &SecretKey, payment_id: &PaymentId, probing_cookie_secret: [u8; 32], secp_ctx: &Secp256k1, pending_events: &Mutex)>>, - logger: &L, completion_action: &mut Option, - ) where - L::Target: Logger, - { + completion_action: &mut Option, + ) { #[cfg(any(test, feature = "_test_utils"))] let DecodedOnionFailure { network_update, @@ -2395,7 +2385,7 @@ impl OutboundPayments { failed_within_blinded_path, hold_times, .. - } = onion_error.decode_onion_failure(secp_ctx, logger, &source); + } = onion_error.decode_onion_failure(secp_ctx, &self.logger, &source); #[cfg(not(any(test, feature = "_test_utils")))] let DecodedOnionFailure { network_update, @@ -2404,7 +2394,7 @@ impl OutboundPayments { failed_within_blinded_path, hold_times, .. - } = onion_error.decode_onion_failure(secp_ctx, logger, &source); + } = onion_error.decode_onion_failure(secp_ctx, &self.logger, &source); let payment_is_probe = payment_is_probe(payment_hash, &payment_id, probing_cookie_secret); let mut session_priv_bytes = [0; 32]; @@ -2429,7 +2419,7 @@ impl OutboundPayments { if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) { if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) { log_trace!( - logger, + self.logger, "Received duplicative fail for HTLC with payment_hash {}", &payment_hash ); @@ -2437,7 +2427,7 @@ impl OutboundPayments { } if payment.get().is_fulfilled() { log_trace!( - logger, + self.logger, "Received failure of HTLC with payment_hash {} after payment completion", &payment_hash ); @@ -2485,14 +2475,18 @@ impl OutboundPayments { is_retryable_now } else { log_trace!( - logger, + self.logger, "Received duplicative fail for HTLC with payment_hash {}", &payment_hash ); return; }; core::mem::drop(outbounds); - log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", &payment_hash); + log_trace!( + self.logger, + "Failing outbound payment HTLC with payment_hash {}", + &payment_hash + ); let path_failure = { if payment_is_probe { @@ -2614,9 +2608,9 @@ impl OutboundPayments { invoice_requests } - pub(super) fn insert_from_monitor_on_startup( + pub(super) fn insert_from_monitor_on_startup( &self, payment_id: PaymentId, payment_hash: PaymentHash, session_priv_bytes: [u8; 32], - path: &Path, best_block_height: u32, logger: L, + path: &Path, best_block_height: u32, ) { let path_amt = path.final_value_msat(); let path_fee = path.fee_msat(); @@ -2666,12 +2660,12 @@ impl OutboundPayments { entry.get_mut().insert(session_priv_bytes, &path) }, }; - log_info!(logger, "{} a pending payment path for {} msat for session priv {} on an existing pending payment with payment hash {}", + log_info!(self.logger, "{} a pending payment path for {} msat for session priv {} on an existing pending payment with payment hash {}", if newly_added { "Added" } else { "Had" }, path_amt, log_bytes!(session_priv_bytes), payment_hash); }, hash_map::Entry::Vacant(entry) => { entry.insert(new_retryable!()); - log_info!(logger, "Added a pending payment for {} msat with payment hash {} for path with session priv {}", + log_info!(self.logger, "Added a pending payment for {} msat with payment hash {} for path with session priv {}", path_amt, payment_hash, log_bytes!(session_priv_bytes)); }, } @@ -2866,8 +2860,8 @@ mod tests { #[cfg(feature = "std")] #[rustfmt::skip] fn do_fails_paying_after_expiration(on_retry: bool) { - let outbound_payments = OutboundPayments::new(new_hash_map()); let logger = test_utils::TestLogger::new(); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); let scorer = RwLock::new(test_utils::TestScorer::new()); let router = test_utils::TestRouter::new(network_graph, &logger, &scorer); @@ -2888,7 +2882,7 @@ mod tests { &&keys_manager, 0, None).unwrap(); outbound_payments.find_route_and_send_payment( PaymentHash([0; 32]), PaymentId([0; 32]), expired_route_params, &&router, vec![], - &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, + &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &pending_events, &|_| Ok(())); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 1); @@ -2899,7 +2893,7 @@ mod tests { let err = outbound_payments.send_payment( PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), Retry::Attempts(0), expired_route_params, &&router, vec![], || InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, |_| Ok(())).unwrap_err(); + &&keys_manager, &&keys_manager, 0, &pending_events, |_| Ok(())).unwrap_err(); if let RetryableSendFailure::PaymentExpired = err { } else { panic!("Unexpected error"); } } } @@ -2911,8 +2905,8 @@ mod tests { } #[rustfmt::skip] fn do_find_route_error(on_retry: bool) { - let outbound_payments = OutboundPayments::new(new_hash_map()); let logger = test_utils::TestLogger::new(); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); let scorer = RwLock::new(test_utils::TestScorer::new()); let router = test_utils::TestRouter::new(network_graph, &logger, &scorer); @@ -2932,7 +2926,7 @@ mod tests { &&keys_manager, 0, None).unwrap(); outbound_payments.find_route_and_send_payment( PaymentHash([0; 32]), PaymentId([0; 32]), route_params, &&router, vec![], - &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, + &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &pending_events, &|_| Ok(())); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 1); @@ -2941,7 +2935,7 @@ mod tests { let err = outbound_payments.send_payment( PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), Retry::Attempts(0), route_params, &&router, vec![], || InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, |_| Ok(())).unwrap_err(); + &&keys_manager, &&keys_manager, 0, &pending_events, |_| Ok(())).unwrap_err(); if let RetryableSendFailure::RouteNotFound = err { } else { panic!("Unexpected error"); } } @@ -2950,8 +2944,8 @@ mod tests { #[test] #[rustfmt::skip] fn initial_send_payment_path_failed_evs() { - let outbound_payments = OutboundPayments::new(new_hash_map()); let logger = test_utils::TestLogger::new(); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); let scorer = RwLock::new(test_utils::TestScorer::new()); let router = test_utils::TestRouter::new(network_graph, &logger, &scorer); @@ -2990,7 +2984,7 @@ mod tests { outbound_payments.send_payment( PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, + &&keys_manager, &&keys_manager, 0, &pending_events, |_| Err(APIError::ChannelUnavailable { err: "test".to_owned() })).unwrap(); let mut events = pending_events.lock().unwrap(); assert_eq!(events.len(), 2); @@ -3008,7 +3002,7 @@ mod tests { outbound_payments.send_payment( PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, + &&keys_manager, &&keys_manager, 0, &pending_events, |_| Err(APIError::MonitorUpdateInProgress)).unwrap(); assert_eq!(pending_events.lock().unwrap().len(), 0); @@ -3016,7 +3010,7 @@ mod tests { outbound_payments.send_payment( PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([1; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, + &&keys_manager, &&keys_manager, 0, &pending_events, |_| Err(APIError::APIMisuseError { err: "test".to_owned() })).unwrap(); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 2); @@ -3033,7 +3027,8 @@ mod tests { #[rustfmt::skip] fn removes_stale_awaiting_invoice_using_absolute_timeout() { let pending_events = Mutex::new(VecDeque::new()); - let outbound_payments = OutboundPayments::new(new_hash_map()); + let logger = test_utils::TestLogger::new(); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let payment_id = PaymentId([0; 32]); let absolute_expiry = 100; let tick_interval = 10; @@ -3088,7 +3083,8 @@ mod tests { #[rustfmt::skip] fn removes_stale_awaiting_invoice_using_timer_ticks() { let pending_events = Mutex::new(VecDeque::new()); - let outbound_payments = OutboundPayments::new(new_hash_map()); + let logger = test_utils::TestLogger::new(); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let payment_id = PaymentId([0; 32]); let timer_ticks = 3; let expiration = StaleExpiration::TimerTicks(timer_ticks); @@ -3142,7 +3138,8 @@ mod tests { #[rustfmt::skip] fn removes_abandoned_awaiting_invoice() { let pending_events = Mutex::new(VecDeque::new()); - let outbound_payments = OutboundPayments::new(new_hash_map()); + let logger = test_utils::TestLogger::new(); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let payment_id = PaymentId([0; 32]); let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100)); @@ -3182,7 +3179,7 @@ mod tests { let nonce = Nonce([0; 16]); let pending_events = Mutex::new(VecDeque::new()); - let outbound_payments = OutboundPayments::new(new_hash_map()); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let payment_id = PaymentId([0; 32]); let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100)); @@ -3207,7 +3204,7 @@ mod tests { outbound_payments.send_payment_for_bolt12_invoice( &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, - &secp_ctx, 0, &&logger, &pending_events, |_| panic!() + &secp_ctx, 0, &pending_events, |_| panic!() ), Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::PaymentExpired)), ); @@ -3235,7 +3232,7 @@ mod tests { let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); let pending_events = Mutex::new(VecDeque::new()); - let outbound_payments = OutboundPayments::new(new_hash_map()); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let expanded_key = ExpandedKey::new([42; 32]); let nonce = Nonce([0; 16]); let payment_id = PaymentId([0; 32]); @@ -3270,7 +3267,7 @@ mod tests { outbound_payments.send_payment_for_bolt12_invoice( &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, - &secp_ctx, 0, &&logger, &pending_events, |_| panic!() + &secp_ctx, 0, &pending_events, |_| panic!() ), Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::RouteNotFound)), ); @@ -3298,7 +3295,7 @@ mod tests { let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); let pending_events = Mutex::new(VecDeque::new()); - let outbound_payments = OutboundPayments::new(new_hash_map()); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let expanded_key = ExpandedKey::new([42; 32]); let nonce = Nonce([0; 16]); let payment_id = PaymentId([0; 32]); @@ -3346,7 +3343,7 @@ mod tests { outbound_payments.send_payment_for_bolt12_invoice( &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, - &secp_ctx, 0, &&logger, &pending_events, |_| panic!() + &secp_ctx, 0, &pending_events, |_| panic!() ), Err(Bolt12PaymentError::UnexpectedInvoice), ); @@ -3366,7 +3363,7 @@ mod tests { outbound_payments.send_payment_for_bolt12_invoice( &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, - &secp_ctx, 0, &&logger, &pending_events, |_| Ok(()) + &secp_ctx, 0, &pending_events, |_| Ok(()) ), Ok(()), ); @@ -3377,7 +3374,7 @@ mod tests { outbound_payments.send_payment_for_bolt12_invoice( &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, - &secp_ctx, 0, &&logger, &pending_events, |_| panic!() + &secp_ctx, 0, &pending_events, |_| panic!() ), Err(Bolt12PaymentError::DuplicateInvoice), ); @@ -3406,7 +3403,8 @@ mod tests { #[rustfmt::skip] fn time_out_unreleased_async_payments() { let pending_events = Mutex::new(VecDeque::new()); - let outbound_payments = OutboundPayments::new(new_hash_map()); + let logger = test_utils::TestLogger::new(); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let payment_id = PaymentId([0; 32]); let absolute_expiry = 60; @@ -3456,7 +3454,8 @@ mod tests { #[rustfmt::skip] fn abandon_unreleased_async_payment() { let pending_events = Mutex::new(VecDeque::new()); - let outbound_payments = OutboundPayments::new(new_hash_map()); + let logger = test_utils::TestLogger::new(); + let outbound_payments = OutboundPayments::new(new_hash_map(), &logger); let payment_id = PaymentId([0; 32]); let absolute_expiry = 60;