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,25 @@ 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.
+					match self.pending_outbound_payments.pending_outbound_payments.lock().unwrap().entry(payment_id) {
+						hash_map::Entry::Occupied(entry) => match entry.get() {
+							PendingOutboundPayment::AwaitingInvoice {
+								retry_strategy, route_params_config, ..
+							} => {
+								*entry.into_mut() = PendingOutboundPayment::InvoiceReceived {
+									payment_hash: invoice.payment_hash(),
+									retry_strategy: *retry_strategy,
+									route_params_config: *route_params_config,
+								};
+							},
+							_ => return None
+						},
+						hash_map::Entry::Vacant(_) => return None,
+					}
+
 					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,8 @@ 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),
-		}
+		let (retry_strategy, params_config) = self
+			.mark_invoice_received(payment_id, payment_hash, with_manual_handling)?;
 
 		if invoice.invoice_features().requires_unknown_bits_from(&features) {
 			self.abandon_payment(
@@ -1789,6 +1772,54 @@ impl OutboundPayments {
 		}
 	}
 
+	fn mark_invoice_received(
+		&self, payment_id: PaymentId, payment_hash: PaymentHash, with_manual_handling: bool
+	) -> Result<(Retry, RouteParametersConfig), Bolt12PaymentError> {
+		let retry_strategy;
+		let params_config;
+
+		if with_manual_handling {
+			// 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.
+			match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
+				hash_map::Entry::Occupied(entry) => match entry.get() {
+					PendingOutboundPayment::InvoiceReceived {
+						retry_strategy: retry, route_params_config, ..
+					} => {
+						retry_strategy = *retry;
+						params_config = *route_params_config;
+					},
+					_ => return Err(Bolt12PaymentError::DuplicateInvoice),
+				},
+				hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice),
+			}
+		} else {
+			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),
+			}
+		}
+
+		Ok((retry_strategy, params_config))
+
+	}
+
 	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 +2899,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 +2961,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 +3036,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 +3056,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 +3067,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),
 		);