Await invoice for variable number of timer ticks

PendingOutboundPayment::AwaitingInvoice counts the number of timer ticks
that have passed awaiting a Bolt12Invoice for an InvoiceRequest. When a
constant INVOICE_REQUEST_TIMEOUT_TICKS has passed, the payment is
forgotten. However, this mechanism is insufficient for the Refund
scenario, where the Refund's expiration should be used instead.

Change AwaitingInvoice to store the number of timer ticks remaining
instead of the number that has passed. This allows for a variable number
of ticks that are instead counted down.

Author: jkczyz

lightning/src/ln/outbound_payment.rs

@@ -43,7 +43,7 @@ pub(crate) const IDEMPOTENCY_TIMEOUT_TICKS: u8 = 7;
 /// a response is timed out.
 ///
 /// [`ChannelManager::timer_tick_occurred`]: crate::ln::channelmanager::ChannelManager::timer_tick_occurred
-const INVOICE_REQUEST_TIMEOUT_TICKS: u8 = 3;
+const INVOICE_REQUEST_TIMEOUT_TICKS: Option<u64> = Some(3);
 
 /// Stores the session_priv for each part of a payment that is still pending. For versions 0.0.102
 /// and later, also stores information for retrying the payment.
@@ -52,7 +52,7 @@ pub(crate) enum PendingOutboundPayment {
 		session_privs: HashSet<[u8; 32]>,
 	},
 	AwaitingInvoice {
-		timer_ticks_without_response: u8,
+		timer_ticks_without_response_remaining: Option<u64>,
 		retry_strategy: Retry,
 		max_total_routing_fee_msat: Option<u64>,
 	},
@@ -1274,15 +1274,35 @@ impl OutboundPayments {
 	}
 
 	#[allow(unused)]
