Add WithoutLength wrapper
[rust-lightning] / lightning / src / ln / channelmanager.rs
index f234ad9f91f555075804d87db115d98193f09619..2f0b0e1564a8091f3a37ff490c9353e940a8e8c7 100644 (file)
@@ -34,39 +34,39 @@ use bitcoin::secp256k1::Secp256k1;
 use bitcoin::secp256k1::ecdh::SharedSecret;
 use bitcoin::{LockTime, secp256k1, Sequence};
 
-use chain;
-use chain::{Confirm, ChannelMonitorUpdateStatus, Watch, BestBlock};
-use chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator};
-use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
-use chain::transaction::{OutPoint, TransactionData};
+use crate::chain;
+use crate::chain::{Confirm, ChannelMonitorUpdateStatus, Watch, BestBlock};
+use crate::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator};
+use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
+use crate::chain::transaction::{OutPoint, TransactionData};
 // Since this struct is returned in `list_channels` methods, expose it here in case users want to
 // construct one themselves.
-use ln::{inbound_payment, PaymentHash, PaymentPreimage, PaymentSecret};
-use ln::channel::{Channel, ChannelError, ChannelUpdateStatus, UpdateFulfillCommitFetch};
-use ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
+use crate::ln::{inbound_payment, PaymentHash, PaymentPreimage, PaymentSecret};
+use crate::ln::channel::{Channel, ChannelError, ChannelUpdateStatus, UpdateFulfillCommitFetch};
+use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
 #[cfg(any(feature = "_test_utils", test))]
-use ln::features::InvoiceFeatures;
-use routing::router::{PaymentParameters, Route, RouteHop, RoutePath, RouteParameters};
-use ln::msgs;
-use ln::onion_utils;
-use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
-use ln::wire::Encode;
-use chain::keysinterface::{Sign, KeysInterface, KeysManager, InMemorySigner, Recipient};
-use util::config::{UserConfig, ChannelConfig};
-use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination};
-use util::{byte_utils, events};
-use util::wakers::{Future, Notifier};
-use util::scid_utils::fake_scid;
-use util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
-use util::logger::{Level, Logger};
-use util::errors::APIError;
-
-use io;
-use prelude::*;
+use crate::ln::features::InvoiceFeatures;
+use crate::routing::router::{PaymentParameters, Route, RouteHop, RoutePath, RouteParameters};
+use crate::ln::msgs;
+use crate::ln::onion_utils;
+use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
+use crate::ln::wire::Encode;
+use crate::chain::keysinterface::{Sign, KeysInterface, KeysManager, Recipient};
+use crate::util::config::{UserConfig, ChannelConfig};
+use crate::util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination};
+use crate::util::{byte_utils, events};
+use crate::util::wakers::{Future, Notifier};
+use crate::util::scid_utils::fake_scid;
+use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
+use crate::util::logger::{Level, Logger};
+use crate::util::errors::APIError;
+
+use crate::io;
+use crate::prelude::*;
 use core::{cmp, mem};
 use core::cell::RefCell;
-use io::Read;
-use sync::{Arc, Mutex, MutexGuard, RwLock, RwLockReadGuard};
+use crate::io::Read;
+use crate::sync::{Arc, Mutex, MutexGuard, RwLock, RwLockReadGuard};
 use core::sync::atomic::{AtomicUsize, Ordering};
 use core::time::Duration;
 use core::ops::Deref;
