diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index a0f26bfbac0..3896bbf393d 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -774,10 +774,10 @@ pub enum Event { /// Information for claiming this received payment, based on whether the purpose of the /// payment is to pay an invoice or to send a spontaneous payment. purpose: PaymentPurpose, - /// The `channel_id` indicating over which channel we received the payment. - via_channel_id: Option, - /// The `user_channel_id` indicating over which channel we received the payment. - via_user_channel_id: Option, + /// The `(channel_id, user_channel_id)` pairs over which the payment was received. + /// + /// This will be an incomplete vector for MPP payment events created/serialized using LDK version 0.1.0 and prior. + inbound_channel_ids: Vec<(ChannelId, Option)>, /// The block height at which this payment will be failed back and will no longer be /// eligible for claiming. /// @@ -1506,7 +1506,7 @@ impl Writeable for Event { // drop any channels which have not yet exchanged funding_signed. }, &Event::PaymentClaimable { ref payment_hash, ref amount_msat, counterparty_skimmed_fee_msat, - ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id, + ref purpose, ref receiver_node_id, ref inbound_channel_ids, ref claim_deadline, ref onion_fields, ref payment_id, } => { 1u8.write(writer)?; @@ -1540,13 +1540,22 @@ impl Writeable for Event { } let skimmed_fee_opt = if counterparty_skimmed_fee_msat == 0 { None } else { Some(counterparty_skimmed_fee_msat) }; + + let (via_channel_id_legacy, via_user_channel_id_legacy) = match inbound_channel_ids.last() { + Some((chan_id, user_chan_id)) => (Some(*chan_id), *user_chan_id), + None => (None, None), + }; write_tlv_fields!(writer, { (0, payment_hash, required), (1, receiver_node_id, option), (2, payment_secret, option), - (3, via_channel_id, option), + // Marked as legacy in version 0.2.0; superseded by `inbound_channel_ids`, which + // includes all channel IDs used in the payment instead of only the last one. + (3, via_channel_id_legacy, option), (4, amount_msat, required), - (5, via_user_channel_id, option), + // Marked as legacy in version 0.2.0 for the same reason as `via_channel_id_legacy`; + // superseded by `via_user_channel_ids`. + (5, via_user_channel_id_legacy, option), // Type 6 was `user_payment_id` on 0.0.103 and earlier (7, claim_deadline, option), (8, payment_preimage, option), @@ -1554,6 +1563,7 @@ impl Writeable for Event { (10, skimmed_fee_opt, option), (11, payment_context, option), (13, payment_id, option), + (15, *inbound_channel_ids, optional_vec), }); }, &Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref amount_msat, ref fee_paid_msat } => { @@ -1849,19 +1859,20 @@ impl MaybeReadable for Event { let mut counterparty_skimmed_fee_msat_opt = None; let mut receiver_node_id = None; let mut _user_payment_id = None::; // Used in 0.0.103 and earlier, no longer written in 0.0.116+. - let mut via_channel_id = None; + let mut via_channel_id_legacy = None; let mut claim_deadline = None; - let mut via_user_channel_id = None; + let mut via_user_channel_id_legacy = None; let mut onion_fields = None; let mut payment_context = None; let mut payment_id = None; + let mut inbound_channel_ids_opt = None; read_tlv_fields!(reader, { (0, payment_hash, required), (1, receiver_node_id, option), (2, payment_secret, option), - (3, via_channel_id, option), + (3, via_channel_id_legacy, option), (4, amount_msat, required), - (5, via_user_channel_id, option), + (5, via_user_channel_id_legacy, option), (6, _user_payment_id, option), (7, claim_deadline, option), (8, payment_preimage, option), @@ -1869,6 +1880,7 @@ impl MaybeReadable for Event { (10, counterparty_skimmed_fee_msat_opt, option), (11, payment_context, option), (13, payment_id, option), + (15, inbound_channel_ids_opt, optional_vec), }); let purpose = match payment_secret { Some(secret) => PaymentPurpose::from_parts(payment_preimage, secret, payment_context) @@ -1876,14 +1888,18 @@ impl MaybeReadable for Event { None if payment_preimage.is_some() => PaymentPurpose::SpontaneousPayment(payment_preimage.unwrap()), None => return Err(msgs::DecodeError::InvalidValue), }; + + let inbound_channel_ids = inbound_channel_ids_opt + .or_else(|| via_channel_id_legacy.map(|chan_id| vec![(chan_id, via_user_channel_id_legacy)])) + .unwrap_or_default(); + Ok(Some(Event::PaymentClaimable { receiver_node_id, payment_hash, amount_msat, counterparty_skimmed_fee_msat: counterparty_skimmed_fee_msat_opt.unwrap_or(0), purpose, - via_channel_id, - via_user_channel_id, + inbound_channel_ids, claim_deadline, onion_fields, payment_id, diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 9fc722f8205..e44d70d561c 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -240,8 +240,26 @@ fn mpp_to_one_hop_blinded_path() { Some(payment_secret), ev.clone(), false, None); let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); - pass_along_path(&nodes[0], expected_route[1], amt_msat, payment_hash.clone(), + let event = pass_along_path(&nodes[0], expected_route[1], amt_msat, payment_hash.clone(), Some(payment_secret), ev.clone(), true, None); + + match event.unwrap() { + Event::PaymentClaimable { mut inbound_channel_ids, .. } => { + let mut expected_inbound_channel_ids = nodes[3].node.list_channels() + .iter() + .map(|d| (d.channel_id, Some(d.user_channel_id))) + .collect::>(); + + // `list_channels` returns channels in arbitrary order, so we sort both vectors + // to ensure the comparison is order-agnostic. + inbound_channel_ids.sort(); + expected_inbound_channel_ids.sort(); + + assert_eq!(inbound_channel_ids, expected_inbound_channel_ids); + } + _ => panic!("Unexpected event"), + } + claim_payment_along_route( ClaimAlongRouteArgs::new(&nodes[0], expected_route, payment_preimage) ); diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index e99cf017b66..6c8fa47f9d5 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -124,6 +124,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2; + let user_channel_id = nodes[1].node.list_channels()[0].user_channel_id; let (route, payment_hash_1, payment_preimage_1, payment_secret_1) = get_route_and_payment_hash!(&nodes[0], nodes[1], 1000000); @@ -165,11 +166,11 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { let events_3 = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events_3.len(), 1); match events_3[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => { assert_eq!(payment_hash_1, *payment_hash); assert_eq!(amount_msat, 1_000_000); assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); - assert_eq!(via_channel_id, Some(channel_id)); + assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]); match &purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); @@ -247,6 +248,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2; + let user_channel_id = nodes[1].node.list_channels()[0].user_channel_id; let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000); @@ -547,11 +549,12 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) { let events_5 = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events_5.len(), 1); match events_5[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => { assert_eq!(payment_hash_2, *payment_hash); assert_eq!(amount_msat, 1_000_000); assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); - assert_eq!(via_channel_id, Some(channel_id)); + assert_eq!(inbound_channel_ids.len(), 1); + assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]); match &purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); @@ -601,6 +604,7 @@ fn test_monitor_update_fail_cs() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2; + let user_channel_id = nodes[1].node.list_channels()[0].user_channel_id; let (route, our_payment_hash, payment_preimage, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000); { @@ -665,11 +669,11 @@ fn test_monitor_update_fail_cs() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { + Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => { assert_eq!(payment_hash, our_payment_hash); assert_eq!(amount_msat, 1_000_000); assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); - assert_eq!(via_channel_id, Some(channel_id)); + assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]); match &purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); @@ -1678,12 +1682,12 @@ fn test_monitor_update_fail_claim() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match events[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id, .. } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => { assert_eq!(payment_hash_2, *payment_hash); assert_eq!(1_000_000, amount_msat); assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id()); - assert_eq!(via_channel_id, Some(channel_id)); - assert_eq!(via_user_channel_id, Some(42)); + assert_eq!(inbound_channel_ids.len(), 1); + assert_eq!(*inbound_channel_ids.last().unwrap(), (channel_id, Some(42))); match &purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); @@ -1695,11 +1699,12 @@ fn test_monitor_update_fail_claim() { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => { assert_eq!(payment_hash_3, *payment_hash); assert_eq!(1_000_000, amount_msat); assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id()); - assert_eq!(via_channel_id, Some(channel_id)); + assert_eq!(inbound_channel_ids.len(), 1); + assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(42))]); match &purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 00a536539ea..394608a6662 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -924,6 +924,15 @@ impl ClaimablePayment { self.htlcs.iter().map(|htlc| (htlc.prev_hop.channel_id, htlc.prev_hop.htlc_id)) ) } + + /// Returns the inbound `(channel_id, user_channel_id)` pairs for all HTLCs associated with the payment. + /// + /// Note: The `user_channel_id` will be `None` for HTLCs created using LDK version 0.0.117 or prior. + fn inbound_channel_ids(&self) -> Vec<(ChannelId, Option)> { + self.htlcs.iter().map(|htlc| { + (htlc.prev_hop.channel_id, htlc.prev_hop.user_channel_id) + }).collect() + } } /// Represent the channel funding transaction type. @@ -6282,8 +6291,7 @@ where purpose: $purpose, amount_msat, counterparty_skimmed_fee_msat, - via_channel_id: Some(prev_channel_id), - via_user_channel_id: Some(prev_user_channel_id), + inbound_channel_ids: claimable_payment.inbound_channel_ids(), claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER), onion_fields: claimable_payment.onion_fields.clone(), payment_id: Some(payment_id), @@ -10917,7 +10925,18 @@ where let events = core::cell::RefCell::new(Vec::new()); let event_handler = |event: events::Event| Ok(events.borrow_mut().push(event)); self.process_pending_events(&event_handler); - events.into_inner() + let collected_events = events.into_inner(); + + // To expand the coverage and make sure all events are properly serialised and deserialised, + // we test all generated events round-trip: + for event in &collected_events { + let ser = event.encode(); + if let Some(deser) = events::Event::read(&mut &ser[..]).expect("event should deserialize") { + assert_eq!(&deser, event, "event should roundtrip correctly"); + } + } + + collected_events } #[cfg(feature = "_test_utils")] diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 5e592c858f8..8f332e2d8cf 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2734,7 +2734,7 @@ pub fn do_pass_along_path<'a, 'b, 'c>(args: PassAlongPathArgs) -> Option assert_eq!(events_2.len(), 1); 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, + receiver_node_id, ref inbound_channel_ids, claim_deadline, onion_fields, .. } => { assert_eq!(our_payment_hash, *payment_hash); @@ -2768,8 +2768,12 @@ pub fn do_pass_along_path<'a, 'b, 'c>(args: PassAlongPathArgs) -> Option }, } 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())); + for (chan_id, user_chan_id) in inbound_channel_ids { + assert!(node.node.list_channels().iter().any(|details| &details.channel_id == chan_id)); + if let Some(user_id) = user_chan_id { + assert!(node.node.list_channels().iter().any(|details| &details.user_channel_id == user_id)); + } + } assert!(claim_deadline.unwrap() > node.best_block_info().1); }, _ => panic!("Unexpected event"), diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 1d1884b8d58..d06a567cb08 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2068,11 +2068,11 @@ pub fn test_channel_reserve_holding_cell_htlcs() { let events = nodes[2].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match events[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => { assert_eq!(our_payment_hash_21, *payment_hash); assert_eq!(recv_value_21, amount_msat); assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap()); - assert_eq!(via_channel_id, Some(chan_2.2)); + assert_eq!(inbound_channel_ids.last().unwrap().0, chan_2.2); match &purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); @@ -2084,11 +2084,11 @@ pub fn test_channel_reserve_holding_cell_htlcs() { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => { assert_eq!(our_payment_hash_22, *payment_hash); assert_eq!(recv_value_22, amount_msat); assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap()); - assert_eq!(via_channel_id, Some(chan_2.2)); + assert_eq!(inbound_channel_ids.last().unwrap().0, chan_2.2); match &purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); @@ -4313,11 +4313,12 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken let events_2 = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events_2.len(), 1); match events_2[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => { assert_eq!(payment_hash_1, *payment_hash); assert_eq!(amount_msat, 1_000_000); assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); - assert_eq!(via_channel_id, Some(channel_id)); + assert_eq!(inbound_channel_ids.len(), 1); + assert_eq!(inbound_channel_ids.last().unwrap().0, channel_id); match &purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none());