More tests and some cleanup (#3331)

substrate/core/cli/src/lib.rs

@@ -463,7 +463,6 @@ where
 	config.disable_grandpa = cli.no_grandpa;
 	config.grandpa_voter = cli.grandpa_voter;
 
-
 	let is_dev = cli.shared_params.dev;
 
 	let client_id = config.client_id();
@@ -481,14 +480,12 @@ where
 		cli.pool_config,
 	)?;
 
-
 	if cli.shared_params.dev {
 		config.dev_key_seed = cli.keyring.account
 			.map(|a| format!("//{}", a))
 			.or_else(|| Some("//Alice".into()));
 	}
 
-
 	let rpc_interface: &str = if cli.rpc_external { "0.0.0.0" } else { "127.0.0.1" };
 	let ws_interface: &str = if cli.ws_external { "0.0.0.0" } else { "127.0.0.1" };
 

substrate/core/keystore/src/lib.rs

@@ -190,10 +190,10 @@ impl Store {
 		let file = File::open(path)?;
 
 		let phrase: String = serde_json::from_reader(&file)?;
-		let pair = Pair::from_phrase(
+		let pair = Pair::from_string(
 			&phrase,
 			self.password.as_ref().map(|p| &***p),
-		).map_err(|_| Error::InvalidPhrase)?.0;
+		).map_err(|_| Error::InvalidPhrase)?;
 
 		if &pair.public() == public {
 			Ok(pair)
@@ -318,7 +318,7 @@ impl BareCryptoStore for Store {
 mod tests {
 	use super::*;
 	use tempdir::TempDir;
-	use primitives::crypto::Ss58Codec;
+	use primitives::crypto::{Ss58Codec, key_types};
 
 	#[test]
 	fn basic_store() {
@@ -400,4 +400,26 @@ mod tests {
 
 		assert_eq!(public_keys, store_pubs);
 	}
+
+	#[test]
+	fn store_unknown_and_extract_it() {
+		let temp_dir = TempDir::new("keystore").unwrap();
+		let store = Store::open(temp_dir.path(), None).unwrap();
+
+		let secret_uri = "//Alice";
+		let key_pair = sr25519::AppPair::from_string(secret_uri, None).expect("Generates key pair");
+
+		store.write().insert_unknown(
+			key_types::SR25519,
+			secret_uri,
+			key_pair.public().as_ref(),
+		).expect("Inserts unknown key");
+
+		let store_key_pair = store.read().key_pair_by_type::<sr25519::AppPair>(
+			&key_pair.public(),
+			key_types::SR25519,
+		).expect("Gets key pair from keystore");
+
+		assert_eq!(key_pair.public(), store_key_pair.public());
+	}
 }

substrate/core/primitives/src/testing.rs

@@ -24,7 +24,7 @@ use crate::{ed25519, sr25519, crypto::{Public, Pair, KeyTypeId}};
 #[derive(Default)]
 pub struct KeyStore {
 	/// `KeyTypeId` maps to public keys and public keys map to private keys.
-	keys: std::collections::HashMap<KeyTypeId, std::collections::HashMap<Vec<u8>, Vec<u8>>>,
+	keys: std::collections::HashMap<KeyTypeId, std::collections::HashMap<Vec<u8>, String>>,
 }
 
 #[cfg(feature = "std")]
@@ -41,7 +41,7 @@ impl crate::traits::BareCryptoStore for KeyStore {
 		self.keys.get(&id)
 			.map(|keys|
 				keys.values()
-					.map(|s| sr25519::Pair::from_seed_slice(s).expect("`sr25519` seed slice is valid"))
+					.map(|s| sr25519::Pair::from_string(s, None).expect("`sr25519` seed slice is valid"))
 					.map(|p| p.public())
 					.collect()
 			)
@@ -56,12 +56,12 @@ impl crate::traits::BareCryptoStore for KeyStore {
 		match seed {
 			Some(seed) => {
 				let pair = sr25519::Pair::from_string(seed, None).expect("Generates an `sr25519` pair.");
-				self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), pair.to_raw_vec());
+				self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), seed.into());
 				Ok(pair.public())
 			},
 			None => {
-				let (pair, _) = sr25519::Pair::generate();
-				self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), pair.to_raw_vec());
+				let (pair, phrase, _) = sr25519::Pair::generate_with_phrase(None);
+				self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), phrase);
 				Ok(pair.public())
 			}
 		}
@@ -71,7 +71,7 @@ impl crate::traits::BareCryptoStore for KeyStore {
 		self.keys.get(&id)
 			.and_then(|inner|
 				inner.get(pub_key.as_slice())
-					.map(|s| sr25519::Pair::from_seed_slice(s).expect("`sr25519` seed slice is valid"))
+					.map(|s| sr25519::Pair::from_string(s, None).expect("`sr25519` seed slice is valid"))
 			)
 	}
 
