From c7def0e0d85c806dbdd96b2d9903a5b978fabc4d Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 16 Dec 2021 17:14:05 +0000 Subject: [PATCH] configuration: Consistency checks for PVF pre-checking As was suggested by Alexander Popiak [here][comment], this commit checks the consistency of the configuration. [comment]: https://github.com/paritytech/polkadot/pull/4420#discussion_r764796519 --- runtime/parachains/src/configuration.rs | 68 ++++++++++++++++++++++--- runtime/parachains/src/paras.rs | 6 ++- runtime/parachains/src/scheduler.rs | 4 ++ 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs index d5d357d57b68..92189705937b 100644 --- a/runtime/parachains/src/configuration.rs +++ b/runtime/parachains/src/configuration.rs @@ -205,6 +205,9 @@ pub struct HostConfiguration { /// /// To prevent that, we introduce the minimum number of blocks after which the upgrade can be /// scheduled. This number is controlled by this field. + /// + /// This value should be greater than [`chain_availability_period`] and + /// [`thread_availability_period`]. pub minimum_validation_upgrade_delay: BlockNumber, } @@ -253,14 +256,14 @@ impl> Default for HostConfiguration { /// `group_rotation_frequency` is set to zero. ZeroGroupRotationFrequency, /// `chain_availability_period` is set to zero. @@ -275,6 +278,16 @@ pub enum InconsistentError { MaxHeadDataSizeExceedHardLimit { max_head_data_size: u32 }, /// `max_pov_size` exceeds the hard limit of `MAX_POV_SIZE`. MaxPovSizeExceedHardLimit { max_pov_size: u32 }, + /// `minimum_validation_upgrade_delay` is less than `chain_availability_period`. + MinimumValidationUpgradeDelayLessThanChainAvailabilityPeriod { + minimum_validation_upgrade_delay: BlockNumber, + chain_availability_period: BlockNumber, + }, + /// `minimum_validation_upgrade_delay` is less than `thread_availability_period`. + MinimumValidationUpgradeDelayLessThanThreadAvailabilityPeriod { + minimum_validation_upgrade_delay: BlockNumber, + thread_availability_period: BlockNumber, + }, } impl HostConfiguration { @@ -283,7 +296,7 @@ impl HostConfigurat /// # Errors /// /// This function returns an error if the configuration is inconsistent. - fn check_consistency(&self) -> Result<(), InconsistentError> { + fn check_consistency(&self) -> Result<(), InconsistentError> { use InconsistentError::*; if self.group_rotation_frequency.is_zero() { @@ -316,6 +329,18 @@ impl HostConfigurat return Err(MaxPovSizeExceedHardLimit { max_pov_size: self.max_pov_size }) } + if self.minimum_validation_upgrade_delay <= self.chain_availability_period { + return Err(MinimumValidationUpgradeDelayLessThanChainAvailabilityPeriod { + minimum_validation_upgrade_delay: self.minimum_validation_upgrade_delay.clone(), + chain_availability_period: self.chain_availability_period.clone(), + }) + } else if self.minimum_validation_upgrade_delay <= self.thread_availability_period { + return Err(MinimumValidationUpgradeDelayLessThanThreadAvailabilityPeriod { + minimum_validation_upgrade_delay: self.minimum_validation_upgrade_delay.clone(), + thread_availability_period: self.thread_availability_period.clone(), + }) + } + Ok(()) } @@ -999,6 +1024,8 @@ pub mod pallet { /// Sets the minimum delay between announcing the upgrade block for a parachain until the /// upgrade taking place. + /// + /// See the field documentation for information and constraints for the new value. #[pallet::weight(( T::WeightInfo::set_config_with_block_number(), DispatchClass::Operational, @@ -1440,6 +1467,29 @@ mod tests { Configuration::set_thread_availability_period(Origin::root(), 0), Error::::InvalidNewValue ); + assert_err!( + Configuration::set_no_show_slots(Origin::root(), 0), + Error::::InvalidNewValue + ); + + ::ActiveConfig::put(HostConfiguration { + chain_availability_period: 10, + thread_availability_period: 8, + minimum_validation_upgrade_delay: 11, + ..Default::default() + }); + assert_err!( + Configuration::set_chain_availability_period(Origin::root(), 12), + Error::::InvalidNewValue + ); + assert_err!( + Configuration::set_thread_availability_period(Origin::root(), 12), + Error::::InvalidNewValue + ); + assert_err!( + Configuration::set_minimum_validation_upgrade_delay(Origin::root(), 9), + Error::::InvalidNewValue + ); }); } @@ -1545,6 +1595,13 @@ mod tests { new_config.group_rotation_frequency, ) .unwrap(); + // This comes out of order to satisfy the validity criteria for the chain and thread + // availability periods. + Configuration::set_minimum_validation_upgrade_delay( + Origin::root(), + new_config.minimum_validation_upgrade_delay, + ) + .unwrap(); Configuration::set_chain_availability_period( Origin::root(), new_config.chain_availability_period, @@ -1685,11 +1742,6 @@ mod tests { ) .unwrap(); Configuration::set_pvf_voting_ttl(Origin::root(), new_config.pvf_voting_ttl).unwrap(); - Configuration::set_minimum_validation_upgrade_delay( - Origin::root(), - new_config.minimum_validation_upgrade_delay, - ) - .unwrap(); assert_eq!( ::PendingConfigs::get(), diff --git a/runtime/parachains/src/paras.rs b/runtime/parachains/src/paras.rs index 56eb26f30a06..e5e282f31bdc 100644 --- a/runtime/parachains/src/paras.rs +++ b/runtime/parachains/src/paras.rs @@ -2450,7 +2450,11 @@ mod tests { code_retention_period, validation_upgrade_delay, pvf_checking_enabled: false, - minimum_validation_upgrade_delay: 0, + minimum_validation_upgrade_delay: 2, + // Those are not relevant to this test. However, HostConfiguration is still a + // subject for the consistency check. + chain_availability_period: 1, + thread_availability_period: 1, ..Default::default() }, ..Default::default() diff --git a/runtime/parachains/src/scheduler.rs b/runtime/parachains/src/scheduler.rs index fead0d39ec23..984db51d8902 100644 --- a/runtime/parachains/src/scheduler.rs +++ b/runtime/parachains/src/scheduler.rs @@ -850,6 +850,10 @@ mod tests { scheduling_lookahead: 2, parathread_retries: 1, pvf_checking_enabled: false, + // This field does not affect anything that scheduler does. However, `HostConfiguration` + // is still a subject to consistency test. It requires that `minimum_validation_upgrade_delay` + // is greater than `chain_availability_period` and `thread_availability_period`. + minimum_validation_upgrade_delay: 6, ..Default::default() } }