Fetch Babe configuration from runtime state (#11760)

* Fetch babe config data from runtime state

* Some renaming

* More renaming

* Final nits

* Fix tests and benches

* Rename  to  in BabeConfiguration

* Remove duplicate babe parameter description

Already specified over the 'PRIMARY_PROBABILITY' constant value

* trigger pipeline

* trigger pipeline

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

@@ -119,7 +119,7 @@ use sp_consensus::{
 	SelectChain,
 };
 use sp_consensus_babe::inherents::BabeInherentData;
-use sp_consensus_slots::{Slot, SlotDuration};
+use sp_consensus_slots::Slot;
 use sp_core::{crypto::ByteArray, ExecutionContext};
 use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider};
 use sp_keystore::{SyncCryptoStore, SyncCryptoStorePtr};
@@ -137,8 +137,7 @@ pub use sp_consensus_babe::{
 		PrimaryPreDigest, SecondaryPlainPreDigest,
 	},
 	AuthorityId, AuthorityPair, AuthoritySignature, BabeApi, BabeAuthorityWeight, BabeBlockWeight,
-	BabeEpochConfiguration, BabeGenesisConfiguration, ConsensusLog, BABE_ENGINE_ID,
-	VRF_OUTPUT_LENGTH,
+	BabeConfiguration, BabeEpochConfiguration, ConsensusLog, BABE_ENGINE_ID, VRF_OUTPUT_LENGTH,
 };
 
 pub use aux_schema::load_block_weight as block_weight;
