Skip to content

Commit 591c5c8

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 6afd4fa commit 591c5c8

5 files changed

Lines changed: 59 additions & 35 deletions

File tree

‎lightning/src/events/mod.rs‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -774,10 +774,12 @@ 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`(s) over which the payment was received.
778+
/// This will be an incomplete vector for events created/serialized using LDK version 0.1.0 and prior.
779+
via_channel_ids: Vec<ChannelId>,
780+
/// The `user_channel_id`(s) corresponding to the channels over which the payment was received.
781+
/// This will be an incomplete vector for events created/serialized using LDK version 0.1.0 and prior.
782+
via_user_channel_ids: Vec<u128>,
781783
/// The block height at which this payment will be failed back and will no longer be
782784
/// eligible for claiming.
783785
///
@@ -1506,7 +1508,7 @@ impl Writeable for Event {
15061508
// drop any channels which have not yet exchanged funding_signed.
15071509
},
15081510
&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,
1511+
ref purpose, ref receiver_node_id, ref via_channel_ids, ref via_user_channel_ids,
15101512
ref claim_deadline, ref onion_fields, ref payment_id,
15111513
} => {
15121514
1u8.write(writer)?;
@@ -1540,20 +1542,29 @@ impl Writeable for Event {
15401542
}
15411543
let skimmed_fee_opt = if counterparty_skimmed_fee_msat == 0 { None }
15421544
else { Some(counterparty_skimmed_fee_msat) };
1545+
1546+
let via_channel_id_legacy = via_channel_ids.last().cloned();
1547+
let via_user_channel_id_legacy = via_user_channel_ids.last().cloned();
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 `via_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, *via_channel_ids, optional_vec),
1567+
(17, *via_user_channel_ids, optional_vec),
15571568
});
15581569
},
15591570
&Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref amount_msat, ref fee_paid_msat } => {
@@ -1849,41 +1860,54 @@ impl MaybeReadable for Event {
18491860
let mut counterparty_skimmed_fee_msat_opt = None;
18501861
let mut receiver_node_id = None;
18511862
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;
1863+
let mut via_channel_id_legacy = None;
18531864
let mut claim_deadline = None;
1854-
let mut via_user_channel_id = None;
1865+
let mut via_user_channel_id_legacy = None;
18551866
let mut onion_fields = None;
18561867
let mut payment_context = None;
18571868
let mut payment_id = None;
1869+
let mut via_channel_ids_opt = None;
1870+
let mut via_user_channel_ids_opt = None;
18581871
read_tlv_fields!(reader, {
18591872
(0, payment_hash, required),
18601873
(1, receiver_node_id, option),
18611874
(2, payment_secret, option),
1862-
(3, via_channel_id, option),
1875+
(3, via_channel_id_legacy, option),
18631876
(4, amount_msat, required),
1864-
(5, via_user_channel_id, option),
1877+
(5, via_user_channel_id_legacy, option),
18651878
(6, _user_payment_id, option),
18661879
(7, claim_deadline, option),
18671880
(8, payment_preimage, option),
18681881
(9, onion_fields, option),
18691882
(10, counterparty_skimmed_fee_msat_opt, option),
18701883
(11, payment_context, option),
18711884
(13, payment_id, option),
1885+
(15, via_channel_ids_opt, optional_vec),
1886+
(17, via_user_channel_ids_opt, optional_vec),
18721887
});
18731888
let purpose = match payment_secret {
18741889
Some(secret) => PaymentPurpose::from_parts(payment_preimage, secret, payment_context)
18751890
.map_err(|()| msgs::DecodeError::InvalidValue)?,
18761891
None if payment_preimage.is_some() => PaymentPurpose::SpontaneousPayment(payment_preimage.unwrap()),
18771892
None => return Err(msgs::DecodeError::InvalidValue),
18781893
};
1894+
1895+
let via_channel_ids = via_channel_ids_opt
1896+
.or_else(|| via_channel_id_legacy.map(|id| vec![id]))
1897+
.unwrap_or_default();
1898+
1899+
let via_user_channel_ids = via_user_channel_ids_opt
1900+
.or_else(|| via_user_channel_id_legacy.map(|id| vec![id]))
1901+
.unwrap_or_default();
1902+
18791903
Ok(Some(Event::PaymentClaimable {
18801904
receiver_node_id,
18811905
payment_hash,
18821906
amount_msat,
18831907
counterparty_skimmed_fee_msat: counterparty_skimmed_fee_msat_opt.unwrap_or(0),
18841908
purpose,
1885-
via_channel_id,
1886-
via_user_channel_id,
1909+
via_channel_ids,
1910+
via_user_channel_ids,
18871911
claim_deadline,
18881912
onion_fields,
18891913
payment_id,

‎lightning/src/ln/chanmon_update_fail_tests.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,11 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
165165
let events_3 = nodes[1].node.get_and_clear_pending_events();
166166
assert_eq!(events_3.len(), 1);
167167
match events_3[0] {
168-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
168+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_ids, .. } => {
169169
assert_eq!(payment_hash_1, *payment_hash);
170170
assert_eq!(amount_msat, 1_000_000);
171171
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
172-
assert_eq!(via_channel_id, Some(channel_id));
172+
assert_eq!(*via_channel_ids.last().unwrap(), channel_id);
173173
match &purpose {
174174
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
175175
assert!(payment_preimage.is_none());
@@ -547,11 +547,11 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
547547
let events_5 = nodes[1].node.get_and_clear_pending_events();
548548
assert_eq!(events_5.len(), 1);
549549
match events_5[0] {
550-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
550+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_ids, .. } => {
551551
assert_eq!(payment_hash_2, *payment_hash);
552552
assert_eq!(amount_msat, 1_000_000);
553553
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
554-
assert_eq!(via_channel_id, Some(channel_id));
554+
assert_eq!(*via_channel_ids.last().unwrap(), channel_id);
555555
match &purpose {
556556
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
557557
assert!(payment_preimage.is_none());
@@ -665,11 +665,11 @@ fn test_monitor_update_fail_cs() {
665665
let events = nodes[1].node.get_and_clear_pending_events();
666666
assert_eq!(events.len(), 1);
667667
match events[0] {
668-
Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
668+
Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_ids, .. } => {
669669
assert_eq!(payment_hash, our_payment_hash);
670670
assert_eq!(amount_msat, 1_000_000);
671671
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
672-
assert_eq!(via_channel_id, Some(channel_id));
672+
assert_eq!(*via_channel_ids.last().unwrap(), channel_id);
673673
match &purpose {
674674
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
675675
assert!(payment_preimage.is_none());
@@ -1678,12 +1678,12 @@ fn test_monitor_update_fail_claim() {
16781678
let events = nodes[0].node.get_and_clear_pending_events();
16791679
assert_eq!(events.len(), 2);
16801680
match events[0] {
1681-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id, .. } => {
1681+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_ids, ref via_user_channel_ids, .. } => {
16821682
assert_eq!(payment_hash_2, *payment_hash);
16831683
assert_eq!(1_000_000, amount_msat);
16841684
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));
1685+
assert_eq!(*via_channel_ids.last().unwrap(), channel_id);
1686+
assert_eq!(*via_user_channel_ids.last().unwrap(), 42);
16871687
match &purpose {
16881688
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
16891689
assert!(payment_preimage.is_none());
@@ -1695,11 +1695,11 @@ fn test_monitor_update_fail_claim() {
16951695
_ => panic!("Unexpected event"),
16961696
}
16971697
match events[1] {
1698-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
1698+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_ids, .. } => {
16991699
assert_eq!(payment_hash_3, *payment_hash);
17001700
assert_eq!(1_000_000, amount_msat);
17011701
assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id());
1702-
assert_eq!(via_channel_id, Some(channel_id));
1702+
assert_eq!(*via_channel_ids.last().unwrap(), channel_id);
17031703
match &purpose {
17041704
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
17051705
assert!(payment_preimage.is_none());

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6296,8 +6296,8 @@ where
62966296
purpose: $purpose,
62976297
amount_msat,
62986298
counterparty_skimmed_fee_msat,
6299-
via_channel_id: Some(prev_channel_id),
6300-
via_user_channel_id: Some(prev_user_channel_id),
6299+
via_channel_ids: claimable_payment.get_channel_ids(),
6300+
via_user_channel_ids: claimable_payment.get_user_channel_ids(),
63016301
claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER),
63026302
onion_fields: claimable_payment.onion_fields.clone(),
63036303
payment_id: Some(payment_id),

‎lightning/src/ln/functional_test_utils.rs‎

Lines changed: 3 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 via_channel_ids, ref via_user_channel_ids,
27382738
claim_deadline, onion_fields, ..
27392739
} => {
27402740
assert_eq!(our_payment_hash, *payment_hash);
@@ -2768,8 +2768,8 @@ 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+
assert!(via_channel_ids.iter().all(|id| node.node.list_channels().iter().any(|details| &details.channel_id == id)));
2772+
assert!(via_user_channel_ids.iter().all(|id| node.node.list_channels().iter().any(|details| &details.user_channel_id == id)));
27732773
assert!(claim_deadline.unwrap() > node.best_block_info().1);
27742774
},
27752775
_ => panic!("Unexpected event"),

‎lightning/src/ln/functional_tests.rs‎

Lines changed: 6 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 via_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!(*via_channel_ids.last().unwrap(), 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 via_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!(*via_channel_ids.last().unwrap(), chan_2.2);
20922092
match &purpose {
20932093
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
20942094
assert!(payment_preimage.is_none());
@@ -4313,11 +4313,11 @@ 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 via_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!(*via_channel_ids.last().unwrap(), channel_id);
43214321
match &purpose {
43224322
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
43234323
assert!(payment_preimage.is_none());

0 commit comments

Comments
 (0)