diff --git a/polkadot/runtime/common/src/crowdloan.rs b/polkadot/runtime/common/src/crowdloan.rs index 185d4a3d68..7da0ceb8a4 100644 --- a/polkadot/runtime/common/src/crowdloan.rs +++ b/polkadot/runtime/common/src/crowdloan.rs @@ -294,6 +294,9 @@ decl_module! { fn deposit_event() = default; /// Create a new crowdloaning campaign for a parachain slot with the given lease period range. + /// + /// This applies a lock to your parachain configuration, ensuring that it cannot be changed + /// by the parachain manager. #[weight = T::WeightInfo::create()] pub fn create(origin, #[compact] index: ParaId, @@ -342,6 +345,8 @@ decl_module! { }); NextTrieIndex::put(new_trie_index); + // Add a lock to the para so that the configuration cannot be changed. + T::Registrar::apply_lock(index); Self::deposit_event(RawEvent::Created(index)); } diff --git a/polkadot/runtime/common/src/integration_tests.rs b/polkadot/runtime/common/src/integration_tests.rs index 2df4502958..26166ecefe 100644 --- a/polkadot/runtime/common/src/integration_tests.rs +++ b/polkadot/runtime/common/src/integration_tests.rs @@ -728,8 +728,8 @@ fn basic_swap_works() { assert_eq!(Paras::lifecycle(ParaId::from(2)), Some(ParaLifecycle::Parathread)); // Initiate a swap - assert_ok!(Registrar::swap(para_origin(1).into(), ParaId::from(2))); - assert_ok!(Registrar::swap(para_origin(2).into(), ParaId::from(1))); + assert_ok!(Registrar::swap(para_origin(1).into(), ParaId::from(1), ParaId::from(2))); + assert_ok!(Registrar::swap(para_origin(2).into(), ParaId::from(2), ParaId::from(1))); assert_eq!(Paras::lifecycle(ParaId::from(1)), Some(ParaLifecycle::DowngradingParachain)); assert_eq!(Paras::lifecycle(ParaId::from(2)), Some(ParaLifecycle::UpgradingParathread)); diff --git a/polkadot/runtime/common/src/mock.rs b/polkadot/runtime/common/src/mock.rs index d6bea16652..5e62681bd7 100644 --- a/polkadot/runtime/common/src/mock.rs +++ b/polkadot/runtime/common/src/mock.rs @@ -27,6 +27,7 @@ thread_local! { static OPERATIONS: RefCell> = RefCell::new(Vec::new()); static PARACHAINS: RefCell> = RefCell::new(Vec::new()); static PARATHREADS: RefCell> = RefCell::new(Vec::new()); + static LOCKS: RefCell> = RefCell::new(HashMap::new()); static MANAGERS: RefCell>> = RefCell::new(HashMap::new()); } @@ -47,6 +48,14 @@ impl Registrar for TestRegistrar { PARATHREADS.with(|x| x.borrow().binary_search(&id).is_ok()) } + fn apply_lock(id: ParaId) { + LOCKS.with(|x| x.borrow_mut().insert(id, true)); + } + + fn remove_lock(id: ParaId) { + LOCKS.with(|x| x.borrow_mut().insert(id, false)); + } + fn register( manager: Self::AccountId, id: ParaId, diff --git a/polkadot/runtime/common/src/paras_registrar.rs b/polkadot/runtime/common/src/paras_registrar.rs index 345cb88022..7ba2bca205 100644 --- a/polkadot/runtime/common/src/paras_registrar.rs +++ b/polkadot/runtime/common/src/paras_registrar.rs @@ -43,8 +43,12 @@ use sp_runtime::{RuntimeDebug, traits::Saturating}; #[derive(Encode, Decode, Clone, PartialEq, Eq, Default, RuntimeDebug)] pub struct ParaInfo { + /// The account that has placed a deposit for registering this para. pub(crate) manager: Account, + /// 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, } type BalanceOf = @@ -142,6 +146,8 @@ decl_error! { CannotDowngrade, /// Cannot schedule upgrade of parathread to parachain CannotUpgrade, + /// Para is locked from manipulation by the manager. Must use parachain or relay chain governance. + ParaLocked, } } @@ -179,21 +185,17 @@ decl_module! { /// Deregister a Para Id, freeing all data and returning any deposit. /// - /// The caller must be the para itself or Root and the para must be a parathread. + /// The caller must be Root, the `para` owner, or the `para` itself. The para must be a parathread. #[weight = T::WeightInfo::deregister()] pub fn deregister(origin, id: ParaId) -> DispatchResult { - match ensure_root(origin.clone()) { - Ok(_) => {}, - Err(_) => { - let caller_id = ensure_parachain(::Origin::from(origin))?; - ensure!(caller_id == id, Error::::NotOwner); - }, - }; - + Self::ensure_root_para_or_owner(origin, id)?; Self::do_deregister(id) } - /// Swap a parachain with another parachain or parathread. The origin must be a `Parachain`. + /// Swap a parachain with another parachain or parathread. + /// + /// The origin must be Root, the `para` owner, or the `para` itself. + /// /// The swap will happen only if there is already an opposite swap pending. If there is not, /// the swap will be stored in the pending swaps map, ready for a later confirmatory swap. /// @@ -202,8 +204,9 @@ decl_module! { /// scheduling info (i.e. whether they're a parathread or parachain), auction information /// and the auction deposit are switched. #[weight = T::WeightInfo::swap()] - pub fn swap(origin, other: ParaId) { - let id = ensure_parachain(::Origin::from(origin))?; + pub fn swap(origin, id: ParaId, other: ParaId) { + Self::ensure_root_para_or_owner(origin, id)?; + if PendingSwap::get(other) == Some(id) { if let Some(other_lifecycle) = paras::Module::::lifecycle(other) { if let Some(id_lifecycle) = paras::Module::::lifecycle(id) { @@ -259,6 +262,16 @@ impl Registrar for Module { paras::Module::::is_parachain(id) } + // Apply a lock to the parachain. + fn apply_lock(id: ParaId) { + Paras::::mutate(id, |x| x.as_mut().map(|mut info| info.locked = true)); + } + + // Apply a lock to the parachain. + fn remove_lock(id: ParaId) { + Paras::::mutate(id, |x| x.as_mut().map(|mut info| info.locked = false)); + } + // Register a Para ID under control of `manager`. fn register( manager: T::AccountId, @@ -279,6 +292,9 @@ impl Registrar for Module { // Para backend should think this is a parathread... ensure!(paras::Module::::lifecycle(id) == Some(ParaLifecycle::Parathread), Error::::NotParathread); runtime_parachains::schedule_parathread_upgrade::(id).map_err(|_| Error::::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(()) } @@ -316,6 +332,27 @@ impl Registrar for Module { } impl Module { + /// Ensure the origin is one of Root, the `para` owner, or the `para` itself. + /// If the origin is the `para` owner, the `para` must be unlocked. + fn ensure_root_para_or_owner(origin: ::Origin, id: ParaId) -> DispatchResult { + ensure_signed(origin.clone()).map_err(|e| e.into()) + .and_then(|who| -> DispatchResult { + let para_info = Paras::::get(id).ok_or(Error::::NotRegistered)?; + ensure!(!para_info.locked, Error::::ParaLocked); + ensure!(para_info.manager == who, Error::::NotOwner); + Ok(()) + }) + .or_else(|_| -> DispatchResult { + // Else check if para origin... + let caller_id = ensure_parachain(::Origin::from(origin.clone()))?; + ensure!(caller_id == id, Error::::NotOwner); + Ok(()) + }).or_else(|_| -> DispatchResult { + // Check if root... + ensure_root(origin.clone()).map_err(|e| e.into()) + }) + } + /// Attempt to register a new Para Id under management of `who` in the /// system with the given information. fn do_register( @@ -336,6 +373,7 @@ impl Module { let info = ParaInfo { manager: who.clone(), deposit: deposit, + locked: false, }; Paras::::insert(id, info); @@ -348,7 +386,11 @@ impl Module { /// Deregister a Para Id, freeing all data returning any deposit. fn do_deregister(id: ParaId) -> DispatchResult { - ensure!(paras::Module::::lifecycle(id) == Some(ParaLifecycle::Parathread), Error::::NotParathread); + match paras::Module::::lifecycle(id) { + // Para must be a parathread, or not exist at all. + Some(ParaLifecycle::Parathread) | None => {}, + _ => return Err(Error::::NotParathread.into()) + } runtime_parachains::schedule_para_cleanup::(id).map_err(|_| Error::::CannotDeregister)?; if let Some(info) = Paras::::take(&id) { @@ -401,8 +443,8 @@ mod tests { use frame_system::limits; use frame_support::{ traits::{OnInitialize, OnFinalize}, - error::BadOrigin, assert_ok, assert_noop, parameter_types, + error::BadOrigin, }; use runtime_parachains::{configuration, shared}; use pallet_balances::Error as BalancesError; @@ -713,16 +755,11 @@ mod tests { )); run_to_session(2); assert!(Parachains::is_parathread(32.into())); - // Origin check + // Owner check assert_noop!(Registrar::deregister( - Origin::signed(1), + Origin::signed(2), 32.into(), ), BadOrigin); - // not registered - assert_noop!(Registrar::deregister( - Origin::root(), - 33.into(), - ), Error::::NotParathread); assert_ok!(Registrar::make_parachain(32.into())); run_to_session(4); // Cant directly deregister parachain @@ -765,11 +802,13 @@ mod tests { // Both paras initiate a swap assert_ok!(Registrar::swap( para_origin(23.into()), - 32.into() + 23.into(), + 32.into(), )); assert_ok!(Registrar::swap( para_origin(32.into()), - 23.into() + 32.into(), + 23.into(), )); run_to_session(6); @@ -827,6 +866,33 @@ mod tests { )); }); } + + #[test] + fn para_lock_works() { + new_test_ext().execute_with(|| { + run_to_block(1); + + assert_ok!(Registrar::register( + Origin::signed(1), + 1u32.into(), + vec![1; 3].into(), + WASM_MAGIC.to_vec().into(), + )); + + // Owner can call swap + assert_ok!(Registrar::swap(Origin::signed(1), 1u32.into(), 2u32.into())); + + // 2 session changes to fully onboard. + run_to_session(2); + assert_eq!(Parachains::lifecycle(1u32.into()), Some(ParaLifecycle::Parathread)); + + // Once they begin onboarding, we lock them in. + assert_ok!(Registrar::make_parachain(1u32.into())); + + // Owner cannot call swap anymore + assert_noop!(Registrar::swap(Origin::signed(1), 1u32.into(), 3u32.into()), BadOrigin); + }); + } } #[cfg(feature = "runtime-benchmarks")] @@ -838,7 +904,7 @@ mod benchmarking { use crate::traits::{Registrar as RegistrarT}; use runtime_parachains::{paras, shared, Origin as ParaOrigin}; - use frame_benchmarking::{benchmarks, whitelisted_caller}; + use frame_benchmarking::{benchmarks, whitelisted_caller, impl_benchmark_test_suite}; fn assert_last_event(generic_event: ::Event) { let events = frame_system::Pallet::::events(); @@ -890,7 +956,8 @@ mod benchmarking { deregister { let para = register_para::(1337); next_scheduled_session::(); - }: _(RawOrigin::Root, para) + let caller: T::AccountId = whitelisted_caller(); + }: _(RawOrigin::Signed(caller), para) verify { assert_last_event::(RawEvent::Deregistered(para).into()); } @@ -899,8 +966,7 @@ mod benchmarking { let parathread = register_para::(1337); let parachain = register_para::(1338); - let parathread_origin = para_origin(1337); - let parachain_origin = para_origin(1338); + let parachain_origin = para_origin(parachain.into()); // Actually finish registration process next_scheduled_session::(); @@ -912,10 +978,10 @@ mod benchmarking { assert_eq!(paras::Module::::lifecycle(parachain), Some(ParaLifecycle::Parachain)); assert_eq!(paras::Module::::lifecycle(parathread), Some(ParaLifecycle::Parathread)); - Registrar::::swap(parathread_origin.into(), parachain)?; - }: { - Registrar::::swap(parachain_origin.into(), parathread)?; - } verify { + let caller: T::AccountId = whitelisted_caller(); + Registrar::::swap(parachain_origin.into(), parachain, parathread)?; + }: _(RawOrigin::Signed(caller.clone()), parathread, parachain) + verify { next_scheduled_session::(); // Swapped! assert_eq!(paras::Module::::lifecycle(parachain), Some(ParaLifecycle::Parathread)); @@ -923,19 +989,9 @@ mod benchmarking { } } - #[cfg(test)] - mod tests { - use super::*; - use crate::integration_tests::{new_test_ext, Test}; - use frame_support::assert_ok; - - #[test] - fn test_benchmarks() { - new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_register::()); - assert_ok!(test_benchmark_deregister::()); - assert_ok!(test_benchmark_swap::()); - }); - } - } + impl_benchmark_test_suite!( + Registrar, + crate::integration_tests::new_test_ext(), + crate::integration_tests::Test, + ); } diff --git a/polkadot/runtime/common/src/traits.rs b/polkadot/runtime/common/src/traits.rs index 9c599025c0..fb157cbf84 100644 --- a/polkadot/runtime/common/src/traits.rs +++ b/polkadot/runtime/common/src/traits.rs @@ -47,6 +47,14 @@ pub trait Registrar { Self::is_parathread(id) || Self::is_parachain(id) } + /// Apply a lock to the para registration so that it cannot be modified by + /// the manager directly. Instead the para must use its sovereign governance + /// or the governance of the relay chain. + fn apply_lock(id: ParaId); + + /// Remove any lock on the para registration. + fn remove_lock(id: ParaId); + /// Register a Para ID under control of `who`. Registration may be be /// delayed by session rotation. fn register( diff --git a/polkadot/runtime/parachains/src/paras.rs b/polkadot/runtime/parachains/src/paras.rs index 6a697543d0..016b0bd50e 100644 --- a/polkadot/runtime/parachains/src/paras.rs +++ b/polkadot/runtime/parachains/src/paras.rs @@ -660,20 +660,25 @@ impl Module { /// 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. + /// + /// No-op if para is not registered at all. pub(crate) fn schedule_para_cleanup(id: ParaId) -> DispatchResult { - let scheduled_session = Self::scheduled_session(); - let lifecycle = ParaLifecycles::get(&id).ok_or(Error::::NotRegistered)?; - + let lifecycle = ParaLifecycles::get(&id); match lifecycle { - ParaLifecycle::Parathread => { + // If para is not registered, nothing to do! + None => { + return Ok(()) + }, + Some(ParaLifecycle::Parathread) => { ParaLifecycles::insert(&id, ParaLifecycle::OffboardingParathread); }, - ParaLifecycle::Parachain => { + Some(ParaLifecycle::Parachain) => { ParaLifecycles::insert(&id, ParaLifecycle::OffboardingParachain); }, _ => return Err(Error::::CannotOffboard)?, } + let scheduled_session = Self::scheduled_session(); ActionsQueue::mutate(scheduled_session, |v| { if let Err(i) = v.binary_search(&id) { v.insert(i, id);