From 780f8e09d7f6ed254a585bf435a85a1f08143783 Mon Sep 17 00:00:00 2001
From: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Date: Fri, 19 Jun 2020 15:59:29 +0200
Subject: [PATCH] Root origin use no filter by default. Scheduler and Democracy
dispatch without asserting BaseCallFilter (#6408)
* make system root origin build runtime origin with no filter
* additional doc
---
substrate/frame/democracy/src/tests.rs | 21 +++++++++++++++++++--
substrate/frame/scheduler/src/lib.rs | 17 ++++++++++++++---
substrate/frame/support/src/origin.rs | 24 ++++++++++++++++++++----
substrate/frame/system/src/lib.rs | 3 ++-
4 files changed, 55 insertions(+), 10 deletions(-)
diff --git a/substrate/frame/democracy/src/tests.rs b/substrate/frame/democracy/src/tests.rs
index 85bb1ffcfb7..c1bab3c0212 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 00189c6b5d7..18b4eef0a87 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 038c8540f6e..77fe86cc557 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 71e1f38770d..db6b528bcfb 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.
--
GitLab