@@ -211,12 +210,12 @@ impl From<sp_consensus_babe::Epoch> for Epoch {
 impl Epoch {
 	/// Create the genesis epoch (epoch #0). This is defined to start at the slot of
 	/// the first block, so that has to be provided.
-	pub fn genesis(genesis_config: &BabeGenesisConfiguration, slot: Slot) -> Epoch {
+	pub fn genesis(genesis_config: &BabeConfiguration, slot: Slot) -> Epoch {
 		Epoch {
 			epoch_index: 0,
 			start_slot: slot,
 			duration: genesis_config.epoch_length,
-			authorities: genesis_config.genesis_authorities.clone(),
+			authorities: genesis_config.authorities.clone(),
 			randomness: genesis_config.randomness,
 			config: BabeEpochConfiguration {
 				c: genesis_config.c,
@@ -338,56 +337,36 @@ pub struct BabeIntermediate<B: BlockT> {
 /// Intermediate key for Babe engine.
 pub static INTERMEDIATE_KEY: &[u8] = b"babe1";
 
-/// Configuration for BABE used for defining block verification parameters as
-/// well as authoring (e.g. the slot duration).
-#[derive(Clone)]
-pub struct Config {
-	genesis_config: BabeGenesisConfiguration,
-}
+/// Read configuration from the runtime state at current best block.
+pub fn configuration<B: BlockT, C>(client: &C) -> ClientResult<BabeConfiguration>
+where
+	C: AuxStore + ProvideRuntimeApi<B> + UsageProvider<B>,
+	C::Api: BabeApi<B>,
+{
+	let block_id = if client.usage_info().chain.finalized_state.is_some() {
+		BlockId::Hash(client.usage_info().chain.best_hash)
+	} else {
+		debug!(target: "babe", "No finalized state is available. Reading config from genesis");
+		BlockId::Hash(client.usage_info().chain.genesis_hash)
+	};
 
-impl Config {
-	/// Create a new config by reading the genesis configuration from the runtime.
-	pub fn get<B: BlockT, C>(client: &C) -> ClientResult<Self>
-	where
-		C: AuxStore + ProvideRuntimeApi<B> + UsageProvider<B>,
-		C::Api: BabeApi<B>,
-	{
-		trace!(target: "babe", "Getting slot duration");
+	let runtime_api = client.runtime_api();
+	let version = runtime_api.api_version::<dyn BabeApi<B>>(&block_id)?;
 
-		let mut best_block_id = BlockId::Hash(client.usage_info().chain.best_hash);
-		if client.usage_info().chain.finalized_state.is_none() {
-			debug!(target: "babe", "No finalized state is available. Reading config from genesis");
-			best_block_id = BlockId::Hash(client.usage_info().chain.genesis_hash);
-		}
-		let runtime_api = client.runtime_api();
-
-		let version = runtime_api.api_version::<dyn BabeApi<B>>(&best_block_id)?;
-
-		let genesis_config = if version == Some(1) {
+	let config = match version {
+		Some(1) => {
 			#[allow(deprecated)]
 			{
-				runtime_api.configuration_before_version_2(&best_block_id)?.into()
+				runtime_api.configuration_before_version_2(&block_id)?.into()
 			}
-		} else if version == Some(2) {
-			runtime_api.configuration(&best_block_id)?
-		} else {
+		},
+		Some(2) => runtime_api.configuration(&block_id)?,
+		_ =>
 			return Err(sp_blockchain::Error::VersionInvalid(
 				"Unsupported or invalid BabeApi version".to_string(),
-			))
-		};
-
-		Ok(Config { genesis_config })
-	}
-
-	/// Get the genesis configuration.
-	pub fn genesis_config(&self) -> &BabeGenesisConfiguration {
-		&self.genesis_config
-	}
-
-	/// Get the slot duration defined in the genesis configuration.
-	pub fn slot_duration(&self) -> SlotDuration {
-		SlotDuration::from_millis(self.genesis_config.slot_duration)
-	}
+			)),
+	};
+	Ok(config)
 }
 
 /// Parameters for BABE.
@@ -611,7 +590,7 @@ fn aux_storage_cleanup<C: HeaderMetadata<Block> + HeaderBackend<Block>, Block: B
 
 async fn answer_requests<B: BlockT, C>(
 	mut request_rx: Receiver<BabeRequest<B>>,
-	config: Config,
+	config: BabeConfiguration,
 	client: Arc<C>,
 	epoch_changes: SharedEpochChanges<B, Epoch>,
 ) where
@@ -640,9 +619,7 @@ async fn answer_requests<B: BlockT, C>(
 						.ok_or(Error::<B>::FetchEpoch(parent_hash))?;
 
 					let viable_epoch = epoch_changes
-						.viable_epoch(&epoch_descriptor, |slot| {
-							Epoch::genesis(&config.genesis_config, slot)
-						})
+						.viable_epoch(&epoch_descriptor, |slot| Epoch::genesis(&config, slot))
 						.ok_or(Error::<B>::FetchEpoch(parent_hash))?;
 
 					Ok(sp_consensus_babe::Epoch {
@@ -739,7 +716,7 @@ struct BabeSlotWorker<B: BlockT, C, E, I, SO, L, BS> {
 	keystore: SyncCryptoStorePtr,
 	epoch_changes: SharedEpochChanges<B, Epoch>,
 	slot_notification_sinks: SlotNotificationSinks<B>,
-	config: Config,
+	config: BabeConfiguration,
 	block_proposal_slot_portion: SlotProportion,
 	max_block_proposal_slot_portion: Option<SlotProportion>,
 	telemetry: Option<TelemetryHandle>,
@@ -797,9 +774,7 @@ where
 	fn authorities_len(&self, epoch_descriptor: &Self::EpochData) -> Option<usize> {
 		self.epoch_changes
 			.shared_data()
-			.viable_epoch(epoch_descriptor, |slot| {
-				Epoch::genesis(&self.config.genesis_config, slot)
-			})
+			.viable_epoch(epoch_descriptor, |slot| Epoch::genesis(&self.config, slot))
 			.map(|epoch| epoch.as_ref().authorities.len())
 	}
 
@@ -814,9 +789,7 @@ where
 			slot,
 			self.epoch_changes
 				.shared_data()
-				.viable_epoch(epoch_descriptor, |slot| {
-					Epoch::genesis(&self.config.genesis_config, slot)
-				})?
+				.viable_epoch(epoch_descriptor, |slot| Epoch::genesis(&self.config, slot))?
 				.as_ref(),
 			&self.keystore,
 		);
@@ -1020,7 +993,7 @@ fn find_next_config_digest<B: BlockT>(
 #[derive(Clone)]
 pub struct BabeLink<Block: BlockT> {
 	epoch_changes: SharedEpochChanges<Block, Epoch>,
-	config: Config,
+	config: BabeConfiguration,
 }
 
 impl<Block: BlockT> BabeLink<Block> {
@@ -1030,7 +1003,7 @@ impl<Block: BlockT> BabeLink<Block> {
 	}
 
 	/// Get the config of this link.
-	pub fn config(&self) -> &Config {
+	pub fn config(&self) -> &BabeConfiguration {
 		&self.config
 	}
 }
@@ -1040,7 +1013,7 @@ pub struct BabeVerifier<Block: BlockT, Client, SelectChain, CAW, CIDP> {
 	client: Arc<Client>,
 	select_chain: SelectChain,
 	create_inherent_data_providers: CIDP,
-	config: Config,
+	config: BabeConfiguration,
 	epoch_changes: SharedEpochChanges<Block, Epoch>,
 	can_author_with: CAW,
 	telemetry: Option<TelemetryHandle>,
@@ -1245,9 +1218,7 @@ where
 				.map_err(|e| Error::<Block>::ForkTree(Box::new(e)))?
 				.ok_or(Error::<Block>::FetchEpoch(parent_hash))?;
 			let viable_epoch = epoch_changes
-				.viable_epoch(&epoch_descriptor, |slot| {
-					Epoch::genesis(&self.config.genesis_config, slot)
-				})
+				.viable_epoch(&epoch_descriptor, |slot| Epoch::genesis(&self.config, slot))
 				.ok_or(Error::<Block>::FetchEpoch(parent_hash))?;
 
 			// We add one to the current slot to allow for some small drift.
@@ -1353,7 +1324,7 @@ pub struct BabeBlockImport<Block: BlockT, Client, I> {
 	inner: I,
 	client: Arc<Client>,
 	epoch_changes: SharedEpochChanges<Block, Epoch>,
-	config: Config,
+	config: BabeConfiguration,
 }
 
 impl<Block: BlockT, I: Clone, Client> Clone for BabeBlockImport<Block, Client, I> {
@@ -1372,7 +1343,7 @@ impl<Block: BlockT, Client, I> BabeBlockImport<Block, Client, I> {
 		client: Arc<Client>,
 		epoch_changes: SharedEpochChanges<Block, Epoch>,
 		block_import: I,
-		config: Config,
+		config: BabeConfiguration,
 	) -> Self {
 		BabeBlockImport { client, inner: block_import, epoch_changes, config }
 	}
@@ -1580,9 +1551,7 @@ where
 				old_epoch_changes = Some((*epoch_changes).clone());
 
 				let viable_epoch = epoch_changes
-					.viable_epoch(&epoch_descriptor, |slot| {
-						Epoch::genesis(&self.config.genesis_config, slot)
-					})
+					.viable_epoch(&epoch_descriptor, |slot| Epoch::genesis(&self.config, slot))
 					.ok_or_else(|| {
 						ConsensusError::ClientImport(Error::<Block>::FetchEpoch(parent_hash).into())
 					})?;
@@ -1761,7 +1730,7 @@ where
 /// Also returns a link object used to correctly instantiate the import queue
 /// and background worker.
 pub fn block_import<Client, Block: BlockT, I>(
-	config: Config,
+	config: BabeConfiguration,
 	wrapped_block_import: I,
 	client: Arc<Client>,
 ) -> ClientResult<(BabeBlockImport<Block, Client, I>, BabeLink<Block>)>
@@ -1772,8 +1741,7 @@ where
 		+ PreCommitActions<Block>
 		+ 'static,
 {
-	let epoch_changes =
-		aux_schema::load_epoch_changes::<Block, _>(&*client, &config.genesis_config)?;
+	let epoch_changes = aux_schema::load_epoch_changes::<Block, _>(&*client, &config)?;
 	let link = BabeLink { epoch_changes: epoch_changes.clone(), config: config.clone() };
 
 	// NOTE: this isn't entirely necessary, but since we didn't use to prune the
@@ -1884,9 +1852,9 @@ where
 
 	// Revert epoch changes tree.
 
-	let config = Config::get(&*client)?;
-	let epoch_changes =
-		aux_schema::load_epoch_changes::<Block, Client>(&*client, config.genesis_config())?;
+	// This config is only used on-genesis.
+	let config = configuration(&*client)?;
+	let epoch_changes = aux_schema::load_epoch_changes::<Block, Client>(&*client, &config)?;
 	let mut epoch_changes = epoch_changes.shared_data();
 
 	if revert_up_to_number == Zero::zero() {