From: Viktor Tigerström <11711198+ViktorTigerstrom@users.noreply.github.com>
Date: Mon, 5 Sep 2022 20:16:32 +0000 (+0200)
Subject: Remove `forward_htlc` after `channel_state` lock order
X-Git-Tag: v0.0.112~31^2~1
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=3379f8c4927ce970aeadac935da0204ba4cb40ae;p=rust-lightning

Remove `forward_htlc` after `channel_state` lock order

The `forward_htlc` was prior to this commit only held at the same time
as the `channel_state` lock during the write process of the
`ChannelManager`. This commit removes the lock order dependency, by
taking the `channel_state`lock temporarily during the write process.
---

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index cce6a7f97..85602f3bb 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -6548,30 +6548,37 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
 			best_block.block_hash().write(writer)?;
 		}
 
-		let channel_state = self.channel_state.lock().unwrap();
-		let mut unfunded_channels = 0;
-		for (_, channel) in channel_state.by_id.iter() {
-			if !channel.is_funding_initiated() {
-				unfunded_channels += 1;
+		{
+			// Take `channel_state` lock temporarily to avoid creating a lock order that requires
+			// that the `forward_htlcs` lock is taken after `channel_state`
+			let channel_state = self.channel_state.lock().unwrap();
+			let mut unfunded_channels = 0;
+			for (_, channel) in channel_state.by_id.iter() {
+				if !channel.is_funding_initiated() {
+					unfunded_channels += 1;
+				}
 			}
-		}
-		((channel_state.by_id.len() - unfunded_channels) as u64).write(writer)?;
-		for (_, channel) in channel_state.by_id.iter() {
-			if channel.is_funding_initiated() {
-				channel.write(writer)?;
+			((channel_state.by_id.len() - unfunded_channels) as u64).write(writer)?;
+			for (_, channel) in channel_state.by_id.iter() {
+				if channel.is_funding_initiated() {
+					channel.write(writer)?;
+				}
 			}
 		}
 
-		let forward_htlcs = self.forward_htlcs.lock().unwrap();
-		(forward_htlcs.len() as u64).write(writer)?;
-		for (short_channel_id, pending_forwards) in forward_htlcs.iter() {
-			short_channel_id.write(writer)?;
-			(pending_forwards.len() as u64).write(writer)?;
-			for forward in pending_forwards {
-				forward.write(writer)?;
+		{
+			let forward_htlcs = self.forward_htlcs.lock().unwrap();
+			(forward_htlcs.len() as u64).write(writer)?;
+			for (short_channel_id, pending_forwards) in forward_htlcs.iter() {
+				short_channel_id.write(writer)?;
+				(pending_forwards.len() as u64).write(writer)?;
+				for forward in pending_forwards {
+					forward.write(writer)?;
+				}
 			}
 		}
 
+		let channel_state = self.channel_state.lock().unwrap();
 		let mut htlc_purposes: Vec<&events::PaymentPurpose> = Vec::new();
 		(channel_state.claimable_htlcs.len() as u64).write(writer)?;
 		for (payment_hash, (purpose, previous_hops)) in channel_state.claimable_htlcs.iter() {