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 <[email protected]>

* 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 <[email protected]>

Author: gui1117

fuzzer/src/mock.rs

@@ -57,6 +57,7 @@ impl Convert<u128, u64> for CurrencyToVoteHandler {
 pub struct Test;
 
 impl frame_system::Trait for Test {
+	type BaseCallFilter = ();
 	type Origin = Origin;
 	type DbWeight = ();
 	type BlockExecutionWeight = ();

fuzzer/src/submit_solution.rs

@@ -23,9 +23,9 @@ use honggfuzz::fuzz;
 
 use mock::Test;
 use pallet_staking::testing_utils::*;
-use frame_support::{assert_ok, storage::StorageValue};
+use frame_support::{assert_ok, storage::StorageValue, traits::UnfilteredDispatchable};
 use frame_system::RawOrigin;
-use sp_runtime::{traits::Dispatchable, DispatchError};
+use sp_runtime::DispatchError;
 use sp_core::offchain::{testing::TestOffchainExt, OffchainExt};
 use pallet_staking::{EraElectionStatus, ElectionStatus, Module as Staking, Call as StakingCall};
 
@@ -159,7 +159,7 @@ fn main() {
 				match mode {
 					Mode::WeakerSubmission => {
 						assert_eq!(
-							call.dispatch(origin.clone().into()).unwrap_err().error,
+							call.dispatch_bypass_filter(origin.clone().into()).unwrap_err().error,
 							DispatchError::Module {
 								index: 0,
 								error: 16,
@@ -170,7 +170,7 @@ fn main() {
 					// NOTE: so exhaustive pattern doesn't work here.. maybe some rust issue?
 					// or due to `#[repr(u32)]`?
 					Mode::InitialSubmission | Mode::StrongerSubmission => {
-						assert_ok!(call.dispatch(origin.into()));
+						assert_ok!(call.dispatch_bypass_filter(origin.into()));
 					}
 				};
 			})

src/benchmarking.rs

@@ -21,7 +21,7 @@ use super::*;
 use crate::Module as Staking;
 use testing_utils::*;
 
-use sp_runtime::{traits::{Dispatchable, One}};
+use sp_runtime::traits::One;
 use frame_system::RawOrigin;
 pub use frame_benchmarking::{benchmarks, account};
 const SEED: u32 = 0;
@@ -379,12 +379,12 @@ benchmarks! {
 		let current_era = CurrentEra::get().unwrap();
 		let mut points_total = 0;
 		let mut points_individual = Vec::new();
-		let mut payout_calls = Vec::new();
+		let mut payout_calls_arg = Vec::new();
 
 		for validator in new_validators.iter() {
 			points_total += 10;
 			points_individual.push((validator.clone(), 10));
-			payout_calls.push(Call::<T>::payout_stakers(validator.clone(), current_era))
+			payout_calls_arg.push((validator.clone(), current_era));
 		}
 
 		// Give Era Points
@@ -401,8 +401,8 @@ benchmarks! {
 
 		let caller: T::AccountId = account("caller", 0, SEED);
 	}: {
-		for call in payout_calls {
-			call.dispatch(RawOrigin::Signed(caller.clone()).into())?;
+		for arg in payout_calls_arg {
+			<Staking<T>>::payout_stakers(RawOrigin::Signed(caller.clone()).into(), arg.0, arg.1)?;
 		}
 	}
 

src/mock.rs

@@ -200,6 +200,7 @@ parameter_types! {
 	pub const AvailableBlockRatio: Perbill = Perbill::one();
 }
 impl frame_system::Trait for Test {
+	type BaseCallFilter = ();
 	type Origin = Origin;
 	type Index = AccountIndex;
 	type BlockNumber = BlockNumber;

src/tests.rs

@@ -47,7 +47,7 @@ fn force_unstake_works() {
 		// Force unstake needs correct number of slashing spans (for weight calculation)
 		assert_noop!(Staking::force_unstake(Origin::signed(11), 11, 0), BadOrigin);
 		// We now force them to unstake
-		assert_ok!(Staking::force_unstake(Origin::ROOT, 11, 2));
+		assert_ok!(Staking::force_unstake(Origin::root(), 11, 2));
 		// No longer bonded.
 		assert_eq!(Staking::bonded(&11), None);
 		// Transfer works.
@@ -1477,7 +1477,7 @@ fn on_free_balance_zero_stash_removes_validator() {
 		assert_eq!(Balances::total_balance(&11), 0);
 
 		// Reap the stash
-		assert_ok!(Staking::reap_stash(Origin::NONE, 11, 0));
+		assert_ok!(Staking::reap_stash(Origin::none(), 11, 0));
 
 		// Check storage items do not exist
 		assert!(!<Ledger<Test>>::contains_key(&10));
@@ -1533,7 +1533,7 @@ fn on_free_balance_zero_stash_removes_nominator() {
 		assert_eq!(Balances::total_balance(&11), 0);
 
 		// Reap the stash
-		assert_ok!(Staking::reap_stash(Origin::NONE, 11, 0));
+		assert_ok!(Staking::reap_stash(Origin::none(), 11, 0));
 
 		// Check storage items do not exist
 		assert!(!<Ledger<Test>>::contains_key(&10));
@@ -1928,7 +1928,7 @@ fn offence_forces_new_era() {
 #[test]
 fn offence_ensures_new_era_without_clobbering() {
 	ExtBuilder::default().build_and_execute(|| {
-		assert_ok!(Staking::force_new_era_always(Origin::ROOT));
+		assert_ok!(Staking::force_new_era_always(Origin::root()));
 		assert_eq!(Staking::force_era(), Forcing::ForceAlways);
 
 		on_offence_now(
@@ -2302,8 +2302,8 @@ fn garbage_collection_after_slashing() {
 		assert_eq!(slashing_spans.iter().count(), 2);
 
 		// reap_stash respects num_slashing_spans so that weight is accurate
-		assert_noop!(Staking::reap_stash(Origin::NONE, 11, 0), Error::<Test>::IncorrectSlashingSpans);
-		assert_ok!(Staking::reap_stash(Origin::NONE, 11, 2));
+		assert_noop!(Staking::reap_stash(Origin::none(), 11, 0), Error::<Test>::IncorrectSlashingSpans);
+		assert_ok!(Staking::reap_stash(Origin::none(), 11, 2));
 
 		assert!(<Staking as crate::Store>::SlashingSpans::get(&11).is_none());
 		assert_eq!(<Staking as crate::Store>::SpanSlash::get(&(11, 0)).amount_slashed(), &0);
@@ -2591,11 +2591,11 @@ fn remove_deferred() {
 
 		// fails if empty
 		assert_noop!(
-			Staking::cancel_deferred_slash(Origin::ROOT, 1, vec![]),
+			Staking::cancel_deferred_slash(Origin::root(), 1, vec![]),
 			Error::<Test>::EmptyTargets
 		);
 
-		assert_ok!(Staking::cancel_deferred_slash(Origin::ROOT, 1, vec![0]));
+		assert_ok!(Staking::cancel_deferred_slash(Origin::root(), 1, vec![0]));
 
 		assert_eq!(Balances::free_balance(11), 1000);
 		assert_eq!(Balances::free_balance(101), 2000);
@@ -2692,21 +2692,21 @@ fn remove_multi_deferred() {
 
 		// fails if list is not sorted
 		assert_noop!(
-			Staking::cancel_deferred_slash(Origin::ROOT, 1, vec![2, 0, 4]),
+			Staking::cancel_deferred_slash(Origin::root(), 1, vec![2, 0, 4]),
 			Error::<Test>::NotSortedAndUnique
 		);
 		// fails if list is not unique
 		assert_noop!(
-			Staking::cancel_deferred_slash(Origin::ROOT, 1, vec![0, 2, 2]),
+			Staking::cancel_deferred_slash(Origin::root(), 1, vec![0, 2, 2]),
 			Error::<Test>::NotSortedAndUnique
 		);
 		// fails if bad index
 		assert_noop!(
-			Staking::cancel_deferred_slash(Origin::ROOT, 1, vec![1, 2, 3, 4, 5]),
+			Staking::cancel_deferred_slash(Origin::root(), 1, vec![1, 2, 3, 4, 5]),
 			Error::<Test>::InvalidSlashIndex
 		);
 
-		assert_ok!(Staking::cancel_deferred_slash(Origin::ROOT, 1, vec![0, 2, 4]));
+		assert_ok!(Staking::cancel_deferred_slash(Origin::root(), 1, vec![0, 2, 4]));
 
 		let slashes = <Staking as Store>::UnappliedSlashes::get(&1);
 		assert_eq!(slashes.len(), 2);
@@ -4243,16 +4243,16 @@ fn test_max_nominator_rewarded_per_validator_and_cant_steal_someone_else_reward(
 fn set_history_depth_works() {
 	ExtBuilder::default().build_and_execute(|| {
 		mock::start_era(10);
-		Staking::set_history_depth(Origin::ROOT, 20, 0).unwrap();
+		Staking::set_history_depth(Origin::root(), 20, 0).unwrap();
 		assert!(<Staking as Store>::ErasTotalStake::contains_key(10 - 4));
 		assert!(<Staking as Store>::ErasTotalStake::contains_key(10 - 5));
-		Staking::set_history_depth(Origin::ROOT, 4, 0).unwrap();
+		Staking::set_history_depth(Origin::root(), 4, 0).unwrap();
 		assert!(<Staking as Store>::ErasTotalStake::contains_key(10 - 4));
 		assert!(!<Staking as Store>::ErasTotalStake::contains_key(10 - 5));
-		Staking::set_history_depth(Origin::ROOT, 3, 0).unwrap();
+		Staking::set_history_depth(Origin::root(), 3, 0).unwrap();
 		assert!(!<Staking as Store>::ErasTotalStake::contains_key(10 - 4));
 		assert!(!<Staking as Store>::ErasTotalStake::contains_key(10 - 5));
-		Staking::set_history_depth(Origin::ROOT, 8, 0).unwrap();
+		Staking::set_history_depth(Origin::root(), 8, 0).unwrap();
 		assert!(!<Staking as Store>::ErasTotalStake::contains_key(10 - 4));
 		assert!(!<Staking as Store>::ErasTotalStake::contains_key(10 - 5));
 	});