From 0bcbfd58bc7d3d0f24f8cdf1dfa203dbeda66103 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Wed, 1 Jul 2020 12:31:58 +0800 Subject: [PATCH] update orml-authority (#225) * add AsOrigin * make authority instantiatable * use ensure_origin --- authority/src/lib.rs | 79 +++++++++++++++++++++++------------------- authority/src/mock.rs | 19 ++++++++++ authority/src/tests.rs | 79 +++++++++++++++++++++++++++++------------- 3 files changed, 118 insertions(+), 59 deletions(-) diff --git a/authority/src/lib.rs b/authority/src/lib.rs index 2b0ecbd..258e483 100644 --- a/authority/src/lib.rs +++ b/authority/src/lib.rs @@ -1,11 +1,11 @@ #![cfg_attr(not(feature = "std"), no_std)] -// Disable the following two lints since they originate from an external macro (namely decl_storage) +// Disable the following three lints since they originate from an external macro #![allow(clippy::string_lit_as_bytes)] #![allow(clippy::boxed_local)] #![allow(clippy::borrowed_box)] use frame_support::{ - decl_error, decl_module, + decl_error, decl_module, decl_storage, dispatch::PostDispatchInfo, ensure, traits::{EnsureOrigin, Get}, @@ -15,7 +15,7 @@ use frame_support::{ use frame_system::{self as system}; use orml_traits::{DelayedDispatchTime, DispatchId, Scheduler}; use sp_runtime::{ - traits::{BadOrigin, CheckedAdd, CheckedSub, Dispatchable}, + traits::{CheckedAdd, CheckedSub, Dispatchable}, RuntimeDebug, }; use sp_std::prelude::*; @@ -24,23 +24,20 @@ mod mock; mod tests; #[derive(PartialEq, Eq, Clone, RuntimeDebug)] -pub struct DelayedOrigin<BlockNumber, Origin> { +pub struct DelayedOrigin<BlockNumber, Origin, I> { pub delay: BlockNumber, pub origin: Origin, + _phantom: sp_std::marker::PhantomData<I>, } -/// Origin for the authority module. -pub type Origin<T> = - DelayedOrigin<<T as system::Trait>::BlockNumber, system::RawOrigin<<T as system::Trait>::AccountId>>; - -pub struct EnsureDelayed<Delay, Inner, BlockNumber>(sp_std::marker::PhantomData<(Delay, Inner, BlockNumber)>); - +pub struct EnsureDelayed<Delay, Inner, BlockNumber, I>(sp_std::marker::PhantomData<(Delay, Inner, BlockNumber, I)>); impl< - O: Into<Result<DelayedOrigin<BlockNumber, O>, O>> + From<DelayedOrigin<BlockNumber, O>>, + O: Into<Result<DelayedOrigin<BlockNumber, O, I>, O>> + From<DelayedOrigin<BlockNumber, O, I>>, Delay: Get<BlockNumber>, Inner: EnsureOrigin<O>, BlockNumber: PartialOrd, - > EnsureOrigin<O> for EnsureDelayed<Delay, Inner, BlockNumber> + I, + > EnsureOrigin<O> for EnsureDelayed<Delay, Inner, BlockNumber, I> { type Success = Inner::Success; @@ -55,8 +52,13 @@ impl< } } -pub trait Trait: system::Trait { - type Origin: From<DelayedOrigin<Self::BlockNumber, system::RawOrigin<Self::AccountId>>> +/// Origin for the authority module. +pub type Origin<T, I = DefaultInstance> = + DelayedOrigin<<T as system::Trait>::BlockNumber, system::RawOrigin<<T as system::Trait>::AccountId>, I>; +type CallOf<T, I = DefaultInstance> = <T as Trait<I>>::Call; + +pub trait Trait<I: Instance = DefaultInstance>: system::Trait { + type Origin: From<DelayedOrigin<Self::BlockNumber, system::RawOrigin<Self::AccountId>, I>> + From<system::RawOrigin<Self::AccountId>>; type Call: Parameter + Dispatchable<Origin = <Self as system::Trait>::Origin, PostInfo = PostDispatchInfo> @@ -66,74 +68,81 @@ pub trait Trait: system::Trait { type DelayedDispatchOrigin: EnsureOrigin<<Self as system::Trait>::Origin>; type VetoOrigin: EnsureOrigin<<Self as system::Trait>::Origin>; type InstantDispatchOrigin: EnsureOrigin<<Self as system::Trait>::Origin>; - type Scheduler: Scheduler<Self::BlockNumber, Origin = <Self as Trait>::Origin, Call = <Self as Trait>::Call>; + type Scheduler: Scheduler<Self::BlockNumber, Origin = <Self as Trait<I>>::Origin, Call = <Self as Trait<I>>::Call>; type MinimumDelay: Get<Self::BlockNumber>; + type AsOrigin: Get<<Self as system::Trait>::Origin>; } decl_error! { /// Error for authority module. - pub enum Error for Module<T: Trait> { + pub enum Error for Module<T: Trait<I>, I: Instance> { BlockNumberOverflow, InvalidDelayedDispatchTime, + OriginConvertFailed, } } -type CallOf<T> = <T as Trait>::Call; +decl_storage! { + trait Store for Module<T: Trait<I>, I: Instance=DefaultInstance> as Authority {} +} decl_module! { - pub struct Module<T: Trait> for enum Call where origin: <T as system::Trait>::Origin { - type Error = Error<T>; + pub struct Module<T: Trait<I>, I: Instance = DefaultInstance> for enum Call where origin: <T as system::Trait>::Origin { + type Error = Error<T, I>; const MinimumDelay: T::BlockNumber = T::MinimumDelay::get(); #[weight = (call.get_dispatch_info().weight + 10_000, call.get_dispatch_info().class)] - pub fn dispatch_root(origin, call: Box<CallOf<T>>) { - T::RootDispatchOrigin::try_origin(origin).map_err(|_| BadOrigin)?; - call.dispatch(frame_system::RawOrigin::Root.into()).map(|_| ()).map_err(|e| e.error)?; + pub fn dispatch(origin, call: Box<CallOf<T, I>>) { + T::RootDispatchOrigin::ensure_origin(origin)?; + call.dispatch(T::AsOrigin::get()).map(|_| ()).map_err(|e| e.error)?; } #[weight = (call.get_dispatch_info().weight + 10_000, call.get_dispatch_info().class)] - pub fn schedule_dispatch_root(origin, call: Box<CallOf<T>>, when: DelayedDispatchTime<T::BlockNumber>) { + pub fn schedule_dispatch(origin, call: Box<CallOf<T, I>>, when: DelayedDispatchTime<T::BlockNumber>) { let now = <frame_system::Module<T>>::block_number(); let when_block = match when { DelayedDispatchTime::At(at_block) => { - ensure!(at_block > now, Error::<T>::InvalidDelayedDispatchTime); + ensure!(at_block > now, Error::<T, I>::InvalidDelayedDispatchTime); at_block }, DelayedDispatchTime::After(after_block) => { - now.checked_add(&after_block).ok_or(Error::<T>::BlockNumberOverflow)? + now.checked_add(&after_block).ok_or(Error::<T, I>::BlockNumberOverflow)? }, }; if when_block >= T::MinimumDelay::get() + now { - T::DelayedRootDispatchOrigin::try_origin(origin).map_err(|_| BadOrigin)?; + T::DelayedRootDispatchOrigin::ensure_origin(origin)?; } else { - T::InstantDispatchOrigin::try_origin(origin).map_err(|_| BadOrigin)?; + T::InstantDispatchOrigin::ensure_origin(origin)?; } - // schedule call with Root origin - let _ = T::Scheduler::schedule(frame_system::RawOrigin::Root.into(), *call, when); + let raw_origin: system::RawOrigin<T::AccountId> = T::AsOrigin::get().into().map_err(|_| Error::<T, I>::OriginConvertFailed)?; + + // schedule call with as origin + let _ = T::Scheduler::schedule(raw_origin.into(), *call, when); } #[weight = (call.get_dispatch_info().weight + 10_000, call.get_dispatch_info().class)] - pub fn schedule_dispatch_delayed(origin, call: Box<CallOf<T>>, when: DelayedDispatchTime<T::BlockNumber>) { - T::DelayedDispatchOrigin::try_origin(origin.clone()).map_err(|_| BadOrigin)?; + pub fn schedule_dispatch_delayed(origin, call: Box<CallOf<T, I>>, when: DelayedDispatchTime<T::BlockNumber>) { + T::DelayedDispatchOrigin::ensure_origin(origin.clone())?; let now = <frame_system::Module<T>>::block_number(); let delay_block = match when { DelayedDispatchTime::At(at_block) => { - at_block.checked_sub(&now).ok_or(Error::<T>::InvalidDelayedDispatchTime)? + at_block.checked_sub(&now).ok_or(Error::<T, I>::InvalidDelayedDispatchTime)? }, DelayedDispatchTime::After(after_block) => { - ensure!(after_block.checked_add(&now).is_some(), Error::<T>::BlockNumberOverflow); + ensure!(after_block.checked_add(&now).is_some(), Error::<T, I>::BlockNumberOverflow); after_block }, }; - let raw_origin = origin.into().map_err(|_| BadOrigin)?; + let raw_origin = origin.into().map_err(|_| Error::<T, I>::OriginConvertFailed)?; let delayed_origin = DelayedOrigin{ delay: delay_block, origin: raw_origin, + _phantom: sp_std::marker::PhantomData, }; // dispatch call with DelayedOrigin @@ -142,7 +151,7 @@ decl_module! { #[weight = 0] pub fn veto(origin, dispatch_id: DispatchId) { - T::VetoOrigin::try_origin(origin).map_err(|_| BadOrigin)?; + T::VetoOrigin::ensure_origin(origin)?; T::Scheduler::cancel(dispatch_id); } } diff --git a/authority/src/mock.rs b/authority/src/mock.rs index fa05037..dcf821d 100644 --- a/authority/src/mock.rs +++ b/authority/src/mock.rs @@ -66,10 +66,14 @@ impl Scheduler<BlockNumber> for MockScheduler { ord_parameter_types! { pub const One: AccountId = 1; pub const Two: AccountId = 2; + pub const Three: AccountId = 3; } parameter_types! { pub const MinimumDelay: BlockNumber = 10; + pub const MinimumDelayForInstance1: BlockNumber = 20; + pub AsOrigin: Origin = Origin::root(); + pub AsOriginForInstance1: Origin = system::RawOrigin::Signed(Three::get()).into(); } impl Trait for Runtime { @@ -82,6 +86,20 @@ impl Trait for Runtime { type InstantDispatchOrigin = EnsureSignedBy<Two, AccountId>; type Scheduler = MockScheduler; type MinimumDelay = MinimumDelay; + type AsOrigin = AsOrigin; +} + +impl Trait<Instance1> for Runtime { + type Origin = Origin; + type Call = Call; + type RootDispatchOrigin = EnsureSignedBy<One, AccountId>; + type DelayedRootDispatchOrigin = EnsureSignedBy<One, AccountId>; + type DelayedDispatchOrigin = EnsureSignedBy<One, AccountId>; + type VetoOrigin = EnsureSignedBy<One, AccountId>; + type InstantDispatchOrigin = EnsureSignedBy<Two, AccountId>; + type Scheduler = MockScheduler; + type MinimumDelay = MinimumDelayForInstance1; + type AsOrigin = AsOriginForInstance1; } pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>; @@ -95,6 +113,7 @@ frame_support::construct_runtime!( { System: frame_system::{Module, Call, Event<T>}, Authority: authority::{Module, Call, Origin<T>}, + AuthorityInstance1: authority::<Instance1>::{Module, Call, Origin<T>}, } ); diff --git a/authority/src/tests.rs b/authority/src/tests.rs index 51a9db6..8c6690d 100644 --- a/authority/src/tests.rs +++ b/authority/src/tests.rs @@ -4,55 +4,86 @@ use super::*; use frame_support::{assert_noop, assert_ok}; -use mock::{Authority, Call, ExtBuilder, Origin, Runtime, System}; -use sp_runtime::{traits::Bounded, Perbill}; +use mock::{Authority, AuthorityInstance1, Call, ExtBuilder, Origin, Runtime, System}; +use sp_runtime::{ + traits::{BadOrigin, Bounded}, + Perbill, +}; #[test] -fn dispatch_root_work() { +fn dispatch_work() { ExtBuilder::default().build().execute_with(|| { - let call = Call::System(frame_system::Call::fill_block(Perbill::one())); - assert_ok!(Authority::dispatch_root(Origin::signed(1), Box::new(call.clone()))); - assert_noop!(Authority::dispatch_root(Origin::signed(2), Box::new(call)), BadOrigin); + let ensure_root_call = Call::System(frame_system::Call::fill_block(Perbill::one())); + let ensure_signed_call = Call::System(frame_system::Call::remark(vec![])); + assert_ok!(Authority::dispatch( + Origin::signed(1), + Box::new(ensure_root_call.clone()) + )); + assert_noop!( + Authority::dispatch(Origin::signed(2), Box::new(ensure_root_call.clone())), + BadOrigin + ); + assert_noop!( + Authority::dispatch(Origin::signed(1), Box::new(ensure_signed_call.clone())), + BadOrigin + ); + assert_noop!( + AuthorityInstance1::dispatch(Origin::signed(1), Box::new(ensure_root_call)), + BadOrigin + ); + assert_ok!(AuthorityInstance1::dispatch( + Origin::signed(1), + Box::new(ensure_signed_call) + )); }); } #[test] -fn schedule_dispatch_root_work() { +fn schedule_dispatch_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(10); - let call = Call::System(frame_system::Call::fill_block(Perbill::one())); + let ensure_root_call = Call::System(frame_system::Call::fill_block(Perbill::one())); assert_noop!( - Authority::schedule_dispatch_root(Origin::signed(1), Box::new(call.clone()), DelayedDispatchTime::At(10)), - Error::<Runtime>::InvalidDelayedDispatchTime, + Authority::schedule_dispatch( + Origin::signed(1), + Box::new(ensure_root_call.clone()), + DelayedDispatchTime::At(10) + ), + Error::<Runtime, DefaultInstance>::InvalidDelayedDispatchTime, ); - assert_noop!( - Authority::schedule_dispatch_root( + Authority::schedule_dispatch( Origin::signed(1), - Box::new(call.clone()), + Box::new(ensure_root_call.clone()), DelayedDispatchTime::After(Bounded::max_value()) ), - Error::<Runtime>::BlockNumberOverflow, + Error::<Runtime, DefaultInstance>::BlockNumberOverflow, ); - assert_noop!( - Authority::schedule_dispatch_root(Origin::signed(2), Box::new(call.clone()), DelayedDispatchTime::At(20)), + Authority::schedule_dispatch( + Origin::signed(2), + Box::new(ensure_root_call.clone()), + DelayedDispatchTime::At(20) + ), BadOrigin, ); - assert_ok!(Authority::schedule_dispatch_root( + assert_ok!(Authority::schedule_dispatch( Origin::signed(1), - Box::new(call.clone()), + Box::new(ensure_root_call.clone()), DelayedDispatchTime::At(20) )); - assert_noop!( - Authority::schedule_dispatch_root(Origin::signed(1), Box::new(call.clone()), DelayedDispatchTime::At(19)), + Authority::schedule_dispatch( + Origin::signed(1), + Box::new(ensure_root_call.clone()), + DelayedDispatchTime::At(19) + ), BadOrigin, ); - assert_ok!(Authority::schedule_dispatch_root( + assert_ok!(Authority::schedule_dispatch( Origin::signed(2), - Box::new(call.clone()), + Box::new(ensure_root_call), DelayedDispatchTime::At(19) )); }); @@ -75,7 +106,7 @@ fn schedule_dispatch_delayed_work() { assert_noop!( Authority::schedule_dispatch_delayed(Origin::signed(1), Box::new(call.clone()), DelayedDispatchTime::At(9)), - Error::<Runtime>::InvalidDelayedDispatchTime, + Error::<Runtime, DefaultInstance>::InvalidDelayedDispatchTime, ); assert_noop!( @@ -84,7 +115,7 @@ fn schedule_dispatch_delayed_work() { Box::new(call.clone()), DelayedDispatchTime::After(Bounded::max_value()) ), - Error::<Runtime>::BlockNumberOverflow, + Error::<Runtime, DefaultInstance>::BlockNumberOverflow, ); assert_ok!(Authority::schedule_dispatch_delayed( -- GitLab