@@ -473,6 +473,7 @@ pub(crate) enum PendingOutboundPayment {
        Fulfilled {
                session_privs: HashSet<[u8; 32]>,
                payment_hash: Option<PaymentHash>,
+               timer_ticks_without_htlcs: u8,
        },
        /// 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
@@ -488,12 +489,6 @@ pub(crate) enum PendingOutboundPayment {
 }
 
 impl PendingOutboundPayment {
-       fn is_retryable(&self) -> bool {
-               match self {
-                       PendingOutboundPayment::Retryable { .. } => true,
-                       _ => false,
-               }
-       }
        fn is_fulfilled(&self) -> bool {
                match self {
                        PendingOutboundPayment::Fulfilled { .. } => true,
@@ -532,7 +527,7 @@ impl PendingOutboundPayment {
                                => session_privs,
                });
                let payment_hash = self.payment_hash();
-               *self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash };
+               *self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash, timer_ticks_without_htlcs: 0 };
        }
 
        fn mark_abandoned(&mut self) -> Result<(), ()> {
@@ -617,7 +612,7 @@ impl PendingOutboundPayment {
 /// concrete type of the KeysManager.
 ///
 /// (C-not exported) as Arcs don't make sense in bindings
-pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<InMemorySigner, Arc<M>, Arc<T>, Arc<KeysManager>, Arc<F>, Arc<L>>;
+pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<Arc<M>, Arc<T>, Arc<KeysManager>, Arc<F>, Arc<L>>;
 
 /// SimpleRefChannelManager is a type alias for a ChannelManager reference, and is the reference
 /// counterpart to the SimpleArcChannelManager type alias. Use this type by default when you don't
@@ -629,7 +624,7 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<InMemorySigner, Ar
 /// concrete type of the KeysManager.
 ///
 /// (C-not exported) as Arcs don't make sense in bindings
-pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, M, T, F, L> = ChannelManager<InMemorySigner, &'a M, &'b T, &'c KeysManager, &'d F, &'e L>;
+pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, M, T, F, L> = ChannelManager<&'a M, &'b T, &'c KeysManager, &'d F, &'e L>;
 
 /// Manager which keeps track of a number of channels and sends messages to the appropriate
 /// channel, also tracking HTLC preimages and forwarding onion packets appropriately.
@@ -699,10 +694,10 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, M, T, F, L> = ChannelManage
 //  |                   |
 //  |                   |__`pending_background_events`
 //
-pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
-       where M::Target: chain::Watch<Signer>,
+pub struct ChannelManager<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
+       where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
         T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
+        K::Target: KeysInterface,
         F::Target: FeeEstimator,
                                L::Target: Logger,
 {
@@ -721,9 +716,9 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
 
        /// See `ChannelManager` struct-level documentation for lock order requirements.
        #[cfg(any(test, feature = "_test_utils"))]
-       pub(super) channel_state: Mutex<ChannelHolder<Signer>>,
+       pub(super) channel_state: Mutex<ChannelHolder<<K::Target as KeysInterface>::Signer>>,
        #[cfg(not(any(test, feature = "_test_utils")))]
-       channel_state: Mutex<ChannelHolder<Signer>>,
+       channel_state: Mutex<ChannelHolder<<K::Target as KeysInterface>::Signer>>,
 
        /// Storage for PaymentSecrets and any requirements on future inbound payments before we will
        /// expose them to users via a PaymentReceived event. HTLCs which do not meet the requirements
@@ -959,13 +954,14 @@ 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;
-
 /// The number of ticks of [`ChannelManager::timer_tick_occurred`] until expiry of incomplete MPPs
 pub(crate) const MPP_TIMEOUT_TICKS: u8 = 3;
 
+/// The number of ticks of [`ChannelManager::timer_tick_occurred`] until we time-out the
+/// idempotency of payments by [`PaymentId`]. See
+/// [`ChannelManager::remove_stale_resolved_payments`].
+pub(crate) const IDEMPOTENCY_TIMEOUT_TICKS: u8 = 7;
+
 /// Information needed for constructing an invoice route hint for this channel.
 #[derive(Clone, Debug, PartialEq)]
 pub struct CounterpartyForwardingInfo {
@@ -1206,6 +1202,9 @@ pub enum PaymentSendFailure {
        /// All paths which were attempted failed to send, with no channel state change taking place.
        /// You can freely retry the payment in full (though you probably want to do so over different
        /// paths than the ones selected).
+       ///
+       /// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
+       /// [`ChannelManager::retry_payment`] will *not* work for this payment.
        AllFailedRetrySafe(Vec<APIError>),
        /// Some paths which were attempted failed to send, though possibly not all. At least some
        /// paths have irrevocably committed to the HTLC and retrying the payment in full would result
@@ -1466,6 +1465,23 @@ macro_rules! send_channel_ready {
        }
 }
 
+macro_rules! emit_channel_ready_event {
+       ($self: expr, $channel: expr) => {
+               if $channel.should_emit_channel_ready_event() {
+                       {
+                               let mut pending_events = $self.pending_events.lock().unwrap();
+                               pending_events.push(events::Event::ChannelReady {
+                                       channel_id: $channel.channel_id(),
+                                       user_channel_id: $channel.get_user_id(),
+                                       counterparty_node_id: $channel.get_counterparty_node_id(),
+                                       channel_type: $channel.get_channel_type().clone(),
+                               });
+                       }
+                       $channel.set_channel_ready_event_emitted();
+               }
+       }
+}
+
 macro_rules! handle_chan_restoration_locked {
        ($self: ident, $channel_lock: expr, $channel_state: expr, $channel_entry: expr,
         $raa: expr, $commitment_update: expr, $order: expr, $chanmon_update: expr,
@@ -1509,6 +1525,8 @@ macro_rules! handle_chan_restoration_locked {
                                });
                        }
 
+                       emit_channel_ready_event!($self, $channel_entry.get_mut());
+
                        let funding_broadcastable: Option<Transaction> = $funding_broadcastable; // Force type-checking to resolve
                        if let Some(monitor_update) = chanmon_update {
                                // We only ever broadcast a funding transaction in response to a funding_signed
@@ -1592,10 +1610,10 @@ macro_rules! post_handle_chan_restoration {
        } }
 }
 
-impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<Signer, M, T, K, F, L>
-       where M::Target: chain::Watch<Signer>,
+impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F, L>
+       where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
         T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
+        K::Target: KeysInterface,
         F::Target: FeeEstimator,
         L::Target: Logger,
 {
@@ -1761,7 +1779,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                Ok(temporary_channel_id)
        }
 
-       fn list_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<Signer>)) -> bool>(&self, f: Fn) -> Vec<ChannelDetails> {
+       fn list_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<<K::Target as KeysInterface>::Signer>)) -> bool>(&self, f: Fn) -> Vec<ChannelDetails> {
                let mut res = Vec::new();
                {
                        let channel_state = self.channel_state.lock().unwrap();
@@ -1843,7 +1861,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        }
 
        /// Helper function that issues the channel close events
-       fn issue_channel_close_events(&self, channel: &Channel<Signer>, closure_reason: ClosureReason) {
+       fn issue_channel_close_events(&self, channel: &Channel<<K::Target as KeysInterface>::Signer>, closure_reason: ClosureReason) {
                let mut pending_events_lock = self.pending_events.lock().unwrap();
                match channel.unbroadcasted_funding() {
                        Some(transaction) => {
@@ -1971,7 +1989,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                log_debug!(self.logger, "Finishing force-closure of channel with {} HTLCs to fail", failed_htlcs.len());
                for htlc_source in failed_htlcs.drain(..) {
                        let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
-                       let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id: channel_id };
+                       let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id };
                        self.fail_htlc_backwards_internal(source, &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }, receiver);
                }
                if let Some((funding_txo, monitor_update)) = monitor_update_option {
@@ -2379,7 +2397,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// [`MessageSendEvent::BroadcastChannelUpdate`] event.
        ///
        /// May be called with channel_state already locked!
-       fn get_channel_update_for_broadcast(&self, chan: &Channel<Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
+       fn get_channel_update_for_broadcast(&self, chan: &Channel<<K::Target as KeysInterface>::Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
                if !chan.should_announce() {
                        return Err(LightningError {
                                err: "Cannot broadcast a channel_update for a private channel".to_owned(),
@@ -2398,7 +2416,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// and thus MUST NOT be called unless the recipient of the resulting message has already
        /// provided evidence that they know about the existence of the channel.
        /// May be called with channel_state already locked!
-       fn get_channel_update_for_unicast(&self, chan: &Channel<Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
+       fn get_channel_update_for_unicast(&self, chan: &Channel<<K::Target as KeysInterface>::Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
                log_trace!(self.logger, "Attempting to generate channel update for channel {}", log_bytes!(chan.channel_id()));
                let short_channel_id = match chan.get_short_channel_id().or(chan.latest_inbound_scid_alias()) {
                        None => return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError}),
@@ -2407,7 +2425,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                self.get_channel_update_for_onion(short_channel_id, chan)
        }
-       fn get_channel_update_for_onion(&self, short_channel_id: u64, chan: &Channel<Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
+       fn get_channel_update_for_onion(&self, short_channel_id: u64, chan: &Channel<<K::Target as KeysInterface>::Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
                log_trace!(self.logger, "Generating channel update for channel {}", log_bytes!(chan.channel_id()));
                let were_node_one = PublicKey::from_secret_key(&self.secp_ctx, &self.our_network_key).serialize()[..] < chan.get_counterparty_node_id().serialize()[..];
 
@@ -2434,10 +2452,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        }
 
        // Only public for testing, this should otherwise never be called direcly
-       pub(crate) fn send_payment_along_path(&self, path: &Vec<RouteHop>, payment_params: &Option<PaymentParameters>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>) -> Result<(), APIError> {
+       pub(crate) fn send_payment_along_path(&self, path: &Vec<RouteHop>, payment_params: &Option<PaymentParameters>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>, session_priv_bytes: [u8; 32]) -> Result<(), APIError> {
                log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.first().unwrap().short_channel_id);
                let prng_seed = self.keys_manager.get_secure_random_bytes();
-               let session_priv_bytes = self.keys_manager.get_secure_random_bytes();
                let session_priv = SecretKey::from_slice(&session_priv_bytes[..]).expect("RNG is busted");
 
                let onion_keys = onion_utils::construct_onion_keys(&self.secp_ctx, &path, &session_priv)
@@ -2453,36 +2470,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                let err: Result<(), _> = loop {
                        let mut channel_lock = self.channel_state.lock().unwrap();
 
-                       let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
-                       let payment_entry = pending_outbounds.entry(payment_id);
-                       if let hash_map::Entry::Occupied(payment) = &payment_entry {
-                               if !payment.get().is_retryable() {
-                                       return Err(APIError::RouteError {
-                                               err: "Payment already completed"
-                                       });
-                               }
-                       }
-
                        let id = match channel_lock.short_to_chan_info.get(&path.first().unwrap().short_channel_id) {
                                None => return Err(APIError::ChannelUnavailable{err: "No channel available with first hop!".to_owned()}),
                                Some((_cp_id, chan_id)) => chan_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 {
@@ -2511,9 +2503,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                RAACommitmentOrder::CommitmentFirst, false, true))
                                                {
                                                        (ChannelMonitorUpdateStatus::PermanentFailure, Err(e)) => break Err(e),
-                                                       (ChannelMonitorUpdateStatus::Completed, Ok(())) => {
-                                                               insert_outbound_payment!();
-                                                       },
+                                                       (ChannelMonitorUpdateStatus::Completed, Ok(())) => {},
                                                        (ChannelMonitorUpdateStatus::InProgress, Err(_)) => {
                                                                // Note that MonitorUpdateInProgress here indicates (per function
                                                                // docs) that we will resend the commitment update once monitor
@@ -2521,7 +2511,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                // indicating that it is unsafe to retry the payment wholesale,
                                                                // which we do in the send_payment check for
                                                                // MonitorUpdateInProgress, below.
-                                                               insert_outbound_payment!(); // Only do this after possibly break'ing on Perm failure above.
                                                                return Err(APIError::MonitorUpdateInProgress);
                                                        },
                                                        _ => unreachable!(),
@@ -2540,7 +2529,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                        },
                                                });
                                        },
-                                       None => { insert_outbound_payment!(); },
+                                       None => { },
                                }
                        } else { unreachable!(); }
                        return Ok(());
@@ -2559,14 +2548,20 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// Value parameters are provided via the last hop in route, see documentation for RouteHop
        /// fields for more info.
        ///
-       /// Note that if the payment_hash already exists elsewhere (eg you're sending a duplicative
-       /// payment), we don't do anything to stop you! We always try to ensure that if the provided
-       /// next hop knows the preimage to payment_hash they can claim an additional amount as
-       /// specified in the last hop in the route! Thus, you should probably do your own
-       /// payment_preimage tracking (which you should already be doing as they represent "proof of
-       /// payment") and prevent double-sends yourself.
+       /// If a pending payment is currently in-flight with the same [`PaymentId`] provided, this
+       /// method will error with an [`APIError::RouteError`]. Note, however, that once a payment
+       /// is no longer pending (either via [`ChannelManager::abandon_payment`], or handling of an
+       /// [`Event::PaymentSent`]) LDK will not stop you from sending a second payment with the same
+       /// [`PaymentId`].
        ///
-       /// May generate SendHTLCs message(s) event on success, which should be relayed.
+       /// Thus, in order to ensure duplicate payments are not sent, you should implement your own
+       /// tracking of payments, including state to indicate once a payment has completed. Because you
+       /// should also ensure that [`PaymentHash`]es are not re-used, for simplicity, you should
+       /// consider using the [`PaymentHash`] as the key for tracking payments. In that case, the
+       /// [`PaymentId`] should be a copy of the [`PaymentHash`] bytes.
+       ///
+       /// May generate SendHTLCs message(s) event on success, which should be relayed (e.g. via
+       /// [`PeerManager::process_events`]).
        ///
        /// Each path may have a different return value, and PaymentSendValue may return a Vec with
        /// each entry matching the corresponding-index entry in the route paths, see
@@ -2590,14 +2585,55 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// newer nodes, it will be provided to you in the invoice. If you do not have one, the Route
        /// must not contain multiple paths as multi-path payments require a recipient-provided
        /// payment_secret.
+       ///
        /// If a payment_secret *is* provided, we assume that the invoice had the payment_secret feature
        /// bit set (either as required or as available). If multiple paths are present in the Route,
        /// we assume the invoice had the basic_mpp feature set.
-       pub fn send_payment(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>) -> Result<PaymentId, PaymentSendFailure> {
-               self.send_payment_internal(route, payment_hash, payment_secret, None, None, None)
+       ///
+       /// [`Event::PaymentSent`]: events::Event::PaymentSent
+       /// [`PeerManager::process_events`]: crate::ln::peer_handler::PeerManager::process_events
+       pub fn send_payment(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
+               let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route)?;
+               self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs)
        }
 
-       fn send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, keysend_preimage: Option<PaymentPreimage>, payment_id: Option<PaymentId>, recv_value_msat: Option<u64>) -> Result<PaymentId, PaymentSendFailure> {
+       #[cfg(test)]
+       pub(crate) fn test_add_new_pending_payment(&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId, route: &Route) -> Result<Vec<[u8; 32]>, PaymentSendFailure> {
+               self.add_new_pending_payment(payment_hash, payment_secret, payment_id, route)
+       }
+
+       fn add_new_pending_payment(&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId, route: &Route) -> Result<Vec<[u8; 32]>, PaymentSendFailure> {
+               let mut onion_session_privs = Vec::with_capacity(route.paths.len());
+               for _ in 0..route.paths.len() {
+                       onion_session_privs.push(self.keys_manager.get_secure_random_bytes());
+               }
+
+               let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
+               match pending_outbounds.entry(payment_id) {
+                       hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
+                               err: "Payment already in progress"
+                       })),
+                       hash_map::Entry::Vacant(entry) => {
+                               let payment = entry.insert(PendingOutboundPayment::Retryable {
+                                       session_privs: HashSet::new(),
+                                       pending_amt_msat: 0,
+                                       pending_fee_msat: Some(0),
+                                       payment_hash,
+                                       payment_secret,
+                                       starting_block_height: self.best_block.read().unwrap().height(),
+                                       total_msat: route.get_total_amount(),
+                               });
+
+                               for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) {
+                                       assert!(payment.insert(*session_priv_bytes, path));
+                               }
+
+                               Ok(onion_session_privs)
+                       },
+               }
+       }
+
+       fn send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, keysend_preimage: Option<PaymentPreimage>, payment_id: PaymentId, recv_value_msat: Option<u64>, onion_session_privs: Vec<[u8; 32]>) -> Result<(), PaymentSendFailure> {
                if route.paths.len() < 1 {
                        return Err(PaymentSendFailure::ParameterError(APIError::RouteError{err: "There must be at least one path to send over"}));
                }
@@ -2607,7 +2643,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                let mut total_value = 0;
                let our_node_id = self.get_our_node_id();
                let mut path_errs = Vec::with_capacity(route.paths.len());
-               let payment_id = if let Some(id) = payment_id { id } else { PaymentId(self.keys_manager.get_secure_random_bytes()) };
                'path_check: for path in route.paths.iter() {
                        if path.len() < 1 || path.len() > 20 {
                                path_errs.push(Err(APIError::RouteError{err: "Path didn't go anywhere/had bogus size"}));
@@ -2632,8 +2667,28 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                let cur_height = self.best_block.read().unwrap().height() + 1;
                let mut results = Vec::new();
-               for path in route.paths.iter() {
-                       results.push(self.send_payment_along_path(&path, &route.payment_params, &payment_hash, payment_secret, total_value, cur_height, payment_id, &keysend_preimage));
+               debug_assert_eq!(route.paths.len(), onion_session_privs.len());
+               for (path, session_priv) in route.paths.iter().zip(onion_session_privs.into_iter()) {
+                       let mut path_res = self.send_payment_along_path(&path, &route.payment_params, &payment_hash, payment_secret, total_value, cur_height, payment_id, &keysend_preimage, session_priv);
+                       match path_res {
+                               Ok(_) => {},
+                               Err(APIError::MonitorUpdateInProgress) => {
+                                       // While a MonitorUpdateInProgress is an Err(_), the payment is still
+                                       // considered "in flight" and we shouldn't remove it from the
+                                       // PendingOutboundPayment set.
+                               },
+                               Err(_) => {
+                                       let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
+                                       if let Some(payment) = pending_outbounds.get_mut(&payment_id) {
+                                               let removed = payment.remove(&session_priv, Some(path));
+                                               debug_assert!(removed, "This can't happen as the payment has an entry for this path added by callers");
+                                       } else {
+                                               debug_assert!(false, "This can't happen as the payment was added by callers");
+                                               path_res = Err(APIError::APIMisuseError { err: "Internal error: payment disappeared during processing. Please report this bug!".to_owned() });
+                                       }
+                               }
+                       }
+                       results.push(path_res);
                }
                let mut has_ok = false;
                let mut has_err = false;
@@ -2667,12 +2722,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                } 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());
+                       // If we failed to send any paths, we should remove the new PaymentId from the
+                       // `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
+                       let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
+                       debug_assert!(removed, "We should always have a pending payment to remove here");
                        Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
                } else {
-                       Ok(payment_id)
+                       Ok(())
                }
        }
 
