Skip to content

Commit d85a29d

Browse files
shaavanshaavan
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 d85a29d

File tree

5 files changed

+60
-35
lines changed

5 files changed

+60
-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: 16 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,12 @@ 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.len(), 1);
557+
assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]);
555558
match &purpose {
556559
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
557560
assert!(payment_preimage.is_none());
@@ -601,6 +604,7 @@ fn test_monitor_update_fail_cs() {
601604
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
602605
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
603606
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
607+
let user_channel_id = nodes[1].node.list_channels()[0].user_channel_id;
604608

605609
let (route, our_payment_hash, payment_preimage, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
606610
{
@@ -665,11 +669,11 @@ fn test_monitor_update_fail_cs() {
665669
let events = nodes[1].node.get_and_clear_pending_events();
666670
assert_eq!(events.len(), 1);
667671
match events[0] {
668-
Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
672+
Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
669673
assert_eq!(payment_hash, our_payment_hash);
670674
assert_eq!(amount_msat, 1_000_000);
671675
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
672-
assert_eq!(via_channel_id, Some(channel_id));
676+
assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(user_channel_id))]);
673677
match &purpose {
674678
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
675679
assert!(payment_preimage.is_none());
@@ -1678,12 +1682,12 @@ fn test_monitor_update_fail_claim() {
16781682
let events = nodes[0].node.get_and_clear_pending_events();
16791683
assert_eq!(events.len(), 2);
16801684
match events[0] {
1681-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id, .. } => {
1685+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
16821686
assert_eq!(payment_hash_2, *payment_hash);
16831687
assert_eq!(1_000_000, amount_msat);
16841688
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));
1689+
assert_eq!(inbound_channel_ids.len(), 1);
1690+
assert_eq!(*inbound_channel_ids.last().unwrap(), (channel_id, Some(42)));
16871691
match &purpose {
16881692
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
16891693
assert!(payment_preimage.is_none());
@@ -1695,11 +1699,12 @@ fn test_monitor_update_fail_claim() {
16951699
_ => panic!("Unexpected event"),
16961700
}
16971701
match events[1] {
1698-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
1702+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
16991703
assert_eq!(payment_hash_3, *payment_hash);
17001704
assert_eq!(1_000_000, amount_msat);
17011705
assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id());
1702-
assert_eq!(via_channel_id, Some(channel_id));
1706+
assert_eq!(inbound_channel_ids.len(), 1);
1707+
assert_eq!(*inbound_channel_ids, vec![(channel_id, Some(42))]);
17031708
match &purpose {
17041709
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
17051710
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: 7 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,12 @@ 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+
for (chan_id, user_chan_id) in inbound_channel_ids {
2772+
assert!(node.node.list_channels().iter().any(|details| &details.channel_id == chan_id));
2773+
if let Some(user_id) = user_chan_id {
2774+
assert!(node.node.list_channels().iter().any(|details| &details.user_channel_id == user_id));
2775+
}
2776+
}
27732777
assert!(claim_deadline.unwrap() > node.best_block_info().1);
27742778
},
27752779
_ => panic!("Unexpected event"),

‎lightning/src/ln/functional_tests.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2068,11 +2068,11 @@ pub fn test_channel_reserve_holding_cell_htlcs() {
20682068
let events = nodes[2].node.get_and_clear_pending_events();
20692069
assert_eq!(events.len(), 2);
20702070
match events[0] {
2071-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
2071+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
20722072
assert_eq!(our_payment_hash_21, *payment_hash);
20732073
assert_eq!(recv_value_21, amount_msat);
20742074
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
2075-
assert_eq!(via_channel_id, Some(chan_2.2));
2075+
assert_eq!(inbound_channel_ids.last().unwrap().0, chan_2.2);
20762076
match &purpose {
20772077
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
20782078
assert!(payment_preimage.is_none());
@@ -2084,11 +2084,11 @@ pub fn test_channel_reserve_holding_cell_htlcs() {
20842084
_ => panic!("Unexpected event"),
20852085
}
20862086
match events[1] {
2087-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
2087+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
20882088
assert_eq!(our_payment_hash_22, *payment_hash);
20892089
assert_eq!(recv_value_22, amount_msat);
20902090
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
2091-
assert_eq!(via_channel_id, Some(chan_2.2));
2091+
assert_eq!(inbound_channel_ids.last().unwrap().0, chan_2.2);
20922092
match &purpose {
20932093
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
20942094
assert!(payment_preimage.is_none());
@@ -4313,11 +4313,12 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
43134313
let events_2 = nodes[1].node.get_and_clear_pending_events();
43144314
assert_eq!(events_2.len(), 1);
43154315
match events_2[0] {
4316-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
4316+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref inbound_channel_ids, .. } => {
43174317
assert_eq!(payment_hash_1, *payment_hash);
43184318
assert_eq!(amount_msat, 1_000_000);
43194319
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
4320-
assert_eq!(via_channel_id, Some(channel_id));
4320+
assert_eq!(inbound_channel_ids.len(), 1);
4321+
assert_eq!(inbound_channel_ids.last().unwrap().0, channel_id);
43214322
match &purpose {
43224323
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
43234324
assert!(payment_preimage.is_none());

0 commit comments

Comments
 (0)