add expiry_time to PendingOutboundPayment::StaticInvoiceReceived

Previous this commit if the StaticInvoice has an expiration time
of months or years would make our HTLC to hold for that time until
is abandoned.

This patch adds a defaults of 1 week of expiry time that the HTLC
will be held waiting for the often-offline node comes online.

Author: a-mpch

lightning/src/ln/async_payments_tests.rs

@@ -23,8 +23,9 @@ use crate::ln::msgs::{
 };
 use crate::ln::offers_tests;
 use crate::ln::onion_utils::LocalHTLCFailureReason;
-use crate::ln::outbound_payment::PendingOutboundPayment;
-use crate::ln::outbound_payment::Retry;
+use crate::ln::outbound_payment::{
+	PendingOutboundPayment, Retry, TEST_ASYNC_PAYMENT_TIMEOUT_RELATIVE_EXPIRY,
+};
 use crate::offers::async_receive_offer_cache::{
 	TEST_MAX_CACHED_OFFERS_TARGET, TEST_MAX_UPDATE_ATTEMPTS,
 	TEST_MIN_OFFER_PATHS_RELATIVE_EXPIRY_SECS, TEST_OFFER_REFRESH_THRESHOLD,
@@ -681,6 +682,74 @@ fn expired_static_invoice_fail() {
 	// doesn't currently provide them with a reply path to do so.
 }
 
+#[cfg_attr(feature = "std", ignore)]
+#[test]
+fn timeout_unreleased_payment() {
+	// If a server holds a pending HTLC for too long, payment is considered expired.
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+	let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
+	create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0);
+
+	let sender = &nodes[0];
+	let server = &nodes[1];
+	let recipient = &nodes[2];
+
+	let recipient_id = vec![42; 32];
+	let inv_server_paths =
+		server.node.blinded_paths_for_async_recipient(recipient_id.clone(), None).unwrap();
+	recipient.node.set_paths_to_static_invoice_server(inv_server_paths).unwrap();
+
+	let static_invoice =
+		pass_static_invoice_server_messages(server, recipient, recipient_id.clone()).invoice;
+	let offer = recipient.node.get_async_receive_offer().unwrap();
+
+	let amt_msat = 5000;
+	let payment_id = PaymentId([1; 32]);
+	let params = RouteParametersConfig::default();
+	sender
+		.node
+		.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), params)
+		.unwrap();
+
+	let invreq_om =
+		sender.onion_messenger.next_onion_message_for_peer(server.node.get_our_node_id()).unwrap();
+	server.onion_messenger.handle_onion_message(sender.node.get_our_node_id(), &invreq_om);
+
+	let mut events = server.node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	let reply_path = match events.pop().unwrap() {
+		Event::StaticInvoiceRequested { reply_path, .. } => reply_path,
+		_ => panic!(),
+	};
+
+	server.node.send_static_invoice(static_invoice.clone(), reply_path).unwrap();
+	let static_invoice_om =
+		server.onion_messenger.next_onion_message_for_peer(sender.node.get_our_node_id()).unwrap();
+
+	// We handle the static invoice to held the pending HTLC
+	sender.onion_messenger.handle_onion_message(server.node.get_our_node_id(), &static_invoice_om);
+
+	// We advance enough time to expire the payment.
+	// We add 2 hours as is the margin added to remove stale payments in non-std implementation.
+	let timeout_time_expiry = TEST_ASYNC_PAYMENT_TIMEOUT_RELATIVE_EXPIRY
+		+ Duration::from_secs(7200)
+		+ Duration::from_secs(1);
+	advance_time_by(timeout_time_expiry, sender);
+	sender.node.timer_tick_occurred();
+	let events = sender.node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	match events[0] {
+		Event::PaymentFailed { payment_id: ev_payment_id, reason, .. } => {
+			assert_eq!(reason.unwrap(), PaymentFailureReason::PaymentExpired);
+			assert_eq!(ev_payment_id, payment_id);
+		},
+		_ => panic!(),
+	}
+}
+
 #[test]
 fn async_receive_mpp() {
 	let chanmon_cfgs = create_chanmon_cfgs(4);

lightning/src/ln/outbound_payment.rs

@@ -54,6 +54,15 @@ use crate::sync::Mutex;
 /// [`ChannelManager::timer_tick_occurred`]: crate::ln::channelmanager::ChannelManager::timer_tick_occurred
 pub(crate) const IDEMPOTENCY_TIMEOUT_TICKS: u8 = 7;
 
+#[cfg(async_payments)]
+/// The default relative expiration to wait for a pending outbound HTLC to a often-offline
+/// payee to fulfill.
+const ASYNC_PAYMENT_TIMEOUT_RELATIVE_EXPIRY: Duration = Duration::from_secs(60 * 60 * 24 * 7);
+
+#[cfg(all(async_payments, test))]
+pub(crate) const TEST_ASYNC_PAYMENT_TIMEOUT_RELATIVE_EXPIRY: Duration =
+	ASYNC_PAYMENT_TIMEOUT_RELATIVE_EXPIRY;
+
 /// 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.
 pub(crate) enum PendingOutboundPayment {
@@ -98,6 +107,11 @@ pub(crate) enum PendingOutboundPayment {
 		route_params: RouteParameters,
 		invoice_request: InvoiceRequest,
 		static_invoice: StaticInvoice,
+		// The deadline as duration since the Unix epoch for the async recipient to come online,
+		// after which we'll fail the payment.
+		//
+		// Defaults to [`ASYNC_PAYMENT_TIMEOUT_RELATIVE_EXPIRY`].
+		expiry_time: Duration,
 	},
 	Retryable {
 		retry_strategy: Option<Retry>,
@@ -1164,6 +1178,7 @@ impl OutboundPayments {
 						abandon_with_entry!(entry, PaymentFailureReason::RouteNotFound);
 						return Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::OnionPacketSizeExceeded))
 					}
+					let absolute_expiry = duration_since_epoch.saturating_add(ASYNC_PAYMENT_TIMEOUT_RELATIVE_EXPIRY);
 
 					*entry.into_mut() = PendingOutboundPayment::StaticInvoiceReceived {
 						payment_hash,
@@ -1176,6 +1191,7 @@ impl OutboundPayments {
 							.ok_or(Bolt12PaymentError::UnexpectedInvoice)?
 							.invoice_request,
 						static_invoice: invoice.clone(),
+						expiry_time: absolute_expiry,
 					};
 					return Ok(())
 				},
@@ -2278,11 +2294,12 @@ impl OutboundPayments {
 					true
 				}
 			},
