Merge pull request #1211 from ConorOkus/2021-11-add-max-conversion
[rust-lightning] / lightning / src / ln / channelmanager.rs
index b6d530ac5c3299131c6be3f38b12340f0c01f2c9..47d705fe625b4889d7d3da19681af0d4272b8807 100644 (file)
@@ -413,6 +413,9 @@ struct PeerState {
 ///
 /// For users who don't want to bother doing their own payment preimage storage, we also store that
 /// here.
+///
+/// Note that this struct will be removed entirely soon, in favor of storing no inbound payment data
+/// and instead encoding it in the payment secret.
 struct PendingInboundPayment {
        /// The payment secret that the sender must use for us to accept this payment
        payment_secret: PaymentSecret,
@@ -449,6 +452,18 @@ pub(crate) enum PendingOutboundPayment {
        /// and add a pending payment that was already fulfilled.
        Fulfilled {
                session_privs: HashSet<[u8; 32]>,
+               payment_hash: Option<PaymentHash>,
+       },
+       /// When a payer gives up trying to retry a payment, they inform us, letting us generate a
+       /// `PaymentFailed` event when all HTLCs have irrevocably failed. This avoids a number of race
+       /// conditions in MPP-aware payment retriers (1), where the possibility of multiple
+       /// `PaymentPathFailed` events with `all_paths_failed` can be pending at once, confusing a
+       /// downstream event handler as to when a payment has actually failed.
+       ///
+       /// (1) https://github.com/lightningdevkit/rust-lightning/issues/1164
+       Abandoned {
+               session_privs: HashSet<[u8; 32]>,
+               payment_hash: PaymentHash,
        },
 }
 
@@ -465,6 +480,12 @@ impl PendingOutboundPayment {
                        _ => false,
                }
        }
+       fn abandoned(&self) -> bool {
+               match self {
+                       PendingOutboundPayment::Abandoned { .. } => true,
+                       _ => false,
+               }
+       }
        fn get_pending_fee_msat(&self) -> Option<u64> {
                match self {
                        PendingOutboundPayment::Retryable { pending_fee_msat, .. } => pending_fee_msat.clone(),
@@ -472,15 +493,43 @@ impl PendingOutboundPayment {
                }
        }
 
+       fn payment_hash(&self) -> Option<PaymentHash> {
+               match self {
+                       PendingOutboundPayment::Legacy { .. } => None,
+                       PendingOutboundPayment::Retryable { payment_hash, .. } => Some(*payment_hash),
+                       PendingOutboundPayment::Fulfilled { payment_hash, .. } => *payment_hash,
+                       PendingOutboundPayment::Abandoned { payment_hash, .. } => Some(*payment_hash),
+               }
+       }
+
        fn mark_fulfilled(&mut self) {
                let mut session_privs = HashSet::new();
                core::mem::swap(&mut session_privs, match self {
                        PendingOutboundPayment::Legacy { session_privs } |
                        PendingOutboundPayment::Retryable { session_privs, .. } |
-                       PendingOutboundPayment::Fulfilled { session_privs }
-                               => session_privs
+                       PendingOutboundPayment::Fulfilled { session_privs, .. } |
+                       PendingOutboundPayment::Abandoned { session_privs, .. }
+                               => session_privs,
+               });
+               let payment_hash = self.payment_hash();
+               *self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash };
+       }
+
+       fn mark_abandoned(&mut self) -> Result<(), ()> {
+               let mut session_privs = HashSet::new();
+               let our_payment_hash;
+               core::mem::swap(&mut session_privs, match self {
+                       PendingOutboundPayment::Legacy { .. } |
+                       PendingOutboundPayment::Fulfilled { .. } =>
+                               return Err(()),
+                       PendingOutboundPayment::Retryable { session_privs, payment_hash, .. } |
+                       PendingOutboundPayment::Abandoned { session_privs, payment_hash, .. } => {
+                               our_payment_hash = *payment_hash;
+                               session_privs
+                       },
                });
-               *self = PendingOutboundPayment::Fulfilled { session_privs };
+               *self = PendingOutboundPayment::Abandoned { session_privs, payment_hash: our_payment_hash };
+               Ok(())
        }
 
        /// panics if path is None and !self.is_fulfilled
@@ -488,7 +537,8 @@ impl PendingOutboundPayment {
                let remove_res = match self {
                        PendingOutboundPayment::Legacy { session_privs } |
                        PendingOutboundPayment::Retryable { session_privs, .. } |
-                       PendingOutboundPayment::Fulfilled { session_privs } => {
+                       PendingOutboundPayment::Fulfilled { session_privs, .. } |
+                       PendingOutboundPayment::Abandoned { session_privs, .. } => {
                                session_privs.remove(session_priv)
                        }
                };
@@ -511,7 +561,8 @@ impl PendingOutboundPayment {
                        PendingOutboundPayment::Retryable { session_privs, .. } => {
                                session_privs.insert(session_priv)
                        }
-                       PendingOutboundPayment::Fulfilled { .. } => false
+                       PendingOutboundPayment::Fulfilled { .. } => false,
+                       PendingOutboundPayment::Abandoned { .. } => false,
                };
                if insert_res {
                        if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
@@ -529,7 +580,8 @@ impl PendingOutboundPayment {
                match self {
                        PendingOutboundPayment::Legacy { session_privs } |
                        PendingOutboundPayment::Retryable { session_privs, .. } |
-                       PendingOutboundPayment::Fulfilled { session_privs } => {
+                       PendingOutboundPayment::Fulfilled { session_privs, .. } |
+                       PendingOutboundPayment::Abandoned { session_privs, .. } => {
                                session_privs.len()
                        }
                }
@@ -786,6 +838,10 @@ const CHECK_CLTV_EXPIRY_SANITY: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRA
 #[allow(dead_code)]
 const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER;
 
+/// The number of blocks before we consider an outbound payment for expiry if it doesn't have any
+/// pending HTLCs in flight.
+pub(crate) const PAYMENT_EXPIRY_BLOCKS: u32 = 3;
+
 /// Information needed for constructing an invoice route hint for this channel.
 #[derive(Clone, Debug, PartialEq)]
 pub struct CounterpartyForwardingInfo {
@@ -855,17 +911,30 @@ pub struct ChannelDetails {
        pub unspendable_punishment_reserve: Option<u64>,
        /// The `user_channel_id` passed in to create_channel, or 0 if the channel was inbound.
        pub user_channel_id: u64,
+       /// Our total balance.  This is the amount we would get if we close the channel.
+       /// This value is not exact. Due to various in-flight changes and feerate changes, exactly this
+       /// amount is not likely to be recoverable on close.
+       ///
+       /// This does not include any pending HTLCs which are not yet fully resolved (and, thus, whose
+       /// balance is not available for inclusion in new outbound HTLCs). This further does not include
+       /// any pending outgoing HTLCs which are awaiting some other resolution to be sent.
+       /// This does not consider any on-chain fees.
+       ///
+       /// See also [`ChannelDetails::outbound_capacity_msat`]
+       pub balance_msat: u64,
        /// The available outbound capacity for sending HTLCs to the remote peer. This does not include
-       /// any pending HTLCs which are not yet fully resolved (and, thus, who's balance is not
+       /// any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not
        /// available for inclusion in new outbound HTLCs). This further does not include any pending
        /// outgoing HTLCs which are awaiting some other resolution to be sent.
        ///
+       /// See also [`ChannelDetails::balance_msat`]
+       ///
        /// This value is not exact. Due to various in-flight changes, feerate changes, and our
        /// conflict-avoidance policy, exactly this amount is not likely to be spendable. However, we
        /// should be able to spend nearly this amount.
        pub outbound_capacity_msat: u64,
        /// The available inbound capacity for the remote peer to send HTLCs to us. This does not
-       /// include any pending HTLCs which are not yet fully resolved (and, thus, who's balance is not
+       /// include any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not
        /// available for inclusion in new inbound HTLCs).
        /// Note that there are some corner cases not fully handled here, so the actual available
        /// inbound capacity may be slightly higher than this.
@@ -1436,6 +1505,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        res.reserve(channel_state.by_id.len());
                        for (channel_id, channel) in channel_state.by_id.iter().filter(f) {
                                let (inbound_capacity_msat, outbound_capacity_msat) = channel.get_inbound_outbound_available_balance_msat();
+                               let balance_msat = channel.get_balance_msat();
                                let (to_remote_reserve_satoshis, to_self_reserve_satoshis) =
                                        channel.get_holder_counterparty_selected_channel_reserve_satoshis();
                                res.push(ChannelDetails {
@@ -1450,6 +1520,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        short_channel_id: channel.get_short_channel_id(),
                                        channel_value_satoshis: channel.get_value_satoshis(),
                                        unspendable_punishment_reserve: to_self_reserve_satoshis,
+                                       balance_msat,
                                        inbound_capacity_msat,
                                        outbound_capacity_msat,
                                        user_channel_id: channel.get_user_id(),
@@ -2300,10 +2371,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        ///
        /// Errors returned are a superset of those returned from [`send_payment`], so see
        /// [`send_payment`] documentation for more details on errors. This method will also error if the
-       /// retry amount puts the payment more than 10% over the payment's total amount, or if the payment
-       /// for the given `payment_id` cannot be found (likely due to timeout or success).
+       /// retry amount puts the payment more than 10% over the payment's total amount, if the payment
+       /// for the given `payment_id` cannot be found (likely due to timeout or success), or if
+       /// further retries have been disabled with [`abandon_payment`].
        ///
        /// [`send_payment`]: [`ChannelManager::send_payment`]
+       /// [`abandon_payment`]: [`ChannelManager::abandon_payment`]
        pub fn retry_payment(&self, route: &Route, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
                const RETRY_OVERFLOW_PERCENTAGE: u64 = 10;
                for path in route.paths.iter() {
@@ -2335,8 +2408,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                }))
                                        },
                                        PendingOutboundPayment::Fulfilled { .. } => {
-                                               return Err(PaymentSendFailure::ParameterError(APIError::RouteError {
-                                                       err: "Payment already completed"
+                                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+                                                       err: "Payment already completed".to_owned()
+                                               }));
+                                       },
+                                       PendingOutboundPayment::Abandoned { .. } => {
+                                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+                                                       err: "Payment already abandoned (with some HTLCs still pending)".to_owned()
                                                }));
                                        },
                                }
@@ -2349,6 +2427,37 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                return self.send_payment_internal(route, payment_hash, &payment_secret, None, Some(payment_id), Some(total_msat)).map(|_| ())
        }
 
+       /// Signals that no further retries for the given payment will occur.
+       ///
+       /// After this method returns, any future calls to [`retry_payment`] for the given `payment_id`
+       /// will fail with [`PaymentSendFailure::ParameterError`]. If no such event has been generated,
+       /// an [`Event::PaymentFailed`] event will be generated as soon as there are no remaining
+       /// pending HTLCs for this payment.
+       ///
+       /// Note that calling this method does *not* prevent a payment from succeeding. You must still
+       /// wait until you receive either a [`Event::PaymentFailed`] or [`Event::PaymentSent`] event to
+       /// determine the ultimate status of a payment.
+       ///
+       /// [`retry_payment`]: Self::retry_payment
+       /// [`Event::PaymentFailed`]: events::Event::PaymentFailed
+       /// [`Event::PaymentSent`]: events::Event::PaymentSent
+       pub fn abandon_payment(&self, payment_id: PaymentId) {
+               let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
+
+               let mut outbounds = self.pending_outbound_payments.lock().unwrap();
+               if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
+                       if let Ok(()) = payment.get_mut().mark_abandoned() {
+                               if payment.get().remaining_parts() == 0 {
+                                       self.pending_events.lock().unwrap().push(events::Event::PaymentFailed {
+                                               payment_id,
+                                               payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
+                                       });
+                                       payment.remove();
+                               }
+                       }
+               }
+       }
+
        /// Send a spontaneous payment, which is a payment that does not require the recipient to have
        /// generated an invoice. Optionally, you may specify the preimage. If you do choose to specify
        /// the preimage, it must be a cryptographically secure random value that no intermediate node
@@ -2430,7 +2539,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// Returns an [`APIError::APIMisuseError`] if the funding_transaction spent non-SegWit outputs
        /// or if no output was found which matches the parameters in [`Event::FundingGenerationReady`].
        ///
-       /// Panics if a funding transaction has already been provided for this channel.
+       /// Returns [`APIError::ChannelUnavailable`] if a funding transaction has already been provided
+       /// for the channel or if the channel has been closed as indicated by [`Event::ChannelClosed`].
        ///
        /// May panic if the output found in the funding transaction is duplicative with some other
        /// channel (note that this should be trivially prevented by using unique funding transaction
@@ -2445,6 +2555,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// create a new channel with a conflicting funding transaction.
        ///
        /// [`Event::FundingGenerationReady`]: crate::util::events::Event::FundingGenerationReady
+       /// [`Event::ChannelClosed`]: crate::util::events::Event::ChannelClosed
        pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_transaction: Transaction) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
@@ -2648,7 +2759,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                                htlc_id: prev_htlc_id,
                                                                                incoming_packet_shared_secret: incoming_shared_secret,
                                                                        });
