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

Introduce in-origin filtering #6318

Merged
merged 16 commits into from
Jun 15, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/node-template/pallets/template/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ parameter_types! {
pub const AvailableBlockRatio: Perbill = Perbill::from_percent(75);
}
impl system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Call = ();
type Index = u64;
Expand Down
2 changes: 2 additions & 0 deletions bin/node-template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ parameter_types! {
}

impl system::Trait for Runtime {
/// The basic call filter to use in dispatchable.
type BaseCallFilter = ();
/// The identifier used to distinguish between accounts.
type AccountId = AccountId;
/// The aggregated dispatch type that is available for extrinsics.
Expand Down
15 changes: 2 additions & 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 @@ -155,6 +146,7 @@ parameter_types! {
const_assert!(AvailableBlockRatio::get().deconstruct() >= AVERAGE_ON_INITIALIZE_WEIGHT.deconstruct());

impl frame_system::Trait for Runtime {
type BaseCallFilter = ();
type Origin = Origin;
type Call = Call;
type Index = Index;
Expand Down Expand Up @@ -183,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 @@ -201,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 @@ -251,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
1 change: 1 addition & 0 deletions frame/assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ mod tests {
pub const AvailableBlockRatio: Perbill = Perbill::one();
}
impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type Call = ();
Expand Down
1 change: 1 addition & 0 deletions frame/aura/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ parameter_types! {
}

impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
Expand Down
1 change: 1 addition & 0 deletions frame/authority-discovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ mod tests {
}

impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = BlockNumber;
Expand Down
1 change: 1 addition & 0 deletions frame/authorship/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ mod tests {
}

impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
Expand Down
1 change: 1 addition & 0 deletions frame/babe/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ parameter_types! {
}

impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
Expand Down
1 change: 1 addition & 0 deletions frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,7 @@ impl<T: Subtrait<I>, I: Instance> PartialEq for ElevatedTrait<T, I> {
}
impl<T: Subtrait<I>, I: Instance> Eq for ElevatedTrait<T, I> {}
impl<T: Subtrait<I>, I: Instance> frame_system::Trait for ElevatedTrait<T, I> {
type BaseCallFilter = T::BaseCallFilter;
type Origin = T::Origin;
type Call = T::Call;
type Index = T::Index;
Expand Down
1 change: 1 addition & 0 deletions frame/balances/src/tests_composite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ parameter_types! {
pub const AvailableBlockRatio: Perbill = Perbill::one();
}
impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
Expand Down
1 change: 1 addition & 0 deletions frame/balances/src/tests_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ parameter_types! {
pub const AvailableBlockRatio: Perbill = Perbill::one();
}
impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
Expand Down
11 changes: 8 additions & 3 deletions frame/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ pub use utils::*;
pub use analysis::Analysis;
#[doc(hidden)]
pub use sp_io::storage::root as storage_root;
pub use sp_runtime::traits::{Dispatchable, Zero};
pub use sp_runtime::traits::Zero;
pub use frame_support;
pub use paste;

/// Construct pallet benchmarks for weighing dispatchables.
Expand Down Expand Up @@ -242,7 +243,9 @@ macro_rules! benchmarks_iter {
{ $( $common )* }
( $( $names )* )
$name { $( $code )* }: {
<Call<T> as $crate::Dispatchable>::dispatch(Call::<T>::$dispatch($($arg),*), $origin.into())?;
<
Call<T> as $crate::frame_support::traits::PalletDispatchable
>::dispatch_bypass_filter(Call::<T>::$dispatch($($arg),*), $origin.into())?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>::dispatch_bypass_filter(Call::<T>::$dispatch($($arg),*), $origin.into())?;
>::dispatch_bypass_filter(Call::<T>::$call_name($($arg),*), $origin.into())?;

Just a suggestion, I find it confusing since Call::<T>::$dispatch($($arg),*) reads like we're calling into a function named dispatch, while we're selecting enum variants here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for consistency then we should rename $dispatch by $call_name everywhere in this macro

}
verify $postcode
$( $rest )*
Expand All @@ -262,7 +265,9 @@ macro_rules! benchmarks_iter {
{ $( $common )* }
( $( $names )* )
$name { $( $code )* }: {
<Call<T, I> as $crate::Dispatchable>::dispatch(Call::<T, I>::$dispatch($($arg),*), $origin.into())?;
<
Call<T, I> as $crate::frame_support::traits::PalletDispatchable
>::dispatch_bypass_filter(Call::<T, I>::$dispatch($($arg),*), $origin.into())?;
}
verify $postcode
$( $rest )*
Expand Down
1 change: 1 addition & 0 deletions frame/benchmarking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub trait Trait {
pub struct Test;

impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
Expand Down
13 changes: 7 additions & 6 deletions frame/collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1015,10 +1015,11 @@ mod tests {
pub const MaxProposals: u32 = 100;
}
impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
type Call = ();
type Call = Call;
type Hash = H256;
type Hashing = BlakeTwo256;
type AccountId = u64;
Expand Down Expand Up @@ -1167,7 +1168,7 @@ mod tests {
let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32);
let proposal_weight = proposal.get_dispatch_info().weight;
let hash = BlakeTwo256::hash_of(&proposal);
assert_ok!(Collective::set_members(Origin::ROOT, vec![1, 2, 3], Some(3), MAX_MEMBERS));
assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(3), MAX_MEMBERS));

assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len));
assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true));
Expand All @@ -1192,7 +1193,7 @@ mod tests {
let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32);
let proposal_weight = proposal.get_dispatch_info().weight;
let hash = BlakeTwo256::hash_of(&proposal);
assert_ok!(Collective::set_members(Origin::ROOT, vec![1, 2, 3], Some(1), MAX_MEMBERS));
assert_ok!(Collective::set_members(Origin::root(), vec![1, 2, 3], Some(1), MAX_MEMBERS));

assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len));
assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true));
Expand Down Expand Up @@ -1260,7 +1261,7 @@ mod tests {
Collective::voting(&hash),
Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end })
);
assert_ok!(Collective::set_members(Origin::ROOT, vec![2, 3, 4], None, MAX_MEMBERS));
assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MAX_MEMBERS));
assert_eq!(
Collective::voting(&hash),
Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end })
Expand All @@ -1275,7 +1276,7 @@ mod tests {
Collective::voting(&hash),
Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end })
);
assert_ok!(Collective::set_members(Origin::ROOT, vec![2, 4], None, MAX_MEMBERS));
assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MAX_MEMBERS));
assert_eq!(
Collective::voting(&hash),
Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end })
Expand Down Expand Up @@ -1618,7 +1619,7 @@ mod tests {
// Proposal would normally succeed
assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true));
// But Root can disapprove and remove it anyway
assert_ok!(Collective::disapprove_proposal(Origin::ROOT, hash.clone()));
assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone()));
let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] };
assert_eq!(System::events(), vec![
record(Event::collective_Instance1(RawEvent::Proposed(1, 0, hash.clone(), 2))),
Expand Down
13 changes: 7 additions & 6 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,12 @@ parameter_types! {
pub const AvailableBlockRatio: Perbill = Perbill::one();
}
impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
type Hash = H256;
type Call = ();
type Call = Call;
type Hashing = BlakeTwo256;
type AccountId = u64;
type Lookup = IdentityLookup<Self::AccountId>;
Expand Down Expand Up @@ -936,7 +937,7 @@ fn call_contract_removals() {

#[test]
fn inherent_claim_surcharge_contract_removals() {
removals(|| Contracts::claim_surcharge(Origin::NONE, BOB, Some(ALICE)).is_ok());
removals(|| Contracts::claim_surcharge(Origin::none(), BOB, Some(ALICE)).is_ok());
}

#[test]
Expand All @@ -947,10 +948,10 @@ fn signed_claim_surcharge_contract_removals() {
#[test]
fn claim_surcharge_malus() {
// Test surcharge malus for inherent
claim_surcharge(4, || Contracts::claim_surcharge(Origin::NONE, BOB, Some(ALICE)).is_ok(), true);
claim_surcharge(3, || Contracts::claim_surcharge(Origin::NONE, BOB, Some(ALICE)).is_ok(), true);
claim_surcharge(2, || Contracts::claim_surcharge(Origin::NONE, BOB, Some(ALICE)).is_ok(), true);
claim_surcharge(1, || Contracts::claim_surcharge(Origin::NONE, BOB, Some(ALICE)).is_ok(), false);
claim_surcharge(4, || Contracts::claim_surcharge(Origin::none(), BOB, Some(ALICE)).is_ok(), true);
claim_surcharge(3, || Contracts::claim_surcharge(Origin::none(), BOB, Some(ALICE)).is_ok(), true);
claim_surcharge(2, || Contracts::claim_surcharge(Origin::none(), BOB, Some(ALICE)).is_ok(), true);
claim_surcharge(1, || Contracts::claim_surcharge(Origin::none(), BOB, Some(ALICE)).is_ok(), false);

// Test surcharge malus for signed
claim_surcharge(4, || Contracts::claim_surcharge(Origin::signed(ALICE), BOB, None).is_ok(), true);
Expand Down
18 changes: 9 additions & 9 deletions frame/democracy/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use super::*;
use frame_benchmarking::{benchmarks, account};
use frame_support::{
IterableStorageMap,
traits::{Currency, Get, EnsureOrigin, OnInitialize},
traits::{Currency, Get, EnsureOrigin, OnInitialize, PalletDispatchable},
};
use frame_system::{RawOrigin, Module as System, self, EventRecord};
use sp_runtime::traits::{Bounded, One};
Expand Down Expand Up @@ -212,14 +212,14 @@ benchmarks! {
for i in 0 .. r {
let ref_idx = add_referendum::<T>(i)?;
let call = Call::<T>::emergency_cancel(ref_idx);
call.dispatch(origin.clone())?;
call.dispatch_bypass_filter(origin.clone())?;
}

// Lets now measure one more
let referendum_index = add_referendum::<T>(r)?;
let call = Call::<T>::emergency_cancel(referendum_index);
assert!(Democracy::<T>::referendum_status(referendum_index).is_ok());
}: { call.dispatch(origin)? }
}: { call.dispatch_bypass_filter(origin)? }
verify {
// Referendum has been canceled
assert!(Democracy::<T>::referendum_status(referendum_index).is_err());
Expand All @@ -239,7 +239,7 @@ benchmarks! {
);

let call = Call::<T>::external_propose(proposal_hash);
}: { call.dispatch(origin)? }
}: { call.dispatch_bypass_filter(origin)? }
verify {
// External proposal created
ensure!(<NextExternal<T>>::exists(), "External proposal didn't work");
Expand All @@ -251,7 +251,7 @@ benchmarks! {
let origin = T::ExternalMajorityOrigin::successful_origin();
let proposal_hash = T::Hashing::hash_of(&p);
let call = Call::<T>::external_propose_majority(proposal_hash);
}: { call.dispatch(origin)? }
}: { call.dispatch_bypass_filter(origin)? }
verify {
// External proposal created
ensure!(<NextExternal<T>>::exists(), "External proposal didn't work");
Expand All @@ -263,7 +263,7 @@ benchmarks! {
let origin = T::ExternalDefaultOrigin::successful_origin();
let proposal_hash = T::Hashing::hash_of(&p);
let call = Call::<T>::external_propose_default(proposal_hash);
}: { call.dispatch(origin)? }
}: { call.dispatch_bypass_filter(origin)? }
verify {
// External proposal created
ensure!(<NextExternal<T>>::exists(), "External proposal didn't work");
Expand All @@ -282,7 +282,7 @@ benchmarks! {
let delay = 0;
let call = Call::<T>::fast_track(proposal_hash, voting_period.into(), delay.into());

}: { call.dispatch(origin_fast_track)? }
}: { call.dispatch_bypass_filter(origin_fast_track)? }
verify {
assert_eq!(Democracy::<T>::referendum_count(), 1, "referendum not created")
}
Expand All @@ -306,7 +306,7 @@ benchmarks! {
let call = Call::<T>::veto_external(proposal_hash);
let origin = T::VetoOrigin::successful_origin();
ensure!(NextExternal::<T>::get().is_some(), "no external proposal");
}: { call.dispatch(origin)? }
}: { call.dispatch_bypass_filter(origin)? }
verify {
assert!(NextExternal::<T>::get().is_none());
let (_, new_vetoers) = <Blacklist<T>>::get(&proposal_hash).ok_or("no blacklist")?;
Expand Down Expand Up @@ -347,7 +347,7 @@ benchmarks! {
let origin = T::ExternalMajorityOrigin::successful_origin();
let proposal_hash = T::Hashing::hash_of(&r);
let call = Call::<T>::external_propose_majority(proposal_hash);
call.dispatch(origin)?;
call.dispatch_bypass_filter(origin)?;
// External proposal created
ensure!(<NextExternal<T>>::exists(), "External proposal didn't work");

Expand Down
1 change: 1 addition & 0 deletions frame/democracy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ parameter_types! {
pub const AvailableBlockRatio: Perbill = Perbill::one();
}
impl frame_system::Trait for Test {
type BaseCallFilter = ();
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
Expand Down
6 changes: 3 additions & 3 deletions frame/democracy/src/tests/cancellation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn cancel_referendum_should_work() {
0
);
assert_ok!(Democracy::vote(Origin::signed(1), r, aye(1)));
assert_ok!(Democracy::cancel_referendum(Origin::ROOT, r.into()));
assert_ok!(Democracy::cancel_referendum(Origin::root(), r.into()));

next_block();
next_block();
Expand All @@ -53,8 +53,8 @@ fn cancel_queued_should_work() {

assert!(pallet_scheduler::Agenda::<Test>::get(6)[0].is_some());

assert_noop!(Democracy::cancel_queued(Origin::ROOT, 1), Error::<Test>::ProposalMissing);
assert_ok!(Democracy::cancel_queued(Origin::ROOT, 0));
assert_noop!(Democracy::cancel_queued(Origin::root(), 1), Error::<Test>::ProposalMissing);
assert_ok!(Democracy::cancel_queued(Origin::root(), 0));
assert!(pallet_scheduler::Agenda::<Test>::get(6)[0].is_none());
});
}
Expand Down
Loading