From 3f43e6c1eb6290a014e95c65e735a2f2dc73e099 Mon Sep 17 00:00:00 2001
From: Shaopeng Wang <spxwang@gmail.com>
Date: Thu, 21 May 2020 11:13:42 +1200
Subject: [PATCH] Schedule update configurable origin. (#175)

* Configurable origin.

* Renaming.

* Fix typo.
---
 schedule-update/src/lib.rs   | 13 ++++---
 schedule-update/src/mock.rs  | 22 ++++++++++-
 schedule-update/src/tests.rs | 75 ++++++++++++++++++++++++------------
 3 files changed, 80 insertions(+), 30 deletions(-)

diff --git a/schedule-update/src/lib.rs b/schedule-update/src/lib.rs
index b5ee39c..e7fa428 100644
--- a/schedule-update/src/lib.rs
+++ b/schedule-update/src/lib.rs
@@ -6,7 +6,7 @@ use codec::{Decode, Encode};
 use frame_support::{
 	decl_error, decl_event, decl_module, decl_storage, ensure,
 	storage::IterableStorageDoubleMap,
-	traits::Get,
+	traits::{EnsureOrigin, Get},
 	weights::{DispatchClass, GetDispatchInfo, Weight},
 	Parameter,
 };
@@ -31,6 +31,7 @@ type CallOf<T> = <T as Trait>::Call;
 
 pub trait Trait: frame_system::Trait {
 	type Event: From<Event<Self>> + Into<<Self as frame_system::Trait>::Event>;
+	type DispatchOrigin: EnsureOrigin<Self::Origin>;
 	type Call: Parameter + Dispatchable<Origin = <Self as frame_system::Trait>::Origin> + GetDispatchInfo;
 	type MaxScheduleDispatchWeight: Get<Weight>;
 }
@@ -42,8 +43,8 @@ decl_event!(
 	{
 		/// Add schedule dispatch success (BlockNumber, DispatchId)
 		ScheduleDispatch(BlockNumber, DispatchId),
-		/// Cancel deplayed dispatch success (DispatchId)
-		CancelDeplayedDispatch(DispatchId),
+		/// Cancel delayed dispatch success (DispatchId)
+		CancelDelayedDispatch(DispatchId),
 		/// Schedule dispatch success (BlockNumber, DispatchId)
 		ScheduleDispatchSuccess(BlockNumber, DispatchId),
 		/// Schedule dispatch failed (DispatchId, DispatchError)
@@ -85,6 +86,8 @@ decl_module! {
 		/// Add schedule_update at block_number
 		#[weight = 0]
 		pub fn schedule_dispatch(origin, call: Box<CallOf<T>>, when: DelayedDispatchTime<T::BlockNumber>) {
+			T::DispatchOrigin::try_origin(origin.clone()).map(|_| ()).or_else(ensure_root)?;
+
 			let who = match origin.into() {
 				Ok(frame_system::RawOrigin::Root) => None,
 				Ok(frame_system::RawOrigin::Signed(t)) => Some(t),
@@ -117,7 +120,7 @@ decl_module! {
 
 		/// Cancel schedule_update
 		#[weight = 0]
-		pub fn cancel_deplayed_dispatch(origin, at: T::BlockNumber, id: DispatchId) {
+		pub fn cancel_delayed_dispatch(origin, at: T::BlockNumber, id: DispatchId) {
 			let is_root = ensure_root(origin.clone()).is_ok();
 
 			if let Some((who, _, _)) = <DelayedNormalDispatches<T>>::get(at, id) {
@@ -135,7 +138,7 @@ decl_module! {
 			} else {
 				return Err(Error::<T>::DispatchNotExisted.into());
 			}
-			Self::deposit_event(RawEvent::CancelDeplayedDispatch(id));
+			Self::deposit_event(RawEvent::CancelDelayedDispatch(id));
 		}
 
 		fn on_initialize(now: T::BlockNumber) -> Weight {
diff --git a/schedule-update/src/mock.rs b/schedule-update/src/mock.rs
index 8f1a20a..894cfd1 100644
--- a/schedule-update/src/mock.rs
+++ b/schedule-update/src/mock.rs
@@ -3,7 +3,7 @@
 #![cfg(test)]
 
 use frame_support::{impl_outer_dispatch, impl_outer_event, impl_outer_origin, parameter_types};
-use frame_system as system;
+use frame_system::{self as system, ensure_signed};
 use sp_core::H256;
 use sp_runtime::{testing::Header, traits::IdentityLookup, Perbill};
 
@@ -82,12 +82,32 @@ impl pallet_balances::Trait for Runtime {
 	type AccountStore = System;
 }
 
+pub const ALICE: AccountId = 1u64;
+pub const BOB: AccountId = 2u64;
+
+// A mock schedule origin where only `ALICE` has permission.
+pub struct MockScheduleOrigin;
+
+impl EnsureOrigin<Origin> for MockScheduleOrigin {
+	type Success = ();
+
+	fn try_origin(o: Origin) -> Result<Self::Success, Origin> {
+		let who = ensure_signed(o.clone()).map_err(|_| o.clone())?;
+		if who == ALICE {
+			Ok(())
+		} else {
+			Err(o)
+		}
+	}
+}
+
 parameter_types! {
 	pub const MaxScheduleDispatchWeight: Weight = 150_000_000;
 }
 
 impl Trait for Runtime {
 	type Event = TestEvent;
+	type DispatchOrigin = MockScheduleOrigin;
 	type Call = Call;
 	type MaxScheduleDispatchWeight = MaxScheduleDispatchWeight;
 }
diff --git a/schedule-update/src/tests.rs b/schedule-update/src/tests.rs
index 21583e5..ff263ab 100644
--- a/schedule-update/src/tests.rs
+++ b/schedule-update/src/tests.rs
@@ -4,7 +4,7 @@
 
 use super::*;
 use frame_support::{assert_noop, assert_ok, traits::OnInitialize};
-use mock::{BalancesCall, Call, ExtBuilder, Origin, Runtime, ScheduleUpdateModule, System, TestEvent};
+use mock::{BalancesCall, Call, ExtBuilder, Origin, Runtime, ScheduleUpdateModule, System, TestEvent, ALICE, BOB};
 
 #[test]
 fn schedule_dispatch_should_work() {
@@ -14,7 +14,7 @@ fn schedule_dispatch_should_work() {
 		// NormalDispatches
 		let call = Call::Balances(BalancesCall::transfer(2, 11));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(2)
 		));
@@ -39,26 +39,49 @@ fn schedule_dispatch_should_work() {
 	});
 }
 
+#[test]
+fn schedule_dispatch_works_for_root_origin() {
+	ExtBuilder::default().build().execute_with(|| {
+		let call = Call::Balances(BalancesCall::transfer(2, 11));
+		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
+			Origin::ROOT,
+			Box::new(call),
+			DelayedDispatchTime::At(10)
+		));
+	});
+}
+
+#[test]
+fn schedule_dispatch_fails_if_not_allowed_origin() {
+	ExtBuilder::default().build().execute_with(|| {
+		let call = Call::Balances(BalancesCall::transfer(2, 11));
+		assert_noop!(
+			ScheduleUpdateModule::schedule_dispatch(Origin::signed(BOB), Box::new(call), DelayedDispatchTime::At(10)),
+			DispatchError::BadOrigin,
+		);
+	});
+}
+
 #[test]
 fn schedule_dispatch_should_fail() {
 	ExtBuilder::default().build().execute_with(|| {
 		let call = Call::Balances(BalancesCall::transfer(2, 11));
 		assert_noop!(
-			ScheduleUpdateModule::schedule_dispatch(Origin::signed(1), Box::new(call), DelayedDispatchTime::At(0)),
+			ScheduleUpdateModule::schedule_dispatch(Origin::signed(ALICE), Box::new(call), DelayedDispatchTime::At(0)),
 			Error::<Runtime>::InvalidDelayedDispatchTime
 		);
 	});
 }
 
 #[test]
-fn cancel_deplayed_dispatch_should_work() {
+fn cancel_delayed_dispatch_should_work() {
 	ExtBuilder::default().build().execute_with(|| {
 		System::set_block_number(1);
 
 		// NormalDispatches
 		let call = Call::Balances(BalancesCall::transfer(2, 11));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(2)
 		));
@@ -68,9 +91,13 @@ fn cancel_deplayed_dispatch_should_work() {
 			.iter()
 			.any(|record| record.event == schedule_dispatch_event));
 
-		assert_ok!(ScheduleUpdateModule::cancel_deplayed_dispatch(Origin::signed(1), 2, 0));
+		assert_ok!(ScheduleUpdateModule::cancel_delayed_dispatch(
+			Origin::signed(ALICE),
+			2,
+			0
+		));
 
-		let schedule_dispatch_event = TestEvent::schedule_update(RawEvent::CancelDeplayedDispatch(0));
+		let schedule_dispatch_event = TestEvent::schedule_update(RawEvent::CancelDelayedDispatch(0));
 		assert!(System::events()
 			.iter()
 			.any(|record| record.event == schedule_dispatch_event));
@@ -78,7 +105,7 @@ fn cancel_deplayed_dispatch_should_work() {
 		// root cancel NormalDispatches
 		let call = Call::Balances(BalancesCall::transfer(2, 12));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::After(3)
 		));
@@ -88,9 +115,9 @@ fn cancel_deplayed_dispatch_should_work() {
 			.iter()
 			.any(|record| record.event == schedule_dispatch_event));
 
-		assert_ok!(ScheduleUpdateModule::cancel_deplayed_dispatch(Origin::ROOT, 4, 1));
+		assert_ok!(ScheduleUpdateModule::cancel_delayed_dispatch(Origin::ROOT, 4, 1));
 
-		let schedule_dispatch_event = TestEvent::schedule_update(RawEvent::CancelDeplayedDispatch(1));
+		let schedule_dispatch_event = TestEvent::schedule_update(RawEvent::CancelDelayedDispatch(1));
 		assert!(System::events()
 			.iter()
 			.any(|record| record.event == schedule_dispatch_event));
@@ -108,9 +135,9 @@ fn cancel_deplayed_dispatch_should_work() {
 			.iter()
 			.any(|record| record.event == schedule_dispatch_event));
 
-		assert_ok!(ScheduleUpdateModule::cancel_deplayed_dispatch(Origin::ROOT, 5, 2));
+		assert_ok!(ScheduleUpdateModule::cancel_delayed_dispatch(Origin::ROOT, 5, 2));
 
-		let schedule_dispatch_event = TestEvent::schedule_update(RawEvent::CancelDeplayedDispatch(2));
+		let schedule_dispatch_event = TestEvent::schedule_update(RawEvent::CancelDelayedDispatch(2));
 		assert!(System::events()
 			.iter()
 			.any(|record| record.event == schedule_dispatch_event));
@@ -118,19 +145,19 @@ fn cancel_deplayed_dispatch_should_work() {
 }
 
 #[test]
-fn cancel_deplayed_dispatch_should_fail() {
+fn cancel_delayed_dispatch_should_fail() {
 	ExtBuilder::default().build().execute_with(|| {
 		System::set_block_number(1);
 
 		assert_noop!(
-			ScheduleUpdateModule::cancel_deplayed_dispatch(Origin::signed(1), 2, 0),
+			ScheduleUpdateModule::cancel_delayed_dispatch(Origin::signed(ALICE), 2, 0),
 			Error::<Runtime>::DispatchNotExisted
 		);
 
 		// NormalDispatches
 		let call = Call::Balances(BalancesCall::transfer(2, 11));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(2)
 		));
@@ -141,7 +168,7 @@ fn cancel_deplayed_dispatch_should_fail() {
 			.any(|record| record.event == schedule_dispatch_event));
 
 		assert_noop!(
-			ScheduleUpdateModule::cancel_deplayed_dispatch(Origin::signed(2), 2, 0),
+			ScheduleUpdateModule::cancel_delayed_dispatch(Origin::signed(BOB), 2, 0),
 			Error::<Runtime>::NoPermission
 		);
 
@@ -159,7 +186,7 @@ fn cancel_deplayed_dispatch_should_fail() {
 			.any(|record| record.event == schedule_dispatch_event));
 
 		assert_noop!(
-			ScheduleUpdateModule::cancel_deplayed_dispatch(Origin::signed(2), 5, 1),
+			ScheduleUpdateModule::cancel_delayed_dispatch(Origin::signed(BOB), 5, 1),
 			Error::<Runtime>::NoPermission
 		);
 	});
@@ -173,14 +200,14 @@ fn on_initialize_should_work() {
 		// NormalDispatches
 		let call = Call::Balances(BalancesCall::transfer(2, 11));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(2)
 		));
 
 		let call = Call::Balances(BalancesCall::transfer(2, 12));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(3)
 		));
@@ -258,7 +285,7 @@ fn on_initialize_should_fail() {
 		// NormalDispatches balance not enough
 		let call = Call::Balances(BalancesCall::transfer(2, 110));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(2)
 		));
@@ -286,7 +313,7 @@ fn on_initialize_should_fail() {
 		// OperationalDispatches not root
 		let call = Call::Balances(BalancesCall::set_balance(3, 10, 11));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::After(10)
 		));
@@ -314,21 +341,21 @@ fn on_initialize_weight_exceed() {
 		// NormalDispatches
 		let call = Call::Balances(BalancesCall::transfer(2, 11));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(2)
 		));
 
 		let call = Call::Balances(BalancesCall::transfer(2, 12));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(2)
 		));
 
 		let call = Call::Balances(BalancesCall::transfer(2, 13));
 		assert_ok!(ScheduleUpdateModule::schedule_dispatch(
-			Origin::signed(1),
+			Origin::signed(ALICE),
 			Box::new(call),
 			DelayedDispatchTime::At(2)
 		));
-- 
GitLab