Switch to use NextConfigDescriptor instead of changing runtime interface

client/consensus/babe/src/lib.rs

@@ -64,7 +64,8 @@ pub use sp_consensus_babe::{
 	AuthorityId, AuthorityPair, AuthoritySignature,
 	BabeAuthorityWeight, VRF_OUTPUT_LENGTH,
 	digests::{
-		CompatibleDigestItem, NextEpochDescriptor, PreDigest, PrimaryPreDigest, SecondaryPreDigest,
+		CompatibleDigestItem, NextEpochDescriptor, NextConfigDescriptor,
+		PreDigest, PrimaryPreDigest, SecondaryPreDigest,
 	},
 };
 pub use sp_consensus::SyncOracle;
@@ -172,16 +173,18 @@ impl Epoch {
 	/// the first block, so that has to be provided.
 	pub fn genesis(
 		genesis_config: &BabeGenesisConfiguration,
-		epoch_config: &BabeEpochConfiguration,
 		slot_number: SlotNumber
 	) -> Epoch {
 		Epoch {
 			epoch_index: 0,
 			start_slot: slot_number,
-			duration: epoch_config.epoch_length,
+			duration: genesis_config.epoch_length,
 			authorities: genesis_config.genesis_authorities.clone(),
 			randomness: genesis_config.randomness.clone(),
-			config: epoch_config.clone(),
+			config: BabeEpochConfiguration {
+				c: genesis_config.c,
+				secondary_slots: genesis_config.secondary_slots,
+			},
 		}
 	}
 }
