From f5e7eaf610b50c6a6e3f65649908100ce8bea5b0 Mon Sep 17 00:00:00 2001 From: Shree Vatsa N Date: Wed, 24 Jul 2024 22:13:41 +0530 Subject: [PATCH] membership: Restructure pallet into separate files (#4536) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - What does this PR do? This PR separates `membership` pallet into separate files for `lib`, `mock` & `tests`. - Why are these changes needed? Currently `membership` pallet consists of `lib`, `mock` & `tests` written into a single file. Refactor it into separate files which makes it inline to other pallets and improves readability. - How were these changes implemented and what do they affect? The PR will not have any affect. Signed-off-by: Shreevatsa N --------- Signed-off-by: Shreevatsa N Co-authored-by: Bastian Köcher Co-authored-by: Bastian Köcher --- .../frame/membership/src/benchmarking.rs | 179 +++++++ substrate/frame/membership/src/lib.rs | 499 +----------------- substrate/frame/membership/src/mock.rs | 117 ++++ substrate/frame/membership/src/tests.rs | 217 ++++++++ 4 files changed, 522 insertions(+), 490 deletions(-) create mode 100644 substrate/frame/membership/src/benchmarking.rs create mode 100644 substrate/frame/membership/src/mock.rs create mode 100644 substrate/frame/membership/src/tests.rs diff --git a/substrate/frame/membership/src/benchmarking.rs b/substrate/frame/membership/src/benchmarking.rs new file mode 100644 index 000000000000..515be7eb5386 --- /dev/null +++ b/substrate/frame/membership/src/benchmarking.rs @@ -0,0 +1,179 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// 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 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Membership pallet benchmarking. + +use super::{Pallet as Membership, *}; +use frame_benchmarking::v1::{account, benchmarks_instance_pallet, whitelist, BenchmarkError}; +use frame_support::{assert_ok, traits::EnsureOrigin}; +use frame_system::RawOrigin; + +const SEED: u32 = 0; + +fn set_members, I: 'static>(members: Vec, prime: Option) { + let reset_origin = T::ResetOrigin::try_successful_origin() + .expect("ResetOrigin has no successful origin required for the benchmark"); + let prime_origin = T::PrimeOrigin::try_successful_origin() + .expect("PrimeOrigin has no successful origin required for the benchmark"); + + assert_ok!(Membership::::reset_members(reset_origin, members.clone())); + if let Some(prime) = prime.map(|i| members[i].clone()) { + let prime_lookup = T::Lookup::unlookup(prime); + assert_ok!(Membership::::set_prime(prime_origin, prime_lookup)); + } else { + assert_ok!(Membership::::clear_prime(prime_origin)); + } +} + +benchmarks_instance_pallet! { + add_member { + let m in 1 .. (T::MaxMembers::get() - 1); + + let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); + set_members::(members, None); + let new_member = account::("add", m, SEED); + let new_member_lookup = T::Lookup::unlookup(new_member.clone()); + }: { + assert_ok!(Membership::::add_member( + T::AddOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + new_member_lookup, + )); + } verify { + assert!(Members::::get().contains(&new_member)); + #[cfg(test)] crate::mock::clean(); + } + + // the case of no prime or the prime being removed is surely cheaper than the case of + // reporting a new prime via `MembershipChanged`. + remove_member { + let m in 2 .. T::MaxMembers::get(); + + let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); + set_members::(members.clone(), Some(members.len() - 1)); + + let to_remove = members.first().cloned().unwrap(); + let to_remove_lookup = T::Lookup::unlookup(to_remove.clone()); + }: { + assert_ok!(Membership::::remove_member( + T::RemoveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + to_remove_lookup, + )); + } verify { + assert!(!Members::::get().contains(&to_remove)); + // prime is rejigged + assert!(Prime::::get().is_some() && T::MembershipChanged::get_prime().is_some()); + #[cfg(test)] crate::mock::clean(); + } + + // we remove a non-prime to make sure it needs to be set again. + swap_member { + let m in 2 .. T::MaxMembers::get(); + + let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); + set_members::(members.clone(), Some(members.len() - 1)); + let add = account::("member", m, SEED); + let add_lookup = T::Lookup::unlookup(add.clone()); + let remove = members.first().cloned().unwrap(); + let remove_lookup = T::Lookup::unlookup(remove.clone()); + }: { + assert_ok!(Membership::::swap_member( + T::SwapOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + remove_lookup, + add_lookup, + )); + } verify { + assert!(!Members::::get().contains(&remove)); + assert!(Members::::get().contains(&add)); + // prime is rejigged + assert!(Prime::::get().is_some() && T::MembershipChanged::get_prime().is_some()); + #[cfg(test)] crate::mock::clean(); + } + + // er keep the prime common between incoming and outgoing to make sure it is rejigged. + reset_members { + let m in 1 .. T::MaxMembers::get(); + + let members = (1..m+1).map(|i| account("member", i, SEED)).collect::>(); + set_members::(members.clone(), Some(members.len() - 1)); + let mut new_members = (m..2*m).map(|i| account("member", i, SEED)).collect::>(); + }: { + assert_ok!(Membership::::reset_members( + T::ResetOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + new_members.clone(), + )); + } verify { + new_members.sort(); + assert_eq!(Members::::get(), new_members); + // prime is rejigged + assert!(Prime::::get().is_some() && T::MembershipChanged::get_prime().is_some()); + #[cfg(test)] crate::mock::clean(); + } + + change_key { + let m in 1 .. T::MaxMembers::get(); + + // worse case would be to change the prime + let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); + let prime = members.last().cloned().unwrap(); + set_members::(members.clone(), Some(members.len() - 1)); + + let add = account::("member", m, SEED); + let add_lookup = T::Lookup::unlookup(add.clone()); + whitelist!(prime); + }: { + assert_ok!(Membership::::change_key(RawOrigin::Signed(prime.clone()).into(), add_lookup)); + } verify { + assert!(!Members::::get().contains(&prime)); + assert!(Members::::get().contains(&add)); + // prime is rejigged + assert_eq!(Prime::::get().unwrap(), add); + #[cfg(test)] crate::mock::clean(); + } + + set_prime { + let m in 1 .. T::MaxMembers::get(); + let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); + let prime = members.last().cloned().unwrap(); + let prime_lookup = T::Lookup::unlookup(prime.clone()); + set_members::(members, None); + }: { + assert_ok!(Membership::::set_prime( + T::PrimeOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + prime_lookup, + )); + } verify { + assert!(Prime::::get().is_some()); + assert!(::get_prime().is_some()); + #[cfg(test)] crate::mock::clean(); + } + + clear_prime { + let members = (0..T::MaxMembers::get()).map(|i| account("member", i, SEED)).collect::>(); + let prime = members.last().cloned().unwrap(); + set_members::(members, None); + }: { + assert_ok!(Membership::::clear_prime( + T::PrimeOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + )); + } verify { + assert!(Prime::::get().is_none()); + assert!(::get_prime().is_none()); + #[cfg(test)] crate::mock::clean(); + } + + impl_benchmark_test_suite!(Membership, crate::mock::new_bench_ext(), crate::mock::Test); +} diff --git a/substrate/frame/membership/src/lib.rs b/substrate/frame/membership/src/lib.rs index e38a6ba5d931..bbd17c66be4d 100644 --- a/substrate/frame/membership/src/lib.rs +++ b/substrate/frame/membership/src/lib.rs @@ -35,6 +35,15 @@ use sp_runtime::traits::{StaticLookup, UniqueSaturatedInto}; pub mod migrations; pub mod weights; +#[cfg(test)] +mod mock; + +#[cfg(feature = "runtime-benchmarks")] +pub mod benchmarking; + +#[cfg(test)] +mod tests; + pub use pallet::*; pub use weights::WeightInfo; @@ -403,493 +412,3 @@ impl, I: 'static> SortedMembers for Pallet { } } } - -#[cfg(feature = "runtime-benchmarks")] -mod benchmark { - use super::{Pallet as Membership, *}; - use frame_benchmarking::v1::{account, benchmarks_instance_pallet, whitelist, BenchmarkError}; - use frame_support::{assert_ok, traits::EnsureOrigin}; - use frame_system::RawOrigin; - - const SEED: u32 = 0; - - fn set_members, I: 'static>(members: Vec, prime: Option) { - let reset_origin = T::ResetOrigin::try_successful_origin() - .expect("ResetOrigin has no successful origin required for the benchmark"); - let prime_origin = T::PrimeOrigin::try_successful_origin() - .expect("PrimeOrigin has no successful origin required for the benchmark"); - - assert_ok!(Membership::::reset_members(reset_origin, members.clone())); - if let Some(prime) = prime.map(|i| members[i].clone()) { - let prime_lookup = T::Lookup::unlookup(prime); - assert_ok!(Membership::::set_prime(prime_origin, prime_lookup)); - } else { - assert_ok!(Membership::::clear_prime(prime_origin)); - } - } - - benchmarks_instance_pallet! { - add_member { - let m in 1 .. (T::MaxMembers::get() - 1); - - let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); - set_members::(members, None); - let new_member = account::("add", m, SEED); - let new_member_lookup = T::Lookup::unlookup(new_member.clone()); - }: { - assert_ok!(Membership::::add_member( - T::AddOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, - new_member_lookup, - )); - } verify { - assert!(Members::::get().contains(&new_member)); - #[cfg(test)] crate::tests::clean(); - } - - // the case of no prime or the prime being removed is surely cheaper than the case of - // reporting a new prime via `MembershipChanged`. - remove_member { - let m in 2 .. T::MaxMembers::get(); - - let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); - set_members::(members.clone(), Some(members.len() - 1)); - - let to_remove = members.first().cloned().unwrap(); - let to_remove_lookup = T::Lookup::unlookup(to_remove.clone()); - }: { - assert_ok!(Membership::::remove_member( - T::RemoveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, - to_remove_lookup, - )); - } verify { - assert!(!Members::::get().contains(&to_remove)); - // prime is rejigged - assert!(Prime::::get().is_some() && T::MembershipChanged::get_prime().is_some()); - #[cfg(test)] crate::tests::clean(); - } - - // we remove a non-prime to make sure it needs to be set again. - swap_member { - let m in 2 .. T::MaxMembers::get(); - - let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); - set_members::(members.clone(), Some(members.len() - 1)); - let add = account::("member", m, SEED); - let add_lookup = T::Lookup::unlookup(add.clone()); - let remove = members.first().cloned().unwrap(); - let remove_lookup = T::Lookup::unlookup(remove.clone()); - }: { - assert_ok!(Membership::::swap_member( - T::SwapOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, - remove_lookup, - add_lookup, - )); - } verify { - assert!(!Members::::get().contains(&remove)); - assert!(Members::::get().contains(&add)); - // prime is rejigged - assert!(Prime::::get().is_some() && T::MembershipChanged::get_prime().is_some()); - #[cfg(test)] crate::tests::clean(); - } - - // er keep the prime common between incoming and outgoing to make sure it is rejigged. - reset_members { - let m in 1 .. T::MaxMembers::get(); - - let members = (1..m+1).map(|i| account("member", i, SEED)).collect::>(); - set_members::(members.clone(), Some(members.len() - 1)); - let mut new_members = (m..2*m).map(|i| account("member", i, SEED)).collect::>(); - }: { - assert_ok!(Membership::::reset_members( - T::ResetOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, - new_members.clone(), - )); - } verify { - new_members.sort(); - assert_eq!(Members::::get(), new_members); - // prime is rejigged - assert!(Prime::::get().is_some() && T::MembershipChanged::get_prime().is_some()); - #[cfg(test)] crate::tests::clean(); - } - - change_key { - let m in 1 .. T::MaxMembers::get(); - - // worse case would be to change the prime - let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); - let prime = members.last().cloned().unwrap(); - set_members::(members.clone(), Some(members.len() - 1)); - - let add = account::("member", m, SEED); - let add_lookup = T::Lookup::unlookup(add.clone()); - whitelist!(prime); - }: { - assert_ok!(Membership::::change_key(RawOrigin::Signed(prime.clone()).into(), add_lookup)); - } verify { - assert!(!Members::::get().contains(&prime)); - assert!(Members::::get().contains(&add)); - // prime is rejigged - assert_eq!(Prime::::get().unwrap(), add); - #[cfg(test)] crate::tests::clean(); - } - - set_prime { - let m in 1 .. T::MaxMembers::get(); - let members = (0..m).map(|i| account("member", i, SEED)).collect::>(); - let prime = members.last().cloned().unwrap(); - let prime_lookup = T::Lookup::unlookup(prime.clone()); - set_members::(members, None); - }: { - assert_ok!(Membership::::set_prime( - T::PrimeOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, - prime_lookup, - )); - } verify { - assert!(Prime::::get().is_some()); - assert!(::get_prime().is_some()); - #[cfg(test)] crate::tests::clean(); - } - - clear_prime { - let members = (0..T::MaxMembers::get()).map(|i| account("member", i, SEED)).collect::>(); - let prime = members.last().cloned().unwrap(); - set_members::(members, None); - }: { - assert_ok!(Membership::::clear_prime( - T::PrimeOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, - )); - } verify { - assert!(Prime::::get().is_none()); - assert!(::get_prime().is_none()); - #[cfg(test)] crate::tests::clean(); - } - - impl_benchmark_test_suite!(Membership, crate::tests::new_bench_ext(), crate::tests::Test); - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate as pallet_membership; - - use sp_runtime::{bounded_vec, traits::BadOrigin, BuildStorage}; - - use frame_support::{ - assert_noop, assert_ok, assert_storage_noop, derive_impl, ord_parameter_types, - parameter_types, - traits::{ConstU32, StorageVersion}, - }; - use frame_system::EnsureSignedBy; - - type Block = frame_system::mocking::MockBlock; - - frame_support::construct_runtime!( - pub enum Test - { - System: frame_system, - Membership: pallet_membership, - } - ); - - parameter_types! { - pub static Members: Vec = vec![]; - pub static Prime: Option = None; - } - - #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] - impl frame_system::Config for Test { - type Block = Block; - } - ord_parameter_types! { - pub const One: u64 = 1; - pub const Two: u64 = 2; - pub const Three: u64 = 3; - pub const Four: u64 = 4; - pub const Five: u64 = 5; - } - - pub struct TestChangeMembers; - impl ChangeMembers for TestChangeMembers { - fn change_members_sorted(incoming: &[u64], outgoing: &[u64], new: &[u64]) { - let mut old_plus_incoming = Members::get(); - old_plus_incoming.extend_from_slice(incoming); - old_plus_incoming.sort(); - let mut new_plus_outgoing = new.to_vec(); - new_plus_outgoing.extend_from_slice(outgoing); - new_plus_outgoing.sort(); - assert_eq!(old_plus_incoming, new_plus_outgoing); - - Members::set(new.to_vec()); - Prime::set(None); - } - fn set_prime(who: Option) { - Prime::set(who); - } - fn get_prime() -> Option { - Prime::get() - } - } - - impl InitializeMembers for TestChangeMembers { - fn initialize_members(members: &[u64]) { - MEMBERS.with(|m| *m.borrow_mut() = members.to_vec()); - } - } - - impl Config for Test { - type RuntimeEvent = RuntimeEvent; - type AddOrigin = EnsureSignedBy; - type RemoveOrigin = EnsureSignedBy; - type SwapOrigin = EnsureSignedBy; - type ResetOrigin = EnsureSignedBy; - type PrimeOrigin = EnsureSignedBy; - type MembershipInitialized = TestChangeMembers; - type MembershipChanged = TestChangeMembers; - type MaxMembers = ConstU32<10>; - type WeightInfo = (); - } - - pub(crate) fn new_test_ext() -> sp_io::TestExternalities { - let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - // We use default for brevity, but you can configure as desired if needed. - pallet_membership::GenesisConfig:: { - members: bounded_vec![10, 20, 30], - ..Default::default() - } - .assimilate_storage(&mut t) - .unwrap(); - t.into() - } - - #[cfg(feature = "runtime-benchmarks")] - pub(crate) fn new_bench_ext() -> sp_io::TestExternalities { - frame_system::GenesisConfig::::default().build_storage().unwrap().into() - } - - #[cfg(feature = "runtime-benchmarks")] - pub(crate) fn clean() { - Members::set(vec![]); - Prime::set(None); - } - - #[test] - fn query_membership_works() { - new_test_ext().execute_with(|| { - assert_eq!(crate::Members::::get(), vec![10, 20, 30]); - assert_eq!(MEMBERS.with(|m| m.borrow().clone()), vec![10, 20, 30]); - }); - } - - #[test] - fn prime_member_works() { - new_test_ext().execute_with(|| { - assert_noop!(Membership::set_prime(RuntimeOrigin::signed(4), 20), BadOrigin); - assert_noop!( - Membership::set_prime(RuntimeOrigin::signed(5), 15), - Error::::NotMember - ); - assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 20)); - assert_eq!(crate::Prime::::get(), Some(20)); - assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); - - assert_ok!(Membership::clear_prime(RuntimeOrigin::signed(5))); - assert_eq!(crate::Prime::::get(), None); - assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); - }); - } - - #[test] - fn add_member_works() { - new_test_ext().execute_with(|| { - assert_noop!(Membership::add_member(RuntimeOrigin::signed(5), 15), BadOrigin); - assert_noop!( - Membership::add_member(RuntimeOrigin::signed(1), 10), - Error::::AlreadyMember - ); - assert_ok!(Membership::add_member(RuntimeOrigin::signed(1), 15)); - assert_eq!(crate::Members::::get(), vec![10, 15, 20, 30]); - assert_eq!( - MEMBERS.with(|m| m.borrow().clone()), - crate::Members::::get().to_vec() - ); - }); - } - - #[test] - fn remove_member_works() { - new_test_ext().execute_with(|| { - assert_noop!(Membership::remove_member(RuntimeOrigin::signed(5), 20), BadOrigin); - assert_noop!( - Membership::remove_member(RuntimeOrigin::signed(2), 15), - Error::::NotMember - ); - assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 20)); - assert_ok!(Membership::remove_member(RuntimeOrigin::signed(2), 20)); - assert_eq!(crate::Members::::get(), vec![10, 30]); - assert_eq!( - MEMBERS.with(|m| m.borrow().clone()), - crate::Members::::get().to_vec() - ); - assert_eq!(crate::Prime::::get(), None); - assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); - }); - } - - #[test] - fn swap_member_works() { - new_test_ext().execute_with(|| { - assert_noop!(Membership::swap_member(RuntimeOrigin::signed(5), 10, 25), BadOrigin); - assert_noop!( - Membership::swap_member(RuntimeOrigin::signed(3), 15, 25), - Error::::NotMember - ); - assert_noop!( - Membership::swap_member(RuntimeOrigin::signed(3), 10, 30), - Error::::AlreadyMember - ); - - assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 20)); - assert_ok!(Membership::swap_member(RuntimeOrigin::signed(3), 20, 20)); - assert_eq!(crate::Members::::get(), vec![10, 20, 30]); - assert_eq!(crate::Prime::::get(), Some(20)); - assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); - - assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 10)); - assert_ok!(Membership::swap_member(RuntimeOrigin::signed(3), 10, 25)); - assert_eq!(crate::Members::::get(), vec![20, 25, 30]); - assert_eq!( - MEMBERS.with(|m| m.borrow().clone()), - crate::Members::::get().to_vec() - ); - assert_eq!(crate::Prime::::get(), None); - assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); - }); - } - - #[test] - fn swap_member_works_that_does_not_change_order() { - new_test_ext().execute_with(|| { - assert_ok!(Membership::swap_member(RuntimeOrigin::signed(3), 10, 5)); - assert_eq!(crate::Members::::get(), vec![5, 20, 30]); - assert_eq!( - MEMBERS.with(|m| m.borrow().clone()), - crate::Members::::get().to_vec() - ); - }); - } - - #[test] - fn swap_member_with_identical_arguments_changes_nothing() { - new_test_ext().execute_with(|| { - assert_storage_noop!(assert_ok!(Membership::swap_member( - RuntimeOrigin::signed(3), - 10, - 10 - ))); - }); - } - - #[test] - fn change_key_works() { - new_test_ext().execute_with(|| { - assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 10)); - assert_noop!( - Membership::change_key(RuntimeOrigin::signed(3), 25), - Error::::NotMember - ); - assert_noop!( - Membership::change_key(RuntimeOrigin::signed(10), 20), - Error::::AlreadyMember - ); - assert_ok!(Membership::change_key(RuntimeOrigin::signed(10), 40)); - assert_eq!(crate::Members::::get(), vec![20, 30, 40]); - assert_eq!( - MEMBERS.with(|m| m.borrow().clone()), - crate::Members::::get().to_vec() - ); - assert_eq!(crate::Prime::::get(), Some(40)); - assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); - }); - } - - #[test] - fn change_key_works_that_does_not_change_order() { - new_test_ext().execute_with(|| { - assert_ok!(Membership::change_key(RuntimeOrigin::signed(10), 5)); - assert_eq!(crate::Members::::get(), vec![5, 20, 30]); - assert_eq!( - MEMBERS.with(|m| m.borrow().clone()), - crate::Members::::get().to_vec() - ); - }); - } - - #[test] - fn change_key_with_same_caller_as_argument_changes_nothing() { - new_test_ext().execute_with(|| { - assert_storage_noop!(assert_ok!(Membership::change_key(RuntimeOrigin::signed(10), 10))); - }); - } - - #[test] - fn reset_members_works() { - new_test_ext().execute_with(|| { - assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 20)); - assert_noop!( - Membership::reset_members(RuntimeOrigin::signed(1), bounded_vec![20, 40, 30]), - BadOrigin - ); - - assert_ok!(Membership::reset_members(RuntimeOrigin::signed(4), vec![20, 40, 30])); - assert_eq!(crate::Members::::get(), vec![20, 30, 40]); - assert_eq!( - MEMBERS.with(|m| m.borrow().clone()), - crate::Members::::get().to_vec() - ); - assert_eq!(crate::Prime::::get(), Some(20)); - assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); - - assert_ok!(Membership::reset_members(RuntimeOrigin::signed(4), vec![10, 40, 30])); - assert_eq!(crate::Members::::get(), vec![10, 30, 40]); - assert_eq!( - MEMBERS.with(|m| m.borrow().clone()), - crate::Members::::get().to_vec() - ); - assert_eq!(crate::Prime::::get(), None); - assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); - }); - } - - #[test] - #[should_panic(expected = "Members cannot contain duplicate accounts.")] - fn genesis_build_panics_with_duplicate_members() { - pallet_membership::GenesisConfig:: { - members: bounded_vec![1, 2, 3, 1], - phantom: Default::default(), - } - .build_storage() - .unwrap(); - } - - #[test] - fn migration_v4() { - new_test_ext().execute_with(|| { - use frame_support::traits::PalletInfo; - let old_pallet_name = "OldMembership"; - let new_pallet_name = - ::PalletInfo::name::().unwrap(); - - frame_support::storage::migration::move_pallet( - new_pallet_name.as_bytes(), - old_pallet_name.as_bytes(), - ); - - StorageVersion::new(0).put::(); - - crate::migrations::v4::pre_migrate::(old_pallet_name, new_pallet_name); - crate::migrations::v4::migrate::(old_pallet_name, new_pallet_name); - crate::migrations::v4::post_migrate::(old_pallet_name, new_pallet_name); - }); - } -} diff --git a/substrate/frame/membership/src/mock.rs b/substrate/frame/membership/src/mock.rs new file mode 100644 index 000000000000..2746219e095f --- /dev/null +++ b/substrate/frame/membership/src/mock.rs @@ -0,0 +1,117 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// 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 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Test utilities + +use super::*; +use crate as pallet_membership; + +use sp_runtime::{bounded_vec, BuildStorage}; + +use frame_support::{derive_impl, ord_parameter_types, parameter_types, traits::ConstU32}; +use frame_system::EnsureSignedBy; + +type Block = frame_system::mocking::MockBlock; + +frame_support::construct_runtime!( + pub enum Test + { + System: frame_system, + Membership: pallet_membership, + } +); + +parameter_types! { + pub static Members: Vec = vec![]; + pub static Prime: Option = None; +} + +#[derive_impl(frame_system::config_preludes::TestDefaultConfig)] +impl frame_system::Config for Test { + type Block = Block; +} +ord_parameter_types! { + pub const One: u64 = 1; + pub const Two: u64 = 2; + pub const Three: u64 = 3; + pub const Four: u64 = 4; + pub const Five: u64 = 5; +} + +pub struct TestChangeMembers; +impl ChangeMembers for TestChangeMembers { + fn change_members_sorted(incoming: &[u64], outgoing: &[u64], new: &[u64]) { + let mut old_plus_incoming = Members::get(); + old_plus_incoming.extend_from_slice(incoming); + old_plus_incoming.sort(); + let mut new_plus_outgoing = new.to_vec(); + new_plus_outgoing.extend_from_slice(outgoing); + new_plus_outgoing.sort(); + assert_eq!(old_plus_incoming, new_plus_outgoing); + + Members::set(new.to_vec()); + Prime::set(None); + } + fn set_prime(who: Option) { + Prime::set(who); + } + fn get_prime() -> Option { + Prime::get() + } +} + +impl InitializeMembers for TestChangeMembers { + fn initialize_members(members: &[u64]) { + MEMBERS.with(|m| *m.borrow_mut() = members.to_vec()); + } +} + +impl Config for Test { + type RuntimeEvent = RuntimeEvent; + type AddOrigin = EnsureSignedBy; + type RemoveOrigin = EnsureSignedBy; + type SwapOrigin = EnsureSignedBy; + type ResetOrigin = EnsureSignedBy; + type PrimeOrigin = EnsureSignedBy; + type MembershipInitialized = TestChangeMembers; + type MembershipChanged = TestChangeMembers; + type MaxMembers = ConstU32<10>; + type WeightInfo = (); +} + +pub(crate) fn new_test_ext() -> sp_io::TestExternalities { + let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); + // We use default for brevity, but you can configure as desired if needed. + pallet_membership::GenesisConfig:: { + members: bounded_vec![10, 20, 30], + ..Default::default() + } + .assimilate_storage(&mut t) + .unwrap(); + t.into() +} + +#[cfg(feature = "runtime-benchmarks")] +pub(crate) fn new_bench_ext() -> sp_io::TestExternalities { + frame_system::GenesisConfig::::default().build_storage().unwrap().into() +} + +#[cfg(feature = "runtime-benchmarks")] +pub(crate) fn clean() { + Members::set(vec![]); + Prime::set(None); +} diff --git a/substrate/frame/membership/src/tests.rs b/substrate/frame/membership/src/tests.rs new file mode 100644 index 000000000000..7ab3e75c653f --- /dev/null +++ b/substrate/frame/membership/src/tests.rs @@ -0,0 +1,217 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// 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 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for the module. + +use crate as pallet_membership; +use crate::{mock::*, *}; + +use sp_runtime::{bounded_vec, traits::BadOrigin, BuildStorage}; + +use frame_support::{assert_noop, assert_ok, assert_storage_noop, traits::StorageVersion}; + +#[test] +fn query_membership_works() { + new_test_ext().execute_with(|| { + assert_eq!(crate::Members::::get(), vec![10, 20, 30]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), vec![10, 20, 30]); + }); +} + +#[test] +fn prime_member_works() { + new_test_ext().execute_with(|| { + assert_noop!(Membership::set_prime(RuntimeOrigin::signed(4), 20), BadOrigin); + assert_noop!( + Membership::set_prime(RuntimeOrigin::signed(5), 15), + Error::::NotMember + ); + assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 20)); + assert_eq!(crate::Prime::::get(), Some(20)); + assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); + + assert_ok!(Membership::clear_prime(RuntimeOrigin::signed(5))); + assert_eq!(crate::Prime::::get(), None); + assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); + }); +} + +#[test] +fn add_member_works() { + new_test_ext().execute_with(|| { + assert_noop!(Membership::add_member(RuntimeOrigin::signed(5), 15), BadOrigin); + assert_noop!( + Membership::add_member(RuntimeOrigin::signed(1), 10), + Error::::AlreadyMember + ); + assert_ok!(Membership::add_member(RuntimeOrigin::signed(1), 15)); + assert_eq!(crate::Members::::get(), vec![10, 15, 20, 30]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), crate::Members::::get().to_vec()); + }); +} + +#[test] +fn remove_member_works() { + new_test_ext().execute_with(|| { + assert_noop!(Membership::remove_member(RuntimeOrigin::signed(5), 20), BadOrigin); + assert_noop!( + Membership::remove_member(RuntimeOrigin::signed(2), 15), + Error::::NotMember + ); + assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 20)); + assert_ok!(Membership::remove_member(RuntimeOrigin::signed(2), 20)); + assert_eq!(crate::Members::::get(), vec![10, 30]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), crate::Members::::get().to_vec()); + assert_eq!(crate::Prime::::get(), None); + assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); + }); +} + +#[test] +fn swap_member_works() { + new_test_ext().execute_with(|| { + assert_noop!(Membership::swap_member(RuntimeOrigin::signed(5), 10, 25), BadOrigin); + assert_noop!( + Membership::swap_member(RuntimeOrigin::signed(3), 15, 25), + Error::::NotMember + ); + assert_noop!( + Membership::swap_member(RuntimeOrigin::signed(3), 10, 30), + Error::::AlreadyMember + ); + + assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 20)); + assert_ok!(Membership::swap_member(RuntimeOrigin::signed(3), 20, 20)); + assert_eq!(crate::Members::::get(), vec![10, 20, 30]); + assert_eq!(crate::Prime::::get(), Some(20)); + assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); + + assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 10)); + assert_ok!(Membership::swap_member(RuntimeOrigin::signed(3), 10, 25)); + assert_eq!(crate::Members::::get(), vec![20, 25, 30]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), crate::Members::::get().to_vec()); + assert_eq!(crate::Prime::::get(), None); + assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); + }); +} + +#[test] +fn swap_member_works_that_does_not_change_order() { + new_test_ext().execute_with(|| { + assert_ok!(Membership::swap_member(RuntimeOrigin::signed(3), 10, 5)); + assert_eq!(crate::Members::::get(), vec![5, 20, 30]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), crate::Members::::get().to_vec()); + }); +} + +#[test] +fn swap_member_with_identical_arguments_changes_nothing() { + new_test_ext().execute_with(|| { + assert_storage_noop!(assert_ok!(Membership::swap_member(RuntimeOrigin::signed(3), 10, 10))); + }); +} + +#[test] +fn change_key_works() { + new_test_ext().execute_with(|| { + assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 10)); + assert_noop!( + Membership::change_key(RuntimeOrigin::signed(3), 25), + Error::::NotMember + ); + assert_noop!( + Membership::change_key(RuntimeOrigin::signed(10), 20), + Error::::AlreadyMember + ); + assert_ok!(Membership::change_key(RuntimeOrigin::signed(10), 40)); + assert_eq!(crate::Members::::get(), vec![20, 30, 40]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), crate::Members::::get().to_vec()); + assert_eq!(crate::Prime::::get(), Some(40)); + assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); + }); +} + +#[test] +fn change_key_works_that_does_not_change_order() { + new_test_ext().execute_with(|| { + assert_ok!(Membership::change_key(RuntimeOrigin::signed(10), 5)); + assert_eq!(crate::Members::::get(), vec![5, 20, 30]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), crate::Members::::get().to_vec()); + }); +} + +#[test] +fn change_key_with_same_caller_as_argument_changes_nothing() { + new_test_ext().execute_with(|| { + assert_storage_noop!(assert_ok!(Membership::change_key(RuntimeOrigin::signed(10), 10))); + }); +} + +#[test] +fn reset_members_works() { + new_test_ext().execute_with(|| { + assert_ok!(Membership::set_prime(RuntimeOrigin::signed(5), 20)); + assert_noop!( + Membership::reset_members(RuntimeOrigin::signed(1), bounded_vec![20, 40, 30]), + BadOrigin + ); + + assert_ok!(Membership::reset_members(RuntimeOrigin::signed(4), vec![20, 40, 30])); + assert_eq!(crate::Members::::get(), vec![20, 30, 40]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), crate::Members::::get().to_vec()); + assert_eq!(crate::Prime::::get(), Some(20)); + assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); + + assert_ok!(Membership::reset_members(RuntimeOrigin::signed(4), vec![10, 40, 30])); + assert_eq!(crate::Members::::get(), vec![10, 30, 40]); + assert_eq!(MEMBERS.with(|m| m.borrow().clone()), crate::Members::::get().to_vec()); + assert_eq!(crate::Prime::::get(), None); + assert_eq!(PRIME.with(|m| *m.borrow()), crate::Prime::::get()); + }); +} + +#[test] +#[should_panic(expected = "Members cannot contain duplicate accounts.")] +fn genesis_build_panics_with_duplicate_members() { + pallet_membership::GenesisConfig:: { + members: bounded_vec![1, 2, 3, 1], + phantom: Default::default(), + } + .build_storage() + .unwrap(); +} + +#[test] +fn migration_v4() { + new_test_ext().execute_with(|| { + use frame_support::traits::PalletInfo; + let old_pallet_name = "OldMembership"; + let new_pallet_name = + ::PalletInfo::name::().unwrap(); + + frame_support::storage::migration::move_pallet( + new_pallet_name.as_bytes(), + old_pallet_name.as_bytes(), + ); + + StorageVersion::new(0).put::(); + + crate::migrations::v4::pre_migrate::(old_pallet_name, new_pallet_name); + crate::migrations::v4::migrate::(old_pallet_name, new_pallet_name); + crate::migrations::v4::post_migrate::(old_pallet_name, new_pallet_name); + }); +}