Add InvoiceRequest and Context Fields in AwaitingInvoice

- Introduced `InvoiceRequest` and `context` fields in the `AwaitingInvoice`
  to enable recreation of the `InvoiceRequest` message for retries if the
  corresponding invoice is not received in time.
- Added `awaiting_invoice` flag to track pending outbound payments with
  invoice requests without locking the `pending_outbound_payment` mutex.

Author: shaavan

lightning/src/ln/channelmanager.rs

@@ -3049,7 +3049,7 @@ where
 
 			outbound_scid_aliases: Mutex::new(new_hash_set()),
 			pending_inbound_payments: Mutex::new(new_hash_map()),
-			pending_outbound_payments: OutboundPayments::new(),
+			pending_outbound_payments: OutboundPayments::new(new_hash_map()),
 			forward_htlcs: Mutex::new(new_hash_map()),
 			decode_update_add_htlcs: Mutex::new(new_hash_map()),
 			claimable_payments: Mutex::new(ClaimablePayments { claimable_payments: new_hash_map(), pending_claiming_payments: new_hash_map() }),
@@ -8882,7 +8882,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => {
 
 		let nonce = Nonce::from_entropy_source(entropy);
 		let context = OffersContext::OutboundPayment { payment_id, nonce };
-		let path = $self.create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry))
+		let path = $self.create_blinded_paths_using_absolute_expiry(context.clone(), Some(absolute_expiry))
 			.and_then(|paths| paths.into_iter().next().ok_or(()))
 			.map_err(|_| Bolt12SemanticError::MissingPaths)?;
 
@@ -8898,7 +8898,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => {
 		let expiration = StaleExpiration::AbsoluteTimeout(absolute_expiry);
 		$self.pending_outbound_payments
 			.add_new_awaiting_invoice(
-				payment_id, expiration, retry_strategy, max_total_routing_fee_msat,
+				payment_id, expiration, retry_strategy, max_total_routing_fee_msat, None, context
 			)
 			.map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?;
 
@@ -9017,15 +9017,16 @@ where
 		let invoice_request = builder.build_and_sign()?;
 
 		let context = OffersContext::OutboundPayment { payment_id, nonce };
-		let reply_paths = self.create_blinded_paths(context)
+		let reply_paths = self.create_blinded_paths(context.clone())
 			.map_err(|_| Bolt12SemanticError::MissingPaths)?;
 
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
 		let expiration = StaleExpiration::TimerTicks(1);
 		self.pending_outbound_payments
 			.add_new_awaiting_invoice(
-				payment_id, expiration, retry_strategy, max_total_routing_fee_msat
+				payment_id, expiration, retry_strategy, max_total_routing_fee_msat,
+				Some(invoice_request.clone()), context,
 			)
 			.map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?;
 
@@ -12090,10 +12091,7 @@ where
 			}
 			pending_outbound_payments = Some(outbounds);
 		}
-		let pending_outbounds = OutboundPayments {
-			pending_outbound_payments: Mutex::new(pending_outbound_payments.unwrap()),
-			retry_lock: Mutex::new(())
-		};
+		let pending_outbounds = OutboundPayments::new(pending_outbound_payments.unwrap());
 
 		// We have to replay (or skip, if they were completed after we wrote the `ChannelManager`)
 		// each `ChannelMonitorUpdate` in `in_flight_monitor_updates`. After doing so, we have to

lightning/src/ln/outbound_payment.rs

@@ -13,6 +13,7 @@ use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::secp256k1::{self, Secp256k1, SecretKey};
 
+use crate::blinded_path::message::OffersContext;
 use crate::blinded_path::{IntroductionNode, NodeIdLookUp};
 use crate::blinded_path::payment::advance_path_by_one;
 use crate::events::{self, PaymentFailureReason};
@@ -22,6 +23,7 @@ use crate::ln::channelmanager::{EventCompletionAction, HTLCSource, PaymentId};
 use crate::ln::onion_utils;
 use crate::ln::onion_utils::{DecodedOnionFailure, HTLCFailReason};
 use crate::offers::invoice::Bolt12Invoice;
+use crate::offers::invoice_request::InvoiceRequest;
 use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, PaymentParameters, Route, RouteParameters, Router};
 use crate::sign::{EntropySource, NodeSigner, Recipient};
 use crate::util::errors::APIError;
@@ -33,6 +35,7 @@ use crate::util::ser::ReadableArgs;
 
 use core::fmt::{self, Display, Formatter};
 use core::ops::Deref;
+use core::sync::atomic::{AtomicBool, Ordering};
 use core::time::Duration;
 
 use crate::prelude::*;