-                                                                       match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet) {
+                                                                       match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet, &self.logger) {
                                                                                Err(e) => {
                                                                                        if let ChannelError::Ignore(msg) = e {
                                                                                                log_trace!(self.logger, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg);
@@ -2887,7 +2998,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                                                        purpose: events::PaymentPurpose::InvoicePayment {
                                                                                                                payment_preimage: inbound_payment.get().payment_preimage,
                                                                                                                payment_secret: payment_data.payment_secret,
-                                                                                                               user_payment_id: inbound_payment.get().user_payment_id,
                                                                                                        },
                                                                                                        amt: total_value,
                                                                                                });
@@ -3175,22 +3285,28 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                        final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
                                                                })
                                                        } else { None };
-                                                       self.pending_events.lock().unwrap().push(
-                                                               events::Event::PaymentPathFailed {
-                                                                       payment_id: Some(payment_id),
-                                                                       payment_hash,
-                                                                       rejected_by_dest: false,
-                                                                       network_update: None,
-                                                                       all_paths_failed: payment.get().remaining_parts() == 0,
-                                                                       path: path.clone(),
-                                                                       short_channel_id: None,
-                                                                       retry,
-                                                                       #[cfg(test)]
-                                                                       error_code: None,
-                                                                       #[cfg(test)]
-                                                                       error_data: None,
-                                                               }
-                                                       );
+                                                       let mut pending_events = self.pending_events.lock().unwrap();
+                                                       pending_events.push(events::Event::PaymentPathFailed {
+                                                               payment_id: Some(payment_id),
+                                                               payment_hash,
+                                                               rejected_by_dest: false,
+                                                               network_update: None,
+                                                               all_paths_failed: payment.get().remaining_parts() == 0,
+                                                               path: path.clone(),
+                                                               short_channel_id: None,
+                                                               retry,
+                                                               #[cfg(test)]
+                                                               error_code: None,
+                                                               #[cfg(test)]
+                                                               error_data: None,
+                                                       });
+                                                       if payment.get().abandoned() && payment.get().remaining_parts() == 0 {
+                                                               pending_events.push(events::Event::PaymentFailed {
+                                                                       payment_id,
+                                                                       payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
+                                                               });
+                                                               payment.remove();
+                                                       }
                                                }
                                        } else {
                                                log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -3221,6 +3337,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                session_priv_bytes.copy_from_slice(&session_priv[..]);
                                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
                                let mut all_paths_failed = false;
+                               let mut full_failure_ev = None;
                                if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
                                        if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
                                                log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -3232,6 +3349,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        }
                                        if payment.get().remaining_parts() == 0 {
                                                all_paths_failed = true;
+                                               if payment.get().abandoned() {
+                                                       full_failure_ev = Some(events::Event::PaymentFailed {
+                                                               payment_id,
+                                                               payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
+                                                       });
+                                                       payment.remove();
+                                               }
                                        }
                                } else {
                                        log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -3247,7 +3371,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        })
                                } else { None };
                                log_trace!(self.logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));
-                               match &onion_error {
+
+                               let path_failure = match &onion_error {
                                        &HTLCFailReason::LightningError { ref err } => {
 #[cfg(test)]
                                                let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
@@ -3256,22 +3381,20 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // TODO: If we decided to blame ourselves (or one of our channels) in
                                                // process_onion_failure we should close that channel as it implies our
                                                // next-hop is needlessly blaming us!
-                                               self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentPathFailed {
-                                                               payment_id: Some(payment_id),
-                                                               payment_hash: payment_hash.clone(),
-                                                               rejected_by_dest: !payment_retryable,
-                                                               network_update,
-                                                               all_paths_failed,
-                                                               path: path.clone(),
-                                                               short_channel_id,
-                                                               retry,
+                                               events::Event::PaymentPathFailed {
+                                                       payment_id: Some(payment_id),
+                                                       payment_hash: payment_hash.clone(),
+                                                       rejected_by_dest: !payment_retryable,
+                                                       network_update,
+                                                       all_paths_failed,
+                                                       path: path.clone(),
+                                                       short_channel_id,
+                                                       retry,
 #[cfg(test)]
-                                                               error_code: onion_error_code,
+                                                       error_code: onion_error_code,
 #[cfg(test)]
-                                                               error_data: onion_error_data
-                                                       }
-                                               );
+                                                       error_data: onion_error_data
+                                               }
                                        },
                                        &HTLCFailReason::Reason {
 #[cfg(test)]
@@ -3286,24 +3409,25 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // ChannelDetails.
                                                // TODO: For non-temporary failures, we really should be closing the
                                                // channel here as we apparently can't relay through them anyway.
-                                               self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentPathFailed {
-                                                               payment_id: Some(payment_id),
-                                                               payment_hash: payment_hash.clone(),
-                                                               rejected_by_dest: path.len() == 1,
-                                                               network_update: None,
-                                                               all_paths_failed,
-                                                               path: path.clone(),
-                                                               short_channel_id: Some(path.first().unwrap().short_channel_id),
-                                                               retry,
+                                               events::Event::PaymentPathFailed {
+                                                       payment_id: Some(payment_id),
+                                                       payment_hash: payment_hash.clone(),
+                                                       rejected_by_dest: path.len() == 1,
+                                                       network_update: None,
+                                                       all_paths_failed,
+                                                       path: path.clone(),
+                                                       short_channel_id: Some(path.first().unwrap().short_channel_id),
+                                                       retry,
 #[cfg(test)]
-                                                               error_code: Some(*failure_code),
+                                                       error_code: Some(*failure_code),
 #[cfg(test)]
-                                                               error_data: Some(data.clone()),
-                                                       }
-                                               );
+                                                       error_data: Some(data.clone()),
+                                               }
                                        }
