Merge pull request #2967 from tnull/2024-03-refactor-drop-handle-message

Split `PeerManager::handle_message` to avoid explicit `mem::drop`

diff --combined lightning/src/ln/peer_handler.rs
index cfa32a009fe5f9213be2f250b2e033473b16c1b2,0188130d1f1abee04a330513eecaf9da51ba502b..188023dc99c17316bf65f0190f09d01f14aa3e6d
--- 1/lightning/src/ln/peer_handler.rs
--- 2/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@@ -116,9 -116,7 +116,9 @@@ impl RoutingMessageHandler for Ignoring
        fn handle_query_short_channel_ids(&self, _their_node_id: &PublicKey, _msg: msgs::QueryShortChannelIds) -> Result<(), LightningError> { Ok(()) }
        fn provided_node_features(&self) -> NodeFeatures { NodeFeatures::empty() }
        fn provided_init_features(&self, _their_node_id: &PublicKey) -> InitFeatures {
 -              InitFeatures::empty()
 +              let mut features = InitFeatures::empty();
 +              features.set_gossip_queries_optional();
 +              features
        }
        fn processing_queue_high(&self) -> bool { false }
  }
@@@ -1553,7 -1551,6 +1553,7 @@@ impl<Descriptor: SocketDescriptor, CM: 
                                                                                                }
                                                                                                (msgs::DecodeError::BadLengthDescriptor, _) => return Err(PeerHandleError { }),
                                                                                                (msgs::DecodeError::Io(_), _) => return Err(PeerHandleError { }),
 +                                                                                              (msgs::DecodeError::DangerousValue, _) => return Err(PeerHandleError { }),
                                                                                        }
                                                                                }
                                                                        };
@@@ -1591,15 -1588,37 +1591,37 @@@
        }
  
        /// Process an incoming message and return a decision (ok, lightning error, peer handling error) regarding the next action with the peer
+       ///
        /// Returns the message back if it needs to be broadcasted to all other peers.
        fn handle_message(
                &self,
                peer_mutex: &Mutex<Peer>,
-               mut peer_lock: MutexGuard<Peer>,
-               message: wire::Message<<<CMH as core::ops::Deref>::Target as wire::CustomMessageReader>::CustomMessage>
-       ) -> Result<Option<wire::Message<<<CMH as core::ops::Deref>::Target as wire::CustomMessageReader>::CustomMessage>>, MessageHandlingError> {
+               peer_lock: MutexGuard<Peer>,
+               message: wire::Message<<<CMH as Deref>::Target as wire::CustomMessageReader>::CustomMessage>
+       ) -> Result<Option<wire::Message<<<CMH as Deref>::Target as wire::CustomMessageReader>::CustomMessage>>, MessageHandlingError> {
                let their_node_id = peer_lock.their_node_id.clone().expect("We know the peer's public key by the time we receive messages").0;
                let logger = WithContext::from(&self.logger, Some(their_node_id), None);
+ 
+               let message = match self.do_handle_message_holding_peer_lock(peer_lock, message, &their_node_id, &logger)? {
+                       Some(processed_message) => processed_message,
+                       None => return Ok(None),
+               };
+ 
+               self.do_handle_message_without_peer_lock(peer_mutex, message, &their_node_id, &logger)
+       }
+ 
+       // Conducts all message processing that requires us to hold the `peer_lock`.
+       //
+       // Returns `None` if the message was fully processed and otherwise returns the message back to
+       // allow it to be subsequently processed by `do_handle_message_without_peer_lock`.
+       fn do_handle_message_holding_peer_lock<'a>(
+               &self,
+               mut peer_lock: MutexGuard<Peer>,
+               message: wire::Message<<<CMH as Deref>::Target as wire::CustomMessageReader>::CustomMessage>,
+               their_node_id: &PublicKey,
+               logger: &WithContext<'a, L>
+       ) -> Result<Option<wire::Message<<<CMH as Deref>::Target as wire::CustomMessageReader>::CustomMessage>>, MessageHandlingError>
+       {
                peer_lock.received_message_since_timer_tick = true;
  
                // Need an Init as first message
@@@ -1680,8 -1699,20 +1702,20 @@@
                        peer_lock.received_channel_announce_since_backlogged = true;
                }
  
-               mem::drop(peer_lock);
+               Ok(Some(message))
+       }
  
+       // Conducts all message processing that doesn't require us to hold the `peer_lock`.
+       //
+       // Returns the message back if it needs to be broadcasted to all other peers.
+       fn do_handle_message_without_peer_lock<'a>(
+               &self,
+               peer_mutex: &Mutex<Peer>,
+               message: wire::Message<<<CMH as Deref>::Target as wire::CustomMessageReader>::CustomMessage>,
+               their_node_id: &PublicKey,
+               logger: &WithContext<'a, L>
+       ) -> Result<Option<wire::Message<<<CMH as Deref>::Target as wire::CustomMessageReader>::CustomMessage>>, MessageHandlingError>
+       {
                if is_gossip_msg(message.type_id()) {
                        log_gossip!(logger, "Received message {:?} from {}", message, log_pubkey!(their_node_id));
                } else {
@@@ -1883,7 -1914,7 +1917,7 @@@
                Ok(should_forward)
        }
  
-       fn forward_broadcast_msg(&self, peers: &HashMap<Descriptor, Mutex<Peer>>, msg: &wire::Message<<<CMH as core::ops::Deref>::Target as wire::CustomMessageReader>::CustomMessage>, except_node: Option<&PublicKey>) {
+       fn forward_broadcast_msg(&self, peers: &HashMap<Descriptor, Mutex<Peer>>, msg: &wire::Message<<<CMH as Deref>::Target as wire::CustomMessageReader>::CustomMessage>, except_node: Option<&PublicKey>) {
                match msg {
                        wire::Message::ChannelAnnouncement(ref msg) => {
                                log_gossip!(self.logger, "Sending message to all peers except {:?} or the announced channel's counterparties: {:?}", except_node, msg);
@@@ -2275,7 -2306,7 +2309,7 @@@
                                                                        // We do not have the peers write lock, so we just store that we're
                                                                        // about to disconnect the peer and do it after we finish
                                                                        // processing most messages.
-                                                                       let msg = msg.map(|msg| wire::Message::<<<CMH as core::ops::Deref>::Target as wire::CustomMessageReader>::CustomMessage>::Error(msg));
+                                                                       let msg = msg.map(|msg| wire::Message::<<<CMH as Deref>::Target as wire::CustomMessageReader>::CustomMessage>::Error(msg));
                                                                        peers_to_disconnect.insert(node_id, msg);
                                                                },
                                                                msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {