Refactor: extract most aura logic out to standalone module, make use of these (#13764)

* Extract most aura logic out to standalone module, make use of these

* Update client/consensus/aura/src/standalone.rs

improve docs

Co-authored-by: Bastian Köcher <[email protected]>

* add slot_duration_at

* ".git/.scripts/commands/fmt/fmt.sh"

---------

Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: parity-processbot <>

Author: rphmeier

client/consensus/aura/src/import_queue.rs

@@ -19,7 +19,7 @@
 //! Module implementing the logic for verifying and importing AuRa blocks.
 
 use crate::{
-	aura_err, authorities, find_pre_digest, slot_author, AuthorityId, CompatibilityMode, Error,
+	authorities, standalone::SealVerificationError, AuthorityId, CompatibilityMode, Error,
 	LOG_TARGET,
 };
 use codec::{Codec, Decode, Encode};
@@ -36,7 +36,7 @@ use sp_api::{ApiExt, ProvideRuntimeApi};
 use sp_block_builder::BlockBuilder as BlockBuilderApi;
 use sp_blockchain::HeaderBackend;
 use sp_consensus::Error as ConsensusError;
-use sp_consensus_aura::{digests::CompatibleDigestItem, inherents::AuraInherentData, AuraApi};
+use sp_consensus_aura::{inherents::AuraInherentData, AuraApi};
 use sp_consensus_slots::Slot;
 use sp_core::{crypto::Pair, ExecutionContext};
 use sp_inherents::{CreateInherentDataProviders, InherentDataProvider as _};
@@ -54,7 +54,7 @@ use std::{fmt::Debug, hash::Hash, marker::PhantomData, sync::Arc};
 fn check_header<C, B: BlockT, P: Pair>(
 	client: &C,
 	slot_now: Slot,
-	mut header: B::Header,
+	header: B::Header,
 	hash: B::Hash,
 	authorities: &[AuthorityId<P>],
 	check_for_equivocation: CheckForEquivocation,
@@ -64,27 +64,16 @@ where
 	C: sc_client_api::backend::AuxStore,
 	P::Public: Encode + Decode + PartialEq + Clone,
 {
-	let seal = header.digest_mut().pop().ok_or(Error::HeaderUnsealed(hash))?;
-
-	let sig = seal.as_aura_seal().ok_or_else(|| aura_err(Error::HeaderBadSeal(hash)))?;
-
-	let slot = find_pre_digest::<B, P::Signature>(&header)?;
-
-	if slot > slot_now {
-		header.digest_mut().push(seal);
-		Ok(CheckedHeader::Deferred(header, slot))
-	} else {
-		// check the signature is valid under the expected authority and
-		// chain state.
-		let expected_author =
-			slot_author::<P>(slot, authorities).ok_or(Error::SlotAuthorNotFound)?;
-
-		let pre_hash = header.hash();
-
-		if P::verify(&sig, pre_hash.as_ref(), expected_author) {
-			if check_for_equivocation.check_for_equivocation() {
+	let check_result =
+		crate::standalone::check_header_slot_and_seal::<B, P>(slot_now, header, authorities);
+
+	match check_result {
+		Ok((header, slot, seal)) => {
+			let expected_author = crate::standalone::slot_author::<P>(slot, &authorities);
+			let should_equiv_check = check_for_equivocation.check_for_equivocation();
+			if let (true, Some(expected)) = (should_equiv_check, expected_author) {
 				if let Some(equivocation_proof) =
-					check_equivocation(client, slot_now, slot, &header, expected_author)
+					check_equivocation(client, slot_now, slot, &header, expected)
 						.map_err(Error::Client)?
 				{
 					info!(
@@ -98,9 +87,14 @@ where
 			}
 
 			Ok(CheckedHeader::Checked(header, (slot, seal)))
-		} else {
-			Err(Error::BadSignature(hash))
-		}
+		},
+		Err(SealVerificationError::Deferred(header, slot)) =>
+			Ok(CheckedHeader::Deferred(header, slot)),
+		Err(SealVerificationError::Unsealed) => Err(Error::HeaderUnsealed(hash)),
+		Err(SealVerificationError::BadSeal) => Err(Error::HeaderBadSeal(hash)),
+		Err(SealVerificationError::BadSignature) => Err(Error::BadSignature(hash)),
+		Err(SealVerificationError::SlotAuthorNotFound) => Err(Error::SlotAuthorNotFound),
+		Err(SealVerificationError::InvalidPreDigest(e)) => Err(Error::from(e)),
 	}
 }
 

client/consensus/aura/src/lib.rs

@@ -33,32 +33,30 @@
 use std::{fmt::Debug, hash::Hash, marker::PhantomData, pin::Pin, sync::Arc};
 
 use futures::prelude::*;
-use log::{debug, trace};
 
 use codec::{Codec, Decode, Encode};
 
-use sc_client_api::{backend::AuxStore, BlockOf, UsageProvider};
+use sc_client_api::{backend::AuxStore, BlockOf};
 use sc_consensus::{BlockImport, BlockImportParams, ForkChoiceStrategy, StateAction};
 use sc_consensus_slots::{
 	BackoffAuthoringBlocksStrategy, InherentDataProviderExt, SimpleSlotWorkerToSlotWorker,
 	SlotInfo, StorageChanges,
 };
 use sc_telemetry::TelemetryHandle;
 use sp_api::{Core, ProvideRuntimeApi};
-use sp_application_crypto::{AppCrypto, AppPublic};
-use sp_blockchain::{HeaderBackend, Result as CResult};
+use sp_application_crypto::AppPublic;
+use sp_blockchain::HeaderBackend;
 use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain};
 use sp_consensus_slots::Slot;
-use sp_core::crypto::{ByteArray, Pair, Public};
+use sp_core::crypto::{Pair, Public};
 use sp_inherents::CreateInherentDataProviders;
 use sp_keystore::KeystorePtr;
-use sp_runtime::{
-	traits::{Block as BlockT, Header, Member, NumberFor, Zero},
-	DigestItem,
-};
+use sp_runtime::traits::{Block as BlockT, Header, Member, NumberFor};
 
 mod import_queue;
+pub mod standalone;
 
+pub use crate::standalone::{find_pre_digest, slot_duration};
 pub use import_queue::{
 	build_verifier, import_queue, AuraVerifier, BuildVerifierParams, CheckForEquivocation,
 	ImportQueueParams,
@@ -112,39 +110,6 @@ impl<N> Default for CompatibilityMode<N> {
 	}
 }
 
-/// Get the slot duration for Aura.
-pub fn slot_duration<A, B, C>(client: &C) -> CResult<SlotDuration>
-where
-	A: Codec,
-	B: BlockT,
-	C: AuxStore + ProvideRuntimeApi<B> + UsageProvider<B>,
-	C::Api: AuraApi<B, A>,
-{
-	client
-		.runtime_api()
-		.slot_duration(client.usage_info().chain.best_hash)
-		.map_err(|err| err.into())
-}
-
-/// Get slot author for given block along with authorities.
-fn slot_author<P: Pair>(slot: Slot, authorities: &[AuthorityId<P>]) -> Option<&AuthorityId<P>> {
-	if authorities.is_empty() {
-		return None
-	}
-
-	let idx = *slot % (authorities.len() as u64);
-	assert!(
-		idx <= usize::MAX as u64,
-		"It is impossible to have a vector with length beyond the address space; qed",
-	);
-
-	let current_author = authorities.get(idx as usize).expect(
-		"authorities not empty; index constrained to list length;this is a valid index; qed",
-	);
-
-	Some(current_author)
-}
-
 /// Parameters of [`start_aura`].
 pub struct StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS, N> {
 	/// The duration of a slot.
@@ -412,21 +377,11 @@ where
 		slot: Slot,
 		authorities: &Self::AuxData,
 	) -> Option<Self::Claim> {
-		let expected_author = slot_author::<P>(slot, authorities);
-		expected_author.and_then(|p| {
-			if self
-				.keystore
-				.has_keys(&[(p.to_raw_vec(), sp_application_crypto::key_types::AURA)])
-			{
-				Some(p.clone())
-			} else {
-				None
-			}
-		})
+		crate::standalone::claim_slot::<P>(slot, authorities, &self.keystore).await
 	}
 
 	fn pre_digest_data(&self, slot: Slot, _claim: &Self::Claim) -> Vec<sp_runtime::DigestItem> {
-		vec![<DigestItem as CompatibleDigestItem<P::Signature>>::aura_pre_digest(slot)]
+		vec![crate::standalone::pre_digest::<P>(slot)]
 	}
 
 	async fn block_import_params(
@@ -441,28 +396,8 @@ where
 		sc_consensus::BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
 		ConsensusError,
 	> {
-		let signature = self
-			.keystore
-			.sign_with(
-				<AuthorityId<P> as AppCrypto>::ID,
-				<AuthorityId<P> as AppCrypto>::CRYPTO_ID,
-				public.as_slice(),
-				header_hash.as_ref(),
-			)
-			.map_err(|e| ConsensusError::CannotSign(format!("{}. Key: {:?}", e, public)))?
-			.ok_or_else(|| {
-				ConsensusError::CannotSign(format!(
-					"Could not find key in keystore. Key: {:?}",
-					public
-				))
-			})?;
-		let signature = signature
-			.clone()
-			.try_into()
-			.map_err(|_| ConsensusError::InvalidSignature(signature, public.to_raw_vec()))?;
-
 		let signature_digest_item =
-			<DigestItem as CompatibleDigestItem<P::Signature>>::aura_seal(signature);
+			crate::standalone::seal::<_, P>(header_hash, &public, &self.keystore)?;
 
 		let mut import_block = BlockImportParams::new(BlockOrigin::Own, header);
 		import_block.post_digests.push(signature_digest_item);
@@ -526,11 +461,6 @@ where
 	}
 }
 
-fn aura_err<B: BlockT>(error: Error<B>) -> Error<B> {
-	debug!(target: LOG_TARGET, "{}", error);
-	error
-}
-
 /// Aura Errors
 #[derive(Debug, thiserror::Error)]
 pub enum Error<B: BlockT> {
@@ -569,22 +499,13 @@ impl<B: BlockT> From<Error<B>> for String {
 	}
 }
 
-/// Get pre-digests from the header
-pub fn find_pre_digest<B: BlockT, Signature: Codec>(header: &B::Header) -> Result<Slot, Error<B>> {
-	if header.number().is_zero() {
-		return Ok(0.into())
-	}
-
-	let mut pre_digest: Option<Slot> = None;
-	for log in header.digest().logs() {
-		trace!(target: LOG_TARGET, "Checking log {:?}", log);
-		match (CompatibleDigestItem::<Signature>::as_aura_pre_digest(log), pre_digest.is_some()) {
-			(Some(_), true) => return Err(aura_err(Error::MultipleHeaders)),
-			(None, _) => trace!(target: LOG_TARGET, "Ignoring digest not meant for us"),
-			(s, false) => pre_digest = s,
+impl<B: BlockT> From<crate::standalone::PreDigestLookupError> for Error<B> {
+	fn from(e: crate::standalone::PreDigestLookupError) -> Self {
+		match e {
+			crate::standalone::PreDigestLookupError::MultipleHeaders => Error::MultipleHeaders,
+			crate::standalone::PreDigestLookupError::NoDigestFound => Error::NoDigestFound,
 		}
 	}
-	pre_digest.ok_or_else(|| aura_err(Error::NoDigestFound))
 }
 
 fn authorities<A, B, C>(
@@ -637,7 +558,7 @@ mod tests {
 	use sc_consensus_slots::{BackoffAuthoringOnFinalizedHeadLagging, SimpleSlotWorker};
 	use sc_keystore::LocalKeystore;
 	use sc_network_test::{Block as TestBlock, *};
-	use sp_application_crypto::key_types::AURA;
+	use sp_application_crypto::{key_types::AURA, AppCrypto};
 	use sp_consensus::{DisableProofRecording, NoNetwork as DummyOracle, Proposal};
 	use sp_consensus_aura::sr25519::AuthorityPair;
 	use sp_inherents::InherentData;
@@ -851,22 +772,6 @@ mod tests {
 		.await;
 	}
 
-	#[test]
-	fn authorities_call_works() {
-		let client = substrate_test_runtime_client::new();
-
-		assert_eq!(client.chain_info().best_number, 0);
-		assert_eq!(
-			authorities(&client, client.chain_info().best_hash, 1, &CompatibilityMode::None)
-				.unwrap(),
-			vec![
-				Keyring::Alice.public().into(),
-				Keyring::Bob.public().into(),
-				Keyring::Charlie.public().into()
-			]
-		);
-	}
-
 	#[tokio::test]
 	async fn current_node_authority_should_claim_slot() {
 		let net = AuraTestNet::new(4);