Improve SS58 related errors (#10541)

* Improve SS58 related errors

This improves the SS58 error, especially when it comes to parsing public keys with unknown SS58
address formats.

* Make CI happy

* More fixes

* More

* 🤦

* fml...

substrate/primitives/core/src/crypto.rs

@@ -198,25 +198,45 @@ impl<T: AsRef<str>> From<T> for DeriveJunction {
 }
 
 /// An error type for SS58 decoding.
+#[cfg_attr(feature = "std", derive(thiserror::Error))]
+#[cfg_attr(not(feature = "std"), derive(Debug))]
+#[derive(Clone, Copy, Eq, PartialEq)]
+#[allow(missing_docs)]
 #[cfg(feature = "full_crypto")]
-#[derive(Clone, Copy, Eq, PartialEq, Debug)]
 pub enum PublicError {
-	/// Bad alphabet.
+	#[cfg_attr(feature = "std", error("Base 58 requirement is violated"))]
 	BadBase58,
-	/// Bad length.
+	#[cfg_attr(feature = "std", error("Length is bad"))]
 	BadLength,
-	/// Unknown identifier for the encoding.
-	UnknownVersion,
-	/// Invalid checksum.
+	#[cfg_attr(
+		feature = "std",
+		error(
+			"Unknown SS58 address format `{}`. ` \
+		`To support this address format, you need to call `set_default_ss58_version` at node start up.",
+			_0
+		)
+	)]
+	UnknownSs58AddressFormat(Ss58AddressFormat),
+	#[cfg_attr(feature = "std", error("Invalid checksum"))]
 	InvalidChecksum,
-	/// Invalid format.
+	#[cfg_attr(feature = "std", error("Invalid SS58 prefix byte."))]
+	InvalidPrefix,
+	#[cfg_attr(feature = "std", error("Invalid SS58 format."))]
 	InvalidFormat,
-	/// Invalid derivation path.
+	#[cfg_attr(feature = "std", error("Invalid derivation path."))]
 	InvalidPath,
-	/// Disallowed SS58 Address Format for this datatype.
+	#[cfg_attr(feature = "std", error("Disallowed SS58 Address Format for this datatype."))]
 	FormatNotAllowed,
 }
 
+#[cfg(feature = "std")]
+impl sp_std::fmt::Debug for PublicError {
+	fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result {
+		// Just use the `Display` implementation
+		write!(f, "{}", self)
+	}
+}
+
 /// Key that can be encoded to/from SS58.
 ///
 /// See <https://docs.substrate.io/v3/advanced/ss58/>
@@ -235,7 +255,7 @@ pub trait Ss58Codec: Sized + AsMut<[u8]> + AsRef<[u8]> + ByteArray {
 		Self::from_ss58check_with_version(s).and_then(|(r, v)| match v {
 			v if !v.is_custom() => Ok(r),
 			v if v == default_ss58_version() => Ok(r),
-			_ => Err(PublicError::UnknownVersion),
+			v => Err(PublicError::UnknownSs58AddressFormat(v)),
 		})
 	}
 
@@ -261,7 +281,7 @@ pub trait Ss58Codec: Sized + AsMut<[u8]> + AsRef<[u8]> + ByteArray {
 				let upper = data[1] & 0b00111111;
 				(2, (lower as u16) | ((upper as u16) << 8))
 			},
-			_ => return Err(PublicError::UnknownVersion),
+			_ => return Err(PublicError::InvalidPrefix),
 		};
 		if data.len() != prefix_len + body_len + CHECKSUM_LEN {
 			return Err(PublicError::BadLength)
@@ -290,7 +310,7 @@ pub trait Ss58Codec: Sized + AsMut<[u8]> + AsRef<[u8]> + ByteArray {
 		Self::from_string_with_version(s).and_then(|(r, v)| match v {
 			v if !v.is_custom() => Ok(r),
 			v if v == default_ss58_version() => Ok(r),
-			_ => Err(PublicError::UnknownVersion),
+			v => Err(PublicError::UnknownSs58AddressFormat(v)),
 		})
 	}
 
@@ -359,7 +379,7 @@ static DEFAULT_VERSION: core::sync::atomic::AtomicU16 = std::sync::atomic::Atomi
 	from_known_address_format(Ss58AddressFormatRegistry::SubstrateAccount),
 );
 
