Async recipient: track static invoice creation time

Start tracking invoice creation time in cached async offers. This field will be
used in the next commit to start only updating static invoices for Used offers
every few hours instead of once a minute.

We also remove the blinded path context
StaticInvoicePersisted::path_absolute_expiry field here, replacing it with the
new invoice_created_at field. We don't actually want to terminate early if the
reply path a bit stale like we did before, since we want to use the
invoice_created_at field regardless to drive a faster refresh of the invoice.

Author: valentinewallace

lightning/src/blinded_path/message.rs

@@ -506,10 +506,9 @@ pub enum AsyncPaymentsContext {
 		/// [`StaticInvoice`]: crate::offers::static_invoice::StaticInvoice
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 		offer_id: OfferId,
-		/// The time as duration since the Unix epoch at which this path expires and messages sent over
-		/// it should be ignored. If we receive confirmation of an invoice over this path after its
-		/// expiry, it may be outdated and a new invoice update should be sent instead.
-		path_absolute_expiry: core::time::Duration,
+		/// The time as duration since the Unix epoch at which the invoice corresponding to this path
+		/// was created. Useful to know when an invoice needs replacement.
+		invoice_created_at: core::time::Duration,
 	},
 	/// Context contained within the reply [`BlindedMessagePath`] we put in outbound
 	/// [`HeldHtlcAvailable`] messages, provided back to us in corresponding [`ReleaseHeldHtlc`]
@@ -577,7 +576,7 @@ impl_writeable_tlv_based_enum!(AsyncPaymentsContext,
 	},
 	(3, StaticInvoicePersisted) => {
 		(0, offer_id, required),
-		(2, path_absolute_expiry, required),
+		(2, invoice_created_at, required),
 	},
 	(4, OfferPathsRequest) => {
 		(0, recipient_id, required),

lightning/src/ln/async_payments_tests.rs

@@ -1554,54 +1554,6 @@ fn ignore_expired_offer_paths_message() {
 		.is_none());
 }
 