-                               }
+                               };
+                               let mut pending_events = self.pending_events.lock().unwrap();
+                               pending_events.push(path_failure);
+                               if let Some(ev) = full_failure_ev { pending_events.push(ev); }
                        },
                        HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret, .. }) => {
                                let err_packet = match onion_error {
@@ -3341,19 +3465,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
-       /// Provides a payment preimage in response to a PaymentReceived event, returning true and
-       /// 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!
+       /// Provides a payment preimage in response to [`Event::PaymentReceived`], generating any
+       /// [`MessageSendEvent`]s needed to claim the payment.
        ///
        /// 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.
+       /// Returns whether any HTLCs were claimed, and thus if any new [`MessageSendEvent`]s are now
+       /// pending for processing via [`get_and_clear_pending_msg_events`].
        ///
+       /// [`Event::PaymentReceived`]: crate::util::events::Event::PaymentReceived
        /// [`create_inbound_payment`]: Self::create_inbound_payment
        /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
+       /// [`get_and_clear_pending_msg_events`]: MessageSendEventsProvider::get_and_clear_pending_msg_events
        pub fn claim_funds(&self, payment_preimage: PaymentPreimage) -> bool {
                let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 
@@ -3491,14 +3617,23 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        }
 
        fn finalize_claims(&self, mut sources: Vec<HTLCSource>) {
+               let mut pending_events = self.pending_events.lock().unwrap();
                for source in sources.drain(..) {
-                       if let HTLCSource::OutboundRoute { session_priv, payment_id, .. } = source {
+                       if let HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } = source {
                                let mut session_priv_bytes = [0; 32];
                                session_priv_bytes.copy_from_slice(&session_priv[..]);
                                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
                                if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
                                        assert!(payment.get().is_fulfilled());
-                                       payment.get_mut().remove(&session_priv_bytes, None);
+                                       if payment.get_mut().remove(&session_priv_bytes, None) {
+                                               pending_events.push(
+                                                       events::Event::PaymentPathSuccessful {
+                                                               payment_id,
+                                                               payment_hash: payment.get().payment_hash(),
+                                                               path,
+                                                       }
+                                               );
+                                       }
                                        if payment.get().remaining_parts() == 0 {
                                                payment.remove();
                                        }
@@ -3515,9 +3650,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                session_priv_bytes.copy_from_slice(&session_priv[..]);
                                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
                                if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
-                                       let found_payment = !payment.get().is_fulfilled();
-                                       let fee_paid_msat = payment.get().get_pending_fee_msat();
-                                       payment.get_mut().mark_fulfilled();
+                                       let mut pending_events = self.pending_events.lock().unwrap();
+                                       if !payment.get().is_fulfilled() {
+                                               let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
+                                               let fee_paid_msat = payment.get().get_pending_fee_msat();
+                                               pending_events.push(
+                                                       events::Event::PaymentSent {
+                                                               payment_id: Some(payment_id),
+                                                               payment_preimage,
+                                                               payment_hash,
+                                                               fee_paid_msat,
+                                                       }
+                                               );
+                                               payment.get_mut().mark_fulfilled();
+                                       }
+
                                        if from_onchain {
                                                // We currently immediately remove HTLCs which were fulfilled on-chain.
                                                // This could potentially lead to removing a pending payment too early,
@@ -3525,22 +3672,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // restart.
                                                // TODO: We should have a second monitor event that informs us of payments
                                                // irrevocably fulfilled.
-                                               payment.get_mut().remove(&session_priv_bytes, Some(&path));
+                                               if payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
+                                                       let payment_hash = Some(PaymentHash(Sha256::hash(&payment_preimage.0).into_inner()));
+                                                       pending_events.push(
+                                                               events::Event::PaymentPathSuccessful {
+                                                                       payment_id,
+                                                                       payment_hash,
+                                                                       path,
+                                                               }
+                                                       );
+                                               }
+
                                                if payment.get().remaining_parts() == 0 {
                                                        payment.remove();
                                                }
                                        }
-                                       if found_payment {
-                                               let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
-                                               self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentSent {
-                                                               payment_id: Some(payment_id),
-                                                               payment_preimage,
-                                                               payment_hash: payment_hash,
-                                                               fee_paid_msat,
-                                                       }
-                                               );
-                                       }
                                } else {
                                        log_trace!(self.logger, "Received duplicative fulfill for HTLC with payment_preimage {}", log_bytes!(payment_preimage.0));
                                }
@@ -3646,8 +3792,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(), msg.temporary_channel_id.clone()));
                }
 
