From 770cc24c4774434583609f98eeb9f5ab7f91ca7d Mon Sep 17 00:00:00 2001 From: Wei Tang <hi@that.world> Date: Fri, 24 Apr 2020 15:59:14 +0200 Subject: [PATCH] babe: support online configuration upgrades (#5514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * babe: support online configuration upgrades * Switch to use NextConfigDescriptor instead of changing runtime interface * Fix tests * epoch-changes: map function that allows converting with different epoch types * Add migration script for the epoch config change * Fix migration tests * Fix migration: Epoch should be EpochV0 * Update client/consensus/babe/src/lib.rs Co-Authored-By: André Silva <123550+andresilva@users.noreply.github.com> * Fix new epochChanges version * Fix unused imports Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> --- substrate/bin/node/runtime/src/lib.rs | 4 +- .../client/consensus/babe/rpc/src/lib.rs | 4 +- .../client/consensus/babe/src/authorship.rs | 7 +- .../client/consensus/babe/src/aux_schema.rs | 35 +++-- substrate/client/consensus/babe/src/lib.rs | 138 ++++++++++++------ .../client/consensus/babe/src/migration.rs | 64 ++++++++ substrate/client/consensus/babe/src/tests.rs | 16 +- .../client/consensus/babe/src/verification.rs | 7 +- substrate/client/consensus/epochs/src/lib.rs | 49 +++++++ .../primitives/consensus/babe/src/digests.rs | 21 ++- .../primitives/consensus/babe/src/lib.rs | 33 ++++- substrate/test-utils/runtime/src/lib.rs | 8 +- 12 files changed, 300 insertions(+), 86 deletions(-) create mode 100644 substrate/client/consensus/babe/src/migration.rs diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 16f355e6d60..806ed5460df 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -791,13 +791,13 @@ impl_runtime_apis! { } impl sp_consensus_babe::BabeApi<Block> for Runtime { - fn configuration() -> sp_consensus_babe::BabeConfiguration { + fn configuration() -> sp_consensus_babe::BabeGenesisConfiguration { // The choice of `c` parameter (where `1 - c` represents the // probability of a slot being empty), is done in accordance to the // slot duration and expected target block time, for safely // resisting network delays of maximum two seconds. // <https://research.web3.foundation/en/latest/polkadot/BABE/Babe/#6-practical-results> - sp_consensus_babe::BabeConfiguration { + sp_consensus_babe::BabeGenesisConfiguration { slot_duration: Babe::slot_duration(), epoch_length: EpochDuration::get(), c: PRIMARY_PROBABILITY, diff --git a/substrate/client/consensus/babe/rpc/src/lib.rs b/substrate/client/consensus/babe/rpc/src/lib.rs index cb78504b1f7..296b5cf2378 100644 --- a/substrate/client/consensus/babe/rpc/src/lib.rs +++ b/substrate/client/consensus/babe/rpc/src/lib.rs @@ -118,7 +118,7 @@ impl<B, C, SC> BabeApi for BabeRPCHandler<B, C, SC> for slot_number in epoch_start..epoch_end { let epoch = epoch_data(&shared_epoch, &client, &babe_config, slot_number, &select_chain)?; - if let Some((claim, key)) = authorship::claim_slot(slot_number, &epoch, &babe_config, &keystore) { + if let Some((claim, key)) = authorship::claim_slot(slot_number, &epoch, &keystore) { match claim { PreDigest::Primary { .. } => { claims.entry(key.public()).or_default().primary.push(slot_number); @@ -184,7 +184,7 @@ fn epoch_data<B, C, SC>( &parent.hash(), parent.number().clone(), slot_number, - |slot| babe_config.genesis_epoch(slot), + |slot| Epoch::genesis(&babe_config, slot), ) .map_err(|e| Error::Consensus(ConsensusError::ChainLookup(format!("{:?}", e))))? .ok_or(Error::Consensus(ConsensusError::InvalidAuthoritiesSet)) diff --git a/substrate/client/consensus/babe/src/authorship.rs b/substrate/client/consensus/babe/src/authorship.rs index 074e582bff2..165ff0ca4fe 100644 --- a/substrate/client/consensus/babe/src/authorship.rs +++ b/substrate/client/consensus/babe/src/authorship.rs @@ -19,7 +19,7 @@ use merlin::Transcript; use sp_consensus_babe::{ AuthorityId, BabeAuthorityWeight, BABE_ENGINE_ID, BABE_VRF_PREFIX, - SlotNumber, AuthorityPair, BabeConfiguration + SlotNumber, AuthorityPair, }; use sp_consensus_babe::digests::{PreDigest, PrimaryPreDigest, SecondaryPreDigest}; use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof}; @@ -147,12 +147,11 @@ fn claim_secondary_slot( pub fn claim_slot( slot_number: SlotNumber, epoch: &Epoch, - config: &BabeConfiguration, keystore: &KeyStorePtr, ) -> Option<(PreDigest, AuthorityPair)> { - claim_primary_slot(slot_number, epoch, config.c, keystore) + claim_primary_slot(slot_number, epoch, epoch.config.c, keystore) .or_else(|| { - if config.secondary_slots { + if epoch.config.secondary_slots { claim_secondary_slot( slot_number, &epoch.authorities, diff --git a/substrate/client/consensus/babe/src/aux_schema.rs b/substrate/client/consensus/babe/src/aux_schema.rs index e014c8975ac..9907fcbd724 100644 --- a/substrate/client/consensus/babe/src/aux_schema.rs +++ b/substrate/client/consensus/babe/src/aux_schema.rs @@ -24,13 +24,13 @@ 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 sp_consensus_babe::{BabeBlockWeight, BabeGenesisConfiguration}; use sc_consensus_epochs::{EpochChangesFor, SharedEpochChanges, migration::EpochChangesForV0}; -use crate::Epoch; +use crate::{Epoch, migration::EpochV0}; const BABE_EPOCH_CHANGES_VERSION: &[u8] = b"babe_epoch_changes_version"; const BABE_EPOCH_CHANGES_KEY: &[u8] = b"babe_epoch_changes"; -const BABE_EPOCH_CHANGES_CURRENT_VERSION: u32 = 1; +const BABE_EPOCH_CHANGES_CURRENT_VERSION: u32 = 2; fn block_weight_key<H: Encode>(block_hash: H) -> Vec<u8> { (b"block_weight", block_hash).encode() @@ -53,14 +53,19 @@ 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, + config: &BabeGenesisConfiguration, ) -> ClientResult<SharedEpochChanges<Block, Epoch>> { let version = load_decode::<_, u32>(backend, BABE_EPOCH_CHANGES_VERSION)?; let maybe_epoch_changes = match version { - None => load_decode::<_, EpochChangesForV0<Block, Epoch>>( + None => load_decode::<_, EpochChangesForV0<Block, EpochV0>>( backend, BABE_EPOCH_CHANGES_KEY, - )?.map(|v0| v0.migrate()), + )?.map(|v0| v0.migrate().map(|_, _, epoch| epoch.migrate(config))), + Some(1) => load_decode::<_, EpochChangesFor<Block, EpochV0>>( + backend, + BABE_EPOCH_CHANGES_KEY, + )?.map(|v1| v1.map(|_, _, epoch| epoch.migrate(config))), Some(BABE_EPOCH_CHANGES_CURRENT_VERSION) => load_decode::<_, EpochChangesFor<Block, Epoch>>( backend, BABE_EPOCH_CHANGES_KEY, @@ -131,18 +136,19 @@ pub(crate) fn load_block_weight<H: Encode, B: AuxStore>( #[cfg(test)] mod test { use super::*; - use crate::Epoch; + use crate::migration::EpochV0; use fork_tree::ForkTree; use substrate_test_runtime_client; use sp_core::H256; use sp_runtime::traits::NumberFor; + use sp_consensus_babe::BabeGenesisConfiguration; use sc_consensus_epochs::{PersistedEpoch, PersistedEpochHeader, EpochHeader}; use sp_consensus::Error as ConsensusError; use sc_network_test::Block as TestBlock; #[test] fn load_decode_from_v0_epoch_changes() { - let epoch = Epoch { + let epoch = EpochV0 { start_slot: 0, authorities: vec![], randomness: [0; 32], @@ -160,7 +166,7 @@ mod test { client.insert_aux( &[(BABE_EPOCH_CHANGES_KEY, - &EpochChangesForV0::<TestBlock, Epoch>::from_raw(v0_tree).encode()[..])], + &EpochChangesForV0::<TestBlock, EpochV0>::from_raw(v0_tree).encode()[..])], &[], ).unwrap(); @@ -169,7 +175,16 @@ mod test { None, ); - let epoch_changes = load_epoch_changes::<TestBlock, _>(&client).unwrap(); + let epoch_changes = load_epoch_changes::<TestBlock, _>( + &client, &BabeGenesisConfiguration { + slot_duration: 10, + epoch_length: 4, + c: (3, 10), + genesis_authorities: Vec::new(), + randomness: Default::default(), + secondary_slots: true, + }, + ).unwrap(); assert!( epoch_changes.lock() @@ -192,7 +207,7 @@ mod test { assert_eq!( load_decode::<_, u32>(&client, BABE_EPOCH_CHANGES_VERSION).unwrap(), - Some(1), + Some(2), ); } } diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 59b49541821..bac4e02897d 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -59,11 +59,13 @@ #![forbid(unsafe_code)] #![warn(missing_docs)] pub use sp_consensus_babe::{ - BabeApi, ConsensusLog, BABE_ENGINE_ID, SlotNumber, BabeConfiguration, + BabeApi, ConsensusLog, BABE_ENGINE_ID, SlotNumber, + BabeEpochConfiguration, BabeGenesisConfiguration, AuthorityId, AuthorityPair, AuthoritySignature, BabeAuthorityWeight, VRF_OUTPUT_LENGTH, digests::{ - CompatibleDigestItem, NextEpochDescriptor, PreDigest, PrimaryPreDigest, SecondaryPreDigest, + CompatibleDigestItem, NextEpochDescriptor, NextConfigDescriptor, + PreDigest, PrimaryPreDigest, SecondaryPreDigest, }, }; pub use sp_consensus::SyncOracle; @@ -118,36 +120,43 @@ use sp_api::ApiExt; mod aux_schema; mod verification; +mod migration; pub mod authorship; #[cfg(test)] mod tests; /// BABE epoch information -#[derive(Decode, Encode, Default, PartialEq, Eq, Clone, Debug)] +#[derive(Decode, Encode, PartialEq, Eq, Clone, Debug)] pub struct Epoch { - /// The epoch index + /// The epoch index. pub epoch_index: u64, - /// The starting slot of the epoch, + /// The starting slot of the epoch. pub start_slot: SlotNumber, - /// The duration of this epoch + /// The duration of this epoch. pub duration: SlotNumber, - /// The authorities and their weights + /// The authorities and their weights. pub authorities: Vec<(AuthorityId, BabeAuthorityWeight)>, - /// Randomness for this epoch + /// Randomness for this epoch. pub randomness: [u8; VRF_OUTPUT_LENGTH], + /// Configuration of the epoch. + pub config: BabeEpochConfiguration, } impl EpochT for Epoch { - type NextEpochDescriptor = NextEpochDescriptor; + type NextEpochDescriptor = (NextEpochDescriptor, BabeEpochConfiguration); type SlotNumber = SlotNumber; - fn increment(&self, descriptor: NextEpochDescriptor) -> Epoch { + fn increment( + &self, + (descriptor, config): (NextEpochDescriptor, BabeEpochConfiguration) + ) -> Epoch { Epoch { epoch_index: self.epoch_index + 1, start_slot: self.start_slot + self.duration, duration: self.duration, authorities: descriptor.authorities, randomness: descriptor.randomness, + config, } } @@ -160,6 +169,27 @@ impl EpochT 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_number: SlotNumber + ) -> Epoch { + Epoch { + epoch_index: 0, + start_slot: slot_number, + duration: genesis_config.epoch_length, + authorities: genesis_config.genesis_authorities.clone(), + randomness: genesis_config.randomness.clone(), + config: BabeEpochConfiguration { + c: genesis_config.c, + secondary_slots: genesis_config.secondary_slots, + }, + } + } +} + #[derive(derive_more::Display, Debug)] enum Error<B: BlockT> { #[display(fmt = "Multiple BABE pre-runtime digests, rejecting!")] @@ -168,6 +198,8 @@ enum Error<B: BlockT> { NoPreRuntimeDigest, #[display(fmt = "Multiple BABE epoch change digests, rejecting!")] MultipleEpochChangeDigests, + #[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)] @@ -200,6 +232,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)] @@ -236,7 +270,7 @@ pub static INTERMEDIATE_KEY: &[u8] = b"babe1"; // and `super::babe::Config` can be eliminated. // https://github.com/paritytech/substrate/issues/2434 #[derive(Clone)] -pub struct Config(sc_consensus_slots::SlotDuration<BabeConfiguration>); +pub struct Config(sc_consensus_slots::SlotDuration<BabeGenesisConfiguration>); impl Config { /// Either fetch the slot duration from disk or compute it from the genesis @@ -253,24 +287,12 @@ impl Config { } } } - - /// 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_epoch(&self, slot_number: SlotNumber) -> Epoch { - Epoch { - epoch_index: 0, - start_slot: slot_number, - duration: self.epoch_length, - authorities: self.genesis_authorities.clone(), - randomness: self.randomness.clone(), - } - } } impl std::ops::Deref for Config { - type Target = BabeConfiguration; + type Target = BabeGenesisConfiguration; - fn deref(&self) -> &BabeConfiguration { + fn deref(&self) -> &BabeGenesisConfiguration { &*self.0 } } @@ -428,7 +450,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| self.config.genesis_epoch(slot)) + .viable_epoch(&epoch_descriptor, |slot| Epoch::genesis(&self.config, slot)) .map(|epoch| epoch.as_ref().authorities.len()) } @@ -443,9 +465,8 @@ 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| self.config.genesis_epoch(slot) + |slot| Epoch::genesis(&self.config, slot) )?.as_ref(), - &*self.config, &self.keystore, ); @@ -599,6 +620,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)>)>>); @@ -726,7 +765,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| self.config.genesis_epoch(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. @@ -736,7 +775,6 @@ impl<Block, Client> Verifier<Block> for BabeVerifier<Block, Client> where pre_digest: Some(pre_digest.clone()), slot_now: slot_now + 1, epoch: viable_epoch.as_ref(), - config: &self.config, }; match verification::check_header::<Block>(v_params)? { @@ -958,19 +996,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(), + ) + ) }, } @@ -985,11 +1036,15 @@ impl<Block, Client, Inner> BlockImport<Block> for BabeBlockImport<Block, Client, let viable_epoch = epoch_changes.viable_epoch( &epoch_descriptor, - |slot| self.config.genesis_epoch(slot), + |slot| Epoch::genesis(&self.config, slot) ).ok_or_else(|| { ConsensusError::ClientImport(Error::<Block>::FetchEpoch(parent_hash).into()) })?; + let epoch_config = next_config_digest.unwrap_or_else( + || viable_epoch.as_ref().config.clone() + ); + // restrict info logging during initial sync to avoid spam let log_level = if block.origin == BlockOrigin::NetworkInitialSync { log::Level::Debug @@ -1006,7 +1061,7 @@ impl<Block, Client, Inner> BlockImport<Block> for BabeBlockImport<Block, Client, viable_epoch.as_ref().start_slot, ); - let next_epoch = viable_epoch.increment(next_epoch_descriptor); + let next_epoch = viable_epoch.increment((next_epoch_descriptor, epoch_config)); log!(target: "babe", log_level, @@ -1152,7 +1207,7 @@ pub fn block_import<Client, Block: BlockT, I>( ) -> ClientResult<(BabeBlockImport<Block, Client, I>, BabeLink<Block>)> where Client: AuxStore + HeaderBackend<Block> + HeaderMetadata<Block, Error = sp_blockchain::Error>, { - let epoch_changes = aux_schema::load_epoch_changes::<Block, _>(&*client)?; + let epoch_changes = aux_schema::load_epoch_changes::<Block, _>(&*client, &config)?; let link = BabeLink { epoch_changes: epoch_changes.clone(), time_source: Default::default(), @@ -1245,13 +1300,12 @@ pub mod test_helpers { &parent.hash(), parent.number().clone(), slot_number, - |slot| link.config.genesis_epoch(slot), + |slot| Epoch::genesis(&link.config, slot), ).unwrap().unwrap(); authorship::claim_slot( slot_number, &epoch, - &link.config, keystore, ).map(|(digest, _)| digest) } diff --git a/substrate/client/consensus/babe/src/migration.rs b/substrate/client/consensus/babe/src/migration.rs new file mode 100644 index 00000000000..837704abb1b --- /dev/null +++ b/substrate/client/consensus/babe/src/migration.rs @@ -0,0 +1,64 @@ +use codec::{Encode, Decode}; +use sc_consensus_epochs::Epoch as EpochT; +use crate::{ + Epoch, SlotNumber, AuthorityId, BabeAuthorityWeight, BabeGenesisConfiguration, + BabeEpochConfiguration, VRF_OUTPUT_LENGTH, NextEpochDescriptor, +}; + +/// BABE epoch information, version 0. +#[derive(Decode, Encode, PartialEq, Eq, Clone, Debug)] +pub struct EpochV0 { + /// 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 EpochV0 { + type NextEpochDescriptor = NextEpochDescriptor; + type SlotNumber = SlotNumber; + + fn increment( + &self, + descriptor: NextEpochDescriptor + ) -> EpochV0 { + EpochV0 { + 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 + } +} + +impl EpochV0 { + /// Migrate the sturct to current epoch version. + pub fn migrate(self, config: &BabeGenesisConfiguration) -> Epoch { + Epoch { + epoch_index: self.epoch_index, + start_slot: self.start_slot, + duration: self.duration, + authorities: self.authorities, + randomness: self.randomness, + config: BabeEpochConfiguration { + c: config.c, + secondary_slots: config.secondary_slots, + }, + } + } +} diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index 20b924669d6..3c433b40305 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -127,7 +127,7 @@ impl DummyProposer { &self.parent_hash, self.parent_number, this_slot, - |slot| self.factory.config.genesis_epoch(slot), + |slot| Epoch::genesis(&self.factory.config, slot), ) .expect("client has data to find epoch") .expect("can compute epoch for baked block"); @@ -505,9 +505,13 @@ fn can_author_block() { randomness: [0; 32], epoch_index: 1, duration: 100, + config: BabeEpochConfiguration { + c: (3, 10), + secondary_slots: true, + }, }; - let mut config = crate::BabeConfiguration { + let mut config = crate::BabeGenesisConfiguration { slot_duration: 1000, epoch_length: 100, c: (3, 10), @@ -517,7 +521,7 @@ fn can_author_block() { }; // with secondary slots enabled it should never be empty - match claim_slot(i, &epoch, &config, &keystore) { + match claim_slot(i, &epoch, &keystore) { None => i += 1, Some(s) => debug!(target: "babe", "Authored block {:?}", s.0), } @@ -526,7 +530,7 @@ fn can_author_block() { // of times. config.secondary_slots = false; loop { - match claim_slot(i, &epoch, &config, &keystore) { + match claim_slot(i, &epoch, &keystore) { None => i += 1, Some(s) => { debug!(target: "babe", "Authored block {:?}", s.0); @@ -632,7 +636,7 @@ fn importing_block_one_sets_genesis_epoch() { &mut block_import, ); - let genesis_epoch = data.link.config.genesis_epoch(999); + let genesis_epoch = Epoch::genesis(&data.link.config, 999); let epoch_changes = data.link.epoch_changes.lock(); let epoch_for_second_block = epoch_changes.epoch_data_for_child_of( @@ -640,7 +644,7 @@ fn importing_block_one_sets_genesis_epoch() { &block_hash, 1, 1000, - |slot| data.link.config.genesis_epoch(slot), + |slot| Epoch::genesis(&data.link.config, slot), ).unwrap().unwrap(); assert_eq!(epoch_for_second_block, genesis_epoch); diff --git a/substrate/client/consensus/babe/src/verification.rs b/substrate/client/consensus/babe/src/verification.rs index 2fd37280b3b..1a3eba45843 100644 --- a/substrate/client/consensus/babe/src/verification.rs +++ b/substrate/client/consensus/babe/src/verification.rs @@ -38,8 +38,6 @@ pub(super) struct VerificationParams<'a, B: 'a + BlockT> { pub(super) slot_now: SlotNumber, /// epoch descriptor of the epoch this block _should_ be under, if it's valid. pub(super) epoch: &'a Epoch, - /// genesis config of this BABE chain. - pub(super) config: &'a super::Config, } /// Check a header has been signed by the right key. If the slot is too far in @@ -63,7 +61,6 @@ pub(super) fn check_header<B: BlockT + Sized>( pre_digest, slot_now, epoch, - config, } = params; let authorities = &epoch.authorities; @@ -102,10 +99,10 @@ pub(super) fn check_header<B: BlockT + Sized>( primary, sig, &epoch, - config.c, + epoch.config.c, )?; }, - PreDigest::Secondary(secondary) if config.secondary_slots => { + PreDigest::Secondary(secondary) if epoch.config.secondary_slots => { debug!(target: "babe", "Verifying Secondary block"); check_secondary_header::<B>( diff --git a/substrate/client/consensus/epochs/src/lib.rs b/substrate/client/consensus/epochs/src/lib.rs index 001c172b349..acb07dd668a 100644 --- a/substrate/client/consensus/epochs/src/lib.rs +++ b/substrate/client/consensus/epochs/src/lib.rs @@ -335,6 +335,55 @@ impl<Hash, Number, E: Epoch> EpochChanges<Hash, Number, E> where self.inner.rebalance() } + /// Map the epoch changes from one storing data to a different one. + pub fn map<B, F>(self, mut f: F) -> EpochChanges<Hash, Number, B> where + B: Epoch<SlotNumber=E::SlotNumber>, + F: FnMut(&Hash, &Number, E) -> B, + { + EpochChanges { + inner: self.inner.map(&mut |_, _, header| { + match header { + PersistedEpochHeader::Genesis(epoch_0, epoch_1) => { + PersistedEpochHeader::Genesis( + EpochHeader { + start_slot: epoch_0.start_slot, + end_slot: epoch_0.end_slot, + }, + EpochHeader { + start_slot: epoch_1.start_slot, + end_slot: epoch_1.end_slot, + }, + ) + }, + PersistedEpochHeader::Regular(epoch_n) => { + PersistedEpochHeader::Regular( + EpochHeader { + start_slot: epoch_n.start_slot, + end_slot: epoch_n.end_slot, + }, + ) + }, + } + }), + epochs: self.epochs.into_iter().map(|((hash, number), epoch)| { + let bepoch = match epoch { + PersistedEpoch::Genesis(epoch_0, epoch_1) => { + PersistedEpoch::Genesis( + f(&hash, &number, epoch_0), + f(&hash, &number, epoch_1), + ) + }, + PersistedEpoch::Regular(epoch_n) => { + PersistedEpoch::Regular( + f(&hash, &number, epoch_n) + ) + }, + }; + ((hash, number), bepoch) + }).collect(), + } + } + /// Prune out finalized epochs, except for the ancestor of the finalized /// block. The given slot should be the slot number at which the finalized /// block was authored. diff --git a/substrate/primitives/consensus/babe/src/digests.rs b/substrate/primitives/consensus/babe/src/digests.rs index 6079aa88c87..291b32cf499 100644 --- a/substrate/primitives/consensus/babe/src/digests.rs +++ b/substrate/primitives/consensus/babe/src/digests.rs @@ -18,7 +18,7 @@ #[cfg(feature = "std")] use super::{BABE_ENGINE_ID, AuthoritySignature}; -use super::{AuthorityId, AuthorityIndex, SlotNumber, BabeAuthorityWeight}; +use super::{AuthorityId, AuthorityIndex, SlotNumber, BabeAuthorityWeight, BabeEpochConfiguration}; #[cfg(feature = "std")] use sp_runtime::{DigestItem, generic::OpaqueDigestItemId}; #[cfg(feature = "std")] @@ -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, + }) + } } diff --git a/substrate/primitives/consensus/babe/src/lib.rs b/substrate/primitives/consensus/babe/src/lib.rs index 33701860d1f..37109b69be7 100644 --- a/substrate/primitives/consensus/babe/src/lib.rs +++ b/substrate/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,11 +87,15 @@ 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), } /// Configuration data used by the BABE consensus engine. #[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] -pub struct BabeConfiguration { +pub struct BabeGenesisConfiguration { /// The slot duration in milliseconds for BABE. Currently, only /// the value provided by this type at genesis will be used. /// @@ -121,7 +125,7 @@ pub struct BabeConfiguration { } #[cfg(feature = "std")] -impl sp_consensus::SlotData for BabeConfiguration { +impl sp_consensus::SlotData for BabeGenesisConfiguration { fn slot_duration(&self) -> u64 { self.slot_duration } @@ -129,14 +133,27 @@ impl sp_consensus::SlotData for BabeConfiguration { const SLOT_KEY: &'static [u8] = b"babe_configuration"; } +/// Configuration data used by the BABE consensus engine. +#[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] +pub struct BabeEpochConfiguration { + /// 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), + + /// Whether this chain should run with secondary slots, which are assigned + /// in round-robin manner. + pub secondary_slots: bool, +} + sp_api::decl_runtime_apis! { /// API necessary for block authorship with BABE. pub trait BabeApi { - /// Return the configuration for BABE. Currently, - /// only the value provided by this type at genesis will be used. - /// - /// Dynamic configuration may be supported in the future. - fn configuration() -> BabeConfiguration; + /// Return the genesis configuration for BABE. The configuration is only read on genesis. + fn configuration() -> BabeGenesisConfiguration; /// Returns the slot number that started the current epoch. fn current_epoch_start() -> SlotNumber; diff --git a/substrate/test-utils/runtime/src/lib.rs b/substrate/test-utils/runtime/src/lib.rs index cfafb76521a..c5c438e4de5 100644 --- a/substrate/test-utils/runtime/src/lib.rs +++ b/substrate/test-utils/runtime/src/lib.rs @@ -633,8 +633,8 @@ cfg_if! { } impl sp_consensus_babe::BabeApi<Block> for Runtime { - fn configuration() -> sp_consensus_babe::BabeConfiguration { - sp_consensus_babe::BabeConfiguration { + fn configuration() -> sp_consensus_babe::BabeGenesisConfiguration { + sp_consensus_babe::BabeGenesisConfiguration { slot_duration: 1000, epoch_length: EpochDuration::get(), c: (3, 10), @@ -827,8 +827,8 @@ cfg_if! { } impl sp_consensus_babe::BabeApi<Block> for Runtime { - fn configuration() -> sp_consensus_babe::BabeConfiguration { - sp_consensus_babe::BabeConfiguration { + fn configuration() -> sp_consensus_babe::BabeGenesisConfiguration { + sp_consensus_babe::BabeGenesisConfiguration { slot_duration: 1000, epoch_length: EpochDuration::get(), c: (3, 10), -- GitLab