diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 00a536539ea..91a1885381c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -12477,6 +12477,11 @@ where ); if self.default_configuration.manually_handle_bolt12_invoices { + // Update the corresponding entry in `PendingOutboundPayment` for this invoice. + // This ensures that event generation remains idempotent in case we receive + // the same invoice multiple times. + self.pending_outbound_payments.mark_invoice_received(&invoice, payment_id).ok()?; + let event = Event::InvoiceReceived { payment_id, invoice, context, responder, }; diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 55d2497bf8c..891fe6131e8 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -1186,7 +1186,14 @@ fn pays_bolt12_invoice_asynchronously() { let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); bob.onion_messenger.handle_onion_message(alice_id, &onion_message); - let (invoice, context) = match get_event!(bob, Event::InvoiceReceived) { + // Re-process the same onion message to ensure idempotency — + // we should not generate a duplicate `InvoiceReceived` event. + bob.onion_messenger.handle_onion_message(alice_id, &onion_message); + + let mut events = bob.node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + + let (invoice, context) = match events.pop().unwrap() { Event::InvoiceReceived { payment_id: actual_payment_id, invoice, context, .. } => { assert_eq!(actual_payment_id, payment_id); (invoice, context) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 15a370592c0..88716249dcf 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -73,9 +73,9 @@ pub(crate) enum PendingOutboundPayment { route_params_config: RouteParametersConfig, retryable_invoice_request: Option }, - // This state will never be persisted to disk because we transition from `AwaitingInvoice` to - // `Retryable` atomically within the `ChannelManager::total_consistency_lock`. Useful to avoid - // holding the `OutboundPayments::pending_outbound_payments` lock during pathfinding. + // Represents the state after the invoice has been received, transitioning from the corresponding + // `AwaitingInvoice` state. + // Helps avoid holding the `OutboundPayments::pending_outbound_payments` lock during pathfinding. InvoiceReceived { payment_hash: PaymentHash, retry_strategy: Retry, @@ -862,26 +862,9 @@ impl OutboundPayments { IH: Fn() -> InFlightHtlcs, SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { - let payment_hash = invoice.payment_hash(); - let params_config; - let retry_strategy; - match self.pending_outbound_payments.lock().unwrap().entry(payment_id) { - hash_map::Entry::Occupied(entry) => match entry.get() { - PendingOutboundPayment::AwaitingInvoice { - retry_strategy: retry, route_params_config, .. - } => { - retry_strategy = *retry; - params_config = *route_params_config; - *entry.into_mut() = PendingOutboundPayment::InvoiceReceived { - payment_hash, - retry_strategy: *retry, - route_params_config: *route_params_config, - }; - }, - _ => return Err(Bolt12PaymentError::DuplicateInvoice), - }, - hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice), - } + + let (payment_hash, retry_strategy, params_config, _) = self + .get_received_invoice_details(invoice, payment_id)?; if invoice.invoice_features().requires_unknown_bits_from(&features) { self.abandon_payment( @@ -1789,6 +1772,51 @@ impl OutboundPayments { } } + pub(super) fn mark_invoice_received( + &self, invoice: &Bolt12Invoice, payment_id: PaymentId + ) -> Result<(), Bolt12PaymentError> { + self.get_received_invoice_details(invoice, payment_id) + .and_then(|(_, _, _, invoice_marked_received)| { + invoice_marked_received + .then_some(()) + .ok_or(Bolt12PaymentError::DuplicateInvoice) + }) + } + + fn get_received_invoice_details( + &self, invoice: &Bolt12Invoice, payment_id: PaymentId + ) -> Result<(PaymentHash, Retry, RouteParametersConfig, bool), Bolt12PaymentError> { + match self.pending_outbound_payments.lock().unwrap().entry(payment_id) { + hash_map::Entry::Occupied(entry) => match entry.get() { + PendingOutboundPayment::AwaitingInvoice { + retry_strategy: retry, route_params_config, .. + } => { + let payment_hash = invoice.payment_hash(); + let retry = *retry; + let config = *route_params_config; + *entry.into_mut() = PendingOutboundPayment::InvoiceReceived { + payment_hash, + retry_strategy: retry, + route_params_config: config, + }; + + Ok((payment_hash, retry, config, true)) + }, + // When manual invoice handling is enabled, the corresponding `PendingOutboundPayment` entry + // is already updated at the time the invoice is received. This ensures that `InvoiceReceived` + // event generation remains idempotent, even if the same invoice is received again before the + // event is handled by the user. + PendingOutboundPayment::InvoiceReceived { + retry_strategy, route_params_config, .. + } => { + Ok((invoice.payment_hash(), *retry_strategy, *route_params_config, false)) + }, + _ => Err(Bolt12PaymentError::DuplicateInvoice), + }, + hash_map::Entry::Vacant(_) => Err(Bolt12PaymentError::UnexpectedInvoice), + } + } + fn pay_route_internal( &self, route: &Route, payment_hash: PaymentHash, recipient_onion: &RecipientOnionFields, keysend_preimage: Option, invoice_request: Option<&InvoiceRequest>,