@@ -54,6 +57,8 @@ pub(crate) enum PendingOutboundPayment {
 		expiration: StaleExpiration,
 		retry_strategy: Retry,
 		max_total_routing_fee_msat: Option<u64>,
+		invoice_request: Option<InvoiceRequest>,
+		context: OffersContext,
 	},
 	InvoiceReceived {
 		payment_hash: PaymentHash,
@@ -684,13 +689,19 @@ pub(super) struct SendAlongPathArgs<'a> {
 
 pub(super) struct OutboundPayments {
 	pub(super) pending_outbound_payments: Mutex<HashMap<PaymentId, PendingOutboundPayment>>,
-	pub(super) retry_lock: Mutex<()>,
+	awaiting_invoice: AtomicBool,
+	retry_lock: Mutex<()>,
 }
 
 impl OutboundPayments {
-	pub(super) fn new() -> Self {
+	pub(super) fn new(pending_outbound_payments: HashMap<PaymentId, PendingOutboundPayment>) -> Self {
+		let has_invoice_requests = pending_outbound_payments.values().any(|payment| {
+			matches!(payment, PendingOutboundPayment::AwaitingInvoice { invoice_request: Some(_), .. })
+		});
+
 		Self {
-			pending_outbound_payments: Mutex::new(new_hash_map()),
+			pending_outbound_payments: Mutex::new(pending_outbound_payments),
+			awaiting_invoice: AtomicBool::new(has_invoice_requests),
 			retry_lock: Mutex::new(()),
 		}
 	}
@@ -1351,7 +1362,8 @@ impl OutboundPayments {
 
 	pub(super) fn add_new_awaiting_invoice(
 		&self, payment_id: PaymentId, expiration: StaleExpiration, retry_strategy: Retry,
-		max_total_routing_fee_msat: Option<u64>
+		max_total_routing_fee_msat: Option<u64>, invoice_request: Option<InvoiceRequest>,
+		context: OffersContext,
 	) -> Result<(), ()> {
 		let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
 		match pending_outbounds.entry(payment_id) {
@@ -1361,7 +1373,10 @@ impl OutboundPayments {
 					expiration,
 					retry_strategy,
 					max_total_routing_fee_msat,
+					invoice_request,
+					context,
 				});
+				self.awaiting_invoice.store(true, Ordering::Release);
 
 				Ok(())
 			},
@@ -1827,6 +1842,25 @@ impl OutboundPayments {
 	pub fn clear_pending_payments(&self) {
 		self.pending_outbound_payments.lock().unwrap().clear()
 	}
+
+	pub fn release_invoice_request_awaiting_invoice(&self) -> Vec<(OffersContext, InvoiceRequest)> {
+		if !self.awaiting_invoice.load(Ordering::Acquire) {
+			return vec![];
+		}
+
+		let mut pending_outbound_payments = self.pending_outbound_payments.lock().unwrap();
+		let invoice_requests = pending_outbound_payments.iter_mut()
+			.filter_map(|(_, payment)| match payment {
+				PendingOutboundPayment::AwaitingInvoice { invoice_request, context, ..} => {
+					invoice_request.take().map(|req| (context.clone(), req))
+				}
+				_ => None,
+			})
+			.collect();
+
+		self.awaiting_invoice.store(false, Ordering::Release);
+		invoice_requests
+	}
 }
 
 /// Returns whether a payment with the given [`PaymentHash`] and [`PaymentId`] is, in fact, a
@@ -1882,6 +1916,8 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(0, expiration, required),
 		(2, retry_strategy, required),
 		(4, max_total_routing_fee_msat, option),
+		(5, invoice_request, option),
+		(6, context, required),
 	},
 	(7, InvoiceReceived) => {
 		(0, payment_hash, required),
@@ -1897,6 +1933,7 @@ mod tests {
 
 	use core::time::Duration;
 
+	use crate::blinded_path::message::OffersContext;
 	use crate::blinded_path::EmptyNodeIdLookUp;
 	use crate::events::{Event, PathFailure, PaymentFailureReason};
 	use crate::ln::types::PaymentHash;
@@ -1912,6 +1949,7 @@ mod tests {
 	use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters};
 	use crate::sync::{Arc, Mutex, RwLock};
 	use crate::util::errors::APIError;
+	use crate::util::hash_tables::new_hash_map;
 	use crate::util::test_utils;
 
 	use alloc::collections::VecDeque;
@@ -1946,7 +1984,7 @@ mod tests {
 	}
 	#[cfg(feature = "std")]
 	fn do_fails_paying_after_expiration(on_retry: bool) {
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let logger = test_utils::TestLogger::new();
 		let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger));
 		let scorer = RwLock::new(test_utils::TestScorer::new());
@@ -1990,7 +2028,7 @@ mod tests {
 		do_find_route_error(true);
 	}
 	fn do_find_route_error(on_retry: bool) {
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let logger = test_utils::TestLogger::new();
 		let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger));
 		let scorer = RwLock::new(test_utils::TestScorer::new());
@@ -2029,7 +2067,7 @@ mod tests {
 
 	#[test]
 	fn initial_send_payment_path_failed_evs() {
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let logger = test_utils::TestLogger::new();
 		let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger));
 		let scorer = RwLock::new(test_utils::TestScorer::new());
@@ -2111,7 +2149,7 @@ mod tests {
 	#[test]
 	fn removes_stale_awaiting_invoice_using_absolute_timeout() {
 		let pending_events = Mutex::new(VecDeque::new());
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let payment_id = PaymentId([0; 32]);
 		let absolute_expiry = 100;
 		let tick_interval = 10;
@@ -2120,7 +2158,7 @@ mod tests {
 		assert!(!outbound_payments.has_pending_payments());
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), None
+				payment_id, expiration, Retry::Attempts(0), None, None, OffersContext::Unknown {}
 			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
@@ -2146,30 +2184,30 @@ mod tests {
 
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), None
+				payment_id, expiration, Retry::Attempts(0), None, None, OffersContext::Unknown {}
 			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
 
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), None
+				payment_id, expiration, Retry::Attempts(0), None, None, OffersContext::Unknown {}
 			).is_err()
 		);
 	}
 
 	#[test]
 	fn removes_stale_awaiting_invoice_using_timer_ticks() {
 		let pending_events = Mutex::new(VecDeque::new());
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let payment_id = PaymentId([0; 32]);
 		let timer_ticks = 3;
 		let expiration = StaleExpiration::TimerTicks(timer_ticks);
 
 		assert!(!outbound_payments.has_pending_payments());
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), None
+				payment_id, expiration, Retry::Attempts(0), None, None, OffersContext::Unknown {}
 			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
@@ -2195,29 +2233,29 @@ mod tests {
 
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), None
+				payment_id, expiration, Retry::Attempts(0), None, None, OffersContext::Unknown {}
 			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
 
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), None
+				payment_id, expiration, Retry::Attempts(0), None, None, OffersContext::Unknown {}
 			).is_err()
 		);
 	}
 
 	#[test]
 	fn removes_abandoned_awaiting_invoice() {
 		let pending_events = Mutex::new(VecDeque::new());
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let payment_id = PaymentId([0; 32]);
 		let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
 		assert!(!outbound_payments.has_pending_payments());
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), None
+				payment_id, expiration, Retry::Attempts(0), None, None, OffersContext::Unknown {}
 			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
