Merge pull request #1211 from ConorOkus/2021-11-add-max-conversion
[rust-lightning] / lightning / src / ln / channelmanager.rs
index 4eb67af2e51299d8ba2577b2d72a051576bdaa93..47d705fe625b4889d7d3da19681af0d4272b8807 100644 (file)
@@ -45,7 +45,7 @@ use chain::transaction::{OutPoint, TransactionData};
 use ln::{PaymentHash, PaymentPreimage, PaymentSecret};
 use ln::channel::{Channel, ChannelError, ChannelUpdateStatus, UpdateFulfillCommitFetch};
 use ln::features::{InitFeatures, NodeFeatures};
-use routing::router::{Payee, PaymentPathRetry, Route, RouteHop};
+use routing::router::{Payee, Route, RouteHop, RoutePath, RouteParameters};
 use ln::msgs;
 use ln::msgs::NetAddress;
 use ln::onion_utils;
@@ -173,6 +173,7 @@ struct ClaimableHTLC {
 }
 
 /// A payment identifier used to uniquely identify a payment to LDK.
+/// (C-not exported) as we just use [u8; 32] directly
 #[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)]
 pub struct PaymentId(pub [u8; 32]);
 
@@ -412,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,
@@ -436,6 +440,8 @@ pub(crate) enum PendingOutboundPayment {
                payment_hash: PaymentHash,
                payment_secret: Option<PaymentSecret>,
                pending_amt_msat: u64,
+               /// Used to track the fee paid. Only present if the payment was serialized on 0.0.103+.
+               pending_fee_msat: Option<u64>,
                /// The total payment amount across all paths, used to verify that a retry is not overpaying.
                total_msat: u64,
                /// Our best known block height at the time this payment was initiated.
@@ -446,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,
        },
 }
 
@@ -462,46 +480,97 @@ 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(),
+                       _ => None,
+               }
+       }
+
+       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,
                });
-               *self = PendingOutboundPayment::Fulfilled { session_privs };
+               let payment_hash = self.payment_hash();
+               *self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash };
        }
 
-       /// panics if part_amt_msat is None and !self.is_fulfilled
-       fn remove(&mut self, session_priv: &[u8; 32], part_amt_msat: Option<u64>) -> bool {
+       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::Abandoned { session_privs, payment_hash: our_payment_hash };
+               Ok(())
+       }
+
+       /// panics if path is None and !self.is_fulfilled
+       fn remove(&mut self, session_priv: &[u8; 32], path: Option<&Vec<RouteHop>>) -> bool {
                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)
                        }
                };
                if remove_res {
-                       if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, .. } = self {
-                               *pending_amt_msat -= part_amt_msat.expect("We must only not provide an amount if the payment was already fulfilled");
+                       if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
+                               let path = path.expect("Fulfilling a payment should always come with a path");
+                               let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
+                               *pending_amt_msat -= path_last_hop.fee_msat;
+                               if let Some(fee_msat) = pending_fee_msat.as_mut() {
+                                       *fee_msat -= path.get_path_fees();
+                               }
                        }
                }
                remove_res
        }
 
-       fn insert(&mut self, session_priv: [u8; 32], part_amt_msat: u64) -> bool {
+       fn insert(&mut self, session_priv: [u8; 32], path: &Vec<RouteHop>) -> bool {
                let insert_res = match self {
                        PendingOutboundPayment::Legacy { session_privs } |
                        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, .. } = self {
-                               *pending_amt_msat += part_amt_msat;
+                       if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
+                               let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
+                               *pending_amt_msat += path_last_hop.fee_msat;
+                               if let Some(fee_msat) = pending_fee_msat.as_mut() {
+                                       *fee_msat += path.get_path_fees();
+                               }
                        }
                }
                insert_res
@@ -511,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()
                        }
                }
@@ -768,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 {
@@ -837,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.
@@ -928,7 +1015,16 @@ pub enum PaymentSendFailure {
        /// as they will result in over-/re-payment. These HTLCs all either successfully sent (in the
        /// case of Ok(())) or will send once channel_monitor_updated is called on the next-hop channel
        /// with the latest update_id.
-       PartialFailure(Vec<Result<(), APIError>>),
+       PartialFailure {
+               /// The errors themselves, in the same order as the route hops.
+               results: Vec<Result<(), APIError>>,
+               /// If some paths failed without irrevocably committing to the new HTLC(s), this will
+               /// contain a [`RouteParameters`] object which can be used to calculate a new route that
+               /// will pay all remaining unpaid balance.
+               failed_paths_retry: Option<RouteParameters>,
+               /// The payment id for the payment, which is now at least partially pending.
+               payment_id: PaymentId,
+       },
 }
 
 macro_rules! handle_error {
@@ -1114,7 +1210,7 @@ macro_rules! handle_monitor_err {
                res
        } };
        ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr) => {
-               handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new());
+               handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new())
        }
 }
 
@@ -1371,7 +1467,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        let peer_state = peer_state.lock().unwrap();
                                        let their_features = &peer_state.latest_features;
                                        let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
-                                       Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_channel_id, config)?
+                                       Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features,
+                                               channel_value_satoshis, push_msat, user_channel_id, config, self.best_block.read().unwrap().height())?
                                },
                                None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
                        }
@@ -1408,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 {
@@ -1422,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(),
@@ -1932,17 +2031,24 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update_for_unicast(chan).unwrap())));
                                        }
                                        let cur_height = self.best_block.read().unwrap().height() + 1;
-                                       // Theoretically, channel counterparty shouldn't send us a HTLC expiring now, but we want to be robust wrt to counterparty
-                                       // packet sanitization (see HTLC_FAIL_BACK_BUFFER rational)
+                                       // Theoretically, channel counterparty shouldn't send us a HTLC expiring now,
+                                       // but we want to be robust wrt to counterparty packet sanitization (see
+                                       // HTLC_FAIL_BACK_BUFFER rationale).
                                        if msg.cltv_expiry <= cur_height + HTLC_FAIL_BACK_BUFFER as u32 { // expiry_too_soon
                                                break Some(("CLTV expiry is too close", 0x1000 | 14, Some(self.get_channel_update_for_unicast(chan).unwrap())));
                                        }
                                        if msg.cltv_expiry > cur_height + CLTV_FAR_FAR_AWAY as u32 { // expiry_too_far
                                                break Some(("CLTV expiry is too far in the future", 21, None));
                                        }
-                                       // In theory, we would be safe against unintentional channel-closure, if we only required a margin of LATENCY_GRACE_PERIOD_BLOCKS.
-                                       // But, to be safe against policy reception, we use a longer delay.
-                                       if (*outgoing_cltv_value) as u64 <= (cur_height + HTLC_FAIL_BACK_BUFFER) as u64 {
+                                       // If the HTLC expires ~now, don't bother trying to forward it to our
+                                       // counterparty. They should fail it anyway, but we don't want to bother with
+                                       // the round-trips or risk them deciding they definitely want the HTLC and
+                                       // force-closing to ensure they get it if we're offline.
+                                       // We previously had a much more aggressive check here which tried to ensure
+                                       // our counterparty receives an HTLC which has *our* risk threshold met on it,
+                                       // but there is no need to do that, and since we're a bit conservative with our
+                                       // risk threshold it just results in failing to forward payments.
+                                       if (*outgoing_cltv_value) as u64 <= (cur_height + LATENCY_GRACE_PERIOD_BLOCKS) as u64 {
                                                break Some(("Outgoing CLTV value is too soon", 0x1000 | 14, Some(self.get_channel_update_for_unicast(chan).unwrap())));
                                        }
 
@@ -2058,6 +2164,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                Some(id) => id.clone(),
                        };
 
