From c7e163a81fd7da9a3bec2cb0ab08e7130187bb5f Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:25:35 +0200 Subject: [PATCH 01/14] Rewrite keystore. --- core/keystore/Cargo.toml | 8 +- core/keystore/src/lib.rs | 249 +++++++++++++++++++-------------------- 2 files changed, 121 insertions(+), 136 deletions(-) diff --git a/core/keystore/Cargo.toml b/core/keystore/Cargo.toml index 1d4f146b7ed7f..33dd2c158966c 100644 --- a/core/keystore/Cargo.toml +++ b/core/keystore/Cargo.toml @@ -7,10 +7,4 @@ edition = "2018" [dependencies] derive_more = "0.14.0" substrate-primitives = { path = "../primitives" } -hex = "0.3" -rand = "0.6" -serde_json = "1.0" -subtle = "2.0" - -[dev-dependencies] -tempdir = "0.3" +parking_lot = "0.8.0" diff --git a/core/keystore/src/lib.rs b/core/keystore/src/lib.rs index 77106059f82bd..dd3dd9894780a 100644 --- a/core/keystore/src/lib.rs +++ b/core/keystore/src/lib.rs @@ -14,185 +14,176 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Keystore (and session key management) for ed25519 based chains like Polkadot. +//! Session key store is an in memory key store. #![warn(missing_docs)] +use parking_lot::Mutex; +use substrate_primitives::crypto::{KeyTypeId, Pair}; use std::collections::HashMap; -use std::path::PathBuf; -use std::fs::{self, File}; -use std::io::{self, Write}; +use std::sync::Arc; +use std::time::{Duration, Instant}; -use substrate_primitives::crypto::{KeyTypeId, Pair, Public}; +// 24h in seconds +const INACTIVITY_PERIOD: u64 = 60 * 60 * 24; /// Keystore error. #[derive(Debug, derive_more::Display, derive_more::From)] pub enum Error { - /// IO error. - Io(io::Error), - /// JSON error. - Json(serde_json::Error), - /// Invalid password. - #[display(fmt="Invalid password")] - InvalidPassword, - /// Invalid BIP39 phrase - #[display(fmt="Invalid recovery phrase (BIP39) data")] - InvalidPhrase, /// Invalid seed #[display(fmt="Invalid seed")] InvalidSeed, } +impl std::error::Error for Error {} + /// Keystore Result pub type Result = std::result::Result; -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Error::Io(ref err) => Some(err), - Error::Json(ref err) => Some(err), - _ => None, - } - } +/// Handler for `Store` events. +trait StoreHandler: Send { + /// Called when a new session key is added to the store. + fn on_add_session_key(&mut self, pair: &[u8]); } -/// Key store. +/// Store for session keys. pub struct Store { - path: PathBuf, - additional: HashMap<(KeyTypeId, Vec), Vec>, + handlers: Mutex)>>, } impl Store { - /// Create a new store at the given path. - pub fn open(path: PathBuf) -> Result { - fs::create_dir_all(&path)?; - Ok(Store { path, additional: HashMap::new() }) + /// Create a new store. + pub fn new() -> Self { + Self { + handlers: Default::default(), + } } - fn get_pair(&self, public: &TPair::Public) -> Result> { - let key = (TPair::KEY_TYPE, public.to_raw_vec()); - if let Some(bytes) = self.additional.get(&key) { - let pair = TPair::from_seed_slice(bytes) - .map_err(|_| Error::InvalidSeed)?; - return Ok(Some(pair)); + /// Adds a session key pair. + pub fn add_key(&self, pair: &TPair) { + let value = pair.to_raw_vec(); + let mut handlers = self.handlers.lock(); + for (key_type, handler) in handlers.iter_mut() { + if *key_type == TPair::KEY_TYPE { + handler.on_add_session_key(&value[..]); + } } - Ok(None) } - fn insert_pair(&mut self, pair: &TPair) { - let key = (TPair::KEY_TYPE, pair.public().to_raw_vec()); - self.additional.insert(key, pair.to_raw_vec()); + /// Adds a session key pair from a seed. Used for testing. + pub fn add_key_from_seed(&self, seed: &str) -> Result<()> { + let pair = TPair::from_string(seed, None) + .map_err(|_| Error::InvalidSeed)?; + self.add_key(&pair); + Ok(()) } - /// Generate a new key, placing it into the store. - pub fn generate(&self, password: &str) -> Result { - let (pair, phrase, _) = TPair::generate_with_phrase(Some(password)); - let mut file = File::create(self.key_file_path::(&pair.public()))?; - ::serde_json::to_writer(&file, &phrase)?; - file.flush()?; - Ok(pair) + /// Creates a local session store. + pub fn create_local_store(&self) -> Arc> + where + TPair: Pair + Send + Clone, + TPair::Public: Send + Clone, + { + let session_store = Arc::new(LocalStore::new()); + self.handlers.lock().push((TPair::KEY_TYPE, Box::new(session_store.clone()))); + session_store } +} - /// Create a new key from seed. Do not place it into the store. - pub fn generate_from_seed(&mut self, seed: &str) -> Result { - let pair = TPair::from_string(seed, None) - .ok().ok_or(Error::InvalidSeed)?; - self.insert_pair(&pair); - Ok(pair) - } +/// A local session store supporting a single crypto algorithm. +pub struct LocalStore { + keys: Mutex>, +} - /// Load a key file with given public key. - pub fn load(&self, public: &TPair::Public, password: &str) -> Result { - if let Some(pair) = self.get_pair(public)? { - return Ok(pair) +impl LocalStore +where + TPair: Pair + Send + Clone, + TPair::Public: Send + Clone, +{ + /// Creates a new `LocalStore`. + pub fn new() -> Self { + Self { + keys: Default::default(), } + } - let path = self.key_file_path::(public); - let file = File::open(path)?; + /// Gets the key matching the public key. + pub fn get_key(&self, public: &TPair::Public) -> Option { + let pair = { + let mut keys = self.keys.lock(); + if let Some(value) = keys.get_mut(public) { + value.1 = Instant::now(); + Some(value.0.clone()) + } else { + None + } + }; + self.clean(); + pair + } - let phrase: String = ::serde_json::from_reader(&file)?; - let (pair, _) = TPair::from_phrase(&phrase, Some(password)) - .ok().ok_or(Error::InvalidPhrase)?; - if &pair.public() != public { - return Err(Error::InvalidPassword); - } - Ok(pair) + /// Find keys matching a predicate. + pub fn get_keys(&self, f: impl Fn(&TPair::Public) -> bool) -> Vec { + let filtered_keys = { + self.keys.lock().iter_mut() + .filter(|(public, _)| f(public)) + .map(|(_, value)| { + value.1 = Instant::now(); + value.0.clone() + }) + .collect::>() + }; + self.clean(); + filtered_keys } - /// Get public keys of all stored keys. - pub fn contents(&self) -> Result> { - let mut public_keys: Vec = self.additional.keys() - .filter_map(|(ty, public)| { - if *ty != TPublic::KEY_TYPE { - return None - } - Some(TPublic::from_slice(public)) - }) - .collect(); - - let key_type: [u8; 4] = TPublic::KEY_TYPE.to_le_bytes(); - for entry in fs::read_dir(&self.path)? { - let entry = entry?; - let path = entry.path(); - - // skip directories and non-unicode file names (hex is unicode) - if let Some(name) = path.file_name().and_then(|n| n.to_str()) { - match hex::decode(name) { - Ok(ref hex) => { - if hex[0..4] != key_type { continue } - let public = TPublic::from_slice(&hex[4..]); - public_keys.push(public); - } - _ => continue, - } + /// Cleanup session store. + fn clean(&self) { + let mut keys = self.keys.lock(); + if keys.len() > 1 { + let duration = Duration::new(INACTIVITY_PERIOD, 0); + let new_keys = keys.iter().filter(|(_, (_, instant))| { + Instant::now().duration_since(instant.clone()) < duration + }).map(|(k, v)| (k.clone(), v.clone())).collect::>(); + // If a validator gets unstaked but then restaked some sessions later + // prevent the validator from getting slashed. + if new_keys.len() > 0 { + *keys = new_keys; } } - - Ok(public_keys) } +} - fn key_file_path(&self, public: &TPair::Public) -> PathBuf { - let mut buf = self.path.clone(); - let bytes: [u8; 4] = TPair::KEY_TYPE.to_le_bytes(); - let key_type = hex::encode(bytes); - let key = hex::encode(public.as_slice()); - buf.push(key_type + key.as_str()); - buf +impl StoreHandler for Arc> +where + TPair: Pair + Send, + TPair::Public: Send, +{ + fn on_add_session_key(&mut self, pair: &[u8]) { + let pair = TPair::from_seed_slice(pair) + .expect("store contains valid bytes"); + let mut keys = self.keys.lock(); + keys.insert(pair.public(), (pair, Instant::now())); } } #[cfg(test)] mod tests { use super::*; - use tempdir::TempDir; use substrate_primitives::ed25519; - use substrate_primitives::crypto::Ss58Codec; - - #[test] - fn basic_store() { - let temp_dir = TempDir::new("keystore").unwrap(); - let store = Store::open(temp_dir.path().to_owned()).unwrap(); - - assert!(store.contents::().unwrap().is_empty()); - - let key: ed25519::Pair = store.generate("thepassword").unwrap(); - let key2: ed25519::Pair = store.load(&key.public(), "thepassword").unwrap(); - - assert!(store.load::(&key.public(), "notthepassword").is_err()); - - assert_eq!(key.public(), key2.public()); - - assert_eq!(store.contents::().unwrap()[0], key.public()); - } #[test] - fn test_generate_from_seed() { - let temp_dir = TempDir::new("keystore").unwrap(); - let mut store = Store::open(temp_dir.path().to_owned()).unwrap(); - - let pair: ed25519::Pair = store - .generate_from_seed("0x3d97c819d68f9bafa7d6e79cb991eebcd77d966c5334c0b94d9e1fa7ad0869dc") - .unwrap(); - assert_eq!("5DKUrgFqCPV8iAXx9sjy1nyBygQCeiUYRFWurZGhnrn3HJCA", pair.public().to_ss58check()); + fn test_handler() { + let store = Store::new(); + let local_store = store.create_local_store::(); + + let key_a1 = ed25519::Pair::from_seed(&[0; 32]); + let key_b1 = ed25519::Pair::from_string("//Alice", None).unwrap(); + store.set_key(&key_a1); + store.set_key_from_seed::("//Alice").unwrap(); + let key_a2 = local_store.get_key(&key_a1.public()).unwrap(); + let key_b2 = local_store.get_key(&key_b1.public()).unwrap(); + assert_eq!(key_a1.to_raw_vec(), key_a2.to_raw_vec()); + assert_eq!(key_b1.to_raw_vec(), key_b2.to_raw_vec()); } } From 54b33e21dcca04e42336a1dce572171c7a7047e8 Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:26:58 +0200 Subject: [PATCH 02/14] Implement setKey rpc call. --- core/rpc/Cargo.toml | 1 + core/rpc/src/author/error.rs | 6 ++++++ core/rpc/src/author/mod.rs | 31 ++++++++++++++++++++++++++++++- core/rpc/src/author/tests.rs | 32 +++++++++++++++++++++++++++++++- 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/core/rpc/Cargo.toml b/core/rpc/Cargo.toml index 181cbdfd8ea5f..98b7816a9cf8b 100644 --- a/core/rpc/Cargo.toml +++ b/core/rpc/Cargo.toml @@ -19,6 +19,7 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" client = { package = "substrate-client", path = "../client" } substrate-executor = { path = "../executor" } +keystore = { package = "substrate-keystore", path = "../keystore" } network = { package = "substrate-network", path = "../network" } primitives = { package = "substrate-primitives", path = "../primitives" } state_machine = { package = "substrate-state-machine", path = "../state-machine" } diff --git a/core/rpc/src/author/error.rs b/core/rpc/src/author/error.rs index 82ace88b84b99..cf8a59d2a9c33 100644 --- a/core/rpc/src/author/error.rs +++ b/core/rpc/src/author/error.rs @@ -38,6 +38,12 @@ pub enum Error { /// Incorrect extrinsic format. #[display(fmt="Invalid extrinsic format")] BadFormat, + /// Unknown key type. + #[display(fmt="Invalid key type id")] + UnknownKeyType, + /// Invalid key. + #[display(fmt="Invalid key seed")] + BadSeed, } impl std::error::Error for Error { diff --git a/core/rpc/src/author/mod.rs b/core/rpc/src/author/mod.rs index 5594984d0ea7f..83d105d1843e8 100644 --- a/core/rpc/src/author/mod.rs +++ b/core/rpc/src/author/mod.rs @@ -29,9 +29,11 @@ use crate::rpc::futures::{Sink, Stream, Future}; use crate::subscriptions::Subscriptions; use jsonrpc_derive::rpc; use jsonrpc_pubsub::{typed::Subscriber, SubscriptionId}; +use keystore::Store; use log::warn; use parity_codec::{Encode, Decode}; -use primitives::{Bytes, Blake2Hasher, H256}; +use primitives::{Bytes, Blake2Hasher, H256, ed25519, sr25519}; +use primitives::crypto::{KeyTypeId, Pair, key_types}; use runtime_primitives::{generic, traits}; use self::error::Result; use transaction_pool::{ @@ -72,6 +74,10 @@ pub trait AuthorApi { /// Unsubscribe from extrinsic watching. #[pubsub(subscription = "author_extrinsicUpdate", unsubscribe, name = "author_unwatchExtrinsic")] fn unwatch_extrinsic(&self, metadata: Option, id: SubscriptionId) -> Result; + + /// Add session key. + #[rpc(name = "author_addKey")] + fn add_key(&self, ty: KeyTypeId, key: Vec) -> Result<()>; } /// Authoring API @@ -82,6 +88,8 @@ pub struct Author where P: PoolChainApi + Sync + Send + 'static { pool: Arc>, /// Subscriptions manager subscriptions: Subscriptions, + /// Keystore + keystore: Option>, } impl Author where P: PoolChainApi + Sync + Send + 'static { @@ -90,11 +98,13 @@ impl Author where P: PoolChainApi + Sync + Send + 'sta client: Arc::Block, RA>>, pool: Arc>, subscriptions: Subscriptions, + keystore: Option>, ) -> Self { Author { client, pool, subscriptions, + keystore, } } } @@ -176,4 +186,23 @@ impl AuthorApi, BlockHash

> for Author whe fn unwatch_extrinsic(&self, _metadata: Option, id: SubscriptionId) -> Result { Ok(self.subscriptions.cancel(id)) } + + fn add_key(&self, key_type_id: KeyTypeId, key: Vec) -> Result<()> { + if let Some(keystore) = &self.keystore { + match key_type_id { + key_types::ED25519 => { + let pair = ed25519::Pair::from_seed_slice(&key[..]) + .map_err(|_| error::Error::BadSeed)?; + keystore.add_key(&pair); + } + key_types::SR25519 => { + let pair = sr25519::Pair::from_seed_slice(&key[..]) + .map_err(|_| error::Error::BadSeed)?; + keystore.add_key(&pair); + } + _ => Err(error::Error::UnknownKeyType)? + } + } + Ok(()) + } } diff --git a/core/rpc/src/author/tests.rs b/core/rpc/src/author/tests.rs index cf320ee1442e7..3a6a4f629972f 100644 --- a/core/rpc/src/author/tests.rs +++ b/core/rpc/src/author/tests.rs @@ -23,7 +23,8 @@ use transaction_pool::{ txpool::Pool, ChainApi, }; -use primitives::{H256, blake2_256, hexdisplay::HexDisplay}; +use keystore::Store; +use primitives::{H256, blake2_256, hexdisplay::HexDisplay, crypto::key_types}; use test_client::{self, AccountKeyring, runtime::{Extrinsic, Transfer}}; use tokio::runtime; @@ -45,6 +46,7 @@ fn submit_transaction_should_not_cause_error() { client: client.clone(), pool: Arc::new(Pool::new(Default::default(), ChainApi::new(client))), subscriptions: Subscriptions::new(Arc::new(runtime.executor())), + keystore: None, }; let xt = uxt(AccountKeyring::Alice, 1).encode(); let h: H256 = blake2_256(&xt).into(); @@ -66,6 +68,7 @@ fn submit_rich_transaction_should_not_cause_error() { client: client.clone(), pool: Arc::new(Pool::new(Default::default(), ChainApi::new(client.clone()))), subscriptions: Subscriptions::new(Arc::new(runtime.executor())), + keystore: None, }; let xt = uxt(AccountKeyring::Alice, 0).encode(); let h: H256 = blake2_256(&xt).into(); @@ -89,6 +92,7 @@ fn should_watch_extrinsic() { client, pool: pool.clone(), subscriptions: Subscriptions::new(Arc::new(runtime.executor())), + keystore: None, }; let (subscriber, id_rx, data) = ::jsonrpc_pubsub::typed::Subscriber::new_test("test"); @@ -129,6 +133,7 @@ fn should_return_pending_extrinsics() { client, pool: pool.clone(), subscriptions: Subscriptions::new(Arc::new(runtime.executor())), + keystore: None, }; let ex = uxt(AccountKeyring::Alice, 0); AuthorApi::submit_extrinsic(&p, ex.encode().into()).unwrap(); @@ -147,6 +152,7 @@ fn should_remove_extrinsics() { client, pool: pool.clone(), subscriptions: Subscriptions::new(Arc::new(runtime.executor())), + keystore: None, }; let ex1 = uxt(AccountKeyring::Alice, 0); p.submit_extrinsic(ex1.encode().into()).unwrap(); @@ -165,3 +171,27 @@ fn should_remove_extrinsics() { assert_eq!(removed.len(), 3); } + +#[test] +fn should_set_key() { + 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 store = Arc::new(Store::new()); + let ed25519_store = store.create_local_store::(); + let sr25519_store = store.create_local_store::(); + let p = Author { + client, + pool: pool.clone(), + subscriptions: Subscriptions::new(Arc::new(runtime.executor())), + keystore: Some(store), + }; + let ed_key = ed25519::Pair::from_seed(&[0; 32]); + let sr_key = sr25519::Pair::from_seed(&[1; 32]); + p.add_key(key_types::ED25519, ed_key.to_raw_vec()).unwrap(); + p.add_key(key_types::SR25519, sr_key.to_raw_vec()).unwrap(); + let ed_key2 = ed25519_store.get_key(&ed_key.public()).unwrap(); + let sr_key2 = sr25519_store.get_key(&sr_key.public()).unwrap(); + assert_eq!(ed_key.to_raw_vec(), ed_key2.to_raw_vec()); + assert_eq!(sr_key.to_raw_vec(), sr_key2.to_raw_vec()); +} From e868c72e6057f87579570d57cb74fa90c156d7a1 Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:27:57 +0200 Subject: [PATCH 03/14] Aura handles session key handover. --- core/consensus/aura/Cargo.toml | 1 + core/consensus/aura/src/lib.rs | 38 ++++++++++++++++++++-------------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/core/consensus/aura/Cargo.toml b/core/consensus/aura/Cargo.toml index 03ddf79be3418..0ca357b8846de 100644 --- a/core/consensus/aura/Cargo.toml +++ b/core/consensus/aura/Cargo.toml @@ -23,6 +23,7 @@ futures = "0.1.17" tokio-timer = "0.2.11" parking_lot = "0.8.0" log = "0.4" +keystore = { package = "substrate-keystore", path = "../../keystore" } [dev-dependencies] futures03 = { package = "futures-preview", version = "0.3.0-alpha.17", features = ["compat"] } diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index 2bfd907646ed6..5af5492dfd827 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -51,6 +51,7 @@ use runtime_primitives::traits::{Block as BlockT, Header, DigestItemFor, Provide use primitives::Pair; use inherents::{InherentDataProviders, InherentData}; +use keystore::LocalStore; use futures::{Future, IntoFuture, future}; use parking_lot::Mutex; @@ -131,7 +132,7 @@ impl SlotCompatible for AuraSlotCompatible { /// Start the aura worker. The returned future should be run in a tokio runtime. pub fn start_aura( slot_duration: SlotDuration, - local_key: Arc

