From 4cb0a4b761caa1fd1c3142032ef3b67e7738b4fc Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Fri, 19 May 2023 15:37:47 -0500 Subject: [PATCH] Drop non-matching custom TLVs when receiving MPP --- lightning/src/ln/outbound_payment.rs | 14 +++- lightning/src/ln/payment_tests.rs | 106 +++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 0813809181f..189513da672 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/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; @@ -490,7 +490,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(()) } } diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 07e7a7e82fc..3ab277b92b8 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -3075,6 +3075,112 @@ 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 custom_tlvs = _get_encoded_tlv_stream!({ + (5482373483, 0x01020304, required), + (5482373487, Some(vec![0x42u8; 16]), option), + }); + 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 custom_tlvs = _get_encoded_tlv_stream!({ + (5482373483, 0x01020304, required), + }); + 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