+                       macro_rules! insert_outbound_payment {
+                               () => {
+                                       let payment = payment_entry.or_insert_with(|| PendingOutboundPayment::Retryable {
+                                               session_privs: HashSet::new(),
+                                               pending_amt_msat: 0,
+                                               pending_fee_msat: Some(0),
+                                               payment_hash: *payment_hash,
+                                               payment_secret: *payment_secret,
+                                               starting_block_height: self.best_block.read().unwrap().height(),
+                                               total_msat: total_value,
+                                       });
+                                       assert!(payment.insert(session_priv_bytes, path));
+                               }
+                       }
+
                        let channel_state = &mut *channel_lock;
                        if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) {
                                match {
@@ -2067,7 +2188,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        if !chan.get().is_live() {
                                                return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!".to_owned()});
                                        }
-                                       let send_res = break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(
+                                       break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(
                                                htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute {
                                                        path: path.clone(),
                                                        session_priv: session_priv.clone(),
@@ -2076,19 +2197,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                        payment_secret: payment_secret.clone(),
                                                        payee: payee.clone(),
                                                }, onion_packet, &self.logger),
-                                       channel_state, chan);
-
-                                       let payment = payment_entry.or_insert_with(|| PendingOutboundPayment::Retryable {
-                                               session_privs: HashSet::new(),
-                                               pending_amt_msat: 0,
-                                               payment_hash: *payment_hash,
-                                               payment_secret: *payment_secret,
-                                               starting_block_height: self.best_block.read().unwrap().height(),
-                                               total_msat: total_value,
-                                       });
-                                       assert!(payment.insert(session_priv_bytes, path.last().unwrap().fee_msat));
-
-                                       send_res
+                                       channel_state, chan)
                                } {
                                        Some((update_add, commitment_signed, monitor_update)) => {
                                                if let Err(e) = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
@@ -2098,8 +2207,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                        // is restored. Therefore, we must return an error indicating that
                                                        // it is unsafe to retry the payment wholesale, which we do in the
                                                        // send_payment check for MonitorUpdateFailed, below.
+                                                       insert_outbound_payment!(); // Only do this after possibly break'ing on Perm failure above.
                                                        return Err(APIError::MonitorUpdateFailed);
                                                }
+                                               insert_outbound_payment!();
 
                                                log_debug!(self.logger, "Sending payment along path resulted in a commitment_signed for channel {}", log_bytes!(chan.get().channel_id()));
                                                channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
@@ -2114,7 +2225,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                        },
                                                });
                                        },
-                                       None => {},
+                                       None => { insert_outbound_payment!(); },
                                }
                        } else { unreachable!(); }
                        return Ok(());
@@ -2217,7 +2328,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
                let mut has_ok = false;
                let mut has_err = false;
-               for res in results.iter() {
+               let mut pending_amt_unsent = 0;
+               let mut max_unsent_cltv_delta = 0;
+               for (res, path) in results.iter().zip(route.paths.iter()) {
                        if res.is_ok() { has_ok = true; }
                        if res.is_err() { has_err = true; }
                        if let &Err(APIError::MonitorUpdateFailed) = res {
@@ -2225,12 +2338,29 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                // PartialFailure.
                                has_err = true;
                                has_ok = true;
-                               break;
+                       } else if res.is_err() {
+                               pending_amt_unsent += path.last().unwrap().fee_msat;
+                               max_unsent_cltv_delta = cmp::max(max_unsent_cltv_delta, path.last().unwrap().cltv_expiry_delta);
                        }
                }
                if has_err && has_ok {
-                       Err(PaymentSendFailure::PartialFailure(results))
+                       Err(PaymentSendFailure::PartialFailure {
+                               results,
+                               payment_id,
+                               failed_paths_retry: if pending_amt_unsent != 0 {
+                                       if let Some(payee) = &route.payee {
+                                               Some(RouteParameters {
+                                                       payee: payee.clone(),
+                                                       final_value_msat: pending_amt_unsent,
+                                                       final_cltv_expiry_delta: max_unsent_cltv_delta,
+                                               })
+                                       } else { None }
+                               } else { None },
+                       })
                } else if has_err {
+                       // If we failed to send any paths, we shouldn't have inserted the new PaymentId into
+                       // our `pending_outbound_payments` map at all.
+                       debug_assert!(self.pending_outbound_payments.lock().unwrap().get(&payment_id).is_none());
                        Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
                } else {
                        Ok(payment_id)
@@ -2241,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() {
@@ -2276,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()
                                                }));
                                        },
                                }
@@ -2290,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
@@ -2371,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
@@ -2386,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);
 
@@ -2589,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);
@@ -2828,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,
                                                                                                });
