Add sudo::remove_key (#2165)

Changes:
- Adds a new call `remove_key` to the sudo pallet to permanently remove
the sudo key.
- Remove some clones and general maintenance

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>
This commit is contained in:
Oliver Tale-Yazdi
2023-11-08 12:59:55 +01:00
committed by GitHub
parent 640e385aec
commit 9adb46c868
8 changed files with 241 additions and 161 deletions
+27 -17
View File
@@ -22,41 +22,40 @@ use crate::Pallet;
use frame_benchmarking::v2::*;
use frame_system::RawOrigin;
const SEED: u32 = 0;
fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
frame_system::Pallet::<T>::assert_last_event(generic_event.into());
fn assert_last_event<T: Config>(generic_event: crate::Event<T>) {
let re: <T as Config>::RuntimeEvent = generic_event.into();
frame_system::Pallet::<T>::assert_last_event(re.into());
}
#[benchmarks( where <T as Config>::RuntimeCall: From<frame_system::Call<T>>)]
#[benchmarks(where <T as Config>::RuntimeCall: From<frame_system::Call<T>>)]
mod benchmarks {
use super::*;
#[benchmark]
fn set_key() {
let caller: T::AccountId = whitelisted_caller();
Key::<T>::put(caller.clone());
Key::<T>::put(&caller);
let new_sudoer: T::AccountId = account("sudoer", 0, SEED);
let new_sudoer: T::AccountId = account("sudoer", 0, 0);
let new_sudoer_lookup = T::Lookup::unlookup(new_sudoer.clone());
#[extrinsic_call]
_(RawOrigin::Signed(caller.clone()), new_sudoer_lookup);
assert_last_event::<T>(Event::KeyChanged { old_sudoer: Some(caller) }.into());
assert_last_event::<T>(Event::KeyChanged { old: Some(caller), new: new_sudoer });
}
#[benchmark]
fn sudo() {
let caller: T::AccountId = whitelisted_caller();
Key::<T>::put(caller.clone());
Key::<T>::put(&caller);
let call: <T as Config>::RuntimeCall = frame_system::Call::remark { remark: vec![] }.into();
let call = frame_system::Call::remark { remark: vec![] }.into();
#[extrinsic_call]
_(RawOrigin::Signed(caller.clone()), Box::new(call.clone()));
_(RawOrigin::Signed(caller), Box::new(call));
assert_last_event::<T>(Event::Sudid { sudo_result: Ok(()) }.into())
assert_last_event::<T>(Event::Sudid { sudo_result: Ok(()) })
}
#[benchmark]
@@ -64,15 +63,26 @@ mod benchmarks {
let caller: T::AccountId = whitelisted_caller();
Key::<T>::put(caller.clone());
let call: <T as Config>::RuntimeCall = frame_system::Call::remark { remark: vec![] }.into();
let call = frame_system::Call::remark { remark: vec![] }.into();
let who: T::AccountId = account("as", 0, SEED);
let who_lookup = T::Lookup::unlookup(who.clone());
let who: T::AccountId = account("as", 0, 0);
let who_lookup = T::Lookup::unlookup(who);
#[extrinsic_call]
_(RawOrigin::Signed(caller), who_lookup, Box::new(call.clone()));
_(RawOrigin::Signed(caller), who_lookup, Box::new(call));
assert_last_event::<T>(Event::SudoAsDone { sudo_result: Ok(()) }.into())
assert_last_event::<T>(Event::SudoAsDone { sudo_result: Ok(()) })
}
#[benchmark]
fn remove_key() {
let caller: T::AccountId = whitelisted_caller();
Key::<T>::put(&caller);
#[extrinsic_call]
_(RawOrigin::Signed(caller.clone()));
assert_last_event::<T>(Event::KeyRemoved {});
}
impl_benchmark_test_suite!(Pallet, crate::mock::new_bench_ext(), crate::mock::Test);
+51 -43
View File
@@ -146,7 +146,7 @@ type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup
pub mod pallet {
use super::{DispatchResult, *};
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;
use frame_system::{pallet_prelude::*, RawOrigin};
/// Default preludes for [`Config`].
pub mod config_preludes {
@@ -190,11 +190,6 @@ pub mod pallet {
#[pallet::call]
impl<T: Config> Pallet<T> {
/// Authenticates the sudo key and dispatches a function call with `Root` origin.
///
/// The dispatch origin for this call must be _Signed_.
///
/// ## Complexity
/// - O(1).
#[pallet::call_index(0)]
#[pallet::weight({
let dispatch_info = call.get_dispatch_info();
@@ -207,12 +202,11 @@ pub mod pallet {
origin: OriginFor<T>,
call: Box<<T as Config>::RuntimeCall>,
) -> DispatchResultWithPostInfo {
// This is a public call, so we ensure that the origin is some signed account.
let sender = ensure_signed(origin)?;
ensure!(Self::key().map_or(false, |k| sender == k), Error::<T>::RequireSudo);
Self::ensure_sudo(origin)?;
let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into());
let res = call.dispatch_bypass_filter(RawOrigin::Root.into());
Self::deposit_event(Event::Sudid { sudo_result: res.map(|_| ()).map_err(|e| e.error) });
// Sudo user does not pay a fee.
Ok(Pays::No.into())
}
@@ -222,9 +216,6 @@ pub mod pallet {
/// Sudo user to specify the weight of the call.
///
/// The dispatch origin for this call must be _Signed_.
///
/// ## Complexity
/// - O(1).
#[pallet::call_index(1)]
#[pallet::weight((*weight, call.get_dispatch_info().class))]
pub fn sudo_unchecked_weight(
@@ -232,37 +223,30 @@ pub mod pallet {
call: Box<<T as Config>::RuntimeCall>,
weight: Weight,
) -> DispatchResultWithPostInfo {
// This is a public call, so we ensure that the origin is some signed account.
let sender = ensure_signed(origin)?;
Self::ensure_sudo(origin)?;
let _ = weight; // We don't check the weight witness since it is a root call.
ensure!(Self::key().map_or(false, |k| sender == k), Error::<T>::RequireSudo);
let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into());
let res = call.dispatch_bypass_filter(RawOrigin::Root.into());
Self::deposit_event(Event::Sudid { sudo_result: res.map(|_| ()).map_err(|e| e.error) });
// Sudo user does not pay a fee.
Ok(Pays::No.into())
}
/// Authenticates the current sudo key and sets the given AccountId (`new`) as the new sudo
/// key.
///
/// The dispatch origin for this call must be _Signed_.
///
/// ## Complexity
/// - O(1).
#[pallet::call_index(2)]
#[pallet::weight(T::WeightInfo::set_key())]
pub fn set_key(
origin: OriginFor<T>,
new: AccountIdLookupOf<T>,
) -> DispatchResultWithPostInfo {
// This is a public call, so we ensure that the origin is some signed account.
let sender = ensure_signed(origin)?;
ensure!(Self::key().map_or(false, |k| sender == k), Error::<T>::RequireSudo);
let new = T::Lookup::lookup(new)?;
Self::ensure_sudo(origin)?;
let new = T::Lookup::lookup(new)?;
Self::deposit_event(Event::KeyChanged { old: Key::<T>::get(), new: new.clone() });
Key::<T>::put(new);
Self::deposit_event(Event::KeyChanged { old_sudoer: Key::<T>::get() });
Key::<T>::put(&new);
// Sudo user does not pay a fee.
Ok(Pays::No.into())
}
@@ -271,9 +255,6 @@ pub mod pallet {
/// a given account.
///
/// The dispatch origin for this call must be _Signed_.
///
/// ## Complexity
/// - O(1).
#[pallet::call_index(3)]
#[pallet::weight({
let dispatch_info = call.get_dispatch_info();
@@ -287,17 +268,29 @@ pub mod pallet {
who: AccountIdLookupOf<T>,
call: Box<<T as Config>::RuntimeCall>,
) -> DispatchResultWithPostInfo {
// This is a public call, so we ensure that the origin is some signed account.
let sender = ensure_signed(origin)?;
ensure!(Self::key().map_or(false, |k| sender == k), Error::<T>::RequireSudo);
Self::ensure_sudo(origin)?;
let who = T::Lookup::lookup(who)?;
let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Signed(who).into());
let res = call.dispatch_bypass_filter(RawOrigin::Signed(who).into());
Self::deposit_event(Event::SudoAsDone {
sudo_result: res.map(|_| ()).map_err(|e| e.error),
});
// Sudo user does not pay a fee.
Ok(Pays::No.into())
}
/// Permanently removes the sudo key.
///
/// **This cannot be un-done.**
#[pallet::call_index(4)]
#[pallet::weight(T::WeightInfo::remove_key())]
pub fn remove_key(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
Self::ensure_sudo(origin)?;
Self::deposit_event(Event::KeyRemoved {});
Key::<T>::kill();
// Sudo user does not pay a fee.
Ok(Pays::No.into())
}
@@ -313,9 +306,13 @@ pub mod pallet {
},
/// The sudo key has been updated.
KeyChanged {
/// The old sudo key if one was previously set.
old_sudoer: Option<T::AccountId>,
/// The old sudo key (if one was previously set).
old: Option<T::AccountId>,
/// The new sudo key (if one was set).
new: T::AccountId,
},
/// The key was permanently removed.
KeyRemoved,
/// A [sudo_as](Pallet::sudo_as) call just took place.
SudoAsDone {
/// The result of the call made by the sudo user.
@@ -324,9 +321,9 @@ pub mod pallet {
}
#[pallet::error]
/// Error for the Sudo pallet
/// Error for the Sudo pallet.
pub enum Error<T> {
/// Sender must be the Sudo account
/// Sender must be the Sudo account.
RequireSudo,
}
@@ -345,8 +342,19 @@ pub mod pallet {
#[pallet::genesis_build]
impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
fn build(&self) {
if let Some(ref key) = self.key {
Key::<T>::put(key);
Key::<T>::set(self.key.clone());
}
}
impl<T: Config> Pallet<T> {
/// Ensure that the caller is the sudo key.
pub(crate) fn ensure_sudo(origin: OriginFor<T>) -> DispatchResult {
let sender = ensure_signed(origin)?;
if Self::key().map_or(false, |k| k == sender) {
Ok(())
} else {
Err(Error::<T>::RequireSudo.into())
}
}
}
+3 -1
View File
@@ -156,7 +156,9 @@ pub fn new_test_ext(root_key: u64) -> sp_io::TestExternalities {
sudo::GenesisConfig::<Test> { key: Some(root_key) }
.assimilate_storage(&mut t)
.unwrap();
t.into()
let mut ext: sp_io::TestExternalities = t.into();
ext.execute_with(|| System::set_block_number(1));
ext
}
#[cfg(feature = "runtime-benchmarks")]
+14 -14
View File
@@ -59,9 +59,6 @@ fn sudo_basics() {
#[test]
fn sudo_emits_events_correctly() {
new_test_ext(1).execute_with(|| {
// Set block number to 1 because events are not emitted on block 0.
System::set_block_number(1);
// Should emit event to indicate success when called with the root `key` and `call` is `Ok`.
let call = Box::new(RuntimeCall::Logger(LoggerCall::privileged_i32_log {
i: 42,
@@ -118,9 +115,6 @@ fn sudo_unchecked_weight_basics() {
#[test]
fn sudo_unchecked_weight_emits_events_correctly() {
new_test_ext(1).execute_with(|| {
// Set block number to 1 because events are not emitted on block 0.
System::set_block_number(1);
// Should emit event to indicate success when called with the root `key` and `call` is `Ok`.
let call = Box::new(RuntimeCall::Logger(LoggerCall::privileged_i32_log {
i: 42,
@@ -154,15 +148,24 @@ fn set_key_basics() {
#[test]
fn set_key_emits_events_correctly() {
new_test_ext(1).execute_with(|| {
// Set block number to 1 because events are not emitted on block 0.
System::set_block_number(1);
// A root `key` can change the root `key`.
assert_ok!(Sudo::set_key(RuntimeOrigin::signed(1), 2));
System::assert_has_event(TestEvent::Sudo(Event::KeyChanged { old_sudoer: Some(1) }));
System::assert_has_event(TestEvent::Sudo(Event::KeyChanged { old: Some(1), new: 2 }));
// Double check.
assert_ok!(Sudo::set_key(RuntimeOrigin::signed(2), 4));
System::assert_has_event(TestEvent::Sudo(Event::KeyChanged { old_sudoer: Some(2) }));
System::assert_has_event(TestEvent::Sudo(Event::KeyChanged { old: Some(2), new: 4 }));
});
}
#[test]
fn remove_key_works() {
new_test_ext(1).execute_with(|| {
assert_ok!(Sudo::remove_key(RuntimeOrigin::signed(1)));
assert!(Sudo::key().is_none());
System::assert_has_event(TestEvent::Sudo(Event::KeyRemoved {}));
assert_noop!(Sudo::remove_key(RuntimeOrigin::signed(1)), Error::<Test>::RequireSudo);
assert_noop!(Sudo::set_key(RuntimeOrigin::signed(1), 1), Error::<Test>::RequireSudo);
});
}
@@ -201,9 +204,6 @@ fn sudo_as_basics() {
#[test]
fn sudo_as_emits_events_correctly() {
new_test_ext(1).execute_with(|| {
// Set block number to 1 because events are not emitted on block 0.
System::set_block_number(1);
// A non-privileged function will work when passed to `sudo_as` with the root `key`.
let call = Box::new(RuntimeCall::Logger(LoggerCall::non_privileged_log {
i: 42,
+61 -41
View File
@@ -15,32 +15,29 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//! Autogenerated weights for pallet_sudo.
//! Autogenerated weights for `pallet_sudo`
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2023-06-16, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2023-11-07, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `runner-e8ezs4ez-project-145-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz`
//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
//! HOSTNAME: `runner-yprdrvc7-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz`
//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024`
// Executed Command:
// ./target/production/substrate
// target/production/substrate-node
// benchmark
// pallet
// --chain=dev
// --steps=50
// --repeat=20
// --pallet=pallet_sudo
// --no-storage-info
// --no-median-slopes
// --no-min-squares
// --extrinsic=*
// --execution=wasm
// --wasm-execution=compiled
// --heap-pages=4096
// --output=./frame/sudo/src/weights.rs
// --header=./HEADER-APACHE2
// --template=./.maintain/frame-weight-template.hbs
// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json
// --pallet=pallet_sudo
// --chain=dev
// --header=./substrate/HEADER-APACHE2
// --output=./substrate/frame/sudo/src/weights.rs
// --template=./substrate/.maintain/frame-weight-template.hbs
#![cfg_attr(rustfmt, rustfmt_skip)]
#![allow(unused_parens)]
@@ -50,80 +47,103 @@
use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}};
use core::marker::PhantomData;
/// Weight functions needed for pallet_sudo.
/// Weight functions needed for `pallet_sudo`.
pub trait WeightInfo {
fn set_key() -> Weight;
fn sudo() -> Weight;
fn sudo_as() -> Weight;
fn remove_key() -> Weight;
}
/// Weights for pallet_sudo using the Substrate node and recommended hardware.
/// Weights for `pallet_sudo` using the Substrate node and recommended hardware.
pub struct SubstrateWeight<T>(PhantomData<T>);
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
/// Storage: Sudo Key (r:1 w:1)
/// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen)
/// Storage: `Sudo::Key` (r:1 w:1)
/// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
fn set_key() -> Weight {
// Proof Size summary in bytes:
// Measured: `165`
// Estimated: `1517`
// Minimum execution time: 12_918_000 picoseconds.
Weight::from_parts(13_403_000, 1517)
// Minimum execution time: 9_600_000 picoseconds.
Weight::from_parts(10_076_000, 1517)
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
/// Storage: Sudo Key (r:1 w:0)
/// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen)
/// Storage: `Sudo::Key` (r:1 w:0)
/// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
fn sudo() -> Weight {
// Proof Size summary in bytes:
// Measured: `165`
// Estimated: `1517`
// Minimum execution time: 12_693_000 picoseconds.
Weight::from_parts(13_001_000, 1517)
// Minimum execution time: 10_453_000 picoseconds.
Weight::from_parts(10_931_000, 1517)
.saturating_add(T::DbWeight::get().reads(1_u64))
}
/// Storage: Sudo Key (r:1 w:0)
/// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen)
/// Storage: `Sudo::Key` (r:1 w:0)
/// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
fn sudo_as() -> Weight {
// Proof Size summary in bytes:
// Measured: `165`
// Estimated: `1517`
// Minimum execution time: 12_590_000 picoseconds.
Weight::from_parts(12_994_000, 1517)
// Minimum execution time: 10_202_000 picoseconds.
Weight::from_parts(10_800_000, 1517)
.saturating_add(T::DbWeight::get().reads(1_u64))
}
/// Storage: `Sudo::Key` (r:1 w:1)
/// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
fn remove_key() -> Weight {
// Proof Size summary in bytes:
// Measured: `165`
// Estimated: `1517`
// Minimum execution time: 8_555_000 picoseconds.
Weight::from_parts(8_846_000, 1517)
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
}
}
// For backwards compatibility and tests
// For backwards compatibility and tests.
impl WeightInfo for () {
/// Storage: Sudo Key (r:1 w:1)
/// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen)
/// Storage: `Sudo::Key` (r:1 w:1)
/// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
fn set_key() -> Weight {
// Proof Size summary in bytes:
// Measured: `165`
// Estimated: `1517`
// Minimum execution time: 12_918_000 picoseconds.
Weight::from_parts(13_403_000, 1517)
// Minimum execution time: 9_600_000 picoseconds.
Weight::from_parts(10_076_000, 1517)
.saturating_add(RocksDbWeight::get().reads(1_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
/// Storage: Sudo Key (r:1 w:0)
/// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen)
/// Storage: `Sudo::Key` (r:1 w:0)
/// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
fn sudo() -> Weight {
// Proof Size summary in bytes:
// Measured: `165`
// Estimated: `1517`
// Minimum execution time: 12_693_000 picoseconds.
Weight::from_parts(13_001_000, 1517)
// Minimum execution time: 10_453_000 picoseconds.
Weight::from_parts(10_931_000, 1517)
.saturating_add(RocksDbWeight::get().reads(1_u64))
}
/// Storage: Sudo Key (r:1 w:0)
/// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen)
/// Storage: `Sudo::Key` (r:1 w:0)
/// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
fn sudo_as() -> Weight {
// Proof Size summary in bytes:
// Measured: `165`
// Estimated: `1517`
// Minimum execution time: 12_590_000 picoseconds.
Weight::from_parts(12_994_000, 1517)
// Minimum execution time: 10_202_000 picoseconds.
Weight::from_parts(10_800_000, 1517)
.saturating_add(RocksDbWeight::get().reads(1_u64))
}
/// Storage: `Sudo::Key` (r:1 w:1)
/// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
fn remove_key() -> Weight {
// Proof Size summary in bytes:
// Measured: `165`
// Estimated: `1517`
// Minimum execution time: 8_555_000 picoseconds.
Weight::from_parts(8_846_000, 1517)
.saturating_add(RocksDbWeight::get().reads(1_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
}
}