Aura: Adds some compatibility mode to support old chains (#12492)

* Aura: Adds some compatibility mode to support old chains

In https://github.com/paritytech/substrate/pull/9132 we changed the way how we get the authorities
from the runtime. Before this mentioned pr we would call `initialize_block` before calling the
authorities runtime function. The problem with this was that when you have a block X that would
switch the authority set, it would already be signed by an authority of the new set. This was wrong,
as a block should only be signed by the current authority set. As this change is a hard fork, this
pr brings back the possibility for users that have a chain running with this old logic to upgrade.

They will need to use:
```
CompatibilityMode::UseInitializeBlock { until: some_block_in_the_future }
```

Using this compatibility mode will make the node behave like the old nodes, aka calling
`initialize_block` before doing the actual runtime call to `authorities`. Then when the given
`until` block is being build/imported the node switches to the new behaviour of not calling
`initialize_block` before. This is a hard fork, so the `until` block should be chosen wisely as a
point where all nodes in the network have upgraded.

* Fixes

* Make docs ready

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

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

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

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

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

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

* FMT

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

substrate/bin/node-template/node/src/service.rs

@@ -132,6 +132,7 @@ pub fn new_partial(
 			registry: config.prometheus_registry(),
 			check_for_equivocation: Default::default(),
 			telemetry: telemetry.as_ref().map(|x| x.handle()),
+			compatibility_mode: Default::default(),
 		})?;
 
 	Ok(sc_service::PartialComponents {
@@ -280,6 +281,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError>
 				block_proposal_slot_portion: SlotProportion::new(2f32 / 3f32),
 				max_block_proposal_slot_portion: None,
 				telemetry: telemetry.as_ref().map(|x| x.handle()),
+				compatibility_mode: Default::default(),
 			},
 		)?;
 

substrate/client/consensus/aura/src/import_queue.rs

@@ -18,7 +18,9 @@
 
 //! Module implementing the logic for verifying and importing AuRa blocks.
 
-use crate::{aura_err, authorities, find_pre_digest, slot_author, AuthorityId, Error};
+use crate::{
+	aura_err, authorities, find_pre_digest, slot_author, AuthorityId, CompatibilityMode, Error,
+};
 use codec::{Codec, Decode, Encode};
 use log::{debug, info, trace};
 use prometheus_endpoint::Registry;
@@ -31,21 +33,15 @@ use sc_consensus_slots::{check_equivocation, CheckedHeader, InherentDataProvider
 use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_TRACE};
 use sp_api::{ApiExt, ProvideRuntimeApi};
 use sp_block_builder::BlockBuilder as BlockBuilderApi;
-use sp_blockchain::{
-	well_known_cache_keys::{self, Id as CacheKeyId},
-	HeaderBackend,
-};
+use sp_blockchain::{well_known_cache_keys::Id as CacheKeyId, HeaderBackend};
 use sp_consensus::Error as ConsensusError;
-use sp_consensus_aura::{
-	digests::CompatibleDigestItem, inherents::AuraInherentData, AuraApi, ConsensusLog,
-	AURA_ENGINE_ID,
-};
+use sp_consensus_aura::{digests::CompatibleDigestItem, inherents::AuraInherentData, AuraApi};
 use sp_consensus_slots::Slot;
 use sp_core::{crypto::Pair, ExecutionContext};
 use sp_inherents::{CreateInherentDataProviders, InherentDataProvider as _};
 use sp_runtime::{
-	generic::{BlockId, OpaqueDigestItemId},
-	traits::{Block as BlockT, Header},
+	generic::BlockId,
+	traits::{Block as BlockT, Header, NumberFor},
 	DigestItem,
 };
 use std::{fmt::Debug, hash::Hash, marker::PhantomData, sync::Arc};
@@ -109,32 +105,35 @@ where
 }
 
 /// A verifier for Aura blocks.