@@ -2696,44 +2752,55 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        }
                }
 
+               let mut onion_session_privs = Vec::with_capacity(route.paths.len());
+               for _ in 0..route.paths.len() {
+                       onion_session_privs.push(self.keys_manager.get_secure_random_bytes());
+               }
+
                let (total_msat, payment_hash, payment_secret) = {
-                       let outbounds = self.pending_outbound_payments.lock().unwrap();
-                       if let Some(payment) = outbounds.get(&payment_id) {
-                               match payment {
-                                       PendingOutboundPayment::Retryable {
-                                               total_msat, payment_hash, payment_secret, pending_amt_msat, ..
-                                       } => {
-                                               let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum();
-                                               if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
+                       let mut outbounds = self.pending_outbound_payments.lock().unwrap();
+                       match outbounds.get_mut(&payment_id) {
+                               Some(payment) => {
+                                       let res = match payment {
+                                               PendingOutboundPayment::Retryable {
+                                                       total_msat, payment_hash, payment_secret, pending_amt_msat, ..
+                                               } => {
+                                                       let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum();
+                                                       if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
+                                                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+                                                                       err: format!("retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat).to_string()
+                                                               }))
+                                                       }
+                                                       (*total_msat, *payment_hash, *payment_secret)
+                                               },
+                                               PendingOutboundPayment::Legacy { .. } => {
                                                        return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
-                                                               err: format!("retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat).to_string()
+                                                               err: "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102".to_string()
                                                        }))
-                                               }
-                                               (*total_msat, *payment_hash, *payment_secret)
-                                       },
-                                       PendingOutboundPayment::Legacy { .. } => {
-                                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
-                                                       err: "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102".to_string()
-                                               }))
-                                       },
-                                       PendingOutboundPayment::Fulfilled { .. } => {
-                                               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()
-                                               }));
-                                       },
-                               }
-                       } else {
-                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
-                                       err: format!("Payment with ID {} not found", log_bytes!(payment_id.0)),
-                               }))
+                                               },
+                                               PendingOutboundPayment::Fulfilled { .. } => {
+                                                       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()
+                                                       }));
+                                               },
+                                       };
+                                       for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) {
+                                               assert!(payment.insert(*session_priv_bytes, path));
+                                       }
+                                       res
+                               },
+                               None =>
+                                       return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+                                               err: format!("Payment with ID {} not found", log_bytes!(payment_id.0)),
+                                       })),
                        }
                };
