diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index c7920629bf..0638e62faa 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -26,8 +26,8 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ construct_runtime, parameter_types, traits::{ - Currency, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, - Nothing, OnUnbalanced, U128CurrencyToVote, + Currency, EqualPrivilegeOnly, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, + LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, }, weights::{ constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight, WEIGHT_PER_SECOND}, @@ -345,6 +345,7 @@ impl pallet_scheduler::Config for Runtime { type ScheduleOrigin = EnsureRoot; type MaxScheduledPerBlock = MaxScheduledPerBlock; type WeightInfo = pallet_scheduler::weights::SubstrateWeight; + type OriginPrivilegeCmp = EqualPrivilegeOnly; } parameter_types! { diff --git a/substrate/frame/democracy/src/tests.rs b/substrate/frame/democracy/src/tests.rs index f56667e909..06c4ac666c 100644 --- a/substrate/frame/democracy/src/tests.rs +++ b/substrate/frame/democracy/src/tests.rs @@ -22,7 +22,7 @@ use crate as pallet_democracy; use codec::Encode; use frame_support::{ assert_noop, assert_ok, ord_parameter_types, parameter_types, - traits::{Contains, GenesisBuild, OnInitialize, SortedMembers}, + traits::{Contains, EqualPrivilegeOnly, GenesisBuild, OnInitialize, SortedMembers}, weights::Weight, }; use frame_system::{EnsureRoot, EnsureSignedBy}; @@ -118,6 +118,7 @@ impl pallet_scheduler::Config for Test { type ScheduleOrigin = EnsureRoot; type MaxScheduledPerBlock = (); type WeightInfo = (); + type OriginPrivilegeCmp = EqualPrivilegeOnly; } parameter_types! { pub const ExistentialDeposit: u64 = 1; diff --git a/substrate/frame/scheduler/src/lib.rs b/substrate/frame/scheduler/src/lib.rs index ca9e15812a..d59c42cc85 100644 --- a/substrate/frame/scheduler/src/lib.rs +++ b/substrate/frame/scheduler/src/lib.rs @@ -58,7 +58,7 @@ use frame_support::{ dispatch::{DispatchError, DispatchResult, Dispatchable, Parameter}, traits::{ schedule::{self, DispatchTime}, - EnsureOrigin, Get, IsType, OriginTrait, + EnsureOrigin, Get, IsType, OriginTrait, PrivilegeCmp, }, weights::{GetDispatchInfo, Weight}, }; @@ -69,7 +69,7 @@ use sp_runtime::{ traits::{BadOrigin, One, Saturating, Zero}, RuntimeDebug, }; -use sp_std::{borrow::Borrow, marker::PhantomData, prelude::*}; +use sp_std::{borrow::Borrow, cmp::Ordering, marker::PhantomData, prelude::*}; pub use weights::WeightInfo; /// Just a simple index for naming period tasks. @@ -160,6 +160,15 @@ pub mod pallet { /// Required origin to schedule or cancel calls. type ScheduleOrigin: EnsureOrigin<::Origin>; + /// Compare the privileges of origins. + /// + /// This will be used when canceling a task, to ensure that the origin that tries + /// to cancel has greater or equal privileges as the origin that created the scheduled task. + /// + /// For simplicity the [`EqualPrivilegeOnly`](frame_support::traits::EqualPrivilegeOnly) can + /// be used. This will only check if two given origins are equal. + type OriginPrivilegeCmp: PrivilegeCmp; + /// The maximum number of scheduled calls in the queue for a single block. /// Not strictly enforced, but used for weight estimation. #[pallet::constant] @@ -614,7 +623,10 @@ impl Pallet { Ok(None), |s| -> Result>, DispatchError> { if let (Some(ref o), Some(ref s)) = (origin, s.borrow()) { - if *o != s.origin { + if matches!( + T::OriginPrivilegeCmp::cmp_privilege(o, &s.origin), + Some(Ordering::Less) | None + ) { return Err(BadOrigin.into()) } }; @@ -709,7 +721,10 @@ impl Pallet { Agenda::::try_mutate(when, |agenda| -> DispatchResult { if let Some(s) = agenda.get_mut(i) { if let (Some(ref o), Some(ref s)) = (origin, s.borrow()) { - if *o != s.origin { + if matches!( + T::OriginPrivilegeCmp::cmp_privilege(o, &s.origin), + Some(Ordering::Less) | None + ) { return Err(BadOrigin.into()) } } @@ -832,7 +847,7 @@ mod tests { use crate as scheduler; use frame_support::{ assert_err, assert_noop, assert_ok, ord_parameter_types, parameter_types, - traits::{Contains, OnFinalize, OnInitialize}, + traits::{Contains, EqualPrivilegeOnly, OnFinalize, OnInitialize}, weights::constants::RocksDbWeight, Hashable, }; @@ -980,6 +995,7 @@ mod tests { type ScheduleOrigin = EnsureOneOf, EnsureSignedBy>; type MaxScheduledPerBlock = MaxScheduledPerBlock; type WeightInfo = (); + type OriginPrivilegeCmp = EqualPrivilegeOnly; } pub type LoggerCall = logger::Call; diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 43eadb3a05..bb990e2564 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -50,10 +50,10 @@ pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter mod misc; pub use misc::{ - Backing, ConstU32, EnsureInherentsAreFirst, EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, - GetBacking, GetDefault, HandleLifetime, IsSubType, IsType, Len, OffchainWorker, - OnKilledAccount, OnNewAccount, SameOrOther, Time, TryDrop, UnixTime, WrapperKeepOpaque, - WrapperOpaque, + Backing, ConstU32, EnsureInherentsAreFirst, EqualPrivilegeOnly, EstimateCallFee, ExecuteBlock, + ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, IsSubType, IsType, Len, + OffchainWorker, OnKilledAccount, OnNewAccount, PrivilegeCmp, SameOrOther, Time, TryDrop, + UnixTime, WrapperKeepOpaque, WrapperOpaque, }; mod stored_map; diff --git a/substrate/frame/support/src/traits/misc.rs b/substrate/frame/support/src/traits/misc.rs index 6587945604..0a3fb045d6 100644 --- a/substrate/frame/support/src/traits/misc.rs +++ b/substrate/frame/support/src/traits/misc.rs @@ -21,7 +21,7 @@ use crate::dispatch::Parameter; use codec::{CompactLen, Decode, DecodeAll, Encode, EncodeLike, Input, MaxEncodedLen}; use scale_info::{build::Fields, meta_type, Path, Type, TypeInfo, TypeParameter}; use sp_runtime::{traits::Block as BlockT, DispatchError}; -use sp_std::prelude::*; +use sp_std::{cmp::Ordering, prelude::*}; /// Anything that can have a `::len()` method. pub trait Len { @@ -289,6 +289,26 @@ pub trait ExecuteBlock { fn execute_block(block: Block); } +/// Something that can compare privileges of two origins. +pub trait PrivilegeCmp { + /// Compare the `left` to the `right` origin. + /// + /// The returned ordering should be from the pov of the `left` origin. + /// + /// Should return `None` when it can not compare the given origins. + fn cmp_privilege(left: &Origin, right: &Origin) -> Option; +} + +/// Implementation of [`PrivilegeCmp`] that only checks for equal origins. +/// +/// This means it will either return [`Origin::Equal`] or `None`. +pub struct EqualPrivilegeOnly; +impl PrivilegeCmp for EqualPrivilegeOnly { + fn cmp_privilege(left: &Origin, right: &Origin) -> Option { + (left == right).then(|| Ordering::Equal) + } +} + /// Off-chain computation trait. /// /// Implementing this trait on a module allows you to perform long-running tasks