]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Add new DisconnectPeerWithWarning variant to ErrorAction
authorWilmer Paulino <wilmer@wilmerpaulino.com>
Thu, 18 May 2023 19:12:15 +0000 (12:12 -0700)
committerWilmer Paulino <wilmer@wilmerpaulino.com>
Fri, 26 May 2023 21:40:18 +0000 (14:40 -0700)
lightning/src/ln/msgs.rs
lightning/src/ln/peer_handler.rs

index 76ed56635ac6a57bd22cb82f5bba93e4c210b5c6..d32eb3e4743c460707d2d48b00a195b1f313fc68 100644 (file)
@@ -1137,6 +1137,11 @@ pub enum ErrorAction {
                /// An error message which we should make an effort to send before we disconnect.
                msg: Option<ErrorMessage>
        },
+       /// The peer did something incorrect. Tell them without closing any channels and disconnect them.
+       DisconnectPeerWithWarning {
+               /// A warning message which we should make an effort to send before we disconnect.
+               msg: WarningMessage,
+       },
        /// The peer did something harmless that we weren't able to process, just log and ignore
        // New code should *not* use this. New code must use IgnoreAndLog, below!
        IgnoreError,
index 000e754c53bea88133beca43561b3e28b6a7d9b1..a7f35a59c275baeb70ca6fdb607fe85283c4e965 100644 (file)
@@ -1230,8 +1230,21 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                                                                Ok(x) => x,
                                                                Err(e) => {
                                                                        match e.action {
-                                                                               msgs::ErrorAction::DisconnectPeer { msg: _ } => {
-                                                                                       //TODO: Try to push msg
+                                                                               msgs::ErrorAction::DisconnectPeer { .. } => {
+                                                                                       // We may have an `ErrorMessage` to send to the peer,
+                                                                                       // but writing to the socket while reading can lead to
+                                                                                       // re-entrant code and possibly unexpected behavior. The
+                                                                                       // message send is optimistic anyway, and in this case
+                                                                                       // we immediately disconnect the peer.
+                                                                                       log_debug!(self.logger, "Error handling message{}; disconnecting peer with: {}", OptionalFromDebugger(&peer_node_id), e.err);
+                                                                                       return Err(PeerHandleError { });
+                                                                               },
+                                                                               msgs::ErrorAction::DisconnectPeerWithWarning { .. } => {
+                                                                                       // We have a `WarningMessage` to send to the peer, but
+                                                                                       // writing to the socket while reading can lead to
+                                                                                       // re-entrant code and possibly unexpected behavior. The
+                                                                                       // message send is optimistic anyway, and in this case
+                                                                                       // we immediately disconnect the peer.
                                                                                        log_debug!(self.logger, "Error handling message{}; disconnecting peer with: {}", OptionalFromDebugger(&peer_node_id), e.err);
                                                                                        return Err(PeerHandleError { });
                                                                                },
@@ -1362,7 +1375,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                                                                                Ok(x) => x,
                                                                                Err(e) => {
                                                                                        match e {
-                                                                                               // Note that to avoid recursion we never call
+                                                                                               // Note that to avoid re-entrancy we never call
                                                                                                // `do_attempt_write_data` from here, causing
                                                                                                // the messages enqueued here to not actually
                                                                                                // be sent before the peer is disconnected.
@@ -2064,32 +2077,48 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                                                                        log_pubkey!(node_id), msg.contents.short_channel_id);
                                                        self.enqueue_message(&mut *get_peer_for_forwarding!(node_id), msg);
                                                },
-                                               MessageSendEvent::HandleError { ref node_id, ref action } => {
-                                                       match *action {
-                                                               msgs::ErrorAction::DisconnectPeer { ref msg } => {
+                                               MessageSendEvent::HandleError { node_id, action } => {
+                                                       match action {
+                                                               msgs::ErrorAction::DisconnectPeer { msg } => {
+                                                                       if let Some(msg) = msg.as_ref() {
+                                                                               log_trace!(self.logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
+                                                                                       log_pubkey!(node_id), msg.data);
+                                                                       } else {
+                                                                               log_trace!(self.logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
+                                                                                       log_pubkey!(node_id));
+                                                                       }
+                                                                       // We do not have the peers write lock, so we just store that we're
+                                                                       // about to disconenct 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));
+                                                                       peers_to_disconnect.insert(node_id, msg);
+                                                               },
+                                                               msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
+                                                                       log_trace!(self.logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
+                                                                               log_pubkey!(node_id), msg.data);
                                                                        // We do not have the peers write lock, so we just store that we're
                                                                        // about to disconenct the peer and do it after we finish
                                                                        // processing most messages.
-                                                                       peers_to_disconnect.insert(*node_id, msg.clone());
+                                                                       peers_to_disconnect.insert(node_id, Some(wire::Message::Warning(msg)));
                                                                },
                                                                msgs::ErrorAction::IgnoreAndLog(level) => {
                                                                        log_given_level!(self.logger, level, "Received a HandleError event to be ignored for node {}", log_pubkey!(node_id));
                                                                },
                                                                msgs::ErrorAction::IgnoreDuplicateGossip => {},
                                                                msgs::ErrorAction::IgnoreError => {
-                                                                       log_debug!(self.logger, "Received a HandleError event to be ignored for node {}", log_pubkey!(node_id));
-                                                               },
+                                                                               log_debug!(self.logger, "Received a HandleError event to be ignored for node {}", log_pubkey!(node_id));
+                                                                       },
                                                                msgs::ErrorAction::SendErrorMessage { ref msg } => {
                                                                        log_trace!(self.logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
                                                                                        log_pubkey!(node_id),
                                                                                        msg.data);
-                                                                       self.enqueue_message(&mut *get_peer_for_forwarding!(node_id), msg);
+                                                                       self.enqueue_message(&mut *get_peer_for_forwarding!(&node_id), msg);
                                                                },
                                                                msgs::ErrorAction::SendWarningMessage { ref msg, ref log_level } => {
                                                                        log_given_level!(self.logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
                                                                                        log_pubkey!(node_id),
                                                                                        msg.data);
-                                                                       self.enqueue_message(&mut *get_peer_for_forwarding!(node_id), msg);
+                                                                       self.enqueue_message(&mut *get_peer_for_forwarding!(&node_id), msg);
                                                                },
                                                        }
                                                },
@@ -2139,9 +2168,6 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                                                if let Some(peer_mutex) = peers.remove(&descriptor) {
                                                        let mut peer = peer_mutex.lock().unwrap();
                                                        if let Some(msg) = msg {
-                                                               log_trace!(self.logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
-                                                                               log_pubkey!(node_id),
-                                                                               msg.data);
                                                                self.enqueue_message(&mut *peer, &msg);
                                                                // This isn't guaranteed to work, but if there is enough free
                                                                // room in the send buffer, put the error message there...