Make priviledged functions explicity use origin (#3045)

* Make priviledged functions explicity use `origin`

* Fix typo in docs

* Fix more tests

* Remove `root` pathway, add semicolons
This commit is contained in:
Shawn Tabrizi
2019-07-08 15:51:54 +02:00
committed by Bastian Köcher
parent 29311e98b4
commit 3d72844710
11 changed files with 71 additions and 114 deletions
+3 -1
View File
@@ -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.
/// # </weight>
fn set_balance(
origin,
who: <T::Lookup as StaticLookup>::Source,
#[compact] new_free: T::Balance,
#[compact] new_reserved: T::Balance
) {
ensure_root(origin)?;
let who = T::Lookup::lookup(who)?;
let current_free = <FreeBalance<T, I>>::get(&who);
+3 -2
View File
@@ -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 <Module<T>>::current_schedule().version >= schedule.version {
return Err("new schedule must have a greater version than current");
}
+11 -7
View File
@@ -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: <T::Lookup as StaticLookup>::Source) {
fn remove_member(origin, who: <T::Lookup as StaticLookup>::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)?;
<PresentationDuration<T>>::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)?;
<TermDuration<T>>::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);
+8 -5
View File
@@ -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 = <DispatchQueue<T>>::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();
+4 -3
View File
@@ -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.
<Dummy<T>>::put(new_value);
}
+9 -5
View File
@@ -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|)`.
/// # </weight>
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<T::AccountId>) {
fn set_invulnerables(origin, validators: Vec<T::AccountId>) {
ensure_root(origin)?;
<Invulnerables<T>>::put(validators);
}
}
+3 -3
View File
@@ -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);
+18 -81
View File
@@ -104,6 +104,8 @@ impl<T> 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<T> 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<T> Parameter for T where T: Codec + Clone + Eq {}
/// # use srml_system::{self as system, Trait, ensure_signed, ensure_root};
/// decl_module! {
/// pub struct Module<T: Trait> 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$(<I>, $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$(<I>, $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$(<I>, $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$(<I>, $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$(<I>, $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!() }
}
}
@@ -53,7 +53,8 @@ mod module1 {
fn deposit_event<T, I>() = default;
fn one() {
fn one(origin) {
system::ensure_root(origin)?;
Self::deposit_event(RawEvent::AnotherVariant(3));
}
}
@@ -19,7 +19,7 @@ macro_rules! reserved {
srml_support::decl_module! {
pub struct Module<T: Trait> for enum Call where origin: T::Origin {
fn $reserved() -> Result { unreachable!() }
fn $reserved(_origin) -> Result { unreachable!() }
}
}
}
+9 -5
View File
@@ -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<u8>) {
pub fn set_code(origin, new: Vec<u8>) {
ensure_root(origin)?;
storage::unhashed::put_raw(well_known_keys::CODE, &new);
}
/// Set some items of storage.
fn set_storage(items: Vec<KeyValue>) {
fn set_storage(origin, items: Vec<KeyValue>) {
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<Key>) {
fn kill_storage(origin, keys: Vec<Key>) {
ensure_root(origin)?;
for key in &keys {
storage::unhashed::kill(&key);
}