+               if !self.default_configuration.accept_inbound_channels {
+                       return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone()));
+               }
+
                let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(),
-                               &their_features, msg, 0, &self.default_configuration, self.best_block.read().unwrap().height())
+                               &their_features, msg, 0, &self.default_configuration, self.best_block.read().unwrap().height(), &self.logger)
                        .map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?;
                let mut channel_state_lock = self.channel_state.lock().unwrap();
                let channel_state = &mut *channel_state_lock;
@@ -4519,7 +4669,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
-       fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option<PaymentPreimage>, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> Result<PaymentSecret, APIError> {
+       fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option<PaymentPreimage>, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
                assert!(invoice_expiry_delta_secs <= 60*60*24*365); // Sadly bitcoin timestamps are u32s, so panic before 2106
 
                let payment_secret = PaymentSecret(self.keys_manager.get_secure_random_bytes());
@@ -4529,7 +4679,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                match payment_secrets.entry(payment_hash) {
                        hash_map::Entry::Vacant(e) => {
                                e.insert(PendingInboundPayment {
-                                       payment_secret, min_value_msat, user_payment_id, payment_preimage,
+                                       payment_secret, min_value_msat, payment_preimage,
+                                       user_payment_id: 0, // For compatibility with version 0.0.103 and earlier
                                        // We assume that highest_seen_timestamp is pretty close to the current time -
                                        // its updated when we receive a new block with the maximum time we've seen in
                                        // a header. It should never be more than two hours in the future.
@@ -4561,12 +4712,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// [`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) {
+       pub fn create_inbound_payment(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> (PaymentHash, PaymentSecret) {
                let payment_preimage = PaymentPreimage(self.keys_manager.get_secure_random_bytes());
                let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 
                (payment_hash,
-                       self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs, user_payment_id)
+                       self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs)
                                .expect("RNG Generated Duplicate PaymentHash"))
        }
 
@@ -4580,12 +4731,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// The [`PaymentHash`] (and corresponding [`PaymentPreimage`]) must be globally unique. This
        /// method may return an Err if another payment with the same payment_hash is still pending.
        ///
-       /// `user_payment_id` will be provided back in [`PaymentPurpose::InvoicePayment::user_payment_id`] events to
-       /// allow tracking of which events correspond with which calls to this and
-       /// [`create_inbound_payment`]. `user_payment_id` has no meaning inside of LDK, it is simply
-       /// copied to events and otherwise ignored. It may be used to correlate PaymentReceived events
-       /// with invoice metadata stored elsewhere.
-       ///
        /// `min_value_msat` should be set if the invoice being generated contains a value. Any payment
        /// received for the returned [`PaymentHash`] will be required to be at least `min_value_msat`
        /// before a [`PaymentReceived`] event will be generated, ensuring that we do not provide the
@@ -4614,9 +4759,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        ///
        /// [`create_inbound_payment`]: Self::create_inbound_payment
        /// [`PaymentReceived`]: events::Event::PaymentReceived
-       /// [`PaymentPurpose::InvoicePayment::user_payment_id`]: events::PaymentPurpose::InvoicePayment::user_payment_id
-       pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> Result<PaymentSecret, APIError> {
-               self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs, user_payment_id)
+       pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
+               self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs)
        }
 
        #[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
@@ -4631,6 +4775,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        pub fn has_pending_payments(&self) -> bool {
                !self.pending_outbound_payments.lock().unwrap().is_empty()
        }
+
+       #[cfg(test)]
+       pub fn clear_pending_payments(&self) {
+               self.pending_outbound_payments.lock().unwrap().clear()
+       }
 }
 
 impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> MessageSendEventsProvider for ChannelManager<Signer, M, T, K, F, L>
@@ -4807,14 +4956,19 @@ where
                        inbound_payment.expiry_time > header.time as u64
                });
 
