Introduce stacked filtering (#6273)

* Introduce stacked filtering. * Benchmarks * Remove unneeded crates * Fix proxy type's permissiveness checks. * Repot multisig to make utility stateless. * Repot filter stack impl into macro * Fix wasm build * Tests * Final test. * Tests for the macro * Fix test * Line width * Fix * Update frame/multisig/src/benchmarking.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * Update primitives/std/with_std.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Grumble * Update frame/support/src/traits.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update frame/support/src/traits.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update frame/support/src/traits.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update frame/support/src/traits.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update frame/support/src/traits.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update frame/multisig/src/tests.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * Update frame/multisig/src/tests.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * Grumble * Migration * Grumble * Comments * Migration * Fix * Fix * Line width * Allow unused * Update frame/multisig/src/lib.rs Co-authored-by: Alexander Popiak <alexander.popiak@parity.io> * Fix up grumble. * Remove Utility constraint in NonTransfer Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
2026-04-26 16:57:58 +00:00 · 2020-06-08 13:09:12 +02:00
parent 663cd09be9
commit 15ecac5cb6
19 changed files with 1630 additions and 890 deletions
@@ -40,9 +40,10 @@ use sp_io::hashing::blake2_256;
 use sp_runtime::{DispatchResult, traits::{Dispatchable, Zero}};
 use sp_runtime::traits::Member;
 use frame_support::{
-	decl_module, decl_event, decl_error, decl_storage, Parameter, ensure,
-	traits::{Get, ReservableCurrency, Currency, Filter, InstanceFilter},
-	weights::{GetDispatchInfo, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}},
+	decl_module, decl_event, decl_error, decl_storage, Parameter, ensure, traits::{
+		Get, ReservableCurrency, Currency, Filter, FilterStack, FilterStackGuard,
+		ClearFilterGuard, InstanceFilter
+	}, weights::{GetDispatchInfo, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}},
 	dispatch::{PostDispatchInfo, IsSubType},
 };
 use frame_system::{self as system, ensure_signed};
@@ -65,7 +66,7 @@ pub trait Trait: frame_system::Trait {
 	type Currency: ReservableCurrency<Self::AccountId>;

 	/// Is a given call compatible with the proxying subsystem?
-	type IsCallable: Filter<<Self as Trait>::Call>;
+	type IsCallable: FilterStack<<Self as Trait>::Call>;

 	/// A kind of proxy; specified with the proxy and passed in to the `IsProxyable` fitler.
 	/// The instance filter determines whether a given call may be proxied under this type.
@@ -166,16 +167,22 @@ decl_module! {
 			call: Box<<T as Trait>::Call>
 		) {
 			let who = ensure_signed(origin)?;
-			ensure!(T::IsCallable::filter(&call), Error::<T>::Uncallable);
 			let (_, proxy_type) = Proxies::<T>::get(&real).0.into_iter()
 				.find(|x| &x.0 == &who && force_proxy_type.as_ref().map_or(true, |y| &x.1 == y))
 				.ok_or(Error::<T>::NotProxy)?;
-			match call.is_sub_type() {
-				Some(Call::add_proxy(_, ref pt)) | Some(Call::remove_proxy(_, ref pt)) =>
-					ensure!(pt.is_no_more_permissive(&proxy_type), Error::<T>::NoPermission),
-				_ => (),
-			}
-			ensure!(proxy_type.filter(&call), Error::<T>::Unproxyable);
+
+			// We're now executing as a freshly authenticated new account, so the previous call
+			// restrictions no longer apply.
+			let _clear_guard = ClearFilterGuard::<T::IsCallable, <T as Trait>::Call>::new();
+			let _filter_guard = FilterStackGuard::<T::IsCallable, <T as Trait>::Call>::new(
+				move |c| match c.is_sub_type() {
+					Some(Call::add_proxy(_, ref pt)) | Some(Call::remove_proxy(_, ref pt))
+						if !proxy_type.is_superset(&pt) => false,
+					_ => proxy_type.filter(&c)
+				}
+			);
+			ensure!(T::IsCallable::filter(&call), Error::<T>::Uncallable);
+
 			let e = call.dispatch(frame_system::RawOrigin::Signed(real).into());
 			Self::deposit_event(RawEvent::ProxyExecuted(e.map(|_| ()).map_err(|e| e.error)));
 		}
@@ -23,7 +23,7 @@ use super::*;

 use frame_support::{
 	assert_ok, assert_noop, impl_outer_origin, parameter_types, impl_outer_dispatch,
-	weights::Weight, impl_outer_event, RuntimeDebug, dispatch::DispatchError
+	impl_filter_stack, weights::Weight, impl_outer_event, RuntimeDebug, dispatch::DispatchError
 };
 use codec::{Encode, Decode};
 use sp_core::H256;
@@ -38,6 +38,7 @@ impl_outer_event! {
 		system<T>,
 		pallet_balances<T>,
 		proxy<T>,
+		pallet_utility,
 	}
 }
 impl_outer_dispatch! {
@@ -45,6 +46,7 @@ impl_outer_dispatch! {
 		frame_system::System,
 		pallet_balances::Balances,
 		proxy::Proxy,
+		pallet_utility::Utility,
 	}
 }

