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

Commit

Permalink
remove IsCallable usage. Breaking: utility::batch(root, calls) no lon…
Browse files Browse the repository at this point in the history
…ger bypass BasicCallFilter
  • Loading branch information
thiolliere committed Jun 10, 2020
1 parent 97f9a31 commit 705149d
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 72 deletions.
14 changes: 1 addition & 13 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use frame_support::{
traits::{Currency, Imbalance, KeyOwnerProofSystem, OnUnbalanced, Randomness, LockIdentifier},
};
use frame_system::{EnsureRoot, EnsureOneOf};
use frame_support::traits::{Filter, InstanceFilter};
use frame_support::traits::InstanceFilter;
use codec::{Encode, Decode};
use sp_core::{
crypto::KeyTypeId,
Expand Down Expand Up @@ -113,15 +113,6 @@ pub fn native_version() -> NativeVersion {

type NegativeImbalance = <Balances as Currency<AccountId>>::NegativeImbalance;

pub struct BaseFilter;
impl Filter<Call> for BaseFilter {
fn filter(_call: &Call) -> bool {
true
}
}
pub struct IsCallable;
frame_support::impl_filter_stack!(IsCallable, BaseFilter, Call, is_callable);

pub struct DealWithFees;
impl OnUnbalanced<NegativeImbalance> for DealWithFees {
fn on_unbalanceds<B>(mut fees_then_tips: impl Iterator<Item=NegativeImbalance>) {
Expand Down Expand Up @@ -184,7 +175,6 @@ impl frame_system::Trait for Runtime {
impl pallet_utility::Trait for Runtime {
type Event = Event;
type Call = Call;
type IsCallable = IsCallable;
}

parameter_types! {
Expand All @@ -202,7 +192,6 @@ impl pallet_multisig::Trait for Runtime {
type DepositBase = DepositBase;
type DepositFactor = DepositFactor;
type MaxSignatories = MaxSignatories;
type IsCallable = IsCallable;
}

parameter_types! {
Expand Down Expand Up @@ -252,7 +241,6 @@ impl pallet_proxy::Trait for Runtime {
type Event = Event;
type Call = Call;
type Currency = Balances;
type IsCallable = IsCallable;
type ProxyType = ProxyType;
type ProxyDepositBase = ProxyDepositBase;
type ProxyDepositFactor = ProxyDepositFactor;
Expand Down
16 changes: 2 additions & 14 deletions frame/multisig/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use sp_std::prelude::*;
use codec::{Encode, Decode};
use sp_io::hashing::blake2_256;
use frame_support::{decl_module, decl_event, decl_error, decl_storage, Parameter, ensure, RuntimeDebug};
use frame_support::{traits::{Get, ReservableCurrency, Currency, Filter, FilterStack, ClearFilterGuard},
use frame_support::{traits::{Get, ReservableCurrency, Currency},
weights::{Weight, GetDispatchInfo, DispatchClass, Pays},
dispatch::{DispatchResultWithPostInfo, DispatchErrorWithPostInfo, PostDispatchInfo},
};
Expand Down Expand Up @@ -87,9 +87,6 @@ pub trait Trait: frame_system::Trait {

/// The maximum amount of signatories allowed in the multisig.
type MaxSignatories: Get<u16>;

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

/// A global extrinsic index, formed as the extrinsic index within a block, together with that
Expand Down Expand Up @@ -151,8 +148,6 @@ decl_error! {
WrongTimepoint,
/// A timepoint was given, yet no multisig operation is underway.
UnexpectedTimepoint,
/// A call with a `false` `IsCallable` filter was attempted.
Uncallable,
}
}

Expand All @@ -175,8 +170,6 @@ decl_event! {
/// A multisig operation has been cancelled. First param is the account that is
/// cancelling, third is the multisig account, fourth is hash of the call.
MultisigCancelled(AccountId, Timepoint<BlockNumber>, AccountId, CallHash),
/// A call with a `false` IsCallable filter was attempted.
Uncallable(u32),
}
}

Expand Down Expand Up @@ -220,8 +213,7 @@ decl_module! {
/// Register approval for a dispatch to be made from a deterministic composite account if
/// approved by a total of `threshold - 1` of `other_signatories`.
///
/// If there are enough, then dispatch the call. Calls must each fulfil the `IsCallable`
/// filter.
/// If there are enough, then dispatch the call.
///
/// Payment: `DepositBase` will be reserved if this is the first approval, plus
/// `threshold` times `DepositFactor`. It is returned once this dispatch happens or
Expand Down Expand Up @@ -280,10 +272,6 @@ decl_module! {
call: Box<<T as Trait>::Call>,
) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin)?;
// We're now executing as a freshly authenticated new account, so the previous call
// restrictions no longer apply.
let _guard = ClearFilterGuard::<T::IsCallable, <T as Trait>::Call>::new();
ensure!(T::IsCallable::filter(call.as_ref()), Error::<T>::Uncallable);
ensure!(threshold >= 1, Error::<T>::ZeroThreshold);
let max_sigs = T::MaxSignatories::get() as usize;
ensure!(!other_signatories.is_empty(), Error::<T>::TooFewSignatories);
Expand Down
13 changes: 2 additions & 11 deletions frame/proxy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ 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, FilterStack, ClearFilterGuard, InstanceFilter,
Get, ReservableCurrency, Currency, InstanceFilter,
OriginTrait, IsType,
}, weights::{GetDispatchInfo, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}},
dispatch::{PostDispatchInfo, IsSubType},
Expand All @@ -66,9 +66,6 @@ pub trait Trait: frame_system::Trait {
/// The currency mechanism.
type Currency: ReservableCurrency<Self::AccountId>;

/// Is a given call compatible with the proxying subsystem?
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.
type ProxyType: Parameter + Member + Ord + PartialOrd + InstanceFilter<<Self as Trait>::Call>
Expand Down Expand Up @@ -106,8 +103,6 @@ decl_error! {
NotFound,
/// Sender is not a proxy of the account to be proxied.
NotProxy,
/// A call with a `false` `IsCallable` filter was attempted.
Uncallable,
/// A call which is incompatible with the proxy type's filter was attempted.
Unproxyable,
/// Account is already a proxy.
Expand Down Expand Up @@ -172,11 +167,7 @@ decl_module! {
.find(|x| &x.0 == &who && force_proxy_type.as_ref().map_or(true, |y| &x.1 == y))
.ok_or(Error::<T>::NotProxy)?;

// 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();
ensure!(T::IsCallable::filter(&call), Error::<T>::Uncallable);

// This is a freshly authenticated new account, the origin restrictions doesn't apply.
let mut origin: T::Origin = frame_system::RawOrigin::Signed(real).into();
origin.add_filter(move |c: &<T as frame_system::Trait>::Call| {
let c = <T as Trait>::Call::from_ref(c);
Expand Down
42 changes: 8 additions & 34 deletions frame/utility/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ use sp_std::prelude::*;
use codec::{Encode, Decode};
use sp_core::TypeId;
use sp_io::hashing::blake2_256;
use frame_support::{decl_module, decl_event, decl_error, decl_storage, Parameter, ensure};
use frame_support::{decl_module, decl_event, decl_storage, Parameter};
use frame_support::{
traits::{Filter, FilterStack, ClearFilterGuard, OriginTrait},
traits::OriginTrait,
weights::{Weight, GetDispatchInfo, DispatchClass, FunctionOf, Pays}, dispatch::PostDispatchInfo,
};
use frame_system::{self as system, ensure_signed, ensure_root};
use frame_system::{self as system, ensure_signed};
use sp_runtime::{DispatchError, DispatchResult, traits::Dispatchable};

mod tests;
Expand All @@ -73,22 +73,12 @@ pub trait Trait: frame_system::Trait {
/// The overarching call type.
type Call: Parameter + Dispatchable<Origin=Self::Origin, PostInfo=PostDispatchInfo>
+ GetDispatchInfo + From<frame_system::Call<Self>>;

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

decl_storage! {
trait Store for Module<T: Trait> as Utility {}
}

decl_error! {
pub enum Error for Module<T: Trait> {
/// A call with a `false` `IsCallable` filter was attempted.
Uncallable,
}
}

decl_event! {
/// Events type.
pub enum Event {
Expand All @@ -97,8 +87,6 @@ decl_event! {
BatchInterrupted(u32, DispatchError),
/// Batch of dispatches completed fully with no error.
BatchCompleted,
/// A call with a `false` IsCallable filter was attempted.
Uncallable(u32),
}
}

Expand All @@ -112,16 +100,11 @@ impl TypeId for IndexedUtilityModuleId {

decl_module! {
pub struct Module<T: Trait> for enum Call where origin: T::Origin {
type Error = Error<T>;

/// Deposit one of this module's events by using the default implementation.
fn deposit_event() = default;

/// Send a batch of dispatch calls.
///
/// This will execute until the first one fails and then stop. Calls must fulfil the
/// `IsCallable` filter unless the origin is `Root`.
///
/// May be called from any origin.
///
/// - `calls`: The calls to be dispatched from the same origin.
Expand Down Expand Up @@ -156,12 +139,8 @@ decl_module! {
Pays::Yes,
)]
fn batch(origin, calls: Vec<<T as Trait>::Call>) {
let is_root = ensure_root(origin.clone()).is_ok();
for (index, call) in calls.into_iter().enumerate() {
if !is_root && !T::IsCallable::filter(&call) {
Self::deposit_event(Event::Uncallable(index as u32));
return Ok(())
}
// TODO TODO: root no longer bypass IsCallable filter.
let result = call.dispatch(origin.clone());
if let Err(e) = result {
Self::deposit_event(Event::BatchInterrupted(index as u32, e.error));
Expand All @@ -173,9 +152,6 @@ decl_module! {

/// Send a call through an indexed pseudonym of the sender.
///
/// The call must fulfil only the pre-cleared `IsCallable` filter (i.e. only the level of
/// filtering that remains after calling `take()`).
///
/// NOTE: If you need to ensure that any account-based filtering is honored (i.e. because
/// you expect `proxy` to have been used prior in the call stack and you want it to apply to
/// any sub-accounts), then use `as_limited_sub` instead.
Expand All @@ -195,18 +171,17 @@ decl_module! {
)]
fn as_sub(origin, index: u16, call: Box<<T as Trait>::Call>) -> DispatchResult {
let who = ensure_signed(origin)?;
// We're now executing as a freshly authenticated new account, so the previous call
// restrictions no longer apply.
let _guard = ClearFilterGuard::<T::IsCallable, <T as Trait>::Call>::new();
ensure!(T::IsCallable::filter(&call), Error::<T>::Uncallable);

// This is a freshly authenticated new account, the origin restrictions doesn't apply.
let pseudonym = Self::sub_account_id(who, index);
call.dispatch(frame_system::RawOrigin::Signed(pseudonym).into())
.map(|_| ()).map_err(|e| e.error)
}

/// Send a call through an indexed pseudonym of the sender.
///
/// Calls must each fulfil the `IsCallable` filter; it is not cleared before.
/// Filter from origin are passed along. The call will be dispatched with an origin which
/// use the same filter as the origin of this call.
///
/// NOTE: If you need to ensure that any account-based filtering is not honored (i.e.
/// because you expect `proxy` to have been used prior in the call stack and you do not want
Expand All @@ -228,7 +203,6 @@ decl_module! {
fn as_limited_sub(origin, index: u16, call: Box<<T as Trait>::Call>) -> DispatchResult {
let mut origin = origin;
let who = ensure_signed(origin.clone())?;
ensure!(T::IsCallable::filter(&call), Error::<T>::Uncallable);
let pseudonym = Self::sub_account_id(who, index);
origin.set_caller_from(frame_system::RawOrigin::Signed(pseudonym));
call.dispatch(origin).map(|_| ()).map_err(|e| e.error)
Expand Down

0 comments on commit 705149d

Please sign in to comment.