From eadce66546510133676db3378f212ede2e7c566c Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Wed, 22 Mar 2023 18:35:38 +0100
Subject: [PATCH 01/15] Introduce keystore specialized sign methods

---
 client/authority-discovery/src/error.rs  |   4 +-
 client/authority-discovery/src/worker.rs |  19 +--
 client/consensus/aura/src/lib.rs         |  20 +--
 client/consensus/babe/src/lib.rs         |  40 +++---
 client/consensus/slots/src/lib.rs        |   2 +-
 client/keystore/src/local.rs             | 164 +++++++++++------------
 primitives/consensus/grandpa/src/lib.rs  |   4 +-
 primitives/io/src/lib.rs                 |   9 +-
 primitives/keystore/src/lib.rs           | 139 +++++++++++++------
 primitives/keystore/src/testing.rs       | 137 +++++++++----------
 10 files changed, 287 insertions(+), 251 deletions(-)

diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs
index 89c05b71b9ea6..8e3aac5a606c5 100644
--- a/client/authority-discovery/src/error.rs
+++ b/client/authority-discovery/src/error.rs
@@ -18,8 +18,6 @@
 
 //! Authority discovery errors.
 
-use sp_core::crypto::CryptoTypePublicPair;
-
 /// AuthorityDiscovery Result.
 pub type Result<T> = std::result::Result<T, Error>;
 
