Skip to content

Commit 4a8d01d

Browse files
TheBlueMattTheBlueMatt
committed
Add a claim_deadline field to PaymentClaimable with guarantees
Now that we guarantee `claim_payment` will always succeed we have to let the user know what the deadline is. We still fail payments if they haven't been claimed in time, which we now expose in `PaymentClaimable`.
1 parent ab25589 commit 4a8d01d

File tree

6 files changed

+51
-19
lines changed

6 files changed

+51
-19
lines changed

‎lightning/src/events/mod.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,9 @@ pub enum Event {
321321
///
322322
/// # Note
323323
/// LDK will not stop an inbound payment from being paid multiple times, so multiple
324-
/// `PaymentClaimable` events may be generated for the same payment.
324+
/// `PaymentClaimable` events may be generated for the same payment. In such a case it is
325+
/// polite (and required in the lightning specification) to fail the payment the second time
326+
/// and give the sender their money back rather than accepting double payment.
325327
///
326328
/// # Note
327329
/// This event used to be called `PaymentReceived` in LDK versions 0.0.112 and earlier.
@@ -349,6 +351,14 @@ pub enum Event {
349351
via_channel_id: Option<[u8; 32]>,
350352
/// The `user_channel_id` indicating over which channel we received the payment.
351353
via_user_channel_id: Option<u128>,
354+
/// The block height at which this payment will be failed back and will no longer be
355+
/// eligible for claiming.
356+
///
357+
/// Prior to this height, a call to [`ChannelManager::claim_funds`] is guaranteed to
358+
/// succeed, however you should wait for [`Event::PaymentClaimed`] to be sure.
359+
///
360+
/// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds
361+
claim_deadline: Option<u32>,
352362
},
353363
/// Indicates a payment has been claimed and we've received money!
354364
///
@@ -773,7 +783,7 @@ impl Writeable for Event {
773783
// We never write out FundingGenerationReady events as, upon disconnection, peers
774784
// drop any channels which have not yet exchanged funding_signed.
775785
},
776-
&Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id } => {
786+
&Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id, ref claim_deadline } => {
777787
1u8.write(writer)?;
778788
let mut payment_secret = None;
779789
let payment_preimage;
@@ -794,6 +804,7 @@ impl Writeable for Event {
794804
(4, amount_msat, required),
795805
(5, via_user_channel_id, option),
796806
(6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier
807+
(7, claim_deadline, option),
797808
(8, payment_preimage, option),
798809
});
799810
},
@@ -992,6 +1003,7 @@ impl MaybeReadable for Event {
9921003
let mut receiver_node_id = None;
9931004
let mut _user_payment_id = None::<u64>; // For compatibility with 0.0.103 and earlier
9941005
let mut via_channel_id = None;
1006+
let mut claim_deadline = None;
9951007
let mut via_user_channel_id = None;
9961008
read_tlv_fields!(reader, {
9971009
(0, payment_hash, required),
@@ -1001,6 +1013,7 @@ impl MaybeReadable for Event {
10011013
(4, amount_msat, required),
10021014
(5, via_user_channel_id, option),
10031015
(6, _user_payment_id, option),
1016+
(7, claim_deadline, option),
10041017
(8, payment_preimage, option),
10051018
});
10061019
let purpose = match payment_secret {
@@ -1018,6 +1031,7 @@ impl MaybeReadable for Event {
10181031
purpose,
10191032
via_channel_id,
10201033
via_user_channel_id,
1034+
claim_deadline,
10211035
}))
10221036
};
10231037
f()

