Unverified Commit e4370aa5 authored by Bastian Köcher's avatar Bastian Köcher Committed by GitHub
Browse files

Adds consistency checks for the `HostConfiguration` (#2087)



* Adds consistency checks for the `HostConfiguration`

Besides that it fixes the chain specs to make the consistency checks happy.

* Update runtime/parachains/src/configuration.rs

Co-authored-by: Shawn Tabrizi's avatarShawn Tabrizi <shawntabrizi@gmail.com>

* Review feedback and test fixes etc

* Update

* More

* I'm an idiot

* Fix tests...

Co-authored-by: Shawn Tabrizi's avatarShawn Tabrizi <shawntabrizi@gmail.com>
parent 20f3c411
Pipeline #116458 passed with stages
in 23 minutes and 1 second
......@@ -779,7 +779,10 @@ fn rococo_staging_testnet_config_genesis(wasm_binary: &[u8]) -> rococo_runtime::
max_code_size: 5 * 1024 * 1024,
max_pov_size: 50 * 1024 * 1024,
max_head_data_size: 32 * 1024,
group_rotation_frequency: 10,
group_rotation_frequency: 20,
chain_availability_period: 4,
thread_availability_period: 4,
no_show_slots: 10,
..Default::default()
},
}),
......@@ -1232,7 +1235,10 @@ pub fn rococo_testnet_genesis(
max_code_size: 5 * 1024 * 1024,
max_pov_size: 50 * 1024 * 1024,
max_head_data_size: 32 * 1024,
group_rotation_frequency: 10,
group_rotation_frequency: 20,
chain_availability_period: 4,
thread_availability_period: 4,
no_show_slots: 10,
max_upward_queue_count: 8,
max_upward_queue_size: 8 * 1024,
max_downward_message_size: 1024,
......
......@@ -169,7 +169,10 @@ fn polkadot_testnet_genesis(
max_code_size: 5 * 1024 * 1024,
max_pov_size: 50 * 1024 * 1024,
max_head_data_size: 32 * 1024,
group_rotation_frequency: 10,
group_rotation_frequency: 20,
chain_availability_period: 4,
thread_availability_period: 4,
no_show_slots: 10,
..Default::default()
},
}),
......
......@@ -24,7 +24,7 @@ use bitvec::vec::BitVec;
use primitives::RuntimeDebug;
use runtime_primitives::traits::AppVerify;
use inherents::InherentIdentifier;
use sp_arithmetic::traits::{BaseArithmetic, Saturating, Zero};
use sp_arithmetic::traits::{BaseArithmetic, Saturating};
use application_crypto::KeyTypeId;
pub use runtime_primitives::traits::{BlakeTwo256, Hash as HashT};
......@@ -573,11 +573,7 @@ impl GroupRotationInfo {
impl<N: Saturating + BaseArithmetic + Copy> GroupRotationInfo<N> {
/// Returns the block number of the next rotation after the current block. If the current block
/// is 10 and the rotation frequency is 5, this should return 15.
///
/// If the group rotation frequency is 0, returns 0.
pub fn next_rotation_at(&self) -> N {
if self.group_rotation_frequency.is_zero() { return Zero::zero() }
let cycle_once = self.now + self.group_rotation_frequency;
cycle_once - (
cycle_once.saturating_sub(self.session_start_block) % self.group_rotation_frequency
......@@ -586,10 +582,7 @@ impl<N: Saturating + BaseArithmetic + Copy> GroupRotationInfo<N> {
/// Returns the block number of the last rotation before or including the current block. If the
/// current block is 10 and the rotation frequency is 5, this should return 10.
///
/// If the group rotation frequency is 0, returns 0.
pub fn last_rotation_at(&self) -> N {
if self.group_rotation_frequency.is_zero() { return Zero::zero() }
self.now - (
self.now.saturating_sub(self.session_start_block) % self.group_rotation_frequency
)
......@@ -837,15 +830,6 @@ mod tests {
assert_eq!(info.next_rotation_at(), 20);
assert_eq!(info.last_rotation_at(), 15);
let info = GroupRotationInfo {
session_start_block: 10u32,
now: 11,
group_rotation_frequency: 0,
};
assert_eq!(info.next_rotation_at(), 0);
assert_eq!(info.last_rotation_at(), 0);
}
#[test]
......
......@@ -17,14 +17,10 @@ impl GroupRotationInfo {
/// Returns the block number of the next rotation after the current block. If the current block
/// is 10 and the rotation frequency is 5, this should return 15.
///
/// If the group rotation frequency is 0, returns 0.
fn next_rotation_at(&self) -> BlockNumber;
/// Returns the block number of the last rotation before or including the current block. If the
/// current block is 10 and the rotation frequency is 5, this should return 10.
///
/// If the group rotation frequency is 0, returns 0.
fn last_rotation_at(&self) -> BlockNumber;
}
......
......@@ -28,9 +28,10 @@ use frame_support::{
};
use parity_scale_codec::{Encode, Decode};
use frame_system::ensure_root;
use sp_runtime::traits::Zero;
/// All configuration of the runtime with respect to parachains and parathreads.
#[derive(Clone, Encode, Decode, PartialEq, Default, sp_core::RuntimeDebug)]
#[derive(Clone, Encode, Decode, PartialEq, sp_core::RuntimeDebug)]
#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
pub struct HostConfiguration<BlockNumber> {
/// The minimum frequency at which parachains can update their validation code.
......@@ -50,23 +51,32 @@ pub struct HostConfiguration<BlockNumber> {
pub parathread_cores: u32,
/// The number of retries that a parathread author has to submit their block.
pub parathread_retries: u32,
/// How often parachain groups should be rotated across parachains. Must be non-zero.
/// How often parachain groups should be rotated across parachains.
///
/// Must be non-zero.
pub group_rotation_frequency: BlockNumber,
/// The availability period, in blocks, for parachains. This is the amount of blocks
/// after inclusion that validators have to make the block available and signal its availability to
/// the chain. Must be at least 1.
/// the chain.
///
/// Must be at least 1.
pub chain_availability_period: BlockNumber,
/// The availability period, in blocks, for parathreads. Same as the `chain_availability_period`,
/// but a differing timeout due to differing requirements. Must be at least 1.
/// but a differing timeout due to differing requirements.
///
/// Must be at least 1.
pub thread_availability_period: BlockNumber,
/// The amount of blocks ahead to schedule parachains and parathreads.
pub scheduling_lookahead: u32,
/// The maximum number of validators to have per core. `None` means no maximum.
/// The maximum number of validators to have per core.
///
/// `None` means no maximum.
pub max_validators_per_core: Option<u32>,
/// The amount of sessions to keep for disputes.
pub dispute_period: SessionIndex,
/// The amount of consensus slots that must pass between submitting an assignment and
/// submitting an approval vote before a validator is considered a no-show.
///
/// Must be at least 1.
pub no_show_slots: u32,
/// The number of delay tranches in total.
......@@ -132,6 +142,74 @@ pub struct HostConfiguration<BlockNumber> {
pub hrmp_max_message_num_per_candidate: u32,
}
impl<BlockNumber: Default + From<u32>> Default for HostConfiguration<BlockNumber> {
fn default() -> Self {
Self {
group_rotation_frequency: 1u32.into(),
chain_availability_period: 1u32.into(),
thread_availability_period: 1u32.into(),
no_show_slots: 1u32.into(),
validation_upgrade_frequency: Default::default(),
validation_upgrade_delay: Default::default(),
acceptance_period: Default::default(),
max_code_size: Default::default(),
max_pov_size: Default::default(),
max_head_data_size: Default::default(),
parathread_cores: Default::default(),
parathread_retries: Default::default(),
scheduling_lookahead: Default::default(),
max_validators_per_core: Default::default(),
dispute_period: Default::default(),
n_delay_tranches: Default::default(),
zeroth_delay_tranche_width: Default::default(),
needed_approvals: Default::default(),
relay_vrf_modulo_samples: Default::default(),
max_upward_queue_count: Default::default(),
max_upward_queue_size: Default::default(),
max_downward_message_size: Default::default(),
preferred_dispatchable_upward_messages_step_weight: Default::default(),
max_upward_message_size: Default::default(),
max_upward_message_num_per_candidate: Default::default(),
hrmp_open_request_ttl: Default::default(),
hrmp_sender_deposit: Default::default(),
hrmp_recipient_deposit: Default::default(),
hrmp_channel_max_capacity: Default::default(),
hrmp_channel_max_total_size: Default::default(),
hrmp_max_parachain_inbound_channels: Default::default(),
hrmp_max_parathread_inbound_channels: Default::default(),
hrmp_channel_max_message_size: Default::default(),
hrmp_max_parachain_outbound_channels: Default::default(),
hrmp_max_parathread_outbound_channels: Default::default(),
hrmp_max_message_num_per_candidate: Default::default(),
}
}
}
impl<BlockNumber: Zero> HostConfiguration<BlockNumber> {
/// Checks that this instance is consistent with the requirements on each individual member.
///
/// # Panic
///
/// This function panics if any member is not set properly.
fn check_consistency(&self) {
if self.group_rotation_frequency.is_zero() {
panic!("`group_rotation_frequency` must be non-zero!")
}
if self.chain_availability_period.is_zero() {
panic!("`chain_availability_period` must be at least 1!")
}
if self.thread_availability_period.is_zero() {
panic!("`thread_availability_period` must be at least 1!")
}
if self.no_show_slots.is_zero() {
panic!("`no_show_slots` must be at least 1!")
}
}
}
pub trait Config: frame_system::Config { }
decl_storage! {
......@@ -141,10 +219,18 @@ decl_storage! {
/// Pending configuration (if any) for the next session.
PendingConfig: Option<HostConfiguration<T::BlockNumber>>;
}
add_extra_genesis {
build(|config: &Self| {
config.config.check_consistency();
})
}
}
decl_error! {
pub enum Error for Module<T: Config> { }
pub enum Error for Module<T: Config> {
/// The new value for a configuration parameter is invalid.
InvalidNewValue,
}
}
decl_module! {
......@@ -237,6 +323,9 @@ decl_module! {
#[weight = (1_000, DispatchClass::Operational)]
pub fn set_group_rotation_frequency(origin, new: T::BlockNumber) -> DispatchResult {
ensure_root(origin)?;
ensure!(!new.is_zero(), Error::<T>::InvalidNewValue);
Self::update_config_member(|config| {
sp_std::mem::replace(&mut config.group_rotation_frequency, new) != new
});
......@@ -247,6 +336,9 @@ decl_module! {
#[weight = (1_000, DispatchClass::Operational)]
pub fn set_chain_availability_period(origin, new: T::BlockNumber) -> DispatchResult {
ensure_root(origin)?;
ensure!(!new.is_zero(), Error::<T>::InvalidNewValue);
Self::update_config_member(|config| {
sp_std::mem::replace(&mut config.chain_availability_period, new) != new
});
......@@ -257,6 +349,9 @@ decl_module! {
#[weight = (1_000, DispatchClass::Operational)]
pub fn set_thread_availability_period(origin, new: T::BlockNumber) -> DispatchResult {
ensure_root(origin)?;
ensure!(!new.is_zero(), Error::<T>::InvalidNewValue);
Self::update_config_member(|config| {
sp_std::mem::replace(&mut config.thread_availability_period, new) != new
});
......@@ -298,7 +393,9 @@ decl_module! {
#[weight = (1_000, DispatchClass::Operational)]
pub fn set_no_show_slots(origin, new: u32) -> DispatchResult {
ensure_root(origin)?;
ensure!(new >= 1, "no_show_slots must be at least 1");
ensure!(!new.is_zero(), Error::<T>::InvalidNewValue);
Self::update_config_member(|config| {
sp_std::mem::replace(&mut config.no_show_slots, new) != new
});
......
......@@ -33,7 +33,7 @@ use crate::{
};
/// Information about a session change that has just occurred.
#[derive(Default, Clone)]
#[derive(Clone)]
pub struct SessionChangeNotification<BlockNumber> {
/// The new validators in the session.
pub validators: Vec<ValidatorId>,
......@@ -49,6 +49,19 @@ pub struct SessionChangeNotification<BlockNumber> {
pub session_index: sp_staking::SessionIndex,
}
impl<BlockNumber: Default + From<u32>> Default for SessionChangeNotification<BlockNumber> {
fn default() -> Self {
Self {
validators: Vec::new(),
queued: Vec::new(),
prev_config: HostConfiguration::default(),
new_config: HostConfiguration::default(),
random_seed: Default::default(),
session_index: Default::default(),
}
}
}
#[derive(Encode, Decode)]
struct BufferedSessionChange<N> {
apply_at: N,
......
......@@ -26,7 +26,6 @@ use primitives::v1::{
GroupIndex, CandidateEvent, PersistedValidationData, SessionInfo,
InboundDownwardMessage, InboundHrmpMessage,
};
use sp_runtime::traits::Zero;
use frame_support::debug;
use crate::{initializer, inclusion, scheduler, configuration, paras, session_info, dmp, hrmp};
......@@ -57,10 +56,6 @@ pub fn availability_cores<T: initializer::Config>() -> Vec<CoreState<T::BlockNum
let time_out_at = |backed_in_number, availability_period| {
let time_out_at = backed_in_number + availability_period;
if rotation_info.group_rotation_frequency == Zero::zero() {
return time_out_at;
}
let current_window = rotation_info.last_rotation_at() + availability_period;
let next_rotation = rotation_info.next_rotation_at();
......
......@@ -46,7 +46,7 @@ use frame_support::{
weights::Weight,
};
use parity_scale_codec::{Encode, Decode};
use sp_runtime::traits::{Saturating, Zero};
use sp_runtime::traits::Saturating;
use rand::{SeedableRng, seq::SliceRandom};
use rand_chacha::ChaCha20Rng;
......@@ -568,11 +568,6 @@ impl<T: Config> Module<T> {
if at < session_start_block { return None }
if config.group_rotation_frequency.is_zero() {
// interpret this as "no rotations"
return Some(GroupIndex(core.0));
}
let validator_groups = ValidatorGroups::get();
if core.0 as usize >= validator_groups.len() { return None }
......@@ -599,9 +594,6 @@ impl<T: Config> Module<T> {
/// timeouts, i.e. only within `max(config.chain_availability_period, config.thread_availability_period)`
/// of the last rotation would this return `Some`, unless there are no rotations.
///
/// If there are no rotations (config.group_rotation_frequency == 0),
/// availability timeouts can occur at any block.
///
/// This really should not be a box, but is working around a compiler limitation filed here:
/// https://github.com/rust-lang/rust/issues/73226
/// which prevents us from testing the code if using `impl Trait`.
......@@ -611,12 +603,7 @@ impl<T: Config> Module<T> {
let session_start = <SessionStartBlock<T>>::get();
let blocks_since_session_start = now.saturating_sub(session_start);
let no_rotation = config.group_rotation_frequency.is_zero();
let blocks_since_last_rotation = if no_rotation {
<T::BlockNumber>::zero()
} else {
blocks_since_session_start % config.group_rotation_frequency
};
let blocks_since_last_rotation = blocks_since_session_start % config.group_rotation_frequency;
let absolute_cutoff = sp_std::cmp::max(
config.chain_availability_period,
......@@ -1658,8 +1645,10 @@ mod tests {
} = default_config();
let collator = CollatorId::from(Sr25519Keyring::Alice.public());
assert!(chain_availability_period < thread_availability_period &&
thread_availability_period < group_rotation_frequency);
assert!(
chain_availability_period < thread_availability_period
&& thread_availability_period < group_rotation_frequency
);
let chain_a = ParaId::from(1);
let thread_a = ParaId::from(2);
......@@ -1749,92 +1738,6 @@ mod tests {
});
}
#[test]
fn availability_predicate_no_rotation() {
let genesis_config = MockGenesisConfig {
configuration: crate::configuration::GenesisConfig {
config: HostConfiguration {
group_rotation_frequency: 0, // no rotation
..default_config()
},
..Default::default()
},
..Default::default()
};
let HostConfiguration {
chain_availability_period,
thread_availability_period,
..
} = default_config();
let collator = CollatorId::from(Sr25519Keyring::Alice.public());
let chain_a = ParaId::from(1);
let thread_a = ParaId::from(2);
let schedule_blank_para = |id, is_chain| Paras::schedule_para_initialize(id, ParaGenesisArgs {
genesis_head: Vec::new().into(),
validation_code: Vec::new().into(),
parachain: is_chain,
});
new_test_ext(genesis_config).execute_with(|| {
schedule_blank_para(chain_a, true);
schedule_blank_para(thread_a, false);
// start a new session with our chain & thread registered.
run_to_block(1, |number| match number {
1 => Some(SessionChangeNotification {
new_config: HostConfiguration{
// Note: the `group_rotation_frequency` config change
// is not accounted for on session change
// group_rotation_frequency: 0,
..default_config()
},
validators: vec![
ValidatorId::from(Sr25519Keyring::Alice.public()),
ValidatorId::from(Sr25519Keyring::Bob.public()),
ValidatorId::from(Sr25519Keyring::Charlie.public()),
ValidatorId::from(Sr25519Keyring::Dave.public()),
ValidatorId::from(Sr25519Keyring::Eve.public()),
],
..Default::default()
}),
_ => None,
});
// assign some availability cores.
{
AvailabilityCores::mutate(|cores| {
cores[0] = Some(CoreOccupied::Parachain);
cores[1] = Some(CoreOccupied::Parathread(ParathreadEntry {
claim: ParathreadClaim(thread_a, collator),
retries: 0,
}))
});
}
run_to_block(1 + 1, |_| None);
run_to_block(1 + 1 + 100500, |_| None);
{
let pred = Scheduler::availability_timeout_predicate()
.expect("predicate exists with no rotation");
let now = System::block_number();
assert!(!pred(CoreIndex(0), now)); // assigned: chain
assert!(!pred(CoreIndex(1), now)); // assigned: thread
assert!(pred(CoreIndex(2), now));
// check the tighter bound on chains vs threads.
assert!(pred(CoreIndex(0), now - chain_availability_period));
assert!(pred(CoreIndex(1), now - thread_availability_period));
// check the threshold is exact.
assert!(!pred(CoreIndex(0), now - chain_availability_period + 1));
assert!(!pred(CoreIndex(1), now - thread_availability_period + 1));
}
});
}
#[test]
fn next_up_on_available_uses_next_scheduled_or_none_for_thread() {
let mut config = default_config();
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment