f Reserve manual invoice hashes first

Reject duplicate manual BOLT11 hashes from the pending store
before creating the invoice. This keeps duplicate detection and
registration serialized, while preserving the persist-before-index
ordering for crash recovery.

Co-Authored-By: HAL 9000

Author: tnull

src/payment/bolt11.rs

@@ -123,19 +123,15 @@ impl Bolt11Payment {
 		Ok(())
 	}
 
-	fn has_pending_inbound_payment(&self, payment_hash: &PaymentHash) -> bool {
-		self.pending_payment_store.get_pending_manual_bolt11_by_payment_hash(payment_hash).is_some()
-	}
-
-	fn register_manual_claim_invoice(
-		&self, payment_hash: PaymentHash, amount_msat: Option<u64>, payment_secret: PaymentSecret,
+	fn pending_manual_claim_invoice(
+		payment_hash: PaymentHash, amount_msat: Option<u64>, payment_secret: Option<PaymentSecret>,
 		expiry_secs: u32,
-	) -> Result<(), Error> {
+	) -> PendingPaymentDetails {
 		let payment_id = PaymentId(payment_hash.0);
 		let kind = PaymentKind::Bolt11 {
 			hash: payment_hash,
 			preimage: None,
-			secret: Some(payment_secret),
+			secret: payment_secret,
 			counterparty_skimmed_fee_msat: None,
 		};
 		let payment = PaymentDetails::new(
@@ -147,22 +143,51 @@ impl Bolt11Payment {
 			PaymentStatus::Pending,
 		);
 		let expires_at = Some(Self::current_time_secs().saturating_add(expiry_secs as u64));
+		PendingPaymentDetails::new_with_expiry(payment, Vec::new(), expires_at)
+	}
+
+	fn reserve_manual_claim_invoice(
+		&self, payment_hash: PaymentHash, amount_msat: Option<u64>, expiry_secs: u32,
+	) -> Result<(), Error> {
 		let pending_payment =
-			PendingPaymentDetails::new_with_expiry(payment, Vec::new(), expires_at);
+			Self::pending_manual_claim_invoice(payment_hash, amount_msat, None, expiry_secs);
+		if let Err(e) =
+			self.runtime.block_on(self.pending_payment_store.insert_manual_bolt11(pending_payment))
+		{
+			if e == Error::DuplicatePayment {
+				log_error!(self.logger, "Payment error: an invoice must not be paid twice.");
+			}
+			return Err(e);
+		}
+		Ok(())
+	}
+
+	fn register_manual_claim_invoice(
+		&self, payment_hash: PaymentHash, amount_msat: Option<u64>, payment_secret: PaymentSecret,
+		expiry_secs: u32,
+	) -> Result<(), Error> {
+		let pending_payment = Self::pending_manual_claim_invoice(
+			payment_hash,
+			amount_msat,
+			Some(payment_secret),
+			expiry_secs,
+		);
 		self.runtime.block_on(self.pending_payment_store.insert_or_update(pending_payment))?;
 		Ok(())
 	}
 
+	fn remove_manual_claim_invoice(&self, payment_hash: PaymentHash) -> Result<(), Error> {
+		let payment_id = PaymentId(payment_hash.0);
+		self.runtime.block_on(self.pending_payment_store.remove(&payment_id))
+	}
+
 	pub(crate) fn receive_inner(
 		&self, amount_msat: Option<u64>, invoice_description: &LdkBolt11InvoiceDescription,
 		expiry_secs: u32, manual_claim_payment_hash: Option<PaymentHash>,
 	) -> Result<LdkBolt11Invoice, Error> {
 		if let Some(payment_hash) = manual_claim_payment_hash {
 			self.prune_expired_pending_payments()?;
-			if self.has_pending_inbound_payment(&payment_hash) {
-				log_error!(self.logger, "Payment error: an invoice must not be paid twice.");
-				return Err(Error::DuplicatePayment);
-			}
+			self.reserve_manual_claim_invoice(payment_hash, amount_msat, expiry_secs)?;
 		}
 
 		let invoice = {
@@ -181,6 +206,9 @@ impl Bolt11Payment {
 				},
 				Err(e) => {
 					log_error!(self.logger, "Failed to create invoice: {}", e);
+					if let Some(payment_hash) = manual_claim_payment_hash {
+						self.remove_manual_claim_invoice(payment_hash)?;
+					}
 					return Err(Error::InvoiceCreationFailed);
 				},
 			}
@@ -234,14 +262,11 @@ impl Bolt11Payment {
 	) -> Result<LdkBolt11Invoice, Error> {
 		if let Some(payment_hash) = payment_hash {
 			self.prune_expired_pending_payments()?;
-			if self.has_pending_inbound_payment(&payment_hash) {
-				log_error!(self.logger, "Payment error: an invoice must not be paid twice.");
-				return Err(Error::DuplicatePayment);
-			}
+			self.reserve_manual_claim_invoice(payment_hash, amount_msat, expiry_secs)?;
 		}
 
 		let connection_manager = Arc::clone(&self.connection_manager);
-		let (invoice, chosen_lsp) = self.runtime.block_on(async move {
+		let res = self.runtime.block_on(async move {
 			if let Some(amount_msat) = amount_msat {
 				self.liquidity_source
 					.lsps2_client()
@@ -266,7 +291,16 @@ impl Bolt11Payment {
 					)
 					.await
 			}
-		})?;
+		});
+		let (invoice, chosen_lsp) = match res {
+			Ok(res) => res,
+			Err(e) => {
+				if let Some(payment_hash) = payment_hash {
+					self.remove_manual_claim_invoice(payment_hash)?;
+				}
+				return Err(e);
+			},
+		};
 
 		if let Some(payment_hash) = payment_hash {
 			self.register_manual_claim_invoice(

src/payment/pending_payment_store.rs

@@ -229,6 +229,33 @@ where
 		&self, pending_payment: PendingPaymentDetails,
 	) -> Result<bool, Error> {
 		let _guard = self.mutation_lock.lock().await;
+		self.insert_or_update_locked(pending_payment).await
+	}
+
+	pub(crate) async fn insert_manual_bolt11(
+		&self, pending_payment: PendingPaymentDetails,
+	) -> Result<(), Error> {
+		let _guard = self.mutation_lock.lock().await;
+		let Some(payment_hash) = manual_bolt11_payment_hash(&pending_payment.details) else {
+			debug_assert!(false, "manual BOLT11 insert requires a pending inbound BOLT11 payment");
+			self.insert_or_update_locked(pending_payment).await?;
+			return Ok(());
+		};
+		let duplicate = {
+			let index = self.manual_bolt11_payment_hash_index.lock().expect("lock");
+			index.get(&payment_hash).map_or(false, |ids| !ids.is_empty())
+		};
+		if duplicate {
+			return Err(Error::DuplicatePayment);
+		}
+
+		self.insert_or_update_locked(pending_payment).await?;
+		Ok(())
+	}
+
+	async fn insert_or_update_locked(
+		&self, pending_payment: PendingPaymentDetails,
+	) -> Result<bool, Error> {
 		let id = pending_payment.id();
 		let before = self.inner.get(&id);
 		let updated = self.inner.insert_or_update(pending_payment).await?;
@@ -342,10 +369,14 @@ fn manual_bolt11_payment_hash(payment: &PaymentDetails) -> Option<PaymentHash> {
 #[cfg(test)]
 mod tests {
 	use bitcoin::hashes::Hash;
+	use lightning::util::test_utils::TestLogger;
+	use lightning_types::payment::PaymentSecret;
 
 	use super::*;
+	use crate::io::test_utils::InMemoryStore;
 	use crate::payment::store::ConfirmationStatus;
 	use crate::payment::{PaymentDirection, PaymentKind, PaymentStatus};
+	use crate::types::{DynStore, DynStoreWrapper};
 
 	#[test]
 	fn pending_payment_candidate_lookup() {
@@ -414,6 +445,57 @@ mod tests {
 		)
 	}
 
+	fn pending_store() -> PendingPaymentStore<Arc<TestLogger>> {
+		let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
+		let logger = Arc::new(TestLogger::new());
+		PendingPaymentStore::new(
+			Vec::new(),
+			"pending_payment_store_test_primary".to_string(),
+			"pending_payment_store_test_secondary".to_string(),
+			store,
+			logger,
+		)
+	}
+
+	fn pending_manual_bolt11_payment(
+		payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>,
+	) -> PendingPaymentDetails {
+		let payment_id = PaymentId(payment_hash.0);
+		let details = PaymentDetails::new(
+			payment_id,
+			PaymentKind::Bolt11 {
+				hash: payment_hash,
+				preimage: None,
+				secret: payment_secret,
+				counterparty_skimmed_fee_msat: None,
+			},
+			Some(1_000),
+			None,
+			PaymentDirection::Inbound,
+			PaymentStatus::Pending,
+		);
+		PendingPaymentDetails::new_with_expiry(details, Vec::new(), Some(1_000_000))
+	}
+
+	#[tokio::test]
+	async fn manual_bolt11_insert_rejects_duplicate_hash() {
+		let store = pending_store();
+		let payment_hash = PaymentHash([42; 32]);
+		let pending_payment = pending_manual_bolt11_payment(payment_hash, None);
+		assert_eq!(store.insert_manual_bolt11(pending_payment.clone()).await, Ok(()));
+
+		let duplicate = pending_manual_bolt11_payment(payment_hash, None);
+		assert_eq!(store.insert_manual_bolt11(duplicate).await, Err(Error::DuplicatePayment));
+		assert_eq!(
+			store.get_pending_manual_bolt11_by_payment_hash(&payment_hash),
+			Some(pending_payment)
+		);
+
+		let updated = pending_manual_bolt11_payment(payment_hash, Some(PaymentSecret([43; 32])));
+		assert_eq!(store.insert_or_update(updated.clone()).await, Ok(true));
+		assert_eq!(store.get_pending_manual_bolt11_by_payment_hash(&payment_hash), Some(updated));
+	}
+
 	#[test]
 	fn pending_onchain_conflicts_exclude_current_txid_after_txid_rotation() {
 		let original_txid = test_txid(1);