Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate pallet-alliance to umbrella crate #6541

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions prdoc/pr_6541.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
runcomet marked this conversation as resolved.
Show resolved Hide resolved
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: migrate pallet-alliance to umbrella crate.

doc:
- audience: Runtime Dev
description: |
This PR:
- Imports frame umbrella crate systems into pallet-alliance.
- Includes BoundedVec as part of `runtime::prelude`.
- Includes the event module in `testing_prelude`, this are systems used in asseccing events
in mock runtimes across different crates in the PolkadotSDK.

crates:
- name: pallet-alliance
bump: minor
- name: polkadot-sdk-frame
bump: minor
27 changes: 4 additions & 23 deletions substrate/frame/alliance/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,10 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
array-bytes = { optional = true, workspace = true, default-features = true }
log = { workspace = true }

codec = { features = ["derive"], workspace = true }
scale-info = { features = ["derive"], workspace = true }

sp-core = { workspace = true }
sp-crypto-hashing = { optional = true, workspace = true }
sp-io = { workspace = true }
sp-runtime = { workspace = true }

frame-benchmarking = { optional = true, workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }

frame = { workspace = true, features = ["experimental", "runtime"] }
pallet-identity = { workspace = true }
pallet-collective = { optional = true, workspace = true }

Expand All @@ -44,35 +35,25 @@ pallet-collective = { workspace = true, default-features = true }
default = ["std"]
std = [
"codec/std",
"frame-benchmarking?/std",
"frame-support/std",
"frame-system/std",
"frame/std",
"log/std",
"pallet-balances/std",
"pallet-collective?/std",
"pallet-identity/std",
"scale-info/std",
"sp-core/std",
"sp-crypto-hashing?/std",
"sp-io/std",
"sp-runtime/std",
]
runtime-benchmarks = [
"array-bytes",
"frame-benchmarking/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"frame/runtime-benchmarks",
"pallet-balances/runtime-benchmarks",
"pallet-collective/runtime-benchmarks",
"pallet-identity/runtime-benchmarks",
"sp-crypto-hashing",
"sp-runtime/runtime-benchmarks",
]
try-runtime = [
"frame-support/try-runtime",
"frame-system/try-runtime",
"frame/try-runtime",
"pallet-balances/try-runtime",
"pallet-collective?/try-runtime",
"pallet-identity/try-runtime",
"sp-runtime/try-runtime",
]
17 changes: 7 additions & 10 deletions substrate/frame/alliance/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,18 @@

#![cfg(feature = "runtime-benchmarks")]

use core::{cmp, mem::size_of};
use sp_runtime::traits::{Bounded, Hash, StaticLookup};

use frame_benchmarking::{account, v2::*, BenchmarkError};
use frame_support::traits::{EnsureOrigin, Get, UnfilteredDispatchable};
use frame_system::{pallet_prelude::BlockNumberFor, Pallet as System, RawOrigin as SystemOrigin};

use super::{Call as AllianceCall, Pallet as Alliance, *};
use core::{cmp, mem::size_of};
use frame::{benchmarking::prelude::*, traits::UnfilteredDispatchable};
runcomet marked this conversation as resolved.
Show resolved Hide resolved
use frame_system::Pallet as System;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in my previous observation, please check if assert_last_event is widely used and should be added to the benchmarking prelude.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could leave things as

use frame::benchmarking::prelude::{*, RawOrigin as SystemOrigin, frame_system::Pallet as System};

until we migrate most pallets? @re-gius @kianenigma

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert_last_event are sometimes instantiable, creating one function for both occurrences?

please could you explain a bit more

use RawOrigin as SystemOrigin;

const SEED: u32 = 0;

const MAX_BYTES: u32 = 1_024;

fn assert_last_event<T: Config<I>, I: 'static>(generic_event: <T as Config<I>>::RuntimeEvent) {
frame_system::Pallet::<T>::assert_last_event(generic_event.into());
System::<T>::assert_last_event(generic_event.into());
}

fn cid(input: impl AsRef<[u8]>) -> Cid {
Expand Down Expand Up @@ -193,7 +190,7 @@ mod benchmarks {

// Whitelist voter account from further DB operations.
let voter_key = frame_system::Account::<T>::hashed_key_for(&voter);
frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into());
add_to_whitelist(voter_key.into());

#[extrinsic_call]
_(SystemOrigin::Signed(voter), last_hash, index, approve);
Expand Down Expand Up @@ -264,7 +261,7 @@ mod benchmarks {

// Whitelist voter account from further DB operations.
let voter_key = frame_system::Account::<T>::hashed_key_for(&voter);
frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into());
add_to_whitelist(voter_key.into());

#[extrinsic_call]
close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage);
Expand Down
22 changes: 5 additions & 17 deletions substrate/frame/alliance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,21 +98,9 @@ extern crate alloc;

use alloc::{boxed::Box, vec, vec::Vec};
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;
use sp_runtime::{
traits::{Dispatchable, Saturating, StaticLookup, Zero},
DispatchError, RuntimeDebug,
};