+               let mut pending_events = self.pending_events.lock().unwrap();
                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-               outbounds.retain(|_, payment| {
-                       const PAYMENT_EXPIRY_BLOCKS: u32 = 3;
+               outbounds.retain(|payment_id, payment| {
                        if payment.remaining_parts() != 0 { return true }
-                       if let PendingOutboundPayment::Retryable { starting_block_height, .. } = payment {
-                               return *starting_block_height + PAYMENT_EXPIRY_BLOCKS > height
-                       }
-                       true
+                       if let PendingOutboundPayment::Retryable { starting_block_height, payment_hash, .. } = payment {
+                               if *starting_block_height + PAYMENT_EXPIRY_BLOCKS <= height {
+                                       log_info!(self.logger, "Timing out payment with id {} and hash {}", log_bytes!(payment_id.0), log_bytes!(payment_hash.0));
+                                       pending_events.push(events::Event::PaymentFailed {
+                                               payment_id: *payment_id, payment_hash: *payment_hash,
+                                       });
+                                       false
+                               } else { true }
+                       } else { true }
                });
        }
 
@@ -5555,6 +5709,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
        },
        (1, Fulfilled) => {
                (0, session_privs, required),
+               (1, payment_hash, option),
        },
        (2, Retryable) => {
                (0, session_privs, required),
@@ -5565,6 +5720,10 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
                (8, pending_amt_msat, required),
                (10, starting_block_height, required),
        },
