Refactor key management (#3296)

* Add Call type to extensible transactions.

Cleanup some naming

* Merge Resource and BlockExhausted into just Exhausted

* Fix

* Another fix

* Call

* Some fixes

* Fix srml tests.

* Fix all tests.

* Refactor crypto so each application of it has its own type.

* Introduce new AuthorityProvider API into Aura

This will eventually allow for dynamic determination of authority
keys and avoid having to set them directly on CLI.

* Introduce authority determinator for Babe.

Experiment with modular consensus API.

* Work in progress to introduce KeyTypeId and avoid polluting API
with validator IDs

* Finish up drafting imonline

* Rework offchain workers API.

* Rework API implementation.

* Make it compile for wasm, simplify app_crypto.

* Fix compilation of im-online.

* Fix compilation of im-online.

* Fix more compilation errors.

* Make it compile.

* Fixing tests.

* Rewrite `keystore`

* Fix session tests

* Bring back `TryFrom`'s'

* Fix `srml-grandpa`

* Fix `srml-aura`

* Fix consensus babe

* More fixes

* Make service generate keys from dev_seed

* Build fixes

* Remove offchain tests

* More fixes and cleanups

* Fixes finality grandpa

* Fix `consensus-aura`

* Fix cli

* Fix `node-cli`

* Fix chain_spec builder

* Fix doc tests

* Add authority getter for grandpa.

* Test fix

* Fixes

* Make keystore accessible from the runtime

* Move app crypto to its own crate

* Update `Cargo.lock`

* Make the crypto stuff usable from the runtime

* Adds some runtime crypto tests

* Use last finalized block for grandpa authority

* Fix warning

* Adds `SessionKeys` runtime api

* Remove `FinalityPair` and `ConsensusPair`

* Minor governance tweaks to get it inline with docs.

* Make the governance be up to date with the docs.

* Build fixes.

* Generate the inital session keys

* Failing keystore is a hard error

* Make babe work again

* Fix grandpa

* Fix tests

* Disable `keystore` in consensus critical stuff

* Build fix.

* ImOnline supports multiple authorities at once.

* Update core/application-crypto/src/ed25519.rs

* Merge branch 'master' into gav-in-progress

* Remove unneeded code for now.

* Some `session` testing

* Support querying the public keys

* Cleanup offchain

* Remove warnings

* More cleanup

* Apply suggestions from code review

Co-Authored-By: Benjamin Kampmann <ben.kampmann@googlemail.com>

* More cleanups

* JSONRPC API for setting keys.

Also, rename traits::KeyStore* -> traits::BareCryptoStore*

* Bad merge

* Fix integration tests

* Fix test build

* Test fix

* Fixes

* Warnings

* Another warning

* Bump version.

substrate/core/client/src/call_executor.rs

@@ -83,6 +83,7 @@ where
 		native_call: Option<NC>,
 		side_effects_handler: Option<&mut O>,
 		proof_recorder: &Option<Rc<RefCell<ProofRecorder<B>>>>,
+		enable_keystore: bool,
 	) -> error::Result<NativeOrEncoded<R>> where ExecutionManager<EM>: Clone;
 
 	/// Extract RuntimeVersion of given block
@@ -150,14 +151,20 @@ where
 pub struct LocalCallExecutor<B, E> {
 	backend: Arc<B>,
 	executor: E,
+	keystore: Option<primitives::traits::BareCryptoStorePtr>,
 }
 
 impl<B, E> LocalCallExecutor<B, E> {
 	/// Creates new instance of local call executor.
-	pub fn new(backend: Arc<B>, executor: E) -> Self {
+	pub fn new(
+		backend: Arc<B>,
+		executor: E,
+		keystore: Option<primitives::traits::BareCryptoStorePtr>,
+	) -> Self {
 		LocalCallExecutor {
 			backend,
 			executor,
+			keystore,
 		}
 	}
 }
@@ -167,6 +174,7 @@ impl<B, E> Clone for LocalCallExecutor<B, E> where E: Clone {
 		LocalCallExecutor {
 			backend: self.backend.clone(),
 			executor: self.executor.clone(),
+			keystore: self.keystore.clone(),
 		}
 	}
 }
