babe: support online configuration upgrades (#5514)

* 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>
This commit is contained in:
Wei Tang
2020-04-24 15:59:14 +02:00
committed by GitHub
parent a01e608dff
commit 770cc24c47
12 changed files with 300 additions and 86 deletions
@@ -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,
@@ -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),
);
}
}
+96 -42
View File
@@ -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)
}
@@ -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,
},
}
}
}
+10 -6
View File
@@ -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);
@@ -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>(