diff --git a/substrate/srml/balances/src/lib.rs b/substrate/srml/balances/src/lib.rs index a547252d11..40d4552f90 100644 --- a/substrate/srml/balances/src/lib.rs +++ b/substrate/srml/balances/src/lib.rs @@ -163,7 +163,7 @@ use primitives::traits::{ Zero, SimpleArithmetic, StaticLookup, Member, CheckedAdd, CheckedSub, MaybeSerializeDebug, Saturating, Bounded }; -use system::{IsDeadAccount, OnNewAccount, ensure_signed}; +use system::{IsDeadAccount, OnNewAccount, ensure_signed, ensure_root}; mod mock; mod tests; @@ -436,10 +436,12 @@ decl_module! { /// - Contains a limited number of reads and writes. /// # fn set_balance( + origin, who: ::Source, #[compact] new_free: T::Balance, #[compact] new_reserved: T::Balance ) { + ensure_root(origin)?; let who = T::Lookup::lookup(who)?; let current_free = >::get(&who); diff --git a/substrate/srml/contracts/src/lib.rs b/substrate/srml/contracts/src/lib.rs index 202ed37276..3362dfdbd7 100644 --- a/substrate/srml/contracts/src/lib.rs +++ b/substrate/srml/contracts/src/lib.rs @@ -106,7 +106,7 @@ use srml_support::{ Parameter, StorageMap, StorageValue, decl_module, decl_event, decl_storage, storage::child }; use srml_support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency, Get}; -use system::{ensure_signed, RawOrigin}; +use system::{ensure_signed, RawOrigin, ensure_root}; use substrate_primitives::storage::well_known_keys::CHILD_STORAGE_KEY_PREFIX; use timestamp; @@ -485,7 +485,8 @@ decl_module! { /// Updates the schedule for metering contracts. /// /// The schedule must have a greater version than the stored schedule. - pub fn update_schedule(schedule: Schedule) -> Result { + pub fn update_schedule(origin, schedule: Schedule) -> Result { + ensure_root(origin)?; if >::current_schedule().version >= schedule.version { return Err("new schedule must have a greater version than current"); } diff --git a/substrate/srml/council/src/seats.rs b/substrate/srml/council/src/seats.rs index dc84ba7fa7..892fae4627 100644 --- a/substrate/srml/council/src/seats.rs +++ b/substrate/srml/council/src/seats.rs @@ -29,7 +29,7 @@ use srml_support::{ }; use democracy; use parity_codec::{Encode, Decode}; -use system::{self, ensure_signed}; +use system::{self, ensure_signed, ensure_root}; use super::OnMembersChanged; // no polynomial attacks: @@ -487,7 +487,8 @@ decl_module! { /// Set the desired member count; if lower than the current count, then seats will not be up /// election when they expire. If more, then a new vote will be started if one is not /// already in progress. - fn set_desired_seats(#[compact] count: u32) { + fn set_desired_seats(origin, #[compact] count: u32) { + ensure_root(origin)?; DesiredSeats::put(count); } @@ -495,7 +496,8 @@ decl_module! { /// /// Note: A tally should happen instantly (if not already in a presentation /// period) to fill the seat if removal means that the desired members are not met. - fn remove_member(who: ::Source) { + fn remove_member(origin, who: ::Source) { + ensure_root(origin)?; let who = T::Lookup::lookup(who)?; let new_council: Vec<(T::AccountId, T::BlockNumber)> = Self::active_council() .into_iter() @@ -507,13 +509,15 @@ decl_module! { /// Set the presentation duration. If there is currently a vote being presented for, will /// invoke `finalize_vote`. - fn set_presentation_duration(#[compact] count: T::BlockNumber) { + fn set_presentation_duration(origin, #[compact] count: T::BlockNumber) { + ensure_root(origin)?; >::put(count); } /// Set the presentation duration. If there is current a vote being presented for, will /// invoke `finalize_vote`. - fn set_term_duration(#[compact] count: T::BlockNumber) { + fn set_term_duration(origin, #[compact] count: T::BlockNumber) { + ensure_root(origin)?; >::put(count); } @@ -2318,7 +2322,7 @@ mod tests { assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - assert_ok!(Council::set_desired_seats(3)); + assert_ok!(Council::set_desired_seats(Origin::ROOT, 3)); assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); @@ -2606,7 +2610,7 @@ mod tests { System::set_block_number(8); assert_ok!(Council::set_approvals(Origin::signed(6), vec![false, false, true, false], 1, 0)); - assert_ok!(Council::set_desired_seats(3)); + assert_ok!(Council::set_desired_seats(Origin::ROOT, 3)); assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); diff --git a/substrate/srml/democracy/src/lib.rs b/substrate/srml/democracy/src/lib.rs index 411c859802..c159b4fdd8 100644 --- a/substrate/srml/democracy/src/lib.rs +++ b/substrate/srml/democracy/src/lib.rs @@ -513,16 +513,19 @@ decl_module! { } /// Remove a referendum. - fn cancel_referendum(#[compact] ref_index: ReferendumIndex) { + fn cancel_referendum(origin, #[compact] ref_index: ReferendumIndex) { + ensure_root(origin)?; Self::clear_referendum(ref_index); } /// Cancel a proposal queued for enactment. fn cancel_queued( + origin, #[compact] when: T::BlockNumber, #[compact] which: u32, #[compact] what: ReferendumIndex ) { + ensure_root(origin)?; let which = which as usize; let mut items = >::get(when); if items.get(which).and_then(Option::as_ref).map_or(false, |x| x.1 == what) { @@ -1473,9 +1476,9 @@ mod tests { Some((set_balance_proposal(2), 0)) ]); - assert_noop!(Democracy::cancel_queued(3, 0, 0), "proposal not found"); - assert_noop!(Democracy::cancel_queued(4, 1, 0), "proposal not found"); - assert_ok!(Democracy::cancel_queued(4, 0, 0)); + assert_noop!(Democracy::cancel_queued(Origin::ROOT, 3, 0, 0), "proposal not found"); + assert_noop!(Democracy::cancel_queued(Origin::ROOT, 4, 1, 0), "proposal not found"); + assert_ok!(Democracy::cancel_queued(Origin::ROOT, 4, 0, 0)); assert_eq!(Democracy::dispatch_queue(4), vec![None]); }); } @@ -1774,7 +1777,7 @@ mod tests { 0 ).unwrap(); assert_ok!(Democracy::vote(Origin::signed(1), r, AYE)); - assert_ok!(Democracy::cancel_referendum(r.into())); + assert_ok!(Democracy::cancel_referendum(Origin::ROOT, r.into())); next_block(); next_block(); diff --git a/substrate/srml/example/src/lib.rs b/substrate/srml/example/src/lib.rs index 7e197c13e3..87f04ecfc5 100644 --- a/substrate/srml/example/src/lib.rs +++ b/substrate/srml/example/src/lib.rs @@ -254,7 +254,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use srml_support::{StorageValue, dispatch::Result, decl_module, decl_storage, decl_event}; -use system::ensure_signed; +use system::{ensure_signed, ensure_root}; use sr_primitives::weights::TransactionWeight; /// Our module's configuration trait. All our types and consts go in here. If the @@ -440,7 +440,7 @@ decl_module! { } /// A privileged call; in this case it resets our dummy value to something new. - // Implementation of a privileged call. This doesn't have an `origin` parameter because + // Implementation of a privileged call. The `origin` parameter is ROOT because // it's not (directly) from an extrinsic, but rather the system as a whole has decided // to execute it. Different runtimes have different reasons for allow privileged // calls to be executed - we don't need to care why. Because it's privileged, we can @@ -448,7 +448,8 @@ decl_module! { // without worrying about gameability or attack scenarios. // If you not specify `Result` explicitly as return value, it will be added automatically // for you and `Ok(())` will be returned. - fn set_dummy(#[compact] new_value: T::Balance) { + fn set_dummy(origin, #[compact] new_value: T::Balance) { + ensure_root(origin)?; // Put the new value into storage. >::put(new_value); } diff --git a/substrate/srml/staking/src/lib.rs b/substrate/srml/staking/src/lib.rs index 1a2d1dda28..71164c0a9c 100644 --- a/substrate/srml/staking/src/lib.rs +++ b/substrate/srml/staking/src/lib.rs @@ -288,7 +288,7 @@ use primitives::traits::{ }; #[cfg(feature = "std")] use primitives::{Serialize, Deserialize}; -use system::ensure_signed; +use system::{ensure_signed, ensure_root}; use phragmen::{elect, ACCURACY, ExtendedBalance, equalize}; @@ -903,7 +903,8 @@ decl_module! { } /// The ideal number of validators. - fn set_validator_count(#[compact] new: u32) { + fn set_validator_count(origin, #[compact] new: u32) { + ensure_root(origin)?; ValidatorCount::put(new); } @@ -917,17 +918,20 @@ decl_module! { /// - Triggers the Phragmen election. Expensive but not user-controlled. /// - Depends on state: `O(|edges| * |validators|)`. /// # - fn force_new_era() { + fn force_new_era(origin) { + ensure_root(origin)?; Self::apply_force_new_era() } /// Set the offline slash grace period. - fn set_offline_slash_grace(#[compact] new: u32) { + fn set_offline_slash_grace(origin, #[compact] new: u32) { + ensure_root(origin)?; OfflineSlashGrace::put(new); } /// Set the validators who cannot be slashed (if any). - fn set_invulnerables(validators: Vec) { + fn set_invulnerables(origin, validators: Vec) { + ensure_root(origin)?; >::put(validators); } } diff --git a/substrate/srml/staking/src/tests.rs b/substrate/srml/staking/src/tests.rs index 1491ebe0df..774a801687 100644 --- a/substrate/srml/staking/src/tests.rs +++ b/substrate/srml/staking/src/tests.rs @@ -146,7 +146,7 @@ fn invulnerability_should_work() { with_externalities(&mut ExtBuilder::default().build(), || { // Make account 11 invulnerable - assert_ok!(Staking::set_invulnerables(vec![11])); + assert_ok!(Staking::set_invulnerables(Origin::ROOT, vec![11])); // Give account 11 some funds let _ = Balances::make_free_balance_be(&11, 70); // There is no slash grace -- slash immediately. @@ -213,7 +213,7 @@ fn offline_grace_should_delay_slashing() { // Set offline slash grace let offline_slash_grace = 1; - assert_ok!(Staking::set_offline_slash_grace(offline_slash_grace)); + assert_ok!(Staking::set_offline_slash_grace(Origin::ROOT, offline_slash_grace)); assert_eq!(Staking::offline_slash_grace(), 1); // Check unstake_threshold is 3 (default) @@ -1846,7 +1846,7 @@ fn phragmen_linear_worse_case_equalize() { } assert_eq_uvec!(validator_controllers(), vec![40, 30]); - assert_ok!(Staking::set_validator_count(7)); + assert_ok!(Staking::set_validator_count(Origin::ROOT, 7)); start_era(1); diff --git a/substrate/srml/support/src/dispatch.rs b/substrate/srml/support/src/dispatch.rs index 9ac979f45d..8f3a8224db 100644 --- a/substrate/srml/support/src/dispatch.rs +++ b/substrate/srml/support/src/dispatch.rs @@ -104,6 +104,8 @@ impl Parameter for T where T: Codec + Clone + Eq {} /// * `origin`: Alias of `T::Origin`, declared by the [`impl_outer_origin!`](./macro.impl_outer_origin.html) macro. /// * `Result`: The expected return type from module functions. /// +/// The first parameter of dispatchable functions must always be `origin`. +/// /// ### Shorthand Example /// /// The macro automatically expands a shorthand function declaration to return the `Result` type. @@ -132,8 +134,7 @@ impl Parameter for T where T: Codec + Clone + Eq {} /// /// ### Privileged Function Example /// -/// If the `origin` param is omitted, the macro adds it as the first parameter and adds `ensure_root(origin)` -/// as the first line of the function. These functions are the same: +/// A privileged function checks that the origin of the call is `ROOT`. /// /// ``` /// # #[macro_use] @@ -142,14 +143,8 @@ impl Parameter for T where T: Codec + Clone + Eq {} /// # use srml_system::{self as system, Trait, ensure_signed, ensure_root}; /// decl_module! { /// pub struct Module for enum Call where origin: T::Origin { -/// -/// fn my_privileged_function() -> Result { -/// // Your implementation -/// Ok(()) -/// } -/// -/// fn my_function(origin) -> Result { -/// ensure_root(origin); +/// fn my_privileged_function(origin) -> Result { +/// ensure_root(origin)?; /// // Your implementation /// Ok(()) /// } @@ -626,9 +621,8 @@ macro_rules! decl_module { ) => { compile_error!( "First parameter of dispatch should be marked `origin` only, with no type specified \ - (a bit like `self`). (For root-matching dispatches, ensure the first parameter does \ - not use the `T::Origin` type.)" - ) + (a bit like `self`)." + ); }; // Ignore any ident which is `origin` but has a type, regardless of the type token itself. (@normalize @@ -651,11 +645,10 @@ macro_rules! decl_module { ) => { compile_error!( "First parameter of dispatch should be marked `origin` only, with no type specified \ - (a bit like `self`). (For root-matching dispatches, ensure the first parameter does \ - not use the `T::Origin` type.)" - ) + (a bit like `self`)." + ); }; - // Add root if no origin is defined. + // Ignore any function missing `origin` as the first parameter. (@normalize $(#[$attr:meta])* pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path $(= $module_default_instance:path)?)?> @@ -674,25 +667,10 @@ macro_rules! decl_module { ) $( -> $result:ty )* { $( $impl:tt )* } $($rest:tt)* ) => { - $crate::decl_module!(@normalize - $(#[$attr])* - pub struct $mod_type<$trait_instance: $trait_name$(, $instance: $instantiable $(= $module_default_instance)?)?> - for enum $call_type where origin: $origin_type, system = $system - { $( $other_where_bounds )* } - { $( $deposit_event )* } - { $( $on_initialize )* } - { $( $on_finalize )* } - { $( $offchain )* } - { $( $constants )* } - [ $( $dispatchables )* ] - - $(#[doc = $doc_attr])* - $(#[weight = $weight])? - $fn_vis fn $fn_name( - root $( , $(#[$codec_attr])* $param_name : $param )* - ) $( -> $result )* { $( $impl )* } - - $($rest)* + compile_error!( + "Implicit conversion to privileged function has been removed. \ + First parameter of dispatch should be marked `origin`. \ + For root-matching dispatch, also add `ensure_root(origin)?`." ); }; // Last normalize step. Triggers `@imp` expansion which is the real expansion. @@ -726,15 +704,6 @@ macro_rules! decl_module { // Implementation of Call enum's .dispatch() method. // TODO: this probably should be a different macro? - (@call - root - $mod_type:ident<$trait_instance:ident $(, $instance:ident)?> $fn_name:ident $origin:ident $system:ident [ $( $param_name:ident),* ] - ) => { - { - $system::ensure_root($origin)?; - <$mod_type<$trait_instance $(, $instance)?>>::$fn_name( $( $param_name ),* ) - } - }; (@call $ingore:ident $mod_type:ident<$trait_instance:ident $(, $instance:ident)?> $fn_name:ident $origin:ident $system:ident [ $( $param_name:ident),* ] @@ -890,38 +859,6 @@ macro_rules! decl_module { {} }; - // Expansion for root dispatch functions with no specified result type. - (@impl_function - $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; - $origin_ty:ty; - root; - $(#[doc = $doc_attr:tt])* - $vis:vis fn $name:ident ( root $(, $param:ident : $param_ty:ty )* ) { $( $impl:tt )* } - ) => { - $(#[doc = $doc_attr])* - #[allow(unreachable_code)] - $vis fn $name($( $param: $param_ty ),* ) -> $crate::dispatch::Result { - { $( $impl )* } - Ok(()) - } - }; - - // Expansion for root dispatch functions with explicit return types. - (@impl_function - $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; - $origin_ty:ty; - root; - $(#[doc = $doc_attr:tt])* - $vis:vis fn $name:ident ( - root $(, $param:ident : $param_ty:ty )* - ) -> $result:ty { $( $impl:tt )* } - ) => { - $(#[doc = $doc_attr])* - $vis fn $name($( $param: $param_ty ),* ) -> $result { - $( $impl )* - } - }; - // Expansion for _origin_ dispatch functions with no return type. (@impl_function $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; @@ -1596,7 +1533,7 @@ macro_rules! __function_to_metadata { compile_error!(concat!( "Invalid attribute for parameter `", stringify!($param_name), "`, the following attributes are supported: `#[compact]`" - )) + )); } } @@ -1664,8 +1601,8 @@ mod tests { fn aux_1(_origin, #[compact] _data: u32) -> Result { unreachable!() } fn aux_2(_origin, _data: i32, _data2: String) -> Result { unreachable!() } #[weight = TransactionWeight::Basic(10, 100)] - fn aux_3() -> Result { unreachable!() } - fn aux_4(_data: i32) -> Result { unreachable!() } + fn aux_3(_origin) -> Result { unreachable!() } + fn aux_4(_origin, _data: i32) -> Result { unreachable!() } fn aux_5(_origin, _data: i32, #[compact] _data2: u32) -> Result { unreachable!() } fn on_initialize(n: T::BlockNumber) { if n.into() == 42 { panic!("on_initialize") } } @@ -1673,7 +1610,7 @@ mod tests { fn offchain_worker() {} #[weight = TransactionWeight::Max] - fn weighted() { unreachable!() } + fn weighted(_origin) { unreachable!() } } } diff --git a/substrate/srml/support/test/tests/instance.rs b/substrate/srml/support/test/tests/instance.rs index e557a02ecc..62e7263b51 100644 --- a/substrate/srml/support/test/tests/instance.rs +++ b/substrate/srml/support/test/tests/instance.rs @@ -53,7 +53,8 @@ mod module1 { fn deposit_event() = default; - fn one() { + fn one(origin) { + system::ensure_root(origin)?; Self::deposit_event(RawEvent::AnotherVariant(3)); } } diff --git a/substrate/srml/support/test/tests/reserved_keyword/on_initialize.rs b/substrate/srml/support/test/tests/reserved_keyword/on_initialize.rs index c63153241c..f9c2f5f7f0 100644 --- a/substrate/srml/support/test/tests/reserved_keyword/on_initialize.rs +++ b/substrate/srml/support/test/tests/reserved_keyword/on_initialize.rs @@ -19,7 +19,7 @@ macro_rules! reserved { srml_support::decl_module! { pub struct Module for enum Call where origin: T::Origin { - fn $reserved() -> Result { unreachable!() } + fn $reserved(_origin) -> Result { unreachable!() } } } } diff --git a/substrate/srml/system/src/lib.rs b/substrate/srml/system/src/lib.rs index 75f0491d9d..f51ee1dbfc 100644 --- a/substrate/srml/system/src/lib.rs +++ b/substrate/srml/system/src/lib.rs @@ -85,7 +85,7 @@ use primitives::traits::Zero; use substrate_primitives::storage::well_known_keys; use srml_support::{ storage, decl_module, decl_event, decl_storage, StorageDoubleMap, StorageValue, - StorageMap, Parameter, for_each_tuple, traits::Contains + StorageMap, Parameter, for_each_tuple, traits::Contains, }; use safe_mix::TripletMix; use parity_codec::{Encode, Decode}; @@ -205,24 +205,28 @@ decl_module! { } /// Set the number of pages in the WebAssembly environment's heap. - fn set_heap_pages(pages: u64) { + fn set_heap_pages(origin, pages: u64) { + ensure_root(origin)?; storage::unhashed::put_raw(well_known_keys::HEAP_PAGES, &pages.encode()); } /// Set the new code. - pub fn set_code(new: Vec) { + pub fn set_code(origin, new: Vec) { + ensure_root(origin)?; storage::unhashed::put_raw(well_known_keys::CODE, &new); } /// Set some items of storage. - fn set_storage(items: Vec) { + fn set_storage(origin, items: Vec) { + ensure_root(origin)?; for i in &items { storage::unhashed::put_raw(&i.0, &i.1); } } /// Kill some items from storage. - fn kill_storage(keys: Vec) { + fn kill_storage(origin, keys: Vec) { + ensure_root(origin)?; for key in &keys { storage::unhashed::kill(&key); }