, + session_store: Arc>, client: Arc, select_chain: SC, block_import: I, @@ -146,7 +147,7 @@ pub fn start_aura( SC: SelectChain, E::Proposer: Proposer, <>::Create as IntoFuture>::Future: Send + 'static, - P: Pair + Send + Sync + 'static, + P: Pair + Clone + Send + Sync + 'static, P::Public: Hash + Member + Encode + Decode, P::Signature: Hash + Member + Encode + Decode, H: Header, @@ -159,7 +160,7 @@ pub fn start_aura( client: client.clone(), block_import: Arc::new(Mutex::new(block_import)), env, - local_key, + session_store, sync_oracle: sync_oracle.clone(), force_authoring, }; @@ -177,16 +178,18 @@ pub fn start_aura( )) } -struct AuraWorker { +struct AuraWorker { client: Arc, block_import: Arc>, env: Arc, - local_key: Arc

, + session_store: SS, sync_oracle: SO, force_authoring: bool, } -impl SlotWorker for AuraWorker where +impl SlotWorker + for AuraWorker>, SO> +where B: BlockT, C: ProvideRuntimeApi + ProvideCache + Sync, C::Api: AuraApi>, @@ -195,7 +198,7 @@ impl SlotWorker for AuraWorker w <>::Create as IntoFuture>::Future: Send + 'static, H: Header, I: BlockImport + Send + Sync + 'static, - P: Pair + Send + Sync + 'static, + P: Pair + Clone + Send + Sync + 'static, P::Public: Member + Encode + Decode + Hash, P::Signature: Member + Encode + Decode + Hash + Debug, SO: SyncOracle + Send + Clone, @@ -208,8 +211,6 @@ impl SlotWorker for AuraWorker w chain_head: B::Header, slot_info: SlotInfo, ) -> Self::OnSlot { - let pair = self.local_key.clone(); - let public_key = self.local_key.public(); let client = self.client.clone(); let block_import = self.block_import.clone(); let env = self.env.clone(); @@ -240,9 +241,12 @@ impl SlotWorker for AuraWorker w return Box::new(future::ok(())); } let maybe_author = slot_author::

