Skip to content

Commit 8c91a3e

Browse files
gavofyorkshawntabrizibkchrapopiak
authored
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>
1 parent 8935549 commit 8c91a3e

File tree

3 files changed

+107
-21
lines changed

3 files changed

+107
-21
lines changed

‎Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ frame-benchmarking = { version = "2.0.0-rc2", default-features = false, path = "
2626
[dev-dependencies]
2727
sp-core = { version = "2.0.0-rc2", path = "../../primitives/core" }
2828
pallet-balances = { version = "2.0.0-rc2", path = "../balances" }
29+
pallet-utility = { version = "2.0.0-rc2", path = "../utility" }
2930

3031
[features]
3132
default = ["std"]

‎src/lib.rs

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ use sp_io::hashing::blake2_256;
4040
use sp_runtime::{DispatchResult, traits::{Dispatchable, Zero}};
4141
use sp_runtime::traits::Member;
4242
use frame_support::{
43-
decl_module, decl_event, decl_error, decl_storage, Parameter, ensure,
44-
traits::{Get, ReservableCurrency, Currency, Filter, InstanceFilter},
45-
weights::{GetDispatchInfo, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}},
43+
decl_module, decl_event, decl_error, decl_storage, Parameter, ensure, traits::{
44+
Get, ReservableCurrency, Currency, Filter, FilterStack, FilterStackGuard,
45+
ClearFilterGuard, InstanceFilter
46+
}, weights::{GetDispatchInfo, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}},
4647
dispatch::{PostDispatchInfo, IsSubType},
4748
};
4849
use frame_system::{self as system, ensure_signed};
@@ -65,7 +66,7 @@ pub trait Trait: frame_system::Trait {
6566
type Currency: ReservableCurrency<Self::AccountId>;
6667

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

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

‎src/tests.rs

Lines changed: 88 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use super::*;
2323

2424
use frame_support::{
2525
assert_ok, assert_noop, impl_outer_origin, parameter_types, impl_outer_dispatch,
26-
weights::Weight, impl_outer_event, RuntimeDebug, dispatch::DispatchError
26+
impl_filter_stack, weights::Weight, impl_outer_event, RuntimeDebug, dispatch::DispatchError
2727
};
2828
use codec::{Encode, Decode};
2929
use sp_core::H256;
@@ -38,13 +38,15 @@ impl_outer_event! {
3838
system<T>,
3939
pallet_balances<T>,
4040
proxy<T>,
41+
pallet_utility,
4142
}
4243
}
4344
impl_outer_dispatch! {
4445
pub enum Call for Test where origin: Origin {
4546
frame_system::System,
4647
pallet_balances::Balances,
4748
proxy::Proxy,
49+
pallet_utility::Utility,
4850
}
4951
}
5052