-/// Returns default ss58 format used by the current active process.
+/// Returns default SS58 format used by the current active process.
 #[cfg(feature = "std")]
 pub fn default_ss58_version() -> Ss58AddressFormat {
 	DEFAULT_VERSION.load(std::sync::atomic::Ordering::Relaxed).into()
@@ -371,9 +391,15 @@ pub fn unwrap_or_default_ss58_version(network: Option<Ss58AddressFormat>) -> Ss5
 	network.unwrap_or_else(default_ss58_version)
 }
 
-/// Set the default "version" (actually, this is a bit of a misnomer and the version byte is
-/// typically used not just to encode format/version but also network identity) that is used for
-/// encoding and decoding SS58 addresses.
+/// Set the default SS58 "version".
+///
+/// This SS58 version/format will be used when encoding/decoding SS58 addresses.
+///
+/// If you want to support a custom SS58 prefix (that isn't yet registered in the `ss58-registry`),
+/// you are required to call this function with your desired prefix [`Ss58AddressFormat::custom`].
+/// This will enable the node to decode ss58 addresses with this prefix.
+///
+/// This SS58 version/format is also only used by the node and not by the runtime.
 #[cfg(feature = "std")]
 pub fn set_default_ss58_version(new_default: Ss58AddressFormat) {
 	DEFAULT_VERSION.store(new_default.into(), std::sync::atomic::Ordering::Relaxed);

substrate/primitives/core/src/ecdsa.rs

@@ -22,7 +22,6 @@
 use codec::{Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
 use sp_runtime_interface::pass_by::PassByInner;
-use sp_std::cmp::Ordering;
 
 #[cfg(feature = "std")]
 use crate::crypto::Ss58Codec;
@@ -56,43 +55,12 @@ pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecds");
 type Seed = [u8; 32];
 
 /// The ECDSA compressed public key.
-#[derive(Clone, Encode, Decode, PassByInner, MaxEncodedLen, TypeInfo)]
+#[cfg_attr(feature = "full_crypto", derive(Hash))]
+#[derive(
+	Clone, Encode, Decode, PassByInner, MaxEncodedLen, TypeInfo, Eq, PartialEq, PartialOrd, Ord,
+)]
 pub struct Public(pub [u8; 33]);
 
-impl PartialOrd for Public {
-	fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-		Some(self.cmp(other))
-	}
-}
-
-impl Ord for Public {
-	fn cmp(&self, other: &Self) -> Ordering {
-		self.as_ref().cmp(&other.as_ref())
-	}
-}
-
-impl PartialEq for Public {
-	fn eq(&self, other: &Self) -> bool {
-		self.as_ref() == other.as_ref()
-	}
-}
-
-impl Eq for Public {}
-
-/// An error type for SS58 decoding.
-#[cfg(feature = "std")]
-#[derive(Clone, Copy, Eq, PartialEq, Debug)]
-pub enum PublicError {
-	/// Bad alphabet.
-	BadBase58,
-	/// Bad length.
-	BadLength,
-	/// Unknown version.
-	UnknownVersion,
-	/// Invalid checksum.
-	InvalidChecksum,
-}
-
 impl Public {
 	/// A new instance from the given 33-byte `data`.
 	///
@@ -217,15 +185,9 @@ impl<'de> Deserialize<'de> for Public {
 	}
 }
 
-#[cfg(feature = "full_crypto")]
-impl sp_std::hash::Hash for Public {
-	fn hash<H: sp_std::hash::Hasher>(&self, state: &mut H) {
-		self.as_ref().hash(state);
-	}
-}
-
 /// A signature (a 512-bit value, plus 8 bits for recovery ID).
-#[derive(Encode, Decode, PassByInner, TypeInfo)]
+#[cfg_attr(feature = "full_crypto", derive(Hash))]
+#[derive(Encode, Decode, PassByInner, TypeInfo, PartialEq, Eq)]
 pub struct Signature(pub [u8; 65]);
 
 impl sp_std::convert::TryFrom<&[u8]> for Signature {
@@ -279,14 +241,6 @@ impl Default for Signature {
 	}
 }
 