@@ -2245,13 +2283,13 @@ mod tests {
 		let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
 
 		let pending_events = Mutex::new(VecDeque::new());
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let payment_id = PaymentId([0; 32]);
 		let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), None
+				payment_id, expiration, Retry::Attempts(0), None, None, OffersContext::Unknown {}
 			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
@@ -2298,7 +2336,7 @@ mod tests {
 		let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
 
 		let pending_events = Mutex::new(VecDeque::new());
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let payment_id = PaymentId([0; 32]);
 		let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
@@ -2315,7 +2353,7 @@ mod tests {
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
 				payment_id, expiration, Retry::Attempts(0),
-				Some(invoice.amount_msats() / 100 + 50_000)
+				Some(invoice.amount_msats() / 100 + 50_000), None, OffersContext::Unknown {}
 			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
@@ -2359,7 +2397,7 @@ mod tests {
 		let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
 
 		let pending_events = Mutex::new(VecDeque::new());
-		let outbound_payments = OutboundPayments::new();
+		let outbound_payments = OutboundPayments::new(new_hash_map());
 		let payment_id = PaymentId([0; 32]);
 		let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
@@ -2415,7 +2453,7 @@ mod tests {
 
 		assert!(
 			outbound_payments.add_new_awaiting_invoice(
-				payment_id, expiration, Retry::Attempts(0), Some(1234)
+				payment_id, expiration, Retry::Attempts(0), Some(1234), None, OffersContext::Unknown {}
 			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());

lightning/src/offers/invoice_request.rs

@@ -1038,6 +1038,13 @@ impl Writeable for InvoiceRequestContents {
 	}
 }
 
+impl Readable for InvoiceRequest {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		let bytes: WithoutLength<Vec<u8>> = Readable::read(reader)?;
+		Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue)
+	}
+}
+
 /// Valid type range for invoice_request TLV records.
 pub(super) const INVOICE_REQUEST_TYPES: core::ops::Range<u64> = 80..160;