Move authorities interface from Core to consensus (#1412)

* Move authorities interface from Core to consensus

f

* notify all caches of block insert + create with up-to-date best_fin

* merged authorities_are_cached from light_grandpa_import2

* Add ProvideCache trait

* Create helper function for 'get_cache'

* Fix some formatting

* Bump impl version

* Resolve wasm conflicts

* Apply review comments

* Use try_for_each

* Move authorities interface from Core to consensus

f

* notify all caches of block insert + create with up-to-date best_fin

* merged authorities_are_cached from light_grandpa_import2

* Add ProvideCache trait

* Create helper function for 'get_cache'

* Fix some formatting

* Bump impl version

* Resolve wasm conflicts

* Apply review comments

* Use try_for_each

* Move authorities interface from Core to consensus

f

* notify all caches of block insert + create with up-to-date best_fin

* merged authorities_are_cached from light_grandpa_import2

* Add ProvideCache trait

* Create helper function for 'get_cache'

* Fix some formatting

* Bump impl version

* Resolve wasm conflicts

* Apply review comments

* Use try_for_each

* Increment impl_version

* Update lib.rs

substrate/core/consensus/aura/Cargo.toml

@@ -25,6 +25,7 @@ parking_lot = "0.7.1"
 error-chain = "0.12"
 log = "0.4"
 consensus_common = { package = "substrate-consensus-common", path = "../common" }
+authorities = { package = "substrate-consensus-authorities", path = "../authorities" }
 
 [dev-dependencies]
 keyring = { package = "substrate-keyring", path = "../../keyring" }

substrate/core/consensus/aura/src/lib.rs

@@ -29,21 +29,23 @@
 use std::{sync::Arc, time::Duration, thread, marker::PhantomData, hash::Hash, fmt::Debug};
 
 use parity_codec::{Encode, Decode};
-use consensus_common::{
-	Authorities, BlockImport, Environment, Proposer, ForkChoiceStrategy
+use consensus_common::{self, Authorities, BlockImport, Environment, Proposer,
+	ForkChoiceStrategy, ImportBlock, BlockOrigin, Error as ConsensusError,
 };
+use consensus_common::well_known_cache_keys;
 use consensus_common::import_queue::{Verifier, BasicQueue, SharedBlockImport, SharedJustificationImport};
 use client::ChainHead;
 use client::block_builder::api::BlockBuilder as BlockBuilderApi;
+use client::blockchain::ProvideCache;
 use client::runtime_api::ApiExt;
-use consensus_common::{ImportBlock, BlockOrigin};
 use aura_primitives::AURA_ENGINE_ID;
 use runtime_primitives::{generic, generic::BlockId, Justification};
 use runtime_primitives::traits::{
-	Block, Header, Digest, DigestItemFor, DigestItem, ProvideRuntimeApi
+	Block, Header, Digest, DigestItemFor, DigestItem, ProvideRuntimeApi, AuthorityIdFor,
 };
 use primitives::Pair;
 use inherents::{InherentDataProviders, InherentData, RuntimeString};
+use authorities::AuthoritiesApi;
 
 use futures::{Stream, Future, IntoFuture, future};
 use tokio::timer::Timeout;
@@ -179,12 +181,13 @@ pub fn start_aura_thread<B, C, E, I, P, SO, Error, OnExit>(
 	force_authoring: bool,
 ) -> Result<(), consensus_common::Error> where
 	B: Block + 'static,
-	C: Authorities<B> + ChainHead<B> + Send + Sync + 'static,
+	C: ChainHead<B> + ProvideRuntimeApi + ProvideCache<B> + Send + Sync + 'static,
+	C::Api: AuthoritiesApi<B>,
 	E: Environment<B, Error=Error> + Send + Sync + 'static,
 	E::Proposer: Proposer<B, Error=Error> + Send + 'static,
 	<<E::Proposer as Proposer<B>>::Create as IntoFuture>::Future: Send + 'static,
 	I: BlockImport<B> + Send + Sync + 'static,
-	Error: From<C::Error> + From<I::Error> + 'static,
+	Error: From<I::Error> + 'static,
 	P: Pair + Send + Sync + 'static,
 	P::Public: Encode + Decode + Eq + Clone + Debug + Hash + Send + Sync + 'static,
 	P::Signature: Encode,
@@ -226,12 +229,13 @@ pub fn start_aura<B, C, E, I, P, SO, Error, OnExit>(
 	force_authoring: bool,
 ) -> Result<impl Future<Item=(), Error=()>, consensus_common::Error> where
 	B: Block,
-	C: Authorities<B> + ChainHead<B>,
+	C: ChainHead<B> + ProvideRuntimeApi + ProvideCache<B>,
+	C::Api: AuthoritiesApi<B>,
 	E: Environment<B, Error=Error>,
 	E::Proposer: Proposer<B, Error=Error>,
 	<<E::Proposer as Proposer<B>>::Create as IntoFuture>::Future: Send + 'static,
 	I: BlockImport<B> + Send + Sync + 'static,
-	Error: From<C::Error> + From<I::Error>,
+	Error: From<I::Error>,
 	P: Pair + Send + Sync + 'static,
 	P::Public: Hash + Eq + Send + Sync + Clone + Debug + Encode + Decode + 'static,
 	P::Signature: Encode,
@@ -270,7 +274,8 @@ struct AuraWorker<C, E, I, P, SO> {
 }
 
 impl<B: Block, C, E, I, P, Error, SO> SlotWorker<B> for AuraWorker<C, E, I, P, SO> where
-	C: Authorities<B>,
+	C: ProvideRuntimeApi + ProvideCache<B>,
+	C::Api: AuthoritiesApi<B>,
 	E: Environment<B, Error=Error>,
 	E::Proposer: Proposer<B, Error=Error>,
 	<<E::Proposer as Proposer<B>>::Create as IntoFuture>::Future: Send + 'static,
@@ -278,7 +283,7 @@ impl<B: Block, C, E, I, P, Error, SO> SlotWorker<B> for AuraWorker<C, E, I, P, S
 	P: Pair + Send + Sync + 'static,
 	P::Public: Hash + Eq + Send + Sync + Clone + Debug + Encode + Decode + 'static,
 	P::Signature: Encode,
-	Error: From<C::Error> + From<I::Error>,
+	Error: From<I::Error>,
 	SO: SyncOracle + Send + Clone,
 	DigestItemFor<B>: CompatibleDigestItem<P> + DigestItem<AuthorityId=AuthorityId<P>>,
 	Error: ::std::error::Error + Send + 'static + From<::consensus_common::Error>,
@@ -306,7 +311,7 @@ impl<B: Block, C, E, I, P, Error, SO> SlotWorker<B> for AuraWorker<C, E, I, P, S
 		let (timestamp, slot_num, slot_duration) =
 			(slot_info.timestamp, slot_info.number, slot_info.duration);
 
-		let authorities = match client.authorities(&BlockId::Hash(chain_head.hash())) {
+		let authorities = match authorities(client.as_ref(), &BlockId::Hash(chain_head.hash())) {
 			Ok(authorities) => authorities,
 			Err(e) => {
 				warn!(
@@ -418,7 +423,7 @@ impl<B: Block, C, E, I, P, Error, SO> SlotWorker<B> for AuraWorker<C, E, I, P, S
 						"hash_previously" => ?pre_hash
 					);
 
-					if let Err(e) = block_import.import_block(import_block, None) {
+					if let Err(e) = block_import.import_block(import_block, Default::default()) {
 						warn!(target: "aura", "Error with block built on {:?}: {:?}",
 							  parent_hash, e);
 						telemetry!(CONSENSUS_WARN; "aura.err_with_block_built_on";
@@ -573,13 +578,14 @@ impl<B: Block> ExtraVerification<B> for NothingExtra {
 
 #[forbid(deprecated)]
 impl<B: Block, C, E, P> Verifier<B> for AuraVerifier<C, E, P> where
-	C: Authorities<B> + ProvideRuntimeApi + Send + Sync,
+	C: ProvideRuntimeApi + Send + Sync,
 	C::Api: BlockBuilderApi<B>,
 	DigestItemFor<B>: CompatibleDigestItem<P> + DigestItem<AuthorityId=AuthorityId<P>>,
 	E: ExtraVerification<B>,
 	P: Pair + Send + Sync + 'static,
 	P::Public: Send + Sync + Hash + Eq + Clone + Decode + Encode + Debug + AsRef<P::Public> + 'static,
 	P::Signature: Encode + Decode,
+	Self: Authorities<B>,
 {
 	fn verify(
 		&self,
@@ -593,7 +599,7 @@ impl<B: Block, C, E, P> Verifier<B> for AuraVerifier<C, E, P> where
 			.map_err(|e| format!("Could not extract timestamp and slot: {:?}", e))?;
 		let hash = header.hash();
 		let parent_hash = *header.parent_hash();
-		let authorities = self.client.authorities(&BlockId::Hash(parent_hash))
+		let authorities = self.authorities(&BlockId::Hash(parent_hash))
 			.map_err(|e| format!("Could not fetch authorities at {:?}: {:?}", parent_hash, e))?;
 
 		let extra_verification = self.extra.verify(
@@ -669,6 +675,31 @@ impl<B: Block, C, E, P> Verifier<B> for AuraVerifier<C, E, P> where
 	}
 }
 
+impl<B, C, E, P> Authorities<B> for AuraVerifier<C, E, P> where
+	B: Block,
+	C: ProvideRuntimeApi + ProvideCache<B>,
+	C::Api: AuthoritiesApi<B>,
+{
+	type Error = ConsensusError;
+
+	fn authorities(&self, at: &BlockId<B>) -> Result<Vec<AuthorityIdFor<B>>, Self::Error> {
+		authorities(self.client.as_ref(), at)
+	}
+}
+
+fn authorities<B, C>(client: &C, at: &BlockId<B>) -> Result<Vec<AuthorityIdFor<B>>, ConsensusError> where
+	B: Block,
+	C: ProvideRuntimeApi + ProvideCache<B>,
+	C::Api: AuthoritiesApi<B>,
+{
+	client
+		.cache()
+		.and_then(|cache| cache.get_at(well_known_cache_keys::AUTHORITIES, at)
+			.and_then(|v| Decode::decode(&mut &v[..])))
+		.or_else(|| client.runtime_api().authorities(at).ok())
+		.ok_or_else(|| consensus_common::ErrorKind::InvalidAuthoritiesSet.into())
+}
+
 /// The Aura import queue type.
 pub type AuraImportQueue<B> = BasicQueue<B>;
 
@@ -697,8 +728,8 @@ pub fn import_queue<B, C, E, P>(
 	allow_old_seals: bool,
 ) -> Result<AuraImportQueue<B>, consensus_common::Error> where
 	B: Block,
-	C: 'static + Authorities<B> + ProvideRuntimeApi + Send + Sync,
-	C::Api: BlockBuilderApi<B>,
+	C: 'static + ProvideRuntimeApi + ProvideCache<B> + Send + Sync,
+	C::Api: BlockBuilderApi<B> + AuthoritiesApi<B>,
 	DigestItemFor<B>: CompatibleDigestItem<P> + DigestItem<AuthorityId=AuthorityId<P>>,
 	E: 'static + ExtraVerification<B>,
 	P: Pair + Send + Sync + 'static,
@@ -889,4 +920,16 @@ mod tests {
 
 		runtime.block_on(wait_for.select(drive_to_completion).map_err(|_| ())).unwrap();
 	}
+
+	#[test]
+	fn authorities_call_works() {
+		let client = test_client::new();
+
+		assert_eq!(client.info().unwrap().chain.best_number, 0);
+		assert_eq!(authorities(&client, &BlockId::Number(0)).unwrap(), vec![
+			Keyring::Alice.into(),
+			Keyring::Bob.into(),
+			Keyring::Charlie.into()
+		]);
+	}
 }

substrate/core/consensus/authorities/Cargo.toml

@@ -0,0 +1,29 @@
+[package]
+name = "substrate-consensus-authorities"
+version = "0.1.0"
+authors = ["Parity Technologies <admin@parity.io>"]
+description = "Primitives for Aura consensus"
+edition = "2018"
+
+[dependencies]
+parity-codec = { version = "3.0", default-features = false }
+substrate-client = { path = "../../client", default-features = false }
+primitives = { package = "substrate-primitives", path = "../../primitives", default-features = false }
+runtime_support = { package = "srml-support", path = "../../../srml/support", default-features = false }
+runtime_primitives = { package = "sr-primitives", path = "../../sr-primitives", default-features = false }
+sr-version = { path = "../../sr-version", default-features = false }
+runtime_io = { package = "sr-io", path = "../../sr-io", default-features = false }
+rstd = { package = "sr-std", path = "../../sr-std", default-features = false }
+
+[features]
+default = ["std"]
+std = [
+	"parity-codec/std",
+	"substrate-client/std",
+	"primitives/std",
+	"runtime_support/std",
+	"runtime_primitives/std",
+	"sr-version/std",
+	"runtime_io/std",
+	"rstd/std"
+]

substrate/core/consensus/authorities/src/lib.rs

@@ -0,0 +1,31 @@
+// Copyright 2019 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Substrate is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Authorities API.
+
+#![cfg_attr(not(feature = "std"), no_std)]
+
+use substrate_client::decl_runtime_apis;
+use runtime_primitives::traits::AuthorityIdFor;
+use rstd::vec::Vec;
+
+decl_runtime_apis! {
+	/// Authorities API.
+	pub trait AuthoritiesApi {
+		/// Returns the authorities at the given block.
+		fn authorities() -> Vec<AuthorityIdFor<Block>>;
+	}
+}

substrate/core/consensus/common/src/block_import.rs

@@ -16,9 +16,10 @@
 
 //! Block import helpers.
 
-use runtime_primitives::traits::{AuthorityIdFor, Block as BlockT, DigestItemFor, Header as HeaderT, NumberFor};
+use runtime_primitives::traits::{Block as BlockT, DigestItemFor, Header as HeaderT, NumberFor};
 use runtime_primitives::Justification;
 use std::borrow::Cow;
+use std::collections::HashMap;
 
 /// Block import result.
 #[derive(Debug, PartialEq, Eq)]
@@ -175,11 +176,13 @@ pub trait BlockImport<B: BlockT> {
 		parent_hash: B::Hash,
 	) -> Result<ImportResult, Self::Error>;
 
-	/// Import a Block alongside the new authorities valid from this block forward
+	/// Import a block.
+	///
+	/// Cached data can be accessed through the blockchain cache.
 	fn import_block(
 		&self,
 		block: ImportBlock<B>,
-		new_authorities: Option<Vec<AuthorityIdFor<B>>>,
+		cache: HashMap<Vec<u8>, Vec<u8>>,
 	) -> Result<ImportResult, Self::Error>;
 }
 

substrate/core/consensus/common/src/error.rs

@@ -58,6 +58,12 @@ error_chain! {
 			display("Message signature {:?} by {:?} is invalid.", s, a),
 		}
 
+		/// Invalid authorities set received from the runtime.
+		InvalidAuthoritiesSet {
+			description("authorities set is invalid"),
+			display("Current state of blockchain has invalid authorities set"),
+		}
+
 		/// Account is not an authority.
 		InvalidAuthority(a: Public) {
 			description("Message sender is not a valid authority"),

substrate/core/consensus/common/src/import_queue.rs

@@ -28,6 +28,7 @@ use crate::block_import::{
 	BlockImport, BlockOrigin, ImportBlock, ImportedAux, ImportResult, JustificationImport,
 };
 use crossbeam_channel::{self as channel, Receiver, Sender};
+use parity_codec::Encode;
 
 use std::sync::Arc;
 use std::thread;
@@ -38,6 +39,7 @@ use runtime_primitives::traits::{
 use runtime_primitives::Justification;
 
 use crate::error::Error as ConsensusError;
+use parity_codec::alloc::collections::hash_map::HashMap;
 
 /// Shared block import struct used by the queue.
 pub type SharedBlockImport<B> = Arc<dyn BlockImport<B, Error = ConsensusError> + Send + Sync>;
@@ -566,7 +568,12 @@ pub fn import_single_block<B: BlockT, V: Verifier<B>>(
 			BlockImportError::VerificationFailed(peer.clone(), msg)
 		})?;
 
-	import_error(import_handle.import_block(import_block, new_authorities))
+	let mut cache = HashMap::new();
+	if let Some(authorities) = new_authorities {
+		cache.insert(crate::well_known_cache_keys::AUTHORITIES.to_vec(), authorities.encode());
+	}
+
+	import_error(import_handle.import_block(import_block, cache))
 }
 
 #[cfg(test)]

substrate/core/consensus/common/src/lib.rs

@@ -53,7 +53,9 @@ pub use block_import::{
 
 /// Trait for getting the authorities at a given block.
 pub trait Authorities<B: Block> {
-	type Error: ::std::error::Error + Send + 'static;	/// Get the authorities at the given block.
+	type Error: std::error::Error + Send + 'static;
+
+	/// Get the authorities at the given block.
 	fn authorities(&self, at: &BlockId<B>) -> Result<Vec<AuthorityIdFor<B>>, Self::Error>;
 }
 
@@ -115,3 +117,9 @@ impl<T: SyncOracle> SyncOracle for Arc<T> {
 		T::is_offline(&*self)
 	}
 }
+
+/// A list of all well known keys in the cache.
+pub mod well_known_cache_keys {
+	/// A list of authorities.
+	pub const AUTHORITIES: &'static [u8] = b"auth";
+}