Merge pull request #1895 from TheBlueMatt/2022-12-fix-missing-data
[rust-lightning] / lightning / src / ln / channelmanager.rs
index 6e5df15908adb097ea5b96655b234e46ac66907a..d4209ce9539695138e80e1eeaec5b93671c57c2a 100644 (file)
@@ -55,7 +55,7 @@ use crate::ln::wire::Encode;
 use crate::chain::keysinterface::{Sign, KeysInterface, KeysManager, Recipient};
 use crate::util::config::{UserConfig, ChannelConfig};
 use crate::util::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination};
-use crate::util::{byte_utils, events};
+use crate::util::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};
@@ -726,9 +726,9 @@ pub struct ChannelManager<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        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
+       /// expose them to users via a PaymentClaimable event. HTLCs which do not meet the requirements
        /// here are failed when we process them as pending-forwardable-HTLCs, and entries are removed
-       /// after we generate a PaymentReceived upon receipt of all MPP parts or when they time out.
+       /// after we generate a PaymentClaimable upon receipt of all MPP parts or when they time out.
        ///
        /// See `ChannelManager` struct-level documentation for lock order requirements.
        pending_inbound_payments: Mutex<HashMap<PaymentHash, PendingInboundPayment>>,
@@ -2033,7 +2033,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                        return Err(ReceiveError {
                                msg: "Upstream node set CLTV to the wrong value",
                                err_code: 18,
-                               err_data: byte_utils::be32_to_array(cltv_expiry).to_vec()
+                               err_data: cltv_expiry.to_be_bytes().to_vec()
                        })
                }
                // final_expiry_too_soon
@@ -2055,7 +2055,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                if hop_data.amt_to_forward > amt_msat {
                        return Err(ReceiveError {
                                err_code: 19,
-                               err_data: byte_utils::be64_to_array(amt_msat).to_vec(),
+                               err_data: amt_msat.to_be_bytes().to_vec(),
                                msg: "Upstream node sent less than we were supposed to receive in payment",
                        });
                }
@@ -2412,10 +2412,10 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                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)
-                       .map_err(|_| APIError::RouteError{err: "Pubkey along hop was maliciously selected"})?;
+                       .map_err(|_| APIError::InvalidRoute{err: "Pubkey along hop was maliciously selected"})?;
                let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(path, total_value, payment_secret, cur_height, keysend_preimage)?;
                if onion_utils::route_size_insane(&onion_payloads) {
-                       return Err(APIError::RouteError{err: "Route size too large considering onion data"});
+                       return Err(APIError::InvalidRoute{err: "Route size too large considering onion data"});
                }
                let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash);
 
@@ -2432,7 +2432,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                        if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) {
                                match {
                                        if chan.get().get_counterparty_node_id() != path.first().unwrap().pubkey {
-                                               return Err(APIError::RouteError{err: "Node ID mismatch on first hop!"});
+                                               return Err(APIError::InvalidRoute{err: "Node ID mismatch on first hop!"});
                                        }
                                        if !chan.get().is_live() {
                                                return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!".to_owned()});
@@ -2507,7 +2507,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        /// fields for more info.
        ///
        /// 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
+       /// method will error with an [`APIError::InvalidRoute`]. 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`].
@@ -2526,7 +2526,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        /// PaymentSendFailure for more info.
        ///
        /// In general, a path may raise:
-       ///  * [`APIError::RouteError`] when an invalid route or forwarding parameter (cltv_delta, fee,
+       ///  * [`APIError::InvalidRoute`] when an invalid route or forwarding parameter (cltv_delta, fee,
        ///    node public key) is specified.
        ///  * [`APIError::ChannelUnavailable`] if the next-hop channel is not available for updates
        ///    (including due to previous monitor update failure or new permanent monitor update
@@ -2591,7 +2591,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 
        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"}));
+                       return Err(PaymentSendFailure::ParameterError(APIError::InvalidRoute{err: "There must be at least one path to send over"}));
                }
                if payment_secret.is_none() && route.paths.len() > 1 {
                        return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError{err: "Payment secret is required for multi-path payments".to_string()}));
@@ -2601,12 +2601,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                let mut path_errs = Vec::with_capacity(route.paths.len());
                '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"}));
+                               path_errs.push(Err(APIError::InvalidRoute{err: "Path didn't go anywhere/had bogus size"}));
                                continue 'path_check;
                        }
                        for (idx, hop) in path.iter().enumerate() {
                                if idx != path.len() - 1 && hop.pubkey == our_node_id {
-                                       path_errs.push(Err(APIError::RouteError{err: "Path went through us but wasn't a simple rebalance loop to us"}));
+                                       path_errs.push(Err(APIError::InvalidRoute{err: "Path went through us but wasn't a simple rebalance loop to us"}));
                                        continue 'path_check;
                                }
                        }
