From: Tobin C. Harding <me@tobin.cc>
Date: Tue, 29 Nov 2022 01:24:12 +0000 (+1100)
Subject: Do not lock while looping htlcs_to_fail
X-Git-Tag: v0.0.113~20^2
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=1dd3184805d7e3ed552ea9ad0b2fd9d06fe23eac;p=rust-lightning

Do not lock while looping htlcs_to_fail

Currently we loop over `htlcs_to_fail` locking `channel_state` for each
element only to call `get_htlc_inbound_temp_fail_err_and_data` with the
same inputs on each iteration. This is unnecessary, we can refactor and
call `get_htlc_inbound_temp_fail_err_and_data` outside of the loop.
---

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 687d45c49..97dbb4175 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3974,16 +3974,16 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 		&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())
+			};
 
-			let reason = HTLCFailReason::reason(failure_code, onion_failure_data);
+		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, &reason, receiver);
 		}