RFC 14: Improve locking mechanism for parachains (#1290)

* rfc14

* Update polkadot/runtime/common/src/paras_registrar/mod.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update polkadot/runtime/common/src/paras_registrar/mod.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update polkadot/runtime/common/src/paras_registrar/mod.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* fmt

* fix

* Update polkadot/runtime/common/src/paras_registrar/migration.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* fmt

* 2224 is unlocked

* update migration list

* update comment

* use VersionedMigration

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

polkadot/runtime/common/src/assigned_slots/mod.rs

@@ -739,6 +739,7 @@ mod tests {
 		type UnsignedPriority = ParasUnsignedPriority;
 		type QueueFootprinter = ();
 		type NextSessionRotation = crate::mock::TestNextSessionRotation;
+		type OnNewHead = ();
 	}
 
 	impl parachains_shared::Config for Test {}

polkadot/runtime/common/src/crowdloan/mod.rs

@@ -441,8 +441,6 @@ pub mod pallet {
 			);
 
 			NextFundIndex::<T>::put(new_fund_index);
-			// Add a lock to the para so that the configuration cannot be changed.
-			T::Registrar::apply_lock(index);
 
 			Self::deposit_event(Event::<T>::Created { para_id: index });
 			Ok(())

polkadot/runtime/common/src/integration_tests.rs

@@ -204,6 +204,7 @@ impl paras::Config for Test {
 	type UnsignedPriority = ParasUnsignedPriority;
 	type QueueFootprinter = ();
 	type NextSessionRotation = crate::mock::TestNextSessionRotation;
+	type OnNewHead = ();
 }
 
 parameter_types! {

polkadot/runtime/common/src/paras_registrar/migration.rs

@@ -0,0 +1,71 @@
+// Copyright (C) Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+use super::*;
+use frame_support::traits::{Contains, OnRuntimeUpgrade};
+
+#[derive(Encode, Decode)]
+pub struct ParaInfoV1<Account, Balance> {
+	manager: Account,
+	deposit: Balance,
+	locked: bool,
+}
+
+pub struct VersionUncheckedMigrateToV1<T, UnlockParaIds>(
+	sp_std::marker::PhantomData<(T, UnlockParaIds)>,
+);
+impl<T: Config, UnlockParaIds: Contains<ParaId>> OnRuntimeUpgrade
+	for VersionUncheckedMigrateToV1<T, UnlockParaIds>
+{
+	fn on_runtime_upgrade() -> Weight {
+		let mut count = 0u64;
+		Paras::<T>::translate::<ParaInfoV1<T::AccountId, BalanceOf<T>>, _>(|key, v1| {
+			count.saturating_inc();
+			Some(ParaInfo {
+				manager: v1.manager,
+				deposit: v1.deposit,
+				locked: if UnlockParaIds::contains(&key) { None } else { Some(v1.locked) },
+			})
+		});
+
+		log::info!(target: "runtime::registrar", "Upgraded {} storages to version 1", count);
+		T::DbWeight::get().reads_writes(count, count)
+	}
+
+	#[cfg(feature = "try-runtime")]
+	fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
+		Ok((Paras::<T>::iter_keys().count() as u32).encode())
+	}
+
+	#[cfg(feature = "try-runtime")]
+	fn post_upgrade(state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
+		let old_count = u32::decode(&mut &state[..]).expect("Known good");
+		let new_count = Paras::<T>::iter_values().count() as u32;
+
+		ensure!(old_count == new_count, "Paras count should not change");
+		Ok(())
+	}
+}
+
+#[cfg(feature = "experimental")]
+pub type VersionCheckedMigrateToV1<T, UnlockParaIds> =
+	frame_support::migrations::VersionedMigration<
+		0,
+		1,
+		VersionUncheckedMigrateToV1<T, UnlockParaIds>,
+		super::Pallet<T>,
+		<T as frame_system::Config>::DbWeight,
+	>;

polkadot/runtime/common/src/paras_registrar/mod.rs

@@ -17,6 +17,8 @@
 //! Pallet to handle parachain registration and related fund management.
 //! In essence this is a simple wrapper around `paras`.
 
+pub mod migration;
+
 use frame_support::{
 	dispatch::DispatchResult,
 	ensure,
@@ -35,7 +37,7 @@ use sp_std::{prelude::*, result};
 use crate::traits::{OnSwap, Registrar};
 pub use pallet::*;
 use parity_scale_codec::{Decode, Encode};
-use runtime_parachains::paras::ParaKind;
+use runtime_parachains::paras::{OnNewHead, ParaKind};
 use scale_info::TypeInfo;
 use sp_runtime::{
 	traits::{CheckedSub, Saturating},
@@ -49,7 +51,15 @@ pub struct ParaInfo<Account, Balance> {
 	/// The amount reserved by the `manager` account for the registration.
 	deposit: Balance,
 	/// Whether the para registration should be locked from being controlled by the manager.
-	locked: bool,
+	/// None means the lock had not been explicitly set, and should be treated as false.
+	locked: Option<bool>,
+}
+
+impl<Account, Balance> ParaInfo<Account, Balance> {
+	/// Returns if the para is locked.
+	pub fn is_locked(&self) -> bool {
+		self.locked.unwrap_or(false)
+	}
 }
 
 type BalanceOf<T> =
@@ -96,8 +106,12 @@ pub mod pallet {
 	use frame_support::pallet_prelude::*;
 	use frame_system::pallet_prelude::*;
 
+	/// The current storage version.
+	const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+
 	#[pallet::pallet]
 	#[pallet::without_storage_info]
+	#[pallet::storage_version(STORAGE_VERSION)]
 	pub struct Pallet<T>(_);
 
 	#[pallet::config]
@@ -446,12 +460,12 @@ impl<T: Config> Registrar for Pallet<T> {
 
 	// Apply a lock to the parachain.
 	fn apply_lock(id: ParaId) {
-		Paras::<T>::mutate(id, |x| x.as_mut().map(|info| info.locked = true));
+		Paras::<T>::mutate(id, |x| x.as_mut().map(|info| info.locked = Some(true)));
 	}
 
 	// Remove a lock from the parachain.
 	fn remove_lock(id: ParaId) {
-		Paras::<T>::mutate(id, |x| x.as_mut().map(|info| info.locked = false));
+		Paras::<T>::mutate(id, |x| x.as_mut().map(|info| info.locked = Some(false)));
 	}
 
 	// Register a Para ID under control of `manager`.
@@ -481,9 +495,7 @@ impl<T: Config> Registrar for Pallet<T> {
 		);
 		runtime_parachains::schedule_parathread_upgrade::<T>(id)
 			.map_err(|_| Error::<T>::CannotUpgrade)?;
-		// Once a para has upgraded to a parachain, it can no longer be managed by the owner.
-		// Intentionally, the flag stays with the para even after downgrade.
-		Self::apply_lock(id);
+
 		Ok(())
 	}
 
@@ -533,7 +545,7 @@ impl<T: Config> Pallet<T> {
 			.map_err(|e| e.into())
 			.and_then(|who| -> DispatchResult {
 				let para_info = Paras::<T>::get(id).ok_or(Error::<T>::NotRegistered)?;
-				ensure!(!para_info.locked, Error::<T>::ParaLocked);
+				ensure!(!para_info.is_locked(), Error::<T>::ParaLocked);
 				ensure!(para_info.manager == who, Error::<T>::NotOwner);
 				Ok(())
 			})
@@ -566,7 +578,7 @@ impl<T: Config> Pallet<T> {
 
 		let deposit = deposit_override.unwrap_or_else(T::ParaDeposit::get);
 		<T as Config>::Currency::reserve(&who, deposit)?;
-		let info = ParaInfo { manager: who.clone(), deposit, locked: false };
+		let info = ParaInfo { manager: who.clone(), deposit, locked: None };
 
 		Paras::<T>::insert(id, info);
 		Self::deposit_event(Event::<T>::Reserved { para_id: id, who });
@@ -585,7 +597,7 @@ impl<T: Config> Pallet<T> {
 	) -> DispatchResult {
 		let deposited = if let Some(para_data) = Paras::<T>::get(id) {
 			ensure!(para_data.manager == who, Error::<T>::NotOwner);
-			ensure!(!para_data.locked, Error::<T>::ParaLocked);
+			ensure!(!para_data.is_locked(), Error::<T>::ParaLocked);
 			para_data.deposit
 		} else {
 			ensure!(!ensure_reserved, Error::<T>::NotReserved);
@@ -601,7 +613,7 @@ impl<T: Config> Pallet<T> {
 		} else if let Some(rebate) = deposited.checked_sub(&deposit) {
 			<T as Config>::Currency::unreserve(&who, rebate);
 		};
-		let info = ParaInfo { manager: who.clone(), deposit, locked: false };
+		let info = ParaInfo { manager: who.clone(), deposit, locked: None };
 
 		Paras::<T>::insert(id, info);
 		// We check above that para has no lifecycle, so this should not fail.
@@ -665,6 +677,21 @@ impl<T: Config> Pallet<T> {
 	}
 }
 
+impl<T: Config> OnNewHead for Pallet<T> {
+	fn on_new_head(id: ParaId, _head: &HeadData) -> Weight {
+		// mark the parachain locked if the locked value is not already set
+		let mut writes = 0;
+		if let Some(mut info) = Paras::<T>::get(id) {
+			if info.locked.is_none() {
+				info.locked = Some(true);
+				Paras::<T>::insert(id, info);
+				writes += 1;
+			}
+		}
+		T::DbWeight::get().reads_writes(1, writes)
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;
@@ -784,6 +811,7 @@ mod tests {
 		type UnsignedPriority = ParasUnsignedPriority;
 		type QueueFootprinter = ();
 		type NextSessionRotation = crate::mock::TestNextSessionRotation;
+		type OnNewHead = ();
 	}
 
 	impl configuration::Config for Test {
@@ -1270,8 +1298,10 @@ mod tests {
 			));
 
 			assert_noop!(Registrar::add_lock(RuntimeOrigin::signed(2), para_id), BadOrigin);
-			// Once they begin onboarding, we lock them in.
-			assert_ok!(Registrar::add_lock(RuntimeOrigin::signed(1), para_id));
+
+			// Once they produces new block, we lock them in.
+			Registrar::on_new_head(para_id, &Default::default());
+
 			// Owner cannot pass origin check when checking lock
 			assert_noop!(
 				Registrar::ensure_root_para_or_owner(RuntimeOrigin::signed(1), para_id),
@@ -1283,6 +1313,11 @@ mod tests {
 			assert_ok!(Registrar::remove_lock(para_origin(para_id), para_id));
 			// Owner can pass origin check again
 			assert_ok!(Registrar::ensure_root_para_or_owner(RuntimeOrigin::signed(1), para_id));
+
+			// Won't lock again after it is unlocked
+			Registrar::on_new_head(para_id, &Default::default());
+
+			assert_ok!(Registrar::ensure_root_para_or_owner(RuntimeOrigin::signed(1), para_id));
 		});
 	}