From 4553b288cac6cacc383b8404e63c1cb2a82dfd1c Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Wed, 1 Mar 2023 10:15:07 +0000 Subject: [PATCH 1/9] feat: add CallFilter into batch method --- frame/utility/src/lib.rs | 27 +++++++++++++++++---- frame/utility/src/tests.rs | 48 +++++++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index af212a31eb971..f006c9716d14c 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -59,7 +59,7 @@ pub mod weights; use codec::{Decode, Encode}; use frame_support::{ dispatch::{extract_actual_weight, GetDispatchInfo, PostDispatchInfo}, - traits::{IsSubType, OriginTrait, UnfilteredDispatchable}, + traits::{Contains, IsSubType, OriginTrait, UnfilteredDispatchable}, }; use sp_core::TypeId; use sp_io::hashing::blake2_256; @@ -72,7 +72,7 @@ pub use pallet::*; #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::pallet_prelude::*; + use frame_support::{dispatch::DispatchErrorWithPostInfo, pallet_prelude::*}; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -100,6 +100,9 @@ pub mod pallet { /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; + + /// Filtering calls. + type CallFilter: Contains<::RuntimeCall>; } #[pallet::event] @@ -215,12 +218,26 @@ pub mod pallet { let mut weight = Weight::zero(); for (index, call) in calls.into_iter().enumerate() { let info = call.get_dispatch_info(); + + let result = + (!(is_root || ::CallFilter::contains(&call))).then(|| { + DispatchErrorWithPostInfo { + post_info: None::.into(), + error: >::CallFiltered.into(), + } + }); + // If origin is root, don't apply any dispatch filters; root can call anything. - let result = if is_root { - call.dispatch_bypass_filter(origin.clone()) + let result = if let Some(error) = result { + Err(error) } else { - call.dispatch(origin.clone()) + if is_root { + call.dispatch_bypass_filter(origin.clone()) + } else { + call.dispatch(origin.clone()) + } }; + // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); if let Err(e) = result { diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 6f156e318c8cf..1ea4b1250ad62 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -86,6 +86,12 @@ pub mod example { pub fn big_variant(_origin: OriginFor, _arg: [u8; 400]) -> DispatchResult { Ok(()) } + + #[pallet::call_index(3)] + #[pallet::weight(0)] + pub fn not_batchable(_origin: OriginFor, _arg: u8) -> DispatchResult { + Ok(()) + } } } @@ -245,6 +251,17 @@ impl Contains for TestBaseCallFilter { } } } + +pub struct TestCallFilter; +impl Contains for TestCallFilter { + fn contains(c: &RuntimeCall) -> bool { + match *c { + RuntimeCall::Example(example::Call::not_batchable { .. }) => false, + _ => true, + } + } +} + impl mock_democracy::Config for Test { type RuntimeEvent = RuntimeEvent; type ExternalMajorityOrigin = EnsureProportionAtLeast; @@ -254,6 +271,7 @@ impl Config for Test { type RuntimeCall = RuntimeCall; type PalletsOrigin = OriginCaller; type WeightInfo = (); + type CallFilter = TestCallFilter; } type ExampleCall = example::Call; @@ -447,7 +465,7 @@ fn batch_with_signed_works() { } #[test] -fn batch_with_signed_filters() { +fn batch_with_signed_base_filters() { new_test_ext().execute_with(|| { assert_ok!(Utility::batch( RuntimeOrigin::signed(1), @@ -466,6 +484,34 @@ fn batch_with_signed_filters() { }); } +#[test] +fn batch_with_signed_call_filters() { + new_test_ext().execute_with(|| { + assert_ok!(Utility::batch( + RuntimeOrigin::signed(1), + vec![RuntimeCall::Example(example::Call::not_batchable { arg: 0 })] + ),); + System::assert_last_event( + utility::Event::BatchInterrupted { + index: 0, + error: frame_system::Error::::CallFiltered.into(), + } + .into(), + ); + }); +} + +#[test] +fn batch_with_root_call_filters() { + new_test_ext().execute_with(|| { + assert_ok!(Utility::batch( + RuntimeOrigin::root(), + vec![RuntimeCall::Example(example::Call::not_batchable { arg: 0 })] + ),); + System::assert_last_event(utility::Event::BatchCompleted.into()); + }); +} + #[test] fn batch_early_exit_works() { new_test_ext().execute_with(|| { From 380024dc33a054f5251c9449cbe5a185b238f2cf Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Wed, 1 Mar 2023 11:32:41 +0000 Subject: [PATCH 2/9] feat: add CallFilter into as_derevative method --- frame/utility/src/lib.rs | 9 ++++++++- frame/utility/src/tests.rs | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index f006c9716d14c..b2634fddbfabc 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -291,7 +291,14 @@ pub mod pallet { let pseudonym = Self::derivative_account_id(who, index); origin.set_caller_from(frame_system::RawOrigin::Signed(pseudonym)); let info = call.get_dispatch_info(); - let result = call.dispatch(origin); + + let result = + (!::CallFilter::contains(&call)).then(|| DispatchErrorWithPostInfo { + post_info: None::.into(), + error: >::CallFiltered.into(), + }); + + let result = if let Some(error) = result { Err(error) } else { call.dispatch(origin) }; // Always take into account the base weight of this call. let mut weight = T::WeightInfo::as_derivative() .saturating_add(T::DbWeight::get().reads_writes(1, 1)); diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 1ea4b1250ad62..abc7ce6a2034e 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -402,7 +402,7 @@ fn as_derivative_handles_weight_refund() { } #[test] -fn as_derivative_filters() { +fn as_derivative_basic_filters() { new_test_ext().execute_with(|| { assert_err_ignore_postinfo!( Utility::as_derivative( @@ -418,6 +418,20 @@ fn as_derivative_filters() { }); } +#[test] +fn as_derivative_call_filters() { + new_test_ext().execute_with(|| { + assert_err_ignore_postinfo!( + Utility::as_derivative( + RuntimeOrigin::signed(1), + 1, + Box::new(RuntimeCall::Example(example::Call::not_batchable { arg: 0 })), + ), + DispatchError::from(frame_system::Error::::CallFiltered), + ); + }); +} + #[test] fn batch_with_root_works() { new_test_ext().execute_with(|| { From cae4217c2ba561cae02936f1b6758c4cf7793619 Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Wed, 1 Mar 2023 12:16:22 +0000 Subject: [PATCH 3/9] feat: add CallFilter into batch_all method --- frame/utility/src/lib.rs | 35 ++++++++++++++++++++++++----------- frame/utility/src/tests.rs | 26 +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index b2634fddbfabc..14ff7b1a1ba3c 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -361,19 +361,32 @@ pub mod pallet { let mut weight = Weight::zero(); for (index, call) in calls.into_iter().enumerate() { let info = call.get_dispatch_info(); + + let result = + (!(is_root || ::CallFilter::contains(&call))).then(|| { + DispatchErrorWithPostInfo { + post_info: None::.into(), + error: >::CallFiltered.into(), + } + }); + // If origin is root, bypass any dispatch filter; root can call anything. - let result = if is_root { - call.dispatch_bypass_filter(origin.clone()) + let result = if let Some(error) = result { + Err(error) } else { - let mut filtered_origin = origin.clone(); - // Don't allow users to nest `batch_all` calls. - filtered_origin.add_filter( - move |c: &::RuntimeCall| { - let c = ::RuntimeCall::from_ref(c); - !matches!(c.is_sub_type(), Some(Call::batch_all { .. })) - }, - ); - call.dispatch(filtered_origin) + if is_root { + call.dispatch_bypass_filter(origin.clone()) + } else { + let mut filtered_origin = origin.clone(); + // Don't allow users to nest `batch_all` calls. + filtered_origin.add_filter( + move |c: &::RuntimeCall| { + let c = ::RuntimeCall::from_ref(c); + !matches!(c.is_sub_type(), Some(Call::batch_all { .. })) + }, + ); + call.dispatch(filtered_origin) + } }; // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index abc7ce6a2034e..6af46eda157a9 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -23,7 +23,7 @@ use super::*; use crate as utility; use frame_support::{ - assert_err_ignore_postinfo, assert_noop, assert_ok, + assert_err, assert_err_ignore_postinfo, assert_noop, assert_ok, dispatch::{DispatchError, DispatchErrorWithPostInfo, Dispatchable, Pays}, error::BadOrigin, parameter_types, storage, @@ -781,6 +781,30 @@ fn batch_all_does_not_nest() { }); } +#[test] +fn batch_all_with_signed_call_filters() { + new_test_ext().execute_with(|| { + assert_err_ignore_postinfo!( + Utility::batch_all( + RuntimeOrigin::signed(1), + vec![RuntimeCall::Example(example::Call::not_batchable { arg: 0 })] + ), + DispatchError::from(frame_system::Error::::CallFiltered) + ); + }); +} + +#[test] +fn batch_all_with_root_call_filters() { + new_test_ext().execute_with(|| { + assert_ok!(Utility::batch_all( + RuntimeOrigin::root(), + vec![RuntimeCall::Example(example::Call::not_batchable { arg: 0 })] + ),); + System::assert_last_event(utility::Event::BatchCompleted.into()); + }); +} + #[test] fn batch_limit() { new_test_ext().execute_with(|| { From 3a671949e0a4539ee9a82b234a5e83841a7845df Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Wed, 1 Mar 2023 12:56:09 +0000 Subject: [PATCH 4/9] feat: add CallFilter into force_batch method --- frame/utility/src/lib.rs | 19 ++++++++++++++++--- frame/utility/src/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index 14ff7b1a1ba3c..01e6899cb40f1 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -485,11 +485,24 @@ pub mod pallet { let mut has_error: bool = false; for call in calls.into_iter() { let info = call.get_dispatch_info(); + + let result = + (!(is_root || ::CallFilter::contains(&call))).then(|| { + DispatchErrorWithPostInfo { + post_info: None::.into(), + error: >::CallFiltered.into(), + } + }); + // If origin is root, don't apply any dispatch filters; root can call anything. - let result = if is_root { - call.dispatch_bypass_filter(origin.clone()) + let result = if let Some(error) = result { + Err(error) } else { - call.dispatch(origin.clone()) + if is_root { + call.dispatch_bypass_filter(origin.clone()) + } else { + call.dispatch(origin.clone()) + } }; // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 6af46eda157a9..4d3520ab29867 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -852,6 +852,28 @@ fn force_batch_works() { }); } +#[test] +fn force_batch_with_signed_call_filters() { + new_test_ext().execute_with(|| { + assert_ok!(Utility::force_batch( + RuntimeOrigin::signed(1), + vec![RuntimeCall::Example(example::Call::not_batchable { arg: 0 })] + ),); + System::assert_last_event(utility::Event::BatchCompletedWithErrors.into()); + }); +} + +#[test] +fn force_batch_with_root_call_filters() { + new_test_ext().execute_with(|| { + assert_ok!(Utility::force_batch( + RuntimeOrigin::root(), + vec![RuntimeCall::Example(example::Call::not_batchable { arg: 0 })] + ),); + System::assert_last_event(utility::Event::BatchCompleted.into()); + }); +} + #[test] fn none_origin_does_not_work() { new_test_ext().execute_with(|| { From 5d0a1f12156afd38f3449a37e1c714e927e4faca Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Wed, 1 Mar 2023 13:57:39 +0000 Subject: [PATCH 5/9] refactor --- frame/utility/src/lib.rs | 76 ++++++++++++++------------------------ frame/utility/src/tests.rs | 2 +- 2 files changed, 29 insertions(+), 49 deletions(-) diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index 01e6899cb40f1..d90df374aef6d 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -58,7 +58,9 @@ pub mod weights; use codec::{Decode, Encode}; use frame_support::{ - dispatch::{extract_actual_weight, GetDispatchInfo, PostDispatchInfo}, + dispatch::{ + extract_actual_weight, DispatchErrorWithPostInfo, GetDispatchInfo, PostDispatchInfo, + }, traits::{Contains, IsSubType, OriginTrait, UnfilteredDispatchable}, }; use sp_core::TypeId; @@ -72,7 +74,7 @@ pub use pallet::*; #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{dispatch::DispatchErrorWithPostInfo, pallet_prelude::*}; + use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -219,24 +221,14 @@ pub mod pallet { for (index, call) in calls.into_iter().enumerate() { let info = call.get_dispatch_info(); - let result = - (!(is_root || ::CallFilter::contains(&call))).then(|| { - DispatchErrorWithPostInfo { - post_info: None::.into(), - error: >::CallFiltered.into(), - } - }); - - // If origin is root, don't apply any dispatch filters; root can call anything. - let result = if let Some(error) = result { - Err(error) - } else { + let result = Self::check_call_filter(is_root, &call).and_then(|_| { + // If origin is root, don't apply any dispatch filters; root can call anything. if is_root { call.dispatch_bypass_filter(origin.clone()) } else { call.dispatch(origin.clone()) } - }; + }); // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); @@ -292,13 +284,7 @@ pub mod pallet { origin.set_caller_from(frame_system::RawOrigin::Signed(pseudonym)); let info = call.get_dispatch_info(); - let result = - (!::CallFilter::contains(&call)).then(|| DispatchErrorWithPostInfo { - post_info: None::.into(), - error: >::CallFiltered.into(), - }); - - let result = if let Some(error) = result { Err(error) } else { call.dispatch(origin) }; + let result = Self::check_call_filter(false, &call).and_then(|_| call.dispatch(origin)); // Always take into account the base weight of this call. let mut weight = T::WeightInfo::as_derivative() .saturating_add(T::DbWeight::get().reads_writes(1, 1)); @@ -362,18 +348,8 @@ pub mod pallet { for (index, call) in calls.into_iter().enumerate() { let info = call.get_dispatch_info(); - let result = - (!(is_root || ::CallFilter::contains(&call))).then(|| { - DispatchErrorWithPostInfo { - post_info: None::.into(), - error: >::CallFiltered.into(), - } - }); - - // If origin is root, bypass any dispatch filter; root can call anything. - let result = if let Some(error) = result { - Err(error) - } else { + let result = Self::check_call_filter(is_root, &call).and_then(|_| { + // If origin is root, bypass any dispatch filter; root can call anything. if is_root { call.dispatch_bypass_filter(origin.clone()) } else { @@ -387,7 +363,7 @@ pub mod pallet { ); call.dispatch(filtered_origin) } - }; + }); // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); result.map_err(|mut err| { @@ -486,24 +462,14 @@ pub mod pallet { for call in calls.into_iter() { let info = call.get_dispatch_info(); - let result = - (!(is_root || ::CallFilter::contains(&call))).then(|| { - DispatchErrorWithPostInfo { - post_info: None::.into(), - error: >::CallFiltered.into(), - } - }); - - // If origin is root, don't apply any dispatch filters; root can call anything. - let result = if let Some(error) = result { - Err(error) - } else { + let result = Self::check_call_filter(is_root, &call).and_then(|_| { + // If origin is root, don't apply any dispatch filters; root can call anything. if is_root { call.dispatch_bypass_filter(origin.clone()) } else { call.dispatch(origin.clone()) } - }; + }); // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); if let Err(e) = result { @@ -557,4 +523,18 @@ impl Pallet { Decode::decode(&mut TrailingZeroInput::new(entropy.as_ref())) .expect("infinite length input; no invalid inputs for type; qed") } + + fn check_call_filter( + is_root: bool, + call: &::RuntimeCall, + ) -> Result<(), DispatchErrorWithPostInfo> { + if is_root || ::CallFilter::contains(call) { + return Ok(()) + } + + Err(DispatchErrorWithPostInfo { + post_info: None::.into(), + error: >::CallFiltered.into(), + }) + } } diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 4d3520ab29867..29ce7aeb42f4d 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -23,7 +23,7 @@ use super::*; use crate as utility; use frame_support::{ - assert_err, assert_err_ignore_postinfo, assert_noop, assert_ok, + assert_err_ignore_postinfo, assert_noop, assert_ok, dispatch::{DispatchError, DispatchErrorWithPostInfo, Dispatchable, Pays}, error::BadOrigin, parameter_types, storage, From a6f5d1d851376acfcc00496bcf54b961de33ac28 Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Wed, 1 Mar 2023 14:11:36 +0000 Subject: [PATCH 6/9] docs --- frame/utility/src/lib.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index d90df374aef6d..1cb177dd2ff3e 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -173,8 +173,9 @@ pub mod pallet { /// - `calls`: The calls to be dispatched from the same origin. The number of call must not /// exceed the constant: `batched_calls_limit` (available in constant metadata). /// - /// If origin is root then the calls are dispatched without checking origin filter. (This - /// includes bypassing `frame_system::Config::BaseCallFilter`). + /// If the source is root, then calls are sent without checking the origin and call filters. + /// (This involves bypassing `frame_system::Config::BaseCallFilter` and + /// `Config::CallFilter`). /// /// ## Complexity /// - O(C) where C is the number of calls to be batched. @@ -306,8 +307,9 @@ pub mod pallet { /// - `calls`: The calls to be dispatched from the same origin. The number of call must not /// exceed the constant: `batched_calls_limit` (available in constant metadata). /// - /// If origin is root then the calls are dispatched without checking origin filter. (This - /// includes bypassing `frame_system::Config::BaseCallFilter`). + /// If the source is root, then calls are sent without checking the origin and call filters. + /// (This involves bypassing `frame_system::Config::BaseCallFilter` and + /// `Config::CallFilter`). /// /// ## Complexity /// - O(C) where C is the number of calls to be batched. @@ -418,8 +420,9 @@ pub mod pallet { /// - `calls`: The calls to be dispatched from the same origin. The number of call must not /// exceed the constant: `batched_calls_limit` (available in constant metadata). /// - /// If origin is root then the calls are dispatch without checking origin filter. (This - /// includes bypassing `frame_system::Config::BaseCallFilter`). + /// If the source is root, then calls are sent without checking the origin and call filters. + /// (This involves bypassing `frame_system::Config::BaseCallFilter` and + /// `Config::CallFilter`). /// /// ## Complexity /// - O(C) where C is the number of calls to be batched. From b620e6ad5e9d08b96c02473e6223bf5101724f86 Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Thu, 2 Mar 2023 08:56:11 +0000 Subject: [PATCH 7/9] Refactor --- frame/utility/src/lib.rs | 78 ++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index 1cb177dd2ff3e..359e76dc80bb7 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -61,8 +61,11 @@ use frame_support::{ dispatch::{ extract_actual_weight, DispatchErrorWithPostInfo, GetDispatchInfo, PostDispatchInfo, }, + pallet_prelude::DispatchResultWithPostInfo, traits::{Contains, IsSubType, OriginTrait, UnfilteredDispatchable}, + weights::Weight, }; +use frame_system::pallet_prelude::OriginFor; use sp_core::TypeId; use sp_io::hashing::blake2_256; use sp_runtime::traits::{BadOrigin, Dispatchable, TrailingZeroInput}; @@ -222,14 +225,12 @@ pub mod pallet { for (index, call) in calls.into_iter().enumerate() { let info = call.get_dispatch_info(); - let result = Self::check_call_filter(is_root, &call).and_then(|_| { - // If origin is root, don't apply any dispatch filters; root can call anything. - if is_root { - call.dispatch_bypass_filter(origin.clone()) - } else { - call.dispatch(origin.clone()) - } - }); + // If origin is root, don't apply any dispatch filters; root can call anything. + let result = if is_root { + call.dispatch_bypass_filter(origin.clone()) + } else { + Self::dispatch_filtered(origin.clone(), call) + }; // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); @@ -285,7 +286,7 @@ pub mod pallet { origin.set_caller_from(frame_system::RawOrigin::Signed(pseudonym)); let info = call.get_dispatch_info(); - let result = Self::check_call_filter(false, &call).and_then(|_| call.dispatch(origin)); + let result = Self::dispatch_filtered(origin, *call); // Always take into account the base weight of this call. let mut weight = T::WeightInfo::as_derivative() .saturating_add(T::DbWeight::get().reads_writes(1, 1)); @@ -350,22 +351,20 @@ pub mod pallet { for (index, call) in calls.into_iter().enumerate() { let info = call.get_dispatch_info(); - let result = Self::check_call_filter(is_root, &call).and_then(|_| { - // If origin is root, bypass any dispatch filter; root can call anything. - if is_root { - call.dispatch_bypass_filter(origin.clone()) - } else { - let mut filtered_origin = origin.clone(); - // Don't allow users to nest `batch_all` calls. - filtered_origin.add_filter( - move |c: &::RuntimeCall| { - let c = ::RuntimeCall::from_ref(c); - !matches!(c.is_sub_type(), Some(Call::batch_all { .. })) - }, - ); - call.dispatch(filtered_origin) - } - }); + // If origin is root, bypass any dispatch filter; root can call anything. + let result = if is_root { + call.dispatch_bypass_filter(origin.clone()) + } else { + let mut filtered_origin = origin.clone(); + // Don't allow users to nest `batch_all` calls. + filtered_origin.add_filter( + move |c: &::RuntimeCall| { + let c = ::RuntimeCall::from_ref(c); + !matches!(c.is_sub_type(), Some(Call::batch_all { .. })) + }, + ); + Self::dispatch_filtered(filtered_origin, call) + }; // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); result.map_err(|mut err| { @@ -465,14 +464,13 @@ pub mod pallet { for call in calls.into_iter() { let info = call.get_dispatch_info(); - let result = Self::check_call_filter(is_root, &call).and_then(|_| { - // If origin is root, don't apply any dispatch filters; root can call anything. - if is_root { - call.dispatch_bypass_filter(origin.clone()) - } else { - call.dispatch(origin.clone()) - } - }); + // If origin is root, don't apply any dispatch filters; root can call anything. + let result = if is_root { + call.dispatch_bypass_filter(origin.clone()) + } else { + Self::dispatch_filtered(origin.clone(), call) + }; + // Add the weight of this call. weight = weight.saturating_add(extract_actual_weight(&result, &info)); if let Err(e) = result { @@ -527,16 +525,16 @@ impl Pallet { .expect("infinite length input; no invalid inputs for type; qed") } - fn check_call_filter( - is_root: bool, - call: &::RuntimeCall, - ) -> Result<(), DispatchErrorWithPostInfo> { - if is_root || ::CallFilter::contains(call) { - return Ok(()) + fn dispatch_filtered( + origin: OriginFor, + call: ::RuntimeCall, + ) -> DispatchResultWithPostInfo { + if ::CallFilter::contains(&call) { + return call.dispatch(origin) } Err(DispatchErrorWithPostInfo { - post_info: None::.into(), + post_info: Some(Weight::default()).into(), error: >::CallFiltered.into(), }) } From bd04d8a24d1d6edaa434db35bbd44f9d9799a86c Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Fri, 3 Mar 2023 13:41:15 +0000 Subject: [PATCH 8/9] fix: tests --- bin/node/runtime/src/lib.rs | 1 + frame/contracts/src/tests.rs | 3 +++ frame/proxy/src/tests.rs | 3 ++- frame/treasury/src/tests.rs | 3 ++- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 766bc9515b8ab..d0f4d675f471a 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -249,6 +249,7 @@ impl pallet_utility::Config for Runtime { type RuntimeCall = RuntimeCall; type PalletsOrigin = OriginCaller; type WeightInfo = pallet_utility::weights::SubstrateWeight; + type CallFilter = Everything; } parameter_types! { diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index a07176e9f67b9..665ce6c22be4e 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -40,6 +40,8 @@ use frame_support::{ traits::{ ConstU32, ConstU64, Contains, Currency, ExistenceRequirement, LockableCurrency, OnIdle, OnInitialize, WithdrawReasons, + ConstU32, ConstU64, Contains, Currency, Everything, ExistenceRequirement, Get, + LockableCurrency, OnIdle, OnInitialize, WithdrawReasons, }, weights::{constants::WEIGHT_REF_TIME_PER_SECOND, Weight}, }; @@ -337,6 +339,7 @@ impl pallet_utility::Config for Test { type RuntimeCall = RuntimeCall; type PalletsOrigin = OriginCaller; type WeightInfo = (); + type CallFilter = Everything; } impl pallet_proxy::Config for Test { diff --git a/frame/proxy/src/tests.rs b/frame/proxy/src/tests.rs index f3771083c4dd4..79627418f45e8 100644 --- a/frame/proxy/src/tests.rs +++ b/frame/proxy/src/tests.rs @@ -26,7 +26,7 @@ use codec::{Decode, Encode}; use frame_support::{ assert_noop, assert_ok, dispatch::DispatchError, - traits::{ConstU32, ConstU64, Contains}, + traits::{ConstU32, ConstU64, Contains, Everything}, RuntimeDebug, }; use sp_core::H256; @@ -98,6 +98,7 @@ impl pallet_utility::Config for Test { type RuntimeCall = RuntimeCall; type PalletsOrigin = OriginCaller; type WeightInfo = (); + type CallFilter = Everything; } #[derive( diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 67b21ff6252a8..663006b766066 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -29,7 +29,7 @@ use frame_support::{ assert_err_ignore_postinfo, assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, - traits::{ConstU32, ConstU64, OnInitialize}, + traits::{ConstU32, ConstU64, Everything, OnInitialize}, PalletId, }; @@ -101,6 +101,7 @@ impl pallet_utility::Config for Test { type RuntimeCall = RuntimeCall; type PalletsOrigin = OriginCaller; type WeightInfo = (); + type CallFilter = Everything; } parameter_types! { From a991ebf907cc8695e418b6cefabeb952c2ce58ba Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Fri, 3 Mar 2023 13:45:03 +0000 Subject: [PATCH 9/9] fix: remove CallFilter from as_derevative --- frame/utility/src/lib.rs | 2 +- frame/utility/src/tests.rs | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index 359e76dc80bb7..da78ac2d6c25b 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -286,7 +286,7 @@ pub mod pallet { origin.set_caller_from(frame_system::RawOrigin::Signed(pseudonym)); let info = call.get_dispatch_info(); - let result = Self::dispatch_filtered(origin, *call); + let result = call.dispatch(origin); // Always take into account the base weight of this call. let mut weight = T::WeightInfo::as_derivative() .saturating_add(T::DbWeight::get().reads_writes(1, 1)); diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 29ce7aeb42f4d..76c8620f0a323 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -418,20 +418,6 @@ fn as_derivative_basic_filters() { }); } -#[test] -fn as_derivative_call_filters() { - new_test_ext().execute_with(|| { - assert_err_ignore_postinfo!( - Utility::as_derivative( - RuntimeOrigin::signed(1), - 1, - Box::new(RuntimeCall::Example(example::Call::not_batchable { arg: 0 })), - ), - DispatchError::from(frame_system::Error::::CallFiltered), - ); - }); -} - #[test] fn batch_with_root_works() { new_test_ext().execute_with(|| {