Merge pull request #2127 from TheBlueMatt/2023-03-payment-metadata

Support sending `PaymentMetadata` in HTLCs

lightning-invoice/src/de.rs

@@ -460,6 +460,8 @@ impl FromBase32 for TaggedField {
 				Ok(TaggedField::PrivateRoute(PrivateRoute::from_base32(field_data)?)),
 			constants::TAG_PAYMENT_SECRET =>
 				Ok(TaggedField::PaymentSecret(PaymentSecret::from_base32(field_data)?)),
+			constants::TAG_PAYMENT_METADATA =>
+				Ok(TaggedField::PaymentMetadata(Vec::<u8>::from_base32(field_data)?)),
 			constants::TAG_FEATURES =>
 				Ok(TaggedField::Features(InvoiceFeatures::from_base32(field_data)?)),
 			_ => {

lightning-invoice/src/lib.rs

@@ -218,10 +218,13 @@ pub const DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA: u64 = 18;
 ///  * `D`: exactly one [`TaggedField::Description`] or [`TaggedField::DescriptionHash`]
 ///  * `H`: exactly one [`TaggedField::PaymentHash`]
 ///  * `T`: the timestamp is set
+///  * `C`: the CLTV expiry is set
+///  * `S`: the payment secret is set
+///  * `M`: payment metadata is set
 ///
 /// This is not exported to bindings users as we likely need to manually select one set of boolean type parameters.
 #[derive(Eq, PartialEq, Debug, Clone)]
-pub struct InvoiceBuilder<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> {
+pub struct InvoiceBuilder<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool, M: tb::Bool> {
 	currency: Currency,
 	amount: Option<u64>,
 	si_prefix: Option<SiPrefix>,
@@ -234,6 +237,7 @@ pub struct InvoiceBuilder<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S:
 	phantom_t: core::marker::PhantomData<T>,
 	phantom_c: core::marker::PhantomData<C>,
 	phantom_s: core::marker::PhantomData<S>,
+	phantom_m: core::marker::PhantomData<M>,
 }
 
 /// Represents a syntactically and semantically correct lightning BOLT11 invoice.
@@ -442,6 +446,7 @@ pub enum TaggedField {
 	Fallback(Fallback),
 	PrivateRoute(PrivateRoute),
 	PaymentSecret(PaymentSecret),
+	PaymentMetadata(Vec<u8>),
 	Features(InvoiceFeatures),
 }
 
@@ -506,15 +511,16 @@ pub mod constants {
 	pub const TAG_FALLBACK: u8 = 9;
 	pub const TAG_PRIVATE_ROUTE: u8 = 3;
 	pub const TAG_PAYMENT_SECRET: u8 = 16;
+	pub const TAG_PAYMENT_METADATA: u8 = 27;
 	pub const TAG_FEATURES: u8 = 5;
 }
 
-impl InvoiceBuilder<tb::False, tb::False, tb::False, tb::False, tb::False> {
+impl InvoiceBuilder<tb::False, tb::False, tb::False, tb::False, tb::False, tb::False> {
 	/// Construct new, empty `InvoiceBuilder`. All necessary fields have to be filled first before
 	/// `InvoiceBuilder::build(self)` becomes available.
-	pub fn new(currrency: Currency) -> Self {
+	pub fn new(currency: Currency) -> Self {
 		InvoiceBuilder {
-			currency: currrency,
+			currency,
 			amount: None,
 			si_prefix: None,
 			timestamp: None,
@@ -526,14 +532,15 @@ impl InvoiceBuilder<tb::False, tb::False, tb::False, tb::False, tb::False> {
 			phantom_t: core::marker::PhantomData,
 			phantom_c: core::marker::PhantomData,
 			phantom_s: core::marker::PhantomData,
+			phantom_m: core::marker::PhantomData,
 		}
 	}
 }
 
-impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, T, C, S> {
+impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool, M: tb::Bool> InvoiceBuilder<D, H, T, C, S, M> {
 	/// Helper function to set the completeness flags.
-	fn set_flags<DN: tb::Bool, HN: tb::Bool, TN: tb::Bool, CN: tb::Bool, SN: tb::Bool>(self) -> InvoiceBuilder<DN, HN, TN, CN, SN> {
+	fn set_flags<DN: tb::Bool, HN: tb::Bool, TN: tb::Bool, CN: tb::Bool, SN: tb::Bool, MN: tb::Bool>(self) -> InvoiceBuilder<DN, HN, TN, CN, SN, MN> {
-		InvoiceBuilder::<DN, HN, TN, CN, SN> {
+		InvoiceBuilder::<DN, HN, TN, CN, SN, MN> {
 			currency: self.currency,
 			amount: self.amount,
 			si_prefix: self.si_prefix,
@@ -546,6 +553,7 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBui
 			phantom_t: core::marker::PhantomData,
 			phantom_c: core::marker::PhantomData,
 			phantom_s: core::marker::PhantomData,
+			phantom_m: core::marker::PhantomData,
 		}
 	}
 
@@ -590,7 +598,7 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBui
 	}
 }
 
-impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, tb::True, C, S> {
+impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool, M: tb::Bool> InvoiceBuilder<D, H, tb::True, C, S, M> {
 	/// Builds a [`RawInvoice`] if no [`CreationError`] occurred while construction any of the
 	/// fields.
 	pub fn build_raw(self) -> Result<RawInvoice, CreationError> {
@@ -624,9 +632,9 @@ impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, tb
 	}
 }
 
-impl<H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<tb::False, H, T, C, S> {
+impl<H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool, M: tb::Bool> InvoiceBuilder<tb::False, H, T, C, S, M> {
 	/// Set the description. This function is only available if no description (hash) was set.
-	pub fn description(mut self, description: String) -> InvoiceBuilder<tb::True, H, T, C, S> {
+	pub fn description(mut self, description: String) -> InvoiceBuilder<tb::True, H, T, C, S, M> {
 		match Description::new(description) {
 			Ok(d) => self.tagged_fields.push(TaggedField::Description(d)),
 			Err(e) => self.error = Some(e),
@@ -635,13 +643,13 @@ impl<H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<tb::Fals
 	}
 
 	/// Set the description hash. This function is only available if no description (hash) was set.
-	pub fn description_hash(mut self, description_hash: sha256::Hash) -> InvoiceBuilder<tb::True, H, T, C, S> {
+	pub fn description_hash(mut self, description_hash: sha256::Hash) -> InvoiceBuilder<tb::True, H, T, C, S, M> {
 		self.tagged_fields.push(TaggedField::DescriptionHash(Sha256(description_hash)));
 		self.set_flags()
 	}
 
 	/// Set the description or description hash. This function is only available if no description (hash) was set.
-	pub fn invoice_description(self, description: InvoiceDescription) -> InvoiceBuilder<tb::True, H, T, C, S> {
+	pub fn invoice_description(self, description: InvoiceDescription) -> InvoiceBuilder<tb::True, H, T, C, S, M> {
 		match description {
 			InvoiceDescription::Direct(desc) => {
 				self.description(desc.clone().into_inner())
@@ -653,18 +661,18 @@ impl<H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<tb::Fals
 	}
 }
 
-impl<D: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, tb::False, T, C, S> {
+impl<D: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool, M: tb::Bool> InvoiceBuilder<D, tb::False, T, C, S, M> {
 	/// Set the payment hash. This function is only available if no payment hash was set.
-	pub fn payment_hash(mut self, hash: sha256::Hash) -> InvoiceBuilder<D, tb::True, T, C, S> {
+	pub fn payment_hash(mut self, hash: sha256::Hash) -> InvoiceBuilder<D, tb::True, T, C, S, M> {
 		self.tagged_fields.push(TaggedField::PaymentHash(Sha256(hash)));
 		self.set_flags()
 	}
 }
 
-impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, tb::False, C, S> {
+impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool, M: tb::Bool> InvoiceBuilder<D, H, tb::False, C, S, M> {
 	/// Sets the timestamp to a specific [`SystemTime`].
 	#[cfg(feature = "std")]
-	pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder<D, H, tb::True, C, S> {
+	pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder<D, H, tb::True, C, S, M> {
 		match PositiveTimestamp::from_system_time(time) {
 			Ok(t) => self.timestamp = Some(t),
 			Err(e) => self.error = Some(e),
@@ -675,7 +683,7 @@ impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, tb
 
 	/// Sets the timestamp to a duration since the Unix epoch, dropping the subsecond part (which
 	/// is not representable in BOLT 11 invoices).
-	pub fn duration_since_epoch(mut self, time: Duration) -> InvoiceBuilder<D, H, tb::True, C, S> {
+	pub fn duration_since_epoch(mut self, time: Duration) -> InvoiceBuilder<D, H, tb::True, C, S, M> {
 		match PositiveTimestamp::from_duration_since_epoch(time) {
 			Ok(t) => self.timestamp = Some(t),
 			Err(e) => self.error = Some(e),
@@ -686,34 +694,82 @@ impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, tb
 
 	/// Sets the timestamp to the current system time.
 	#[cfg(feature = "std")]
-	pub fn current_timestamp(mut self) -> InvoiceBuilder<D, H, tb::True, C, S> {
+	pub fn current_timestamp(mut self) -> InvoiceBuilder<D, H, tb::True, C, S, M> {
 		let now = PositiveTimestamp::from_system_time(SystemTime::now());
 		self.timestamp = Some(now.expect("for the foreseeable future this shouldn't happen"));
 		self.set_flags()
 	}
 }
 
-impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, T, tb::False, S> {
+impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, S: tb::Bool, M: tb::Bool> InvoiceBuilder<D, H, T, tb::False, S, M> {
 	/// Sets `min_final_cltv_expiry_delta`.
-	pub fn min_final_cltv_expiry_delta(mut self, min_final_cltv_expiry_delta: u64) -> InvoiceBuilder<D, H, T, tb::True, S> {
+	pub fn min_final_cltv_expiry_delta(mut self, min_final_cltv_expiry_delta: u64) -> InvoiceBuilder<D, H, T, tb::True, S, M> {
 		self.tagged_fields.push(TaggedField::MinFinalCltvExpiryDelta(MinFinalCltvExpiryDelta(min_final_cltv_expiry_delta)));
 		self.set_flags()
 	}
 }
 
-impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, C, tb::False> {
+impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, M: tb::Bool> InvoiceBuilder<D, H, T, C, tb::False, M> {
 	/// Sets the payment secret and relevant features.
-	pub fn payment_secret(mut self, payment_secret: PaymentSecret) -> InvoiceBuilder<D, H, T, C, tb::True> {
+	pub fn payment_secret(mut self, payment_secret: PaymentSecret) -> InvoiceBuilder<D, H, T, C, tb::True, M> {
-		let mut features = InvoiceFeatures::empty();
+		let mut found_features = false;
-		features.set_variable_length_onion_required();
+		for field in self.tagged_fields.iter_mut() {
-		features.set_payment_secret_required();
+			if let TaggedField::Features(f) = field {
+				found_features = true;
+				f.set_variable_length_onion_required();
+				f.set_payment_secret_required();
+			}
+		}
 		self.tagged_fields.push(TaggedField::PaymentSecret(payment_secret));
-		self.tagged_fields.push(TaggedField::Features(features));
+		if !found_features {
+			let mut features = InvoiceFeatures::empty();
+			features.set_variable_length_onion_required();
+			features.set_payment_secret_required();
+			self.tagged_fields.push(TaggedField::Features(features));
+		}
 		self.set_flags()
 	}
 }
 
-impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, C, tb::True> {
+impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, T, C, S, tb::False> {
+	/// 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(mut self, payment_metadata: Vec<u8>) -> InvoiceBuilder<D, H, T, C, S, tb::True> {
+		self.tagged_fields.push(TaggedField::PaymentMetadata(payment_metadata));
+		let mut found_features = false;
+		for field in self.tagged_fields.iter_mut() {
+			if let TaggedField::Features(f) = field {
+				found_features = true;
+				f.set_payment_metadata_optional();
+			}
+		}
+		if !found_features {
+			let mut features = InvoiceFeatures::empty();
+			features.set_payment_metadata_optional();
+			self.tagged_fields.push(TaggedField::Features(features));
+		}
+		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() {
+			if let TaggedField::Features(f) = field {
+				f.set_payment_metadata_required();
+			}
+		}
+		self
+	}
+}
+
+impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, M: tb::Bool> InvoiceBuilder<D, H, T, C, tb::True, M> {
 	/// Sets the `basic_mpp` feature as optional.
 	pub fn basic_mpp(mut self) -> Self {
 		for field in self.tagged_fields.iter_mut() {
@@ -725,7 +781,7 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T,
 	}
 }
 
-impl InvoiceBuilder<tb::True, tb::True, tb::True, tb::True, tb::True> {
+impl<M: tb::Bool> InvoiceBuilder<tb::True, tb::True, tb::True, tb::True, tb::True, M> {
 	/// Builds and signs an invoice using the supplied `sign_function`. This function MAY NOT fail
 	/// and MUST produce a recoverable signature valid for the given hash and if applicable also for
 	/// the included payee public key.
@@ -977,6 +1033,10 @@ impl RawInvoice {
 		find_extract!(self.known_tagged_fields(), TaggedField::PaymentSecret(ref x), x)
 	}
 
+	pub fn payment_metadata(&self) -> Option<&Vec<u8>> {
+		find_extract!(self.known_tagged_fields(), TaggedField::PaymentMetadata(ref x), x)
+	}
+
 	pub fn features(&self) -> Option<&InvoiceFeatures> {
 		find_extract!(self.known_tagged_fields(), TaggedField::Features(ref x), x)
 	}
@@ -1248,6 +1308,11 @@ impl Invoice {
 		self.signed_invoice.payment_secret().expect("was checked by constructor")
 	}
 
+	/// Get the payment metadata blob if one was included in the invoice
+	pub fn payment_metadata(&self) -> Option<&Vec<u8>> {
+		self.signed_invoice.payment_metadata()
+	}
+
 	/// Get the invoice features if they were included in the invoice
 	pub fn features(&self) -> Option<&InvoiceFeatures> {
 		self.signed_invoice.features()
@@ -1396,6 +1461,7 @@ impl TaggedField {
 			TaggedField::Fallback(_) => constants::TAG_FALLBACK,
 			TaggedField::PrivateRoute(_) => constants::TAG_PRIVATE_ROUTE,
 			TaggedField::PaymentSecret(_) => constants::TAG_PAYMENT_SECRET,
+			TaggedField::PaymentMetadata(_) => constants::TAG_PAYMENT_METADATA,
 			TaggedField::Features(_) => constants::TAG_FEATURES,
 		};
 

lightning-invoice/src/payment.rs

@@ -145,8 +145,10 @@ fn pay_invoice_using_amount<P: Deref>(
 	payer: P
 ) -> Result<(), PaymentError> where P::Target: Payer {
 	let payment_hash = PaymentHash((*invoice.payment_hash()).into_inner());
-	let payment_secret = Some(*invoice.payment_secret());
+	let recipient_onion = RecipientOnionFields {
-	let recipient_onion = RecipientOnionFields { payment_secret };
+		payment_secret: Some(*invoice.payment_secret()),
+		payment_metadata: invoice.payment_metadata().map(|v| v.clone()),
+	};
 	let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
 		invoice.min_final_cltv_expiry_delta() as u32)
 		.with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs())
@@ -213,6 +215,8 @@ mod tests {
 	use super::*;
 	use crate::{InvoiceBuilder, Currency};
 	use bitcoin_hashes::sha256::Hash as Sha256;
+	use lightning::events::Event;
+	use lightning::ln::msgs::ChannelMessageHandler;
 	use lightning::ln::{PaymentPreimage, PaymentSecret};
 	use lightning::ln::functional_test_utils::*;
 	use secp256k1::{SecretKey, Secp256k1};
@@ -350,4 +354,52 @@ mod tests {
 			_ => panic!()
 		}
 	}
+
+	#[test]
+	#[cfg(feature = "std")]
+	fn payment_metadata_end_to_end() {
+		// Test that a payment metadata read from an invoice passed to `pay_invoice` makes it all
+		// the way out through the `PaymentClaimable` event.
+		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_announced_chan_between_nodes(&nodes, 0, 1);
+
+		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();
+
+		let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+			.description("test".into())
+			.payment_hash(Sha256::from_slice(&payment_hash.0).unwrap())
+			.payment_secret(payment_secret)
+			.current_timestamp()
+			.min_final_cltv_expiry_delta(144)
+			.amount_milli_satoshis(50_000)
+			.payment_metadata(payment_metadata.clone())
+			.build_signed(|hash| {
+				Secp256k1::new().sign_ecdsa_recoverable(hash,
+					&nodes[1].keys_manager.backing.get_node_secret_key())
+			})
+			.unwrap();
+
+		pay_invoice(&invoice, Retry::Attempts(0), nodes[0].node).unwrap();
+		check_added_monitors(&nodes[0], 1);
+		let send_event = SendEvent::from_node(&nodes[0]);
+		nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send_event.msgs[0]);
+		commitment_signed_dance!(nodes[1], nodes[0], &send_event.commitment_msg, false);
+
+		expect_pending_htlcs_forwardable!(nodes[1]);
+
+		let mut events = nodes[1].node.get_and_clear_pending_events();
+		assert_eq!(events.len(), 1);
+		match events.pop().unwrap() {
+			Event::PaymentClaimable { onion_fields, .. } => {
+				assert_eq!(Some(payment_metadata), onion_fields.unwrap().payment_metadata);
+			},
+			_ => panic!("Unexpected event")
+		}
+	}
 }

lightning-invoice/src/ser.rs

@@ -446,6 +446,9 @@ impl ToBase32 for TaggedField {
 			TaggedField::PaymentSecret(ref payment_secret) => {
 				  write_tagged_field(writer, constants::TAG_PAYMENT_SECRET, payment_secret)
 			},
+			TaggedField::PaymentMetadata(ref payment_metadata) => {
+				  write_tagged_field(writer, constants::TAG_PAYMENT_METADATA, payment_metadata)
+			},
 			TaggedField::Features(ref features) => {
 				write_tagged_field(writer, constants::TAG_FEATURES, features)
 			},

lightning-invoice/tests/ser_de.rs

@@ -332,6 +332,56 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, bool, bool)> {
 			true, // Different features than set in InvoiceBuilder
 			true, // Some unknown fields
 		),
+		( // Older version of the payment metadata test with a payment_pubkey set
+			"lnbc10m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdp9wpshjmt9de6zqmt9w3skgct5vysxjmnnd9jx2mq8q8a04uqnp4q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66sp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q2gqqqqqqsgqy9gw6ymamd20jumvdgpfphkhp8fzhhdhycw36egcmla5vlrtrmhs9t7psfy3hkkdqzm9eq64fjg558znccds5nhsfmxveha5xe0dykgpspdha0".to_owned(),
+			InvoiceBuilder::new(Currency::Bitcoin)
+				.amount_milli_satoshis(1_000_000_000)
+				.duration_since_epoch(Duration::from_secs(1496314658))
+				.payment_hash(sha256::Hash::from_hex(
+					"0001020304050607080900010203040506070809000102030405060708090102"
+				).unwrap())
+				.description("payment metadata inside".to_owned())
+				.payment_metadata(hex::decode("01fafaf0").unwrap())
+				.require_payment_metadata()
+				.payee_pub_key(PublicKey::from_slice(&hex::decode(
+						"03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad"
+					).unwrap()).unwrap())
+				.payment_secret(PaymentSecret([0x11; 32]))
+				.build_raw()
+				.unwrap()
+				.sign(|_| {
+					RecoverableSignature::from_compact(
+						&hex::decode("2150ed137ddb54f9736c6a0290ded709d22bddb7261d1d6518dffb467c6b1eef02afc182491bdacd00b65c83554c914a1c53c61b0a4ef04eccccdfb4365ed259").unwrap(),
+						RecoveryId::from_i32(1).unwrap()
+					)
+				}).unwrap(),
+			false, // Different features than set in InvoiceBuilder
+			true, // Some unknown fields
+		),
+		(
+			"lnbc10m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdp9wpshjmt9de6zqmt9w3skgct5vysxjmnnd9jx2mq8q8a04uqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q2gqqqqqqsgq7hf8he7ecf7n4ffphs6awl9t6676rrclv9ckg3d3ncn7fct63p6s365duk5wrk202cfy3aj5xnnp5gs3vrdvruverwwq7yzhkf5a3xqpd05wjc".to_owned(),
+			InvoiceBuilder::new(Currency::Bitcoin)
+				.amount_milli_satoshis(1_000_000_000)
+				.duration_since_epoch(Duration::from_secs(1496314658))
+				.payment_hash(sha256::Hash::from_hex(
+					"0001020304050607080900010203040506070809000102030405060708090102"
+				).unwrap())
+				.description("payment metadata inside".to_owned())
+				.payment_metadata(hex::decode("01fafaf0").unwrap())
+				.require_payment_metadata()
+				.payment_secret(PaymentSecret([0x11; 32]))
+				.build_raw()
+				.unwrap()
+				.sign(|_| {
+					RecoverableSignature::from_compact(
+						&hex::decode("f5d27be7d9c27d3aa521bc35d77cabd6bda18f1f61716445b19e27e4e17a887508ea8de5a8e1d94f561248f65434e61a221160dac1f1991b9c0f1057b269d898").unwrap(),
+						RecoveryId::from_i32(1).unwrap()
+					)
+				}).unwrap(),
+			false, // Different features than set in InvoiceBuilder
+			true, // Some unknown fields
+		),
+
 	]
 }
 

lightning/src/events/mod.rs

@@ -21,7 +21,7 @@ pub mod bump_transaction;
 pub use bump_transaction::BumpTransactionEvent;
 
 use crate::chain::keysinterface::SpendableOutputDescriptor;
-use crate::ln::channelmanager::{InterceptId, PaymentId};
+use crate::ln::channelmanager::{InterceptId, PaymentId, RecipientOnionFields};
 use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS;
 use crate::ln::features::ChannelTypeFeatures;
 use crate::ln::msgs;
@@ -232,8 +232,11 @@ pub enum HTLCDestination {
 	///
 	/// Some of the reasons may include:
 	/// * HTLC Timeouts
-	/// * Expected MPP amount has already been reached
+	/// * Excess HTLCs for a payment that we have already fully received, over-paying for the
-	/// * Claimable amount does not match expected amount
+	///   payment,
+	/// * The counterparty node modified the HTLC in transit,
+	/// * A probing attack where an intermediary node is trying to detect if we are the ultimate
+	///   recipient for a payment.
 	FailedPayment {
 		/// The payment hash of the payment we attempted to process.
 		payment_hash: PaymentHash
@@ -379,6 +382,11 @@ pub enum Event {
 		/// The hash for which the preimage should be handed to the ChannelManager. Note that LDK will
 		/// not stop you from registering duplicate payment hashes for inbound payments.
 		payment_hash: PaymentHash,
+		/// The fields in the onion which were received with each HTLC. Only fields which were
+		/// identical in each HTLC involved in the payment will be included here.
+		///
+		/// Payments received on LDK versions prior to 0.0.115 will have this field unset.
+		onion_fields: Option<RecipientOnionFields>,
 		/// The value, in thousandths of a satoshi, that this payment is for.
 		amount_msat: u64,
 		/// Information for claiming this received payment, based on whether the purpose of the
@@ -820,7 +828,10 @@ impl Writeable for Event {
 				// We never write out FundingGenerationReady events as, upon disconnection, peers
 				// drop any channels which have not yet exchanged funding_signed.
 			},
-			&Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id, ref claim_deadline } => {
+			&Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose,
+				ref receiver_node_id, ref via_channel_id, ref via_user_channel_id,
+				ref claim_deadline, ref onion_fields
+			} => {
 				1u8.write(writer)?;
 				let mut payment_secret = None;
 				let payment_preimage;
@@ -843,6 +854,7 @@ impl Writeable for Event {
 					(6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier
 					(7, claim_deadline, option),
 					(8, payment_preimage, option),
+					(9, onion_fields, option),
 				});
 			},
 			&Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
@@ -1043,6 +1055,7 @@ impl MaybeReadable for Event {
 					let mut via_channel_id = None;
 					let mut claim_deadline = None;
 					let mut via_user_channel_id = None;
+					let mut onion_fields = None;
 					read_tlv_fields!(reader, {
 						(0, payment_hash, required),
 						(1, receiver_node_id, option),
@@ -1053,6 +1066,7 @@ impl MaybeReadable for Event {
 						(6, _user_payment_id, option),
 						(7, claim_deadline, option),
 						(8, payment_preimage, option),
+						(9, onion_fields, option),
 					});
 					let purpose = match payment_secret {
 						Some(secret) => PaymentPurpose::InvoicePayment {
@@ -1070,6 +1084,7 @@ impl MaybeReadable for Event {
 						via_channel_id,
 						via_user_channel_id,
 						claim_deadline,
+						onion_fields,
 					}))
 				};
 				f()

lightning/src/ln/channelmanager.rs

@@ -106,11 +106,13 @@ pub(super) enum PendingHTLCRouting {
 	},
 	Receive {
 		payment_data: msgs::FinalOnionHopData,
+		payment_metadata: Option<Vec<u8>>,
 		incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed
 		phantom_shared_secret: Option<[u8; 32]>,
 	},
 	ReceiveKeysend {
 		payment_preimage: PaymentPreimage,
+		payment_metadata: Option<Vec<u8>>,
 		incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed
 	},
 }
@@ -470,6 +472,12 @@ impl_writeable_tlv_based!(ClaimingPayment, {
 	(4, receiver_node_id, required),
 });
 
+struct ClaimablePayment {
+	purpose: events::PaymentPurpose,
+	onion_fields: Option<RecipientOnionFields>,
+	htlcs: Vec<ClaimableHTLC>,
+}
+
 /// Information about claimable or being-claimed payments
 struct ClaimablePayments {
 	/// Map from payment hash to the payment data and any HTLCs which are to us and can be
@@ -480,7 +488,7 @@ struct ClaimablePayments {
 	///
 	/// When adding to the map, [`Self::pending_claiming_payments`] must also be checked to ensure
 	/// we don't get a duplicate payment.
-	claimable_htlcs: HashMap<PaymentHash, (events::PaymentPurpose, Vec<ClaimableHTLC>)>,
+	claimable_payments: HashMap<PaymentHash, ClaimablePayment>,
 
 	/// Map from payment hash to the payment data for HTLCs which we have begun claiming, but which
 	/// are waiting on a [`ChannelMonitorUpdate`] to complete in order to be surfaced to the user
@@ -1761,7 +1769,7 @@ where
 			pending_inbound_payments: Mutex::new(HashMap::new()),
 			pending_outbound_payments: OutboundPayments::new(),
 			forward_htlcs: Mutex::new(HashMap::new()),
-			claimable_payments: Mutex::new(ClaimablePayments { claimable_htlcs: HashMap::new(), pending_claiming_payments: HashMap::new() }),
+			claimable_payments: Mutex::new(ClaimablePayments { claimable_payments: HashMap::new(), pending_claiming_payments: HashMap::new() }),
 			pending_intercepted_htlcs: Mutex::new(HashMap::new()),
 			id_to_peer: Mutex::new(HashMap::new()),
 			short_to_chan_info: FairRwLock::new(HashMap::new()),
@@ -2256,7 +2264,7 @@ where
 					msg: "Got non final data with an HMAC of 0",
 				});
 			},
-			msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage } => {
+			msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage, payment_metadata } => {
 				if payment_data.is_some() && keysend_preimage.is_some() {
 					return Err(ReceiveError {
 						err_code: 0x4000|22,
@@ -2266,6 +2274,7 @@ where
 				} else if let Some(data) = payment_data {
 					PendingHTLCRouting::Receive {
 						payment_data: data,
+						payment_metadata,
 						incoming_cltv_expiry: hop_data.outgoing_cltv_value,
 						phantom_shared_secret,
 					}
@@ -2286,6 +2295,7 @@ where
 
 					PendingHTLCRouting::ReceiveKeysend {
 						payment_preimage,
+						payment_metadata,
 						incoming_cltv_expiry: hop_data.outgoing_cltv_value,
 					}
 				} else {
@@ -2799,6 +2809,11 @@ where
 		self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, recipient_onion, payment_id, route, None, &self.entropy_source, best_block_height)
 	}
 
+	#[cfg(test)]
+	pub(crate) fn test_set_payment_metadata(&self, payment_id: PaymentId, new_payment_metadata: Option<Vec<u8>>) {
+		self.pending_outbound_payments.test_set_payment_metadata(payment_id, new_payment_metadata);
+	}
+
 
 	/// Signals that no further retries for the given payment should occur. Useful if you have a
 	/// pending outbound payment with retries remaining, but wish to stop retrying the payment before
@@ -3383,7 +3398,7 @@ where
 						}
 					}
 				} else {
-					for forward_info in pending_forwards.drain(..) {
+					'next_forwardable_htlc: for forward_info in pending_forwards.drain(..) {
 						match forward_info {
 							HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
 								prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
@@ -3391,13 +3406,19 @@ where
 									routing, incoming_shared_secret, payment_hash, incoming_amt_msat, outgoing_amt_msat, ..
 								}
 							}) => {
-								let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret) = match routing {
+								let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret, mut onion_fields) = match routing {
-									PendingHTLCRouting::Receive { payment_data, incoming_cltv_expiry, phantom_shared_secret } => {
+									PendingHTLCRouting::Receive { payment_data, payment_metadata, incoming_cltv_expiry, phantom_shared_secret } => {
 										let _legacy_hop_data = Some(payment_data.clone());
-										(incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data }, Some(payment_data), phantom_shared_secret)
+										let onion_fields =
+											RecipientOnionFields { payment_secret: Some(payment_data.payment_secret), payment_metadata };
+										(incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data },
+											Some(payment_data), phantom_shared_secret, onion_fields)
+									},
+									PendingHTLCRouting::ReceiveKeysend { payment_preimage, payment_metadata, incoming_cltv_expiry } => {
+										let onion_fields = RecipientOnionFields { payment_secret: None, payment_metadata };
+										(incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage),
+											None, None, onion_fields)
 									},
-									PendingHTLCRouting::ReceiveKeysend { payment_preimage, incoming_cltv_expiry } =>
-										(incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), None, None),
 									_ => {
 										panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive");
 									}
@@ -3422,8 +3443,11 @@ where
 									onion_payload,
 								};
 
+								let mut committed_to_claimable = false;
+
 								macro_rules! fail_htlc {
 									($htlc: expr, $payment_hash: expr) => {
+										debug_assert!(!committed_to_claimable);
 										let mut htlc_msat_height_data = $htlc.value.to_be_bytes().to_vec();
 										htlc_msat_height_data.extend_from_slice(
 											&self.best_block.read().unwrap().height().to_be_bytes(),
@@ -3438,6 +3462,7 @@ where
 											HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
 											HTLCDestination::FailedPayment { payment_hash: $payment_hash },
 										));
+										continue 'next_forwardable_htlc;
 									}
 								}
 								let phantom_shared_secret = claimable_htlc.prev_hop.phantom_shared_secret;
@@ -3459,15 +3484,28 @@ where
 										let mut claimable_payments = self.claimable_payments.lock().unwrap();
 										if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) {
 											fail_htlc!(claimable_htlc, payment_hash);
-											continue
 										}
-										let (_, ref mut htlcs) = claimable_payments.claimable_htlcs.entry(payment_hash)
+										let ref mut claimable_payment = claimable_payments.claimable_payments
-											.or_insert_with(|| (purpose(), Vec::new()));
+											.entry(payment_hash)
+											// Note that if we insert here we MUST NOT fail_htlc!()
+											.or_insert_with(|| {
+												committed_to_claimable = true;
+												ClaimablePayment {
+													purpose: purpose(), htlcs: Vec::new(), onion_fields: None,
+												}
+											});
+										if let Some(earlier_fields) = &mut claimable_payment.onion_fields {
+											if earlier_fields.check_merge(&mut onion_fields).is_err() {
+												fail_htlc!(claimable_htlc, payment_hash);
+											}
+										} else {
+											claimable_payment.onion_fields = Some(onion_fields);
+										}
+										let ref mut htlcs = &mut claimable_payment.htlcs;
 										if htlcs.len() == 1 {
 											if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload {
 												log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash", log_bytes!(payment_hash.0));
 												fail_htlc!(claimable_htlc, payment_hash);
-												continue
 											}
 										}
 										let mut total_value = claimable_htlc.sender_intended_value;
@@ -3496,6 +3534,9 @@ where
 												log_bytes!(payment_hash.0));
 											fail_htlc!(claimable_htlc, payment_hash);
 										} else if total_value >= $payment_data.total_msat {
+											#[allow(unused_assignments)] {
+												committed_to_claimable = true;
+											}
 											let prev_channel_id = prev_funding_outpoint.to_channel_id();
 											htlcs.push(claimable_htlc);
 											let amount_msat = htlcs.iter().map(|htlc| htlc.value).sum();
@@ -3508,6 +3549,7 @@ where
 												via_channel_id: Some(prev_channel_id),
 												via_user_channel_id: Some(prev_user_channel_id),
 												claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER),
+												onion_fields: claimable_payment.onion_fields.clone(),
 											});
 											payment_claimable_generated = true;
 										} else {
@@ -3515,6 +3557,9 @@ where
 											// payment value yet, wait until we receive more
 											// MPP parts.
 											htlcs.push(claimable_htlc);
+											#[allow(unused_assignments)] {
+												committed_to_claimable = true;
+											}
 										}
 										payment_claimable_generated
 									}}
@@ -3537,7 +3582,6 @@ where
 													Err(()) => {
 														log_trace!(self.logger, "Failing new HTLC with payment_hash {} as payment verification failed", log_bytes!(payment_hash.0));
 														fail_htlc!(claimable_htlc, payment_hash);
-														continue
 													}
 												};
 												if let Some(min_final_cltv_expiry_delta) = min_final_cltv_expiry_delta {
@@ -3546,7 +3590,6 @@ where
 														log_trace!(self.logger, "Failing new HTLC with payment_hash {} as its CLTV expiry was too soon (had {}, earliest expected {})",
 															log_bytes!(payment_hash.0), cltv_expiry, expected_min_expiry_height);
 														fail_htlc!(claimable_htlc, payment_hash);
-														continue;
 													}
 												}
 												check_total_value!(payment_data, payment_preimage);
@@ -3555,15 +3598,18 @@ where
 												let mut claimable_payments = self.claimable_payments.lock().unwrap();
 												if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) {
 													fail_htlc!(claimable_htlc, payment_hash);
-													continue
 												}
-												match claimable_payments.claimable_htlcs.entry(payment_hash) {
+												match claimable_payments.claimable_payments.entry(payment_hash) {
 													hash_map::Entry::Vacant(e) => {
 														let amount_msat = claimable_htlc.value;
 														claimable_htlc.total_value_received = Some(amount_msat);
 														let claim_deadline = Some(claimable_htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER);
 														let purpose = events::PaymentPurpose::SpontaneousPayment(preimage);
-														e.insert((purpose.clone(), vec![claimable_htlc]));
+														e.insert(ClaimablePayment {
+															purpose: purpose.clone(),
+															onion_fields: Some(onion_fields.clone()),
+															htlcs: vec![claimable_htlc],
+														});
 														let prev_channel_id = prev_funding_outpoint.to_channel_id();
 														new_events.push(events::Event::PaymentClaimable {
 															receiver_node_id: Some(receiver_node_id),
@@ -3573,6 +3619,7 @@ where
 															via_channel_id: Some(prev_channel_id),
 															via_user_channel_id: Some(prev_user_channel_id),
 															claim_deadline,
+															onion_fields: Some(onion_fields),
 														});
 													},
 													hash_map::Entry::Occupied(_) => {
@@ -3587,7 +3634,6 @@ where
 										if payment_data.is_none() {
 											log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} because we already have an inbound payment with the same payment hash", log_bytes!(payment_hash.0));
 											fail_htlc!(claimable_htlc, payment_hash);
-											continue
 										};
 										let payment_data = payment_data.unwrap();
 										if inbound_payment.get().payment_secret != payment_data.payment_secret {
@@ -3832,24 +3878,27 @@ where
 				}
 			}
 
-			self.claimable_payments.lock().unwrap().claimable_htlcs.retain(|payment_hash, (_, htlcs)| {
+			self.claimable_payments.lock().unwrap().claimable_payments.retain(|payment_hash, payment| {
-				if htlcs.is_empty() {
+				if payment.htlcs.is_empty() {
 					// This should be unreachable
 					debug_assert!(false);
 					return false;
 				}
-				if let OnionPayload::Invoice { .. } = htlcs[0].onion_payload {
+				if let OnionPayload::Invoice { .. } = payment.htlcs[0].onion_payload {
 					// Check if we've received all the parts we need for an MPP (the value of the parts adds to total_msat).
 					// In this case we're not going to handle any timeouts of the parts here.
 					// This condition determining whether the MPP is complete here must match
 					// exactly the condition used in `process_pending_htlc_forwards`.
-					if htlcs[0].total_msat <= htlcs.iter().fold(0, |total, htlc| total + htlc.sender_intended_value) {
+					if payment.htlcs[0].total_msat <= payment.htlcs.iter()
+						.fold(0, |total, htlc| total + htlc.sender_intended_value)
+					{
 						return true;
-					} else if htlcs.into_iter().any(|htlc| {
+					} else if payment.htlcs.iter_mut().any(|htlc| {
 						htlc.timer_ticks += 1;
 						return htlc.timer_ticks >= MPP_TIMEOUT_TICKS
 					}) {
-						timed_out_mpp_htlcs.extend(htlcs.drain(..).map(|htlc: ClaimableHTLC| (htlc.prev_hop, *payment_hash)));
+						timed_out_mpp_htlcs.extend(payment.htlcs.drain(..)
+							.map(|htlc: ClaimableHTLC| (htlc.prev_hop, *payment_hash)));
 						return false;
 					}
 				}
@@ -3904,9 +3953,9 @@ where
 	pub fn fail_htlc_backwards_with_reason(&self, payment_hash: &PaymentHash, failure_code: FailureCode) {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
-		let removed_source = self.claimable_payments.lock().unwrap().claimable_htlcs.remove(payment_hash);
+		let removed_source = self.claimable_payments.lock().unwrap().claimable_payments.remove(payment_hash);
-		if let Some((_, mut sources)) = removed_source {
+		if let Some(payment) = removed_source {
-			for htlc in sources.drain(..) {
+			for htlc in payment.htlcs {
 				let reason = self.get_htlc_fail_reason_from_failure_code(failure_code, &htlc);
 				let source = HTLCSource::PreviousHopData(htlc.prev_hop);
 				let receiver = HTLCDestination::FailedPayment { payment_hash: *payment_hash };
@@ -4083,9 +4132,9 @@ where
 
 		let mut sources = {
 			let mut claimable_payments = self.claimable_payments.lock().unwrap();
-			if let Some((payment_purpose, sources)) = claimable_payments.claimable_htlcs.remove(&payment_hash) {
+			if let Some(payment) = claimable_payments.claimable_payments.remove(&payment_hash) {
 				let mut receiver_node_id = self.our_network_pubkey;
-				for htlc in sources.iter() {
+				for htlc in payment.htlcs.iter() {
 					if htlc.prev_hop.phantom_shared_secret.is_some() {
 						let phantom_pubkey = self.node_signer.get_node_id(Recipient::PhantomNode)
 							.expect("Failed to get node_id for phantom node recipient");
@@ -4095,15 +4144,15 @@ where
 				}
 
 				let dup_purpose = claimable_payments.pending_claiming_payments.insert(payment_hash,
-					ClaimingPayment { amount_msat: sources.iter().map(|source| source.value).sum(),
+					ClaimingPayment { amount_msat: payment.htlcs.iter().map(|source| source.value).sum(),
-					payment_purpose, receiver_node_id,
+					payment_purpose: payment.purpose, receiver_node_id,
 				});
 				if dup_purpose.is_some() {
 					debug_assert!(false, "Shouldn't get a duplicate pending claim event ever");
 					log_error!(self.logger, "Got a duplicate pending claimable event on payment hash {}! Please report this bug",
 						log_bytes!(payment_hash.0));
 				}
-				sources
+				payment.htlcs
 			} else { return; }
 		};
 		debug_assert!(!sources.is_empty());
@@ -6175,8 +6224,8 @@ where
 		}
 
 		if let Some(height) = height_opt {
-			self.claimable_payments.lock().unwrap().claimable_htlcs.retain(|payment_hash, (_, htlcs)| {
+			self.claimable_payments.lock().unwrap().claimable_payments.retain(|payment_hash, payment| {
-				htlcs.retain(|htlc| {
+				payment.htlcs.retain(|htlc| {
 					// If height is approaching the number of blocks we think it takes us to get
 					// our commitment transaction confirmed before the HTLC expires, plus the
 					// number of blocks we generally consider it to take to do a commitment update,
@@ -6191,7 +6240,7 @@ where
 						false
 					} else { true }
 				});
-				!htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
+				!payment.htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
 			});
 
 			let mut intercepted_htlcs = self.pending_intercepted_htlcs.lock().unwrap();
@@ -6776,10 +6825,12 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting,
 		(0, payment_data, required),
 		(1, phantom_shared_secret, option),
 		(2, incoming_cltv_expiry, required),
+		(3, payment_metadata, option),
 	},
 	(2, ReceiveKeysend) => {
 		(0, payment_preimage, required),
 		(2, incoming_cltv_expiry, required),
+		(3, payment_metadata, option),
 	},
 ;);
 
@@ -7112,14 +7163,16 @@ where
 		let pending_outbound_payments = self.pending_outbound_payments.pending_outbound_payments.lock().unwrap();
 
 		let mut htlc_purposes: Vec<&events::PaymentPurpose> = Vec::new();
-		(claimable_payments.claimable_htlcs.len() as u64).write(writer)?;
+		let mut htlc_onion_fields: Vec<&_> = Vec::new();
-		for (payment_hash, (purpose, previous_hops)) in claimable_payments.claimable_htlcs.iter() {
+		(claimable_payments.claimable_payments.len() as u64).write(writer)?;
+		for (payment_hash, payment) in claimable_payments.claimable_payments.iter() {
 			payment_hash.write(writer)?;
-			(previous_hops.len() as u64).write(writer)?;
+			(payment.htlcs.len() as u64).write(writer)?;
-			for htlc in previous_hops.iter() {
+			for htlc in payment.htlcs.iter() {
 				htlc.write(writer)?;
 			}
-			htlc_purposes.push(purpose);
+			htlc_purposes.push(&payment.purpose);
+			htlc_onion_fields.push(&payment.onion_fields);
 		}
 
 		let mut monitor_update_blocked_actions_per_peer = None;
@@ -7234,6 +7287,7 @@ where
 			(7, self.fake_scid_rand_bytes, required),
 			(9, htlc_purposes, vec_type),
 			(11, self.probing_cookie_secret, required),
+			(13, htlc_onion_fields, optional_vec),
 		});
 
 		Ok(())
@@ -7612,6 +7666,7 @@ where
 		let mut fake_scid_rand_bytes: Option<[u8; 32]> = None;
 		let mut probing_cookie_secret: Option<[u8; 32]> = None;
 		let mut claimable_htlc_purposes = None;
+		let mut claimable_htlc_onion_fields = None;
 		let mut pending_claiming_payments = Some(HashMap::new());
 		let mut monitor_update_blocked_actions_per_peer = Some(Vec::new());
 		read_tlv_fields!(reader, {
@@ -7624,6 +7679,7 @@ where
 			(7, fake_scid_rand_bytes, option),
 			(9, claimable_htlc_purposes, vec_type),
 			(11, probing_cookie_secret, option),
+			(13, claimable_htlc_onion_fields, optional_vec),
 		});
 		if fake_scid_rand_bytes.is_none() {
 			fake_scid_rand_bytes = Some(args.entropy_source.get_secure_random_bytes());
@@ -7687,6 +7743,7 @@ where
 										session_privs: [session_priv_bytes].iter().map(|a| *a).collect(),
 										payment_hash: htlc.payment_hash,
 										payment_secret: None, // only used for retries, and we'll never retry on startup
+										payment_metadata: None, // only used for retries, and we'll never retry on startup
 										keysend_preimage: None, // only used for retries, and we'll never retry on startup
 										pending_amt_msat: path_amt,
 										pending_fee_msat: Some(path_fee),
@@ -7771,22 +7828,39 @@ where
 		let inbound_pmt_key_material = args.node_signer.get_inbound_payment_key_material();
 		let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material);
 
-		let mut claimable_htlcs = HashMap::with_capacity(claimable_htlcs_list.len());
+		let mut claimable_payments = HashMap::with_capacity(claimable_htlcs_list.len());
-		if let Some(mut purposes) = claimable_htlc_purposes {
+		if let Some(purposes) = claimable_htlc_purposes {
 			if purposes.len() != claimable_htlcs_list.len() {
 				return Err(DecodeError::InvalidValue);
 			}
-			for (purpose, (payment_hash, previous_hops)) in purposes.drain(..).zip(claimable_htlcs_list.drain(..)) {
+			if let Some(onion_fields) = claimable_htlc_onion_fields {
-				claimable_htlcs.insert(payment_hash, (purpose, previous_hops));
+				if onion_fields.len() != claimable_htlcs_list.len() {
+					return Err(DecodeError::InvalidValue);
+				}
+				for (purpose, (onion, (payment_hash, htlcs))) in
+					purposes.into_iter().zip(onion_fields.into_iter().zip(claimable_htlcs_list.into_iter()))
+				{
+					let existing_payment = claimable_payments.insert(payment_hash, ClaimablePayment {
+						purpose, htlcs, onion_fields: onion,
+					});
+					if existing_payment.is_some() { return Err(DecodeError::InvalidValue); }
+				}
+			} else {
+				for (purpose, (payment_hash, htlcs)) in purposes.into_iter().zip(claimable_htlcs_list.into_iter()) {
+					let existing_payment = claimable_payments.insert(payment_hash, ClaimablePayment {
+						purpose, htlcs, onion_fields: None,
+					});
+					if existing_payment.is_some() { return Err(DecodeError::InvalidValue); }
+				}
 			}
 		} else {
 			// LDK versions prior to 0.0.107 did not write a `pending_htlc_purposes`, but do
 			// include a `_legacy_hop_data` in the `OnionPayload`.
-			for (payment_hash, previous_hops) in claimable_htlcs_list.drain(..) {
+			for (payment_hash, htlcs) in claimable_htlcs_list.drain(..) {
-				if previous_hops.is_empty() {
+				if htlcs.is_empty() {
 					return Err(DecodeError::InvalidValue);
 				}
-				let purpose = match &previous_hops[0].onion_payload {
+				let purpose = match &htlcs[0].onion_payload {
 					OnionPayload::Invoice { _legacy_hop_data } => {
 						if let Some(hop_data) = _legacy_hop_data {
 							events::PaymentPurpose::InvoicePayment {
@@ -7807,7 +7881,9 @@ where
 					OnionPayload::Spontaneous(payment_preimage) =>
 						events::PaymentPurpose::SpontaneousPayment(*payment_preimage),
 				};
-				claimable_htlcs.insert(payment_hash, (purpose, previous_hops));
+				claimable_payments.insert(payment_hash, ClaimablePayment {
+					purpose, htlcs, onion_fields: None,
+				});
 			}
 		}
 
@@ -7859,17 +7935,17 @@ where
 
 		for (_, monitor) in args.channel_monitors.iter() {
 			for (payment_hash, payment_preimage) in monitor.get_stored_preimages() {
-				if let Some((payment_purpose, claimable_htlcs)) = claimable_htlcs.remove(&payment_hash) {
+				if let Some(payment) = claimable_payments.remove(&payment_hash) {
 					log_info!(args.logger, "Re-claiming HTLCs with payment hash {} as we've released the preimage to a ChannelMonitor!", log_bytes!(payment_hash.0));
 					let mut claimable_amt_msat = 0;
 					let mut receiver_node_id = Some(our_network_pubkey);
-					let phantom_shared_secret = claimable_htlcs[0].prev_hop.phantom_shared_secret;
+					let phantom_shared_secret = payment.htlcs[0].prev_hop.phantom_shared_secret;
 					if phantom_shared_secret.is_some() {
 						let phantom_pubkey = args.node_signer.get_node_id(Recipient::PhantomNode)
 							.expect("Failed to get node_id for phantom node recipient");
 						receiver_node_id = Some(phantom_pubkey)
 					}
-					for claimable_htlc in claimable_htlcs {
+					for claimable_htlc in payment.htlcs {
 						claimable_amt_msat += claimable_htlc.value;
 
 						// Add a holding-cell claim of the payment to the Channel, which should be
@@ -7903,7 +7979,7 @@ where
 					pending_events_read.push(events::Event::PaymentClaimed {
 						receiver_node_id,
 						payment_hash,
-						purpose: payment_purpose,
+						purpose: payment.purpose,
 						amount_msat: claimable_amt_msat,
 					});
 				}
@@ -7934,7 +8010,7 @@ where
 			pending_intercepted_htlcs: Mutex::new(pending_intercepted_htlcs.unwrap()),
 
 			forward_htlcs: Mutex::new(forward_htlcs),
-			claimable_payments: Mutex::new(ClaimablePayments { claimable_htlcs, pending_claiming_payments: pending_claiming_payments.unwrap() }),
+			claimable_payments: Mutex::new(ClaimablePayments { claimable_payments, pending_claiming_payments: pending_claiming_payments.unwrap() }),
 			outbound_scid_aliases: Mutex::new(outbound_scid_aliases),
 			id_to_peer: Mutex::new(id_to_peer),
 			short_to_chan_info: FairRwLock::new(short_to_chan_info),

lightning/src/ln/features.rs

@@ -45,17 +45,25 @@
 //!     (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md) for more information).
 //! - `OnionMessages` - requires/supports forwarding onion messages
 //!     (see [BOLT-7](https://github.com/lightning/bolts/pull/759/files) for more information).
-//!     TODO: update link
+//     TODO: update link
 //! - `ChannelType` - node supports the channel_type field in open/accept
 //!     (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md) for more information).
 //! - `SCIDPrivacy` - supply channel aliases for routing
 //!     (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md) for more information).
+//! - `PaymentMetadata` - include additional data in invoices which is passed to recipients in the
+//!      onion.
+//!      (see [BOLT-11](https://github.com/lightning/bolts/blob/master/11-payment-encoding.md) for
+//!      more).
+//! - `ZeroConf` - supports accepting HTLCs and using channels prior to funding confirmation
+//!      (see
+//!      [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#the-channel_ready-message)
+//!      for more info).
 //! - `Keysend` - send funds to a node without an invoice
 //!     (see the [`Keysend` feature assignment proposal](https://github.com/lightning/bolts/issues/605#issuecomment-606679798) for more information).
 //! - `AnchorsZeroFeeHtlcTx` - requires/supports that commitment transactions include anchor outputs
-//!   and HTLC transactions are pre-signed with zero fee (see
+//!     and HTLC transactions are pre-signed with zero fee (see
-//!   [BOLT-3](https://github.com/lightning/bolts/blob/master/03-transactions.md) for more
+//!     [BOLT-3](https://github.com/lightning/bolts/blob/master/03-transactions.md) for more
-//!   information).
+//!     information).
 //!
 //! [BOLT #9]: https://github.com/lightning/bolts/blob/master/09-features.md
 //! [messages]: crate::ln::msgs
@@ -160,6 +168,14 @@ mod sealed {
 		VariableLengthOnion | PaymentSecret,
 		// Byte 2
 		BasicMPP,
+		// Byte 3
+		,
+		// Byte 4
+		,
+		// Byte 5
+		,
+		// Byte 6
+		PaymentMetadata,
 	]);
 	define_context!(OfferContext, []);
 	define_context!(InvoiceRequestContext, []);
@@ -259,6 +275,7 @@ mod sealed {
 					}
 
 					flags[Self::BYTE_OFFSET] |= Self::REQUIRED_MASK;
+					flags[Self::BYTE_OFFSET] &= !Self::OPTIONAL_MASK;
 				}
 
 				/// Sets the feature's optional (odd) bit in the given flags.
@@ -376,12 +393,16 @@ mod sealed {
 	define_feature!(47, SCIDPrivacy, [InitContext, NodeContext, ChannelTypeContext],
 		"Feature flags for only forwarding with SCID aliasing. Called `option_scid_alias` in the BOLTs",
 		set_scid_privacy_optional, set_scid_privacy_required, supports_scid_privacy, requires_scid_privacy);
+	define_feature!(49, PaymentMetadata, [InvoiceContext],
+		"Feature flags for payment metadata in invoices.", set_payment_metadata_optional,
+		set_payment_metadata_required, supports_payment_metadata, requires_payment_metadata);
 	define_feature!(51, ZeroConf, [InitContext, NodeContext, ChannelTypeContext],
 		"Feature flags for accepting channels with zero confirmations. Called `option_zeroconf` in the BOLTs",
 		set_zero_conf_optional, set_zero_conf_required, supports_zero_conf, requires_zero_conf);
 	define_feature!(55, Keysend, [NodeContext],
 		"Feature flags for keysend payments.", set_keysend_optional, set_keysend_required,
 		supports_keysend, requires_keysend);
+	// Note: update the module-level docs when a new feature bit is added!
 
 	#[cfg(test)]
 	define_feature!(123456789, UnknownFeature,
@@ -885,13 +906,13 @@ mod tests {
 	#[test]
 	fn convert_to_context_with_unknown_flags() {
 		// Ensure the `from` context has fewer known feature bytes than the `to` context.
-		assert!(<sealed::InvoiceContext as sealed::Context>::KNOWN_FEATURE_MASK.len() <
+		assert!(<sealed::ChannelContext as sealed::Context>::KNOWN_FEATURE_MASK.len() <
-			<sealed::NodeContext as sealed::Context>::KNOWN_FEATURE_MASK.len());
+			<sealed::InvoiceContext as sealed::Context>::KNOWN_FEATURE_MASK.len());
-		let mut invoice_features = InvoiceFeatures::empty();
+		let mut channel_features = ChannelFeatures::empty();
-		invoice_features.set_unknown_feature_optional();
+		channel_features.set_unknown_feature_optional();
-		assert!(invoice_features.supports_unknown_bits());
+		assert!(channel_features.supports_unknown_bits());
-		let node_features: NodeFeatures = invoice_features.to_context();
+		let invoice_features: InvoiceFeatures = channel_features.to_context_internal();
-		assert!(!node_features.supports_unknown_bits());
+		assert!(!invoice_features.supports_unknown_bits());
 	}
 
 	#[test]

lightning/src/ln/functional_test_utils.rs

@@ -2034,21 +2034,26 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p
 			let events_2 = node.node.get_and_clear_pending_events();
 			if payment_claimable_expected {
 				assert_eq!(events_2.len(), 1);
-				match events_2[0] {
+				match &events_2[0] {
-					Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_id, ref via_user_channel_id, claim_deadline } => {
+					Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat,
+						receiver_node_id, ref via_channel_id, ref via_user_channel_id,
+						claim_deadline, onion_fields,
+					} => {
 						assert_eq!(our_payment_hash, *payment_hash);
 						assert_eq!(node.node.get_our_node_id(), receiver_node_id.unwrap());
+						assert!(onion_fields.is_some());
 						match &purpose {
 							PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 								assert_eq!(expected_preimage, *payment_preimage);
 								assert_eq!(our_payment_secret.unwrap(), *payment_secret);
+								assert_eq!(Some(*payment_secret), onion_fields.as_ref().unwrap().payment_secret);
 							},
 							PaymentPurpose::SpontaneousPayment(payment_preimage) => {
 								assert_eq!(expected_preimage.unwrap(), *payment_preimage);
 								assert!(our_payment_secret.is_none());
 							},
 						}
-						assert_eq!(amount_msat, recv_value);
+						assert_eq!(*amount_msat, recv_value);
 						assert!(node.node.list_channels().iter().any(|details| details.channel_id == via_channel_id.unwrap()));
 						assert!(node.node.list_channels().iter().any(|details| details.user_channel_id == via_user_channel_id.unwrap()));
 						assert!(claim_deadline.unwrap() > node.best_block_info().1);

lightning/src/ln/msgs.rs

@@ -42,7 +42,7 @@ use crate::io_extras::read_to_end;
 
 use crate::events::{MessageSendEventsProvider, OnionMessageProvider};
 use crate::util::logger;
-use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};
+use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, WithoutLength, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};
 
 use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
 
@@ -1168,6 +1168,7 @@ mod fuzzy_internal_msgs {
 		},
 		FinalNode {
 			payment_data: Option<FinalOnionHopData>,
+			payment_metadata: Option<Vec<u8>>,
 			keysend_preimage: Option<PaymentPreimage>,
 		},
 	}
@@ -1661,11 +1662,12 @@ impl Writeable for OnionHopData {
 					(6, short_channel_id, required)
 				});
 			},
-			OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
+			OnionHopDataFormat::FinalNode { ref payment_data, ref payment_metadata, ref keysend_preimage } => {
 				_encode_varint_length_prefixed_tlv!(w, {
 					(2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 					(4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 					(8, payment_data, option),
+					(16, payment_metadata.as_ref().map(|m| WithoutLength(m)), option),
 					(5482373484, keysend_preimage, option)
 				});
 			},
@@ -1680,29 +1682,33 @@ impl Readable for OnionHopData {
 		let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
 		let mut short_id: Option<u64> = None;
 		let mut payment_data: Option<FinalOnionHopData> = None;
+		let mut payment_metadata: Option<WithoutLength<Vec<u8>>> = None;
 		let mut keysend_preimage: Option<PaymentPreimage> = None;
 		read_tlv_fields!(r, {
 			(2, amt, required),
 			(4, cltv_value, required),
 			(6, short_id, option),
 			(8, payment_data, option),
+			(16, payment_metadata, option),
 			// See https://github.com/lightning/blips/blob/master/blip-0003.md
 			(5482373484, keysend_preimage, option)
 		});
 
 		let format = if let Some(short_channel_id) = short_id {
 			if payment_data.is_some() { return Err(DecodeError::InvalidValue); }
+			if payment_metadata.is_some() { return Err(DecodeError::InvalidValue); }
 			OnionHopDataFormat::NonFinalNode {
 				short_channel_id,
 			}
 		} else {
-			if let &Some(ref data) = &payment_data {
+			if let Some(data) = &payment_data {
 				if data.total_msat > MAX_VALUE_MSAT {
 					return Err(DecodeError::InvalidValue);
 				}
 			}
 			OnionHopDataFormat::FinalNode {
 				payment_data,
+				payment_metadata: payment_metadata.map(|w| w.0),
 				keysend_preimage,
 			}
 		};
@@ -2880,6 +2886,7 @@ mod tests {
 		let mut msg = msgs::OnionHopData {
 			format: OnionHopDataFormat::FinalNode {
 				payment_data: None,
+				payment_metadata: None,
 				keysend_preimage: None,
 			},
 			amt_to_forward: 0x0badf00d01020304,
@@ -2903,6 +2910,7 @@ mod tests {
 					payment_secret: expected_payment_secret,
 					total_msat: 0x1badca1f
 				}),
+				payment_metadata: None,
 				keysend_preimage: None,
 			},
 			amt_to_forward: 0x0badf00d01020304,
@@ -2917,6 +2925,7 @@ mod tests {
 				payment_secret,
 				total_msat: 0x1badca1f
 			}),
+			payment_metadata: None,
 			keysend_preimage: None,
 		} = msg.format {
 			assert_eq!(payment_secret, expected_payment_secret);

lightning/src/ln/onion_utils.rs

@@ -170,6 +170,7 @@ pub(super) fn build_onion_payloads(path: &Vec<RouteHop>, total_msat: u64, mut re
 							total_msat,
 						})
 					} else { None },
+					payment_metadata: recipient_onion.payment_metadata.take(),
 					keysend_preimage: *keysend_preimage,
 				}
 			} else {

lightning/src/ln/outbound_payment.rs

@@ -46,6 +46,7 @@ pub(crate) enum PendingOutboundPayment {
 		session_privs: HashSet<[u8; 32]>,
 		payment_hash: PaymentHash,
 		payment_secret: Option<PaymentSecret>,
+		payment_metadata: Option<Vec<u8>>,
 		keysend_preimage: Option<PaymentPreimage>,
 		pending_amt_msat: u64,
 		/// Used to track the fee paid. Only present if the payment was serialized on 0.0.103+.
@@ -405,7 +406,7 @@ pub enum PaymentSendFailure {
 ///
 /// This should generally be constructed with data communicated to us from the recipient (via a
 /// BOLT11 or BOLT12 invoice).
-#[derive(Clone)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct RecipientOnionFields {
 	/// The [`PaymentSecret`] is an arbitrary 32 bytes provided by the recipient for us to repeat
 	/// in the onion. It is unrelated to `payment_hash` (or [`PaymentPreimage`]) and exists to
@@ -419,14 +420,32 @@ pub struct RecipientOnionFields {
 	/// receives, thus you should generally never be providing a secret here for spontaneous
 	/// payments.
 	pub payment_secret: Option<PaymentSecret>,
+	/// The payment metadata serves a similar purpose as [`Self::payment_secret`] but is of
+	/// arbitrary length. This gives recipients substantially more flexibility to receive
+	/// additional data.
+	///
+	/// In LDK, while the [`Self::payment_secret`] is fixed based on an internal authentication
+	/// scheme to authenticate received payments against expected payments and invoices, this field
+	/// is not used in LDK for received payments, and can be used to store arbitrary data in
+	/// invoices which will be received with the payment.
+	///
+	/// Note that this field was added to the lightning specification more recently than
+	/// [`Self::payment_secret`] and while nearly all lightning senders support secrets, metadata
+	/// may not be supported as universally.
+	pub payment_metadata: Option<Vec<u8>>,
 }
 
+impl_writeable_tlv_based!(RecipientOnionFields, {
+	(0, payment_secret, option),
+	(2, payment_metadata, option),
+});
+
 impl RecipientOnionFields {
 	/// Creates a [`RecipientOnionFields`] from only a [`PaymentSecret`]. This is the most common
 	/// set of onion fields for today's BOLT11 invoices - most nodes require a [`PaymentSecret`]
 	/// but do not require or provide any further data.
 	pub fn secret_only(payment_secret: PaymentSecret) -> Self {
-		Self { payment_secret: Some(payment_secret) }
+		Self { payment_secret: Some(payment_secret), payment_metadata: None }
 	}
 
 	/// Creates a new [`RecipientOnionFields`] with no fields. This generally does not create
@@ -435,7 +454,21 @@ impl RecipientOnionFields {
 	///
 	/// [`ChannelManager::send_spontaneous_payment`]: super::channelmanager::ChannelManager::send_spontaneous_payment
 	pub fn spontaneous_empty() -> Self {
-		Self { payment_secret: None }
+		Self { payment_secret: None, payment_metadata: None }
+	}
+
+	/// When we have received some HTLC(s) towards an MPP payment, as we receive further HTLC(s) we
+	/// have to make sure that some fields match exactly across the parts. For those that aren't
+	/// required to match, if they don't match we should remove them so as to not expose data
+	/// that's dependent on the HTLC receive order to users.
+	///
+	/// Here we implement this, first checking compatibility then mutating two objects and then
+	/// dropping any remaining non-matching fields from both.
+	pub(super) fn check_merge(&mut self, further_htlc_fields: &mut Self) -> Result<(), ()> {
+		if self.payment_secret != further_htlc_fields.payment_secret { return Err(()); }
+		if self.payment_metadata != further_htlc_fields.payment_metadata { return Err(()); }
+		// For custom TLVs we should just drop non-matching ones, but not reject the payment.
+		Ok(())
 	}
 }
 
@@ -722,7 +755,7 @@ impl OutboundPayments {
 				hash_map::Entry::Occupied(mut payment) => {
 					let res = match payment.get() {
 						PendingOutboundPayment::Retryable {
-							total_msat, keysend_preimage, payment_secret, pending_amt_msat, ..
+							total_msat, keysend_preimage, payment_secret, payment_metadata, pending_amt_msat, ..
 						} => {
 							let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum();
 							if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
@@ -732,6 +765,7 @@ impl OutboundPayments {
 							}
 							(*total_msat, RecipientOnionFields {
 									payment_secret: *payment_secret,
+									payment_metadata: payment_metadata.clone(),
 								}, *keysend_preimage)
 						},
 						PendingOutboundPayment::Legacy { .. } => {
@@ -886,6 +920,18 @@ impl OutboundPayments {
 		}
 	}
 
+	#[cfg(test)]
+	pub(super) fn test_set_payment_metadata(
+		&self, payment_id: PaymentId, new_payment_metadata: Option<Vec<u8>>
+	) {
+		match self.pending_outbound_payments.lock().unwrap().get_mut(&payment_id).unwrap() {
+			PendingOutboundPayment::Retryable { payment_metadata, .. } => {
+				*payment_metadata = new_payment_metadata;
+			},
+			_ => panic!("Need a retryable payment to update metadata on"),
+		}
+	}
+
 	#[cfg(test)]
 	pub(super) fn test_add_new_pending_payment<ES: Deref>(
 		&self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId,
@@ -917,6 +963,7 @@ impl OutboundPayments {
 					pending_fee_msat: Some(0),
 					payment_hash,
 					payment_secret: recipient_onion.payment_secret,
+					payment_metadata: recipient_onion.payment_metadata,
 					keysend_preimage,
 					starting_block_height: best_block_height,
 					total_msat: route.get_total_amount(),
@@ -1358,6 +1405,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(4, payment_secret, option),
 		(5, keysend_preimage, option),
 		(6, total_msat, required),
+		(7, payment_metadata, option),
 		(8, pending_amt_msat, required),
 		(10, starting_block_height, required),
 		(not_written, retry_strategy, (static_value, None)),

lightning/src/ln/payment_tests.rs

@@ -3017,3 +3017,151 @@ fn claim_from_closed_chan() {
 	do_claim_from_closed_chan(true);
 	do_claim_from_closed_chan(false);
 }
+
+fn do_test_payment_metadata_consistency(do_reload: bool, do_modify: bool) {
+	// Check that a payment metadata received on one HTLC that doesn't match the one received on
+	// another results in the HTLC being rejected.
+	//
+	// We first set up a diamond shaped network, allowing us to split a payment into two HTLCs, the
+	// first of which we'll deliver and the second of which we'll fail and then re-send with
+	// modified payment metadata, which will in turn result in it being failed by the recipient.
+	let chanmon_cfgs = create_chanmon_cfgs(4);
+	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+	let mut config = test_default_channel_config();
+	config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 50;
+	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, Some(config), Some(config), Some(config)]);
+
+	let persister;
+	let new_chain_monitor;
+	let nodes_0_deserialized;
+
+	let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
+	let chan_id_bd = create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 1_000_000, 0).2;
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 1_000_000, 0);
+	let chan_id_cd = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).2;
+
+	// Pay more than half of each channel's max, requiring MPP
+	let amt_msat = 750_000_000;
+	let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[3], Some(amt_msat));
+	let payment_id = PaymentId(payment_hash.0);
+	let payment_metadata = vec![44, 49, 52, 142];
+
+	let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV)
+		.with_features(nodes[1].node.invoice_features());
+	let mut route_params = RouteParameters {
+		payment_params,
+		final_value_msat: amt_msat,
+	};
+
+	// Send the MPP payment, delivering the updated commitment state to nodes[1].
+	nodes[0].node.send_payment(payment_hash, RecipientOnionFields {
+			payment_secret: Some(payment_secret), payment_metadata: Some(payment_metadata),
+		}, payment_id, route_params.clone(), Retry::Attempts(1)).unwrap();
+	check_added_monitors!(nodes[0], 2);
+
+	let mut send_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(send_events.len(), 2);
+	let first_send = SendEvent::from_event(send_events.pop().unwrap());
+	let second_send = SendEvent::from_event(send_events.pop().unwrap());
+
+	let (b_recv_ev, c_recv_ev) = if first_send.node_id == nodes[1].node.get_our_node_id() {
+		(&first_send, &second_send)
+	} else {
+		(&second_send, &first_send)
+	};
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &b_recv_ev.msgs[0]);
+	commitment_signed_dance!(nodes[1], nodes[0], b_recv_ev.commitment_msg, false, true);
+
+	expect_pending_htlcs_forwardable!(nodes[1]);
+	check_added_monitors(&nodes[1], 1);
+	let b_forward_ev = SendEvent::from_node(&nodes[1]);
+	nodes[3].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &b_forward_ev.msgs[0]);
+	commitment_signed_dance!(nodes[3], nodes[1], b_forward_ev.commitment_msg, false, true);
+
+	expect_pending_htlcs_forwardable!(nodes[3]);
+
+	// Before delivering the second MPP HTLC to nodes[2], disconnect nodes[2] and nodes[3], which
+	// will result in nodes[2] failing the HTLC back.
+	nodes[2].node.peer_disconnected(&nodes[3].node.get_our_node_id());
+	nodes[3].node.peer_disconnected(&nodes[2].node.get_our_node_id());
+
+	nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &c_recv_ev.msgs[0]);
+	commitment_signed_dance!(nodes[2], nodes[0], c_recv_ev.commitment_msg, false, true);
+
+	let cs_fail = get_htlc_update_msgs(&nodes[2], &nodes[0].node.get_our_node_id());
+	nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &cs_fail.update_fail_htlcs[0]);
+	commitment_signed_dance!(nodes[0], nodes[2], cs_fail.commitment_signed, false, true);
+
+	let payment_fail_retryable_evs = nodes[0].node.get_and_clear_pending_events();
+	assert_eq!(payment_fail_retryable_evs.len(), 2);
+	if let Event::PaymentPathFailed { .. } = payment_fail_retryable_evs[0] {} else { panic!(); }
+	if let Event::PendingHTLCsForwardable { .. } = payment_fail_retryable_evs[1] {} else { panic!(); }
+
+	// Before we allow the HTLC to be retried, optionally change the payment_metadata we have
+	// stored for our payment.
+	if do_modify {
+		nodes[0].node.test_set_payment_metadata(payment_id, Some(Vec::new()));
+	}
+
+	// Optionally reload nodes[3] to check that the payment_metadata is properly serialized with
+	// the payment state.
+	if do_reload {
+		let mon_bd = get_monitor!(nodes[3], chan_id_bd).encode();
+		let mon_cd = get_monitor!(nodes[3], chan_id_cd).encode();
+		reload_node!(nodes[3], config, &nodes[3].node.encode(), &[&mon_bd, &mon_cd],
+			persister, new_chain_monitor, nodes_0_deserialized);
+		nodes[1].node.peer_disconnected(&nodes[3].node.get_our_node_id());
+		reconnect_nodes(&nodes[1], &nodes[3], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+	}
+	reconnect_nodes(&nodes[2], &nodes[3], (true, true), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+
+	// Create a new channel between C and D as A will refuse to retry on the existing one because
+	// it just failed.
+	let chan_id_cd_2 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).2;
+
+	// Now retry the failed HTLC.
+	nodes[0].node.process_pending_htlc_forwards();
+	check_added_monitors(&nodes[0], 1);
+	let as_resend = SendEvent::from_node(&nodes[0]);
+	nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_resend.msgs[0]);
+	commitment_signed_dance!(nodes[2], nodes[0], as_resend.commitment_msg, false, true);
+
+	expect_pending_htlcs_forwardable!(nodes[2]);
+	check_added_monitors(&nodes[2], 1);
+	let cs_forward = SendEvent::from_node(&nodes[2]);
+	nodes[3].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &cs_forward.msgs[0]);
+	commitment_signed_dance!(nodes[3], nodes[2], cs_forward.commitment_msg, false, true);
+
+	// Finally, check that nodes[3] does the correct thing - either accepting the payment or, if
+	// the payment metadata was modified, failing only the one modified HTLC and retaining the
+	// other.
+	if do_modify {
+		expect_pending_htlcs_forwardable_ignore!(nodes[3]);
+		nodes[3].node.process_pending_htlc_forwards();
+		expect_pending_htlcs_forwardable_conditions(nodes[3].node.get_and_clear_pending_events(),
+			&[HTLCDestination::FailedPayment {payment_hash}]);
+		nodes[3].node.process_pending_htlc_forwards();
+
+		check_added_monitors(&nodes[3], 1);
+		let ds_fail = get_htlc_update_msgs(&nodes[3], &nodes[2].node.get_our_node_id());
+
+		nodes[2].node.handle_update_fail_htlc(&nodes[3].node.get_our_node_id(), &ds_fail.update_fail_htlcs[0]);
+		commitment_signed_dance!(nodes[2], nodes[3], ds_fail.commitment_signed, false, true);
+		expect_pending_htlcs_forwardable_conditions(nodes[2].node.get_and_clear_pending_events(),
+			&[HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_id_cd_2 }]);
+	} else {
+		expect_pending_htlcs_forwardable!(nodes[3]);
+		expect_payment_claimable!(nodes[3], payment_hash, payment_secret, amt_msat);
+		claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
+	}
+}
+
+#[test]
+fn test_payment_metadata_consistency() {
+	do_test_payment_metadata_consistency(true, true);
+	do_test_payment_metadata_consistency(true, false);
+	do_test_payment_metadata_consistency(false, true);
+	do_test_payment_metadata_consistency(false, false);
+}