@@ -197,6 +205,7 @@ where
 			&self.executor,
 			method,
 			call_data,
+			self.keystore.clone(),
 		).execute_using_consensus_failure_handler::<_, NeverNativeValue, fn() -> _>(
 			strategy.get_manager(),
 			false,
@@ -229,6 +238,7 @@ where
 		native_call: Option<NC>,
 		side_effects_handler: Option<&mut O>,
 		recorder: &Option<Rc<RefCell<ProofRecorder<Block>>>>,
+		enable_keystore: bool,
 	) -> Result<NativeOrEncoded<R>, error::Error> where ExecutionManager<EM>: Clone {
 		match initialize_block {
 			InitializeBlock::Do(ref init_block)
@@ -239,6 +249,12 @@ where
 			_ => {},
 		}
 
+		let keystore = if enable_keystore {
+			self.keystore.clone()
+		} else {
+			None
+		};
+
 		let mut state = self.backend.state_at(*at)?;
 
 		let result = match recorder {
@@ -262,6 +278,7 @@ where
 					&self.executor,
 					method,
 					call_data,
+					keystore,
 				)
 				.execute_using_consensus_failure_handler(
 					execution_manager,
@@ -279,6 +296,7 @@ where
 				&self.executor,
 				method,
 				call_data,
+				keystore,
 			)
 			.execute_using_consensus_failure_handler(
 				execution_manager,
@@ -294,7 +312,14 @@ where
 	fn runtime_version(&self, id: &BlockId<Block>) -> error::Result<RuntimeVersion> {
 		let mut overlay = OverlayedChanges::default();
 		let state = self.backend.state_at(*id)?;
-		let mut ext = Ext::new(&mut overlay, &state, self.backend.changes_trie_storage(), NeverOffchainExt::new());
+
+		let mut ext = Ext::new(
+			&mut overlay,
+			&state,
+			self.backend.changes_trie_storage(),
+			NeverOffchainExt::new(),
+			None,
+		);
 		let version = self.executor.runtime_version(&mut ext);
 		self.backend.destroy_state(state)?;
 		version.ok_or(error::Error::VersionInvalid.into())
@@ -330,6 +355,7 @@ where
 			&self.executor,
 			method,
 			call_data,
+			self.keystore.clone(),
 		).execute_using_consensus_failure_handler(
 			manager,
 			true,
@@ -356,6 +382,7 @@ where
 			&self.executor,
 			method,
 			call_data,
+			self.keystore.clone(),
 		)
 		.map_err(Into::into)
 	}

substrate/core/client/src/client.rs

@@ -20,66 +20,59 @@ use std::{
 	marker::PhantomData, collections::{HashSet, BTreeMap, HashMap}, sync::Arc,
 	panic::UnwindSafe, result, cell::RefCell, rc::Rc,
 };
-use crate::error::Error;
+use log::{info, trace, warn};
 use futures::channel::mpsc;
 use parking_lot::{Mutex, RwLock};
-use primitives::NativeOrEncoded;
-use sr_primitives::{
-	Justification,
-	generic::{BlockId, SignedBlock},
-};
-use consensus::{
-	Error as ConsensusError, BlockImportParams,
-	ImportResult, BlockOrigin, ForkChoiceStrategy,
-	well_known_cache_keys::Id as CacheKeyId,
-	SelectChain, self,
-};
-use sr_primitives::traits::{
-	Block as BlockT, Header as HeaderT, Zero, NumberFor,
-	ApiRef, ProvideRuntimeApi, SaturatedConversion, One, DigestFor,
-};
-use sr_primitives::generic::DigestItem;
-use sr_primitives::BuildStorage;
-use crate::runtime_api::{
-	CallRuntimeAt, ConstructRuntimeApi, Core as CoreApi, ProofRecorder,
-	InitializeBlock,
-};
+use codec::{Encode, Decode};
+use hash_db::{Hasher, Prefix};
 use primitives::{
 	Blake2Hasher, H256, ChangesTrieConfiguration, convert_hash,
-	NeverNativeValue, ExecutionContext
+	NeverNativeValue, ExecutionContext,
+	storage::{StorageKey, StorageData, well_known_keys}, NativeOrEncoded
+};
+use substrate_telemetry::{telemetry, SUBSTRATE_INFO};
+use sr_primitives::{
+	Justification, BuildStorage,
+	generic::{BlockId, SignedBlock, DigestItem},
+	traits::{
+		Block as BlockT, Header as HeaderT, Zero, NumberFor,
+		ApiRef, ProvideRuntimeApi, SaturatedConversion, One, DigestFor,
+	},
 };
-use primitives::storage::{StorageKey, StorageData};
-use primitives::storage::well_known_keys;
-use codec::{Encode, Decode};
 use state_machine::{
 	DBValue, Backend as StateBackend, CodeExecutor, ChangesTrieAnchorBlockId,
 	ExecutionStrategy, ExecutionManager, prove_read, prove_child_read,
 	ChangesTrieRootsStorage, ChangesTrieStorage,
 	key_changes, key_changes_proof, OverlayedChanges, NeverOffchainExt,
 };
-use hash_db::{Hasher, Prefix};
-
-use crate::backend::{
-	self, BlockImportOperation, PrunableStateChangesTrieStorage,
-	StorageCollection, ChildStorageCollection
-};
-use crate::blockchain::{
-	self, Info as ChainInfo, Backend as ChainBackend,
-	HeaderBackend as ChainHeaderBackend, ProvideCache, Cache,
-};
-use crate::call_executor::{CallExecutor, LocalCallExecutor};
 use executor::{RuntimeVersion, RuntimeInfo};
-use crate::notifications::{StorageNotifications, StorageEventStream};
-use crate::light::{call_executor::prove_execution, fetcher::ChangesProof};
-use crate::cht;
-use crate::error;
-use crate::in_mem;
-use crate::block_builder::{self, api::BlockBuilder as BlockBuilderAPI};
-use crate::genesis;
-use substrate_telemetry::{telemetry, SUBSTRATE_INFO};
-
-use log::{info, trace, warn};
+use consensus::{
+	Error as ConsensusError, BlockImportParams,
+	ImportResult, BlockOrigin, ForkChoiceStrategy,
+	well_known_cache_keys::Id as CacheKeyId,
+	SelectChain, self,
+};
 
+use crate::{
+	runtime_api::{
+		CallRuntimeAt, ConstructRuntimeApi, Core as CoreApi, ProofRecorder,
+		InitializeBlock,
+	},
+	backend::{
+		self, BlockImportOperation, PrunableStateChangesTrieStorage,
+		StorageCollection, ChildStorageCollection
+	},
+	blockchain::{
+		self, Info as ChainInfo, Backend as ChainBackend,
+		HeaderBackend as ChainHeaderBackend, ProvideCache, Cache,
+	},
+	call_executor::{CallExecutor, LocalCallExecutor},
+	notifications::{StorageNotifications, StorageEventStream},
+	light::{call_executor::prove_execution, fetcher::ChangesProof},
+	block_builder::{self, api::BlockBuilder as BlockBuilderAPI},
+	error::Error,
+	cht, error, in_mem, genesis
+};
 
 /// Type that implements `futures::Stream` of block import events.
 pub type ImportNotifications<Block> = mpsc::UnboundedReceiver<BlockImportNotification<Block>>;
@@ -260,6 +253,7 @@ impl<H> PrePostHeader<H> {
 pub fn new_in_mem<E, Block, S, RA>(
 	executor: E,
 	genesis_storage: S,
+	keystore: Option<primitives::traits::BareCryptoStorePtr>,
 ) -> error::Result<Client<
 	in_mem::Backend<Block, Blake2Hasher>,
 	LocalCallExecutor<in_mem::Backend<Block, Blake2Hasher>, E>,
@@ -270,7 +264,7 @@ pub fn new_in_mem<E, Block, S, RA>(
 	S: BuildStorage,
 	Block: BlockT<Hash=H256>,
 {
-	new_with_backend(Arc::new(in_mem::Backend::new()), executor, genesis_storage)
+	new_with_backend(Arc::new(in_mem::Backend::new()), executor, genesis_storage, keystore)
 }
 
 /// Create a client with the explicitly provided backend.
@@ -279,6 +273,7 @@ pub fn new_with_backend<B, E, Block, S, RA>(
 	backend: Arc<B>,
 	executor: E,
 	build_genesis_storage: S,
+	keystore: Option<primitives::traits::BareCryptoStorePtr>,
 ) -> error::Result<Client<B, LocalCallExecutor<B, E>, Block, RA>>
 	where
 		E: CodeExecutor<Blake2Hasher> + RuntimeInfo,
@@ -286,10 +281,24 @@ pub fn new_with_backend<B, E, Block, S, RA>(
 		Block: BlockT<Hash=H256>,
 		B: backend::LocalBackend<Block, Blake2Hasher>
 {
-	let call_executor = LocalCallExecutor::new(backend.clone(), executor);
+	let call_executor = LocalCallExecutor::new(backend.clone(), executor, keystore);
 	Client::new(backend, call_executor, build_genesis_storage, Default::default())
 }
 
+/// Figure out the block type for a given type (for now, just a `Client`).
+pub trait BlockOf {
+	/// The type of the block.
+	type Type: BlockT<Hash=H256>;
+}
+
+impl<B, E, Block, RA> BlockOf for Client<B, E, Block, RA> where
+	B: backend::Backend<Block, Blake2Hasher>,
+	E: CallExecutor<Block, Blake2Hasher>,
+	Block: BlockT<Hash=H256>,
+{
+	type Type = Block;
+}
+
 impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	B: backend::Backend<Block, Blake2Hasher>,
 	E: CallExecutor<Block, Blake2Hasher>,
@@ -362,7 +371,8 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	pub fn storage(&self, id: &BlockId<Block>, key: &StorageKey) -> error::Result<Option<StorageData>> {
 		Ok(self.state_at(id)?
 			.storage(&key.0).map_err(|e| error::Error::from_state(Box::new(e)))?
-			.map(StorageData))
+			.map(StorageData)
+		)
 	}
 
 	/// Given a `BlockId` and a key, return the value under the hash in that block.
@@ -1414,6 +1424,8 @@ impl<B, E, Block, RA> CallRuntimeAt<Block> for Client<B, E, Block, RA> where
 		context: ExecutionContext,
 		recorder: &Option<Rc<RefCell<ProofRecorder<Block>>>>,
 	) -> error::Result<NativeOrEncoded<R>> {
+		let enable_keystore = context.enable_keystore();
+
 		let manager = match context {
 			ExecutionContext::BlockConstruction =>
 				self.execution_strategies.block_construction.get_manager(),
@@ -1443,6 +1455,7 @@ impl<B, E, Block, RA> CallRuntimeAt<Block> for Client<B, E, Block, RA> where
 			native_call,
 			offchain_extensions.as_mut(),
 			recorder,
+			enable_keystore,
 		)
 	}
 

substrate/core/client/src/genesis.rs

@@ -96,6 +96,7 @@ mod tests {
 			&executor(),
 			"Core_initialize_block",
 			&header.encode(),
+			None,
 		).execute(
 			ExecutionStrategy::NativeElseWasm,
 		).unwrap();
@@ -109,6 +110,7 @@ mod tests {
 				&executor(),
 				"BlockBuilder_apply_extrinsic",
 				&tx.encode(),
+				None,
 			).execute(
 				ExecutionStrategy::NativeElseWasm,
 			).unwrap();
@@ -122,6 +124,7 @@ mod tests {
 			&executor(),
 			"BlockBuilder_finalize_block",
 			&[],
+			None,
 		).execute(
 			ExecutionStrategy::NativeElseWasm,
 		).unwrap();
@@ -148,7 +151,7 @@ mod tests {
 	#[test]
 	fn construct_genesis_should_work_with_native() {
 		let mut storage = GenesisConfig::new(false,
-			vec![Sr25519Keyring::One.into(), Sr25519Keyring::Two.into()],
+			vec![Sr25519Keyring::One.public().into(), Sr25519Keyring::Two.public().into()],
 			vec![AccountKeyring::One.into(), AccountKeyring::Two.into()],
 			1000,
 			None,
@@ -170,6 +173,7 @@ mod tests {
 			&executor(),
 			"Core_execute_block",
 			&b1data,
+			None,
 		).execute(
 			ExecutionStrategy::NativeElseWasm,
 		).unwrap();
@@ -178,7 +182,7 @@ mod tests {
 	#[test]
 	fn construct_genesis_should_work_with_wasm() {
 		let mut storage = GenesisConfig::new(false,
-			vec![Sr25519Keyring::One.into(), Sr25519Keyring::Two.into()],
+			vec![Sr25519Keyring::One.public().into(), Sr25519Keyring::Two.public().into()],
 			vec![AccountKeyring::One.into(), AccountKeyring::Two.into()],
 			1000,
 			None,
@@ -200,6 +204,7 @@ mod tests {
 			&executor(),
 			"Core_execute_block",
 			&b1data,
+			None,
 		).execute(
 			ExecutionStrategy::AlwaysWasm,
 		).unwrap();
@@ -208,7 +213,7 @@ mod tests {
 	#[test]
 	fn construct_genesis_with_bad_transaction_should_panic() {
 		let mut storage = GenesisConfig::new(false,
-			vec![Sr25519Keyring::One.into(), Sr25519Keyring::Two.into()],
+			vec![Sr25519Keyring::One.public().into(), Sr25519Keyring::Two.public().into()],
 			vec![AccountKeyring::One.into(), AccountKeyring::Two.into()],
 			68,
 			None,
@@ -230,6 +235,7 @@ mod tests {
 			&executor(),
 			"Core_execute_block",
 			&b1data,
+			None,
 		).execute(
 			ExecutionStrategy::NativeElseWasm,
 		);

substrate/core/client/src/lib.rs

@@ -62,7 +62,8 @@
 //! 	backend.clone(),
 //! 	LocalCallExecutor::new(
 //! 		backend.clone(),
-//! 		NativeExecutor::<LocalExecutor>::new(None)
+//! 		NativeExecutor::<LocalExecutor>::new(None),
+//! 		None,
 //!		),
 //! 	// This parameter provides the storage for the chain genesis.
 //! 	<(StorageOverlay, ChildrenStorageOverlay)>::default(),
@@ -114,7 +115,7 @@ pub use crate::client::{
 	new_in_mem,
 	BlockBody, BlockStatus, ImportNotifications, FinalityNotifications, BlockchainEvents,
 	BlockImportNotification, Client, ClientInfo, ExecutionStrategies, FinalityNotification,
-	LongestChain,
+	LongestChain, BlockOf,
 	utils,
 };
 #[cfg(feature = "std")]

substrate/core/client/src/light/call_executor.rs

@@ -94,8 +94,8 @@ where
 		call_data: &[u8],
 		_strategy: ExecutionStrategy,
 		_side_effects_handler: Option<&mut O>,
-	)
-		-> ClientResult<Vec<u8>> {
+	) -> ClientResult<Vec<u8>>
+	{
 		let block_hash = self.blockchain.expect_block_hash_from_id(id)?;
 		let block_header = self.blockchain.expect_header(id.clone())?;
 
@@ -130,6 +130,7 @@ where
 		_native_call: Option<NC>,
 		side_effects_handler: Option<&mut O>,
 		_recorder: &Option<Rc<RefCell<ProofRecorder<Block>>>>,
+		_enable_keystore: bool,
 	) -> ClientResult<NativeOrEncoded<R>> where ExecutionManager<EM>: Clone {
 		let block_initialized = match initialize_block {
 			InitializeBlock::Do(ref init_block) => {
@@ -143,11 +144,23 @@ where
 			return Err(ClientError::NotAvailableOnLightClient.into());
 		}
 
-		self.call(at, method, call_data, (&execution_manager).into(), side_effects_handler).map(NativeOrEncoded::Encoded)
+		self.call(
+			at,
+			method,
+			call_data,
+			(&execution_manager).into(),
+			side_effects_handler,
+		).map(NativeOrEncoded::Encoded)
 	}
 
 	fn runtime_version(&self, id: &BlockId<Block>) -> ClientResult<RuntimeVersion> {
-		let call_result = self.call(id, "Core_version", &[], ExecutionStrategy::NativeElseWasm, NeverOffchainExt::new())?;
+		let call_result = self.call(
+			id,
+			"Core_version",
+			&[],
+			ExecutionStrategy::NativeElseWasm,
+			NeverOffchainExt::new()
+		)?;
 		RuntimeVersion::decode(&mut call_result.as_slice())
 			.map_err(|_| ClientError::VersionInvalid.into())
 	}
@@ -270,6 +283,7 @@ impl<Block, B, Remote, Local> CallExecutor<Block, Blake2Hasher> for
 		native_call: Option<NC>,
 		side_effects_handler: Option<&mut O>,
 		recorder: &Option<Rc<RefCell<ProofRecorder<Block>>>>,
+		enable_keystore: bool,
 	) -> ClientResult<NativeOrEncoded<R>> where ExecutionManager<EM>: Clone {
 		// there's no actual way/need to specify native/wasm execution strategy on light node
 		// => we can safely ignore passed values
@@ -296,6 +310,7 @@ impl<Block, B, Remote, Local> CallExecutor<Block, Blake2Hasher> for
 				native_call,
 				side_effects_handler,
 				recorder,
+				enable_keystore,
 			).map_err(|e| ClientError::Execution(Box::new(e.to_string()))),
 			false => CallExecutor::contextual_call::<
 				_,
@@ -318,6 +333,7 @@ impl<Block, B, Remote, Local> CallExecutor<Block, Blake2Hasher> for
 				native_call,
 				side_effects_handler,
 				recorder,
+				enable_keystore,
 			).map_err(|e| ClientError::Execution(Box::new(e.to_string()))),
 		}
 	}
@@ -463,6 +479,7 @@ pub fn check_execution_proof<Header, E, H>(
 		executor,
 		"Core_initialize_block",
 		&next_block.encode(),
+		None,
 	)?;
 
 	// execute method
@@ -472,6 +489,7 @@ pub fn check_execution_proof<Header, E, H>(
 		executor,
 		&request.method,
 		&request.call_data,
+		None,
 	)?;
 
 	Ok(local_result)

substrate/core/client/src/light/fetcher.rs

@@ -424,7 +424,6 @@ impl<E, Block, H, S, F> FetchChecker<Block> for LightDataChecker<E, H, Block, S,
 		request: &RemoteBodyRequest<Block::Header>,
 		body: Vec<Block::Extrinsic>
 	) -> ClientResult<Vec<Block::Extrinsic>> {
-
 		// TODO: #2621
 		let	extrinsics_root = HashFor::<Block>::ordered_trie_root(body.iter().map(Encode::encode));
 		if *request.header.extrinsics_root() == extrinsics_root {

substrate/core/client/src/light/mod.rs

@@ -73,7 +73,7 @@ pub fn new_light<B, S, F, GS, RA, E>(
 		E: CodeExecutor<Blake2Hasher> + RuntimeInfo,
 {
 	let remote_executor = RemoteCallExecutor::new(backend.blockchain().clone(), fetcher);
-	let local_executor = LocalCallExecutor::new(backend.clone(), code_executor);
+	let local_executor = LocalCallExecutor::new(backend.clone(), code_executor, None);
 	let executor = RemoteOrLocalCallExecutor::new(backend.clone(), remote_executor, local_executor);
 	Client::new(backend, executor, genesis_storage, Default::default())
 }