From 11d4785bdd35836ffb7479a1cfaa259f2eb0e053 Mon Sep 17 00:00:00 2001 From: Zeke Mostov <32168567+emostov@users.noreply.github.com> Date: Mon, 15 Mar 2021 19:54:35 -0700 Subject: [PATCH 1/6] Migrate pallet-proxy to pallet attribute macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #7882. Converts the `Proxy` pallet to the new pallet attribute macro introduced in #6877. [Upgrade guidelines used](https://substrate.dev/rustdocs/v3.0.0/frame_support/attr.pallet.html#upgrade-guidelines). ## ⚠️ Breaking Change ⚠️ From [checking upgrade guidelines](https://crates.parity.io/frame_support/attr.pallet.html#checking-upgrade-guidelines) > storages now use PalletInfo for module_prefix instead of the one given to `decl_storage`: use of this pallet in `construct_runtime!` needs careful updating of the name in order to not break storage or to upgrade storage (moreover for instantiable pallet). If pallet is published, make sure to warn about this breaking change. So users of the `Assets` pallet must be careful about the name they used in `construct_runtime!`. Hence the `runtime-migration` label, which might not be needed depending on the configuration of the `Assets` pallet. ### Notes There are some changes to the docs in metadata for the constants. The docs in the metadata for constants are now more complete. --- frame/proxy/src/benchmarking.rs | 8 +- frame/proxy/src/lib.rs | 368 ++++++++++++++++++-------------- frame/proxy/src/tests.rs | 45 ++-- 3 files changed, 232 insertions(+), 189 deletions(-) diff --git a/frame/proxy/src/benchmarking.rs b/frame/proxy/src/benchmarking.rs index 130c980011871..2fb99c57c1155 100644 --- a/frame/proxy/src/benchmarking.rs +++ b/frame/proxy/src/benchmarking.rs @@ -90,7 +90,7 @@ benchmarks! { let call: ::Call = frame_system::Call::::remark(vec![]).into(); }: _(RawOrigin::Signed(caller), real, Some(T::ProxyType::default()), Box::new(call)) verify { - assert_last_event::(RawEvent::ProxyExecuted(Ok(())).into()) + assert_last_event::(Event::ProxyExecuted(Ok(())).into()) } proxy_announced { @@ -111,7 +111,7 @@ benchmarks! { add_announcements::(a, Some(delegate.clone()), None)?; }: _(RawOrigin::Signed(caller), delegate, real, Some(T::ProxyType::default()), Box::new(call)) verify { - assert_last_event::(RawEvent::ProxyExecuted(Ok(())).into()) + assert_last_event::(Event::ProxyExecuted(Ok(())).into()) } remove_announcement { @@ -169,7 +169,7 @@ benchmarks! { let call_hash = T::CallHasher::hash_of(&call); }: _(RawOrigin::Signed(caller.clone()), real.clone(), call_hash) verify { - assert_last_event::(RawEvent::Announced(real, caller, call_hash).into()); + assert_last_event::(Event::Announced(real, caller, call_hash).into()); } add_proxy { @@ -220,7 +220,7 @@ benchmarks! { ) verify { let anon_account = Module::::anonymous_account(&caller, &T::ProxyType::default(), 0, None); - assert_last_event::(RawEvent::AnonymousCreated( + assert_last_event::(Event::AnonymousCreated( anon_account, caller, T::ProxyType::default(), diff --git a/frame/proxy/src/lib.rs b/frame/proxy/src/lib.rs index 1e5aaadcc62d3..408f9ef76dfbf 100644 --- a/frame/proxy/src/lib.rs +++ b/frame/proxy/src/lib.rs @@ -15,11 +15,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! # Proxy Module -//! A module allowing accounts to give permission to other accounts to dispatch types of calls from +//! # Proxy Pallet +//! A pallet allowing accounts to give permission to other accounts to dispatch types of calls from //! their signed origin. //! -//! The accounts to which permission is delegated may be requied to announce the action that they +//! The accounts to which permission is delegated may be required to announce the action that they //! wish to execute some duration prior to execution happens. In this case, the target account may //! reject the announcement and in doing so, veto the execution. //! @@ -45,73 +45,23 @@ pub mod weights; use sp_std::prelude::*; use codec::{Encode, Decode}; use sp_io::hashing::blake2_256; -use sp_runtime::{DispatchResult, traits::{Dispatchable, Zero, Hash, Member, Saturating}}; +use sp_runtime::{ + DispatchResult, + traits::{Dispatchable, Zero, Hash, Saturating} +}; use frame_support::{ - decl_module, decl_event, decl_error, decl_storage, Parameter, ensure, RuntimeDebug, traits::{ - Get, ReservableCurrency, Currency, InstanceFilter, OriginTrait, IsType, IsSubType, - }, weights::{Weight, GetDispatchInfo}, dispatch::PostDispatchInfo, storage::IterableStorageMap, + RuntimeDebug, ensure, + dispatch::{DispatchResultWithPostInfo, PostDispatchInfo}, + traits::{Get, ReservableCurrency, Currency, InstanceFilter, OriginTrait, IsType, IsSubType}, + weights::{Weight, GetDispatchInfo} }; -use frame_system::{self as system, ensure_signed}; +use frame_system::{self as system}; use frame_support::dispatch::DispatchError; pub use weights::WeightInfo; -type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; - -/// Configuration trait. -pub trait Config: frame_system::Config { - /// The overarching event type. - type Event: From> + Into<::Event>; - - /// The overarching call type. - type Call: Parameter + Dispatchable - + GetDispatchInfo + From> + IsSubType> - + IsType<::Call>; - - /// The currency mechanism. - type Currency: ReservableCurrency; - - /// A kind of proxy; specified with the proxy and passed in to the `IsProxyable` fitler. - /// The instance filter determines whether a given call may be proxied under this type. - /// - /// IMPORTANT: `Default` must be provided and MUST BE the the *most permissive* value. - type ProxyType: Parameter + Member + Ord + PartialOrd + InstanceFilter<::Call> - + Default; - - /// The base amount of currency needed to reserve for creating a proxy. - /// - /// This is held for an additional storage item whose value size is - /// `sizeof(Balance)` bytes and whose key size is `sizeof(AccountId)` bytes. - type ProxyDepositBase: Get>; - - /// The amount of currency needed per proxy added. - /// - /// This is held for adding 32 bytes plus an instance of `ProxyType` more into a pre-existing - /// storage value. - type ProxyDepositFactor: Get>; - - /// The maximum amount of proxies allowed for a single account. - type MaxProxies: Get; - - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; - - /// The maximum amount of time-delayed announcements that are allowed to be pending. - type MaxPending: Get; - - /// The type of hash used for hashing the call. - type CallHasher: Hash; - - /// The base amount of currency needed to reserve for creating an announcement. - /// - /// This is held when a new storage item holding a `Balance` is created (typically 16 bytes). - type AnnouncementDepositBase: Get>; +type CallHashOf = <::CallHasher as Hash>::Output; - /// The amount of currency needed per announcement made. - /// - /// This is held for adding an `AccountId`, `Hash` and `BlockNumber` (typically 68 bytes) - /// into a pre-existing storage value. - type AnnouncementDepositFactor: Get>; -} +type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; /// The parameters under which a particular account has a proxy relationship with some other /// account. @@ -137,84 +87,86 @@ pub struct Announcement { height: BlockNumber, } -type CallHashOf = <::CallHasher as Hash>::Output; +pub use pallet::*; -decl_storage! { - trait Store for Module as Proxy { - /// The set of account proxies. Maps the account which has delegated to the accounts - /// which are being delegated to, together with the amount held on deposit. - pub Proxies get(fn proxies): map hasher(twox_64_concat) T::AccountId - => (Vec>, BalanceOf); +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use super::{ *, DispatchResult }; - /// The announcements made by the proxy (key). - pub Announcements get(fn announcements): map hasher(twox_64_concat) T::AccountId - => (Vec, T::BlockNumber>>, BalanceOf); - } -} + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); -decl_error! { - pub enum Error for Module { - /// There are too many proxies registered or too many announcements pending. - TooMany, - /// Proxy registration not found. - NotFound, - /// Sender is not a proxy of the account to be proxied. - NotProxy, - /// A call which is incompatible with the proxy type's filter was attempted. - Unproxyable, - /// Account is already a proxy. - Duplicate, - /// Call may not be made by proxy because it may escalate its privileges. - NoPermission, - /// Announcement, if made at all, was made too recently. - Unannounced, - /// Cannot add self as proxy. - NoSelfProxy, - } -} + /// Configuration trait. + #[pallet::config] + pub trait Config: frame_system::Config { + /// The overarching event type. + type Event: From> + IsType<::Event>; -decl_event! { - /// Events type. - pub enum Event where - AccountId = ::AccountId, - ProxyType = ::ProxyType, - Hash = CallHashOf, - { - /// A proxy was executed correctly, with the given \[result\]. - ProxyExecuted(DispatchResult), - /// Anonymous account has been created by new proxy with given - /// disambiguation index and proxy type. \[anonymous, who, proxy_type, disambiguation_index\] - AnonymousCreated(AccountId, AccountId, ProxyType, u16), - /// An announcement was placed to make a call in the future. \[real, proxy, call_hash\] - Announced(AccountId, AccountId, Hash), - } -} + /// The overarching call type. + type Call: Parameter + Dispatchable + + GetDispatchInfo + From> + IsSubType> + + IsType<::Call>; -decl_module! { - pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + /// The currency mechanism. + type Currency: ReservableCurrency; - /// Deposit one of this module's events by using the default implementation. - fn deposit_event() = default; + /// A kind of proxy; specified with the proxy and passed in to the `IsProxyable` fitler. + /// The instance filter determines whether a given call may be proxied under this type. + /// + /// IMPORTANT: `Default` must be provided and MUST BE the the *most permissive* value. + type ProxyType: Parameter + Member + Ord + PartialOrd + InstanceFilter<::Call> + + Default; /// The base amount of currency needed to reserve for creating a proxy. - const ProxyDepositBase: BalanceOf = T::ProxyDepositBase::get(); + /// + /// This is held for an additional storage item whose value size is + /// `sizeof(Balance)` bytes and whose key size is `sizeof(AccountId)` bytes. + #[pallet::constant] + type ProxyDepositBase: Get>; /// The amount of currency needed per proxy added. - const ProxyDepositFactor: BalanceOf = T::ProxyDepositFactor::get(); + /// + /// This is held for adding 32 bytes plus an instance of `ProxyType` more into a pre-existing + /// storage value. + #[pallet::constant] + type ProxyDepositFactor: Get>; /// The maximum amount of proxies allowed for a single account. - const MaxProxies: u16 = T::MaxProxies::get(); + #[pallet::constant] + type MaxProxies: Get; - /// `MaxPending` metadata shadow. - const MaxPending: u32 = T::MaxPending::get(); + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; - /// `AnnouncementDepositBase` metadata shadow. - const AnnouncementDepositBase: BalanceOf = T::AnnouncementDepositBase::get(); + /// The maximum amount of time-delayed announcements that are allowed to be pending. + #[pallet::constant] + type MaxPending: Get; - /// `AnnouncementDepositFactor` metadata shadow. - const AnnouncementDepositFactor: BalanceOf = T::AnnouncementDepositFactor::get(); + /// The type of hash used for hashing the call. + type CallHasher: Hash; + + /// The base amount of currency needed to reserve for creating an announcement. + /// + /// This is held when a new storage item holding a `Balance` is created (typically 16 bytes). + #[pallet::constant] + type AnnouncementDepositBase: Get>; + + /// The amount of currency needed per announcement made. + /// + /// This is held for adding an `AccountId`, `Hash` and `BlockNumber` (typically 68 bytes) + /// into a pre-existing storage value. + #[pallet::constant] + type AnnouncementDepositFactor: Get>; + } + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::call] + impl Pallet { /// Dispatch the given `call` from an account that the sender is authorised for through /// `add_proxy`. /// @@ -230,24 +182,27 @@ decl_module! { /// # /// Weight is a function of the number of proxies the user has (P). /// # - #[weight = { + #[pallet::weight({ let di = call.get_dispatch_info(); (T::WeightInfo::proxy(T::MaxProxies::get().into()) .saturating_add(di.weight) // AccountData for inner call origin accountdata. .saturating_add(T::DbWeight::get().reads_writes(1, 1)), di.class) - }] - fn proxy(origin, + })] + pub(super) fn proxy( + origin: OriginFor, real: T::AccountId, force_proxy_type: Option, call: Box<::Call>, - ) { + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let def = Self::find_proxy(&real, &who, force_proxy_type)?; ensure!(def.delay.is_zero(), Error::::Unannounced); Self::do_proxy(def, real, *call); + + Ok(().into()) } /// Register a proxy account for the sender that is able to make calls on its behalf. @@ -263,12 +218,13 @@ decl_module! { /// # /// Weight is a function of the number of proxies the user has (P). /// # - #[weight = T::WeightInfo::add_proxy(T::MaxProxies::get().into())] - fn add_proxy(origin, + #[pallet::weight(T::WeightInfo::add_proxy(T::MaxProxies::get().into()))] + pub(super) fn add_proxy( + origin: OriginFor, delegate: T::AccountId, proxy_type: T::ProxyType, delay: T::BlockNumber, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; Self::add_proxy_delegate(&who, delegate, proxy_type, delay) } @@ -284,12 +240,13 @@ decl_module! { /// # /// Weight is a function of the number of proxies the user has (P). /// # - #[weight = T::WeightInfo::remove_proxy(T::MaxProxies::get().into())] - fn remove_proxy(origin, + #[pallet::weight(T::WeightInfo::remove_proxy(T::MaxProxies::get().into()))] + pub(super) fn remove_proxy( + origin: OriginFor, delegate: T::AccountId, proxy_type: T::ProxyType, delay: T::BlockNumber, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; Self::remove_proxy_delegate(&who, delegate, proxy_type, delay) } @@ -304,11 +261,13 @@ decl_module! { /// # /// Weight is a function of the number of proxies the user has (P). /// # - #[weight = T::WeightInfo::remove_proxies(T::MaxProxies::get().into())] - fn remove_proxies(origin) { + #[pallet::weight(T::WeightInfo::remove_proxies(T::MaxProxies::get().into()))] + pub(super) fn remove_proxies(origin: OriginFor) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let (_, old_deposit) = Proxies::::take(&who); T::Currency::unreserve(&who, old_deposit); + + Ok(().into()) } /// Spawn a fresh new account that is guaranteed to be otherwise inaccessible, and @@ -334,8 +293,13 @@ decl_module! { /// Weight is a function of the number of proxies the user has (P). /// # /// TODO: Might be over counting 1 read - #[weight = T::WeightInfo::anonymous(T::MaxProxies::get().into())] - fn anonymous(origin, proxy_type: T::ProxyType, delay: T::BlockNumber, index: u16) { + #[pallet::weight(T::WeightInfo::anonymous(T::MaxProxies::get().into()))] + pub(super) fn anonymous( + origin: OriginFor, + proxy_type: T::ProxyType, + delay: T::BlockNumber, + index: u16 + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let anonymous = Self::anonymous_account(&who, &proxy_type, index, None); @@ -348,7 +312,9 @@ decl_module! { delay, }; Proxies::::insert(&anonymous, (vec![proxy_def], deposit)); - Self::deposit_event(RawEvent::AnonymousCreated(anonymous, who, proxy_type, index)); + Self::deposit_event(Event::AnonymousCreated(anonymous, who, proxy_type, index)); + + Ok(().into()) } /// Removes a previously spawned anonymous proxy. @@ -371,14 +337,15 @@ decl_module! { /// # /// Weight is a function of the number of proxies the user has (P). /// # - #[weight = T::WeightInfo::kill_anonymous(T::MaxProxies::get().into())] - fn kill_anonymous(origin, + #[pallet::weight(T::WeightInfo::kill_anonymous(T::MaxProxies::get().into()))] + pub(super) fn kill_anonymous( + origin: OriginFor, spawner: T::AccountId, proxy_type: T::ProxyType, index: u16, - #[compact] height: T::BlockNumber, - #[compact] ext_index: u32, - ) { + #[pallet::compact] height: T::BlockNumber, + #[pallet::compact] ext_index: u32, + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let when = (height, ext_index); @@ -387,6 +354,8 @@ decl_module! { let (_, deposit) = Proxies::::take(&who); T::Currency::unreserve(&spawner, deposit); + + Ok(().into()) } /// Publish the hash of a proxy-call that will be made in the future. @@ -410,8 +379,8 @@ decl_module! { /// - A: the number of announcements made. /// - P: the number of proxies the user has. /// # - #[weight = T::WeightInfo::announce(T::MaxPending::get(), T::MaxProxies::get().into())] - fn announce(origin, real: T::AccountId, call_hash: CallHashOf) { + #[pallet::weight(T::WeightInfo::announce(T::MaxPending::get(), T::MaxProxies::get().into()))] + pub(super) fn announce(origin: OriginFor, real: T::AccountId, call_hash: CallHashOf) -> DispatchResultWithPostInfo{ let who = ensure_signed(origin)?; Proxies::::get(&real).0.into_iter() .find(|x| &x.delegate == &who) @@ -435,7 +404,9 @@ decl_module! { ).map(|d| d.expect("Just pushed; pending.len() > 0; rejig_deposit returns Some; qed")) .map(|d| *deposit = d) })?; - Self::deposit_event(RawEvent::Announced(real, who, call_hash)); + Self::deposit_event(Event::Announced(real, who, call_hash)); + + Ok(().into()) } /// Remove a given announcement. @@ -454,10 +425,13 @@ decl_module! { /// - A: the number of announcements made. /// - P: the number of proxies the user has. /// # - #[weight = T::WeightInfo::remove_announcement(T::MaxPending::get(), T::MaxProxies::get().into())] - fn remove_announcement(origin, real: T::AccountId, call_hash: CallHashOf) { + #[pallet::weight(T::WeightInfo::remove_announcement(T::MaxPending::get(), T::MaxProxies::get().into()))] + pub(super) fn remove_announcement(origin: OriginFor, real: T::AccountId, call_hash: CallHashOf) + -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; Self::edit_announcements(&who, |ann| ann.real != real || ann.call_hash != call_hash)?; + + Ok(().into()) } /// Remove the given announcement of a delegate. @@ -476,13 +450,16 @@ decl_module! { /// - A: the number of announcements made. /// - P: the number of proxies the user has. /// # - #[weight = T::WeightInfo::reject_announcement(T::MaxPending::get(), T::MaxProxies::get().into())] - fn reject_announcement(origin, delegate: T::AccountId, call_hash: CallHashOf) { + #[pallet::weight(T::WeightInfo::reject_announcement(T::MaxPending::get(), T::MaxProxies::get().into()))] + pub(super) fn reject_announcement(origin: OriginFor, delegate: T::AccountId, call_hash: CallHashOf) + -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; Self::edit_announcements(&delegate, |ann| ann.real != who || ann.call_hash != call_hash)?; + + Ok(().into()) } - /// Dispatch the given `call` from an account that the sender is authorised for through + /// Dispatch the given `call` from an account that the sender is authorized for through /// `add_proxy`. /// /// Removes any corresponding announcement(s). @@ -499,20 +476,21 @@ decl_module! { /// - A: the number of announcements made. /// - P: the number of proxies the user has. /// # - #[weight = { + #[pallet::weight({ let di = call.get_dispatch_info(); (T::WeightInfo::proxy_announced(T::MaxPending::get(), T::MaxProxies::get().into()) .saturating_add(di.weight) // AccountData for inner call origin accountdata. .saturating_add(T::DbWeight::get().reads_writes(1, 1)), di.class) - }] - fn proxy_announced(origin, + })] + pub(super) fn proxy_announced( + origin: OriginFor, delegate: T::AccountId, real: T::AccountId, force_proxy_type: Option, call: Box<::Call>, - ) { + ) -> DispatchResultWithPostInfo { ensure_signed(origin)?; let def = Self::find_proxy(&real, &delegate, force_proxy_type)?; @@ -523,8 +501,72 @@ decl_module! { ).map_err(|_| Error::::Unannounced)?; Self::do_proxy(def, real, *call); + + Ok(().into()) } } + + #[pallet::event] + #[pallet::metadata(T::AccountId = "AccountId", T::ProxyType = "ProxyType", CallHashOf = "Hash")] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event + { + /// A proxy was executed correctly, with the given \[result\]. + ProxyExecuted(DispatchResult), + /// Anonymous account has been created by new proxy with given + /// disambiguation index and proxy type. \[anonymous, who, proxy_type, disambiguation_index\] + AnonymousCreated(T::AccountId, T::AccountId, T::ProxyType, u16), + /// An announcement was placed to make a call in the future. \[real, proxy, call_hash\] + Announced(T::AccountId, T::AccountId, CallHashOf), + } + + /// Old name generated by `decl_event`. + #[deprecated(note="use `Event` instead")] + pub type RawEvent = Event; + + #[pallet::error] + pub enum Error { + /// There are too many proxies registered or too many announcements pending. + TooMany, + /// Proxy registration not found. + NotFound, + /// Sender is not a proxy of the account to be proxied. + NotProxy, + /// A call which is incompatible with the proxy type's filter was attempted. + Unproxyable, + /// Account is already a proxy. + Duplicate, + /// Call may not be made by proxy because it may escalate its privileges. + NoPermission, + /// Announcement, if made at all, was made too recently. + Unannounced, + /// Cannot add self as proxy. + NoSelfProxy, + } + + /// The set of account proxies. Maps the account which has delegated to the accounts + /// which are being delegated to, together with the amount held on deposit. + #[pallet::storage] + #[pallet::getter(fn proxies)] + pub type Proxies = StorageMap< + _, + Twox64Concat, + T::AccountId, + (Vec>, BalanceOf), + ValueQuery + >; + + /// The announcements made by the proxy (key). + #[pallet::storage] + #[pallet::getter(fn announcements)] + pub type Announcements = StorageMap< + _, + Twox64Concat, + T::AccountId, + (Vec, T::BlockNumber>>, BalanceOf), + ValueQuery + >; + } impl Module { @@ -568,7 +610,7 @@ impl Module { delegatee: T::AccountId, proxy_type: T::ProxyType, delay: T::BlockNumber, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { ensure!(delegator != &delegatee, Error::::NoSelfProxy); Proxies::::try_mutate(delegator, |(ref mut proxies, ref mut deposit)| { ensure!(proxies.len() < T::MaxProxies::get() as usize, Error::::TooMany); @@ -582,7 +624,7 @@ impl Module { T::Currency::unreserve(delegator, *deposit - new_deposit); } *deposit = new_deposit; - Ok(()) + Ok(().into()) }) } @@ -599,7 +641,7 @@ impl Module { delegatee: T::AccountId, proxy_type: T::ProxyType, delay: T::BlockNumber, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { Proxies::::try_mutate_exists(delegator, |x| { let (mut proxies, old_deposit) = x.take().ok_or(Error::::NotFound)?; let proxy_def = ProxyDefinition { delegate: delegatee, proxy_type, delay }; @@ -614,7 +656,7 @@ impl Module { if !proxies.is_empty() { *x = Some((proxies, new_deposit)) } - Ok(()) + Ok(().into()) }) } @@ -701,7 +743,7 @@ impl Module { } }); let e = call.dispatch(origin); - Self::deposit_event(RawEvent::ProxyExecuted(e.map(|_| ()).map_err(|e| e.error))); + Self::deposit_event(Event::ProxyExecuted(e.map(|_| ()).map_err(|e| e.error))); } } diff --git a/frame/proxy/src/tests.rs b/frame/proxy/src/tests.rs index b31ef1dfdb2fe..4d179968dd715 100644 --- a/frame/proxy/src/tests.rs +++ b/frame/proxy/src/tests.rs @@ -150,6 +150,7 @@ use pallet_balances::Error as BalancesError; use pallet_balances::Event as BalancesEvent; use pallet_utility::Call as UtilityCall; use pallet_utility::Event as UtilityEvent; +use super::Event as ProxyEvent; use super::Call as ProxyCall; pub fn new_test_ext() -> sp_io::TestExternalities { @@ -309,11 +310,11 @@ fn filtering_works() { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 1))); assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Ok(()))); + expect_event(ProxyEvent::ProxyExecuted(Ok(()))); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Ok(()))); + expect_event(ProxyEvent::ProxyExecuted(Ok(()))); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); let derivative_id = Utility::derivative_account_id(1, 0); assert!(Balances::mutate_account(&derivative_id, |a| a.free = 1000).is_ok()); @@ -321,42 +322,42 @@ fn filtering_works() { let call = Box::new(Call::Utility(UtilityCall::as_derivative(0, inner.clone()))); assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Ok(()))); + expect_event(ProxyEvent::ProxyExecuted(Ok(()))); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); let call = Box::new(Call::Utility(UtilityCall::batch(vec![*inner]))); assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); - expect_events(vec![UtilityEvent::BatchCompleted.into(), RawEvent::ProxyExecuted(Ok(())).into()]); + expect_events(vec![UtilityEvent::BatchCompleted.into(), ProxyEvent::ProxyExecuted(Ok(())).into()]); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); expect_events(vec![ UtilityEvent::BatchInterrupted(0, DispatchError::BadOrigin).into(), - RawEvent::ProxyExecuted(Ok(())).into(), + ProxyEvent::ProxyExecuted(Ok(())).into(), ]); let inner = Box::new(Call::Proxy(ProxyCall::add_proxy(5, ProxyType::Any, 0))); let call = Box::new(Call::Utility(UtilityCall::batch(vec![*inner]))); assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); - expect_events(vec![UtilityEvent::BatchCompleted.into(), RawEvent::ProxyExecuted(Ok(())).into()]); + expect_events(vec![UtilityEvent::BatchCompleted.into(), ProxyEvent::ProxyExecuted(Ok(())).into()]); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); expect_events(vec![ UtilityEvent::BatchInterrupted(0, DispatchError::BadOrigin).into(), - RawEvent::ProxyExecuted(Ok(())).into(), + ProxyEvent::ProxyExecuted(Ok(())).into(), ]); let call = Box::new(Call::Proxy(ProxyCall::remove_proxies())); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); - expect_events(vec![BalancesEvent::::Unreserved(1, 5).into(), RawEvent::ProxyExecuted(Ok(())).into()]); + expect_events(vec![BalancesEvent::::Unreserved(1, 5).into(), ProxyEvent::ProxyExecuted(Ok(())).into()]); }); } @@ -411,18 +412,18 @@ fn proxying_works() { Error::::NotProxy ); assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Ok(()))); + expect_event(ProxyEvent::ProxyExecuted(Ok(()))); assert_eq!(Balances::free_balance(6), 1); let call = Box::new(Call::System(SystemCall::set_code(vec![]))); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); let call = Box::new(Call::Balances(BalancesCall::transfer_keep_alive(6, 1))); assert_ok!(Call::Proxy(super::Call::proxy(1, None, call.clone())).dispatch(Origin::signed(2))); - expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); + expect_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin))); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - expect_event(RawEvent::ProxyExecuted(Ok(()))); + expect_event(ProxyEvent::ProxyExecuted(Ok(()))); assert_eq!(Balances::free_balance(6), 2); }); } @@ -432,7 +433,7 @@ fn anonymous_works() { new_test_ext().execute_with(|| { assert_ok!(Proxy::anonymous(Origin::signed(1), ProxyType::Any, 0, 0)); let anon = Proxy::anonymous_account(&1, &ProxyType::Any, 0, None); - expect_event(RawEvent::AnonymousCreated(anon.clone(), 1, ProxyType::Any, 0)); + expect_event(ProxyEvent::AnonymousCreated(anon.clone(), 1, ProxyType::Any, 0)); // other calls to anonymous allowed as long as they're not exactly the same. assert_ok!(Proxy::anonymous(Origin::signed(1), ProxyType::JustTransfer, 0, 0)); @@ -449,13 +450,13 @@ fn anonymous_works() { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 1))); assert_ok!(Balances::transfer(Origin::signed(3), anon, 5)); assert_ok!(Proxy::proxy(Origin::signed(1), anon, None, call)); - expect_event(RawEvent::ProxyExecuted(Ok(()))); + expect_event(ProxyEvent::ProxyExecuted(Ok(()))); assert_eq!(Balances::free_balance(6), 1); let call = Box::new(Call::Proxy(ProxyCall::kill_anonymous(1, ProxyType::Any, 0, 1, 0))); assert_ok!(Proxy::proxy(Origin::signed(2), anon2, None, call.clone())); let de = DispatchError::from(Error::::NoPermission).stripped(); - expect_event(RawEvent::ProxyExecuted(Err(de))); + expect_event(ProxyEvent::ProxyExecuted(Err(de))); assert_noop!( Proxy::kill_anonymous(Origin::signed(1), 1, ProxyType::Any, 0, 1, 0), Error::::NoPermission From d2acec52181adbbe11ad041720e68b7cec395774 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Mon, 15 Mar 2021 20:15:38 -0700 Subject: [PATCH 2/6] Line widths --- frame/proxy/src/lib.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/frame/proxy/src/lib.rs b/frame/proxy/src/lib.rs index 408f9ef76dfbf..d888092356b64 100644 --- a/frame/proxy/src/lib.rs +++ b/frame/proxy/src/lib.rs @@ -380,7 +380,11 @@ pub mod pallet { /// - P: the number of proxies the user has. /// # #[pallet::weight(T::WeightInfo::announce(T::MaxPending::get(), T::MaxProxies::get().into()))] - pub(super) fn announce(origin: OriginFor, real: T::AccountId, call_hash: CallHashOf) -> DispatchResultWithPostInfo{ + pub(super) fn announce( + origin: OriginFor, + real: T::AccountId, + call_hash: CallHashOf + ) -> DispatchResultWithPostInfo{ let who = ensure_signed(origin)?; Proxies::::get(&real).0.into_iter() .find(|x| &x.delegate == &who) @@ -425,9 +429,14 @@ pub mod pallet { /// - A: the number of announcements made. /// - P: the number of proxies the user has. /// # - #[pallet::weight(T::WeightInfo::remove_announcement(T::MaxPending::get(), T::MaxProxies::get().into()))] - pub(super) fn remove_announcement(origin: OriginFor, real: T::AccountId, call_hash: CallHashOf) - -> DispatchResultWithPostInfo { + #[pallet::weight( + T::WeightInfo::remove_announcement(T::MaxPending::get(), T::MaxProxies::get().into()) + )] + pub(super) fn remove_announcement( + origin: OriginFor, + real: T::AccountId, + call_hash: CallHashOf + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; Self::edit_announcements(&who, |ann| ann.real != real || ann.call_hash != call_hash)?; @@ -450,9 +459,14 @@ pub mod pallet { /// - A: the number of announcements made. /// - P: the number of proxies the user has. /// # - #[pallet::weight(T::WeightInfo::reject_announcement(T::MaxPending::get(), T::MaxProxies::get().into()))] - pub(super) fn reject_announcement(origin: OriginFor, delegate: T::AccountId, call_hash: CallHashOf) - -> DispatchResultWithPostInfo { + #[pallet::weight( + T::WeightInfo::reject_announcement(T::MaxPending::get(), T::MaxProxies::get().into()) + )] + pub(super) fn reject_announcement( + origin: OriginFor, + delegate: T::AccountId, + call_hash: CallHashOf + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; Self::edit_announcements(&delegate, |ann| ann.real != who || ann.call_hash != call_hash)?; From 3a3403508073ff5157ae6eaa46a518890789fb2b Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Mon, 15 Mar 2021 20:20:25 -0700 Subject: [PATCH 3/6] Move use pallet::* to top --- frame/proxy/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/proxy/src/lib.rs b/frame/proxy/src/lib.rs index d888092356b64..b2be8f79fc830 100644 --- a/frame/proxy/src/lib.rs +++ b/frame/proxy/src/lib.rs @@ -59,6 +59,8 @@ use frame_system::{self as system}; use frame_support::dispatch::DispatchError; pub use weights::WeightInfo; +pub use pallet::*; + type CallHashOf = <::CallHasher as Hash>::Output; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; @@ -87,8 +89,6 @@ pub struct Announcement { height: BlockNumber, } -pub use pallet::*; - #[frame_support::pallet] pub mod pallet { use frame_support::pallet_prelude::*; From aefea832dcf6e80bb0f1bf04ef022ba4259b5e7e Mon Sep 17 00:00:00 2001 From: Zeke Mostov <32168567+emostov@users.noreply.github.com> Date: Tue, 16 Mar 2021 10:48:55 -0700 Subject: [PATCH 4/6] Update frame/proxy/src/lib.rs Co-authored-by: Guillaume Thiolliere --- frame/proxy/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/proxy/src/lib.rs b/frame/proxy/src/lib.rs index b2be8f79fc830..934a5d4f94f2a 100644 --- a/frame/proxy/src/lib.rs +++ b/frame/proxy/src/lib.rs @@ -93,7 +93,7 @@ pub struct Announcement { pub mod pallet { use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; - use super::{ *, DispatchResult }; + use super::{*, DispatchResult}; #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] From 0c9b4bb2743132ea141fcb8a43dae3fe0de897de Mon Sep 17 00:00:00 2001 From: Zeke Mostov <32168567+emostov@users.noreply.github.com> Date: Tue, 16 Mar 2021 12:16:24 -0700 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Guillaume Thiolliere --- frame/proxy/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/proxy/src/lib.rs b/frame/proxy/src/lib.rs index 934a5d4f94f2a..8c634f206638b 100644 --- a/frame/proxy/src/lib.rs +++ b/frame/proxy/src/lib.rs @@ -570,7 +570,7 @@ pub mod pallet { ValueQuery >; - /// The announcements made by the proxy (key). + /// The announcements made by the proxy (key). #[pallet::storage] #[pallet::getter(fn announcements)] pub type Announcements = StorageMap< From c78205e285656834203fdd86089eacd605effa44 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Tue, 16 Mar 2021 20:04:51 -0700 Subject: [PATCH 6/6] Append ProxyDepositFactor doc comment --- frame/proxy/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/proxy/src/lib.rs b/frame/proxy/src/lib.rs index 8c634f206638b..c3ff658daf33e 100644 --- a/frame/proxy/src/lib.rs +++ b/frame/proxy/src/lib.rs @@ -130,7 +130,8 @@ pub mod pallet { /// The amount of currency needed per proxy added. /// /// This is held for adding 32 bytes plus an instance of `ProxyType` more into a pre-existing - /// storage value. + /// storage value. Thus, when configuring `ProxyDepositFactor` one should take into account + /// `32 + proxy_type.encode().len()` bytes of data. #[pallet::constant] type ProxyDepositFactor: Get>;