From 46709e83811e8ae193a328b7e7ebc263b8de55b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com> Date: Thu, 8 Aug 2019 00:05:15 +0200 Subject: [PATCH] More tests and some cleanup (#3331) --- substrate/core/cli/src/lib.rs | 3 - substrate/core/keystore/src/lib.rs | 28 +++++++- substrate/core/primitives/src/testing.rs | 73 +++++++++++++++++---- substrate/core/primitives/src/traits.rs | 6 +- substrate/core/rpc/src/author/mod.rs | 3 +- substrate/core/rpc/src/author/tests.rs | 83 ++++++++++-------------- 6 files changed, 126 insertions(+), 70 deletions(-) diff --git a/substrate/core/cli/src/lib.rs b/substrate/core/cli/src/lib.rs index 0f6769a6990..6be285ee371 100644 --- a/substrate/core/cli/src/lib.rs +++ b/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" }; diff --git a/substrate/core/keystore/src/lib.rs b/substrate/core/keystore/src/lib.rs index 5a6df04b77e..9125ddbda65 100644 --- a/substrate/core/keystore/src/lib.rs +++ b/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()); + } } diff --git a/substrate/core/primitives/src/testing.rs b/substrate/core/primitives/src/testing.rs index 6d91c83ccb6..932c8b9cb1e 100644 --- a/substrate/core/primitives/src/testing.rs +++ b/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()); + } } diff --git a/substrate/core/primitives/src/traits.rs b/substrate/core/primitives/src/traits.rs index 6fa84088db7..8e2f0c0213a 100644 --- a/substrate/core/primitives/src/traits.rs +++ b/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. diff --git a/substrate/core/rpc/src/author/mod.rs b/substrate/core/rpc/src/author/mod.rs index 226fd2b105c..47f481f00f1 100644 --- a/substrate/core/rpc/src/author/mod.rs +++ b/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>, diff --git a/substrate/core/rpc/src/author/tests.rs b/substrate/core/rpc/src/author/tests.rs index 93c39a4bc5a..18a2e417395 100644 --- a/substrate/core/rpc/src/author/tests.rs +++ b/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()); +} \ No newline at end of file -- GitLab