diff --git a/polkadot/runtime/common/src/paras_registrar.rs b/polkadot/runtime/common/src/paras_registrar.rs
index a714a45a5577ffc5eb612104d0b0f715445b04be..1b7a450ca6646300ab701428dd9d93d5de09695c 100644
--- a/polkadot/runtime/common/src/paras_registrar.rs
+++ b/polkadot/runtime/common/src/paras_registrar.rs
@@ -157,6 +157,8 @@ pub mod pallet {
ParaLocked,
/// The ID given for registration has not been reserved.
NotReserved,
+ /// Registering parachain with empty code is not allowed.
+ EmptyCode,
}
/// Pending swap operations.
@@ -547,6 +549,7 @@ impl<T: Config> Pallet<T> {
parachain: bool,
) -> Result<(ParaGenesisArgs, BalanceOf<T>), sp_runtime::DispatchError> {
let config = configuration::Pallet::<T>::config();
+ ensure!(validation_code.0.len() > 0, Error::<T>::EmptyCode);
ensure!(validation_code.0.len() <= config.max_code_size as usize, Error::<T>::CodeTooLarge);
ensure!(
genesis_head.0.len() <= config.max_head_data_size as usize,
diff --git a/polkadot/runtime/parachains/src/lib.rs b/polkadot/runtime/parachains/src/lib.rs
index f1d8473f8894e04c7428303afc0a9f3c23e55599..4398238c319bbf2b795244bd226c1c0b6c8d7170 100644
--- a/polkadot/runtime/parachains/src/lib.rs
+++ b/polkadot/runtime/parachains/src/lib.rs
@@ -52,6 +52,8 @@ pub use paras::ParaLifecycle;
use primitives::v1::Id as ParaId;
/// Schedule a para to be initialized at the start of the next session with the given genesis data.
+///
+/// See [`paras::Pallet::schedule_para_initialize`] for more details.
pub fn schedule_para_initialize<T: paras::Config>(
id: ParaId,
genesis: paras::ParaGenesisArgs,
@@ -60,6 +62,8 @@ pub fn schedule_para_initialize<T: paras::Config>(
}
/// Schedule a para to be cleaned up at the start of the next session.
+///
+/// See [`paras::Pallet::schedule_para_cleanup`] for more details.
pub fn schedule_para_cleanup<T: paras::Config>(id: primitives::v1::Id) -> Result<(), ()> {
<paras::Pallet<T>>::schedule_para_cleanup(id).map_err(|_| ())
}
diff --git a/polkadot/runtime/parachains/src/paras.rs b/polkadot/runtime/parachains/src/paras.rs
index 7c786360e57c9b97606a8ca8896244fa65b4eda5..961dfb833fce713ea5cba02ed352e408f242a310 100644
--- a/polkadot/runtime/parachains/src/paras.rs
+++ b/polkadot/runtime/parachains/src/paras.rs
@@ -870,8 +870,10 @@ impl<T: Config> Pallet<T> {
pub(crate) fn schedule_para_initialize(id: ParaId, genesis: ParaGenesisArgs) -> DispatchResult {
let scheduled_session = Self::scheduled_session();
- // Make sure parachain isn't already in our system.
+ // Make sure parachain isn't already in our system and that the onboarding parameters are
+ // valid.
ensure!(Self::can_schedule_para_initialize(&id, &genesis), Error::<T>::CannotOnboard);
+ ensure!(!genesis.validation_code.0.is_empty(), Error::<T>::CannotOnboard);
ParaLifecycles::<T>::insert(&id, ParaLifecycle::Onboarding);
UpcomingParasGenesis::<T>::insert(&id, genesis);
@@ -886,10 +888,24 @@ impl<T: Config> Pallet<T> {
/// Schedule a para to be cleaned up at the start of the next session.
///
- /// Will return error if para is not a stable parachain or parathread.
+ /// Will return error if either is true:
+ ///
+ /// - para is not a stable parachain or parathread (i.e. [`ParaLifecycle::is_stable`] is `false`)
+ /// - para has a pending upgrade.
///
/// No-op if para is not registered at all.
pub(crate) fn schedule_para_cleanup(id: ParaId) -> DispatchResult {
+ // Disallow offboarding in case there is an upcoming upgrade.
+ //
+ // This is not a fundamential limitation but rather simplification: it allows us to get
+ // away without introducing additional logic for pruning and, more importantly, enacting
+ // ongoing PVF pre-checking votings. It also removes some nasty edge cases.
+ //
+ // This implicitly assumes that the given para exists, i.e. it's lifecycle != None.
+ if FutureCodeHash::<T>::contains_key(&id) {
+ return Err(Error::<T>::CannotOffboard.into())
+ }
+
let lifecycle = ParaLifecycles::<T>::get(&id);
match lifecycle {
// If para is not registered, nothing to do!
@@ -1178,12 +1194,12 @@ impl<T: Config> Pallet<T> {
#[cfg(test)]
mod tests {
use super::*;
- use frame_support::assert_ok;
+ use frame_support::{assert_err, assert_ok};
use primitives::v1::BlockNumber;
use crate::{
configuration::HostConfiguration,
- mock::{new_test_ext, Configuration, MockGenesisConfig, Paras, ParasShared, System},
+ mock::{new_test_ext, Configuration, MockGenesisConfig, Paras, ParasShared, System, Test},
};
fn run_to_block(to: BlockNumber, new_session: Option<Vec<BlockNumber>>) {
@@ -1335,6 +1351,32 @@ mod tests {
);
}
+ #[test]
+ fn schedule_para_init_rejects_empty_code() {
+ new_test_ext(MockGenesisConfig::default()).execute_with(|| {
+ assert_err!(
+ Paras::schedule_para_initialize(
+ 1000.into(),
+ ParaGenesisArgs {
+ parachain: false,
+ genesis_head: Default::default(),
+ validation_code: ValidationCode(vec![]),
+ }
+ ),
+ Error::<Test>::CannotOnboard,
+ );
+
+ assert_ok!(Paras::schedule_para_initialize(
+ 1000.into(),
+ ParaGenesisArgs {
+ parachain: false,
+ genesis_head: Default::default(),
+ validation_code: ValidationCode(vec![1]),
+ }
+ ));
+ });
+ }
+
#[test]
fn para_past_code_pruning_in_initialize() {
let code_retention_period = 10;
@@ -1817,55 +1859,51 @@ mod tests {
expected_at
};
- assert_ok!(Paras::schedule_para_cleanup(para_id));
+ // Cannot offboard while an upgrade is pending.
+ assert_err!(Paras::schedule_para_cleanup(para_id), Error::<Test>::CannotOffboard);
- // Just scheduling cleanup shouldn't change anything.
- {
- assert_eq!(
- <Paras as Store>::ActionsQueue::get(Paras::scheduled_session()),
- vec![para_id],
- );
- assert_eq!(Paras::parachains(), vec![para_id]);
-
- assert!(Paras::past_code_meta(¶_id).most_recent_change().is_none());
- assert_eq!(<Paras as Store>::FutureCodeUpgrades::get(¶_id), Some(expected_at));
- assert_eq!(<Paras as Store>::FutureCodeHash::get(¶_id), Some(new_code.hash()));
- assert_eq!(Paras::current_code(¶_id), Some(original_code.clone()));
- check_code_is_stored(&original_code);
- check_code_is_stored(&new_code);
+ // Enact the upgrade.
+ //
+ // For that run to block #7 and submit a new head.
+ assert_eq!(expected_at, 7);
+ run_to_block(7, None);
+ assert_eq!(<frame_system::Pallet<Test>>::block_number(), 7);
+ Paras::note_new_head(para_id, Default::default(), expected_at);
- assert_eq!(<Paras as Store>::Heads::get(¶_id), Some(Default::default()));
- }
+ assert_ok!(Paras::schedule_para_cleanup(para_id));
- // run to block #4, with a 2 session changes at the end of the block 2 & 3.
- run_to_block(4, Some(vec![3, 4]));
+ // run to block #10, with a 2 session changes at the end of the block 7 & 8 (so 8 and 9
+ // observe the new sessions).
+ run_to_block(10, Some(vec![8, 9]));
// cleaning up the parachain should place the current parachain code
// into the past code buffer & schedule cleanup.
- assert_eq!(Paras::past_code_meta(¶_id).most_recent_change(), Some(3));
- assert_eq!(
- <Paras as Store>::PastCodeHash::get(&(para_id, 3)),
- Some(original_code.hash())
- );
- assert_eq!(<Paras as Store>::PastCodePruning::get(), vec![(para_id, 3)]);
+ //
+ // Why 7 and 8? See above, the clean up scheduled above was processed at the block 8.
+ // The initial upgrade was enacted at the block 7.
+ assert_eq!(Paras::past_code_meta(¶_id).most_recent_change(), Some(8));
+ assert_eq!(<Paras as Store>::PastCodeHash::get(&(para_id, 8)), Some(new_code.hash()));
+ assert_eq!(<Paras as Store>::PastCodePruning::get(), vec![(para_id, 7), (para_id, 8)]);
check_code_is_stored(&original_code);
+ check_code_is_stored(&new_code);
// any future upgrades haven't been used to validate yet, so those
// are cleaned up immediately.
assert!(<Paras as Store>::FutureCodeUpgrades::get(¶_id).is_none());
assert!(<Paras as Store>::FutureCodeHash::get(¶_id).is_none());
assert!(Paras::current_code(¶_id).is_none());
- check_code_is_not_stored(&new_code);
// run to do the final cleanup
- let cleaned_up_at = 3 + code_retention_period + 1;
+ let cleaned_up_at = 8 + code_retention_period + 1;
run_to_block(cleaned_up_at, None);
// now the final cleanup: last past code cleaned up, and this triggers meta cleanup.
assert_eq!(Paras::past_code_meta(¶_id), Default::default());
- assert!(<Paras as Store>::PastCodeHash::get(&(para_id, 3)).is_none());
+ assert!(<Paras as Store>::PastCodeHash::get(&(para_id, 7)).is_none());
+ assert!(<Paras as Store>::PastCodeHash::get(&(para_id, 8)).is_none());
assert!(<Paras as Store>::PastCodePruning::get().is_empty());
check_code_is_not_stored(&original_code);
+ check_code_is_not_stored(&new_code);
});
}
diff --git a/polkadot/runtime/parachains/src/scheduler.rs b/polkadot/runtime/parachains/src/scheduler.rs
index ca658be2cc289319451fb30671f24617c6fb141c..706fd844b31665b681f55f0f6e0b8922d658fec0 100644
--- a/polkadot/runtime/parachains/src/scheduler.rs
+++ b/polkadot/runtime/parachains/src/scheduler.rs
@@ -777,7 +777,7 @@ mod tests {
id,
ParaGenesisArgs {
genesis_head: Vec::new().into(),
- validation_code: Vec::new().into(),
+ validation_code: vec![1, 2, 3].into(),
parachain: is_chain,
}
));