-               return self.send_payment_internal(route, payment_hash, &payment_secret, None, Some(payment_id), Some(total_msat)).map(|_| ())
+               self.send_payment_internal(route, payment_hash, &payment_secret, None, payment_id, Some(total_msat), onion_session_privs)
        }
 
        /// Signals that no further retries for the given payment will occur.
@@ -2773,7 +2840,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// would be able to guess -- otherwise, an intermediate node may claim the payment and it will
        /// never reach the recipient.
        ///
-       /// See [`send_payment`] documentation for more details on the return value of this function.
+       /// See [`send_payment`] documentation for more details on the return value of this function
+       /// and idempotency guarantees provided by the [`PaymentId`] key.
        ///
        /// Similar to regular payments, you MUST NOT reuse a `payment_preimage` value. See
        /// [`send_payment`] for more information about the risks of duplicate preimage usage.
@@ -2781,14 +2849,16 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// Note that `route` must have exactly one path.
        ///
        /// [`send_payment`]: Self::send_payment
-       pub fn send_spontaneous_payment(&self, route: &Route, payment_preimage: Option<PaymentPreimage>) -> Result<(PaymentHash, PaymentId), PaymentSendFailure> {
+       pub fn send_spontaneous_payment(&self, route: &Route, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId) -> Result<PaymentHash, PaymentSendFailure> {
                let preimage = match payment_preimage {
                        Some(p) => p,
                        None => PaymentPreimage(self.keys_manager.get_secure_random_bytes()),
                };
                let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
-               match self.send_payment_internal(route, payment_hash, &None, Some(preimage), None, None) {
-                       Ok(payment_id) => Ok((payment_hash, payment_id)),
+               let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route)?;
+
+               match self.send_payment_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs) {
+                       Ok(()) => Ok(payment_hash),
                        Err(e) => Err(e)
                }
        }
@@ -2808,9 +2878,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
 
                let route = Route { paths: vec![hops], payment_params: None };
+               let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route)?;
 
-               match self.send_payment_internal(&route, payment_hash, &None, None, Some(payment_id), None) {
-                       Ok(payment_id) => Ok((payment_hash, payment_id)),
+               match self.send_payment_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs) {
+                       Ok(()) => Ok((payment_hash, payment_id)),
                        Err(e) => Err(e)
                }
        }
@@ -2832,7 +2903,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        /// Handles the generation of a funding transaction, optionally (for tests) with a function
        /// which checks the correctness of the funding transaction given the associated channel.
