Skip to content

Commit c62aa23

Browse files
committed
Update PaymentClaimable fields to improve MPP clarity
Previously, `channel_id` in `PaymentClaimable` only listed a single inbound channel, which was misleading for MPP payments arriving via multiple channels. To better represent MPP scenarios, this update introduces: - `via_channel_ids`: A list of all inbound channels used in the payment. - `via_user_channel_ids`: The corresponding user-defined channel IDs for each inbound channel. This change ensures a more accurate representation of multi-path payments while maintaining backward compatibility.
1 parent e61f68b commit c62aa23

File tree

5 files changed

+57
-35
lines changed

5 files changed

+57
-35
lines changed

lightning/src/events/mod.rs

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -774,10 +774,10 @@ pub enum Event {
774774
/// Information for claiming this received payment, based on whether the purpose of the
775775
/// payment is to pay an invoice or to send a spontaneous payment.
776776
purpose: PaymentPurpose,
777-
/// The `channel_id` indicating over which channel we received the payment.
778-
via_channel_id: Option<ChannelId>,
779-
/// The `user_channel_id` indicating over which channel we received the payment.
780-
via_user_channel_id: Option<u128>,
777+
/// The `(channel_id, user_channel_id)` pairs over which the payment was received.
778+
///
779+
/// This will be an incomplete vector for MPP payment events created/serialized using LDK version 0.1.0 and prior.
780+
inbound_channel_ids: Vec<(ChannelId, Option<u128>)>,
781781
/// The block height at which this payment will be failed back and will no longer be
782782
/// eligible for claiming.
783783
///
@@ -1506,7 +1506,7 @@ impl Writeable for Event {
15061506
// drop any channels which have not yet exchanged funding_signed.
15071507
},
15081508
&Event::PaymentClaimable { ref payment_hash, ref amount_msat, counterparty_skimmed_fee_msat,
1509-
ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id,
1509+
ref purpose, ref receiver_node_id, ref inbound_channel_ids,
15101510
ref claim_deadline, ref onion_fields, ref payment_id,
15111511
} => {
15121512
1u8.write(writer)?;
@@ -1540,20 +1540,30 @@ impl Writeable for Event {
15401540
}
15411541
let skimmed_fee_opt = if counterparty_skimmed_fee_msat == 0 { None }
15421542
else { Some(counterparty_skimmed_fee_msat) };
1543+
1544+
let (via_channel_id_legacy, via_user_channel_id_legacy) = match inbound_channel_ids.last() {
1545+
Some((chan_id, user_chan_id)) => (Some(*chan_id), *user_chan_id),
1546+
None => (None, None),
1547+
};
15431548
write_tlv_fields!(writer, {
15441549
(0, payment_hash, required),
15451550
(1, receiver_node_id, option),
15461551
(2, payment_secret, option),
1547-
(3, via_channel_id, option),
1552+
// Marked as legacy in version 0.2.0; superseded by `inbound_channel_ids`, which
1553+
// includes all channel IDs used in the payment instead of only the last one.
1554+
(3, via_channel_id_legacy, option),
15481555
(4, amount_msat, required),
1549-
(5, via_user_channel_id, option),
1556+
// Marked as legacy in version 0.2.0 for the same reason as `via_channel_id_legacy`;
1557+
// superseded by `via_user_channel_ids`.
1558+
(5, via_user_channel_id_legacy, option),
15501559
// Type 6 was `user_payment_id` on 0.0.103 and earlier
15511560
(7, claim_deadline, option),
15521561
(8, payment_preimage, option),
15531562
(9, onion_fields, option),
15541563
(10, skimmed_fee_opt, option),
15551564
(11, payment_context, option),
15561565
(13, payment_id, option),
1566+
(15, *inbound_channel_ids, optional_vec),
15571567
});
15581568
},
15591569
&Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref amount_msat, ref fee_paid_msat } => {
@@ -1849,41 +1859,47 @@ impl MaybeReadable for Event {
18491859
let mut counterparty_skimmed_fee_msat_opt = None;
18501860
let mut receiver_node_id = None;
18511861
let mut _user_payment_id = None::<u64>; // Used in 0.0.103 and earlier, no longer written in 0.0.116+.
1852-
let mut via_channel_id = None;
1862+
let mut via_channel_id_legacy = None;
18531863
let mut claim_deadline = None;
1854-
let mut via_user_channel_id = None;
1864+
let mut via_user_channel_id_legacy = None;
18551865
let mut onion_fields = None;
18561866
let mut payment_context = None;
18571867
let mut payment_id = None;
1868+
let mut inbound_channel_ids_opt = None;
18581869
read_tlv_fields!(reader, {
18591870
(0, payment_hash, required),
18601871
(1, receiver_node_id, option),
18611872
(2, payment_secret, option),
1862-
(3, via_channel_id, option),
1873+
(3, via_channel_id_legacy, option),
18631874
(4, amount_msat, required),
1864-
(5, via_user_channel_id, option),
1875+
(5, via_user_channel_id_legacy, option),
18651876
(6, _user_payment_id, option),
18661877
(7, claim_deadline, option),
18671878
(8, payment_preimage, option),
18681879
(9, onion_fields, option),
18691880
(10, counterparty_skimmed_fee_msat_opt, option),
18701881
(11, payment_context, option),
18711882
(13, payment_id, option),
1883+
(15, inbound_channel_ids_opt, optional_vec),
18721884
});
18731885
let purpose = match payment_secret {
18741886
Some(secret) => PaymentPurpose::from_parts(payment_preimage, secret, payment_context)
18751887
.map_err(|()| msgs::DecodeError::InvalidValue)?,
18761888
None if payment_preimage.is_some() => PaymentPurpose::SpontaneousPayment(payment_preimage.unwrap()),
18771889
None => return Err(msgs::DecodeError::InvalidValue),
18781890
};
1891+
1892+
let inbound_channel_ids = inbound_channel_ids_opt
1893+
.or_else(|| via_channel_id_legacy.map(|chan_id| vec![(chan_id, via_user_channel_id_legacy)]))
1894+
.unwrap_or_default();
1895+
18791896
Ok(Some(Event::PaymentClaimable {
18801897
receiver_node_id,
18811898
payment_hash,
18821899
amount_msat,
18831900
counterparty_skimmed_fee_msat: counterparty_skimmed_fee_msat_opt.unwrap_or(0),
18841901
purpose,
1885-
via_channel_id,
1886-
via_user_channel_id,
1902+
inbound_channel_ids,
18871903
claim_deadline,
18881904
onion_fields,
18891905
payment_id,

lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
124124
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
125125
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
126126
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
127+
let user_channel_id = nodes[1].node.list_channels()[0].user_channel_id;
127128

128129
let (route, payment_hash_1, payment_preimage_1, payment_secret_1) = get_route_and_payment_hash!(&nodes[0], nodes[1], 1000000);
129130

@@ -165,11 +166,11 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
165166
let events_3 = nodes[1].node.get_and_clear_pending_events();
166167
assert_eq!(events_3.len(), 1);
167168
match events_3[0] {
168-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
169+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
169170
assert_eq!(payment_hash_1, *payment_hash);
170171
assert_eq!(amount_msat, 1_000_000);
171172
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
172-
assert_eq!(via_channel_id, Some(channel_id));
173+
assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]);
173174
match &purpose {
174175
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
175176
assert!(payment_preimage.is_none());
@@ -247,6 +248,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
247248
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
248249
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
249250
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
251+
let user_channel_id = nodes[1].node.list_channels()[0].user_channel_id;
250252

251253
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
252254

@@ -547,11 +549,11 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
547549
let events_5 = nodes[1].node.get_and_clear_pending_events();
548550
assert_eq!(events_5.len(), 1);
549551
match events_5[0] {
550-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
552+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
551553
assert_eq!(payment_hash_2, *payment_hash);
552554
assert_eq!(amount_msat, 1_000_000);
553555
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
554-
assert_eq!(via_channel_id, Some(channel_id));
556+
assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]);
555557
match &purpose {
556558
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
557559
assert!(payment_preimage.is_none());
@@ -601,6 +603,7 @@ fn test_monitor_update_fail_cs() {
601603
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
602604
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
603605
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
606+
let user_channel_id = nodes[1].node.list_channels()[0].user_channel_id;
604607

605608
let (route, our_payment_hash, payment_preimage, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
606609
{
@@ -665,11 +668,11 @@ fn test_monitor_update_fail_cs() {
665668
let events = nodes[1].node.get_and_clear_pending_events();
666669
assert_eq!(events.len(), 1);
667670
match events[0] {
668-
Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
671+
Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
669672
assert_eq!(payment_hash, our_payment_hash);
670673
assert_eq!(amount_msat, 1_000_000);
671674
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
672-
assert_eq!(via_channel_id, Some(channel_id));
675+
assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]);
673676
match &purpose {
674677
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
675678
assert!(payment_preimage.is_none());
@@ -1678,12 +1681,11 @@ fn test_monitor_update_fail_claim() {
16781681
let events = nodes[0].node.get_and_clear_pending_events();
16791682
assert_eq!(events.len(), 2);
16801683
match events[0] {
1681-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id, .. } => {
1684+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
16821685
assert_eq!(payment_hash_2, *payment_hash);
16831686
assert_eq!(1_000_000, amount_msat);
16841687
assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id());
1685-
assert_eq!(via_channel_id, Some(channel_id));
1686-
assert_eq!(via_user_channel_id, Some(42));
1688+
assert_eq!(*inbound_channel_ids.last().unwrap(), (channel_id, Some(42)));
16871689
match &purpose {
16881690
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
16891691
assert!(payment_preimage.is_none());
@@ -1695,11 +1697,11 @@ fn test_monitor_update_fail_claim() {
16951697
_ => panic!("Unexpected event"),
16961698
}
16971699
match events[1] {
1698-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
1700+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
16991701
assert_eq!(payment_hash_3, *payment_hash);
17001702
assert_eq!(1_000_000, amount_msat);
17011703
assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id());
1702-
assert_eq!(via_channel_id, Some(channel_id));
1704+
assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(42))]);
17031705
match &purpose {
17041706
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
17051707
assert!(payment_preimage.is_none());

lightning/src/ln/channelmanager.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6291,8 +6291,7 @@ where
62916291
purpose: $purpose,
62926292
amount_msat,
62936293
counterparty_skimmed_fee_msat,
6294-
via_channel_id: Some(prev_channel_id),
6295-
via_user_channel_id: Some(prev_user_channel_id),
6294+
inbound_channel_ids: claimable_payment.inbound_channel_ids(),
62966295
claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER),
62976296
onion_fields: claimable_payment.onion_fields.clone(),
62986297
payment_id: Some(payment_id),

lightning/src/ln/functional_test_utils.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2734,7 +2734,7 @@ pub fn do_pass_along_path<'a, 'b, 'c>(args: PassAlongPathArgs) -> Option<Event>
27342734
assert_eq!(events_2.len(), 1);
27352735
match &events_2[0] {
27362736
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat,
2737-
receiver_node_id, ref via_channel_id, ref via_user_channel_id,
2737+
receiver_node_id, ref inbound_channel_ids,
27382738
claim_deadline, onion_fields, ..
27392739
} => {
27402740
assert_eq!(our_payment_hash, *payment_hash);
@@ -2768,8 +2768,11 @@ pub fn do_pass_along_path<'a, 'b, 'c>(args: PassAlongPathArgs) -> Option<Event>
27682768
},
27692769
}
27702770
assert_eq!(*amount_msat, recv_value);
2771-
assert!(node.node.list_channels().iter().any(|details| details.channel_id == via_channel_id.unwrap()));
2772-
assert!(node.node.list_channels().iter().any(|details| details.user_channel_id == via_user_channel_id.unwrap()));
2771+
let channels = node.node.list_channels();
2772+
for (chan_id, user_chan_id) in inbound_channel_ids {
2773+
let chan = channels.iter().find(|details| &details.channel_id == chan_id).unwrap();
2774+
assert_eq!(*user_chan_id, Some(chan.user_channel_id));
2775+
}
27732776
assert!(claim_deadline.unwrap() > node.best_block_info().1);
27742777
},
27752778
_ => panic!("Unexpected event"),

lightning/src/ln/functional_tests.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,6 +1874,7 @@ pub fn test_channel_reserve_holding_cell_htlcs() {
18741874
let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
18751875
let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 190000, 1001);
18761876
let chan_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 190000, 1001);
1877+
let chan_2_user_id = nodes[2].node.list_channels()[0].user_channel_id;
18771878

18781879
let mut stat01 = get_channel_value_stat!(nodes[0], nodes[1], chan_1.2);
18791880
let mut stat11 = get_channel_value_stat!(nodes[1], nodes[0], chan_1.2);
@@ -2068,11 +2069,11 @@ pub fn test_channel_reserve_holding_cell_htlcs() {
20682069
let events = nodes[2].node.get_and_clear_pending_events();
20692070
assert_eq!(events.len(), 2);
20702071
match events[0] {
2071-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
2072+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
20722073
assert_eq!(our_payment_hash_21, *payment_hash);
20732074
assert_eq!(recv_value_21, amount_msat);
20742075
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
2075-
assert_eq!(via_channel_id, Some(chan_2.2));
2076+
assert_eq!(*inbound_channel_ids, vec![(chan_2.2, Some(chan_2_user_id))]);
20762077
match &purpose {
20772078
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
20782079
assert!(payment_preimage.is_none());
@@ -2084,11 +2085,11 @@ pub fn test_channel_reserve_holding_cell_htlcs() {
20842085
_ => panic!("Unexpected event"),
20852086
}
20862087
match events[1] {
2087-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
2088+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
20882089
assert_eq!(our_payment_hash_22, *payment_hash);
20892090
assert_eq!(recv_value_22, amount_msat);
20902091
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
2091-
assert_eq!(via_channel_id, Some(chan_2.2));
2092+
assert_eq!(*inbound_channel_ids, vec![(chan_2.2, Some(chan_2_user_id))]);
20922093
match &purpose {
20932094
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
20942095
assert!(payment_preimage.is_none());
@@ -4200,6 +4201,7 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
42004201
} else {
42014202
create_announced_chan_between_nodes(&nodes, 0, 1).2
42024203
};
4204+
let user_channel_id = nodes[1].node.list_channels()[0].user_channel_id;
42034205

42044206
let (route, payment_hash_1, payment_preimage_1, payment_secret_1) = get_route_and_payment_hash!(nodes[0], nodes[1], 1_000_000);
42054207

@@ -4313,11 +4315,11 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
43134315
let events_2 = nodes[1].node.get_and_clear_pending_events();
43144316
assert_eq!(events_2.len(), 1);
43154317
match events_2[0] {
4316-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
4318+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
43174319
assert_eq!(payment_hash_1, *payment_hash);
43184320
assert_eq!(amount_msat, 1_000_000);
43194321
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
4320-
assert_eq!(via_channel_id, Some(channel_id));
4322+
assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]);
43214323
match &purpose {
43224324
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
43234325
assert!(payment_preimage.is_none());

0 commit comments

Comments
 (0)