Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand PaymentClaimable to include all inbound channel IDs for a payment #3655

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Expand PaymentClaimable to include all inbound channel IDs for a payment #3655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83ca899
Introduce ClaimablePayment utilities
shaavan Mar 7, 2025
fa90cc6
Update `PaymentClaimable` fields to improve MPP clarity
shaavan Mar 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions lightning/src/events/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,10 +774,12 @@ 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<ChannelId>,
/// The `user_channel_id` indicating over which channel we received the payment.
via_user_channel_id: Option<u128>,
/// The `channel_id`(s) over which the payment was received.
/// This will be an empty vector for events created/serialised using LDK version 0.0.112 and prior.
via_channel_ids: Vec<ChannelId>,
/// The `user_channel_id`(s) corresponding to the channels over which the payment was received.
/// This will be an empty vector for HTLC events created/serialised using LDK version 0.0.112 and prior.
Comment on lines +778 to +781
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that the list will be incomplete in the case of an MPP payment for versions 0.1.* and below?

via_user_channel_ids: Vec<u128>,
/// The block height at which this payment will be failed back and will no longer be
/// eligible for claiming.
///
Expand Down Expand Up @@ -1506,7 +1508,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 via_channel_ids, ref via_user_channel_ids,
ref claim_deadline, ref onion_fields, ref payment_id,
} => {
1u8.write(writer)?;
Expand Down Expand Up @@ -1540,20 +1542,29 @@ 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_channel_ids.last().cloned();
let via_user_channel_id_legacy = via_user_channel_ids.last().cloned();
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 `via_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),
(9, onion_fields, option),
(10, skimmed_fee_opt, option),
(11, payment_context, option),
(13, payment_id, option),
(15, *via_channel_ids, optional_vec),
(17, *via_user_channel_ids, optional_vec),
});
},
&Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref amount_msat, ref fee_paid_msat } => {
Expand Down Expand Up @@ -1849,41 +1860,54 @@ impl MaybeReadable for Event {
let mut counterparty_skimmed_fee_msat_opt = None;
let mut receiver_node_id = None;
let mut _user_payment_id = None::<u64>; // 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 via_channel_ids_opt = None;
let mut via_user_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),
(9, onion_fields, option),
(10, counterparty_skimmed_fee_msat_opt, option),
(11, payment_context, option),
(13, payment_id, option),
(15, via_channel_ids_opt, optional_vec),
(17, via_user_channel_ids_opt, optional_vec),
});
let purpose = match payment_secret {
Some(secret) => PaymentPurpose::from_parts(payment_preimage, secret, payment_context)
.map_err(|()| msgs::DecodeError::InvalidValue)?,
None if payment_preimage.is_some() => PaymentPurpose::SpontaneousPayment(payment_preimage.unwrap()),
None => return Err(msgs::DecodeError::InvalidValue),
};

let via_channel_ids = via_channel_ids_opt
.or_else(|| via_channel_id_legacy.map(|id| vec![id]))
.unwrap_or_default();

let via_user_channel_ids = via_user_channel_ids_opt
.or_else(|| via_user_channel_id_legacy.map(|id| vec![id]))
.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,
via_channel_ids,
via_user_channel_ids,
claim_deadline,
onion_fields,
payment_id,
Expand Down
22 changes: 11 additions & 11 deletions lightning/src/ln/chanmon_update_fail_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,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 via_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!(*via_channel_ids.last().unwrap(), channel_id);
match &purpose {
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down Expand Up @@ -547,11 +547,11 @@ 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 via_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!(*via_channel_ids.last().unwrap(), channel_id);
match &purpose {
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down Expand Up @@ -665,11 +665,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 via_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!(*via_channel_ids.last().unwrap(), channel_id);
match &purpose {
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down Expand Up @@ -1678,12 +1678,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 via_channel_ids, ref via_user_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!(*via_channel_ids.last().unwrap(), channel_id);
assert_eq!(*via_user_channel_ids.last().unwrap(), 42);
match &purpose {
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand All @@ -1695,11 +1695,11 @@ 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 via_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!(*via_channel_ids.last().unwrap(), channel_id);
match &purpose {
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down
16 changes: 14 additions & 2 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,18 @@ impl ClaimablePayment {
self.htlcs.iter().map(|htlc| (htlc.prev_hop.channel_id, htlc.prev_hop.htlc_id))
)
}

/// Returns the inbound `channel_id`s for all HTLCs associated with the payment.
fn get_channel_ids(&self) -> Vec<ChannelId> {
self.htlcs.iter().map(|htlc| htlc.prev_hop.channel_id).collect()
}

/// Returns the inbound `user_channel_id`s for all HTLCs associated with the payment.
///
/// Note: This list will be incomplete for HTLCs created using LDK version 0.0.112 or earlier.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/0.0.112/0.1 and also note it on the above method?

fn get_user_channel_ids(&self) -> Vec<u128> {
self.htlcs.iter().filter_map(|htlc| htlc.prev_hop.user_channel_id).collect()
}
}

/// Represent the channel funding transaction type.
Expand Down Expand Up @@ -6282,8 +6294,8 @@ 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),
via_channel_ids: claimable_payment.get_channel_ids(),
via_user_channel_ids: claimable_payment.get_user_channel_ids(),
claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER),
onion_fields: claimable_payment.onion_fields.clone(),
payment_id: Some(payment_id),
Expand Down
6 changes: 3 additions & 3 deletions lightning/src/ln/functional_test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2734,7 +2734,7 @@ pub fn do_pass_along_path<'a, 'b, 'c>(args: PassAlongPathArgs) -> Option<Event>
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 via_channel_ids, ref via_user_channel_ids,
claim_deadline, onion_fields, ..
} => {
assert_eq!(our_payment_hash, *payment_hash);
Expand Down Expand Up @@ -2768,8 +2768,8 @@ pub fn do_pass_along_path<'a, 'b, 'c>(args: PassAlongPathArgs) -> Option<Event>
},
}
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()));
assert!(via_channel_ids.iter().all(|id| node.node.list_channels().iter().any(|details| &details.channel_id == id)));
assert!(via_user_channel_ids.iter().all(|id| node.node.list_channels().iter().any(|details| &details.user_channel_id == id)));
assert!(claim_deadline.unwrap() > node.best_block_info().1);
},
_ => panic!("Unexpected event"),
Expand Down
12 changes: 6 additions & 6 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 via_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!(*via_channel_ids.last().unwrap(), chan_2.2);
match &purpose {
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand All @@ -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 via_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!(*via_channel_ids.last().unwrap(), chan_2.2);
match &purpose {
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down Expand Up @@ -4313,11 +4313,11 @@ 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 via_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!(*via_channel_ids.last().unwrap(), channel_id);
match &purpose {
PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand Down
Loading