XCM Transact safe call filter update (#956)

* XCM Transact safe call filter update * Revert unintended Astar changes
AstarNetwork · Jun 13, 2023 · 24df29c · 24df29c
1 parent 146c822
commit 24df29c
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 52 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/collator/Cargo.toml b/bin/collator/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "astar-collator"
-version = "5.10.0"
+version = "5.11.0"
 description = "Astar collator implementation in Rust."
 build = "build.rs"
 default-run = "astar-collator"

diff --git a/runtime/astar/Cargo.toml b/runtime/astar/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "astar-runtime"
-version = "5.10.0"
+version = "5.11.0"
 build = "build.rs"
 authors.workspace = true
 edition.workspace = true

diff --git a/runtime/astar/src/lib.rs b/runtime/astar/src/lib.rs
@@ -139,7 +139,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("astar"),
     impl_name: create_runtime_str!("astar"),
     authoring_version: 1,
-    spec_version: 60,
+    spec_version: 61,
     impl_version: 0,
     apis: RUNTIME_API_VERSIONS,
     transaction_version: 2,

diff --git a/runtime/astar/src/xcm_config.rs b/runtime/astar/src/xcm_config.rs
@@ -144,17 +144,10 @@ match_types! {
 
 /// A call filter for the XCM Transact instruction. This is a temporary measure until we properly
 /// account for proof size weights.
-///
-/// Calls that are allowed through this filter must:
-/// 1. Have a fixed weight;
-/// 2. Cannot lead to another call being made
-/// 3. Have a defined proof size weight, e.g. no unbounded vecs in call parameters. - TODO: shouldn't max XCM weight handle this?
 pub struct SafeCallFilter;
-
 impl SafeCallFilter {
-    // 1. RuntimeCall::Multisig(..) - contains `Vec` in argument so we should avoid this
-    // 2. RuntimeCall::EVM(..) & RuntimeCall::Ethereum(..) have to be prohibited since we cannot measure PoV size properly
-    // 3. RuntimeCall::Contracts(..) it should be safe to allow for such calls but perhaps it's better to do more delibrate testing on Shibuya/RocStar.
+    // 1. RuntimeCall::EVM(..) & RuntimeCall::Ethereum(..) have to be prohibited since we cannot measure PoV size properly
+    // 2. RuntimeCall::Contracts(..) can be allowed, but it hasn't been tested properly yet.
 
     /// Checks whether the base (non-composite) call is allowed to be executed via `Transact` XCM instruction.
     pub fn allow_base_call(call: &RuntimeCall) -> bool {
@@ -166,25 +159,58 @@ impl SafeCallFilter {
             | RuntimeCall::DappsStaking(..)
             | RuntimeCall::Assets(..)
             | RuntimeCall::PolkadotXcm(..)
-            | RuntimeCall::Session(..) => true,
+            | RuntimeCall::Session(..)
+            | RuntimeCall::Proxy(
+                pallet_proxy::Call::add_proxy { .. }
+                | pallet_proxy::Call::remove_proxy { .. }
+                | pallet_proxy::Call::remove_proxies { .. }
+                | pallet_proxy::Call::create_pure { .. }
+                | pallet_proxy::Call::kill_pure { .. }
+                | pallet_proxy::Call::announce { .. }
+                | pallet_proxy::Call::remove_announcement { .. }
+                | pallet_proxy::Call::reject_announcement { .. },
+            )
+            | RuntimeCall::Multisig(
+                pallet_multisig::Call::approve_as_multi { .. }
+                | pallet_multisig::Call::cancel_as_multi { .. },
+            ) => true,
+            _ => false,
+        }
+    }
+    /// Checks whether composite call is allowed to be executed via `Transact` XCM instruction.
+    ///
+    /// Each composite call's subcalls are checked against base call filter. No nesting of composite calls is allowed.
+    pub fn allow_composite_call(call: &RuntimeCall) -> bool {
+        match call {
+            RuntimeCall::Proxy(pallet_proxy::Call::proxy { call, .. }) => {
+                Self::allow_base_call(call)
+            }
+            RuntimeCall::Proxy(pallet_proxy::Call::proxy_announced { call, .. }) => {
+                Self::allow_base_call(call)
+            }
+            RuntimeCall::Utility(pallet_utility::Call::batch { calls, .. }) => {
+                calls.iter().all(|call| Self::allow_base_call(call))
+            }
+            RuntimeCall::Utility(pallet_utility::Call::batch_all { calls, .. }) => {
+                calls.iter().all(|call| Self::allow_base_call(call))
+            }
+            RuntimeCall::Utility(pallet_utility::Call::as_derivative { call, .. }) => {
+                Self::allow_base_call(call)
+            }
+            RuntimeCall::Multisig(pallet_multisig::Call::as_multi_threshold_1 { call, .. }) => {
+                Self::allow_base_call(call)
+            }
+            RuntimeCall::Multisig(pallet_multisig::Call::as_multi { call, .. }) => {
+                Self::allow_base_call(call)
+            }
             _ => false,
         }
     }
 }
 
 impl Contains<RuntimeCall> for SafeCallFilter {
     fn contains(call: &RuntimeCall) -> bool {
-        #[cfg(feature = "runtime-benchmarks")]
-        {
-            if matches!(
-                call,
-                RuntimeCall::System(frame_system::Call::remark_with_event { .. })
-            ) {
-                return true;
-            }
-        }
-
-        Self::allow_base_call(call)
+        Self::allow_base_call(call) || Self::allow_composite_call(call)
     }
 }
 

diff --git a/runtime/local/Cargo.toml b/runtime/local/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "local-runtime"
-version = "5.10.0"
+version = "5.11.0"
 build = "build.rs"
 authors.workspace = true
 edition.workspace = true

diff --git a/runtime/shibuya/Cargo.toml b/runtime/shibuya/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "shibuya-runtime"
-version = "5.10.0"
+version = "5.11.0"
 build = "build.rs"
 authors.workspace = true
 edition.workspace = true

diff --git a/runtime/shiden/Cargo.toml b/runtime/shiden/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "shiden-runtime"
-version = "5.10.0"
+version = "5.11.0"
 build = "build.rs"
 authors.workspace = true
 edition.workspace = true

diff --git a/runtime/shiden/src/lib.rs b/runtime/shiden/src/lib.rs
@@ -139,7 +139,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("shiden"),
     impl_name: create_runtime_str!("shiden"),
     authoring_version: 1,
-    spec_version: 100,
+    spec_version: 101,
     impl_version: 0,
     apis: RUNTIME_API_VERSIONS,
     transaction_version: 2,

diff --git a/runtime/shiden/src/xcm_config.rs b/runtime/shiden/src/xcm_config.rs
@@ -150,17 +150,10 @@ match_types! {
 
 /// A call filter for the XCM Transact instruction. This is a temporary measure until we properly
 /// account for proof size weights.
-///
-/// Calls that are allowed through this filter must:
-/// 1. Have a fixed weight;
-/// 2. Cannot lead to another call being made (Astar: we slightly loosen this requirement)
-/// 3. Have a defined proof size weight, e.g. no unbounded vecs in call parameters. - TODO: shouldn't max XCM weight handle this?
 pub struct SafeCallFilter;
-
 impl SafeCallFilter {
-    // 1. RuntimeCall::Multisig(..) - contains `Vec` in argument so we should avoid this
-    // 2. RuntimeCall::EVM(..) & RuntimeCall::Ethereum(..) have to be prohibited since we cannot measure PoV size properly
-    // 3. RuntimeCall::Contracts(..) it should be safe to allow for such calls but perhaps it's better to do more delibrate testing on Shibuya/RocStar.
+    // 1. RuntimeCall::EVM(..) & RuntimeCall::Ethereum(..) have to be prohibited since we cannot measure PoV size properly
+    // 2. RuntimeCall::Contracts(..) can be allowed, but it hasn't been tested properly yet.
 
     /// Checks whether the base (non-composite) call is allowed to be executed via `Transact` XCM instruction.
     pub fn allow_base_call(call: &RuntimeCall) -> bool {
@@ -182,6 +175,10 @@ impl SafeCallFilter {
                 | pallet_proxy::Call::announce { .. }
                 | pallet_proxy::Call::remove_announcement { .. }
                 | pallet_proxy::Call::reject_announcement { .. },
+            )
+            | RuntimeCall::Multisig(
+                pallet_multisig::Call::approve_as_multi { .. }
+                | pallet_multisig::Call::cancel_as_multi { .. },
             ) => true,
             _ => false,
         }
@@ -206,23 +203,19 @@ impl SafeCallFilter {
             RuntimeCall::Utility(pallet_utility::Call::as_derivative { call, .. }) => {
                 Self::allow_base_call(call)
             }
+            RuntimeCall::Multisig(pallet_multisig::Call::as_multi_threshold_1 { call, .. }) => {
+                Self::allow_base_call(call)
+            }
+            RuntimeCall::Multisig(pallet_multisig::Call::as_multi { call, .. }) => {
+                Self::allow_base_call(call)
+            }
             _ => false,
         }
     }
 }
 
 impl Contains<RuntimeCall> for SafeCallFilter {
     fn contains(call: &RuntimeCall) -> bool {
-        #[cfg(feature = "runtime-benchmarks")]
-        {
-            if matches!(
-                call,
-                RuntimeCall::System(frame_system::Call::remark_with_event { .. })
-            ) {
-                return true;
-            }
-        }
-
         Self::allow_base_call(call) || Self::allow_composite_call(call)
     }
 }