@@ -3107,32 +3276,37 @@ 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 path_last_hop = path.last().expect("Outbound payments must have had a valid path");
-                                               if payment.get_mut().remove(&session_priv_bytes, Some(path_last_hop.fee_msat)) &&
-                                                       !payment.get().is_fulfilled()
-                                               {
+                                               if payment.get_mut().remove(&session_priv_bytes, Some(&path)) && !payment.get().is_fulfilled() {
                                                        let retry = if let Some(payee_data) = payee {
-                                                               Some(PaymentPathRetry {
+                                                               let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
+                                                               Some(RouteParameters {
                                                                        payee: payee_data,
                                                                        final_value_msat: path_last_hop.fee_msat,
                                                                        final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
                                                                })
                                                        } else { None };
-                                                       self.pending_events.lock().unwrap().push(
-                                                               events::Event::PaymentPathFailed {
-                                                                       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));
@@ -3163,9 +3337,9 @@ 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 path_last_hop = path.last().expect("Outbound payments must have had a valid path");
+                               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_last_hop.fee_msat)) {
+                                       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));
                                                return;
                                        }
@@ -3175,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));
@@ -3182,14 +3363,16 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                }
                                mem::drop(channel_state_lock);
                                let retry = if let Some(payee_data) = payee {
-                                       Some(PaymentPathRetry {
+                                       let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
+                                       Some(RouteParameters {
                                                payee: payee_data.clone(),
                                                final_value_msat: path_last_hop.fee_msat,
                                                final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
                                        })
                                } 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());
@@ -3198,21 +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_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)]
@@ -3227,23 +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_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 {
@@ -3281,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());
 
@@ -3431,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();
                                        }
@@ -3454,9 +3649,22 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                let mut session_priv_bytes = [0; 32];
                                session_priv_bytes.copy_from_slice(&session_priv[..]);
                                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-                               let found_payment = if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
-                                       let found_payment = !payment.get().is_fulfilled();
-                                       payment.get_mut().mark_fulfilled();
+                               if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
+                                       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,
@@ -3464,21 +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.last().unwrap().fee_msat));
+                                               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();
                                                }
                                        }
-                                       found_payment
-                               } else { false };
-                               if found_payment {
-                                       let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
-                                       self.pending_events.lock().unwrap().push(
-                                               events::Event::PaymentSent {
-                                                       payment_preimage,
-                                                       payment_hash: payment_hash
-                                               }
-                                       );
                                } else {
                                        log_trace!(self.logger, "Received duplicative fulfill for HTLC with payment_preimage {}", log_bytes!(payment_preimage.0));
                                }
@@ -3584,7 +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()));
                }
 
-               let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(), &their_features, msg, 0, &self.default_configuration)
+               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(), &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;
@@ -4456,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());
@@ -4466,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.
@@ -4498,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"))
        }
 
@@ -4517,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
@@ -4551,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"))]
@@ -4568,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>
@@ -4744,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 }
                });
        }
 
@@ -4789,7 +5006,7 @@ where
        /// Calls a function which handles an on-chain event (blocks dis/connected, transactions
        /// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by
        /// the function.
-       fn do_chain_event<FN: Fn(&mut Channel<Signer>) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage>>
+       fn do_chain_event<FN: Fn(&mut Channel<Signer>) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), ClosureReason>>
                        (&self, height_opt: Option<u32>, f: FN) {
                // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
                // during initialization prior to the chain_monitor being fully configured in some cases.
@@ -4834,7 +5051,7 @@ where
                                                }
                                                short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
                                        }
-                               } else if let Err(e) = res {
+                               } else if let Err(reason) = res {
                                        if let Some(short_id) = channel.get_short_channel_id() {
                                                short_to_id.remove(&short_id);
                                        }
@@ -4846,10 +5063,14 @@ where
                                                        msg: update
                                                });
                                        }
-                                       self.issue_channel_close_events(channel, ClosureReason::CommitmentTxConfirmed);
+                                       let reason_message = format!("{}", reason);
+                                       self.issue_channel_close_events(channel, reason);
                                        pending_msg_events.push(events::MessageSendEvent::HandleError {
                                                node_id: channel.get_counterparty_node_id(),
-                                               action: msgs::ErrorAction::SendErrorMessage { msg: e },
+                                               action: msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage {
+                                                       channel_id: channel.channel_id(),
+                                                       data: reason_message,
+                                               } },
                                        });
                                        return false;
                                }