-			PendingOutboundPayment::StaticInvoiceReceived { route_params, payment_hash, .. } => {
-				let is_stale =
+			PendingOutboundPayment::StaticInvoiceReceived { route_params, payment_hash, expiry_time, .. } => {
+				let is_stale = *expiry_time < duration_since_epoch;
+				let is_static_invoice_stale =
 					route_params.payment_params.expiry_time.unwrap_or(u64::MAX) <
 					duration_since_epoch.as_secs();
-				if is_stale {
+				if is_stale || is_static_invoice_stale {
 					let fail_ev = events::Event::PaymentFailed {
 						payment_id: *payment_id,
 						payment_hash: Some(*payment_hash),
@@ -2697,6 +2714,9 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(6, route_params, required),
 		(8, invoice_request, required),
 		(10, static_invoice, required),
+		// Added in 0.2. Prior versions would have this TLV type defaulted to 0, which is safe because
+		// the type is not used.
+		(11, expiry_time, (default_value, Duration::from_secs(0))),
 	},
 	// Added in 0.1. Prior versions will drop these outbounds on downgrade, which is safe because
 	// no HTLCs are in-flight.
@@ -3347,6 +3367,7 @@ mod tests {
 			route_params,
 			invoice_request: dummy_invoice_request(),
 			static_invoice: dummy_static_invoice(),
+			expiry_time: Duration::from_secs(absolute_expiry + 2),
 		};
 		outbounds.insert(payment_id, outbound);
 		core::mem::drop(outbounds);
@@ -3396,6 +3417,7 @@ mod tests {
 			route_params,
 			invoice_request: dummy_invoice_request(),
 			static_invoice: dummy_static_invoice(),
+			expiry_time: now(),
 		};
 		outbounds.insert(payment_id, outbound);
 		core::mem::drop(outbounds);

pending_changelog/3918-expiry-time-when-waiting-often-offline-peer-asyncpayment.txt

@@ -0,0 +1,4 @@
+ ## API Updates (0.2)
+
+* Upgrading to v0.2.0 will timeout any pending async payment waiting for the often offline peer
+  come online.