-#[cfg_attr(feature = "std", ignore)]
-#[test]
-fn ignore_expired_invoice_persisted_message() {
-	// If the recipient receives a static_invoice_persisted message over an expired reply path, it
-	// should be ignored.
-	let chanmon_cfgs = create_chanmon_cfgs(2);
-	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
-	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-	create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
-	let server = &nodes[0];
-	let recipient = &nodes[1];
-
-	let recipient_id = vec![42; 32];
-	let inv_server_paths =
-		server.node.blinded_paths_for_async_recipient(recipient_id, None).unwrap();
-	recipient.node.set_paths_to_static_invoice_server(inv_server_paths).unwrap();
-
-	// Exchange messages until we can extract the final static_invoice_persisted OM.
-	recipient.node.timer_tick_occurred();
-	let serve_static_invoice = invoice_flow_up_to_send_serve_static_invoice(server, recipient).1;
-	server
-		.onion_messenger
-		.handle_onion_message(recipient.node.get_our_node_id(), &serve_static_invoice);
-	let mut events = server.node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	let ack_path = match events.pop().unwrap() {
-		Event::PersistStaticInvoice { invoice_persisted_path, .. } => invoice_persisted_path,
-		_ => panic!(),
-	};
-
-	server.node.static_invoice_persisted(ack_path);
-	let invoice_persisted = server
-		.onion_messenger
-		.next_onion_message_for_peer(recipient.node.get_our_node_id())
-		.unwrap();
-	assert!(matches!(
-		recipient.onion_messenger.peel_onion_message(&invoice_persisted).unwrap(),
-		PeeledOnion::AsyncPayments(AsyncPaymentsMessage::StaticInvoicePersisted(_), _, _)
-	));
-
-	advance_time_by(TEST_TEMP_REPLY_PATH_RELATIVE_EXPIRY + Duration::from_secs(1), recipient);
-	recipient
-		.onion_messenger
-		.handle_onion_message(server.node.get_our_node_id(), &invoice_persisted);
-	assert!(recipient.node.get_async_receive_offer().is_err());
-}
-
 #[test]
 fn limit_offer_paths_requests() {
 	// Limit the number of offer_paths_requests sent to the server if they aren't responding.

lightning/src/offers/async_receive_offer_cache.rs

@@ -30,12 +30,20 @@ use crate::blinded_path::message::AsyncPaymentsContext;
 enum OfferStatus {
 	/// This offer has been returned to the user from the cache, so it needs to be stored until it
 	/// expires and its invoice needs to be kept updated.
-	Used,
+	Used {
+		/// The creation time of the invoice that was last confirmed as persisted by the server. Useful
+		/// to know when the invoice needs refreshing.
+		invoice_created_at: Duration,
+	},
 	/// This offer has not yet been returned to the user, and is safe to replace to ensure we always
 	/// have a maximally fresh offer. We always want to have at least 1 offer in this state,
 	/// preferably a few so we can respond to user requests for new offers without returning the same
 	/// one multiple times. Returning a new offer each time is better for privacy.
-	Ready,
+	Ready {
+		/// The creation time of the invoice that was last confirmed as persisted by the server. Useful
+		/// to know when the invoice needs refreshing.
+		invoice_created_at: Duration,
+	},
 	/// This offer's invoice is not yet confirmed as persisted by the static invoice server, so it is
 	/// not yet ready to receive payments.
 	Pending,
@@ -60,8 +68,12 @@ struct AsyncReceiveOffer {
 }
 
 impl_writeable_tlv_based_enum!(OfferStatus,
-	(0, Used) => {},
-	(1, Ready) => {},
+	(0, Used) => {
+		(0, invoice_created_at, required),
+	},
+	(1, Ready) => {
+		(0, invoice_created_at, required),
+	},
 	(2, Pending) => {},
 );
 
@@ -216,16 +228,18 @@ impl AsyncReceiveOfferCache {
 		// Find the freshest unused offer. See `OfferStatus::Ready`.
 		let newest_unused_offer_opt = self
 			.unused_ready_offers()
-			.max_by(|(_, offer_a), (_, offer_b)| offer_a.created_at.cmp(&offer_b.created_at))
-			.map(|(idx, offer)| (idx, offer.offer.clone()));
-		if let Some((idx, newest_ready_offer)) = newest_unused_offer_opt {
-			self.offers[idx].as_mut().map(|offer| offer.status = OfferStatus::Used);
+			.max_by(|(_, offer_a, _), (_, offer_b, _)| offer_a.created_at.cmp(&offer_b.created_at))
+			.map(|(idx, offer, invoice_created_at)| (idx, offer.offer.clone(), invoice_created_at));
+		if let Some((idx, newest_ready_offer, invoice_created_at)) = newest_unused_offer_opt {
+			self.offers[idx]
+				.as_mut()
+				.map(|offer| offer.status = OfferStatus::Used { invoice_created_at });
 			return Ok((newest_ready_offer, true));
 		}
 
 		// If no unused offers are available, return the used offer with the latest absolute expiry
 		self.offers_with_idx()
-			.filter(|(_, offer)| matches!(offer.status, OfferStatus::Used))
+			.filter(|(_, offer)| matches!(offer.status, OfferStatus::Used { .. }))
 			.max_by(|a, b| {
 				let abs_expiry_a = a.1.offer.absolute_expiry().unwrap_or(Duration::MAX);
 				let abs_expiry_b = b.1.offer.absolute_expiry().unwrap_or(Duration::MAX);
@@ -338,9 +352,9 @@ impl AsyncReceiveOfferCache {
 		}
 
 		// If all of our offers are already used or pending, then none are available to be replaced
-		let no_replaceable_offers = self
-			.offers_with_idx()
-			.all(|(_, offer)| matches!(offer.status, OfferStatus::Used | OfferStatus::Pending));
+		let no_replaceable_offers = self.offers_with_idx().all(|(_, offer)| {
+			matches!(offer.status, OfferStatus::Used { .. } | OfferStatus::Pending)
+		});
 		if no_replaceable_offers {
 			return None;
 		}
@@ -350,7 +364,7 @@ impl AsyncReceiveOfferCache {
 		let num_payable_offers = self
 			.offers_with_idx()
 			.filter(|(_, offer)| {
-				matches!(offer.status, OfferStatus::Used | OfferStatus::Ready { .. })
+				matches!(offer.status, OfferStatus::Used { .. } | OfferStatus::Ready { .. })
 			})
 			.count();
 		if num_payable_offers <= 1 {
@@ -361,10 +375,10 @@ impl AsyncReceiveOfferCache {
 		// were last updated, so they are stale enough to warrant replacement.
 		let awhile_ago = duration_since_epoch.saturating_sub(OFFER_REFRESH_THRESHOLD);
 		self.unused_ready_offers()
-			.filter(|(_, offer)| offer.created_at < awhile_ago)
+			.filter(|(_, offer, _)| offer.created_at < awhile_ago)
 			// Get the stalest offer and return its index
-			.min_by(|(_, offer_a), (_, offer_b)| offer_a.created_at.cmp(&offer_b.created_at))
-			.map(|(idx, _)| idx)
+			.min_by(|(_, offer_a, _), (_, offer_b, _)| offer_a.created_at.cmp(&offer_b.created_at))
+			.map(|(idx, _, _)| idx)
 	}
 
 	/// Returns an iterator over (offer_idx, offer)
@@ -378,11 +392,11 @@ impl AsyncReceiveOfferCache {
 		})
 	}
 
-	/// Returns an iterator over (offer_idx, offer) where all returned offers are
+	/// Returns an iterator over (offer_idx, offer, invoice_created_at) where all returned offers are
 	/// [`OfferStatus::Ready`]
-	fn unused_ready_offers(&self) -> impl Iterator<Item = (usize, &AsyncReceiveOffer)> {
+	fn unused_ready_offers(&self) -> impl Iterator<Item = (usize, &AsyncReceiveOffer, Duration)> {
 		self.offers_with_idx().filter_map(|(idx, offer)| match offer.status {
-			OfferStatus::Ready => Some((idx, offer)),
+			OfferStatus::Ready { invoice_created_at } => Some((idx, offer, invoice_created_at)),
 			_ => None,
 		})
 	}
@@ -408,7 +422,7 @@ impl AsyncReceiveOfferCache {
 		// them a fresh invoice on each timer tick.
 		self.offers_with_idx().filter_map(|(idx, offer)| {
 			let needs_invoice_update =
-				offer.status == OfferStatus::Used || offer.status == OfferStatus::Pending;
+				matches!(offer.status, OfferStatus::Used { .. } | OfferStatus::Pending);
 			if needs_invoice_update {
 				let offer_slot = idx.try_into().unwrap_or(u16::MAX);
 				Some((
@@ -431,29 +445,25 @@ impl AsyncReceiveOfferCache {
 	/// is needed.
 	///
 	/// [`StaticInvoicePersisted`]: crate::onion_message::async_payments::StaticInvoicePersisted
-	pub(super) fn static_invoice_persisted(
-		&mut self, context: AsyncPaymentsContext, duration_since_epoch: Duration,
-	) -> bool {
-		let offer_id = match context {
-			AsyncPaymentsContext::StaticInvoicePersisted { path_absolute_expiry, offer_id } => {
-				if duration_since_epoch > path_absolute_expiry {
-					return false;
-				}
-				offer_id
+	pub(super) fn static_invoice_persisted(&mut self, context: AsyncPaymentsContext) -> bool {
+		let (invoice_created_at, offer_id) = match context {
+			AsyncPaymentsContext::StaticInvoicePersisted { invoice_created_at, offer_id } => {
+				(invoice_created_at, offer_id)
 			},
 			_ => return false,
 		};
 
 		let mut offers = self.offers.iter_mut();
 		let offer_entry = offers.find(|o| o.as_ref().map_or(false, |o| o.offer.id() == offer_id));
 		if let Some(Some(ref mut offer)) = offer_entry {
-			if offer.status == OfferStatus::Used {
-				// We succeeded in updating the invoice for a used offer, no re-persistence of the cache
-				// needed
-				return false;
+			match offer.status {
+				OfferStatus::Used { invoice_created_at: ref mut inv_created_at }
+				| OfferStatus::Ready { invoice_created_at: ref mut inv_created_at } => {
+					*inv_created_at = core::cmp::min(invoice_created_at, *inv_created_at);
+				},
+				OfferStatus::Pending => offer.status = OfferStatus::Ready { invoice_created_at },
 			}
 
-			offer.status = OfferStatus::Ready;
 			return true;
 		}
 
@@ -465,7 +475,7 @@ impl AsyncReceiveOfferCache {
 		self.offers_with_idx()
 			.filter_map(|(_, offer)| {
 				if matches!(offer.status, OfferStatus::Ready { .. })
-					|| matches!(offer.status, OfferStatus::Used)
+					|| matches!(offer.status, OfferStatus::Used { .. })
 				{
 					Some(offer.offer.clone())
 				} else {

lightning/src/offers/flow.rs

@@ -1331,7 +1331,6 @@ where
 		ES::Target: EntropySource,
 		R::Target: Router,
 	{
-		let duration_since_epoch = self.duration_since_epoch();
 		let mut serve_static_invoice_msgs = Vec::new();
 		{
 			let cache = self.async_receive_offer_cache.lock().unwrap();
@@ -1352,10 +1351,8 @@ where
 				};
 
 				let reply_path_context = {
-					let path_absolute_expiry =
-						duration_since_epoch.saturating_add(TEMP_REPLY_PATH_RELATIVE_EXPIRY);
 					MessageContext::AsyncPayments(AsyncPaymentsContext::StaticInvoicePersisted {
-						path_absolute_expiry,
+						invoice_created_at: invoice.created_at(),
 						offer_id: offer.id(),
 					})
 				};
@@ -1533,11 +1530,9 @@ where
 		};
 
 		let reply_path_context = {
-			let path_absolute_expiry =
-				duration_since_epoch.saturating_add(TEMP_REPLY_PATH_RELATIVE_EXPIRY);
 			MessageContext::AsyncPayments(AsyncPaymentsContext::StaticInvoicePersisted {
 				offer_id,
-				path_absolute_expiry,
+				invoice_created_at: invoice.created_at(),
 			})
 		};
 
@@ -1658,7 +1653,7 @@ where
 	#[cfg(async_payments)]
 	pub fn handle_static_invoice_persisted(&self, context: AsyncPaymentsContext) -> bool {
 		let mut cache = self.async_receive_offer_cache.lock().unwrap();
-		cache.static_invoice_persisted(context, self.duration_since_epoch())
+		cache.static_invoice_persisted(context)
 	}
 
 	/// Get the encoded [`AsyncReceiveOfferCache`] for persistence.