@@ -94,30 +96,39 @@ impl pallet_balances::Trait for Test {
 	type ExistentialDeposit = ExistentialDeposit;
 	type AccountStore = System;
 }
+impl pallet_utility::Trait for Test {
+	type Event = TestEvent;
+	type Call = Call;
+	type IsCallable = IsCallable;
+}
 parameter_types! {
 	pub const ProxyDepositBase: u64 = 1;
 	pub const ProxyDepositFactor: u64 = 1;
-	pub const MaxProxies: u16 = 3;
+	pub const MaxProxies: u16 = 4;
 }
+pub struct IsCallable;
+impl_filter_stack!(crate::tests::IsCallable, crate::tests::BaseFilter, crate::tests::Call, is_callable);
 #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, RuntimeDebug)]
 pub enum ProxyType {
 	Any,
 	JustTransfer,
+	JustUtility,
 }
 impl Default for ProxyType { fn default() -> Self { Self::Any } }
 impl InstanceFilter<Call> for ProxyType {
 	fn filter(&self, c: &Call) -> bool {
 		match self {
 			ProxyType::Any => true,
-			ProxyType::JustTransfer => match c {
-				Call::Balances(pallet_balances::Call::transfer(..)) => true,
-				_ => false,
-			}
+			ProxyType::JustTransfer => matches!(c, Call::Balances(pallet_balances::Call::transfer(..))),
+			ProxyType::JustUtility => matches!(c, Call::Utility(..)),
 		}
 	}
+	fn is_superset(&self, o: &Self) -> bool {
+		self == &ProxyType::Any || self == o
+	}
 }
