From b7afe48ed0bfef30836e7ca6359c2d8bb594d16e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 27 Dec 2024 13:43:13 +0100 Subject: [PATCH 1/5] paras-registrar: Improve error reporting (#6989) This pr improves the error reporting by paras registrar when an owner wants to access a locked parachain. Closes: https://github.com/paritytech/polkadot-sdk/issues/6745 --------- Co-authored-by: command-bot <> --- .../runtime/common/src/paras_registrar/mod.rs | 31 ++++++++++--------- .../common/src/paras_registrar/tests.rs | 2 +- prdoc/pr_6989.prdoc | 10 ++++++ 3 files changed, 27 insertions(+), 16 deletions(-) create mode 100644 prdoc/pr_6989.prdoc diff --git a/polkadot/runtime/common/src/paras_registrar/mod.rs b/polkadot/runtime/common/src/paras_registrar/mod.rs index 07832bba18ed..aed0729c9d51 100644 --- a/polkadot/runtime/common/src/paras_registrar/mod.rs +++ b/polkadot/runtime/common/src/paras_registrar/mod.rs @@ -561,15 +561,16 @@ impl Pallet { origin: ::RuntimeOrigin, 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)?; + if let Ok(who) = ensure_signed(origin.clone()) { + let para_info = Paras::::get(id).ok_or(Error::::NotRegistered)?; + + if para_info.manager == who { ensure!(!para_info.is_locked(), Error::::ParaLocked); - ensure!(para_info.manager == who, Error::::NotOwner); - Ok(()) - }) - .or_else(|_| -> DispatchResult { Self::ensure_root_or_para(origin, id) }) + return Ok(()) + } + } + + Self::ensure_root_or_para(origin, id) } /// Ensure the origin is one of Root or the `para` itself. @@ -577,14 +578,14 @@ impl Pallet { origin: ::RuntimeOrigin, id: ParaId, ) -> DispatchResult { - if let Ok(caller_id) = ensure_parachain(::RuntimeOrigin::from(origin.clone())) - { - // Check if matching para id... - ensure!(caller_id == id, Error::::NotOwner); - } else { - // Check if root... - ensure_root(origin.clone())?; + if ensure_root(origin.clone()).is_ok() { + return Ok(()) } + + let caller_id = ensure_parachain(::RuntimeOrigin::from(origin))?; + // Check if matching para id... + ensure!(caller_id == id, Error::::NotOwner); + Ok(()) } diff --git a/polkadot/runtime/common/src/paras_registrar/tests.rs b/polkadot/runtime/common/src/paras_registrar/tests.rs index 252de8f349da..66fef31c9afd 100644 --- a/polkadot/runtime/common/src/paras_registrar/tests.rs +++ b/polkadot/runtime/common/src/paras_registrar/tests.rs @@ -442,7 +442,7 @@ fn para_lock_works() { // Owner cannot pass origin check when checking lock assert_noop!( mock::Registrar::ensure_root_para_or_owner(RuntimeOrigin::signed(1), para_id), - BadOrigin + Error::::ParaLocked, ); // Owner cannot remove lock. assert_noop!(mock::Registrar::remove_lock(RuntimeOrigin::signed(1), para_id), BadOrigin); diff --git a/prdoc/pr_6989.prdoc b/prdoc/pr_6989.prdoc new file mode 100644 index 000000000000..86c56698d41e --- /dev/null +++ b/prdoc/pr_6989.prdoc @@ -0,0 +1,10 @@ +title: 'paras-registrar: Improve error reporting' +doc: +- audience: Runtime User + description: |- + This pr improves the error reporting by paras registrar when an owner wants to access a locked parachain. + + Closes: https://github.com/paritytech/polkadot-sdk/issues/6745 +crates: +- name: polkadot-runtime-common + bump: patch From b4bf289fe260fbb3e533c7b605b41fb7646f936e Mon Sep 17 00:00:00 2001 From: Ayevbeosa Iyamu Date: Fri, 27 Dec 2024 15:29:31 +0100 Subject: [PATCH 2/5] Update cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- .../runtimes/assets/common/src/fungible_conversion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs b/cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs index 6b5c6d02404c..65cad5d93c7b 100644 --- a/cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs +++ b/cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs @@ -117,7 +117,7 @@ impl< for_tuples!( #( match Tuple::contains(location) { o @ true => return o, _ => () } )* ); - tracing::trace!(target: "xcm::contains", "did not match location: {:?}", &location); + tracing::trace!(target: "xcm::contains", ?location, "did not match location"); false } } From 61c41ed2326f2df9be2d20cfef968d0838b1e0f4 Mon Sep 17 00:00:00 2001 From: Ayevbeosa Iyamu Date: Fri, 27 Dec 2024 15:30:00 +0100 Subject: [PATCH 3/5] Update cumulus/parachains/runtimes/assets/common/src/benchmarks.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- cumulus/parachains/runtimes/assets/common/src/benchmarks.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs b/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs index 44dae5c6f741..615089ad5856 100644 --- a/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs +++ b/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs @@ -40,8 +40,9 @@ impl, SelfParaId: Get, PalletId: Get, L: TryFrom Date: Fri, 27 Dec 2024 15:31:28 +0100 Subject: [PATCH 4/5] Update cumulus/parachains/runtimes/assets/common/src/benchmarks.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- cumulus/parachains/runtimes/assets/common/src/benchmarks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs b/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs index 615089ad5856..d144523aaacb 100644 --- a/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs +++ b/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs @@ -55,7 +55,7 @@ impl, SelfParaId: Get, PalletId: Get, L: TryFrom Date: Fri, 27 Dec 2024 15:31:53 +0100 Subject: [PATCH 5/5] Update cumulus/parachains/runtimes/assets/common/src/matching.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- cumulus/parachains/runtimes/assets/common/src/matching.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/assets/common/src/matching.rs b/cumulus/parachains/runtimes/assets/common/src/matching.rs index 617fd26685ad..96863d638098 100644 --- a/cumulus/parachains/runtimes/assets/common/src/matching.rs +++ b/cumulus/parachains/runtimes/assets/common/src/matching.rs @@ -33,7 +33,7 @@ impl> ContainsPair for IsForeignConcreteAsset { fn contains(asset: &Asset, origin: &Location) -> bool { - tracing::trace!(target: "xcm::contains", "IsForeignConcreteAsset asset: {:?}, origin: {:?}", asset, origin); + tracing::trace!(target: "xcm::contains", ?asset, ?origin, "IsForeignConcreteAsset"); matches!(asset.id, AssetId(ref id) if IsForeign::contains(id, origin)) } }