@@ -3085,20 +3085,20 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                let next_hop_scid = match self.channel_state.lock().unwrap().by_id.get(next_hop_channel_id) {
                        Some(chan) => {
                                if !chan.is_usable() {
-                                       return Err(APIError::APIMisuseError {
-                                               err: format!("Channel with id {:?} not fully established", next_hop_channel_id)
+                                       return Err(APIError::ChannelUnavailable {
+                                               err: format!("Channel with id {} not fully established", log_bytes!(*next_hop_channel_id))
                                        })
                                }
                                chan.get_short_channel_id().unwrap_or(chan.outbound_scid_alias())
                        },
-                       None => return Err(APIError::APIMisuseError {
-                               err: format!("Channel with id {:?} not found", next_hop_channel_id)
+                       None => return Err(APIError::ChannelUnavailable {
+                               err: format!("Channel with id {} not found", log_bytes!(*next_hop_channel_id))
                        })
                };
 
                let payment = self.pending_intercepted_htlcs.lock().unwrap().remove(&intercept_id)
                        .ok_or_else(|| APIError::APIMisuseError {
-                               err: format!("Payment with intercept id {:?} not found", intercept_id.0)
+                               err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.0))
                        })?;
 
                let routing = match payment.forward_info.routing {
@@ -3133,7 +3133,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 
                let payment = self.pending_intercepted_htlcs.lock().unwrap().remove(&intercept_id)
                        .ok_or_else(|| APIError::APIMisuseError {
-                               err: format!("Payment with InterceptId {:?} not found", intercept_id)
+                               err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.0))
                        })?;
 
                if let PendingHTLCRouting::Forward { short_channel_id, .. } = payment.forward_info.routing {
@@ -3444,9 +3444,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 
                                                                macro_rules! fail_htlc {
                                                                        ($htlc: expr, $payment_hash: expr) => {
-                                                                               let mut htlc_msat_height_data = byte_utils::be64_to_array($htlc.value).to_vec();
+                                                                               let mut htlc_msat_height_data = $htlc.value.to_be_bytes().to_vec();
                                                                                htlc_msat_height_data.extend_from_slice(
-                                                                                       &byte_utils::be32_to_array(self.best_block.read().unwrap().height()),
+                                                                                       &self.best_block.read().unwrap().height().to_be_bytes(),
                                                                                );
                                                                                failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                                                                                short_channel_id: $htlc.prev_hop.short_channel_id,
@@ -3469,7 +3469,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 
                                                                macro_rules! check_total_value {
                                                                        ($payment_data: expr, $payment_preimage: expr) => {{
-                                                                               let mut payment_received_generated = false;
+                                                                               let mut payment_claimable_generated = false;
                                                                                let purpose = || {
                                                                                        events::PaymentPurpose::InvoicePayment {
                                                                                                payment_preimage: $payment_preimage,
@@ -3508,7 +3508,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                                                                } else if total_value == $payment_data.total_msat {
                                                                                        let prev_channel_id = prev_funding_outpoint.to_channel_id();
                                                                                        htlcs.push(claimable_htlc);
-                                                                                       new_events.push(events::Event::PaymentReceived {
+                                                                                       new_events.push(events::Event::PaymentClaimable {
                                                                                                receiver_node_id: Some(receiver_node_id),
                                                                                                payment_hash,
                                                                                                purpose: purpose(),
@@ -3516,14 +3516,14 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                                                                                via_channel_id: Some(prev_channel_id),
                                                                                                via_user_channel_id: Some(prev_user_channel_id),
                                                                                        });
-                                                                                       payment_received_generated = true;
+                                                                                       payment_claimable_generated = true;
                                                                                } else {
                                                                                        // Nothing to do - we haven't reached the total
                                                                                        // payment value yet, wait until we receive more
                                                                                        // MPP parts.
                                                                                        htlcs.push(claimable_htlc);
                                                                                }
-                                                                               payment_received_generated
+                                                                               payment_claimable_generated
                                                                        }}
                                                                }
 
@@ -3554,7 +3554,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                                                                                                let purpose = events::PaymentPurpose::SpontaneousPayment(preimage);
                                                                                                                e.insert((purpose.clone(), vec![claimable_htlc]));
                                                                                                                let prev_channel_id = prev_funding_outpoint.to_channel_id();
-                                                                                                               new_events.push(events::Event::PaymentReceived {
+                                                                                                               new_events.push(events::Event::PaymentClaimable {
                                                                                                                        receiver_node_id: Some(receiver_node_id),
                                                                                                                        payment_hash,
                                                                                                                        amount_msat: outgoing_amt_msat,
@@ -3586,8 +3586,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                                                                                log_bytes!(payment_hash.0), payment_data.total_msat, inbound_payment.get().min_value_msat.unwrap());
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                } else {
-                                                                                       let payment_received_generated = check_total_value!(payment_data, inbound_payment.get().payment_preimage);
-                                                                                       if payment_received_generated {
+                                                                                       let payment_claimable_generated = check_total_value!(payment_data, inbound_payment.get().payment_preimage);
+                                                                                       if payment_claimable_generated {
                                                                                                inbound_payment.remove_entry();
                                                                                        }
                                                                                }
@@ -3884,12 +3884,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        }
 
        /// Indicates that the preimage for payment_hash is unknown or the received amount is incorrect
-       /// after a PaymentReceived event, failing the HTLC back to its origin and freeing resources
+       /// after a PaymentClaimable event, failing the HTLC back to its origin and freeing resources
        /// along the path (including in our own channel on which we received it).
        ///
        /// Note that in some cases around unclean shutdown, it is possible the payment may have
        /// already been claimed by you via [`ChannelManager::claim_funds`] prior to you seeing (a
-       /// second copy of) the [`events::Event::PaymentReceived`] event. Alternatively, the payment
+       /// second copy of) the [`events::Event::PaymentClaimable`] event. Alternatively, the payment
        /// may have already been failed automatically by LDK if it was nearing its expiration time.
        ///
        /// While LDK will never claim a payment automatically on your behalf (i.e. without you calling
@@ -3902,9 +3902,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                let removed_source = self.claimable_htlcs.lock().unwrap().remove(payment_hash);
                if let Some((_, mut sources)) = removed_source {
                        for htlc in sources.drain(..) {
-                               let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
-                               htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
-                                               self.best_block.read().unwrap().height()));
+                               let mut htlc_msat_height_data = htlc.value.to_be_bytes().to_vec();
+                               htlc_msat_height_data.extend_from_slice(&self.best_block.read().unwrap().height().to_be_bytes());
                                let source = HTLCSource::PreviousHopData(htlc.prev_hop);
                                let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
                                let receiver = HTLCDestination::FailedPayment { payment_hash: *payment_hash };
@@ -4122,7 +4121,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                }
        }
 
-       /// Provides a payment preimage in response to [`Event::PaymentReceived`], generating any
+       /// Provides a payment preimage in response to [`Event::PaymentClaimable`], generating any
        /// [`MessageSendEvent`]s needed to claim the payment.
        ///
        /// Note that calling this method does *not* guarantee that the payment has been claimed. You
@@ -4130,11 +4129,11 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        /// provided to your [`EventHandler`] when [`process_pending_events`] is next called.
        ///
        /// 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`
+       /// [`create_inbound_payment_for_hash`] you must check that the amount in the `PaymentClaimable`
        /// 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.
        ///
-       /// [`Event::PaymentReceived`]: crate::util::events::Event::PaymentReceived
+       /// [`Event::PaymentClaimable`]: crate::util::events::Event::PaymentClaimable
        /// [`Event::PaymentClaimed`]: crate::util::events::Event::PaymentClaimed
        /// [`process_pending_events`]: EventsProvider::process_pending_events
        /// [`create_inbound_payment`]: Self::create_inbound_payment
@@ -4242,9 +4241,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                        mem::drop(channel_state_lock);
                        if !valid_mpp {
                                for htlc in sources.drain(..) {
-                                       let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
-                                       htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
-                                               self.best_block.read().unwrap().height()));
+                                       let mut htlc_msat_height_data = htlc.value.to_be_bytes().to_vec();
+                                       htlc_msat_height_data.extend_from_slice(&self.best_block.read().unwrap().height().to_be_bytes());
                                        let source = HTLCSource::PreviousHopData(htlc.prev_hop);
                                        let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
                                        let receiver = HTLCDestination::FailedPayment { payment_hash };
@@ -4752,7 +4750,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                        if chan.get().get_counterparty_node_id() != *counterparty_node_id {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id));
                                        }
-                                       (try_chan_entry!(self, chan.get_mut().funding_created(msg, best_block, &self.logger), chan), chan.remove())
+                                       (try_chan_entry!(self, chan.get_mut().funding_created(msg, best_block, &self.keys_manager, &self.logger), chan), chan.remove())
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id))
                        }
@@ -4823,7 +4821,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                                        if chan.get().get_counterparty_node_id() != *counterparty_node_id {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
                                        }
-                                       let (monitor, funding_tx, channel_ready) = match chan.get_mut().funding_signed(&msg, best_block, &self.logger) {
+                                       let (monitor, funding_tx, channel_ready) = match chan.get_mut().funding_signed(&msg, best_block, &self.keys_manager, &self.logger) {
                                                Ok(update) => update,
                                                Err(e) => try_chan_entry!(self, Err(e), chan),
                                        };
@@ -5684,8 +5682,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        /// This differs from [`create_inbound_payment_for_hash`] only in that it generates the
        /// [`PaymentHash`] and [`PaymentPreimage`] for you.
        ///
-       /// The [`PaymentPreimage`] will ultimately be returned to you in the [`PaymentReceived`], which
-       /// will have the [`PaymentReceived::payment_preimage`] field filled in. That should then be
+       /// The [`PaymentPreimage`] will ultimately be returned to you in the [`PaymentClaimable`], which
+       /// will have the [`PaymentClaimable::payment_preimage`] field filled in. That should then be
        /// passed directly to [`claim_funds`].
        ///
        /// See [`create_inbound_payment_for_hash`] for detailed documentation on behavior and requirements.
@@ -5701,8 +5699,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        /// Errors if `min_value_msat` is greater than total bitcoin supply.
        ///
        /// [`claim_funds`]: Self::claim_funds
-       /// [`PaymentReceived`]: events::Event::PaymentReceived
-       /// [`PaymentReceived::payment_preimage`]: events::Event::PaymentReceived::payment_preimage
+       /// [`PaymentClaimable`]: events::Event::PaymentClaimable
+       /// [`PaymentClaimable::payment_preimage`]: events::Event::PaymentClaimable::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) -> Result<(PaymentHash, PaymentSecret), ()> {
                inbound_payment::create(&self.inbound_payment_key, min_value_msat, invoice_expiry_delta_secs, &self.keys_manager, self.highest_seen_timestamp.load(Ordering::Acquire) as u64)
@@ -5728,7 +5726,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        /// Gets a [`PaymentSecret`] for a given [`PaymentHash`], for which the payment preimage is
        /// stored external to LDK.
        ///
-       /// A [`PaymentReceived`] event will only be generated if the [`PaymentSecret`] matches a
+       /// A [`PaymentClaimable`] event will only be generated if the [`PaymentSecret`] matches a
        /// payment secret fetched via this method or [`create_inbound_payment`], and which is at least
        /// the `min_value_msat` provided here, if one is provided.
        ///
@@ -5738,7 +5736,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        ///
        /// `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
+       /// before a [`PaymentClaimable`] event will be generated, ensuring that we do not provide the
        /// sender "proof-of-payment" unless they have paid the required amount.
        ///
        /// `invoice_expiry_delta_secs` describes the number of seconds that the invoice is valid for
@@ -5749,9 +5747,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        ///
        /// Note that we use block header time to time-out pending inbound payments (with some margin
        /// to compensate for the inaccuracy of block header timestamps). Thus, in practice we will
-       /// accept a payment and generate a [`PaymentReceived`] event for some time after the expiry.
+       /// accept a payment and generate a [`PaymentClaimable`] event for some time after the expiry.
        /// If you need exact expiry semantics, you should enforce them upon receipt of
-       /// [`PaymentReceived`].
+       /// [`PaymentClaimable`].
        ///
        /// Note that invoices generated for inbound payments should have their `min_final_cltv_expiry`
        /// set to at least [`MIN_FINAL_CLTV_EXPIRY`].
@@ -5767,7 +5765,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
        /// Errors if `min_value_msat` is greater than total bitcoin supply.
        ///
        /// [`create_inbound_payment`]: Self::create_inbound_payment
-       /// [`PaymentReceived`]: events::Event::PaymentReceived
+       /// [`PaymentClaimable`]: events::Event::PaymentClaimable
        pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, ()> {
                inbound_payment::create_from_hash(&self.inbound_payment_key, min_value_msat, payment_hash, invoice_expiry_delta_secs, self.highest_seen_timestamp.load(Ordering::Acquire) as u64)
        }
@@ -5845,7 +5843,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                let mut inflight_htlcs = InFlightHtlcs::new();
 
                for chan in self.channel_state.lock().unwrap().by_id.values() {
-                       for htlc_source in chan.inflight_htlc_sources() {
+                       for (htlc_source, _) in chan.inflight_htlc_sources() {
                                if let HTLCSource::OutboundRoute { path, .. } = htlc_source {
                                        inflight_htlcs.process_path(path, self.get_our_node_id());
                                }
@@ -5863,6 +5861,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                events.into_inner()
        }
 
+       #[cfg(test)]
+       pub fn pop_pending_event(&self) -> Option<events::Event> {
+               let mut events = self.pending_events.lock().unwrap();
+               if events.is_empty() { None } else { Some(events.remove(0)) }
+       }
+
        #[cfg(test)]
        pub fn has_pending_payments(&self) -> bool {
                !self.pending_outbound_payments.lock().unwrap().is_empty()
@@ -6219,8 +6223,8 @@ where
                                        // number of blocks we generally consider it to take to do a commitment update,
                                        // just give up on it and fail the HTLC.
                                        if height >= htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER {
-                                               let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
-                                               htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height));
+                                               let mut htlc_msat_height_data = htlc.value.to_be_bytes().to_vec();
+                                               htlc_msat_height_data.extend_from_slice(&height.to_be_bytes());
 
                                                timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(),
                                                        HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
@@ -7346,6 +7350,25 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                                user_channel_id: channel.get_user_id(),
                                                reason: ClosureReason::OutdatedChannelManager
                                        });
+                                       for (channel_htlc_source, payment_hash) in channel.inflight_htlc_sources() {
+                                               let mut found_htlc = false;
+                                               for (monitor_htlc_source, _) in monitor.get_all_current_outbound_htlcs() {
+                                                       if *channel_htlc_source == monitor_htlc_source { found_htlc = true; break; }
+                                               }
+                                               if !found_htlc {
+                                                       // If we have some HTLCs in the channel which are not present in the newer
+                                                       // ChannelMonitor, they have been removed and should be failed back to
+                                                       // ensure we don't forget them entirely. Note that if the missing HTLC(s)
+                                                       // were actually claimed we'd have generated and ensured the previous-hop
+                                                       // claim update ChannelMonitor updates were persisted prior to persising
+                                                       // the ChannelMonitor update for the forward leg, so attempting to fail the
+                                                       // backwards leg of the HTLC will simply be rejected.
+                                                       log_info!(args.logger,
+                                                               "Failing HTLC with hash {} as it is missing in the ChannelMonitor for channel {} but was present in the (stale) ChannelManager",
+                                                               log_bytes!(channel.channel_id()), log_bytes!(payment_hash.0));
+                                                       failed_htlcs.push((channel_htlc_source.clone(), *payment_hash, channel.get_counterparty_node_id(), channel.channel_id()));
+                                               }
+                                       }
                                } else {
                                        log_info!(args.logger, "Successfully loaded channel {}", log_bytes!(channel.channel_id()));
                                        if let Some(short_channel_id) = channel.get_short_channel_id() {
@@ -7426,16 +7449,6 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                None => continue,
                        }
                }
-               if forward_htlcs_count > 0 {
-                       // If we have pending HTLCs to forward, assume we either dropped a
-                       // `PendingHTLCsForwardable` or the user received it but never processed it as they
-                       // shut down before the timer hit. Either way, set the time_forwardable to a small
-                       // constant as enough time has likely passed that we should simply handle the forwards
-                       // now, or at least after the user gets a chance to reconnect to our peers.
-                       pending_events_read.push(events::Event::PendingHTLCsForwardable {
-                               time_forwardable: Duration::from_secs(2),
-                       });
-               }
 
                let background_event_count: u64 = Readable::read(reader)?;
                let mut pending_background_events_read: Vec<BackgroundEvent> = Vec::with_capacity(cmp::min(background_event_count as usize, MAX_ALLOC_SIZE/mem::size_of::<BackgroundEvent>()));
@@ -7546,10 +7559,44 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                                        }
                                                }
                                        }
