Include Nonce in payer_metadata again

lightning/src/blinded_path/message.rs

@@ -466,15 +466,13 @@ pub enum OffersContext {
 	OutboundPaymentForRefund {
 		/// Payment ID used when creating a [`Refund`].
 		///
-		/// [`Refund`]: crate::offers::refund::Refund
-		payment_id: PaymentId,
-
-		/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid [`Refund`] and
-		/// for deriving its signing keys.
+		/// Used when handling a received [`Bolt12Invoice`] to confirm it arrived over the reply path
+		/// created for this payment, rather than one an attacker could use to learn our identity by
+		/// observing which payment we make. The invoice itself is verified using its payer metadata.
 		///
-		/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 		/// [`Refund`]: crate::offers::refund::Refund
-		nonce: Nonce,
+		/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
+		payment_id: PaymentId,
 	},
 	/// Context used by a [`BlindedMessagePath`] as a reply path for an [`InvoiceRequest`].
 	///
@@ -487,15 +485,13 @@ pub enum OffersContext {
 	OutboundPaymentForOffer {
 		/// Payment ID used when creating an [`InvoiceRequest`].
 		///
-		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
-		payment_id: PaymentId,
-
-		/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid
-		/// [`InvoiceRequest`] and for deriving its signing keys.
+		/// Used when handling a received [`Bolt12Invoice`] to confirm it arrived over the reply path
+		/// created for this payment, rather than one an attacker could use to learn our identity by
+		/// observing which payment we make. The invoice itself is verified using its payer metadata.
 		///
-		/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
-		nonce: Nonce,
+		/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
+		payment_id: PaymentId,
 	},
 	/// Context used by a [`BlindedMessagePath`] as a reply path for a [`Bolt12Invoice`].
 	///
@@ -678,7 +674,6 @@ impl_ser_tlv_based_enum!(OffersContext,
 	},
 	(1, OutboundPaymentForRefund) => {
 		(0, payment_id, required),
-		(1, nonce, required),
 	},
 	(2, InboundPayment) => {
 		(0, payment_hash, required),
@@ -690,7 +685,6 @@ impl_ser_tlv_based_enum!(OffersContext,
 	},
 	(4, OutboundPaymentForOffer) => {
 		(0, payment_id, required),
-		(1, nonce, required),
 	},
 );
 

lightning/src/ln/channelmanager.rs

@@ -15075,13 +15075,13 @@ impl<
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
 		self.flow.enqueue_invoice_request(
-			invoice_request.clone(), payment_id, nonce,
+			invoice_request.clone(), payment_id,
 			self.get_peers_for_blinded_path()
 		)?;
 
 		let retryable_invoice_request = RetryableInvoiceRequest {
 			invoice_request: invoice_request.clone(),
-			nonce,
+			nonce: Some(nonce),
 			needs_retry: true,
 		};
 
@@ -17231,11 +17231,11 @@ impl<
 		for (payment_id, retryable_invoice_request) in
 			self.pending_outbound_payments.release_invoice_requests_awaiting_invoice()
 		{
-			let RetryableInvoiceRequest { invoice_request, nonce, .. } = retryable_invoice_request;
+			let RetryableInvoiceRequest { invoice_request, .. } = retryable_invoice_request;
 
 			let peers = self.get_peers_for_blinded_path();
 			let enqueue_invreq_res =
-				self.flow.enqueue_invoice_request(invoice_request, payment_id, nonce, peers);
+				self.flow.enqueue_invoice_request(invoice_request, payment_id, peers);
 			if enqueue_invreq_res.is_err() {
 				log_warn!(
 					self.logger,

lightning/src/ln/outbound_payment.rs

@@ -173,14 +173,18 @@ pub(crate) enum PendingOutboundPayment {
 #[derive(Clone)]
 pub(crate) struct RetryableInvoiceRequest {
 	pub(crate) invoice_request: InvoiceRequest,
-	pub(crate) nonce: Nonce,
+	// No longer used, but written so that the payment can be retried after downgrading to a
+	// version that verifies invoices using the nonce instead of the payer metadata. Set when
+	// creating an invoice request and otherwise retains the value read from disk, which may have
+	// been written by such a version.
+	pub(crate) nonce: Option<Nonce>,
 	pub(super) needs_retry: bool,
 }
 
 impl_ser_tlv_based!(RetryableInvoiceRequest, {
 	(0, invoice_request, required),
 	(1, needs_retry, (default_value, true)),
-	(2, nonce, required),
+	(2, nonce, option),
 });
 
 impl PendingOutboundPayment {

lightning/src/offers/flow.rs

@@ -484,14 +484,14 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
 		Ok(InvreqResponseInstructions::SendInvoice(invoice_request))
 	}
 
-	/// Verifies a [`Bolt12Invoice`] using the provided [`OffersContext`] or the invoice's payer
-	/// metadata, returning the corresponding [`PaymentId`] if successful.
+	/// Verifies a [`Bolt12Invoice`] using the invoice's payer metadata, returning the
+	/// corresponding [`PaymentId`] if successful.
 	///
 	/// - If an [`OffersContext::OutboundPaymentForOffer`] or
-	///   [`OffersContext::OutboundPaymentForRefund`] with a `nonce` is provided, verification is
-	///   performed using this to form the payer metadata.
-	/// - If no context is provided and the invoice corresponds to a [`Refund`] without blinded paths,
-	///   verification is performed using the [`Bolt12Invoice::payer_metadata`].
+	///   [`OffersContext::OutboundPaymentForRefund`] is provided, the extracted [`PaymentId`] must
+	///   also match the context's `payment_id`.
+	/// - If no context is provided, the invoice must correspond to a [`Refund`] without blinded
+	///   paths.
 	/// - If neither condition is met, verification fails.
 	pub fn verify_bolt12_invoice(
 		&self, invoice: &Bolt12Invoice, context: Option<&OffersContext>,
@@ -503,16 +503,20 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
 			None if invoice.is_for_refund_without_paths() => {
 				invoice.verify_using_metadata(expanded_key, secp_ctx)
 			},
-			Some(&OffersContext::OutboundPaymentForOffer { payment_id, nonce, .. }) => {
+			Some(&OffersContext::OutboundPaymentForOffer { payment_id }) => {
 				if invoice.is_for_offer() {
-					invoice.verify_using_payer_data(payment_id, nonce, expanded_key, secp_ctx)
+					invoice.verify_using_metadata(expanded_key, secp_ctx).and_then(|extracted| {
+						(extracted == payment_id).then(|| payment_id).ok_or(())
+					})
 				} else {
 					Err(())
 				}
 			},
-			Some(&OffersContext::OutboundPaymentForRefund { payment_id, nonce, .. }) => {
+			Some(&OffersContext::OutboundPaymentForRefund { payment_id }) => {
 				if invoice.is_for_refund() {
-					invoice.verify_using_payer_data(payment_id, nonce, expanded_key, secp_ctx)
+					invoice.verify_using_metadata(expanded_key, secp_ctx).and_then(|extracted| {
+						(extracted == payment_id).then(|| payment_id).ok_or(())
+					})
 				} else {
 					Err(())
 				}
@@ -689,7 +693,7 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
 
 		let nonce = Nonce::from_entropy_source(entropy);
 		let context =
-			MessageContext::Offers(OffersContext::OutboundPaymentForRefund { payment_id, nonce });
+			MessageContext::Offers(OffersContext::OutboundPaymentForRefund { payment_id });
 
 		// Create the base builder with common properties
 		let mut builder = RefundBuilder::deriving_signing_pubkey(
@@ -1085,13 +1089,6 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
 	/// over those blinded paths, which can be verified against the intended outbound payment,
 	/// ensuring the invoice corresponds to a payment we actually want to make.
 	///
-	/// # Nonce
-	/// The nonce is used to create a unique [`MessageContext`] for the reply paths.
-	/// These will be used to verify the corresponding [`Bolt12Invoice`] when it is received.
-	///
-	/// Note: The provided [`Nonce`] MUST be the same as the [`Nonce`] used for creating the
-	/// [`InvoiceRequest`] to ensure correct verification of the corresponding [`Bolt12Invoice`].
-	///
 	/// See [`OffersMessageFlow::create_invoice_request_builder`] for more details.
 	///
 	/// # Peers
@@ -1103,11 +1100,10 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
 	/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
 	/// [`supports_onion_messages`]: crate::types::features::Features::supports_onion_messages
 	pub fn enqueue_invoice_request(
-		&self, invoice_request: InvoiceRequest, payment_id: PaymentId, nonce: Nonce,
+		&self, invoice_request: InvoiceRequest, payment_id: PaymentId,
 		peers: Vec<MessageForwardNode>,
 	) -> Result<(), Bolt12SemanticError> {
-		let context =
-			MessageContext::Offers(OffersContext::OutboundPaymentForOffer { payment_id, nonce });
+		let context = MessageContext::Offers(OffersContext::OutboundPaymentForOffer { payment_id });
 		let reply_paths = self
 			.create_blinded_paths(peers, context)
 			.map_err(|_| Bolt12SemanticError::MissingPaths)?;

lightning/src/offers/invoice.rs

@@ -133,7 +133,6 @@ use crate::offers::invoice_request::{
 use crate::offers::merkle::{
 	self, SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream,
 };
-use crate::offers::nonce::Nonce;
 use crate::offers::offer::{
 	Amount, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OfferId, OfferTlvStream,
 	OfferTlvStreamRef, Quantity, EXPERIMENTAL_OFFER_TYPES, OFFER_TYPES,
@@ -1008,30 +1007,17 @@ impl Bolt12Invoice {
 				(&invoice_request.inner.payer.0, INVOICE_REQUEST_IV_BYTES)
 			},
 			InvoiceContents::ForRefund { refund, .. } => {
-				(&refund.payer.0, REFUND_IV_BYTES_WITH_METADATA)
+				let iv_bytes = if refund.paths().is_empty() {
+					REFUND_IV_BYTES_WITH_METADATA
+				} else {
+					REFUND_IV_BYTES_WITHOUT_METADATA
+				};
+				(&refund.payer.0, iv_bytes)
 			},
 		};
 		self.contents.verify(&self.bytes, metadata, key, iv_bytes, secp_ctx)
 	}
 
-	/// Verifies that the invoice was for a request or refund created using the given key by
-	/// checking a payment id and nonce included with the [`BlindedMessagePath`] for which the invoice was
-	/// sent through.
-	pub fn verify_using_payer_data<T: secp256k1::Signing>(
-		&self, payment_id: PaymentId, nonce: Nonce, key: &ExpandedKey, secp_ctx: &Secp256k1<T>,
-	) -> Result<PaymentId, ()> {
-		let metadata = Metadata::payer_data(payment_id, nonce, key);
-		let iv_bytes = match &self.contents {
-			InvoiceContents::ForOffer { .. } => INVOICE_REQUEST_IV_BYTES,
-			InvoiceContents::ForRefund { .. } => REFUND_IV_BYTES_WITHOUT_METADATA,
-		};
-		self.contents.verify(&self.bytes, &metadata, key, iv_bytes, secp_ctx).and_then(
-			|extracted_payment_id| {
-				(payment_id == extracted_payment_id).then(|| payment_id).ok_or(())
-			},
-		)
-	}
-
 	pub(crate) fn as_tlv_stream(&self) -> FullInvoiceTlvStreamRef<'_> {
 		let (
 			payer_tlv_stream,
@@ -1892,6 +1878,8 @@ mod tests {
 		let secp_ctx = Secp256k1::new();
 		let payment_id = PaymentId([1; 32]);
 		let encrypted_payment_id = expanded_key.crypt_for_offer(payment_id.0, nonce);
+		let mut payer_metadata = encrypted_payment_id.to_vec();
+		payer_metadata.extend_from_slice(nonce.as_slice());
 
 		let payment_paths = payment_paths();
 		let payment_hash = payment_hash();
@@ -1913,7 +1901,7 @@ mod tests {
 		unsigned_invoice.write(&mut buffer).unwrap();
 
 		assert_eq!(unsigned_invoice.bytes, buffer.as_slice());
-		assert_eq!(unsigned_invoice.payer_metadata(), &encrypted_payment_id);
+		assert_eq!(unsigned_invoice.payer_metadata(), payer_metadata.as_slice());
 		assert_eq!(
 			unsigned_invoice.offer_chains(),
 			Some(vec![ChainHash::using_genesis_block(Network::Bitcoin)])
@@ -1957,7 +1945,7 @@ mod tests {
 		invoice.write(&mut buffer).unwrap();
 
 		assert_eq!(invoice.bytes, buffer.as_slice());
-		assert_eq!(invoice.payer_metadata(), &encrypted_payment_id);
+		assert_eq!(invoice.payer_metadata(), payer_metadata.as_slice());
 		assert_eq!(
 			invoice.offer_chains(),
 			Some(vec![ChainHash::using_genesis_block(Network::Bitcoin)])
@@ -1975,10 +1963,7 @@ mod tests {
 		assert_eq!(invoice.amount_msats(), 1000);
 		assert_eq!(invoice.invoice_request_features(), &InvoiceRequestFeatures::empty());
 		assert_eq!(invoice.quantity(), None);
-		assert_eq!(
-			invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx),
-			Ok(payment_id),
-		);
+		assert_eq!(invoice.verify_using_metadata(&expanded_key, &secp_ctx), Ok(payment_id));
 		assert_eq!(invoice.payer_note(), None);
 		assert_eq!(invoice.payment_paths(), payment_paths.as_slice());
 		assert_eq!(invoice.created_at(), now);
@@ -2001,7 +1986,7 @@ mod tests {
 		assert_eq!(
 			invoice.as_tlv_stream(),
 			(
-				PayerTlvStreamRef { metadata: Some(&encrypted_payment_id.to_vec()) },
+				PayerTlvStreamRef { metadata: Some(&payer_metadata) },
 				OfferTlvStreamRef {
 					chains: None,
 					metadata: None,

lightning/src/offers/invoice_request.rs

@@ -1588,6 +1588,8 @@ mod tests {
 		let secp_ctx = Secp256k1::new();
 		let payment_id = PaymentId([1; 32]);
 		let encrypted_payment_id = expanded_key.crypt_for_offer(payment_id.0, nonce);
+		let mut payer_metadata = encrypted_payment_id.to_vec();
+		payer_metadata.extend_from_slice(nonce.as_slice());
 
 		let invoice_request = OfferBuilder::new(recipient_pubkey())
 			.amount_msats(1000)
@@ -1602,7 +1604,7 @@ mod tests {
 		invoice_request.write(&mut buffer).unwrap();
 
 		assert_eq!(invoice_request.bytes, buffer.as_slice());
-		assert_eq!(invoice_request.payer_metadata(), &encrypted_payment_id);
+		assert_eq!(invoice_request.payer_metadata(), payer_metadata.as_slice());
 		assert_eq!(
 			invoice_request.chains(),
 			vec![ChainHash::using_genesis_block(Network::Bitcoin)]
@@ -1634,7 +1636,7 @@ mod tests {
 		assert_eq!(
 			invoice_request.as_tlv_stream(),
 			(
-				PayerTlvStreamRef { metadata: Some(&encrypted_payment_id.to_vec()) },
+				PayerTlvStreamRef { metadata: Some(&payer_metadata) },
 				OfferTlvStreamRef {
 					chains: None,
 					metadata: None,
@@ -1735,10 +1737,10 @@ mod tests {
 			.unwrap()
 			.sign(recipient_sign)
 			.unwrap();
-		assert!(invoice.verify_using_metadata(&expanded_key, &secp_ctx).is_err());
-		assert!(invoice
-			.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)
-			.is_ok());
+		match invoice.verify_using_metadata(&expanded_key, &secp_ctx) {
+			Ok(payment_id) => assert_eq!(payment_id, PaymentId([1; 32])),
+			Err(()) => panic!("verification failed"),
+		}
 
 		// Fails verification with altered fields
 		let (
@@ -1774,9 +1776,7 @@ mod tests {
 			.unwrap();
 
 		let invoice = Bolt12Invoice::try_from(encoded_invoice).unwrap();
-		assert!(invoice
-			.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)
-			.is_err());
+		assert!(invoice.verify_using_metadata(&expanded_key, &secp_ctx).is_err());
 
 		// Fails verification with altered payer id
 		let (
@@ -1812,9 +1812,7 @@ mod tests {
 			.unwrap();
 
 		let invoice = Bolt12Invoice::try_from(encoded_invoice).unwrap();
-		assert!(invoice
-			.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)
-			.is_err());
+		assert!(invoice.verify_using_metadata(&expanded_key, &secp_ctx).is_err());
 	}
 
 	#[test]