use frame_support::{
dispatch::{DispatchResult, DispatchResultWithPostInfo, GetDispatchInfo, PostDispatchInfo},
ensure,
traits::{
ChangeMembers, Currency, Get, InitializeMembers, IsSubType, OnUnbalanced,
ReservableCurrency,
},
weights::Weight,
use frame::{
prelude::*,
traits::{ChangeMembers, Currency, InitializeMembers, OnUnbalanced, ReservableCurrency},
};
use scale_info::TypeInfo;

Expand Down Expand Up @@ -215,7 +203,7 @@ type UnscrupulousItemOf<T, I> =

type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;

#[frame_support::pallet]
#[frame::pallet]
pub mod pallet {
use super::*;

Expand Down Expand Up @@ -399,7 +387,7 @@ pub mod pallet {
}

#[pallet::genesis_config]
#[derive(frame_support::DefaultNoBound)]
#[derive(DefaultNoBound)]
pub struct GenesisConfig<T: Config<I>, I: 'static = ()> {
pub fellows: Vec<T::AccountId>,
pub allies: Vec<T::AccountId>,
Expand Down
3 changes: 2 additions & 1 deletion substrate/frame/alliance/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@
// limitations under the License.

use crate::{Config, Pallet, Weight, LOG_TARGET};
use frame_support::{pallet_prelude::*, storage::migration, traits::OnRuntimeUpgrade};
use frame::{testing_prelude::*, traits::OnRuntimeUpgrade};
use log;
use storage::migration;

/// The in-code storage version.
pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
Expand Down
26 changes: 9 additions & 17 deletions substrate/frame/alliance/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,11 @@
// limitations under the License.

//! Test utilities

pub use sp_core::H256;
use sp_runtime::traits::Hash;
pub use sp_runtime::{
traits::{BlakeTwo256, IdentifyAccount, Lazy, Verify},
BuildStorage,
};

pub use frame_support::{
assert_noop, assert_ok, derive_impl, ord_parameter_types, parameter_types,
traits::EitherOfDiverse, BoundedVec,
use frame::{
arithmetic::Perbill,
testing_prelude::*,
traits::{EitherOfDiverse, IdentifyAccount, Lazy, Verify},
};
use frame_system::{EnsureRoot, EnsureSignedBy};
use pallet_identity::{
legacy::{IdentityField, IdentityInfo},
Data, IdentityOf, Judgement, SuperOf,
Expand Down Expand Up @@ -63,7 +55,7 @@ parameter_types! {
pub const MotionDuration: BlockNumber = MOTION_DURATION_IN_BLOCKS;
pub const MaxProposals: u32 = 100;
pub const MaxMembers: u32 = 100;
pub MaxProposalWeight: Weight = sp_runtime::Perbill::from_percent(50) * BlockWeights::get().max_block;
pub MaxProposalWeight: Weight = Perbill::from_percent(50) * BlockWeights::get().max_block;
}
type AllianceCollective = pallet_collective::Instance1;
impl pallet_collective::Config<AllianceCollective> for Test {
Expand Down Expand Up @@ -237,7 +229,7 @@ impl Config for Test {

type Block = frame_system::mocking::MockBlock<Test>;

frame_support::construct_runtime!(
construct_runtime!(
pub enum Test
{
System: frame_system,
Expand Down Expand Up @@ -268,7 +260,7 @@ pub(super) fn test_identity_info_deposit() -> <Test as pallet_balances::Config>:
byte_deposit * test_identity_info().encoded_size() as u64 + basic_deposit
}

pub fn new_test_ext() -> sp_io::TestExternalities {
pub fn new_test_ext() -> TestExternalities {
let mut t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();

pallet_balances::GenesisConfig::<Test> {
Expand All @@ -295,7 +287,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
.assimilate_storage(&mut t)
.unwrap();

let mut ext = sp_io::TestExternalities::new(t);
let mut ext = TestExternalities::new(t);
ext.execute_with(|| {
assert_ok!(Identity::add_registrar(RuntimeOrigin::signed(1), 1));

Expand Down Expand Up @@ -372,7 +364,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
}

#[cfg(feature = "runtime-benchmarks")]
pub fn new_bench_ext() -> sp_io::TestExternalities {
pub fn new_bench_ext() -> TestExternalities {
RuntimeGenesisConfig::default().build_storage().unwrap().into()
}

Expand Down
5 changes: 1 addition & 4 deletions substrate/frame/alliance/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@

//! Tests for the alliance pallet.

use frame_support::{assert_noop, assert_ok};
use frame_system::{EventRecord, Phase};
use sp_runtime::traits::BadOrigin;

use super::*;
use crate::{self as alliance, mock::*};
use frame::{deps::sp_runtime::DispatchError::BadOrigin, testing_prelude::*};

type AllianceMotionEvent = pallet_collective::Event<Test, pallet_collective::Instance1>;

Expand Down
3 changes: 1 addition & 2 deletions substrate/frame/alliance/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@

use alloc::vec::Vec;
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{traits::ConstU32, BoundedVec};
use frame::runtime::prelude::*;
use scale_info::TypeInfo;
use sp_runtime::RuntimeDebug;

/// A Multihash instance that only supports the basic functionality and no hashing.
#[derive(
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/alliance/src/weights.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions substrate/frame/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ pub mod weights_prelude {
pub mod testing_prelude {
pub use crate::{prelude::*, runtime::prelude::*};

/// Testing includes event cheking.
pub use crate::event::*;

/// Testing includes building a runtime, so we bring in all preludes related to runtimes as
/// well.
pub use super::runtime::testing_prelude::*;
Expand Down Expand Up @@ -342,6 +345,9 @@ pub mod runtime {
/// Consider using the new version of this [`frame_construct_runtime`].
pub use frame_support::construct_runtime;

/// Related to runtime contruction.
runcomet marked this conversation as resolved.
Show resolved Hide resolved
pub use frame_support::BoundedVec;

/// Macro to amalgamate the runtime into `struct Runtime`.
///
/// This is the newer version of [`construct_runtime`].
Expand Down Expand Up @@ -513,6 +519,11 @@ pub mod arithmetic {
pub use sp_arithmetic::{traits::*, *};
}

/// Event checking system for mock runtimes.
pub mod event {
pub use frame_system::{EventRecord, Phase};
}

/// All derive macros used in frame.
///
/// This is already part of the [`prelude`].
Expand Down
Loading