+       (3, Abandoned) => {
+               (0, session_privs, required),
+               (2, payment_hash, required),
+       },
 );
 
 impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable for ChannelManager<Signer, M, T, K, F, L>
@@ -5658,7 +5817,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
                // For backwards compat, write the session privs and their total length.
                let mut num_pending_outbounds_compat: u64 = 0;
                for (_, outbound) in pending_outbound_payments.iter() {
-                       if !outbound.is_fulfilled() {
+                       if !outbound.is_fulfilled() && !outbound.abandoned() {
                                num_pending_outbounds_compat += outbound.remaining_parts() as u64;
                        }
                }
@@ -5672,6 +5831,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
                                        }
                                }
                                PendingOutboundPayment::Fulfilled { .. } => {},
+                               PendingOutboundPayment::Abandoned { .. } => {},
                        }
                }
 
@@ -5870,6 +6030,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                                reason: ClosureReason::OutdatedChannelManager
                                        });
                                } else {
+                                       log_info!(args.logger, "Successfully loaded channel {}", log_bytes!(channel.channel_id()));
                                        if let Some(short_channel_id) = channel.get_short_channel_id() {
                                                short_to_id.insert(short_channel_id, channel.channel_id());
                                        }
@@ -5887,6 +6048,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 
                for (ref funding_txo, ref mut monitor) in args.channel_monitors.iter_mut() {
                        if !funding_txo_set.contains(funding_txo) {
+                               log_info!(args.logger, "Broadcasting latest holder commitment transaction for closed channel {}", log_bytes!(funding_txo.to_channel_id()));
                                monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
                        }
                }
@@ -6321,9 +6483,10 @@ mod tests {
                nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa);
                check_added_monitors!(nodes[0], 1);
 
-               // Note that successful MPP payments will generate 1 event upon the first path's success. No
-               // further events will be generated for subsequence path successes.
+               // Note that successful MPP payments will generate a single PaymentSent event upon the first
+               // path's success and a PaymentPathSuccessful event for each path's success.
                let events = nodes[0].node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 3);
                match events[0] {
                        Event::PaymentSent { payment_id: ref id, payment_preimage: ref preimage, payment_hash: ref hash, .. } => {
                                assert_eq!(Some(payment_id), *id);
@@ -6332,6 +6495,22 @@ mod tests {
                        },
                        _ => panic!("Unexpected event"),
                }
+               match events[1] {
+                       Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
+                               assert_eq!(payment_id, *actual_payment_id);
+                               assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
+                               assert_eq!(route.paths[0], *path);
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+               match events[2] {
+                       Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
+                               assert_eq!(payment_id, *actual_payment_id);
+                               assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
+                               assert_eq!(route.paths[0], *path);
+                       },
+                       _ => panic!("Unexpected event"),
+               }
        }
 
        #[test]
@@ -6558,7 +6737,7 @@ pub mod bench {
        use ln::msgs::{ChannelMessageHandler, Init};
        use routing::network_graph::NetworkGraph;
        use routing::router::{Payee, get_route};
-       use routing::scorer::Scorer;
+       use routing::scoring::Scorer;
        use util::test_utils;
        use util::config::UserConfig;
        use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose};
@@ -6676,7 +6855,7 @@ pub mod bench {
                                payment_preimage.0[0..8].copy_from_slice(&payment_count.to_le_bytes());
                                payment_count += 1;
                                let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
-                               let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, 0).unwrap();
+                               let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200).unwrap();
 
                                $node_a.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
                                let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());