Merge pull request #1893 from valentinewallace/2022-12-jit-forwards-followup
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Thu, 1 Dec 2022 21:51:39 +0000 (21:51 +0000)
committerGitHub <noreply@github.com>
Thu, 1 Dec 2022 21:51:39 +0000 (21:51 +0000)
HTLC JIT channel interception followup + minor cleanups

1  2 
lightning/src/ln/channelmanager.rs

index 97dbb4175c5e41bc282d571c4b843c992d851849,c45a0abf4a2ee67c8c9b02612b1ff413119b2398..306739ad6e0d0d7fd9a7e5e2df3c053201f02f7d
@@@ -287,16 -287,6 +287,16 @@@ pub(super) enum HTLCFailReason 
        }
  }
  
 +impl HTLCFailReason {
 +      pub(super) fn reason(failure_code: u16, data: Vec<u8>) -> Self {
 +              Self::Reason { failure_code, data }
 +      }
 +
 +      pub(super) fn from_failure_code(failure_code: u16) -> Self {
 +              Self::Reason { failure_code, data: Vec::new() }
 +      }
 +}
 +
  struct ReceiveError {
        err_code: u16,
        err_data: Vec<u8>,
@@@ -1886,9 -1876,8 +1886,9 @@@ impl<M: Deref, T: Deref, K: Deref, F: D
                };
  
                for htlc_source in failed_htlcs.drain(..) {
 +                      let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(*counterparty_node_id), channel_id: *channel_id };
 -                      self.fail_htlc_backwards_internal(htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }, receiver);
 +                      self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver);
                }
  
                let _ = handle_error!(self, result, *counterparty_node_id);
                log_debug!(self.logger, "Finishing force-closure of channel with {} HTLCs to fail", failed_htlcs.len());
                for htlc_source in failed_htlcs.drain(..) {
                        let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
 +                      let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id };
 -                      self.fail_htlc_backwards_internal(source, &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }, receiver);
 +                      self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
                }
                if let Some((funding_txo, monitor_update)) = monitor_update_option {
                        // There isn't anything we can do if we get an update failure - we're already
                let session_priv = SecretKey::from_slice(&session_priv_bytes[..]).expect("RNG is busted");
  
                let onion_keys = onion_utils::construct_onion_keys(&self.secp_ctx, &path, &session_priv)
-                       .map_err(|_| APIError::RouteError{err: "Pubkey along hop was maliciously selected"})?;
+                       .map_err(|_| APIError::InvalidRoute{err: "Pubkey along hop was maliciously selected"})?;
                let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(path, total_value, payment_secret, cur_height, keysend_preimage)?;
                if onion_utils::route_size_insane(&onion_payloads) {
-                       return Err(APIError::RouteError{err: "Route size too large considering onion data"});
+                       return Err(APIError::InvalidRoute{err: "Route size too large considering onion data"});
                }
                let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash);
  
                        if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) {
                                match {
                                        if chan.get().get_counterparty_node_id() != path.first().unwrap().pubkey {
-                                               return Err(APIError::RouteError{err: "Node ID mismatch on first hop!"});
+                                               return Err(APIError::InvalidRoute{err: "Node ID mismatch on first hop!"});
                                        }
                                        if !chan.get().is_live() {
                                                return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!".to_owned()});
        /// fields for more info.
        ///
        /// If a pending payment is currently in-flight with the same [`PaymentId`] provided, this
-       /// method will error with an [`APIError::RouteError`]. Note, however, that once a payment
+       /// method will error with an [`APIError::InvalidRoute`]. Note, however, that once a payment
        /// is no longer pending (either via [`ChannelManager::abandon_payment`], or handling of an
        /// [`Event::PaymentSent`]) LDK will not stop you from sending a second payment with the same
        /// [`PaymentId`].
        /// PaymentSendFailure for more info.
        ///
        /// In general, a path may raise:
-       ///  * [`APIError::RouteError`] when an invalid route or forwarding parameter (cltv_delta, fee,
+       ///  * [`APIError::InvalidRoute`] when an invalid route or forwarding parameter (cltv_delta, fee,
        ///    node public key) is specified.
        ///  * [`APIError::ChannelUnavailable`] if the next-hop channel is not available for updates
        ///    (including due to previous monitor update failure or new permanent monitor update
  
        fn send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, keysend_preimage: Option<PaymentPreimage>, payment_id: PaymentId, recv_value_msat: Option<u64>, onion_session_privs: Vec<[u8; 32]>) -> Result<(), PaymentSendFailure> {
                if route.paths.len() < 1 {
-                       return Err(PaymentSendFailure::ParameterError(APIError::RouteError{err: "There must be at least one path to send over"}));
+                       return Err(PaymentSendFailure::ParameterError(APIError::InvalidRoute{err: "There must be at least one path to send over"}));
                }
                if payment_secret.is_none() && route.paths.len() > 1 {
                        return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError{err: "Payment secret is required for multi-path payments".to_string()}));
                let mut path_errs = Vec::with_capacity(route.paths.len());
                'path_check: for path in route.paths.iter() {
                        if path.len() < 1 || path.len() > 20 {
-                               path_errs.push(Err(APIError::RouteError{err: "Path didn't go anywhere/had bogus size"}));
+                               path_errs.push(Err(APIError::InvalidRoute{err: "Path didn't go anywhere/had bogus size"}));
                                continue 'path_check;
                        }
                        for (idx, hop) in path.iter().enumerate() {
                                if idx != path.len() - 1 && hop.pubkey == our_node_id {
-                                       path_errs.push(Err(APIError::RouteError{err: "Path went through us but wasn't a simple rebalance loop to us"}));
+                                       path_errs.push(Err(APIError::InvalidRoute{err: "Path went through us but wasn't a simple rebalance loop to us"}));
                                        continue 'path_check;
                                }
                        }
                let next_hop_scid = match self.channel_state.lock().unwrap().by_id.get(next_hop_channel_id) {
                        Some(chan) => {
                                if !chan.is_usable() {
-                                       return Err(APIError::APIMisuseError {
-                                               err: format!("Channel with id {:?} not fully established", next_hop_channel_id)
+                                       return Err(APIError::ChannelUnavailable {
+                                               err: format!("Channel with id {} not fully established", log_bytes!(*next_hop_channel_id))
                                        })
                                }
                                chan.get_short_channel_id().unwrap_or(chan.outbound_scid_alias())
                        },
-                       None => return Err(APIError::APIMisuseError {
-                               err: format!("Channel with id {:?} not found", next_hop_channel_id)
+                       None => return Err(APIError::ChannelUnavailable {
+                               err: format!("Channel with id {} not found", log_bytes!(*next_hop_channel_id))
                        })
                };
  
                let payment = self.pending_intercepted_htlcs.lock().unwrap().remove(&intercept_id)
                        .ok_or_else(|| APIError::APIMisuseError {
-                               err: format!("Payment with intercept id {:?} not found", intercept_id.0)
+                               err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.0))
                        })?;
  
                let routing = match payment.forward_info.routing {
  
                let payment = self.pending_intercepted_htlcs.lock().unwrap().remove(&intercept_id)
                        .ok_or_else(|| APIError::APIMisuseError {
-                               err: format!("Payment with InterceptId {:?} not found", intercept_id)
+                               err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.0))
                        })?;
  
                if let PendingHTLCRouting::Forward { short_channel_id, .. } = payment.forward_info.routing {
                                phantom_shared_secret: None,
                        });
  
 -                      let failure_reason = HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() };
 +                      let failure_reason = HTLCFailReason::from_failure_code(0x4000 | 10);
                        let destination = HTLCDestination::UnknownNextHop { requested_forward_scid: short_channel_id };
 -                      self.fail_htlc_backwards_internal(htlc_source, &payment.forward_info.payment_hash, failure_reason, destination);
 +                      self.fail_htlc_backwards_internal(&htlc_source, &payment.forward_info.payment_hash, &failure_reason, destination);
                } else { unreachable!() } // Only `PendingHTLCRouting::Forward`s are intercepted
  
                Ok(())
                                                                                                };
  
                                                                                                failed_forwards.push((htlc_source, payment_hash,
 -                                                                                                      HTLCFailReason::Reason { failure_code: $err_code, data: $err_data },
 +                                                                                                      HTLCFailReason::reason($err_code, $err_data),
                                                                                                        reason
                                                                                                ));
                                                                                                continue;
                                                                                                }
                                                                                                let (failure_code, data) = self.get_htlc_temp_fail_err_and_data(0x1000|7, short_chan_id, chan.get());
                                                                                                failed_forwards.push((htlc_source, payment_hash,
 -                                                                                                      HTLCFailReason::Reason { failure_code, data },
 +                                                                                                      HTLCFailReason::reason(failure_code, data),
                                                                                                        HTLCDestination::NextHopChannel { node_id: Some(chan.get().get_counterparty_node_id()), channel_id: forward_chan_id }
                                                                                                ));
                                                                                                continue;
                                                                                                incoming_packet_shared_secret: $htlc.prev_hop.incoming_packet_shared_secret,
                                                                                                phantom_shared_secret,
                                                                                        }), payment_hash,
 -                                                                                      HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data },
 +                                                                                      HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
                                                                                        HTLCDestination::FailedPayment { payment_hash: $payment_hash },
                                                                                ));
                                                                        }
                }
  
                for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) {
 -                      self.fail_htlc_backwards_internal(htlc_source, &payment_hash, failure_reason, destination);
 +                      self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination);
                }
                self.forward_htlcs(&mut phantom_receives);
  
                        });
  
                        for htlc_source in timed_out_mpp_htlcs.drain(..) {
 +                              let source = HTLCSource::PreviousHopData(htlc_source.0.clone());
 +                              let reason = HTLCFailReason::from_failure_code(23);
                                let receiver = HTLCDestination::FailedPayment { payment_hash: htlc_source.1 };
 -                              self.fail_htlc_backwards_internal(HTLCSource::PreviousHopData(htlc_source.0.clone()), &htlc_source.1, HTLCFailReason::Reason { failure_code: 23, data: Vec::new() }, receiver );
 +                              self.fail_htlc_backwards_internal(&source, &htlc_source.1, &reason, receiver);
                        }
  
                        for (err, counterparty_node_id) in handle_errors.drain(..) {
                                let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
                                htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
                                                self.best_block.read().unwrap().height()));
 -                              self.fail_htlc_backwards_internal(
 -                                              HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash,
 -                                              HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data },
 -                                              HTLCDestination::FailedPayment { payment_hash: *payment_hash });
 +                              let source = HTLCSource::PreviousHopData(htlc.prev_hop);
 +                              let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
 +                              let receiver = HTLCDestination::FailedPayment { payment_hash: *payment_hash };
 +                              self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
                        }
                }
        }
                &self, mut htlcs_to_fail: Vec<(HTLCSource, PaymentHash)>, channel_id: [u8; 32],
                counterparty_node_id: &PublicKey
        ) {
 -              for (htlc_src, payment_hash) in htlcs_to_fail.drain(..) {
 -                      let (failure_code, onion_failure_data) =
 -                              match self.channel_state.lock().unwrap().by_id.entry(channel_id) {
 -                                      hash_map::Entry::Occupied(chan_entry) => {
 -                                              self.get_htlc_inbound_temp_fail_err_and_data(0x1000|7, &chan_entry.get())
 -                                      },
 -                                      hash_map::Entry::Vacant(_) => (0x4000|10, Vec::new())
 -                              };
 +              let (failure_code, onion_failure_data) =
 +                      match self.channel_state.lock().unwrap().by_id.entry(channel_id) {
 +                              hash_map::Entry::Occupied(chan_entry) => {
 +                                      self.get_htlc_inbound_temp_fail_err_and_data(0x1000|7, &chan_entry.get())
 +                              },
 +                              hash_map::Entry::Vacant(_) => (0x4000|10, Vec::new())
 +                      };
  
 +              for (htlc_src, payment_hash) in htlcs_to_fail.drain(..) {
 +                      let reason = HTLCFailReason::reason(failure_code, onion_failure_data.clone());
                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id };
 -                      self.fail_htlc_backwards_internal(htlc_src, &payment_hash, HTLCFailReason::Reason { failure_code, data: onion_failure_data }, receiver);
 +                      self.fail_htlc_backwards_internal(&htlc_src, &payment_hash, &reason, receiver);
                }
        }
  
        /// Fails an HTLC backwards to the sender of it to us.
        /// Note that we do not assume that channels corresponding to failed HTLCs are still available.
 -      fn fail_htlc_backwards_internal(&self, source: HTLCSource, payment_hash: &PaymentHash, onion_error: HTLCFailReason,destination: HTLCDestination) {
 +      fn fail_htlc_backwards_internal(&self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, destination: HTLCDestination) {
                #[cfg(debug_assertions)]
                {
                        // Ensure that the `channel_state` lock is not held when calling this function.
                // from block_connected which may run during initialization prior to the chain_monitor
                // being fully configured. See the docs for `ChannelManagerReadArgs` for more.
                match source {
 -                      HTLCSource::OutboundRoute { ref path, session_priv, payment_id, ref payment_params, .. } => {
 +                      HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, ref payment_params, .. } => {
                                let mut session_priv_bytes = [0; 32];
                                session_priv_bytes.copy_from_slice(&session_priv[..]);
                                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
                                let mut all_paths_failed = false;
                                let mut full_failure_ev = None;
 -                              if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
 +                              if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) {
                                        if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
                                                log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
                                                return;
                                                all_paths_failed = true;
                                                if payment.get().abandoned() {
                                                        full_failure_ev = Some(events::Event::PaymentFailed {
 -                                                              payment_id,
 +                                                              payment_id: *payment_id,
                                                                payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
                                                        });
                                                        payment.remove();
                                                if self.payment_is_probe(payment_hash, &payment_id) {
                                                        if !payment_retryable {
                                                                events::Event::ProbeSuccessful {
 -                                                                      payment_id,
 +                                                                      payment_id: *payment_id,
                                                                        payment_hash: payment_hash.clone(),
                                                                        path: path.clone(),
                                                                }
                                                        } else {
                                                                events::Event::ProbeFailed {
 -                                                                      payment_id,
 +                                                                      payment_id: *payment_id,
                                                                        payment_hash: payment_hash.clone(),
                                                                        path: path.clone(),
                                                                        short_channel_id,
                                                                retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
                                                        }
                                                        events::Event::PaymentPathFailed {
 -                                                              payment_id: Some(payment_id),
 +                                                              payment_id: Some(*payment_id),
                                                                payment_hash: payment_hash.clone(),
                                                                payment_failed_permanently: !payment_retryable,
                                                                network_update,
  
                                                if self.payment_is_probe(payment_hash, &payment_id) {
                                                        events::Event::ProbeFailed {
 -                                                              payment_id,
 +                                                              payment_id: *payment_id,
                                                                payment_hash: payment_hash.clone(),
                                                                path: path.clone(),
                                                                short_channel_id: Some(scid),
                                                        }
                                                } else {
                                                        events::Event::PaymentPathFailed {
 -                                                              payment_id: Some(payment_id),
 +                                                              payment_id: Some(*payment_id),
                                                                payment_hash: payment_hash.clone(),
                                                                payment_failed_permanently: false,
                                                                network_update: None,
                                pending_events.push(path_failure);
                                if let Some(ev) = full_failure_ev { pending_events.push(ev); }
                        },
 -                      HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret, phantom_shared_secret, outpoint }) => {
 +                      HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
                                let err_packet = match onion_error {
 -                                      HTLCFailReason::Reason { failure_code, data } => {
 +                                      HTLCFailReason::Reason { ref failure_code, ref data } => {
                                                log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code);
                                                if let Some(phantom_ss) = phantom_shared_secret {
 -                                                      let phantom_packet = onion_utils::build_failure_packet(&phantom_ss, failure_code, &data[..]).encode();
 -                                                      let encrypted_phantom_packet = onion_utils::encrypt_failure_packet(&phantom_ss, &phantom_packet);
 -                                                      onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &encrypted_phantom_packet.data[..])
 +                                                      let phantom_packet = onion_utils::build_failure_packet(phantom_ss, *failure_code, &data[..]).encode();
 +                                                      let encrypted_phantom_packet = onion_utils::encrypt_failure_packet(phantom_ss, &phantom_packet);
 +                                                      onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &encrypted_phantom_packet.data[..])
                                                } else {
 -                                                      let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode();
 -                                                      onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet)
 +                                                      let packet = onion_utils::build_failure_packet(incoming_packet_shared_secret, *failure_code, &data[..]).encode();
 +                                                      onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &packet)
                                                }
                                        },
                                        HTLCFailReason::LightningError { err } => {
                                                log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards with pre-built LightningError", log_bytes!(payment_hash.0));
 -                                              onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &err.data)
 +                                              onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &err.data)
                                        }
                                };
  
                                if forward_htlcs.is_empty() {
                                        forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS));
                                }
 -                              match forward_htlcs.entry(short_channel_id) {
 +                              match forward_htlcs.entry(*short_channel_id) {
                                        hash_map::Entry::Occupied(mut entry) => {
 -                                              entry.get_mut().push(HTLCForwardInfo::FailHTLC { htlc_id, err_packet });
 +                                              entry.get_mut().push(HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet });
                                        },
                                        hash_map::Entry::Vacant(entry) => {
 -                                              entry.insert(vec!(HTLCForwardInfo::FailHTLC { htlc_id, err_packet }));
 +                                              entry.insert(vec!(HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet }));
                                        }
                                }
                                mem::drop(forward_htlcs);
                                }
                                pending_events.push(events::Event::HTLCHandlingFailed {
                                        prev_channel_id: outpoint.to_channel_id(),
 -                                      failed_next_destination: destination
 +                                      failed_next_destination: destination,
                                });
                        },
                }
                                        let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
                                        htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
                                                self.best_block.read().unwrap().height()));
 -                                      self.fail_htlc_backwards_internal(
 -                                              HTLCSource::PreviousHopData(htlc.prev_hop), &payment_hash,
 -                                              HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_height_data },
 -                                              HTLCDestination::FailedPayment { payment_hash } );
 +                                      let source = HTLCSource::PreviousHopData(htlc.prev_hop);
 +                                      let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
 +                                      let receiver = HTLCDestination::FailedPayment { payment_hash };
 +                                      self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
                                }
                        }
  
                self.finalize_claims(finalized_claims);
                for failure in pending_failures.drain(..) {
                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id: funding_txo.to_channel_id() };
 -                      self.fail_htlc_backwards_internal(failure.0, &failure.1, failure.2, receiver);
 +                      self.fail_htlc_backwards_internal(&failure.0, &failure.1, &failure.2, receiver);
                }
        }
  
                };
                for htlc_source in dropped_htlcs.drain(..) {
                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id: msg.channel_id };
 -                      self.fail_htlc_backwards_internal(htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }, receiver);
 +                      let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
 +                      self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver);
                }
  
                let _ = handle_error!(self, result, *counterparty_node_id);
                                        let chan_err: ChannelError = ChannelError::Close("Got update_fail_malformed_htlc with BADONION not set".to_owned());
                                        try_chan_entry!(self, Err(chan_err), chan);
                                }
 -                              try_chan_entry!(self, chan.get_mut().update_fail_malformed_htlc(&msg, HTLCFailReason::Reason { failure_code: msg.failure_code, data: Vec::new() }), chan);
 +                              try_chan_entry!(self, chan.get_mut().update_fail_malformed_htlc(&msg, HTLCFailReason::from_failure_code(msg.failure_code)), chan);
                                Ok(())
                        },
                        hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
                                                                                });
  
                                                                                failed_intercept_forwards.push((htlc_source, forward_info.payment_hash,
 -                                                                                              HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() },
 +                                                                                              HTLCFailReason::from_failure_code(0x4000 | 10),
                                                                                                HTLCDestination::InvalidForward { requested_forward_scid: scid },
                                                                                ));
                                                                        }
                        }
  
                        for (htlc_source, payment_hash, failure_reason, destination) in failed_intercept_forwards.drain(..) {
 -                              self.fail_htlc_backwards_internal(htlc_source, &payment_hash, failure_reason, destination);
 +                              self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination);
                        }
  
                        if !new_intercept_events.is_empty() {
                        {
                                for failure in pending_failures.drain(..) {
                                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(*counterparty_node_id), channel_id: channel_outpoint.to_channel_id() };
 -                                      self.fail_htlc_backwards_internal(failure.0, &failure.1, failure.2, receiver);
 +                                      self.fail_htlc_backwards_internal(&failure.0, &failure.1, &failure.2, receiver);
                                }
                                self.forward_htlcs(&mut [(short_channel_id, channel_outpoint, user_channel_id, pending_forwards)]);
                                self.finalize_claims(finalized_claim_htlcs);
                                                } else {
                                                        log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
                                                        let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
 -                                                      self.fail_htlc_backwards_internal(htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }, receiver);
 +                                                      let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
 +                                                      self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);
                                                }
                                        },
                                        MonitorEvent::CommitmentTxConfirmed(funding_outpoint) |
