Refactor epoch changes to a separate crate (paritytech#4785)

* Init epoch changes module

* Initial integration of new epoch changes module for BABE

* Fix all initial compile errors

* rename: digest -> digests

* Fix babe tests

* Bump impl_version

* Fix more test issues

* Remove test flag for tree

It unfortunately won't work for multiple crates.

* Update cargo lock

* Fix duplicate parking_lot version

* Add missing license header

Cargo.toml

@@ -25,8 +25,9 @@ members = [
 	"client/consensus/babe",
 	"client/consensus/manual-seal",
 	"client/consensus/pow",
-	"client/consensus/slots",
 	"client/consensus/uncles",
+	"client/consensus/slots",
+	"client/consensus/epochs",
 	"client/db",
 	"client/executor",
 	"client/executor/common",

bin/node/runtime/src/lib.rs

@@ -80,7 +80,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
 	spec_version: 214,
-	impl_version: 0,
+	impl_version: 1,
 	apis: RUNTIME_API_VERSIONS,
 };
 

client/consensus/babe/Cargo.toml

@@ -22,6 +22,7 @@ sc-telemetry = { version = "2.0.0", path = "../../telemetry" }
 sc-keystore = { version = "2.0.0", path = "../../keystore" }
 sc-client-api = { version = "2.0.0", path = "../../api" }
 sc-client = { version = "0.8", path = "../../" }
+sc-consensus-epochs = { version = "0.8", path = "../epochs" }
 sp-api = { version = "2.0.0", path = "../../../primitives/api" }
 sp-block-builder = { version = "2.0.0", path = "../../../primitives/block-builder" }
 sp-blockchain = { version = "2.0.0", path = "../../../primitives/blockchain" }

client/consensus/babe/src/authorship.rs

@@ -17,13 +17,17 @@
 //! BABE authority selection and slot claiming.
 
 use merlin::Transcript;
-use sp_consensus_babe::{AuthorityId, BabeAuthorityWeight, BABE_ENGINE_ID, BABE_VRF_PREFIX};
-use sp_consensus_babe::{Epoch, SlotNumber, AuthorityPair, BabePreDigest, BabeConfiguration};
+use sp_consensus_babe::{
+	AuthorityId, BabeAuthorityWeight, BABE_ENGINE_ID, BABE_VRF_PREFIX,
+	SlotNumber, AuthorityPair, BabeConfiguration
+};
+use sp_consensus_babe::digests::PreDigest;
 use sp_core::{U256, blake2_256};
 use codec::Encode;
 use schnorrkel::vrf::VRFInOut;
 use sp_core::Pair;
 use sc_keystore::KeyStorePtr;
+use super::Epoch;
 
 /// Calculates the primary selection threshold for a given authority, taking
 /// into account `c` (`1 - c` represents the probability of a slot being empty).
@@ -104,7 +108,7 @@ fn claim_secondary_slot(
 	authorities: &[(AuthorityId, BabeAuthorityWeight)],
 	keystore: &KeyStorePtr,
 	randomness: [u8; 32],
-) -> Option<(BabePreDigest, AuthorityPair)> {
+) -> Option<(PreDigest, AuthorityPair)> {
 	if authorities.is_empty() {
 		return None;
 	}
@@ -124,7 +128,7 @@ fn claim_secondary_slot(
 		})
 	{
 		if pair.public() == *expected_author {
-			let pre_digest = BabePreDigest::Secondary {
+			let pre_digest = PreDigest::Secondary {
 				slot_number,
 				authority_index: authority_index as u32,
 			};
@@ -145,7 +149,7 @@ pub(super) fn claim_slot(
 	epoch: &Epoch,
 	config: &BabeConfiguration,
 	keystore: &KeyStorePtr,
-) -> Option<(BabePreDigest, AuthorityPair)> {
+) -> Option<(PreDigest, AuthorityPair)> {
 	claim_primary_slot(slot_number, epoch, config.c, keystore)
 		.or_else(|| {
 			if config.secondary_slots {
@@ -175,7 +179,7 @@ fn claim_primary_slot(
 	epoch: &Epoch,
 	c: (u64, u64),
 	keystore: &KeyStorePtr,
-) -> Option<(BabePreDigest, AuthorityPair)> {
+) -> Option<(PreDigest, AuthorityPair)> {
 	let Epoch { authorities, randomness, epoch_index, .. } = epoch;
 	let keystore = keystore.read();
 
@@ -196,7 +200,7 @@ fn claim_primary_slot(
 		let pre_digest = get_keypair(&pair)
 			.vrf_sign_after_check(transcript, |inout| super::authorship::check_primary_threshold(inout, threshold))
 			.map(|s| {
-				BabePreDigest::Primary {
+				PreDigest::Primary {
 					slot_number,
 					vrf_output: s.0.to_output(),
 					vrf_proof: s.1,

client/consensus/babe/src/aux_schema.rs

@@ -16,15 +16,17 @@
 
 //! Schema for BABE epoch changes in the aux-db.
 
+use std::sync::Arc;
+use parking_lot::Mutex;
 use log::info;
 use codec::{Decode, Encode};
 
 use sc_client_api::backend::AuxStore;
 use sp_blockchain::{Result as ClientResult, Error as ClientError};
 use sp_runtime::traits::Block as BlockT;
 use sp_consensus_babe::BabeBlockWeight;
-
-use super::{epoch_changes::EpochChangesFor, SharedEpochChanges};
+use sc_consensus_epochs::{EpochChangesFor, SharedEpochChanges};
+use crate::Epoch;
 
 const BABE_EPOCH_CHANGES: &[u8] = b"babe_epoch_changes";
 
@@ -49,14 +51,14 @@ fn load_decode<B, T>(backend: &B, key: &[u8]) -> ClientResult<Option<T>>
 /// Load or initialize persistent epoch change data from backend.
 pub(crate) fn load_epoch_changes<Block: BlockT, B: AuxStore>(
 	backend: &B,
-) -> ClientResult<SharedEpochChanges<Block>> {
-	let epoch_changes = load_decode::<_, EpochChangesFor<Block>>(backend, BABE_EPOCH_CHANGES)?
-		.map(Into::into)
+) -> ClientResult<SharedEpochChanges<Block, Epoch>> {
+	let epoch_changes = load_decode::<_, EpochChangesFor<Block, Epoch>>(backend, BABE_EPOCH_CHANGES)?
+		.map(|v| Arc::new(Mutex::new(v)))
 		.unwrap_or_else(|| {
 			info!(target: "babe",
 				"Creating empty BABE epoch changes on what appears to be first startup."
 			);
-			SharedEpochChanges::new()
+			SharedEpochChanges::<Block, Epoch>::default()
 		});
 
 	// rebalance the tree after deserialization. this isn't strictly necessary
@@ -70,7 +72,7 @@ pub(crate) fn load_epoch_changes<Block: BlockT, B: AuxStore>(
 
 /// Update the epoch changes on disk after a change.
 pub(crate) fn write_epoch_changes<Block: BlockT, F, R>(
-	epoch_changes: &EpochChangesFor<Block>,
+	epoch_changes: &EpochChangesFor<Block, Epoch>,
 	write_aux: F,
 ) -> R where
 	F: FnOnce(&[(&'static [u8], &[u8])]) -> R,

client/consensus/babe/src/lib.rs

@@ -59,8 +59,10 @@
 #![forbid(unsafe_code)]
 #![warn(missing_docs)]
 pub use sp_consensus_babe::{
-	BabeApi, ConsensusLog, BABE_ENGINE_ID, BabePreDigest, SlotNumber, BabeConfiguration,
-	CompatibleDigestItem,
+	BabeApi, ConsensusLog, BABE_ENGINE_ID, SlotNumber, BabeConfiguration,
+	AuthorityId, AuthorityPair, AuthoritySignature,
+	BabeAuthorityWeight, VRF_OUTPUT_LENGTH,
+	digests::{PreDigest, CompatibleDigestItem, NextEpochDescriptor},
 };
 pub use sp_consensus::SyncOracle;
 use std::{collections::HashMap, sync::Arc, u64, pin::Pin, time::{Instant, Duration}};
@@ -101,26 +103,58 @@ use log::{warn, debug, info, trace};
 use sc_consensus_slots::{
 	SlotWorker, SlotInfo, SlotCompatible, StorageChanges, CheckedHeader, check_equivocation,
 };
-use epoch_changes::descendent_query;
+use sc_consensus_epochs::{descendent_query, SharedEpochChanges, EpochChangesFor, Epoch as EpochT};
 use sp_blockchain::{
 	Result as ClientResult, Error as ClientError,
 	HeaderBackend, ProvideCache, HeaderMetadata
 };
 use schnorrkel::SignatureError;
-
+use codec::{Encode, Decode};
 use sp_api::ApiExt;
 
 mod aux_schema;
 mod verification;
-mod epoch_changes;
 mod authorship;
 #[cfg(test)]
 mod tests;
-pub use sp_consensus_babe::{
-	AuthorityId, AuthorityPair, AuthoritySignature, Epoch, NextEpochDescriptor,
-};
-pub use epoch_changes::{EpochChanges, EpochChangesFor, SharedEpochChanges};
 
+/// BABE epoch information
+#[derive(Decode, Encode, Default, PartialEq, Eq, Clone, Debug)]
+pub struct Epoch {
+	/// The epoch index
+	pub epoch_index: u64,
+	/// The starting slot of the epoch,
+	pub start_slot: SlotNumber,
+	/// The duration of this epoch
+	pub duration: SlotNumber,
+	/// The authorities and their weights
+	pub authorities: Vec<(AuthorityId, BabeAuthorityWeight)>,
+	/// Randomness for this epoch
+	pub randomness: [u8; VRF_OUTPUT_LENGTH],
+}
+
+impl EpochT for Epoch {
+	type NextEpochDescriptor = NextEpochDescriptor;
+	type SlotNumber = SlotNumber;
+
+	fn increment(&self, descriptor: NextEpochDescriptor) -> Epoch {
+		Epoch {
+			epoch_index: self.epoch_index + 1,
+			start_slot: self.start_slot + self.duration,
+			duration: self.duration,
+			authorities: descriptor.authorities,
+			randomness: descriptor.randomness,
+		}
+	}
+
+	fn start_slot(&self) -> SlotNumber {
+		self.start_slot
+	}
+
+	fn end_slot(&self) -> SlotNumber {
+		self.start_slot + self.duration
+	}
+}
 
 #[derive(derive_more::Display, Debug)]
 enum Error<B: BlockT> {
@@ -343,7 +377,7 @@ struct BabeWorker<B: BlockT, C, E, I, SO> {
 	sync_oracle: SO,
 	force_authoring: bool,
 	keystore: KeyStorePtr,
-	epoch_changes: SharedEpochChanges<B>,
+	epoch_changes: SharedEpochChanges<B, Epoch>,
 	config: Config,
 }
 
@@ -361,7 +395,7 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeWork
 	Error: std::error::Error + Send + From<ConsensusError> + From<I::Error> + 'static,
 {
 	type EpochData = Epoch;
-	type Claim = (BabePreDigest, AuthorityPair);
+	type Claim = (PreDigest, AuthorityPair);
 	type SyncOracle = SO;
 	type CreateProposer = Pin<Box<
 		dyn Future<Output = Result<E::Proposer, sp_consensus::Error>> + Send + 'static
@@ -533,12 +567,12 @@ impl<B, C, E, I, Error, SO> SlotWorker<B> for BabeWorker<B, C, E, I, SO> where
 
 /// Extract the BABE pre digest from the given header. Pre-runtime digests are
 /// mandatory, the function will return `Err` if none is found.
-fn find_pre_digest<B: BlockT>(header: &B::Header) -> Result<BabePreDigest, Error<B>>
+fn find_pre_digest<B: BlockT>(header: &B::Header) -> Result<PreDigest, Error<B>>
 {
 	// genesis block doesn't contain a pre digest so let's generate a
 	// dummy one to not break any invariants in the rest of the code
 	if header.number().is_zero() {
-		return Ok(BabePreDigest::Secondary {
+		return Ok(PreDigest::Secondary {
 			slot_number: 0,
 			authority_index: 0,
 		});
@@ -597,7 +631,7 @@ impl SlotCompatible for TimeSource {
 #[derive(Clone)]
 pub struct BabeLink<Block: BlockT> {
 	time_source: TimeSource,
-	epoch_changes: SharedEpochChanges<Block>,
+	epoch_changes: SharedEpochChanges<Block, Epoch>,
 	config: Config,
 }
 /// A verifier for Babe blocks.
@@ -606,7 +640,7 @@ pub struct BabeVerifier<B, E, Block: BlockT, RA, PRA> {
 	api: Arc<PRA>,
 	inherent_data_providers: sp_inherents::InherentDataProviders,
 	config: Config,
-	epoch_changes: SharedEpochChanges<Block>,
+	epoch_changes: SharedEpochChanges<Block, Epoch>,
 	time_source: TimeSource,
 }
 
@@ -711,7 +745,7 @@ impl<B, E, Block, RA, PRA> Verifier<Block> for BabeVerifier<B, E, Block, RA, PRA
 		let mut inherent_data = self
 			.inherent_data_providers
 			.create_inherent_data()
-			.map_err( Error::<Block>::Runtime)?;
+			.map_err(Error::<Block>::Runtime)?;
 
 		let (_, slot_now, _) = self.time_source.extract_timestamp_and_slot(&inherent_data)
 			.map_err(Error::<Block>::Extraction)?;
@@ -855,7 +889,7 @@ pub struct BabeBlockImport<B, E, Block: BlockT, I, RA, PRA> {
 	inner: I,
 	client: Arc<Client<B, E, Block, RA>>,
 	api: Arc<PRA>,
-	epoch_changes: SharedEpochChanges<Block>,
+	epoch_changes: SharedEpochChanges<Block, Epoch>,
 	config: Config,
 }
 
@@ -875,7 +909,7 @@ impl<B, E, Block: BlockT, I, RA, PRA> BabeBlockImport<B, E, Block, I, RA, PRA> {
 	fn new(
 		client: Arc<Client<B, E, Block, RA>>,
 		api: Arc<PRA>,
-		epoch_changes: SharedEpochChanges<Block>,
+		epoch_changes: SharedEpochChanges<Block, Epoch>,
 		block_import: I,
 		config: Config,
 	) -> Self {
@@ -1114,7 +1148,7 @@ impl<B, E, Block, I, RA, PRA> BlockImport<Block> for BabeBlockImport<B, E, Block
 /// Gets the best finalized block and its slot, and prunes the given epoch tree.
 fn prune_finalized<B, E, Block, RA>(
 	client: &Client<B, E, Block, RA>,
-	epoch_changes: &mut EpochChangesFor<Block>,
+	epoch_changes: &mut EpochChangesFor<Block, Epoch>,
 ) -> Result<(), ConsensusError> where
 	Block: BlockT,
 	E: CallExecutor<Block> + Send + Sync,
@@ -1161,7 +1195,7 @@ pub fn block_import<B, E, Block: BlockT, I, RA, PRA>(
 	RA: Send + Sync,
 	Client<B, E, Block, RA>: AuxStore,
 {
-	let epoch_changes = aux_schema::load_epoch_changes(&*client)?;
+	let epoch_changes = aux_schema::load_epoch_changes::<Block, _>(&*client)?;
 	let link = BabeLink {
 		epoch_changes: epoch_changes.clone(),
 		time_source: Default::default(),
@@ -1245,7 +1279,7 @@ pub mod test_helpers {
 		client: &C,
 		keystore: &KeyStorePtr,
 		link: &BabeLink<B>,
-	) -> Option<BabePreDigest> where
+	) -> Option<PreDigest> where
 		B: BlockT,
 		C: ProvideRuntimeApi<B> +
 			ProvideCache<B> +

client/consensus/babe/src/tests.rs

@@ -59,7 +59,7 @@ type Mutator = Arc<dyn Fn(&mut TestHeader, Stage) + Send + Sync>;
 #[derive(Clone)]
 struct DummyFactory {
 	client: Arc<TestClient>,
-	epoch_changes: crate::SharedEpochChanges<TestBlock>,
+	epoch_changes: SharedEpochChanges<TestBlock, Epoch>,
 	config: Config,
 	mutator: Mutator,
 }
@@ -105,7 +105,6 @@ impl DummyProposer {
 			>
 		>
 	{
-		use codec::Encode;
 		let block_builder = self.factory.client.new_block_at(
 			&BlockId::Hash(self.parent_hash),
 			pre_digests,
@@ -558,7 +557,7 @@ fn propose_and_import_block<Transaction>(
 	let pre_digest = sp_runtime::generic::Digest {
 		logs: vec![
 			Item::babe_pre_digest(
-				BabePreDigest::Secondary {
+				PreDigest::Secondary {
 					authority_index: 0,
 					slot_number,
 				},