From d3a8497757b2ea9500063b30b058fb86453b9982 Mon Sep 17 00:00:00 2001 From: Niklas Date: Thu, 2 Dec 2021 18:35:58 +0100 Subject: [PATCH] Revert "Move EnsureOneOf into the dispatch.rs, make an interface more general (#10379)" This reverts commit 089aa2509f928b161b0f970d1ff670635f6a26fe. --- bin/node/runtime/src/lib.rs | 7 +++- frame/identity/src/tests.rs | 10 ++--- frame/scheduler/src/lib.rs | 6 +-- frame/support/src/traits.rs | 2 +- frame/support/src/traits/dispatch.rs | 60 +--------------------------- frame/system/src/lib.rs | 25 +++++++++++- frame/system/src/tests.rs | 11 +++++ 7 files changed, 50 insertions(+), 71 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index e0f364223c495..299b7257f9d45 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -37,7 +37,7 @@ use frame_support::{ }; use frame_system::{ limits::{BlockLength, BlockWeights}, - EnsureRoot, + EnsureOneOf, EnsureRoot, }; pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; @@ -529,6 +529,7 @@ impl pallet_staking::Config for Runtime { type SlashDeferDuration = SlashDeferDuration; /// A super-majority of the council can cancel the slash. type SlashCancelOrigin = EnsureOneOf< + AccountId, EnsureRoot, pallet_collective::EnsureProportionAtLeast<_3, _4, AccountId, CouncilCollective>, >; @@ -718,6 +719,7 @@ impl pallet_democracy::Config for Runtime { // To cancel a proposal before it has been passed, the technical committee must be unanimous or // Root must agree. type CancelProposalOrigin = EnsureOneOf< + AccountId, EnsureRoot, pallet_collective::EnsureProportionAtLeast<_1, _1, AccountId, TechnicalCollective>, >; @@ -808,6 +810,7 @@ impl pallet_collective::Config for Runtime { } type EnsureRootOrHalfCouncil = EnsureOneOf< + AccountId, EnsureRoot, pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>, >; @@ -847,10 +850,12 @@ impl pallet_treasury::Config for Runtime { type PalletId = TreasuryPalletId; type Currency = Balances; type ApproveOrigin = EnsureOneOf< + AccountId, EnsureRoot, pallet_collective::EnsureProportionAtLeast<_3, _5, AccountId, CouncilCollective>, >; type RejectOrigin = EnsureOneOf< + AccountId, EnsureRoot, pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>, >; diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs index 31b93e70fd3dc..c842b0e2f64be 100644 --- a/frame/identity/src/tests.rs +++ b/frame/identity/src/tests.rs @@ -21,10 +21,8 @@ use super::*; use crate as pallet_identity; use codec::{Decode, Encode}; -use frame_support::{ - assert_noop, assert_ok, ord_parameter_types, parameter_types, traits::EnsureOneOf, BoundedVec, -}; -use frame_system::{EnsureRoot, EnsureSignedBy}; +use frame_support::{assert_noop, assert_ok, ord_parameter_types, parameter_types, BoundedVec}; +use frame_system::{EnsureOneOf, EnsureRoot, EnsureSignedBy}; use sp_core::H256; use sp_runtime::{ testing::Header, @@ -102,8 +100,8 @@ ord_parameter_types! { pub const One: u64 = 1; pub const Two: u64 = 2; } -type EnsureOneOrRoot = EnsureOneOf, EnsureSignedBy>; -type EnsureTwoOrRoot = EnsureOneOf, EnsureSignedBy>; +type EnsureOneOrRoot = EnsureOneOf, EnsureSignedBy>; +type EnsureTwoOrRoot = EnsureOneOf, EnsureSignedBy>; impl pallet_identity::Config for Test { type Event = Event; type Currency = Balances; diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 7540d36f0a2fc..d25fc3b376e83 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -801,11 +801,11 @@ mod tests { use crate as scheduler; use frame_support::{ assert_err, assert_noop, assert_ok, ord_parameter_types, parameter_types, - traits::{Contains, EnsureOneOf, EqualPrivilegeOnly, OnFinalize, OnInitialize}, + traits::{Contains, EqualPrivilegeOnly, OnFinalize, OnInitialize}, weights::constants::RocksDbWeight, Hashable, }; - use frame_system::{EnsureRoot, EnsureSignedBy}; + use frame_system::{EnsureOneOf, EnsureRoot, EnsureSignedBy}; use sp_core::H256; use sp_runtime::{ testing::Header, @@ -946,7 +946,7 @@ mod tests { type PalletsOrigin = OriginCaller; type Call = Call; type MaximumWeight = MaximumSchedulerWeight; - type ScheduleOrigin = EnsureOneOf, EnsureSignedBy>; + type ScheduleOrigin = EnsureOneOf, EnsureSignedBy>; type MaxScheduledPerBlock = MaxScheduledPerBlock; type WeightInfo = (); type OriginPrivilegeCmp = EqualPrivilegeOnly; diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 9b5453c0a01ae..bb990e25646db 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -84,7 +84,7 @@ pub use storage::{ }; mod dispatch; -pub use dispatch::{EnsureOneOf, EnsureOrigin, OriginTrait, UnfilteredDispatchable}; +pub use dispatch::{EnsureOrigin, OriginTrait, UnfilteredDispatchable}; mod voting; pub use voting::{CurrencyToVote, SaturatingCurrencyToVote, U128CurrencyToVote}; diff --git a/frame/support/src/traits/dispatch.rs b/frame/support/src/traits/dispatch.rs index d7605593cd604..92b832ba32961 100644 --- a/frame/support/src/traits/dispatch.rs +++ b/frame/support/src/traits/dispatch.rs @@ -18,7 +18,7 @@ //! Traits for dealing with dispatching calls and the origin from which they are dispatched. use crate::dispatch::DispatchResultWithPostInfo; -use sp_runtime::{traits::BadOrigin, Either}; +use sp_runtime::traits::BadOrigin; /// Some sort of check on the origin is performed by this object. pub trait EnsureOrigin { @@ -94,61 +94,3 @@ pub trait OriginTrait: Sized { /// Create with system signed origin and `frame_system::Config::BaseCallFilter`. fn signed(by: Self::AccountId) -> Self; } - -/// The "OR gate" implementation of `EnsureOrigin`. -/// -/// Origin check will pass if `L` or `R` origin check passes. `L` is tested first. -pub struct EnsureOneOf(sp_std::marker::PhantomData<(L, R)>); - -impl, R: EnsureOrigin> - EnsureOrigin for EnsureOneOf -{ - type Success = Either; - fn try_origin(o: OuterOrigin) -> Result { - L::try_origin(o) - .map_or_else(|o| R::try_origin(o).map(|o| Either::Right(o)), |o| Ok(Either::Left(o))) - } - - #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> OuterOrigin { - L::successful_origin() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - struct EnsureSuccess; - struct EnsureFail; - - impl EnsureOrigin<()> for EnsureSuccess { - type Success = (); - fn try_origin(_: ()) -> Result { - Ok(()) - } - #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> () { - () - } - } - - impl EnsureOrigin<()> for EnsureFail { - type Success = (); - fn try_origin(_: ()) -> Result { - Err(()) - } - #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> () { - () - } - } - - #[test] - fn ensure_one_of_test() { - assert!(>::try_origin(()).is_ok()); - assert!(>::try_origin(()).is_ok()); - assert!(>::try_origin(()).is_ok()); - assert!(>::try_origin(()).is_err()); - } -} diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 37444f905e686..0b00a7d8f5973 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -73,7 +73,7 @@ use sp_runtime::{ CheckEqual, Dispatchable, Hash, Lookup, LookupError, MaybeDisplay, MaybeMallocSizeOf, MaybeSerializeDeserialize, Member, One, Saturating, SimpleBitOps, StaticLookup, Zero, }, - DispatchError, Perbill, RuntimeDebug, + DispatchError, Either, Perbill, RuntimeDebug, }; #[cfg(any(feature = "std", test))] use sp_std::map; @@ -902,6 +902,29 @@ impl EnsureOrigin for EnsureNever { } } +/// The "OR gate" implementation of `EnsureOrigin`. +/// +/// Origin check will pass if `L` or `R` origin check passes. `L` is tested first. +pub struct EnsureOneOf(sp_std::marker::PhantomData<(AccountId, L, R)>); +impl< + AccountId, + O: Into, O>> + From>, + L: EnsureOrigin, + R: EnsureOrigin, + > EnsureOrigin for EnsureOneOf +{ + type Success = Either; + fn try_origin(o: O) -> Result { + L::try_origin(o) + .map_or_else(|o| R::try_origin(o).map(|o| Either::Right(o)), |o| Ok(Either::Left(o))) + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> O { + L::successful_origin() + } +} + /// Ensure that the origin `o` represents a signed extrinsic (i.e. transaction). /// Returns `Ok` with the account that signed the extrinsic or an `Err` otherwise. pub fn ensure_signed(o: OuterOrigin) -> Result diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index 5d83482acf02c..a4dd3403f2c3a 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -464,6 +464,17 @@ fn events_not_emitted_during_genesis() { }); } +#[test] +fn ensure_one_of_works() { + fn ensure_root_or_signed(o: RawOrigin) -> Result, Origin> { + EnsureOneOf::, EnsureSigned>::try_origin(o.into()) + } + + assert_eq!(ensure_root_or_signed(RawOrigin::Root).unwrap(), Either::Left(())); + assert_eq!(ensure_root_or_signed(RawOrigin::Signed(0)).unwrap(), Either::Right(0)); + assert!(ensure_root_or_signed(RawOrigin::None).is_err()); +} + #[test] fn extrinsics_root_is_calculated_correctly() { new_test_ext().execute_with(|| {