@@ -60,7 +58,7 @@ pub enum Error {
 	ParsingLibp2pIdentity(#[from] libp2p::identity::error::DecodingError),
 
 	#[error("Failed to sign using a specific public key.")]
-	MissingSignature(CryptoTypePublicPair),
+	MissingSignature(Vec<u8>),
 
 	#[error("Failed to sign using all public keys.")]
 	Signing,
diff --git a/client/authority-discovery/src/worker.rs b/client/authority-discovery/src/worker.rs
index 5d7d21a5b6de4..d0c2b846e7522 100644
--- a/client/authority-discovery/src/worker.rs
+++ b/client/authority-discovery/src/worker.rs
@@ -32,7 +32,7 @@ use std::{
 use futures::{channel::mpsc, future, stream::Fuse, FutureExt, Stream, StreamExt};
 
 use addr_cache::AddrCache;
-use codec::Decode;
+use codec::{Decode, Encode};
 use ip_network::IpNetwork;
 use libp2p::{
 	core::multiaddr,
@@ -52,7 +52,7 @@ use sp_authority_discovery::{
 };
 use sp_blockchain::HeaderBackend;
 
-use sp_core::crypto::{key_types, CryptoTypePublicPair, Pair};
+use sp_core::crypto::{key_types, ByteArray, Pair, Wraps};
 use sp_keystore::{Keystore, KeystorePtr};
 use sp_runtime::traits::Block as BlockT;
 
@@ -127,7 +127,7 @@ pub struct Worker<Client, Network, Block, DhtEventStream> {
 	publish_if_changed_interval: ExpIncInterval,
 	/// List of keys onto which addresses have been published at the latest publication.
 	/// Used to check whether they have changed.
-	latest_published_keys: HashSet<CryptoTypePublicPair>,
+	latest_published_keys: HashSet<AuthorityId>,
 	/// Same value as in the configuration.
 	publish_non_global_ips: bool,
 	/// Same value as in the configuration.
@@ -339,7 +339,7 @@ where
 		let keys = Worker::<Client, Network, Block, DhtEventStream>::get_own_public_keys_within_authority_set(
 			key_store.clone(),
 			self.client.as_ref(),
-		).await?.into_iter().map(Into::into).collect::<HashSet<_>>();
+		).await?.into_iter().collect::<HashSet<_>>();
 
 		if only_if_changed && keys == self.latest_published_keys {
 			return Ok(())
@@ -664,15 +664,18 @@ fn sign_record_with_authority_ids(
 	serialized_record: Vec<u8>,
 	peer_signature: Option<schema::PeerSignature>,
 	key_store: &dyn Keystore,
-	keys: Vec<CryptoTypePublicPair>,
+	keys: Vec<AuthorityId>,
 ) -> Result<Vec<(KademliaKey, Vec<u8>)>> {
 	let mut result = Vec::with_capacity(keys.len());
 
 	for key in keys.iter() {
 		let auth_signature = key_store
-			.sign_with(key_types::AUTHORITY_DISCOVERY, key, &serialized_record)
+			.sr25519_sign(key_types::AUTHORITY_DISCOVERY, key.as_inner_ref(), &serialized_record)
 			.map_err(|_| Error::Signing)?
-			.ok_or_else(|| Error::MissingSignature(key.clone()))?;
+			.ok_or_else(|| Error::MissingSignature(key.to_raw_vec()))?;
+
+		// Scale encode
+		let auth_signature = auth_signature.encode();
 
 		let signed_record = schema::SignedAuthorityRecord {
 			record: serialized_record.clone(),
@@ -681,7 +684,7 @@ fn sign_record_with_authority_ids(
 		}
 		.encode_to_vec();
 
-		result.push((hash_authority_id(&key.1), signed_record));
+		result.push((hash_authority_id(key.as_slice()), signed_record));
 	}
 
 	Ok(result)
diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs
index d06a6016a2297..b852316944b4a 100644
--- a/client/consensus/aura/src/lib.rs
+++ b/client/consensus/aura/src/lib.rs
@@ -406,17 +406,17 @@ where
 		)
 	}
 
-	fn authorities_len(&self, epoch_data: &Self::AuxData) -> Option<usize> {
-		Some(epoch_data.len())
+	fn authorities_len(&self, authorities: &Self::AuxData) -> Option<usize> {
+		Some(authorities.len())
 	}
 
 	async fn claim_slot(
 		&self,
 		_header: &B::Header,
 		slot: Slot,
-		epoch_data: &Self::AuxData,
+		authorities: &Self::AuxData,
 	) -> Option<Self::Claim> {
-		let expected_author = slot_author::<P>(slot, epoch_data);
+		let expected_author = slot_author::<P>(slot, authorities);
 		expected_author.and_then(|p| {
 			if self
 				.keystore
@@ -440,18 +440,20 @@ where
 		body: Vec<B::Extrinsic>,
 		storage_changes: StorageChanges<<Self::BlockImport as BlockImport<B>>::Transaction, B>,
 		public: Self::Claim,
-		_epoch: Self::AuxData,
+		_authorities: Self::AuxData,
 	) -> Result<
 		sc_consensus::BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
 		sp_consensus::Error,
 	> {
-		// sign the pre-sealed hash of the block and then
-		// add it to a digest item.
-		let public_type_pair = public.to_public_crypto_pair();
 		let public = public.to_raw_vec();
 		let signature = self
 			.keystore
-			.sign_with(<AuthorityId<P> as AppKey>::ID, &public_type_pair, header_hash.as_ref())
+			.sign_with(
+				<AuthorityId<P> as AppKey>::ID,
+				<AuthorityId<P> as AppKey>::CRYPTO_ID,
+				&public,
+				header_hash.as_ref(),
+			)
 			.map_err(|e| sp_consensus::Error::CannotSign(public.clone(), e.to_string()))?
 			.ok_or_else(|| {
 				sp_consensus::Error::CannotSign(
diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs
index e2c2793b11c9d..2bf803b804349 100644
--- a/client/consensus/babe/src/lib.rs
+++ b/client/consensus/babe/src/lib.rs
@@ -117,7 +117,10 @@ use sp_blockchain::{
 use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain};
 use sp_consensus_babe::inherents::BabeInherentData;
 use sp_consensus_slots::Slot;
-use sp_core::{crypto::ByteArray, ExecutionContext};
+use sp_core::{
+	crypto::{ByteArray, Wraps},
+	ExecutionContext,
+};
 use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider};
 use sp_keystore::KeystorePtr;
 use sp_runtime::{
@@ -274,7 +277,7 @@ pub enum Error<B: BlockT> {
 	MultipleConfigChangeDigests,
 	/// Could not extract timestamp and slot
 	#[error("Could not extract timestamp and slot: {0}")]
-	Extraction(sp_consensus::Error),
+	Extraction(ConsensusError),
 	/// Could not fetch epoch
 	#[error("Could not fetch epoch at {0:?}")]
 	FetchEpoch(B::Hash),
@@ -471,7 +474,7 @@ pub fn start_babe<B, C, SC, E, I, SO, CIDP, BS, L, Error>(
 		max_block_proposal_slot_portion,
 		telemetry,
 	}: BabeParams<B, C, SC, E, I, SO, L, CIDP, BS>,
-) -> Result<BabeWorker<B>, sp_consensus::Error>
+) -> Result<BabeWorker<B>, ConsensusError>
 where
 	B: BlockT,
 	C: ProvideRuntimeApi<B>
@@ -739,7 +742,7 @@ where
 	type SyncOracle = SO;
 	type JustificationSyncLink = L;
 	type CreateProposer =
-		Pin<Box<dyn Future<Output = Result<E::Proposer, sp_consensus::Error>> + Send + 'static>>;
+		Pin<Box<dyn Future<Output = Result<E::Proposer, ConsensusError>> + Send + 'static>>;
 	type Proposer = E::Proposer;
 	type BlockImport = I;
 	type AuxData = ViableEpochDescriptor<B::Hash, NumberFor<B>, Epoch>;
@@ -762,7 +765,7 @@ where
 				slot,
 			)
 			.map_err(|e| ConsensusError::ChainLookup(e.to_string()))?
-			.ok_or(sp_consensus::Error::InvalidAuthoritiesSet)
+			.ok_or(ConsensusError::InvalidAuthoritiesSet)
 	}
 
 	fn authorities_len(&self, epoch_descriptor: &Self::AuxData) -> Option<usize> {
@@ -827,28 +830,21 @@ where
 		(_, public): Self::Claim,
 		epoch_descriptor: Self::AuxData,
 	) -> Result<
-		sc_consensus::BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
-		sp_consensus::Error,
+		BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
+		ConsensusError,
 	> {
-		// sign the pre-sealed hash of the block and then
-		// add it to a digest item.
-		let public_type_pair = public.clone().into();
-		let public = public.to_raw_vec();
 		let signature = self
 			.keystore
-			.sign_with(<AuthorityId as AppKey>::ID, &public_type_pair, header_hash.as_ref())
-			.map_err(|e| sp_consensus::Error::CannotSign(public.clone(), e.to_string()))?
+			.sr25519_sign(<AuthorityId as AppKey>::ID, public.as_inner_ref(), header_hash.as_ref())
+			.map_err(|e| ConsensusError::CannotSign(public.to_raw_vec(), e.to_string()))?
 			.ok_or_else(|| {
-				sp_consensus::Error::CannotSign(
-					public.clone(),
+				ConsensusError::CannotSign(
+					public.to_raw_vec(),
 					"Could not find key in keystore.".into(),
 				)
 			})?;
-		let signature: AuthoritySignature = signature
-			.clone()
-			.try_into()
-			.map_err(|_| sp_consensus::Error::InvalidSignature(signature, public))?;
-		let digest_item = <DigestItem as CompatibleDigestItem>::babe_seal(signature);
+
+		let digest_item = <DigestItem as CompatibleDigestItem>::babe_seal(signature.into());
 
 		let mut import_block = BlockImportParams::new(BlockOrigin::Own, header);
 		import_block.post_digests.push(digest_item);
@@ -894,7 +890,7 @@ where
 		Box::pin(
 			self.env
 				.init(block)
-				.map_err(|e| sp_consensus::Error::ClientImport(format!("{:?}", e))),
+				.map_err(|e| ConsensusError::ClientImport(format!("{:?}", e))),
 		)
 	}
 
@@ -1182,7 +1178,7 @@ where
 			.create_inherent_data_providers
 			.create_inherent_data_providers(parent_hash, ())
 			.await
-			.map_err(|e| Error::<Block>::Client(sp_consensus::Error::from(e).into()))?;
+			.map_err(|e| Error::<Block>::Client(ConsensusError::from(e).into()))?;
 
 		let slot_now = create_inherent_data_providers.slot();
 
diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs
index f1d817ab7d8cf..e4147340793a4 100644
--- a/client/consensus/slots/src/lib.rs
+++ b/client/consensus/slots/src/lib.rs
@@ -150,7 +150,7 @@ pub trait SimpleSlotWorker<B: BlockT> {
 		body: Vec<B::Extrinsic>,
 		storage_changes: StorageChanges<<Self::BlockImport as BlockImport<B>>::Transaction, B>,
 		public: Self::Claim,
-		epoch: Self::AuxData,
+		aux_data: Self::AuxData,
 	) -> Result<
 		sc_consensus::BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
 		sp_consensus::Error,
diff --git a/client/keystore/src/local.rs b/client/keystore/src/local.rs
index a4af7811721ce..75d7bf931e034 100644
--- a/client/keystore/src/local.rs
+++ b/client/keystore/src/local.rs
@@ -20,11 +20,8 @@
 use parking_lot::RwLock;
 use sp_application_crypto::{ecdsa, ed25519, sr25519, AppKey, AppPair, IsWrappedBy};
 use sp_core::{
-	crypto::{
-		ByteArray, CryptoTypePublicPair, ExposeSecret, KeyTypeId, Pair as PairT, SecretString,
-	},
+	crypto::{ByteArray, ExposeSecret, KeyTypeId, Pair as PairT, SecretString},
 	sr25519::{Pair as Sr25519Pair, Public as Sr25519Public},
-	Encode,
 };
 use sp_keystore::{
 	vrf::{make_transcript, VRFSignature, VRFTranscriptData},
@@ -69,60 +66,6 @@ impl LocalKeystore {
 }
 
 impl Keystore for LocalKeystore {
-	fn keys(&self, id: KeyTypeId) -> std::result::Result<Vec<CryptoTypePublicPair>, TraitError> {
-		let raw_keys = self.0.read().raw_public_keys(id)?;
-		Ok(raw_keys.into_iter().fold(Vec::new(), |mut v, k| {
-			v.push(CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone()));
-			v.push(CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone()));
-			v.push(CryptoTypePublicPair(ecdsa::CRYPTO_ID, k));
-			v
-		}))
-	}
-
-	fn sign_with(
-		&self,
-		id: KeyTypeId,
-		key: &CryptoTypePublicPair,
-		msg: &[u8],
-	) -> std::result::Result<Option<Vec<u8>>, TraitError> {
-		match key.0 {
-			ed25519::CRYPTO_ID => {
-				let pub_key = ed25519::Public::from_slice(key.1.as_slice()).map_err(|()| {
-					TraitError::Other("Corrupted public key - Invalid size".into())
-				})?;
-				let key_pair = self
-					.0
-					.read()
-					.key_pair_by_type::<ed25519::Pair>(&pub_key, id)
-					.map_err(TraitError::from)?;
-				key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
-			},
-			sr25519::CRYPTO_ID => {
-				let pub_key = sr25519::Public::from_slice(key.1.as_slice()).map_err(|()| {
-					TraitError::Other("Corrupted public key - Invalid size".into())
-				})?;
-				let key_pair = self
-					.0
-					.read()
-					.key_pair_by_type::<sr25519::Pair>(&pub_key, id)
-					.map_err(TraitError::from)?;
-				key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
-			},
-			ecdsa::CRYPTO_ID => {
-				let pub_key = ecdsa::Public::from_slice(key.1.as_slice()).map_err(|()| {
-					TraitError::Other("Corrupted public key - Invalid size".into())
-				})?;
-				let key_pair = self
-					.0
-					.read()
-					.key_pair_by_type::<ecdsa::Pair>(&pub_key, id)
-					.map_err(TraitError::from)?;
-				key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
-			},
-			_ => Err(TraitError::KeyNotSupported(id)),
-		}
-	}
-
 	fn sr25519_public_keys(&self, key_type: KeyTypeId) -> Vec<sr25519::Public> {
 		self.0
 			.read()
@@ -153,6 +96,38 @@ impl Keystore for LocalKeystore {
 		Ok(pair.public())
 	}
 
+	fn sr25519_sign(
+		&self,
+		id: KeyTypeId,
+		public: &sr25519::Public,
+		msg: &[u8],
+	) -> std::result::Result<Option<sr25519::Signature>, TraitError> {
+		let res = self
+			.0
+			.read()
+			.key_pair_by_type::<sr25519::Pair>(public, id)
+			.map_err(TraitError::from)?
+			.map(|pair| pair.sign(msg));
+		Ok(res)
+	}
+
+	fn sr25519_vrf_sign(
+		&self,
+		key_type: KeyTypeId,
+		public: &Sr25519Public,
+		transcript_data: VRFTranscriptData,
+	) -> std::result::Result<Option<VRFSignature>, TraitError> {
+		let transcript = make_transcript(transcript_data);
+		let pair = self.0.read().key_pair_by_type::<Sr25519Pair>(public, key_type)?;
+
+		if let Some(pair) = pair {
+			let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
+			Ok(Some(VRFSignature { output: inout.to_output(), proof }))
+		} else {
+			Ok(None)
+		}
+	}
+
 	fn ed25519_public_keys(&self, key_type: KeyTypeId) -> Vec<ed25519::Public> {
 		self.0
 			.read()
@@ -183,6 +158,21 @@ impl Keystore for LocalKeystore {
 		Ok(pair.public())
 	}
 
+	fn ed25519_sign(
+		&self,
+		id: KeyTypeId,
+		public: &ed25519::Public,
+		msg: &[u8],
+	) -> std::result::Result<Option<ed25519::Signature>, TraitError> {
+		let res = self
+			.0
+			.read()
+			.key_pair_by_type::<ed25519::Pair>(public, id)
+			.map_err(TraitError::from)?
+			.map(|pair| pair.sign(msg));
+		Ok(res)
+	}
+
 	fn ecdsa_public_keys(&self, key_type: KeyTypeId) -> Vec<ecdsa::Public> {
 		self.0
 			.read()
@@ -213,6 +203,32 @@ impl Keystore for LocalKeystore {
 		Ok(pair.public())
 	}
 
+	fn ecdsa_sign(
+		&self,
+		id: KeyTypeId,
+		public: &ecdsa::Public,
+		msg: &[u8],
+	) -> std::result::Result<Option<ecdsa::Signature>, TraitError> {
+		let res = self
+			.0
+			.read()
+			.key_pair_by_type::<ecdsa::Pair>(public, id)
+			.map_err(TraitError::from)?
+			.map(|pair| pair.sign(msg));
+		Ok(res)
+	}
+
+	fn ecdsa_sign_prehashed(
+		&self,
+		id: KeyTypeId,
+		public: &ecdsa::Public,
+		msg: &[u8; 32],
+	) -> std::result::Result<Option<ecdsa::Signature>, TraitError> {
+		let pair = self.0.read().key_pair_by_type::<ecdsa::Pair>(public, id)?;
+
+		pair.map(|k| k.sign_prehashed(msg)).map(Ok).transpose()
+	}
+
 	fn insert(
 		&self,
 		key_type: KeyTypeId,
@@ -222,39 +238,15 @@ impl Keystore for LocalKeystore {
 		self.0.write().insert(key_type, suri, public).map_err(|_| ())
 	}
 
+	fn keys(&self, id: KeyTypeId) -> std::result::Result<Vec<Vec<u8>>, TraitError> {
+		self.0.read().raw_public_keys(id).map_err(|e| e.into())
+	}
+
 	fn has_keys(&self, public_keys: &[(Vec<u8>, KeyTypeId)]) -> bool {
 		public_keys
 			.iter()
 			.all(|(p, t)| self.0.read().key_phrase_by_type(p, *t).ok().flatten().is_some())
 	}
-
-	fn sr25519_vrf_sign(
-		&self,
-		key_type: KeyTypeId,
-		public: &Sr25519Public,
-		transcript_data: VRFTranscriptData,
-	) -> std::result::Result<Option<VRFSignature>, TraitError> {
-		let transcript = make_transcript(transcript_data);
-		let pair = self.0.read().key_pair_by_type::<Sr25519Pair>(public, key_type)?;
-
-		if let Some(pair) = pair {
-			let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
-			Ok(Some(VRFSignature { output: inout.to_output(), proof }))
-		} else {
-			Ok(None)
-		}
-	}
-
-	fn ecdsa_sign_prehashed(
-		&self,
-		id: KeyTypeId,
-		public: &ecdsa::Public,
-		msg: &[u8; 32],
-	) -> std::result::Result<Option<ecdsa::Signature>, TraitError> {
-		let pair = self.0.read().key_pair_by_type::<ecdsa::Pair>(public, id)?;
-
-		pair.map(|k| k.sign_prehashed(msg)).map(Ok).transpose()
-	}
 }
 
 impl Into<KeystorePtr> for LocalKeystore {
diff --git a/primitives/consensus/grandpa/src/lib.rs b/primitives/consensus/grandpa/src/lib.rs
index 7f5857e4ba26a..16a041d414356 100644
--- a/primitives/consensus/grandpa/src/lib.rs
+++ b/primitives/consensus/grandpa/src/lib.rs
@@ -452,11 +452,11 @@ where
 	N: Encode,
 {
 	use sp_application_crypto::AppKey;
-	use sp_core::crypto::Public;
+	use sp_core::crypto::Wraps;
 
 	let encoded = localized_payload(round, set_id, &message);
 	let signature = keystore
-		.sign_with(AuthorityId::ID, &public.to_public_crypto_pair(), &encoded[..])
+		.ed25519_sign(AuthorityId::ID, public.as_inner_ref(), &encoded[..])
 		.ok()
 		.flatten()?
 		.try_into()
diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs
index 709e1fa642f80..306ac8e60c529 100644
--- a/primitives/io/src/lib.rs
+++ b/primitives/io/src/lib.rs
@@ -762,10 +762,9 @@ pub trait Crypto {
 	) -> Option<ed25519::Signature> {
 		self.extension::<KeystoreExt>()
 			.expect("No `keystore` associated for the current context!")
-			.sign_with(id, &pub_key.into(), msg)
+			.ed25519_sign(id, pub_key, msg)
 			.ok()
 			.flatten()
-			.and_then(|sig| ed25519::Signature::from_slice(&sig))
 	}
 
 	/// Verify `ed25519` signature.
@@ -902,10 +901,9 @@ pub trait Crypto {
 	) -> Option<sr25519::Signature> {
 		self.extension::<KeystoreExt>()
 			.expect("No `keystore` associated for the current context!")
-			.sign_with(id, &pub_key.into(), msg)
+			.sr25519_sign(id, pub_key, msg)
 			.ok()
 			.flatten()
-			.and_then(|sig| sr25519::Signature::from_slice(&sig))
 	}
 
 	/// Verify an `sr25519` signature.
@@ -949,10 +947,9 @@ pub trait Crypto {
 	) -> Option<ecdsa::Signature> {
 		self.extension::<KeystoreExt>()
 			.expect("No `keystore` associated for the current context!")
-			.sign_with(id, &pub_key.into(), msg)
+			.ecdsa_sign(id, pub_key, msg)
 			.ok()
 			.flatten()
-			.and_then(|sig| ecdsa::Signature::from_slice(&sig))
 	}
 
 	/// Sign the given a pre-hashed `msg` with the `ecdsa` key that corresponds to the given public
diff --git a/primitives/keystore/src/lib.rs b/primitives/keystore/src/lib.rs
index d1cbe227ed551..2b4322a426b93 100644
--- a/primitives/keystore/src/lib.rs
+++ b/primitives/keystore/src/lib.rs
@@ -21,7 +21,7 @@ pub mod vrf;
 
 use crate::vrf::{VRFSignature, VRFTranscriptData};
 use sp_core::{
-	crypto::{CryptoTypePublicPair, KeyTypeId},
+	crypto::{ByteArray, CryptoTypeId, KeyTypeId},
 	ecdsa, ed25519, sr25519,
 };
 use std::sync::Arc;
@@ -59,6 +59,33 @@ pub trait Keystore: Send + Sync {
 		seed: Option<&str>,
 	) -> Result<sr25519::Public, Error>;
 
+	/// TODO
+	fn sr25519_sign(
+		&self,
+		id: KeyTypeId,
+		public: &sr25519::Public,
+		msg: &[u8],
+	) -> Result<Option<sr25519::Signature>, Error>;
+
+	/// Generate VRF signature  for given transcript data.
+	///
+	/// Receives KeyTypeId and Public key to be able to map
+	/// them to a private key that exists in the keystore which
+	/// is, in turn, used for signing the provided transcript.
+	///
+	/// Returns a result containing the signature data.
+	/// Namely, VRFOutput and VRFProof which are returned
+	/// inside the `VRFSignature` container struct.
+	///
+	/// This function will return `None` if the given `key_type` and `public` combination
+	/// doesn't exist in the keystore or an `Err` when something failed.
+	fn sr25519_vrf_sign(
+		&self,
+		key_type: KeyTypeId,
+		public: &sr25519::Public,
+		transcript_data: VRFTranscriptData,
+	) -> Result<Option<VRFSignature>, Error>;
+
 	/// Returns all ed25519 public keys for the given key type.
 	fn ed25519_public_keys(&self, id: KeyTypeId) -> Vec<ed25519::Public>;
 
@@ -73,6 +100,14 @@ pub trait Keystore: Send + Sync {
 		seed: Option<&str>,
 	) -> Result<ed25519::Public, Error>;
 
+	/// TODO
+	fn ed25519_sign(
+		&self,
+		id: KeyTypeId,
+		public: &ed25519::Public,
+		msg: &[u8],
+	) -> Result<Option<ed25519::Signature>, Error>;
+
 	/// Returns all ecdsa public keys for the given key type.
 	fn ecdsa_public_keys(&self, id: KeyTypeId) -> Vec<ecdsa::Public>;
 
@@ -84,51 +119,21 @@ pub trait Keystore: Send + Sync {
 	fn ecdsa_generate_new(&self, id: KeyTypeId, seed: Option<&str>)
 		-> Result<ecdsa::Public, Error>;
 
-	/// Insert a new secret key.
-	fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()>;
-
-	/// List all supported keys
-	///
-	/// Returns a set of public keys the signer supports.
-	fn keys(&self, id: KeyTypeId) -> Result<Vec<CryptoTypePublicPair>, Error>;
-
-	/// Checks if the private keys for the given public key and key type combinations exist.
+	/// Generate an ECDSA signature for a given message.
 	///
-	/// Returns `true` iff all private keys could be found.
-	fn has_keys(&self, public_keys: &[(Vec<u8>, KeyTypeId)]) -> bool;
-
-	/// Sign with key
-	///
-	/// Signs a message with the private key that matches
-	/// the public key passed.
+	/// Receives [`KeyTypeId`] and an [`ecdsa::Public`] key to be able to map
+	/// them to a private key that exists in the keystore. This private key is,
+	/// in turn, used for signing the provided pre-hashed message.
 	///
-	/// Returns the SCALE encoded signature if key is found and supported, `None` if the key doesn't
-	/// exist or an error when something failed.
-	fn sign_with(
+	/// Returns an [`ecdsa::Signature`] or `None` in case the given `id` and
+	/// `public` combination doesn't exist in the keystore. An `Err` will be
+	/// returned if generating the signature itself failed.
+	fn ecdsa_sign(
 		&self,
 		id: KeyTypeId,
-		key: &CryptoTypePublicPair,
+		public: &ecdsa::Public,
 		msg: &[u8],
-	) -> Result<Option<Vec<u8>>, Error>;
-
-	/// Generate VRF signature for given transcript data.
-	///
-	/// Receives KeyTypeId and Public key to be able to map
-	/// them to a private key that exists in the keystore which
-	/// is, in turn, used for signing the provided transcript.
-	///
-	/// Returns a result containing the signature data.
-	/// Namely, VRFOutput and VRFProof which are returned
-	/// inside the `VRFSignature` container struct.
-	///
-	/// This function will return `None` if the given `key_type` and `public` combination
-	/// doesn't exist in the keystore or an `Err` when something failed.
-	fn sr25519_vrf_sign(
-		&self,
-		key_type: KeyTypeId,
-		public: &sr25519::Public,
-		transcript_data: VRFTranscriptData,
-	) -> Result<Option<VRFSignature>, Error>;
+	) -> Result<Option<ecdsa::Signature>, Error>;
 
 	/// Generate an ECDSA signature for a given pre-hashed message.
 	///
@@ -148,6 +153,58 @@ pub trait Keystore: Send + Sync {
 		public: &ecdsa::Public,
 		msg: &[u8; 32],
 	) -> Result<Option<ecdsa::Signature>, Error>;
+
+	/// Insert a new secret key.
+	fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()>;
+
+	/// List all supported keys
+	///
+	/// Returns a set of public keys the signer supports.
+	fn keys(&self, id: KeyTypeId) -> Result<Vec<Vec<u8>>, Error>;
+
+	/// Checks if the private keys for the given public key and key type combinations exist.
+	///
+	/// Returns `true` iff all private keys could be found.
+	fn has_keys(&self, public_keys: &[(Vec<u8>, KeyTypeId)]) -> bool;
+
+	/// Convenience method to sign a message using an opaque key type.
+	///
+	/// The message is signed using the cryptographic primitive specified by `KeyCryptoId`.
+	///
+	/// Schemes supported by the default trait implementation: sr25519, ed25519 and ecdsa.
+	/// To support more schemes you can overwrite this method.
+	///
+	/// Returns the SCALE encoded signature if key is found and supported, `None` if the key doesn't
+	/// exist or an error when something failed.
+	fn sign_with(
+		&self,
+		id: KeyTypeId,
+		crypto_id: CryptoTypeId,
+		public: &[u8],
+		msg: &[u8],
+	) -> Result<Option<Vec<u8>>, Error> {
+		use codec::Encode;
+
+		let signature = match crypto_id {
+			sr25519::CRYPTO_ID => {
+				let public = sr25519::Public::from_slice(public)
+					.map_err(|_| Error::ValidationError("Invalid public key format".into()))?;
+				self.sr25519_sign(id, &public, msg)?.map(|s| s.encode())
+			},
+			ed25519::CRYPTO_ID => {
+				let public = ed25519::Public::from_slice(public)
+					.map_err(|_| Error::ValidationError("Invalid public key format".into()))?;
+				self.ed25519_sign(id, &public, msg)?.map(|s| s.encode())
+			},
+			ecdsa::CRYPTO_ID => {
+				let public = ecdsa::Public::from_slice(public)
+					.map_err(|_| Error::ValidationError("Invalid public key format".into()))?;
+				self.ecdsa_sign(id, &public, msg)?.map(|s| s.encode())
+			},
+			_ => return Err(Error::KeyNotSupported(id)),
+		};
+		Ok(signature)
+	}
 }
 
 /// A shared pointer to a keystore implementation.
diff --git a/primitives/keystore/src/testing.rs b/primitives/keystore/src/testing.rs
index 73b560d563a17..7feec2faed492 100644
--- a/primitives/keystore/src/testing.rs
+++ b/primitives/keystore/src/testing.rs
@@ -18,7 +18,7 @@
 //! Types that should only be used for testing!
 
 use sp_core::{
-	crypto::{ByteArray, CryptoTypePublicPair, KeyTypeId, Pair},
+	crypto::{ByteArray, KeyTypeId, Pair},
 	ecdsa, ed25519, sr25519,
 };
 
@@ -68,21 +68,6 @@ impl MemoryKeystore {
 }
 
 impl Keystore for MemoryKeystore {
-	fn keys(&self, id: KeyTypeId) -> Result<Vec<CryptoTypePublicPair>, Error> {
-		self.keys
-			.read()
-			.get(&id)
-			.map(|map| {
-				Ok(map.keys().fold(Vec::new(), |mut v, k| {
-					v.push(CryptoTypePublicPair(sr25519::CRYPTO_ID, k.clone()));
-					v.push(CryptoTypePublicPair(ed25519::CRYPTO_ID, k.clone()));
-					v.push(CryptoTypePublicPair(ecdsa::CRYPTO_ID, k.clone()));
-					v
-				}))
-			})
-			.unwrap_or_else(|| Ok(vec![]))
-	}
-
 	fn sr25519_public_keys(&self, id: KeyTypeId) -> Vec<sr25519::Public> {
 		self.keys
 			.read()
@@ -127,6 +112,29 @@ impl Keystore for MemoryKeystore {
 		}
 	}
 
+	fn sr25519_sign(
+		&self,
+		id: KeyTypeId,
+		public: &sr25519::Public,
+		msg: &[u8],
+	) -> Result<Option<sr25519::Signature>, Error> {
+		Ok(self.sr25519_key_pair(id, public).map(|pair| pair.sign(msg)))
+	}
+
+	fn sr25519_vrf_sign(
+		&self,
+		key_type: KeyTypeId,
+		public: &sr25519::Public,
+		transcript_data: VRFTranscriptData,
+	) -> Result<Option<VRFSignature>, Error> {
+		let transcript = make_transcript(transcript_data);
+		let pair =
+			if let Some(k) = self.sr25519_key_pair(key_type, public) { k } else { return Ok(None) };
+
+		let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
+		Ok(Some(VRFSignature { output: inout.to_output(), proof }))
+	}
+
 	fn ed25519_public_keys(&self, id: KeyTypeId) -> Vec<ed25519::Public> {
 		self.keys
 			.read()
@@ -171,6 +179,15 @@ impl Keystore for MemoryKeystore {
 		}
 	}
 
+	fn ed25519_sign(
+		&self,
+		id: KeyTypeId,
+		public: &ed25519::Public,
+		msg: &[u8],
+	) -> Result<Option<ed25519::Signature>, Error> {
+		Ok(self.ed25519_key_pair(id, public).map(|pair| pair.sign(msg)))
+	}
+
 	fn ecdsa_public_keys(&self, id: KeyTypeId) -> Vec<ecdsa::Public> {
 		self.keys
 			.read()
@@ -214,6 +231,25 @@ impl Keystore for MemoryKeystore {
 		}
 	}
 
+	fn ecdsa_sign(
+		&self,
+		id: KeyTypeId,
+		public: &ecdsa::Public,
+		msg: &[u8],
+	) -> Result<Option<ecdsa::Signature>, Error> {
+		Ok(self.ecdsa_key_pair(id, public).map(|pair| pair.sign(msg)))
+	}
+
+	fn ecdsa_sign_prehashed(
+		&self,
+		id: KeyTypeId,
+		public: &ecdsa::Public,
+		msg: &[u8; 32],
+	) -> Result<Option<ecdsa::Signature>, Error> {
+		let pair = self.ecdsa_key_pair(id, public);
+		pair.map(|pair| pair.sign_prehashed(msg)).map(Ok).transpose()
+	}
+
 	fn insert(&self, id: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> {
 		self.keys
 			.write()
@@ -223,66 +259,21 @@ impl Keystore for MemoryKeystore {
 		Ok(())
 	}
 
+	fn keys(&self, id: KeyTypeId) -> Result<Vec<Vec<u8>>, Error> {
+		let keys = self
+			.keys
+			.read()
+			.get(&id)
+			.map(|map| map.keys().cloned().collect())
+			.unwrap_or_default();
+		Ok(keys)
+	}
+
 	fn has_keys(&self, public_keys: &[(Vec<u8>, KeyTypeId)]) -> bool {
 		public_keys
 			.iter()
 			.all(|(k, t)| self.keys.read().get(t).and_then(|s| s.get(k)).is_some())
 	}
-
-	fn sign_with(
-		&self,
-		id: KeyTypeId,
-		key: &CryptoTypePublicPair,
-		msg: &[u8],
-	) -> Result<Option<Vec<u8>>, Error> {
-		use codec::Encode;
-
-		match key.0 {
-			ed25519::CRYPTO_ID => {
-				let key_pair = self
-					.ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice()).unwrap());
-
-				key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
-			},
-			sr25519::CRYPTO_ID => {
-				let key_pair = self
-					.sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice()).unwrap());
-
-				key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
-			},
-			ecdsa::CRYPTO_ID => {
-				let key_pair =
-					self.ecdsa_key_pair(id, &ecdsa::Public::from_slice(key.1.as_slice()).unwrap());
-
-				key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
-			},
-			_ => Err(Error::KeyNotSupported(id)),
-		}
-	}
-
-	fn sr25519_vrf_sign(
-		&self,
-		key_type: KeyTypeId,
-		public: &sr25519::Public,
-		transcript_data: VRFTranscriptData,
-	) -> Result<Option<VRFSignature>, Error> {
-		let transcript = make_transcript(transcript_data);
-		let pair =
-			if let Some(k) = self.sr25519_key_pair(key_type, public) { k } else { return Ok(None) };
-
-		let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
-		Ok(Some(VRFSignature { output: inout.to_output(), proof }))
-	}
-
-	fn ecdsa_sign_prehashed(
-		&self,
-		id: KeyTypeId,
-		public: &ecdsa::Public,
-		msg: &[u8; 32],
-	) -> Result<Option<ecdsa::Signature>, Error> {
-		let pair = self.ecdsa_key_pair(id, public);
-		pair.map(|k| k.sign_prehashed(msg)).map(Ok).transpose()
-	}
 }
 
 impl Into<KeystorePtr> for MemoryKeystore {
@@ -306,7 +297,7 @@ mod tests {
 
 		let public = store.ed25519_generate_new(ED25519, None).expect("Generates key");
 
-		let public_keys = store.keys(ED25519).unwrap();
+		let public_keys = store.ed25519_public_keys(ED25519);
 
 		assert!(public_keys.contains(&public.into()));
 	}
@@ -322,7 +313,7 @@ mod tests {
 			.insert(SR25519, secret_uri, key_pair.public().as_ref())
 			.expect("Inserts unknown key");
 
-		let public_keys = store.keys(SR25519).unwrap();
+		let public_keys = store.sr25519_public_keys(SR25519);
 
 		assert!(public_keys.contains(&key_pair.public().into()));
 	}

From ae5bb3eafe8f615ac40bcfb103f99b7d8df42934 Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Wed, 22 Mar 2023 18:36:18 +0100
Subject: [PATCH 02/15] Get rid of 'AppKey::UntypedGeneric' associated type.

Untyped generics are accessible using associated types 'Generic' associated type.
I.e. <T as AppKey>::Public::Generic
---
 primitives/application-crypto/src/lib.rs    |  5 -----
 primitives/application-crypto/src/traits.rs | 18 ++++++++----------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs
index 0685477ddff8d..43eaa8228e548 100644
--- a/primitives/application-crypto/src/lib.rs
+++ b/primitives/application-crypto/src/lib.rs
@@ -171,7 +171,6 @@ macro_rules! app_crypto_pair {
 		}
 
 		impl $crate::AppKey for Pair {
-			type UntypedGeneric = $pair;
 			type Public = Public;
 			type Pair = Pair;
 			type Signature = Signature;
@@ -239,7 +238,6 @@ macro_rules! app_crypto_public_full_crypto {
 		}
 
 		impl $crate::AppKey for Public {
-			type UntypedGeneric = $public;
 			type Public = Public;
 			type Pair = Pair;
 			type Signature = Signature;
@@ -273,7 +271,6 @@ macro_rules! app_crypto_public_not_full_crypto {
 		impl $crate::CryptoType for Public {}
 
 		impl $crate::AppKey for Public {
-			type UntypedGeneric = $public;
 			type Public = Public;
 			type Signature = Signature;
 			const ID: $crate::KeyTypeId = $key_type;
@@ -451,7 +448,6 @@ macro_rules! app_crypto_signature_full_crypto {
 		}
 
 		impl $crate::AppKey for Signature {
-			type UntypedGeneric = $sig;
 			type Public = Public;
 			type Pair = Pair;
 			type Signature = Signature;
@@ -483,7 +479,6 @@ macro_rules! app_crypto_signature_not_full_crypto {
 		impl $crate::CryptoType for Signature {}
 
 		impl $crate::AppKey for Signature {
-			type UntypedGeneric = $sig;
 			type Public = Public;
 			type Signature = Signature;
 			const ID: $crate::KeyTypeId = $key_type;
diff --git a/primitives/application-crypto/src/traits.rs b/primitives/application-crypto/src/traits.rs
index e3586ccecd0d8..eb6cb19ca9146 100644
--- a/primitives/application-crypto/src/traits.rs
+++ b/primitives/application-crypto/src/traits.rs
@@ -24,23 +24,21 @@ use sp_std::{fmt::Debug, vec::Vec};
 
 /// An application-specific key.
 pub trait AppKey: 'static + Send + Sync + Sized + CryptoType + Clone {
-	/// The corresponding type as a generic crypto type.
-	type UntypedGeneric: IsWrappedBy<Self>;
+	/// Identifier for application-specific key type.
+	const ID: KeyTypeId;
+
+	/// Identifier of the crypto type of this application-specific key type.
+	const CRYPTO_ID: CryptoTypeId;
 
 	/// The corresponding public key type in this application scheme.
 	type Public: AppPublic;
 
-	/// The corresponding key pair type in this application scheme.
-	#[cfg(feature = "full_crypto")]
-	type Pair: AppPair;
-
 	/// The corresponding signature type in this application scheme.
 	type Signature: AppSignature;
 
-	/// An identifier for this application-specific key type.
-	const ID: KeyTypeId;
-	/// The identifier of the crypto type of this application-specific key type.
-	const CRYPTO_ID: CryptoTypeId;
+	/// The corresponding key pair type in this application scheme.
+	#[cfg(feature = "full_crypto")]
+	type Pair: AppPair;
 }
 
 /// Type which implements Hash in std, not when no-std (std variant).

From 04c685b522ccb27c20cef3110658de321e819f3c Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Wed, 22 Mar 2023 19:02:49 +0100
Subject: [PATCH 03/15] Get rid of 'CryptoTypePublicPair'

---
 client/rpc/src/author/tests.rs           | 14 ++++-------
 primitives/application-crypto/src/lib.rs | 22 +++-------------
 primitives/core/src/crypto.rs            | 32 +++++-------------------
 primitives/core/src/ecdsa.rs             | 19 +++-----------
 primitives/core/src/ed25519.rs           | 20 +++------------
 primitives/core/src/sr25519.rs           | 21 +++-------------
 6 files changed, 24 insertions(+), 104 deletions(-)

diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs
index 75b6390c83cc7..fbbd1a92fdaa7 100644
--- a/client/rpc/src/author/tests.rs
+++ b/client/rpc/src/author/tests.rs
@@ -31,8 +31,8 @@ use sc_transaction_pool_api::TransactionStatus;
 use sp_core::{
 	blake2_256,
 	bytes::to_hex,
-	crypto::{ByteArray, CryptoTypePublicPair, Pair},
-	ed25519, sr25519,
+	crypto::{ByteArray, Pair},
+	ed25519,
 	testing::{ED25519, SR25519},
 	H256,
 };
@@ -227,9 +227,7 @@ async fn author_should_insert_key() {
 	api.call::<_, ()>("author_insertKey", params).await.unwrap();
 	let pubkeys = setup.keystore.keys(ED25519).unwrap();
 
-	assert!(
-		pubkeys.contains(&CryptoTypePublicPair(ed25519::CRYPTO_ID, keypair.public().to_raw_vec()))
-	);
+	assert!(pubkeys.contains(&keypair.public().to_raw_vec()));
 }
 
 #[tokio::test]
@@ -242,10 +240,8 @@ async fn author_should_rotate_keys() {
 		SessionKeys::decode(&mut &new_pubkeys[..]).expect("SessionKeys decode successfully");
 	let ed25519_pubkeys = setup.keystore.keys(ED25519).unwrap();
 	let sr25519_pubkeys = setup.keystore.keys(SR25519).unwrap();
-	assert!(ed25519_pubkeys
-		.contains(&CryptoTypePublicPair(ed25519::CRYPTO_ID, session_keys.ed25519.to_raw_vec())));
-	assert!(sr25519_pubkeys
-		.contains(&CryptoTypePublicPair(sr25519::CRYPTO_ID, session_keys.sr25519.to_raw_vec())));
+	assert!(ed25519_pubkeys.contains(&session_keys.ed25519.to_raw_vec()));
+	assert!(sr25519_pubkeys.contains(&session_keys.sr25519.to_raw_vec()));
 }
 
 #[tokio::test]
diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs
index 43eaa8228e548..1b285c315c274 100644
--- a/primitives/application-crypto/src/lib.rs
+++ b/primitives/application-crypto/src/lib.rs
@@ -27,10 +27,7 @@ pub use sp_core::crypto::{DeriveError, DeriveJunction, Pair, SecretStringError,
 #[doc(hidden)]
 pub use sp_core::{
 	self,
-	crypto::{
-		ByteArray, CryptoType, CryptoTypePublicPair, Derive, IsWrappedBy, Public, UncheckedFrom,
-		Wraps,
-	},
+	crypto::{ByteArray, CryptoType, Derive, IsWrappedBy, Public, UncheckedFrom, Wraps},
 	RuntimeDebug,
 };
 
@@ -303,9 +300,10 @@ macro_rules! app_crypto_public_common {
 		impl $crate::ByteArray for Public {
 			const LEN: usize = <$public>::LEN;
 		}
+
 		impl $crate::Public for Public {
-			fn to_public_crypto_pair(&self) -> $crate::CryptoTypePublicPair {
-				$crate::CryptoTypePublicPair($crypto_type, $crate::ByteArray::to_raw_vec(self))
+			fn crypto_id(&self) -> $crate::CryptoTypeId {
+				$crypto_type
 			}
 		}
 
@@ -347,18 +345,6 @@ macro_rules! app_crypto_public_common {
 			}
 		}
 
-		impl From<Public> for $crate::CryptoTypePublicPair {
-			fn from(key: Public) -> Self {
-				(&key).into()
-			}
-		}
-
-		impl From<&Public> for $crate::CryptoTypePublicPair {
-			fn from(key: &Public) -> Self {
-				$crate::CryptoTypePublicPair($crypto_type, $crate::ByteArray::to_raw_vec(key))
-			}
-		}
-
 		impl<'a> TryFrom<&'a [u8]> for Public {
 			type Error = ();
 
diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs
index 80e65b342cbd7..f8e986cab5a7b 100644
--- a/primitives/core/src/crypto.rs
+++ b/primitives/core/src/crypto.rs
@@ -19,8 +19,6 @@
 //! Cryptographic utilities.
 // end::description[]
 
-#[cfg(feature = "std")]
-use crate::hexdisplay::HexDisplay;
 use crate::{ed25519, sr25519};
 #[cfg(feature = "std")]
 use base58::{FromBase58, ToBase58};
@@ -488,8 +486,8 @@ pub trait ByteArray: AsRef<[u8]> + AsMut<[u8]> + for<'a> TryFrom<&'a [u8], Error
 
 /// Trait suitable for typical cryptographic PKI key public type.
 pub trait Public: ByteArray + Derive + CryptoType + PartialEq + Eq + Clone + Send + Sync {
-	/// Return `CryptoTypePublicPair` from public key.
-	fn to_public_crypto_pair(&self) -> CryptoTypePublicPair;
+	/// Return `CryptoTypeId` from public key.
+	fn crypto_id(&self) -> CryptoTypeId;
 }
 
 /// An opaque 32-byte cryptographic identifier.
@@ -690,8 +688,8 @@ mod dummy {
 		}
 	}
 	impl Public for Dummy {
-		fn to_public_crypto_pair(&self) -> CryptoTypePublicPair {
-			CryptoTypePublicPair(CryptoTypeId(*b"dumm"), <Self as ByteArray>::to_raw_vec(self))
+		fn crypto_id(&self) -> CryptoTypeId {
+			CryptoTypeId(*b"dumm")
 		}
 	}
 
@@ -1118,24 +1116,6 @@ impl<'a> TryFrom<&'a str> for KeyTypeId {
 #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub struct CryptoTypeId(pub [u8; 4]);
 
-/// A type alias of CryptoTypeId & a public key
-#[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)]
-#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
-pub struct CryptoTypePublicPair(pub CryptoTypeId, pub Vec<u8>);
-
-#[cfg(feature = "std")]
-impl sp_std::fmt::Display for CryptoTypePublicPair {
-	fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
-		let id = match str::from_utf8(&(self.0).0[..]) {
-			Ok(id) => id.to_string(),
-			Err(_) => {
-				format!("{:#?}", self.0)
-			},
-		};
-		write!(f, "{}-{}", id, HexDisplay::from(&self.1))
-	}
-}
-
 /// Known key types; this also functions as a global registry of key types for projects wishing to
 /// avoid collisions with each other.
 ///
@@ -1224,8 +1204,8 @@ mod tests {
 		}
 	}
 	impl Public for TestPublic {
-		fn to_public_crypto_pair(&self) -> CryptoTypePublicPair {
-			CryptoTypePublicPair(CryptoTypeId(*b"dumm"), self.to_raw_vec())
+		fn crypto_id(&self) -> CryptoTypeId {
+			CryptoTypeId(*b"dumm")
 		}
 	}
 	impl Pair for TestPair {
diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs
index e63b283d26ee3..9680b2e9c3fbd 100644
--- a/primitives/core/src/ecdsa.rs
+++ b/primitives/core/src/ecdsa.rs
@@ -24,8 +24,7 @@ use sp_runtime_interface::pass_by::PassByInner;
 #[cfg(feature = "std")]
 use crate::crypto::Ss58Codec;
 use crate::crypto::{
-	ByteArray, CryptoType, CryptoTypeId, CryptoTypePublicPair, Derive, Public as TraitPublic,
-	UncheckedFrom,
+	ByteArray, CryptoType, CryptoTypeId, Derive, Public as TraitPublic, UncheckedFrom,
 };
 #[cfg(feature = "full_crypto")]
 use crate::{
@@ -104,20 +103,8 @@ impl ByteArray for Public {
 }
 
 impl TraitPublic for Public {
-	fn to_public_crypto_pair(&self) -> CryptoTypePublicPair {
-		CryptoTypePublicPair(CRYPTO_ID, self.to_raw_vec())
-	}
-}
-
-impl From<Public> for CryptoTypePublicPair {
-	fn from(key: Public) -> Self {
-		(&key).into()
-	}
-}
-
-impl From<&Public> for CryptoTypePublicPair {
-	fn from(key: &Public) -> Self {
-		CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec())
+	fn crypto_id(&self) -> CryptoTypeId {
+		CRYPTO_ID
 	}
 }
 
diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs
index 503c127a9e06b..a988ff8705abc 100644
--- a/primitives/core/src/ed25519.rs
+++ b/primitives/core/src/ed25519.rs
@@ -31,9 +31,7 @@ use scale_info::TypeInfo;
 
 #[cfg(feature = "std")]
 use crate::crypto::Ss58Codec;
-use crate::crypto::{
-	CryptoType, CryptoTypeId, CryptoTypePublicPair, Derive, Public as TraitPublic, UncheckedFrom,
-};
+use crate::crypto::{CryptoType, CryptoTypeId, Derive, Public as TraitPublic, UncheckedFrom};
 #[cfg(feature = "full_crypto")]
 use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError};
 #[cfg(feature = "full_crypto")]
@@ -356,25 +354,13 @@ impl ByteArray for Public {
 }
 
 impl TraitPublic for Public {
-	fn to_public_crypto_pair(&self) -> CryptoTypePublicPair {
-		CryptoTypePublicPair(CRYPTO_ID, self.to_raw_vec())
+	fn crypto_id(&self) -> CryptoTypeId {
+		CRYPTO_ID
 	}
 }
 
 impl Derive for Public {}
 
-impl From<Public> for CryptoTypePublicPair {
-	fn from(key: Public) -> Self {
-		(&key).into()
-	}
-}
-
-impl From<&Public> for CryptoTypePublicPair {
-	fn from(key: &Public) -> Self {
-		CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec())
-	}
-}
-
 /// Derive a single hard junction.
 #[cfg(feature = "full_crypto")]
 fn derive_hard_junction(secret_seed: &Seed, cc: &[u8; 32]) -> Seed {
diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs
index 8dbcca9e2673e..b2ea44c02792a 100644
--- a/primitives/core/src/sr25519.rs
+++ b/primitives/core/src/sr25519.rs
@@ -34,10 +34,7 @@ use schnorrkel::{
 use sp_std::vec::Vec;
 
 use crate::{
-	crypto::{
-		ByteArray, CryptoType, CryptoTypeId, CryptoTypePublicPair, Derive, Public as TraitPublic,
-		UncheckedFrom,
-	},
+	crypto::{ByteArray, CryptoType, CryptoTypeId, Derive, Public as TraitPublic, UncheckedFrom},
 	hash::{H256, H512},
 };
 use codec::{Decode, Encode, MaxEncodedLen};
@@ -387,20 +384,8 @@ impl ByteArray for Public {
 }
 
 impl TraitPublic for Public {
-	fn to_public_crypto_pair(&self) -> CryptoTypePublicPair {
-		CryptoTypePublicPair(CRYPTO_ID, self.to_raw_vec())
-	}
-}
-
-impl From<Public> for CryptoTypePublicPair {
-	fn from(key: Public) -> Self {
-		(&key).into()
-	}
-}
-
-impl From<&Public> for CryptoTypePublicPair {
-	fn from(key: &Public) -> Self {
-		CryptoTypePublicPair(CRYPTO_ID, key.to_raw_vec())
+	fn crypto_id(&self) -> CryptoTypeId {
+		CRYPTO_ID
 	}
 }
 

From 8b0131b3e33601da5346782cf8a993c722b4af2a Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Wed, 22 Mar 2023 19:29:29 +0100
Subject: [PATCH 04/15] Trivial fix

---
 bin/node/cli/src/service.rs                       | 10 ++++------
 primitives/application-crypto/test/src/ecdsa.rs   | 11 +++++++----
 primitives/application-crypto/test/src/ed25519.rs | 11 +++++++----
 primitives/application-crypto/test/src/sr25519.rs | 11 +++++++----
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs
index be8553b7b3a98..79dbeeee48628 100644
--- a/bin/node/cli/src/service.rs
+++ b/bin/node/cli/src/service.rs
@@ -592,7 +592,7 @@ mod tests {
 	use sc_service_test::TestNetNode;
 	use sc_transaction_pool_api::{ChainEvent, MaintainedTransactionPool};
 	use sp_consensus::{BlockOrigin, Environment, Proposer};
-	use sp_core::{crypto::Pair as CryptoPair, Public};
+	use sp_core::crypto::{Pair as CryptoPair, Wraps};
 	use sp_inherents::InherentDataProvider;
 	use sp_keyring::AccountKeyring;
 	use sp_keystore::KeystorePtr;
@@ -737,16 +737,14 @@ mod tests {
 				// add it to a digest item.
 				let to_sign = pre_hash.encode();
 				let signature = keystore
-					.sign_with(
+					.sr25519_sign(
 						sp_consensus_babe::AuthorityId::ID,
-						&alice.to_public_crypto_pair(),
+						alice.as_inner_ref(),
 						&to_sign,
 					)
 					.unwrap()
-					.unwrap()
-					.try_into()
 					.unwrap();
-				let item = <DigestItem as CompatibleDigestItem>::babe_seal(signature);
+				let item = <DigestItem as CompatibleDigestItem>::babe_seal(signature.into());
 				slot += 1;
 
 				let mut params = BlockImportParams::new(BlockOrigin::File, new_header);
diff --git a/primitives/application-crypto/test/src/ecdsa.rs b/primitives/application-crypto/test/src/ecdsa.rs
index fe9e17a65b1a7..99ca6f4c4adf2 100644
--- a/primitives/application-crypto/test/src/ecdsa.rs
+++ b/primitives/application-crypto/test/src/ecdsa.rs
@@ -17,8 +17,11 @@
 
 //! Integration tests for ecdsa
 use sp_api::ProvideRuntimeApi;
-use sp_application_crypto::ecdsa::{AppPair, AppPublic};
-use sp_core::{crypto::Pair, testing::ECDSA};
+use sp_application_crypto::ecdsa::AppPair;
+use sp_core::{
+	crypto::{ByteArray, Pair},
+	testing::ECDSA,
+};
 use sp_keystore::{testing::MemoryKeystore, Keystore};
 use std::sync::Arc;
 use substrate_test_runtime_client::{
@@ -35,6 +38,6 @@ fn ecdsa_works_in_runtime() {
 		.expect("Tests `ecdsa` crypto.");
 
 	let supported_keys = keystore.keys(ECDSA).unwrap();
-	assert!(supported_keys.contains(&public.clone().into()));
-	assert!(AppPair::verify(&signature, "ecdsa", &AppPublic::from(public)));
+	assert!(supported_keys.contains(&public.to_raw_vec()));
+	assert!(AppPair::verify(&signature, "ecdsa", &public));
 }
diff --git a/primitives/application-crypto/test/src/ed25519.rs b/primitives/application-crypto/test/src/ed25519.rs
index 5687bd013970a..f4553f95bf1f8 100644
--- a/primitives/application-crypto/test/src/ed25519.rs
+++ b/primitives/application-crypto/test/src/ed25519.rs
@@ -18,8 +18,11 @@
 //! Integration tests for ed25519
 
 use sp_api::ProvideRuntimeApi;
-use sp_application_crypto::ed25519::{AppPair, AppPublic};
-use sp_core::{crypto::Pair, testing::ED25519};
+use sp_application_crypto::ed25519::AppPair;
+use sp_core::{
+	crypto::{ByteArray, Pair},
+	testing::ED25519,
+};
 use sp_keystore::{testing::MemoryKeystore, Keystore};
 use std::sync::Arc;
 use substrate_test_runtime_client::{
@@ -36,6 +39,6 @@ fn ed25519_works_in_runtime() {
 		.expect("Tests `ed25519` crypto.");
 
 	let supported_keys = keystore.keys(ED25519).unwrap();
-	assert!(supported_keys.contains(&public.clone().into()));
-	assert!(AppPair::verify(&signature, "ed25519", &AppPublic::from(public)));
+	assert!(supported_keys.contains(&public.to_raw_vec()));
+	assert!(AppPair::verify(&signature, "ed25519", &public));
 }
diff --git a/primitives/application-crypto/test/src/sr25519.rs b/primitives/application-crypto/test/src/sr25519.rs
index 92886b80cbfb3..736521d7d9f3a 100644
--- a/primitives/application-crypto/test/src/sr25519.rs
+++ b/primitives/application-crypto/test/src/sr25519.rs
@@ -18,8 +18,11 @@
 //! Integration tests for sr25519
 
 use sp_api::ProvideRuntimeApi;
-use sp_application_crypto::sr25519::{AppPair, AppPublic};
-use sp_core::{crypto::Pair, testing::SR25519};
+use sp_application_crypto::sr25519::AppPair;
+use sp_core::{
+	crypto::{ByteArray, Pair},
+	testing::SR25519,
+};
 use sp_keystore::{testing::MemoryKeystore, Keystore};
 use std::sync::Arc;
 use substrate_test_runtime_client::{
@@ -36,6 +39,6 @@ fn sr25519_works_in_runtime() {
 		.expect("Tests `sr25519` crypto.");
 
 	let supported_keys = keystore.keys(SR25519).unwrap();
-	assert!(supported_keys.contains(&public.clone().into()));
-	assert!(AppPair::verify(&signature, "sr25519", &AppPublic::from(public)));
+	assert!(supported_keys.contains(&public.to_raw_vec()));
+	assert!(AppPair::verify(&signature, "sr25519", &public));
 }

From a7ad896eda0a4ac19595ce79e0fcce5232e1bebf Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Wed, 22 Mar 2023 19:40:12 +0100
Subject: [PATCH 05/15] Small refactory of local keystore implementations

---
 client/keystore/src/local.rs | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/client/keystore/src/local.rs b/client/keystore/src/local.rs
index 75d7bf931e034..160ddc66d69b7 100644
--- a/client/keystore/src/local.rs
+++ b/client/keystore/src/local.rs
@@ -105,8 +105,7 @@ impl Keystore for LocalKeystore {
 		let res = self
 			.0
 			.read()
-			.key_pair_by_type::<sr25519::Pair>(public, id)
-			.map_err(TraitError::from)?
+			.key_pair_by_type::<sr25519::Pair>(public, id)?
 			.map(|pair| pair.sign(msg));
 		Ok(res)
 	}
@@ -117,15 +116,12 @@ impl Keystore for LocalKeystore {
 		public: &Sr25519Public,
 		transcript_data: VRFTranscriptData,
 	) -> std::result::Result<Option<VRFSignature>, TraitError> {
-		let transcript = make_transcript(transcript_data);
-		let pair = self.0.read().key_pair_by_type::<Sr25519Pair>(public, key_type)?;
-
-		if let Some(pair) = pair {
+		let res = self.0.read().key_pair_by_type::<Sr25519Pair>(public, key_type)?.map(|pair| {
+			let transcript = make_transcript(transcript_data);
 			let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
-			Ok(Some(VRFSignature { output: inout.to_output(), proof }))
-		} else {
-			Ok(None)
-		}
+			VRFSignature { output: inout.to_output(), proof }
+		});
+		Ok(res)
 	}
 
 	fn ed25519_public_keys(&self, key_type: KeyTypeId) -> Vec<ed25519::Public> {
@@ -167,8 +163,7 @@ impl Keystore for LocalKeystore {
 		let res = self
 			.0
 			.read()
-			.key_pair_by_type::<ed25519::Pair>(public, id)
-			.map_err(TraitError::from)?
+			.key_pair_by_type::<ed25519::Pair>(public, id)?
 			.map(|pair| pair.sign(msg));
 		Ok(res)
 	}
@@ -212,8 +207,7 @@ impl Keystore for LocalKeystore {
 		let res = self
 			.0
 			.read()
-			.key_pair_by_type::<ecdsa::Pair>(public, id)
-			.map_err(TraitError::from)?
+			.key_pair_by_type::<ecdsa::Pair>(public, id)?
 			.map(|pair| pair.sign(msg));
 		Ok(res)
 	}
@@ -224,9 +218,12 @@ impl Keystore for LocalKeystore {
 		public: &ecdsa::Public,
 		msg: &[u8; 32],
 	) -> std::result::Result<Option<ecdsa::Signature>, TraitError> {
-		let pair = self.0.read().key_pair_by_type::<ecdsa::Pair>(public, id)?;
-
-		pair.map(|k| k.sign_prehashed(msg)).map(Ok).transpose()
+		let res = self
+			.0
+			.read()
+			.key_pair_by_type::<ecdsa::Pair>(public, id)?
+			.map(|pair| pair.sign_prehashed(msg));
+		Ok(res)
 	}
 
 	fn insert(

From f2dd7a5ddaefe6d34eb9954ad45d09960988b16f Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Thu, 23 Mar 2023 07:19:28 +0100
Subject: [PATCH 06/15] Remove 'crypto_id' method from 'Public'

---
 primitives/application-crypto/src/lib.rs    |  6 +-----
 primitives/application-crypto/src/traits.rs |  3 ++-
 primitives/core/src/crypto.rs               | 17 +++--------------
 primitives/core/src/ecdsa.rs                |  6 +-----
 primitives/core/src/ed25519.rs              |  6 +-----
 primitives/core/src/sr25519.rs              |  6 +-----
 6 files changed, 9 insertions(+), 35 deletions(-)

diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs
index 1b285c315c274..7a387277784da 100644
--- a/primitives/application-crypto/src/lib.rs
+++ b/primitives/application-crypto/src/lib.rs
@@ -301,11 +301,7 @@ macro_rules! app_crypto_public_common {
 			const LEN: usize = <$public>::LEN;
 		}
 
-		impl $crate::Public for Public {
-			fn crypto_id(&self) -> $crate::CryptoTypeId {
-				$crypto_type
-			}
-		}
+		impl $crate::Public for Public {}
 
 		impl $crate::AppPublic for Public {
 			type Generic = $public;
diff --git a/primitives/application-crypto/src/traits.rs b/primitives/application-crypto/src/traits.rs
index eb6cb19ca9146..0031637886765 100644
--- a/primitives/application-crypto/src/traits.rs
+++ b/primitives/application-crypto/src/traits.rs
@@ -80,7 +80,8 @@ pub trait AppPublic:
 pub trait AppPair: AppKey + Pair<Public = <Self as AppKey>::Public> {
 	/// The wrapped type which is just a plain instance of `Pair`.
 	type Generic: IsWrappedBy<Self>
-		+ Pair<Public = <<Self as AppKey>::Public as AppPublic>::Generic>;
+		+ Pair<Public = <<Self as AppKey>::Public as AppPublic>::Generic>
+		+ Pair<Signature = <<Self as AppKey>::Signature as AppSignature>::Generic>;
 }
 
 /// A application's signature.
diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs
index f8e986cab5a7b..2e60ac2370a16 100644
--- a/primitives/core/src/crypto.rs
+++ b/primitives/core/src/crypto.rs
@@ -485,10 +485,7 @@ pub trait ByteArray: AsRef<[u8]> + AsMut<[u8]> + for<'a> TryFrom<&'a [u8], Error
 }
 
 /// Trait suitable for typical cryptographic PKI key public type.
-pub trait Public: ByteArray + Derive + CryptoType + PartialEq + Eq + Clone + Send + Sync {
-	/// Return `CryptoTypeId` from public key.
-	fn crypto_id(&self) -> CryptoTypeId;
-}
+pub trait Public: ByteArray + Derive + CryptoType + PartialEq + Eq + Clone + Send + Sync {}
 
 /// An opaque 32-byte cryptographic identifier.
 #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, MaxEncodedLen, TypeInfo)]
@@ -687,11 +684,7 @@ mod dummy {
 			b""
 		}
 	}
-	impl Public for Dummy {
-		fn crypto_id(&self) -> CryptoTypeId {
-			CryptoTypeId(*b"dumm")
-		}
-	}
+	impl Public for Dummy {}
 
 	impl Pair for Dummy {
 		type Public = Dummy;
@@ -1203,11 +1196,7 @@ mod tests {
 			vec![]
 		}
 	}
-	impl Public for TestPublic {
-		fn crypto_id(&self) -> CryptoTypeId {
-			CryptoTypeId(*b"dumm")
-		}
-	}
+	impl Public for TestPublic {}
 	impl Pair for TestPair {
 		type Public = TestPublic;
 		type Seed = [u8; 8];
diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs
index 9680b2e9c3fbd..8dc4a55130fd6 100644
--- a/primitives/core/src/ecdsa.rs
+++ b/primitives/core/src/ecdsa.rs
@@ -102,11 +102,7 @@ impl ByteArray for Public {
 	const LEN: usize = 33;
 }
 
-impl TraitPublic for Public {
-	fn crypto_id(&self) -> CryptoTypeId {
-		CRYPTO_ID
-	}
-}
+impl TraitPublic for Public {}
 
 impl Derive for Public {}
 
diff --git a/primitives/core/src/ed25519.rs b/primitives/core/src/ed25519.rs
index a988ff8705abc..884d29dc122e9 100644
--- a/primitives/core/src/ed25519.rs
+++ b/primitives/core/src/ed25519.rs
@@ -353,11 +353,7 @@ impl ByteArray for Public {
 	const LEN: usize = 32;
 }
 
-impl TraitPublic for Public {
-	fn crypto_id(&self) -> CryptoTypeId {
-		CRYPTO_ID
-	}
-}
+impl TraitPublic for Public {}
 
 impl Derive for Public {}
 
diff --git a/primitives/core/src/sr25519.rs b/primitives/core/src/sr25519.rs
index b2ea44c02792a..d7c1c79c3f42b 100644
--- a/primitives/core/src/sr25519.rs
+++ b/primitives/core/src/sr25519.rs
@@ -383,11 +383,7 @@ impl ByteArray for Public {
 	const LEN: usize = 32;
 }
 
-impl TraitPublic for Public {
-	fn crypto_id(&self) -> CryptoTypeId {
-		CRYPTO_ID
-	}
-}
+impl TraitPublic for Public {}
 
 #[cfg(feature = "std")]
 impl From<MiniSecretKey> for Pair {

From c3d7ccd5d6c94d3e2204ec629fe83ff21ddd3a42 Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Thu, 23 Mar 2023 09:33:41 +0100
Subject: [PATCH 07/15] Trivial rename of 'AppKey' to 'AppCrypto'

---
 bin/node/executor/tests/submit_transaction.rs |  2 +-
 client/consensus/aura/src/lib.rs              |  6 +++---
 client/consensus/babe/rpc/src/lib.rs          |  2 +-
 client/consensus/babe/src/authorship.rs       |  2 +-
 client/consensus/babe/src/lib.rs              |  8 ++++++--
 client/consensus/grandpa/src/lib.rs           |  2 +-
 client/keystore/src/local.rs                  |  6 +++---
 primitives/application-crypto/src/lib.rs      | 10 +++++-----
 primitives/application-crypto/src/traits.rs   | 12 ++++++------
 primitives/consensus/grandpa/src/lib.rs       |  2 +-
 primitives/runtime/src/traits.rs              | 18 ++++++++++--------
 11 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/bin/node/executor/tests/submit_transaction.rs b/bin/node/executor/tests/submit_transaction.rs
index 9bff415d27e45..b260f90a87466 100644
--- a/bin/node/executor/tests/submit_transaction.rs
+++ b/bin/node/executor/tests/submit_transaction.rs
@@ -18,7 +18,7 @@
 use codec::Decode;
 use frame_system::offchain::{SendSignedTransaction, Signer, SubmitTransaction};
 use kitchensink_runtime::{Executive, Indices, Runtime, UncheckedExtrinsic};
-use sp_application_crypto::AppKey;
+use sp_application_crypto::AppCrypto;
 use sp_core::offchain::{testing::TestTransactionPoolExt, TransactionPoolExt};
 use sp_keyring::sr25519::Keyring::Alice;
 use sp_keystore::{testing::MemoryKeystore, Keystore, KeystoreExt};
diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs
index b852316944b4a..8d1d41952c0b7 100644
--- a/client/consensus/aura/src/lib.rs
+++ b/client/consensus/aura/src/lib.rs
@@ -45,7 +45,7 @@ use sc_consensus_slots::{
 };
 use sc_telemetry::TelemetryHandle;
 use sp_api::{Core, ProvideRuntimeApi};
-use sp_application_crypto::{AppKey, AppPublic};
+use sp_application_crypto::{AppCrypto, AppPublic};
 use sp_blockchain::{HeaderBackend, Result as CResult};
 use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain};
 use sp_consensus_slots::Slot;
@@ -449,8 +449,8 @@ where
 		let signature = self
 			.keystore
 			.sign_with(
-				<AuthorityId<P> as AppKey>::ID,
-				<AuthorityId<P> as AppKey>::CRYPTO_ID,
+				<AuthorityId<P> as AppCrypto>::ID,
+				<AuthorityId<P> as AppCrypto>::CRYPTO_ID,
 				&public,
 				header_hash.as_ref(),
 			)
diff --git a/client/consensus/babe/rpc/src/lib.rs b/client/consensus/babe/rpc/src/lib.rs
index 005d84938e28e..cdc8dbfd80a34 100644
--- a/client/consensus/babe/rpc/src/lib.rs
+++ b/client/consensus/babe/rpc/src/lib.rs
@@ -30,7 +30,7 @@ use sc_consensus_epochs::{descendent_query, Epoch as EpochT, SharedEpochChanges}
 use sc_rpc_api::DenyUnsafe;
 use serde::{Deserialize, Serialize};
 use sp_api::ProvideRuntimeApi;
-use sp_application_crypto::AppKey;
+use sp_application_crypto::AppCrypto;
 use sp_blockchain::{Error as BlockChainError, HeaderBackend, HeaderMetadata};
 use sp_consensus::{Error as ConsensusError, SelectChain};
 use sp_consensus_babe::{
diff --git a/client/consensus/babe/src/authorship.rs b/client/consensus/babe/src/authorship.rs
index c4b43b0d074b9..3ff4c393ffd67 100644
--- a/client/consensus/babe/src/authorship.rs
+++ b/client/consensus/babe/src/authorship.rs
@@ -22,7 +22,7 @@ use super::Epoch;
 use codec::Encode;
 use sc_consensus_epochs::Epoch as EpochT;
 use schnorrkel::{keys::PublicKey, vrf::VRFInOut};
-use sp_application_crypto::AppKey;
+use sp_application_crypto::AppCrypto;
 use sp_consensus_babe::{
 	digests::{PreDigest, PrimaryPreDigest, SecondaryPlainPreDigest, SecondaryVRFPreDigest},
 	make_transcript, make_transcript_data, AuthorityId, BabeAuthorityWeight, Slot, BABE_VRF_PREFIX,
diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs
index 2bf803b804349..db6e5e3ecdd42 100644
--- a/client/consensus/babe/src/lib.rs
+++ b/client/consensus/babe/src/lib.rs
@@ -108,7 +108,7 @@ use sc_consensus_slots::{
 };
 use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_TRACE};
 use sp_api::{ApiExt, ProvideRuntimeApi};
-use sp_application_crypto::AppKey;
+use sp_application_crypto::AppCrypto;
 use sp_block_builder::BlockBuilder as BlockBuilderApi;
 use sp_blockchain::{
 	Backend as _, BlockStatus, Error as ClientError, ForkBackend, HeaderBackend, HeaderMetadata,
@@ -835,7 +835,11 @@ where
 	> {
 		let signature = self
 			.keystore
-			.sr25519_sign(<AuthorityId as AppKey>::ID, public.as_inner_ref(), header_hash.as_ref())
+			.sr25519_sign(
+				<AuthorityId as AppCrypto>::ID,
+				public.as_inner_ref(),
+				header_hash.as_ref(),
+			)
 			.map_err(|e| ConsensusError::CannotSign(public.to_raw_vec(), e.to_string()))?
 			.ok_or_else(|| {
 				ConsensusError::CannotSign(
diff --git a/client/consensus/grandpa/src/lib.rs b/client/consensus/grandpa/src/lib.rs
index cf98e17920d5d..90cb8a51fa6f4 100644
--- a/client/consensus/grandpa/src/lib.rs
+++ b/client/consensus/grandpa/src/lib.rs
@@ -72,7 +72,7 @@ use sc_network::types::ProtocolName;
 use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO};
 use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver};
 use sp_api::ProvideRuntimeApi;
-use sp_application_crypto::AppKey;
+use sp_application_crypto::AppCrypto;
 use sp_blockchain::{Error as ClientError, HeaderBackend, HeaderMetadata, Result as ClientResult};
 use sp_consensus::SelectChain;
 use sp_consensus_grandpa::{
diff --git a/client/keystore/src/local.rs b/client/keystore/src/local.rs
index 160ddc66d69b7..372b680c7a8d4 100644
--- a/client/keystore/src/local.rs
+++ b/client/keystore/src/local.rs
@@ -18,7 +18,7 @@
 //! Local keystore implementation
 
 use parking_lot::RwLock;
-use sp_application_crypto::{ecdsa, ed25519, sr25519, AppKey, AppPair, IsWrappedBy};
+use sp_application_crypto::{ecdsa, ed25519, sr25519, AppCrypto, AppPair, IsWrappedBy};
 use sp_core::{
 	crypto::{ByteArray, ExposeSecret, KeyTypeId, Pair as PairT, SecretString},
 	sr25519::{Pair as Sr25519Pair, Public as Sr25519Public},
@@ -59,7 +59,7 @@ impl LocalKeystore {
 	/// `Err(_)` when something failed.
 	pub fn key_pair<Pair: AppPair>(
 		&self,
-		public: &<Pair as AppKey>::Public,
+		public: &<Pair as AppCrypto>::Public,
 	) -> Result<Option<Pair>> {
 		self.0.read().key_pair::<Pair>(public)
 	}
@@ -445,7 +445,7 @@ impl KeystoreInner {
 	/// when something failed.
 	pub fn key_pair<Pair: AppPair>(
 		&self,
-		public: &<Pair as AppKey>::Public,
+		public: &<Pair as AppCrypto>::Public,
 	) -> Result<Option<Pair>> {
 		self.key_pair_by_type::<Pair::Generic>(IsWrappedBy::from_ref(public), Pair::ID)
 			.map(|v| v.map(Into::into))
diff --git a/primitives/application-crypto/src/lib.rs b/primitives/application-crypto/src/lib.rs
index 7a387277784da..bf738813333be 100644
--- a/primitives/application-crypto/src/lib.rs
+++ b/primitives/application-crypto/src/lib.rs
@@ -167,7 +167,7 @@ macro_rules! app_crypto_pair {
 			}
 		}
 
-		impl $crate::AppKey for Pair {
+		impl $crate::AppCrypto for Pair {
 			type Public = Public;
 			type Pair = Pair;
 			type Signature = Signature;
@@ -234,7 +234,7 @@ macro_rules! app_crypto_public_full_crypto {
 			type Pair = Pair;
 		}
 
-		impl $crate::AppKey for Public {
+		impl $crate::AppCrypto for Public {
 			type Public = Public;
 			type Pair = Pair;
 			type Signature = Signature;
@@ -267,7 +267,7 @@ macro_rules! app_crypto_public_not_full_crypto {
 
 		impl $crate::CryptoType for Public {}
 
-		impl $crate::AppKey for Public {
+		impl $crate::AppCrypto for Public {
 			type Public = Public;
 			type Signature = Signature;
 			const ID: $crate::KeyTypeId = $key_type;
@@ -429,7 +429,7 @@ macro_rules! app_crypto_signature_full_crypto {
 			type Pair = Pair;
 		}
 
-		impl $crate::AppKey for Signature {
+		impl $crate::AppCrypto for Signature {
 			type Public = Public;
 			type Pair = Pair;
 			type Signature = Signature;
@@ -460,7 +460,7 @@ macro_rules! app_crypto_signature_not_full_crypto {
 
 		impl $crate::CryptoType for Signature {}
 
-		impl $crate::AppKey for Signature {
+		impl $crate::AppCrypto for Signature {
 			type Public = Public;
 			type Signature = Signature;
 			const ID: $crate::KeyTypeId = $key_type;
diff --git a/primitives/application-crypto/src/traits.rs b/primitives/application-crypto/src/traits.rs
index 0031637886765..f672efb8a890a 100644
--- a/primitives/application-crypto/src/traits.rs
+++ b/primitives/application-crypto/src/traits.rs
@@ -23,7 +23,7 @@ use sp_core::crypto::{CryptoType, CryptoTypeId, IsWrappedBy, KeyTypeId, Public};
 use sp_std::{fmt::Debug, vec::Vec};
 
 /// An application-specific key.
-pub trait AppKey: 'static + Send + Sync + Sized + CryptoType + Clone {
+pub trait AppCrypto: 'static + Send + Sync + Sized + CryptoType + Clone {
 	/// Identifier for application-specific key type.
 	const ID: KeyTypeId;
 
@@ -61,7 +61,7 @@ impl<T: sp_std::hash::Hash> MaybeDebugHash for T {}
 
 /// A application's public key.
 pub trait AppPublic:
-	AppKey + Public + Ord + PartialOrd + Eq + PartialEq + Debug + MaybeHash + codec::Codec
+	AppCrypto + Public + Ord + PartialOrd + Eq + PartialEq + Debug + MaybeHash + codec::Codec
 {
 	/// The wrapped type which is just a plain instance of `Public`.
 	type Generic: IsWrappedBy<Self>
@@ -77,15 +77,15 @@ pub trait AppPublic:
 
 /// A application's key pair.
 #[cfg(feature = "full_crypto")]
-pub trait AppPair: AppKey + Pair<Public = <Self as AppKey>::Public> {
+pub trait AppPair: AppCrypto + Pair<Public = <Self as AppCrypto>::Public> {
 	/// The wrapped type which is just a plain instance of `Pair`.
 	type Generic: IsWrappedBy<Self>
-		+ Pair<Public = <<Self as AppKey>::Public as AppPublic>::Generic>
-		+ Pair<Signature = <<Self as AppKey>::Signature as AppSignature>::Generic>;
+		+ Pair<Public = <<Self as AppCrypto>::Public as AppPublic>::Generic>
+		+ Pair<Signature = <<Self as AppCrypto>::Signature as AppSignature>::Generic>;
 }
 
 /// A application's signature.
-pub trait AppSignature: AppKey + Eq + PartialEq + Debug + MaybeHash {
+pub trait AppSignature: AppCrypto + Eq + PartialEq + Debug + MaybeHash {
 	/// The wrapped type which is just a plain instance of `Signature`.
 	type Generic: IsWrappedBy<Self> + Eq + PartialEq + Debug + MaybeHash;
 }
diff --git a/primitives/consensus/grandpa/src/lib.rs b/primitives/consensus/grandpa/src/lib.rs
index 16a041d414356..347f968d6848a 100644
--- a/primitives/consensus/grandpa/src/lib.rs
+++ b/primitives/consensus/grandpa/src/lib.rs
@@ -451,7 +451,7 @@ where
 	H: Encode,
 	N: Encode,
 {
-	use sp_application_crypto::AppKey;
+	use sp_application_crypto::AppCrypto;
 	use sp_core::crypto::Wraps;
 
 	let encoded = localized_payload(round, set_id, &message);
diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs
index 4bba6f6bc1283..4872e6314700a 100644
--- a/primitives/runtime/src/traits.rs
+++ b/primitives/runtime/src/traits.rs
@@ -30,7 +30,7 @@ use crate::{
 use impl_trait_for_tuples::impl_for_tuples;
 #[cfg(feature = "std")]
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
-use sp_application_crypto::AppKey;
+use sp_application_crypto::{AppCrypto, AppPublic};
 pub use sp_arithmetic::traits::{
 	checked_pow, ensure_pow, AtLeast32Bit, AtLeast32BitUnsigned, Bounded, CheckedAdd, CheckedDiv,
 	CheckedMul, CheckedShl, CheckedShr, CheckedSub, Ensure, EnsureAdd, EnsureAddAssign, EnsureDiv,
@@ -148,10 +148,10 @@ pub trait AppVerify {
 }
 
 impl<
-		S: Verify<Signer = <<T as AppKey>::Public as sp_application_crypto::AppPublic>::Generic>
+		S: Verify<Signer = <<T as AppCrypto>::Public as sp_application_crypto::AppPublic>::Generic>
 			+ From<T>,
 		T: sp_application_crypto::Wraps<Inner = S>
-			+ sp_application_crypto::AppKey
+			+ sp_application_crypto::AppCrypto
 			+ sp_application_crypto::AppSignature
 			+ AsRef<S>
 			+ AsMut<S>
@@ -159,16 +159,18 @@ impl<
 	> AppVerify for T
 where
 	<S as Verify>::Signer: IdentifyAccount<AccountId = <S as Verify>::Signer>,
-	<<T as AppKey>::Public as sp_application_crypto::AppPublic>::Generic: IdentifyAccount<
-		AccountId = <<T as AppKey>::Public as sp_application_crypto::AppPublic>::Generic,
+	<<T as AppCrypto>::Public as sp_application_crypto::AppPublic>::Generic: IdentifyAccount<
+		AccountId = <<T as AppCrypto>::Public as sp_application_crypto::AppPublic>::Generic,
 	>,
 {
-	type AccountId = <T as AppKey>::Public;
-	fn verify<L: Lazy<[u8]>>(&self, msg: L, signer: &<T as AppKey>::Public) -> bool {
+	type AccountId = <T as AppCrypto>::Public;
+	fn verify<L: Lazy<[u8]>>(&self, msg: L, signer: &<T as AppCrypto>::Public) -> bool {
 		use sp_application_crypto::IsWrappedBy;
 		let inner: &S = self.as_ref();
 		let inner_pubkey =
-			<<T as AppKey>::Public as sp_application_crypto::AppPublic>::Generic::from_ref(signer);
+			<<T as AppCrypto>::Public as sp_application_crypto::AppPublic>::Generic::from_ref(
+				signer,
+			);
 		Verify::verify(inner, msg, inner_pubkey)
 	}
 }

From e0c445c41cfccc55a263c8f08402018f58e52d9b Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Thu, 23 Mar 2023 12:36:48 +0100
Subject: [PATCH 08/15] Remove unused import

---
 primitives/runtime/src/traits.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs
index 4872e6314700a..95f977077e704 100644
--- a/primitives/runtime/src/traits.rs
+++ b/primitives/runtime/src/traits.rs
@@ -30,7 +30,7 @@ use crate::{
 use impl_trait_for_tuples::impl_for_tuples;
 #[cfg(feature = "std")]
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
-use sp_application_crypto::{AppCrypto, AppPublic};
+use sp_application_crypto::AppCrypto;
 pub use sp_arithmetic::traits::{
 	checked_pow, ensure_pow, AtLeast32Bit, AtLeast32BitUnsigned, Bounded, CheckedAdd, CheckedDiv,
 	CheckedMul, CheckedShl, CheckedShr, CheckedSub, Ensure, EnsureAdd, EnsureAddAssign, EnsureDiv,

From 35d0680a18058a145f97c1e14de0da3b7eb84788 Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Thu, 23 Mar 2023 17:14:07 +0100
Subject: [PATCH 09/15] Improve docs

---
 client/keystore/src/local.rs       |  52 +++++++-------
 primitives/keystore/src/lib.rs     | 112 +++++++++++++++--------------
 primitives/keystore/src/testing.rs |  80 +++++++++++----------
 3 files changed, 132 insertions(+), 112 deletions(-)

diff --git a/client/keystore/src/local.rs b/client/keystore/src/local.rs
index 372b680c7a8d4..6f8d5d64d158c 100644
--- a/client/keystore/src/local.rs
+++ b/client/keystore/src/local.rs
@@ -83,13 +83,15 @@ impl Keystore for LocalKeystore {
 	/// If the `[seed]` is `Some` then the key will be ephemeral and stored in memory.
 	fn sr25519_generate_new(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		seed: Option<&str>,
 	) -> std::result::Result<sr25519::Public, TraitError> {
 		let pair = match seed {
-			Some(seed) =>
-				self.0.write().insert_ephemeral_from_seed_by_type::<sr25519::Pair>(seed, id),
-			None => self.0.write().generate_by_type::<sr25519::Pair>(id),
+			Some(seed) => self
+				.0
+				.write()
+				.insert_ephemeral_from_seed_by_type::<sr25519::Pair>(seed, key_type),
+			None => self.0.write().generate_by_type::<sr25519::Pair>(key_type),
 		}
 		.map_err(|e| -> TraitError { e.into() })?;
 
@@ -98,14 +100,14 @@ impl Keystore for LocalKeystore {
 
 	fn sr25519_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &sr25519::Public,
 		msg: &[u8],
 	) -> std::result::Result<Option<sr25519::Signature>, TraitError> {
 		let res = self
 			.0
 			.read()
-			.key_pair_by_type::<sr25519::Pair>(public, id)?
+			.key_pair_by_type::<sr25519::Pair>(public, key_type)?
 			.map(|pair| pair.sign(msg));
 		Ok(res)
 	}
@@ -141,13 +143,15 @@ impl Keystore for LocalKeystore {
 	/// If the `[seed]` is `Some` then the key will be ephemeral and stored in memory.
 	fn ed25519_generate_new(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		seed: Option<&str>,
 	) -> std::result::Result<ed25519::Public, TraitError> {
 		let pair = match seed {
-			Some(seed) =>
-				self.0.write().insert_ephemeral_from_seed_by_type::<ed25519::Pair>(seed, id),
-			None => self.0.write().generate_by_type::<ed25519::Pair>(id),
+			Some(seed) => self
+				.0
+				.write()
+				.insert_ephemeral_from_seed_by_type::<ed25519::Pair>(seed, key_type),
+			None => self.0.write().generate_by_type::<ed25519::Pair>(key_type),
 		}
 		.map_err(|e| -> TraitError { e.into() })?;
 
@@ -156,14 +160,14 @@ impl Keystore for LocalKeystore {
 
 	fn ed25519_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ed25519::Public,
 		msg: &[u8],
 	) -> std::result::Result<Option<ed25519::Signature>, TraitError> {
 		let res = self
 			.0
 			.read()
-			.key_pair_by_type::<ed25519::Pair>(public, id)?
+			.key_pair_by_type::<ed25519::Pair>(public, key_type)?
 			.map(|pair| pair.sign(msg));
 		Ok(res)
 	}
@@ -185,13 +189,13 @@ impl Keystore for LocalKeystore {
 	/// If the `[seed]` is `Some` then the key will be ephemeral and stored in memory.
 	fn ecdsa_generate_new(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		seed: Option<&str>,
 	) -> std::result::Result<ecdsa::Public, TraitError> {
 		let pair = match seed {
 			Some(seed) =>
-				self.0.write().insert_ephemeral_from_seed_by_type::<ecdsa::Pair>(seed, id),
-			None => self.0.write().generate_by_type::<ecdsa::Pair>(id),
+				self.0.write().insert_ephemeral_from_seed_by_type::<ecdsa::Pair>(seed, key_type),
+			None => self.0.write().generate_by_type::<ecdsa::Pair>(key_type),
 		}
 		.map_err(|e| -> TraitError { e.into() })?;
 
@@ -200,28 +204,28 @@ impl Keystore for LocalKeystore {
 
 	fn ecdsa_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ecdsa::Public,
 		msg: &[u8],
 	) -> std::result::Result<Option<ecdsa::Signature>, TraitError> {
 		let res = self
 			.0
 			.read()
-			.key_pair_by_type::<ecdsa::Pair>(public, id)?
+			.key_pair_by_type::<ecdsa::Pair>(public, key_type)?
 			.map(|pair| pair.sign(msg));
 		Ok(res)
 	}
 
 	fn ecdsa_sign_prehashed(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ecdsa::Public,
 		msg: &[u8; 32],
 	) -> std::result::Result<Option<ecdsa::Signature>, TraitError> {
 		let res = self
 			.0
 			.read()
-			.key_pair_by_type::<ecdsa::Pair>(public, id)?
+			.key_pair_by_type::<ecdsa::Pair>(public, key_type)?
 			.map(|pair| pair.sign_prehashed(msg));
 		Ok(res)
 	}
@@ -235,8 +239,8 @@ impl Keystore for LocalKeystore {
 		self.0.write().insert(key_type, suri, public).map_err(|_| ())
 	}
 
-	fn keys(&self, id: KeyTypeId) -> std::result::Result<Vec<Vec<u8>>, TraitError> {
-		self.0.read().raw_public_keys(id).map_err(|e| e.into())
+	fn keys(&self, key_type: KeyTypeId) -> std::result::Result<Vec<Vec<u8>>, TraitError> {
+		self.0.read().raw_public_keys(key_type).map_err(|e| e.into())
 	}
 
 	fn has_keys(&self, public_keys: &[(Vec<u8>, KeyTypeId)]) -> bool {
@@ -407,12 +411,12 @@ impl KeystoreInner {
 	}
 
 	/// Returns a list of raw public keys filtered by `KeyTypeId`
-	fn raw_public_keys(&self, id: KeyTypeId) -> Result<Vec<Vec<u8>>> {
+	fn raw_public_keys(&self, key_type: KeyTypeId) -> Result<Vec<Vec<u8>>> {
 		let mut public_keys: Vec<Vec<u8>> = self
 			.additional
 			.keys()
 			.into_iter()
-			.filter_map(|k| if k.0 == id { Some(k.1.clone()) } else { None })
+			.filter_map(|k| if k.0 == key_type { Some(k.1.clone()) } else { None })
 			.collect();
 
 		if let Some(path) = &self.path {
@@ -424,7 +428,7 @@ impl KeystoreInner {
 				if let Some(name) = path.file_name().and_then(|n| n.to_str()) {
 					match array_bytes::hex2bytes(name) {
 						Ok(ref hex) if hex.len() > 4 => {
-							if hex[0..4] != id.0 {
+							if hex[0..4] != key_type.0 {
 								continue
 							}
 							let public = hex[4..].to_vec();
diff --git a/primitives/keystore/src/lib.rs b/primitives/keystore/src/lib.rs
index 2b4322a426b93..577969cdfecd8 100644
--- a/primitives/keystore/src/lib.rs
+++ b/primitives/keystore/src/lib.rs
@@ -45,40 +45,44 @@ pub enum Error {
 
 /// Something that generates, stores and provides access to secret keys.
 pub trait Keystore: Send + Sync {
-	/// Returns all sr25519 public keys for the given key type.
-	fn sr25519_public_keys(&self, id: KeyTypeId) -> Vec<sr25519::Public>;
+	/// Returns all the sr25519 public keys for the given key type.
+	fn sr25519_public_keys(&self, key_type: KeyTypeId) -> Vec<sr25519::Public>;
 
 	/// Generate a new sr25519 key pair for the given key type and an optional seed.
 	///
-	/// If the given seed is `Some(_)`, the key pair will only be stored in memory.
-	///
-	/// Returns the public key of the generated key pair.
+	/// Returns an `sr25519::Public` key of the generated key pair or an `Err` if
+	/// something failed during key generation.
 	fn sr25519_generate_new(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		seed: Option<&str>,
 	) -> Result<sr25519::Public, Error>;
 
-	/// TODO
+	/// Generate an sr25519 signature for a given message.
+	///
+	/// Receives [`KeyTypeId`] and an [`sr25519::Public`] key to be able to map
+	/// them to a private key that exists in the keystore.
+	///
+	/// Returns an [`sr25519::Signature`] or `None` in case the given `key_type`
+	/// and `public` combination doesn't exist in the keystore.
+	/// An `Err` will be returned if generating the signature itself failed.
 	fn sr25519_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &sr25519::Public,
 		msg: &[u8],
 	) -> Result<Option<sr25519::Signature>, Error>;
 
-	/// Generate VRF signature  for given transcript data.
+	/// Generate an sr25519 VRF signature for a given transcript data.
 	///
-	/// Receives KeyTypeId and Public key to be able to map
-	/// them to a private key that exists in the keystore which
-	/// is, in turn, used for signing the provided transcript.
+	/// Receives [`KeyTypeId`] and an [`sr25519::Public`] key to be able to map
+	/// them to a private key that exists in the keystore.
 	///
 	/// Returns a result containing the signature data.
-	/// Namely, VRFOutput and VRFProof which are returned
-	/// inside the `VRFSignature` container struct.
-	///
-	/// This function will return `None` if the given `key_type` and `public` combination
-	/// doesn't exist in the keystore or an `Err` when something failed.
+	/// Namely, VRFOutput and VRFProof which are returned inside the `VRFSignature`
+	/// container struct.
+	/// Returns `None` if the given `key_type` and `public` combination doesn't
+	/// exist in the keystore or an `Err` when something failed.
 	fn sr25519_vrf_sign(
 		&self,
 		key_type: KeyTypeId,
@@ -87,69 +91,72 @@ pub trait Keystore: Send + Sync {
 	) -> Result<Option<VRFSignature>, Error>;
 
 	/// Returns all ed25519 public keys for the given key type.
-	fn ed25519_public_keys(&self, id: KeyTypeId) -> Vec<ed25519::Public>;
+	fn ed25519_public_keys(&self, key_type: KeyTypeId) -> Vec<ed25519::Public>;
 
 	/// Generate a new ed25519 key pair for the given key type and an optional seed.
 	///
-	/// If the given seed is `Some(_)`, the key pair will only be stored in memory.
-	///
-	/// Returns the public key of the generated key pair.
+	/// Returns an `ed25519::Public` key of the generated key pair or an `Err` if
+	/// something failed during key generation.
 	fn ed25519_generate_new(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		seed: Option<&str>,
 	) -> Result<ed25519::Public, Error>;
 
-	/// TODO
+	/// Generate an ed25519 signature for a given message.
+	///
+	/// Receives [`KeyTypeId`] and an [`ed25519::Public`] key to be able to map
+	/// them to a private key that exists in the keystore.
+	///
+	/// Returns an [`ed25519::Signature`] or `None` in case the given `key_type`
+	/// and `public` combination doesn't exist in the keystore.
+	/// An `Err` will be returned if generating the signature itself failed.
 	fn ed25519_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ed25519::Public,
 		msg: &[u8],
 	) -> Result<Option<ed25519::Signature>, Error>;
 
 	/// Returns all ecdsa public keys for the given key type.
-	fn ecdsa_public_keys(&self, id: KeyTypeId) -> Vec<ecdsa::Public>;
+	fn ecdsa_public_keys(&self, key_type: KeyTypeId) -> Vec<ecdsa::Public>;
 
 	/// Generate a new ecdsa key pair for the given key type and an optional seed.
 	///
-	/// If the given seed is `Some(_)`, the key pair will only be stored in memory.
-	///
-	/// Returns the public key of the generated key pair.
-	fn ecdsa_generate_new(&self, id: KeyTypeId, seed: Option<&str>)
-		-> Result<ecdsa::Public, Error>;
+	/// Returns an `ecdsa::Public` key of the generated key pair or an `Err` if
+	/// something failed during key generation.
+	fn ecdsa_generate_new(
+		&self,
+		key_type: KeyTypeId,
+		seed: Option<&str>,
+	) -> Result<ecdsa::Public, Error>;
 
-	/// Generate an ECDSA signature for a given message.
+	/// Generate an ecdsa signature for a given message.
 	///
 	/// Receives [`KeyTypeId`] and an [`ecdsa::Public`] key to be able to map
-	/// them to a private key that exists in the keystore. This private key is,
-	/// in turn, used for signing the provided pre-hashed message.
+	/// them to a private key that exists in the keystore.
 	///
-	/// Returns an [`ecdsa::Signature`] or `None` in case the given `id` and
-	/// `public` combination doesn't exist in the keystore. An `Err` will be
-	/// returned if generating the signature itself failed.
+	/// Returns an [`ecdsa::Signature`] or `None` in case the given `key_type`
+	/// and `public` combination doesn't exist in the keystore.
+	/// An `Err` will be returned if generating the signature itself failed.
 	fn ecdsa_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ecdsa::Public,
 		msg: &[u8],
 	) -> Result<Option<ecdsa::Signature>, Error>;
 
-	/// Generate an ECDSA signature for a given pre-hashed message.
+	/// Generate an ecdsa signature for a given pre-hashed message.
 	///
 	/// Receives [`KeyTypeId`] and an [`ecdsa::Public`] key to be able to map
-	/// them to a private key that exists in the keystore. This private key is,
-	/// in turn, used for signing the provided pre-hashed message.
+	/// them to a private key that exists in the keystore.
 	///
-	/// The `msg` argument provided should be a hashed message for which an
-	/// ECDSA signature should be generated.
-	///
-	/// Returns an [`ecdsa::Signature`] or `None` in case the given `id` and
-	/// `public` combination doesn't exist in the keystore. An `Err` will be
-	/// returned if generating the signature itself failed.
+	/// Returns an [`ecdsa::Signature`] or `None` in case the given `key_type`
+	/// and `public` combination doesn't exist in the keystore.
+	/// An `Err` will be returned if generating the signature itself failed.
 	fn ecdsa_sign_prehashed(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ecdsa::Public,
 		msg: &[u8; 32],
 	) -> Result<Option<ecdsa::Signature>, Error>;
@@ -157,19 +164,20 @@ pub trait Keystore: Send + Sync {
 	/// Insert a new secret key.
 	fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()>;
 
-	/// List all supported keys
+	/// List all supported keys of a given type.
 	///
-	/// Returns a set of public keys the signer supports.
-	fn keys(&self, id: KeyTypeId) -> Result<Vec<Vec<u8>>, Error>;
+	/// Returns a set of public keys the signer supports in raw format.
+	fn keys(&self, key_type: KeyTypeId) -> Result<Vec<Vec<u8>>, Error>;
 
 	/// Checks if the private keys for the given public key and key type combinations exist.
 	///
 	/// Returns `true` iff all private keys could be found.
 	fn has_keys(&self, public_keys: &[(Vec<u8>, KeyTypeId)]) -> bool;
 
-	/// Convenience method to sign a message using an opaque key type.
+	/// Convenience method to sign a message using the given key type and a raw public key
+	/// for secret lookup.
 	///
-	/// The message is signed using the cryptographic primitive specified by `KeyCryptoId`.
+	/// The message is signed using the cryptographic primitive specified by `crypto_id`.
 	///
 	/// Schemes supported by the default trait implementation: sr25519, ed25519 and ecdsa.
 	/// To support more schemes you can overwrite this method.
diff --git a/primitives/keystore/src/testing.rs b/primitives/keystore/src/testing.rs
index 7feec2faed492..de034c65889ff 100644
--- a/primitives/keystore/src/testing.rs
+++ b/primitives/keystore/src/testing.rs
@@ -42,36 +42,44 @@ impl MemoryKeystore {
 		Self::default()
 	}
 
-	fn sr25519_key_pair(&self, id: KeyTypeId, pub_key: &sr25519::Public) -> Option<sr25519::Pair> {
-		self.keys.read().get(&id).and_then(|inner| {
-			inner.get(pub_key.as_slice()).map(|s| {
+	fn sr25519_key_pair(
+		&self,
+		key_type: KeyTypeId,
+		public: &sr25519::Public,
+	) -> Option<sr25519::Pair> {
+		self.keys.read().get(&key_type).and_then(|inner| {
+			inner.get(public.as_slice()).map(|s| {
 				sr25519::Pair::from_string(s, None).expect("`sr25519` seed slice is valid")
 			})
 		})
 	}
 
-	fn ed25519_key_pair(&self, id: KeyTypeId, pub_key: &ed25519::Public) -> Option<ed25519::Pair> {
-		self.keys.read().get(&id).and_then(|inner| {
-			inner.get(pub_key.as_slice()).map(|s| {
+	fn ed25519_key_pair(
+		&self,
+		key_type: KeyTypeId,
+		public: &ed25519::Public,
+	) -> Option<ed25519::Pair> {
+		self.keys.read().get(&key_type).and_then(|inner| {
+			inner.get(public.as_slice()).map(|s| {
 				ed25519::Pair::from_string(s, None).expect("`ed25519` seed slice is valid")
 			})
 		})
 	}
 
-	fn ecdsa_key_pair(&self, id: KeyTypeId, pub_key: &ecdsa::Public) -> Option<ecdsa::Pair> {
-		self.keys.read().get(&id).and_then(|inner| {
+	fn ecdsa_key_pair(&self, key_type: KeyTypeId, public: &ecdsa::Public) -> Option<ecdsa::Pair> {
+		self.keys.read().get(&key_type).and_then(|inner| {
 			inner
-				.get(pub_key.as_slice())
+				.get(public.as_slice())
 				.map(|s| ecdsa::Pair::from_string(s, None).expect("`ecdsa` seed slice is valid"))
 		})
 	}
 }
 
 impl Keystore for MemoryKeystore {
-	fn sr25519_public_keys(&self, id: KeyTypeId) -> Vec<sr25519::Public> {
+	fn sr25519_public_keys(&self, key_type: KeyTypeId) -> Vec<sr25519::Public> {
 		self.keys
 			.read()
-			.get(&id)
+			.get(&key_type)
 			.map(|keys| {
 				keys.values()
 					.map(|s| {
@@ -85,7 +93,7 @@ impl Keystore for MemoryKeystore {
 
 	fn sr25519_generate_new(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		seed: Option<&str>,
 	) -> Result<sr25519::Public, Error> {
 		match seed {
@@ -95,7 +103,7 @@ impl Keystore for MemoryKeystore {
 				})?;
 				self.keys
 					.write()
-					.entry(id)
+					.entry(key_type)
 					.or_default()
 					.insert(pair.public().to_raw_vec(), seed.into());
 				Ok(pair.public())
@@ -104,7 +112,7 @@ impl Keystore for MemoryKeystore {
 				let (pair, phrase, _) = sr25519::Pair::generate_with_phrase(None);
 				self.keys
 					.write()
-					.entry(id)
+					.entry(key_type)
 					.or_default()
 					.insert(pair.public().to_raw_vec(), phrase);
 				Ok(pair.public())
@@ -114,11 +122,11 @@ impl Keystore for MemoryKeystore {
 
 	fn sr25519_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &sr25519::Public,
 		msg: &[u8],
 	) -> Result<Option<sr25519::Signature>, Error> {
-		Ok(self.sr25519_key_pair(id, public).map(|pair| pair.sign(msg)))
+		Ok(self.sr25519_key_pair(key_type, public).map(|pair| pair.sign(msg)))
 	}
 
 	fn sr25519_vrf_sign(
@@ -135,10 +143,10 @@ impl Keystore for MemoryKeystore {
 		Ok(Some(VRFSignature { output: inout.to_output(), proof }))
 	}
 
-	fn ed25519_public_keys(&self, id: KeyTypeId) -> Vec<ed25519::Public> {
+	fn ed25519_public_keys(&self, key_type: KeyTypeId) -> Vec<ed25519::Public> {
 		self.keys
 			.read()
-			.get(&id)
+			.get(&key_type)
 			.map(|keys| {
 				keys.values()
 					.map(|s| {
@@ -152,7 +160,7 @@ impl Keystore for MemoryKeystore {
 
 	fn ed25519_generate_new(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		seed: Option<&str>,
 	) -> Result<ed25519::Public, Error> {
 		match seed {
@@ -162,7 +170,7 @@ impl Keystore for MemoryKeystore {
 				})?;
 				self.keys
 					.write()
-					.entry(id)
+					.entry(key_type)
 					.or_default()
 					.insert(pair.public().to_raw_vec(), seed.into());
 				Ok(pair.public())
@@ -171,7 +179,7 @@ impl Keystore for MemoryKeystore {
 				let (pair, phrase, _) = ed25519::Pair::generate_with_phrase(None);
 				self.keys
 					.write()
-					.entry(id)
+					.entry(key_type)
 					.or_default()
 					.insert(pair.public().to_raw_vec(), phrase);
 				Ok(pair.public())
@@ -181,17 +189,17 @@ impl Keystore for MemoryKeystore {
 
 	fn ed25519_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ed25519::Public,
 		msg: &[u8],
 	) -> Result<Option<ed25519::Signature>, Error> {
-		Ok(self.ed25519_key_pair(id, public).map(|pair| pair.sign(msg)))
+		Ok(self.ed25519_key_pair(key_type, public).map(|pair| pair.sign(msg)))
 	}
 
-	fn ecdsa_public_keys(&self, id: KeyTypeId) -> Vec<ecdsa::Public> {
+	fn ecdsa_public_keys(&self, key_type: KeyTypeId) -> Vec<ecdsa::Public> {
 		self.keys
 			.read()
-			.get(&id)
+			.get(&key_type)
 			.map(|keys| {
 				keys.values()
 					.map(|s| {
@@ -205,7 +213,7 @@ impl Keystore for MemoryKeystore {
 
 	fn ecdsa_generate_new(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		seed: Option<&str>,
 	) -> Result<ecdsa::Public, Error> {
 		match seed {
@@ -214,7 +222,7 @@ impl Keystore for MemoryKeystore {
 					.map_err(|_| Error::ValidationError("Generates an `ecdsa` pair.".to_owned()))?;
 				self.keys
 					.write()
-					.entry(id)
+					.entry(key_type)
 					.or_default()
 					.insert(pair.public().to_raw_vec(), seed.into());
 				Ok(pair.public())
@@ -223,7 +231,7 @@ impl Keystore for MemoryKeystore {
 				let (pair, phrase, _) = ecdsa::Pair::generate_with_phrase(None);
 				self.keys
 					.write()
-					.entry(id)
+					.entry(key_type)
 					.or_default()
 					.insert(pair.public().to_raw_vec(), phrase);
 				Ok(pair.public())
@@ -233,37 +241,37 @@ impl Keystore for MemoryKeystore {
 
 	fn ecdsa_sign(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ecdsa::Public,
 		msg: &[u8],
 	) -> Result<Option<ecdsa::Signature>, Error> {
-		Ok(self.ecdsa_key_pair(id, public).map(|pair| pair.sign(msg)))
+		Ok(self.ecdsa_key_pair(key_type, public).map(|pair| pair.sign(msg)))
 	}
 
 	fn ecdsa_sign_prehashed(
 		&self,
-		id: KeyTypeId,
+		key_type: KeyTypeId,
 		public: &ecdsa::Public,
 		msg: &[u8; 32],
 	) -> Result<Option<ecdsa::Signature>, Error> {
-		let pair = self.ecdsa_key_pair(id, public);
+		let pair = self.ecdsa_key_pair(key_type, public);
 		pair.map(|pair| pair.sign_prehashed(msg)).map(Ok).transpose()
 	}
 
-	fn insert(&self, id: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> {
+	fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> {
 		self.keys
 			.write()
-			.entry(id)
+			.entry(key_type)
 			.or_default()
 			.insert(public.to_owned(), suri.to_string());
 		Ok(())
 	}
 
-	fn keys(&self, id: KeyTypeId) -> Result<Vec<Vec<u8>>, Error> {
+	fn keys(&self, key_type: KeyTypeId) -> Result<Vec<Vec<u8>>, Error> {
 		let keys = self
 			.keys
 			.read()
-			.get(&id)
+			.get(&key_type)
 			.map(|map| map.keys().cloned().collect())
 			.unwrap_or_default();
 		Ok(keys)

From ba729ff2da8cbafc387606a53272269b2413f4b4 Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Thu, 23 Mar 2023 17:47:30 +0100
Subject: [PATCH 10/15] Better signature related errors for authority-discovery

---
 client/authority-discovery/src/error.rs  | 8 ++++----
 client/authority-discovery/src/worker.rs | 8 +++++---
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs
index 8e3aac5a606c5..0479167159998 100644
--- a/client/authority-discovery/src/error.rs
+++ b/client/authority-discovery/src/error.rs
@@ -57,11 +57,11 @@ pub enum Error {
 	#[error("Failed to parse a libp2p key.")]
 	ParsingLibp2pIdentity(#[from] libp2p::identity::error::DecodingError),
 
-	#[error("Failed to sign using a specific public key.")]
-	MissingSignature(Vec<u8>),
+	#[error("Failed to sign network message. Reason: {0}.")]
+	CannotSignNetwork(String),
 
-	#[error("Failed to sign using all public keys.")]
-	Signing,
+	#[error("Failed to sign using key: {0:?}. Reason: {1}.")]
+	CannotSign(Vec<u8>, String),
 
 	#[error("Failed to register Prometheus metric.")]
 	Prometheus(#[from] prometheus_endpoint::PrometheusError),
diff --git a/client/authority-discovery/src/worker.rs b/client/authority-discovery/src/worker.rs
index d0c2b846e7522..3b570a13d5913 100644
--- a/client/authority-discovery/src/worker.rs
+++ b/client/authority-discovery/src/worker.rs
@@ -654,7 +654,7 @@ fn sign_record_with_peer_id(
 ) -> Result<schema::PeerSignature> {
 	let signature = network
 		.sign_with_local_identity(serialized_record)
-		.map_err(|_| Error::Signing)?;
+		.map_err(|e| Error::CannotSignNetwork(e.to_string()))?;
 	let public_key = signature.public_key.to_protobuf_encoding();
 	let signature = signature.bytes;
 	Ok(schema::PeerSignature { signature, public_key })
@@ -671,8 +671,10 @@ fn sign_record_with_authority_ids(
 	for key in keys.iter() {
 		let auth_signature = key_store
 			.sr25519_sign(key_types::AUTHORITY_DISCOVERY, key.as_inner_ref(), &serialized_record)
-			.map_err(|_| Error::Signing)?
-			.ok_or_else(|| Error::MissingSignature(key.to_raw_vec()))?;
+			.map_err(|e| Error::CannotSign(key.to_raw_vec(), e.to_string()))?
+			.ok_or_else(|| {
+				Error::CannotSign(key.to_raw_vec(), "Could not find key in keystore".into())
+			})?;
 
 		// Scale encode
 		let auth_signature = auth_signature.encode();

From 799b22590b4376271aa173534bb1559c4cec2bad Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Fri, 24 Mar 2023 10:07:22 +0100
Subject: [PATCH 11/15] Apply review suggestion

---
 client/keystore/src/local.rs | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/client/keystore/src/local.rs b/client/keystore/src/local.rs
index 6f8d5d64d158c..abdf97bb22aad 100644
--- a/client/keystore/src/local.rs
+++ b/client/keystore/src/local.rs
@@ -18,10 +18,10 @@
 //! Local keystore implementation
 
 use parking_lot::RwLock;
-use sp_application_crypto::{ecdsa, ed25519, sr25519, AppCrypto, AppPair, IsWrappedBy};
+use sp_application_crypto::{AppCrypto, AppPair, IsWrappedBy};
 use sp_core::{
-	crypto::{ByteArray, ExposeSecret, KeyTypeId, Pair as PairT, SecretString},
-	sr25519::{Pair as Sr25519Pair, Public as Sr25519Public},
+	crypto::{ByteArray, ExposeSecret, KeyTypeId, Pair as CorePair, SecretString},
+	ecdsa, ed25519, sr25519,
 };
 use sp_keystore::{
 	vrf::{make_transcript, VRFSignature, VRFTranscriptData},
@@ -115,10 +115,10 @@ impl Keystore for LocalKeystore {
 	fn sr25519_vrf_sign(
 		&self,
 		key_type: KeyTypeId,
-		public: &Sr25519Public,
+		public: &sr25519::Public,
 		transcript_data: VRFTranscriptData,
 	) -> std::result::Result<Option<VRFSignature>, TraitError> {
-		let res = self.0.read().key_pair_by_type::<Sr25519Pair>(public, key_type)?.map(|pair| {
+		let res = self.0.read().key_pair_by_type::<sr25519::Pair>(public, key_type)?.map(|pair| {
 			let transcript = make_transcript(transcript_data);
 			let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
 			VRFSignature { output: inout.to_output(), proof }
@@ -298,7 +298,12 @@ impl KeystoreInner {
 	/// Insert the given public/private key pair with the given key type.
 	///
 	/// Does not place it into the file system store.
-	fn insert_ephemeral_pair<Pair: PairT>(&mut self, pair: &Pair, seed: &str, key_type: KeyTypeId) {
+	fn insert_ephemeral_pair<Pair: CorePair>(
+		&mut self,
+		pair: &Pair,
+		seed: &str,
+		key_type: KeyTypeId,
+	) {
 		let key = (key_type, pair.public().to_raw_vec());
 		self.additional.insert(key, seed.into());
 	}
@@ -318,7 +323,7 @@ impl KeystoreInner {
 	///
 	/// Places it into the file system store, if a path is configured. Otherwise insert
 	/// it into the memory cache only.
-	fn generate_by_type<Pair: PairT>(&mut self, key_type: KeyTypeId) -> Result<Pair> {
+	fn generate_by_type<Pair: CorePair>(&mut self, key_type: KeyTypeId) -> Result<Pair> {
 		let (pair, phrase, _) = Pair::generate_with_phrase(self.password());
 		if let Some(path) = self.key_file_path(pair.public().as_slice(), key_type) {
 			Self::write_to_file(path, &phrase)?;
@@ -347,7 +352,7 @@ impl KeystoreInner {
 	/// Create a new key from seed.
 	///
 	/// Does not place it into the file system store.
-	fn insert_ephemeral_from_seed_by_type<Pair: PairT>(
+	fn insert_ephemeral_from_seed_by_type<Pair: CorePair>(
 		&mut self,
 		seed: &str,
 		key_type: KeyTypeId,
@@ -379,7 +384,7 @@ impl KeystoreInner {
 	}
 
 	/// Get a key pair for the given public key and key type.
-	fn key_pair_by_type<Pair: PairT>(
+	fn key_pair_by_type<Pair: CorePair>(
 		&self,
 		public: &Pair::Public,
 		key_type: KeyTypeId,

From b5034e62c70f66f8a3e7c4f6ea23c786fb8e33de Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Fri, 24 Mar 2023 11:03:46 +0100
Subject: [PATCH 12/15] Apply review suggestions

Co-authored-by: Koute <koute@users.noreply.github.com>
---
 client/consensus/babe/src/lib.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs
index db6e5e3ecdd42..b76cd940efa0d 100644
--- a/client/consensus/babe/src/lib.rs
+++ b/client/consensus/babe/src/lib.rs
@@ -894,7 +894,7 @@ where
 		Box::pin(
 			self.env
 				.init(block)
-				.map_err(|e| ConsensusError::ClientImport(format!("{:?}", e))),
+				.map_err(|e| ConsensusError::ClientImport(e.to_string())),
 		)
 	}
 

From e75b94c4e07906af745432bd9a790c1d20c87df6 Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Fri, 24 Mar 2023 12:03:05 +0100
Subject: [PATCH 13/15] Authority discoverty signing error revisited

---
 client/authority-discovery/src/error.rs  | 7 ++-----
 client/authority-discovery/src/worker.rs | 8 ++++----
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/client/authority-discovery/src/error.rs b/client/authority-discovery/src/error.rs
index 0479167159998..447116b0f6b56 100644
--- a/client/authority-discovery/src/error.rs
+++ b/client/authority-discovery/src/error.rs
@@ -57,11 +57,8 @@ pub enum Error {
 	#[error("Failed to parse a libp2p key.")]
 	ParsingLibp2pIdentity(#[from] libp2p::identity::error::DecodingError),
 
-	#[error("Failed to sign network message. Reason: {0}.")]
-	CannotSignNetwork(String),
-
-	#[error("Failed to sign using key: {0:?}. Reason: {1}.")]
-	CannotSign(Vec<u8>, String),
+	#[error("Failed to sign: {0}.")]
+	CannotSign(String),
 
 	#[error("Failed to register Prometheus metric.")]
 	Prometheus(#[from] prometheus_endpoint::PrometheusError),
diff --git a/client/authority-discovery/src/worker.rs b/client/authority-discovery/src/worker.rs
index 3b570a13d5913..7cc2baa08f7c8 100644
--- a/client/authority-discovery/src/worker.rs
+++ b/client/authority-discovery/src/worker.rs
@@ -43,6 +43,7 @@ use log::{debug, error, log_enabled};
 use prometheus_endpoint::{register, Counter, CounterVec, Gauge, Opts, U64};
 use prost::Message;
 use rand::{seq::SliceRandom, thread_rng};
+
 use sc_network::{
 	event::DhtEvent, KademliaKey, NetworkDHTProvider, NetworkSigner, NetworkStateInfo, Signature,
 };
@@ -51,7 +52,6 @@ use sp_authority_discovery::{
 	AuthorityDiscoveryApi, AuthorityId, AuthorityPair, AuthoritySignature,
 };
 use sp_blockchain::HeaderBackend;
-
 use sp_core::crypto::{key_types, ByteArray, Pair, Wraps};
 use sp_keystore::{Keystore, KeystorePtr};
 use sp_runtime::traits::Block as BlockT;
@@ -654,7 +654,7 @@ fn sign_record_with_peer_id(
 ) -> Result<schema::PeerSignature> {
 	let signature = network
 		.sign_with_local_identity(serialized_record)
-		.map_err(|e| Error::CannotSignNetwork(e.to_string()))?;
+		.map_err(|e| Error::CannotSign(format!("{} (network packet)", e)))?;
 	let public_key = signature.public_key.to_protobuf_encoding();
 	let signature = signature.bytes;
 	Ok(schema::PeerSignature { signature, public_key })
@@ -671,9 +671,9 @@ fn sign_record_with_authority_ids(
 	for key in keys.iter() {
 		let auth_signature = key_store
 			.sr25519_sign(key_types::AUTHORITY_DISCOVERY, key.as_inner_ref(), &serialized_record)
-			.map_err(|e| Error::CannotSign(key.to_raw_vec(), e.to_string()))?
+			.map_err(|e| Error::CannotSign(format!("{}. Key: {:?}", e, key)))?
 			.ok_or_else(|| {
-				Error::CannotSign(key.to_raw_vec(), "Could not find key in keystore".into())
+				Error::CannotSign(format!("Could not find key in keystore. Key: {:?}", key))
 			})?;
 
 		// Scale encode

From de251c559113e807cd7e987ae8d91d5d2c52a36f Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Fri, 24 Mar 2023 12:13:15 +0100
Subject: [PATCH 14/15] Signing error revisited for babe and aura as well

---
 client/consensus/aura/src/lib.rs         | 36 +++++++++++-------------
 client/consensus/babe/src/lib.rs         | 21 +++++---------
 primitives/consensus/common/src/error.rs |  4 +--
 3 files changed, 25 insertions(+), 36 deletions(-)

diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs
index 8d1d41952c0b7..aeff7de48551c 100644
--- a/client/consensus/aura/src/lib.rs
+++ b/client/consensus/aura/src/lib.rs
@@ -205,7 +205,7 @@ pub fn start_aura<P, B, C, SC, I, PF, SO, L, CIDP, BS, Error>(
 		telemetry,
 		compatibility_mode,
 	}: StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS, NumberFor<B>>,
-) -> Result<impl Future<Output = ()>, sp_consensus::Error>
+) -> Result<impl Future<Output = ()>, ConsensusError>
 where
 	P: Pair + Send + Sync,
 	P::Public: AppPublic + Hash + Member + Encode + Decode,
@@ -222,7 +222,7 @@ where
 	CIDP: CreateInherentDataProviders<B, ()> + Send + 'static,
 	CIDP::InherentDataProviders: InherentDataProviderExt + Send,
 	BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + Sync + 'static,
-	Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
+	Error: std::error::Error + Send + From<ConsensusError> + 'static,
 {
 	let worker = build_aura_worker::<P, _, _, _, _, _, _, _, _>(BuildAuraWorkerParams {
 		client,
@@ -320,7 +320,7 @@ where
 	P::Public: AppPublic + Hash + Member + Encode + Decode,
 	P::Signature: TryFrom<Vec<u8>> + Hash + Member + Encode + Decode,
 	I: BlockImport<B, Transaction = sp_api::TransactionFor<C, B>> + Send + Sync + 'static,
-	Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
+	Error: std::error::Error + Send + From<ConsensusError> + 'static,
 	SO: SyncOracle + Send + Sync + Clone,
 	L: sc_consensus::JustificationSyncLink<B>,
 	BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + Sync + 'static,
@@ -374,13 +374,13 @@ where
 	SO: SyncOracle + Send + Clone + Sync,
 	L: sc_consensus::JustificationSyncLink<B>,
 	BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + Sync + 'static,
-	Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
+	Error: std::error::Error + Send + From<ConsensusError> + 'static,
 {
 	type BlockImport = I;
 	type SyncOracle = SO;
 	type JustificationSyncLink = L;
 	type CreateProposer =
-		Pin<Box<dyn Future<Output = Result<E::Proposer, sp_consensus::Error>> + Send + 'static>>;
+		Pin<Box<dyn Future<Output = Result<E::Proposer, ConsensusError>> + Send + 'static>>;
 	type Proposer = E::Proposer;
 	type Claim = P::Public;
 	type AuxData = Vec<AuthorityId<P>>;
@@ -393,11 +393,7 @@ where
 		&mut self.block_import
 	}
 
-	fn aux_data(
-		&self,
-		header: &B::Header,
-		_slot: Slot,
-	) -> Result<Self::AuxData, sp_consensus::Error> {
+	fn aux_data(&self, header: &B::Header, _slot: Slot) -> Result<Self::AuxData, ConsensusError> {
 		authorities(
 			self.client.as_ref(),
 			header.hash(),
@@ -443,7 +439,7 @@ where
 		_authorities: Self::AuxData,
 	) -> Result<
 		sc_consensus::BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
-		sp_consensus::Error,
+		ConsensusError,
 	> {
 		let public = public.to_raw_vec();
 		let signature = self
@@ -454,17 +450,17 @@ where
 				&public,
 				header_hash.as_ref(),
 			)
-			.map_err(|e| sp_consensus::Error::CannotSign(public.clone(), e.to_string()))?
+			.map_err(|e| ConsensusError::CannotSign(format!("{}. Key: {:?}", e, public)))?
 			.ok_or_else(|| {
-				sp_consensus::Error::CannotSign(
-					public.clone(),
-					"Could not find key in keystore.".into(),
-				)
+				ConsensusError::CannotSign(format!(
+					"Could not find key in keystore. Key: {:?}",
+					public
+				))
 			})?;
 		let signature = signature
 			.clone()
 			.try_into()
-			.map_err(|_| sp_consensus::Error::InvalidSignature(signature, public))?;
+			.map_err(|_| ConsensusError::InvalidSignature(signature, public))?;
 
 		let signature_digest_item =
 			<DigestItem as CompatibleDigestItem<P::Signature>>::aura_seal(signature);
@@ -509,7 +505,7 @@ where
 	fn proposer(&mut self, block: &B::Header) -> Self::CreateProposer {
 		self.env
 			.init(block)
-			.map_err(|e| sp_consensus::Error::ClientImport(format!("{:?}", e)))
+			.map_err(|e| ConsensusError::ClientImport(format!("{:?}", e)))
 			.boxed()
 	}
 
@@ -622,14 +618,14 @@ where
 							Default::default(),
 						),
 					)
-					.map_err(|_| sp_consensus::Error::InvalidAuthoritiesSet)?;
+					.map_err(|_| ConsensusError::InvalidAuthoritiesSet)?;
 			},
 	}
 
 	runtime_api
 		.authorities(parent_hash)
 		.ok()
-		.ok_or(sp_consensus::Error::InvalidAuthoritiesSet)
+		.ok_or(ConsensusError::InvalidAuthoritiesSet)
 }
 
 #[cfg(test)]
diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs
index b76cd940efa0d..4bc6169db26e5 100644
--- a/client/consensus/babe/src/lib.rs
+++ b/client/consensus/babe/src/lib.rs
@@ -117,10 +117,7 @@ use sp_blockchain::{
 use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain};
 use sp_consensus_babe::inherents::BabeInherentData;
 use sp_consensus_slots::Slot;
-use sp_core::{
-	crypto::{ByteArray, Wraps},
-	ExecutionContext,
-};
+use sp_core::{crypto::Wraps, ExecutionContext};
 use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider};
 use sp_keystore::KeystorePtr;
 use sp_runtime::{
@@ -840,12 +837,12 @@ where
 				public.as_inner_ref(),
 				header_hash.as_ref(),
 			)
-			.map_err(|e| ConsensusError::CannotSign(public.to_raw_vec(), e.to_string()))?
+			.map_err(|e| ConsensusError::CannotSign(format!("{}. Key: {:?}", e, public)))?
 			.ok_or_else(|| {
-				ConsensusError::CannotSign(
-					public.to_raw_vec(),
-					"Could not find key in keystore.".into(),
-				)
+				ConsensusError::CannotSign(format!(
+					"Could not find key in keystore. Key: {:?}",
+					public
+				))
 			})?;
 
 		let digest_item = <DigestItem as CompatibleDigestItem>::babe_seal(signature.into());
@@ -891,11 +888,7 @@ where
 	}
 
 	fn proposer(&mut self, block: &B::Header) -> Self::CreateProposer {
-		Box::pin(
-			self.env
-				.init(block)
-				.map_err(|e| ConsensusError::ClientImport(e.to_string())),
-		)
+		Box::pin(self.env.init(block).map_err(|e| ConsensusError::ClientImport(e.to_string())))
 	}
 
 	fn telemetry(&self) -> Option<TelemetryHandle> {
diff --git a/primitives/consensus/common/src/error.rs b/primitives/consensus/common/src/error.rs
index e881259da11e4..fb8d0447fe3d6 100644
--- a/primitives/consensus/common/src/error.rs
+++ b/primitives/consensus/common/src/error.rs
@@ -48,8 +48,8 @@ pub enum Error {
 	#[error("Chain lookup failed: {0}")]
 	ChainLookup(String),
 	/// Signing failed.
-	#[error("Failed to sign using key: {0:?}. Reason: {1}")]
-	CannotSign(Vec<u8>, String),
+	#[error("Failed to sign: {0}")]
+	CannotSign(String),
 	/// Some other error.
 	#[error(transparent)]
 	Other(#[from] Box<dyn std::error::Error + Sync + Send + 'static>),

From 21920455d40e920d02bfd78eae2da808b4fd0c8e Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Fri, 24 Mar 2023 13:25:07 +0100
Subject: [PATCH 15/15] Further cleanup

---
 bin/node/cli/src/service.rs              | 8 ++------
 client/authority-discovery/src/worker.rs | 4 ++--
 client/consensus/aura/src/lib.rs         | 5 ++---
 client/consensus/babe/src/lib.rs         | 8 ++------
 primitives/consensus/grandpa/src/lib.rs  | 3 +--
 5 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs
index 79dbeeee48628..ca6830085b055 100644
--- a/bin/node/cli/src/service.rs
+++ b/bin/node/cli/src/service.rs
@@ -592,7 +592,7 @@ mod tests {
 	use sc_service_test::TestNetNode;
 	use sc_transaction_pool_api::{ChainEvent, MaintainedTransactionPool};
 	use sp_consensus::{BlockOrigin, Environment, Proposer};
-	use sp_core::crypto::{Pair as CryptoPair, Wraps};
+	use sp_core::crypto::Pair;
 	use sp_inherents::InherentDataProvider;
 	use sp_keyring::AccountKeyring;
 	use sp_keystore::KeystorePtr;
@@ -737,11 +737,7 @@ mod tests {
 				// add it to a digest item.
 				let to_sign = pre_hash.encode();
 				let signature = keystore
-					.sr25519_sign(
-						sp_consensus_babe::AuthorityId::ID,
-						alice.as_inner_ref(),
-						&to_sign,
-					)
+					.sr25519_sign(sp_consensus_babe::AuthorityId::ID, alice.as_ref(), &to_sign)
 					.unwrap()
 					.unwrap();
 				let item = <DigestItem as CompatibleDigestItem>::babe_seal(signature.into());
diff --git a/client/authority-discovery/src/worker.rs b/client/authority-discovery/src/worker.rs
index 7cc2baa08f7c8..43e44cac4d3fe 100644
--- a/client/authority-discovery/src/worker.rs
+++ b/client/authority-discovery/src/worker.rs
@@ -52,7 +52,7 @@ use sp_authority_discovery::{
 	AuthorityDiscoveryApi, AuthorityId, AuthorityPair, AuthoritySignature,
 };
 use sp_blockchain::HeaderBackend;
-use sp_core::crypto::{key_types, ByteArray, Pair, Wraps};
+use sp_core::crypto::{key_types, ByteArray, Pair};
 use sp_keystore::{Keystore, KeystorePtr};
 use sp_runtime::traits::Block as BlockT;
 
@@ -670,7 +670,7 @@ fn sign_record_with_authority_ids(
 
 	for key in keys.iter() {
 		let auth_signature = key_store
-			.sr25519_sign(key_types::AUTHORITY_DISCOVERY, key.as_inner_ref(), &serialized_record)
+			.sr25519_sign(key_types::AUTHORITY_DISCOVERY, key.as_ref(), &serialized_record)
 			.map_err(|e| Error::CannotSign(format!("{}. Key: {:?}", e, key)))?
 			.ok_or_else(|| {
 				Error::CannotSign(format!("Could not find key in keystore. Key: {:?}", key))
diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs
index aeff7de48551c..a48eeac5ce8b2 100644
--- a/client/consensus/aura/src/lib.rs
+++ b/client/consensus/aura/src/lib.rs
@@ -441,13 +441,12 @@ where
 		sc_consensus::BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
 		ConsensusError,
 	> {
-		let public = public.to_raw_vec();
 		let signature = self
 			.keystore
 			.sign_with(
 				<AuthorityId<P> as AppCrypto>::ID,
 				<AuthorityId<P> as AppCrypto>::CRYPTO_ID,
-				&public,
+				public.as_slice(),
 				header_hash.as_ref(),
 			)
 			.map_err(|e| ConsensusError::CannotSign(format!("{}. Key: {:?}", e, public)))?
@@ -460,7 +459,7 @@ where
 		let signature = signature
 			.clone()
 			.try_into()
-			.map_err(|_| ConsensusError::InvalidSignature(signature, public))?;
+			.map_err(|_| ConsensusError::InvalidSignature(signature, public.to_raw_vec()))?;
 
 		let signature_digest_item =
 			<DigestItem as CompatibleDigestItem<P::Signature>>::aura_seal(signature);
diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs
index 4bc6169db26e5..a59870ced0693 100644
--- a/client/consensus/babe/src/lib.rs
+++ b/client/consensus/babe/src/lib.rs
@@ -117,7 +117,7 @@ use sp_blockchain::{
 use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain};
 use sp_consensus_babe::inherents::BabeInherentData;
 use sp_consensus_slots::Slot;
-use sp_core::{crypto::Wraps, ExecutionContext};
+use sp_core::ExecutionContext;
 use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider};
 use sp_keystore::KeystorePtr;
 use sp_runtime::{
@@ -832,11 +832,7 @@ where
 	> {
 		let signature = self
 			.keystore
-			.sr25519_sign(
-				<AuthorityId as AppCrypto>::ID,
-				public.as_inner_ref(),
-				header_hash.as_ref(),
-			)
+			.sr25519_sign(<AuthorityId as AppCrypto>::ID, public.as_ref(), header_hash.as_ref())
 			.map_err(|e| ConsensusError::CannotSign(format!("{}. Key: {:?}", e, public)))?
 			.ok_or_else(|| {
 				ConsensusError::CannotSign(format!(
diff --git a/primitives/consensus/grandpa/src/lib.rs b/primitives/consensus/grandpa/src/lib.rs
index 347f968d6848a..728ce3092888b 100644
--- a/primitives/consensus/grandpa/src/lib.rs
+++ b/primitives/consensus/grandpa/src/lib.rs
@@ -452,11 +452,10 @@ where
 	N: Encode,
 {
 	use sp_application_crypto::AppCrypto;
-	use sp_core::crypto::Wraps;
 
 	let encoded = localized_payload(round, set_id, &message);
 	let signature = keystore
-		.ed25519_sign(AuthorityId::ID, public.as_inner_ref(), &encoded[..])
+		.ed25519_sign(AuthorityId::ID, public.as_ref(), &encoded[..])
 		.ok()
 		.flatten()?
 		.try_into()