Fail PendingInboundPayments after their expiry time is reached
[rust-lightning] / lightning / src / ln / channelmanager.rs
index bfe7ee39ea76d25ea176c2e6efa7eab5670cd602..ea3c2be917f140308b0d29c92414ccb3fe553f04 100644 (file)
@@ -451,7 +451,7 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
        /// Storage for PaymentSecrets and any requirements on future inbound payments before we will
        /// expose them to users via a PaymentReceived event. HTLCs which do not meet the requirements
        /// here are failed when we process them as pending-forwardable-HTLCs, and entries are removed
-       /// after we generate a PaymentReceived upon receipt of all MPP parts.
+       /// after we generate a PaymentReceived upon receipt of all MPP parts or when they time out.
        /// Locked *after* channel_state.
        pending_inbound_payments: Mutex<HashMap<PaymentHash, PendingInboundPayment>>,
 
@@ -2024,7 +2024,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                                        } else if total_value == payment_data.total_msat {
                                                                                                new_events.push(events::Event::PaymentReceived {
                                                                                                        payment_hash,
-                                                                                                       payment_secret: Some(payment_data.payment_secret),
+                                                                                                       payment_preimage: inbound_payment.get().payment_preimage,
+                                                                                                       payment_secret: payment_data.payment_secret,
                                                                                                        amt: total_value,
                                                                                                        user_payment_id: inbound_payment.get().user_payment_id,
                                                                                                });
@@ -2125,7 +2126,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// along the path (including in our own channel on which we received it).
        /// Returns false if no payment was found to fail backwards, true if the process of failing the
        /// HTLC backwards has been started.
-       pub fn fail_htlc_backwards(&self, payment_hash: &PaymentHash, _payment_secret: &Option<PaymentSecret>) -> bool {
+       pub fn fail_htlc_backwards(&self, payment_hash: &PaymentHash) -> bool {
                let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
 
                let mut channel_state = Some(self.channel_state.lock().unwrap());
@@ -2294,18 +2295,16 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// generating message events for the net layer to claim the payment, if possible. Thus, you
        /// should probably kick the net layer to go send messages if this returns true!
        ///
-       /// You must specify the expected amounts for this HTLC, and we will only claim HTLCs
-       /// available within a few percent of the expected amount. This is critical for several
-       /// reasons : a) it avoids providing senders with `proof-of-payment` (in the form of the
-       /// payment_preimage without having provided the full value and b) it avoids certain
-       /// privacy-breaking recipient-probing attacks which may reveal payment activity to
-       /// motivated attackers.
-       ///
-       /// Note that the privacy concerns in (b) are not relevant in payments with a payment_secret
-       /// set. Thus, for such payments we will claim any payments which do not under-pay.
+       /// 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 `PaymentReceived`
+       /// event matches your expectation. If you fail to do so and call this method, you may provide
+       /// the sender "proof-of-payment" when they did not fulfill the full expected payment.
        ///
        /// May panic if called except in response to a PaymentReceived event.
-       pub fn claim_funds(&self, payment_preimage: PaymentPreimage, _payment_secret: &Option<PaymentSecret>, expected_amount: u64) -> bool {
+       ///
+       /// [`create_inbound_payment`]: Self::create_inbound_payment
+       /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
+       pub fn claim_funds(&self, payment_preimage: PaymentPreimage) -> bool {
                let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 
                let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
@@ -2326,7 +2325,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        // we got all the HTLCs and then a channel closed while we were waiting for the user to
                        // provide the preimage, so worrying too much about the optimal handling isn't worth
                        // it.
-                       let mut valid_mpp = sources[0].payment_data.total_msat >= expected_amount;
+                       let mut valid_mpp = true;
                        for htlc in sources.iter() {
                                if let None = channel_state.as_ref().unwrap().short_to_id.get(&htlc.prev_hop.short_channel_id) {
                                        valid_mpp = false;
@@ -3412,8 +3411,15 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// This differs from [`create_inbound_payment_for_hash`] only in that it generates the
        /// [`PaymentHash`] and [`PaymentPreimage`] for you, returning the first and storing the second.
        ///
+       /// The [`PaymentPreimage`] will ultimately be returned to you in the [`PaymentReceived`], which
+       /// will have the [`PaymentReceived::payment_preimage`] field filled in. That should then be
+       /// passed directly to [`claim_funds`].
+       ///
        /// See [`create_inbound_payment_for_hash`] for detailed documentation on behavior and requirements.
        ///
+       /// [`claim_funds`]: Self::claim_funds
+       /// [`PaymentReceived`]: events::Event::PaymentReceived
+       /// [`PaymentReceived::payment_preimage`]: events::Event::PaymentReceived::payment_preimage
        /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
        pub fn create_inbound_payment(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> (PaymentHash, PaymentSecret) {
                let payment_preimage = PaymentPreimage(self.keys_manager.get_secure_random_bytes());
@@ -3595,6 +3601,10 @@ where
                }
                max_time!(self.last_node_announcement_serial);
                max_time!(self.highest_seen_timestamp);
+               let mut payment_secrets = self.pending_inbound_payments.lock().unwrap();
+               payment_secrets.retain(|_, inbound_payment| {
+                       inbound_payment.expiry_time > header.time as u64
+               });
        }
 
        fn get_relevant_txids(&self) -> Vec<Txid> {
@@ -4848,7 +4858,7 @@ pub mod bench {
 
                                expect_pending_htlcs_forwardable!(NodeHolder { node: &$node_b });
                                expect_payment_received!(NodeHolder { node: &$node_b }, payment_hash, payment_secret, 10_000);
-                               assert!($node_b.claim_funds(payment_preimage, &Some(payment_secret), 10_000));
+                               assert!($node_b.claim_funds(payment_preimage));
 
                                match $node_b.get_and_clear_pending_msg_events().pop().unwrap() {
                                        MessageSendEvent::UpdateHTLCs { node_id, updates } => {