(slot_num, &authorities); - let proposal_work = match maybe_author { + let maybe_pair = maybe_author + .map(|author| self.session_store.get_key(author)) + .unwrap_or(None); + let (proposal_work, pair) = match maybe_pair { None => return Box::new(future::ok(())), - Some(author) => if author == &public_key { + Some(pair) => { debug!( target: "aura", "Starting authorship at slot {}; timestamp = {}", slot_num, @@ -267,7 +271,7 @@ impl SlotWorker for AuraWorker w let remaining_duration = slot_info.remaining_duration(); // deadline our production to approx. the end of the // slot - Timeout::new( + let timeout = Timeout::new( proposer.propose( slot_info.inherent_data, generic::Digest { @@ -278,9 +282,8 @@ impl SlotWorker for AuraWorker w remaining_duration, ).into_future(), remaining_duration, - ) - } else { - return Box::new(future::ok(())); + ); + (timeout, pair) } }; @@ -852,9 +855,12 @@ mod tests { &inherent_data_providers, slot_duration.get() ).expect("Registers aura inherent data provider"); + let store = keystore::Store::new(); + let session_store = store.create_local_store::(); + store.set_key::(&key.clone().into()); let aura = start_aura::<_, _, _, _, _, sr25519::Pair, _, _, _>( slot_duration, - Arc::new(key.clone().into()), + session_store, client.clone(), select_chain, client, From d09907d9aab43bb7eff14c7297fc33a08b192d44 Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:28:38 +0200 Subject: [PATCH 04/14] Babe handles session key handover. --- Cargo.lock | 9 ++--- core/consensus/babe/Cargo.toml | 1 + core/consensus/babe/primitives/src/lib.rs | 13 +++++-- core/consensus/babe/src/lib.rs | 46 ++++++++++++++--------- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 48edf4fa283ef..310c4114935c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4294,6 +4294,7 @@ dependencies = [ "substrate-executor 2.0.0", "substrate-inherents 2.0.0", "substrate-keyring 2.0.0", + "substrate-keystore 2.0.0", "substrate-network 2.0.0", "substrate-primitives 2.0.0", "substrate-service 2.0.0", @@ -4340,6 +4341,7 @@ dependencies = [ "substrate-executor 2.0.0", "substrate-inherents 2.0.0", "substrate-keyring 2.0.0", + "substrate-keystore 2.0.0", "substrate-network 2.0.0", "substrate-primitives 2.0.0", "substrate-service 2.0.0", @@ -4520,12 +4522,8 @@ name = "substrate-keystore" version = "2.0.0" dependencies = [ "derive_more 0.14.1 (registry+https://github.com/rust-lang/crates.io-index)", - "hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", - "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_json 1.0.40 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "substrate-primitives 2.0.0", - "subtle 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)", - "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -4682,6 +4680,7 @@ dependencies = [ "sr-version 2.0.0", "substrate-client 2.0.0", "substrate-executor 2.0.0", + "substrate-keystore 2.0.0", "substrate-network 2.0.0", "substrate-primitives 2.0.0", "substrate-state-machine 2.0.0", diff --git a/core/consensus/babe/Cargo.toml b/core/consensus/babe/Cargo.toml index 8b473932dd409..6e84e67a253d2 100644 --- a/core/consensus/babe/Cargo.toml +++ b/core/consensus/babe/Cargo.toml @@ -28,6 +28,7 @@ log = "0.4.6" schnorrkel = "0.1.1" rand = "0.6.5" merlin = "1.0.3" +keystore = { package = "substrate-keystore", path = "../../keystore" } [dev-dependencies] keyring = { package = "substrate-keyring", path = "../../keyring" } diff --git a/core/consensus/babe/primitives/src/lib.rs b/core/consensus/babe/primitives/src/lib.rs index a7b49364f4ac8..18c92a5dbe74a 100644 --- a/core/consensus/babe/primitives/src/lib.rs +++ b/core/consensus/babe/primitives/src/lib.rs @@ -24,16 +24,21 @@ mod digest; use parity_codec::{Encode, Decode}; use rstd::vec::Vec; use runtime_primitives::ConsensusEngineId; -use substrate_primitives::sr25519::Public; use substrate_client::decl_runtime_apis; #[cfg(feature = "std")] pub use digest::{BabePreDigest, CompatibleDigestItem}; pub use digest::{BABE_VRF_PREFIX, RawBabePreDigest}; -/// A Babe authority identifier. Necessarily equivalent to the schnorrkel public key used in -/// the main Babe module. If that ever changes, then this must, too. -pub type AuthorityId = Public; +/// The BABE crypto scheme defined via the keypair type. +#[cfg(feature = "std")] +pub type AuthorityPair = substrate_primitives::sr25519::Pair; + +/// Identity of a BABE authority. +pub type AuthorityId = substrate_primitives::sr25519::Public; + +/// Signature for a BABE authority. +pub type AuthoritySignature = substrate_primitives::sr25519::Signature; /// The `ConsensusEngineId` of BABE. pub const BABE_ENGINE_ID: ConsensusEngineId = *b"BABE"; diff --git a/core/consensus/babe/src/lib.rs b/core/consensus/babe/src/lib.rs index be5b476dd2782..dd61e908ff7ec 100644 --- a/core/consensus/babe/src/lib.rs +++ b/core/consensus/babe/src/lib.rs @@ -36,7 +36,7 @@ use std::{collections::HashMap, sync::Arc, u64, fmt::{Debug, Display}, time::{In use runtime_support::serde::{Serialize, Deserialize}; use parity_codec::{Decode, Encode}; use parking_lot::{Mutex, MutexGuard}; -use primitives::{Blake2Hasher, H256, Pair, Public, sr25519}; +use primitives::{Blake2Hasher, H256, Pair, Public}; use merlin::Transcript; use inherents::{InherentDataProviders, InherentData}; use substrate_telemetry::{ @@ -78,6 +78,7 @@ use futures::{Future, IntoFuture, future, stream::Stream}; use futures03::{StreamExt as _, TryStreamExt as _}; use tokio_timer::Timeout; use log::{error, warn, debug, info, trace}; +use keystore::LocalStore; use slots::{SlotWorker, SlotData, SlotInfo, SlotCompatible, SignedDuration}; @@ -142,7 +143,7 @@ pub struct BabeParams { pub config: Config, /// The key of the node we are running on. - pub local_key: Arc, + pub session_store: Arc>, /// The client to use pub client: Arc, @@ -172,7 +173,7 @@ pub struct BabeParams { /// Start the babe worker. The returned future should be run in a tokio runtime. pub fn start_babe(BabeParams { config, - local_key, + session_store, client, select_chain, block_import, @@ -201,7 +202,7 @@ pub fn start_babe(BabeParams { client: client.clone(), block_import: Arc::new(Mutex::new(block_import)), env, - local_key, + session_store, sync_oracle: sync_oracle.clone(), force_authoring, threshold: config.threshold(), @@ -221,7 +222,7 @@ struct BabeWorker { client: Arc, block_import: Arc>, env: Arc, - local_key: Arc, + session_store: Arc>, sync_oracle: SO, force_authoring: bool, threshold: u64, @@ -249,7 +250,6 @@ impl SlotWorker for BabeWorker w chain_head: B::Header, slot_info: SlotInfo, ) -> Self::OnSlot { - let pair = self.local_key.clone(); let ref client = self.client; let block_import = self.block_import.clone(); let ref env = self.env; @@ -287,12 +287,12 @@ impl SlotWorker for BabeWorker w return Box::new(future::ok(())); } - let proposal_work = if let Some(((inout, vrf_proof, _batchable_proof), authority_index)) = claim_slot( + let proposal_work = if let Some(((inout, vrf_proof, _batchable_proof), authority_index, pair)) = claim_slot( &randomness, slot_info.number, epoch_index, epoch, - &pair, + &self.session_store, self.threshold, ) { debug!( @@ -325,7 +325,7 @@ impl SlotWorker for BabeWorker w // deadline our production to approx. the end of the slot let remaining_duration = slot_info.remaining_duration(); - Timeout::new( + let timeout = Timeout::new( proposer.propose( slot_info.inherent_data, generic::Digest { @@ -336,7 +336,8 @@ impl SlotWorker for BabeWorker w remaining_duration, ).into_future(), remaining_duration, - ) + ); + (timeout, pair) } else { return Box::new(future::ok(())); }; @@ -488,7 +489,7 @@ fn check_header( } else { let (pre_hash, author) = (header.hash(), &authorities[authority_index as usize]); - if sr25519::Pair::verify(&sig, pre_hash, author.clone()) { + if AuthorityPair::verify(&sig, pre_hash, author.clone()) { let (inout, _batchable_proof) = { let transcript = make_transcript( &randomness, @@ -769,7 +770,7 @@ fn register_babe_inherent_data_provider( } } -fn get_keypair(q: &sr25519::Pair) -> &Keypair { +fn get_keypair(q: &AuthorityPair) -> &Keypair { q.as_ref() } @@ -800,11 +801,20 @@ fn claim_slot( slot_number: u64, epoch: u64, Epoch { ref authorities, .. }: Epoch, - key: &sr25519::Pair, + session_store: &Arc>, threshold: u64, -) -> Option<((VRFInOut, VRFProof, VRFProofBatchable), usize)> { - let public = &key.public(); - let authority_index = authorities.iter().position(|s| &s.0 == public)?; +) -> Option<((VRFInOut, VRFProof, VRFProofBatchable), usize, AuthorityPair)> { + let mut keys = authorities.iter() + .enumerate() + .filter_map(|(index, public)| { + session_store.get_key(public).map(|key| (index, key)) + }) + .collect::>(); + let (index, key) = if keys.len() == 1 { + keys.remove(0) + } else { + return None; + }; let transcript = make_transcript(randomness, slot_number, epoch); // Compute the threshold we will use. @@ -813,9 +823,9 @@ fn claim_slot( // be empty. Therefore, this division is safe. let threshold = threshold / authorities.len() as u64; - get_keypair(key) + get_keypair(&key) .vrf_sign_n_check(transcript, |inout| check(inout, threshold)) - .map(|s|(s, authority_index)) + .map(|vrf| (vrf, index, key)) } fn initialize_authorities_cache(client: &C) -> Result<(), ConsensusError> where From 6d0f725bd271d37ef2eb184bf1b1486d442f8597 Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:29:31 +0200 Subject: [PATCH 05/14] Grandpa handles session key handover. --- core/finality-grandpa/Cargo.toml | 1 + .../src/communication/gossip.rs | 1 - .../src/communication/tests.rs | 1 - core/finality-grandpa/src/environment.rs | 20 +++---- core/finality-grandpa/src/lib.rs | 53 +++++++++++-------- core/finality-grandpa/src/observer.rs | 2 +- core/finality-grandpa/src/tests.rs | 44 ++++++++------- 7 files changed, 65 insertions(+), 57 deletions(-) diff --git a/core/finality-grandpa/Cargo.toml b/core/finality-grandpa/Cargo.toml index 3cbb56df81c11..e1e66ebe7ceb3 100644 --- a/core/finality-grandpa/Cargo.toml +++ b/core/finality-grandpa/Cargo.toml @@ -26,6 +26,7 @@ service = { package = "substrate-service", path = "../service", optional = true srml-finality-tracker = { path = "../../srml/finality-tracker" } fg_primitives = { package = "substrate-finality-grandpa-primitives", path = "primitives" } grandpa = { package = "finality-grandpa", version = "0.8.1", features = ["derive-codec"] } +keystore = { package = "substrate-keystore", path = "../keystore" } [dev-dependencies] grandpa = { package = "finality-grandpa", version = "0.8.1", features = ["derive-codec", "test-helpers"] } diff --git a/core/finality-grandpa/src/communication/gossip.rs b/core/finality-grandpa/src/communication/gossip.rs index dfaa96628f2dc..96f41c05e3a95 100644 --- a/core/finality-grandpa/src/communication/gossip.rs +++ b/core/finality-grandpa/src/communication/gossip.rs @@ -1239,7 +1239,6 @@ mod tests { crate::Config { gossip_duration: Duration::from_millis(10), justification_period: 256, - local_key: None, name: None, } } diff --git a/core/finality-grandpa/src/communication/tests.rs b/core/finality-grandpa/src/communication/tests.rs index 5760b3936cd98..13fd07e82ff53 100644 --- a/core/finality-grandpa/src/communication/tests.rs +++ b/core/finality-grandpa/src/communication/tests.rs @@ -133,7 +133,6 @@ fn config() -> crate::Config { crate::Config { gossip_duration: std::time::Duration::from_millis(10), justification_period: 256, - local_key: None, name: None, } } diff --git a/core/finality-grandpa/src/environment.rs b/core/finality-grandpa/src/environment.rs index e3189ecd92c95..eb93a4884087a 100644 --- a/core/finality-grandpa/src/environment.rs +++ b/core/finality-grandpa/src/environment.rs @@ -310,6 +310,7 @@ pub(crate) struct Environment, RA, SC> { pub(crate) select_chain: SC, pub(crate) voters: Arc>, pub(crate) config: Config, + pub(crate) local_key: Option, pub(crate) authority_set: SharedAuthoritySet>, pub(crate) consensus_changes: SharedConsensusChanges>, pub(crate) network: crate::communication::NetworkBridge, @@ -498,14 +499,13 @@ where let prevote_timer = Delay::new(now + self.config.gossip_duration * 2); let precommit_timer = Delay::new(now + self.config.gossip_duration * 4); - let local_key = self.config.local_key.as_ref() - .filter(|pair| self.voters.contains_key(&pair.public().into())); + let local_id = self.local_key.as_ref().map(|pair| pair.public().clone()); let (incoming, outgoing) = self.network.round_communication( crate::communication::Round(round), crate::communication::SetId(self.set_id), self.voters.clone(), - local_key.cloned(), + self.local_key.clone().map(Arc::new), self.voter_set_state.has_voted(), ); @@ -521,7 +521,7 @@ where let outgoing = Box::new(outgoing.sink_map_err(Into::into)); voter::RoundData { - voter_id: self.config.local_key.as_ref().map(|pair| pair.public().clone()), + voter_id: local_id, prevote_timer: Box::new(prevote_timer.map_err(|e| Error::Timer(e).into())), precommit_timer: Box::new(precommit_timer.map_err(|e| Error::Timer(e).into())), incoming, @@ -530,9 +530,7 @@ where } fn proposed(&self, _round: u64, propose: PrimaryPropose) -> Result<(), Self::Error> { - let local_id = self.config.local_key.as_ref() - .map(|pair| pair.public().into()) - .filter(|id| self.voters.contains_key(&id)); + let local_id = self.local_key.as_ref().map(|pair| pair.public().clone()); let local_id = match local_id { Some(id) => id, @@ -569,9 +567,7 @@ where } fn prevoted(&self, _round: u64, prevote: Prevote) -> Result<(), Self::Error> { - let local_id = self.config.local_key.as_ref() - .map(|pair| pair.public().into()) - .filter(|id| self.voters.contains_key(&id)); + let local_id = self.local_key.as_ref().map(|pair| pair.public().clone()); let local_id = match local_id { Some(id) => id, @@ -611,9 +607,7 @@ where } fn precommitted(&self, _round: u64, precommit: Precommit) -> Result<(), Self::Error> { - let local_id = self.config.local_key.as_ref() - .map(|pair| pair.public().into()) - .filter(|id| self.voters.contains_key(&id)); + let local_id = self.local_key.as_ref().map(|pair| pair.public().clone()); let local_id = match local_id { Some(id) => id, diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index 534a5d9256036..1fc3f8ed21f2d 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -63,9 +63,10 @@ use runtime_primitives::traits::{ }; use fg_primitives::GrandpaApi; use inherents::InherentDataProviders; +use keystore::LocalStore; use runtime_primitives::generic::BlockId; use consensus_common::SelectChain; -use substrate_primitives::{ed25519, H256, Pair, Blake2Hasher}; +use substrate_primitives::{ed25519, H256, Blake2Hasher}; use substrate_telemetry::{telemetry, CONSENSUS_INFO, CONSENSUS_DEBUG, CONSENSUS_WARN}; use serde_json; @@ -198,8 +199,6 @@ pub struct Config { /// at least every justification_period blocks. There are some other events which might cause /// justification generation. pub justification_period: u32, - /// The local signing key. - pub local_key: Option>, /// Some local identifier of the voter. pub name: Option, } @@ -396,7 +395,7 @@ where } fn global_communication, B, E, N, RA>( - local_key: Option<&Arc>, + is_voter: bool, set_id: u64, voters: &Arc>, client: &Arc>, @@ -417,11 +416,6 @@ fn global_communication, B, E, N, RA>( RA: Send + Sync, NumberFor: BlockNumberOps, { - - let is_voter = local_key - .map(|pair| voters.contains_key(&pair.public().into())) - .unwrap_or(false); - // verification stream let (global_in, global_out) = network.global_communication( communication::SetId(set_id), @@ -475,6 +469,8 @@ fn register_finality_tracker_inherent_data_provider, N, RA, SC, X> { /// Configuration for the GRANDPA service. pub config: Config, + /// The session store. + pub session_store: Arc>, /// A link to the block import worker. pub link: LinkHalf, /// The Network instance. @@ -505,6 +501,7 @@ pub fn run_grandpa_voter, N, RA, SC, X>( { let GrandpaParams { config, + session_store, link, network, inherent_data_providers, @@ -556,9 +553,14 @@ pub fn run_grandpa_voter, N, RA, SC, X>( }; let voters = authority_set.current_authorities(); + let local_key = session_store + .get_keys(|public| voters.contains_key(public)) + .into_iter() + .next(); let initial_environment = Arc::new(Environment { inner: client.clone(), config: config.clone(), + local_key, select_chain: select_chain.clone(), voters: Arc::new(voters), network: network.clone(), @@ -571,21 +573,16 @@ pub fn run_grandpa_voter, N, RA, SC, X>( initial_environment.update_voter_set_state(|voter_set_state| { match voter_set_state { VoterSetState::Live { current_round: HasVoted::Yes(id, _), completed_rounds } => { - let local_id = config.local_key.clone().map(|pair| pair.public()); - let has_voted = match local_id { - Some(local_id) => if *id == local_id { - // keep the previous votes - return Ok(None); - } else { - HasVoted::No - }, - _ => HasVoted::No, - }; + let has_voted = session_store.get_key(id).is_some(); + if has_voted { + // keep the previous votes + return Ok(None); + } // NOTE: only updated on disk when the voter first // proposes/prevotes/precommits or completes a round. Ok(Some(VoterSetState::Live { - current_round: has_voted, + current_round: HasVoted::No, completed_rounds: completed_rounds.clone(), })) }, @@ -610,8 +607,12 @@ pub fn run_grandpa_voter, N, RA, SC, X>( chain_info.chain.finalized_number, ); + let is_voter = session_store + .get_keys(|public| env.voters.contains_key(public)) + .len() > 0; + let global_comms = global_communication( - config.local_key.as_ref(), + is_voter, env.set_id, &env.voters, &client, @@ -642,6 +643,7 @@ pub fn run_grandpa_voter, N, RA, SC, X>( let client = client.clone(); let config = config.clone(); + let session_store = session_store.clone(); let network = network.clone(); let select_chain = select_chain.clone(); let authority_set = authority_set.clone(); @@ -683,12 +685,17 @@ pub fn run_grandpa_voter, N, RA, SC, X>( aux_schema::write_voter_set_state(&**client.backend(), &set_state)?; let set_state: SharedVoterSetState<_> = set_state.into(); - + let voters: VoterSet<_> = new.authorities.into_iter().collect(); + let local_key = session_store + .get_keys(|public| voters.contains_key(public)) + .into_iter() + .next(); let env = Arc::new(Environment { inner: client, select_chain, config, - voters: Arc::new(new.authorities.into_iter().collect()), + local_key, + voters: Arc::new(voters), set_id: new.set_id, network, authority_set, diff --git a/core/finality-grandpa/src/observer.rs b/core/finality-grandpa/src/observer.rs index 2c0818c2d7095..6a2baa93022ca 100644 --- a/core/finality-grandpa/src/observer.rs +++ b/core/finality-grandpa/src/observer.rs @@ -185,7 +185,7 @@ pub fn run_grandpa_observer, N, RA, SC>( // start global communication stream for the current set let (global_in, _) = global_communication( - None, + false, set_id, &voters, &client, diff --git a/core/finality-grandpa/src/tests.rs b/core/finality-grandpa/src/tests.rs index 698996995f450..9ca66d523bb78 100644 --- a/core/finality-grandpa/src/tests.rs +++ b/core/finality-grandpa/src/tests.rs @@ -24,6 +24,7 @@ use parking_lot::Mutex; use futures03::{StreamExt as _, TryStreamExt as _}; use tokio::runtime::current_thread; use keyring::ed25519::{Keyring as AuthorityKeyring}; +use keystore::{Store, LocalStore}; use client::{ error::Result, runtime_api::{Core, RuntimeVersion, ApiExt}, @@ -38,7 +39,7 @@ use parity_codec::Decode; use runtime_primitives::traits::{ApiRef, ProvideRuntimeApi, Header as HeaderT}; use runtime_primitives::generic::BlockId; use substrate_primitives::{NativeOrEncoded, ExecutionContext}; -use fg_primitives::AuthorityId; +use fg_primitives::{AuthorityId, AuthorityPair}; use authorities::AuthoritySet; use finality_proof::{FinalityProofProvider, AuthoritySetForFinalityProver, AuthoritySetForFinalityChecker}; @@ -342,7 +343,16 @@ impl AuthoritySetForFinalityChecker for TestApi { const TEST_GOSSIP_DURATION: Duration = Duration::from_millis(500); -fn make_ids(keys: &[AuthorityKeyring]) -> Vec<(substrate_primitives::ed25519::Public, u64)> { +fn session_store_from_keys(keys: Vec) -> LocalStore { + let store = Store::new(); + let session_store = store.create_local_store(); + for key in keys { + store.set_key(&key); + } + session_store +} + +fn make_ids(keys: &[AuthorityKeyring]) -> Vec<(AuthorityId, u64)> { keys.iter() .map(|key| AuthorityId::from_raw(key.to_raw_public())) .map(|id| (id, 1)) @@ -405,9 +415,9 @@ fn run_to_completion_with( config: Config { gossip_duration: TEST_GOSSIP_DURATION, justification_period: 32, - local_key: Some(Arc::new(key.clone().into())), name: Some(format!("peer#{}", peer_id)), }, + session_store: session_store_from_keys(vec![key.clone().into()]), link: link, network: net_service, inherent_data_providers: InherentDataProviders::new(), @@ -482,10 +492,10 @@ fn finalize_3_voters_1_full_observer() { let all_peers = peers.iter() .cloned() - .map(|key| Some(Arc::new(key.into()))) - .chain(::std::iter::once(None)); + .map(|key| vec![key.into()]) + .chain(vec![]); - for (peer_id, local_key) in all_peers.enumerate() { + for (peer_id, keys) in all_peers.enumerate() { let (client, net_service, link) = { let net = net.lock(); let link = net.peers[peer_id].data.lock().take().expect("link initialized at startup; qed"); @@ -506,9 +516,9 @@ fn finalize_3_voters_1_full_observer() { config: Config { gossip_duration: TEST_GOSSIP_DURATION, justification_period: 32, - local_key, name: Some(format!("peer#{}", peer_id)), }, + session_store: session_store_from_keys(keys), link: link, network: net_service, inherent_data_providers: InherentDataProviders::new(), @@ -640,10 +650,10 @@ fn transition_3_voters_twice_1_full_observer() { .cloned() .collect::>() // deduplicate .into_iter() - .map(|key| Some(Arc::new(key.into()))) + .map(|key| vec![key.into()]) .enumerate(); - for (peer_id, local_key) in all_peers { + for (peer_id, keys) in all_peers { let (client, net_service, link) = { let net = net.lock(); let link = net.peers[peer_id].data.lock().take().expect("link initialized at startup; qed"); @@ -674,9 +684,9 @@ fn transition_3_voters_twice_1_full_observer() { config: Config { gossip_duration: TEST_GOSSIP_DURATION, justification_period: 32, - local_key, name: Some(format!("peer#{}", peer_id)), }, + session_store: session_store_from_keys(keys), link: link, network: net_service, inherent_data_providers: InherentDataProviders::new(), @@ -1089,9 +1099,9 @@ fn voter_persists_its_votes() { config: Config { gossip_duration: TEST_GOSSIP_DURATION, justification_period: 32, - local_key: Some(Arc::new(peers[0].clone().into())), name: Some(format!("peer#{}", 0)), }, + session_store: session_store_from_keys(vec![peers[0].clone().into()]), link: link, network: net.lock().peers[0].network_service().clone(), inherent_data_providers: InherentDataProviders::new(), @@ -1141,7 +1151,6 @@ fn voter_persists_its_votes() { let config = Config { gossip_duration: TEST_GOSSIP_DURATION, justification_period: 32, - local_key: Some(Arc::new(peers[1].clone().into())), name: Some(format!("peer#{}", 1)), }; @@ -1164,7 +1173,7 @@ fn voter_persists_its_votes() { communication::Round(1), communication::SetId(0), Arc::new(VoterSet::from_iter(voters)), - Some(config.local_key.unwrap()), + Some(Arc::new(peers[1].clone().into())), HasVoted::No, ); @@ -1290,7 +1299,6 @@ fn finalize_3_voters_1_light_observer() { Config { gossip_duration: TEST_GOSSIP_DURATION, justification_period: 32, - local_key: None, name: Some("observer".to_string()), }, link, @@ -1411,14 +1419,14 @@ fn voter_catches_up_to_latest_round_when_behind() { let net = Arc::new(Mutex::new(net)); let mut finality_notifications = Vec::new(); - let voter = |local_key, peer_id, link, net: Arc>| -> Box + Send> { + let voter = |keys, peer_id, link, net: Arc>| -> Box + Send> { let grandpa_params = GrandpaParams { config: Config { gossip_duration: TEST_GOSSIP_DURATION, justification_period: 32, - local_key, name: Some(format!("peer#{}", peer_id)), }, + session_store: session_store_from_keys(keys), link: link, network: net.lock().peer(peer_id).network_service().clone(), inherent_data_providers: InherentDataProviders::new(), @@ -1447,7 +1455,7 @@ fn voter_catches_up_to_latest_round_when_behind() { .for_each(move |_| Ok(())) ); - let voter = voter(Some(Arc::new((*key).into())), peer_id, link, net.clone()); + let voter = voter(vec![(*key).into()], peer_id, link, net.clone()); runtime.spawn(voter); } @@ -1483,7 +1491,7 @@ fn voter_catches_up_to_latest_round_when_behind() { .collect() .map(|_| set_state); - let voter = voter(None, peer_id, link, net); + let voter = voter(vec![], peer_id, link, net); runtime.spawn(voter).unwrap(); From 11d59042a0c0fdfe4df0882867710acb7d83178c Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:34:09 +0200 Subject: [PATCH 06/14] Update service. --- core/service/src/components.rs | 5 +- core/service/src/config.rs | 28 ++++---- core/service/src/lib.rs | 119 +++++---------------------------- core/service/test/src/lib.rs | 13 ++-- 4 files changed, 38 insertions(+), 127 deletions(-) diff --git a/core/service/src/components.rs b/core/service/src/components.rs index e81380b3202fd..7e216fdbaf873 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -23,6 +23,7 @@ use client_db; use client::{self, Client, runtime_api}; use crate::{error, Service, AuthorityKeyProvider}; use consensus_common::{import_queue::ImportQueue, SelectChain}; +use keystore::Store; use network::{self, OnDemand, FinalityProofProvider, NetworkStateInfo, config::BoxFinalityProofRequestBuilder}; use substrate_executor::{NativeExecutor, NativeExecutionDispatch}; use transaction_pool::txpool::{self, Options as TransactionPoolOptions, Pool as TransactionPool}; @@ -159,6 +160,7 @@ pub trait StartRPC { system_info: SystemInfo, task_executor: TaskExecutor, transaction_pool: Arc>, + keystore: Option, ) -> rpc::RpcHandler; } @@ -172,11 +174,12 @@ impl StartRPC for C where rpc_system_info: SystemInfo, task_executor: TaskExecutor, transaction_pool: Arc>, + keystore: Option, ) -> rpc::RpcHandler { let subscriptions = rpc::apis::Subscriptions::new(task_executor.clone()); let chain = rpc::apis::chain::Chain::new(client.clone(), subscriptions.clone()); let state = rpc::apis::state::State::new(client.clone(), subscriptions.clone()); - let author = rpc::apis::author::Author::new(client, transaction_pool, subscriptions); + let author = rpc::apis::author::Author::new(client, transaction_pool, subscriptions, keystore); let system = rpc::apis::system::System::new(rpc_system_info, system_send_back); rpc::rpc_handler::, ComponentExHash, _, _, _, _>( state, diff --git a/core/service/src/config.rs b/core/service/src/config.rs index 6f2f51ebfb6ea..8497de91d5dcf 100644 --- a/core/service/src/config.rs +++ b/core/service/src/config.rs @@ -40,12 +40,14 @@ pub struct Configuration { pub impl_commit: &'static str, /// Node roles. pub roles: Roles, + /// AURA enabled. + pub aura: bool, + /// GRANDPA voter enabled. + pub grandpa_voter: bool, /// Extrinsic pool configuration. pub transaction_pool: transaction_pool::txpool::Options, /// Network configuration. pub network: NetworkConfiguration, - /// Path to key files. - pub keystore_path: Option, /// Path to the database. pub database_path: PathBuf, /// Cache Size for internal database in MiB @@ -56,8 +58,8 @@ pub struct Configuration { pub state_cache_child_ratio: Option, /// Pruning settings. pub pruning: PruningMode, - /// Additional key seeds. - pub keys: Vec, + /// Key seed for authority keys. + pub key: Option, /// Chain configuration. pub chain_spec: ChainSpec, /// Custom configuration. @@ -83,15 +85,10 @@ pub struct Configuration { pub default_heap_pages: Option, /// Should offchain workers be executed. pub offchain_worker: bool, + /// Password for offchain worker. + pub offchain_worker_password: Protected, /// Enable authoring even when offline. pub force_authoring: bool, - /// Disable GRANDPA when running in validator mode - pub disable_grandpa: bool, - /// Run GRANDPA voter even when no additional key seed is specified. This can for example be of interest when - /// running a sentry node in front of a validator, thus needing to forward GRANDPA gossip messages. - pub grandpa_voter: bool, - /// Node keystore's password - pub password: Protected, } impl Configuration { @@ -104,14 +101,15 @@ impl Configuration Configuration { NetworkStatus>, NetworkState )>>>>, transaction_pool: Arc>, - keystore: ComponentAuthorityKeyProvider, exit: ::exit_future::Exit, signal: Option, /// Sender for futures that must be spawned as background tasks. @@ -172,40 +171,7 @@ impl Service { // Create client let executor = NativeExecutor::new(config.default_heap_pages); - let mut keystore = if let Some(keystore_path) = config.keystore_path.as_ref() { - match Keystore::open(keystore_path.clone()) { - Ok(ks) => Some(ks), - Err(err) => { - error!("Failed to initialize keystore: {}", err); - None - } - } - } else { - None - }; - - // Keep the public key for telemetry - let public_key: String; - - // This is meant to be for testing only - // FIXME #1063 remove this - if let Some(keystore) = keystore.as_mut() { - for seed in &config.keys { - keystore.generate_from_seed::(seed)?; - } - - public_key = match keystore.contents::()?.get(0) { - Some(public_key) => public_key.to_string(), - None => { - let key: ed25519::Pair = keystore.generate(&config.password.as_ref())?; - let public_key = key.public(); - info!("Generated a new keypair: {:?}", public_key); - public_key.to_string() - } - } - } else { - public_key = format!(""); - } + let keystore = Store::new(); let (client, on_demand) = Components::build_client(&config, executor)?; let select_chain = Components::build_select_chain(&mut config, client.clone())?; @@ -268,11 +234,7 @@ impl Service { let keystore_authority_key = AuthorityKeyProvider { _marker: PhantomData, - roles: config.roles, - password: config.password.clone(), - keystore: keystore.map(Arc::new), }; - #[allow(deprecated)] let offchain_storage = client.backend().offchain_storage(); let offchain_workers = match (config.offchain_worker, offchain_storage) { @@ -280,8 +242,8 @@ impl Service { Some(Arc::new(offchain::OffchainWorkers::new( client.clone(), db, - keystore_authority_key.clone(), - config.password.clone(), + keystore_authority_key, + config.offchain_worker_password.clone(), ))) }, (true, None) => { @@ -420,6 +382,7 @@ impl Service { system_info.clone(), Arc::new(SpawnTaskHandle { sender: to_spawn_tx.clone() }), transaction_pool.clone(), + Some(keystore.clone()), ) }; let rpc_handlers = gen_handler(); @@ -440,12 +403,13 @@ impl Service { // Telemetry let telemetry = config.telemetry_endpoints.clone().map(|endpoints| { - let is_authority = config.roles == Roles::AUTHORITY; let network_id = network.local_peer_id().to_base58(); let name = config.name.clone(); let impl_name = config.impl_name.to_owned(); let version = version.clone(); let chain_name = config.chain_spec.name().to_owned(); + let aura = config.aura.clone(); + let grandpa_voter = config.grandpa_voter.clone(); let telemetry_connection_sinks_ = telemetry_connection_sinks.clone(); let telemetry = tel::init_telemetry(tel::TelemetryConfig { endpoints, @@ -463,9 +427,9 @@ impl Service { "implementation" => impl_name.clone(), "version" => version.clone(), "config" => "", + "aura" => aura, + "grandpa_voter" => grandpa_voter, "chain" => chain_name.clone(), - "pubkey" => &public_key, - "authority" => is_authority, "network_id" => network_id.clone() ); @@ -490,7 +454,7 @@ impl Service { to_spawn_tx, to_spawn_rx, to_poll: Vec::new(), - keystore: keystore_authority_key, + keystore, config, exit, rpc_handlers, @@ -501,20 +465,6 @@ impl Service { }) } - /// give the authority key, if we are an authority and have a key - pub fn authority_key(&self) -> Option> { - use offchain::AuthorityKeyProvider; - - self.keystore.authority_key(&BlockId::Number(Zero::zero())) - } - - /// give the authority key, if we are an authority and have a key - pub fn fg_authority_key(&self) -> Option> { - use offchain::AuthorityKeyProvider; - - self.keystore.fg_authority_key(&BlockId::Number(Zero::zero())) - } - /// return a shared instance of Telemetry (if enabled) pub fn telemetry(&self) -> Option { self._telemetry.as_ref().map(|t| t.clone()) @@ -547,6 +497,11 @@ impl Service { self.rpc_handlers.handle_request(request, mem.metadata.clone()) } + /// Get store. + pub fn store(&self) -> &Store { + &self.keystore + } + /// Get shared client instance. pub fn client(&self) -> Arc> { self.client.clone() @@ -923,49 +878,11 @@ where type FinalityPair = FinalityPair; fn authority_key(&self, _at: &BlockId) -> Option { - if self.roles != Roles::AUTHORITY { - return None - } - - let keystore = match self.keystore { - Some(ref keystore) => keystore, - None => return None - }; - - let loaded_key = keystore - .contents() - .map(|keys| keys.get(0) - .map(|k| keystore.load(k, self.password.as_ref())) - ); - - if let Ok(Some(Ok(key))) = loaded_key { - Some(key) - } else { - None - } + None } fn fg_authority_key(&self, _at: &BlockId) -> Option { - if self.roles != Roles::AUTHORITY { - return None - } - - let keystore = match self.keystore { - Some(ref keystore) => keystore, - None => return None - }; - - let loaded_key = keystore - .contents() - .map(|keys| keys.get(0) - .map(|k| keystore.load(k, self.password.as_ref())) - ); - - if let Ok(Some(Ok(key))) = loaded_key { - Some(key) - } else { - None - } + None } } diff --git a/core/service/test/src/lib.rs b/core/service/test/src/lib.rs index 353b326a91611..3f2cd9994a63d 100644 --- a/core/service/test/src/lib.rs +++ b/core/service/test/src/lib.rs @@ -135,10 +135,6 @@ fn node_config ( ) -> FactoryFullConfiguration { let root = root.path().join(format!("node-{}", index)); - let mut keys = Vec::new(); - if let Some(seed) = key_seed { - keys.push(seed); - } let config_path = Some(String::from(root.join("network").to_str().unwrap())); let net_config_path = config_path.clone(); @@ -171,15 +167,16 @@ fn node_config ( impl_version: "0.1", impl_commit: "", roles: role, + aura: role == Roles::AUTHORITY, + grandpa_voter: role == Roles::AUTHORITY, transaction_pool: Default::default(), network: network_config, - keystore_path: Some(root.join("key")), database_path: root.join("db"), database_cache_size: None, state_cache_size: 16777216, state_cache_child_ratio: None, pruning: Default::default(), - keys: keys, + key: key_seed, chain_spec: (*spec).clone(), custom: Default::default(), name: format!("Node {}", index), @@ -192,10 +189,8 @@ fn node_config ( telemetry_external_transport: None, default_heap_pages: None, offchain_worker: false, + offchain_worker_password: "".to_string().into(), force_authoring: false, - disable_grandpa: false, - grandpa_voter: false, - password: "".to_string().into(), } } From 6b1c3e32b6d747346abeaedd00a3907167af460a Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:34:38 +0200 Subject: [PATCH 07/14] Update cli. --- core/cli/src/lib.rs | 69 +++++++++++++++++------------------------- core/cli/src/params.rs | 21 +++++-------- 2 files changed, 35 insertions(+), 55 deletions(-) diff --git a/core/cli/src/lib.rs b/core/cli/src/lib.rs index cc31af184ed9c..e92498d7d771e 100644 --- a/core/cli/src/lib.rs +++ b/core/cli/src/lib.rs @@ -361,8 +361,8 @@ fn fill_network_configuration( Ok(()) } -fn input_keystore_password() -> Result { - rpassword::read_password_from_tty(Some("Keystore password: ")) +fn input_password(prompt: &str) -> Result { + rpassword::read_password_from_tty(Some(prompt)) .map_err(|e| format!("{:?}", e)) } @@ -375,9 +375,6 @@ where { let spec = load_spec(&cli.shared_params, spec_factory)?; let mut config = service::Configuration::default_with_spec(spec.clone()); - if cli.interactive_password { - config.password = input_keystore_password()?.into() - } config.impl_name = impl_name; config.impl_commit = version.commit; @@ -401,8 +398,6 @@ where let base_path = base_path(&cli.shared_params, version); - config.keystore_path = cli.keystore_path.or_else(|| Some(keystore_path(&base_path, config.chain_spec.id()))); - config.database_path = db_path(&base_path, config.chain_spec.id()); config.database_cache_size = cli.database_cache_size; config.state_cache_size = cli.state_cache_size; @@ -414,15 +409,6 @@ where ), }; - let role = - if cli.light { - service::Roles::LIGHT - } else if cli.validator || cli.shared_params.dev { - service::Roles::AUTHORITY - } else { - service::Roles::FULL - }; - let exec = cli.execution_strategies; let exec_all_or = |strat: params::ExecutionStrategy| exec.execution.unwrap_or(strat).into(); config.execution_strategies = ExecutionStrategies { @@ -433,19 +419,30 @@ where other: exec_all_or(exec.execution_other), }; - config.offchain_worker = match (cli.offchain_worker, role) { - (params::OffchainWorkerEnabled::WhenValidating, service::Roles::AUTHORITY) => true, + let is_dev = cli.shared_params.dev; + + config.aura = cli.aura || cli.validator || is_dev; + config.grandpa_voter = cli.grandpa_voter || cli.validator || is_dev; + + config.roles = if cli.light { + service::Roles::LIGHT + } else if config.aura { + service::Roles::AUTHORITY + } else { + service::Roles::FULL + }; + + config.offchain_worker = match (cli.offchain_worker, config.aura) { + (params::OffchainWorkerEnabled::WhenValidating, true) => true, (params::OffchainWorkerEnabled::Always, _) => true, (params::OffchainWorkerEnabled::Never, _) => false, (params::OffchainWorkerEnabled::WhenValidating, _) => false, }; - config.roles = role; - config.disable_grandpa = cli.no_grandpa; - config.grandpa_voter = cli.grandpa_voter; - - - let is_dev = cli.shared_params.dev; + if cli.offchain_worker_password { + config.offchain_worker_password = + input_password("Offchain worker password: ")?.into() + } let client_id = config.client_id(); fill_network_configuration( @@ -462,16 +459,12 @@ where cli.pool_config, )?; - if let Some(key) = cli.key { - config.keys.push(key); + config.key = cli.key; + if config.key.is_none() { + config.key = cli.keyring.account.map(|account| format!("//{}", account)); } - - if cli.shared_params.dev && cli.keyring.account.is_none() { - config.keys.push("//Alice".into()); - } - - if let Some(account) = cli.keyring.account { - config.keys.push(format!("//{}", account)); + if is_dev && config.key.is_none() { + config.key = Some("//Alice".into()); } let rpc_interface: &str = if cli.rpc_external { "0.0.0.0" } else { "127.0.0.1" }; @@ -506,7 +499,7 @@ where } // Imply forced authoring on --dev - config.force_authoring = cli.shared_params.dev || cli.force_authoring; + config.force_authoring = cli.force_authoring || is_dev; Ok(config) } @@ -721,14 +714,6 @@ fn parse_address( Ok(address) } -fn keystore_path(base_path: &Path, chain_id: &str) -> PathBuf { - let mut path = base_path.to_owned(); - path.push("chains"); - path.push(chain_id); - path.push("keystore"); - path -} - fn db_path(base_path: &Path, chain_id: &str) -> PathBuf { let mut path = base_path.to_owned(); path.push("chains"); diff --git a/core/cli/src/params.rs b/core/cli/src/params.rs index 78899ccd4cb01..1595612515b13 100644 --- a/core/cli/src/params.rs +++ b/core/cli/src/params.rs @@ -305,24 +305,19 @@ pub struct ExecutionStrategies { /// The `run` command used to run a node. #[derive(Debug, StructOpt, Clone)] pub struct RunCmd { - /// Specify custom keystore path - #[structopt(long = "keystore-path", value_name = "PATH", parse(from_os_str))] - pub keystore_path: Option, - /// Specify additional key seed #[structopt(long = "key", value_name = "STRING")] pub key: Option, - /// Enable validator mode + /// Shortcut for `--aura` and `--grandpa-voter`. #[structopt(long = "validator")] pub validator: bool, - /// Disable GRANDPA when running in validator mode - #[structopt(long = "no-grandpa")] - pub no_grandpa: bool, + /// Enable AURA. + #[structopt(long = "aura")] + pub aura: bool, - /// Run GRANDPA voter even when no additional key seed via `--key` is specified. This can for example be of interest - /// when running a sentry node in front of a validator, thus needing to forward GRANDPA gossip messages. + /// Enable GRANDPA voter. #[structopt(long = "grandpa-voter")] pub grandpa_voter: bool, @@ -421,9 +416,9 @@ pub struct RunCmd { #[structopt(long = "force-authoring")] pub force_authoring: bool, - /// Interactive password for validator key. - #[structopt(short = "i")] - pub interactive_password: bool, + /// Interactive password for offchain worker keys + #[structopt(long = "offchain-worker-password")] + pub offchain_worker_password: bool, } /// Stores all required Cli values for a keyring test account. From 19193d593cac10aaa8c7f4183b85683d12cf709e Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:35:22 +0200 Subject: [PATCH 08/14] Update node. --- node/cli/Cargo.toml | 1 - node/cli/src/service.rs | 78 ++++++++++++++++++++--------------------- 2 files changed, 38 insertions(+), 41 deletions(-) diff --git a/node/cli/Cargo.toml b/node/cli/Cargo.toml index 17efcc400f35b..1c036268a61dc 100644 --- a/node/cli/Cargo.toml +++ b/node/cli/Cargo.toml @@ -30,7 +30,6 @@ grandpa = { package = "substrate-finality-grandpa", path = "../../core/finality- grandpa_primitives = { package = "substrate-finality-grandpa-primitives", path = "../../core/finality-grandpa/primitives" } sr-primitives = { path = "../../core/sr-primitives" } node-executor = { path = "../executor" } -substrate-keystore = { path = "../../core/keystore" } substrate-telemetry = { package = "substrate-telemetry", path = "../../core/telemetry" } structopt = "0.2" transaction-factory = { path = "../../test-utils/transaction-factory" } diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 53d6f927b485b..dabcb24579bee 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -24,9 +24,8 @@ use std::time::Duration; use aura::{import_queue, start_aura, AuraImportQueue, SlotDuration}; use client::{self, LongestChain}; use grandpa::{self, FinalityProofProvider as GrandpaFinalityProofProvider}; +use grandpa_primitives::{AuthorityPair as GrandpaPair}; use node_executor; -use primitives::Pair; -use grandpa_primitives::AuthorityPair as GrandpaPair; use futures::prelude::*; use node_primitives::{AuraPair, Block}; use node_runtime::{GenesisConfig, RuntimeApi}; @@ -39,7 +38,6 @@ use transaction_pool::{self, txpool::{Pool as TransactionPool}}; use inherents::InherentDataProviders; use network::construct_simple_protocol; use substrate_service::construct_service_factory; -use log::info; use substrate_service::TelemetryOnConnect; construct_simple_protocol! { @@ -86,8 +84,12 @@ construct_service_factory! { let (block_import, link_half) = service.config.custom.grandpa_import_setup.take() .expect("Link Half and Block Import are present for Full Services or setup failed before. qed"); - if let Some(aura_key) = service.authority_key() { - info!("Using aura key {}", aura_key.public()); + if service.config.aura { + let local_store = service.store() + .create_local_store::(); + if let Some(seed) = &service.config.key { + service.store().set_key_from_seed::(seed.as_str())?; + } let proposer = Arc::new(substrate_basic_authorship::ProposerFactory { client: service.client(), @@ -100,7 +102,7 @@ construct_service_factory! { let aura = start_aura( SlotDuration::get_or_compute(&*client)?, - Arc::new(aura_key), + local_store, client, select_chain, block_import, @@ -113,44 +115,40 @@ construct_service_factory! { service.spawn_task(Box::new(select)); } - let grandpa_key = if service.config.disable_grandpa { - None - } else { - service.fg_authority_key() - }; - let config = grandpa::Config { - local_key: grandpa_key.map(Arc::new), // FIXME #1578 make this available through chainspec gossip_duration: Duration::from_millis(333), justification_period: 4096, name: Some(service.config.name.clone()) }; - match config.local_key { - None if !service.config.grandpa_voter => { - service.spawn_task(Box::new(grandpa::run_grandpa_observer( - config, - link_half, - service.network(), - service.on_exit(), - )?)); - }, - // Either config.local_key is set, or user forced voter service via `--grandpa-voter` flag. - _ => { - let telemetry_on_connect = TelemetryOnConnect { - telemetry_connection_sinks: service.telemetry_on_connect_stream(), - }; - let grandpa_config = grandpa::GrandpaParams { - config: config, - link: link_half, - network: service.network(), - inherent_data_providers: service.config.custom.inherent_data_providers.clone(), - on_exit: service.on_exit(), - telemetry_on_connect: Some(telemetry_on_connect), - }; - service.spawn_task(Box::new(grandpa::run_grandpa_voter(grandpa_config)?)); - }, + if service.config.grandpa_voter { + let local_store = service.store() + .create_local_store::(); + if let Some(seed) = &service.config.key { + service.store().set_key_from_seed::(seed.as_str())?; + } + + let telemetry_on_connect = TelemetryOnConnect { + telemetry_connection_sinks: service.telemetry_on_connect_stream(), + }; + let grandpa_config = grandpa::GrandpaParams { + config, + session_store: local_store, + link: link_half, + network: service.network(), + inherent_data_providers: service.config.custom.inherent_data_providers.clone(), + on_exit: service.on_exit(), + telemetry_on_connect: Some(telemetry_on_connect), + }; + service.spawn_task(Box::new(grandpa::run_grandpa_voter(grandpa_config)?)); + } else { + service.spawn_task(Box::new(grandpa::run_grandpa_observer( + config, + link_half, + service.network(), + service.on_exit(), + )?)); } Ok(service) @@ -160,7 +158,6 @@ construct_service_factory! { { |config| >::new(config) }, FullImportQueue = AuraImportQueue { |config: &mut FactoryFullConfiguration , client: Arc>, select_chain: Self::SelectChain| { - let slot_duration = SlotDuration::get_or_compute(&*client)?; let (block_import, link_half) = grandpa::block_import::<_, _, _, RuntimeApi, FullClient, _>( client.clone(), client.clone(), select_chain @@ -170,7 +167,7 @@ construct_service_factory! { config.custom.grandpa_import_setup = Some((block_import.clone(), link_half)); import_queue::<_, _, AuraPair>( - slot_duration, + SlotDuration::get_or_compute(&*client)?, Box::new(block_import), Some(Box::new(justification_import)), None, @@ -189,7 +186,8 @@ construct_service_factory! { client.clone(), Arc::new(fetch_checker), client.clone() )?; let finality_proof_import = block_import.clone(); - let finality_proof_request_builder = finality_proof_import.create_finality_proof_request_builder(); + let finality_proof_request_builder = finality_proof_import + .create_finality_proof_request_builder(); import_queue::<_, _, AuraPair>( SlotDuration::get_or_compute(&*client)?, From 2ba9f668ad3349e9d359dd19f9e09c7da50a9761 Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:35:42 +0200 Subject: [PATCH 09/14] Update node-template. --- node-template/src/service.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/node-template/src/service.rs b/node-template/src/service.rs index 2a981c3bcc348..181b8a16f3792 100644 --- a/node-template/src/service.rs +++ b/node-template/src/service.rs @@ -3,7 +3,6 @@ #![warn(unused_extern_crates)] use std::sync::Arc; -use log::info; use transaction_pool::{self, txpool::{Pool as TransactionPool}}; use node_template_runtime::{self, GenesisConfig, opaque::Block, RuntimeApi, WASM_BINARY}; use substrate_service::{ @@ -15,7 +14,7 @@ use basic_authorship::ProposerFactory; use consensus::{import_queue, start_aura, AuraImportQueue, SlotDuration}; use futures::prelude::*; use substrate_client::{self as client, LongestChain}; -use primitives::{ed25519::Pair, Pair as PairT}; +use primitives::ed25519::Pair; use inherents::InherentDataProviders; use network::{config::DummyFinalityProofRequestBuilder, construct_simple_protocol}; use substrate_executor::native_executor_instance; @@ -68,8 +67,12 @@ construct_service_factory! { }, AuthoritySetup = { |service: Self::FullService| { - if let Some(key) = service.authority_key() { - info!("Using authority key {}", key.public()); + if service.config.aura { + let session_store = service.store().create_local_store::(); + if let Some(seed) = &service.config.key { + service.store().set_key_from_seed::(seed.as_str())?; + } + let proposer = Arc::new(ProposerFactory { client: service.client(), transaction_pool: service.transaction_pool(), @@ -79,7 +82,7 @@ construct_service_factory! { .ok_or_else(|| ServiceError::SelectChainRequired)?; let aura = start_aura( SlotDuration::get_or_compute(&*client)?, - Arc::new(key), + session_store, client.clone(), select_chain, client, From 9bd603e1c14a7e3606d204ad7e1139b535ebb3e5 Mon Sep 17 00:00:00 2001 From: David Craven Date: Fri, 12 Jul 2019 14:36:03 +0200 Subject: [PATCH 10/14] Update Cargo.lock --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 310c4114935c0..f8a178669cffe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2273,7 +2273,6 @@ dependencies = [ "substrate-finality-grandpa-primitives 2.0.0", "substrate-inherents 2.0.0", "substrate-keyring 2.0.0", - "substrate-keystore 2.0.0", "substrate-network 2.0.0", "substrate-primitives 2.0.0", "substrate-service 2.0.0", @@ -4474,6 +4473,7 @@ dependencies = [ "substrate-finality-grandpa-primitives 2.0.0", "substrate-inherents 2.0.0", "substrate-keyring 2.0.0", + "substrate-keystore 2.0.0", "substrate-network 2.0.0", "substrate-primitives 2.0.0", "substrate-service 2.0.0", From 5142a676f9dd3d1d70aad86ba393d9087a349029 Mon Sep 17 00:00:00 2001 From: David Craven Date: Sun, 21 Jul 2019 14:35:45 +0200 Subject: [PATCH 11/14] Implement AuthorityKeyProvider. --- core/service/src/lib.rs | 70 +++++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index cc3e9d4781c6d..8167e90f97621 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -41,7 +41,6 @@ use keystore::{LocalStore, Store}; use network::{NetworkState, NetworkStateInfo}; use log::{log, info, warn, debug, error, Level}; use parity_codec::{Encode, Decode}; -use primitives::Pair; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{Header, NumberFor, SaturatedConversion, Zero}; use substrate_executor::NativeExecutor; @@ -85,6 +84,8 @@ pub struct Service { NetworkStatus>, NetworkState )>>>>, transaction_pool: Arc>, + consensus_keystore: Arc>>, + finality_keystore: Arc>>, exit: ::exit_future::Exit, signal: Option, /// Sender for futures that must be spawned as background tasks. @@ -171,8 +172,6 @@ impl Service { // Create client let executor = NativeExecutor::new(config.default_heap_pages); - let keystore = Store::new(); - let (client, on_demand) = Components::build_client(&config, executor)?; let select_chain = Components::build_select_chain(&mut config, client.clone())?; let (import_queue, finality_proof_request_builder) = Components::build_import_queue( @@ -232,9 +231,18 @@ impl Service { let network = network_mut.service().clone(); let network_status_sinks = Arc::new(Mutex::new(Vec::new())); - let keystore_authority_key = AuthorityKeyProvider { + let keystore = Store::new(); + let consensus_keystore = keystore + .create_local_store::(config.key.as_ref()); + let finality_keystore = keystore + .create_local_store::(config.key.as_ref()); + + let authority_key_provider = AuthorityKeyProvider { _marker: PhantomData, + consensus_keystore: consensus_keystore.clone(), + finality_keystore: finality_keystore.clone(), }; + #[allow(deprecated)] let offchain_storage = client.backend().offchain_storage(); let offchain_workers = match (config.offchain_worker, offchain_storage) { @@ -242,7 +250,7 @@ impl Service { Some(Arc::new(offchain::OffchainWorkers::new( client.clone(), db, - keystore_authority_key, + authority_key_provider, config.offchain_worker_password.clone(), ))) }, @@ -454,7 +462,8 @@ impl Service { to_spawn_tx, to_spawn_rx, to_poll: Vec::new(), - keystore, + consensus_keystore, + finality_keystore, config, exit, rpc_handlers, @@ -465,6 +474,20 @@ impl Service { }) } + /// Returns the consensus keystore. + pub fn consensus_keystore( + &self + ) -> Arc>> { + self.consensus_keystore.clone() + } + + /// Returns the finality keystore. + pub fn finality_keystore( + &self + ) -> Arc>> { + self.finality_keystore.clone() + } + /// return a shared instance of Telemetry (if enabled) pub fn telemetry(&self) -> Option { self._telemetry.as_ref().map(|t| t.clone()) @@ -497,11 +520,6 @@ impl Service { self.rpc_handlers.handle_request(request, mem.metadata.clone()) } - /// Get store. - pub fn store(&self) -> &Store { - &self.keystore - } - /// Get shared client instance. pub fn client(&self) -> Arc> { self.client.clone() @@ -861,9 +879,9 @@ impl network::TransactionPool, ComponentBlock< /// A provider of current authority key. pub struct AuthorityKeyProvider { _marker: PhantomData<(Block, ConsensusPair, FinalityPair)>, - roles: Roles, - keystore: Option>, - password: crypto::Protected, + client: Arc>, + consensus_store: Arc>, + finality_store: Arc>, } impl @@ -877,15 +895,33 @@ where type ConsensusPair = ConsensusPair; type FinalityPair = FinalityPair; - fn authority_key(&self, _at: &BlockId) -> Option { - None + fn authority_key(&self, at: &BlockId) -> Option { + let authorities = self.client + .runtime_api() + .authorities(at) + .unwrap_or_default(); + self.consensus_store.get_keys(|public| { + authorities.iter().find(|aid| aid == &public).is_some() + }) + .into_iter() + .next() } fn fg_authority_key(&self, _at: &BlockId) -> Option { - None + let authorities = self.client + .runtime_api() + .grandpa_authorities(at) + .unwrap_or_default(); + self.finality_store.get_keys(|public| { + authorities.iter().find(|(aid, _)| aid == public).is_some() + }) + .into_iter() + .next() } } + + /// Constructs a service factory with the given name that implements the `ServiceFactory` trait. /// The required parameters are required to be given in the exact order. Some parameters are followed /// by `{}` blocks. These blocks are required and used to initialize the given parameter. From 4a0227145ea825705e8bb7985a86a88e6e4c7fb9 Mon Sep 17 00:00:00 2001 From: David Craven Date: Tue, 23 Jul 2019 15:29:18 +0200 Subject: [PATCH 12/14] Fix some stuff. --- core/keystore/src/lib.rs | 4 +-- core/service/src/components.rs | 14 +++++--- core/service/src/lib.rs | 63 +++++++++++++++++++++++++--------- 3 files changed, 57 insertions(+), 24 deletions(-) diff --git a/core/keystore/src/lib.rs b/core/keystore/src/lib.rs index dd3dd9894780a..f25d8a9e19691 100644 --- a/core/keystore/src/lib.rs +++ b/core/keystore/src/lib.rs @@ -179,8 +179,8 @@ mod tests { let key_a1 = ed25519::Pair::from_seed(&[0; 32]); let key_b1 = ed25519::Pair::from_string("//Alice", None).unwrap(); - store.set_key(&key_a1); - store.set_key_from_seed::("//Alice").unwrap(); + store.add_key(&key_a1); + store.add_key_from_seed::("//Alice").unwrap(); let key_a2 = local_store.get_key(&key_a1.public()).unwrap(); let key_b2 = local_store.get_key(&key_b1.public()).unwrap(); assert_eq!(key_a1.to_raw_vec(), key_a2.to_raw_vec()); diff --git a/core/service/src/components.rs b/core/service/src/components.rs index 7e216fdbaf873..6c245f6e7c294 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -23,7 +23,7 @@ use client_db; use client::{self, Client, runtime_api}; use crate::{error, Service, AuthorityKeyProvider}; use consensus_common::{import_queue::ImportQueue, SelectChain}; -use keystore::Store; +use keystore::{LocalStore, Store}; use network::{self, OnDemand, FinalityProofProvider, NetworkStateInfo, config::BoxFinalityProofRequestBuilder}; use substrate_executor::{NativeExecutor, NativeExecutionDispatch}; use transaction_pool::txpool::{self, Options as TransactionPoolOptions, Pool as TransactionPool}; @@ -136,8 +136,12 @@ pub type ComponentConsensusPair = <::Factory as ServiceFacto pub type ComponentFinalityPair = <::Factory as ServiceFactory>::FinalityPair; /// AuthorityKeyProvider type for `Components` -pub type ComponentAuthorityKeyProvider = - AuthorityKeyProvider, ComponentConsensusPair, ComponentFinalityPair>; +pub type ComponentAuthorityKeyProvider = AuthorityKeyProvider< + ComponentClient, + ComponentBlock, + LocalStore>, + LocalStore> +>; /// Extrinsic hash type for `Components` pub type ComponentExHash = <::TransactionPoolApi as txpool::ChainApi>::Hash; @@ -160,7 +164,7 @@ pub trait StartRPC { system_info: SystemInfo, task_executor: TaskExecutor, transaction_pool: Arc>, - keystore: Option, + keystore: Option>, ) -> rpc::RpcHandler; } @@ -174,7 +178,7 @@ impl StartRPC for C where rpc_system_info: SystemInfo, task_executor: TaskExecutor, transaction_pool: Arc>, - keystore: Option, + keystore: Option>, ) -> rpc::RpcHandler { let subscriptions = rpc::apis::Subscriptions::new(task_executor.clone()); let chain = rpc::apis::chain::Chain::new(client.clone(), subscriptions.clone()); diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 8167e90f97621..61f1ad0848511 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -41,8 +41,11 @@ use keystore::{LocalStore, Store}; use network::{NetworkState, NetworkStateInfo}; use log::{log, info, warn, debug, error, Level}; use parity_codec::{Encode, Decode}; +use primitives::Pair; use runtime_primitives::generic::BlockId; -use runtime_primitives::traits::{Header, NumberFor, SaturatedConversion, Zero}; +use runtime_primitives::traits::{ + Header, NumberFor, ProvideRuntimeApi, SaturatedConversion +}; use substrate_executor::NativeExecutor; use sysinfo::{get_current_pid, ProcessExt, System, SystemExt}; use tel::{telemetry, SUBSTRATE_INFO}; @@ -231,16 +234,27 @@ impl Service { let network = network_mut.service().clone(); let network_status_sinks = Arc::new(Mutex::new(Vec::new())); - let keystore = Store::new(); + let keystore = Arc::new(Store::new()); + let consensus_keystore = keystore - .create_local_store::(config.key.as_ref()); + .create_local_store::>(); + if let Some(seed) = config.key { + keystore.add_key_from_seed::>( + seed.as_str()); + } + let finality_keystore = keystore - .create_local_store::(config.key.as_ref()); + .create_local_store::>(); + if let Some(seed) = config.key { + keystore.add_key_from_seed::>( + seed.as_str()); + } let authority_key_provider = AuthorityKeyProvider { _marker: PhantomData, - consensus_keystore: consensus_keystore.clone(), - finality_keystore: finality_keystore.clone(), + client: client.clone(), + consensus_store: consensus_keystore.clone(), + finality_store: finality_keystore.clone(), }; #[allow(deprecated)] @@ -875,22 +889,37 @@ impl network::TransactionPool, ComponentBlock< } } -#[derive(Clone)] /// A provider of current authority key. -pub struct AuthorityKeyProvider { - _marker: PhantomData<(Block, ConsensusPair, FinalityPair)>, - client: Arc>, - consensus_store: Arc>, - finality_store: Arc>, +pub struct AuthorityKeyProvider { + _marker: PhantomData, + client: Arc, + consensus_store: Arc, + finality_store: Arc, +} + +impl Clone for AuthorityKeyProvider { + fn clone(&self) -> Self { + Self { + _marker: self._marker.clone(), + client: self.client.clone(), + consensus_store: self.consensus_store.clone(), + finality_store: self.finality_store.clone(), + } + } } -impl +impl offchain::AuthorityKeyProvider - for AuthorityKeyProvider + for AuthorityKeyProvider< + Client, + Block, + LocalStore, + LocalStore> where + Client: ProvideRuntimeApi + 'static, Block: runtime_primitives::traits::Block, - ConsensusPair: Pair, - FinalityPair: Pair, + ConsensusPair: Pair + Clone, + FinalityPair: Pair + Clone, { type ConsensusPair = ConsensusPair; type FinalityPair = FinalityPair; @@ -907,7 +936,7 @@ where .next() } - fn fg_authority_key(&self, _at: &BlockId) -> Option { + fn fg_authority_key(&self, at: &BlockId) -> Option { let authorities = self.client .runtime_api() .grandpa_authorities(at) From 16b8903b228562d9ba7ad888a741a49e8d1938c8 Mon Sep 17 00:00:00 2001 From: David Craven Date: Tue, 23 Jul 2019 18:31:37 +0200 Subject: [PATCH 13/14] Refactor offchain api. --- core/executor/src/wasm_executor.rs | 5 +- core/offchain/src/api.rs | 217 +++++++++-------------------- core/offchain/src/lib.rs | 124 +++++++++++------ core/offchain/src/testing.rs | 4 +- core/primitives/src/crypto.rs | 25 ++++ core/primitives/src/offchain.rs | 55 ++------ core/sr-io/src/lib.rs | 5 +- core/sr-io/with_std.rs | 4 +- core/sr-io/without_std.rs | 5 +- core/state-machine/src/lib.rs | 5 +- 10 files changed, 206 insertions(+), 243 deletions(-) diff --git a/core/executor/src/wasm_executor.rs b/core/executor/src/wasm_executor.rs index 30d5ccd542581..817fdfa190ad0 100644 --- a/core/executor/src/wasm_executor.rs +++ b/core/executor/src/wasm_executor.rs @@ -711,10 +711,7 @@ impl_function_executor!(this: FunctionExecutor<'e, E>, Ok(if res.is_ok() { 0 } else { 1 }) }, - ext_new_crypto_key(crypto: u32) -> u64 => { - let kind = offchain::CryptoKind::try_from(crypto) - .map_err(|_| "crypto kind OOB while ext_new_crypto_key: wasm")?; - + ext_new_crypto_key(kind: u32) -> u64 => { let res = this.ext.offchain() .map(|api| api.new_crypto_key(kind)) .ok_or_else(|| "Calling unavailable API ext_new_crypto_key: wasm")?; diff --git a/core/offchain/src/api.rs b/core/offchain/src/api.rs index 949602f3f7243..47be4793662a3 100644 --- a/core/offchain/src/api.rs +++ b/core/offchain/src/api.rs @@ -22,7 +22,7 @@ use std::{ thread::sleep, }; use client::backend::OffchainStorage; -use crate::AuthorityKeyProvider; +use crate::{AuthorityKeyProviders, OffchainKey}; use futures::{Stream, Future, sync::mpsc}; use log::{info, debug, warn, error}; use parity_codec::{Encode, Decode}; @@ -30,11 +30,10 @@ use primitives::offchain::{ Timestamp, HttpRequestId, HttpRequestStatus, HttpError, Externalities as OffchainExt, - CryptoKind, CryptoKey, - StorageKind, + CryptoKey, StorageKind, OpaqueNetworkState, OpaquePeerId, OpaqueMultiaddr, }; -use primitives::crypto::{Pair, Public, Protected}; +use primitives::crypto::{key_types, KeyTypeId, Pair, Protected}; use primitives::{ed25519, sr25519}; use runtime_primitives::{ generic::BlockId, @@ -52,128 +51,50 @@ enum ExtMessage { /// A persisted key seed. #[derive(Encode, Decode)] struct StoredKey { - kind: CryptoKind, + kind: KeyTypeId, phrase: String, } impl StoredKey { - fn generate_with_phrase(kind: CryptoKind, password: Option<&str>) -> Self { + fn generate_with_phrase( + kind: KeyTypeId, + password: Option<&str>, + ) -> Result { match kind { - CryptoKind::Ed25519 => { + key_types::ED25519 => { let phrase = ed25519::Pair::generate_with_phrase(password).1; - Self { kind, phrase } + Ok(Self { kind, phrase }) } - CryptoKind::Sr25519 => { + key_types::SR25519 => { let phrase = sr25519::Pair::generate_with_phrase(password).1; - Self { kind, phrase } + Ok(Self { kind, phrase }) + } + _ => { + error!("Unknown key type {:?}", kind); + Err(()) } } } - fn to_local_key(&self, password: Option<&str>) -> Result { - match self.kind { - CryptoKind::Ed25519 => { - ed25519::Pair::from_phrase(&self.phrase, password) - .map(|x| LocalKey::Ed25519(x.0)) - } - CryptoKind::Sr25519 => { - sr25519::Pair::from_phrase(&self.phrase, password) - .map(|x| LocalKey::Sr25519(x.0)) - } - } - .map_err(|e| { + fn to_offchain_key(&self, password: Option<&str>) -> Result, ()> { + let err = |e| { warn!("Error recovering Offchain Worker key. Password invalid? {:?}", e); () - }) - } -} - -enum LocalKey { - Ed25519(ed25519::Pair), - Sr25519(sr25519::Pair), -} - -impl LocalKey { - fn public(&self) -> Result, ()> { - match self { - LocalKey::Ed25519(pair) => Ok(pair.public().to_raw_vec()), - LocalKey::Sr25519(pair) => Ok(pair.public().to_raw_vec()), - } - } - - fn sign(&self, data: &[u8]) -> Result, ()> { - match self { - LocalKey::Ed25519(pair) => { - let sig = pair.sign(data); - let bytes: &[u8] = sig.as_ref(); - Ok(bytes.to_vec()) - } - LocalKey::Sr25519(pair) => { - let sig = pair.sign(data); - let bytes: &[u8] = sig.as_ref(); - Ok(bytes.to_vec()) - } - } - } - - fn verify(&self, msg: &[u8], signature: &[u8]) -> Result { - match self { - LocalKey::Ed25519(pair) => { - Ok(ed25519::Pair::verify_weak(signature, msg, pair.public())) - } - LocalKey::Sr25519(pair) => { - Ok(sr25519::Pair::verify_weak(signature, msg, pair.public())) - } - } - } -} - -/// A key. -enum Key { - LocalKey(LocalKey), - AuthorityKey(ConsensusPair), - FgAuthorityKey(FinalityPair), -} - -impl Key { - fn public(&self) -> Result, ()> { - match self { - Key::LocalKey(local) => { - local.public() - } - Key::AuthorityKey(pair) => { - Ok(pair.public().to_raw_vec()) - } - Key::FgAuthorityKey(pair) => { - Ok(pair.public().to_raw_vec()) - } - } - } - - fn sign(&self, data: &[u8]) -> Result, ()> { - match self { - Key::LocalKey(local) => { - local.sign(data) - } - Key::AuthorityKey(pair) => { - Ok(pair.sign(data).as_ref().to_vec()) - } - Key::FgAuthorityKey(pair) => { - Ok(pair.sign(data).as_ref().to_vec()) - } - } - } - - fn verify(&self, msg: &[u8], signature: &[u8]) -> Result { - match self { - Key::LocalKey(local) => { - local.verify(msg, signature) + }; + match self.kind { + key_types::ED25519 => { + let pair = ed25519::Pair::from_phrase(&self.phrase, password) + .map_err(err)?.0; + Ok(Box::new(pair)) } - Key::AuthorityKey(pair) => { - Ok(ConsensusPair::verify_weak(signature, msg, pair.public())) + key_types::SR25519 => { + let pair = sr25519::Pair::from_phrase(&self.phrase, password) + .map_err(err)?.0; + Ok(Box::new(pair)) } - Key::FgAuthorityKey(pair) => { - Ok(FinalityPair::verify_weak(signature, msg, pair.public())) + _ => { + error!("Unknown key type {:?}", self.kind); + Err(()) } } } @@ -182,11 +103,11 @@ impl Key { /// Asynchronous offchain API. /// /// NOTE this is done to prevent recursive calls into the runtime (which are not supported currently). -pub(crate) struct Api { +pub(crate) struct Api { sender: mpsc::UnboundedSender, db: Storage, keys_password: Protected, - key_provider: KeyProvider, + key_provider: Arc>, network_state: Arc, at: BlockId, } @@ -203,9 +124,8 @@ const KEYS_PREFIX: &[u8] = b"keys"; const NEXT_ID: &[u8] = b"crypto_key_id"; -impl Api where +impl Api where Storage: OffchainStorage, - KeyProvider: AuthorityKeyProvider, Block: traits::Block, { fn password(&self) -> Option<&str> { @@ -215,7 +135,7 @@ impl Api where fn read_key( &self, key: CryptoKey, - ) -> Result, ()> { + ) -> Result, ()> { match key { CryptoKey::LocalKey { id, kind } => { let key = self.db.get(KEYS_PREFIX, &id.encode()) @@ -230,28 +150,20 @@ impl Api where ); return Err(()) } - Ok(Key::LocalKey(key.to_local_key(self.password())?)) - } - CryptoKey::AuthorityKey => { - let key = self.key_provider - .authority_key(&self.at) - .ok_or(())?; - Ok(Key::AuthorityKey(key)) + Ok(key.to_offchain_key(self.password())?) } - CryptoKey::FgAuthorityKey => { - let key = self.key_provider - .fg_authority_key(&self.at) - .ok_or(())?; - Ok(Key::FgAuthorityKey(key)) + CryptoKey::AuthorityKey { kind } => { + self.key_provider + .authority_key(kind, &self.at) + .ok_or(()) } } } } -impl OffchainExt for Api +impl OffchainExt for Api where Storage: OffchainStorage, - KeyProvider: AuthorityKeyProvider, Block: traits::Block, { fn submit_transaction(&mut self, ext: Vec) -> Result<(), ()> { @@ -261,8 +173,8 @@ where .map_err(|_| ()) } - fn new_crypto_key(&mut self, kind: CryptoKind) -> Result { - let key = StoredKey::generate_with_phrase(kind, self.password()); + fn new_crypto_key(&mut self, kind: KeyTypeId) -> Result { + let key = StoredKey::generate_with_phrase(kind, self.password())?; let (id, id_encoded) = loop { let encoded = self.db.get(KEYS_PREFIX, NEXT_ID); let encoded_slice = encoded.as_ref().map(|x| x.as_slice()); @@ -506,14 +418,14 @@ pub(crate) struct AsyncApi { impl AsyncApi { /// Creates new Offchain extensions API implementation an the asynchronous processing part. - pub fn new>( + pub fn new( transaction_pool: Arc>, db: S, keys_password: Protected, - key_provider: P, + key_provider: Arc>, at: BlockId, network_state: Arc, - ) -> (Api, AsyncApi) { + ) -> (Api, AsyncApi) { let (sender, rx) = mpsc::unbounded(); let api = Api { @@ -569,6 +481,7 @@ impl AsyncApi { mod tests { use super::*; use std::convert::TryFrom; + use primitives::crypto; use runtime_primitives::traits::Zero; use client_db::offchain::LocalStorage; use crate::tests::TestProvider; @@ -587,7 +500,9 @@ mod tests { } } - fn offchain_api() -> (Api, Block>, AsyncApi) { + fn offchain_api( + key: Option + ) -> (Api, AsyncApi) { let _ = env_logger::try_init(); let db = LocalStorage::new_test(); let client = Arc::new(test_client::new()); @@ -596,7 +511,12 @@ mod tests { ); let mock = Arc::new(MockNetworkStateInfo()); - AsyncApi::new(pool, db, "pass".to_owned().into(), TestProvider::default(), BlockId::Number(Zero::zero()), mock) + let mut key_provider = AuthorityKeyProviders::new(); + if let Some(key) = key { + key_provider.register(0xdeadbeef, TestProvider::new(key)); + } + AsyncApi::new(pool, db, "pass".to_owned().into(), Arc::new(key_provider), + BlockId::Number(Zero::zero()), mock) } #[test] @@ -637,7 +557,7 @@ mod tests { fn should_set_and_get_local_storage() { // given let kind = StorageKind::PERSISTENT; - let mut api = offchain_api().0; + let mut api = offchain_api(None).0; let key = b"test"; // when @@ -652,7 +572,7 @@ mod tests { fn should_compare_and_set_local_storage() { // given let kind = StorageKind::PERSISTENT; - let mut api = offchain_api().0; + let mut api = offchain_api(None).0; let key = b"test"; api.local_storage_set(kind, key, b"value"); @@ -667,9 +587,9 @@ mod tests { #[test] fn should_create_a_new_key_and_sign_and_verify_stuff() { - let test = |kind: CryptoKind| { + let test = |kind: KeyTypeId| { // given - let mut api = offchain_api().0; + let mut api = offchain_api(None).0; let msg = b"Hello world!"; // when @@ -685,26 +605,27 @@ mod tests { assert_eq!(res, false); }; - test(CryptoKind::Ed25519); - test(CryptoKind::Sr25519); + test(crypto::key_types::ED25519); + test(crypto::key_types::SR25519); } #[test] fn should_sign_and_verify_with_authority_key() { // given - let mut api = offchain_api().0; - api.key_provider.ed_key = Some(ed25519::Pair::generate().0); + let key = ed25519::Pair::generate().0; + let mut api = offchain_api(Some(key)).0; let msg = b"Hello world!"; // when - let signature = api.sign(CryptoKey::AuthorityKey, msg).unwrap(); + let key = CryptoKey::AuthorityKey { kind: 0xdeadbeef }; + let signature = api.sign(key, msg).unwrap(); // then - let res = api.verify(CryptoKey::AuthorityKey, msg, &signature).unwrap(); + let res = api.verify(key, msg, &signature).unwrap(); assert_eq!(res, true); - let res = api.verify(CryptoKey::AuthorityKey, msg, &[]).unwrap(); + let res = api.verify(key, msg, &[]).unwrap(); assert_eq!(res, false); - let res = api.verify(CryptoKey::AuthorityKey, b"Different msg", &signature).unwrap(); + let res = api.verify(key, b"Different msg", &signature).unwrap(); assert_eq!(res, false); } diff --git a/core/offchain/src/lib.rs b/core/offchain/src/lib.rs index d4947e271f573..c759690b2e087 100644 --- a/core/offchain/src/lib.rs +++ b/core/offchain/src/lib.rs @@ -34,6 +34,7 @@ #![warn(missing_docs)] use std::{ + collections::HashMap, fmt, marker::PhantomData, sync::Arc, @@ -43,7 +44,7 @@ use client::runtime_api::ApiExt; use log::{debug, warn}; use primitives::{ ExecutionContext, - crypto, + crypto::{self, Public}, }; use runtime_primitives::{ generic::BlockId, @@ -59,61 +60,110 @@ pub mod testing; pub use offchain_primitives::OffchainWorkerApi; -/// Provides currently configured authority key. -pub trait AuthorityKeyProvider: Clone + 'static { - /// The crypto used by the block authoring algorithm. - type ConsensusPair: crypto::Pair; - /// The crypto used by the finality gadget. - type FinalityPair: crypto::Pair; +/// Trait that allows boxing keys for the offchain api. +pub trait OffchainKey { + /// Return the public key as a byte vector. + fn public(&self) -> Result, ()>; + /// Sign data and return the signature as a byte vector. + fn sign(&self, data: &[u8]) -> Result, ()>; + /// Verify if the msg matches the signature. + fn verify(&self, msg: &[u8], signature: &[u8]) -> Result; +} + +impl OffchainKey for T { + fn public(&self) -> Result, ()> { + Ok(::public(&self).to_raw_vec()) + } + + fn sign(&self, data: &[u8]) -> Result, ()> { + let sig = ::sign(&self, data); + let bytes: &[u8] = sig.as_ref(); + Ok(bytes.to_vec()) + } + + fn verify(&self, msg: &[u8], signature: &[u8]) -> Result { + Ok(Self::verify_weak(signature, msg, ::public(&self))) + } +} + +/// The trait to implement a key provider. +pub trait AuthorityKeyProvider { + /// Returns the key configured at a given block id. + fn authority_key(&self, at: &BlockId) -> Option>; +} + +/// Handles registering and querying `AuthorityKeyProvider` implementations. +pub struct AuthorityKeyProviders { + providers: HashMap>> +} - /// Returns currently configured authority key. - fn authority_key(&self, block_id: &BlockId) -> Option; +impl AuthorityKeyProviders { + /// Creates a new AuthorityKeyProviders. + pub fn new() -> Self { + Self { + providers: Default::default() + } + } - /// Returns currently configured finality gadget authority key. - fn fg_authority_key(&self, block_id: &BlockId) -> Option; + /// Register an AuthorityKeyProvider. + pub fn register( + &mut self, + kind: crypto::KeyProviderId, + provider: impl AuthorityKeyProvider + 'static, + ) { + self.providers.insert(kind, Box::new(provider)); + } + + /// Query the registered AuthorityKeyProvider's for a key. + pub fn authority_key( + &self, + kind: crypto::KeyProviderId, + at: &BlockId, + ) -> Option> { + self.providers.get(&kind) + .map(|provider| provider.authority_key(at)) + .unwrap_or(None) + } } /// An offchain workers manager. pub struct OffchainWorkers< Client, Storage, - KeyProvider, Block: traits::Block, > { client: Arc, db: Storage, - authority_key: KeyProvider, + key_provider: Arc>, keys_password: crypto::Protected, _block: PhantomData, } -impl OffchainWorkers< +impl OffchainWorkers< Client, Storage, - KeyProvider, Block, > { /// Creates new `OffchainWorkers`. pub fn new( client: Arc, db: Storage, - authority_key: KeyProvider, + key_provider: Arc>, keys_password: crypto::Protected, ) -> Self { Self { client, db, - authority_key, + key_provider, keys_password, _block: PhantomData, } } } -impl fmt::Debug for OffchainWorkers< +impl fmt::Debug for OffchainWorkers< Client, Storage, - KeyProvider, Block, > { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -121,16 +171,14 @@ impl fmt::Debug for Offchain } } -impl OffchainWorkers< +impl OffchainWorkers< Client, Storage, - KeyProvider, Block, > where Block: traits::Block, Client: ProvideRuntimeApi + Send + Sync + 'static, Client::Api: OffchainWorkerApi, - KeyProvider: AuthorityKeyProvider + Send, Storage: client::backend::OffchainStorage + 'static, { /// Start the offchain workers after given block. @@ -153,7 +201,7 @@ impl OffchainWorkers< pool.clone(), self.db.clone(), self.keys_password.clone(), - self.authority_key.clone(), + self.key_provider.clone(), at.clone(), network_state.clone(), ); @@ -197,7 +245,7 @@ fn spawn_worker(f: impl FnOnce() -> () + Send + 'static) { mod tests { use super::*; use futures::Future; - use primitives::{ed25519, sr25519}; + use primitives::ed25519; use network::{Multiaddr, PeerId}; struct MockNetworkStateInfo(); @@ -215,30 +263,21 @@ mod tests { #[derive(Clone)] pub(crate) struct TestProvider { _marker: PhantomData, - pub(crate) sr_key: Option, - pub(crate) ed_key: Option, + key: ed25519::Pair, } - impl Default for TestProvider { - fn default() -> Self { + impl TestProvider { + pub fn new(key: ed25519::Pair) -> Self { Self { _marker: PhantomData, - sr_key: None, - ed_key: None, + key, } } } impl AuthorityKeyProvider for TestProvider { - type ConsensusPair = ed25519::Pair; - type FinalityPair = sr25519::Pair; - - fn authority_key(&self, _: &BlockId) -> Option { - self.ed_key.clone() - } - - fn fg_authority_key(&self, _: &BlockId) -> Option { - self.sr_key.clone() + fn authority_key(&self, _: &BlockId) -> Option> { + Some(Box::new(self.key.clone())) } } @@ -253,7 +292,12 @@ mod tests { let mock = Arc::new(MockNetworkStateInfo()); // when - let offchain = OffchainWorkers::new(client, db, TestProvider::default(), "".to_owned().into()); + let offchain = OffchainWorkers::new( + client, + db, + Arc::new(AuthorityKeyProviders::new()), + "".to_owned().into() + ); runtime.executor().spawn(offchain.on_block_imported(&0u64, &pool, mock.clone())); // then diff --git a/core/offchain/src/testing.rs b/core/offchain/src/testing.rs index 6f473a9cd49a0..0025f609f45f2 100644 --- a/core/offchain/src/testing.rs +++ b/core/offchain/src/testing.rs @@ -22,13 +22,13 @@ use std::{ }; use client::backend::OffchainStorage; use parking_lot::RwLock; +use primitives::crypto::KeyProviderId; use primitives::offchain::{ self, HttpError, HttpRequestId as RequestId, HttpRequestStatus as RequestStatus, Timestamp, - CryptoKind, CryptoKey, StorageKind, OpaqueNetworkState, @@ -148,7 +148,7 @@ impl offchain::Externalities for TestOffchainExt { unimplemented!("not needed in tests so far") } - fn new_crypto_key(&mut self, _crypto: CryptoKind) -> Result { + fn new_crypto_key(&mut self, _crypto: KeyProviderId) -> Result { unimplemented!("not needed in tests so far") } diff --git a/core/primitives/src/crypto.rs b/core/primitives/src/crypto.rs index 327a8a3eb1254..aea0a64fd90ee 100644 --- a/core/primitives/src/crypto.rs +++ b/core/primitives/src/crypto.rs @@ -625,6 +625,31 @@ pub trait TypedKey { const KEY_TYPE: KeyTypeId; } +/// An identifier for a key provider. +/// +/// 0-1024 are reserved. +pub type KeyProviderId = u32; + +/// Constant key proviers. +pub mod key_providers { + use super::KeyProviderId; + + /// Aura key provider. + pub const AURA: KeyProviderId = 10; + + /// Babe key provider. + pub const BABE: KeyProviderId = 20; + + /// Grandpa key provider. + pub const GRANDPA: KeyProviderId = 30; +} + +/// A trait for something that has a key provider ID. +pub trait TypedKeyProvider { + /// The type ID of this key provider. + const KEY_PROVIDER_TYPE: KeyProviderId; +} + #[cfg(test)] mod tests { use crate::DeriveJunction; diff --git a/core/primitives/src/offchain.rs b/core/primitives/src/offchain.rs index fc1d4f0bdabee..9ca58234edfe1 100644 --- a/core/primitives/src/offchain.rs +++ b/core/primitives/src/offchain.rs @@ -58,35 +58,6 @@ impl From for u32 { } } -/// A type of supported crypto. -#[derive(Clone, Copy, PartialEq, Eq, Encode, Decode)] -#[cfg_attr(feature = "std", derive(Debug))] -#[repr(C)] -pub enum CryptoKind { - /// SR25519 crypto (Schnorrkel) - Sr25519 = crypto::key_types::SR25519 as isize, - /// ED25519 crypto (Edwards) - Ed25519 = crypto::key_types::ED25519 as isize, -} - -impl TryFrom for CryptoKind { - type Error = (); - - fn try_from(kind: u32) -> Result { - match kind { - e if e == CryptoKind::Sr25519 as isize as u32 => Ok(CryptoKind::Sr25519), - e if e == CryptoKind::Ed25519 as isize as u32 => Ok(CryptoKind::Ed25519), - _ => Err(()), - } - } -} - -impl From for u32 { - fn from(c: CryptoKind) -> Self { - c as isize as u32 - } -} - /// Key to use in the offchain worker crypto api. #[derive(Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "std", derive(Debug))] @@ -96,12 +67,13 @@ pub enum CryptoKey { /// The id of the key. id: u16, /// The kind of the key. - kind: CryptoKind, + kind: crypto::KeyTypeId, }, /// Use the key the block authoring algorithm uses. - AuthorityKey, - /// Use the key the finality gadget uses. - FgAuthorityKey, + AuthorityKey { + /// The kind of key provider. + kind: crypto::KeyProviderId, + }, } impl TryFrom for CryptoKey { @@ -111,11 +83,13 @@ impl TryFrom for CryptoKey { match key & 0xFF { 0 => { let id = (key >> 8 & 0xFFFF) as u16; - let kind = CryptoKind::try_from((key >> 32) as u32)?; + let kind = (key >> 32) as u32; Ok(CryptoKey::LocalKey { id, kind }) } - 1 => Ok(CryptoKey::AuthorityKey), - 2 => Ok(CryptoKey::FgAuthorityKey), + 1 => { + let kind = (key >> 32) as u32; + Ok(CryptoKey::AuthorityKey { kind }) + }, _ => Err(()), } } @@ -127,8 +101,9 @@ impl From for u64 { CryptoKey::LocalKey { id, kind } => { ((kind as u64) << 32) | ((id as u64) << 8) } - CryptoKey::AuthorityKey => 1, - CryptoKey::FgAuthorityKey => 2, + CryptoKey::AuthorityKey { kind } => { + (kind as u64) << 32 + } } } } @@ -317,7 +292,7 @@ pub trait Externalities { /// Create new key(pair) for signing/encryption/decryption. /// /// Returns an error if given crypto kind is not supported. - fn new_crypto_key(&mut self, crypto: CryptoKind) -> Result; + fn new_crypto_key(&mut self, crypto: crypto::KeyTypeId) -> Result; /// Returns the locally configured authority public key, if available. fn pubkey(&self, key: CryptoKey) -> Result, ()>; @@ -466,7 +441,7 @@ impl Externalities for Box { (&mut **self).submit_transaction(ex) } - fn new_crypto_key(&mut self, crypto: CryptoKind) -> Result { + fn new_crypto_key(&mut self, crypto: crypto::KeyTypeId) -> Result { (&mut **self).new_crypto_key(crypto) } diff --git a/core/sr-io/src/lib.rs b/core/sr-io/src/lib.rs index 6ffb15ffdf2d3..46e62608b5bd8 100644 --- a/core/sr-io/src/lib.rs +++ b/core/sr-io/src/lib.rs @@ -33,10 +33,11 @@ use rstd::vec::Vec; pub use codec; pub use primitives::Blake2Hasher; +use primitives::crypto::KeyTypeId; use primitives::offchain::{ Timestamp, HttpRequestId, HttpRequestStatus, HttpError, - CryptoKind, CryptoKey, + CryptoKey, StorageKind, OpaqueNetworkState, }; @@ -249,7 +250,7 @@ export_api! { /// Create new key(pair) for signing/encryption/decryption. /// /// Returns an error if given crypto kind is not supported. - fn new_crypto_key(crypto: CryptoKind) -> Result; + fn new_crypto_key(crypto: KeyTypeId) -> Result; /// Encrypt a piece of data using given crypto key. /// diff --git a/core/sr-io/with_std.rs b/core/sr-io/with_std.rs index 18cb2fd2dfda6..9ded5a1e8d976 100644 --- a/core/sr-io/with_std.rs +++ b/core/sr-io/with_std.rs @@ -28,7 +28,7 @@ pub use substrate_state_machine::{ }; use environmental::environmental; -use primitives::{offchain, hexdisplay::HexDisplay, H256}; +use primitives::{crypto::KeyTypeId, offchain, hexdisplay::HexDisplay, H256}; #[cfg(feature = "std")] use std::collections::HashMap; @@ -281,7 +281,7 @@ impl OffchainApi for () { }, "authority_pubkey can be called only in the offchain worker context") } - fn new_crypto_key(crypto: offchain::CryptoKind) -> Result { + fn new_crypto_key(crypto: KeyTypeId) -> Result { with_offchain(|ext| { ext.new_crypto_key(crypto) }, "new_crypto_key can be called only in the offchain worker context") diff --git a/core/sr-io/without_std.rs b/core/sr-io/without_std.rs index 001b697934c99..178bd6fb113b3 100644 --- a/core/sr-io/without_std.rs +++ b/core/sr-io/without_std.rs @@ -20,7 +20,7 @@ pub use rstd::{mem, slice}; use core::{intrinsics, panic::PanicInfo}; use rstd::{vec::Vec, cell::Cell, convert::TryInto, convert::TryFrom}; -use primitives::{offchain, Blake2Hasher}; +use primitives::{offchain, Blake2Hasher, crypto::KeyTypeId}; #[cfg(not(feature = "no_panic_handler"))] #[panic_handler] @@ -942,8 +942,7 @@ impl OffchainApi for () { } } - fn new_crypto_key(crypto: offchain::CryptoKind) -> Result { - let crypto = crypto.into(); + fn new_crypto_key(crypto: KeyTypeId) -> Result { let ret = unsafe { ext_new_crypto_key.get()(crypto) }; diff --git a/core/state-machine/src/lib.rs b/core/state-machine/src/lib.rs index f2275cc41fd0c..6ee2b6d7add53 100644 --- a/core/state-machine/src/lib.rs +++ b/core/state-machine/src/lib.rs @@ -24,7 +24,8 @@ use log::warn; use hash_db::Hasher; use parity_codec::{Decode, Encode}; use primitives::{ - storage::well_known_keys, NativeOrEncoded, NeverNativeValue, offchain, + crypto::KeyTypeId, offchain, storage::well_known_keys, + NativeOrEncoded, NeverNativeValue, }; pub mod backend; @@ -255,7 +256,7 @@ impl offchain::Externalities for NeverOffchainExt { fn new_crypto_key( &mut self, - _crypto: offchain::CryptoKind, + _crypto: KeyTypeId, ) -> Result { unreachable!() } From 923f08e9256d3b12155b462fc95466e304650a3d Mon Sep 17 00:00:00 2001 From: David Craven Date: Tue, 23 Jul 2019 21:27:18 +0200 Subject: [PATCH 14/14] Get node to build. --- Cargo.lock | 4 ++ core/consensus/aura/Cargo.toml | 1 + core/consensus/aura/src/lib.rs | 57 +++++++++++++++ core/finality-grandpa/Cargo.toml | 1 + core/finality-grandpa/src/lib.rs | 53 ++++++++++++++ core/offchain/src/lib.rs | 2 +- core/service/src/components.rs | 79 +++++++++++--------- core/service/src/lib.rs | 119 +++++++------------------------ node/cli/Cargo.toml | 2 + node/cli/src/service.rs | 50 ++++++++----- srml/im-online/src/lib.rs | 8 ++- 11 files changed, 225 insertions(+), 151 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f8a178669cffe..11fb7c76ed743 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2273,7 +2273,9 @@ dependencies = [ "substrate-finality-grandpa-primitives 2.0.0", "substrate-inherents 2.0.0", "substrate-keyring 2.0.0", + "substrate-keystore 2.0.0", "substrate-network 2.0.0", + "substrate-offchain 2.0.0", "substrate-primitives 2.0.0", "substrate-service 2.0.0", "substrate-service-test 2.0.0", @@ -4295,6 +4297,7 @@ dependencies = [ "substrate-keyring 2.0.0", "substrate-keystore 2.0.0", "substrate-network 2.0.0", + "substrate-offchain 2.0.0", "substrate-primitives 2.0.0", "substrate-service 2.0.0", "substrate-telemetry 2.0.0", @@ -4475,6 +4478,7 @@ dependencies = [ "substrate-keyring 2.0.0", "substrate-keystore 2.0.0", "substrate-network 2.0.0", + "substrate-offchain 2.0.0", "substrate-primitives 2.0.0", "substrate-service 2.0.0", "substrate-telemetry 2.0.0", diff --git a/core/consensus/aura/Cargo.toml b/core/consensus/aura/Cargo.toml index 0ca357b8846de..6f603ea973f65 100644 --- a/core/consensus/aura/Cargo.toml +++ b/core/consensus/aura/Cargo.toml @@ -24,6 +24,7 @@ tokio-timer = "0.2.11" parking_lot = "0.8.0" log = "0.4" keystore = { package = "substrate-keystore", path = "../../keystore" } +offchain = { package = "substrate-offchain", path = "../../offchain" } [dev-dependencies] futures03 = { package = "futures-preview", version = "0.3.0-alpha.17", features = ["compat"] } diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index 5af5492dfd827..982c0adf0c297 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -75,6 +75,63 @@ mod digest; type AuthorityId

=

::Public; +/// Aura key provider for offchain workers. +pub struct AuraKeyProvider { + _marker: PhantomData, + client: Arc, + keystore: Arc>, +} + +impl AuraKeyProvider +where + Block: BlockT, + Client: ProvideRuntimeApi + Send + Sync, + Client::Api: AuraApi>, + TPair: Pair, + TPair::Public: Encode + Decode, +{ + /// Create a new `AuraKeyProvider`. + pub fn new( + client: Arc, + keystore: Arc>, + ) -> Self { + Self { + _marker: PhantomData, + client, + keystore, + } + } +} + +impl + offchain::AuthorityKeyProvider for AuraKeyProvider +where + Block: BlockT, + Client: ProvideRuntimeApi + Send + Sync, + Client::Api: AuraApi>, + TPair: Pair, + TPair::Public: Encode + Decode, +{ + fn authority_key( + &self, + at: &BlockId, + ) -> Option> { + let authorities = self.client + .runtime_api() + .authorities(at) + .unwrap_or_default(); + if let Some(key) = self.keystore.get_keys(|public| { + authorities.iter().find(|aid| aid == &public).is_some() + }) + .into_iter() + .next() + { + return Some(Box::new(key)) + } + None + } +} + /// A slot duration. Create with `get_or_compute`. #[derive(Clone, Copy, Debug, Encode, Decode, Hash, PartialOrd, Ord, PartialEq, Eq)] pub struct SlotDuration(slots::SlotDuration); diff --git a/core/finality-grandpa/Cargo.toml b/core/finality-grandpa/Cargo.toml index e1e66ebe7ceb3..42642396dd280 100644 --- a/core/finality-grandpa/Cargo.toml +++ b/core/finality-grandpa/Cargo.toml @@ -27,6 +27,7 @@ srml-finality-tracker = { path = "../../srml/finality-tracker" } fg_primitives = { package = "substrate-finality-grandpa-primitives", path = "primitives" } grandpa = { package = "finality-grandpa", version = "0.8.1", features = ["derive-codec"] } keystore = { package = "substrate-keystore", path = "../keystore" } +offchain = { package = "substrate-offchain", path = "../offchain" } [dev-dependencies] grandpa = { package = "finality-grandpa", version = "0.8.1", features = ["derive-codec", "test-helpers"] } diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index 1fc3f8ed21f2d..92436f2280ad2 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -76,6 +76,7 @@ use grandpa::Error as GrandpaError; use grandpa::{voter, round::State as RoundState, BlockNumberOps, voter_set::VoterSet}; use std::fmt; +use std::marker::PhantomData; use std::sync::Arc; use std::time::Duration; @@ -190,6 +191,58 @@ type CommunicationOutH = grandpa::voter::CommunicationOut< AuthorityId, >; +/// Aura key provider for offchain workers. +pub struct GrandpaKeyProvider { + _marker: PhantomData, + client: Arc, + keystore: Arc>, +} + +impl GrandpaKeyProvider +where + Block: BlockT, + Client: ProvideRuntimeApi + Send + Sync, + Client::Api: GrandpaApi, +{ + /// Creates a new `GrandpaKeyProvider`. + pub fn new( + client: Arc, + keystore: Arc>, + ) -> Self { + Self { + _marker: PhantomData, + client, + keystore, + } + } +} + +impl offchain::AuthorityKeyProvider for GrandpaKeyProvider +where + Block: BlockT, + Client: ProvideRuntimeApi + Send + Sync, + Client::Api: GrandpaApi, +{ + fn authority_key( + &self, + at: &BlockId, + ) -> Option> { + let authorities = self.client + .runtime_api() + .grandpa_authorities(at) + .unwrap_or_default(); + if let Some(key) = self.keystore.get_keys(|public| { + authorities.iter().find(|(aid, _)| aid == public).is_some() + }) + .into_iter() + .next() + { + return Some(Box::new(key)) + } + None + } +} + /// Configuration for the GRANDPA service. #[derive(Clone)] pub struct Config { diff --git a/core/offchain/src/lib.rs b/core/offchain/src/lib.rs index c759690b2e087..5bfc1041e78ed 100644 --- a/core/offchain/src/lib.rs +++ b/core/offchain/src/lib.rs @@ -87,7 +87,7 @@ impl OffchainKey for T { } /// The trait to implement a key provider. -pub trait AuthorityKeyProvider { +pub trait AuthorityKeyProvider: Send + Sync { /// Returns the key configured at a given block id. fn authority_key(&self, at: &BlockId) -> Option>; } diff --git a/core/service/src/components.rs b/core/service/src/components.rs index 6c245f6e7c294..f7bb966282cde 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -21,9 +21,9 @@ use serde::{Serialize, de::DeserializeOwned}; use crate::chain_spec::ChainSpec; use client_db; use client::{self, Client, runtime_api}; -use crate::{error, Service, AuthorityKeyProvider}; +use crate::{error, Service}; use consensus_common::{import_queue::ImportQueue, SelectChain}; -use keystore::{LocalStore, Store}; +use keystore::Store; use network::{self, OnDemand, FinalityProofProvider, NetworkStateInfo, config::BoxFinalityProofRequestBuilder}; use substrate_executor::{NativeExecutor, NativeExecutionDispatch}; use transaction_pool::txpool::{self, Options as TransactionPoolOptions, Pool as TransactionPool}; @@ -31,7 +31,7 @@ use runtime_primitives::{ BuildStorage, traits::{Block as BlockT, Header as HeaderT, ProvideRuntimeApi}, generic::BlockId }; use crate::config::Configuration; -use primitives::{Blake2Hasher, H256, Pair}; +use primitives::{Blake2Hasher, H256}; use rpc::{self, apis::system::SystemInfo}; use futures::{prelude::*, future::Executor, sync::mpsc}; @@ -129,20 +129,6 @@ pub type ComponentOffchainStorage = < /// Block type for `Components` pub type ComponentBlock = <::Factory as ServiceFactory>::Block; -/// ConsensusPair type for `Components` -pub type ComponentConsensusPair = <::Factory as ServiceFactory>::ConsensusPair; - -/// FinalityPair type for `Components` -pub type ComponentFinalityPair = <::Factory as ServiceFactory>::FinalityPair; - -/// AuthorityKeyProvider type for `Components` -pub type ComponentAuthorityKeyProvider = AuthorityKeyProvider< - ComponentClient, - ComponentBlock, - LocalStore>, - LocalStore> ->; - /// Extrinsic hash type for `Components` pub type ComponentExHash = <::TransactionPoolApi as txpool::ChainApi>::Hash; @@ -248,7 +234,6 @@ pub trait OffchainWorker { offchain: &offchain::OffchainWorkers< ComponentClient, ComponentOffchainStorage, - ComponentAuthorityKeyProvider, ComponentBlock >, pool: &Arc>, @@ -265,7 +250,6 @@ impl OffchainWorker for C where offchain: &offchain::OffchainWorkers< ComponentClient, ComponentOffchainStorage, - ComponentAuthorityKeyProvider, ComponentBlock >, pool: &Arc>, @@ -300,10 +284,6 @@ pub type TaskExecutor = Arc + pub trait ServiceFactory: 'static + Sized { /// Block type. type Block: BlockT; - /// Consensus crypto type. - type ConsensusPair: Pair; - /// Finality crypto type. - type FinalityPair: Pair; /// The type that implements the runtime API. type RuntimeApi: Send + Sync; /// Network protocol extensions. @@ -352,6 +332,13 @@ pub trait ServiceFactory: 'static + Sized { client: Arc>, ) -> Result; + /// Build the authority key provider for full client. + fn build_authority_key_provider( + keystore: &Arc, + seed: Option<&str>, + client: Arc>, + ) -> offchain::AuthorityKeyProviders; + /// Build full service. fn new_full(config: FactoryFullConfiguration) -> Result; @@ -396,9 +383,9 @@ pub trait Components: Sized + 'static { /// Associated service factory. type Factory: ServiceFactory; /// Client backend. - type Backend: 'static + client::backend::Backend, Blake2Hasher>; + type Backend: 'static + client::backend::Backend, Blake2Hasher>; /// Client executor. - type Executor: 'static + client::CallExecutor, Blake2Hasher> + Send + Sync + Clone; + type Executor: 'static + client::CallExecutor, Blake2Hasher> + Send + Sync + Clone; /// The type that implements the runtime API. type RuntimeApi: Send + Sync; /// A type that can start all runtime-dependent services. @@ -406,13 +393,13 @@ pub trait Components: Sized + 'static { // TODO: Traitify transaction pool and allow people to implement their own. (#1242) /// Extrinsic pool type. type TransactionPoolApi: 'static + txpool::ChainApi< - Hash = as BlockT>::Hash, - Block = FactoryBlock + Hash = as BlockT>::Hash, + Block = ComponentBlock >; /// Our Import Queue - type ImportQueue: ImportQueue> + 'static; + type ImportQueue: ImportQueue> + 'static; /// The Fork Choice Strategy for the chain - type SelectChain: SelectChain>; + type SelectChain: SelectChain>; /// Create client. fn build_client( @@ -421,7 +408,7 @@ pub trait Components: Sized + 'static { ) -> Result< ( Arc>, - Option>>> + Option>>> ), error::Error >; @@ -436,18 +423,25 @@ pub trait Components: Sized + 'static { config: &mut FactoryFullConfiguration, client: Arc>, select_chain: Option, - ) -> Result<(Self::ImportQueue, Option>>), error::Error>; + ) -> Result<(Self::ImportQueue, Option>>), error::Error>; /// Finality proof provider for serving network requests. fn build_finality_proof_provider( client: Arc> - ) -> Result::Block>>>, error::Error>; + ) -> Result>>>, error::Error>; /// Build fork choice selector fn build_select_chain( config: &mut FactoryFullConfiguration, client: Arc> ) -> Result, error::Error>; + + /// Build the authority key provider. + fn build_authority_key_provider( + keystore: &Arc, + seed: Option<&str>, + client: Arc>, + ) -> offchain::AuthorityKeyProviders>; } /// A struct that implement `Components` for the full client. @@ -553,9 +547,17 @@ impl Components for FullComponents { fn build_finality_proof_provider( client: Arc> - ) -> Result::Block>>>, error::Error> { + ) -> Result>>>, error::Error> { Factory::build_finality_proof_provider(client) } + + fn build_authority_key_provider( + keystore: &Arc, + seed: Option<&str>, + client: Arc>, + ) -> offchain::AuthorityKeyProviders> { + Factory::build_authority_key_provider(keystore, seed, client) + } } /// A struct that implement `Components` for the light client. @@ -647,15 +649,24 @@ impl Components for LightComponents { fn build_finality_proof_provider( _client: Arc> - ) -> Result::Block>>>, error::Error> { + ) -> Result>>>, error::Error> { Ok(None) } + fn build_select_chain( _config: &mut FactoryFullConfiguration, _client: Arc> ) -> Result, error::Error> { Ok(None) } + + fn build_authority_key_provider( + _keystore: &Arc, + _seed: Option<&str>, + _client: Arc>, + ) -> offchain::AuthorityKeyProviders> { + offchain::AuthorityKeyProviders::new() + } } #[cfg(test)] diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 61f1ad0848511..03b69b2e1c2ec 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -37,14 +37,13 @@ use client::{BlockchainEvents, backend::Backend, runtime_api::BlockT}; use exit_future::Signal; use futures::prelude::*; use futures03::stream::{StreamExt as _, TryStreamExt as _}; -use keystore::{LocalStore, Store}; +use keystore::Store; use network::{NetworkState, NetworkStateInfo}; use log::{log, info, warn, debug, error, Level}; use parity_codec::{Encode, Decode}; -use primitives::Pair; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{ - Header, NumberFor, ProvideRuntimeApi, SaturatedConversion + Header, NumberFor, SaturatedConversion }; use substrate_executor::NativeExecutor; use sysinfo::{get_current_pid, ProcessExt, System, SystemExt}; @@ -59,13 +58,12 @@ pub use transaction_pool::txpool::{ pub use client::FinalityNotifications; pub use components::{ - ServiceFactory, FullBackend, FullExecutor, LightBackend, ComponentAuthorityKeyProvider, + ServiceFactory, FullBackend, FullExecutor, LightBackend, LightExecutor, Components, PoolApi, ComponentClient, ComponentOffchainStorage, ComponentBlock, FullClient, LightClient, FullComponents, LightComponents, CodeExecutor, NetworkService, FactoryChainSpec, FactoryBlock, FactoryFullConfiguration, RuntimeGenesis, FactoryGenesis, ComponentExHash, ComponentExtrinsic, FactoryExtrinsic, - ComponentConsensusPair, ComponentFinalityPair, }; use components::{StartRPC, MaintainTransactionPool, OffchainWorker}; #[doc(hidden)] @@ -87,8 +85,6 @@ pub struct Service { NetworkStatus>, NetworkState )>>>>, transaction_pool: Arc>, - consensus_keystore: Arc>>, - finality_keystore: Arc>>, exit: ::exit_future::Exit, signal: Option, /// Sender for futures that must be spawned as background tasks. @@ -108,7 +104,6 @@ pub struct Service { _offchain_workers: Option, ComponentOffchainStorage, - ComponentAuthorityKeyProvider, ComponentBlock> >>, } @@ -235,27 +230,11 @@ impl Service { let network_status_sinks = Arc::new(Mutex::new(Vec::new())); let keystore = Arc::new(Store::new()); - - let consensus_keystore = keystore - .create_local_store::>(); - if let Some(seed) = config.key { - keystore.add_key_from_seed::>( - seed.as_str()); - } - - let finality_keystore = keystore - .create_local_store::>(); - if let Some(seed) = config.key { - keystore.add_key_from_seed::>( - seed.as_str()); - } - - let authority_key_provider = AuthorityKeyProvider { - _marker: PhantomData, - client: client.clone(), - consensus_store: consensus_keystore.clone(), - finality_store: finality_keystore.clone(), - }; + let authority_key_provider = Components::build_authority_key_provider( + &keystore, + config.key.as_ref().map(|s| s.as_str()), + client.clone(), + ); #[allow(deprecated)] let offchain_storage = client.backend().offchain_storage(); @@ -264,7 +243,7 @@ impl Service { Some(Arc::new(offchain::OffchainWorkers::new( client.clone(), db, - authority_key_provider, + Arc::new(authority_key_provider), config.offchain_worker_password.clone(), ))) }, @@ -476,8 +455,6 @@ impl Service { to_spawn_tx, to_spawn_rx, to_poll: Vec::new(), - consensus_keystore, - finality_keystore, config, exit, rpc_handlers, @@ -488,20 +465,6 @@ impl Service { }) } - /// Returns the consensus keystore. - pub fn consensus_keystore( - &self - ) -> Arc>> { - self.consensus_keystore.clone() - } - - /// Returns the finality keystore. - pub fn finality_keystore( - &self - ) -> Arc>> { - self.finality_keystore.clone() - } - /// return a shared instance of Telemetry (if enabled) pub fn telemetry(&self) -> Option { self._telemetry.as_ref().map(|t| t.clone()) @@ -708,7 +671,7 @@ fn build_network_future< "Polling the network future took {:?}", polling_dur ); - + Ok(Async::NotReady) }) } @@ -908,48 +871,6 @@ impl Clone for AuthorityKeyProvider { } } -impl - offchain::AuthorityKeyProvider - for AuthorityKeyProvider< - Client, - Block, - LocalStore, - LocalStore> -where - Client: ProvideRuntimeApi + 'static, - Block: runtime_primitives::traits::Block, - ConsensusPair: Pair + Clone, - FinalityPair: Pair + Clone, -{ - type ConsensusPair = ConsensusPair; - type FinalityPair = FinalityPair; - - fn authority_key(&self, at: &BlockId) -> Option { - let authorities = self.client - .runtime_api() - .authorities(at) - .unwrap_or_default(); - self.consensus_store.get_keys(|public| { - authorities.iter().find(|aid| aid == &public).is_some() - }) - .into_iter() - .next() - } - - fn fg_authority_key(&self, at: &BlockId) -> Option { - let authorities = self.client - .runtime_api() - .grandpa_authorities(at) - .unwrap_or_default(); - self.finality_store.get_keys(|public| { - authorities.iter().find(|(aid, _)| aid == public).is_some() - }) - .into_iter() - .next() - } -} - - /// Constructs a service factory with the given name that implements the `ServiceFactory` trait. /// The required parameters are required to be given in the exact order. Some parameters are followed @@ -999,8 +920,6 @@ where /// struct Factory { /// // Declare the block type /// Block = Block, -/// ConsensusPair = primitives::ed25519::Pair, -/// FinalityPair = primitives::ed25519::Pair, /// RuntimeApi = RuntimeApi, /// // Declare the network protocol and give an initializer. /// NetworkProtocol = NodeProtocol { |config| Ok(NodeProtocol::new()) }, @@ -1013,6 +932,11 @@ where /// Configuration = (), /// FullService = FullComponents /// { |config| >::new(config) }, +/// KeystoreSetup = { +/// |_keystore: &Arc, _seed: Option<&str>, _client: Arc>| { +/// offchain::AuthorityKeyProviders::new() +/// } +/// }, /// // Setup as Consensus Authority (if the role and key are given) /// AuthoritySetup = { /// |service: Self::FullService| { @@ -1044,8 +968,6 @@ macro_rules! construct_service_factory { $(#[$attr:meta])* struct $name:ident { Block = $block:ty, - ConsensusPair = $consensus_pair:ty, - FinalityPair = $finality_pair:ty, RuntimeApi = $runtime_api:ty, NetworkProtocol = $protocol:ty { $( $protocol_init:tt )* }, RuntimeDispatch = $dispatch:ty, @@ -1054,6 +976,7 @@ macro_rules! construct_service_factory { Genesis = $genesis:ty, Configuration = $config:ty, FullService = $full_service:ty { $( $full_service_init:tt )* }, + KeystoreSetup = { $( $keystore_setup:tt )* }, AuthoritySetup = { $( $authority_setup:tt )* }, LightService = $light_service:ty { $( $light_service_init:tt )* }, FullImportQueue = $full_import_queue:ty @@ -1071,8 +994,6 @@ macro_rules! construct_service_factory { #[allow(unused_variables)] impl $crate::ServiceFactory for $name { type Block = $block; - type ConsensusPair = $consensus_pair; - type FinalityPair = $finality_pair; type RuntimeApi = $runtime_api; type NetworkProtocol = $protocol; type RuntimeDispatch = $dispatch; @@ -1136,6 +1057,14 @@ macro_rules! construct_service_factory { ( $( $finality_proof_provider_init )* ) (client) } + fn build_authority_key_provider( + keystore: &Arc, + seed: Option<&str>, + client: Arc<$crate::FullClient>, + ) -> offchain::AuthorityKeyProviders { + ( $( $keystore_setup )* ) (keystore, seed, client) + } + fn new_light( config: $crate::FactoryFullConfiguration ) -> $crate::Result diff --git a/node/cli/Cargo.toml b/node/cli/Cargo.toml index 1c036268a61dc..9576930180dfd 100644 --- a/node/cli/Cargo.toml +++ b/node/cli/Cargo.toml @@ -41,6 +41,8 @@ finality_tracker = { package = "srml-finality-tracker", path = "../../srml/final contracts = { package = "srml-contracts", path = "../../srml/contracts" } system = { package = "srml-system", path = "../../srml/system" } balances = { package = "srml-balances", path = "../../srml/balances" } +keystore = { package = "substrate-keystore", path = "../../core/keystore" } +offchain = { package = "substrate-offchain", path = "../../core/offchain" } [dev-dependencies] consensus-common = { package = "substrate-consensus-common", path = "../../core/consensus/common" } diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index dabcb24579bee..bbf1976ba08e2 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -21,10 +21,13 @@ use std::sync::Arc; use std::time::Duration; -use aura::{import_queue, start_aura, AuraImportQueue, SlotDuration}; +use aura::{import_queue, start_aura, AuraKeyProvider, AuraImportQueue, SlotDuration}; use client::{self, LongestChain}; -use grandpa::{self, FinalityProofProvider as GrandpaFinalityProofProvider}; +use grandpa::{self, FinalityProofProvider as GrandpaFinalityProofProvider, GrandpaKeyProvider}; use grandpa_primitives::{AuthorityPair as GrandpaPair}; +use keystore::{LocalStore, Store}; +use offchain::AuthorityKeyProviders; +use primitives::crypto::key_providers; use node_executor; use futures::prelude::*; use node_primitives::{AuraPair, Block}; @@ -50,6 +53,7 @@ pub struct NodeConfig { /// grandpa connection to import block // FIXME #1134 rather than putting this on the config, let's have an actual intermediate setup state pub grandpa_import_setup: Option<(grandpa::BlockImportForService, grandpa::LinkHalfForService)>, + pub keystores: Option<(Arc>, Arc>)>, inherent_data_providers: InherentDataProviders, } @@ -57,6 +61,7 @@ impl Default for NodeConfig where F: substrate_service::ServiceFactory { fn default() -> NodeConfig { NodeConfig { grandpa_import_setup: None, + keystores: None, inherent_data_providers: InherentDataProviders::new(), } } @@ -65,8 +70,6 @@ impl Default for NodeConfig where F: substrate_service::ServiceFactory { construct_service_factory! { struct Factory { Block = Block, - ConsensusPair = AuraPair, - FinalityPair = GrandpaPair, RuntimeApi = RuntimeApi, NetworkProtocol = NodeProtocol { |config| Ok(NodeProtocol::new()) }, RuntimeDispatch = node_executor::Executor, @@ -79,18 +82,35 @@ construct_service_factory! { FullService = FullComponents { |config: FactoryFullConfiguration| FullComponents::::new(config) }, + KeystoreSetup = { + |config: FactoryFullConfiguration, keystore: &Arc, + seed: Option<&str>, client: Arc>| { + let aura_keystore = keystore.create_local_store::(); + if let Some(seed) = seed { + keystore.add_key_from_seed::(seed)?; + } + let aura_keyprovider = AuraKeyProvider::new(client.clone(), aura_keystore.clone()); + + let grandpa_keystore = keystore.create_local_store::(); + if let Some(seed) = seed { + keystore.add_key_from_seed::(seed)?; + } + let grandpa_keyprovider = GrandpaKeyProvider::new(client, grandpa_keystore.clone()); + + let mut authority_key_provider = AuthorityKeyProviders::new(); + authority_key_provider.register(key_providers::AURA, aura_keyprovider); + authority_key_provider.register(key_providers::GRANDPA, grandpa_keyprovider); + Ok(authority_key_provider) + } + }, AuthoritySetup = { |mut service: Self::FullService| { let (block_import, link_half) = service.config.custom.grandpa_import_setup.take() .expect("Link Half and Block Import are present for Full Services or setup failed before. qed"); + let (aura_keystore, grandpa_keystore) = service.config.custom.keystores.take() + .expect("Keystores are present for Full Services or setup failed before. qed"); if service.config.aura { - let local_store = service.store() - .create_local_store::(); - if let Some(seed) = &service.config.key { - service.store().set_key_from_seed::(seed.as_str())?; - } - let proposer = Arc::new(substrate_basic_authorship::ProposerFactory { client: service.client(), transaction_pool: service.transaction_pool(), @@ -102,7 +122,7 @@ construct_service_factory! { let aura = start_aura( SlotDuration::get_or_compute(&*client)?, - local_store, + aura_keystore, client, select_chain, block_import, @@ -123,18 +143,12 @@ construct_service_factory! { }; if service.config.grandpa_voter { - let local_store = service.store() - .create_local_store::(); - if let Some(seed) = &service.config.key { - service.store().set_key_from_seed::(seed.as_str())?; - } - let telemetry_on_connect = TelemetryOnConnect { telemetry_connection_sinks: service.telemetry_on_connect_stream(), }; let grandpa_config = grandpa::GrandpaParams { config, - session_store: local_store, + session_store: grandpa_keystore, link: link_half, network: service.network(), inherent_data_providers: service.config.custom.inherent_data_providers.clone(), diff --git a/srml/im-online/src/lib.rs b/srml/im-online/src/lib.rs index f1868c599b719..f1f7594a443fd 100644 --- a/srml/im-online/src/lib.rs +++ b/srml/im-online/src/lib.rs @@ -214,8 +214,10 @@ decl_module! { fn offchain_worker(now: T::BlockNumber) { fn gossip_at(block_number: T::BlockNumber) -> Result<(), OffchainErr> { // we run only when a local authority key is configured - if let Ok(key) = sr_io::pubkey(CryptoKey::AuthorityKey) { - let authority_id = ::AuthorityId::decode(&mut &key[..]) + // TODO fix + let key = CryptoKey::AuthorityKey { kind: 10 }; // aura + if let Ok(pubkey) = sr_io::pubkey(key) { + let authority_id = ::AuthorityId::decode(&mut &pubkey[..]) .ok_or(OffchainErr::DecodeAuthorityId)?; let network_state = sr_io::network_state().map_err(|_| OffchainErr::NetworkState)?; @@ -226,7 +228,7 @@ decl_module! { authority_id, }; - let signature = sr_io::sign(CryptoKey::AuthorityKey, &heartbeat_data.encode()) + let signature = sr_io::sign(key, &heartbeat_data.encode()) .map_err(|_| OffchainErr::FailedSigning)?; let call = Call::heartbeat(heartbeat_data, signature); let ex = T::UncheckedExtrinsic::new_unsigned(call.into())