Migrate custom error trait impls to thiserror (#1856)

* Migrate to thiserror

* missing bits

* review comment

* Apply suggestions from code review

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* From<scale_decode::visitor::Error> to remove Into::intos

* scale crates for core::error::Error

* bump msrv 1.81

* make signer crate compile

---------

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Co-authored-by: James Wilson <james.wilson@parity.io>

signer/src/crypto/secret_uri.rs

@@ -2,13 +2,13 @@
 // This file is dual-licensed as Apache-2.0 or GPL-3.0.
 // see LICENSE for license details.
 
-use core::fmt::Display;
-
 use super::DeriveJunction;
 use alloc::{string::ToString, vec::Vec};
 use regex::Regex;
 use secrecy::SecretString;
 
+use thiserror::Error as DeriveError;
+
 // This code is taken from sp_core::crypto::DeriveJunction. The logic should be identical,
 // though the code is tweaked a touch!
 
@@ -116,23 +116,13 @@ impl core::str::FromStr for SecretUri {
 }
 
 /// This is returned if `FromStr` cannot parse a string into a `SecretUri`.
-#[derive(Debug, Copy, Clone, PartialEq)]
+#[derive(Debug, Copy, Clone, PartialEq, DeriveError)]
 pub enum SecretUriError {
     /// Parsing the secret URI from a string failed; wrong format.
+    #[error("Invalid secret phrase format")]
     InvalidFormat,
 }
 
-impl Display for SecretUriError {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self {
-            SecretUriError::InvalidFormat => write!(f, "Invalid secret phrase format"),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for SecretUriError {}
-
 once_static_cloned! {
     /// Interpret a phrase like:
     ///

signer/src/ecdsa.rs

@@ -7,11 +7,13 @@ use codec::Encode;
 use polkadot_sdk::sp_crypto_hashing;
 
 use crate::crypto::{seed_from_entropy, DeriveJunction, SecretUri};
-use core::{fmt::Display, str::FromStr};
+use core::str::FromStr;
 use hex::FromHex;
 use secp256k1::{ecdsa::RecoverableSignature, Message, Secp256k1, SecretKey};
 use secrecy::ExposeSecret;
 
+use thiserror::Error as DeriveError;
+
 const SECRET_KEY_LENGTH: usize = 32;
 
 /// Seed bytes used to generate a key pair.
@@ -222,33 +224,33 @@ pub(crate) mod internal {
 }
 
 /// An error handed back if creating a keypair fails.
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, DeriveError)]
 pub enum Error {
     /// Invalid seed.
+    #[error("Invalid seed (was it the wrong length?)")]
     InvalidSeed,
     /// Invalid seed.
+    #[error("Invalid seed for ECDSA, contained soft junction")]
     SoftJunction,
     /// Invalid phrase.
+    #[error("Cannot parse phrase: {0}")]
     Phrase(bip39::Error),
     /// Invalid hex.
+    #[error("Cannot parse hex string: {0}")]
     Hex(hex::FromHexError),
 }
-impl Display for Error {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self {
-            Error::InvalidSeed => write!(f, "Invalid seed (was it the wrong length?)"),
-            Error::SoftJunction => write!(f, "Invalid seed for ECDSA, contained soft junction"),
-            Error::Phrase(e) => write!(f, "Cannot parse phrase: {e}"),
-            Error::Hex(e) => write!(f, "Cannot parse hex string: {e}"),
-        }
+
+impl From<hex::FromHexError> for Error {
+    fn from(err: hex::FromHexError) -> Self {
+        Error::Hex(err)
     }
 }
 
-impl_from!(bip39::Error => Error::Phrase);
-impl_from!(hex::FromHexError => Error::Hex);
-
-#[cfg(feature = "std")]
-impl std::error::Error for Error {}
+impl From<bip39::Error> for Error {
+    fn from(err: bip39::Error) -> Self {
+        Error::Phrase(err)
+    }
+}
 
 /// Dev accounts, helpful for testing but not to be used in production,
 /// since the secret keys are known.

signer/src/eth.rs

@@ -6,11 +6,12 @@
 
 use crate::ecdsa;
 use alloc::format;
-use core::fmt::{Display, Formatter};
 use core::str::FromStr;
 use keccak_hash::keccak;
 use secp256k1::Message;
 
+use thiserror::Error as DeriveError;
+
 const SECRET_KEY_LENGTH: usize = 32;
 
 /// Bytes representing a private key.
@@ -201,29 +202,16 @@ pub fn verify<M: AsRef<[u8]>>(sig: &Signature, message: M, pubkey: &PublicKey) -
 }
 
 /// An error handed back if creating a keypair fails.
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, DeriveError)]
 pub enum Error {
     /// Invalid seed.
+    #[error("Invalid seed (was it the wrong length?)")]
     InvalidSeed,
     /// Invalid derivation path.
+    #[error("Could not derive from path; some values in the path may have been >= 2^31?")]
     DeriveFromPath,
 }
 