@@ -94,30 +96,39 @@ impl pallet_balances::Trait for Test {
9496
type ExistentialDeposit = ExistentialDeposit;
9597
type AccountStore = System;
9698
}
99+
impl pallet_utility::Trait for Test {
100+
type Event = TestEvent;
101+
type Call = Call;
102+
type IsCallable = IsCallable;
103+
}
97104
parameter_types! {
98105
pub const ProxyDepositBase: u64 = 1;
99106
pub const ProxyDepositFactor: u64 = 1;
100-
pub const MaxProxies: u16 = 3;
107+
pub const MaxProxies: u16 = 4;
101108
}
109+
pub struct IsCallable;
110+
impl_filter_stack!(crate::tests::IsCallable, crate::tests::BaseFilter, crate::tests::Call, is_callable);
102111
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, RuntimeDebug)]
103112
pub enum ProxyType {
104113
Any,
105114
JustTransfer,
115+
JustUtility,
106116
}
107117
impl Default for ProxyType { fn default() -> Self { Self::Any } }
108118
impl InstanceFilter<Call> for ProxyType {
109119
fn filter(&self, c: &Call) -> bool {
110120
match self {
111121
ProxyType::Any => true,
112-
ProxyType::JustTransfer => match c {
113-
Call::Balances(pallet_balances::Call::transfer(..)) => true,
114-
_ => false,
115-
}
122+
ProxyType::JustTransfer => matches!(c, Call::Balances(pallet_balances::Call::transfer(..))),
123+
ProxyType::JustUtility => matches!(c, Call::Utility(..)),
116124
}
117125
}
126+
fn is_superset(&self, o: &Self) -> bool {
127+
self == &ProxyType::Any || self == o
128+
}
118129
}
119-
pub struct TestIsCallable;
120-
impl Filter<Call> for TestIsCallable {
130+
pub struct BaseFilter;
131+
impl Filter<Call> for BaseFilter {
121132
fn filter(c: &Call) -> bool {
122133
match *c {
123134
// Remark is used as a no-op call in the benchmarking
@@ -131,7 +142,7 @@ impl Trait for Test {
131142
type Event = TestEvent;
132143
type Call = Call;
133144
type Currency = Balances;
134-
type IsCallable = TestIsCallable;
145+
type IsCallable = IsCallable;
135146
type ProxyType = ProxyType;
136147
type ProxyDepositBase = ProxyDepositBase;
137148
type ProxyDepositFactor = ProxyDepositFactor;
@@ -140,11 +151,15 @@ impl Trait for Test {
140151

141152
type System = frame_system::Module<Test>;
142153
type Balances = pallet_balances::Module<Test>;
154+
type Utility = pallet_utility::Module<Test>;
143155
type Proxy = Module<Test>;
144156

145157
use frame_system::Call as SystemCall;
146158
use pallet_balances::Call as BalancesCall;
147159
use pallet_balances::Error as BalancesError;
160+
use pallet_utility::Call as UtilityCall;
161+
use pallet_utility::Error as UtilityError;
162+
use pallet_utility::Event as UtilityEvent;
148163
use super::Call as ProxyCall;
149164

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

183+
fn last_events(n: usize) -> Vec<TestEvent> {
184+
system::Module::<Test>::events().into_iter().rev().take(n).rev().map(|e| e.event).collect()
185+
}
186+
187+
fn expect_events(e: Vec<TestEvent>) {
188+
assert_eq!(last_events(e.len()), e);
189+
}
190+
191+
#[test]
192+
fn filtering_works() {
193+
new_test_ext().execute_with(|| {
194+
Balances::mutate_account(&1, |a| a.free = 1000);
195+
assert_ok!(Proxy::add_proxy(Origin::signed(1), 2, ProxyType::Any));
196+
assert_ok!(Proxy::add_proxy(Origin::signed(1), 3, ProxyType::JustTransfer));
197+
assert_ok!(Proxy::add_proxy(Origin::signed(1), 4, ProxyType::JustUtility));
198+
199+
let call = Box::new(Call::Balances(BalancesCall::transfer(6, 1)));
200+
assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
201+
expect_event(RawEvent::ProxyExecuted(Ok(())));
202+
assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()));
203+
expect_event(RawEvent::ProxyExecuted(Ok(())));
204+
assert_noop!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()), Error::<Test>::Uncallable);
205+
206+
let sub_id = Utility::sub_account_id(1, 0);
207+
Balances::mutate_account(&sub_id, |a| a.free = 1000);
208+
let inner = Box::new(Call::Balances(BalancesCall::transfer(6, 1)));
209+
210+
let call = Box::new(Call::Utility(UtilityCall::as_sub(0, inner.clone())));
211+
assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
212+
expect_event(RawEvent::ProxyExecuted(Ok(())));
213+
assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
214+
assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
215+
expect_event(RawEvent::ProxyExecuted(Ok(())));
216+
217+
let call = Box::new(Call::Utility(UtilityCall::as_limited_sub(0, inner.clone())));
218+
assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
219+
expect_event(RawEvent::ProxyExecuted(Ok(())));
220+
assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
221+
assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
222+
let de = DispatchError::from(UtilityError::<Test>::Uncallable).stripped();
223+
expect_event(RawEvent::ProxyExecuted(Err(de)));
224+
225+
let call = Box::new(Call::Utility(UtilityCall::batch(vec![*inner])));
226+
assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
227+
expect_events(vec![UtilityEvent::BatchCompleted.into(), RawEvent::ProxyExecuted(Ok(())).into()]);
228+
assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
229+
assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
230+
expect_events(vec![UtilityEvent::Uncallable(0).into(), RawEvent::ProxyExecuted(Ok(())).into()]);
231+
232+
let inner = Box::new(Call::Proxy(ProxyCall::add_proxy(5, ProxyType::Any)));
233+
let call = Box::new(Call::Utility(UtilityCall::batch(vec![*inner])));
234+
assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
235+
expect_events(vec![UtilityEvent::BatchCompleted.into(), RawEvent::ProxyExecuted(Ok(())).into()]);
236+
assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
237+
assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
238+
expect_events(vec![UtilityEvent::Uncallable(0).into(), RawEvent::ProxyExecuted(Ok(())).into()]);
239+
});
240+
}
241+
168242
#[test]
169243
fn add_remove_proxies_works() {
170244
new_test_ext().execute_with(|| {
@@ -175,8 +249,12 @@ fn add_remove_proxies_works() {
175249
assert_eq!(Balances::reserved_balance(1), 3);
176250
assert_ok!(Proxy::add_proxy(Origin::signed(1), 3, ProxyType::Any));
177251
assert_eq!(Balances::reserved_balance(1), 4);
252+
assert_ok!(Proxy::add_proxy(Origin::signed(1), 4, ProxyType::JustUtility));
253+
assert_eq!(Balances::reserved_balance(1), 5);
178254
assert_noop!(Proxy::add_proxy(Origin::signed(1), 4, ProxyType::Any), Error::<Test>::TooMany);
179255
assert_noop!(Proxy::remove_proxy(Origin::signed(1), 3, ProxyType::JustTransfer), Error::<Test>::NotFound);
256+
assert_ok!(Proxy::remove_proxy(Origin::signed(1), 4, ProxyType::JustUtility));
257+
assert_eq!(Balances::reserved_balance(1), 4);
180258
assert_ok!(Proxy::remove_proxy(Origin::signed(1), 3, ProxyType::Any));
181259
assert_eq!(Balances::reserved_balance(1), 3);
182260
assert_ok!(Proxy::remove_proxy(Origin::signed(1), 2, ProxyType::Any));
@@ -218,7 +296,7 @@ fn proxying_works() {
218296
assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::<Test>::Uncallable);
219297

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

0 commit comments

Comments
 (0)