From 076be4204025bac1589f45bf3e253113e2fb3cbd Mon Sep 17 00:00:00 2001 From: fmoletta <99273364+fmoletta@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:02:15 -0300 Subject: [PATCH] feat(l1): show the disconnect reason in the HandshakeError when a peer disconnects mid-handshake (#1667) **Motivation** When connecting to the sepolia testnet I encountered a lot of handshake errors but all of them said "Expected Hello" or "Expected Status" with no other information. Most of these handshake failures are due to the peer sending a disconnect message instead. This PR aims to improve observability & debugging experience by showing the disconnect reason in the `HandshakeError`. **Description** * Show the disconnect reason in the `HandshakeError` message when a `Disconnect` message is received during the handshake process (p2p) Closes None --- crates/networking/p2p/rlpx/connection.rs | 47 +++++++++++++++--------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/crates/networking/p2p/rlpx/connection.rs b/crates/networking/p2p/rlpx/connection.rs index 900098c5a4..59fd467902 100644 --- a/crates/networking/p2p/rlpx/connection.rs +++ b/crates/networking/p2p/rlpx/connection.rs @@ -238,24 +238,31 @@ impl RLPxConnection { self.send(hello_msg).await?; // Receive Hello message - if let Message::Hello(hello_message) = self.receive().await? { - self.capabilities = hello_message.capabilities; - - // Check if we have any capability in common - for cap in self.capabilities.clone() { - if SUPPORTED_CAPABILITIES.contains(&cap) { - return Ok(()); + match self.receive().await? { + Message::Hello(hello_message) => { + self.capabilities = hello_message.capabilities; + + // Check if we have any capability in common + for cap in self.capabilities.clone() { + if SUPPORTED_CAPABILITIES.contains(&cap) { + return Ok(()); + } } + // Return error if not + Err(RLPxError::HandshakeError( + "No matching capabilities".to_string(), + )) + } + Message::Disconnect(disconnect) => Err(RLPxError::HandshakeError(format!( + "Peer disconnected due to: {}", + disconnect.reason() + ))), + _ => { + // Fail if it is not a hello message + Err(RLPxError::HandshakeError( + "Expected Hello message".to_string(), + )) } - // Return error if not - Err(RLPxError::HandshakeError( - "No matching capabilities".to_string(), - )) - } else { - // Fail if it is not a hello message - Err(RLPxError::HandshakeError( - "Expected Hello message".to_string(), - )) } } @@ -479,7 +486,13 @@ impl RLPxConnection { debug!("Received Status"); backend::validate_status(msg_data, &self.storage)? } - _msg => { + Message::Disconnect(disconnect) => { + return Err(RLPxError::HandshakeError(format!( + "Peer disconnected due to: {}", + disconnect.reason() + ))) + } + _ => { return Err(RLPxError::HandshakeError( "Expected a Status message".to_string(), ))