f - test case for paying fresh invoice first and other comments

Author: elnosh

lightning/src/events/mod.rs

@@ -1655,12 +1655,10 @@ pub enum Event {
 		/// The invoice that should be persisted and later provided to payers when handling a future
 		/// [`Event::StaticInvoiceRequested`].
 		invoice: StaticInvoice,
-		/// The path to where invoice requests will be forwarded. As a static invoice
-		/// server, if we receive an invoice request on behalf of an async recipient, a static
-		/// invoice will be provided to the payer. However, we'll also forward the invoice
-		/// request to this path to the async recipient in case it is online so that the
-		/// recipient can provide a new invoice. This path should be persisted and later
-		/// provided to [`ChannelManager::send_response_static_invoice_request`].
+		/// The path to where invoice requests will be forwarded. If we receive an invoice
+		/// request, we'll forward it to the async recipient over this path in case the
+		/// recipient is online to provide a new invoice. This path should be persisted and
+		/// later provided to [`ChannelManager::send_response_static_invoice_request`].
 		///
 		/// [`ChannelManager::send_response_static_invoice_request`]: crate::ln::channelmanager::ChannelManager::send_response_static_invoice_request
 		invoice_request_path: BlindedMessagePath,
@@ -1718,8 +1716,9 @@ pub enum Event {
 		///
 		/// [`ChannelManager::send_response_static_invoice_request`]: crate::ln::channelmanager::ChannelManager::send_response_static_invoice_request
 		reply_path: Responder,
-		/// The invoice request that will be forwarded to the async recipient to give it a
-		/// chance to provide an invoice in case it is online. It should be provided to [`ChannelManager::send_response_static_invoice_request`].
+		/// The invoice request that will be forwarded to the async recipient to give the
+		/// recipient a chance to provide an invoice in case it is online. It should be
+		/// provided to [`ChannelManager::send_response_static_invoice_request`].
 		///
 		/// [`ChannelManager::send_response_static_invoice_request`]: crate::ln::channelmanager::ChannelManager::send_response_static_invoice_request
 		invoice_request: InvoiceRequest,

lightning/src/ln/async_payments_tests.rs

@@ -742,7 +742,7 @@ fn ignore_duplicate_invoice() {
 			static_invoice.clone(),
 			reply_path,
 			invoice_request,
-			invoice_flow_res.invoice_request_path,
+			invoice_flow_res.invoice_request_path.clone(),
 		)
 		.unwrap();
 
@@ -751,17 +751,15 @@ fn ignore_duplicate_invoice() {
 	// payer.
 	let invreq_om =
 		always_online_node.onion_messenger.next_onion_message_for_peer(async_recipient_id).unwrap();
-
 	let peeled_msg = async_recipient.onion_messenger.peel_onion_message(&invreq_om).unwrap();
 	assert!(matches!(peeled_msg, PeeledOnion::Offers(OffersMessage::InvoiceRequest(_), _, _)));
 
 	let static_invoice_om =
 		always_online_node.onion_messenger.next_onion_message_for_peer(sender_node_id).unwrap();
-
 	let peeled_msg = sender.onion_messenger.peel_onion_message(&static_invoice_om).unwrap();
 	assert!(matches!(peeled_msg, PeeledOnion::Offers(OffersMessage::StaticInvoice(_), _, _)));
 
-	// Handling the `invoice_request` from the async recipient we should get back a invoice.
+	// Handling the `invoice_request` from the async recipient we should get back an invoice.
 	async_recipient.onion_messenger.handle_onion_message(always_online_node_id, &invreq_om);
 	let invoice_om =
 		async_recipient.onion_messenger.next_onion_message_for_peer(sender_node_id).unwrap();
@@ -782,7 +780,6 @@ fn ignore_duplicate_invoice() {
 
 	let release_held_htlc_om =
 		async_recipient.onion_messenger.next_onion_message_for_peer(sender_node_id).unwrap();
-
 	sender.onion_messenger.handle_onion_message(async_recipient_id, &release_held_htlc_om);
 
 	let mut events = sender.node.get_and_clear_pending_msg_events();
@@ -797,7 +794,7 @@ fn ignore_duplicate_invoice() {
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let (res, _) =
 		claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
+	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice.clone())));
 
 	// After paying the static invoice, check that regular invoice received from async recipient is ignored.
 	match sender.onion_messenger.peel_onion_message(&invoice_om) {
@@ -809,6 +806,90 @@ fn ignore_duplicate_invoice() {
 		},
 		_ => panic!(),
 	}
+
+	// Now handle case where the sender pays regular invoice and ignores static invoice.
+	let payment_id = PaymentId([2; 32]);
+	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(always_online_node_id).unwrap();
+	always_online_node.onion_messenger.handle_onion_message(sender_node_id, &invreq_om);
+
+	let mut events = always_online_node.node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	let (reply_path, invoice_request) = match events.pop().unwrap() {
+		Event::StaticInvoiceRequested {
+			recipient_id: ev_id,
+			invoice_slot: _,
+			reply_path,
+			invoice_request,
+		} => {
+			assert_eq!(recipient_id, ev_id);
+			(reply_path, invoice_request)
+		},
+		_ => panic!(),
+	};
+
+	always_online_node
+		.node
+		.send_response_static_invoice_request(
+			static_invoice.clone(),
+			reply_path,
+			invoice_request,
+			invoice_flow_res.invoice_request_path,
+		)
+		.unwrap();
+
+	let invreq_om =
+		always_online_node.onion_messenger.next_onion_message_for_peer(async_recipient_id).unwrap();
+	let peeled_msg = async_recipient.onion_messenger.peel_onion_message(&invreq_om).unwrap();
+	assert!(matches!(peeled_msg, PeeledOnion::Offers(OffersMessage::InvoiceRequest(_), _, _)));
+
+	let static_invoice_om =
+		always_online_node.onion_messenger.next_onion_message_for_peer(sender_node_id).unwrap();
+	let peeled_msg = sender.onion_messenger.peel_onion_message(&static_invoice_om).unwrap();
+	assert!(matches!(peeled_msg, PeeledOnion::Offers(OffersMessage::StaticInvoice(_), _, _)));
+
+	async_recipient.onion_messenger.handle_onion_message(always_online_node_id, &invreq_om);
+	let invoice_om =
+		async_recipient.onion_messenger.next_onion_message_for_peer(sender_node_id).unwrap();
+
+	let (invoice, context) = match sender.onion_messenger.peel_onion_message(&invoice_om) {
+		Ok(PeeledOnion::Offers(OffersMessage::Invoice(invoice), context, _)) => (invoice, context),
+		_ => panic!(),
+	};
+
+	assert!(matches!(
+		sender.node.send_payment_for_bolt12_invoice(&invoice, context.as_ref()),
+		Ok(())
+	));
+
+	let mut events = sender.node.get_and_clear_pending_msg_events();
+	assert_eq!(events.len(), 1);
+	let ev = remove_first_msg_event_to_node(&always_online_node_id, &mut events);
+	let payment_hash = extract_payment_hash(&ev);
+	check_added_monitors!(sender, 1);
+
+	let args = PassAlongPathArgs::new(sender, route[0], amt_msat, payment_hash, ev)
+		.without_clearing_recipient_events();
+	do_pass_along_path(args);
+
+	let payment_preimage = match get_event!(async_recipient, Event::PaymentClaimable) {
+		Event::PaymentClaimable { purpose, .. } => purpose.preimage().unwrap(),
+		_ => panic!("No Event::PaymentClaimable"),
+	};
+
+	// After paying invoice, check that static invoice is ignored.
+	let res = claim_payment(sender, route[0], payment_preimage);
+	assert_eq!(res, Some(PaidBolt12Invoice::Bolt12Invoice(invoice)));
+
+	sender.onion_messenger.handle_onion_message(always_online_node_id, &static_invoice_om);
+	let async_pmts_msgs = AsyncPaymentsMessageHandler::release_pending_messages(sender.node);
+	assert!(async_pmts_msgs.is_empty());
+	assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
 }
 
 #[test]

lightning/src/ln/channelmanager.rs

@@ -5292,9 +5292,10 @@ where
 		self.flow.static_invoice_persisted(invoice_persisted_path);
 	}
 
-	/// When handling an [`Event::StaticInvoiceRequested`], this should be called to forward the
-	/// [`InvoiceRequest`] over the `invoice_request_path` to the async recipient if it is online
-	/// and it will forward the [`StaticInvoice`] to the responder.
+	/// Forwards a [`StaticInvoice`] to a payer in response to an
+	/// [`Event::StaticInvoiceRequested`]. Also forwards the payer's [`InvoiceRequest`] to the
+	/// async recipient, in case the recipient is online to provide the payer with a fresh
+	/// [`Bolt12Invoice`].
 	pub fn send_response_static_invoice_request(
 		&self, invoice: StaticInvoice, responder: Responder, invoice_request: InvoiceRequest,
 		invoice_request_path: BlindedMessagePath,

lightning/src/offers/flow.rs

@@ -396,7 +396,7 @@ pub enum InvreqResponseInstructions {
 	SendInvoice(VerifiedInvoiceRequest),
 	/// We are a static invoice server and should respond to this invoice request by retrieving the
 	/// [`StaticInvoice`] corresponding to the `recipient_id` and `invoice_slot` and calling
-	/// `OffersMessageFlow::enqueue_static_invoice`.
+	/// [`OffersMessageFlow::enqueue_static_invoice`].
 	///
 	/// [`StaticInvoice`]: crate::offers::static_invoice::StaticInvoice
 	SendStaticInvoice {
@@ -406,8 +406,9 @@ pub enum InvreqResponseInstructions {
 		recipient_id: Vec<u8>,
 		/// The slot number for the specific invoice being requested by the payer.
 		invoice_slot: u16,
-		/// The invoice request that should be forwarded to the async recipient in case it is
-		/// online to respond.
+		/// The invoice request that should be forwarded to the async recipient in case the
+		/// recipient is online to respond. Should be forwarded by calling
+		/// [`OffersMessageFlow::enqueue_invoice_request_to_forward`].
 		invoice_request: InvoiceRequest,
 	},
 }
@@ -1123,9 +1124,9 @@ where
 
 	/// Forwards an [`InvoiceRequest`] to the specified [`BlindedMessagePath`]. If we receive an
 	/// invoice request as a static invoice server on behalf of an often-offline recipient this
-	/// can be used to forward the request to the recipient to give it a chance to provide an
-	/// invoice if it is online. The reply_path [`Responder`] provided is the path to the sender
-	/// where the recipient can send the invoice.
+	/// can be used to forward the request to give the recipient a chance to provide an
+	/// invoice if the recipient is online. The reply_path [`Responder`] provided is the path to
+	/// the sender where the recipient can send the invoice.
 	///
 	/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 	/// [`BlindedMessagePath`]: crate::blinded_path::message::BlindedMessagePath

lightning/src/offers/invoice_request.rs

@@ -581,15 +581,17 @@ impl AsRef<TaggedHash> for UnsignedInvoiceRequest {
 /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 /// [`Offer`]: crate::offers::offer::Offer
 #[derive(Clone, Debug)]
+#[cfg_attr(test, derive(PartialEq))]
 pub struct InvoiceRequest {
 	pub(super) bytes: Vec<u8>,
 	pub(super) contents: InvoiceRequestContents,
 	signature: Signature,
 }
 
+#[cfg(not(test))]
 impl PartialEq for InvoiceRequest {
 	fn eq(&self, other: &Self) -> bool {
-		self.bytes.eq(&other.bytes)
+		self.bytes.eq(&other.bytes) && self.signature.eq(&other.signature)
 	}
 }