Refactoring: Remove ExtraVerification in the import queue (#2844)

* Remove ExtraVerification in the import queue

* Fix Babe

* Fix node-template

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

@@ -68,7 +68,7 @@ use substrate_telemetry::{telemetry, CONSENSUS_TRACE, CONSENSUS_DEBUG, CONSENSUS
 use slots::{CheckedHeader, SlotData, SlotWorker, SlotInfo, SlotCompatible, slot_now, check_equivocation};
 
 pub use aura_primitives::*;
-pub use consensus_common::{SyncOracle, ExtraVerification};
+pub use consensus_common::SyncOracle;
 pub use digest::CompatibleDigestItem;
 
 mod digest;
@@ -446,14 +446,13 @@ fn check_header<C, B: Block, P: Pair>(
 }
 
 /// A verifier for Aura blocks.
-pub struct AuraVerifier<C, E, P> {
+pub struct AuraVerifier<C, P> {
 	client: Arc<C>,
-	extra: E,
 	phantom: PhantomData<P>,
 	inherent_data_providers: inherents::InherentDataProviders,
 }
 
-impl<C, E, P> AuraVerifier<C, E, P>
+impl<C, P> AuraVerifier<C, P>
 	where P: Send + Sync + 'static
 {
 	fn check_inherents<B: Block>(
@@ -505,24 +504,11 @@ impl<C, E, P> AuraVerifier<C, E, P>
 	}
 }
 
-/// No-op extra verification.
-#[derive(Debug, Clone, Copy)]
-pub struct NothingExtra;
-
-impl<B: Block> ExtraVerification<B> for NothingExtra {
-	type Verified = Result<(), String>;
-
-	fn verify(&self, _: &B::Header, _: Option<&[B::Extrinsic]>) -> Self::Verified {
-		Ok(())
-	}
-}
-
 #[forbid(deprecated)]
-impl<B: Block, C, E, P> Verifier<B> for AuraVerifier<C, E, P> where
+impl<B: Block, C, P> Verifier<B> for AuraVerifier<C, P> where
 	C: ProvideRuntimeApi + Send + Sync + client::backend::AuxStore,
 	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,
@@ -543,11 +529,6 @@ impl<B: Block, C, E, P> Verifier<B> for AuraVerifier<C, E, P> where
 		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(
-			&header,
-			body.as_ref().map(|x| &x[..]),
-		);
-
 		// we add one to allow for some small drift.
 		// FIXME #1019 in the future, alter this queue to allow deferring of
 		// headers
@@ -588,8 +569,6 @@ impl<B: Block, C, E, P> Verifier<B> for AuraVerifier<C, E, P> where
 				trace!(target: "aura", "Checked {:?}; importing.", pre_header);
 				telemetry!(CONSENSUS_TRACE; "aura.checked_and_importing"; "pre_header" => ?pre_header);
 
-				extra_verification.into_future().wait()?;
-
 				let new_authorities = pre_header.digest()
 					.log(DigestItem::as_authorities_change)
 					.map(|digest| digest.to_vec());
@@ -618,7 +597,7 @@ 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
+impl<B, C, P> Authorities<B> for AuraVerifier<C, P> where
 	B: Block,
 	C: ProvideRuntimeApi + ProvideCache<B>,
 	C::Api: AuthoritiesApi<B>,
@@ -697,21 +676,19 @@ fn register_aura_inherent_data_provider(
 }
 
 /// Start an import queue for the Aura consensus algorithm.
-pub fn import_queue<B, C, E, P>(
+pub fn import_queue<B, C, P>(
 	slot_duration: SlotDuration,
 	block_import: SharedBlockImport<B>,
 	justification_import: Option<SharedJustificationImport<B>>,
 	finality_proof_import: Option<SharedFinalityProofImport<B>>,
 	finality_proof_request_builder: Option<SharedFinalityProofRequestBuilder<B>>,
 	client: Arc<C>,
-	extra: E,
 	inherent_data_providers: InherentDataProviders,
 ) -> Result<AuraImportQueue<B>, consensus_common::Error> where
 	B: Block,
 	C: 'static + ProvideRuntimeApi + ProvideCache<B> + Send + Sync + AuxStore,
 	C::Api: BlockBuilderApi<B> + AuthoritiesApi<B>,
 	DigestItemFor<B>: CompatibleDigestItem<P> + DigestItem<AuthorityId=AuthorityId<P>>,
-	E: 'static + ExtraVerification<B>,
 	P: Pair + Send + Sync + 'static,
 	P::Public: Clone + Eq + Send + Sync + Hash + Debug + Encode + Decode + AsRef<P::Public>,
 	P::Signature: Encode + Decode,
@@ -722,7 +699,6 @@ pub fn import_queue<B, C, E, P>(
 	let verifier = Arc::new(
 		AuraVerifier {
 			client: client.clone(),
-			extra,
 			inherent_data_providers,
 			phantom: PhantomData,
 		}
@@ -794,7 +770,7 @@ mod tests {
 
 	impl TestNetFactory for AuraTestNet {
 		type Specialization = DummySpecialization;
-		type Verifier = AuraVerifier<PeersFullClient, NothingExtra, sr25519::Pair>;
+		type Verifier = AuraVerifier<PeersFullClient, sr25519::Pair>;
 		type PeerData = ();
 
 		/// Create new test network with peers and given config.
@@ -821,7 +797,6 @@ mod tests {
 					assert_eq!(slot_duration.get(), SLOT_DURATION);
 					Arc::new(AuraVerifier {
 						client,
-						extra: NothingExtra,
 						inherent_data_providers,
 						phantom: Default::default(),
 					})

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

@@ -31,7 +31,6 @@ use digest::CompatibleDigestItem;
 pub use digest::{BabePreDigest, BABE_VRF_PREFIX};
 pub use babe_primitives::*;
 pub use consensus_common::SyncOracle;
-use consensus_common::ExtraVerification;
 use runtime_primitives::{generic, generic::BlockId, Justification};
 use runtime_primitives::traits::{
 	Block, Header, Digest, DigestItemFor, DigestItem, ProvideRuntimeApi, AuthorityIdFor,
@@ -522,14 +521,13 @@ fn check_header<B: Block + Sized, C: AuxStore>(
 }
 
 /// A verifier for Babe blocks.
-pub struct BabeVerifier<C, E> {
+pub struct BabeVerifier<C> {
 	client: Arc<C>,
-	extra: E,
 	inherent_data_providers: inherents::InherentDataProviders,
 	threshold: u64,
 }
 
-impl<C, E> BabeVerifier<C, E> {
+impl<C> BabeVerifier<C> {
 	fn check_inherents<B: Block>(
 		&self,
 		block: B,
@@ -554,23 +552,10 @@ impl<C, E> BabeVerifier<C, E> {
 	}
 }
 
-/// No-op extra verification.
-#[derive(Debug, Clone, Copy)]
-pub struct NothingExtra;
-
-impl<B: Block> ExtraVerification<B> for NothingExtra {
-	type Verified = Result<(), String>;
-
-	fn verify(&self, _: &B::Header, _: Option<&[B::Extrinsic]>) -> Self::Verified {
-		Ok(())
-	}
-}
-
-impl<B: Block, C, E> Verifier<B> for BabeVerifier<C, E> where
+impl<B: Block, C> Verifier<B> for BabeVerifier<C> where
 	C: ProvideRuntimeApi + Send + Sync + AuxStore,
 	C::Api: BlockBuilderApi<B>,
 	DigestItemFor<B>: CompatibleDigestItem + DigestItem<AuthorityId=Public>,
-	E: ExtraVerification<B>,
 	Self: Authorities<B>,
 {
 	fn verify(
@@ -601,11 +586,6 @@ impl<B: Block, C, E> Verifier<B> for BabeVerifier<C, E> where
 		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(
-			&header,
-			body.as_ref().map(|x| &x[..]),
-		);
-
 		// we add one to allow for some small drift.
 		// FIXME #1019 in the future, alter this queue to allow deferring of
 		// headers
@@ -645,8 +625,6 @@ impl<B: Block, C, E> Verifier<B> for BabeVerifier<C, E> where
 					"babe.checked_and_importing";
 					"pre_header" => ?pre_header);
 
-				extra_verification.into_future().wait()?;
-
 				let new_authorities = pre_header.digest()
 					.log(DigestItem::as_authorities_change)
 					.map(|digest| digest.to_vec());
@@ -676,7 +654,7 @@ impl<B: Block, C, E> Verifier<B> for BabeVerifier<C, E> where
 	}
 }
 
-impl<B, C, E> Authorities<B> for BabeVerifier<C, E> where
+impl<B, C> Authorities<B> for BabeVerifier<C> where
 	B: Block,
 	C: ProvideRuntimeApi + ProvideCache<B>,
 	C::Api: AuthoritiesApi<B>,
@@ -851,7 +829,7 @@ mod tests {
 
 	impl TestNetFactory for BabeTestNet {
 		type Specialization = DummySpecialization;
-		type Verifier = BabeVerifier<PeersFullClient, NothingExtra>;
+		type Verifier = BabeVerifier<PeersFullClient>;
 		type PeerData = ();
 
 		/// Create new test network with peers and given config.
@@ -880,7 +858,6 @@ mod tests {
 			assert_eq!(config.get(), SLOT_DURATION);
 			Arc::new(BabeVerifier {
 				client,
-				extra: NothingExtra,
 				inherent_data_providers,
 				threshold: config.threshold(),
 			})

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

@@ -126,20 +126,6 @@ impl<T: SyncOracle> SyncOracle for Arc<T> {
 	}
 }
 
-/// Extra verification for blocks.
-pub trait ExtraVerification<B: Block>: Send + Sync {
-	/// Future that resolves when the block is verified, or fails with error if
-	/// not.
-	type Verified: IntoFuture<Item=(),Error=String>;
-
-	/// Do additional verification for this block.
-	fn verify(
-		&self,
-		header: &B::Header,
-		body: Option<&[B::Extrinsic]>,
-	) -> Self::Verified;
-}
-
 /// A list of all well known keys in the cache.
 pub mod well_known_cache_keys {
 	/// The type representing cache keys.

substrate/node-template/src/service.rs

@@ -13,7 +13,7 @@ use substrate_service::{
 	error::{Error as ServiceError},
 };
 use basic_authorship::ProposerFactory;
-use consensus::{import_queue, start_aura, AuraImportQueue, SlotDuration, NothingExtra};
+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};
@@ -99,14 +99,13 @@ construct_service_factory! {
 			Self::Block,
 		>
 			{ |config: &mut FactoryFullConfiguration<Self> , client: Arc<FullClient<Self>>, _select_chain: Self::SelectChain| {
-					import_queue::<_, _, _, Pair>(
+					import_queue::<_, _, Pair>(
 						SlotDuration::get_or_compute(&*client)?,
 						client.clone(),
 						None,
 						None,
 						None,
 						client,
-						NothingExtra,
 						config.custom.inherent_data_providers.clone(),
 					).map_err(Into::into)
 				}
@@ -115,14 +114,13 @@ construct_service_factory! {
 			Self::Block,
 		>
 			{ |config: &mut FactoryFullConfiguration<Self>, client: Arc<LightClient<Self>>| {
-					import_queue::<_, _, _, Pair>(
+					import_queue::<_, _, Pair>(
 						SlotDuration::get_or_compute(&*client)?,
 						client.clone(),
 						None,
 						None,
 						None,
 						client,
-						NothingExtra,
 						config.custom.inherent_data_providers.clone(),
 					).map_err(Into::into)
 				}

substrate/node/cli/src/service.rs

@@ -22,7 +22,7 @@ use std::sync::Arc;
 use std::time::Duration;
 
 use client::{self, LongestChain};
-use consensus::{import_queue, start_aura, AuraImportQueue, SlotDuration, NothingExtra};
+use consensus::{import_queue, start_aura, AuraImportQueue, SlotDuration};
 use grandpa::{self, FinalityProofProvider as GrandpaFinalityProofProvider};
 use node_executor;
 use primitives::{Pair as PairT, ed25519};
@@ -167,14 +167,13 @@ construct_service_factory! {
 
 				config.custom.grandpa_import_setup = Some((block_import.clone(), link_half));
 
-				import_queue::<_, _, _, ed25519::Pair>(
+				import_queue::<_, _, ed25519::Pair>(
 					slot_duration,
 					block_import,
 					Some(justification_import),
 					None,
 					None,
 					client,
-					NothingExtra,
 					config.custom.inherent_data_providers.clone(),
 				).map_err(Into::into)
 			}},
@@ -192,14 +191,13 @@ construct_service_factory! {
 				let finality_proof_import = block_import.clone();
 				let finality_proof_request_builder = finality_proof_import.create_finality_proof_request_builder();
 
-				import_queue::<_, _, _, ed25519::Pair>(
+				import_queue::<_, _, ed25519::Pair>(
 					SlotDuration::get_or_compute(&*client)?,
 					block_import,
 					None,
 					Some(finality_proof_import),
 					Some(finality_proof_request_builder),
 					client,
-					NothingExtra,
 					config.custom.inherent_data_providers.clone(),
 				).map_err(Into::into)
 			}},