-       fn funding_transaction_generated_intern<FundingOutput: Fn(&Channel<Signer>, &Transaction) -> Result<OutPoint, APIError>>(
+       fn funding_transaction_generated_intern<FundingOutput: Fn(&Channel<<K::Target as KeysInterface>::Signer>, &Transaction) -> Result<OutPoint, APIError>>(
                &self, temporary_channel_id: &[u8; 32], _counterparty_node_id: &PublicKey, funding_transaction: Transaction, find_funding_output: FundingOutput
        ) -> Result<(), APIError> {
                let (chan, msg) = {
@@ -3492,7 +3563,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                self.process_background_events();
        }
 
-       fn update_channel_fee(&self, short_to_chan_info: &mut HashMap<u64, (PublicKey, [u8; 32])>, pending_msg_events: &mut Vec<events::MessageSendEvent>, chan_id: &[u8; 32], chan: &mut Channel<Signer>, new_feerate: u32) -> (bool, NotifyOption, Result<(), MsgHandleErrInternal>) {
+       fn update_channel_fee(&self, short_to_chan_info: &mut HashMap<u64, (PublicKey, [u8; 32])>, pending_msg_events: &mut Vec<events::MessageSendEvent>, chan_id: &[u8; 32], chan: &mut Channel<<K::Target as KeysInterface>::Signer>, new_feerate: u32) -> (bool, NotifyOption, Result<(), MsgHandleErrInternal>) {
                if !chan.is_outbound() { return (true, NotifyOption::SkipPersist, Ok(())); }
                // If the feerate has decreased by less than half, don't bother
                if new_feerate <= chan.get_feerate() && new_feerate * 2 > chan.get_feerate() {
@@ -3578,6 +3649,45 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                });
        }
 
+       fn remove_stale_resolved_payments(&self) {
+               // If an outbound payment was completed, and no pending HTLCs remain, we should remove it
+               // from the map. However, if we did that immediately when the last payment HTLC is claimed,
+               // this could race the user making a duplicate send_payment call and our idempotency
+               // guarantees would be violated. Instead, we wait a few timer ticks to do the actual
+               // removal. This should be more than sufficient to ensure the idempotency of any
+               // `send_payment` calls that were made at the same time the `PaymentSent` event was being
+               // processed.
+               let mut pending_outbound_payments = self.pending_outbound_payments.lock().unwrap();
+               let pending_events = self.pending_events.lock().unwrap();
+               pending_outbound_payments.retain(|payment_id, payment| {
+                       if let PendingOutboundPayment::Fulfilled { session_privs, timer_ticks_without_htlcs, .. } = payment {
+                               let mut no_remaining_entries = session_privs.is_empty();
+                               if no_remaining_entries {
+                                       for ev in pending_events.iter() {
+                                               match ev {
+                                                       events::Event::PaymentSent { payment_id: Some(ev_payment_id), .. } |
+                                                       events::Event::PaymentPathSuccessful { payment_id: ev_payment_id, .. } |
+                                                       events::Event::PaymentPathFailed { payment_id: Some(ev_payment_id), .. } => {
+                                                               if payment_id == ev_payment_id {
+                                                                       no_remaining_entries = false;
+                                                                       break;
+                                                               }
+                                                       },
+                                                       _ => {},
+                                               }
+                                       }
+                               }
+                               if no_remaining_entries {
+                                       *timer_ticks_without_htlcs += 1;
+                                       *timer_ticks_without_htlcs <= IDEMPOTENCY_TIMEOUT_TICKS
+                               } else {
+                                       *timer_ticks_without_htlcs = 0;
+                                       true
+                               }
+                       } else { true }
+               });
+       }
+
        /// Performs actions which should happen on startup and roughly once per minute thereafter.
        ///
        /// This currently includes:
@@ -3681,6 +3791,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        for (err, counterparty_node_id) in handle_errors.drain(..) {
                                let _ = handle_error!(self, err, counterparty_node_id);
                        }
+
+                       self.remove_stale_resolved_payments();
+
                        should_persist
                });
        }
@@ -3723,7 +3836,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        ///
        /// This is for failures on the channel on which the HTLC was *received*, not failures
        /// forwarding
-       fn get_htlc_inbound_temp_fail_err_and_data(&self, desired_err_code: u16, chan: &Channel<Signer>) -> (u16, Vec<u8>) {
+       fn get_htlc_inbound_temp_fail_err_and_data(&self, desired_err_code: u16, chan: &Channel<<K::Target as KeysInterface>::Signer>) -> (u16, Vec<u8>) {
                // We can't be sure what SCID was used when relaying inbound towards us, so we have to
                // guess somewhat. If its a public channel, we figure best to just use the real SCID (as
                // we're not leaking that we have a channel with the counterparty), otherwise we try to use
@@ -3743,7 +3856,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        /// Gets an HTLC onion failure code and error data for an `UPDATE` error, given the error code
        /// that we want to return and a channel.
-       fn get_htlc_temp_fail_err_and_data(&self, desired_err_code: u16, scid: u64, chan: &Channel<Signer>) -> (u16, Vec<u8>) {
+       fn get_htlc_temp_fail_err_and_data(&self, desired_err_code: u16, scid: u64, chan: &Channel<<K::Target as KeysInterface>::Signer>) -> (u16, Vec<u8>) {
                debug_assert_eq!(desired_err_code & 0x1000, 0x1000);
                if let Ok(upd) = self.get_channel_update_for_onion(scid, chan) {
                        let mut enc = VecWriter(Vec::with_capacity(upd.serialized_length() + 6));
@@ -3862,7 +3975,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                }
                                                        } else {
                                                                events::Event::ProbeFailed {
-                                                                       payment_id: payment_id,
+                                                                       payment_id,
                                                                        payment_hash: payment_hash.clone(),
                                                                        path: path.clone(),
                                                                        short_channel_id,
@@ -3909,7 +4022,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                                                if self.payment_is_probe(payment_hash, &payment_id) {
                                                        events::Event::ProbeFailed {
-                                                               payment_id: payment_id,
+                                                               payment_id,
                                                                payment_hash: payment_hash.clone(),
                                                                path: path.clone(),
                                                                short_channel_id: Some(scid),
@@ -4113,7 +4226,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
-       fn claim_funds_from_hop(&self, channel_state_lock: &mut MutexGuard<ChannelHolder<Signer>>, prev_hop: HTLCPreviousHopData, payment_preimage: PaymentPreimage) -> ClaimFundsFromHop {
+       fn claim_funds_from_hop(&self, channel_state_lock: &mut MutexGuard<ChannelHolder<<K::Target as KeysInterface>::Signer>>, prev_hop: HTLCPreviousHopData, payment_preimage: PaymentPreimage) -> ClaimFundsFromHop {
                //TODO: Delay the claimed_funds relaying just like we do outbound relay!
                let channel_state = &mut **channel_state_lock;
                let chan_id = match channel_state.short_to_chan_info.get(&prev_hop.short_channel_id) {
@@ -4198,15 +4311,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                        }
                                                );
                                        }
-                                       if payment.get().remaining_parts() == 0 {
-                                               payment.remove();
-                                       }
                                }
                        }
                }
        }
 
-       fn claim_funds_internal(&self, mut channel_state_lock: MutexGuard<ChannelHolder<Signer>>, source: HTLCSource, payment_preimage: PaymentPreimage, forwarded_htlc_value_msat: Option<u64>, from_onchain: bool, next_channel_id: [u8; 32]) {
+       fn claim_funds_internal(&self, mut channel_state_lock: MutexGuard<ChannelHolder<<K::Target as KeysInterface>::Signer>>, source: HTLCSource, payment_preimage: PaymentPreimage, forwarded_htlc_value_msat: Option<u64>, from_onchain: bool, next_channel_id: [u8; 32]) {
                match source {
                        HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
                                mem::drop(channel_state_lock);
@@ -4246,10 +4356,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                }
                                                        );
                                                }
-
-                                               if payment.get().remaining_parts() == 0 {
-                                                       payment.remove();
-                                               }
                                        }
                                } else {
                                        log_trace!(self.logger, "Received duplicative fulfill for HTLC with payment_preimage {}", log_bytes!(payment_preimage.0));
@@ -4672,6 +4778,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                });
                                        }
                                }
