f - Combine hash maps so only a single look-up is necessary

Author: jkczyz

lightning-invoice/src/payment.rs

@@ -60,9 +60,7 @@ where
 	router: R,
 	logger: L,
 	event_handler: E,
-	// Lock order: payment_attempts -> invoice_cache
-	invoice_cache: Mutex<HashMap<PaymentHash, Invoice>>,
-	payment_attempts: Mutex<HashMap<PaymentHash, usize>>,
+	invoice_cache: Mutex<HashMap<PaymentHash, (Invoice, usize)>>,
 	retry_attempts: usize,
 }
 
@@ -122,7 +120,6 @@ where
 			logger,
 			event_handler,
 			invoice_cache: Mutex::new(HashMap::new()),
-			payment_attempts: Mutex::new(HashMap::new()),
 			retry_attempts: 0,
 		}
 	}
@@ -139,23 +136,15 @@ where
 		let mut invoice_cache = self.invoice_cache.lock().unwrap();
 		match invoice_cache.entry(payment_hash) {
 			hash_map::Entry::Vacant(entry) => {
-				self.pay_invoice_internal(invoice)?;
-				entry.insert(invoice.clone());
+				self.pay_cached_invoice(invoice)?;
+				entry.insert((invoice.clone(), 0));
 				Ok(())
 			},
 			hash_map::Entry::Occupied(_) => Err(PaymentError::Invoice("payment pending")),
 		}
 	}
 
-	fn pay_cached_invoice(&self, payment_hash: &PaymentHash) -> Result<(), PaymentError> {
-		let invoice_cache = self.invoice_cache.lock().unwrap();
-		match invoice_cache.get(payment_hash) {
-			Some(invoice) => self.pay_invoice_internal(invoice),
-			None => Err(PaymentError::Invoice("unknown payment hash")),
-		}
-	}
-
-	fn pay_invoice_internal(&self, invoice: &Invoice) -> Result<(), PaymentError> {
+	fn pay_cached_invoice(&self, invoice: &Invoice) -> Result<(), PaymentError> {
 		let payer = self.payer.node_id();
 		let payee = invoice.recover_payee_pub_key();
 		let payee_features = invoice.features().cloned();
@@ -199,40 +188,39 @@ where
 	fn handle_event(&self, event: &Event) {
 		match event {
 			Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => {
-				let mut attempts_by_payment_hash = self.payment_attempts.lock().unwrap();
-				let attempts = attempts_by_payment_hash
-					.entry(*payment_hash)
-					.and_modify(|attempts| *attempts += 1)
-					.or_insert(1);
-				if !rejected_by_dest {
-					let max_payment_attempts = self.retry_attempts + 1;
-					if *attempts < max_payment_attempts {
-						if self.pay_cached_invoice(payment_hash).is_ok() {
-							log_trace!(self.logger, "Payment {} failed; retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
-							return;
+				let mut invoice_cache = self.invoice_cache.lock().unwrap();
+				if let hash_map::Entry::Occupied(mut entry) = invoice_cache.entry(*payment_hash) {
+					let (invoice, attempts) = entry.get_mut();
+					*attempts += 1;
+					if !rejected_by_dest {
+						let max_payment_attempts = self.retry_attempts + 1;
+						if *attempts < max_payment_attempts {
+							if self.pay_cached_invoice(invoice).is_ok() {
+								log_trace!(self.logger, "Payment {} failed; retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
+								return;
+							} else {
+								log_trace!(self.logger, "Error retrying payment {}; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
+							}
 						} else {
-							log_trace!(self.logger, "Error retrying payment {}; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
+							log_trace!(self.logger, "Payment {} failed; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
 						}
 					} else {
-						log_trace!(self.logger, "Payment {} failed; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
+						log_trace!(self.logger, "Payment {} rejected by destination; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
 					}
+
+					// Either the payment was rejected, exceeded the maximum attempts, or failed.
+					entry.remove();
 				} else {
-					log_trace!(self.logger, "Payment {} rejected by destination; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
+					log_trace!(self.logger, "Unknown payment {}; cannot retry)", log_bytes!(payment_hash.0));
 				}
-
-				// Either the payment was rejected, exceeded the maximum attempts, or failed retry.
-				self.remove_cached_invoice(payment_hash);
-				attempts_by_payment_hash.remove(payment_hash);
 			},
 			Event::PaymentSent { payment_preimage } => {
 				let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
-				let mut attempts_by_payment_hash = self.payment_attempts.lock().unwrap();
-				let attempts = attempts_by_payment_hash
+				let mut invoice_cache = self.invoice_cache.lock().unwrap();
+				let attempts = invoice_cache
 					.remove(&payment_hash)
-					.map_or(1, |attempts| attempts + 1);
+					.map_or(1, |(_, attempts)| attempts + 1);
 				log_trace!(self.logger, "Payment {} succeeded (attempts: {})", log_bytes!(payment_hash.0), attempts);
-
-				self.remove_cached_invoice(&payment_hash);
 			},
 			_ => {},
 		}