+                                       for (htlc_source, htlc) in monitor.get_all_current_outbound_htlcs() {
+                                               if let HTLCSource::PreviousHopData(prev_hop_data) = htlc_source {
+                                                       // The ChannelMonitor is now responsible for this HTLC's
+                                                       // failure/success and will let us know what its outcome is. If we
+                                                       // still have an entry for this HTLC in `forward_htlcs`, we were
+                                                       // apparently not persisted after the monitor was when forwarding
+                                                       // the payment.
+                                                       forward_htlcs.retain(|_, forwards| {
+                                                               forwards.retain(|forward| {
+                                                                       if let HTLCForwardInfo::AddHTLC(htlc_info) = forward {
+                                                                               if htlc_info.prev_short_channel_id == prev_hop_data.short_channel_id &&
+                                                                                       htlc_info.prev_htlc_id == prev_hop_data.htlc_id
+                                                                               {
+                                                                                       log_info!(args.logger, "Removing pending to-forward HTLC with hash {} as it was forwarded to the closed channel {}",
+                                                                                               log_bytes!(htlc.payment_hash.0), log_bytes!(monitor.get_funding_txo().0.to_channel_id()));
+                                                                                       false
+                                                                               } else { true }
+                                                                       } else { true }
+                                                               });
+                                                               !forwards.is_empty()
+                                                       })
+                                               }
+                                       }
                                }
                        }
                }
 
+               if !forward_htlcs.is_empty() {
+                       // If we have pending HTLCs to forward, assume we either dropped a
+                       // `PendingHTLCsForwardable` or the user received it but never processed it as they
+                       // shut down before the timer hit. Either way, set the time_forwardable to a small
+                       // constant as enough time has likely passed that we should simply handle the forwards
+                       // now, or at least after the user gets a chance to reconnect to our peers.
+                       pending_events_read.push(events::Event::PendingHTLCsForwardable {
+                               time_forwardable: Duration::from_secs(2),
+                       });
+               }
+
                let inbound_pmt_key_material = args.keys_manager.get_inbound_payment_key_material();
                let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material);
 
@@ -8483,7 +8530,7 @@ pub mod bench {
                                $node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &get_event_msg!(NodeHolder { node: &$node_a }, MessageSendEvent::SendRevokeAndACK, $node_b.get_our_node_id()));
 
                                expect_pending_htlcs_forwardable!(NodeHolder { node: &$node_b });
-                               expect_payment_received!(NodeHolder { node: &$node_b }, payment_hash, payment_secret, 10_000);
+                               expect_payment_claimable!(NodeHolder { node: &$node_b }, payment_hash, payment_secret, 10_000);
                                $node_b.claim_funds(payment_preimage);
                                expect_payment_claimed!(NodeHolder { node: &$node_b }, payment_hash, 10_000);