Refactor push_htlc_failure out from fail_htlcs_backwards_internal

We plan to fail back HTLCs that have not been forwarded yet, so we
refactored some of the failure event push logic out into it's own helper
to reuse it.

Author: wpaulino

lightning/src/ln/channelmanager.rs

@@ -5283,15 +5283,6 @@ where
 	/// 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) {
-		// Ensure that no peer state channel storage lock is held when calling this function.
-		// This ensures that future code doesn't introduce a lock-order requirement for
-		// `forward_htlcs` to be locked after the `per_peer_state` peer locks, which calling
-		// this function with any `per_peer_state` peer lock acquired would.
-		#[cfg(debug_assertions)]
-		for (_, peer) in self.per_peer_state.read().unwrap().iter() {
-			debug_assert_ne!(peer.held_by_thread(), LockHeldState::HeldByThread);
-		}
-
 		//TODO: There is a timing attack here where if a node fails an HTLC back to us they can
 		//identify whether we sent it or not based on the (I presume) very different runtime
 		//between the branches here. We should make this async and move it into the forward HTLCs
@@ -5339,28 +5330,41 @@ where
 					}
 				};
 
-				let mut push_forward_ev = false;
-				let mut forward_htlcs = self.forward_htlcs.lock().unwrap();
-				if forward_htlcs.is_empty() {
-					push_forward_ev = true;
-				}
-				match forward_htlcs.entry(*short_channel_id) {
-					hash_map::Entry::Occupied(mut entry) => {
-						entry.get_mut().push(failure);
-					},
-					hash_map::Entry::Vacant(entry) => {
-						entry.insert(vec!(failure));
-					}
-				}
-				mem::drop(forward_htlcs);
-				if push_forward_ev { self.push_pending_forwards_ev(); }
-				let mut pending_events = self.pending_events.lock().unwrap();
-				pending_events.push_back((events::Event::HTLCHandlingFailed {
-					prev_channel_id: outpoint.to_channel_id(),
-					failed_next_destination: destination,
-				}, None));
+				self.push_htlc_failure(*short_channel_id, outpoint.to_channel_id(), failure, destination);
+			}
+		}
+	}
+
+	fn push_htlc_failure(&self, short_channel_id: u64, channel_id: ChannelId, failure: HTLCForwardInfo, destination: HTLCDestination) {
+		// Ensure that no peer state channel storage lock is held when calling this function.
+		// This ensures that future code doesn't introduce a lock-order requirement for
+		// `forward_htlcs` to be locked after the `per_peer_state` peer locks, which calling
+		// this function with any `per_peer_state` peer lock acquired would.
+		#[cfg(debug_assertions)]
+		for (_, peer) in self.per_peer_state.read().unwrap().iter() {
+			debug_assert_ne!(peer.held_by_thread(), LockHeldState::HeldByThread);
+		}
+
+		let mut push_forward_ev = false;
+		let mut forward_htlcs = self.forward_htlcs.lock().unwrap();
+		if forward_htlcs.is_empty() {
+			push_forward_ev = true;
+		}
+		match forward_htlcs.entry(short_channel_id) {
+			hash_map::Entry::Occupied(mut entry) => {
+				entry.get_mut().push(failure);
 			},
+			hash_map::Entry::Vacant(entry) => {
+				entry.insert(vec!(failure));
+			}
 		}
+		mem::drop(forward_htlcs);
+		if push_forward_ev { self.push_pending_forwards_ev(); }
+		let mut pending_events = self.pending_events.lock().unwrap();
+		pending_events.push_back((events::Event::HTLCHandlingFailed {
+			prev_channel_id: channel_id,
+			failed_next_destination: destination,
+		}, None));
 	}
 
 	/// Provides a payment preimage in response to [`Event::PaymentClaimable`], generating any