Avoid unnecessary immediate retake `per_peer_state` lock

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 7b2865bb1475c6e80f96b4859a9eafa10116c839..b5fa260980d7c1b0379754a2bba096eeabd112b9 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1515,12 +1515,19 @@ where
                        return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
                }
 
-               let channel = {
-                       let per_peer_state = self.per_peer_state.read().unwrap();
-                       match per_peer_state.get(&their_network_key) {
-                               Some(peer_state) => {
+               let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
+               // We want to make sure the lock is actually acquired by PersistenceNotifierGuard.
+               debug_assert!(&self.total_consistency_lock.try_write().is_err());
+
+               let mut channel_state = self.channel_state.lock().unwrap();
+               let per_peer_state = self.per_peer_state.read().unwrap();
+
+               match per_peer_state.get(&their_network_key) {
+                       None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
+                       Some(peer_state_mutex) => {
+                               let mut peer_state = peer_state_mutex.lock().unwrap();
+                               let channel = {
                                        let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
-                                       let peer_state = peer_state.lock().unwrap();
                                        let their_features = &peer_state.latest_features;
                                        let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
                                        match Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key,
@@ -1533,38 +1540,28 @@ where
                                                        return Err(e);
                                                },
                                        }
-                               },
-                               None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
-                       }
-               };
-               let res = channel.get_open_channel(self.genesis_hash.clone());
-
-               let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
-               // We want to make sure the lock is actually acquired by PersistenceNotifierGuard.
-               debug_assert!(&self.total_consistency_lock.try_write().is_err());
+                               };
+                               let res = channel.get_open_channel(self.genesis_hash.clone());
 
-               let temporary_channel_id = channel.channel_id();
-               let mut channel_state = self.channel_state.lock().unwrap();
-               let per_peer_state = self.per_peer_state.read().unwrap();
-               if let Some(peer_state_mutex) = per_peer_state.get(&their_network_key){
-                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
-                       let peer_state = &mut *peer_state_lock;
-                       match peer_state.channel_by_id.entry(temporary_channel_id) {
-                       hash_map::Entry::Occupied(_) => {
-                               if cfg!(fuzzing) {
-                                       return Err(APIError::APIMisuseError { err: "Fuzzy bad RNG".to_owned() });
-                               } else {
-                                       panic!("RNG is bad???");
+                               let temporary_channel_id = channel.channel_id();
+                               match peer_state.channel_by_id.entry(temporary_channel_id) {
+                                       hash_map::Entry::Occupied(_) => {
+                                               if cfg!(fuzzing) {
+                                                       return Err(APIError::APIMisuseError { err: "Fuzzy bad RNG".to_owned() });
+                                               } else {
+                                                       panic!("RNG is bad???");
+                                               }
+                                       },
+                                       hash_map::Entry::Vacant(entry) => { entry.insert(channel); }
                                }
+
+                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannel {
+                                       node_id: their_network_key,
+                                       msg: res,
+                               });
+                               Ok(temporary_channel_id)
                        },
-                       hash_map::Entry::Vacant(entry) => { entry.insert(channel); }
-                       }
-               } else { return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }) }
-               channel_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannel {
-                       node_id: their_network_key,
-                       msg: res,
-               });
-               Ok(temporary_channel_id)
+               }
        }
 
        fn list_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<<K::Target as SignerProvider>::Signer>)) -> bool + Copy>(&self, f: Fn) -> Vec<ChannelDetails> {
@@ -2533,12 +2530,13 @@ where
        fn funding_transaction_generated_intern<FundingOutput: Fn(&Channel<<K::Target as SignerProvider>::Signer>, &Transaction) -> Result<OutPoint, APIError>>(
                &self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction, find_funding_output: FundingOutput
        ) -> Result<(), APIError> {
-               let (chan, msg) = {
-                       let (res, chan) = {
-                               let per_peer_state = self.per_peer_state.read().unwrap();
-                               if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
-                                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
-                                       let peer_state = &mut *peer_state_lock;
+               let mut channel_state = self.channel_state.lock().unwrap();
+               let per_peer_state = self.per_peer_state.read().unwrap();
+               if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
+                       let peer_state = &mut *peer_state_lock;
+                       let (chan, msg) = {
+                               let (res, chan) = {
                                        match peer_state.channel_by_id.remove(temporary_channel_id) {
                                                Some(mut chan) => {
                                                        let funding_txo = find_funding_output(&chan, &funding_transaction)?;
@@ -2551,30 +2549,22 @@ where
                                                },
                                                None => { return Err(APIError::ChannelUnavailable { err: "No such channel".to_owned() }) },
                                        }
-                               } else {
-                                       return Err(APIError::APIMisuseError { err: format!("Can't find a peer with a node_id matching the passed counterparty_node_id {}", counterparty_node_id) })
+                               };
+                               match handle_error!(self, res, chan.get_counterparty_node_id()) {
+                                       Ok(funding_msg) => {
+                                               (chan, funding_msg)
+                                       },
+                                       Err(_) => { return Err(APIError::ChannelUnavailable {
+                                               err: "Error deriving keys or signing initial commitment transactions - either our RNG or our counterparty's RNG is broken or the Signer refused to sign".to_owned()
+                                       }) },
                                }
                        };
-                       match handle_error!(self, res, chan.get_counterparty_node_id()) {
-                               Ok(funding_msg) => {
-                                       (chan, funding_msg)
-                               },
-                               Err(_) => { return Err(APIError::ChannelUnavailable {
-                                       err: "Error deriving keys or signing initial commitment transactions - either our RNG or our counterparty's RNG is broken or the Signer refused to sign".to_owned()
-                               }) },
-                       }
-               };
 
-               let mut channel_state = self.channel_state.lock().unwrap();
-               channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
-                       node_id: chan.get_counterparty_node_id(),
-                       msg,
-               });
-               mem::drop(channel_state);
-               let per_peer_state = self.per_peer_state.read().unwrap();
-               if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
-                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
-                       let peer_state = &mut *peer_state_lock;
+                       channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
+                               node_id: chan.get_counterparty_node_id(),
+                               msg,
+                       });
+                       mem::drop(channel_state);
                        match peer_state.channel_by_id.entry(chan.channel_id()) {
                                hash_map::Entry::Occupied(_) => {
                                        panic!("Generated duplicate funding txid?");
@@ -2587,8 +2577,10 @@ where
                                        e.insert(chan);
                                }
                        }
-               } else { return Err(APIError::ChannelUnavailable { err: format!("Peer with counterparty_node_id {} disconnected and closed the channel", counterparty_node_id) }) }
-               Ok(())
+                       Ok(())
+               } else {
+                       return Err(APIError::APIMisuseError { err: format!("Can't find a peer with a node_id matching the passed counterparty_node_id {}", counterparty_node_id) })
+               }
        }
 
        #[cfg(test)]