diff --git a/Cargo.lock b/Cargo.lock index b42b637bcf396..742dd0ed85b51 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8539,6 +8539,7 @@ version = "0.9.0" dependencies = [ "merlin", "parity-scale-codec", + "serde", "sp-api", "sp-application-crypto", "sp-consensus", diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 3b40dde377218..43383bb3c3a96 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -305,6 +305,7 @@ pub fn testnet_genesis( }, pallet_babe: BabeConfig { authorities: vec![], + epoch_config: Some(node_runtime::BABE_GENESIS_EPOCH_CONFIG), }, pallet_im_online: ImOnlineConfig { keys: vec![], diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index b032fcb174e79..55f42b5723ba8 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -119,6 +119,13 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { transaction_version: 2, }; +/// The BABE epoch configuration at genesis. +pub const BABE_GENESIS_EPOCH_CONFIG: sp_consensus_babe::BabeEpochConfiguration = + sp_consensus_babe::BabeEpochConfiguration { + c: PRIMARY_PROBABILITY, + allowed_slots: sp_consensus_babe::AllowedSlots::PrimaryAndSecondaryPlainSlots + }; + /// Native version. #[cfg(any(feature = "std", test))] pub fn native_version() -> NativeVersion { @@ -1274,10 +1281,10 @@ impl_runtime_apis! { sp_consensus_babe::BabeGenesisConfiguration { slot_duration: Babe::slot_duration(), epoch_length: EpochDuration::get(), - c: PRIMARY_PROBABILITY, + c: BABE_GENESIS_EPOCH_CONFIG.c, genesis_authorities: Babe::authorities(), randomness: Babe::randomness(), - allowed_slots: sp_consensus_babe::AllowedSlots::PrimaryAndSecondaryPlainSlots, + allowed_slots: BABE_GENESIS_EPOCH_CONFIG.allowed_slots, } } diff --git a/bin/node/testing/src/genesis.rs b/bin/node/testing/src/genesis.rs index 22187f404cfea..25b728ebe193e 100644 --- a/bin/node/testing/src/genesis.rs +++ b/bin/node/testing/src/genesis.rs @@ -23,7 +23,7 @@ use sp_keyring::{Ed25519Keyring, Sr25519Keyring}; use node_runtime::{ GenesisConfig, BalancesConfig, SessionConfig, StakingConfig, SystemConfig, GrandpaConfig, IndicesConfig, ContractsConfig, SocietyConfig, wasm_binary_unwrap, - AccountId, StakerStatus, + AccountId, StakerStatus, BabeConfig, BABE_GENESIS_EPOCH_CONFIG, }; use node_runtime::constants::currency::*; use sp_core::ChangesTrieConfiguration; @@ -100,7 +100,10 @@ pub fn config_endowed( pallet_contracts: ContractsConfig { current_schedule: Default::default(), }, - pallet_babe: Default::default(), + pallet_babe: BabeConfig { + authorities: vec![], + epoch_config: Some(BABE_GENESIS_EPOCH_CONFIG), + }, pallet_grandpa: GrandpaConfig { authorities: vec![], }, diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 7f2f47da5d5de..861f82c0090a5 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -517,6 +517,7 @@ async fn answer_requests( duration: viable_epoch.as_ref().duration, authorities: viable_epoch.as_ref().authorities.clone(), randomness: viable_epoch.as_ref().randomness, + config: viable_epoch.as_ref().config.clone(), }) }; diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index 9fdb080574b79..29c815444a3a9 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -42,7 +42,8 @@ use sp_timestamp::OnTimestampSet; use sp_consensus_babe::{ digests::{NextConfigDescriptor, NextEpochDescriptor, PreDigest}, - BabeAuthorityWeight, ConsensusLog, Epoch, EquivocationProof, Slot, BABE_ENGINE_ID, + BabeAuthorityWeight, BabeEpochConfiguration, ConsensusLog, Epoch, + EquivocationProof, Slot, BABE_ENGINE_ID, }; use sp_consensus_vrf::schnorrkel; @@ -187,8 +188,8 @@ decl_storage! { // variable to its underlying value. pub Randomness get(fn randomness): schnorrkel::Randomness; - /// Next epoch configuration, if changed. - NextEpochConfig: Option; + /// Pending epoch configuration change that will be applied when the next epoch is enacted. + PendingEpochConfigChange: Option; /// Next epoch randomness. NextRandomness: schnorrkel::Randomness; @@ -225,10 +226,21 @@ decl_storage! { /// on block finalization. Querying this storage entry outside of block /// execution context should always yield zero. Lateness get(fn lateness): T::BlockNumber; + + /// The configuration for the current epoch. Should never be `None` as it is initialized in genesis. + EpochConfig: Option; + + /// The configuration for the next epoch, `None` if the config will not change + /// (you can fallback to `EpochConfig` instead in that case). + NextEpochConfig: Option; } add_extra_genesis { config(authorities): Vec<(AuthorityId, BabeAuthorityWeight)>; - build(|config| Module::::initialize_authorities(&config.authorities)) + config(epoch_config): Option; + build(|config| { + Module::::initialize_authorities(&config.authorities); + EpochConfig::put(config.epoch_config.clone().expect("epoch_config must not be None")); + }) } } @@ -326,7 +338,7 @@ decl_module! { config: NextConfigDescriptor, ) { ensure_root(origin)?; - NextEpochConfig::put(config); + PendingEpochConfigChange::put(config); } } } @@ -490,8 +502,16 @@ impl Module { }; Self::deposit_consensus(ConsensusLog::NextEpochData(next_epoch)); - if let Some(next_config) = NextEpochConfig::take() { - Self::deposit_consensus(ConsensusLog::NextConfigData(next_config)); + if let Some(next_config) = NextEpochConfig::get() { + EpochConfig::put(next_config); + } + + if let Some(pending_epoch_config_change) = PendingEpochConfigChange::take() { + let next_epoch_config: BabeEpochConfiguration = + pending_epoch_config_change.clone().into(); + NextEpochConfig::put(next_epoch_config); + + Self::deposit_consensus(ConsensusLog::NextConfigData(pending_epoch_config_change)); } } @@ -510,6 +530,7 @@ impl Module { duration: T::EpochDuration::get(), authorities: Self::authorities(), randomness: Self::randomness(), + config: EpochConfig::get().expect("EpochConfig is initialized in genesis; we never `take` or `kill` it; qed"), } } @@ -527,6 +548,9 @@ impl Module { duration: T::EpochDuration::get(), authorities: NextAuthorities::get(), randomness: NextRandomness::get(), + config: NextEpochConfig::get().unwrap_or_else(|| { + EpochConfig::get().expect("EpochConfig is initialized in genesis; we never `take` or `kill` it; qed") + }), } } @@ -835,3 +859,49 @@ fn compute_randomness( sp_io::hashing::blake2_256(&s) } + +pub mod migrations { + use super::*; + use frame_support::pallet_prelude::{ValueQuery, StorageValue}; + + /// Something that can return the storage prefix of the `Babe` pallet. + pub trait BabePalletPrefix: Config { + fn pallet_prefix() -> &'static str; + } + + struct __OldNextEpochConfig(sp_std::marker::PhantomData); + impl frame_support::traits::StorageInstance for __OldNextEpochConfig { + fn pallet_prefix() -> &'static str { T::pallet_prefix() } + const STORAGE_PREFIX: &'static str = "NextEpochConfig"; + } + + type OldNextEpochConfig = StorageValue< + __OldNextEpochConfig, Option, ValueQuery + >; + + /// A storage migration that adds the current epoch configuration for Babe + /// to storage. + pub fn add_epoch_configuration( + epoch_config: BabeEpochConfiguration, + ) -> Weight { + let mut writes = 0; + let mut reads = 0; + + if let Some(pending_change) = OldNextEpochConfig::::get() { + PendingEpochConfigChange::put(pending_change); + + writes += 1; + } + + reads += 1; + + OldNextEpochConfig::::kill(); + + EpochConfig::put(epoch_config.clone()); + NextEpochConfig::put(epoch_config); + + writes += 3; + + T::DbWeight::get().writes(writes) + T::DbWeight::get().reads(reads) + } +} diff --git a/frame/babe/src/tests.rs b/frame/babe/src/tests.rs index c7261d7f1f963..6515e5fdaaf99 100644 --- a/frame/babe/src/tests.rs +++ b/frame/babe/src/tests.rs @@ -25,7 +25,7 @@ use frame_support::{ }; use mock::*; use pallet_session::ShouldEndSession; -use sp_consensus_babe::{AllowedSlots, Slot}; +use sp_consensus_babe::{AllowedSlots, Slot, BabeEpochConfiguration}; use sp_core::crypto::Pair; const EMPTY_RANDOMNESS: [u8; 32] = [ @@ -231,11 +231,31 @@ fn can_enact_next_config() { assert_eq!(Babe::epoch_index(), 0); go_to_block(2, 7); + let current_config = BabeEpochConfiguration { + c: (0, 4), + allowed_slots: sp_consensus_babe::AllowedSlots::PrimarySlots, + }; + + let next_config = BabeEpochConfiguration { + c: (1, 4), + allowed_slots: sp_consensus_babe::AllowedSlots::PrimarySlots, + }; + + let next_next_config = BabeEpochConfiguration { + c: (2, 4), + allowed_slots: sp_consensus_babe::AllowedSlots::PrimarySlots, + }; + + EpochConfig::put(current_config); + NextEpochConfig::put(next_config.clone()); + + assert_eq!(NextEpochConfig::get(), Some(next_config.clone())); + Babe::plan_config_change( Origin::root(), NextConfigDescriptor::V1 { - c: (1, 4), - allowed_slots: AllowedSlots::PrimarySlots, + c: next_next_config.c, + allowed_slots: next_next_config.allowed_slots, }, ).unwrap(); @@ -243,10 +263,13 @@ fn can_enact_next_config() { Babe::on_finalize(9); let header = System::finalize(); + assert_eq!(EpochConfig::get(), Some(next_config)); + assert_eq!(NextEpochConfig::get(), Some(next_next_config.clone())); + let consensus_log = sp_consensus_babe::ConsensusLog::NextConfigData( - sp_consensus_babe::digests::NextConfigDescriptor::V1 { - c: (1, 4), - allowed_slots: AllowedSlots::PrimarySlots, + NextConfigDescriptor::V1 { + c: next_next_config.c, + allowed_slots: next_next_config.allowed_slots, } ); let consensus_digest = DigestItem::Consensus(BABE_ENGINE_ID, consensus_log.encode()); @@ -291,6 +314,11 @@ fn only_root_can_enact_config_change() { #[test] fn can_fetch_current_and_next_epoch_data() { new_test_ext(5).execute_with(|| { + EpochConfig::put(BabeEpochConfiguration { + c: (1, 4), + allowed_slots: sp_consensus_babe::AllowedSlots::PrimarySlots, + }); + // genesis authorities should be used for the first and second epoch assert_eq!( Babe::current_epoch().authorities, @@ -809,3 +837,54 @@ fn valid_equivocation_reports_dont_pay_fees() { assert_eq!(post_info.pays_fee, Pays::Yes); }) } + +#[test] +fn add_epoch_configurations_migration_works() { + use frame_support::storage::migration::{ + put_storage_value, get_storage_value, + }; + + impl crate::migrations::BabePalletPrefix for Test { + fn pallet_prefix() -> &'static str { + "Babe" + } + } + + new_test_ext(1).execute_with(|| { + let next_config_descriptor = NextConfigDescriptor::V1 { + c: (3, 4), + allowed_slots: AllowedSlots::PrimarySlots + }; + + put_storage_value( + b"Babe", + b"NextEpochConfig", + &[], + Some(next_config_descriptor.clone()) + ); + + assert!(get_storage_value::>( + b"Babe", + b"NextEpochConfig", + &[], + ).is_some()); + + let current_epoch = BabeEpochConfiguration { + c: (1, 4), + allowed_slots: sp_consensus_babe::AllowedSlots::PrimarySlots, + }; + + crate::migrations::add_epoch_configuration::( + current_epoch.clone() + ); + + assert!(get_storage_value::>( + b"Babe", + b"NextEpochConfig", + &[], + ).is_none()); + + assert_eq!(EpochConfig::get(), Some(current_epoch)); + assert_eq!(PendingEpochConfigChange::get(), Some(next_config_descriptor)); + }); +} diff --git a/primitives/consensus/babe/Cargo.toml b/primitives/consensus/babe/Cargo.toml index fb02014eeef51..a8ab03dcdaa49 100644 --- a/primitives/consensus/babe/Cargo.toml +++ b/primitives/consensus/babe/Cargo.toml @@ -26,6 +26,7 @@ sp-inherents = { version = "3.0.0", default-features = false, path = "../../inhe sp-keystore = { version = "0.9.0", default-features = false, path = "../../keystore", optional = true } sp-runtime = { version = "3.0.0", default-features = false, path = "../../runtime" } sp-timestamp = { version = "3.0.0", default-features = false, path = "../../timestamp" } +serde = { version = "1.0.123", features = ["derive"], optional = true } [features] default = ["std"] @@ -43,4 +44,5 @@ std = [ "sp-keystore", "sp-runtime/std", "sp-timestamp/std", + "serde", ] diff --git a/primitives/consensus/babe/src/lib.rs b/primitives/consensus/babe/src/lib.rs index 6987796c114a0..1b416c996fcf0 100644 --- a/primitives/consensus/babe/src/lib.rs +++ b/primitives/consensus/babe/src/lib.rs @@ -30,6 +30,8 @@ pub use sp_consensus_vrf::schnorrkel::{ use codec::{Decode, Encode}; #[cfg(feature = "std")] +use serde::{Serialize, Deserialize}; +#[cfg(feature = "std")] use sp_keystore::vrf::{VRFTranscriptData, VRFTranscriptValue}; use sp_runtime::{traits::Header, ConsensusEngineId, RuntimeDebug}; use sp_std::vec::Vec; @@ -216,6 +218,7 @@ pub struct BabeGenesisConfiguration { /// Types of allowed slots. #[derive(Clone, Copy, PartialEq, Eq, Encode, Decode, RuntimeDebug)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub enum AllowedSlots { /// Only allow primary slots. PrimarySlots, @@ -248,6 +251,7 @@ impl sp_consensus::SlotData for BabeGenesisConfiguration { /// Configuration data used by the BABE consensus engine. #[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] 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 @@ -362,6 +366,8 @@ pub struct Epoch { pub authorities: Vec<(AuthorityId, BabeAuthorityWeight)>, /// Randomness for this epoch. pub randomness: [u8; VRF_OUTPUT_LENGTH], + /// Configuration of the epoch. + pub config: BabeEpochConfiguration, } sp_api::decl_runtime_apis! {