@@ -79,7 +79,7 @@ impl crate::traits::BareCryptoStore for KeyStore {
 		self.keys.get(&id)
 			.map(|keys|
 				keys.values()
-					.map(|s| ed25519::Pair::from_seed_slice(s).expect("`ed25519` seed slice is valid"))
+					.map(|s| ed25519::Pair::from_string(s, None).expect("`ed25519` seed slice is valid"))
 					.map(|p| p.public())
 					.collect()
 			)
@@ -94,12 +94,12 @@ impl crate::traits::BareCryptoStore for KeyStore {
 		match seed {
 			Some(seed) => {
 				let pair = ed25519::Pair::from_string(seed, None).expect("Generates an `ed25519` pair.");
-				self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), pair.to_raw_vec());
+				self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), seed.into());
 				Ok(pair.public())
 			},
 			None => {
-				let (pair, _) = ed25519::Pair::generate();
-				self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), pair.to_raw_vec());
+				let (pair, phrase, _) = ed25519::Pair::generate_with_phrase(None);
+				self.keys.entry(id).or_default().insert(pair.public().to_raw_vec(), phrase);
 				Ok(pair.public())
 			}
 		}
@@ -109,7 +109,58 @@ impl crate::traits::BareCryptoStore for KeyStore {
 		self.keys.get(&id)
 			.and_then(|inner|
 				inner.get(pub_key.as_slice())
-					.map(|s| ed25519::Pair::from_seed_slice(s).expect("`ed25519` seed slice is valid"))
+					.map(|s| ed25519::Pair::from_string(s, None).expect("`ed25519` seed slice is valid"))
 			)
 	}
+
+	fn insert_unknown(&mut self, id: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> {
+		self.keys.entry(id).or_default().insert(public.to_owned(), suri.to_string());
+		Ok(())
+	}
+
+	fn password(&self) -> Option<&str> {
+		None
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use crate::{sr25519, crypto::key_types, traits::BareCryptoStore};
+
+	#[test]
+	fn store_key_and_extract() {
+		let store = KeyStore::new();
+
+		let public = store.write()
+			.ed25519_generate_new(key_types::ED25519, None)
+			.expect("Genrates key");
+
+		let store_key_pair = store.read()
+			.ed25519_key_pair(key_types::ED25519, &public)
+			.expect("Key should exists in store");
+
+		assert_eq!(public, store_key_pair.public());
+	}
+
+	#[test]
+	fn store_unknown_and_extract_it() {
+		let store = KeyStore::new();
+
+		let secret_uri = "//Alice";
+		let key_pair = sr25519::Pair::from_string(secret_uri, None).expect("Generates key pair");
+
+		store.write().insert_unknown(
+			key_types::SR25519,
+			secret_uri,
+			key_pair.public().as_ref(),
+		).expect("Inserts unknown key");
+
+		let store_key_pair = store.read().sr25519_key_pair(
+			key_types::SR25519,
+			&key_pair.public(),
+		).expect("Gets key pair from keystore");
+
+		assert_eq!(key_pair.public(), store_key_pair.public());
+	}
 }

substrate/core/primitives/src/traits.rs

@@ -59,12 +59,10 @@ pub trait BareCryptoStore: Send + Sync {
 	/// Places it into the file system store.
 	///
 	/// `Err` if there's some sort of weird filesystem error, but should generally be `Ok`.
-	fn insert_unknown(&mut self, _key_type: KeyTypeId, _suri: &str, _public: &[u8]) -> Result<(), ()> {
-		Err(())
-	}
+	fn insert_unknown(&mut self, _key_type: KeyTypeId, _suri: &str, _public: &[u8]) -> Result<(), ()>;
 
 	/// Get the password for this store.
-	fn password(&self) -> Option<&str> { None }
+	fn password(&self) -> Option<&str>;
 }
 
 /// A pointer to the key store.

substrate/core/rpc/src/author/mod.rs

@@ -141,7 +141,8 @@ impl<B, E, P, RA> AuthorApi<ExHash<P>, BlockHash<P>> for Author<B, E, P, RA> whe
 {
 	type Metadata = crate::metadata::Metadata;
 
-	fn insert_key(&self,
+	fn insert_key(
+		&self,
 		key_type: String,
 		suri: String,
 		maybe_public: Option<Bytes>,

substrate/core/rpc/src/author/tests.rs

@@ -23,12 +23,12 @@ use transaction_pool::{
 	txpool::Pool,
 	ChainApi,
 };
-use primitives::{H256, blake2_256, hexdisplay::HexDisplay, traits::BareCryptoStore};
+use primitives::{
+	H256, blake2_256, hexdisplay::HexDisplay, traits::BareCryptoStore, testing::KeyStore,
+	ed25519, crypto::key_types,
+};
 use test_client::{self, AccountKeyring, runtime::{Extrinsic, Transfer}};
 use tokio::runtime;
-use std::collections::HashMap;
-use sr_primitives::KeyTypeId;
-use parking_lot::RwLock;
 
 fn uxt(sender: AccountKeyring, nonce: u64) -> Extrinsic {
 	let tx = Transfer {
@@ -40,50 +40,11 @@ fn uxt(sender: AccountKeyring, nonce: u64) -> Extrinsic {
 	tx.into_signed_tx()
 }
 
-#[derive(Default)]
-struct TestKeyStore {
-	keys: HashMap<KeyTypeId, HashMap<Vec<u8>, String>>,
-}
-
-impl BareCryptoStore for TestKeyStore {
-	fn sr25519_public_keys(&self, _id: KeyTypeId) -> Vec<sr25519::Public> { vec![] }
-	fn sr25519_generate_new(&mut self, _id: KeyTypeId, _seed: Option<&str>)
-		-> std::result::Result<sr25519::Public, String>
-	{
-		Err("unimplemented".into())
-	}
-	fn sr25519_key_pair(&self, _id: KeyTypeId, _pub_key: &sr25519::Public) -> Option<sr25519::Pair> {
-		None
-	}
-	fn ed25519_public_keys(&self, _id: KeyTypeId) -> Vec<ed25519::Public> { vec![] }
-	fn ed25519_generate_new(&mut self, _id: KeyTypeId, _seed: Option<&str>)
-		-> std::result::Result<ed25519::Public, String>
-	{
-		Err("unimplemented".into())
-	}
-	fn ed25519_key_pair(&self, _id: KeyTypeId, _pub_key: &ed25519::Public) -> Option<ed25519::Pair> {
-		None
-	}
-
-	fn insert_unknown(&mut self, key_type: KeyTypeId, suri: &str, public: &[u8])
-		-> std::result::Result<(), ()>
-	{
-		self.keys
-			.entry(key_type)
-			.or_default()
-			.insert(public.to_owned(), suri.to_owned());
-		Ok(())
-	}
-
-	fn password(&self) -> Option<&str> { None }
-}
-
 #[test]
 fn submit_transaction_should_not_cause_error() {
 	let runtime = runtime::Runtime::new().unwrap();
 	let client = Arc::new(test_client::new());
-	let keystore = TestKeyStore::default();
-	let keystore = Arc::new(RwLock::new(keystore));
+	let keystore = KeyStore::new();
 	let p = Author {
 		client: client.clone(),
 		pool: Arc::new(Pool::new(Default::default(), ChainApi::new(client))),
@@ -106,7 +67,7 @@ fn submit_transaction_should_not_cause_error() {
 fn submit_rich_transaction_should_not_cause_error() {
 	let runtime = runtime::Runtime::new().unwrap();
 	let client = Arc::new(test_client::new());
-	let keystore = Arc::new(RwLock::new(TestKeyStore::default()));
+	let keystore = KeyStore::new();
 	let p = Author {
 		client: client.clone(),
 		pool: Arc::new(Pool::new(Default::default(), ChainApi::new(client.clone()))),
@@ -131,7 +92,7 @@ fn should_watch_extrinsic() {
 	let mut runtime = runtime::Runtime::new().unwrap();
 	let client = Arc::new(test_client::new());
 	let pool = Arc::new(Pool::new(Default::default(), ChainApi::new(client.clone())));
-	let keystore = Arc::new(RwLock::new(TestKeyStore::default()));
+	let keystore = KeyStore::new();
 	let p = Author {
 		client,
 		pool: pool.clone(),
@@ -173,7 +134,7 @@ fn should_return_pending_extrinsics() {
 	let runtime = runtime::Runtime::new().unwrap();
 	let client = Arc::new(test_client::new());
 	let pool = Arc::new(Pool::new(Default::default(), ChainApi::new(client.clone())));
-	let keystore = Arc::new(RwLock::new(TestKeyStore::default()));
+	let keystore = KeyStore::new();
 	let p = Author {
 		client,
 		pool: pool.clone(),
@@ -193,7 +154,7 @@ fn should_remove_extrinsics() {
 	let runtime = runtime::Runtime::new().unwrap();
 	let client = Arc::new(test_client::new());
 	let pool = Arc::new(Pool::new(Default::default(), ChainApi::new(client.clone())));
-	let keystore = Arc::new(RwLock::new(TestKeyStore::default()));
+	let keystore = KeyStore::new();
 	let p = Author {
 		client,
 		pool: pool.clone(),
@@ -217,3 +178,29 @@ fn should_remove_extrinsics() {
 
  	assert_eq!(removed.len(), 3);
 }
+
+#[test]
+fn should_insert_key() {
+	let runtime = runtime::Runtime::new().unwrap();
+	let client = Arc::new(test_client::new());
+	let keystore = KeyStore::new();
+	let p = Author {
+		client: client.clone(),
+		pool: Arc::new(Pool::new(Default::default(), ChainApi::new(client))),
+		subscriptions: Subscriptions::new(Arc::new(runtime.executor())),
+		keystore: keystore.clone(),
+	};
+
+	let suri = "//Alice";
+	let key_pair = ed25519::Pair::from_string(suri, None).expect("Generates keypair");
+	p.insert_key(
+		String::from_utf8(key_types::ED25519.0.to_vec()).expect("Keytype is a valid string"),
+		suri.to_string(),
+		Some(key_pair.public().0.to_vec().into()),
+	).expect("Insert key");
+
+	let store_key_pair = keystore.read()
+		.ed25519_key_pair(key_types::ED25519, &key_pair.public()).expect("Key exists in store");
+
+	assert_eq!(key_pair.public(), store_key_pair.public());
+}