Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Adding CallFilter to pallet-utility. #13523

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ impl pallet_utility::Config for Runtime {
type RuntimeCall = RuntimeCall;
type PalletsOrigin = OriginCaller;
type WeightInfo = pallet_utility::weights::SubstrateWeight<Runtime>;
type CallFilter = Everything;
}

parameter_types! {
Expand Down
3 changes: 3 additions & 0 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion frame/proxy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -98,6 +98,7 @@ impl pallet_utility::Config for Test {
type RuntimeCall = RuntimeCall;
type PalletsOrigin = OriginCaller;
type WeightInfo = ();
type CallFilter = Everything;
}

#[derive(
Expand Down
3 changes: 2 additions & 1 deletion frame/treasury/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -101,6 +101,7 @@ impl pallet_utility::Config for Test {
type RuntimeCall = RuntimeCall;
type PalletsOrigin = OriginCaller;
type WeightInfo = ();
type CallFilter = Everything;
}

parameter_types! {
Expand Down
53 changes: 42 additions & 11 deletions frame/utility/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,14 @@ pub mod weights;

use codec::{Decode, Encode};
use frame_support::{
dispatch::{extract_actual_weight, GetDispatchInfo, PostDispatchInfo},
traits::{IsSubType, OriginTrait, UnfilteredDispatchable},
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};
Expand Down Expand Up @@ -100,6 +105,9 @@ pub mod pallet {

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;

/// Filtering calls.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be worth mentioning that this is applied to inner dispatches of certain calls, and how it related to system's base-call filter.

type CallFilter: Contains<<Self as Config>::RuntimeCall>;
}

#[pallet::event]
Expand Down Expand Up @@ -168,8 +176,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.
Expand Down Expand Up @@ -215,12 +224,14 @@ pub mod pallet {
let mut weight = Weight::zero();
for (index, call) in calls.into_iter().enumerate() {
let info = call.get_dispatch_info();

// 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 {
call.dispatch(origin.clone())
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 {
Expand Down Expand Up @@ -274,6 +285,7 @@ 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);
// Always take into account the base weight of this call.
let mut weight = T::WeightInfo::as_derivative()
Expand All @@ -296,8 +308,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.
Expand Down Expand Up @@ -337,6 +350,7 @@ pub mod pallet {
let mut weight = Weight::zero();
for (index, call) in calls.into_iter().enumerate() {
let info = call.get_dispatch_info();

// If origin is root, bypass any dispatch filter; root can call anything.
let result = if is_root {
call.dispatch_bypass_filter(origin.clone())
Expand All @@ -349,7 +363,7 @@ pub mod pallet {
!matches!(c.is_sub_type(), Some(Call::batch_all { .. }))
},
);
call.dispatch(filtered_origin)
Self::dispatch_filtered(filtered_origin, call)
};
// Add the weight of this call.
weight = weight.saturating_add(extract_actual_weight(&result, &info));
Expand Down Expand Up @@ -405,8 +419,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.
Expand Down Expand Up @@ -448,12 +463,14 @@ pub mod pallet {
let mut has_error: bool = false;
for call in calls.into_iter() {
let info = call.get_dispatch_info();

// 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 {
call.dispatch(origin.clone())
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 {
Expand Down Expand Up @@ -507,4 +524,18 @@ impl<T: Config> Pallet<T> {
Decode::decode(&mut TrailingZeroInput::new(entropy.as_ref()))
.expect("infinite length input; no invalid inputs for type; qed")
}

fn dispatch_filtered(
origin: OriginFor<T>,
call: <T as Config>::RuntimeCall,
) -> DispatchResultWithPostInfo {
if <T as Config>::CallFilter::contains(&call) {
return call.dispatch(origin)
}

Err(DispatchErrorWithPostInfo {
post_info: Some(Weight::default()).into(),
error: <frame_system::Error<T>>::CallFiltered.into(),
})
}
}
96 changes: 94 additions & 2 deletions frame/utility/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ pub mod example {
pub fn big_variant(_origin: OriginFor<T>, _arg: [u8; 400]) -> DispatchResult {
Ok(())
}

#[pallet::call_index(3)]
#[pallet::weight(0)]
pub fn not_batchable(_origin: OriginFor<T>, _arg: u8) -> DispatchResult {
Ok(())
}
}
}

Expand Down Expand Up @@ -245,6 +251,17 @@ impl Contains<RuntimeCall> for TestBaseCallFilter {
}
}
}

pub struct TestCallFilter;
impl Contains<RuntimeCall> 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<u64, Instance1, 3, 4>;
Expand All @@ -254,6 +271,7 @@ impl Config for Test {
type RuntimeCall = RuntimeCall;
type PalletsOrigin = OriginCaller;
type WeightInfo = ();
type CallFilter = TestCallFilter;
}

type ExampleCall = example::Call<Test>;
Expand Down Expand Up @@ -384,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(
Expand Down Expand Up @@ -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),
Expand All @@ -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::<Test>::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(|| {
Expand Down Expand Up @@ -721,6 +767,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::<Test>::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(|| {
Expand Down Expand Up @@ -768,6 +838,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(|| {
Expand Down