Remove invoice_id from static invoice server protocol

lightning/src/blinded_path/message.rs

@@ -353,7 +353,7 @@ pub enum OffersContext {
 	StaticInvoiceRequested {
 		/// An identifier for the async recipient for whom we as a static invoice server are serving
 		/// [`StaticInvoice`]s. Used paired with the
-		/// [`OffersContext::StaticInvoiceRequested::invoice_id`] when looking up a corresponding
+		/// [`OffersContext::StaticInvoiceRequested::invoice_slot`] when looking up a corresponding
 		/// [`StaticInvoice`] to return to the payer if the recipient is offline. This id was previously
 		/// provided via [`AsyncPaymentsContext::ServeStaticInvoice::recipient_id`].
 		///
@@ -364,15 +364,15 @@ pub enum OffersContext {
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 		recipient_id: Vec<u8>,
 
-		/// A random unique identifier for a specific [`StaticInvoice`] that the recipient previously
+		/// The slot number for a specific [`StaticInvoice`] that the recipient previously
 		/// requested be served on their behalf. Useful when paired with the
 		/// [`OffersContext::StaticInvoiceRequested::recipient_id`] to pull that specific invoice from
 		/// the database when payers send an [`InvoiceRequest`]. This id was previously
-		/// provided via [`AsyncPaymentsContext::ServeStaticInvoice::invoice_id`].
+		/// provided via [`AsyncPaymentsContext::ServeStaticInvoice::invoice_slot`].
 		///
 		/// [`StaticInvoice`]: crate::offers::static_invoice::StaticInvoice
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
-		invoice_id: u128,
+		invoice_slot: u16,
 
 		/// The time as duration since the Unix epoch at which this path expires and messages sent over
 		/// it should be ignored.
@@ -448,6 +448,14 @@ pub enum AsyncPaymentsContext {
 	/// [`OfferPathsRequest`]: crate::onion_message::async_payments::OfferPathsRequest
 	/// [`OfferPaths`]: crate::onion_message::async_payments::OfferPaths
 	OfferPaths {
+		/// The "slot" in the static invoice server's database that the invoice corresponding to these
+		/// offer paths should go into, originally set by us in [`OfferPathsRequest::invoice_slot`]. This
+		/// value allows us as the recipient to replace a specific invoice that is stored by the server,
+		/// which is useful for limiting the number of invoices stored by the server while also keeping
+		/// all the invoices persisted with the server fresh.
+		///
+		/// [`OfferPathsRequest::invoice_slot`]: crate::onion_message::async_payments::OfferPathsRequest::invoice_slot
+		invoice_slot: u16,
 		/// The time as duration since the Unix epoch at which this path expires and messages sent over
 		/// it should be ignored.
 		///
@@ -466,7 +474,7 @@ pub enum AsyncPaymentsContext {
 		/// An identifier for the async recipient that is requesting that a [`StaticInvoice`] be served
 		/// on their behalf.
 		///
-		/// Useful when surfaced alongside the below `invoice_id` when payers send an
+		/// Useful when surfaced alongside the below `invoice_slot` when payers send an
 		/// [`InvoiceRequest`], to pull the specific static invoice from the database.
 		///
 		/// Also useful to rate limit the invoices being persisted on behalf of a particular recipient.
@@ -477,15 +485,15 @@ pub enum AsyncPaymentsContext {
 		/// [`StaticInvoice`]: crate::offers::static_invoice::StaticInvoice
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 		recipient_id: Vec<u8>,
-		/// A random identifier for the specific [`StaticInvoice`] that the recipient is requesting be
+		/// The slot number for the specific [`StaticInvoice`] that the recipient is requesting be
 		/// served on their behalf. Useful when surfaced alongside the above `recipient_id` when payers
 		/// send an [`InvoiceRequest`], to pull the specific static invoice from the database. This id
 		/// will be provided back to us as the static invoice server via
-		/// [`OffersContext::StaticInvoiceRequested::invoice_id`].
+		/// [`OffersContext::StaticInvoiceRequested::invoice_slot`].
 		///
 		/// [`StaticInvoice`]: crate::offers::static_invoice::StaticInvoice
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
-		invoice_id: u128,
+		invoice_slot: u16,
 		/// The time as duration since the Unix epoch at which this path expires and messages sent over
 		/// it should be ignored.
 		///
@@ -559,7 +567,7 @@ impl_writeable_tlv_based_enum!(OffersContext,
 	},
 	(3, StaticInvoiceRequested) => {
 		(0, recipient_id, required),
-		(2, invoice_id, required),
+		(2, invoice_slot, required),
 		(4, path_absolute_expiry, required),
 	},
 );
@@ -573,6 +581,7 @@ impl_writeable_tlv_based_enum!(AsyncPaymentsContext,
 	},
 	(2, OfferPaths) => {
 		(0, path_absolute_expiry, required),
+		(2, invoice_slot, required),
 	},
 	(3, StaticInvoicePersisted) => {
 		(0, offer_id, required),
@@ -584,7 +593,7 @@ impl_writeable_tlv_based_enum!(AsyncPaymentsContext,
 	},
 	(5, ServeStaticInvoice) => {
 		(0, recipient_id, required),
-		(2, invoice_id, required),
+		(2, invoice_slot, required),
 		(4, path_absolute_expiry, required),
 	},
 );

lightning/src/events/mod.rs

@@ -1661,18 +1661,11 @@ pub enum Event {
 		/// [`ChannelManager::blinded_paths_for_async_recipient`].
 		///
 		/// When an [`Event::StaticInvoiceRequested`] comes in for the invoice, this id will be surfaced
-		/// and can be used alongside the `invoice_id` to retrieve the invoice from the database.
+		/// and can be used alongside the `invoice_slot` to retrieve the invoice from the database.
 		///
 		///[`ChannelManager::blinded_paths_for_async_recipient`]: crate::ln::channelmanager::ChannelManager::blinded_paths_for_async_recipient
 		recipient_id: Vec<u8>,
-		/// A random identifier for the invoice. When an [`Event::StaticInvoiceRequested`] comes in for
-		/// the invoice, this id will be surfaced and can be used alongside the `recipient_id` to
-		/// retrieve the invoice from the database.
-		///
-		/// Note that this id will remain the same for all invoice updates corresponding to a particular
-		/// offer that the recipient has cached.
-		invoice_id: u128,
-		/// Once the [`StaticInvoice`], `invoice_slot` and `invoice_id` are persisted,
+		/// Once the [`StaticInvoice`] and `invoice_slot` are persisted,
 		/// [`ChannelManager::static_invoice_persisted`] should be called with this responder to confirm
 		/// to the recipient that their [`Offer`] is ready to be used for async payments.
 		///
@@ -1688,7 +1681,7 @@ pub enum Event {
 	/// them via [`ChannelManager::set_paths_to_static_invoice_server`].
 	///
 	/// If we previously persisted a [`StaticInvoice`] from an [`Event::PersistStaticInvoice`] that
-	/// matches the below `recipient_id` and `invoice_id`, that invoice should be retrieved now
+	/// matches the below `recipient_id` and `invoice_slot`, that invoice should be retrieved now
 	/// and forwarded to the payer via [`ChannelManager::send_static_invoice`].
 	///
 	/// [`ChannelManager::blinded_paths_for_async_recipient`]: crate::ln::channelmanager::ChannelManager::blinded_paths_for_async_recipient
@@ -1697,13 +1690,13 @@ pub enum Event {
 	/// [`ChannelManager::send_static_invoice`]: crate::ln::channelmanager::ChannelManager::send_static_invoice
 	StaticInvoiceRequested {
 		/// An identifier for the recipient previously surfaced in
-		/// [`Event::PersistStaticInvoice::recipient_id`]. Useful when paired with the `invoice_id` to
+		/// [`Event::PersistStaticInvoice::recipient_id`]. Useful when paired with the `invoice_slot` to
 		/// retrieve the [`StaticInvoice`] requested by the payer.
 		recipient_id: Vec<u8>,
-		/// A random identifier for the invoice being requested, previously surfaced in
-		/// [`Event::PersistStaticInvoice::invoice_id`]. Useful when paired with the `recipient_id` to
+		/// The slot number for the invoice being requested, previously surfaced in
+		/// [`Event::PersistStaticInvoice::invoice_slot`]. Useful when paired with the `recipient_id` to
 		/// retrieve the [`StaticInvoice`] requested by the payer.
-		invoice_id: u128,
+		invoice_slot: u16,
 		/// The path over which the [`StaticInvoice`] will be sent to the payer, which should be
 		/// provided to [`ChannelManager::send_static_invoice`] along with the invoice.
 		///

lightning/src/ln/async_payments_tests.rs

@@ -66,7 +66,6 @@ use core::time::Duration;
 struct StaticInvoiceServerFlowResult {
 	invoice: StaticInvoice,
 	invoice_slot: u16,
-	invoice_id: u128,
 
 	// Returning messages that were sent along the way allows us to test handling duplicate messages.
 	offer_paths_request: msgs::OnionMessage,
@@ -148,16 +147,15 @@ fn pass_static_invoice_server_messages(
 	// that the static invoice should be persisted.
 	let mut events = server.node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);
-	let (invoice, invoice_slot, invoice_id, ack_path) = match events.pop().unwrap() {
+	let (invoice, invoice_slot, ack_path) = match events.pop().unwrap() {
 		Event::PersistStaticInvoice {
 			invoice,
 			invoice_persisted_path,
 			recipient_id: ev_id,
 			invoice_slot,
-			invoice_id,
 		} => {
 			assert_eq!(recipient_id, ev_id);
-			(invoice, invoice_slot, invoice_id, invoice_persisted_path)
+			(invoice, invoice_slot, invoice_persisted_path)
 		},
 		_ => panic!(),
 	};
@@ -183,7 +181,6 @@ fn pass_static_invoice_server_messages(
 		static_invoice_persisted_message: invoice_persisted_om,
 		invoice,
 		invoice_slot,
-		invoice_id,
 	}
 }
 
@@ -209,7 +206,7 @@ fn pass_async_payments_oms(
 	let mut events = always_online_recipient_counterparty.node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);
 	let reply_path = match events.pop().unwrap() {
-		Event::StaticInvoiceRequested { recipient_id: ev_id, invoice_id: _, reply_path } => {
+		Event::StaticInvoiceRequested { recipient_id: ev_id, invoice_slot: _, reply_path } => {
 			assert_eq!(recipient_id, ev_id);
 			reply_path
 		},
@@ -573,7 +570,7 @@ fn ignore_unexpected_static_invoice() {
 	let mut events = nodes[1].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);
 	let reply_path = match events.pop().unwrap() {
-		Event::StaticInvoiceRequested { recipient_id: ev_id, invoice_id: _, reply_path } => {
+		Event::StaticInvoiceRequested { recipient_id: ev_id, invoice_slot: _, reply_path } => {
 			assert_eq!(recipient_id, ev_id);
 			reply_path
 		},
@@ -1626,13 +1623,13 @@ fn limit_serve_static_invoice_requests() {
 
 	// Build the target number of offers interactively with the static invoice server.
 	let mut offer_paths_req = None;
-	let mut invoice_ids = new_hash_set();
+	let mut invoice_slots = new_hash_set();
 	for expected_inv_slot in 0..TEST_MAX_CACHED_OFFERS_TARGET {
 		let flow_res = pass_static_invoice_server_messages(server, recipient, recipient_id.clone());
 		assert_eq!(flow_res.invoice_slot, expected_inv_slot as u16);
 
 		offer_paths_req = Some(flow_res.offer_paths_request);
-		invoice_ids.insert(flow_res.invoice_id);
+		invoice_slots.insert(flow_res.invoice_slot);
 
 		// Trigger a cache refresh
 		recipient.node.timer_tick_occurred();
@@ -1641,8 +1638,8 @@ fn limit_serve_static_invoice_requests() {
 		recipient.node.flow.test_get_async_receive_offers().len(),
 		TEST_MAX_CACHED_OFFERS_TARGET
 	);
-	// Check that all invoice ids are unique.
-	assert_eq!(invoice_ids.len(), TEST_MAX_CACHED_OFFERS_TARGET);
+	// Check that all invoice slot numbers are unique.
+	assert_eq!(invoice_slots.len(), TEST_MAX_CACHED_OFFERS_TARGET);
 
 	// Force allowing more offer paths request attempts so we can check that the recipient will not
 	// attempt to build any further offers.
@@ -1822,16 +1819,15 @@ fn refresh_static_invoices_for_used_offers() {
 		Event::PersistStaticInvoice {
 			invoice,
 			invoice_slot,
-			invoice_id,
 			invoice_persisted_path,
 			recipient_id: ev_id,
 		} => {
 			assert_ne!(original_invoice, invoice);
 			assert_eq!(recipient_id, ev_id);
 			assert_eq!(invoice_slot, flow_res.invoice_slot);
-			// When we update the invoice corresponding to a specific offer, the invoice_id stays the
+			// When we update the invoice corresponding to a specific offer, the invoice_slot stays the
 			// same.
-			assert_eq!(invoice_id, flow_res.invoice_id);
+			assert_eq!(invoice_slot, flow_res.invoice_slot);
 			(invoice, invoice_persisted_path)
 		},
 		_ => panic!(),

lightning/src/ln/channelmanager.rs

@@ -14317,11 +14317,9 @@ where
 
 				let invoice_request = match self.flow.verify_invoice_request(invoice_request, context) {
 					Ok(InvreqResponseInstructions::SendInvoice(invoice_request)) => invoice_request,
-					Ok(InvreqResponseInstructions::SendStaticInvoice {
-						recipient_id: _recipient_id, invoice_id: _invoice_id
-					}) => {
+					Ok(InvreqResponseInstructions::SendStaticInvoice { recipient_id, invoice_slot }) => {
 						self.pending_events.lock().unwrap().push_back((Event::StaticInvoiceRequested {
-							recipient_id: _recipient_id, invoice_id: _invoice_id, reply_path: responder
+							recipient_id, invoice_slot, reply_path: responder
 						}, None));
 
 						return None
@@ -14443,29 +14441,28 @@ where
 	L::Target: Logger,
 {
 	fn handle_offer_paths_request(
-		&self, _message: OfferPathsRequest, _context: AsyncPaymentsContext,
-		_responder: Option<Responder>,
+		&self, message: OfferPathsRequest, context: AsyncPaymentsContext,
+		responder: Option<Responder>,
 	) -> Option<(OfferPaths, ResponseInstruction)> {
 		let peers = self.get_peers_for_blinded_path();
-		let entropy = &*self.entropy_source;
 		let (message, reply_path_context) =
-			match self.flow.handle_offer_paths_request(_context, peers, entropy) {
+			match self.flow.handle_offer_paths_request(&message, context, peers) {
 				Some(msg) => msg,
 				None => return None,
 			};
-		_responder.map(|resp| (message, resp.respond_with_reply_path(reply_path_context)))
+		responder.map(|resp| (message, resp.respond_with_reply_path(reply_path_context)))
 	}
 
 	fn handle_offer_paths(
-		&self, _message: OfferPaths, _context: AsyncPaymentsContext, _responder: Option<Responder>,
+		&self, message: OfferPaths, context: AsyncPaymentsContext, responder: Option<Responder>,
 	) -> Option<(ServeStaticInvoice, ResponseInstruction)> {
-		let responder = match _responder {
+		let responder = match responder {
 			Some(responder) => responder,
 			None => return None,
 		};
 		let (serve_static_invoice, reply_context) = match self.flow.handle_offer_paths(
-			_message,
-			_context,
+			message,
+			context,
 			responder.clone(),
 			self.get_peers_for_blinded_path(),
 			self.list_usable_channels(),
@@ -14484,52 +14481,51 @@ where
 	}
 
 	fn handle_serve_static_invoice(
-		&self, _message: ServeStaticInvoice, _context: AsyncPaymentsContext,
-		_responder: Option<Responder>,
+		&self, message: ServeStaticInvoice, context: AsyncPaymentsContext,
+		responder: Option<Responder>,
 	) {
-		let responder = match _responder {
+		let responder = match responder {
 			Some(resp) => resp,
 			None => return,
 		};
 
-		let (recipient_id, invoice_id) =
-			match self.flow.verify_serve_static_invoice_message(&_message, _context) {
-				Ok((recipient_id, inv_id)) => (recipient_id, inv_id),
+		let (recipient_id, invoice_slot) =
+			match self.flow.verify_serve_static_invoice_message(&message, context) {
+				Ok((recipient_id, inv_slot)) => (recipient_id, inv_slot),
 				Err(()) => return,
 			};
 
 		let mut pending_events = self.pending_events.lock().unwrap();
 		pending_events.push_back((
 			Event::PersistStaticInvoice {
-				invoice: _message.invoice,
-				invoice_slot: _message.invoice_slot,
+				invoice: message.invoice,
+				invoice_slot,
 				recipient_id,
-				invoice_id,
 				invoice_persisted_path: responder,
 			},
 			None,
 		));
 	}
 
 	fn handle_static_invoice_persisted(
-		&self, _message: StaticInvoicePersisted, _context: AsyncPaymentsContext,
+		&self, _message: StaticInvoicePersisted, context: AsyncPaymentsContext,
 	) {
-		let should_persist = self.flow.handle_static_invoice_persisted(_context);
+		let should_persist = self.flow.handle_static_invoice_persisted(context);
 		if should_persist {
 			let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 		}
 	}
 
 	fn handle_held_htlc_available(
-		&self, _message: HeldHtlcAvailable, _context: AsyncPaymentsContext,
-		_responder: Option<Responder>,
+		&self, _message: HeldHtlcAvailable, context: AsyncPaymentsContext,
+		responder: Option<Responder>,
 	) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> {
-		self.flow.verify_inbound_async_payment_context(_context).ok()?;
-		return _responder.map(|responder| (ReleaseHeldHtlc {}, responder.respond()));
+		self.flow.verify_inbound_async_payment_context(context).ok()?;
+		return responder.map(|responder| (ReleaseHeldHtlc {}, responder.respond()));
 	}
 
-	fn handle_release_held_htlc(&self, _message: ReleaseHeldHtlc, _context: AsyncPaymentsContext) {
-		let payment_id = match _context {
+	fn handle_release_held_htlc(&self, _message: ReleaseHeldHtlc, context: AsyncPaymentsContext) {
+		let payment_id = match context {
 			AsyncPaymentsContext::OutboundPayment { payment_id } => payment_id,
 			_ => return,
 		};