errors: Add DialError error and ListDialFailures event for better…

… error reporting (#206)

The purpose of this PR is to pave the way for making the Identify
protocol more robust, which is currently linked with the low number of
peers and connective issues over a long period of time
- paritytech/polkadot-sdk#4925

This PR adds a coherent `DialError` that exposes the minimal information
users need to know about dial failures.
- paritytech/polkadot-sdk#5239

A new litep2p event is added for reporting multiple dial errors that
occur on different protocols back to the user:

```rust
    /// A list of multiple dial failures.
    ListDialFailures {
        /// List of errors.
        ///
        /// Depending on the transport, the address might be different for each error.
        errors: Vec<(Multiaddr, DialError)>,
    },
```

This event eases the debugging of substrate connectivity issues. At the
same time, it can be used in a future PR to inform back to the Identify
protocol which self-reported addresses of some peers are unreachable:
- #203

### Next Steps
- Add more tests
- Warp sync + sync full nodes since this is touching individual
transports

### Future Work
- The overarching `litep2p::Error` needs a closer look and a
refactoring:
  - #204
  - #128
  
- ConnectionError event for individual transports can be simplified:
  - #205
  
- I've observed some inconsistencies in handling TCP vs WebSocket
connection timeouts. I believe that we can have another pass and share
even more code between them:
  - #70

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Dmitry Markin <dmitry@markin.tech>

src/crypto/ed25519.rs

@@ -21,7 +21,10 @@
 
 //! Ed25519 keys.
 
-use crate::{error::Error, PeerId};
+use crate::{
+    error::{Error, ParseError},
+    PeerId,
+};
 
 use ed25519_dalek::{self as ed25519, Signer as _, Verifier as _};
 use std::fmt;
@@ -131,11 +134,13 @@ impl PublicKey {
 
     /// Try to parse a public key from a byte array containing the actual key as produced by
     /// `to_bytes`.
-    pub fn try_from_bytes(k: &[u8]) -> crate::Result<PublicKey> {
-        let k = <[u8; 32]>::try_from(k)
-            .map_err(|e| Error::Other(format!("Failed to parse ed25519 public key: {e}")))?;
+    pub fn try_from_bytes(k: &[u8]) -> Result<PublicKey, ParseError> {
+        let k = <[u8; 32]>::try_from(k).map_err(|_| ParseError::InvalidPublicKey)?;
+
+        // The error type of the verifying key is deliberately opaque as to avoid side-channel
+        // leakage. We can't provide a more specific error type here.
         ed25519::VerifyingKey::from_bytes(&k)
-            .map_err(|e| Error::Other(format!("Failed to parse ed25519 public key: {e}")))
+            .map_err(|_| ParseError::InvalidPublicKey)
             .map(PublicKey)
     }
 

src/crypto/mod.rs

@@ -21,7 +21,7 @@
 
 //! Crypto-related code.
 
-use crate::{error::*, peer_id::*};
+use crate::{error::ParseError, peer_id::*};
 
 pub mod ed25519;
 pub(crate) mod noise;
@@ -65,11 +65,10 @@ impl PublicKey {
 
     /// Decode a public key from a protobuf structure, e.g. read from storage
     /// or received from another node.
-    pub fn from_protobuf_encoding(bytes: &[u8]) -> crate::Result<PublicKey> {
+    pub fn from_protobuf_encoding(bytes: &[u8]) -> Result<PublicKey, ParseError> {
         use prost::Message;
 
-        let pubkey = keys_proto::PublicKey::decode(bytes)
-            .map_err(|error| Error::Other(format!("Invalid Protobuf: {error:?}")))?;
+        let pubkey = keys_proto::PublicKey::decode(bytes)?;
 
         pubkey.try_into()
     }
@@ -92,19 +91,16 @@ impl From<&PublicKey> for keys_proto::PublicKey {
 }
 
 impl TryFrom<keys_proto::PublicKey> for PublicKey {
-    type Error = Error;
+    type Error = ParseError;
 
     fn try_from(pubkey: keys_proto::PublicKey) -> Result<Self, Self::Error> {
         let key_type = keys_proto::KeyType::try_from(pubkey.r#type)
-            .map_err(|_| Error::Other(format!("Unknown key type: {}", pubkey.r#type)))?;
-
-        match key_type {
-            keys_proto::KeyType::Ed25519 =>
-                Ok(ed25519::PublicKey::try_from_bytes(&pubkey.data).map(PublicKey::Ed25519)?),
-            _ => Err(Error::Other(format!(
-                "Unsupported key type: {}",
-                key_type.as_str_name()
-            ))),
+            .map_err(|_| ParseError::UnknownKeyType(pubkey.r#type))?;
+
+        if key_type == keys_proto::KeyType::Ed25519 {
+            Ok(ed25519::PublicKey::try_from_bytes(&pubkey.data).map(PublicKey::Ed25519)?)
+        } else {
+            Err(ParseError::UnknownKeyType(key_type as i32))
         }
     }
 }

src/crypto/noise/mod.rs

@@ -24,7 +24,8 @@
 use crate::{
     config::Role,
     crypto::{ed25519::Keypair, PublicKey},
-    error, PeerId,
+    error::{NegotiationError, ParseError},
+    PeerId,
 };
 
 use bytes::{Buf, Bytes, BytesMut};
@@ -103,7 +104,7 @@ impl NoiseContext {
         keypair: snow::Keypair,
         id_keys: &Keypair,
         role: Role,
-    ) -> crate::Result<Self> {
+    ) -> Result<Self, NegotiationError> {
         let noise_payload = handshake_schema::NoiseHandshakePayload {
             identity_key: Some(PublicKey::Ed25519(id_keys.public()).to_protobuf_encoding()),
             identity_sig: Some(
@@ -113,7 +114,7 @@ impl NoiseContext {
         };
 
         let mut payload = Vec::with_capacity(noise_payload.encoded_len());
-        noise_payload.encode(&mut payload)?;
+        noise_payload.encode(&mut payload).map_err(ParseError::from)?;
 
         Ok(Self {
             noise: NoiseState::Handshake(noise),
@@ -123,7 +124,7 @@ impl NoiseContext {
         })
     }
 
-    pub fn new(keypair: &Keypair, role: Role) -> crate::Result<Self> {
+    pub fn new(keypair: &Keypair, role: Role) -> Result<Self, NegotiationError> {
         tracing::trace!(target: LOG_TARGET, ?role, "create new noise configuration");
 
         let builder: Builder<'_> = Builder::with_resolver(
@@ -144,7 +145,7 @@ impl NoiseContext {
 
     /// Create new [`NoiseContext`] with prologue.
     #[cfg(feature = "webrtc")]
-    pub fn with_prologue(id_keys: &Keypair, prologue: Vec<u8>) -> crate::Result<Self> {
+    pub fn with_prologue(id_keys: &Keypair, prologue: Vec<u8>) -> Result<Self, NegotiationError> {
         let noise: Builder<'_> = Builder::with_resolver(
             NOISE_PARAMETERS.parse().expect("qed; Valid noise pattern"),
             Box::new(protocol::Resolver),
@@ -162,45 +163,44 @@ impl NoiseContext {
 
     /// Get remote public key from the received Noise payload.
     #[cfg(feature = "webrtc")]
-    pub fn get_remote_public_key(&mut self, reply: &[u8]) -> crate::Result<PublicKey> {
+    pub fn get_remote_public_key(&mut self, reply: &[u8]) -> Result<PublicKey, NegotiationError> {
         let (len_slice, reply) = reply.split_at(2);
-        let len = u16::from_be_bytes(len_slice.try_into().map_err(|_| error::Error::InvalidData)?)
-            as usize;
+        let len = u16::from_be_bytes(
+            len_slice
+                .try_into()
+                .map_err(|_| NegotiationError::ParseError(ParseError::InvalidPublicKey))?,
+        ) as usize;
 
         let mut buffer = vec![0u8; len];
 
         let NoiseState::Handshake(ref mut noise) = self.noise else {
             tracing::error!(target: LOG_TARGET, "invalid state to read the second handshake message");
             debug_assert!(false);
-            return Err(error::Error::Other(
-                "Noise state missmatch: expected handshake".into(),
-            ));
+            return Err(NegotiationError::StateMismatch);
         };
 
         let res = noise.read_message(reply, &mut buffer)?;
         buffer.truncate(res);
 
-        let payload = handshake_schema::NoiseHandshakePayload::decode(buffer.as_slice())?;
+        let payload = handshake_schema::NoiseHandshakePayload::decode(buffer.as_slice())
+            .map_err(|err| NegotiationError::ParseError(err.into()))?;
 
-        PublicKey::from_protobuf_encoding(&payload.identity_key.ok_or(
-            error::Error::NegotiationError(error::NegotiationError::PeerIdMissing),
-        )?)
+        let identity = payload.identity_key.ok_or(NegotiationError::PeerIdMissing)?;
+        PublicKey::from_protobuf_encoding(&identity).map_err(|err| err.into())
     }
 
     /// Get first message.
     ///
     /// Listener only sends one message (the payload)
-    pub fn first_message(&mut self, role: Role) -> crate::Result<Vec<u8>> {
+    pub fn first_message(&mut self, role: Role) -> Result<Vec<u8>, NegotiationError> {
         match role {
             Role::Dialer => {
                 tracing::trace!(target: LOG_TARGET, "get noise dialer first message");
 
                 let NoiseState::Handshake(ref mut noise) = self.noise else {
                     tracing::error!(target: LOG_TARGET, "invalid state to read the first handshake message");
                     debug_assert!(false);
-                    return Err(error::Error::Other(
-                        "Noise state missmatch: expected handshake".into(),
-                    ));
+                    return Err(NegotiationError::StateMismatch);
                 };
 
                 let mut buffer = vec![0u8; 256];
@@ -220,15 +220,13 @@ impl NoiseContext {
     /// Get second message.
     ///
     /// Only the dialer sends the second message.
-    pub fn second_message(&mut self) -> crate::Result<Vec<u8>> {
+    pub fn second_message(&mut self) -> Result<Vec<u8>, NegotiationError> {
         tracing::trace!(target: LOG_TARGET, "get noise paylod message");
 
         let NoiseState::Handshake(ref mut noise) = self.noise else {
             tracing::error!(target: LOG_TARGET, "invalid state to read the first handshake message");
             debug_assert!(false);
-            return Err(error::Error::Other(
-                "Noise state missmatch: expected handshake".into(),
-            ));
+            return Err(NegotiationError::StateMismatch);
         };
 
         let mut buffer = vec![0u8; 2048];
@@ -246,7 +244,7 @@ impl NoiseContext {
     async fn read_handshake_message<T: AsyncRead + AsyncWrite + Unpin>(
         &mut self,
         io: &mut T,
-    ) -> crate::Result<Bytes> {
+    ) -> Result<Bytes, NegotiationError> {
         let mut size = BytesMut::zeroed(2);
         io.read_exact(&mut size).await?;
         let size = size.get_u16();
@@ -260,9 +258,7 @@ impl NoiseContext {
         let NoiseState::Handshake(ref mut noise) = self.noise else {
             tracing::error!(target: LOG_TARGET, "invalid state to read handshake message");
             debug_assert!(false);
-            return Err(error::Error::Other(
-                "Noise state missmatch: expected handshake".into(),
-            ));
+            return Err(NegotiationError::StateMismatch);
         };
 
         let nread = noise.read_message(&message, &mut out)?;
@@ -286,13 +282,10 @@ impl NoiseContext {
     }
 
     /// Convert Noise into transport mode.
-    fn into_transport(self) -> crate::Result<NoiseContext> {
+    fn into_transport(self) -> Result<NoiseContext, NegotiationError> {
         let transport = match self.noise {
             NoiseState::Handshake(noise) => noise.into_transport_mode()?,
-            NoiseState::Transport(_) =>
-                return Err(error::Error::Other(
-                    "Noise state missmatch: expected handshake".into(),
-                )),
+            NoiseState::Transport(_) => return Err(NegotiationError::StateMismatch),
         };
 
         Ok(NoiseContext {
@@ -664,15 +657,15 @@ impl<S: AsyncRead + AsyncWrite + Unpin> AsyncWrite for NoiseSocket<S> {
 }
 
 /// Try to parse `PeerId` from received `NoiseHandshakePayload`
-fn parse_peer_id(buf: &[u8]) -> crate::Result<PeerId> {
+fn parse_peer_id(buf: &[u8]) -> Result<PeerId, NegotiationError> {
     match handshake_schema::NoiseHandshakePayload::decode(buf) {
         Ok(payload) => {
-            let public_key = PublicKey::from_protobuf_encoding(&payload.identity_key.ok_or(
-                error::Error::NegotiationError(error::NegotiationError::PeerIdMissing),
-            )?)?;
+            let identity = payload.identity_key.ok_or(NegotiationError::PeerIdMissing)?;
+
+            let public_key = PublicKey::from_protobuf_encoding(&identity)?;
             Ok(PeerId::from_public_key(&public_key))
         }
-        Err(err) => Err(From::from(err)),
+        Err(err) => Err(ParseError::from(err).into()),
     }
 }
 
@@ -683,7 +676,7 @@ pub async fn handshake<S: AsyncRead + AsyncWrite + Unpin>(
     role: Role,
     max_read_ahead_factor: usize,
     max_write_buffer_size: usize,
-) -> crate::Result<(NoiseSocket<S>, PeerId)> {
+) -> Result<(NoiseSocket<S>, PeerId), NegotiationError> {
     tracing::debug!(target: LOG_TARGET, ?role, "start noise handshake");
 
     let mut noise = NoiseContext::new(keypair, role)?;
@@ -797,7 +790,7 @@ mod tests {
     #[test]
     fn invalid_peer_id_schema() {
         match parse_peer_id(&vec![1, 2, 3, 4]).unwrap_err() {
-            crate::Error::ParseError(_) => {}
+            NegotiationError::ParseError(_) => {}
             _ => panic!("invalid error"),
         }
     }