Merge pull request #2536 from waterson/test-channel-signer
[rust-lightning] / lightning / src / ln / channelmanager.rs
index a161ff58f04aea9bad238258ca6b74b7be1e305a..64d5521dcb95f78ba3317398e19438f074b19dbc 100644 (file)
@@ -39,7 +39,7 @@ use crate::events;
 use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination, PaymentFailureReason};
 // Since this struct is returned in `list_channels` methods, expose it here in case users want to
 // construct one themselves.
-use crate::ln::{inbound_payment, PaymentHash, PaymentPreimage, PaymentSecret};
+use crate::ln::{inbound_payment, ChannelId, PaymentHash, PaymentPreimage, PaymentSecret};
 use crate::ln::channel::{Channel, ChannelContext, ChannelError, ChannelUpdateStatus, ShutdownResult, UnfundedChannelContext, UpdateFulfillCommitFetch, OutboundV1Channel, InboundV1Channel};
 use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
 #[cfg(any(feature = "_test_utils", test))]
@@ -55,7 +55,7 @@ use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
 use crate::ln::outbound_payment;
 use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment, SendAlongPathArgs};
 use crate::ln::wire::Encode;
-use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, ChannelSigner, WriteableEcdsaChannelSigner};
+use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, WriteableEcdsaChannelSigner};
 use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate};
 use crate::util::wakers::{Future, Notifier};
 use crate::util::scid_utils::fake_scid;
@@ -181,6 +181,7 @@ pub(super) enum HTLCForwardInfo {
 pub(crate) struct HTLCPreviousHopData {
        // Note that this may be an outbound SCID alias for the associated channel.
        short_channel_id: u64,
+       user_channel_id: Option<u128>,
        htlc_id: u64,
        incoming_packet_shared_secret: [u8; 32],
        phantom_shared_secret: Option<[u8; 32]>,
@@ -221,6 +222,17 @@ struct ClaimableHTLC {
        counterparty_skimmed_fee_msat: Option<u64>,
 }
 
+impl From<&ClaimableHTLC> for events::ClaimedHTLC {
+       fn from(val: &ClaimableHTLC) -> Self {
+               events::ClaimedHTLC {
+                       channel_id: val.prev_hop.outpoint.to_channel_id(),
+                       user_channel_id: val.prev_hop.user_channel_id.unwrap_or(0),
+                       cltv_expiry: val.cltv_expiry,
+                       value_msat: val.value,
+               }
+       }
+}
+
 /// A payment identifier used to uniquely identify a payment to LDK.
 ///
 /// This is not exported to bindings users as we just use [u8; 32] directly
@@ -240,6 +252,12 @@ impl Readable for PaymentId {
        }
 }
 
+impl core::fmt::Display for PaymentId {
+       fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+               crate::util::logger::DebugBytes(&self.0).fmt(f)
+       }
+}
+
 /// An identifier used to uniquely identify an intercepted HTLC to LDK.
 ///
 /// This is not exported to bindings users as we just use [u8; 32] directly
