diff --git a/substrate/frame/democracy/src/tests.rs b/substrate/frame/democracy/src/tests.rs index 85bb1ffcfb76e797bba062e5caf5cb44cb585974..c1bab3c0212e318cbb8822af14bd03c7a9987b80 100644 --- a/substrate/frame/democracy/src/tests.rs +++ b/substrate/frame/democracy/src/tests.rs @@ -22,7 +22,8 @@ use std::cell::RefCell; use codec::Encode; use frame_support::{ impl_outer_origin, impl_outer_dispatch, assert_noop, assert_ok, parameter_types, - impl_outer_event, ord_parameter_types, traits::{Contains, OnInitialize}, weights::Weight, + impl_outer_event, ord_parameter_types, traits::{Contains, OnInitialize, Filter}, + weights::Weight, }; use sp_core::H256; use sp_runtime::{ @@ -74,6 +75,14 @@ impl_outer_event! { } } +// Test that a fitlered call can be dispatched. +pub struct BaseFilter; +impl Filter<Call> for BaseFilter { + fn filter(call: &Call) -> bool { + !matches!(call, &Call::Balances(pallet_balances::Call::set_balance(..))) + } +} + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, Eq, PartialEq, Debug)] pub struct Test; @@ -84,7 +93,7 @@ parameter_types! { pub const AvailableBlockRatio: Perbill = Perbill::one(); } impl frame_system::Trait for Test { - type BaseCallFilter = (); + type BaseCallFilter = BaseFilter; type Origin = Origin; type Index = u64; type BlockNumber = u64; @@ -225,6 +234,14 @@ fn set_balance_proposal(value: u64) -> Vec<u8> { Call::Balances(pallet_balances::Call::set_balance(42, value, 0)).encode() } +#[test] +fn set_balance_proposal_is_correctly_filtered_out() { + for i in 0..10 { + let call = Call::decode(&mut &set_balance_proposal(i)[..]).unwrap(); + assert!(!<Test as frame_system::Trait>::BaseCallFilter::filter(&call)); + } +} + fn set_balance_proposal_hash(value: u64) -> H256 { BlakeTwo256::hash(&set_balance_proposal(value)[..]) } diff --git a/substrate/frame/scheduler/src/lib.rs b/substrate/frame/scheduler/src/lib.rs index 00189c6b5d778bfaafb92c35ffed2eb2cf1d5a42..18b4eef0a87dfe4ff1e971b5218b1395b6914acb 100644 --- a/substrate/frame/scheduler/src/lib.rs +++ b/substrate/frame/scheduler/src/lib.rs @@ -399,7 +399,7 @@ mod tests { use frame_support::{ impl_outer_event, impl_outer_origin, impl_outer_dispatch, parameter_types, assert_ok, - traits::{OnInitialize, OnFinalize}, + traits::{OnInitialize, OnFinalize, Filter}, weights::constants::RocksDbWeight, }; use sp_core::H256; @@ -469,6 +469,15 @@ mod tests { scheduler<T>, } } + + // Scheduler must dispatch with root and no filter, this tests base filter is indeed not used. + pub struct BaseFilter; + impl Filter<Call> for BaseFilter { + fn filter(call: &Call) -> bool { + !matches!(call, Call::Logger(_)) + } + } + // For testing the pallet, we construct most of a mock runtime. This means // first constructing a configuration type (`Test`) which `impl`s each of the // configuration traits of pallets we want to use. @@ -481,7 +490,7 @@ mod tests { pub const AvailableBlockRatio: Perbill = Perbill::one(); } impl system::Trait for Test { - type BaseCallFilter = (); + type BaseCallFilter = BaseFilter; type Origin = Origin; type Call = Call; type Index = u64; @@ -540,7 +549,9 @@ mod tests { #[test] fn basic_scheduling_works() { new_test_ext().execute_with(|| { - Scheduler::do_schedule(4, None, 127, Call::Logger(logger::Call::log(42, 1000))); + let call = Call::Logger(logger::Call::log(42, 1000)); + assert!(!<Test as frame_system::Trait>::BaseCallFilter::filter(&call)); + Scheduler::do_schedule(4, None, 127, call); run_to_block(3); assert!(logger::log().is_empty()); run_to_block(4); diff --git a/substrate/frame/support/src/origin.rs b/substrate/frame/support/src/origin.rs index 038c8540f6eca817b91372e06f48de9f80a87687..77fe86cc5575bf9ac2f1b157e3571eede59e4b4e 100644 --- a/substrate/frame/support/src/origin.rs +++ b/substrate/frame/support/src/origin.rs @@ -163,8 +163,8 @@ macro_rules! impl_outer_origin { Modules { }; $( $module:ident $( < $generic:ident > )? $( { $generic_instance:ident } )? ,)* ) => { - // WARNING: All instance must hold the filter `frame_system::Trait::BaseCallFilter`. - // One can use `OriginTrait::reset_filter` to do so. + // WARNING: All instance must hold the filter `frame_system::Trait::BaseCallFilter`, except + // when caller is system Root. One can use `OriginTrait::reset_filter` to do so. #[derive(Clone)] pub struct $name { caller: $caller_name, @@ -241,28 +241,40 @@ macro_rules! impl_outer_origin { #[allow(dead_code)] impl $name { + /// Create with system none origin and `frame-system::Trait::BaseCallFilter`. pub fn none() -> Self { $system::RawOrigin::None.into() } + /// Create with system root origin and no filter. pub fn root() -> Self { $system::RawOrigin::Root.into() } + /// Create with system signed origin and `frame-system::Trait::BaseCallFilter`. pub fn signed(by: <$runtime as $system::Trait>::AccountId) -> Self { $system::RawOrigin::Signed(by).into() } } impl From<$system::Origin<$runtime>> for $name { + /// Convert to runtime origin: + /// * root origin is built with no filter + /// * others use `frame-system::Trait::BaseCallFilter` fn from(x: $system::Origin<$runtime>) -> Self { let mut o = $name { caller: $caller_name::system(x), filter: $crate::sp_std::rc::Rc::new(Box::new(|_| true)), }; - $crate::traits::OriginTrait::reset_filter(&mut o); + + // Root has no filter + if !matches!(o.caller, $caller_name::system($system::Origin::<$runtime>::Root)) { + $crate::traits::OriginTrait::reset_filter(&mut o); + } + o } } impl Into<$crate::sp_std::result::Result<$system::Origin<$runtime>, $name>> for $name { + /// NOTE: converting to pallet origin loses the origin filter information. fn into(self) -> $crate::sp_std::result::Result<$system::Origin<$runtime>, Self> { if let $caller_name::system(l) = self.caller { Ok(l) @@ -272,6 +284,8 @@ macro_rules! impl_outer_origin { } } impl From<Option<<$runtime as $system::Trait>::AccountId>> for $name { + /// Convert to runtime origin with caller being system signed or none and use filter + /// `frame-system::Trait::BaseCallFilter`. fn from(x: Option<<$runtime as $system::Trait>::AccountId>) -> Self { <$system::Origin<$runtime>>::from(x).into() } @@ -279,6 +293,7 @@ macro_rules! impl_outer_origin { $( $crate::paste::item! { impl From<$module::Origin < $( $generic )? $(, $module::$generic_instance )? > > for $name { + /// Convert to runtime origin using `frame-system::Trait::BaseCallFilter`. fn from(x: $module::Origin < $( $generic )? $(, $module::$generic_instance )? >) -> Self { let mut o = $name { caller: $caller_name::[< $module $( _ $generic_instance )? >](x), @@ -294,6 +309,7 @@ macro_rules! impl_outer_origin { $name, >> for $name { + /// NOTE: converting to pallet origin loses the origin filter information. fn into(self) -> $crate::sp_std::result::Result< $module::Origin < $( $generic )? $(, $module::$generic_instance )? >, Self, @@ -402,7 +418,7 @@ mod tests { #[test] fn test_default_filter() { assert_eq!(OriginWithSystem::root().filter_call(&0), true); - assert_eq!(OriginWithSystem::root().filter_call(&1), false); + assert_eq!(OriginWithSystem::root().filter_call(&1), true); assert_eq!(OriginWithSystem::none().filter_call(&0), true); assert_eq!(OriginWithSystem::none().filter_call(&1), false); assert_eq!(OriginWithSystem::signed(0).filter_call(&0), true); diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 71e1f38770d0e2e7a69ae8e64c5b03dcfe639195..db6b528bcfbc95ed2b3018d180727caa4e600261 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -149,7 +149,8 @@ pub fn extrinsics_data_root<H: Hash>(xts: Vec<Vec<u8>>) -> H::Output { } pub trait Trait: 'static + Eq + Clone { - /// The basic call filter to use in Origin. + /// The basic call filter to use in Origin. All origins are built with this filter as base, + /// except Root. type BaseCallFilter: Filter<Self::Call>; /// The `Origin` type used by dispatchable calls.