Make InvoiceReceived event generation idempotent

Ensures `InvoiceReceived` events are not generated multiple times
when `manually_handle_bolt12_invoice` is enabled.
Duplicate events for the same invoice could cause confusion—this
change introduces an idempotency check to prevent that.

Author: shaavan

lightning/src/ln/channelmanager.rs

@@ -4824,7 +4824,8 @@ where
 				invoice, payment_id, &self.router, self.list_usable_channels(), features,
 				|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, &self,
 				&self.secp_ctx, best_block_height, &self.logger, &self.pending_events,
-				|args| self.send_payment_along_path(args)
+				|args| self.send_payment_along_path(args),
+				self.default_configuration.manually_handle_bolt12_invoices
 			)
 	}
 
@@ -12477,6 +12478,12 @@ 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(payment_id, invoice.payment_hash()).ok()?;
+
 					let event = Event::InvoiceReceived {
 						payment_id, invoice, context, responder,
 					};

lightning/src/ln/outbound_payment.rs

@@ -851,7 +851,7 @@ impl OutboundPayments {
 		entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL,
 		secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32, logger: &L,
 		pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
-		send_payment_along_path: SP,
+		send_payment_along_path: SP, with_manual_handling: bool
 	) -> Result<(), Bolt12PaymentError>
 	where
 		R::Target: Router,
@@ -863,25 +863,14 @@ impl OutboundPayments {
 		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),
-		}
+
+		// 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
+		// current one is handled by the user.
+		if !with_manual_handling { self.mark_invoice_received(payment_id, payment_hash)? }
+
+		let (retry_strategy, params_config) = self.received_invoice_details(payment_id)?;
 
 		if invoice.invoice_features().requires_unknown_bits_from(&features) {
 			self.abandon_payment(
@@ -1789,6 +1778,44 @@ impl OutboundPayments {
 		}
 	}
 
+	pub(super) fn mark_invoice_received(
+		&self, payment_id: PaymentId, payment_hash: PaymentHash
+	) -> Result<(), 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, ..
+				} => {
+					*entry.into_mut() = PendingOutboundPayment::InvoiceReceived {
+						payment_hash,
+						retry_strategy: *retry,
+						route_params_config: *route_params_config,
+					};
+
+					Ok(())
+				},
+				_ => Err(Bolt12PaymentError::DuplicateInvoice),
+			},
+			hash_map::Entry::Vacant(_) => Err(Bolt12PaymentError::UnexpectedInvoice),
+		}
+	}
+
+	fn received_invoice_details(
+		&self, payment_id: PaymentId,
+	) -> Result<(Retry, RouteParametersConfig), Bolt12PaymentError> {
+		match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
+			hash_map::Entry::Occupied(entry) => match entry.get() {
+				PendingOutboundPayment::InvoiceReceived {
+					retry_strategy, route_params_config, ..
+				} => {
+					return Ok((*retry_strategy, *route_params_config))
+				},
+				_ => return Err(Bolt12PaymentError::DuplicateInvoice),
+			},
+			hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice),
+		}
+	}
+
 	fn pay_route_internal<NS: Deref, F>(
 		&self, route: &Route, payment_hash: PaymentHash, recipient_onion: &RecipientOnionFields,
 		keysend_preimage: Option<PaymentPreimage>, invoice_request: Option<&InvoiceRequest>,
@@ -2868,7 +2895,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, &&logger, &pending_events, |_| panic!(), false
 			),
 			Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::PaymentExpired)),
 		);
@@ -2930,7 +2957,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, &&logger, &pending_events, |_| panic!(), false
 			),
 			Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::RouteNotFound)),
 		);
@@ -3005,7 +3032,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, &&logger, &pending_events, |_| panic!(), false
 			),
 			Err(Bolt12PaymentError::UnexpectedInvoice),
 		);
@@ -3025,7 +3052,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, &&logger, &pending_events, |_| Ok(()), false
 			),
 			Ok(()),
 		);
@@ -3036,7 +3063,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, &&logger, &pending_events, |_| panic!(), false
 			),
 			Err(Bolt12PaymentError::DuplicateInvoice),
 		);