-impl Display for Error {
-    fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
-        match self {
-            Error::InvalidSeed => write!(f, "Invalid seed (was it the wrong length?)"),
-            Error::DeriveFromPath => write!(
-                f,
-                "Could not derive from path; some values in the path may have been >= 2^31?"
-            ),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for Error {}
-
 /// Dev accounts, helpful for testing but not to be used in production,
 /// since the secret keys are known.
 pub mod dev {

signer/src/polkadot_js_compat.rs

@@ -5,7 +5,6 @@
 //! A Polkadot-JS account loader.
 
 use base64::Engine;
-use core::fmt::Display;
 use crypto_secretbox::{
     aead::{Aead, KeyInit},
     Key, Nonce, XSalsa20Poly1305,
@@ -13,6 +12,8 @@ use crypto_secretbox::{
 use serde::Deserialize;
 use subxt_core::utils::AccountId32;
 
+use thiserror::Error as DeriveError;
+
 use crate::sr25519;
 
 /// Given a JSON keypair as exported from Polkadot-JS, this returns an [`sr25519::Keypair`]
@@ -22,15 +23,19 @@ pub fn decrypt_json(json: &str, password: &str) -> Result<sr25519::Keypair, Erro
 }
 
 /// Error
-#[derive(Debug)]
+#[derive(Debug, DeriveError)]
 pub enum Error {
     /// Error decoding JSON.
-    Json(serde_json::Error),
+    #[error("Invalid JSON: {0}")]
+    Json(#[from] serde_json::Error),
     /// The keypair has an unsupported encoding.
+    #[error("Unsupported encoding.")]
     UnsupportedEncoding,
     /// Base64 decoding error.
-    Base64(base64::DecodeError),
+    #[error("Base64 decoding error: {0}")]
+    Base64(#[from] base64::DecodeError),
     /// Wrong Scrypt parameters
+    #[error("Unsupported Scrypt parameters: N: {n}, p: {p}, r: {r}")]
     UnsupportedScryptParameters {
         /// N
         n: u32,
@@ -40,37 +45,16 @@ pub enum Error {
         r: u32,
     },
     /// Decryption error.
-    Secretbox(crypto_secretbox::Error),
+    #[error("Decryption error: {0}")]
+    Secretbox(#[from] crypto_secretbox::Error),
     /// sr25519 keypair error.
-    Sr25519(sr25519::Error),
+    #[error(transparent)]
+    Sr25519(#[from] sr25519::Error),
     /// The decrypted keys are not valid.
+    #[error("The decrypted keys are not valid.")]
     InvalidKeys,
 }
 
-impl_from!(serde_json::Error => Error::Json);
-impl_from!(base64::DecodeError => Error::Base64);
-impl_from!(crypto_secretbox::Error => Error::Secretbox);
-impl_from!(sr25519::Error => Error::Sr25519);
-
-impl Display for Error {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self {
-            Error::Json(e) => write!(f, "Invalid JSON: {e}"),
-            Error::UnsupportedEncoding => write!(f, "Unsupported encoding."),
-            Error::Base64(e) => write!(f, "Base64 decoding error: {e}"),
-            Error::UnsupportedScryptParameters { n, p, r } => {
-                write!(f, "Unsupported Scrypt parameters: N: {n}, p: {p}, r: {r}")
-            }
-            Error::Secretbox(e) => write!(f, "Decryption error: {e}"),
-            Error::Sr25519(e) => write!(f, "{e}"),
-            Error::InvalidKeys => write!(f, "The decrypted keys are not valid."),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for Error {}
-
 #[derive(Deserialize)]
 struct EncryptionMetadata {
     /// Descriptor for the content

signer/src/sr25519.rs

@@ -4,7 +4,7 @@
 
 //! An sr25519 keypair implementation.
 
-use core::{fmt::Display, str::FromStr};
+use core::str::FromStr;
 
 use crate::crypto::{seed_from_entropy, DeriveJunction, SecretUri};
 
@@ -15,6 +15,8 @@ use schnorrkel::{
 };
 use secrecy::ExposeSecret;
 
+use thiserror::Error as DeriveError;
+
 const SECRET_KEY_LENGTH: usize = schnorrkel::keys::MINI_SECRET_KEY_LENGTH;
 const SIGNING_CTX: &[u8] = b"substrate";
 
@@ -203,35 +205,39 @@ pub fn verify<M: AsRef<[u8]>>(sig: &Signature, message: M, pubkey: &PublicKey) -
 }
 
 /// An error handed back if creating a keypair fails.
-#[derive(Debug)]
+#[derive(Debug, DeriveError)]
 pub enum Error {
     /// Invalid seed.
+    #[error("Invalid seed (was it the wrong length?)")]
     InvalidSeed,
     /// Invalid phrase.
+    #[error("Cannot parse phrase: {0}")]
     Phrase(bip39::Error),
     /// Invalid hex.
+    #[error("Cannot parse hex string: {0}")]
     Hex(hex::FromHexError),
     /// Signature error.
+    #[error("Signature error: {0}")]
     Signature(schnorrkel::SignatureError),
 }
 
-impl_from!(bip39::Error => Error::Phrase);
-impl_from!(hex::FromHexError => Error::Hex);
-impl_from!(schnorrkel::SignatureError => Error::Signature);
-
-impl Display for Error {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self {
-            Error::InvalidSeed => write!(f, "Invalid seed (was it the wrong length?)"),
-            Error::Phrase(e) => write!(f, "Cannot parse phrase: {e}"),
-            Error::Hex(e) => write!(f, "Cannot parse hex string: {e}"),
-            Error::Signature(e) => write!(f, "Signature error: {e}"),
-        }
+impl From<schnorrkel::SignatureError> for Error {
+    fn from(value: schnorrkel::SignatureError) -> Self {
+        Error::Signature(value)
     }
 }
 
-#[cfg(feature = "std")]
-impl std::error::Error for Error {}
+impl From<hex::FromHexError> for Error {
+    fn from(err: hex::FromHexError) -> Self {
+        Error::Hex(err)
+    }
+}
+
+impl From<bip39::Error> for Error {
+    fn from(err: bip39::Error) -> Self {
+        Error::Phrase(err)
+    }
+}
 
 /// Dev accounts, helpful for testing but not to be used in production,
 /// since the secret keys are known.

signer/src/utils.rs

@@ -35,13 +35,3 @@ macro_rules! once_static_cloned {
         )+
     };
 }
-
-macro_rules! impl_from {
-    ($module_path:path => $delegate_ty:ident :: $variant:ident) => {
-        impl From<$module_path> for $delegate_ty {
-            fn from(val: $module_path) -> Self {
-                $delegate_ty::$variant(val.into())
-            }
-        }
-    };
-}