-pub struct AuraVerifier<C, P, CIDP> {
+pub struct AuraVerifier<C, P, CIDP, N> {
 	client: Arc<C>,
 	phantom: PhantomData<P>,
 	create_inherent_data_providers: CIDP,
 	check_for_equivocation: CheckForEquivocation,
 	telemetry: Option<TelemetryHandle>,
+	compatibility_mode: CompatibilityMode<N>,
 }
 
-impl<C, P, CIDP> AuraVerifier<C, P, CIDP> {
+impl<C, P, CIDP, N> AuraVerifier<C, P, CIDP, N> {
 	pub(crate) fn new(
 		client: Arc<C>,
 		create_inherent_data_providers: CIDP,
 		check_for_equivocation: CheckForEquivocation,
 		telemetry: Option<TelemetryHandle>,
+		compatibility_mode: CompatibilityMode<N>,
 	) -> Self {
 		Self {
 			client,
 			create_inherent_data_providers,
 			check_for_equivocation,
 			telemetry,
+			compatibility_mode,
 			phantom: PhantomData,
 		}
 	}
 }
 
-impl<C, P, CIDP> AuraVerifier<C, P, CIDP>
+impl<C, P, CIDP, N> AuraVerifier<C, P, CIDP, N>
 where
 	P: Send + Sync + 'static,
 	CIDP: Send,
@@ -172,9 +171,9 @@ where
 }
 
 #[async_trait::async_trait]
-impl<B: BlockT, C, P, CIDP> Verifier<B> for AuraVerifier<C, P, CIDP>
+impl<B: BlockT, C, P, CIDP> Verifier<B> for AuraVerifier<C, P, CIDP, NumberFor<B>>
 where
-	C: ProvideRuntimeApi<B> + Send + Sync + sc_client_api::backend::AuxStore + BlockOf,
+	C: ProvideRuntimeApi<B> + Send + Sync + sc_client_api::backend::AuxStore,
 	C::Api: BlockBuilderApi<B> + AuraApi<B, AuthorityId<P>> + ApiExt<B>,
 	P: Pair + Send + Sync + 'static,
 	P::Public: Send + Sync + Hash + Eq + Clone + Decode + Encode + Debug + 'static,
@@ -188,8 +187,13 @@ where
 	) -> Result<(BlockImportParams<B, ()>, Option<Vec<(CacheKeyId, Vec<u8>)>>), String> {
 		let hash = block.header.hash();
 		let parent_hash = *block.header.parent_hash();
-		let authorities = authorities(self.client.as_ref(), &BlockId::Hash(parent_hash))
-			.map_err(|e| format!("Could not fetch authorities at {:?}: {}", parent_hash, e))?;
+		let authorities = authorities(
+			self.client.as_ref(),
+			parent_hash,
+			*block.header.number(),
+			&self.compatibility_mode,
+		)
+		.map_err(|e| format!("Could not fetch authorities at {:?}: {}", parent_hash, e))?;
 
 		let create_inherent_data_providers = self
 			.create_inherent_data_providers
@@ -259,28 +263,12 @@ where
 					"pre_header" => ?pre_header,
 				);
 
-				// Look for an authorities-change log.
-				let maybe_keys = pre_header
-					.digest()
-					.logs()
-					.iter()
-					.filter_map(|l| {
-						l.try_to::<ConsensusLog<AuthorityId<P>>>(OpaqueDigestItemId::Consensus(
-							&AURA_ENGINE_ID,
-						))
-					})
-					.find_map(|l| match l {
-						ConsensusLog::AuthoritiesChange(a) =>
-							Some(vec![(well_known_cache_keys::AUTHORITIES, a.encode())]),
-						_ => None,
-					});
-
 				block.header = pre_header;
 				block.post_digests.push(seal);
 				block.fork_choice = Some(ForkChoiceStrategy::LongestChain);
 				block.post_hash = Some(hash);
 
-				Ok((block, maybe_keys))
+				Ok((block, None))
 			},
 			CheckedHeader::Deferred(a, b) => {
 				debug!(target: "aura", "Checking {:?} failed; {:?}, {:?}.", hash, a, b);
@@ -323,7 +311,7 @@ impl Default for CheckForEquivocation {
 }
 
 /// Parameters of [`import_queue`].
-pub struct ImportQueueParams<'a, Block, I, C, S, CIDP> {
+pub struct ImportQueueParams<'a, Block: BlockT, I, C, S, CIDP> {
 	/// The block import to use.
 	pub block_import: I,
 	/// The justification import.
@@ -340,6 +328,10 @@ pub struct ImportQueueParams<'a, Block, I, C, S, CIDP> {
 	pub check_for_equivocation: CheckForEquivocation,
 	/// Telemetry instance used to report telemetry metrics.
 	pub telemetry: Option<TelemetryHandle>,
+	/// Compatibility mode that should be used.
+	///
+	/// If in doubt, use `Default::default()`.
+	pub compatibility_mode: CompatibilityMode<NumberFor<Block>>,
 }
 
 /// Start an import queue for the Aura consensus algorithm.
@@ -353,6 +345,7 @@ pub fn import_queue<P, Block, I, C, S, CIDP>(
 		registry,
 		check_for_equivocation,
 		telemetry,
+		compatibility_mode,
 	}: ImportQueueParams<Block, I, C, S, CIDP>,
 ) -> Result<DefaultImportQueue<Block, C>, sp_consensus::Error>
 where