@@@ -6198,8 -6181,9 +6198,8 @@@ wher
                                if let Ok((channel_ready_opt, mut timed_out_pending_htlcs, announcement_sigs)) = res {
                                        for (source, payment_hash) in timed_out_pending_htlcs.drain(..) {
                                                let (failure_code, data) = self.get_htlc_inbound_temp_fail_err_and_data(0x1000|14 /* expiry_too_soon */, &channel);
 -                                              timed_out_htlcs.push((source, payment_hash, HTLCFailReason::Reason {
 -                                                      failure_code, data,
 -                                              }, HTLCDestination::NextHopChannel { node_id: Some(channel.get_counterparty_node_id()), channel_id: channel.channel_id() }));
 +                                              timed_out_htlcs.push((source, payment_hash, HTLCFailReason::reason(failure_code, data),
 +                                                      HTLCDestination::NextHopChannel { node_id: Some(channel.get_counterparty_node_id()), channel_id: channel.channel_id() }));
                                        }
                                        if let Some(channel_ready) = channel_ready_opt {
                                                send_channel_ready!(self, pending_msg_events, channel, channel_ready);
                                        if height >= htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER {
                                                let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
                                                htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height));
 -                                              timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(), HTLCFailReason::Reason {
 -                                                      failure_code: 0x4000 | 15,
 -                                                      data: htlc_msat_height_data
 -                                              }, HTLCDestination::FailedPayment { payment_hash: payment_hash.clone() }));
 +
 +                                              timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(),
 +                                                      HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
 +                                                      HTLCDestination::FailedPayment { payment_hash: payment_hash.clone() }));
                                                false
                                        } else { true }
                                });
                                                _ => unreachable!(),
                                        };
                                        timed_out_htlcs.push((prev_hop_data, htlc.forward_info.payment_hash,
 -                                                      HTLCFailReason::Reason { failure_code: 0x2000 | 2, data: Vec::new() },
 +                                                      HTLCFailReason::from_failure_code(0x2000 | 2),
                                                        HTLCDestination::InvalidForward { requested_forward_scid }));
                                        log_trace!(self.logger, "Timing out intercepted HTLC with requested forward scid {}", requested_forward_scid);
                                        false
                self.handle_init_event_channel_failures(failed_channels);
  
                for (source, payment_hash, reason, destination) in timed_out_htlcs.drain(..) {
 -                      self.fail_htlc_backwards_internal(source, &payment_hash, reason, destination);
 +                      self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, destination);
                }
        }
  
@@@ -7809,8 -7793,7 +7809,8 @@@ impl<'a, M: Deref, T: Deref, K: Deref, 
                for htlc_source in failed_htlcs.drain(..) {
                        let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id };
 -                      channel_manager.fail_htlc_backwards_internal(source, &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }, receiver);
 +                      let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
 +                      channel_manager.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
                }
  
                //TODO: Broadcast channel update for closed channels, but only after we've made a