f - fail back non-matching even custom tlvs

Adds logic + refactors test to more easily test different cases of
non-matching vs. missing tlvs.

Also adds modifying the second parameter in `check_merge` to match
documentation, although this isn't used currently.

Author: alecchendev

lightning/src/ln/outbound_payment.rs

@@ -505,8 +505,13 @@ impl RecipientOnionFields {
 		if self.payment_secret != further_htlc_fields.payment_secret { return Err(()); }
 		if self.payment_metadata != further_htlc_fields.payment_metadata { return Err(()); }
 
-		if let (Some(tlvs), Some(further_tlvs)) = (&mut self.custom_tlvs, &further_htlc_fields.custom_tlvs) {
+		if let (Some(tlvs), Some(further_tlvs)) = (&mut self.custom_tlvs, &mut further_htlc_fields.custom_tlvs) {
+			let even_tlvs: Vec<&(u64, Vec<u8>)> = tlvs.iter().filter(|(typ, _)| *typ % 2 == 0).collect();
+			let further_even_tlvs: Vec<&(u64, Vec<u8>)> = further_tlvs.iter().filter(|(typ, _)| *typ % 2 == 0).collect();
+			if even_tlvs != further_even_tlvs { return Err(()) }
+
 			tlvs.retain(|tlv| further_tlvs.iter().any(|further_tlv| tlv == further_tlv));
+			further_tlvs.retain(|further_tlv| tlvs.iter().any(|tlv| tlv == further_tlv));
 		}
 		Ok(())
 	}

lightning/src/ln/payment_tests.rs

@@ -3262,7 +3262,43 @@ fn do_test_custom_tlvs(spontaneous: bool) {
 
 #[test]
 fn test_custom_tlvs_consistency() {
-	// Test that if we recieve two HTLCs with different custom TLVs we drop the non-matching TLVs
+	let even_type_1 = 1 << 16;
+	let odd_type_1	= (1 << 16)+ 1;
+	let even_type_2 = (1 << 16) + 2;
+	let odd_type_2	= (1 << 16) + 3;
+	let value_1 = || vec![1, 2, 3, 4];
+	let differing_value_1 = || vec![1, 2, 3, 5];
+	let value_2 = || vec![42u8; 16];
+
+	// Drop missing odd tlvs
+	do_test_custom_tlvs_consistency(
+		vec![(odd_type_1, value_1()), (odd_type_2, value_2())],
+		vec![(odd_type_1, value_1())],
+		Some(vec![(odd_type_1, value_1())]),
+	);
+	// Drop non-matching odd tlvs
+	do_test_custom_tlvs_consistency(
+		vec![(odd_type_1, value_1()), (odd_type_2, value_2())],
+		vec![(odd_type_1, differing_value_1()), (odd_type_2, value_2())],
+		Some(vec![(odd_type_2, value_2())]),
+	);
+	// Fail missing even tlvs
+	do_test_custom_tlvs_consistency(
+		vec![(odd_type_1, value_1()), (even_type_2, value_2())],
+		vec![(odd_type_1, value_1())],
+		None,
+	);
+	// Fail non-matching even tlvs
+	do_test_custom_tlvs_consistency(
+		vec![(even_type_1, value_1()), (odd_type_2, value_2())],
+		vec![(even_type_1, differing_value_1()), (odd_type_2, value_2())],
+		None,
+	);
+}
+
+fn do_test_custom_tlvs_consistency(first_tlvs: Vec<(u64, Vec<u8>)>, second_tlvs: Vec<(u64, Vec<u8>)>,
+	expected_receive_tlvs: Option<Vec<(u64, Vec<u8>)>>) {
+
 	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]);
@@ -3271,7 +3307,7 @@ fn test_custom_tlvs_consistency() {
 	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 chan_2_3 = 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();
@@ -3286,14 +3322,12 @@ fn test_custom_tlvs_consistency() {
 	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 custom_tlvs = vec![
-		(5482373483, vec![1, 2, 3, 4]),
-		(5482373487, vec![0x42u8; 16]),
-	];
+
+	// Send first part
 	let onion_fields = RecipientOnionFields {
 		payment_secret: Some(our_payment_secret),
 		payment_metadata: None,
-		custom_tlvs: Some(custom_tlvs.clone())
+		custom_tlvs: Some(first_tlvs)
 	};
 	let session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash,
 			onion_fields.clone(), payment_id, &route).unwrap();
@@ -3310,13 +3344,11 @@ fn test_custom_tlvs_consistency() {
 	}
 	assert!(nodes[3].node.get_and_clear_pending_events().is_empty());
 
-	let custom_tlvs = vec![
-		(5482373483, vec![1, 2, 3, 4]),
-	];
+	// Send second part
 	let onion_fields = RecipientOnionFields {
 		payment_secret: Some(our_payment_secret),
 		payment_metadata: None,
-		custom_tlvs: Some(custom_tlvs.clone())
+		custom_tlvs: Some(second_tlvs)
 	};
 	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();
@@ -3344,27 +3376,49 @@ fn test_custom_tlvs_consistency() {
 	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"),
-	}
+	if let Some(expected_tlvs) = expected_receive_tlvs {
+		// Claim and match expected
+		let events = nodes[3].node.get_and_clear_pending_events();
+		println!("events: {:?}", 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(), expected_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);
+	} else {
+		// Expect fail back
+		let expected_destinations = vec![HTLCDestination::FailedPayment { payment_hash: our_payment_hash }];
+		expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[3], expected_destinations);
+		check_added_monitors!(nodes[3], 1);
+
+		let fail_updates_1 = 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(), &fail_updates_1.update_fail_htlcs[0]);
+		commitment_signed_dance!(nodes[2], nodes[3], fail_updates_1.commitment_signed, false);
+
+		expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[2], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_2_3.2 }]);
+		check_added_monitors!(nodes[2], 1);
+
+		let fail_updates_2 = 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(), &fail_updates_2.update_fail_htlcs[0]);
+		commitment_signed_dance!(nodes[0], nodes[2], fail_updates_2.commitment_signed, false);
 
-	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);
+		expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain());
+	}
 }
 
 fn do_test_payment_metadata_consistency(do_reload: bool, do_modify: bool) {