Skip to content

Commit 5885a54

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 304068d commit 5885a54

File tree

5 files changed

+54
-31
lines changed

5 files changed

+54
-31
lines changed

‎lightning/src/events/mod.rs‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -774,10 +774,16 @@ 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) indicating over which channel(s) we received the payment.
778+
/// - In a non-MPP scenario, this will contain a single `channel_id` where the payment was received.
779+
/// - In an MPP scenario, this will contain multiple `channel_id`s corresponding to the channels over
780+
/// which the payment parts were received.
781+
via_channel_ids: Vec<ChannelId>,
782+
/// The `user_channel_id`(s) indicating over which channel(s) we received the payment.
783+
/// - In a non-MPP scenario, this will contain a single `user_channel_id`.
784+
/// - In an MPP scenario, this will contain multiple `user_channel_id`s corresponding to the channels
785+
/// over which the payment parts were received.
786+
via_user_channel_ids: Vec<u128>,
781787
/// The block height at which this payment will be failed back and will no longer be
782788
/// eligible for claiming.
783789
///
@@ -1506,7 +1512,7 @@ impl Writeable for Event {
15061512
// drop any channels which have not yet exchanged funding_signed.
15071513
},
15081514
&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,
1515+
ref purpose, ref receiver_node_id, ref via_channel_ids, ref via_user_channel_ids,
15101516
ref claim_deadline, ref onion_fields, ref payment_id,
15111517
} => {
15121518
1u8.write(writer)?;
@@ -1544,16 +1550,20 @@ impl Writeable for Event {
15441550
(0, payment_hash, required),
15451551
(1, receiver_node_id, option),
15461552
(2, payment_secret, option),
1547-
(3, via_channel_id, option),
1553+
// legacy field
1554+
// (3, via_channel_id, option),
15481555
(4, amount_msat, required),
1549-
(5, via_user_channel_id, option),
1556+
// legacy field
1557+
// (5, via_user_channel_id, option),
15501558
// Type 6 was `user_payment_id` on 0.0.103 and earlier
15511559
(7, claim_deadline, option),
15521560
(8, payment_preimage, option),
15531561
(9, onion_fields, option),
15541562
(10, skimmed_fee_opt, option),
15551563
(11, payment_context, option),
15561564
(13, payment_id, option),
1565+
(15, *via_channel_ids, optional_vec),
1566+
(17, *via_user_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 } => {
@@ -1855,6 +1865,8 @@ impl MaybeReadable for Event {
18551865
let mut onion_fields = None;
18561866
let mut payment_context = None;
18571867
let mut payment_id = None;
1868+
let mut via_channel_ids_opt = None;
1869+
let mut via_user_channel_ids_opt = None;
18581870
read_tlv_fields!(reader, {
18591871
(0, payment_hash, required),
18601872
(1, receiver_node_id, option),
@@ -1869,21 +1881,32 @@ impl MaybeReadable for Event {
18691881
(10, counterparty_skimmed_fee_msat_opt, option),
18701882
(11, payment_context, option),
18711883
(13, payment_id, option),
1884+
(15, via_channel_ids_opt, optional_vec),
1885+
(17, via_user_channel_ids_opt, optional_vec),
18721886
});
18731887
let purpose = match payment_secret {
18741888
Some(secret) => PaymentPurpose::from_parts(payment_preimage, secret, payment_context)
18751889
.map_err(|()| msgs::DecodeError::InvalidValue)?,
18761890
None if payment_preimage.is_some() => PaymentPurpose::SpontaneousPayment(payment_preimage.unwrap()),
18771891
None => return Err(msgs::DecodeError::InvalidValue),
18781892
};
1893+
1894+
let via_channel_ids = via_channel_ids_opt
1895+
.or_else(|| via_channel_id.map(|id| vec![id]))
1896+
.unwrap_or_default();
1897+
1898+
let via_user_channel_ids = via_user_channel_ids_opt
1899+
.or_else(|| via_user_channel_id.map(|id| vec![id]))
1900+
.unwrap_or_default();
1901+
18791902
Ok(Some(Event::PaymentClaimable {
18801903
receiver_node_id,
18811904
payment_hash,
18821905
amount_msat,
18831906
counterparty_skimmed_fee_msat: counterparty_skimmed_fee_msat_opt.unwrap_or(0),
18841907
purpose,
1885-
via_channel_id,
1886-
via_user_channel_id,
1908+
via_channel_ids,
1909+
via_user_channel_ids,
18871910
claim_deadline,
18881911
onion_fields,
18891912
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
@@ -6292,8 +6292,8 @@ where
62926292
purpose: $purpose,
62936293
amount_msat,
62946294
counterparty_skimmed_fee_msat,
6295-
via_channel_id: Some(prev_channel_id),
6296-
via_user_channel_id: Some(prev_user_channel_id),
6295+
via_channel_ids: claimable_payment.get_channel_ids(),
6296+
via_user_channel_ids: claimable_payment.get_user_channel_ids(),
62976297
claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER),
62986298
onion_fields: claimable_payment.onion_fields.clone(),
62996299
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)