Merge pull request #2412 from valentinewallace/2023-07-construct-blinded-paths
[rust-lightning] / lightning / src / ln / channelmanager.rs
index eb13f6b4aa1901620596a49af90abd3efbdf3603..6393117b7f0b0291d2a86e4e82c84ee310f2669b 100644 (file)
@@ -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;
@@ -252,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
@@ -659,23 +665,23 @@ 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<[u8; 32], 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<[u8; 32], 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<[u8; 32], InboundV1Channel<SP>>,
        /// `temporary_channel_id` -> `InboundChannelRequest`.
        ///
        /// When manual channel acceptance is enabled, this holds all unaccepted inbound channels where
@@ -721,7 +727,7 @@ pub(super) struct PeerState<Signer: ChannelSigner> {
        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`.
@@ -1146,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
@@ -1594,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) =
@@ -2299,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(&(&[u8; 32], &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
@@ -2425,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) => {
@@ -3115,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(),
@@ -3140,7 +3148,7 @@ 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> {
+       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 {}", log_bytes!(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}),
@@ -3150,7 +3158,7 @@ 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> {
+       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 {}", log_bytes!(chan.context.channel_id()));
                let were_node_one = self.our_network_pubkey.serialize()[..] < chan.context.get_counterparty_node_id().serialize()[..];
 
@@ -3444,7 +3452,7 @@ 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>>(
+       fn funding_transaction_generated_intern<FundingOutput: Fn(&OutboundV1Channel<SP>, &Transaction) -> Result<OutPoint, APIError>>(
                &self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction, find_funding_output: FundingOutput
        ) -> Result<(), APIError> {
                let per_peer_state = self.per_peer_state.read().unwrap();
@@ -3960,7 +3968,7 @@ where
                                                                                        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),
@@ -3976,7 +3984,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");
                                                                                        }
@@ -4116,11 +4124,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 {
@@ -4138,7 +4146,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; }
@@ -4149,7 +4157,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)] {
@@ -4203,7 +4211,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);
                                                                                                        }
                                                                                                };
@@ -4211,7 +4219,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);
                                                                                                        }
                                                                                                }
@@ -4229,16 +4237,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 {
@@ -4362,7 +4370,7 @@ 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: &[u8; 32], 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() {
@@ -4521,7 +4529,7 @@ where
 
                                        let process_unfunded_channel_tick = |
                                                chan_id: &[u8; 32],
-                                               chan_context: &mut ChannelContext<<SP::Target as SignerProvider>::Signer>,
+                                               chan_context: &mut ChannelContext<SP>,
                                                unfunded_chan_context: &mut UnfundedChannelContext,
                                                pending_msg_events: &mut Vec<MessageSendEvent>,
                                        | {
@@ -4711,7 +4719,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
@@ -4731,7 +4739,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));
@@ -4808,7 +4816,7 @@ where
                                { 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);
+                               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;
@@ -4903,13 +4911,13 @@ where
                                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 {
@@ -5194,7 +5202,7 @@ 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>)
@@ -5429,7 +5437,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();
                {
@@ -5447,7 +5455,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() {
@@ -5893,7 +5901,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.
@@ -6325,10 +6333,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));
+                                                       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);
@@ -7055,7 +7063,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.
@@ -8601,13 +8609,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<[u8; 32], 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)?;
@@ -8651,7 +8659,7 @@ 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));
+                                                               log_bytes!(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()));
                                                }
                                        }
@@ -8752,7 +8760,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());
@@ -9004,7 +9012,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();
@@ -9024,7 +9032,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));
                                                                }
                                                        }
                                                }
@@ -9046,7 +9054,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, log_bytes!(monitor.get_funding_txo().0.to_channel_id()));
                                                                                                false
                                                                                        } else { true }
                                                                                } else { true }
@@ -9056,7 +9064,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, log_bytes!(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
@@ -9175,7 +9183,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);
                                                                                }
                                                                        }
@@ -9242,7 +9250,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;
@@ -10542,6 +10550,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)]
@@ -10574,13 +10592,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 }
        }