@@ -194,8 +197,8 @@ enum Error<B: BlockT> {
 	NoPreRuntimeDigest,
 	#[display(fmt = "Multiple BABE epoch change digests, rejecting!")]
 	MultipleEpochChangeDigests,
-	#[display(fmt = "Fetch new epoch configuration failed")]
-	FetchEpochConfig(B::Hash),
+	#[display(fmt = "Multiple BABE config change digests, rejecting!")]
+	MultipleConfigChangeDigests,
 	#[display(fmt = "Could not extract timestamp and slot: {:?}", _0)]
 	Extraction(sp_consensus::Error),
 	#[display(fmt = "Could not fetch epoch at {:?}", _0)]
@@ -228,6 +231,8 @@ enum Error<B: BlockT> {
 	FetchParentHeader(sp_blockchain::Error),
 	#[display(fmt = "Expected epoch change to happen at {:?}, s{}", _0, _1)]
 	ExpectedEpochChange(B::Hash, u64),
+	#[display(fmt = "Unexpected config change")]
+	UnexpectedConfigChange,
 	#[display(fmt = "Unexpected epoch change")]
 	UnexpectedEpochChange,
 	#[display(fmt = "Parent block of {} has no associated weight", _0)]
@@ -283,9 +288,7 @@ impl Config {
 		C: AuxStore + ProvideRuntimeApi<B>, C::Api: BabeApi<B, Error = sp_blockchain::Error>,
 	{
 		trace!(target: "babe", "Getting slot duration");
-		match sc_consensus_slots::SlotDuration::get_or_compute(client, |a, b| {
-			a.genesis_configuration(b)
-		}).map(Self) {
+		match sc_consensus_slots::SlotDuration::get_or_compute(client, |a, b| a.configuration(b)).map(Self) {
 			Ok(s) => Ok(s),
 			Err(s) => {
 				warn!(target: "babe", "Failed to get slot duration");
@@ -456,12 +459,7 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeWork
 
 	fn authorities_len(&self, epoch_descriptor: &Self::EpochData) -> Option<usize> {
 		self.epoch_changes.lock()
-			.viable_epoch(&epoch_descriptor, |slot| {
-				let epoch_config = self.client.runtime_api()
-					.epoch_configuration(&BlockId::number(Zero::zero()))
-					.ok()?;
-				Some(Epoch::genesis(&self.config, &epoch_config, slot))
-			})
+			.viable_epoch(&epoch_descriptor, |slot| Epoch::genesis(&self.config, slot))
 			.map(|epoch| epoch.as_ref().authorities.len())
 	}
 
@@ -476,12 +474,7 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeWork
 			slot_number,
 			self.epoch_changes.lock().viable_epoch(
 				&epoch_descriptor,
-				|slot| {
-					let epoch_config = self.client.runtime_api()
-						.epoch_configuration(&BlockId::number(Zero::zero()))
-						.ok()?;
-					Some(Epoch::genesis(&self.config, &epoch_config, slot))
-				}
+				|slot| Epoch::genesis(&self.config, slot)
 			)?.as_ref(),
 			&self.keystore,
 		);
@@ -646,6 +639,24 @@ fn find_next_epoch_digest<B: BlockT>(header: &B::Header)
 	Ok(epoch_digest)
 }
 
+/// Extract the BABE config change digest from the given header, if it exists.
+fn find_next_config_digest<B: BlockT>(header: &B::Header)
+	-> Result<Option<NextConfigDescriptor>, Error<B>>
+	where DigestItemFor<B>: CompatibleDigestItem,
+{
+	let mut config_digest: Option<_> = None;
+	for log in header.digest().logs() {
+		trace!(target: "babe", "Checking log {:?}, looking for epoch change digest.", log);
+		let log = log.try_to::<ConsensusLog>(OpaqueDigestItemId::Consensus(&BABE_ENGINE_ID));
+		match (log, config_digest.is_some()) {
+			(Some(ConsensusLog::NextConfigData(_)), true) => return Err(babe_err(Error::MultipleConfigChangeDigests)),
+			(Some(ConsensusLog::NextConfigData(config)), false) => config_digest = Some(config),
+			_ => trace!(target: "babe", "Ignoring digest not meant for us"),
+		}
+	}
+
+	Ok(config_digest)
+}
 
 #[derive(Default, Clone)]
 struct TimeSource(Arc<Mutex<(Option<Duration>, Vec<(Instant, u64)>)>>);
@@ -773,12 +784,7 @@ impl<Block, Client> Verifier<Block> for BabeVerifier<Block, Client> where
 			.ok_or_else(|| Error::<Block>::FetchEpoch(parent_hash))?;
 		let viable_epoch = epoch_changes.viable_epoch(
 			&epoch_descriptor,
-			|slot| {
-				let epoch_config = self.client.runtime_api()
-					.epoch_configuration(&BlockId::number(Zero::zero()))
-					.ok()?;
-				Some(Epoch::genesis(&self.config, &epoch_config, slot))
-			}
+			|slot| Epoch::genesis(&self.config, slot)
 		).ok_or_else(|| Error::<Block>::FetchEpoch(parent_hash))?;
 
 		// We add one to the current slot to allow for some small drift.
@@ -1009,19 +1015,32 @@ impl<Block, Client, Inner> BlockImport<Block> for BabeBlockImport<Block, Client,
 		// search for this all the time so we can reject unexpected announcements.
 		let next_epoch_digest = find_next_epoch_digest::<Block>(&block.header)
 			.map_err(|e| ConsensusError::ClientImport(e.to_string()))?;
+		let next_config_digest = find_next_config_digest::<Block>(&block.header)
+			.map_err(|e| ConsensusError::ClientImport(e.to_string()))?;
 
-		match (first_in_epoch, next_epoch_digest.is_some()) {
-			(true, true) => {},
-			(false, false) => {},
-			(true, false) => {
+		match (first_in_epoch, next_epoch_digest.is_some(), next_config_digest.is_some()) {
+			(true, true, _) => {},
+			(false, false, false) => {},
+			(false, false, true) => {
+				return Err(
+					ConsensusError::ClientImport(
+						babe_err(Error::<Block>::UnexpectedConfigChange).into(),
+					)
+				)
+			},
+			(true, false, _) => {
 				return Err(
 					ConsensusError::ClientImport(
 						babe_err(Error::<Block>::ExpectedEpochChange(hash, slot_number)).into(),
 					)
-				);
+				)
 			},
-			(false, true) => {
-				return Err(ConsensusError::ClientImport(Error::<Block>::UnexpectedEpochChange.into()));
+			(false, true, _) => {
+				return Err(
+					ConsensusError::ClientImport(
+						babe_err(Error::<Block>::UnexpectedEpochChange).into(),
+					)
+				)
 			},
 		}
 
@@ -1036,21 +1055,14 @@ impl<Block, Client, Inner> BlockImport<Block> for BabeBlockImport<Block, Client,
 
 			let viable_epoch = epoch_changes.viable_epoch(
 				&epoch_descriptor,
-				|slot| {
-					let epoch_config = self.client.runtime_api()
-						.epoch_configuration(&BlockId::number(Zero::zero()))
-						.ok()?;
-					Some(Epoch::genesis(&self.config, &epoch_config, slot))
-				}
+				|slot| Epoch::genesis(&self.config, slot)
 			).ok_or_else(|| {
 				ConsensusError::ClientImport(Error::<Block>::FetchEpoch(parent_hash).into())
 			})?;
 
-			let epoch_config = self.client.runtime_api()
-				.epoch_configuration(&BlockId::hash(parent_hash))
-				.map_err(|_| ConsensusError::ClientImport(
-					Error::<Block>::FetchEpochConfig(parent_hash).into()
-				))?;
+			let epoch_config = next_config_digest.unwrap_or_else(
+				|| viable_epoch.as_ref().config.clone()
+			);
 
 			babe_info!("👶 New epoch {} launching at block {} (block slot {} >= start slot {}).",
 					   viable_epoch.as_ref().epoch_index,

client/consensus/epochs/src/lib.rs

@@ -394,11 +394,11 @@ impl<Hash, Number, E: Epoch> EpochChanges<Hash, Number, E> where
 		descriptor: &ViableEpochDescriptor<Hash, Number, E>,
 		make_genesis: G,
 	) -> Option<ViableEpoch<E, &E>> where
-		G: FnOnce(E::SlotNumber) -> Option<E>
+		G: FnOnce(E::SlotNumber) -> E
 	{
 		match descriptor {
 			ViableEpochDescriptor::UnimportedGenesis(slot_number) => {
-				make_genesis(*slot_number).map(ViableEpoch::UnimportedGenesis)
+				Some(ViableEpoch::UnimportedGenesis(make_genesis(*slot_number)))
 			},
 			ViableEpochDescriptor::Signaled(identifier, _) => {
 				self.epoch(&identifier).map(ViableEpoch::Signaled)

primitives/consensus/babe/src/digests.rs

@@ -17,7 +17,7 @@
 //! Private implementation details of BABE digests.
 
 #[cfg(feature = "std")]
-use super::{BABE_ENGINE_ID, AuthoritySignature};
+use super::{BABE_ENGINE_ID, AuthoritySignature, BabeEpochConfiguration};
 use super::{AuthorityId, AuthorityIndex, SlotNumber, BabeAuthorityWeight};
 #[cfg(feature = "std")]
 use sp_runtime::{DigestItem, generic::OpaqueDigestItemId};
@@ -135,7 +135,7 @@ impl TryFrom<RawPreDigest> for PreDigest {
 
 /// Information about the next epoch. This is broadcast in the first block
 /// of the epoch.
-#[derive(Decode, Encode, Default, PartialEq, Eq, Clone, RuntimeDebug)]
+#[derive(Decode, Encode, PartialEq, Eq, Clone, RuntimeDebug)]
 pub struct NextEpochDescriptor {
 	/// The authorities.
 	pub authorities: Vec<(AuthorityId, BabeAuthorityWeight)>,
@@ -144,6 +144,10 @@ pub struct NextEpochDescriptor {
 	pub randomness: Randomness,
 }
 
+/// Information about the next epoch config, if changed. This is broadcast in the first
+/// block of the epoch, and applies using the same rules as `NextEpochDescriptor`.
+pub type NextConfigDescriptor = BabeEpochConfiguration;
+
 /// A digest item which is usable with BABE consensus.
 #[cfg(feature = "std")]
 pub trait CompatibleDigestItem: Sized {
@@ -159,8 +163,11 @@ pub trait CompatibleDigestItem: Sized {
 	/// If this item is a BABE signature, return the signature.
 	fn as_babe_seal(&self) -> Option<AuthoritySignature>;
 
-	/// If this item is a BABE epoch, return it.
+	/// If this item is a BABE epoch descriptor, return it.
 	fn as_next_epoch_descriptor(&self) -> Option<NextEpochDescriptor>;
+
+	/// If this item is a BABE config descriptor, return it.
+	fn as_next_config_descriptor(&self) -> Option<NextConfigDescriptor>;
 }
 
 #[cfg(feature = "std")]
@@ -190,4 +197,12 @@ impl<Hash> CompatibleDigestItem for DigestItem<Hash> where
 				_ => None,
 			})
 	}
+
+	fn as_next_config_descriptor(&self) -> Option<NextConfigDescriptor> {
+		self.try_to(OpaqueDigestItemId::Consensus(&BABE_ENGINE_ID))
+			.and_then(|x: super::ConsensusLog| match x {
+				super::ConsensusLog::NextConfigData(n) => Some(n),
+				_ => None,
+			})
+	}
 }

primitives/consensus/babe/src/lib.rs

@@ -29,7 +29,7 @@ pub use sp_consensus_vrf::schnorrkel::{
 use codec::{Encode, Decode};
 use sp_std::vec::Vec;
 use sp_runtime::{ConsensusEngineId, RuntimeDebug};
-use crate::digests::NextEpochDescriptor;
+use crate::digests::{NextEpochDescriptor, NextConfigDescriptor};
 
 mod app {
 	use sp_application_crypto::{app_crypto, key_types::BABE, sr25519};
@@ -87,20 +87,41 @@ pub enum ConsensusLog {
 	/// Disable the authority with given index.
 	#[codec(index = "2")]
 	OnDisabled(AuthorityIndex),
+	/// The epoch has changed, and the epoch after the current one will
+	/// enact different epoch configurations.
+	#[codec(index = "3")]
+	NextConfigData(NextConfigDescriptor),
 }
 
-/// Genesis configuration data used by the BABE consensus engine.
+/// Configuration data used by the BABE consensus engine.
 #[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)]
 pub struct BabeGenesisConfiguration {
 	/// The slot duration in milliseconds for BABE. Currently, only
 	/// the value provided by this type at genesis will be used.
+	///
+	/// Dynamic slot duration may be supported in the future.
 	pub slot_duration: u64,
 
+	/// The duration of epochs in slots.
+	pub epoch_length: SlotNumber,
+
+	/// A constant value that is used in the threshold calculation formula.
+	/// Expressed as a rational where the first member of the tuple is the
+	/// numerator and the second is the denominator. The rational should
+	/// represent a value between 0 and 1.
+	/// In the threshold formula calculation, `1 - c` represents the probability
+	/// of a slot being empty.
+	pub c: (u64, u64),
+
 	/// The authorities for the genesis epoch.
 	pub genesis_authorities: Vec<(AuthorityId, BabeAuthorityWeight)>,
 
 	/// The randomness for the genesis epoch.
 	pub randomness: Randomness,
+
+	/// Whether this chain should run with secondary slots, which are assigned
+	/// in round-robin manner.
+	pub secondary_slots: bool,
 }
 
 #[cfg(feature = "std")]
@@ -115,9 +136,6 @@ impl sp_consensus::SlotData for BabeGenesisConfiguration {
 /// Configuration data used by the BABE consensus engine.
 #[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)]
 pub struct BabeEpochConfiguration {
-	/// The duration of epochs in slots.
-	pub epoch_length: SlotNumber,
-
 	/// A constant value that is used in the threshold calculation formula.
 	/// Expressed as a rational where the first member of the tuple is the
 	/// numerator and the second is the denominator. The rational should
@@ -134,11 +152,8 @@ pub struct BabeEpochConfiguration {
 sp_api::decl_runtime_apis! {
 	/// API necessary for block authorship with BABE.
 	pub trait BabeApi {
-		/// Return the epoch configuration for BABE. The configuration is read once every epoch.
-		fn epoch_configuration() -> BabeEpochConfiguration;
-
 		/// Return the genesis configuration for BABE. The configuration is only read on genesis.
-		fn genesis_configuration() -> BabeGenesisConfiguration;
+		fn configuration() -> BabeGenesisConfiguration;
 
 		/// Returns the slot number that started the current epoch.
 		fn current_epoch_start() -> SlotNumber;