+
+                               emit_channel_ready_event!(self, chan.get_mut());
+
                                Ok(())
                        },
                        hash_map::Entry::Vacant(_) => Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
@@ -4795,7 +4904,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
                                }
 
-                               let create_pending_htlc_status = |chan: &Channel<Signer>, pending_forward_info: PendingHTLCStatus, error_code: u16| {
+                               let create_pending_htlc_status = |chan: &Channel<<K::Target as KeysInterface>::Signer>, pending_forward_info: PendingHTLCStatus, error_code: u16| {
                                        // If the update_add is completely bogus, the call will Err and we will close,
                                        // but if we've sent a shutdown and they haven't acknowledged it yet, we just
                                        // want to reject the new HTLC and fail it backwards instead of forwarding.
@@ -5568,10 +5677,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        }
 }
 
-impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> MessageSendEventsProvider for ChannelManager<Signer, M, T, K, F, L>
-       where M::Target: chain::Watch<Signer>,
+impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> MessageSendEventsProvider for ChannelManager<M, T, K, F, L>
+       where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
         T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
+        K::Target: KeysInterface,
         F::Target: FeeEstimator,
                                L::Target: Logger,
 {
@@ -5607,11 +5716,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> MessageSend
        }
 }
 
-impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> EventsProvider for ChannelManager<Signer, M, T, K, F, L>
+impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> EventsProvider for ChannelManager<M, T, K, F, L>
 where
-       M::Target: chain::Watch<Signer>,
+       M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
        T::Target: BroadcasterInterface,
-       K::Target: KeysInterface<Signer = Signer>,
+       K::Target: KeysInterface,
        F::Target: FeeEstimator,
        L::Target: Logger,
 {
@@ -5643,11 +5752,11 @@ where
        }
 }
 
-impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> chain::Listen for ChannelManager<Signer, M, T, K, F, L>
+impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> chain::Listen for ChannelManager<M, T, K, F, L>
 where
-       M::Target: chain::Watch<Signer>,
+       M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
        T::Target: BroadcasterInterface,
-       K::Target: KeysInterface<Signer = Signer>,
+       K::Target: KeysInterface,
        F::Target: FeeEstimator,
        L::Target: Logger,
 {
@@ -5680,11 +5789,11 @@ where
        }
 }
 
-impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> chain::Confirm for ChannelManager<Signer, M, T, K, F, L>
+impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> chain::Confirm for ChannelManager<M, T, K, F, L>
 where
-       M::Target: chain::Watch<Signer>,
+       M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
        T::Target: BroadcasterInterface,
-       K::Target: KeysInterface<Signer = Signer>,
+       K::Target: KeysInterface,
        F::Target: FeeEstimator,
        L::Target: Logger,
 {
@@ -5742,21 +5851,6 @@ where
                payment_secrets.retain(|_, inbound_payment| {
                        inbound_payment.expiry_time > header.time as u64
                });
-
-               let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-               let mut pending_events = self.pending_events.lock().unwrap();
-               outbounds.retain(|payment_id, payment| {
-                       if payment.remaining_parts() != 0 { return 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 }
-               });
        }
 
        fn get_relevant_txids(&self) -> Vec<Txid> {
@@ -5782,18 +5876,18 @@ where
        }
 }
 
-impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<Signer, M, T, K, F, L>
+impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F, L>
 where
-       M::Target: chain::Watch<Signer>,
+       M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
        T::Target: BroadcasterInterface,
-       K::Target: KeysInterface<Signer = Signer>,
+       K::Target: KeysInterface,
        F::Target: FeeEstimator,
        L::Target: Logger,
 {
        /// 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::ChannelReady>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), ClosureReason>>
+       fn do_chain_event<FN: Fn(&mut Channel<<K::Target as KeysInterface>::Signer>) -> Result<(Option<msgs::ChannelReady>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), 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.
@@ -5829,6 +5923,9 @@ where
                                                        log_trace!(self.logger, "Sending channel_ready WITHOUT channel_update for {}", log_bytes!(channel.channel_id()));
                                                }
                                        }
+
+                                       emit_channel_ready_event!(self, channel);
+
                                        if let Some(announcement_sigs) = announcement_sigs {
                                                log_trace!(self.logger, "Sending announcement_signatures for channel {}", log_bytes!(channel.channel_id()));
                                                pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
@@ -5951,11 +6048,11 @@ where
        }
 }
 
-impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
-       ChannelMessageHandler for ChannelManager<Signer, M, T, K, F, L>
-       where M::Target: chain::Watch<Signer>,
+impl<M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
+       ChannelMessageHandler for ChannelManager<M, T, K, F, L>
+       where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
         T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
