Drop non-matching custom TLVs when receiving MPP

Author: alecchendev

lightning/src/ln/outbound_payment.rs

@@ -26,7 +26,7 @@ use crate::util::logger::Logger;
 use crate::util::time::Time;
 #[cfg(all(not(feature = "no-std"), test))]
 use crate::util::time::tests::SinceEpoch;
-use crate::util::ser::ReadableArgs;
+use crate::util::ser::{ReadableArgs, Writeable};
 
 use core::fmt::{self, Display, Formatter};
 use core::ops::Deref;
@@ -491,7 +491,17 @@ impl RecipientOnionFields {
 	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.
+
+		if let (Some(tlvs), Some(further_tlvs)) = (&mut self.custom_tlvs, &further_htlc_fields.custom_tlvs) {
+			let custom_tlvs = TlvStream::new(tlvs);
+			let mut further_custom_tlvs = TlvStream::new(further_tlvs);
+			let mut remaining_tlvs = Vec::new();
+			custom_tlvs.filter(|tlv| further_custom_tlvs.any(|further_tlv| tlv == &further_tlv))
+				.for_each(|tlv| tlv.write(&mut remaining_tlvs).unwrap());
+			// Since both TLV streams should be sorted we could implement linear time intersection
+			// let remaining_tlvs = custom_tlvs.intersection(further_custom_tlvs);
+			self.custom_tlvs = Some(remaining_tlvs);
+		}
 		Ok(())
 	}
 }

lightning/src/ln/payment_tests.rs

@@ -3087,6 +3087,111 @@ fn do_test_custom_tlvs(spontaneous: bool) {
 	claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage);
 }
 
+#[test]
+fn do_test_custom_tlvs_consistency() {
+	// Test that if we recieve two HTLCs with different custom TLVs we drop the non-matching TLVs
+	let chanmon_cfgs = create_chanmon_cfgs(4);
+	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
+	let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0);
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 100_000, 0);
+	create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 100_000, 0);
+	create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 100_000, 0);
+
+	let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV)
+		.with_bolt11_features(nodes[3].node.invoice_features()).unwrap();
+	let mut route = get_route!(nodes[0], payment_params, 15_000_000).unwrap();
+	assert_eq!(route.paths.len(), 2);
+	route.paths.sort_by(|path_a, _| {
+		// Sort the path so that the path through nodes[1] comes first
+		if path_a.hops[0].pubkey == nodes[1].node.get_our_node_id() {
+			core::cmp::Ordering::Less } else { core::cmp::Ordering::Greater }
+	});
+
+	let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[3]);
+	let payment_id = PaymentId([42; 32]);
+	let amt_msat = 15_000_000;
+	let field1 = 0x01020304;
+	let field2 = Some(vec![0x42u8; 16]);
+	let custom_tlvs = create_custom_tlvs(field1, field2);
+	let onion_fields = RecipientOnionFields {
+		payment_secret: Some(our_payment_secret),
+		payment_metadata: None,
+		custom_tlvs: Some(custom_tlvs.clone())
+	};
+	let session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash,
+			onion_fields.clone(), payment_id, &route).unwrap();
+	let cur_height = nodes[0].best_block_info().1;
+	nodes[0].node.test_send_payment_along_path(&route.paths[0], &our_payment_hash,
+		onion_fields.clone(), amt_msat, cur_height, payment_id,
+		&None, session_privs[0]).unwrap();
+	check_added_monitors!(nodes[0], 1);
+
+	{
+		let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+		assert_eq!(events.len(), 1);
+		pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], amt_msat, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), false, None);
+	}
+	assert!(nodes[3].node.get_and_clear_pending_events().is_empty());
+
+	let field1 = 0x01020304;
+	let field2 = None;
+	let custom_tlvs = create_custom_tlvs(field1, field2);
+	let onion_fields = RecipientOnionFields {
+		payment_secret: Some(our_payment_secret),
+		payment_metadata: None,
+		custom_tlvs: Some(custom_tlvs.clone())
+	};
+	nodes[0].node.test_send_payment_along_path(&route.paths[1], &our_payment_hash,
+		onion_fields.clone(), amt_msat, cur_height, payment_id, &None, session_privs[1]).unwrap();
+	check_added_monitors!(nodes[0], 1);
+
+	{
+		let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+		assert_eq!(events.len(), 1);
+		let payment_event = SendEvent::from_event(events.pop().unwrap());
+
+		nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+		commitment_signed_dance!(nodes[2], nodes[0], payment_event.commitment_msg, false);
+
+		expect_pending_htlcs_forwardable!(nodes[2]);
+		check_added_monitors!(nodes[2], 1);
+
+		let mut events = nodes[2].node.get_and_clear_pending_msg_events();
+		assert_eq!(events.len(), 1);
+		let payment_event = SendEvent::from_event(events.pop().unwrap());
+
+		nodes[3].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &payment_event.msgs[0]);
+		check_added_monitors!(nodes[3], 0);
+		commitment_signed_dance!(nodes[3], nodes[2], payment_event.commitment_msg, true, true);
+	}
+	expect_pending_htlcs_forwardable_ignore!(nodes[3]);
+	nodes[3].node.process_pending_htlc_forwards();
+
+	let events = nodes[3].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	match events[0] {
+		Event::PaymentClaimable { ref purpose, amount_msat, ref onion_fields, .. } => {
+			match &purpose {
+				PaymentPurpose::InvoicePayment { payment_secret, .. } => {
+					assert_eq!(our_payment_secret, *payment_secret);
+					assert_eq!(Some(*payment_secret), onion_fields.as_ref().unwrap().payment_secret);
+				},
+				PaymentPurpose::SpontaneousPayment(payment_preimage) => {
+					assert_eq!(our_payment_preimage, *payment_preimage);
+				},
+			}
+			assert_eq!(amount_msat, amt_msat);
+			assert_eq!(onion_fields.clone().unwrap().custom_tlvs.unwrap(), custom_tlvs);
+		},
+		_ => panic!("Unexpected event"),
+	}
+
+	do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
+	expect_payment_sent(&nodes[0], our_payment_preimage, Some(Some(2000)), true);
+}
 
 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