@@ -5488,15 +5709,21 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
        },
        (1, Fulfilled) => {
                (0, session_privs, required),
+               (1, payment_hash, option),
        },
        (2, Retryable) => {
                (0, session_privs, required),
+               (1, pending_fee_msat, option),
                (2, payment_hash, required),
                (4, payment_secret, option),
                (6, total_msat, required),
                (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>
@@ -5590,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;
                        }
                }
@@ -5604,6 +5831,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
                                        }
                                }
                                PendingOutboundPayment::Fulfilled { .. } => {},
+                               PendingOutboundPayment::Abandoned { .. } => {},
                        }
                }
 
@@ -5767,7 +5995,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                let mut short_to_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
                let mut channel_closures = Vec::new();
                for _ in 0..channel_count {
-                       let mut channel: Channel<Signer> = Channel::read(reader, &args.keys_manager)?;
+                       let mut channel: Channel<Signer> = Channel::read(reader, (&args.keys_manager, best_block_height))?;
                        let funding_txo = channel.get_funding_txo().ok_or(DecodeError::InvalidValue)?;
                        funding_txo_set.insert(funding_txo.clone());
                        if let Some(ref mut monitor) = args.channel_monitors.get_mut(&funding_txo) {
@@ -5802,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());
                                        }
@@ -5819,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);
                        }
                }
