Commit to payment_metadata in inbound payment HMAC

fuzz/src/chanmon_consistency.rs

@@ -640,7 +640,7 @@ fn get_payment_secret_hash(
 	payment_preimage.0[0..8].copy_from_slice(&payment_ctr.to_be_bytes());
 	let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).to_byte_array());
 	let payment_secret = dest
-		.create_inbound_payment_for_hash(payment_hash, None, 3600, None)
+		.create_inbound_payment_for_hash(payment_hash, None, 3600, None, None)
 		.expect("create_inbound_payment_for_hash failed");
 	assert!(payment_preimages.borrow_mut().insert(payment_hash, payment_preimage).is_none());
 	(payment_secret, payment_hash)

fuzz/src/full_stack.rs

@@ -837,11 +837,10 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger + MaybeSend + MaybeSync>
 			},
 			16 => {
 				let payment_preimage = PaymentPreimage(keys_manager.get_secure_random_bytes());
-				let payment_hash =
-					PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
+				let hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
 				// Note that this may fail - our hashes may collide and we'll end up trying to
 				// double-register the same payment_hash.
-				let _ = channelmanager.create_inbound_payment_for_hash(payment_hash, None, 1, None);
+				let _ = channelmanager.create_inbound_payment_for_hash(hash, None, 1, None, None);
 			},
 			9 => {
 				for payment in payments_received.drain(..) {

lightning-invoice/src/lib.rs

@@ -880,11 +880,10 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool>
 {
 	/// Sets the payment metadata.
 	///
-	/// By default features are set to *optionally* allow the sender to include the payment metadata.
-	/// If you wish to require that the sender include the metadata (and fail to parse the invoice if
-	/// they don't support payment metadata fields), you need to call
-	/// [`InvoiceBuilder::require_payment_metadata`] after this.
-	pub fn payment_metadata(
+	/// This marks the payment metadata as optional, allowing a legacy sender that doesn't
+	/// understand payment metadata to ignore it. Note that LDK by default commits to the payment
+	/// metadata in its payment secret, implicitly making it required.
+	pub fn optional_payment_metadata(
 		mut self, payment_metadata: Vec<u8>,
 	) -> InvoiceBuilder<D, H, T, C, S, tb::True> {
 		self.tagged_fields.push(TaggedField::PaymentMetadata(payment_metadata));
@@ -902,20 +901,23 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool>
 		}
 		self.set_flags()
 	}
-}
 
-impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool>
-	InvoiceBuilder<D, H, T, C, S, tb::True>
-{
-	/// Sets forwarding of payment metadata as required. A reader of the invoice which does not
-	/// support sending payment metadata will fail to read the invoice.
-	pub fn require_payment_metadata(mut self) -> InvoiceBuilder<D, H, T, C, S, tb::True> {
-		for field in self.tagged_fields.iter_mut() {
+	/// Sets the payment metadata.
+	///
+	/// By default features are set to *require* the sender to include the payment metadata.
+	/// If you wish to support legacy senders that ignore the metadata, you can call
+	/// [`InvoiceBuilder::optional_payment_metadata`] instead. Note that LDK by default commits to
+	/// the payment metadata in its payment secret, implicitly making it required.
+	pub fn payment_metadata(
+		self, payment_metadata: Vec<u8>,
+	) -> InvoiceBuilder<D, H, T, C, S, tb::True> {
+		let mut res = self.optional_payment_metadata(payment_metadata);
+		for field in res.tagged_fields.iter_mut() {
 			if let TaggedField::Features(f) = field {
 				f.set_payment_metadata_required();
 			}
 		}
-		self
+		res
 	}
 }
 

lightning-invoice/tests/ser_de.rs

@@ -418,7 +418,6 @@ fn get_test_tuples() -> Vec<(String, SignedRawBolt11Invoice, bool, bool)> {
 			))
 				.description("payment metadata inside".to_owned())
 				.payment_metadata(<Vec<u8>>::from_hex("01fafaf0").unwrap())
-				.require_payment_metadata()
 				.payee_pub_key(PublicKey::from_slice(&<Vec<u8>>::from_hex(
 						"03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad"
 					).unwrap()).unwrap())
@@ -450,7 +449,6 @@ fn get_test_tuples() -> Vec<(String, SignedRawBolt11Invoice, bool, bool)> {
 			))
 				.description("payment metadata inside".to_owned())
 				.payment_metadata(<Vec<u8>>::from_hex("01fafaf0").unwrap())
-				.require_payment_metadata()
 				.payment_secret(PaymentSecret([0x11; 32]))
 				.build_raw()
 				.unwrap()

lightning-liquidity/tests/lsps2_integration_tests.rs

@@ -122,7 +122,7 @@ fn create_jit_invoice(
 	let min_final_cltv_expiry_delta = MIN_FINAL_CLTV_EXPIRY_DELTA + 2;
 	let (payment_hash, payment_secret) = node
 		.node
-		.create_inbound_payment(None, expiry_secs, Some(min_final_cltv_expiry_delta))
+		.create_inbound_payment(None, expiry_secs, Some(min_final_cltv_expiry_delta), None)
 		.map_err(|e| {
 			log_error!(node.logger, "Failed to register inbound payment: {:?}", e);
 		})?;

lightning/src/ln/bolt11_payment_tests.rs

@@ -31,7 +31,7 @@ fn payment_metadata_end_to_end_for_invoice_with_amount() {
 	let payment_metadata = vec![42, 43, 44, 45, 46, 47, 48, 49, 42];
 
 	let (payment_hash, payment_secret) =
-		nodes[1].node.create_inbound_payment(None, 7200, None).unwrap();
+		nodes[1].node.create_inbound_payment(None, 7200, None, Some(&payment_metadata)).unwrap();
 
 	let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
 	let invoice = InvoiceBuilder::new(Currency::Bitcoin)
@@ -98,7 +98,7 @@ fn payment_metadata_end_to_end_for_invoice_with_no_amount() {
 	let payment_metadata = vec![42, 43, 44, 45, 46, 47, 48, 49, 42];
 
 	let (payment_hash, payment_secret) =
-		nodes[1].node.create_inbound_payment(None, 7200, None).unwrap();
+		nodes[1].node.create_inbound_payment(None, 7200, None, Some(&payment_metadata)).unwrap();
 
 	let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
 	let invoice = InvoiceBuilder::new(Currency::Bitcoin)

lightning/src/ln/channelmanager.rs

@@ -8611,6 +8611,7 @@ impl<
 							let verify_res = inbound_payment::verify(
 								payment_hash,
 								&payment_data,
+								onion_fields.payment_metadata.as_deref(),
 								self.highest_seen_timestamp.load(Ordering::Acquire) as u64,
 								&self.inbound_payment_key,
 								&self.logger,
@@ -14272,7 +14273,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 	) -> Result<Bolt11Invoice, SignOrCreationError<()>> {
 		let Bolt11InvoiceParameters {
 			amount_msats, description, invoice_expiry_delta_secs, min_final_cltv_expiry_delta,
-			payment_hash,
+			payment_hash, payment_metadata,
 		} = params;
 
 		let currency =
@@ -14305,6 +14306,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 						payment_hash, amount_msats,
 						invoice_expiry_delta_secs.unwrap_or(DEFAULT_EXPIRY_TIME as u32),
 						min_final_cltv_expiry_delta,
+						payment_metadata.as_deref(),
 					)
 					.map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?;
 				(payment_hash, payment_secret)
@@ -14314,6 +14316,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 					.create_inbound_payment(
 						amount_msats, invoice_expiry_delta_secs.unwrap_or(DEFAULT_EXPIRY_TIME as u32),
 						min_final_cltv_expiry_delta,
+						payment_metadata.as_deref(),
 					)
 					.map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?
 			},
@@ -14352,7 +14355,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 			invoice = invoice.private_route(hint);
 		}
 
-		let raw_invoice = invoice.build_raw().map_err(|e| SignOrCreationError::CreationError(e))?;
+		let raw_invoice = if let Some(payment_metadata) = payment_metadata {
+			invoice.payment_metadata(payment_metadata).build_raw()
+		} else {
+			invoice.build_raw()
+		}.map_err(|e| SignOrCreationError::CreationError(e))?;
 		let signature = self.node_signer.sign_invoice(&raw_invoice, Recipient::Node);
 
 		raw_invoice
@@ -14431,6 +14438,14 @@ pub struct Bolt11InvoiceParameters {
 	/// involving another protocol where the payment hash is also involved outside the scope of
 	/// lightning.
 	pub payment_hash: Option<PaymentHash>,
+
+	/// The `payment_metadata` to include in the invoice. This is provided back to us in the payment
+	/// onion by the sender, available as [`RecipientOnionFields::payment_metadata`] via
+	/// [`Event::PaymentClaimable::onion_fields`].
+	///
+	/// Note that because it is exposed to the sender in the invoice you should consider encrypting
+	/// it. It is committed to, however, so cannot be modified by the sender.
+	pub payment_metadata: Option<Vec<u8>>,
 }
 
 impl Default for Bolt11InvoiceParameters {
@@ -14441,6 +14456,7 @@ impl Default for Bolt11InvoiceParameters {
 			invoice_expiry_delta_secs: None,
 			min_final_cltv_expiry_delta: None,
 			payment_hash: None,
+			payment_metadata: None,
 		}
 	}
 }
@@ -14932,7 +14948,7 @@ impl<
 			refund,
 			self.list_usable_channels(),
 			|amount_msats, relative_expiry| {
-				self.create_inbound_payment(Some(amount_msats), relative_expiry, None)
+				self.create_inbound_payment(Some(amount_msats), relative_expiry, None, None)
 					.map_err(|()| Bolt12SemanticError::InvalidAmount)
 			},
 		)?;
@@ -14975,7 +14991,7 @@ impl<
 	/// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
 	pub fn create_inbound_payment(
 		&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32,
-		min_final_cltv_expiry_delta: Option<u16>,
+		min_final_cltv_expiry_delta: Option<u16>, payment_metadata: Option<&[u8]>,
 	) -> Result<(PaymentHash, PaymentSecret), ()> {
 		inbound_payment::create(
 			&self.inbound_payment_key,
@@ -14984,6 +15000,7 @@ impl<
 			&self.entropy_source,
 			self.highest_seen_timestamp.load(Ordering::Acquire) as u64,
 			min_final_cltv_expiry_delta,
+			payment_metadata,
 		)
 	}
 
@@ -15003,6 +15020,9 @@ impl<
 	/// before a [`PaymentClaimable`] event will be generated, ensuring that we do not provide the
 	/// sender "proof-of-payment" unless they have paid the required amount.
 	///
+	/// The returned secret commits to the `payment_metadata` and thus the invoice's metadata must
+	/// match what is provided here.
+	///
 	/// `invoice_expiry_delta_secs` describes the number of seconds that the invoice is valid for
 	/// in excess of the current time. This should roughly match the expiry time set in the invoice.
 	/// After this many seconds, we will remove the inbound payment, resulting in any attempts to
@@ -15036,6 +15056,7 @@ impl<
 	pub fn create_inbound_payment_for_hash(
 		&self, payment_hash: PaymentHash, min_value_msat: Option<u64>,
 		invoice_expiry_delta_secs: u32, min_final_cltv_expiry: Option<u16>,
+		payment_metadata: Option<&[u8]>,
 	) -> Result<PaymentSecret, ()> {
 		inbound_payment::create_from_hash(
 			&self.inbound_payment_key,
@@ -15044,18 +15065,25 @@ impl<
 			invoice_expiry_delta_secs,
 			self.highest_seen_timestamp.load(Ordering::Acquire) as u64,
 			min_final_cltv_expiry,
+			payment_metadata,
 		)
 	}
 
-	/// Gets an LDK-generated payment preimage from a payment hash and payment secret that were
+	/// Gets an LDK-generated payment preimage from a payment hash, metadata and secret that were
 	/// previously returned from [`create_inbound_payment`].
 	///
 	/// [`create_inbound_payment`]: Self::create_inbound_payment
 	pub fn get_payment_preimage(
 		&self, payment_hash: PaymentHash, payment_secret: PaymentSecret,
+		payment_metadata: Option<&[u8]>,
 	) -> Result<PaymentPreimage, APIError> {
 		let expanded_key = &self.inbound_payment_key;
-		inbound_payment::get_payment_preimage(payment_hash, payment_secret, expanded_key)
+		inbound_payment::get_payment_preimage(
+			payment_hash,
+			payment_secret,
+			payment_metadata,
+			expanded_key,
+		)
 	}
 
 	/// [`BlindedMessagePath`]s for an async recipient to communicate with this node and interactively
@@ -17108,7 +17136,8 @@ impl<
 					self.create_inbound_payment(
 						Some(amount_msats),
 						relative_expiry,
-						None
+						None,
+						None,
 					).map_err(|_| Bolt12SemanticError::InvalidAmount)
 				};
 
