From 2f179585229880a596ab3b8b04a4be6c7db15efa Mon Sep 17 00:00:00 2001 From: seemantaggarwal <32275622+seemantaggarwal@users.noreply.github.com> Date: Thu, 9 Jan 2025 20:18:59 +0530 Subject: [PATCH] Migrating salary pallet to use umbrella crate (#7048) # Description Migrating salary pallet to use umbrella crate. It is a follow-up from https://github.com/paritytech/polkadot-sdk/pull/7025 Why did I create this new branch? I did this, so that the unnecessary cargo fmt changes from the previous branch are discarded and hence opened this new PR. ## Review Notes This PR migrates pallet-salary to use the umbrella crate. Added change: Explanation requested for why `TestExternalities` was replaced by `TestState` as testing_prelude already includes it `pub use sp_io::TestExternalities as TestState;` I have also modified the defensive! macro to be compatible with umbrella crate as it was being used in the salary pallet --- Cargo.lock | 8 +----- prdoc/pr_7048.prdoc | 17 ++++++++++++ substrate/frame/salary/Cargo.toml | 26 +++--------------- substrate/frame/salary/src/benchmarking.rs | 7 ++--- substrate/frame/salary/src/lib.rs | 27 +++++-------------- .../frame/salary/src/tests/integration.rs | 25 +++++------------ substrate/frame/salary/src/tests/unit.rs | 24 ++++++----------- substrate/frame/salary/src/weights.rs | 2 +- substrate/frame/src/lib.rs | 23 +++++++++------- 9 files changed, 60 insertions(+), 99 deletions(-) create mode 100644 prdoc/pr_7048.prdoc diff --git a/Cargo.lock b/Cargo.lock index 0a22179eb3d7..4e2272bdc988 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15064,17 +15064,11 @@ dependencies = [ name = "pallet-salary" version = "13.0.0" dependencies = [ - "frame-benchmarking 28.0.0", - "frame-support 28.0.0", - "frame-system 28.0.0", "log", "pallet-ranked-collective 28.0.0", "parity-scale-codec", + "polkadot-sdk-frame 0.1.0", "scale-info", - "sp-arithmetic 23.0.0", - "sp-core 28.0.0", - "sp-io 30.0.0", - "sp-runtime 31.0.1", ] [[package]] diff --git a/prdoc/pr_7048.prdoc b/prdoc/pr_7048.prdoc new file mode 100644 index 000000000000..0f3856bc1287 --- /dev/null +++ b/prdoc/pr_7048.prdoc @@ -0,0 +1,17 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: '[pallet-salary] Migrate to using frame umbrella crate' + +doc: + - audience: Runtime Dev + description: > + This PR migrates the `pallet-salary` to use the FRAME umbrella crate. + This is part of the ongoing effort to migrate all pallets to use the FRAME umbrella crate. + The effort is tracked [here](https://github.com/paritytech/polkadot-sdk/issues/6504). + +crates: + - name: pallet-salary + bump: minor + - name: polkadot-sdk-frame + bump: minor diff --git a/substrate/frame/salary/Cargo.toml b/substrate/frame/salary/Cargo.toml index b3ed95bf1de5..626993a0547b 100644 --- a/substrate/frame/salary/Cargo.toml +++ b/substrate/frame/salary/Cargo.toml @@ -17,43 +17,25 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { features = ["derive"], workspace = true } -frame-benchmarking = { optional = true, workspace = true } -frame-support = { workspace = true } -frame-system = { workspace = true } +frame = { workspace = true, features = ["experimental", "runtime"] } log = { workspace = true } pallet-ranked-collective = { optional = true, workspace = true } scale-info = { features = ["derive"], workspace = true } -sp-arithmetic = { workspace = true } -sp-core = { workspace = true } -sp-io = { workspace = true } -sp-runtime = { workspace = true } [features] default = ["std"] std = [ "codec/std", - "frame-benchmarking?/std", - "frame-support/experimental", - "frame-support/std", - "frame-system/std", + "frame/std", "log/std", "pallet-ranked-collective/std", "scale-info/std", - "sp-arithmetic/std", - "sp-core/std", - "sp-io/std", - "sp-runtime/std", ] runtime-benchmarks = [ - "frame-benchmarking/runtime-benchmarks", - "frame-support/runtime-benchmarks", - "frame-system/runtime-benchmarks", + "frame/runtime-benchmarks", "pallet-ranked-collective/runtime-benchmarks", - "sp-runtime/runtime-benchmarks", ] try-runtime = [ - "frame-support/try-runtime", - "frame-system/try-runtime", + "frame/try-runtime", "pallet-ranked-collective?/try-runtime", - "sp-runtime/try-runtime", ] diff --git a/substrate/frame/salary/src/benchmarking.rs b/substrate/frame/salary/src/benchmarking.rs index aeae8d2d67f8..6dfd6f6dd488 100644 --- a/substrate/frame/salary/src/benchmarking.rs +++ b/substrate/frame/salary/src/benchmarking.rs @@ -22,10 +22,7 @@ use super::*; use crate::Pallet as Salary; -use frame_benchmarking::v2::*; -use frame_system::{Pallet as System, RawOrigin}; -use sp_core::Get; - +use frame::benchmarking::prelude::*; const SEED: u32 = 0; fn ensure_member_with_salary, I: 'static>(who: &T::AccountId) { @@ -37,7 +34,7 @@ fn ensure_member_with_salary, I: 'static>(who: &T::AccountId) { for _ in 0..255 { let r = T::Members::rank_of(who).expect("prior guard ensures `who` is a member; qed"); if !T::Salary::get_salary(r, &who).is_zero() { - break + break; } T::Members::promote(who).unwrap(); } diff --git a/substrate/frame/salary/src/lib.rs b/substrate/frame/salary/src/lib.rs index efb4f5d3c542..6a843625f4a7 100644 --- a/substrate/frame/salary/src/lib.rs +++ b/substrate/frame/salary/src/lib.rs @@ -19,20 +19,10 @@ #![cfg_attr(not(feature = "std"), no_std)] -use codec::{Decode, Encode, MaxEncodedLen}; use core::marker::PhantomData; -use scale_info::TypeInfo; -use sp_arithmetic::traits::{Saturating, Zero}; -use sp_runtime::{Perbill, RuntimeDebug}; - -use frame_support::{ - defensive, - dispatch::DispatchResultWithPostInfo, - ensure, - traits::{ - tokens::{GetSalary, Pay, PaymentStatus}, - RankedMembers, RankedMembersSwapHandler, - }, +use frame::{ + prelude::*, + traits::tokens::{GetSalary, Pay, PaymentStatus}, }; #[cfg(test)] @@ -85,12 +75,9 @@ pub struct ClaimantStatus { status: ClaimState, } -#[frame_support::pallet] +#[frame::pallet] pub mod pallet { use super::*; - use frame_support::{dispatch::Pays, pallet_prelude::*}; - use frame_system::pallet_prelude::*; - #[pallet::pallet] pub struct Pallet(PhantomData<(T, I)>); @@ -460,15 +447,15 @@ impl, I: 'static> ) { if who == new_who { defensive!("Should not try to swap with self"); - return + return; } if Claimant::::contains_key(new_who) { defensive!("Should not try to overwrite existing claimant"); - return + return; } let Some(claimant) = Claimant::::take(who) else { - frame_support::defensive!("Claimant should exist when swapping"); + defensive!("Claimant should exist when swapping"); return; }; diff --git a/substrate/frame/salary/src/tests/integration.rs b/substrate/frame/salary/src/tests/integration.rs index 0c1fb8bbdcba..e4e9c8f6a31b 100644 --- a/substrate/frame/salary/src/tests/integration.rs +++ b/substrate/frame/salary/src/tests/integration.rs @@ -19,25 +19,14 @@ use crate as pallet_salary; use crate::*; -use frame_support::{ - assert_noop, assert_ok, derive_impl, hypothetically, - pallet_prelude::Weight, - parameter_types, - traits::{ConstU64, EitherOf, MapSuccess, NoOpPoll}, -}; +use frame::{deps::sp_io, testing_prelude::*}; use pallet_ranked_collective::{EnsureRanked, Geometric}; -use sp_core::{ConstU16, Get}; -use sp_runtime::{ - traits::{Convert, ReduceBy, ReplaceWithDefault}, - BuildStorage, -}; type Rank = u16; type Block = frame_system::mocking::MockBlock; -frame_support::construct_runtime!( - pub enum Test - { +construct_runtime!( + pub struct Test { System: frame_system, Salary: pallet_salary, Club: pallet_ranked_collective, @@ -145,9 +134,9 @@ impl pallet_ranked_collective::Config for Test { type BenchmarkSetup = Salary; } -pub fn new_test_ext() -> sp_io::TestExternalities { +pub fn new_test_ext() -> TestState { let t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - let mut ext = sp_io::TestExternalities::new(t); + let mut ext = TestState::new(t); ext.execute_with(|| System::set_block_number(1)); ext } @@ -194,7 +183,7 @@ fn swap_exhaustive_works() { // The events mess up the storage root: System::reset_events(); - sp_io::storage::root(sp_runtime::StateVersion::V1) + sp_io::storage::root(StateVersion::V1) }); let root_swap = hypothetically!({ @@ -207,7 +196,7 @@ fn swap_exhaustive_works() { // The events mess up the storage root: System::reset_events(); - sp_io::storage::root(sp_runtime::StateVersion::V1) + sp_io::storage::root(StateVersion::V1) }); assert_eq!(root_add, root_swap); diff --git a/substrate/frame/salary/src/tests/unit.rs b/substrate/frame/salary/src/tests/unit.rs index db1c8b947ef5..3bb7bc4adf1e 100644 --- a/substrate/frame/salary/src/tests/unit.rs +++ b/substrate/frame/salary/src/tests/unit.rs @@ -17,23 +17,15 @@ //! The crate's tests. -use std::collections::BTreeMap; - -use core::cell::RefCell; -use frame_support::{ - assert_noop, assert_ok, derive_impl, - pallet_prelude::Weight, - parameter_types, - traits::{tokens::ConvertRank, ConstU64}, -}; -use sp_runtime::{traits::Identity, BuildStorage, DispatchResult}; - use crate as pallet_salary; use crate::*; +use core::cell::RefCell; +use frame::{deps::sp_runtime::traits::Identity, testing_prelude::*, traits::tokens::ConvertRank}; +use std::collections::BTreeMap; -type Block = frame_system::mocking::MockBlock; +type Block = MockBlock; -frame_support::construct_runtime!( +construct_runtime!( pub enum Test { System: frame_system, @@ -124,7 +116,7 @@ impl RankedMembers for TestClub { } fn demote(who: &Self::AccountId) -> DispatchResult { CLUB.with(|club| match club.borrow().get(who) { - None => Err(sp_runtime::DispatchError::Unavailable), + None => Err(DispatchError::Unavailable), Some(&0) => { club.borrow_mut().remove(&who); Ok(()) @@ -156,9 +148,9 @@ impl Config for Test { type Budget = Budget; } -pub fn new_test_ext() -> sp_io::TestExternalities { +pub fn new_test_ext() -> TestState { let t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - let mut ext = sp_io::TestExternalities::new(t); + let mut ext = TestState::new(t); ext.execute_with(|| System::set_block_number(1)); ext } diff --git a/substrate/frame/salary/src/weights.rs b/substrate/frame/salary/src/weights.rs index f1cdaaa225a4..43c001b30d33 100644 --- a/substrate/frame/salary/src/weights.rs +++ b/substrate/frame/salary/src/weights.rs @@ -46,8 +46,8 @@ #![allow(unused_imports)] #![allow(missing_docs)] -use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; use core::marker::PhantomData; +use frame::weights_prelude::*; /// Weight functions needed for `pallet_salary`. pub trait WeightInfo { diff --git a/substrate/frame/src/lib.rs b/substrate/frame/src/lib.rs index 15601ebde1f2..23d22683be2d 100644 --- a/substrate/frame/src/lib.rs +++ b/substrate/frame/src/lib.rs @@ -203,8 +203,12 @@ pub mod prelude { /// Dispatch types from `frame-support`, other fundamental traits #[doc(no_inline)] pub use frame_support::dispatch::{GetDispatchInfo, PostDispatchInfo}; - pub use frame_support::traits::{ - Contains, EstimateNextSessionRotation, IsSubType, OnRuntimeUpgrade, OneSessionHandler, + pub use frame_support::{ + defensive, defensive_assert, + traits::{ + Contains, EitherOf, EstimateNextSessionRotation, IsSubType, MapSuccess, NoOpPoll, + OnRuntimeUpgrade, OneSessionHandler, RankedMembers, RankedMembersSwapHandler, + }, }; /// Pallet prelude of `frame-system`. @@ -228,11 +232,10 @@ pub mod prelude { /// Runtime traits #[doc(no_inline)] pub use sp_runtime::traits::{ - BlockNumberProvider, Bounded, DispatchInfoOf, Dispatchable, SaturatedConversion, - Saturating, StaticLookup, TrailingZeroInput, + BlockNumberProvider, Bounded, Convert, DispatchInfoOf, Dispatchable, ReduceBy, + ReplaceWithDefault, SaturatedConversion, Saturating, StaticLookup, TrailingZeroInput, }; - - /// Other runtime types and traits + /// Other error/result types for runtime #[doc(no_inline)] pub use sp_runtime::{ BoundToRuntimeAppPublic, DispatchErrorWithPostInfo, DispatchResultWithInfo, TokenError, @@ -262,7 +265,7 @@ pub mod benchmarking { pub use frame_benchmarking::benchmarking::*; // The system origin, which is very often needed in benchmarking code. Might be tricky only // if the pallet defines its own `#[pallet::origin]` and call it `RawOrigin`. - pub use frame_system::RawOrigin; + pub use frame_system::{Pallet as System, RawOrigin}; } #[deprecated( @@ -319,7 +322,7 @@ pub mod testing_prelude { /// Other helper macros from `frame_support` that help with asserting in tests. pub use frame_support::{ assert_err, assert_err_ignore_postinfo, assert_error_encoded_size, assert_noop, assert_ok, - assert_storage_noop, storage_alias, + assert_storage_noop, hypothetically, storage_alias, }; pub use frame_system::{self, mocking::*}; @@ -330,7 +333,7 @@ pub mod testing_prelude { pub use sp_io::TestExternalities as TestState; /// Commonly used runtime traits for testing. - pub use sp_runtime::traits::BadOrigin; + pub use sp_runtime::{traits::BadOrigin, StateVersion}; } /// All of the types and tools needed to build FRAME-based runtimes. @@ -508,7 +511,7 @@ pub mod runtime { #[cfg(feature = "std")] pub mod testing_prelude { pub use sp_core::storage::Storage; - pub use sp_runtime::BuildStorage; + pub use sp_runtime::{BuildStorage, DispatchError}; } }