From e819879dd1907cc7dd530530bdd7401546e904a0 Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Fri, 16 Sep 2022 13:58:22 +0000 Subject: [PATCH] fix: only root can set/change scheduled ops priorities --- --- a/pallets/scheduler/src/lib.rs +++ b/pallets/scheduler/src/lib.rs @@ -86,7 +86,7 @@ use sp_std::{borrow::Borrow, cmp::Ordering, marker::PhantomData, prelude::*}; use frame_support::{ - dispatch::{DispatchError, DispatchResult, Dispatchable, Parameter, GetDispatchInfo}, + dispatch::{DispatchError, DispatchResult, Dispatchable, UnfilteredDispatchable, Parameter}, traits::{ schedule::{self, DispatchTime, MaybeHashed}, NamedReservableCurrency, EnsureOrigin, Get, IsType, OriginTrait, PrivilegeCmp, @@ -135,6 +135,12 @@ pub type Scheduled = ScheduledV3; +pub enum ScheduledEnsureOriginSuccess { + Root, + Signed(AccountId), + Unsigned, +} + #[cfg(feature = "runtime-benchmarks")] mod preimage_provider { use frame_support::traits::PreimageRecipient; @@ -191,7 +197,7 @@ use frame_support::{ dispatch::PostDispatchInfo, pallet_prelude::*, - traits::{schedule::LookupError, PreimageProvider}, + traits::{schedule::{LookupError, LOWEST_PRIORITY}, PreimageProvider}, }; use frame_system::pallet_prelude::*; @@ -222,11 +228,10 @@ /// The aggregated call type. type RuntimeCall: Parameter - + Dispatchable< - RuntimeOrigin = ::RuntimeOrigin, - PostInfo = PostDispatchInfo, - > + GetDispatchInfo - + From>; + + Dispatchable::RuntimeOrigin, PostInfo = PostDispatchInfo> + + UnfilteredDispatchable::RuntimeOrigin> + + GetDispatchInfo + + From>; /// The maximum weight that may be scheduled per block for any dispatchables of less /// priority than `schedule::HARD_DEADLINE`. @@ -234,7 +239,10 @@ type MaximumWeight: Get; /// Required origin to schedule or cancel calls. - type ScheduleOrigin: EnsureOrigin<::RuntimeOrigin>; + type ScheduleOrigin: EnsureOrigin<::RuntimeOrigin, Success = ScheduledEnsureOriginSuccess>; + + /// Required origin to set/change calls' priority. + type PrioritySetOrigin: EnsureOrigin<::RuntimeOrigin>; /// Compare the privileges of origins. /// @@ -285,7 +293,7 @@ /// Resolve the call dispatch, including any post-dispatch operations. fn dispatch_call( - signer: T::AccountId, + signer: Option, function: ::RuntimeCall, ) -> Result< Result>, @@ -317,6 +325,12 @@ Scheduled { when: T::BlockNumber, index: u32 }, /// Canceled some task. Canceled { when: T::BlockNumber, index: u32 }, + /// Scheduled task's priority has changed + PriorityChanged { + when: T::BlockNumber, + index: u32, + priority: schedule::Priority, + }, /// Dispatched some task. Dispatched { task: TaskAddress, @@ -432,26 +446,24 @@ continue; } - let sender = ensure_signed( - <::RuntimeOrigin as From>::from( - s.origin.clone(), - ) - .into(), - ) - .unwrap(); + let scheduled_origin = <::RuntimeOrigin as From>::from(s.origin.clone()); + let ensured_origin = T::ScheduleOrigin::ensure_origin(scheduled_origin.into()).unwrap(); - // // if call have id it was be reserved - // if s.maybe_id.is_some() { - // let _ = T::CallExecutor::pay_for_call( - // s.maybe_id.unwrap(), - // sender.clone(), - // call.clone(), - // ); - // } - - // Execute transaction via chain default pipeline - // That means dispatch will be processed like any user's extrinsic e.g. transaction fees will be taken - let r = T::CallExecutor::dispatch_call(sender, call.clone()); + let r; + match ensured_origin { + ScheduledEnsureOriginSuccess::Root => { + r = Ok(call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into())); + }, + ScheduledEnsureOriginSuccess::Signed(sender) => { + // Execute transaction via chain default pipeline + // That means dispatch will be processed like any user's extrinsic e.g. transaction fees will be taken + r = T::CallExecutor::dispatch_call(Some(sender), call.clone()); + }, + ScheduledEnsureOriginSuccess::Unsigned => { + // Unsigned version of the above + r = T::CallExecutor::dispatch_call(None, call.clone()); + } + } let mut actual_call_weight: Weight = item_weight; let result: Result<_, DispatchError> = match r { @@ -521,16 +533,21 @@ id: ScheduledId, when: T::BlockNumber, maybe_periodic: Option>, - priority: schedule::Priority, + priority: Option, call: Box>, ) -> DispatchResult { T::ScheduleOrigin::ensure_origin(origin.clone())?; + + if priority.is_some() { + T::PrioritySetOrigin::ensure_origin(origin.clone())?; + } + let origin = ::RuntimeOrigin::from(origin); Self::do_schedule_named( id, DispatchTime::At(when), maybe_periodic, - priority, + priority.unwrap_or(LOWEST_PRIORITY), origin.caller().clone(), *call, )?; @@ -557,21 +574,37 @@ id: ScheduledId, after: T::BlockNumber, maybe_periodic: Option>, - priority: schedule::Priority, + priority: Option, call: Box>, ) -> DispatchResult { T::ScheduleOrigin::ensure_origin(origin.clone())?; + + if priority.is_some() { + T::PrioritySetOrigin::ensure_origin(origin.clone())?; + } + let origin = ::RuntimeOrigin::from(origin); Self::do_schedule_named( id, DispatchTime::After(after), maybe_periodic, - priority, + priority.unwrap_or(LOWEST_PRIORITY), origin.caller().clone(), *call, )?; Ok(()) } + + #[pallet::weight(::WeightInfo::change_named_priority(T::MaxScheduledPerBlock::get()))] + pub fn change_named_priority( + origin: OriginFor, + id: ScheduledId, + priority: schedule::Priority, + ) -> DispatchResult { + T::PrioritySetOrigin::ensure_origin(origin.clone())?; + let origin = ::Origin::from(origin); + Self::do_change_named_priority(origin.caller().clone(), id, priority) + } } } @@ -724,4 +757,31 @@ } }) } + + fn do_change_named_priority( + origin: T::PalletsOrigin, + id: ScheduledId, + priority: schedule::Priority, + ) -> DispatchResult { + match Lookup::::get(id) { + Some((when, index)) => { + let i = index as usize; + Agenda::::try_mutate(when, |agenda| { + if let Some(Some(s)) = agenda.get_mut(i) { + if matches!( + T::OriginPrivilegeCmp::cmp_privilege(&origin, &s.origin), + Some(Ordering::Less) | None + ) { + return Err(BadOrigin.into()); + } + + s.priority = priority; + Self::deposit_event(Event::PriorityChanged { when, index, priority }); + } + Ok(()) + }) + }, + None => Err(Error::::NotFound.into()) + } + } } -- gitstuff