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`.

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 2d576d33a0581404baf686b64587e65a6d0b3ab6..318cc2a6d1aabe183f44861e7bd84aea96d2355a 100644 (file)
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -321,7 +321,9 @@ pub enum Event {
        ///
        /// # Note
        /// LDK will not stop an inbound payment from being paid multiple times, so multiple
-       /// `PaymentClaimable` events may be generated for the same payment.
+       /// `PaymentClaimable` events may be generated for the same payment. In such a case it is
+       /// polite (and required in the lightning specification) to fail the payment the second time
+       /// and give the sender their money back rather than accepting double payment.
        ///
        /// # Note
        /// This event used to be called `PaymentReceived` in LDK versions 0.0.112 and earlier.
@@ -349,6 +351,14 @@ pub enum Event {
                via_channel_id: Option<[u8; 32]>,
                /// The `user_channel_id` indicating over which channel we received the payment.
                via_user_channel_id: Option<u128>,
+               /// The block height at which this payment will be failed back and will no longer be
+               /// eligible for claiming.
+               ///
+               /// Prior to this height, a call to [`ChannelManager::claim_funds`] is guaranteed to
+               /// succeed, however you should wait for [`Event::PaymentClaimed`] to be sure.
+               ///
+               /// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds
+               claim_deadline: Option<u32>,
        },
        /// Indicates a payment has been claimed and we've received money!
        ///
@@ -773,7 +783,7 @@ impl Writeable for Event {
                                // We never write out FundingGenerationReady events as, upon disconnection, peers
                                // drop any channels which have not yet exchanged funding_signed.
                        },
-                       &Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id } => {
+                       &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 } => {
                                1u8.write(writer)?;
                                let mut payment_secret = None;
                                let payment_preimage;
@@ -794,6 +804,7 @@ impl Writeable for Event {
                                        (4, amount_msat, required),
                                        (5, via_user_channel_id, option),
                                        (6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier
+                                       (7, claim_deadline, option),
                                        (8, payment_preimage, option),
                                });
                        },
@@ -992,6 +1003,7 @@ impl MaybeReadable for Event {
                                        let mut receiver_node_id = None;
                                        let mut _user_payment_id = None::<u64>; // For compatibility with 0.0.103 and earlier
                                        let mut via_channel_id = None;
+                                       let mut claim_deadline = None;
                                        let mut via_user_channel_id = None;
                                        read_tlv_fields!(reader, {
                                                (0, payment_hash, required),
@@ -1001,6 +1013,7 @@ impl MaybeReadable for Event {
                                                (4, amount_msat, required),
                                                (5, via_user_channel_id, option),
                                                (6, _user_payment_id, option),
+                                               (7, claim_deadline, option),
                                                (8, payment_preimage, option),
                                        });
                                        let purpose = match payment_secret {
@@ -1018,6 +1031,7 @@ impl MaybeReadable for Event {
                                                purpose,
                                                via_channel_id,
                                                via_user_channel_id,
+                                               claim_deadline,
                                        }))
                                };
                                f()

diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
index 94df7371a27b55205091a5900b6a87c06872975a..d12fe5c3a6ba1ac788887641ddd04c67d4468c11 100644 (file)
--- a/lightning/src/ln/chanmon_update_fail_tests.rs
+++ b/lightning/src/ln/chanmon_update_fail_tests.rs
@@ -204,7 +204,7 @@ 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, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        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());
@@ -573,7 +573,7 @@ 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, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        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());
@@ -690,7 +690,7 @@ 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, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        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());
@@ -1668,7 +1668,7 @@ 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, via_channel_id, via_user_channel_id, .. } => {
                        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());
@@ -1685,7 +1685,7 @@ 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, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        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());

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 8b536c5dfc6464fc99e73460ec82ff86809c7fc1..241235fd20aeb011eab547ba76d5c1702eb784d7 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3363,8 +3363,10 @@ where
                                                                                        }
                                                                                }
                                                                                let mut total_value = claimable_htlc.sender_intended_value;
+                                                                               let mut earliest_expiry = claimable_htlc.cltv_expiry;
                                                                                for htlc in htlcs.iter() {
                                                                                        total_value += htlc.sender_intended_value;
+                                                                                       earliest_expiry = cmp::min(earliest_expiry, htlc.cltv_expiry);
                                                                                        match &htlc.onion_payload {
                                                                                                OnionPayload::Invoice { .. } => {
                                                                                                        if htlc.total_msat != $payment_data.total_msat {
@@ -3397,6 +3399,7 @@ where
                                                                                                amount_msat,
                                                                                                via_channel_id: Some(prev_channel_id),
                                                                                                via_user_channel_id: Some(prev_user_channel_id),
+                                                                                               claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER),
                                                                                        });
                                                                                        payment_claimable_generated = true;
                                                                                } else {
@@ -3450,6 +3453,7 @@ where
                                                                                                        hash_map::Entry::Vacant(e) => {
                                                                                                                let amount_msat = claimable_htlc.value;
                                                                                                                claimable_htlc.total_value_received = Some(amount_msat);
+                                                                                                               let claim_deadline = Some(claimable_htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER);
                                                                                                                let purpose = events::PaymentPurpose::SpontaneousPayment(preimage);
                                                                                                                e.insert((purpose.clone(), vec![claimable_htlc]));
                                                                                                                let prev_channel_id = prev_funding_outpoint.to_channel_id();
@@ -3460,6 +3464,7 @@ where
                                                                                                                        purpose,
                                                                                                                        via_channel_id: Some(prev_channel_id),
                                                                                                                        via_user_channel_id: Some(prev_user_channel_id),
+                                                                                                                       claim_deadline,
                                                                                                                });
                                                                                                        },
                                                                                                        hash_map::Entry::Occupied(_) => {
@@ -3935,9 +3940,10 @@ where
        /// Provides a payment preimage in response to [`Event::PaymentClaimable`], generating any
        /// [`MessageSendEvent`]s needed to claim the payment.
        ///
-       /// Note that calling this method does *not* guarantee that the payment has been claimed. You
-       /// *must* wait for an [`Event::PaymentClaimed`] event which upon a successful claim will be
-       /// provided to your [`EventHandler`] when [`process_pending_events`] is next called.
+       /// This method is guaranteed to ensure the payment has been claimed but only if the current
+       /// height is strictly below [`Event::PaymentClaimable::claim_deadline`]. To avoid race
+       /// conditions, you should wait for an [`Event::PaymentClaimed`] before considering the payment
+       /// successful. It will generally be available in the next [`process_pending_events`] call.
        ///
        /// Note that if you did not set an `amount_msat` when calling [`create_inbound_payment`] or
        /// [`create_inbound_payment_for_hash`] you must check that the amount in the `PaymentClaimable`
@@ -3945,6 +3951,7 @@ where
        /// the sender "proof-of-payment" when they did not fulfill the full expected payment.
        ///
        /// [`Event::PaymentClaimable`]: crate::events::Event::PaymentClaimable
+       /// [`Event::PaymentClaimable::claim_deadline`]: crate::events::Event::PaymentClaimable::claim_deadline
        /// [`Event::PaymentClaimed`]: crate::events::Event::PaymentClaimed
        /// [`process_pending_events`]: EventsProvider::process_pending_events
        /// [`create_inbound_payment`]: Self::create_inbound_payment

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index d7fd9c8e99393220192514ee9ed29e19ab31196e..2d52135da09169eaaf3f96aac31e5cefdcf7464f 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1680,7 +1680,7 @@ macro_rules! expect_payment_claimable {
                let events = $node.node.get_and_clear_pending_events();
                assert_eq!(events.len(), 1);
                match events[0] {
-                       $crate::events::Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id: _, via_user_channel_id: _ } => {
+                       $crate::events::Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, .. } => {
                                assert_eq!($expected_payment_hash, *payment_hash);
                                assert_eq!($expected_recv_value, amount_msat);
                                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>,
        payment_id
 }
 
-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>) {
+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> {
        let mut payment_event = SendEvent::from_event(ev);
        let mut prev_node = origin_node;
+       let mut event = None;
 
        for (idx, &node) in expected_path.iter().enumerate() {
                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
                        if payment_claimable_expected {
                                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 } => {
+                                       Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_id, ref via_user_channel_id, claim_deadline } => {
                                                assert_eq!(our_payment_hash, *payment_hash);
                                                assert_eq!(node.node.get_our_node_id(), receiver_node_id.unwrap());
                                                match &purpose {
@@ -1996,9 +1997,11 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p
                                                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!(claim_deadline.unwrap() > node.best_block_info().1);
                                        },
                                        _ => panic!("Unexpected event"),
                                }
+                               event = Some(events_2[0].clone());
                        } else {
                                assert!(events_2.is_empty());
                        }
@@ -2012,10 +2015,11 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p
 
                prev_node = node;
        }
+       event
 }
 
-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>) {
-       do_pass_along_path(origin_node, expected_path, recv_value, our_payment_hash, our_payment_secret, ev, payment_claimable_expected, true, expected_preimage);
+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> {
+       do_pass_along_path(origin_node, expected_path, recv_value, our_payment_hash, our_payment_secret, ev, payment_claimable_expected, true, expected_preimage)
 }
 
 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) {

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 4f73627f4b4d24ef6ab414311f09b95d64d82208..9c0a4b4b066ac1e50c5cacabf836979cd9dec792 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -1966,7 +1966,7 @@ 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, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        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());
@@ -1982,7 +1982,7 @@ 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, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        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());
@@ -3764,7 +3764,7 @@ 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, via_user_channel_id: _ } => {
+               Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => {
                        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());

diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index fb50f5a4a38d4d2629e21954679a6c850b420a58..028af20ee600c8402b0f7352f79404273b85052e 100644 (file)
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -2898,9 +2898,16 @@ fn do_claim_from_closed_chan(fail_payment: bool) {
        assert_eq!(send_msgs.len(), 2);
        pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 10_000_000,
                payment_hash, Some(payment_secret), send_msgs.remove(0), false, None);
-       pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 10_000_000,
+       let receive_event = pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 10_000_000,
                payment_hash, Some(payment_secret), send_msgs.remove(0), true, None);
 
+       match receive_event.unwrap() {
+               Event::PaymentClaimable { claim_deadline, .. } => {
+                       assert_eq!(claim_deadline.unwrap(), final_cltv - HTLC_FAIL_BACK_BUFFER);
+               },
+               _ => panic!(),
+       }
+
        // Ensure that the claim_deadline is correct, with the payment failing at exactly the given
        // height.
        connect_blocks(&nodes[3], final_cltv - HTLC_FAIL_BACK_BUFFER - nodes[3].best_block_info().1