-impl PartialEq for Signature {
-	fn eq(&self, b: &Self) -> bool {
-		self.0[..] == b.0[..]
-	}
-}
-
-impl Eq for Signature {}
-
 impl From<Signature> for [u8; 65] {
 	fn from(v: Signature) -> [u8; 65] {
 		v.0
@@ -323,13 +277,6 @@ impl sp_std::fmt::Debug for Signature {
 	}
 }
 
-#[cfg(feature = "full_crypto")]
-impl sp_std::hash::Hash for Signature {
-	fn hash<H: sp_std::hash::Hasher>(&self, state: &mut H) {
-		sp_std::hash::Hash::hash(&self.0[..], state);
-	}
-}
-
 impl UncheckedFrom<[u8; 65]> for Signature {
 	fn unchecked_from(data: [u8; 65]) -> Signature {
 		Signature(data)

substrate/primitives/core/src/ed25519.rs

@@ -211,7 +211,8 @@ impl<'de> Deserialize<'de> for Public {
 }
 
 /// A signature (a 512-bit value).
-#[derive(Encode, Decode, PassByInner, TypeInfo)]
+#[cfg_attr(feature = "full_crypto", derive(Hash))]
+#[derive(Encode, Decode, PassByInner, TypeInfo, PartialEq, Eq)]
 pub struct Signature(pub [u8; 64]);
 
 impl sp_std::convert::TryFrom<&[u8]> for Signature {
@@ -259,14 +260,6 @@ impl Clone for Signature {
 	}
 }
 
-impl PartialEq for Signature {
-	fn eq(&self, b: &Self) -> bool {
-		self.0[..] == b.0[..]
-	}
-}
-
-impl Eq for Signature {}
-
 impl From<Signature> for H512 {
 	fn from(v: Signature) -> H512 {
 		H512::from(v.0)
@@ -309,13 +302,6 @@ impl sp_std::fmt::Debug for Signature {
 	}
 }
 
-#[cfg(feature = "full_crypto")]
-impl sp_std::hash::Hash for Signature {
-	fn hash<H: sp_std::hash::Hasher>(&self, state: &mut H) {
-		sp_std::hash::Hash::hash(&self.0[..], state);
-	}
-}
-
 impl UncheckedFrom<[u8; 64]> for Signature {
 	fn unchecked_from(data: [u8; 64]) -> Signature {
 		Signature(data)
@@ -360,24 +346,6 @@ pub struct LocalizedSignature {
 	pub signature: Signature,
 }
 
-/// An error type for SS58 decoding.
-#[cfg(feature = "std")]
-#[derive(Clone, Copy, Eq, PartialEq, Debug, thiserror::Error)]
-pub enum PublicError {
-	/// Bad alphabet.
-	#[error("Base 58 requirement is violated")]
-	BadBase58,
-	/// Bad length.
-	#[error("Length is bad")]
-	BadLength,
-	/// Unknown version.
-	#[error("Unknown version")]
-	UnknownVersion,
-	/// Invalid checksum.
-	#[error("Invalid checksum")]
-	InvalidChecksum,
-}
-
 impl Public {
 	/// A new instance from the given 32-byte `data`.
 	///

substrate/primitives/core/src/sr25519.rs

@@ -211,7 +211,8 @@ impl<'de> Deserialize<'de> for Public {
 /// An Schnorrkel/Ristretto x25519 ("sr25519") signature.
 ///
 /// Instead of importing it for the local module, alias it to be available as a public type
-#[derive(Encode, Decode, PassByInner, TypeInfo)]
+#[cfg_attr(feature = "full_crypto", derive(Hash))]
+#[derive(Encode, Decode, PassByInner, TypeInfo, PartialEq, Eq)]
 pub struct Signature(pub [u8; 64]);
 
 impl sp_std::convert::TryFrom<&[u8]> for Signature {
@@ -259,14 +260,6 @@ impl Clone for Signature {
 	}
 }
 
-impl PartialEq for Signature {
-	fn eq(&self, b: &Self) -> bool {
-		self.0[..] == b.0[..]
-	}
-}
-
-impl Eq for Signature {}
-
 impl From<Signature> for [u8; 64] {
 	fn from(v: Signature) -> [u8; 64] {
 		v.0
@@ -316,13 +309,6 @@ impl sp_std::fmt::Debug for Signature {
 	}
 }
 
-#[cfg(feature = "full_crypto")]
-impl sp_std::hash::Hash for Signature {
-	fn hash<H: sp_std::hash::Hasher>(&self, state: &mut H) {
-		sp_std::hash::Hash::hash(&self.0[..], state);
-	}
-}
-
 /// A localized signature also contains sender information.
 /// NOTE: Encode and Decode traits are supported in ed25519 but not possible for now here.
 #[cfg(feature = "std")]