@@ -377,18 +370,19 @@ where
 	CIDP: CreateInherentDataProviders<Block, ()> + Sync + Send + 'static,
 	CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync,
 {
-	let verifier = build_verifier::<P, _, _>(BuildVerifierParams {
+	let verifier = build_verifier::<P, _, _, _>(BuildVerifierParams {
 		client,
 		create_inherent_data_providers,
 		check_for_equivocation,
 		telemetry,
+		compatibility_mode,
 	});
 
 	Ok(BasicQueue::new(verifier, Box::new(block_import), justification_import, spawner, registry))
 }
 
 /// Parameters of [`build_verifier`].
-pub struct BuildVerifierParams<C, CIDP> {
+pub struct BuildVerifierParams<C, CIDP, N> {
 	/// The client to interact with the chain.
 	pub client: Arc<C>,
 	/// Something that can create the inherent data providers.
@@ -397,21 +391,27 @@ pub struct BuildVerifierParams<C, CIDP> {
 	pub check_for_equivocation: CheckForEquivocation,
 	/// Telemetry instance used to report telemetry metrics.
 	pub telemetry: Option<TelemetryHandle>,
+	/// Compatibility mode that should be used.
+	///
+	/// If in doubt, use `Default::default()`.
+	pub compatibility_mode: CompatibilityMode<N>,
 }
 
 /// Build the [`AuraVerifier`]
-pub fn build_verifier<P, C, CIDP>(
+pub fn build_verifier<P, C, CIDP, N>(
 	BuildVerifierParams {
 		client,
 		create_inherent_data_providers,
 		check_for_equivocation,
 		telemetry,
-	}: BuildVerifierParams<C, CIDP>,
-) -> AuraVerifier<C, P, CIDP> {
-	AuraVerifier::<_, P, _>::new(
+		compatibility_mode,
+	}: BuildVerifierParams<C, CIDP, N>,
+) -> AuraVerifier<C, P, CIDP, N> {
+	AuraVerifier::<_, P, _, _>::new(
 		client,
 		create_inherent_data_providers,
 		check_for_equivocation,
 		telemetry,
+		compatibility_mode,
 	)
 }

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

@@ -44,7 +44,7 @@ use sc_consensus_slots::{
 	SlotInfo, StorageChanges,
 };
 use sc_telemetry::TelemetryHandle;
-use sp_api::ProvideRuntimeApi;
+use sp_api::{Core, ProvideRuntimeApi};
 use sp_application_crypto::{AppKey, AppPublic};
 use sp_blockchain::{HeaderBackend, Result as CResult};
 use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain};
@@ -74,6 +74,43 @@ pub use sp_consensus_aura::{
 
 type AuthorityId<P> = <P as Pair>::Public;
 
+/// Run `AURA` in a compatibility mode.
+///
+/// This is required for when the chain was launched and later there
+/// was a consensus breaking change.
+#[derive(Debug, Clone)]
+pub enum CompatibilityMode<N> {
+	/// Don't use any compatibility mode.
+	None,
+	/// Call `initialize_block` before doing any runtime calls.
+	///
+	/// Previously the node would execute `initialize_block` before fetchting the authorities
+	/// from the runtime. This behaviour changed in: <https://github.com/paritytech/substrate/pull/9132>
+	///
+	/// By calling `initialize_block` before fetching the authorities, on a block that
+	/// would enact a new validator set, the block would already be build/sealed by an
+	/// authority of the new set. With this mode disabled (the default) a block that enacts a new
+	/// set isn't sealed/built by an authority of the new set, however to make new nodes be able to
+	/// sync old chains this compatibility mode exists.
+	UseInitializeBlock {
+		/// The block number until this compatibility mode should be executed. The first runtime
+		/// call in the context of the `until` block (importing it/building it) will disable the
+		/// compatibility mode (i.e. at `until` the default rules will apply). When enabling this
+		/// compatibility mode the `until` block should be a future block on which all nodes will
+		/// have upgraded to a release that includes the updated compatibility mode configuration.
+		/// At `until` block there will be a hard fork when the authority set changes, between the
+		/// old nodes (running with `initialize_block`, i.e. without the compatibility mode
+		/// configuration) and the new nodes.
+		until: N,
+	},
+}
+
+impl<N> Default for CompatibilityMode<N> {
+	fn default() -> Self {
+		Self::None
+	}
+}
+
 /// Get the slot duration for Aura.
 pub fn slot_duration<A, B, C>(client: &C) -> CResult<SlotDuration>
 where
@@ -106,7 +143,7 @@ fn slot_author<P: Pair>(slot: Slot, authorities: &[AuthorityId<P>]) -> Option<&A
 }
 
 /// Parameters of [`start_aura`].
-pub struct StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS> {
+pub struct StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS, N> {
 	/// The duration of a slot.
 	pub slot_duration: SlotDuration,
 	/// The client to interact with the chain.
@@ -140,6 +177,10 @@ pub struct StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS> {
 	pub max_block_proposal_slot_portion: Option<SlotProportion>,
 	/// Telemetry instance used to report telemetry metrics.
 	pub telemetry: Option<TelemetryHandle>,
+	/// Compatibility mode that should be used.
+	///
+	/// If in doubt, use `Default::default()`.
+	pub compatibility_mode: CompatibilityMode<N>,
 }
 
 /// Start the aura worker. The returned future should be run in a futures executor.
@@ -159,7 +200,8 @@ pub fn start_aura<P, B, C, SC, I, PF, SO, L, CIDP, BS, Error>(
 		block_proposal_slot_portion,
 		max_block_proposal_slot_portion,
 		telemetry,
-	}: StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS>,
+		compatibility_mode,
+	}: StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS, NumberFor<B>>,
 ) -> Result<impl Future<Output = ()>, sp_consensus::Error>
 where
 	P: Pair + Send + Sync,
@@ -191,6 +233,7 @@ where
 		telemetry,
 		block_proposal_slot_portion,
 		max_block_proposal_slot_portion,
+		compatibility_mode,
 	});
 
 	Ok(sc_consensus_slots::start_slot_worker(
@@ -203,7 +246,7 @@ where
 }
 
 /// Parameters of [`build_aura_worker`].
-pub struct BuildAuraWorkerParams<C, I, PF, SO, L, BS> {
+pub struct BuildAuraWorkerParams<C, I, PF, SO, L, BS, N> {
 	/// The client to interact with the chain.
 	pub client: Arc<C>,
 	/// The block import.
@@ -231,6 +274,10 @@ pub struct BuildAuraWorkerParams<C, I, PF, SO, L, BS> {
 	pub max_block_proposal_slot_portion: Option<SlotProportion>,
 	/// Telemetry instance used to report telemetry metrics.
 	pub telemetry: Option<TelemetryHandle>,
+	/// Compatibility mode that should be used.
+	///
+	/// If in doubt, use `Default::default()`.
+	pub compatibility_mode: CompatibilityMode<N>,
 }
 
 /// Build the aura worker.
@@ -249,7 +296,8 @@ pub fn build_aura_worker<P, B, C, PF, I, SO, L, BS, Error>(
 		max_block_proposal_slot_portion,
 		telemetry,
 		force_authoring,
-	}: BuildAuraWorkerParams<C, I, PF, SO, L, BS>,
+		compatibility_mode,
+	}: BuildAuraWorkerParams<C, I, PF, SO, L, BS, NumberFor<B>>,
 ) -> impl sc_consensus_slots::SimpleSlotWorker<
 	B,
 	Proposer = PF::Proposer,
@@ -286,11 +334,12 @@ where
 		telemetry,
 		block_proposal_slot_portion,
 		max_block_proposal_slot_portion,
+		compatibility_mode,
 		_key_type: PhantomData::<P>,
 	}
 }
 
-struct AuraWorker<C, E, I, P, SO, L, BS> {
+struct AuraWorker<C, E, I, P, SO, L, BS, N> {
 	client: Arc<C>,
 	block_import: I,
 	env: E,
@@ -302,12 +351,13 @@ struct AuraWorker<C, E, I, P, SO, L, BS> {
 	block_proposal_slot_portion: SlotProportion,
 	max_block_proposal_slot_portion: Option<SlotProportion>,
 	telemetry: Option<TelemetryHandle>,
+	compatibility_mode: CompatibilityMode<N>,
 	_key_type: PhantomData<P>,
 }
 
 #[async_trait::async_trait]
 impl<B, C, E, I, P, Error, SO, L, BS> sc_consensus_slots::SimpleSlotWorker<B>
-	for AuraWorker<C, E, I, P, SO, L, BS>
+	for AuraWorker<C, E, I, P, SO, L, BS, NumberFor<B>>
 where
 	B: BlockT,
 	C: ProvideRuntimeApi<B> + BlockOf + HeaderBackend<B> + Sync,
@@ -345,7 +395,12 @@ where
 		header: &B::Header,
 		_slot: Slot,
 	) -> Result<Self::AuxData, sp_consensus::Error> {
-		authorities(self.client.as_ref(), &BlockId::Hash(header.hash()))
+		authorities(
+			self.client.as_ref(),
+			header.hash(),
+			*header.number() + 1u32.into(),
+			&self.compatibility_mode,
+		)
 	}
 
 	fn authorities_len(&self, epoch_data: &Self::AuxData) -> Option<usize> {
@@ -535,16 +590,42 @@ pub fn find_pre_digest<B: BlockT, Signature: Codec>(header: &B::Header) -> Resul
 	pre_digest.ok_or_else(|| aura_err(Error::NoDigestFound))
 }
 
-fn authorities<A, B, C>(client: &C, at: &BlockId<B>) -> Result<Vec<A>, ConsensusError>
+fn authorities<A, B, C>(
+	client: &C,
+	parent_hash: B::Hash,
+	context_block_number: NumberFor<B>,
+	compatibility_mode: &CompatibilityMode<NumberFor<B>>,
+) -> Result<Vec<A>, ConsensusError>
 where
 	A: Codec + Debug,
 	B: BlockT,
-	C: ProvideRuntimeApi<B> + BlockOf,
+	C: ProvideRuntimeApi<B>,
 	C::Api: AuraApi<B, A>,
 {
-	client
-		.runtime_api()
-		.authorities(at)
+	let runtime_api = client.runtime_api();
+
+	match compatibility_mode {
+		CompatibilityMode::None => {},
+		// Use `initialize_block` until we hit the block that should disable the mode.
+		CompatibilityMode::UseInitializeBlock { until } =>
+			if *until > context_block_number {
+				runtime_api
+					.initialize_block(
+						&BlockId::Hash(parent_hash),
+						&B::Header::new(
+							context_block_number,
+							Default::default(),
+							Default::default(),
+							parent_hash,
+							Default::default(),
+						),
+					)
+					.map_err(|_| sp_consensus::Error::InvalidAuthoritiesSet)?;
+			},
+	}
+
+	runtime_api
+		.authorities(&BlockId::Hash(parent_hash))
 		.ok()
 		.ok_or(sp_consensus::Error::InvalidAuthoritiesSet)
 }
@@ -631,6 +712,7 @@ mod tests {
 				InherentDataProviders = (InherentDataProvider,),
 			>,
 		>,
+		u64,
 	>;
 	type AuraPeer = Peer<(), PeersClient>;
 
@@ -660,6 +742,7 @@ mod tests {
 				}),
 				CheckForEquivocation::Yes,
 				None,
+				CompatibilityMode::None,
 			)
 		}
 
