Crypto Pair trait refactory (#13657)

* Crypto pair refactory

* Remove unused method

* Apply review suggestions

* Remove leftovers

* Associated type is not really required

* Fix after refactory

* Fix benchmark-ui test

---------

Co-authored-by: Anton <anton.kalyaev@gmail.com>

frame/support/test/tests/benchmark_ui/bad_return_type_blank_with_question.stderr

@@ -7,4 +7,4 @@ error[E0277]: the `?` operator can only be used in a function that returns `Resu
 15 |         something()?;
    |                    ^ cannot use the `?` operator in a function that returns `()`
    |
-   = help: the trait `FromResidual<Result<std::convert::Infallible, frame_benchmarking::BenchmarkError>>` is not implemented for `()`
+   = help: the trait `FromResidual<Result<Infallible, frame_benchmarking::BenchmarkError>>` is not implemented for `()`

primitives/application-crypto/src/lib.rs

@@ -23,7 +23,7 @@
 pub use sp_core::crypto::{key_types, CryptoTypeId, KeyTypeId};
 #[doc(hidden)]
 #[cfg(feature = "full_crypto")]
-pub use sp_core::crypto::{DeriveJunction, Pair, SecretStringError, Ss58Codec};
+pub use sp_core::crypto::{DeriveError, DeriveJunction, Pair, SecretStringError, Ss58Codec};
 #[doc(hidden)]
 pub use sp_core::{
 	self,
@@ -129,15 +129,14 @@ macro_rules! app_crypto_pair {
 			type Public = Public;
 			type Seed = <$pair as $crate::Pair>::Seed;
 			type Signature = Signature;
-			type DeriveError = <$pair as $crate::Pair>::DeriveError;
 
 			$crate::app_crypto_pair_functions_if_std!($pair);
 
 			fn derive<Iter: Iterator<Item = $crate::DeriveJunction>>(
 				&self,
 				path: Iter,
 				seed: Option<Self::Seed>,
-			) -> Result<(Self, Option<Self::Seed>), Self::DeriveError> {
+			) -> Result<(Self, Option<Self::Seed>), $crate::DeriveError> {
 				self.0.derive(path, seed).map(|x| (Self(x.0), x.1))
 			}
 			fn from_seed(seed: &Self::Seed) -> Self {

primitives/core/src/crypto.rs

@@ -24,6 +24,8 @@ use crate::hexdisplay::HexDisplay;
 use crate::{ed25519, sr25519};
 #[cfg(feature = "std")]
 use base58::{FromBase58, ToBase58};
+#[cfg(feature = "std")]
+use bip39::{Language, Mnemonic, MnemonicType};
 use codec::{Decode, Encode, MaxEncodedLen};
 #[cfg(feature = "std")]
 use rand::{rngs::OsRng, RngCore};
@@ -52,10 +54,6 @@ pub const DEV_PHRASE: &str =
 /// The address of the associated root phrase for our publicly known keys.
 pub const DEV_ADDRESS: &str = "5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV";
 
-/// The infallible type.
-#[derive(crate::RuntimeDebug)]
-pub enum Infallible {}
-
 /// The length of the junction identifier. Note that this is also referred to as the
 /// `CHAIN_CODE_LENGTH` in the context of Schnorrkel.
 #[cfg(feature = "full_crypto")]
@@ -108,6 +106,16 @@ pub enum SecretStringError {
 	InvalidPath,
 }
 
+/// An error when deriving a key.
+#[cfg_attr(feature = "std", derive(thiserror::Error))]
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg(feature = "full_crypto")]
+pub enum DeriveError {
+	/// A soft key was found in the path (and is unsupported).
+	#[cfg_attr(feature = "std", error("Soft key in path"))]
+	SoftKeyInPath,
+}
+
 /// A since derivation junction description. It is the single parameter used when creating
 /// a new secret key from an existing secret key and, in the case of `SoftRaw` and `SoftIndex`
 /// a new public key from an existing public key.
@@ -691,40 +699,45 @@ mod dummy {
 		type Public = Dummy;
 		type Seed = Dummy;
 		type Signature = Dummy;
-		type DeriveError = ();
+
 		#[cfg(feature = "std")]
 		fn generate_with_phrase(_: Option<&str>) -> (Self, String, Self::Seed) {
 			Default::default()
 		}
+
 		#[cfg(feature = "std")]
 		fn from_phrase(_: &str, _: Option<&str>) -> Result<(Self, Self::Seed), SecretStringError> {
 			Ok(Default::default())
 		}
+
 		fn derive<Iter: Iterator<Item = DeriveJunction>>(
 			&self,
 			_: Iter,
 			_: Option<Dummy>,
-		) -> Result<(Self, Option<Dummy>), Self::DeriveError> {
+		) -> Result<(Self, Option<Dummy>), DeriveError> {
 			Ok((Self, None))
 		}
-		fn from_seed(_: &Self::Seed) -> Self {
-			Self
-		}
+
 		fn from_seed_slice(_: &[u8]) -> Result<Self, SecretStringError> {
 			Ok(Self)
 		}
+
 		fn sign(&self, _: &[u8]) -> Self::Signature {
 			Self
 		}
+
 		fn verify<M: AsRef<[u8]>>(_: &Self::Signature, _: M, _: &Self::Public) -> bool {
 			true
 		}
+
 		fn verify_weak<P: AsRef<[u8]>, M: AsRef<[u8]>>(_: &[u8], _: M, _: P) -> bool {
 			true
 		}
+
 		fn public(&self) -> Self::Public {
 			Self
 		}
+
 		fn to_raw_vec(&self) -> Vec<u8> {
 			vec![]
 		}
@@ -845,9 +858,6 @@ pub trait Pair: CryptoType + Sized + Clone + Send + Sync + 'static {
 	/// and verified with the message and a public key.
 	type Signature: AsRef<[u8]>;
 
-	/// Error returned from the `derive` function.
-	type DeriveError;
-
 	/// Generate new secure (random) key pair.
 	///
 	/// This is only for ephemeral keys really, since you won't have access to the secret key
@@ -866,27 +876,47 @@ pub trait Pair: CryptoType + Sized + Clone + Send + Sync + 'static {
 	/// This is generally slower than `generate()`, so prefer that unless you need to persist
 	/// the key from the current session.
 	#[cfg(feature = "std")]
-	fn generate_with_phrase(password: Option<&str>) -> (Self, String, Self::Seed);
+	fn generate_with_phrase(password: Option<&str>) -> (Self, String, Self::Seed) {
+		let mnemonic = Mnemonic::new(MnemonicType::Words12, Language::English);
+		let phrase = mnemonic.phrase();
+		let (pair, seed) = Self::from_phrase(phrase, password)
+			.expect("All phrases generated by Mnemonic are valid; qed");
+		(pair, phrase.to_owned(), seed)
+	}
 
 	/// Returns the KeyPair from the English BIP39 seed `phrase`, or `None` if it's invalid.
 	#[cfg(feature = "std")]
 	fn from_phrase(
 		phrase: &str,
 		password: Option<&str>,
-	) -> Result<(Self, Self::Seed), SecretStringError>;
+	) -> Result<(Self, Self::Seed), SecretStringError> {
+		let mnemonic = Mnemonic::from_phrase(phrase, Language::English)
+			.map_err(|_| SecretStringError::InvalidPhrase)?;
+		let big_seed =
+			substrate_bip39::seed_from_entropy(mnemonic.entropy(), password.unwrap_or(""))
+				.map_err(|_| SecretStringError::InvalidSeed)?;
+		let mut seed = Self::Seed::default();
+		let seed_slice = seed.as_mut();
+		let seed_len = seed_slice.len();
+		debug_assert!(seed_len <= big_seed.len());
+		seed_slice[..seed_len].copy_from_slice(&big_seed[..seed_len]);
+		Self::from_seed_slice(seed_slice).map(|x| (x, seed))
+	}
 
 	/// Derive a child key from a series of given junctions.
 	fn derive<Iter: Iterator<Item = DeriveJunction>>(
 		&self,
 		path: Iter,
 		seed: Option<Self::Seed>,
-	) -> Result<(Self, Option<Self::Seed>), Self::DeriveError>;
+	) -> Result<(Self, Option<Self::Seed>), DeriveError>;
 
 	/// Generate new key pair from the provided `seed`.
 	///
 	/// @WARNING: THIS WILL ONLY BE SECURE IF THE `seed` IS SECURE. If it can be guessed
 	/// by an attacker then they can also derive your key.
-	fn from_seed(seed: &Self::Seed) -> Self;
+	fn from_seed(seed: &Self::Seed) -> Self {
+		Self::from_seed_slice(seed.as_ref()).expect("seed has valid length; qed")
+	}
 
 	/// Make a new key pair from secret seed material. The slice must be the correct size or
 	/// it will return `None`.
@@ -1202,14 +1232,15 @@ mod tests {
 		type Public = TestPublic;
 		type Seed = [u8; 8];
 		type Signature = [u8; 0];
-		type DeriveError = ();
 
 		fn generate() -> (Self, <Self as Pair>::Seed) {
 			(TestPair::Generated, [0u8; 8])
 		}
+
 		fn generate_with_phrase(_password: Option<&str>) -> (Self, String, <Self as Pair>::Seed) {
 			(TestPair::GeneratedWithPhrase, "".into(), [0u8; 8])
 		}
+
 		fn from_phrase(
 			phrase: &str,
 			password: Option<&str>,
@@ -1222,11 +1253,12 @@ mod tests {
 				[0u8; 8],
 			))
 		}
+
 		fn derive<Iter: Iterator<Item = DeriveJunction>>(
 			&self,
 			path_iter: Iter,
 			_: Option<[u8; 8]>,
-		) -> Result<(Self, Option<[u8; 8]>), Self::DeriveError> {
+		) -> Result<(Self, Option<[u8; 8]>), DeriveError> {
 			Ok((
 				match self.clone() {
 					TestPair::Standard { phrase, password, path } => TestPair::Standard {
@@ -1240,34 +1272,37 @@ mod tests {
 						if path_iter.count() == 0 {
 							x
 						} else {
-							return Err(())
+							return Err(DeriveError::SoftKeyInPath)
 						},
 				},
 				None,
 			))
 		}
-		fn from_seed(_seed: &<TestPair as Pair>::Seed) -> Self {
-			TestPair::Seed(_seed.as_ref().to_owned())
-		}
+
 		fn sign(&self, _message: &[u8]) -> Self::Signature {
 			[]
 		}
+
 		fn verify<M: AsRef<[u8]>>(_: &Self::Signature, _: M, _: &Self::Public) -> bool {
 			true
 		}
+
 		fn verify_weak<P: AsRef<[u8]>, M: AsRef<[u8]>>(
 			_sig: &[u8],
 			_message: M,
 			_pubkey: P,
 		) -> bool {
 			true
 		}
+
 		fn public(&self) -> Self::Public {
 			TestPublic
 		}
+
 		fn from_seed_slice(seed: &[u8]) -> Result<Self, SecretStringError> {
 			Ok(TestPair::Seed(seed.to_owned()))
 		}
+
 		fn to_raw_vec(&self) -> Vec<u8> {
 			vec![]
 		}

primitives/core/src/ecdsa.rs

@@ -29,11 +29,9 @@ use crate::crypto::{
 };
 #[cfg(feature = "full_crypto")]
 use crate::{
-	crypto::{DeriveJunction, Pair as TraitPair, SecretStringError},
+	crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError},
 	hashing::blake2_256,
 };
-#[cfg(feature = "std")]
-use bip39::{Language, Mnemonic, MnemonicType};
 #[cfg(all(feature = "full_crypto", not(feature = "std")))]
 use secp256k1::Secp256k1;
 #[cfg(feature = "std")]
@@ -367,13 +365,6 @@ fn derive_hard_junction(secret_seed: &Seed, cc: &[u8; 32]) -> Seed {
 	("Secp256k1HDKD", secret_seed, cc).using_encoded(sp_core_hashing::blake2_256)
 }
 
-/// An error when deriving a key.
-#[cfg(feature = "full_crypto")]
-pub enum DeriveError {
-	/// A soft key was found in the path (and is unsupported).
-	SoftKeyInPath,
-}
-
 /// A key pair.
 #[cfg(feature = "full_crypto")]
 #[derive(Clone)]
@@ -387,44 +378,6 @@ impl TraitPair for Pair {
 	type Public = Public;
 	type Seed = Seed;
 	type Signature = Signature;
-	type DeriveError = DeriveError;
-
-	/// Generate new secure (random) key pair and provide the recovery phrase.
-	///
-	/// You can recover the same key later with `from_phrase`.
-	#[cfg(feature = "std")]
-	fn generate_with_phrase(password: Option<&str>) -> (Pair, String, Seed) {
-		let mnemonic = Mnemonic::new(MnemonicType::Words12, Language::English);
-		let phrase = mnemonic.phrase();
-		let (pair, seed) = Self::from_phrase(phrase, password)
-			.expect("All phrases generated by Mnemonic are valid; qed");
-		(pair, phrase.to_owned(), seed)
-	}
-
-	/// Generate key pair from given recovery phrase and password.
-	#[cfg(feature = "std")]
-	fn from_phrase(
-		phrase: &str,
-		password: Option<&str>,
-	) -> Result<(Pair, Seed), SecretStringError> {
-		let big_seed = substrate_bip39::seed_from_entropy(
-			Mnemonic::from_phrase(phrase, Language::English)
-				.map_err(|_| SecretStringError::InvalidPhrase)?
-				.entropy(),
-			password.unwrap_or(""),
-		)
-		.map_err(|_| SecretStringError::InvalidSeed)?;
-		let mut seed = Seed::default();
-		seed.copy_from_slice(&big_seed[0..32]);
-		Self::from_seed_slice(&big_seed[0..32]).map(|x| (x, seed))
-	}
-
-	/// Make a new key pair from secret seed material.
-	///
-	/// You should never need to use this; generate(), generate_with_phrase
-	fn from_seed(seed: &Seed) -> Pair {
-		Self::from_seed_slice(&seed[..]).expect("seed has valid length; qed")
-	}
 
 	/// Make a new key pair from secret seed material. The slice must be 32 bytes long or it
 	/// will return `None`.