This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Manual Para Lock #5451
Merged
paritytech-processbot
merged 14 commits into
master
from
shawntabrizi-remove-para-lock-check
Oct 11, 2022
Merged
Manual Para Lock #5451
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
75b0741
remove para lock check for now
shawntabrizi 634ec65
fmt
shawntabrizi f494ab6
manual para lock
shawntabrizi 229c6dc
expose schedule_code_upgrade and set_current_head
shawntabrizi 4f22311
extrinsics and benchmarks
shawntabrizi d4743ed
use zero
shawntabrizi 95bcb3e
add weights
shawntabrizi e76ff03
fix variable name
shawntabrizi 3256a23
Merge branch 'master' into shawntabrizi-remove-para-lock-check
shawntabrizi f565074
Merge branch 'master' into shawntabrizi-remove-para-lock-check
shawntabrizi 1ca7b21
add and fix comments
shawntabrizi 53e047c
fix weights
shawntabrizi 9b7c552
Merge remote-tracking branch 'origin/master' into shawntabrizi-remove…
ded84e9
add back default lock
shawntabrizi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,8 @@ pub trait WeightInfo { | |
fn force_register() -> Weight; | ||
fn deregister() -> Weight; | ||
fn swap() -> Weight; | ||
fn schedule_code_upgrade(b: u32) -> Weight; | ||
fn set_current_head(b: u32) -> Weight; | ||
} | ||
|
||
pub struct TestWeightInfo; | ||
|
@@ -79,6 +81,12 @@ impl WeightInfo for TestWeightInfo { | |
fn swap() -> Weight { | ||
Weight::zero() | ||
} | ||
fn schedule_code_upgrade(_b: u32) -> Weight { | ||
Weight::zero() | ||
} | ||
fn set_current_head(_b: u32) -> Weight { | ||
Weight::zero() | ||
} | ||
} | ||
|
||
#[frame_support::pallet] | ||
|
@@ -318,11 +326,11 @@ pub mod pallet { | |
/// Remove a manager lock from a para. This will allow the manager of a | ||
/// previously locked para to deregister or swap a para without using governance. | ||
/// | ||
/// Can only be called by the Root origin. | ||
/// Can only be called by the Root origin or the parachain. | ||
#[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] | ||
pub fn force_remove_lock(origin: OriginFor<T>, para: ParaId) -> DispatchResult { | ||
ensure_root(origin)?; | ||
Self::remove_lock(para); | ||
pub fn remove_lock(origin: OriginFor<T>, para: ParaId) -> DispatchResult { | ||
Self::ensure_root_or_para(origin, para)?; | ||
<Self as Registrar>::remove_lock(para); | ||
Ok(()) | ||
} | ||
|
||
|
@@ -348,6 +356,45 @@ pub mod pallet { | |
NextFreeParaId::<T>::set(id + 1); | ||
Ok(()) | ||
} | ||
|
||
/// Add a manager lock from a para. This will prevent the manager of a | ||
/// para to deregister or swap a para. | ||
/// | ||
/// Can be called by Root, the parachain, or the parachain manager if the parachain is unlocked. | ||
#[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] | ||
pub fn add_lock(origin: OriginFor<T>, para: ParaId) -> DispatchResult { | ||
Self::ensure_root_para_or_owner(origin, para)?; | ||
<Self as Registrar>::apply_lock(para); | ||
Ok(()) | ||
} | ||
|
||
/// Schedule a parachain upgrade. | ||
/// | ||
/// Can be called by Root, the parachain, or the parachain manager if the parachain is unlocked. | ||
#[pallet::weight(<T as Config>::WeightInfo::schedule_code_upgrade(new_code.0.len() as u32))] | ||
pub fn schedule_code_upgrade( | ||
origin: OriginFor<T>, | ||
para: ParaId, | ||
new_code: ValidationCode, | ||
) -> DispatchResult { | ||
Self::ensure_root_para_or_owner(origin, para)?; | ||
runtime_parachains::schedule_code_upgrade::<T>(para, new_code)?; | ||
Ok(()) | ||
} | ||
|
||
/// Set the parachain's current head. | ||
/// | ||
/// Can be called by Root, the parachain, or the parachain manager if the parachain is unlocked. | ||
#[pallet::weight(<T as Config>::WeightInfo::set_current_head(new_head.0.len() as u32))] | ||
pub fn set_current_head( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is not tested, only the |
||
origin: OriginFor<T>, | ||
para: ParaId, | ||
new_head: HeadData, | ||
) -> DispatchResult { | ||
Self::ensure_root_para_or_owner(origin, para)?; | ||
runtime_parachains::set_current_head::<T>(para, new_head); | ||
Ok(()) | ||
} | ||
} | ||
} | ||
|
||
|
@@ -379,7 +426,7 @@ impl<T: Config> Registrar for Pallet<T> { | |
Paras::<T>::mutate(id, |x| x.as_mut().map(|mut info| info.locked = true)); | ||
} | ||
|
||
// Apply a lock to the parachain. | ||
// Remove a lock from the parachain. | ||
fn remove_lock(id: ParaId) { | ||
Paras::<T>::mutate(id, |x| x.as_mut().map(|mut info| info.locked = false)); | ||
} | ||
|
@@ -467,17 +514,23 @@ impl<T: Config> Pallet<T> { | |
ensure!(para_info.manager == who, Error::<T>::NotOwner); | ||
Ok(()) | ||
}) | ||
.or_else(|_| -> DispatchResult { | ||
// Else check if para origin... | ||
let caller_id = | ||
ensure_parachain(<T as Config>::RuntimeOrigin::from(origin.clone()))?; | ||
ensure!(caller_id == id, Error::<T>::NotOwner); | ||
Ok(()) | ||
}) | ||
.or_else(|_| -> DispatchResult { | ||
// Check if root... | ||
ensure_root(origin.clone()).map_err(|e| e.into()) | ||
}) | ||
.or_else(|_| -> DispatchResult { Self::ensure_root_or_para(origin, id) }) | ||
} | ||
|
||
/// Ensure the origin is one of Root or the `para` itself. | ||
fn ensure_root_or_para( | ||
shawntabrizi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
origin: <T as frame_system::Config>::RuntimeOrigin, | ||
id: ParaId, | ||
) -> DispatchResult { | ||
if let Ok(caller_id) = ensure_parachain(<T as Config>::RuntimeOrigin::from(origin.clone())) | ||
{ | ||
// Check if matching para id... | ||
ensure!(caller_id == id, Error::<T>::NotOwner); | ||
} else { | ||
// Check if root... | ||
ensure_root(origin.clone())?; | ||
} | ||
Ok(()) | ||
} | ||
|
||
fn do_reserve( | ||
|
@@ -1087,21 +1140,20 @@ mod tests { | |
vec![1, 2, 3].into(), | ||
)); | ||
|
||
// Owner can call swap | ||
assert_ok!(Registrar::swap(RuntimeOrigin::signed(1), para_id, para_id + 1)); | ||
|
||
// 2 session changes to fully onboard. | ||
run_to_session(2); | ||
assert_eq!(Parachains::lifecycle(para_id), Some(ParaLifecycle::Parathread)); | ||
|
||
assert_noop!(Registrar::add_lock(RuntimeOrigin::signed(2), para_id), BadOrigin); | ||
// Once they begin onboarding, we lock them in. | ||
assert_ok!(Registrar::make_parachain(para_id)); | ||
|
||
// Owner cannot call swap anymore | ||
assert_ok!(Registrar::add_lock(RuntimeOrigin::signed(1), para_id)); | ||
// Owner cannot pass origin check when checking lock | ||
assert_noop!( | ||
Registrar::swap(RuntimeOrigin::signed(1), para_id, para_id + 2), | ||
Registrar::ensure_root_para_or_owner(RuntimeOrigin::signed(1), para_id), | ||
BadOrigin | ||
); | ||
// Owner cannot remove lock. | ||
assert_noop!(Registrar::remove_lock(RuntimeOrigin::signed(1), para_id), BadOrigin); | ||
// Para can. | ||
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)); | ||
}); | ||
} | ||
|
||
|
@@ -1227,6 +1279,7 @@ mod benchmarking { | |
use crate::traits::Registrar as RegistrarT; | ||
use frame_support::assert_ok; | ||
use frame_system::RawOrigin; | ||
use primitives::v2::{MAX_CODE_SIZE, MAX_HEAD_DATA_SIZE}; | ||
use runtime_parachains::{paras, shared, Origin as ParaOrigin}; | ||
use sp_runtime::traits::Bounded; | ||
|
||
|
@@ -1343,6 +1396,18 @@ mod benchmarking { | |
assert_eq!(paras::Pallet::<T>::lifecycle(parathread), Some(ParaLifecycle::Parachain)); | ||
} | ||
|
||
schedule_code_upgrade { | ||
let b in 1 .. MAX_CODE_SIZE; | ||
let new_code = ValidationCode(vec![0; b as usize]); | ||
let para_id = ParaId::from(1000); | ||
}: _(RawOrigin::Root, para_id, new_code) | ||
|
||
set_current_head { | ||
let b in 1 .. MAX_HEAD_DATA_SIZE; | ||
let new_head = HeadData(vec![0; b as usize]); | ||
let para_id = ParaId::from(1000); | ||
}: _(RawOrigin::Root, para_id, new_head) | ||
|
||
impl_benchmark_test_suite!( | ||
Registrar, | ||
crate::integration_tests::new_test_ext(), | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We somehow need to tell people to use this then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand correctly that a user has no guarantee that the chain will ever become trustless?
Maybe we can add a block number to the config so that after that block the chain can be locked by anyone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that is not our job nor the job of the relay chain to ensure that a chain is trustless. Maybe this is some very specific chain controlled by one entity. I think it would be fine if this entity keeps control all the time. I mean at some point there is maybe one parachain that controlls another parachain. If the first parachain is trustless, the second one is trustless as well :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, we cant make assumptions about the origin. So if its already trust-less, then its not an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My take on this is that we need to tell the UIs to put a "warning" flag on parachains which are unlocked, which inform users of the power that these parachains keep centralized. If the parachains want to remove this UI element, they will call this function.