-pub struct TestIsCallable;
-impl Filter<Call> for TestIsCallable {
+pub struct BaseFilter;
+impl Filter<Call> for BaseFilter {
 	fn filter(c: &Call) -> bool {
 		match *c {
 			// Remark is used as a no-op call in the benchmarking
@@ -131,7 +142,7 @@ impl Trait for Test {
 	type Event = TestEvent;
 	type Call = Call;
 	type Currency = Balances;
-	type IsCallable = TestIsCallable;
+	type IsCallable = IsCallable;
 	type ProxyType = ProxyType;
 	type ProxyDepositBase = ProxyDepositBase;
 	type ProxyDepositFactor = ProxyDepositFactor;
@@ -140,11 +151,15 @@ impl Trait for Test {

 type System = frame_system::Module<Test>;
 type Balances = pallet_balances::Module<Test>;
+type Utility = pallet_utility::Module<Test>;
 type Proxy = Module<Test>;

 use frame_system::Call as SystemCall;
 use pallet_balances::Call as BalancesCall;
 use pallet_balances::Error as BalancesError;
+use pallet_utility::Call as UtilityCall;
+use pallet_utility::Error as UtilityError;
+use pallet_utility::Event as UtilityEvent;
 use super::Call as ProxyCall;

 pub fn new_test_ext() -> sp_io::TestExternalities {
@@ -165,6 +180,65 @@ fn expect_event<E: Into<TestEvent>>(e: E) {
 	assert_eq!(last_event(), e.into());
 }

+fn last_events(n: usize) -> Vec<TestEvent> {
+	system::Module::<Test>::events().into_iter().rev().take(n).rev().map(|e| e.event).collect()
+}
+
+fn expect_events(e: Vec<TestEvent>) {
+	assert_eq!(last_events(e.len()), e);
+}
+
+#[test]
+fn filtering_works() {
+	new_test_ext().execute_with(|| {
+		Balances::mutate_account(&1, |a| a.free = 1000);
+		assert_ok!(Proxy::add_proxy(Origin::signed(1), 2, ProxyType::Any));
+		assert_ok!(Proxy::add_proxy(Origin::signed(1), 3, ProxyType::JustTransfer));
+		assert_ok!(Proxy::add_proxy(Origin::signed(1), 4, ProxyType::JustUtility));
+
+		let call = Box::new(Call::Balances(BalancesCall::transfer(6, 1)));
+		assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
+		expect_event(RawEvent::ProxyExecuted(Ok(())));
+		assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()));
+		expect_event(RawEvent::ProxyExecuted(Ok(())));
+		assert_noop!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()), Error::<Test>::Uncallable);
+
+		let sub_id = Utility::sub_account_id(1, 0);
+		Balances::mutate_account(&sub_id, |a| a.free = 1000);
+		let inner = Box::new(Call::Balances(BalancesCall::transfer(6, 1)));
+
+		let call = Box::new(Call::Utility(UtilityCall::as_sub(0, inner.clone())));
+		assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
+		expect_event(RawEvent::ProxyExecuted(Ok(())));
+		assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
+		assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
+		expect_event(RawEvent::ProxyExecuted(Ok(())));
+
+		let call = Box::new(Call::Utility(UtilityCall::as_limited_sub(0, inner.clone())));
+		assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
+		expect_event(RawEvent::ProxyExecuted(Ok(())));
+		assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
+		assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
+		let de = DispatchError::from(UtilityError::<Test>::Uncallable).stripped();
+		expect_event(RawEvent::ProxyExecuted(Err(de)));
+
+		let call = Box::new(Call::Utility(UtilityCall::batch(vec![*inner])));
+		assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
+		expect_events(vec![UtilityEvent::BatchCompleted.into(), RawEvent::ProxyExecuted(Ok(())).into()]);
+		assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
+		assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
+		expect_events(vec![UtilityEvent::Uncallable(0).into(), RawEvent::ProxyExecuted(Ok(())).into()]);
+
+		let inner = Box::new(Call::Proxy(ProxyCall::add_proxy(5, ProxyType::Any)));
+		let call = Box::new(Call::Utility(UtilityCall::batch(vec![*inner])));
+		assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
+		expect_events(vec![UtilityEvent::BatchCompleted.into(), RawEvent::ProxyExecuted(Ok(())).into()]);
+		assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
+		assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
+		expect_events(vec![UtilityEvent::Uncallable(0).into(), RawEvent::ProxyExecuted(Ok(())).into()]);
+	});
+}
+
 #[test]
 fn add_remove_proxies_works() {
 	new_test_ext().execute_with(|| {
@@ -175,8 +249,12 @@ fn add_remove_proxies_works() {
 		assert_eq!(Balances::reserved_balance(1), 3);
 		assert_ok!(Proxy::add_proxy(Origin::signed(1), 3, ProxyType::Any));
 		assert_eq!(Balances::reserved_balance(1), 4);
+		assert_ok!(Proxy::add_proxy(Origin::signed(1), 4, ProxyType::JustUtility));
+		assert_eq!(Balances::reserved_balance(1), 5);
 		assert_noop!(Proxy::add_proxy(Origin::signed(1), 4, ProxyType::Any), Error::<Test>::TooMany);
 		assert_noop!(Proxy::remove_proxy(Origin::signed(1), 3, ProxyType::JustTransfer), Error::<Test>::NotFound);
+		assert_ok!(Proxy::remove_proxy(Origin::signed(1), 4, ProxyType::JustUtility));
+		assert_eq!(Balances::reserved_balance(1), 4);
 		assert_ok!(Proxy::remove_proxy(Origin::signed(1), 3, ProxyType::Any));
 		assert_eq!(Balances::reserved_balance(1), 3);
 		assert_ok!(Proxy::remove_proxy(Origin::signed(1), 2, ProxyType::Any));
@@ -218,7 +296,7 @@ fn proxying_works() {
 		assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);

 		let call = Box::new(Call::Balances(BalancesCall::transfer_keep_alive(6, 1)));
-		assert_noop!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()), Error::<Test>::Unproxyable);
+		assert_noop!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()), Error::<Test>::Uncallable);
 		assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()));
 		expect_event(RawEvent::ProxyExecuted(Ok(())));
 		assert_eq!(Balances::free_balance(6), 2);