diff --git a/Cargo.lock b/Cargo.lock index 1e624fe65b33d..e98d42f5076bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6184,6 +6184,8 @@ dependencies = [ "frame-support", "frame-system", "pallet-balances", + "pallet-proxy", + "pallet-utility", "parity-scale-codec", "scale-info", "sp-core", @@ -6513,6 +6515,8 @@ dependencies = [ "frame-support", "frame-system", "pallet-balances", + "pallet-proxy", + "pallet-utility", "parity-scale-codec", "scale-info", "sp-core", diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 61e051120ffbe..b549f5f6ed0c7 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -373,8 +373,6 @@ pub fn testnet_genesis( min_join_bond: 1 * DOLLARS, ..Default::default() }, - safe_mode: Default::default(), - tx_pause: Default::default(), } } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index e6ef73a5a3269..d20f10014f3d1 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -33,9 +33,8 @@ use frame_support::{ parameter_types, traits::{ AsEnsureOriginWithArg, ConstBool, ConstU128, ConstU16, ConstU32, Contains, Currency, - EitherOfDiverse, EqualPrivilegeOnly, Imbalance, InsideBoth, InstanceFilter, - KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, PalletInfoAccess, - U128CurrencyToVote, + EitherOfDiverse, EnsureOrigin, EqualPrivilegeOnly, Imbalance, InsideBoth, InstanceFilter, + KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, }, weights::{ constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight, WEIGHT_PER_SECOND}, @@ -45,7 +44,7 @@ use frame_support::{ }; use frame_system::{ limits::{BlockLength, BlockWeights}, - EnsureRoot, EnsureRootWithSuccess, EnsureSigned, + EnsureRoot, EnsureRootWithSuccess, EnsureSigned, RawOrigin, }; pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; @@ -201,57 +200,189 @@ parameter_types! { const_assert!(NORMAL_DISPATCH_RATIO.deconstruct() >= AVERAGE_ON_INITIALIZE_RATIO.deconstruct()); -pub struct UnpausablePallets; -impl Contains> for UnpausablePallets { - fn contains(pallet: &pallet_tx_pause::PalletNameOf) -> bool { - pallet.as_ref() == - as PalletInfoAccess>::name() - .as_bytes() - .to_vec() +/// Filter to block balance pallet calls +/// Used for both SafeMode and TxPause pallets +/// Therefor we include both so they cannot affect each other +pub struct UnfilterableCalls; +impl Contains for UnfilterableCalls { + fn contains(call: &RuntimeCall) -> bool { + match call { + RuntimeCall::System(_) | RuntimeCall::SafeMode(_) | RuntimeCall::TxPause(_) => true, + RuntimeCall::Balances(_) => false, + _ => false, + } + } +} + +use pallet_tx_pause::FullNameOf; +pub struct UnfilterableCallNames; +/// Make Balances::transfer_keep_alive unfilterable, accept all others. +impl Contains> for UnfilterableCallNames { + fn contains(full_name: &FullNameOf) -> bool { + let unpausables: Vec> = vec![( + b"Balances".to_vec().try_into().unwrap(), + Some(b"transfer_keep_alive".to_vec().try_into().unwrap()), + )]; + + for unpausable_call in unpausables { + let (pallet_name, maybe_call_name) = full_name; + if pallet_name == &unpausable_call.0 { + if unpausable_call.1.is_none() { + return true + } + return maybe_call_name == &unpausable_call.1 + } + } + + false } } impl pallet_tx_pause::Config for Runtime { - type Event = Event; - type UnpausablePallets = UnpausablePallets; + type RuntimeEvent = RuntimeEvent; + type RuntimeCall = RuntimeCall; type PauseOrigin = EnsureRoot; type UnpauseOrigin = EnsureRoot; + type UnfilterableCallNames = UnfilterableCallNames; type MaxNameLen = ConstU32<256>; type PauseTooLongNames = ConstBool; type WeightInfo = pallet_tx_pause::weights::SubstrateWeight; } -parameter_types! { - // signed config - pub const ActivateStakeAmount: Balance = 1 * DOLLARS; //TODO This needs to be something sensible for the implications of enablement! - pub const ExtendStakeAmount: Balance = 1 * DOLLARS; //TODO This needs to be something sensible for the implications of enablement! - pub BlockHeight: BlockNumber = System::block_number(); // TODO ensure this plus config below is correct +/// An origin that can enable the safe-mode by force. +pub enum ForceActivateOrigin { + Weak, + Medium, + Strong, +} + +/// An origin that can extend the safe-mode by force. +pub enum ForceExtendOrigin { + Weak, + Medium, + Strong, +} + +impl ForceActivateOrigin { + /// The duration of how long the safe-mode will be activated. + pub fn duration(&self) -> u32 { + match self { + Self::Weak => 5, + Self::Medium => 7, + Self::Strong => 11, + } + } + + /// Account id of the origin. + pub fn acc(&self) -> AccountId { + match self { + Self::Weak => sp_core::ed25519::Public::from_raw([0; 32]).into(), + Self::Medium => sp_core::ed25519::Public::from_raw([1; 32]).into(), + Self::Strong => sp_core::ed25519::Public::from_raw([2; 32]).into(), + } + } + + /// Signed origin. + pub fn signed(&self) -> ::Origin { + RawOrigin::Signed(self.acc()).into() + } +} + +impl ForceExtendOrigin { + /// The duration of how long the safe-mode will be extended. + pub fn duration(&self) -> u32 { + match self { + Self::Weak => 13, + Self::Medium => 17, + Self::Strong => 19, + } + } + + /// Account id of the origin. + pub fn acc(&self) -> AccountId { + match self { + Self::Weak => sp_core::ed25519::Public::from_raw([0; 32]).into(), + Self::Medium => sp_core::ed25519::Public::from_raw([1; 32]).into(), + Self::Strong => sp_core::ed25519::Public::from_raw([2; 32]).into(), + } + } + + /// Signed origin. + pub fn signed(&self) -> ::Origin { + RawOrigin::Signed(self.acc()).into() + } +} + +impl, O>> + From>> EnsureOrigin + for ForceActivateOrigin +{ + type Success = u32; + + fn try_origin(o: O) -> Result { + o.into().and_then(|o| match o { + RawOrigin::Signed(acc) if acc == ForceActivateOrigin::Weak.acc() => + Ok(ForceActivateOrigin::Weak.duration()), + RawOrigin::Signed(acc) if acc == ForceActivateOrigin::Medium.acc() => + Ok(ForceActivateOrigin::Medium.duration()), + RawOrigin::Signed(acc) if acc == ForceActivateOrigin::Strong.acc() => + Ok(ForceActivateOrigin::Strong.duration()), + r => Err(O::from(r)), + }) + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> O { + O::from(RawOrigin::Signed(ForceActivateOrigin::Strong.acc())) + } +} + +impl, O>> + From>> EnsureOrigin + for ForceExtendOrigin +{ + type Success = u32; + + fn try_origin(o: O) -> Result { + o.into().and_then(|o| match o { + RawOrigin::Signed(acc) if acc == ForceExtendOrigin::Weak.acc() => + Ok(ForceExtendOrigin::Weak.duration()), + RawOrigin::Signed(acc) if acc == ForceExtendOrigin::Medium.acc() => + Ok(ForceExtendOrigin::Medium.duration()), + RawOrigin::Signed(acc) if acc == ForceExtendOrigin::Strong.acc() => + Ok(ForceExtendOrigin::Strong.duration()), + r => Err(O::from(r)), + }) + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> O { + O::from(RawOrigin::Signed(ForceActivateOrigin::Strong.acc())) + } } parameter_types! { - // signed config - pub const ActivateStakeAmount: Balance = 1 * DOLLARS; //TODO This needs to be something sensible for the implications of enablement! - pub const ExtendStakeAmount: Balance = 1 * DOLLARS; //TODO This needs to be something sensible for the implications of enablement! - pub BlockHeight: BlockNumber = System::block_number(); // TODO ensure this plus config below is correct + pub const SignedActivationDuration: u32 = 3; + pub const SignedExtendDuration: u32 = 30; + pub const ActivateReservationAmount: Balance = 10 * DOLLARS; //TODO This needs to be something sensible for the implications of enablement! + pub const ExtendReservationAmount: Balance = 10 * DOLLARS; //TODO This needs to be something sensible for the implications of enablement! } impl pallet_safe_mode::Config for Runtime { - type Event = Event; + type RuntimeEvent = RuntimeEvent; type Currency = Balances; - type SafeModeFilter = Nothing; // TODO add TxPause pallet - type ActivateDuration = ConstU32<{ 2 * DAYS }>; - type ExtendDuration = ConstU32<{ 1 * DAYS }>; - type EnableOrigin = EnsureRootWithSuccess; - type ExtendOrigin = EnsureRootWithSuccess; - type DeactivateOrigin = EnsureRoot; - type RepayOrigin = EnsureRoot; - type ActivateStakeAmount = ActivateStakeAmount; - type ExtendStakeAmount = ExtendStakeAmount; + type UnfilterableCalls = UnfilterableCalls; + type SignedActivationDuration = ConstU32<{ 2 * DAYS }>; + type ActivateReservationAmount = ActivateReservationAmount; + type SignedExtendDuration = ConstU32<{ 1 * DAYS }>; + type ExtendReservationAmount = ExtendReservationAmount; + type ForceActivateOrigin = ForceActivateOrigin; + type ForceExtendOrigin = ForceExtendOrigin; + type ForceDeactivateOrigin = EnsureRoot; + type RepayOrigin = EnsureRoot; type WeightInfo = pallet_safe_mode::weights::SubstrateWeight; } impl frame_system::Config for Runtime { - type BaseCallFilter = InsideBoth; + type BaseCallFilter = InsideBoth; // TODO consider Exclude or NotInside for UnfilterableCalls -> see TheseExcept ) type BlockWeights = RuntimeBlockWeights; type BlockLength = RuntimeBlockLength; type DbWeight = RocksDbWeight; @@ -312,6 +443,11 @@ parameter_types! { pub const AnnouncementDepositFactor: Balance = deposit(0, 66); } +// pub enum PausePresets { +// ..., // todo + +// } + /// The type used to represent the kinds of proxying allowed. #[derive( Copy, @@ -482,7 +618,7 @@ parameter_types! { impl pallet_balances::Config for Runtime { type MaxLocks = MaxLocks; type MaxReserves = MaxReserves; - type ReserveIdentifier = [u8; 8]; + type ReserveIdentifier = Self::BlockNumber; type Balance = Balance; type DustRemoval = (); type RuntimeEvent = RuntimeEvent; diff --git a/frame/safe-mode/Cargo.toml b/frame/safe-mode/Cargo.toml index 2ec371617b8cc..ea1f77133be16 100644 --- a/frame/safe-mode/Cargo.toml +++ b/frame/safe-mode/Cargo.toml @@ -20,12 +20,17 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys scale-info = { version = "2.0.1", default-features = false, features = ["derive"] } sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } +pallet-balances = { version = "4.0.0-dev", path = "../balances", default-features = false, optional = true } +pallet-utility = { version = "4.0.0-dev", path = "../utility", default-features = false, optional = true } +pallet-proxy = { version = "4.0.0-dev", path = "../proxy", default-features = false, optional = true } [dev-dependencies] sp-core = { version = "6.0.0", path = "../../primitives/core" } sp-std = { version = "4.0.0", path = "../../primitives/std" } sp-io = { version = "6.0.0", path = "../../primitives/io" } pallet-balances = { version = "4.0.0-dev", path = "../balances" } +pallet-utility = { version = "4.0.0-dev", path = "../utility" } +pallet-proxy = { version = "4.0.0-dev", path = "../proxy" } [features] default = ["std"] @@ -33,8 +38,11 @@ std = [ "codec/std", "scale-info/std", "frame-benchmarking/std", - "frame-support/std", "frame-system/std", + "frame-support/std", + "pallet-balances?/std", + "pallet-utility?/std", + "pallet-proxy?/std", "sp-runtime/std", "sp-std/std", ] @@ -42,6 +50,9 @@ runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", + "pallet-balances/runtime-benchmarks", + "pallet-utility/runtime-benchmarks", + "pallet-proxy/runtime-benchmarks", ] try-runtime = [ "frame-support/try-runtime", diff --git a/frame/safe-mode/src/benchmarking.rs b/frame/safe-mode/src/benchmarking.rs index 8fb175e8dd141..0c43a4b30a2d3 100644 --- a/frame/safe-mode/src/benchmarking.rs +++ b/frame/safe-mode/src/benchmarking.rs @@ -7,7 +7,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -17,79 +17,139 @@ #![cfg(feature = "runtime-benchmarks")] -use super::{Call as SafeModeCall, Pallet as SafeMode, *}; +use super::{Pallet as SafeMode, *}; use frame_benchmarking::{benchmarks, whitelisted_caller}; -use frame_support::traits::Currency; +use frame_support::traits::{Currency, UnfilteredDispatchable}; use frame_system::{Pallet as System, RawOrigin}; use sp_runtime::traits::Bounded; benchmarks! { - activate { + activate { let caller: T::AccountId = whitelisted_caller(); - let origin = RawOrigin::Signed(caller.clone()); + let origin = RawOrigin::Signed(caller.clone()); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - }: activate(origin) - verify { + }: _(origin) + verify { assert_eq!( - SafeMode::::activated().unwrap(), - System::::block_number() + T::ActivateDuration::get() + SafeMode::::active_until().unwrap(), + System::::block_number() + T::SignedActivationDuration::get() ); - } - -// force_activate { -// /* code to set the initial state */ -// }: { -// /* code to test the function benchmarked */ -// } -// verify { -// /* optional verification */ -// } - -// extend { -// /* code to set the initial state */ -// }: { -// /* code to test the function benchmarked */ -// } -// verify { -// /* optional verification */ -// } - -// force_extend { -// /* code to set the initial state */ -// }: { -// /* code to test the function benchmarked */ -// } -// verify { -// /* optional verification */ -// } - -// force_inactivate { -// /* code to set the initial state */ -// }: { -// /* code to test the function benchmarked */ -// } -// verify { -// /* optional verification */ -// } - -// repay_stake { -// /* code to set the initial state */ -// }: { -// /* code to test the function benchmarked */ -// } -// verify { -// /* optional verification */ -// } - -// slash_stake { -// /* code to set the initial state */ -// }: { -// /* code to test the function benchmarked */ -// } -// verify { -// /* optional verification */ -// } - - impl_benchmark_test_suite!(SafeMode, crate::mock::new_test_ext(), crate::mock::Test); + } + + force_activate { + let force_origin = T::ForceActivateOrigin::successful_origin(); + let duration = T::ForceActivateOrigin::ensure_origin(force_origin.clone()).unwrap(); + let call = Call::::force_activate {}; + }: { call.dispatch_bypass_filter(force_origin)? } + verify { + assert_eq!( + SafeMode::::active_until().unwrap(), + System::::block_number() + + duration + ); + } + + extend { + let caller: T::AccountId = whitelisted_caller(); + let origin = RawOrigin::Signed(caller.clone()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + + assert!(SafeMode::::activate(origin.clone().into()).is_ok()); + }: _(origin) + verify { + assert_eq!( + SafeMode::::active_until().unwrap(), + System::::block_number() + T::SignedActivationDuration::get() + T::SignedExtendDuration::get() + ); + } + + force_extend { + let caller: T::AccountId = whitelisted_caller(); + let origin = RawOrigin::Signed(caller.clone()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + + assert!(SafeMode::::activate(origin.clone().into()).is_ok()); + + let force_origin = T::ForceExtendOrigin::successful_origin(); + let extension = T::ForceExtendOrigin::ensure_origin(force_origin.clone()).unwrap(); + let call = Call::::force_extend {}; + }: { call.dispatch_bypass_filter(force_origin)? } + verify { + assert_eq!( + SafeMode::::active_until().unwrap(), + System::::block_number() + T::SignedActivationDuration::get() + extension + ); + } + + force_deactivate { + let caller: T::AccountId = whitelisted_caller(); + let origin = RawOrigin::Signed(caller.clone()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + + assert!(SafeMode::::activate(origin.clone().into()).is_ok()); + + let force_origin = T::ForceDeactivateOrigin::successful_origin(); + let call = Call::::force_deactivate {}; + }: { call.dispatch_bypass_filter(force_origin)? } + verify { + assert_eq!( + SafeMode::::active_until(), + None + ); + } + + release_reservation { + let caller: T::AccountId = whitelisted_caller(); + let origin = RawOrigin::Signed(caller.clone()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + + let activated_at_block: T::BlockNumber = System::::block_number(); + assert!(SafeMode::::activate(origin.clone().into()).is_ok()); + let current_reservation = Reservations::::get(&caller, activated_at_block).unwrap_or_default(); + assert_eq!( + T::Currency::free_balance(&caller), + BalanceOf::::max_value() - T::ActivateReservationAmount::get().unwrap() + ); + + let force_origin = T::ForceDeactivateOrigin::successful_origin(); + assert!(SafeMode::::force_deactivate(force_origin.clone()).is_ok()); + + let repay_origin = T::RepayOrigin::successful_origin(); + let call = Call::::release_reservation { account: caller.clone(), block: activated_at_block.clone()}; + }: { call.dispatch_bypass_filter(repay_origin)? } + verify { + assert_eq!( + T::Currency::free_balance(&caller), + BalanceOf::::max_value() + ); + } + + slash_reservation { + let caller: T::AccountId = whitelisted_caller(); + let origin = RawOrigin::Signed(caller.clone()); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + + let activated_at_block: T::BlockNumber = System::::block_number(); + assert!(SafeMode::::activate(origin.clone().into()).is_ok()); + let current_reservation = Reservations::::get(&caller, activated_at_block).unwrap_or_default(); + assert_eq!( + T::Currency::free_balance(&caller), + BalanceOf::::max_value() - T::ActivateReservationAmount::get().unwrap() + ); + + let force_origin = T::ForceDeactivateOrigin::successful_origin(); + assert!(SafeMode::::force_deactivate(force_origin.clone()).is_ok()); + + let repay_origin = T::RepayOrigin::successful_origin(); + let call = Call::::slash_reservation { account: caller.clone(), block: activated_at_block.clone()}; + }: { call.dispatch_bypass_filter(repay_origin)? } + verify { + assert_eq!( + T::Currency::free_balance(&caller), + BalanceOf::::max_value() - T::ActivateReservationAmount::get().unwrap() + ); + } + + impl_benchmark_test_suite!(SafeMode, crate::mock::new_test_ext(), crate::mock::Test); } diff --git a/frame/safe-mode/src/lib.rs b/frame/safe-mode/src/lib.rs index db0ae1d2d5650..bcf05934aa34c 100644 --- a/frame/safe-mode/src/lib.rs +++ b/frame/safe-mode/src/lib.rs @@ -17,7 +17,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -#[cfg(feature = "runtime-benchmarks")] mod benchmarking; #[cfg(test)] pub mod mock; @@ -28,8 +27,8 @@ pub mod weights; use frame_support::{ pallet_prelude::*, traits::{ - CallMetadata, Contains, Currency, Defensive, GetCallMetadata, PalletInfoAccess, - ReservableCurrency, + CallMetadata, Contains, Currency, Defensive, GetCallMetadata, NamedReservableCurrency, + PalletInfoAccess, }, weights::Weight, }; @@ -56,50 +55,56 @@ pub mod pallet { /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; - /// Currency type for this pallet. - type Currency: ReservableCurrency; + /// Currency type for this pallet, used for reservations. + type Currency: NamedReservableCurrency< + Self::AccountId, + ReserveIdentifier = Self::BlockNumber, + >; - /// Contains all calls that can be dispatched even when the safe-mode is activated. + /// Contains all runtime calls in any pallet that can be dispatched even while the safe-mode + /// is activated. /// - /// The `SafeMode` pallet is always included and does not need to be added here. - type SafeModeFilter: Contains; + /// The safe-mode pallet cannot disable it's own calls, and does not need to be explicitly + /// added here. + type UnfilterableCalls: Contains; /// How long the safe-mode will stay active when activated with [`Pallet::activate`]. #[pallet::constant] - type ActivateDuration: Get; + type SignedActivationDuration: Get; /// For how many blocks the safe-mode can be extended by each [`Pallet::extend`] call. /// /// This does not impose a hard limit as the safe-mode can be extended multiple times. #[pallet::constant] - type ExtendDuration: Get; + type SignedExtendDuration: Get; /// The amount that will be reserved upon calling [`Pallet::activate`]. /// /// `None` disallows permissionlessly enabling the safe-mode. #[pallet::constant] - type ActivateStakeAmount: Get>>; + type ActivateReservationAmount: Get>>; /// The amount that will be reserved upon calling [`Pallet::extend`]. /// /// `None` disallows permissionlessly extending the safe-mode. #[pallet::constant] - type ExtendStakeAmount: Get>>; + type ExtendReservationAmount: Get>>; - /// The origin that can call [`Pallet::force_activate`]. + /// The origin that may call [`Pallet::force_activate`]. /// /// The `Success` value is the number of blocks that this origin can activate safe-mode for. type ForceActivateOrigin: EnsureOrigin; - /// The origin that can call [`Pallet::force_extend`]. + /// The origin that may call [`Pallet::force_extend`]. /// /// The `Success` value is the number of blocks that this origin can extend the safe-mode. type ForceExtendOrigin: EnsureOrigin; - /// The origin that can call [`Pallet::force_inactivate`]. - type ForceInactivateOrigin: EnsureOrigin; + /// The origin that may call [`Pallet::force_activate`]. + type ForceDeactivateOrigin: EnsureOrigin; - /// The origin that can call [`Pallet::repay_stake`] and [`Pallet::slash_stake`]. + /// The origin that may call [`Pallet::release_reservation`] and + /// [`Pallet::slash_reservation`]. type RepayOrigin: EnsureOrigin; // Weight information for extrinsics in this pallet. @@ -117,27 +122,28 @@ pub mod pallet { /// A value that is required for the extrinsic was not configured. NotConfigured, - /// There is no balance staked. - NotStaked, + /// There is no balance reserved. + NoReservation, } #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { /// The safe-mode was activated until inclusively this \[block\]. - Activated(T::BlockNumber), + Activated { block: T::BlockNumber }, /// The safe-mode was extended until inclusively this \[block\]. - Extended(T::BlockNumber), + Extended { block: T::BlockNumber }, /// Exited safe-mode for a specific \[reason\]. - Exited(ExitReason), + Exited { reason: ExitReason }, - /// An account got repaid its stake. \[account, amount\] - StakeRepaid(T::AccountId, BalanceOf), + /// An account had reserve repaid previously reserved at a block. \[block, account, + /// amount\] + ReservationRepaid { block: T::BlockNumber, account: T::AccountId, amount: BalanceOf }, - /// An account got slashed its stake. \[account, amount\] - StakeSlashed(T::AccountId, BalanceOf), + /// An account had reserve slashed previously reserved at a block. \[account, amount\] + ReservationSlashed { block: T::BlockNumber, account: T::AccountId, amount: BalanceOf }, } /// The reason why the safe-mode was exited. @@ -146,23 +152,23 @@ pub mod pallet { /// The safe-mode was automatically exited after its duration ran out. Timeout, - /// The safe-mode was forcefully exited by [`Pallet::force_inactivate`]. + /// The safe-mode was forcefully exited by [`Pallet::force_deactivate`]. Force, } /// Contains the last block number that the safe-mode will stay activated. /// /// This is set to `None` if the safe-mode is inactive. - /// The safe-mode is automatically inactivated when the current block number is greater than + /// The safe-mode is automatically deactivated when the current block number is greater than /// this. #[pallet::storage] #[pallet::getter(fn active_until)] pub type ActiveUntil = StorageValue<_, T::BlockNumber, OptionQuery>; - /// Holds the stake that was reserved from a user at a specific block number. + /// Holds the reserve that was reserved from a user at a specific block number. #[pallet::storage] - #[pallet::getter(fn stakes)] - pub type Stakes = StorageDoubleMap< + #[pallet::getter(fn reserves)] + pub type Reservations = StorageDoubleMap< _, Twox64Concat, T::AccountId, @@ -199,21 +205,23 @@ pub mod pallet { #[pallet::call] impl Pallet { - /// Activate safe-mode permissionlessly for [`Config::ActivateDuration`] blocks. + /// Activate safe-mode permissionlessly for [`Config::SignedActivationDuration`] blocks. /// - /// Reserves [`Config::ActivateStakeAmount`] from the caller's account. + /// Reserves [`Config::ActivateReservationAmount`] from the caller's account. /// Emits an [`Event::Activated`] event on success. /// Errors with [`Error::IsActive`] if the safe-mode is already activated. + /// Errors with [`Error::NotConfigured`] if the reservation amount is `None` . /// /// ### Safety /// - /// Can be permanently disabled by configuring [`Config::ActivateStakeAmount`] to `None`. + /// This may be called by any signed origin with [`Config::ActivateReservationAmount`] free + /// currency to reserve. This call can be disabled for all origins by configuring + /// [`Config::ActivateReservationAmount`] to `None`. #[pallet::weight(T::WeightInfo::activate())] - // #[pallet::weight(0)] pub fn activate(origin: OriginFor) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_activate(Some(who), T::ActivateDuration::get()) + Self::do_activate(Some(who), T::SignedActivationDuration::get()) } /// Activate safe-mode by force for a per-origin configured number of blocks. @@ -224,106 +232,119 @@ pub mod pallet { /// ### Safety /// /// Can only be called by the [`Config::ForceActivateOrigin`] origin. - #[pallet::weight(0)] + #[pallet::weight(T::WeightInfo::force_activate())] pub fn force_activate(origin: OriginFor) -> DispatchResult { let duration = T::ForceActivateOrigin::ensure_origin(origin)?; Self::do_activate(None, duration) } - /// Extend the safe-mode permissionlessly for [`Config::ExtendDuration`] blocks. + /// Extend the safe-mode permissionlessly for [`Config::SignedExtendDuration`] blocks. /// - /// Reserves [`Config::ExtendStakeAmount`] from the caller's account. + /// Reserves [`Config::ExtendReservationAmount`] from the caller's account. + /// Emits an [`Event::Extended`] event on success. /// Errors with [`Error::IsInactive`] if the safe-mode is active. + /// Errors with [`Error::NotConfigured`] if the reservation amount is `None` . /// /// ### Safety /// - /// Can be permanently disabled by configuring [`Config::ActivateStakeAmount`] to `None`. - #[pallet::weight(0)] + /// This may be called by any signed origin with [`Config::ExtendReservationAmount`] free + /// currency to reserve. This call can be disabled for all origins by configuring + /// [`Config::ExtendReservationAmount`] to `None`. + #[pallet::weight(T::WeightInfo::extend())] pub fn extend(origin: OriginFor) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_extend(Some(who), T::ExtendDuration::get()) + Self::do_extend(Some(who), T::SignedExtendDuration::get()) } /// Extend the safe-mode by force a per-origin configured number of blocks. /// + /// Emits an [`Event::Extended`] event on success. /// Errors with [`Error::IsInactive`] if the safe-mode is inactive. /// /// ### Safety /// /// Can only be called by the [`Config::ForceExtendOrigin`] origin. - #[pallet::weight(0)] + #[pallet::weight(T::WeightInfo::force_extend())] pub fn force_extend(origin: OriginFor) -> DispatchResult { let duration = T::ForceExtendOrigin::ensure_origin(origin)?; Self::do_extend(None, duration) } - /// Inactivate safe-mode by force. + /// Deactivate safe-mode by force. + /// + /// Emits an [`Event::Exited`] with [`ExitReason::Force`] event on success. + /// Errors with [`Error::IsInactive`] if the safe-mode is inactive. + /// + /// Note: `safe-mode` will be automatically deactivated by [`Pallet::on_initialize`] hook + /// after the block height is greater than [`ActiveUntil`] found in storage. + /// Emits an [`Event::Exited`] with [`ExitReason::Timeout`] event on hook. /// - /// Note: safe-mode will be automatically inactivated by [`Pallet::on_initialize`] hook - /// after the block height is greater than [`ActiveUntil`] found in storage. Errors with - /// [`Error::IsInactive`] if the safe-mode is inactive. /// /// ### Safety /// - /// Can only be called by the [`Config::ForceInactivateOrigin`] origin. - #[pallet::weight(0)] - pub fn force_inactivate(origin: OriginFor) -> DispatchResult { - T::ForceInactivateOrigin::ensure_origin(origin.clone())?; + /// Can only be called by the [`Config::ForceDeactivateOrigin`] origin. + #[pallet::weight(T::WeightInfo::force_deactivate())] + pub fn force_deactivate(origin: OriginFor) -> DispatchResult { + T::ForceDeactivateOrigin::ensure_origin(origin)?; - Self::do_inactivate(ExitReason::Force) + Self::do_deactivate(ExitReason::Force) } - /// Repay an honest account that activated safe-mode earlier. + /// Release a currency reservation for an account that activated safe-mode at a specific + /// block earlier. This cannot be called while safe-mode is active. /// - /// Emits a [`Event::StakeRepaid`] event on success. - /// Errors if the safe-mode is already activated. + /// Emits a [`Event::ReservationRepaid`] event on success. + /// Errors with [`Error::IsActive`] if the safe-mode presently activated. + /// Errors with [`Error::NoReservation`] if the payee has no named reserved currency at the + /// block specified. /// /// ### Safety /// /// Can only be called by the [`Config::RepayOrigin`] origin. - #[pallet::weight(0)] - pub fn repay_stake( + #[pallet::weight(T::WeightInfo::release_reservation())] + pub fn release_reservation( origin: OriginFor, account: T::AccountId, block: T::BlockNumber, ) -> DispatchResult { - T::RepayOrigin::ensure_origin(origin.clone())?; + T::RepayOrigin::ensure_origin(origin)?; - Self::do_repay_stake(account, block) + Self::do_release_reservation(account, block) } - /// Slash a dishonest account that put the chain into safe-mode earlier. + /// Slash a reservation for an account that activated or extended safe-mode at a specific + /// block earlier. This cannot be called while safe-mode is active. /// - /// Errors if the safe-mode is already activated. - /// Emits a [`Event::StakeSlashed`] event on success. + /// Emits a [`Event::ReservationSlashed`] event on success. + /// Errors with [`Error::IsActive`] if the safe-mode presently activated. /// /// ### Safety /// /// Can only be called by the [`Config::RepayOrigin`] origin. - #[pallet::weight(0)] - pub fn slash_stake( + #[pallet::weight(T::WeightInfo::slash_reservation())] + pub fn slash_reservation( origin: OriginFor, account: T::AccountId, block: T::BlockNumber, ) -> DispatchResult { - T::RepayOrigin::ensure_origin(origin.clone())?; + T::RepayOrigin::ensure_origin(origin)?; - Self::do_slash_stake(account, block) + Self::do_slash_reservation(account, block) } } #[pallet::hooks] impl Hooks> for Pallet { - /// Automatically inactivates the safe-mode when the period ran out. + /// Automatically deactivates the safe-mode when the period runs out. /// /// Bypasses any call filters to avoid getting rejected by them. fn on_initialize(current: T::BlockNumber) -> Weight { match ActiveUntil::::get() { Some(limit) if current > limit => { - let _ = Self::do_inactivate(ExitReason::Timeout) + let _ = Self::do_deactivate(ExitReason::Timeout) .defensive_proof("Must be inactive; qed"); T::DbWeight::get().reads_writes(1, 1) }, @@ -337,73 +358,77 @@ impl Pallet { /// Logic for the [`crate::Pallet::activate`] and [`crate::Pallet::force_activate`] calls. fn do_activate(who: Option, duration: T::BlockNumber) -> DispatchResult { if let Some(who) = who { - let stake = T::ActivateStakeAmount::get().ok_or(Error::::NotConfigured)?; - Self::reserve(who, stake)?; + let reserve = T::ActivateReservationAmount::get().ok_or(Error::::NotConfigured)?; + Self::reserve(who, reserve)?; } ensure!(!ActiveUntil::::exists(), Error::::IsActive); let limit = >::block_number().saturating_add(duration); ActiveUntil::::put(limit); - Self::deposit_event(Event::Activated(limit)); + Self::deposit_event(Event::Activated { block: limit }); Ok(()) } /// Logic for the [`crate::Pallet::extend`] and [`crate::Pallet::force_extend`] calls. fn do_extend(who: Option, duration: T::BlockNumber) -> DispatchResult { if let Some(who) = who { - let stake = T::ExtendStakeAmount::get().ok_or(Error::::NotConfigured)?; - Self::reserve(who, stake)?; + let reserve = T::ExtendReservationAmount::get().ok_or(Error::::NotConfigured)?; + Self::reserve(who, reserve)?; } let limit = ActiveUntil::::take().ok_or(Error::::IsInactive)?.saturating_add(duration); ActiveUntil::::put(limit); - Self::deposit_event(Event::::Extended(limit)); + Self::deposit_event(Event::::Extended { block: limit }); Ok(()) } - /// Logic for the [`crate::Pallet::force_inactivate`] call. + /// Logic for the [`crate::Pallet::force_deactivate`] call. /// /// Errors if the safe-mode is already inactive. /// Does not check the origin. - fn do_inactivate(reason: ExitReason) -> DispatchResult { + fn do_deactivate(reason: ExitReason) -> DispatchResult { let _limit = ActiveUntil::::take().ok_or(Error::::IsInactive)?; - Self::deposit_event(Event::Exited(reason)); + Self::deposit_event(Event::Exited { reason }); Ok(()) } - /// Logic for the [`crate::Pallet::repay_stake`] call. + /// Logic for the [`crate::Pallet::release_reservation`] call. /// /// Errors if the safe-mode is active. /// Does not check the origin. - fn do_repay_stake(account: T::AccountId, block: T::BlockNumber) -> DispatchResult { + fn do_release_reservation(account: T::AccountId, block: T::BlockNumber) -> DispatchResult { ensure!(!Self::is_activated(), Error::::IsActive); - let stake = Stakes::::take(&account, block).ok_or(Error::::NotStaked)?; + let reserve = Reservations::::take(&account, block).ok_or(Error::::NoReservation)?; - T::Currency::unreserve(&account, stake); - Self::deposit_event(Event::::StakeRepaid(account, stake)); + T::Currency::unreserve_named(&block, &account, reserve); + Self::deposit_event(Event::::ReservationRepaid { block, account, amount: reserve }); Ok(()) } - /// Logic for the [`crate::Pallet::slash_stake`] call. + /// Logic for the [`crate::Pallet::slash_reservation`] call. /// /// Errors if the safe-mode is activated. /// Does not check the origin. - fn do_slash_stake(account: T::AccountId, block: T::BlockNumber) -> DispatchResult { + fn do_slash_reservation(account: T::AccountId, block: T::BlockNumber) -> DispatchResult { ensure!(!Self::is_activated(), Error::::IsActive); - let stake = Stakes::::take(&account, block).ok_or(Error::::NotStaked)?; + let reserve = Reservations::::take(&account, block).ok_or(Error::::NoReservation)?; - T::Currency::slash_reserved(&account, stake); - Self::deposit_event(Event::::StakeSlashed(account, stake)); + T::Currency::slash_reserved_named(&block, &account, reserve); + Self::deposit_event(Event::::ReservationSlashed { block, account, amount: reserve }); Ok(()) } - /// Reserve `stake` amount from `who` and store it in `Stakes`. - fn reserve(who: T::AccountId, stake: BalanceOf) -> DispatchResult { - T::Currency::reserve(&who, stake)?; + /// Reserve `reserve` amount from `who` and store it in `Reservations`. + fn reserve(who: T::AccountId, reserve: BalanceOf) -> DispatchResult { let block = >::block_number(); - let current_stake = Stakes::::get(&who, block).unwrap_or_default(); - Stakes::::insert(&who, block, current_stake.saturating_add(stake)); + T::Currency::reserve_named(&block, &who, reserve)?; + let current_reservation = Reservations::::get(&who, block).unwrap_or_default(); + // Reservation is mapped to the block that an extrinsic calls activate or extend, + // therefore we prevent abuse in a block by adding to present value in the same block. TODO: + // should we? Why not just fail? Calls in other blocks to activate or extend are stored in a + // new Reservations item. + Reservations::::insert(&who, block, current_reservation.saturating_add(reserve)); Ok(()) } @@ -424,7 +449,7 @@ impl Pallet { } if Self::is_activated() { - T::SafeModeFilter::contains(call) + T::UnfilterableCalls::contains(call) } else { true } diff --git a/frame/safe-mode/src/mock.rs b/frame/safe-mode/src/mock.rs index 35066c9f250aa..8d3c823e44fb1 100644 --- a/frame/safe-mode/src/mock.rs +++ b/frame/safe-mode/src/mock.rs @@ -22,7 +22,7 @@ use crate as pallet_safe_mode; use frame_support::{ parameter_types, - traits::{Everything, InsideBoth, SortedMembers}, + traits::{ConstU64, Everything, InsideBoth, InstanceFilter, SortedMembers}, }; use frame_system::{EnsureSignedBy, RawOrigin}; use sp_core::H256; @@ -63,7 +63,7 @@ impl frame_system::Config for Test { parameter_types! { pub const ExistentialDeposit: u64 = 1; - pub const MaxLocks: u32 = 10; + pub const MaxReserves: u32 = 10; } impl pallet_balances::Config for Test { type Balance = u64; @@ -72,18 +72,77 @@ impl pallet_balances::Config for Test { type ExistentialDeposit = ExistentialDeposit; type AccountStore = System; type WeightInfo = (); - type MaxLocks = MaxLocks; - type MaxReserves = (); - type ReserveIdentifier = [u8; 8]; + type MaxLocks = (); + type MaxReserves = MaxReserves; + type ReserveIdentifier = Self::BlockNumber; } -/// Filter to block balance pallet calls -pub struct MockSafeModeFilter; -impl Contains for MockSafeModeFilter { +impl pallet_utility::Config for Test { + type RuntimeEvent = RuntimeEvent; + type RuntimeCall = RuntimeCall; + type PalletsOrigin = OriginCaller; + type WeightInfo = (); +} + +#[derive( + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Encode, + Decode, + RuntimeDebug, + MaxEncodedLen, + scale_info::TypeInfo, +)] +pub enum ProxyType { + Any, + JustTransfer, + JustUtility, +} +impl Default for ProxyType { + fn default() -> Self { + Self::Any + } +} +impl InstanceFilter for ProxyType { + fn filter(&self, c: &RuntimeCall) -> bool { + match self { + ProxyType::Any => true, + ProxyType::JustTransfer => { + matches!(c, RuntimeCall::Balances(pallet_balances::Call::transfer { .. })) + }, + ProxyType::JustUtility => matches!(c, RuntimeCall::Utility { .. }), + } + } + fn is_superset(&self, o: &Self) -> bool { + self == &ProxyType::Any || self == o + } +} +impl pallet_proxy::Config for Test { + type RuntimeEvent = RuntimeEvent; + type RuntimeCall = RuntimeCall; + type Currency = Balances; + type ProxyType = ProxyType; + type ProxyDepositBase = ConstU64<1>; + type ProxyDepositFactor = ConstU64<1>; + type MaxProxies = ConstU32<4>; + type WeightInfo = (); + type CallHasher = BlakeTwo256; + type MaxPending = ConstU32<2>; + type AnnouncementDepositBase = ConstU64<1>; + type AnnouncementDepositFactor = ConstU64<1>; +} + +/// Filter to allow all everything except balance calls +pub struct UnfilterableCalls; +impl Contains for UnfilterableCalls { fn contains(call: &RuntimeCall) -> bool { match call { - RuntimeCall::System(_) | RuntimeCall::SafeMode(_) => true, RuntimeCall::Balances(_) => false, + _ => true, } } } @@ -152,7 +211,7 @@ impl ForceExtendOrigin { } } -impl, O>> + From> + std::fmt::Debug> EnsureOrigin +impl, O>> + From>> EnsureOrigin for ForceActivateOrigin { type Success = u64; @@ -168,9 +227,14 @@ impl, O>> + From> + std::fmt::Debug r => Err(O::from(r)), }) } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> O { + O::from(RawOrigin::Signed(ForceActivateOrigin::Strong.acc())) + } } -impl, O>> + From> + std::fmt::Debug> EnsureOrigin +impl, O>> + From>> EnsureOrigin for ForceExtendOrigin { type Success = u64; @@ -186,19 +250,24 @@ impl, O>> + From> + std::fmt::Debug r => Err(O::from(r)), }) } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> O { + O::from(RawOrigin::Signed(ForceExtendOrigin::Strong.acc())) + } } parameter_types! { - pub const ActivateDuration: u64 = 3; - pub const ExtendDuration: u64 = 30; - pub const ActivateStakeAmount: u64 = 100; - pub const ExtendStakeAmount: u64 = 100; - pub const DeactivateOrigin: u64 =3; + pub const SignedActivationDuration: u64 = 3; + pub const SignedExtendDuration: u64 = 30; + pub const ActivateReservationAmount: u64 = 100; + pub const ExtendReservationAmount: u64 = 100; + pub const ForceDeactivateOrigin: u64 = 3; pub const RepayOrigin: u64 = 4; } // Required impl to use some ::get() in tests -impl SortedMembers for DeactivateOrigin { +impl SortedMembers for ForceDeactivateOrigin { fn sorted_members() -> Vec { vec![Self::get()] } @@ -216,14 +285,14 @@ impl SortedMembers for RepayOrigin { impl Config for Test { type RuntimeEvent = RuntimeEvent; type Currency = Balances; - type SafeModeFilter = MockSafeModeFilter; - type ActivateDuration = ActivateDuration; - type ExtendDuration = ExtendDuration; - type ActivateStakeAmount = ActivateStakeAmount; + type UnfilterableCalls = UnfilterableCalls; + type SignedActivationDuration = SignedActivationDuration; + type ActivateReservationAmount = ActivateReservationAmount; + type SignedExtendDuration = SignedExtendDuration; + type ExtendReservationAmount = ExtendReservationAmount; type ForceActivateOrigin = ForceActivateOrigin; type ForceExtendOrigin = ForceExtendOrigin; - type ExtendStakeAmount = ExtendStakeAmount; - type ForceInactivateOrigin = EnsureSignedBy; + type ForceDeactivateOrigin = EnsureSignedBy; type RepayOrigin = EnsureSignedBy; type WeightInfo = (); } @@ -239,6 +308,8 @@ frame_support::construct_runtime!( { System: frame_system, Balances: pallet_balances, + Utility: pallet_utility, + Proxy: pallet_proxy, SafeMode: pallet_safe_mode, } ); @@ -266,11 +337,6 @@ pub fn new_test_ext() -> sp_io::TestExternalities { ext } -#[cfg(feature = "runtime-benchmarks")] -pub fn new_bench_ext() -> sp_io::TestExternalities { - GenesisConfig::default().build_storage().unwrap().into() -} - pub fn next_block() { SafeMode::on_finalize(System::block_number()); Balances::on_finalize(System::block_number()); diff --git a/frame/safe-mode/src/tests.rs b/frame/safe-mode/src/tests.rs index 2e75f40e0f2c3..54771712a994c 100644 --- a/frame/safe-mode/src/tests.rs +++ b/frame/safe-mode/src/tests.rs @@ -29,10 +29,9 @@ fn fails_to_filter_calls_to_safe_mode_pallet() { new_test_ext().execute_with(|| { assert_ok!(SafeMode::activate(Origin::signed(0))); let activated_at_block = System::block_number(); - let call = RuntimeCall::Balances(pallet_balances::Call::transfer { dest: 1, value: 1 }); assert_err!( - call.clone().dispatch(Origin::signed(0)), + call_transfer().dispatch(Origin::signed(0)), frame_system::Error::::CallFiltered ); // TODO ^^^ consider refactor to throw a safe mode error, not generic `CallFiltered` @@ -41,11 +40,11 @@ fn fails_to_filter_calls_to_safe_mode_pallet() { assert_ok!(SafeMode::extend(Origin::signed(0))); assert_ok!(SafeMode::force_extend(ForceExtendOrigin::Weak.signed())); assert_err!( - call.clone().dispatch(Origin::signed(0)), + call_transfer().dispatch(Origin::signed(0)), frame_system::Error::::CallFiltered ); - assert_ok!(SafeMode::force_inactivate(Origin::signed(mock::DeactivateOrigin::get()))); - assert_ok!(SafeMode::repay_stake( + assert_ok!(SafeMode::force_deactivate(Origin::signed(mock::ForceDeactivateOrigin::get()))); + assert_ok!(SafeMode::release_reservation( Origin::signed(mock::RepayOrigin::get()), 0, activated_at_block @@ -54,11 +53,11 @@ fn fails_to_filter_calls_to_safe_mode_pallet() { next_block(); assert_ok!(SafeMode::activate(Origin::signed(0))); assert_err!( - call.clone().dispatch(Origin::signed(0)), + call_transfer().dispatch(Origin::signed(0)), frame_system::Error::::CallFiltered ); - assert_ok!(SafeMode::force_inactivate(Origin::signed(mock::DeactivateOrigin::get()))); - assert_ok!(SafeMode::slash_stake( + assert_ok!(SafeMode::force_deactivate(Origin::signed(mock::ForceDeactivateOrigin::get()))); + assert_ok!(SafeMode::slash_reservation( Origin::signed(mock::RepayOrigin::get()), 0, activated_at_block + 2 @@ -66,6 +65,14 @@ fn fails_to_filter_calls_to_safe_mode_pallet() { }); } +#[test] +fn fails_to_activate_if_activated() { + new_test_ext().execute_with(|| { + assert_ok!(SafeMode::activate(Origin::signed(0))); + assert_noop!(SafeMode::activate(Origin::signed(2)), Error::::IsActive); + }); +} + #[test] fn fails_to_extend_if_not_activated() { new_test_ext().execute_with(|| { @@ -74,15 +81,42 @@ fn fails_to_extend_if_not_activated() { }); } +#[test] +fn fails_to_release_reservations_with_wrong_block() { + new_test_ext().execute_with(|| { + let activated_at_block = System::block_number(); + assert_ok!(SafeMode::activate(Origin::signed(0))); + run_to(mock::SignedActivationDuration::get() + activated_at_block + 1); + + assert_err!( + SafeMode::release_reservation( + Origin::signed(mock::RepayOrigin::get()), + 0, + activated_at_block + 1 + ), + Error::::NoReservation + ); + + assert_err!( + SafeMode::slash_reservation( + Origin::signed(mock::RepayOrigin::get()), + 0, + activated_at_block + 1 + ), + Error::::NoReservation + ); + }); +} + // GENERAL SUCCESS/POSITIVE TESTS --------------------- #[test] -fn can_automatically_inactivate_after_timeout() { +fn can_automatically_deactivate_after_timeout() { new_test_ext().execute_with(|| { let activated_at_block = System::block_number(); assert_ok!(SafeMode::force_activate(ForceActivateOrigin::Weak.signed())); run_to(ForceActivateOrigin::Weak.duration() + activated_at_block + 1); - SafeMode::on_initialize(System::block_number()); + assert_eq!(SafeMode::active_until(), None); }); } @@ -90,18 +124,84 @@ fn can_automatically_inactivate_after_timeout() { #[test] fn can_filter_balance_calls_when_activated() { new_test_ext().execute_with(|| { - let call = RuntimeCall::Balances(pallet_balances::Call::transfer { dest: 1, value: 1 }); - - assert_ok!(call.clone().dispatch(Origin::signed(0))); + assert_ok!(call_transfer().dispatch(Origin::signed(0))); assert_ok!(SafeMode::activate(Origin::signed(0))); assert_err!( - call.clone().dispatch(Origin::signed(0)), + call_transfer().dispatch(Origin::signed(0)), frame_system::Error::::CallFiltered ); // TODO ^^^ consider refactor to throw a safe mode error, not generic `CallFiltered` }); } +#[test] +fn can_filter_balance_in_batch_when_activated() { + new_test_ext().execute_with(|| { + let batch_call = + RuntimeCall::Utility(pallet_utility::Call::batch { calls: vec![call_transfer()] }); + + assert_ok!(batch_call.clone().dispatch(Origin::signed(0))); + + assert_ok!(SafeMode::activate(Origin::signed(0))); + + assert_ok!(batch_call.clone().dispatch(Origin::signed(0))); + System::assert_last_event( + pallet_utility::Event::BatchInterrupted { + index: 0, + error: frame_system::Error::::CallFiltered.into(), + } + .into(), + ); + }); +} + +#[test] +fn can_filter_balance_in_proxy_when_activated() { + new_test_ext().execute_with(|| { + assert_ok!(Proxy::add_proxy(Origin::signed(1), 2, ProxyType::JustTransfer, 0)); + + assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, Box::new(call_transfer()))); + System::assert_last_event(pallet_proxy::Event::ProxyExecuted { result: Ok(()) }.into()); + + assert_ok!(SafeMode::force_activate(ForceActivateOrigin::Weak.signed())); + + assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, Box::new(call_transfer()))); + System::assert_last_event( + pallet_proxy::Event::ProxyExecuted { + result: DispatchError::from(frame_system::Error::::CallFiltered).into(), + } + .into(), + ); + }); +} + +#[test] +fn can_repay_independent_reservations_by_block() { + new_test_ext().execute_with(|| { + let activated_at_block_0 = System::block_number(); + assert_ok!(SafeMode::activate(Origin::signed(0))); + run_to(mock::SignedActivationDuration::get() + activated_at_block_0 + 1); + + let activated_at_block_1 = System::block_number(); + assert_ok!(SafeMode::activate(Origin::signed(0))); + run_to(mock::SignedActivationDuration::get() + activated_at_block_1 + 1); + + assert_ok!(SafeMode::release_reservation( + Origin::signed(mock::RepayOrigin::get()), + 0, + activated_at_block_0 + )); + assert_eq!(Balances::free_balance(&0), 1234 - mock::ActivateReservationAmount::get()); // accounts set in mock genesis + + assert_ok!(SafeMode::slash_reservation( + Origin::signed(mock::RepayOrigin::get()), + 0, + activated_at_block_1 + )); + assert_eq!(Balances::total_balance(&0), 1234 - mock::ActivateReservationAmount::get()); // accounts set in mock genesis + }); +} + // SIGNED ORIGIN CALL TESTS --------------------- #[test] @@ -110,12 +210,12 @@ fn can_activate_with_signed_origin() { assert_ok!(SafeMode::activate(Origin::signed(0))); assert_eq!( SafeMode::active_until().unwrap(), - System::block_number() + mock::ActivateDuration::get() + System::block_number() + mock::SignedActivationDuration::get() ); - assert_eq!(Balances::reserved_balance(0), mock::ActivateStakeAmount::get()); + assert_eq!(Balances::reserved_balance(0), mock::ActivateReservationAmount::get()); assert_noop!(SafeMode::activate(Origin::signed(0)), Error::::IsActive); // Assert the stake. - assert_eq!(Stakes::::get(0, 1), Some(mock::ActivateStakeAmount::get())); + assert_eq!(Reservations::::get(0, 1), Some(mock::ActivateReservationAmount::get())); }); } @@ -126,11 +226,13 @@ fn can_extend_with_signed_origin() { assert_ok!(SafeMode::extend(Origin::signed(0))); assert_eq!( SafeMode::active_until().unwrap(), - System::block_number() + mock::ActivateDuration::get() + mock::ExtendDuration::get() + System::block_number() + + mock::SignedActivationDuration::get() + + mock::SignedExtendDuration::get() ); assert_eq!( Balances::reserved_balance(0), - mock::ActivateStakeAmount::get() + mock::ExtendStakeAmount::get() + mock::ActivateReservationAmount::get() + mock::ExtendReservationAmount::get() ); }); } @@ -143,13 +245,13 @@ fn fails_signed_origin_when_explicit_origin_required() { assert_err!(SafeMode::force_activate(Origin::signed(0)), DispatchError::BadOrigin); assert_err!(SafeMode::force_extend(Origin::signed(0)), DispatchError::BadOrigin); - assert_err!(SafeMode::force_inactivate(Origin::signed(0)), DispatchError::BadOrigin); + assert_err!(SafeMode::force_deactivate(Origin::signed(0)), DispatchError::BadOrigin); assert_err!( - SafeMode::slash_stake(Origin::signed(0), 0, activated_at_block), + SafeMode::slash_reservation(Origin::signed(0), 0, activated_at_block), DispatchError::BadOrigin ); assert_err!( - SafeMode::repay_stake(Origin::signed(0), 0, activated_at_block), + SafeMode::release_reservation(Origin::signed(0), 0, activated_at_block), DispatchError::BadOrigin ); }); @@ -158,14 +260,14 @@ fn fails_signed_origin_when_explicit_origin_required() { // CONFIGURED ORIGIN CALL TESTS --------------------- #[test] -fn fails_force_inactivate_if_not_activated() { +fn fails_force_deactivate_if_not_activated() { new_test_ext().execute_with(|| { assert_noop!( - SafeMode::force_inactivate(Origin::signed(mock::DeactivateOrigin::get())), + SafeMode::force_deactivate(Origin::signed(mock::ForceDeactivateOrigin::get())), Error::::IsInactive ); assert_noop!( - SafeMode::force_inactivate(Origin::signed(mock::DeactivateOrigin::get())), + SafeMode::force_deactivate(Origin::signed(mock::ForceDeactivateOrigin::get())), Error::::IsInactive ); }); @@ -188,16 +290,16 @@ fn can_force_activate_with_config_origin() { } #[test] -fn can_force_inactivate_with_config_origin() { +fn can_force_deactivate_with_config_origin() { new_test_ext().execute_with(|| { assert_eq!(SafeMode::active_until(), None); assert_err!( - SafeMode::force_inactivate(Origin::signed(mock::DeactivateOrigin::get())), + SafeMode::force_deactivate(Origin::signed(mock::ForceDeactivateOrigin::get())), Error::::IsInactive ); assert_ok!(SafeMode::force_activate(ForceActivateOrigin::Weak.signed())); assert_eq!(Balances::reserved_balance(ForceActivateOrigin::Weak.acc()), 0); - assert_ok!(SafeMode::force_inactivate(Origin::signed(mock::DeactivateOrigin::get()))); + assert_ok!(SafeMode::force_deactivate(Origin::signed(mock::ForceDeactivateOrigin::get()))); }); } @@ -218,47 +320,94 @@ fn can_force_extend_with_config_origin() { ForceExtendOrigin::Medium.duration() ); assert_eq!(Balances::reserved_balance(ForceActivateOrigin::Weak.acc()), 0); - assert_eq!(Balances::reserved_balance(mock::ExtendDuration::get()), 0); + assert_eq!(Balances::reserved_balance(mock::SignedExtendDuration::get()), 0); }); } #[test] -fn can_repay_stake_with_config_origin() { +fn can_release_reservation_with_config_origin() { new_test_ext().execute_with(|| { let activated_at_block = System::block_number(); assert_ok!(SafeMode::activate(Origin::signed(0))); assert_err!( - SafeMode::repay_stake(Origin::signed(mock::RepayOrigin::get()), 0, activated_at_block), + SafeMode::release_reservation( + Origin::signed(mock::RepayOrigin::get()), + 0, + activated_at_block + ), Error::::IsActive ); - run_to(mock::ActivateDuration::get() + activated_at_block + 1); - SafeMode::on_initialize(System::block_number()); - assert_ok!(SafeMode::repay_stake( + run_to(mock::SignedActivationDuration::get() + activated_at_block + 1); + + assert_ok!(SafeMode::release_reservation( Origin::signed(mock::RepayOrigin::get()), 0, activated_at_block )); - // TODO: test accounting is correct + assert_eq!(Balances::free_balance(&0), 1234); // accounts set in mock genesis + + Balances::make_free_balance_be(&0, 1234); + let activated_and_extended_at_block = System::block_number(); + assert_ok!(SafeMode::activate(Origin::signed(0))); + assert_ok!(SafeMode::extend(Origin::signed(0))); + run_to( + mock::SignedActivationDuration::get() + + mock::SignedExtendDuration::get() + + activated_and_extended_at_block + + 1, + ); + + assert_ok!(SafeMode::release_reservation( + Origin::signed(mock::RepayOrigin::get()), + 0, + activated_and_extended_at_block + )); + assert_eq!(Balances::free_balance(&0), 1234); // accounts set in mock genesis }); } #[test] -fn can_slash_stake_with_config_origin() { +fn can_slash_reservation_with_config_origin() { new_test_ext().execute_with(|| { let activated_at_block = System::block_number(); assert_ok!(SafeMode::activate(Origin::signed(0))); assert_err!( - SafeMode::slash_stake(Origin::signed(mock::RepayOrigin::get()), 0, activated_at_block), + SafeMode::slash_reservation( + Origin::signed(mock::RepayOrigin::get()), + 0, + activated_at_block + ), Error::::IsActive ); - run_to(mock::ActivateDuration::get() + activated_at_block + 1); - SafeMode::on_initialize(System::block_number()); - assert_ok!(SafeMode::slash_stake( + run_to(mock::SignedActivationDuration::get() + activated_at_block + 1); + + assert_ok!(SafeMode::slash_reservation( Origin::signed(mock::RepayOrigin::get()), 0, activated_at_block )); - // TODO: test accounting is correct + assert_eq!(Balances::free_balance(&0), 1234 - mock::ActivateReservationAmount::get()); // accounts set in mock genesis + + Balances::make_free_balance_be(&0, 1234); + let activated_and_extended_at_block = System::block_number(); + assert_ok!(SafeMode::activate(Origin::signed(0))); + assert_ok!(SafeMode::extend(Origin::signed(0))); + run_to( + mock::SignedActivationDuration::get() + + mock::SignedExtendDuration::get() + + activated_and_extended_at_block + + 1, + ); + + assert_ok!(SafeMode::slash_reservation( + Origin::signed(mock::RepayOrigin::get()), + 0, + activated_and_extended_at_block + )); + assert_eq!( + Balances::free_balance(&0), + 1234 - mock::ActivateReservationAmount::get() - mock::ExtendReservationAmount::get() + ); // accounts set in mock genesis }); } @@ -273,15 +422,19 @@ fn fails_when_explicit_origin_required() { DispatchError::BadOrigin ); assert_err!( - SafeMode::force_inactivate(ForceActivateOrigin::Weak.signed()), + SafeMode::force_deactivate(ForceActivateOrigin::Weak.signed()), DispatchError::BadOrigin ); assert_err!( - SafeMode::slash_stake(ForceActivateOrigin::Weak.signed(), 0, activated_at_block), + SafeMode::slash_reservation(ForceActivateOrigin::Weak.signed(), 0, activated_at_block), DispatchError::BadOrigin ); assert_err!( - SafeMode::repay_stake(ForceActivateOrigin::Weak.signed(), 0, activated_at_block), + SafeMode::release_reservation( + ForceActivateOrigin::Weak.signed(), + 0, + activated_at_block + ), DispatchError::BadOrigin ); @@ -290,37 +443,37 @@ fn fails_when_explicit_origin_required() { DispatchError::BadOrigin ); assert_err!( - SafeMode::force_inactivate(ForceExtendOrigin::Weak.signed()), + SafeMode::force_deactivate(ForceExtendOrigin::Weak.signed()), DispatchError::BadOrigin ); assert_err!( - SafeMode::slash_stake(ForceExtendOrigin::Weak.signed(), 0, activated_at_block), + SafeMode::slash_reservation(ForceExtendOrigin::Weak.signed(), 0, activated_at_block), DispatchError::BadOrigin ); assert_err!( - SafeMode::repay_stake(ForceExtendOrigin::Weak.signed(), 0, activated_at_block), + SafeMode::release_reservation(ForceExtendOrigin::Weak.signed(), 0, activated_at_block), DispatchError::BadOrigin ); assert_err!( - SafeMode::force_activate(Origin::signed(mock::DeactivateOrigin::get())), + SafeMode::force_activate(Origin::signed(mock::ForceDeactivateOrigin::get())), DispatchError::BadOrigin ); assert_err!( - SafeMode::force_extend(Origin::signed(mock::DeactivateOrigin::get())), + SafeMode::force_extend(Origin::signed(mock::ForceDeactivateOrigin::get())), DispatchError::BadOrigin ); assert_err!( - SafeMode::slash_stake( - Origin::signed(mock::DeactivateOrigin::get()), + SafeMode::slash_reservation( + Origin::signed(mock::ForceDeactivateOrigin::get()), 0, activated_at_block ), DispatchError::BadOrigin ); assert_err!( - SafeMode::repay_stake( - Origin::signed(mock::DeactivateOrigin::get()), + SafeMode::release_reservation( + Origin::signed(mock::ForceDeactivateOrigin::get()), 0, activated_at_block ), @@ -336,8 +489,12 @@ fn fails_when_explicit_origin_required() { DispatchError::BadOrigin ); assert_err!( - SafeMode::force_inactivate(Origin::signed(mock::RepayOrigin::get())), + SafeMode::force_deactivate(Origin::signed(mock::RepayOrigin::get())), DispatchError::BadOrigin ); }); } + +fn call_transfer() -> RuntimeCall { + RuntimeCall::Balances(pallet_balances::Call::transfer { dest: 1, value: 1 }) +} diff --git a/frame/safe-mode/src/weights.rs b/frame/safe-mode/src/weights.rs index dc1b64d905c6f..d24f31f944d3c 100644 --- a/frame/safe-mode/src/weights.rs +++ b/frame/safe-mode/src/weights.rs @@ -56,27 +56,129 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_safe_mode. pub trait WeightInfo { fn activate() -> Weight; + fn force_activate() -> Weight; + fn extend() -> Weight; + fn force_extend() -> Weight; + fn force_deactivate() -> Weight; + fn release_reservation() -> Weight; + fn slash_reservation() -> Weight; } /// Weights for pallet_safe_mode using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - // Storage: SafeMode Stakes (r:1 w:1) + // Storage: SafeMode Reservations (r:1 w:1) // Storage: SafeMode Enabled (r:1 w:1) fn activate() -> Weight { Weight::from_ref_time(51_688_000 as u64) .saturating_add(T::DbWeight::get().reads(2 as u64)) .saturating_add(T::DbWeight::get().writes(2 as u64)) } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn force_activate() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn extend() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn force_extend() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn force_deactivate() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn release_reservation() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn slash_reservation() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(T::DbWeight::get().reads(2 as u64)) + .saturating_add(T::DbWeight::get().writes(2 as u64)) + } } // For backwards compatibility and tests impl WeightInfo for () { - // Storage: SafeMode Stakes (r:1 w:1) + // Storage: SafeMode Reservations (r:1 w:1) // Storage: SafeMode Enabled (r:1 w:1) fn activate() -> Weight { Weight::from_ref_time(51_688_000 as u64) .saturating_add(RocksDbWeight::get().reads(2 as u64)) .saturating_add(RocksDbWeight::get().writes(2 as u64)) } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn force_activate() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn extend() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn force_extend() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn force_deactivate() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn release_reservation() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(2 as u64)) + } + + // Storage: SafeMode Reservations (r:1 w:1) + // Storage: SafeMode Enabled (r:1 w:1) + fn slash_reservation() -> Weight { + Weight::from_ref_time(51_688_000 as u64) + .saturating_add(RocksDbWeight::get().reads(2 as u64)) + .saturating_add(RocksDbWeight::get().writes(2 as u64)) + } } diff --git a/frame/tx-pause/Cargo.toml b/frame/tx-pause/Cargo.toml index da32e18ef387c..0fe99c20353b4 100644 --- a/frame/tx-pause/Cargo.toml +++ b/frame/tx-pause/Cargo.toml @@ -20,13 +20,17 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys scale-info = { version = "2.0.1", default-features = false, features = ["derive"] } sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } - +pallet-balances = { version = "4.0.0-dev", path = "../balances", default-features = false, optional = true } +pallet-utility = { version = "4.0.0-dev", path = "../utility", default-features = false, optional = true } +pallet-proxy = { version = "4.0.0-dev", path = "../proxy", default-features = false, optional = true } [dev-dependencies] sp-core = { version = "6.0.0", path = "../../primitives/core" } sp-std = { version = "4.0.0", path = "../../primitives/std" } sp-io = { version = "6.0.0", path = "../../primitives/io" } pallet-balances = { version = "4.0.0-dev", path = "../balances" } +pallet-utility = { version = "4.0.0-dev", path = "../utility" } +pallet-proxy = { version = "4.0.0-dev", path = "../proxy" } [features] @@ -37,6 +41,9 @@ std = [ "frame-benchmarking/std", "frame-support/std", "frame-system/std", + "pallet-balances?/std", + "pallet-utility?/std", + "pallet-proxy?/std", "sp-runtime/std", "sp-std/std", ] @@ -44,6 +51,9 @@ runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", + "pallet-balances/runtime-benchmarks", + "pallet-utility/runtime-benchmarks", + "pallet-proxy/runtime-benchmarks", ] try-runtime = [ "frame-support/try-runtime", diff --git a/frame/tx-pause/src/benchmarking.rs b/frame/tx-pause/src/benchmarking.rs index d605e8fef826a..6ee76448130d6 100644 --- a/frame/tx-pause/src/benchmarking.rs +++ b/frame/tx-pause/src/benchmarking.rs @@ -20,39 +20,38 @@ use super::{Pallet as TxPause, *}; use frame_benchmarking::benchmarks; -use frame_support::traits::UnfilteredDispatchable; benchmarks! { - pause_call { - let pallet: PalletNameOf = b"SomePalletName".to_vec().try_into().unwrap(); - let function: FunctionNameOf = b"some_fn_name".to_vec().try_into().unwrap(); - let origin = T::PauseOrigin::successful_origin(); - let call = Call::::pause_call { pallet: pallet.clone(), function: function.clone() }; + pause_call { + let pallet_name: PalletNameOf = b"SomePalletName".to_vec().try_into().unwrap(); + let call_name: CallNameOf = b"some_call_name".to_vec().try_into().unwrap(); + let origin = T::PauseOrigin::successful_origin(); + let call = Call::::pause_call { pallet_name: pallet_name.clone(), call_name: call_name.clone() }; - }: { call.dispatch_bypass_filter(origin)?} - verify { - assert![TxPause::::paused_calls((pallet.clone(),function.clone())).is_some()] - } + }: _(origin, pallet_name.clone(), call_name.clone()) + verify { + assert!(TxPause::::paused_calls((pallet_name.clone(), call_name.clone())).is_some()) + } unpause_call { - let pallet: PalletNameOf = b"SomePalletName".to_vec().try_into().unwrap(); - let function: FunctionNameOf = b"some_fn_name".to_vec().try_into().unwrap(); - let pause_origin = T::PauseOrigin::successful_origin(); - - // Set - TxPause::::pause_call( - pause_origin, - pallet.clone(), - function.clone(), - )?; - - let unpause_origin = T::UnpauseOrigin::successful_origin(); - let call = Call::::unpause_call { pallet: pallet.clone(), function: function.clone() }; - - }: { call.dispatch_bypass_filter(unpause_origin)?} - verify { - assert![TxPause::::paused_calls((pallet.clone(),function.clone())).is_none()] - } - - impl_benchmark_test_suite!(TxPause, crate::mock::new_test_ext(), crate::mock::Test); + let pallet_name: PalletNameOf = b"SomePalletName".to_vec().try_into().unwrap(); + let call_name: CallNameOf = b"some_call_name".to_vec().try_into().unwrap(); + let pause_origin = T::PauseOrigin::successful_origin(); + + // Set + TxPause::::pause_call( + pause_origin, + pallet_name.clone(), + call_name.clone(), + )?; + + let unpause_origin = T::UnpauseOrigin::successful_origin(); + let call = Call::::unpause_call { pallet_name: pallet_name.clone(), call_name: call_name.clone() }; + + }: _(unpause_origin, pallet_name.clone(), call_name.clone()) + verify { + assert!(TxPause::::paused_calls((pallet_name.clone(), call_name.clone())).is_none()) + } + + impl_benchmark_test_suite!(TxPause, crate::mock::new_test_ext(), crate::mock::Test); } diff --git a/frame/tx-pause/src/lib.rs b/frame/tx-pause/src/lib.rs index 86e1a1426c7db..0808d2ab5240a 100644 --- a/frame/tx-pause/src/lib.rs +++ b/frame/tx-pause/src/lib.rs @@ -17,7 +17,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -#[cfg(feature = "runtime-benchmarks")] mod benchmarking; #[cfg(test)] pub mod mock; @@ -26,10 +25,12 @@ mod tests; pub mod weights; use frame_support::{ + dispatch::GetDispatchInfo, pallet_prelude::*, - traits::{CallMetadata, Contains, GetCallMetadata}, + traits::{CallMetadata, Contains, GetCallMetadata, IsSubType, IsType}, }; use frame_system::pallet_prelude::*; +use sp_runtime::{traits::Dispatchable, DispatchResult}; use sp_std::{convert::TryInto, prelude::*}; pub use pallet::*; @@ -37,6 +38,7 @@ pub use weights::*; pub type PalletNameOf = BoundedVec::MaxNameLen>; pub type CallNameOf = BoundedVec::MaxNameLen>; +pub type FullNameOf = (PalletNameOf, Option>); #[frame_support::pallet] pub mod pallet { @@ -51,18 +53,28 @@ pub mod pallet { /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; + /// The overarching call type. + type RuntimeCall: Parameter + + Dispatchable + + GetDispatchInfo + + GetCallMetadata + + From> + + IsSubType> + + IsType<::RuntimeCall>; + /// The only origin that can pause calls. type PauseOrigin: EnsureOrigin; /// The only origin that can un-pause calls. type UnpauseOrigin: EnsureOrigin; - /// Pallets that are safe and can never be paused. + /// Contains all calls that cannot be paused. /// - /// The tx-pause pallet is always assumed to be safe itself. - type UnpausablePallets: Contains>; + /// The `TxMode` pallet cannot pause it's own calls, and does not need to be explicitly + /// added here. + type UnfilterableCallNames: Contains>; - /// Maximum length for pallet- and function-names. + /// Maximum length for pallet and call SCALE encoded string names. /// /// Too long names will not be truncated but handled like /// [`Self::PauseTooLongNames`] specifies. @@ -92,15 +104,18 @@ pub mod pallet { /// The call is listed as safe and cannot be paused. IsUnpausable, + + // The call does not exist in the runtime. + NoSuchCall, } #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { - /// This call got paused. - CallPaused(PalletNameOf, CallNameOf), - /// This call got un-paused. - CallUnpaused(PalletNameOf, CallNameOf), + /// This call is now paused. \[pallet_name, call_name\] + CallPaused { pallet_name: PalletNameOf, call_name: CallNameOf }, + /// This call is now unpaused. \[pallet_name, call_name\] + CallUnpaused { pallet_name: PalletNameOf, call_name: CallNameOf }, } /// The set of calls that are explicitly paused. @@ -129,8 +144,8 @@ pub mod pallet { #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - for (pallet, function) in &self.paused { - PausedCalls::::insert((pallet, function), ()); + for (pallet_name, call_name) in &self.paused { + PausedCalls::::insert((pallet_name, call_name), ()); } } } @@ -144,14 +159,14 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::pause_call())] pub fn pause_call( origin: OriginFor, - pallet: PalletNameOf, - call: CallNameOf, + pallet_name: PalletNameOf, + call_name: CallNameOf, ) -> DispatchResult { T::PauseOrigin::ensure_origin(origin)?; - Self::ensure_can_pause(&pallet, &call)?; - PausedCalls::::insert((&pallet, &call), ()); - Self::deposit_event(Event::CallPaused(pallet, call)); + Self::ensure_can_pause(&pallet_name, &call_name)?; + PausedCalls::::insert((&pallet_name, &call_name), ()); + Self::deposit_event(Event::CallPaused { pallet_name, call_name }); Ok(()) } @@ -163,14 +178,14 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::unpause_call())] pub fn unpause_call( origin: OriginFor, - pallet: PalletNameOf, - call: CallNameOf, + pallet_name: PalletNameOf, + call_name: CallNameOf, ) -> DispatchResult { T::UnpauseOrigin::ensure_origin(origin)?; - Self::ensure_can_unpause(&pallet, &call)?; - PausedCalls::::remove((&pallet, &call)); - Self::deposit_event(Event::CallUnpaused(pallet, call)); + Self::ensure_can_unpause(&pallet_name, &call_name)?; + PausedCalls::::remove((&pallet_name, &call_name)); + Self::deposit_event(Event::CallUnpaused { pallet_name, call_name }); Ok(()) } @@ -179,34 +194,35 @@ pub mod pallet { impl Pallet { /// Return whether this call is paused. - pub fn is_paused_unbound(pallet: Vec, call: Vec) -> bool { - let pallet = PalletNameOf::::try_from(pallet); - let call = CallNameOf::::try_from(call); + pub fn is_paused_unbound(pallet_name: Vec, call_name: Vec) -> bool { + let pallet_name = PalletNameOf::::try_from(pallet_name); + let call_name = CallNameOf::::try_from(call_name); - match (pallet, call) { - (Ok(pallet), Ok(call)) => Self::is_paused(&pallet, &call), + match (pallet_name, call_name) { + (Ok(pallet_name), Ok(call_name)) => Self::is_paused(&pallet_name, &call_name), _ => T::PauseTooLongNames::get(), } } /// Return whether this call is paused. - pub fn is_paused(pallet: &PalletNameOf, call: &CallNameOf) -> bool { - >::contains_key((pallet, call)) + pub fn is_paused(pallet_name: &PalletNameOf, call_name: &CallNameOf) -> bool { + >::contains_key((pallet_name, call_name)) } /// Ensure that this call can be paused. pub fn ensure_can_pause( - pallet: &PalletNameOf, - call: &CallNameOf, + pallet_name: &PalletNameOf, + call_name: &CallNameOf, ) -> Result<(), Error> { // The `TxPause` pallet can never be paused. - if pallet.as_ref() == ::name().as_bytes().to_vec() { + if pallet_name.as_ref() == ::name().as_bytes().to_vec() { return Err(Error::::IsUnpausable) } - if T::UnpausablePallets::contains(&pallet) { + let full_name: FullNameOf = (pallet_name.clone(), Some(call_name.clone())); + if T::UnfilterableCallNames::contains(&full_name) { return Err(Error::::IsUnpausable) } - if Self::is_paused(pallet, call) { + if Self::is_paused(&pallet_name, &call_name) { return Err(Error::::IsPaused) } Ok(()) @@ -214,10 +230,10 @@ impl Pallet { /// Ensure that this call can be un-paused. pub fn ensure_can_unpause( - pallet: &PalletNameOf, - call: &CallNameOf, + pallet_name: &PalletNameOf, + call_name: &CallNameOf, ) -> Result<(), Error> { - if Self::is_paused(pallet, call) { + if Self::is_paused(&pallet_name, &call_name) { // SAFETY: Everything that is paused, can be un-paused. Ok(()) } else { @@ -226,12 +242,12 @@ impl Pallet { } } -impl Contains for Pallet +impl Contains<::RuntimeCall> for Pallet where ::RuntimeCall: GetCallMetadata, { /// Return whether the call is allowed to be dispatched. - fn contains(call: &T::RuntimeCall) -> bool { + fn contains(call: &::RuntimeCall) -> bool { let CallMetadata { pallet_name, function_name } = call.get_call_metadata(); !Pallet::::is_paused_unbound(pallet_name.into(), function_name.into()) } diff --git a/frame/tx-pause/src/mock.rs b/frame/tx-pause/src/mock.rs index 1ed00c9805dba..d7fe7af6cede4 100644 --- a/frame/tx-pause/src/mock.rs +++ b/frame/tx-pause/src/mock.rs @@ -22,7 +22,7 @@ use crate as pallet_tx_pause; use frame_support::{ parameter_types, - traits::{Everything, InsideBoth, SortedMembers}, + traits::{ConstU64, Everything, InsideBoth, InstanceFilter, SortedMembers}, }; use frame_system::EnsureSignedBy; use sp_core::H256; @@ -77,6 +77,65 @@ impl pallet_balances::Config for Test { type ReserveIdentifier = [u8; 8]; } +impl pallet_utility::Config for Test { + type RuntimeEvent = RuntimeEvent; + type RuntimeCall = RuntimeCall; + type PalletsOrigin = OriginCaller; + type WeightInfo = (); +} + +#[derive( + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Encode, + Decode, + RuntimeDebug, + MaxEncodedLen, + scale_info::TypeInfo, +)] +pub enum ProxyType { + Any, + JustTransfer, + JustUtility, +} +impl Default for ProxyType { + fn default() -> Self { + Self::Any + } +} +impl InstanceFilter for ProxyType { + fn filter(&self, c: &RuntimeCall) -> bool { + match self { + ProxyType::Any => true, + ProxyType::JustTransfer => { + matches!(c, RuntimeCall::Balances(pallet_balances::Call::transfer { .. })) + }, + ProxyType::JustUtility => matches!(c, RuntimeCall::Utility { .. }), + } + } + fn is_superset(&self, o: &Self) -> bool { + self == &ProxyType::Any || self == o + } +} +impl pallet_proxy::Config for Test { + type RuntimeEvent = RuntimeEvent; + type RuntimeCall = RuntimeCall; + type Currency = Balances; + type ProxyType = ProxyType; + type ProxyDepositBase = ConstU64<1>; + type ProxyDepositFactor = ConstU64<1>; + type MaxProxies = ConstU32<4>; + type WeightInfo = (); + type CallHasher = BlakeTwo256; + type MaxPending = ConstU32<2>; + type AnnouncementDepositBase = ConstU64<1>; + type AnnouncementDepositFactor = ConstU64<1>; +} + parameter_types! { pub const PauseOrigin: u64 = 1; pub const UnpauseOrigin: u64 = 2; @@ -84,16 +143,34 @@ parameter_types! { pub const PauseTooLongNames: bool = false; } -pub struct MockUnpausablePallets; - -impl Contains> for MockUnpausablePallets { - fn contains(pallet: &PalletNameOf) -> bool { - let unpausables: Vec> = - vec![b"UnpausablePallet".to_vec().try_into().unwrap()]; - - unpausables.iter().any(|i| i == pallet) +#[derive(Copy, Clone, Encode, Decode, RuntimeDebug, MaxEncodedLen, scale_info::TypeInfo)] +pub struct UnfilterableCallNames; + +/// Make Balances::transfer_keep_alive and all DummyPallet calls unfilterable, accept all others. +impl Contains> for UnfilterableCallNames { + fn contains(full_name: &FullNameOf) -> bool { + let unpausables: Vec> = vec![ + ( + b"Balances".to_vec().try_into().unwrap(), + Some(b"transfer_keep_alive".to_vec().try_into().unwrap()), + ), + (b"DummyPallet".to_vec().try_into().unwrap(), None), + ]; + + for unpausable_call in unpausables { + let (pallet_name, maybe_call_name) = full_name; + if pallet_name == &unpausable_call.0 { + if unpausable_call.1.is_none() { + return true + } + return maybe_call_name == &unpausable_call.1 + } + } + + false } } + // Required impl to use some ::get() in tests impl SortedMembers for PauseOrigin { fn sorted_members() -> Vec { @@ -112,9 +189,10 @@ impl SortedMembers for UnpauseOrigin { impl Config for Test { type RuntimeEvent = RuntimeEvent; + type RuntimeCall = RuntimeCall; type PauseOrigin = EnsureSignedBy; type UnpauseOrigin = EnsureSignedBy; - type UnpausablePallets = MockUnpausablePallets; + type UnfilterableCallNames = UnfilterableCallNames; type MaxNameLen = MaxNameLen; type PauseTooLongNames = PauseTooLongNames; type WeightInfo = (); @@ -131,6 +209,8 @@ frame_support::construct_runtime!( { System: frame_system, Balances: pallet_balances, + Utility: pallet_utility, + Proxy: pallet_proxy, TxPause: pallet_tx_pause, } ); diff --git a/frame/tx-pause/src/tests.rs b/frame/tx-pause/src/tests.rs index f474d11eeb838..493de946492e6 100644 --- a/frame/tx-pause/src/tests.rs +++ b/frame/tx-pause/src/tests.rs @@ -25,41 +25,51 @@ use frame_support::{assert_err, assert_noop, assert_ok, dispatch::Dispatchable}; // GENERAL SUCCESS/POSITIVE TESTS --------------------- #[test] -fn can_set_arbitrary_pause() { +fn can_pause_specific_call() { new_test_ext().execute_with(|| { + assert_ok!(call_transfer(1, 1).dispatch(Origin::signed(0))); + assert_ok!(TxPause::pause_call( Origin::signed(mock::PauseOrigin::get()), - name(b"SomePallet"), - name(b"some_function"), + name(b"Balances"), + name(b"transfer"), )); + + assert_err!( + call_transfer(2, 1).dispatch(Origin::signed(2)), + frame_system::Error::::CallFiltered + ); + assert_ok!(call_transfer_keep_alive(3, 1).dispatch(Origin::signed(3))); }); } #[test] -fn can_pause_system_call() { +fn can_unpause_specific_call() { new_test_ext().execute_with(|| { - let call = RuntimeCall::System(frame_system::Call::remark { remark: vec![] }); - assert_ok!(TxPause::pause_call( Origin::signed(mock::PauseOrigin::get()), - name(b"System"), - name(b"remark"), + name(b"Balances"), + name(b"transfer"), )); - assert_err!( - call.clone().dispatch(Origin::signed(0)), + call_transfer(2, 1).dispatch(Origin::signed(2)), frame_system::Error::::CallFiltered ); + + assert_ok!(TxPause::unpause_call( + Origin::signed(mock::UnpauseOrigin::get()), + name(b"Balances"), + name(b"transfer"), + )); + assert_ok!(call_transfer(4, 1).dispatch(Origin::signed(0))); }); } #[test] -fn can_pause_specific_call() { +fn can_filter_balance_in_batch_when_paused() { new_test_ext().execute_with(|| { - let call_paused = - RuntimeCall::Balances(pallet_balances::Call::transfer { dest: 1, value: 1 }); - let call_not_paused = - RuntimeCall::Balances(pallet_balances::Call::transfer_keep_alive { dest: 1, value: 1 }); + let batch_call = + RuntimeCall::Utility(pallet_utility::Call::batch { calls: vec![call_transfer(1, 1)] }); assert_ok!(TxPause::pause_call( Origin::signed(mock::PauseOrigin::get()), @@ -67,36 +77,35 @@ fn can_pause_specific_call() { name(b"transfer"), )); - assert_err!( - call_paused.clone().dispatch(Origin::signed(0)), - frame_system::Error::::CallFiltered + assert_ok!(batch_call.clone().dispatch(Origin::signed(0))); + System::assert_last_event( + pallet_utility::Event::BatchInterrupted { + index: 0, + error: frame_system::Error::::CallFiltered.into(), + } + .into(), ); - assert_ok!(call_not_paused.clone().dispatch(Origin::signed(0))); }); } #[test] -fn can_unpause_specific_call() { +fn can_filter_balance_in_proxy_when_paused() { new_test_ext().execute_with(|| { - let call_paused = - RuntimeCall::Balances(pallet_balances::Call::transfer { dest: 1, value: 1 }); - assert_ok!(TxPause::pause_call( Origin::signed(mock::PauseOrigin::get()), name(b"Balances"), name(b"transfer"), )); - assert_err!( - call_paused.clone().dispatch(Origin::signed(0)), - frame_system::Error::::CallFiltered - ); - assert_ok!(TxPause::unpause_call( - Origin::signed(mock::UnpauseOrigin::get()), - name(b"Balances"), - name(b"transfer"), - )); - assert_ok!(call_paused.clone().dispatch(Origin::signed(0))); + assert_ok!(Proxy::add_proxy(Origin::signed(1), 2, ProxyType::JustTransfer, 0)); + + assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, Box::new(call_transfer(1, 1)))); + System::assert_last_event( + pallet_proxy::Event::ProxyExecuted { + result: DispatchError::from(frame_system::Error::::CallFiltered).into(), + } + .into(), + ); }); } @@ -109,7 +118,7 @@ fn fails_to_pause_self() { TxPause::pause_call( Origin::signed(mock::PauseOrigin::get()), name(b"TxPause"), - name(b"should_not_matter"), + name(b"pause_call") ), Error::::IsUnpausable ); @@ -122,8 +131,22 @@ fn fails_to_pause_unpausable_pallet() { assert_noop!( TxPause::pause_call( Origin::signed(mock::PauseOrigin::get()), - name(b"UnpausablePallet"), - name(b"should_not_matter"), + name(b"DummyPallet"), + name(b"any-call") + ), + Error::::IsUnpausable + ); + }); +} + +#[test] +fn fails_to_pause_unpausable_call() { + new_test_ext().execute_with(|| { + assert_noop!( + TxPause::pause_call( + Origin::signed(mock::PauseOrigin::get()), + name(b"Balances"), + name(b"transfer_keep_alive"), ), Error::::IsUnpausable ); @@ -135,15 +158,15 @@ fn fails_to_pause_already_paused_pallet() { new_test_ext().execute_with(|| { assert_ok!(TxPause::pause_call( Origin::signed(mock::PauseOrigin::get()), - name(b"SomePallet"), - name(b"some_function"), + name(b"Balances"), + name(b"transfer"), )); assert_noop!( TxPause::pause_call( Origin::signed(mock::PauseOrigin::get()), - name(b"SomePallet"), - name(b"some_function"), + name(b"Balances"), + name(b"transfer"), ), Error::::IsPaused ); @@ -156,8 +179,8 @@ fn fails_to_unpause_not_paused_pallet() { assert_noop!( TxPause::unpause_call( Origin::signed(mock::UnpauseOrigin::get()), - name(b"SomePallet"), - name(b"some_function"), + name(b"Balances"), + name(b"transfer"), ), Error::::IsUnpaused ); @@ -167,3 +190,11 @@ fn fails_to_unpause_not_paused_pallet() { fn name(bytes: &[u8]) -> BoundedVec { bytes.to_vec().try_into().unwrap() } + +fn call_transfer(dest: u64, value: u64) -> RuntimeCall { + RuntimeCall::Balances(pallet_balances::Call::transfer { dest, value }) +} + +fn call_transfer_keep_alive(dest: u64, value: u64) -> RuntimeCall { + RuntimeCall::Balances(pallet_balances::Call::transfer_keep_alive { dest, value }) +} diff --git a/frame/tx-pause/src/weights.rs b/frame/tx-pause/src/weights.rs index 10a296924b660..bd1d7de74c64b 100644 --- a/frame/tx-pause/src/weights.rs +++ b/frame/tx-pause/src/weights.rs @@ -18,7 +18,7 @@ //! Autogenerated weights for pallet_tx_pause //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-09-19, STEPS: `1`, REPEAT: 1, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2022-10-11, STEPS: `1`, REPEAT: 1, LOW RANGE: `[]`, HIGH RANGE: `[]` //! HOSTNAME: `pop-os`, CPU: `12th Gen Intel(R) Core(TM) i7-12700H` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 @@ -64,13 +64,13 @@ pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { // Storage: TxPause PausedCalls (r:1 w:1) fn pause_call() -> Weight { - Weight::from_ref_time(55_270_000 as u64) + Weight::from_ref_time(38_905_000 as u64) .saturating_add(T::DbWeight::get().reads(1 as u64)) .saturating_add(T::DbWeight::get().writes(1 as u64)) } // Storage: TxPause PausedCalls (r:1 w:1) fn unpause_call() -> Weight { - Weight::from_ref_time(55_290_000 as u64) + Weight::from_ref_time(32_892_000 as u64) .saturating_add(T::DbWeight::get().reads(1 as u64)) .saturating_add(T::DbWeight::get().writes(1 as u64)) } @@ -80,13 +80,13 @@ impl WeightInfo for SubstrateWeight { impl WeightInfo for () { // Storage: TxPause PausedCalls (r:1 w:1) fn pause_call() -> Weight { - Weight::from_ref_time(55_270_000 as u64) + Weight::from_ref_time(38_905_000 as u64) .saturating_add(RocksDbWeight::get().reads(1 as u64)) .saturating_add(RocksDbWeight::get().writes(1 as u64)) } // Storage: TxPause PausedCalls (r:1 w:1) fn unpause_call() -> Weight { - Weight::from_ref_time(55_290_000 as u64) + Weight::from_ref_time(32_892_000 as u64) .saturating_add(RocksDbWeight::get().reads(1 as u64)) .saturating_add(RocksDbWeight::get().writes(1 as u64)) }