-	pub(super) fn add_new_awaiting_invoice(
+	pub(super) fn add_new_awaiting_invoice_for_offer(
 		&self, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option<u64>
+	) -> Result<(), ()> {
+		self.add_new_awaiting_invoice(
+			payment_id, retry_strategy, max_total_routing_fee_msat, INVOICE_REQUEST_TIMEOUT_TICKS
+		)
+	}
+
+	#[allow(unused)]
+	pub(super) fn add_new_awaiting_invoice_for_refund(
+		&self, payment_id: PaymentId, retry_strategy: Retry,
+		max_total_routing_fee_msat: Option<u64>, timer_ticks_before_expiration: Option<u64>
+	) -> Result<(), ()> {
+		self.add_new_awaiting_invoice(
+			payment_id, retry_strategy, max_total_routing_fee_msat, timer_ticks_before_expiration
+		)
+	}
+
+	#[allow(unused)]
+	fn add_new_awaiting_invoice(
+		&self, payment_id: PaymentId, retry_strategy: Retry,
+		max_total_routing_fee_msat: Option<u64>, timer_ticks_before_expiration: Option<u64>
 	) -> Result<(), ()> {
 		let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
 		match pending_outbounds.entry(payment_id) {
 			hash_map::Entry::Occupied(_) => Err(()),
 			hash_map::Entry::Vacant(entry) => {
 				entry.insert(PendingOutboundPayment::AwaitingInvoice {
-					timer_ticks_without_response: 0,
+					timer_ticks_without_response_remaining: timer_ticks_before_expiration,
 					retry_strategy,
 					max_total_routing_fee_msat,
 				});
@@ -1550,9 +1570,11 @@ impl OutboundPayments {
 					*timer_ticks_without_htlcs = 0;
 					true
 				}
-			} else if let PendingOutboundPayment::AwaitingInvoice { timer_ticks_without_response, .. } = payment {
-				*timer_ticks_without_response += 1;
-				if *timer_ticks_without_response <= INVOICE_REQUEST_TIMEOUT_TICKS {
+			} else if let PendingOutboundPayment::AwaitingInvoice {
+				timer_ticks_without_response_remaining: Some(timer_ticks_remaining), ..
+			} = payment {
+				if *timer_ticks_remaining > 0 {
+					*timer_ticks_remaining -= 1;
 					true
 				} else {
 					#[cfg(invreqfailed)]
@@ -1778,7 +1800,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(2, payment_hash, required),
 	},
 	(5, AwaitingInvoice) => {
-		(0, timer_ticks_without_response, required),
+		(0, timer_ticks_without_response_remaining, option),
 		(2, retry_strategy, required),
 		(4, max_total_routing_fee_msat, option),
 	},
@@ -2014,11 +2036,13 @@ mod tests {
 
 		assert!(!outbound_payments.has_pending_payments());
 		assert!(
-			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+			outbound_payments.add_new_awaiting_invoice(
+				payment_id, Retry::Attempts(0), None, INVOICE_REQUEST_TIMEOUT_TICKS
+			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
 
-		for _ in 0..INVOICE_REQUEST_TIMEOUT_TICKS {
+		for _ in 0..INVOICE_REQUEST_TIMEOUT_TICKS.unwrap() {
 			outbound_payments.remove_stale_payments(&pending_events);
 			assert!(outbound_payments.has_pending_payments());
 			assert!(pending_events.lock().unwrap().is_empty());
@@ -2034,13 +2058,16 @@ mod tests {
 		assert!(pending_events.lock().unwrap().is_empty());
 
 		assert!(
-			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+			outbound_payments.add_new_awaiting_invoice(
+				payment_id, Retry::Attempts(0), None, INVOICE_REQUEST_TIMEOUT_TICKS
+			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
 
 		assert!(
-			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None)
-				.is_err()
+			outbound_payments.add_new_awaiting_invoice(
+				payment_id, Retry::Attempts(0), None, INVOICE_REQUEST_TIMEOUT_TICKS
+			).is_err()
 		);
 	}
 
@@ -2053,7 +2080,9 @@ mod tests {
 
 		assert!(!outbound_payments.has_pending_payments());
 		assert!(
-			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+			outbound_payments.add_new_awaiting_invoice(
+				payment_id, Retry::Attempts(0), None, INVOICE_REQUEST_TIMEOUT_TICKS
+			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
 
@@ -2083,7 +2112,9 @@ mod tests {
 		let payment_id = PaymentId([0; 32]);
 
 		assert!(
-			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+			outbound_payments.add_new_awaiting_invoice(
+				payment_id, Retry::Attempts(0), None, INVOICE_REQUEST_TIMEOUT_TICKS
+			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
 
@@ -2140,9 +2171,11 @@ mod tests {
 			.build().unwrap()
 			.sign(recipient_sign).unwrap();
 
-		assert!(outbound_payments.add_new_awaiting_invoice(
-				payment_id, Retry::Attempts(0), Some(invoice.amount_msats() / 100 + 50_000))
-			.is_ok()
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(
+				payment_id, Retry::Attempts(0), Some(invoice.amount_msats() / 100 + 50_000),
+				INVOICE_REQUEST_TIMEOUT_TICKS
+			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
 
@@ -2196,9 +2229,11 @@ mod tests {
 			.build().unwrap()
 			.sign(recipient_sign).unwrap();
 
-		assert!(outbound_payments.add_new_awaiting_invoice(
-				payment_id, Retry::Attempts(0), Some(invoice.amount_msats() / 100 + 50_000))
-			.is_ok()
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(
+				payment_id, Retry::Attempts(0), Some(invoice.amount_msats() / 100 + 50_000),
+				INVOICE_REQUEST_TIMEOUT_TICKS
+			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());
 
@@ -2292,7 +2327,9 @@ mod tests {
 		assert!(pending_events.lock().unwrap().is_empty());
 
 		assert!(
-			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), Some(1234)).is_ok()
+			outbound_payments.add_new_awaiting_invoice(
+				payment_id, Retry::Attempts(0), Some(1234), INVOICE_REQUEST_TIMEOUT_TICKS
+			).is_ok()
 		);
 		assert!(outbound_payments.has_pending_payments());