@@ -749,6 +832,7 @@ mod tests {
 					block_proposal_slot_portion: SlotProportion::new(0.5),
 					max_block_proposal_slot_portion: None,
 					telemetry: None,
+					compatibility_mode: CompatibilityMode::None,
 				})
 				.expect("Starts aura"),
 			);
@@ -769,7 +853,8 @@ mod tests {
 
 		assert_eq!(client.chain_info().best_number, 0);
 		assert_eq!(
-			authorities(&client, &BlockId::Hash(client.chain_info().best_hash)).unwrap(),
+			authorities(&client, client.chain_info().best_hash, 1, &CompatibilityMode::None)
+				.unwrap(),
 			vec![
 				Keyring::Alice.public().into(),
 				Keyring::Bob.public().into(),
@@ -814,6 +899,7 @@ mod tests {
 			_key_type: PhantomData::<AuthorityPair>,
 			block_proposal_slot_portion: SlotProportion::new(0.5),
 			max_block_proposal_slot_portion: None,
+			compatibility_mode: Default::default(),
 		};
 
 		let head = Header::new(
@@ -866,6 +952,7 @@ mod tests {
 			_key_type: PhantomData::<AuthorityPair>,
 			block_proposal_slot_portion: SlotProportion::new(0.5),
 			max_block_proposal_slot_portion: None,
+			compatibility_mode: Default::default(),
 		};
 
 		let head = client.header(&BlockId::Number(0)).unwrap().unwrap();

substrate/client/service/src/client/client.rs

@@ -641,7 +641,7 @@ where
 						if state_root != *import_headers.post().state_root() {
 							// State root mismatch when importing state. This should not happen in
 							// safe fast sync mode, but may happen in unsafe mode.
-							warn!("Error imporing state: State root mismatch.");
+							warn!("Error importing state: State root mismatch.");
 							return Err(Error::InvalidStateRoot)
 						}
 						None

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

@@ -89,7 +89,7 @@ sp_api::decl_runtime_apis! {
 		/// Currently, only the value provided by this type at genesis will be used.
 		fn slot_duration() -> SlotDuration;
 
-		// Return the current set of authorities.
+		/// Return the current set of authorities.
 		fn authorities() -> Vec<AuthorityId>;
 	}
 }