From: Matt Corallo <git@bluematt.me>
Date: Wed, 12 Jan 2022 19:58:08 +0000 (+0000)
Subject: Make lockorder consistent in channelmanager
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=bf99544c6e10a63800191ad62e542962d49095b3;p=rust-lightning

Make lockorder consistent in channelmanager

This resolves a lockorder inversion in
`ChannelManager::finalize_claims` where `pending_outbound_payments`
is locked after `pending_events`, opposite of, for example, the
lockorder in `ChannelManager::fail_htlc_backwards_internal` where
`pending_outbound_payments` is locked at the top of the
`HTLCSource::OutboundRoute` handling and then `pending_events` is
locked at the end.
---

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index c51d17f67..b075fb569 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3927,12 +3927,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	}
 
 	fn finalize_claims(&self, mut sources: Vec<HTLCSource>) {
+		let mut outbounds = self.pending_outbound_payments.lock().unwrap();
 		let mut pending_events = self.pending_events.lock().unwrap();
 		for source in sources.drain(..) {
 			if let HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } = source {
 				let mut session_priv_bytes = [0; 32];
 				session_priv_bytes.copy_from_slice(&session_priv[..]);
-				let mut outbounds = self.pending_outbound_payments.lock().unwrap();
 				if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
 					assert!(payment.get().is_fulfilled());
 					if payment.get_mut().remove(&session_priv_bytes, None) {
@@ -5317,8 +5317,8 @@ where
 			inbound_payment.expiry_time > header.time as u64
 		});
 
-		let mut pending_events = self.pending_events.lock().unwrap();
 		let mut outbounds = self.pending_outbound_payments.lock().unwrap();
+		let mut pending_events = self.pending_events.lock().unwrap();
 		outbounds.retain(|payment_id, payment| {
 			if payment.remaining_parts() != 0 { return true }
 			if let PendingOutboundPayment::Retryable { starting_block_height, payment_hash, .. } = payment {
@@ -6147,6 +6147,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
 			peer_state.latest_features.write(writer)?;
 		}
 
+		let pending_inbound_payments = self.pending_inbound_payments.lock().unwrap();
+		let pending_outbound_payments = self.pending_outbound_payments.lock().unwrap();
 		let events = self.pending_events.lock().unwrap();
 		(events.len() as u64).write(writer)?;
 		for event in events.iter() {
@@ -6168,14 +6170,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
 		(self.last_node_announcement_serial.load(Ordering::Acquire) as u32).write(writer)?;
 		(self.highest_seen_timestamp.load(Ordering::Acquire) as u32).write(writer)?;
 
-		let pending_inbound_payments = self.pending_inbound_payments.lock().unwrap();
 		(pending_inbound_payments.len() as u64).write(writer)?;
 		for (hash, pending_payment) in pending_inbound_payments.iter() {
 			hash.write(writer)?;
 			pending_payment.write(writer)?;
 		}
 
-		let pending_outbound_payments = self.pending_outbound_payments.lock().unwrap();
 		// For backwards compat, write the session privs and their total length.
 		let mut num_pending_outbounds_compat: u64 = 0;
 		for (_, outbound) in pending_outbound_payments.iter() {