@@ -5947,16 +6177,18 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                                        session_priv_bytes[..].copy_from_slice(&session_priv[..]);
                                                        match pending_outbound_payments.as_mut().unwrap().entry(payment_id) {
                                                                hash_map::Entry::Occupied(mut entry) => {
-                                                                       let newly_added = entry.get_mut().insert(session_priv_bytes, path_amt);
+                                                                       let newly_added = entry.get_mut().insert(session_priv_bytes, &path);
                                                                        log_info!(args.logger, "{} a pending payment path for {} msat for session priv {} on an existing pending payment with payment hash {}",
                                                                                if newly_added { "Added" } else { "Had" }, path_amt, log_bytes!(session_priv_bytes), log_bytes!(htlc.payment_hash.0));
                                                                },
                                                                hash_map::Entry::Vacant(entry) => {
+                                                                       let path_fee = path.get_path_fees();
                                                                        entry.insert(PendingOutboundPayment::Retryable {
                                                                                session_privs: [session_priv_bytes].iter().map(|a| *a).collect(),
                                                                                payment_hash: htlc.payment_hash,
                                                                                payment_secret,
                                                                                pending_amt_msat: path_amt,
+                                                                               pending_fee_msat: Some(path_fee),
                                                                                total_msat: path_amt,
                                                                                starting_block_height: best_block_height,
                                                                        });
@@ -6032,12 +6264,11 @@ mod tests {
        use core::time::Duration;
        use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
        use ln::channelmanager::{PaymentId, PaymentSendFailure};
-       use ln::features::{InitFeatures, InvoiceFeatures};
+       use ln::features::InitFeatures;
        use ln::functional_test_utils::*;
        use ln::msgs;
        use ln::msgs::ChannelMessageHandler;
-       use routing::router::{Payee, get_keysend_route, get_route};
-       use routing::scorer::Scorer;
+       use routing::router::{Payee, RouteParameters, find_route};
        use util::errors::APIError;
        use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
        use util::test_utils;
@@ -6252,16 +6483,34 @@ 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_preimage: ref preimage, payment_hash: ref hash } => {
+                       Event::PaymentSent { payment_id: ref id, payment_preimage: ref preimage, payment_hash: ref hash, .. } => {
+                               assert_eq!(Some(payment_id), *id);
                                assert_eq!(payment_preimage, *preimage);
                                assert_eq!(our_payment_hash, *hash);
                        },
                        _ => 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]
@@ -6275,17 +6524,22 @@ mod tests {
                let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
-               let logger = test_utils::TestLogger::new();
-               let scorer = Scorer::new(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // To start (1), send a regular payment but don't claim it.
                let expected_route = [&nodes[1]];
                let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &expected_route, 100_000);
 
                // Next, attempt a keysend payment and make sure it fails.
-               let payee = Payee::new(expected_route.last().unwrap().node.get_our_node_id())
-                       .with_features(InvoiceFeatures::known());
-               let route = get_route(&nodes[0].node.get_our_node_id(), &payee, &nodes[0].net_graph_msg_handler.network_graph, None, 100_000, TEST_FINAL_CLTV, &logger, &scorer).unwrap();
+               let params = RouteParameters {
+                       payee: Payee::for_keysend(expected_route.last().unwrap().node.get_our_node_id()),
+                       final_value_msat: 100_000,
+                       final_cltv_expiry_delta: TEST_FINAL_CLTV,
+               };
+               let route = find_route(
+                       &nodes[0].node.get_our_node_id(), &params, nodes[0].network_graph, None,
+                       nodes[0].logger, &scorer
+               ).unwrap();
                nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -6313,7 +6567,10 @@ mod tests {
 
                // To start (2), send a keysend payment but don't claim it.
                let payment_preimage = PaymentPreimage([42; 32]);
-               let route = get_route(&nodes[0].node.get_our_node_id(), &payee, &nodes[0].net_graph_msg_handler.network_graph, None, 100_000, TEST_FINAL_CLTV, &logger, &scorer).unwrap();
+               let route = find_route(
+                       &nodes[0].node.get_our_node_id(), &params, nodes[0].network_graph, None,
+                       nodes[0].logger, &scorer
+               ).unwrap();
                let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -6365,12 +6622,18 @@ mod tests {
                nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known() });
 
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known());
-               let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
+               let params = RouteParameters {
+                       payee: Payee::for_keysend(payee_pubkey),
+                       final_value_msat: 10000,
+                       final_cltv_expiry_delta: 40,
+               };
+               let network_graph = nodes[0].network_graph;
                let first_hops = nodes[0].node.list_usable_channels();
-               let scorer = Scorer::new(0);
-               let route = get_keysend_route(&payer_pubkey, network_graph, &payee_pubkey,
-                                  Some(&first_hops.iter().collect::<Vec<_>>()), &vec![], 10000, 40,
-                                  nodes[0].logger, &scorer).unwrap();
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let route = find_route(
+                       &payer_pubkey, &params, network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
+                       nodes[0].logger, &scorer
+               ).unwrap();
 
                let test_preimage = PaymentPreimage([42; 32]);
                let mismatch_payment_hash = PaymentHash([43; 32]);
@@ -6402,12 +6665,18 @@ mod tests {
                nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known() });
 
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known());
-               let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
+               let params = RouteParameters {
+                       payee: Payee::for_keysend(payee_pubkey),
+                       final_value_msat: 10000,
+                       final_cltv_expiry_delta: 40,
+               };
+               let network_graph = nodes[0].network_graph;
                let first_hops = nodes[0].node.list_usable_channels();
-               let scorer = Scorer::new(0);
-               let route = get_keysend_route(&payer_pubkey, network_graph, &payee_pubkey,
-                                  Some(&first_hops.iter().collect::<Vec<_>>()), &vec![], 10000, 40,
-                                  nodes[0].logger, &scorer).unwrap();
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let route = find_route(
+                       &payer_pubkey, &params, network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
+                       nodes[0].logger, &scorer
+               ).unwrap();
 
                let test_preimage = PaymentPreimage([42; 32]);
                let test_secret = PaymentSecret([43; 32]);
@@ -6468,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};
@@ -6576,9 +6845,9 @@ pub mod bench {
                macro_rules! send_payment {
                        ($node_a: expr, $node_b: expr) => {
                                let usable_channels = $node_a.list_usable_channels();
-                               let payee = Payee::new($node_b.get_our_node_id())
+                               let payee = Payee::from_node_id($node_b.get_our_node_id())
                                        .with_features(InvoiceFeatures::known());
-                               let scorer = Scorer::new(0);
+                               let scorer = Scorer::with_fixed_penalty(0);
                                let route = get_route(&$node_a.get_our_node_id(), &payee, &dummy_graph,
                                        Some(&usable_channels.iter().map(|r| r).collect::<Vec<_>>()), 10_000, TEST_FINAL_CLTV, &logger_a, &scorer).unwrap();
 
@@ -6586,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());