Introduce in-origin filtering (#6318)

* impl filter in origin * remove IsCallable usage. Breaking: utility::batch(root, calls) no longer bypass BasicCallFilter * rename BasicCallFilter -> BaseCallFilter * refactor code * Apply suggestions from code review Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * remove forgotten temporar comment * better add suggestion in another PR * refactor: use Clone instead of mem::replace * fix tests * fix tests * fix tests * fix benchmarks * Make root bypass filter in utility::batch * fix unused imports Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
paritytech · Jun 15, 2020 · 0db70ea · 0db70ea
1 parent f837c39
commit 0db70ea
Show file tree

Hide file tree

Showing 79 changed files with 536 additions and 302 deletions.
diff --git a/bin/node-template/pallets/template/src/mock.rs b/bin/node-template/pallets/template/src/mock.rs
@@ -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;

diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs
@@ -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.

diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
@@ -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,
@@ -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>) {
@@ -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;
@@ -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! {
@@ -201,7 +192,6 @@ impl pallet_multisig::Trait for Runtime {
 	type DepositBase = DepositBase;
 	type DepositFactor = DepositFactor;
 	type MaxSignatories = MaxSignatories;
-	type IsCallable = IsCallable;
 }
 
 parameter_types! {
@@ -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;

diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs
@@ -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 = ();

diff --git a/frame/aura/src/mock.rs b/frame/aura/src/mock.rs
@@ -46,6 +46,7 @@ parameter_types! {
 }
 
 impl frame_system::Trait for Test {
+	type BaseCallFilter = ();
 	type Origin = Origin;
 	type Index = u64;
 	type BlockNumber = u64;

diff --git a/frame/authority-discovery/src/lib.rs b/frame/authority-discovery/src/lib.rs
@@ -143,6 +143,7 @@ mod tests {
 	}
 
 	impl frame_system::Trait for Test {
+		type BaseCallFilter = ();
 		type Origin = Origin;
 		type Index = u64;
 		type BlockNumber = BlockNumber;

diff --git a/frame/authorship/src/lib.rs b/frame/authorship/src/lib.rs
@@ -418,6 +418,7 @@ mod tests {
 	}
 
 	impl frame_system::Trait for Test {
+		type BaseCallFilter = ();
 		type Origin = Origin;
 		type Index = u64;
 		type BlockNumber = u64;

diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs
@@ -57,6 +57,7 @@ parameter_types! {
 }
 
 impl frame_system::Trait for Test {
+	type BaseCallFilter = ();
 	type Origin = Origin;
 	type Index = u64;
 	type BlockNumber = u64;

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
@@ -848,6 +848,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;

diff --git a/frame/balances/src/tests_composite.rs b/frame/balances/src/tests_composite.rs
@@ -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;

diff --git a/frame/balances/src/tests_local.rs b/frame/balances/src/tests_local.rs
@@ -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;

diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs
@@ -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.
@@ -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::UnfilteredDispatchable
+				>::dispatch_bypass_filter(Call::<T>::$dispatch($($arg),*), $origin.into())?;
 			}
 			verify $postcode
 			$( $rest )*
@@ -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::UnfilteredDispatchable
+				>::dispatch_bypass_filter(Call::<T, I>::$dispatch($($arg),*), $origin.into())?;
 			}
 			verify $postcode
 			$( $rest )*

diff --git a/frame/benchmarking/src/tests.rs b/frame/benchmarking/src/tests.rs
@@ -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;

diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs
@@ -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;
@@ -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));
@@ -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));
@@ -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 })
@@ -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 })
@@ -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))),

diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs
@@ -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>;
@@ -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]
@@ -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);

diff --git a/frame/democracy/src/benchmarking.rs b/frame/democracy/src/benchmarking.rs
@@ -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, UnfilteredDispatchable},
 };
 use frame_system::{RawOrigin, Module as System, self, EventRecord};
 use sp_runtime::traits::{Bounded, One};
@@ -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());
@@ -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");
@@ -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");
@@ -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");
@@ -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")
 	}
@@ -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")?;
@@ -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");
 

diff --git a/frame/democracy/src/tests.rs b/frame/democracy/src/tests.rs
@@ -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;

diff --git a/frame/democracy/src/tests/cancellation.rs b/frame/democracy/src/tests/cancellation.rs
@@ -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();
@@ -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());
 	});
 }