From ebd686a40742ad983fb0d3516adc7ba95b6744e4 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 26 Dec 2022 15:33:59 +0100 Subject: [PATCH] refactor(net): rename and restructure wire error types (#614) --- crates/net/eth-wire/src/errors/eth.rs | 53 +++++++++++++++++++ crates/net/eth-wire/src/errors/mod.rs | 7 +++ .../eth-wire/src/{error.rs => errors/p2p.rs} | 52 +----------------- crates/net/eth-wire/src/ethstream.rs | 22 ++++---- crates/net/eth-wire/src/lib.rs | 2 +- crates/net/eth-wire/src/p2pstream.rs | 2 +- crates/net/eth-wire/src/pinger.rs | 2 +- crates/net/network/src/error.rs | 8 +-- crates/net/network/src/peers/manager.rs | 8 +-- crates/net/network/src/session/active.rs | 4 +- crates/net/network/src/session/handle.rs | 2 +- crates/net/network/src/session/mod.rs | 2 +- crates/net/network/src/swarm.rs | 2 +- 13 files changed, 89 insertions(+), 77 deletions(-) create mode 100644 crates/net/eth-wire/src/errors/eth.rs create mode 100644 crates/net/eth-wire/src/errors/mod.rs rename crates/net/eth-wire/src/{error.rs => errors/p2p.rs} (62%) diff --git a/crates/net/eth-wire/src/errors/eth.rs b/crates/net/eth-wire/src/errors/eth.rs new file mode 100644 index 0000000000..c988e70342 --- /dev/null +++ b/crates/net/eth-wire/src/errors/eth.rs @@ -0,0 +1,53 @@ +//! Error handling for (`EthStream`)[crate::EthStream] +use crate::{errors::P2PStreamError, DisconnectReason}; +use reth_primitives::{Chain, ValidationError, H256}; +use std::io; + +/// Errors when sending/receiving messages +#[derive(thiserror::Error, Debug)] +#[allow(missing_docs)] +pub enum EthStreamError { + #[error(transparent)] + Io(#[from] io::Error), + #[error(transparent)] + Rlp(#[from] reth_rlp::DecodeError), + #[error(transparent)] + P2PStreamError(#[from] P2PStreamError), + #[error(transparent)] + EthHandshakeError(#[from] EthHandshakeError), + #[error("message size ({0}) exceeds max length (10MB)")] + MessageTooBig(usize), +} + +// === impl EthStreamError === + +impl EthStreamError { + /// Returns the [`DisconnectReason`] if the error is a disconnect message + pub fn as_disconnected(&self) -> Option { + if let EthStreamError::P2PStreamError(err) = self { + err.as_disconnected() + } else { + None + } + } +} + +/// Error variants that can occur during the `eth` sub-protocol handshake. +#[derive(thiserror::Error, Debug)] +#[allow(missing_docs)] +pub enum EthHandshakeError { + #[error("status message can only be recv/sent in handshake")] + StatusNotInHandshake, + #[error("received non-status message when trying to handshake")] + NonStatusMessageInHandshake, + #[error("no response received when sending out handshake")] + NoResponse, + #[error(transparent)] + InvalidFork(#[from] ValidationError), + #[error("mismatched genesis in Status message. expected: {expected:?}, got: {got:?}")] + MismatchedGenesis { expected: H256, got: H256 }, + #[error("mismatched protocol version in Status message. expected: {expected:?}, got: {got:?}")] + MismatchedProtocolVersion { expected: u8, got: u8 }, + #[error("mismatched chain in Status message. expected: {expected:?}, got: {got:?}")] + MismatchedChain { expected: Chain, got: Chain }, +} diff --git a/crates/net/eth-wire/src/errors/mod.rs b/crates/net/eth-wire/src/errors/mod.rs new file mode 100644 index 0000000000..be3f8ced7f --- /dev/null +++ b/crates/net/eth-wire/src/errors/mod.rs @@ -0,0 +1,7 @@ +//! Error types for stream variants + +mod eth; +mod p2p; + +pub use eth::*; +pub use p2p::*; diff --git a/crates/net/eth-wire/src/error.rs b/crates/net/eth-wire/src/errors/p2p.rs similarity index 62% rename from crates/net/eth-wire/src/error.rs rename to crates/net/eth-wire/src/errors/p2p.rs index 31f4b102d8..93562c2c69 100644 --- a/crates/net/eth-wire/src/error.rs +++ b/crates/net/eth-wire/src/errors/p2p.rs @@ -1,60 +1,10 @@ -//! Error cases when handling a [`crate::EthStream`] +//! Error handling for [`P2PStream`](crate::P2PStream) use std::io; -use reth_primitives::{Chain, ValidationError, H256}; - use crate::{ capability::SharedCapabilityError, disconnect::UnknownDisconnectReason, DisconnectReason, }; -/// Errors when sending/receiving messages -#[derive(thiserror::Error, Debug)] -#[allow(missing_docs)] -pub enum EthStreamError { - #[error(transparent)] - Io(#[from] io::Error), - #[error(transparent)] - Rlp(#[from] reth_rlp::DecodeError), - #[error(transparent)] - P2PStreamError(#[from] P2PStreamError), - #[error(transparent)] - HandshakeError(#[from] HandshakeError), - #[error("message size ({0}) exceeds max length (10MB)")] - MessageTooBig(usize), -} - -// === impl EthStreamError === - -impl EthStreamError { - /// Returns the [`DisconnectReason`] if the error is a disconnect message - pub fn as_disconnected(&self) -> Option { - if let EthStreamError::P2PStreamError(err) = self { - err.as_disconnected() - } else { - None - } - } -} - -#[derive(thiserror::Error, Debug)] -#[allow(missing_docs)] -pub enum HandshakeError { - #[error("status message can only be recv/sent in handshake")] - StatusNotInHandshake, - #[error("received non-status message when trying to handshake")] - NonStatusMessageInHandshake, - #[error("no response received when sending out handshake")] - NoResponse, - #[error(transparent)] - InvalidFork(#[from] ValidationError), - #[error("mismatched genesis in Status message. expected: {expected:?}, got: {got:?}")] - MismatchedGenesis { expected: H256, got: H256 }, - #[error("mismatched protocol version in Status message. expected: {expected:?}, got: {got:?}")] - MismatchedProtocolVersion { expected: u8, got: u8 }, - #[error("mismatched chain in Status message. expected: {expected:?}, got: {got:?}")] - MismatchedChain { expected: Chain, got: Chain }, -} - /// Errors when sending/receiving p2p messages. These should result in kicking the peer. #[derive(thiserror::Error, Debug)] #[allow(missing_docs)] diff --git a/crates/net/eth-wire/src/ethstream.rs b/crates/net/eth-wire/src/ethstream.rs index 977ee4e07a..19c24a2c7e 100644 --- a/crates/net/eth-wire/src/ethstream.rs +++ b/crates/net/eth-wire/src/ethstream.rs @@ -1,5 +1,5 @@ use crate::{ - error::{EthStreamError, HandshakeError}, + errors::{EthHandshakeError, EthStreamError}, message::{EthBroadcastMessage, ProtocolBroadcastMessage}, types::{EthMessage, ProtocolMessage, Status}, }; @@ -65,7 +65,7 @@ where .inner .next() .await - .ok_or(EthStreamError::HandshakeError(HandshakeError::NoResponse))??; + .ok_or(EthStreamError::EthHandshakeError(EthHandshakeError::NoResponse))??; if their_msg.len() > MAX_MESSAGE_SIZE { return Err(EthStreamError::MessageTooBig(their_msg.len())) @@ -84,7 +84,7 @@ where match msg.message { EthMessage::Status(resp) => { if status.genesis != resp.genesis { - return Err(HandshakeError::MismatchedGenesis { + return Err(EthHandshakeError::MismatchedGenesis { expected: status.genesis, got: resp.genesis, } @@ -92,7 +92,7 @@ where } if status.version != resp.version { - return Err(HandshakeError::MismatchedProtocolVersion { + return Err(EthHandshakeError::MismatchedProtocolVersion { expected: status.version, got: resp.version, } @@ -100,14 +100,14 @@ where } if status.chain != resp.chain { - return Err(HandshakeError::MismatchedChain { + return Err(EthHandshakeError::MismatchedChain { expected: status.chain, got: resp.chain, } .into()) } - fork_filter.validate(resp.forkid).map_err(HandshakeError::InvalidFork)?; + fork_filter.validate(resp.forkid).map_err(EthHandshakeError::InvalidFork)?; // now we can create the `EthStream` because the peer has successfully completed // the handshake @@ -115,7 +115,9 @@ where Ok((stream, resp)) } - _ => Err(EthStreamError::HandshakeError(HandshakeError::NonStatusMessageInHandshake)), + _ => Err(EthStreamError::EthHandshakeError( + EthHandshakeError::NonStatusMessageInHandshake, + )), } } } @@ -201,8 +203,8 @@ where }; if matches!(msg.message, EthMessage::Status(_)) { - return Poll::Ready(Some(Err(EthStreamError::HandshakeError( - HandshakeError::StatusNotInHandshake, + return Poll::Ready(Some(Err(EthStreamError::EthHandshakeError( + EthHandshakeError::StatusNotInHandshake, )))) } @@ -223,7 +225,7 @@ where fn start_send(self: Pin<&mut Self>, item: EthMessage) -> Result<(), Self::Error> { if matches!(item, EthMessage::Status(_)) { - return Err(EthStreamError::HandshakeError(HandshakeError::StatusNotInHandshake)) + return Err(EthStreamError::EthHandshakeError(EthHandshakeError::StatusNotInHandshake)) } let mut bytes = BytesMut::new(); diff --git a/crates/net/eth-wire/src/lib.rs b/crates/net/eth-wire/src/lib.rs index 3cac4f8e6f..309f38a226 100644 --- a/crates/net/eth-wire/src/lib.rs +++ b/crates/net/eth-wire/src/lib.rs @@ -9,7 +9,7 @@ pub mod builder; pub mod capability; mod disconnect; -pub mod error; +pub mod errors; mod ethstream; mod hello; mod p2pstream; diff --git a/crates/net/eth-wire/src/p2pstream.rs b/crates/net/eth-wire/src/p2pstream.rs index c21db5fdc7..5bba6a390b 100644 --- a/crates/net/eth-wire/src/p2pstream.rs +++ b/crates/net/eth-wire/src/p2pstream.rs @@ -1,7 +1,7 @@ #![allow(dead_code, unreachable_pub, missing_docs, unused_variables)] use crate::{ capability::{Capability, SharedCapability}, - error::{P2PHandshakeError, P2PStreamError}, + errors::{P2PHandshakeError, P2PStreamError}, pinger::{Pinger, PingerEvent}, DisconnectReason, HelloMessage, }; diff --git a/crates/net/eth-wire/src/pinger.rs b/crates/net/eth-wire/src/pinger.rs index 3c3eac4dd0..e8ef15a3c2 100644 --- a/crates/net/eth-wire/src/pinger.rs +++ b/crates/net/eth-wire/src/pinger.rs @@ -1,4 +1,4 @@ -use crate::error::PingerError; +use crate::errors::PingerError; use std::{ pin::Pin, task::{Context, Poll}, diff --git a/crates/net/network/src/error.rs b/crates/net/network/src/error.rs index 695387e2ee..76c9096afd 100644 --- a/crates/net/network/src/error.rs +++ b/crates/net/network/src/error.rs @@ -2,7 +2,7 @@ use crate::session::PendingSessionHandshakeError; use reth_eth_wire::{ - error::{EthStreamError, HandshakeError, P2PHandshakeError, P2PStreamError}, + errors::{EthHandshakeError, EthStreamError, P2PHandshakeError, P2PStreamError}, DisconnectReason, }; use std::fmt; @@ -48,7 +48,7 @@ impl SessionError for EthStreamError { EthStreamError::P2PStreamError(P2PStreamError::HandshakeError( P2PHandshakeError::NonHelloMessageInHandshake, )) => true, - EthStreamError::HandshakeError(err) => !matches!(err, HandshakeError::NoResponse), + EthStreamError::EthHandshakeError(err) => !matches!(err, EthHandshakeError::NoResponse), _ => false, } } @@ -83,7 +83,7 @@ impl SessionError for EthStreamError { P2PStreamError::MismatchedProtocolVersion { .. } ) } - EthStreamError::HandshakeError(err) => !matches!(err, HandshakeError::NoResponse), + EthStreamError::EthHandshakeError(err) => !matches!(err, EthHandshakeError::NoResponse), _ => false, } } @@ -91,7 +91,7 @@ impl SessionError for EthStreamError { fn should_backoff(&self) -> bool { matches!( self, - EthStreamError::HandshakeError(HandshakeError::NoResponse) | + EthStreamError::EthHandshakeError(EthHandshakeError::NoResponse) | EthStreamError::P2PStreamError(P2PStreamError::HandshakeError( P2PHandshakeError::NoResponse )) diff --git a/crates/net/network/src/peers/manager.rs b/crates/net/network/src/peers/manager.rs index 4454b86c2e..546d391443 100644 --- a/crates/net/network/src/peers/manager.rs +++ b/crates/net/network/src/peers/manager.rs @@ -7,7 +7,7 @@ use crate::{ session::{Direction, PendingSessionHandshakeError}, }; use futures::StreamExt; -use reth_eth_wire::{error::EthStreamError, DisconnectReason}; +use reth_eth_wire::{errors::EthStreamError, DisconnectReason}; use reth_net_common::ban_list::BanList; use reth_primitives::{ForkId, PeerId}; use std::{ @@ -871,7 +871,7 @@ mod test { PeersConfig, }; use reth_eth_wire::{ - error::{EthStreamError, HandshakeError, P2PHandshakeError, P2PStreamError}, + errors::{EthHandshakeError, EthStreamError, P2PHandshakeError, P2PStreamError}, DisconnectReason, }; use reth_net_common::ban_list::BanList; @@ -1042,8 +1042,8 @@ mod test { peers.on_pending_session_dropped( &socket_addr, &peer, - &PendingSessionHandshakeError::Eth(EthStreamError::HandshakeError( - HandshakeError::NoResponse, + &PendingSessionHandshakeError::Eth(EthStreamError::EthHandshakeError( + EthHandshakeError::NoResponse, )), ); diff --git a/crates/net/network/src/session/active.rs b/crates/net/network/src/session/active.rs index 1e795ca420..c4fd61b370 100644 --- a/crates/net/network/src/session/active.rs +++ b/crates/net/network/src/session/active.rs @@ -12,7 +12,7 @@ use futures::{stream::Fuse, SinkExt, StreamExt}; use reth_ecies::stream::ECIESStream; use reth_eth_wire::{ capability::Capabilities, - error::{EthStreamError, HandshakeError, P2PStreamError}, + errors::{EthHandshakeError, EthStreamError, P2PStreamError}, message::{EthBroadcastMessage, RequestPair}, DisconnectReason, EthMessage, EthStream, P2PStream, }; @@ -136,7 +136,7 @@ impl ActiveSession { match msg { msg @ EthMessage::Status(_) => { return Some(( - EthStreamError::HandshakeError(HandshakeError::StatusNotInHandshake), + EthStreamError::EthHandshakeError(EthHandshakeError::StatusNotInHandshake), msg, )) } diff --git a/crates/net/network/src/session/handle.rs b/crates/net/network/src/session/handle.rs index 9becad183f..2021afc3aa 100644 --- a/crates/net/network/src/session/handle.rs +++ b/crates/net/network/src/session/handle.rs @@ -6,7 +6,7 @@ use crate::{ use reth_ecies::{stream::ECIESStream, ECIESError}; use reth_eth_wire::{ capability::{Capabilities, CapabilityMessage}, - error::EthStreamError, + errors::EthStreamError, DisconnectReason, EthStream, P2PStream, Status, }; use reth_primitives::PeerId; diff --git a/crates/net/network/src/session/mod.rs b/crates/net/network/src/session/mod.rs index aa848c60a0..ce3277810a 100644 --- a/crates/net/network/src/session/mod.rs +++ b/crates/net/network/src/session/mod.rs @@ -16,7 +16,7 @@ use futures::{future::Either, io, FutureExt, StreamExt}; use reth_ecies::{stream::ECIESStream, ECIESError}; use reth_eth_wire::{ capability::{Capabilities, CapabilityMessage}, - error::EthStreamError, + errors::EthStreamError, DisconnectReason, HelloMessage, Status, UnauthedEthStream, UnauthedP2PStream, }; use reth_primitives::{ForkFilter, ForkId, ForkTransition, PeerId, H256, U256}; diff --git a/crates/net/network/src/swarm.rs b/crates/net/network/src/swarm.rs index b281a0ded7..7e3c3d00db 100644 --- a/crates/net/network/src/swarm.rs +++ b/crates/net/network/src/swarm.rs @@ -8,7 +8,7 @@ use crate::{ use futures::Stream; use reth_eth_wire::{ capability::{Capabilities, CapabilityMessage}, - error::EthStreamError, + errors::EthStreamError, Status, }; use reth_primitives::PeerId;