@@ -395,13 +413,13 @@ impl Into<u16> for FailureCode {
 
 struct MsgHandleErrInternal {
        err: msgs::LightningError,
-       chan_id: Option<([u8; 32], u128)>, // If Some a channel of ours has been closed
+       chan_id: Option<(ChannelId, u128)>, // If Some a channel of ours has been closed
        shutdown_finish: Option<(ShutdownResult, Option<msgs::ChannelUpdate>)>,
        channel_capacity: Option<u64>,
 }
 impl MsgHandleErrInternal {
        #[inline]
-       fn send_err_msg_no_close(err: String, channel_id: [u8; 32]) -> Self {
+       fn send_err_msg_no_close(err: String, channel_id: ChannelId) -> Self {
                Self {
                        err: LightningError {
                                err: err.clone(),
@@ -422,7 +440,7 @@ impl MsgHandleErrInternal {
                Self { err, chan_id: None, shutdown_finish: None, channel_capacity: None }
        }
        #[inline]
-       fn from_finish_shutdown(err: String, channel_id: [u8; 32], user_channel_id: u128, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>, channel_capacity: u64) -> Self {
+       fn from_finish_shutdown(err: String, channel_id: ChannelId, user_channel_id: u128, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>, channel_capacity: u64) -> Self {
                Self {
                        err: LightningError {
                                err: err.clone(),
@@ -439,7 +457,7 @@ impl MsgHandleErrInternal {
                }
        }
        #[inline]
-       fn from_chan_no_close(err: ChannelError, channel_id: [u8; 32]) -> Self {
+       fn from_chan_no_close(err: ChannelError, channel_id: ChannelId) -> Self {
                Self {
                        err: match err {
                                ChannelError::Warn(msg) =>  LightningError {
@@ -496,11 +514,15 @@ struct ClaimingPayment {
        amount_msat: u64,
        payment_purpose: events::PaymentPurpose,
        receiver_node_id: PublicKey,
+       htlcs: Vec<events::ClaimedHTLC>,
+       sender_intended_value: Option<u64>,
 }
 impl_writeable_tlv_based!(ClaimingPayment, {
        (0, amount_msat, required),
        (2, payment_purpose, required),
        (4, receiver_node_id, required),
+       (5, htlcs, optional_vec),
+       (7, sender_intended_value, option),
 });
 
 struct ClaimablePayment {
@@ -560,7 +582,7 @@ enum BackgroundEvent {
        /// on a channel.
        MonitorUpdatesComplete {
                counterparty_node_id: PublicKey,
-               channel_id: [u8; 32],
+               channel_id: ChannelId,
        },
 }
 
@@ -621,7 +643,7 @@ pub(crate) enum RAAMonitorUpdateBlockingAction {
        /// durably to disk.
        ForwardedPaymentInboundClaim {
                /// The upstream channel ID (i.e. the inbound edge).
-               channel_id: [u8; 32],
+               channel_id: ChannelId,
                /// The HTLC ID on the inbound edge.
                htlc_id: u64,
        },
@@ -643,30 +665,30 @@ impl_writeable_tlv_based_enum!(RAAMonitorUpdateBlockingAction,
 
 
 /// State we hold per-peer.
-pub(super) struct PeerState<Signer: ChannelSigner> {
+pub(super) struct PeerState<SP: Deref> where SP::Target: SignerProvider {
        /// `channel_id` -> `Channel`.
        ///
        /// Holds all funded channels where the peer is the counterparty.
-       pub(super) channel_by_id: HashMap<[u8; 32], Channel<Signer>>,
+       pub(super) channel_by_id: HashMap<ChannelId, Channel<SP>>,
        /// `temporary_channel_id` -> `OutboundV1Channel`.
        ///
        /// Holds all outbound V1 channels where the peer is the counterparty. Once an outbound channel has
        /// been assigned a `channel_id`, the entry in this map is removed and one is created in
        /// `channel_by_id`.
-       pub(super) outbound_v1_channel_by_id: HashMap<[u8; 32], OutboundV1Channel<Signer>>,
+       pub(super) outbound_v1_channel_by_id: HashMap<ChannelId, OutboundV1Channel<SP>>,
        /// `temporary_channel_id` -> `InboundV1Channel`.
        ///
        /// Holds all inbound V1 channels where the peer is the counterparty. Once an inbound channel has
        /// been assigned a `channel_id`, the entry in this map is removed and one is created in
        /// `channel_by_id`.
-       pub(super) inbound_v1_channel_by_id: HashMap<[u8; 32], InboundV1Channel<Signer>>,
+       pub(super) inbound_v1_channel_by_id: HashMap<ChannelId, InboundV1Channel<SP>>,
        /// `temporary_channel_id` -> `InboundChannelRequest`.
        ///
        /// When manual channel acceptance is enabled, this holds all unaccepted inbound channels where
        /// the peer is the counterparty. If the channel is accepted, then the entry in this table is
        /// removed, and an InboundV1Channel is created and placed in the `inbound_v1_channel_by_id` table. If
        /// the channel is rejected, then the entry is simply removed.
-       pub(super) inbound_channel_request_by_id: HashMap<[u8; 32], InboundChannelRequest>,
+       pub(super) inbound_channel_request_by_id: HashMap<ChannelId, InboundChannelRequest>,
        /// The latest `InitFeatures` we heard from the peer.
        latest_features: InitFeatures,
        /// Messages to send to the peer - pushed to in the same lock that they are generated in (except
@@ -693,19 +715,19 @@ pub(super) struct PeerState<Signer: ChannelSigner> {
        /// same `temporary_channel_id` (or final `channel_id` in the case of 0conf channels or prior
        /// to funding appearing on-chain), the downstream `ChannelMonitor` set is required to ensure
        /// duplicates do not occur, so such channels should fail without a monitor update completing.
-       monitor_update_blocked_actions: BTreeMap<[u8; 32], Vec<MonitorUpdateCompletionAction>>,
+       monitor_update_blocked_actions: BTreeMap<ChannelId, Vec<MonitorUpdateCompletionAction>>,
        /// If another channel's [`ChannelMonitorUpdate`] needs to complete before a channel we have
        /// with this peer can complete an RAA [`ChannelMonitorUpdate`] (e.g. because the RAA update
        /// will remove a preimage that needs to be durably in an upstream channel first), we put an
        /// entry here to note that the channel with the key's ID is blocked on a set of actions.
-       actions_blocking_raa_monitor_updates: BTreeMap<[u8; 32], Vec<RAAMonitorUpdateBlockingAction>>,
+       actions_blocking_raa_monitor_updates: BTreeMap<ChannelId, Vec<RAAMonitorUpdateBlockingAction>>,
        /// The peer is currently connected (i.e. we've seen a
        /// [`ChannelMessageHandler::peer_connected`] and no corresponding
        /// [`ChannelMessageHandler::peer_disconnected`].
        is_connected: bool,
 }
 
-impl <Signer: ChannelSigner> PeerState<Signer> {
+impl <SP: Deref> PeerState<SP> where SP::Target: SignerProvider {
        /// Indicates that a peer meets the criteria where we're ok to remove it from our storage.
        /// If true is passed for `require_disconnected`, the function will return false if we haven't
        /// disconnected from the node already, ie. `PeerState::is_connected` is set to `true`.
@@ -726,11 +748,11 @@ impl <Signer: ChannelSigner> PeerState<Signer> {
        }
 
        // Returns a bool indicating if the given `channel_id` matches a channel we have with this peer.
-       fn has_channel(&self, channel_id: &[u8; 32]) -> bool {
-               self.channel_by_id.contains_key(channel_id) ||
-                       self.outbound_v1_channel_by_id.contains_key(channel_id) ||
-                       self.inbound_v1_channel_by_id.contains_key(channel_id) ||
-                       self.inbound_channel_request_by_id.contains_key(channel_id)
+       fn has_channel(&self, channel_id: &ChannelId) -> bool {
+               self.channel_by_id.contains_key(&channel_id) ||
+                       self.outbound_v1_channel_by_id.contains_key(&channel_id) ||
+                       self.inbound_v1_channel_by_id.contains_key(&channel_id) ||
+                       self.inbound_channel_request_by_id.contains_key(&channel_id)
        }
 }
 
@@ -1077,7 +1099,7 @@ where
        /// required to access the channel with the `counterparty_node_id`.
        ///
        /// See `ChannelManager` struct-level documentation for lock order requirements.
-       id_to_peer: Mutex<HashMap<[u8; 32], PublicKey>>,
+       id_to_peer: Mutex<HashMap<ChannelId, PublicKey>>,
 
        /// SCIDs (and outbound SCID aliases) -> `counterparty_node_id`s and `channel_id`s.
        ///
@@ -1091,9 +1113,9 @@ where
        ///
        /// See `ChannelManager` struct-level documentation for lock order requirements.
        #[cfg(test)]
-       pub(super) short_to_chan_info: FairRwLock<HashMap<u64, (PublicKey, [u8; 32])>>,
+       pub(super) short_to_chan_info: FairRwLock<HashMap<u64, (PublicKey, ChannelId)>>,
        #[cfg(not(test))]
-       short_to_chan_info: FairRwLock<HashMap<u64, (PublicKey, [u8; 32])>>,
+       short_to_chan_info: FairRwLock<HashMap<u64, (PublicKey, ChannelId)>>,
 
        our_network_pubkey: PublicKey,
 
@@ -1130,9 +1152,9 @@ where
        ///
        /// See `ChannelManager` struct-level documentation for lock order requirements.
        #[cfg(not(any(test, feature = "_test_utils")))]
-       per_peer_state: FairRwLock<HashMap<PublicKey, Mutex<PeerState<<SP::Target as SignerProvider>::Signer>>>>,
+       per_peer_state: FairRwLock<HashMap<PublicKey, Mutex<PeerState<SP>>>>,
        #[cfg(any(test, feature = "_test_utils"))]
-       pub(super) per_peer_state: FairRwLock<HashMap<PublicKey, Mutex<PeerState<<SP::Target as SignerProvider>::Signer>>>>,
+       pub(super) per_peer_state: FairRwLock<HashMap<PublicKey, Mutex<PeerState<SP>>>>,
 
        /// The set of events which we need to give to the user to handle. In some cases an event may
        /// require some further action after the user handles it (currently only blocking a monitor
@@ -1144,7 +1166,11 @@ where
        /// could be in the middle of being processed without the direct mutex held.
        ///
        /// See `ChannelManager` struct-level documentation for lock order requirements.
+       #[cfg(not(any(test, feature = "_test_utils")))]
        pending_events: Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
+       #[cfg(any(test, feature = "_test_utils"))]
+       pub(crate) pending_events: Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
+
        /// A simple atomic flag to ensure only one task at a time can be processing events asynchronously.
        pending_events_processor: AtomicBool,
 
@@ -1391,7 +1417,7 @@ pub struct ChannelDetails {
        /// thereafter this is the txid of the funding transaction xor the funding transaction output).
        /// Note that this means this value is *not* persistent - it can change once during the
        /// lifetime of the channel.
-       pub channel_id: [u8; 32],
+       pub channel_id: ChannelId,
        /// Parameters which apply to our counterparty. See individual fields for more information.
        pub counterparty: ChannelCounterparty,
        /// The Channel's funding transaction output, if we've negotiated the funding transaction with
@@ -1574,11 +1600,13 @@ impl ChannelDetails {
                self.short_channel_id.or(self.outbound_scid_alias)
        }
 
-       fn from_channel_context<Signer: WriteableEcdsaChannelSigner, F: Deref>(
-               context: &ChannelContext<Signer>, best_block_height: u32, latest_features: InitFeatures,
+       fn from_channel_context<SP: Deref, F: Deref>(
+               context: &ChannelContext<SP>, best_block_height: u32, latest_features: InitFeatures,
                fee_estimator: &LowerBoundedFeeEstimator<F>
        ) -> Self
-       where F::Target: FeeEstimator
+       where
+               SP::Target: SignerProvider,
+               F::Target: FeeEstimator
        {
                let balance = context.get_available_balances(fee_estimator);
                let (to_remote_reserve_satoshis, to_self_reserve_satoshis) =
@@ -1788,7 +1816,7 @@ macro_rules! convert_chan_err {
                                (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore(msg), $channel_id.clone()))
                        },
                        ChannelError::Close(msg) => {
-                               log_error!($self.logger, "Closing channel {} due to close-required error: {}", log_bytes!($channel_id[..]), msg);
+                               log_error!($self.logger, "Closing channel {} due to close-required error: {}", &$channel_id, msg);
                                update_maps_on_chan_removal!($self, &$channel.context);
                                let shutdown_res = $channel.context.force_shutdown(true);
                                (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.context.get_user_id(),
@@ -1801,7 +1829,7 @@ macro_rules! convert_chan_err {
                        // We should only ever have `ChannelError::Close` when unfunded channels error.
                        // In any case, just close the channel.
                        ChannelError::Warn(msg) | ChannelError::Ignore(msg) | ChannelError::Close(msg) => {
-                               log_error!($self.logger, "Closing unfunded channel {} due to an error: {}", log_bytes!($channel_id[..]), msg);
+                               log_error!($self.logger, "Closing unfunded channel {} due to an error: {}", &$channel_id, msg);
                                update_maps_on_chan_removal!($self, &$channel_context);
                                let shutdown_res = $channel_context.force_shutdown(false);
                                (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel_context.get_user_id(),
@@ -1974,12 +2002,12 @@ macro_rules! handle_new_monitor_update {
                match $update_res {
                        ChannelMonitorUpdateStatus::InProgress => {
                                log_debug!($self.logger, "ChannelMonitor update for {} in flight, holding messages until the update completes.",
-                                       log_bytes!($chan.context.channel_id()[..]));
+                                       &$chan.context.channel_id());
                                Ok(false)
                        },
                        ChannelMonitorUpdateStatus::PermanentFailure => {
                                log_error!($self.logger, "Closing channel {} due to monitor update ChannelMonitorUpdateStatus::PermanentFailure",
-                                       log_bytes!($chan.context.channel_id()[..]));
+                                       &$chan.context.channel_id());
                                update_maps_on_chan_removal!($self, &$chan.context);
                                let res = Err(MsgHandleErrInternal::from_finish_shutdown(
                                        "ChannelMonitor storage failure".to_owned(), $chan.context.channel_id(),
@@ -2228,7 +2256,7 @@ where
        /// [`Event::FundingGenerationReady::user_channel_id`]: events::Event::FundingGenerationReady::user_channel_id
        /// [`Event::FundingGenerationReady::temporary_channel_id`]: events::Event::FundingGenerationReady::temporary_channel_id
        /// [`Event::ChannelClosed::channel_id`]: events::Event::ChannelClosed::channel_id
-       pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_channel_id: u128, override_config: Option<UserConfig>) -> Result<[u8; 32], APIError> {
+       pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_channel_id: u128, override_config: Option<UserConfig>) -> Result<ChannelId, APIError> {
                if channel_value_satoshis < 1000 {
                        return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
                }
@@ -2279,7 +2307,7 @@ where
                Ok(temporary_channel_id)
        }
 
-       fn list_funded_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<<SP::Target as SignerProvider>::Signer>)) -> bool + Copy>(&self, f: Fn) -> Vec<ChannelDetails> {
+       fn list_funded_channels_with_filter<Fn: FnMut(&(&ChannelId, &Channel<SP>)) -> bool + Copy>(&self, f: Fn) -> Vec<ChannelDetails> {
                // Allocate our best estimate of the number of channels we have in the `res`
                // Vec. Sadly the `short_to_chan_info` map doesn't cover channels without
                // a scid or a scid alias, and the `id_to_peer` shouldn't be used outside
@@ -2405,7 +2433,7 @@ where
        }
 
        /// Helper function that issues the channel close events
-       fn issue_channel_close_events(&self, context: &ChannelContext<<SP::Target as SignerProvider>::Signer>, closure_reason: ClosureReason) {
+       fn issue_channel_close_events(&self, context: &ChannelContext<SP>, closure_reason: ClosureReason) {
                let mut pending_events_lock = self.pending_events.lock().unwrap();
                match context.unbroadcasted_funding() {
                        Some(transaction) => {
@@ -2424,7 +2452,7 @@ where
                }, None));
        }
 
-       fn close_channel_internal(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option<u32>, override_shutdown_script: Option<ShutdownScript>) -> Result<(), APIError> {
+       fn close_channel_internal(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option<u32>, override_shutdown_script: Option<ShutdownScript>) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
                let mut failed_htlcs: Vec<(HTLCSource, PaymentHash)>;
@@ -2516,7 +2544,7 @@ where
        /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background
        /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal
        /// [`SendShutdown`]: crate::events::MessageSendEvent::SendShutdown
-       pub fn close_channel(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey) -> Result<(), APIError> {
+       pub fn close_channel(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey) -> Result<(), APIError> {
                self.close_channel_internal(channel_id, counterparty_node_id, None, None)
        }
 
@@ -2550,7 +2578,7 @@ where
        /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background
        /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal
        /// [`SendShutdown`]: crate::events::MessageSendEvent::SendShutdown
-       pub fn close_channel_with_feerate_and_script(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option<u32>, shutdown_script: Option<ShutdownScript>) -> Result<(), APIError> {
+       pub fn close_channel_with_feerate_and_script(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option<u32>, shutdown_script: Option<ShutdownScript>) -> Result<(), APIError> {
                self.close_channel_internal(channel_id, counterparty_node_id, target_feerate_sats_per_1000_weight, shutdown_script)
        }
 
@@ -2575,7 +2603,7 @@ where
 
        /// `peer_msg` should be set when we receive a message from a peer, but not set when the
        /// user closes, which will be re-exposed as the `ChannelClosed` reason.
-       fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: &PublicKey, peer_msg: Option<&String>, broadcast: bool)
+       fn force_close_channel_with_peer(&self, channel_id: &ChannelId, peer_node_id: &PublicKey, peer_msg: Option<&String>, broadcast: bool)
        -> Result<PublicKey, APIError> {
                let per_peer_state = self.per_peer_state.read().unwrap();
                let peer_state_mutex = per_peer_state.get(peer_node_id)
@@ -2589,33 +2617,33 @@ where
                                ClosureReason::HolderForceClosed
                        };
                        if let hash_map::Entry::Occupied(chan) = peer_state.channel_by_id.entry(channel_id.clone()) {
-                               log_error!(self.logger, "Force-closing channel {}", log_bytes!(channel_id[..]));
+                               log_error!(self.logger, "Force-closing channel {}", &channel_id);
                                self.issue_channel_close_events(&chan.get().context, closure_reason);
                                let mut chan = remove_channel!(self, chan);
                                self.finish_force_close_channel(chan.context.force_shutdown(broadcast));
                                (self.get_channel_update_for_broadcast(&chan).ok(), chan.context.get_counterparty_node_id())
                        } else if let hash_map::Entry::Occupied(chan) = peer_state.outbound_v1_channel_by_id.entry(channel_id.clone()) {
-                               log_error!(self.logger, "Force-closing channel {}", log_bytes!(channel_id[..]));
+                               log_error!(self.logger, "Force-closing channel {}", &channel_id);
                                self.issue_channel_close_events(&chan.get().context, closure_reason);
                                let mut chan = remove_channel!(self, chan);
                                self.finish_force_close_channel(chan.context.force_shutdown(false));
                                // Unfunded channel has no update
                                (None, chan.context.get_counterparty_node_id())
                        } else if let hash_map::Entry::Occupied(chan) = peer_state.inbound_v1_channel_by_id.entry(channel_id.clone()) {
-                               log_error!(self.logger, "Force-closing channel {}", log_bytes!(channel_id[..]));
+                               log_error!(self.logger, "Force-closing channel {}", &channel_id);
                                self.issue_channel_close_events(&chan.get().context, closure_reason);
                                let mut chan = remove_channel!(self, chan);
                                self.finish_force_close_channel(chan.context.force_shutdown(false));
                                // Unfunded channel has no update
                                (None, chan.context.get_counterparty_node_id())
                        } else if peer_state.inbound_channel_request_by_id.remove(channel_id).is_some() {
-                               log_error!(self.logger, "Force-closing channel {}", log_bytes!(channel_id[..]));
+                               log_error!(self.logger, "Force-closing channel {}", &channel_id);
                                // N.B. that we don't send any channel close event here: we
                                // don't have a user_channel_id, and we never sent any opening
                                // events anyway.
                                (None, *peer_node_id)
                        } else {
-                               return Err(APIError::ChannelUnavailable{ err: format!("Channel with id {} not found for the passed counterparty node_id {}", log_bytes!(*channel_id), peer_node_id) });
+                               return Err(APIError::ChannelUnavailable{ err: format!("Channel with id {} not found for the passed counterparty node_id {}", channel_id, peer_node_id) });
                        }
                };
                if let Some(update) = update_opt {
@@ -2628,7 +2656,7 @@ where
                Ok(counterparty_node_id)
        }
 
-       fn force_close_sending_error(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey, broadcast: bool) -> Result<(), APIError> {
+       fn force_close_sending_error(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, broadcast: bool) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
                match self.force_close_channel_with_peer(channel_id, counterparty_node_id, None, broadcast) {
                        Ok(counterparty_node_id) => {
@@ -2654,7 +2682,7 @@ where
        /// rejecting new HTLCs on the given channel. Fails if `channel_id` is unknown to
        /// the manager, or if the `counterparty_node_id` isn't the counterparty of the corresponding
        /// channel.
-       pub fn force_close_broadcasting_latest_txn(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey)
+       pub fn force_close_broadcasting_latest_txn(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey)
        -> Result<(), APIError> {
                self.force_close_sending_error(channel_id, counterparty_node_id, true)
        }
@@ -2665,7 +2693,7 @@ where
        ///
        /// You can always get the latest local transaction(s) to broadcast from
        /// [`ChannelMonitor::get_latest_holder_commitment_txn`].
-       pub fn force_close_without_broadcasting_txn(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey)
+       pub fn force_close_without_broadcasting_txn(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey)
        -> Result<(), APIError> {
                self.force_close_sending_error(channel_id, counterparty_node_id, false)
        }
@@ -2894,9 +2922,9 @@ where
                                        short_channel_id, amt_to_forward, outgoing_cltv_value
                                }, ..
                        } => {
-                               let next_pk = onion_utils::next_hop_packet_pubkey(&self.secp_ctx,
+                               let next_packet_pk = onion_utils::next_hop_pubkey(&self.secp_ctx,
                                        msg.onion_routing_packet.public_key.unwrap(), &shared_secret);
-                               (short_channel_id, amt_to_forward, outgoing_cltv_value, Some(next_pk))
+                               (short_channel_id, amt_to_forward, outgoing_cltv_value, Some(next_packet_pk))
                        },
                        // We'll do receive checks in [`Self::construct_pending_htlc_info`] so we have access to the
                        // inbound channel's state.
@@ -3095,7 +3123,7 @@ where
        ///
        /// [`channel_update`]: msgs::ChannelUpdate
        /// [`internal_closing_signed`]: Self::internal_closing_signed
-       fn get_channel_update_for_broadcast(&self, chan: &Channel<<SP::Target as SignerProvider>::Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
+       fn get_channel_update_for_broadcast(&self, chan: &Channel<SP>) -> Result<msgs::ChannelUpdate, LightningError> {
                if !chan.context.should_announce() {
                        return Err(LightningError {
                                err: "Cannot broadcast a channel_update for a private channel".to_owned(),
@@ -3105,7 +3133,7 @@ where
                if chan.context.get_short_channel_id().is_none() {
                        return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError});
                }
-               log_trace!(self.logger, "Attempting to generate broadcast channel update for channel {}", log_bytes!(chan.context.channel_id()));
+               log_trace!(self.logger, "Attempting to generate broadcast channel update for channel {}", &chan.context.channel_id());
                self.get_channel_update_for_unicast(chan)
        }
 
@@ -3120,8 +3148,8 @@ where
        ///
        /// [`channel_update`]: msgs::ChannelUpdate
        /// [`internal_closing_signed`]: Self::internal_closing_signed
-       fn get_channel_update_for_unicast(&self, chan: &Channel<<SP::Target as SignerProvider>::Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
-               log_trace!(self.logger, "Attempting to generate channel update for channel {}", log_bytes!(chan.context.channel_id()));
+       fn get_channel_update_for_unicast(&self, chan: &Channel<SP>) -> Result<msgs::ChannelUpdate, LightningError> {
+               log_trace!(self.logger, "Attempting to generate channel update for channel {}", &chan.context.channel_id());
                let short_channel_id = match chan.context.get_short_channel_id().or(chan.context.latest_inbound_scid_alias()) {
                        None => return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError}),
                        Some(id) => id,
@@ -3130,8 +3158,8 @@ where
                self.get_channel_update_for_onion(short_channel_id, chan)
        }
 
-       fn get_channel_update_for_onion(&self, short_channel_id: u64, chan: &Channel<<SP::Target as SignerProvider>::Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
-               log_trace!(self.logger, "Generating channel update for channel {}", log_bytes!(chan.context.channel_id()));
+       fn get_channel_update_for_onion(&self, short_channel_id: u64, chan: &Channel<SP>) -> Result<msgs::ChannelUpdate, LightningError> {
+               log_trace!(self.logger, "Generating channel update for channel {}", &chan.context.channel_id());
                let were_node_one = self.our_network_pubkey.serialize()[..] < chan.context.get_counterparty_node_id().serialize()[..];
 
                let enabled = chan.context.is_usable() && match chan.channel_update_status() {
@@ -3182,7 +3210,9 @@ where
                // The top-level caller should hold the total_consistency_lock read lock.
                debug_assert!(self.total_consistency_lock.try_write().is_err());
 
-               log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.hops.first().unwrap().short_channel_id);
+               log_trace!(self.logger,
+                       "Attempting to send payment with payment hash {} along path with next hop {}",
+                       payment_hash, path.hops.first().unwrap().short_channel_id);
                let prng_seed = self.entropy_source.get_secure_random_bytes();
                let session_priv = SecretKey::from_slice(&session_priv_bytes[..]).expect("RNG is busted");
 
@@ -3424,8 +3454,8 @@ where
 
        /// 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(&OutboundV1Channel<<SP::Target as SignerProvider>::Signer>, &Transaction) -> Result<OutPoint, APIError>>(
-               &self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction, find_funding_output: FundingOutput
+       fn funding_transaction_generated_intern<FundingOutput: Fn(&OutboundV1Channel<SP>, &Transaction) -> Result<OutPoint, APIError>>(
+               &self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction, find_funding_output: FundingOutput
        ) -> Result<(), APIError> {
                let per_peer_state = self.per_peer_state.read().unwrap();
                let peer_state_mutex = per_peer_state.get(counterparty_node_id)
@@ -3433,7 +3463,7 @@ where
 
                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
-               let (chan, msg) = match peer_state.outbound_v1_channel_by_id.remove(temporary_channel_id) {
+               let (chan, msg) = match peer_state.outbound_v1_channel_by_id.remove(&temporary_channel_id) {
                        Some(chan) => {
                                let funding_txo = find_funding_output(&chan, &funding_transaction)?;
 
@@ -3462,7 +3492,7 @@ where
                                return Err(APIError::ChannelUnavailable {
                                        err: format!(
                                                "Channel with id {} not found for the passed counterparty node_id {}",
-                                               log_bytes!(*temporary_channel_id), counterparty_node_id),
+                                               temporary_channel_id, counterparty_node_id),
                                })
                        },
                };
@@ -3487,7 +3517,7 @@ where
        }
 
        #[cfg(test)]
-       pub(crate) fn funding_transaction_generated_unchecked(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction, output_index: u16) -> Result<(), APIError> {
+       pub(crate) fn funding_transaction_generated_unchecked(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction, output_index: u16) -> Result<(), APIError> {
                self.funding_transaction_generated_intern(temporary_channel_id, counterparty_node_id, funding_transaction, |_, tx| {
                        Ok(OutPoint { txid: tx.txid(), index: output_index })
                })
@@ -3523,7 +3553,7 @@ where
        ///
        /// [`Event::FundingGenerationReady`]: crate::events::Event::FundingGenerationReady
        /// [`Event::ChannelClosed`]: crate::events::Event::ChannelClosed
-       pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction) -> Result<(), APIError> {
+       pub fn funding_transaction_generated(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
                for inp in funding_transaction.input.iter() {
@@ -3596,7 +3626,7 @@ where
        /// [`ChannelUnavailable`]: APIError::ChannelUnavailable
        /// [`APIMisuseError`]: APIError::APIMisuseError
        pub fn update_partial_channel_config(
-               &self, counterparty_node_id: &PublicKey, channel_ids: &[[u8; 32]], config_update: &ChannelConfigUpdate,
+               &self, counterparty_node_id: &PublicKey, channel_ids: &[ChannelId], config_update: &ChannelConfigUpdate,
        ) -> Result<(), APIError> {
                if config_update.cltv_expiry_delta.map(|delta| delta < MIN_CLTV_EXPIRY_DELTA).unwrap_or(false) {
                        return Err(APIError::APIMisuseError {
@@ -3613,7 +3643,7 @@ where
                for channel_id in channel_ids {
                        if !peer_state.has_channel(channel_id) {
                                return Err(APIError::ChannelUnavailable {
-                                       err: format!("Channel with ID {} was not found for the passed counterparty_node_id {}", log_bytes!(*channel_id), counterparty_node_id),
+                                       err: format!("Channel with ID {} was not found for the passed counterparty_node_id {}", channel_id, counterparty_node_id),
                                });
                        };
                }
@@ -3645,7 +3675,7 @@ where
                                return Err(APIError::ChannelUnavailable {
                                        err: format!(
                                                "Channel with ID {} for passed counterparty_node_id {} disappeared after we confirmed its existence - this should not be reachable!",
-                                               log_bytes!(*channel_id), counterparty_node_id),
+                                               channel_id, counterparty_node_id),
                                });
                        };
                        let mut config = context.config();
@@ -3680,7 +3710,7 @@ where
        /// [`ChannelUnavailable`]: APIError::ChannelUnavailable
        /// [`APIMisuseError`]: APIError::APIMisuseError
        pub fn update_channel_config(
-               &self, counterparty_node_id: &PublicKey, channel_ids: &[[u8; 32]], config: &ChannelConfig,
+               &self, counterparty_node_id: &PublicKey, channel_ids: &[ChannelId], config: &ChannelConfig,
        ) -> Result<(), APIError> {
                return self.update_partial_channel_config(counterparty_node_id, channel_ids, &(*config).into());
        }
@@ -3710,7 +3740,7 @@ where
        /// [`HTLCIntercepted::expected_outbound_amount_msat`]: events::Event::HTLCIntercepted::expected_outbound_amount_msat
        // TODO: when we move to deciding the best outbound channel at forward time, only take
        // `next_node_id` and not `next_hop_channel_id`
-       pub fn forward_intercepted_htlc(&self, intercept_id: InterceptId, next_hop_channel_id: &[u8; 32], next_node_id: PublicKey, amt_to_forward_msat: u64) -> Result<(), APIError> {
+       pub fn forward_intercepted_htlc(&self, intercept_id: InterceptId, next_hop_channel_id: &ChannelId, next_node_id: PublicKey, amt_to_forward_msat: u64) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
                let next_hop_scid = {
@@ -3719,18 +3749,18 @@ where
                                .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", next_node_id) })?;
                        let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
-                       match peer_state.channel_by_id.get(next_hop_channel_id) {
+                       match peer_state.channel_by_id.get(&next_hop_channel_id) {
                                Some(chan) => {
                                        if !chan.context.is_usable() {
                                                return Err(APIError::ChannelUnavailable {
-                                                       err: format!("Channel with id {} not fully established", log_bytes!(*next_hop_channel_id))
+                                                       err: format!("Channel with id {} not fully established", next_hop_channel_id)
                                                })
                                        }
                                        chan.context.get_short_channel_id().unwrap_or(chan.context.outbound_scid_alias())
                                },
                                None => return Err(APIError::ChannelUnavailable {
                                        err: format!("Funded channel with id {} not found for the passed counterparty node_id {}. Channel may still be opening.",
-                                               log_bytes!(*next_hop_channel_id), next_node_id)
+                                               next_hop_channel_id, next_node_id)
                                })
                        }
                };
@@ -3781,6 +3811,7 @@ where
                if let PendingHTLCRouting::Forward { short_channel_id, .. } = payment.forward_info.routing {
                        let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                short_channel_id: payment.prev_short_channel_id,
+                               user_channel_id: Some(payment.prev_user_channel_id),
                                outpoint: payment.prev_funding_outpoint,
                                htlc_id: payment.prev_htlc_id,
                                incoming_packet_shared_secret: payment.forward_info.incoming_shared_secret,
@@ -3828,6 +3859,7 @@ where
 
                                                                                                let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                                                                                        short_channel_id: prev_short_channel_id,
+                                                                                                       user_channel_id: Some(prev_user_channel_id),
                                                                                                        outpoint: prev_funding_outpoint,
                                                                                                        htlc_id: prev_htlc_id,
                                                                                                        incoming_packet_shared_secret: incoming_shared_secret,
@@ -3932,15 +3964,16 @@ where
                                                        for forward_info in pending_forwards.drain(..) {
                                                                match forward_info {
                                                                        HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
-                                                                               prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id: _,
+                                                                               prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
                                                                                forward_info: PendingHTLCInfo {
                                                                                        incoming_shared_secret, payment_hash, outgoing_amt_msat, outgoing_cltv_value,
                                                                                        routing: PendingHTLCRouting::Forward { onion_packet, .. }, skimmed_fee_msat, ..
                                                                                },
                                                                        }) => {
-                                                                               log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id);
+                                                                               log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, &payment_hash, short_chan_id);
                                                                                let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                                                                        short_channel_id: prev_short_channel_id,
+                                                                                       user_channel_id: Some(prev_user_channel_id),
                                                                                        outpoint: prev_funding_outpoint,
                                                                                        htlc_id: prev_htlc_id,
                                                                                        incoming_packet_shared_secret: incoming_shared_secret,
@@ -3953,7 +3986,7 @@ where
                                                                                        &self.logger)
                                                                                {
                                                                                        if let ChannelError::Ignore(msg) = e {
-                                                                                               log_trace!(self.logger, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg);
+                                                                                               log_trace!(self.logger, "Failed to forward HTLC with payment_hash {}: {}", &payment_hash, msg);
                                                                                        } else {
                                                                                                panic!("Stated return value requirements in send_htlc() were not met");
                                                                                        }
@@ -4022,6 +4055,7 @@ where
                                                                let claimable_htlc = ClaimableHTLC {
                                                                        prev_hop: HTLCPreviousHopData {
                                                                                short_channel_id: prev_short_channel_id,
+                                                                               user_channel_id: Some(prev_user_channel_id),
                                                                                outpoint: prev_funding_outpoint,
                                                                                htlc_id: prev_htlc_id,
                                                                                incoming_packet_shared_secret: incoming_shared_secret,
@@ -4051,6 +4085,7 @@ where
                                                                                );
                                                                                failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                                                                                short_channel_id: $htlc.prev_hop.short_channel_id,
+                                                                                               user_channel_id: $htlc.prev_hop.user_channel_id,
                                                                                                outpoint: prev_funding_outpoint,
                                                                                                htlc_id: $htlc.prev_hop.htlc_id,
                                                                                                incoming_packet_shared_secret: $htlc.prev_hop.incoming_packet_shared_secret,
@@ -4091,11 +4126,11 @@ where
                                                                                        });
                                                                                if $purpose != claimable_payment.purpose {
                                                                                        let log_keysend = |keysend| if keysend { "keysend" } else { "non-keysend" };
-                                                                                       log_trace!(self.logger, "Failing new {} HTLC with payment_hash {} as we already had an existing {} HTLC with the same payment hash", log_keysend(is_keysend), log_bytes!(payment_hash.0), log_keysend(!is_keysend));
+                                                                                       log_trace!(self.logger, "Failing new {} HTLC with payment_hash {} as we already had an existing {} HTLC with the same payment hash", log_keysend(is_keysend), &payment_hash, log_keysend(!is_keysend));
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                }
                                                                                if !self.default_configuration.accept_mpp_keysend && is_keysend && !claimable_payment.htlcs.is_empty() {
-                                                                                       log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash and our config states we don't accept MPP keysend", log_bytes!(payment_hash.0));
+                                                                                       log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash and our config states we don't accept MPP keysend", &payment_hash);
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                }
                                                                                if let Some(earlier_fields) = &mut claimable_payment.onion_fields {
@@ -4113,7 +4148,7 @@ where
                                                                                        earliest_expiry = cmp::min(earliest_expiry, htlc.cltv_expiry);
                                                                                        if htlc.total_msat != claimable_htlc.total_msat {
                                                                                                log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the HTLCs had inconsistent total values (eg {} and {})",
-                                                                                                       log_bytes!(payment_hash.0), claimable_htlc.total_msat, htlc.total_msat);
+                                                                                                       &payment_hash, claimable_htlc.total_msat, htlc.total_msat);
                                                                                                total_value = msgs::MAX_VALUE_MSAT;
                                                                                        }
                                                                                        if total_value >= msgs::MAX_VALUE_MSAT { break; }
@@ -4124,7 +4159,7 @@ where
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                } else if total_value - claimable_htlc.sender_intended_value >= claimable_htlc.total_msat {
                                                                                        log_trace!(self.logger, "Failing HTLC with payment_hash {} as payment is already claimable",
-                                                                                               log_bytes!(payment_hash.0));
+                                                                                               &payment_hash);
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                } else if total_value >= claimable_htlc.total_msat {
                                                                                        #[allow(unused_assignments)] {
@@ -4178,7 +4213,7 @@ where
                                                                                                let (payment_preimage, min_final_cltv_expiry_delta) = match inbound_payment::verify(payment_hash, &payment_data, self.highest_seen_timestamp.load(Ordering::Acquire) as u64, &self.inbound_payment_key, &self.logger) {
                                                                                                        Ok(result) => result,
                                                                                                        Err(()) => {
-                                                                                                               log_trace!(self.logger, "Failing new HTLC with payment_hash {} as payment verification failed", log_bytes!(payment_hash.0));
+                                                                                                               log_trace!(self.logger, "Failing new HTLC with payment_hash {} as payment verification failed", &payment_hash);
                                                                                                                fail_htlc!(claimable_htlc, payment_hash);
                                                                                                        }
                                                                                                };
@@ -4186,7 +4221,7 @@ where
                                                                                                        let expected_min_expiry_height = (self.current_best_block().height() + min_final_cltv_expiry_delta as u32) as u64;
                                                                                                        if (cltv_expiry as u64) < expected_min_expiry_height {
                                                                                                                log_trace!(self.logger, "Failing new HTLC with payment_hash {} as its CLTV expiry was too soon (had {}, earliest expected {})",
-                                                                                                                       log_bytes!(payment_hash.0), cltv_expiry, expected_min_expiry_height);
+                                                                                                                       &payment_hash, cltv_expiry, expected_min_expiry_height);
                                                                                                                fail_htlc!(claimable_htlc, payment_hash);
                                                                                                        }
                                                                                                }
@@ -4204,16 +4239,16 @@ where
                                                                        },
                                                                        hash_map::Entry::Occupied(inbound_payment) => {
                                                                                if let OnionPayload::Spontaneous(_) = claimable_htlc.onion_payload {
-                                                                                       log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} because we already have an inbound payment with the same payment hash", log_bytes!(payment_hash.0));
+                                                                                       log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} because we already have an inbound payment with the same payment hash", &payment_hash);
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                }
                                                                                let payment_data = payment_data.unwrap();
                                                                                if inbound_payment.get().payment_secret != payment_data.payment_secret {
-                                                                                       log_trace!(self.logger, "Failing new HTLC with payment_hash {} as it didn't match our expected payment secret.", log_bytes!(payment_hash.0));
+                                                                                       log_trace!(self.logger, "Failing new HTLC with payment_hash {} as it didn't match our expected payment secret.", &payment_hash);
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                } else if inbound_payment.get().min_value_msat.is_some() && payment_data.total_msat < inbound_payment.get().min_value_msat.unwrap() {
                                                                                        log_trace!(self.logger, "Failing new HTLC with payment_hash {} as it didn't match our minimum value (had {}, needed {}).",
-                                                                                               log_bytes!(payment_hash.0), payment_data.total_msat, inbound_payment.get().min_value_msat.unwrap());
+                                                                                               &payment_hash, payment_data.total_msat, inbound_payment.get().min_value_msat.unwrap());
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                } else {
                                                                                        let purpose = events::PaymentPurpose::InvoicePayment {
@@ -4337,21 +4372,21 @@ where
                let _ = self.process_background_events();
        }
 
-       fn update_channel_fee(&self, chan_id: &[u8; 32], chan: &mut Channel<<SP::Target as SignerProvider>::Signer>, new_feerate: u32) -> NotifyOption {
+       fn update_channel_fee(&self, chan_id: &ChannelId, chan: &mut Channel<SP>, new_feerate: u32) -> NotifyOption {
                if !chan.context.is_outbound() { return NotifyOption::SkipPersist; }
                // If the feerate has decreased by less than half, don't bother
                if new_feerate <= chan.context.get_feerate_sat_per_1000_weight() && new_feerate * 2 > chan.context.get_feerate_sat_per_1000_weight() {
                        log_trace!(self.logger, "Channel {} does not qualify for a feerate change from {} to {}.",
-                               log_bytes!(chan_id[..]), chan.context.get_feerate_sat_per_1000_weight(), new_feerate);
+                               &chan_id, chan.context.get_feerate_sat_per_1000_weight(), new_feerate);
                        return NotifyOption::SkipPersist;
                }
                if !chan.context.is_live() {
                        log_trace!(self.logger, "Channel {} does not qualify for a feerate change from {} to {} as it cannot currently be updated (probably the peer is disconnected).",
-                               log_bytes!(chan_id[..]), chan.context.get_feerate_sat_per_1000_weight(), new_feerate);
+                               &chan_id, chan.context.get_feerate_sat_per_1000_weight(), new_feerate);
                        return NotifyOption::SkipPersist;
                }
                log_trace!(self.logger, "Channel {} qualifies for a feerate change from {} to {}.",
-                       log_bytes!(chan_id[..]), chan.context.get_feerate_sat_per_1000_weight(), new_feerate);
+                       &chan_id, chan.context.get_feerate_sat_per_1000_weight(), new_feerate);
 
                chan.queue_update_fee(new_feerate, &self.fee_estimator, &self.logger);
                NotifyOption::DoPersist
@@ -4479,7 +4514,7 @@ where
 
                                                if chan.should_disconnect_peer_awaiting_response() {
                                                        log_debug!(self.logger, "Disconnecting peer {} due to not making any progress on channel {}",
-                                                                       counterparty_node_id, log_bytes!(*chan_id));
+                                                                       counterparty_node_id, chan_id);
                                                        pending_msg_events.push(MessageSendEvent::HandleError {
                                                                node_id: counterparty_node_id,
                                                                action: msgs::ErrorAction::DisconnectPeerWithWarning {
@@ -4495,8 +4530,8 @@ where
                                        });
 
                                        let process_unfunded_channel_tick = |
-                                               chan_id: &[u8; 32],
-                                               chan_context: &mut ChannelContext<<SP::Target as SignerProvider>::Signer>,
+                                               chan_id: &ChannelId,
+                                               chan_context: &mut ChannelContext<SP>,
                                                unfunded_chan_context: &mut UnfundedChannelContext,
                                                pending_msg_events: &mut Vec<MessageSendEvent>,
                                        | {
@@ -4504,7 +4539,7 @@ where
                                                if unfunded_chan_context.should_expire_unfunded_channel() {
                                                        log_error!(self.logger,
                                                                "Force-closing pending channel with ID {} for not establishing in a timely manner",
-                                                               log_bytes!(&chan_id[..]));
+                                                               &chan_id);
                                                        update_maps_on_chan_removal!(self, &chan_context);
                                                        self.issue_channel_close_events(&chan_context, ClosureReason::HolderForceClosed);
                                                        self.finish_force_close_channel(chan_context.force_shutdown(false));
@@ -4529,7 +4564,7 @@ where
 
                                        for (chan_id, req) in peer_state.inbound_channel_request_by_id.iter_mut() {
                                                if { req.ticks_remaining -= 1 ; req.ticks_remaining } <= 0 {
-                                                       log_error!(self.logger, "Force-closing unaccepted inbound channel {} for not accepting in a timely manner", log_bytes!(&chan_id[..]));
+                                                       log_error!(self.logger, "Force-closing unaccepted inbound channel {} for not accepting in a timely manner", &chan_id);
                                                        peer_state.pending_msg_events.push(
                                                                events::MessageSendEvent::HandleError {
                                                                        node_id: counterparty_node_id,
@@ -4686,7 +4721,7 @@ where
        ///
        /// 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<<SP::Target as SignerProvider>::Signer>) -> (u16, Vec<u8>) {
+       fn get_htlc_inbound_temp_fail_err_and_data(&self, desired_err_code: u16, chan: &Channel<SP>) -> (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
@@ -4706,7 +4741,7 @@ where
 
        /// 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<<SP::Target as SignerProvider>::Signer>) -> (u16, Vec<u8>) {
+       fn get_htlc_temp_fail_err_and_data(&self, desired_err_code: u16, scid: u64, chan: &Channel<SP>) -> (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));
@@ -4732,7 +4767,7 @@ where
        // failed backwards or, if they were one of our outgoing HTLCs, then their failure needs to
        // be surfaced to the user.
        fn fail_holding_cell_htlcs(
-               &self, mut htlcs_to_fail: Vec<(HTLCSource, PaymentHash)>, channel_id: [u8; 32],
+               &self, mut htlcs_to_fail: Vec<(HTLCSource, PaymentHash)>, channel_id: ChannelId,
                counterparty_node_id: &PublicKey
        ) {
                let (failure_code, onion_failure_data) = {
@@ -4782,8 +4817,8 @@ where
                                        &self.pending_events, &self.logger)
                                { self.push_pending_forwards_ev(); }
                        },
-                       HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
-                               log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with {:?}", log_bytes!(payment_hash.0), onion_error);
+                       HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint, .. }) => {
+                               log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with {:?}", &payment_hash, onion_error);
                                let err_packet = onion_error.get_encrypted_failure_packet(incoming_packet_shared_secret, phantom_shared_secret);
 
                                let mut push_forward_ev = false;
@@ -4869,20 +4904,22 @@ where
                                        }
                                }
 
+                               let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect();
+                               let sender_intended_value = payment.htlcs.first().map(|htlc| htlc.total_msat);
                                let dup_purpose = claimable_payments.pending_claiming_payments.insert(payment_hash,
                                        ClaimingPayment { amount_msat: payment.htlcs.iter().map(|source| source.value).sum(),
-                                       payment_purpose: payment.purpose, receiver_node_id,
+                                       payment_purpose: payment.purpose, receiver_node_id, htlcs, sender_intended_value
                                });
                                if dup_purpose.is_some() {
                                        debug_assert!(false, "Shouldn't get a duplicate pending claim event ever");
                                        log_error!(self.logger, "Got a duplicate pending claimable event on payment hash {}! Please report this bug",
-                                               log_bytes!(payment_hash.0));
+                                               &payment_hash);
                                }
 
                                if let Some(RecipientOnionFields { ref custom_tlvs, .. }) = payment.onion_fields {
                                        if !custom_tlvs_known && custom_tlvs.iter().any(|(typ, _)| typ % 2 == 0) {
                                                log_info!(self.logger, "Rejecting payment with payment hash {} as we cannot accept payment with unknown even TLVs: {}",
-                                                       log_bytes!(payment_hash.0), log_iter!(custom_tlvs.iter().map(|(typ, _)| typ).filter(|typ| *typ % 2 == 0)));
+                                                       &payment_hash, log_iter!(custom_tlvs.iter().map(|(typ, _)| typ).filter(|typ| *typ % 2 == 0)));
                                                claimable_payments.pending_claiming_payments.remove(&payment_hash);
                                                mem::drop(claimable_payments);
                                                for htlc in payment.htlcs {
@@ -5006,7 +5043,7 @@ where
                                        if let UpdateFulfillCommitFetch::NewClaim { htlc_value_msat, monitor_update } = fulfill_res {
                                                if let Some(action) = completion_action(Some(htlc_value_msat)) {
                                                        log_trace!(self.logger, "Tracking monitor update completion action for channel {}: {:?}",
-                                                               log_bytes!(chan_id), action);
+                                                               &chan_id, action);
                                                        peer_state.monitor_update_blocked_actions.entry(chan_id).or_insert(Vec::new()).push(action);
                                                }
                                                if !during_init {
@@ -5083,12 +5120,18 @@ where
                self.pending_outbound_payments.finalize_claims(sources, &self.pending_events);
        }
 
-       fn claim_funds_internal(&self, source: HTLCSource, payment_preimage: PaymentPreimage, forwarded_htlc_value_msat: Option<u64>, from_onchain: bool, next_channel_id: [u8; 32]) {
+       fn claim_funds_internal(&self, source: HTLCSource, payment_preimage: PaymentPreimage, forwarded_htlc_value_msat: Option<u64>, from_onchain: bool, next_channel_outpoint: OutPoint) {
                match source {
                        HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
                                debug_assert!(self.background_events_processed_since_startup.load(Ordering::Acquire),
                                        "We don't support claim_htlc claims during startup - monitors may not be available yet");
-                               self.pending_outbound_payments.claim_htlc(payment_id, payment_preimage, session_priv, path, from_onchain, &self.pending_events, &self.logger);
+                               let ev_completion_action = EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
+                                       channel_funding_outpoint: next_channel_outpoint,
+                                       counterparty_node_id: path.hops[0].pubkey,
+                               };
+                               self.pending_outbound_payments.claim_htlc(payment_id, payment_preimage,
+                                       session_priv, path, from_onchain, ev_completion_action, &self.pending_events,
+                                       &self.logger);
                        },
                        HTLCSource::PreviousHopData(hop_data) => {
                                let prev_outpoint = hop_data.outpoint;
@@ -5104,7 +5147,7 @@ where
                                                                        fee_earned_msat,
                                                                        claim_from_onchain_tx: from_onchain,
                                                                        prev_channel_id: Some(prev_outpoint.to_channel_id()),
-                                                                       next_channel_id: Some(next_channel_id),
+                                                                       next_channel_id: Some(next_channel_outpoint.to_channel_id()),
                                                                        outbound_amount_forwarded_msat: forwarded_htlc_value_msat,
                                                                },
                                                                downstream_counterparty_and_funding_outpoint: None,
@@ -5129,9 +5172,20 @@ where
                        match action {
                                MonitorUpdateCompletionAction::PaymentClaimed { payment_hash } => {
                                        let payment = self.claimable_payments.lock().unwrap().pending_claiming_payments.remove(&payment_hash);
-                                       if let Some(ClaimingPayment { amount_msat, payment_purpose: purpose, receiver_node_id }) = payment {
+                                       if let Some(ClaimingPayment {
+                                               amount_msat,
+                                               payment_purpose: purpose,
+                                               receiver_node_id,
+                                               htlcs,
+                                               sender_intended_value: sender_intended_total_msat,
+                                       }) = payment {
                                                self.pending_events.lock().unwrap().push_back((events::Event::PaymentClaimed {
-                                                       payment_hash, purpose, amount_msat, receiver_node_id: Some(receiver_node_id),
+                                                       payment_hash,
+                                                       purpose,
+                                                       amount_msat,
+                                                       receiver_node_id: Some(receiver_node_id),
+                                                       htlcs,
+                                                       sender_intended_total_msat,
                                                }, None));
                                        }
                                },
@@ -5150,13 +5204,13 @@ where
        /// Handles a channel reentering a functional state, either due to reconnect or a monitor
        /// update completion.
        fn handle_channel_resumption(&self, pending_msg_events: &mut Vec<MessageSendEvent>,
-               channel: &mut Channel<<SP::Target as SignerProvider>::Signer>, raa: Option<msgs::RevokeAndACK>,
+               channel: &mut Channel<SP>, raa: Option<msgs::RevokeAndACK>,
                commitment_update: Option<msgs::CommitmentUpdate>, order: RAACommitmentOrder,
                pending_forwards: Vec<(PendingHTLCInfo, u64)>, funding_broadcastable: Option<Transaction>,
                channel_ready: Option<msgs::ChannelReady>, announcement_sigs: Option<msgs::AnnouncementSignatures>)
        -> Option<(u64, OutPoint, u128, Vec<(PendingHTLCInfo, u64)>)> {
                log_trace!(self.logger, "Handling channel resumption for channel {} with {} RAA, {} commitment update, {} pending forwards, {}broadcasting funding, {} channel ready, {} announcement",
-                       log_bytes!(channel.context.channel_id()),
+                       &channel.context.channel_id(),
                        if raa.is_some() { "an" } else { "no" },
                        if commitment_update.is_some() { "a" } else { "no" }, pending_forwards.len(),
                        if funding_broadcastable.is_some() { "" } else { "not " },
@@ -5284,7 +5338,7 @@ where
        ///
        /// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
        /// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
-       pub fn accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
+       pub fn accept_inbound_channel(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
                self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, false, user_channel_id)
        }
 
@@ -5306,11 +5360,11 @@ where
        ///
        /// [`Event::OpenChannelRequest`]: events::Event::OpenChannelRequest
        /// [`Event::ChannelClosed::user_channel_id`]: events::Event::ChannelClosed::user_channel_id
-       pub fn accept_inbound_channel_from_trusted_peer_0conf(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
+       pub fn accept_inbound_channel_from_trusted_peer_0conf(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, user_channel_id: u128) -> Result<(), APIError> {
                self.do_accept_inbound_channel(temporary_channel_id, counterparty_node_id, true, user_channel_id)
        }
 
-       fn do_accept_inbound_channel(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, accept_0conf: bool, user_channel_id: u128) -> Result<(), APIError> {
+       fn do_accept_inbound_channel(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, accept_0conf: bool, user_channel_id: u128) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
                let peers_without_funded_channels =
@@ -5332,7 +5386,7 @@ where
                                InboundV1Channel::new(&self.fee_estimator, &self.entropy_source, &self.signer_provider,
                                        counterparty_node_id.clone(), &self.channel_type_features(), &peer_state.latest_features,
                                        &unaccepted_channel.open_channel_msg, user_channel_id, &self.default_configuration, best_block_height,
-                                       &self.logger, /*outbound_scid_alias=*/0, accept_0conf).map_err(|e| APIError::ChannelUnavailable { err: e.to_string() })
+                                       &self.logger, accept_0conf).map_err(|e| APIError::ChannelUnavailable { err: e.to_string() })
                        }
                        _ => Err(APIError::APIMisuseError { err: "No such channel awaiting to be accepted.".to_owned() })
                }?;
@@ -5385,7 +5439,7 @@ where
        /// The filter is called for each peer and provided with the number of unfunded, inbound, and
        /// non-0-conf channels we have with the peer.
        fn peers_without_funded_channels<Filter>(&self, maybe_count_peer: Filter) -> usize
-       where Filter: Fn(&PeerState<<SP::Target as SignerProvider>::Signer>) -> bool {
+       where Filter: Fn(&PeerState<SP>) -> bool {
                let mut peers_without_funded_channels = 0;
                let best_block_height = self.best_block.read().unwrap().height();
                {
@@ -5403,7 +5457,7 @@ where
        }
 
        fn unfunded_channel_count(
-               peer: &PeerState<<SP::Target as SignerProvider>::Signer>, best_block_height: u32
+               peer: &PeerState<SP>, best_block_height: u32
        ) -> usize {
                let mut num_unfunded_channels = 0;
                for (_, chan) in peer.channel_by_id.iter() {
@@ -5495,7 +5549,7 @@ where
                let user_channel_id = u128::from_be_bytes(random_bytes);
                let mut channel = match InboundV1Channel::new(&self.fee_estimator, &self.entropy_source, &self.signer_provider,
                        counterparty_node_id.clone(), &self.channel_type_features(), &peer_state.latest_features, msg, user_channel_id,
-                       &self.default_configuration, best_block_height, &self.logger, /*outbound_scid_alias=*/0, /*is_0conf=*/false)
+                       &self.default_configuration, best_block_height, &self.logger, /*is_0conf=*/false)
                {
                        Err(e) => {
                                return Err(MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id));
@@ -5676,7 +5730,7 @@ where
                                let announcement_sigs_opt = try_chan_entry!(self, chan.get_mut().channel_ready(&msg, &self.node_signer,
                                        self.genesis_hash.clone(), &self.default_configuration, &self.best_block.read().unwrap(), &self.logger), chan);
                                if let Some(announcement_sigs) = announcement_sigs_opt {
-                                       log_trace!(self.logger, "Sending announcement_signatures for channel {}", log_bytes!(chan.get().context.channel_id()));
+                                       log_trace!(self.logger, "Sending announcement_signatures for channel {}", &chan.get().context.channel_id());
                                        peer_state.pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
                                                node_id: counterparty_node_id.clone(),
                                                msg: announcement_sigs,
@@ -5687,7 +5741,7 @@ where
                                        // counterparty's announcement_signatures. Thus, we only bother to send a
                                        // channel_update here if the channel is not public, i.e. we're not sending an
                                        // announcement_signatures.
-                                       log_trace!(self.logger, "Sending private initial channel_update for our counterparty on channel {}", log_bytes!(chan.get().context.channel_id()));
+                                       log_trace!(self.logger, "Sending private initial channel_update for our counterparty on channel {}", &chan.get().context.channel_id());
                                        if let Ok(msg) = self.get_channel_update_for_unicast(chan.get()) {
                                                peer_state.pending_msg_events.push(events::MessageSendEvent::SendChannelUpdate {
                                                        node_id: counterparty_node_id.clone(),
@@ -5721,13 +5775,13 @@ where
                        // TODO(dunxen): Fix this duplication when we switch to a single map with enums as per
                        // https://github.com/lightningdevkit/rust-lightning/issues/2422
                        if let hash_map::Entry::Occupied(chan_entry) = peer_state.outbound_v1_channel_by_id.entry(msg.channel_id.clone()) {
-                               log_error!(self.logger, "Immediately closing unfunded channel {} as peer asked to cooperatively shut it down (which is unnecessary)", log_bytes!(&msg.channel_id[..]));
+                               log_error!(self.logger, "Immediately closing unfunded channel {} as peer asked to cooperatively shut it down (which is unnecessary)", &msg.channel_id);
                                self.issue_channel_close_events(&chan_entry.get().context, ClosureReason::CounterpartyCoopClosedUnfundedChannel);
                                let mut chan = remove_channel!(self, chan_entry);
                                self.finish_force_close_channel(chan.context.force_shutdown(false));
                                return Ok(());
                        } else if let hash_map::Entry::Occupied(chan_entry) = peer_state.inbound_v1_channel_by_id.entry(msg.channel_id.clone()) {
-                               log_error!(self.logger, "Immediately closing unfunded channel {} as peer asked to cooperatively shut it down (which is unnecessary)", log_bytes!(&msg.channel_id[..]));
+                               log_error!(self.logger, "Immediately closing unfunded channel {} as peer asked to cooperatively shut it down (which is unnecessary)", &msg.channel_id);
                                self.issue_channel_close_events(&chan_entry.get().context, ClosureReason::CounterpartyCoopClosedUnfundedChannel);
                                let mut chan = remove_channel!(self, chan_entry);
                                self.finish_force_close_channel(chan.context.force_shutdown(false));
@@ -5735,7 +5789,7 @@ where
                        } else if let hash_map::Entry::Occupied(mut chan_entry) = peer_state.channel_by_id.entry(msg.channel_id.clone()) {
                                if !chan_entry.get().received_shutdown() {
                                        log_info!(self.logger, "Received a shutdown message from our counterparty for channel {}{}.",
-                                               log_bytes!(msg.channel_id),
+                                               &msg.channel_id,
                                                if chan_entry.get().sent_shutdown() { " after we initiated shutdown" } else { "" });
                                }
 
@@ -5849,7 +5903,7 @@ where
                                                        chan.get().context.config().accept_underpaying_htlcs, next_packet_pk_opt),
                                        Err(e) => PendingHTLCStatus::Fail(e)
                                };
-                               let create_pending_htlc_status = |chan: &Channel<<SP::Target as SignerProvider>::Signer>, pending_forward_info: PendingHTLCStatus, error_code: u16| {
+                               let create_pending_htlc_status = |chan: &Channel<SP>, 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.
@@ -5879,6 +5933,7 @@ where
        }
 
        fn internal_update_fulfill_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> Result<(), MsgHandleErrInternal> {
+               let funding_txo;
                let (htlc_source, forwarded_htlc_value) = {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        let peer_state_mutex = per_peer_state.get(counterparty_node_id)
@@ -5890,12 +5945,14 @@ where
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
-                                       try_chan_entry!(self, chan.get_mut().update_fulfill_htlc(&msg), chan)
+                                       let res = try_chan_entry!(self, chan.get_mut().update_fulfill_htlc(&msg), chan);
+                                       funding_txo = chan.get().context.get_funding_txo().expect("We won't accept a fulfill until funded");
+                                       res
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id))
                        }
                };
-               self.claim_funds_internal(htlc_source, msg.payment_preimage.clone(), Some(forwarded_htlc_value), false, msg.channel_id);
+               self.claim_funds_internal(htlc_source, msg.payment_preimage.clone(), Some(forwarded_htlc_value), false, funding_txo);
                Ok(())
        }
 
@@ -6006,6 +6063,7 @@ where
                                                                                log_info!(self.logger, "Failed to forward incoming HTLC: detected duplicate intercepted payment over short channel id {}", scid);
                                                                                let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                                                                        short_channel_id: prev_short_channel_id,
+                                                                                       user_channel_id: Some(prev_user_channel_id),
                                                                                        outpoint: prev_funding_outpoint,
                                                                                        htlc_id: prev_htlc_id,
                                                                                        incoming_packet_shared_secret: forward_info.incoming_shared_secret,
@@ -6068,7 +6126,7 @@ where
        /// completes. Note that this needs to happen in the same [`PeerState`] mutex as any release of
        /// the [`ChannelMonitorUpdate`] in question.
        fn raa_monitor_updates_held(&self,
-               actions_blocking_raa_monitor_updates: &BTreeMap<[u8; 32], Vec<RAAMonitorUpdateBlockingAction>>,
+               actions_blocking_raa_monitor_updates: &BTreeMap<ChannelId, Vec<RAAMonitorUpdateBlockingAction>>,
                channel_funding_outpoint: OutPoint, counterparty_node_id: PublicKey
        ) -> bool {
                actions_blocking_raa_monitor_updates
@@ -6092,10 +6150,18 @@ where
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
-                                       let funding_txo = chan.get().context.get_funding_txo();
-                                       let (htlcs_to_fail, monitor_update_opt) = try_chan_entry!(self, chan.get_mut().revoke_and_ack(&msg, &self.fee_estimator, &self.logger), chan);
+                                       let funding_txo_opt = chan.get().context.get_funding_txo();
+                                       let mon_update_blocked = if let Some(funding_txo) = funding_txo_opt {
+                                               self.raa_monitor_updates_held(
+                                                       &peer_state.actions_blocking_raa_monitor_updates, funding_txo,
+                                                       *counterparty_node_id)
+                                       } else { false };
+                                       let (htlcs_to_fail, monitor_update_opt) = try_chan_entry!(self,
+                                               chan.get_mut().revoke_and_ack(&msg, &self.fee_estimator, &self.logger, mon_update_blocked), chan);
                                        let res = if let Some(monitor_update) = monitor_update_opt {
-                                               handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update,
+                                               let funding_txo = funding_txo_opt
+                                                       .expect("Funding outpoint must have been set for RAA handling to succeed");
+                                               handle_new_monitor_update!(self, funding_txo, monitor_update,
                                                        peer_state_lock, peer_state, per_peer_state, chan).map(|_| ())
                                        } else { Ok(()) };
                                        (htlcs_to_fail, res)
@@ -6187,7 +6253,7 @@ where
                                if were_node_one == msg_from_node_one {
                                        return Ok(NotifyOption::SkipPersist);
                                } else {
-                                       log_debug!(self.logger, "Received channel_update for channel {}.", log_bytes!(chan_id));
+                                       log_debug!(self.logger, "Received channel_update for channel {}.", &chan_id);
                                        try_chan_entry!(self, chan.get_mut().channel_update(&msg), chan);
                                }
                        },
@@ -6269,10 +6335,10 @@ where
                                match monitor_event {
                                        MonitorEvent::HTLCEvent(htlc_update) => {
                                                if let Some(preimage) = htlc_update.payment_preimage {
-                                                       log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0));
-                                                       self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint.to_channel_id());
+                                                       log_trace!(self.logger, "Claiming HTLC with preimage {} from our monitor", &preimage);
+                                                       self.claim_funds_internal(htlc_update.source, preimage, htlc_update.htlc_value_satoshis.map(|v| v * 1000), true, funding_outpoint);
                                                } else {
-                                                       log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
+                                                       log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", &htlc_update.payment_hash);
                                                        let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
                                                        let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
                                                        self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);
@@ -6371,7 +6437,7 @@ where
                                                if let Some(monitor_update) = monitor_opt {
                                                        has_monitor_update = true;
 
-                                                       let channel_id: [u8; 32] = *channel_id;
+                                                       let channel_id: ChannelId = *channel_id;
                                                        let res = handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update,
                                                                peer_state_lock, peer_state, per_peer_state, chan, MANUALLY_REMOVING,
                                                                peer_state.channel_by_id.remove(&channel_id));
@@ -6705,7 +6771,7 @@ where
                                        // blocking monitor updates for this channel. If we do, release the monitor
                                        // update(s) when those blockers complete.
                                        log_trace!(self.logger, "Delaying monitor unlock for channel {} as another channel's mon update needs to complete first",
-                                               log_bytes!(&channel_funding_outpoint.to_channel_id()[..]));
+                                               &channel_funding_outpoint.to_channel_id());
                                        break;
                                }
 
@@ -6713,7 +6779,7 @@ where
                                        debug_assert_eq!(chan.get().context.get_funding_txo().unwrap(), channel_funding_outpoint);
                                        if let Some((monitor_update, further_update_exists)) = chan.get_mut().unblock_next_blocked_monitor_update() {
                                                log_debug!(self.logger, "Unlocking monitor updating for channel {} and updating monitor",
-                                                       log_bytes!(&channel_funding_outpoint.to_channel_id()[..]));
+                                                       &channel_funding_outpoint.to_channel_id());
                                                if let Err(e) = handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update,
                                                        peer_state_lck, peer_state, per_peer_state, chan)
                                                {
@@ -6726,7 +6792,7 @@ where
                                                }
                                        } else {
                                                log_trace!(self.logger, "Unlocked monitor updating for channel {} without monitors to update",
-                                                       log_bytes!(&channel_funding_outpoint.to_channel_id()[..]));
+                                                       &channel_funding_outpoint.to_channel_id());
                                        }
                                }
                        } else {
@@ -6999,7 +7065,7 @@ where
        /// Calls a function which handles an on-chain event (blocks dis/connected, transactions
        /// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by
        /// the function.
-       fn do_chain_event<FN: Fn(&mut Channel<<SP::Target as SignerProvider>::Signer>) -> Result<(Option<msgs::ChannelReady>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), ClosureReason>>
+       fn do_chain_event<FN: Fn(&mut Channel<SP>) -> 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.
@@ -7024,7 +7090,7 @@ where
                                                if let Some(channel_ready) = channel_ready_opt {
                                                        send_channel_ready!(self, pending_msg_events, channel, channel_ready);
                                                        if channel.context.is_usable() {
-                                                               log_trace!(self.logger, "Sending channel_ready with private initial channel_update for our counterparty on channel {}", log_bytes!(channel.context.channel_id()));
+                                                               log_trace!(self.logger, "Sending channel_ready with private initial channel_update for our counterparty on channel {}", &channel.context.channel_id());
                                                                if let Ok(msg) = self.get_channel_update_for_unicast(channel) {
                                                                        pending_msg_events.push(events::MessageSendEvent::SendChannelUpdate {
                                                                                node_id: channel.context.get_counterparty_node_id(),
@@ -7032,7 +7098,7 @@ where
                                                                        });
                                                                }
                                                        } else {
-                                                               log_trace!(self.logger, "Sending channel_ready WITHOUT channel_update for {}", log_bytes!(channel.context.channel_id()));
+                                                               log_trace!(self.logger, "Sending channel_ready WITHOUT channel_update for {}", &channel.context.channel_id());
                                                        }
                                                }
 
@@ -7042,7 +7108,7 @@ where
                                                }
 
                                                if let Some(announcement_sigs) = announcement_sigs {
-                                                       log_trace!(self.logger, "Sending announcement_signatures for channel {}", log_bytes!(channel.context.channel_id()));
+                                                       log_trace!(self.logger, "Sending announcement_signatures for channel {}", &channel.context.channel_id());
                                                        pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
                                                                node_id: channel.context.get_counterparty_node_id(),
                                                                msg: announcement_sigs,
@@ -7124,6 +7190,7 @@ where
                                if height >= htlc.forward_info.outgoing_cltv_value - HTLC_FAIL_BACK_BUFFER {
                                        let prev_hop_data = HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                                short_channel_id: htlc.prev_short_channel_id,
+                                               user_channel_id: Some(htlc.prev_user_channel_id),
                                                htlc_id: htlc.prev_htlc_id,
                                                incoming_packet_shared_secret: htlc.forward_info.incoming_shared_secret,
                                                phantom_shared_secret: None,
@@ -7489,8 +7556,48 @@ where
        fn handle_error(&self, counterparty_node_id: &PublicKey, msg: &msgs::ErrorMessage) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
-               if msg.channel_id == [0; 32] {
-                       let channel_ids: Vec<[u8; 32]> = {
+               match &msg.data as &str {
+                       "cannot co-op close channel w/ active htlcs"|
+                       "link failed to shutdown" =>
+                       {
+                               // LND hasn't properly handled shutdown messages ever, and force-closes any time we
+                               // send one while HTLCs are still present. The issue is tracked at
+                               // https://github.com/lightningnetwork/lnd/issues/6039 and has had multiple patches
+                               // to fix it but none so far have managed to land upstream. The issue appears to be
+                               // very low priority for the LND team despite being marked "P1".
+                               // We're not going to bother handling this in a sensible way, instead simply
+                               // repeating the Shutdown message on repeat until morale improves.
+                               if !msg.channel_id.is_zero() {
+                                       let per_peer_state = self.per_peer_state.read().unwrap();
+                                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
+                                       if peer_state_mutex_opt.is_none() { return; }
+                                       let mut peer_state = peer_state_mutex_opt.unwrap().lock().unwrap();
+                                       if let Some(chan) = peer_state.channel_by_id.get(&msg.channel_id) {
+                                               if let Some(msg) = chan.get_outbound_shutdown() {
+                                                       peer_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
+                                                               node_id: *counterparty_node_id,
+                                                               msg,
+                                                       });
+                                               }
+                                               peer_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
+                                                       node_id: *counterparty_node_id,
+                                                       action: msgs::ErrorAction::SendWarningMessage {
+                                                               msg: msgs::WarningMessage {
+                                                                       channel_id: msg.channel_id,
+                                                                       data: "You appear to be exhibiting LND bug 6039, we'll keep sending you shutdown messages until you handle them correctly".to_owned()
+                                                               },
+                                                               log_level: Level::Trace,
+                                                       }
+                                               });
+                                       }
+                               }
+                               return;
+                       }
+                       _ => {}
+               }
+
+               if msg.channel_id.is_zero() {
+                       let channel_ids: Vec<ChannelId> = {
                                let per_peer_state = self.per_peer_state.read().unwrap();
                                let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
                                if peer_state_mutex_opt.is_none() { return; }
@@ -7712,7 +7819,7 @@ impl Writeable for ChannelDetails {
 
 impl Readable for ChannelDetails {
        fn read<R: Read>(reader: &mut R) -> Result<Self, DecodeError> {
-               _init_and_read_tlv_fields!(reader, {
+               _init_and_read_len_prefixed_tlv_fields!(reader, {
                        (1, inbound_scid_alias, option),
                        (2, channel_id, required),
                        (3, channel_type, option),
@@ -7899,7 +8006,8 @@ impl_writeable_tlv_based!(HTLCPreviousHopData, {
        (1, phantom_shared_secret, option),
        (2, outpoint, required),
        (4, htlc_id, required),
-       (6, incoming_packet_shared_secret, required)
+       (6, incoming_packet_shared_secret, required),
+       (7, user_channel_id, option),
 });
 
 impl Writeable for ClaimableHTLC {
@@ -7925,7 +8033,7 @@ impl Writeable for ClaimableHTLC {
 
 impl Readable for ClaimableHTLC {
        fn read<R: Read>(reader: &mut R) -> Result<Self, DecodeError> {
-               _init_and_read_tlv_fields!(reader, {
+               _init_and_read_len_prefixed_tlv_fields!(reader, {
                        (0, prev_hop, required),
                        (1, total_msat, option),
                        (2, value_ser, required),
@@ -8503,13 +8611,13 @@ where
 
                let channel_count: u64 = Readable::read(reader)?;
                let mut funding_txo_set = HashSet::with_capacity(cmp::min(channel_count as usize, 128));
-               let mut peer_channels: HashMap<PublicKey, HashMap<[u8; 32], Channel<<SP::Target as SignerProvider>::Signer>>> = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
+               let mut peer_channels: HashMap<PublicKey, HashMap<ChannelId, Channel<SP>>> = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
                let mut id_to_peer = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
                let mut short_to_chan_info = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
                let mut channel_closures = VecDeque::new();
                let mut close_background_events = Vec::new();
                for _ in 0..channel_count {
-                       let mut channel: Channel<<SP::Target as SignerProvider>::Signer> = Channel::read(reader, (
+                       let mut channel: Channel<SP> = Channel::read(reader, (
                                &args.entropy_source, &args.signer_provider, best_block_height, &provided_channel_type_features(&args.default_config)
                        ))?;
                        let funding_txo = channel.context.get_funding_txo().ok_or(DecodeError::InvalidValue)?;
@@ -8523,7 +8631,7 @@ where
                                        log_error!(args.logger, "A ChannelManager is stale compared to the current ChannelMonitor!");
                                        log_error!(args.logger, " The channel will be force-closed and the latest commitment transaction from the ChannelMonitor broadcast.");
                                        log_error!(args.logger, " The ChannelMonitor for channel {} is at update_id {} but the ChannelManager is at update_id {}.",
-                                               log_bytes!(channel.context.channel_id()), monitor.get_latest_update_id(), channel.context.get_latest_monitor_update_id());
+                                               &channel.context.channel_id(), monitor.get_latest_update_id(), channel.context.get_latest_monitor_update_id());
                                        let (monitor_update, mut new_failed_htlcs) = channel.context.force_shutdown(true);
                                        if let Some((counterparty_node_id, funding_txo, update)) = monitor_update {
                                                close_background_events.push(BackgroundEvent::MonitorUpdateRegeneratedOnStartup {
@@ -8553,13 +8661,13 @@ where
                                                        // 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.context.channel_id()), log_bytes!(payment_hash.0));
+                                                               &channel.context.channel_id(), &payment_hash);
                                                        failed_htlcs.push((channel_htlc_source.clone(), *payment_hash, channel.context.get_counterparty_node_id(), channel.context.channel_id()));
                                                }
                                        }
                                } else {
                                        log_info!(args.logger, "Successfully loaded channel {} at update_id {} against monitor at update id {}",
-                                               log_bytes!(channel.context.channel_id()), channel.context.get_latest_monitor_update_id(),
+                                               &channel.context.channel_id(), channel.context.get_latest_monitor_update_id(),
                                                monitor.get_latest_update_id());
                                        if let Some(short_channel_id) = channel.context.get_short_channel_id() {
                                                short_to_chan_info.insert(short_channel_id, (channel.context.get_counterparty_node_id(), channel.context.channel_id()));
@@ -8592,7 +8700,7 @@ where
                                        channel_capacity_sats: Some(channel.context.get_value_satoshis()),
                                }, None));
                        } else {
-                               log_error!(args.logger, "Missing ChannelMonitor for channel {} needed by ChannelManager.", log_bytes!(channel.context.channel_id()));
+                               log_error!(args.logger, "Missing ChannelMonitor for channel {} needed by ChannelManager.", &channel.context.channel_id());
                                log_error!(args.logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,");
                                log_error!(args.logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!");
                                log_error!(args.logger, " Without the ChannelMonitor we cannot continue without risking funds.");
@@ -8604,7 +8712,7 @@ where
                for (funding_txo, _) in args.channel_monitors.iter() {
                        if !funding_txo_set.contains(funding_txo) {
                                log_info!(args.logger, "Queueing monitor update to ensure missing channel {} is force closed",
-                                       log_bytes!(funding_txo.to_channel_id()));
+                                       &funding_txo.to_channel_id());
                                let monitor_update = ChannelMonitorUpdate {
                                        update_id: CLOSED_CHANNEL_UPDATE_ID,
                                        updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast: true }],
@@ -8654,7 +8762,7 @@ where
                };
 
                let peer_count: u64 = Readable::read(reader)?;
-               let mut per_peer_state = HashMap::with_capacity(cmp::min(peer_count as usize, MAX_ALLOC_SIZE/mem::size_of::<(PublicKey, Mutex<PeerState<<SP::Target as SignerProvider>::Signer>>)>()));
+               let mut per_peer_state = HashMap::with_capacity(cmp::min(peer_count as usize, MAX_ALLOC_SIZE/mem::size_of::<(PublicKey, Mutex<PeerState<SP>>)>()));
                for _ in 0..peer_count {
                        let peer_pubkey = Readable::read(reader)?;
                        let peer_chans = peer_channels.remove(&peer_pubkey).unwrap_or(HashMap::new());
@@ -8788,7 +8896,7 @@ where
                                $chan_in_flight_upds.retain(|upd| upd.update_id > $monitor.get_latest_update_id());
                                for update in $chan_in_flight_upds.iter() {
                                        log_trace!(args.logger, "Replaying ChannelMonitorUpdate {} for {}channel {}",
-                                               update.update_id, $channel_info_log, log_bytes!($funding_txo.to_channel_id()));
+                                               update.update_id, $channel_info_log, &$funding_txo.to_channel_id());
                                        max_in_flight_update_id = cmp::max(max_in_flight_update_id, update.update_id);
                                        pending_background_events.push(
                                                BackgroundEvent::MonitorUpdateRegeneratedOnStartup {
@@ -8836,7 +8944,7 @@ where
                                        // If the channel is ahead of the monitor, return InvalidValue:
                                        log_error!(args.logger, "A ChannelMonitor is stale compared to the current ChannelManager! This indicates a potentially-critical violation of the chain::Watch API!");
                                        log_error!(args.logger, " The ChannelMonitor for channel {} is at update_id {} with update_id through {} in-flight",
-                                               log_bytes!(chan.context.channel_id()), monitor.get_latest_update_id(), max_in_flight_update_id);
+                                               &chan.context.channel_id(), monitor.get_latest_update_id(), max_in_flight_update_id);
                                        log_error!(args.logger, " but the ChannelManager is at update_id {}.", chan.get_latest_unblocked_monitor_update_id());
                                        log_error!(args.logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,");
                                        log_error!(args.logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!");
@@ -8862,7 +8970,7 @@ where
                                } else {
                                        log_error!(args.logger, "A ChannelMonitor is missing even though we have in-flight updates for it! This indicates a potentially-critical violation of the chain::Watch API!");
                                        log_error!(args.logger, " The ChannelMonitor for channel {} is missing.",
-                                               log_bytes!(funding_txo.to_channel_id()));
+                                               &funding_txo.to_channel_id());
                                        log_error!(args.logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,");
                                        log_error!(args.logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!");
                                        log_error!(args.logger, " Without the latest ChannelMonitor we cannot continue without risking funds.");
@@ -8906,7 +9014,7 @@ where
                                                                hash_map::Entry::Occupied(mut entry) => {
                                                                        let newly_added = entry.get_mut().insert(session_priv_bytes, &path);
                                                                        log_info!(args.logger, "{} a pending payment path for {} msat for session priv {} on an existing pending payment with payment hash {}",
-                                                                               if newly_added { "Added" } else { "Had" }, path_amt, log_bytes!(session_priv_bytes), log_bytes!(htlc.payment_hash.0));
+                                                                               if newly_added { "Added" } else { "Had" }, path_amt, log_bytes!(session_priv_bytes), &htlc.payment_hash);
                                                                },
                                                                hash_map::Entry::Vacant(entry) => {
                                                                        let path_fee = path.fee_msat();
@@ -8926,7 +9034,7 @@ where
                                                                                starting_block_height: best_block_height,
                                                                        });
                                                                        log_info!(args.logger, "Added a pending payment for {} msat with payment hash {} for path with session priv {}",
-                                                                               path_amt, log_bytes!(htlc.payment_hash.0),  log_bytes!(session_priv_bytes));
+                                                                               path_amt, &htlc.payment_hash,  log_bytes!(session_priv_bytes));
                                                                }
                                                        }
                                                }
@@ -8948,7 +9056,7 @@ where
                                                                                if let HTLCForwardInfo::AddHTLC(htlc_info) = forward {
                                                                                        if pending_forward_matches_htlc(&htlc_info) {
                                                                                                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()));
+                                                                                                       &htlc.payment_hash, &monitor.get_funding_txo().0.to_channel_id());
                                                                                                false
                                                                                        } else { true }
                                                                                } else { true }
@@ -8958,7 +9066,7 @@ where
                                                                pending_intercepted_htlcs.as_mut().unwrap().retain(|intercepted_id, htlc_info| {
                                                                        if pending_forward_matches_htlc(&htlc_info) {
                                                                                log_info!(args.logger, "Removing pending intercepted 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()));
+                                                                                       &htlc.payment_hash, &monitor.get_funding_txo().0.to_channel_id());
                                                                                pending_events_read.retain(|(event, _)| {
                                                                                        if let Event::HTLCIntercepted { intercept_id: ev_id, .. } = event {
                                                                                                intercepted_id != ev_id
@@ -8979,7 +9087,13 @@ where
                                                                        // generating a `PaymentPathSuccessful` event but regenerating
                                                                        // it and the `PaymentSent` on every restart until the
                                                                        // `ChannelMonitor` is removed.
-                                                                       pending_outbounds.claim_htlc(payment_id, preimage, session_priv, path, false, &pending_events, &args.logger);
+                                                                       let compl_action =
+                                                                               EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
+                                                                                       channel_funding_outpoint: monitor.get_funding_txo().0,
+                                                                                       counterparty_node_id: path.hops[0].pubkey,
+                                                                               };
+                                                                       pending_outbounds.claim_htlc(payment_id, preimage, session_priv,
+                                                                               path, false, compl_action, &pending_events, &args.logger);
                                                                        pending_events_read = pending_events.into_inner().unwrap();
                                                                }
                                                        },
@@ -9000,7 +9114,7 @@ where
                                                                        // downstream chan is closed (because we don't have a
                                                                        // channel_id -> peer map entry).
                                                                        counterparty_opt.is_none(),
-                                                                       monitor.get_funding_txo().0.to_channel_id()))
+                                                                       monitor.get_funding_txo().0))
                                                        } else { None }
                                                } else {
                                                        // If it was an outbound payment, we've handled it above - if a preimage
@@ -9071,7 +9185,7 @@ where
                                                                        None => match inbound_payment::verify(payment_hash, &hop_data, 0, &expanded_inbound_key, &args.logger) {
                                                                                Ok((payment_preimage, _)) => payment_preimage,
                                                                                Err(()) => {
-                                                                                       log_error!(args.logger, "Failed to read claimable payment data for HTLC with payment hash {} - was not a pending inbound payment and didn't match our payment key", log_bytes!(payment_hash.0));
+                                                                                       log_error!(args.logger, "Failed to read claimable payment data for HTLC with payment hash {} - was not a pending inbound payment and didn't match our payment key", &payment_hash);
                                                                                        return Err(DecodeError::InvalidValue);
                                                                                }
                                                                        }
@@ -9138,7 +9252,7 @@ where
                for (_, monitor) in args.channel_monitors.iter() {
                        for (payment_hash, payment_preimage) in monitor.get_stored_preimages() {
                                if let Some(payment) = claimable_payments.remove(&payment_hash) {
-                                       log_info!(args.logger, "Re-claiming HTLCs with payment hash {} as we've released the preimage to a ChannelMonitor!", log_bytes!(payment_hash.0));
+                                       log_info!(args.logger, "Re-claiming HTLCs with payment hash {} as we've released the preimage to a ChannelMonitor!", &payment_hash);
                                        let mut claimable_amt_msat = 0;
                                        let mut receiver_node_id = Some(our_network_pubkey);
                                        let phantom_shared_secret = payment.htlcs[0].prev_hop.phantom_shared_secret;
@@ -9147,7 +9261,7 @@ where
                                                        .expect("Failed to get node_id for phantom node recipient");
                                                receiver_node_id = Some(phantom_pubkey)
                                        }
-                                       for claimable_htlc in payment.htlcs {
+                                       for claimable_htlc in &payment.htlcs {
                                                claimable_amt_msat += claimable_htlc.value;
 
                                                // Add a holding-cell claim of the payment to the Channel, which should be
@@ -9183,6 +9297,8 @@ where
                                                payment_hash,
                                                purpose: payment.purpose,
                                                amount_msat: claimable_amt_msat,
+                                               htlcs: payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(),
+                                               sender_intended_total_msat: payment.htlcs.first().map(|htlc| htlc.total_msat),
                                        }, None));
                                }
                        }
@@ -9269,12 +9385,12 @@ where
                        channel_manager.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
                }
 
-               for (source, preimage, downstream_value, downstream_closed, downstream_chan_id) in pending_claims_to_replay {
+               for (source, preimage, downstream_value, downstream_closed, downstream_funding) in pending_claims_to_replay {
                        // We use `downstream_closed` in place of `from_onchain` here just as a guess - we
                        // don't remember in the `ChannelMonitor` where we got a preimage from, but if the
                        // channel is closed we just assume that it probably came from an on-chain claim.
                        channel_manager.claim_funds_internal(source, preimage, Some(downstream_value),
-                               downstream_closed, downstream_chan_id);
+                               downstream_closed, downstream_funding);
                }
 
                //TODO: Broadcast channel update for closed channels, but only after we've made a
@@ -9292,6 +9408,7 @@ mod tests {
        use core::sync::atomic::Ordering;
        use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
        use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
+       use crate::ln::ChannelId;
        use crate::ln::channelmanager::{inbound_payment, PaymentId, PaymentSendFailure, RecipientOnionFields, InterceptId};
        use crate::ln::functional_test_utils::*;
        use crate::ln::msgs::{self, ErrorAction};
@@ -9452,6 +9569,7 @@ mod tests {
 
                let bs_first_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_first_updates.update_fulfill_htlcs[0]);
+               expect_payment_sent(&nodes[0], payment_preimage, None, false, false);
                nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_updates.commitment_signed);
                check_added_monitors!(nodes[0], 1);
                let (as_first_raa, as_first_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
@@ -9479,16 +9597,8 @@ mod tests {
                // Note that successful MPP payments will generate a single PaymentSent event upon the first
                // path's success and a PaymentPathSuccessful event for each path's success.
                let events = nodes[0].node.get_and_clear_pending_events();
-               assert_eq!(events.len(), 3);
+               assert_eq!(events.len(), 2);
                match events[0] {
-                       Event::PaymentSent { payment_id: ref id, payment_preimage: ref preimage, payment_hash: ref hash, .. } => {
-                               assert_eq!(Some(payment_id), *id);
-                               assert_eq!(payment_preimage, *preimage);
-                               assert_eq!(our_payment_hash, *hash);
-                       },
-                       _ => panic!("Unexpected event"),
-               }
-               match events[1] {
                        Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
                                assert_eq!(payment_id, *actual_payment_id);
                                assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
@@ -9496,7 +9606,7 @@ mod tests {
                        },
                        _ => panic!("Unexpected event"),
                }
-               match events[2] {
+               match events[1] {
                        Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
                                assert_eq!(payment_id, *actual_payment_id);
                                assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
@@ -9872,7 +9982,7 @@ mod tests {
                nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
 
                let (temporary_channel_id, tx, _funding_output) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 1_000_000, 42);
-               let channel_id = &tx.txid().into_inner();
+               let channel_id = ChannelId::from_bytes(tx.txid().into_inner());
                {
                        // Ensure that the `id_to_peer` map is empty until either party has received the
                        // funding transaction, and have the real `channel_id`.
@@ -9886,7 +9996,7 @@ mod tests {
                        // as it has the funding transaction.
                        let nodes_0_lock = nodes[0].node.id_to_peer.lock().unwrap();
                        assert_eq!(nodes_0_lock.len(), 1);
-                       assert!(nodes_0_lock.contains_key(channel_id));
+                       assert!(nodes_0_lock.contains_key(&channel_id));
                }
 
                assert_eq!(nodes[1].node.id_to_peer.lock().unwrap().len(), 0);
@@ -9897,7 +10007,7 @@ mod tests {
                {
                        let nodes_0_lock = nodes[0].node.id_to_peer.lock().unwrap();
                        assert_eq!(nodes_0_lock.len(), 1);
-                       assert!(nodes_0_lock.contains_key(channel_id));
+                       assert!(nodes_0_lock.contains_key(&channel_id));
                }
                expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id());
 
@@ -9906,7 +10016,7 @@ mod tests {
                        // as it has the funding transaction.
                        let nodes_1_lock = nodes[1].node.id_to_peer.lock().unwrap();
                        assert_eq!(nodes_1_lock.len(), 1);
-                       assert!(nodes_1_lock.contains_key(channel_id));
+                       assert!(nodes_1_lock.contains_key(&channel_id));
                }
                check_added_monitors!(nodes[1], 1);
                let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id());
@@ -9917,7 +10027,7 @@ mod tests {
                let (announcement, nodes_0_update, nodes_1_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &channel_ready);
                update_nodes_with_chan_announce(&nodes, 0, 1, &announcement, &nodes_0_update, &nodes_1_update);
 
-               nodes[0].node.close_channel(channel_id, &nodes[1].node.get_our_node_id()).unwrap();
+               nodes[0].node.close_channel(&channel_id, &nodes[1].node.get_our_node_id()).unwrap();
                nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()));
                let nodes_1_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id());
                nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &nodes_1_shutdown);
@@ -9931,7 +10041,7 @@ mod tests {
                        // party's signature for the fee negotiated closing transaction.)
                        let nodes_0_lock = nodes[0].node.id_to_peer.lock().unwrap();
                        assert_eq!(nodes_0_lock.len(), 1);
-                       assert!(nodes_0_lock.contains_key(channel_id));
+                       assert!(nodes_0_lock.contains_key(&channel_id));
                }
 
                {
@@ -9941,7 +10051,7 @@ mod tests {
                        // kept in the `nodes[1]`'s `id_to_peer` map.
                        let nodes_1_lock = nodes[1].node.id_to_peer.lock().unwrap();
                        assert_eq!(nodes_1_lock.len(), 1);
-                       assert!(nodes_1_lock.contains_key(channel_id));
+                       assert!(nodes_1_lock.contains_key(&channel_id));
                }
 
                nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id()));
@@ -9957,7 +10067,7 @@ mod tests {
                        // doesn't have `nodes[0]`'s signature for the closing transaction yet.
                        let nodes_1_lock = nodes[1].node.id_to_peer.lock().unwrap();
                        assert_eq!(nodes_1_lock.len(), 1);
-                       assert!(nodes_1_lock.contains_key(channel_id));
+                       assert!(nodes_1_lock.contains_key(&channel_id));
                }
 
                let (_nodes_0_update, closing_signed_node_0) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
@@ -10008,7 +10118,7 @@ mod tests {
                let nodes = create_network(2, &node_cfg, &node_chanmgr);
 
                // Dummy values
-               let channel_id = [4; 32];
+               let channel_id = ChannelId::from_bytes([4; 32]);
                let unkown_public_key = PublicKey::from_secret_key(&Secp256k1::signing_only(), &SecretKey::from_slice(&[42; 32]).unwrap());
                let intercept_id = InterceptId([0; 32]);
 
@@ -10063,11 +10173,11 @@ mod tests {
                                check_added_monitors!(nodes[0], 1);
                                expect_channel_pending_event(&nodes[0], &nodes[1].node.get_our_node_id());
                        }
-                       open_channel_msg.temporary_channel_id = nodes[0].keys_manager.get_secure_random_bytes();
+                       open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
                }
 
                // A MAX_UNFUNDED_CHANS_PER_PEER + 1 channel will be summarily rejected
-               open_channel_msg.temporary_channel_id = nodes[0].keys_manager.get_secure_random_bytes();
+               open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
                nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
                assert_eq!(get_err_msg(&nodes[1], &nodes[0].node.get_our_node_id()).channel_id,
                        open_channel_msg.temporary_channel_id);
@@ -10118,7 +10228,7 @@ mod tests {
                for i in 0..super::MAX_UNFUNDED_CHANNEL_PEERS - 1 {
                        nodes[1].node.handle_open_channel(&peer_pks[i], &open_channel_msg);
                        get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, peer_pks[i]);
-                       open_channel_msg.temporary_channel_id = nodes[0].keys_manager.get_secure_random_bytes();
+                       open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
                }
                nodes[1].node.handle_open_channel(&last_random_pk, &open_channel_msg);
                assert_eq!(get_err_msg(&nodes[1], &last_random_pk).channel_id,
@@ -10158,7 +10268,7 @@ mod tests {
                for _ in 0..super::MAX_UNFUNDED_CHANS_PER_PEER {
                        nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
                        get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
-                       open_channel_msg.temporary_channel_id = nodes[0].keys_manager.get_secure_random_bytes();
+                       open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
                }
 
                // Once we have MAX_UNFUNDED_CHANS_PER_PEER unfunded channels, new inbound channels will be
@@ -10210,7 +10320,7 @@ mod tests {
                                _ => panic!("Unexpected event"),
                        }
                        get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, random_pk);
-                       open_channel_msg.temporary_channel_id = nodes[0].keys_manager.get_secure_random_bytes();
+                       open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
                }
 
                // If we try to accept a channel from another peer non-0conf it will fail.
@@ -10426,7 +10536,7 @@ mod tests {
 
                // If we provide a channel_id not associated with the peer, we should get an error and no updates
                // should be applied to ensure update atomicity as specified in the API docs.
-               let bad_channel_id = [10; 32];
+               let bad_channel_id = ChannelId::v1_from_funding_txid(&[10; 32], 10);
                let current_fee = nodes[0].node.list_channels()[0].config.unwrap().forwarding_fee_proportional_millionths;
                let new_fee = current_fee + 100;
                assert!(
@@ -10443,6 +10553,16 @@ mod tests {
                let events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 0);
        }
+
+       #[test]
+       fn test_payment_display() {
+               let payment_id = PaymentId([42; 32]);
+               assert_eq!(format!("{}", &payment_id), "2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a");
+               let payment_hash = PaymentHash([42; 32]);
+               assert_eq!(format!("{}", &payment_hash), "2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a");
+               let payment_preimage = PaymentPreimage([42; 32]);
+               assert_eq!(format!("{}", &payment_preimage), "2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a");
+       }
 }
 
 #[cfg(ldk_bench)]
@@ -10463,7 +10583,7 @@ pub mod bench {
        use bitcoin::hashes::sha256::Hash as Sha256;
        use bitcoin::{Block, BlockHeader, PackedLockTime, Transaction, TxMerkleNode, TxOut};
 
-       use crate::sync::{Arc, Mutex};
+       use crate::sync::{Arc, Mutex, RwLock};
 
        use criterion::Criterion;
 
@@ -10475,13 +10595,13 @@ pub mod bench {
                &'a test_utils::TestFeeEstimator, &'a test_utils::TestRouter<'a>,
                &'a test_utils::TestLogger>;
 
-       struct ANodeHolder<'a, P: Persist<InMemorySigner>> {
-               node: &'a Manager<'a, P>,
+       struct ANodeHolder<'node_cfg, 'chan_mon_cfg: 'node_cfg, P: Persist<InMemorySigner>> {
+               node: &'node_cfg Manager<'chan_mon_cfg, P>,
        }
-       impl<'a, P: Persist<InMemorySigner>> NodeHolder for ANodeHolder<'a, P> {
-               type CM = Manager<'a, P>;
+       impl<'node_cfg, 'chan_mon_cfg: 'node_cfg, P: Persist<InMemorySigner>> NodeHolder for ANodeHolder<'node_cfg, 'chan_mon_cfg, P> {
+               type CM = Manager<'chan_mon_cfg, P>;
                #[inline]
-               fn node(&self) -> &Manager<'a, P> { self.node }
+               fn node(&self) -> &Manager<'chan_mon_cfg, P> { self.node }
                #[inline]
                fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { None }
        }
@@ -10500,7 +10620,7 @@ pub mod bench {
                let tx_broadcaster = test_utils::TestBroadcaster::new(network);
                let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
                let logger_a = test_utils::TestLogger::with_id("node a".to_owned());
-               let scorer = Mutex::new(test_utils::TestScorer::new());
+               let scorer = RwLock::new(test_utils::TestScorer::new());
                let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(network, &logger_a)), &scorer);
 
                let mut config: UserConfig = Default::default();