‎lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
204204
let events_3 = nodes[1].node.get_and_clear_pending_events();
205205
assert_eq!(events_3.len(), 1);
206206
match events_3[0] {
207-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
207+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
208208
assert_eq!(payment_hash_1, *payment_hash);
209209
assert_eq!(amount_msat, 1_000_000);
210210
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
@@ -573,7 +573,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
573573
let events_5 = nodes[1].node.get_and_clear_pending_events();
574574
assert_eq!(events_5.len(), 1);
575575
match events_5[0] {
576-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
576+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
577577
assert_eq!(payment_hash_2, *payment_hash);
578578
assert_eq!(amount_msat, 1_000_000);
579579
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
@@ -690,7 +690,7 @@ fn test_monitor_update_fail_cs() {
690690
let events = nodes[1].node.get_and_clear_pending_events();
691691
assert_eq!(events.len(), 1);
692692
match events[0] {
693-
Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
693+
Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
694694
assert_eq!(payment_hash, our_payment_hash);
695695
assert_eq!(amount_msat, 1_000_000);
696696
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());
@@ -1668,7 +1668,7 @@ fn test_monitor_update_fail_claim() {
16681668
let events = nodes[0].node.get_and_clear_pending_events();
16691669
assert_eq!(events.len(), 2);
16701670
match events[0] {
1671-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id } => {
1671+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id, .. } => {
16721672
assert_eq!(payment_hash_2, *payment_hash);
16731673
assert_eq!(1_000_000, amount_msat);
16741674
assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id());
@@ -1685,7 +1685,7 @@ fn test_monitor_update_fail_claim() {
16851685
_ => panic!("Unexpected event"),
16861686
}
16871687
match events[1] {
1688-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
1688+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
16891689
assert_eq!(payment_hash_3, *payment_hash);
16901690
assert_eq!(1_000_000, amount_msat);
16911691
assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id());

‎lightning/src/ln/channelmanager.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3363,8 +3363,10 @@ where
33633363
}
33643364
}
33653365
let mut total_value = claimable_htlc.sender_intended_value;
3366+
let mut earliest_expiry = claimable_htlc.cltv_expiry;
33663367
for htlc in htlcs.iter() {
33673368
total_value += htlc.sender_intended_value;
3369+
earliest_expiry = cmp::min(earliest_expiry, htlc.cltv_expiry);
33683370
match &htlc.onion_payload {
33693371
OnionPayload::Invoice { .. } => {
33703372
if htlc.total_msat != $payment_data.total_msat {
@@ -3397,6 +3399,7 @@ where
33973399
amount_msat,
33983400
via_channel_id: Some(prev_channel_id),
33993401
via_user_channel_id: Some(prev_user_channel_id),
3402+
claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER),
34003403
});
34013404
payment_claimable_generated = true;
34023405
} else {
@@ -3450,6 +3453,7 @@ where
34503453
hash_map::Entry::Vacant(e) => {
34513454
let amount_msat = claimable_htlc.value;
34523455
claimable_htlc.total_value_received = Some(amount_msat);
3456+
let claim_deadline = Some(claimable_htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER);
34533457
let purpose = events::PaymentPurpose::SpontaneousPayment(preimage);
34543458
e.insert((purpose.clone(), vec![claimable_htlc]));
34553459
let prev_channel_id = prev_funding_outpoint.to_channel_id();
@@ -3460,6 +3464,7 @@ where
34603464
purpose,
34613465
via_channel_id: Some(prev_channel_id),
34623466
via_user_channel_id: Some(prev_user_channel_id),
3467+
claim_deadline,
34633468
});
34643469
},
34653470
hash_map::Entry::Occupied(_) => {
@@ -3935,16 +3940,18 @@ where
39353940
/// Provides a payment preimage in response to [`Event::PaymentClaimable`], generating any
39363941
/// [`MessageSendEvent`]s needed to claim the payment.
39373942
///
3938-
/// Note that calling this method does *not* guarantee that the payment has been claimed. You
3939-
/// *must* wait for an [`Event::PaymentClaimed`] event which upon a successful claim will be
3940-
/// provided to your [`EventHandler`] when [`process_pending_events`] is next called.
3943+
/// This method is guaranteed to ensure the payment has been claimed but only if the current
3944+
/// height is strictly below [`Event::PaymentClaimable::claim_deadline`]. To avoid race
3945+
/// conditions, you should wait for an [`Event::PaymentClaimed`] before considering the payment
3946+
/// successful. It will generally be available in the next [`process_pending_events`] call.
39413947
///
39423948
/// Note that if you did not set an `amount_msat` when calling [`create_inbound_payment`] or
39433949
/// [`create_inbound_payment_for_hash`] you must check that the amount in the `PaymentClaimable`
39443950
/// event matches your expectation. If you fail to do so and call this method, you may provide
39453951
/// the sender "proof-of-payment" when they did not fulfill the full expected payment.
39463952
///
39473953
/// [`Event::PaymentClaimable`]: crate::events::Event::PaymentClaimable
3954+
/// [`Event::PaymentClaimable::claim_deadline`]: crate::events::Event::PaymentClaimable::claim_deadline
39483955
/// [`Event::PaymentClaimed`]: crate::events::Event::PaymentClaimed
39493956
/// [`process_pending_events`]: EventsProvider::process_pending_events
39503957
/// [`create_inbound_payment`]: Self::create_inbound_payment

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,7 +1680,7 @@ macro_rules! expect_payment_claimable {
16801680
let events = $node.node.get_and_clear_pending_events();
16811681
assert_eq!(events.len(), 1);
16821682
match events[0] {
1683-
$crate::events::Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id: _, via_user_channel_id: _ } => {
1683+
$crate::events::Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, .. } => {
16841684
assert_eq!($expected_payment_hash, *payment_hash);
16851685
assert_eq!($expected_recv_value, amount_msat);
16861686
assert_eq!($expected_receiver_node_id, receiver_node_id.unwrap());
@@ -1962,9 +1962,10 @@ pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
19621962
payment_id
19631963
}
19641964

1965-
pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option<PaymentSecret>, ev: MessageSendEvent, payment_claimable_expected: bool, clear_recipient_events: bool, expected_preimage: Option<PaymentPreimage>) {
1965+
pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option<PaymentSecret>, ev: MessageSendEvent, payment_claimable_expected: bool, clear_recipient_events: bool, expected_preimage: Option<PaymentPreimage>) -> Option<Event> {
19661966
let mut payment_event = SendEvent::from_event(ev);
19671967
let mut prev_node = origin_node;
1968+
let mut event = None;
19681969

19691970
for (idx, &node) in expected_path.iter().enumerate() {
19701971
assert_eq!(node.node.get_our_node_id(), payment_event.node_id);
@@ -1980,7 +1981,7 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p
19801981
if payment_claimable_expected {
19811982
assert_eq!(events_2.len(), 1);
19821983
match events_2[0] {
1983-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_id, ref via_user_channel_id } => {
1984+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_id, ref via_user_channel_id, claim_deadline } => {
19841985
assert_eq!(our_payment_hash, *payment_hash);
19851986
assert_eq!(node.node.get_our_node_id(), receiver_node_id.unwrap());
19861987
match &purpose {
@@ -1996,9 +1997,11 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p
19961997
assert_eq!(amount_msat, recv_value);
19971998
assert!(node.node.list_channels().iter().any(|details| details.channel_id == via_channel_id.unwrap()));
19981999
assert!(node.node.list_channels().iter().any(|details| details.user_channel_id == via_user_channel_id.unwrap()));
2000+
assert!(claim_deadline.unwrap() > node.best_block_info().1);
19992001
},
20002002
_ => panic!("Unexpected event"),
20012003
}
2004+
event = Some(events_2[0].clone());
20022005
} else {
20032006
assert!(events_2.is_empty());
20042007
}
@@ -2012,10 +2015,11 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p
20122015

20132016
prev_node = node;
20142017
}
2018+
event
20152019
}
20162020

2017-
pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option<PaymentSecret>, ev: MessageSendEvent, payment_claimable_expected: bool, expected_preimage: Option<PaymentPreimage>) {
2018-
do_pass_along_path(origin_node, expected_path, recv_value, our_payment_hash, our_payment_secret, ev, payment_claimable_expected, true, expected_preimage);
2021+
pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option<PaymentSecret>, ev: MessageSendEvent, payment_claimable_expected: bool, expected_preimage: Option<PaymentPreimage>) -> Option<Event> {
2022+
do_pass_along_path(origin_node, expected_path, recv_value, our_payment_hash, our_payment_secret, ev, payment_claimable_expected, true, expected_preimage)
20192023
}
20202024

20212025
pub fn pass_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) {

‎lightning/src/ln/functional_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1966,7 +1966,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
19661966
let events = nodes[2].node.get_and_clear_pending_events();
19671967
assert_eq!(events.len(), 2);
19681968
match events[0] {
1969-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
1969+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
19701970
assert_eq!(our_payment_hash_21, *payment_hash);
19711971
assert_eq!(recv_value_21, amount_msat);
19721972
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
@@ -1982,7 +1982,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
19821982
_ => panic!("Unexpected event"),
19831983
}
19841984
match events[1] {
1985-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
1985+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
19861986
assert_eq!(our_payment_hash_22, *payment_hash);
19871987
assert_eq!(recv_value_22, amount_msat);
19881988
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
@@ -3764,7 +3764,7 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
37643764
let events_2 = nodes[1].node.get_and_clear_pending_events();
37653765
assert_eq!(events_2.len(), 1);
37663766
match events_2[0] {
3767-
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => {
3767+
Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
37683768
assert_eq!(payment_hash_1, *payment_hash);
37693769
assert_eq!(amount_msat, 1_000_000);
37703770
assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id());

‎lightning/src/ln/payment_tests.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2898,9 +2898,16 @@ fn do_claim_from_closed_chan(fail_payment: bool) {
28982898
assert_eq!(send_msgs.len(), 2);
28992899
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 10_000_000,
29002900
payment_hash, Some(payment_secret), send_msgs.remove(0), false, None);
2901-
pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 10_000_000,
2901+
let receive_event = pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 10_000_000,
29022902
payment_hash, Some(payment_secret), send_msgs.remove(0), true, None);
29032903

2904+
match receive_event.unwrap() {
2905+
Event::PaymentClaimable { claim_deadline, .. } => {
2906+
assert_eq!(claim_deadline.unwrap(), final_cltv - HTLC_FAIL_BACK_BUFFER);
2907+
},
2908+
_ => panic!(),
2909+
}
2910+
29042911
// Ensure that the claim_deadline is correct, with the payment failing at exactly the given
29052912
// height.
29062913
connect_blocks(&nodes[3], final_cltv - HTLC_FAIL_BACK_BUFFER - nodes[3].best_block_info().1

0 commit comments

Comments
 (0)