@@ -21319,15 +21348,15 @@ mod tests {
 		// payment verification fails as expected.
 		let mut bad_payment_hash = payment_hash.clone();
 		bad_payment_hash.0[0] += 1;
-		match inbound_payment::verify(bad_payment_hash, &payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger) {
+		match inbound_payment::verify(bad_payment_hash, &payment_data, None, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger) {
 			Ok(_) => panic!("Unexpected ok"),
 			Err(()) => {
 				nodes[0].logger.assert_log_contains("lightning::ln::inbound_payment", "Failing HTLC with user-generated payment_hash", 1);
 			}
 		}
 
 		// Check that using the original payment hash succeeds.
-		assert!(inbound_payment::verify(payment_hash, &payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger).is_ok());
+		assert!(inbound_payment::verify(payment_hash, &payment_data, None, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger).is_ok());
 	}
 
 	fn check_not_connected_to_peer_error<T>(
@@ -22000,7 +22029,7 @@ pub mod bench {
 				payment_preimage.0[0..8].copy_from_slice(&payment_count.to_le_bytes());
 				payment_count += 1;
 				let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
-				let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, None).unwrap();
+				let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, None, None).unwrap();
 
 				$node_a.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret, 10_000),
 					PaymentId(payment_hash.0),

lightning/src/ln/functional_test_utils.rs

@@ -2807,6 +2807,7 @@ pub fn get_payment_preimage_hash(
 			min_value_msat,
 			7200,
 			min_final_cltv_expiry_delta,
+			None,
 		)
 		.unwrap();
 	(payment_preimage, payment_hash, payment_secret)