+        K::Target: KeysInterface,
         F::Target: FeeEstimator,
         L::Target: Logger,
 {
@@ -6428,7 +6525,7 @@ impl Writeable for ClaimableHTLC {
 
 impl Readable for ClaimableHTLC {
        fn read<R: Read>(reader: &mut R) -> Result<Self, DecodeError> {
-               let mut prev_hop = ::util::ser::OptionDeserWrapper(None);
+               let mut prev_hop = crate::util::ser::OptionDeserWrapper(None);
                let mut value = 0;
                let mut payment_data: Option<msgs::FinalOnionHopData> = None;
                let mut cltv_expiry = 0;
@@ -6478,7 +6575,7 @@ impl Readable for HTLCSource {
                let id: u8 = Readable::read(reader)?;
                match id {
                        0 => {
-                               let mut session_priv: ::util::ser::OptionDeserWrapper<SecretKey> = ::util::ser::OptionDeserWrapper(None);
+                               let mut session_priv: crate::util::ser::OptionDeserWrapper<SecretKey> = crate::util::ser::OptionDeserWrapper(None);
                                let mut first_hop_htlc_msat: u64 = 0;
                                let mut path = Some(Vec::new());
                                let mut payment_id = None;
@@ -6499,7 +6596,7 @@ impl Readable for HTLCSource {
                                }
                                Ok(HTLCSource::OutboundRoute {
                                        session_priv: session_priv.0.unwrap(),
-                                       first_hop_htlc_msat: first_hop_htlc_msat,
+                                       first_hop_htlc_msat,
                                        path: path.unwrap(),
                                        payment_id: payment_id.unwrap(),
                                        payment_secret,
@@ -6513,7 +6610,7 @@ impl Readable for HTLCSource {
 }
 
 impl Writeable for HTLCSource {
-       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::io::Error> {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), crate::io::Error> {
                match self {
                        HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, payment_secret, payment_params } => {
                                0u8.write(writer)?;
@@ -6523,7 +6620,7 @@ impl Writeable for HTLCSource {
                                        (1, payment_id_opt, option),
                                        (2, first_hop_htlc_msat, required),
                                        (3, payment_secret, option),
-                                       (4, path, vec_type),
+                                       (4, *path, vec_type),
                                        (5, payment_params, option),
                                 });
                        }
@@ -6574,6 +6671,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
        (1, Fulfilled) => {
                (0, session_privs, required),
                (1, payment_hash, option),
+               (3, timer_ticks_without_htlcs, (default_value, 0)),
        },
        (2, Retryable) => {
                (0, session_privs, required),
@@ -6590,10 +6688,10 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
        },
 );
 
-impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable for ChannelManager<Signer, M, T, K, F, L>
-       where M::Target: chain::Watch<Signer>,
+impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable for ChannelManager<M, T, K, F, L>
+       where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
         T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
+        K::Target: KeysInterface,
         F::Target: FeeEstimator,
         L::Target: Logger,
 {
@@ -6839,29 +6937,29 @@ impl<'a, Signer: 'a + Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 
 // Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the
 // SipmleArcChannelManager type:
-impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
-       ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (BlockHash, Arc<ChannelManager<Signer, M, T, K, F, L>>)
-       where M::Target: chain::Watch<Signer>,
+impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
+       ReadableArgs<ChannelManagerReadArgs<'a, <K::Target as KeysInterface>::Signer, M, T, K, F, L>> for (BlockHash, Arc<ChannelManager<M, T, K, F, L>>)
+       where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
         T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
+        K::Target: KeysInterface,
         F::Target: FeeEstimator,
         L::Target: Logger,
 {
-       fn read<R: io::Read>(reader: &mut R, args: ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>) -> Result<Self, DecodeError> {
-               let (blockhash, chan_manager) = <(BlockHash, ChannelManager<Signer, M, T, K, F, L>)>::read(reader, args)?;
+       fn read<R: io::Read>(reader: &mut R, args: ChannelManagerReadArgs<'a, <K::Target as KeysInterface>::Signer, M, T, K, F, L>) -> Result<Self, DecodeError> {
+               let (blockhash, chan_manager) = <(BlockHash, ChannelManager<M, T, K, F, L>)>::read(reader, args)?;
                Ok((blockhash, Arc::new(chan_manager)))
        }
 }
 
-impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
-       ReadableArgs<ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>> for (BlockHash, ChannelManager<Signer, M, T, K, F, L>)
-       where M::Target: chain::Watch<Signer>,
+impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
+       ReadableArgs<ChannelManagerReadArgs<'a, <K::Target as KeysInterface>::Signer, M, T, K, F, L>> for (BlockHash, ChannelManager<M, T, K, F, L>)
+       where M::Target: chain::Watch<<K::Target as KeysInterface>::Signer>,
         T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
+        K::Target: KeysInterface,
         F::Target: FeeEstimator,
         L::Target: Logger,
 {
-       fn read<R: io::Read>(reader: &mut R, mut args: ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>) -> Result<Self, DecodeError> {
+       fn read<R: io::Read>(reader: &mut R, mut args: ChannelManagerReadArgs<'a, <K::Target as KeysInterface>::Signer, M, T, K, F, L>) -> Result<Self, DecodeError> {
                let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                let genesis_hash: BlockHash = Readable::read(reader)?;
@@ -6877,7 +6975,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                let mut short_to_chan_info = 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, best_block_height))?;
+                       let mut channel: Channel<<K::Target as KeysInterface>::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) {
@@ -6921,6 +7019,16 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                        }
                                        by_id.insert(channel.channel_id(), channel);
                                }
+                       } else if channel.is_awaiting_initial_mon_persist() {
+                               // If we were persisted and shut down while the initial ChannelMonitor persistence
+                               // was in-progress, we never broadcasted the funding transaction and can still
+                               // safely discard the channel.
+                               let _ = channel.force_shutdown(false);
+                               channel_closures.push(events::Event::ChannelClosed {
+                                       channel_id: channel.channel_id(),
+                                       user_channel_id: channel.get_user_id(),
+                                       reason: ClosureReason::DisconnectedPeer,
+                               });
                        } else {
                                log_error!(args.logger, "Missing ChannelMonitor for channel {} needed by ChannelManager.", log_bytes!(channel.channel_id()));
                                log_error!(args.logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,");
@@ -7295,16 +7403,16 @@ mod tests {
        use bitcoin::hashes::sha256::Hash as Sha256;
        use core::time::Duration;
        use core::sync::atomic::Ordering;
-       use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
-       use ln::channelmanager::{self, inbound_payment, PaymentId, PaymentSendFailure};
-       use ln::functional_test_utils::*;
-       use ln::msgs;
-       use ln::msgs::ChannelMessageHandler;
-       use routing::router::{PaymentParameters, RouteParameters, find_route};
-       use util::errors::APIError;
-       use util::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
-       use util::test_utils;
-       use chain::keysinterface::KeysInterface;
+       use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
+       use crate::ln::channelmanager::{self, inbound_payment, PaymentId, PaymentSendFailure};
+       use crate::ln::functional_test_utils::*;
+       use crate::ln::msgs;
+       use crate::ln::msgs::ChannelMessageHandler;
+       use crate::routing::router::{PaymentParameters, RouteParameters, find_route};
+       use crate::util::errors::APIError;
+       use crate::util::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
+       use crate::util::test_utils;
+       use crate::chain::keysinterface::KeysInterface;
 
        #[test]
        fn test_notify_limits() {
@@ -7396,18 +7504,22 @@ mod tests {
 
                // First, send a partial MPP payment.
                let (route, our_payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], 100_000);
+               let mut mpp_route = route.clone();
+               mpp_route.paths.push(mpp_route.paths[0].clone());
+
                let payment_id = PaymentId([42; 32]);
                // Use the utility function send_payment_along_path to send the payment with MPP data which
                // indicates there are more HTLCs coming.
                let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match.
-               nodes[0].node.send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
+               let session_privs = nodes[0].node.add_new_pending_payment(our_payment_hash, Some(payment_secret), payment_id, &mpp_route).unwrap();
+               nodes[0].node.send_payment_along_path(&mpp_route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[0]).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
                pass_along_path(&nodes[0], &[&nodes[1]], 200_000, our_payment_hash, Some(payment_secret), events.drain(..).next().unwrap(), false, None);
 
                // Next, send a keysend payment with the same payment_hash and make sure it fails.
-               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
+               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), PaymentId(payment_preimage.0)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -7430,7 +7542,7 @@ mod tests {
                expect_payment_failed!(nodes[0], our_payment_hash, true);
 
                // Send the second half of the original MPP payment.
-               nodes[0].node.send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
+               nodes[0].node.send_payment_along_path(&mpp_route.paths[1], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[1]).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -7528,7 +7640,7 @@ mod tests {
                        &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
                        None, nodes[0].logger, &scorer, &random_seed_bytes
                ).unwrap();
-               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
+               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), PaymentId(payment_preimage.0)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -7561,7 +7673,7 @@ mod tests {
                        &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
                        None, nodes[0].logger, &scorer, &random_seed_bytes
                ).unwrap();
-               let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
+               let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), PaymentId(payment_preimage.0)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -7571,7 +7683,7 @@ mod tests {
 
                // Next, attempt a regular payment and make sure it fails.
                let payment_secret = PaymentSecret([43; 32]);
-               nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
+               nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -7614,7 +7726,7 @@ mod tests {
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], channelmanager::provided_init_features(), channelmanager::provided_init_features());
                let route_params = RouteParameters {
                        payment_params: PaymentParameters::for_keysend(payee_pubkey),
-                       final_value_msat: 10000,
+                       final_value_msat: 10_000,
                        final_cltv_expiry_delta: 40,
                };
                let network_graph = nodes[0].network_graph;
@@ -7628,7 +7740,8 @@ mod tests {
 
                let test_preimage = PaymentPreimage([42; 32]);
                let mismatch_payment_hash = PaymentHash([43; 32]);
-               let _ = nodes[0].node.send_payment_internal(&route, mismatch_payment_hash, &None, Some(test_preimage), None, None).unwrap();
+               let session_privs = nodes[0].node.add_new_pending_payment(mismatch_payment_hash, None, PaymentId(mismatch_payment_hash.0), &route).unwrap();
+               nodes[0].node.send_payment_internal(&route, mismatch_payment_hash, &None, Some(test_preimage), PaymentId(mismatch_payment_hash.0), None, session_privs).unwrap();
                check_added_monitors!(nodes[0], 1);
 
                let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
@@ -7658,7 +7771,7 @@ mod tests {
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], channelmanager::provided_init_features(), channelmanager::provided_init_features());
                let route_params = RouteParameters {
                        payment_params: PaymentParameters::for_keysend(payee_pubkey),
-                       final_value_msat: 10000,
+                       final_value_msat: 10_000,
                        final_cltv_expiry_delta: 40,
                };
                let network_graph = nodes[0].network_graph;
@@ -7673,7 +7786,8 @@ mod tests {
                let test_preimage = PaymentPreimage([42; 32]);
                let test_secret = PaymentSecret([43; 32]);
                let payment_hash = PaymentHash(Sha256::hash(&test_preimage.0).into_inner());
-               let _ = nodes[0].node.send_payment_internal(&route, payment_hash, &Some(test_secret), Some(test_preimage), None, None).unwrap();
+               let session_privs = nodes[0].node.add_new_pending_payment(payment_hash, Some(test_secret), PaymentId(payment_hash.0), &route).unwrap();
+               nodes[0].node.send_payment_internal(&route, payment_hash, &Some(test_secret), Some(test_preimage), PaymentId(payment_hash.0), None, session_privs).unwrap();
                check_added_monitors!(nodes[0], 1);
 
                let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
@@ -7710,7 +7824,7 @@ mod tests {
                route.paths[1][0].short_channel_id = chan_2_id;
                route.paths[1][1].short_channel_id = chan_4_id;
 
-               match nodes[0].node.send_payment(&route, payment_hash, &None).unwrap_err() {
+               match nodes[0].node.send_payment(&route, payment_hash, &None, PaymentId(payment_hash.0)).unwrap_err() {
                        PaymentSendFailure::ParameterError(APIError::APIMisuseError { ref err }) => {
                                assert!(regex::Regex::new(r"Payment secret is required for multi-path payments").unwrap().is_match(err))                        },
                        _ => panic!("unexpected error")
@@ -7862,34 +7976,33 @@ mod tests {
 
 #[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))]
 pub mod bench {
-       use chain::Listen;
-       use chain::chainmonitor::{ChainMonitor, Persist};
-       use chain::keysinterface::{KeysManager, KeysInterface, InMemorySigner};
-       use ln::channelmanager::{self, BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage};
-       use ln::features::{InitFeatures, InvoiceFeatures};
-       use ln::functional_test_utils::*;
-       use ln::msgs::{ChannelMessageHandler, Init};
-       use routing::gossip::NetworkGraph;
-       use routing::router::{PaymentParameters, get_route};
-       use util::test_utils;
-       use util::config::UserConfig;
-       use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
+       use crate::chain::Listen;
+       use crate::chain::chainmonitor::{ChainMonitor, Persist};
+       use crate::chain::keysinterface::{KeysManager, KeysInterface, InMemorySigner};
+       use crate::ln::channelmanager::{self, BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage, PaymentId};
+       use crate::ln::functional_test_utils::*;
+       use crate::ln::msgs::{ChannelMessageHandler, Init};
+       use crate::routing::gossip::NetworkGraph;
+       use crate::routing::router::{PaymentParameters, get_route};
+       use crate::util::test_utils;
+       use crate::util::config::UserConfig;
+       use crate::util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
 
        use bitcoin::hashes::Hash;
        use bitcoin::hashes::sha256::Hash as Sha256;
        use bitcoin::{Block, BlockHeader, PackedLockTime, Transaction, TxMerkleNode, TxOut};
 
-       use sync::{Arc, Mutex};
+       use crate::sync::{Arc, Mutex};
 
        use test::Bencher;
 
        struct NodeHolder<'a, P: Persist<InMemorySigner>> {
-               node: &'a ChannelManager<InMemorySigner,
+               node: &'a ChannelManager<
                        &'a ChainMonitor<InMemorySigner, &'a test_utils::TestChainSource,
                                &'a test_utils::TestBroadcaster, &'a test_utils::TestFeeEstimator,
                                &'a test_utils::TestLogger, &'a P>,
                        &'a test_utils::TestBroadcaster, &'a KeysManager,
-                       &'a test_utils::TestFeeEstimator, &'a test_utils::TestLogger>
+                       &'a test_utils::TestFeeEstimator, &'a test_utils::TestLogger>,
        }
 
        #[cfg(test)]
@@ -7972,6 +8085,24 @@ pub mod bench {
                        _ => panic!(),
                }
 
+               let events_a = node_a.get_and_clear_pending_events();
+               assert_eq!(events_a.len(), 1);
+               match events_a[0] {
+                       Event::ChannelReady{ ref counterparty_node_id, .. } => {
+                               assert_eq!(*counterparty_node_id, node_b.get_our_node_id());
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+
+               let events_b = node_b.get_and_clear_pending_events();
+               assert_eq!(events_b.len(), 1);
+               match events_b[0] {
+                       Event::ChannelReady{ ref counterparty_node_id, .. } => {
+                               assert_eq!(*counterparty_node_id, node_a.get_our_node_id());
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+
                let dummy_graph = NetworkGraph::new(genesis_hash, &logger_a);
 
                let mut payment_count: u64 = 0;
@@ -7993,7 +8124,7 @@ pub mod bench {
                                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).unwrap();
 
-                               $node_a.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
+                               $node_a.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)).unwrap();
                                let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());
                                $node_b.handle_update_add_htlc(&$node_a.get_our_node_id(), &payment_event.msgs[0]);
                                $node_b.handle_commitment_signed(&$node_a.get_our_node_id(), &payment_event.commitment_msg);