diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs index 05b3427ef431..a73d8a895d87 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs @@ -34,6 +34,7 @@ mod tracks; use super::*; use crate::xcm_config::{FellowshipAdminBodyId, LocationToAccountId, WndAssetHub}; use frame_support::traits::{EitherOf, MapSuccess, TryMapSuccess}; +use frame_system::EnsureRootWithSuccess; pub use origins::pallet_origins as pallet_ambassador_origins; use origins::pallet_origins::{ EnsureAmbassadorsVoice, EnsureAmbassadorsVoiceFrom, EnsureHeadAmbassadorsVoice, Origin, @@ -99,11 +100,17 @@ pub type PromoteOrigin = EitherOf< >, >; +/// Exchange is by any of: +/// - Root can exchange arbitrarily. +/// - the Fellows origin +pub type ExchangeOrigin = EitherOf>, Fellows>; + impl pallet_ranked_collective::Config for Runtime { type WeightInfo = weights::pallet_ranked_collective_ambassador_collective::WeightInfo; type RuntimeEvent = RuntimeEvent; type PromoteOrigin = PromoteOrigin; type DemoteOrigin = DemoteOrigin; + type ExchangeOrigin = ExchangeOrigin; type Polls = AmbassadorReferenda; type MinRankOfClass = sp_runtime::traits::Identity; type VoteWeight = pallet_ranked_collective::Linear; diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs index 273fa6a34150..0bdb0586d475 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs @@ -136,6 +136,11 @@ impl pallet_ranked_collective::Config for Runtime Replace>, >, >; + // Exchange is by any of: + // - Root can exchange arbitrarily. + // - the Fellows origin + type ExchangeOrigin = + EitherOf>, Fellows>; type Polls = FellowshipReferenda; type MinRankOfClass = tracks::MinRankOfClass; type VoteWeight = pallet_ranked_collective::Geometric; diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_ranked_collective_ambassador_collective.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_ranked_collective_ambassador_collective.rs index a6372c4b89dc..8f6aa583a41c 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_ranked_collective_ambassador_collective.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_ranked_collective_ambassador_collective.rs @@ -174,4 +174,7 @@ impl pallet_ranked_collective::WeightInfo for WeightInf .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(n.into()))) .saturating_add(Weight::from_parts(0, 2550).saturating_mul(n.into())) } + fn exchange_member() -> Weight { + todo!() + } } diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_ranked_collective_fellowship_collective.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_ranked_collective_fellowship_collective.rs index 9c773c56ac39..6dfe9b88ff63 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_ranked_collective_fellowship_collective.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_ranked_collective_fellowship_collective.rs @@ -173,4 +173,7 @@ impl pallet_ranked_collective::WeightInfo for WeightInf .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(n.into()))) .saturating_add(Weight::from_parts(0, 2550).saturating_mul(n.into())) } + fn exchange_member() -> Weight { + todo!() + } } diff --git a/polkadot/runtime/rococo/src/governance/fellowship.rs b/polkadot/runtime/rococo/src/governance/fellowship.rs index b5df6cf2df34..70477dd87ef5 100644 --- a/polkadot/runtime/rococo/src/governance/fellowship.rs +++ b/polkadot/runtime/rococo/src/governance/fellowship.rs @@ -337,6 +337,11 @@ impl pallet_ranked_collective::Config for Runtime TryMapSuccess>>, >, >; + // Exchange is by any of: + // - Root can exchange arbitrarily. + // - the Fellows origin; + type ExchangeOrigin = + EitherOf>, Fellows>; type Polls = FellowshipReferenda; type MinRankOfClass = sp_runtime::traits::Identity; type VoteWeight = pallet_ranked_collective::Geometric; diff --git a/polkadot/runtime/rococo/src/weights/pallet_ranked_collective.rs b/polkadot/runtime/rococo/src/weights/pallet_ranked_collective.rs index 8a556c3a248e..ce9d5fcc0c71 100644 --- a/polkadot/runtime/rococo/src/weights/pallet_ranked_collective.rs +++ b/polkadot/runtime/rococo/src/weights/pallet_ranked_collective.rs @@ -17,10 +17,10 @@ //! Autogenerated weights for `pallet_ranked_collective` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-07-11, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-01-24, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `runner-xerhrdyb-project-163-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` -//! EXECUTION: `Some(Wasm)`, WASM-EXECUTION: `Compiled`, CHAIN: `Some("rococo-dev")`, DB CACHE: 1024 +//! HOSTNAME: `runner-grjcggob-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("rococo-dev")`, DB CACHE: 1024 // Executed Command: // target/production/polkadot @@ -29,14 +29,13 @@ // --steps=50 // --repeat=20 // --extrinsic=* -// --execution=wasm // --wasm-execution=compiled // --heap-pages=4096 -// --json-file=/builds/parity/mirrors/polkadot/.git/.artifacts/bench.json +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json // --pallet=pallet_ranked_collective // --chain=rococo-dev -// --header=./file_header.txt -// --output=./runtime/rococo/src/weights/ +// --header=./polkadot/file_header.txt +// --output=./polkadot/runtime/rococo/src/weights/ #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -61,8 +60,8 @@ impl pallet_ranked_collective::WeightInfo for WeightInf // Proof Size summary in bytes: // Measured: `42` // Estimated: `3507` - // Minimum execution time: 17_632_000 picoseconds. - Weight::from_parts(18_252_000, 0) + // Minimum execution time: 13_480_000 picoseconds. + Weight::from_parts(13_786_000, 0) .saturating_add(Weight::from_parts(0, 3507)) .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(4)) @@ -71,24 +70,24 @@ impl pallet_ranked_collective::WeightInfo for WeightInf /// Proof: `FellowshipCollective::Members` (`max_values`: None, `max_size`: Some(42), added: 2517, mode: `MaxEncodedLen`) /// Storage: `FellowshipCollective::MemberCount` (r:11 w:11) /// Proof: `FellowshipCollective::MemberCount` (`max_values`: None, `max_size`: Some(14), added: 2489, mode: `MaxEncodedLen`) - /// Storage: `FellowshipCollective::IdToIndex` (r:11 w:11) + /// Storage: `FellowshipCollective::IdToIndex` (r:11 w:22) /// Proof: `FellowshipCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) - /// Storage: `FellowshipCollective::IndexToId` (r:11 w:11) + /// Storage: `FellowshipCollective::IndexToId` (r:11 w:22) /// Proof: `FellowshipCollective::IndexToId` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) /// The range of component `r` is `[0, 10]`. fn remove_member(r: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `517 + r * (281 ±0)` + // Measured: `516 + r * (281 ±0)` // Estimated: `3519 + r * (2529 ±0)` - // Minimum execution time: 27_960_000 picoseconds. - Weight::from_parts(30_632_408, 0) + // Minimum execution time: 28_771_000 picoseconds. + Weight::from_parts(29_256_825, 0) .saturating_add(Weight::from_parts(0, 3519)) - // Standard Error: 22_806 - .saturating_add(Weight::from_parts(13_000_901, 0).saturating_mul(r.into())) + // Standard Error: 21_594 + .saturating_add(Weight::from_parts(14_649_527, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(4)) .saturating_add(T::DbWeight::get().reads((3_u64).saturating_mul(r.into()))) - .saturating_add(T::DbWeight::get().writes(4)) - .saturating_add(T::DbWeight::get().writes((3_u64).saturating_mul(r.into()))) + .saturating_add(T::DbWeight::get().writes(6)) + .saturating_add(T::DbWeight::get().writes((5_u64).saturating_mul(r.into()))) .saturating_add(Weight::from_parts(0, 2529).saturating_mul(r.into())) } /// Storage: `FellowshipCollective::Members` (r:1 w:1) @@ -104,11 +103,11 @@ impl pallet_ranked_collective::WeightInfo for WeightInf // Proof Size summary in bytes: // Measured: `214 + r * (17 ±0)` // Estimated: `3507` - // Minimum execution time: 19_900_000 picoseconds. - Weight::from_parts(20_908_316, 0) + // Minimum execution time: 16_117_000 picoseconds. + Weight::from_parts(16_978_453, 0) .saturating_add(Weight::from_parts(0, 3507)) - // Standard Error: 4_878 - .saturating_add(Weight::from_parts(330_385, 0).saturating_mul(r.into())) + // Standard Error: 4_511 + .saturating_add(Weight::from_parts(324_261, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(4)) } @@ -116,22 +115,22 @@ impl pallet_ranked_collective::WeightInfo for WeightInf /// Proof: `FellowshipCollective::Members` (`max_values`: None, `max_size`: Some(42), added: 2517, mode: `MaxEncodedLen`) /// Storage: `FellowshipCollective::MemberCount` (r:1 w:1) /// Proof: `FellowshipCollective::MemberCount` (`max_values`: None, `max_size`: Some(14), added: 2489, mode: `MaxEncodedLen`) - /// Storage: `FellowshipCollective::IdToIndex` (r:1 w:1) + /// Storage: `FellowshipCollective::IdToIndex` (r:1 w:2) /// Proof: `FellowshipCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) - /// Storage: `FellowshipCollective::IndexToId` (r:1 w:1) + /// Storage: `FellowshipCollective::IndexToId` (r:1 w:2) /// Proof: `FellowshipCollective::IndexToId` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) /// The range of component `r` is `[0, 10]`. fn demote_member(r: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `532 + r * (72 ±0)` // Estimated: `3519` - // Minimum execution time: 27_697_000 picoseconds. - Weight::from_parts(30_341_815, 0) + // Minimum execution time: 28_995_000 picoseconds. + Weight::from_parts(31_343_215, 0) .saturating_add(Weight::from_parts(0, 3519)) - // Standard Error: 17_010 - .saturating_add(Weight::from_parts(642_213, 0).saturating_mul(r.into())) + // Standard Error: 16_438 + .saturating_add(Weight::from_parts(637_462, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(4)) - .saturating_add(T::DbWeight::get().writes(4)) + .saturating_add(T::DbWeight::get().writes(6)) } /// Storage: `FellowshipCollective::Members` (r:1 w:0) /// Proof: `FellowshipCollective::Members` (`max_values`: None, `max_size`: Some(42), added: 2517, mode: `MaxEncodedLen`) @@ -143,10 +142,10 @@ impl pallet_ranked_collective::WeightInfo for WeightInf /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(38963), added: 41438, mode: `MaxEncodedLen`) fn vote() -> Weight { // Proof Size summary in bytes: - // Measured: `638` + // Measured: `603` // Estimated: `83866` - // Minimum execution time: 48_275_000 picoseconds. - Weight::from_parts(49_326_000, 0) + // Minimum execution time: 38_820_000 picoseconds. + Weight::from_parts(40_240_000, 0) .saturating_add(Weight::from_parts(0, 83866)) .saturating_add(T::DbWeight::get().reads(5)) .saturating_add(T::DbWeight::get().writes(4)) @@ -160,16 +159,34 @@ impl pallet_ranked_collective::WeightInfo for WeightInf /// The range of component `n` is `[0, 100]`. fn cleanup_poll(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `434 + n * (50 ±0)` + // Measured: `400 + n * (50 ±0)` // Estimated: `4365 + n * (2540 ±0)` - // Minimum execution time: 15_506_000 picoseconds. - Weight::from_parts(17_634_029, 0) + // Minimum execution time: 12_972_000 picoseconds. + Weight::from_parts(15_829_333, 0) .saturating_add(Weight::from_parts(0, 4365)) - // Standard Error: 2_117 - .saturating_add(Weight::from_parts(1_126_879, 0).saturating_mul(n.into())) + // Standard Error: 1_754 + .saturating_add(Weight::from_parts(1_116_520, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(n.into()))) .saturating_add(Weight::from_parts(0, 2540).saturating_mul(n.into())) } + /// Storage: `FellowshipCollective::Members` (r:2 w:2) + /// Proof: `FellowshipCollective::Members` (`max_values`: None, `max_size`: Some(42), added: 2517, mode: `MaxEncodedLen`) + /// Storage: `FellowshipCollective::MemberCount` (r:2 w:2) + /// Proof: `FellowshipCollective::MemberCount` (`max_values`: None, `max_size`: Some(14), added: 2489, mode: `MaxEncodedLen`) + /// Storage: `FellowshipCollective::IdToIndex` (r:2 w:4) + /// Proof: `FellowshipCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) + /// Storage: `FellowshipCollective::IndexToId` (r:0 w:2) + /// Proof: `FellowshipCollective::IndexToId` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) + fn exchange_member() -> Weight { + // Proof Size summary in bytes: + // Measured: `337` + // Estimated: `6048` + // Minimum execution time: 44_601_000 picoseconds. + Weight::from_parts(45_714_000, 0) + .saturating_add(Weight::from_parts(0, 6048)) + .saturating_add(T::DbWeight::get().reads(6)) + .saturating_add(T::DbWeight::get().writes(10)) + } } diff --git a/prdoc/pr_2587.prdoc b/prdoc/pr_2587.prdoc new file mode 100644 index 000000000000..1ea23d5cca7b --- /dev/null +++ b/prdoc/pr_2587.prdoc @@ -0,0 +1,9 @@ +title: Allow fellowship members to swap their AccountIds. + +doc: + - audience: Runtime User + description: | + Add a `exchange_member` extrinsic to the `ranked-collective` pallet that allows to vote on + swapping the AccountId of a member. This can be used to recover lost access to a collective. + +crates: [ ] diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 92c08fb311a9..d0a327fd9502 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1016,6 +1016,7 @@ impl pallet_ranked_collective::Config for Runtime { type RuntimeEvent = RuntimeEvent; type PromoteOrigin = EnsureRootWithSuccess>; type DemoteOrigin = EnsureRootWithSuccess>; + type ExchangeOrigin = EnsureRootWithSuccess>; type Polls = RankedPolls; type MinRankOfClass = traits::Identity; type VoteWeight = pallet_ranked_collective::Geometric; diff --git a/substrate/frame/ranked-collective/src/benchmarking.rs b/substrate/frame/ranked-collective/src/benchmarking.rs index 518428880e4d..8d3eca98cc76 100644 --- a/substrate/frame/ranked-collective/src/benchmarking.rs +++ b/substrate/frame/ranked-collective/src/benchmarking.rs @@ -173,5 +173,20 @@ benchmarks_instance_pallet! { assert_eq!(Voting::::iter().count(), 0); } + exchange_member { + let who = make_member::(1); + let who_lookup = T::Lookup::unlookup(who.clone()); + let new_who = account::("new-member", 0, SEED); + let new_who_lookup = T::Lookup::unlookup(new_who.clone()); + let origin = + T::ExchangeOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let call = Call::::exchange_member { who: who_lookup, new_who:new_who_lookup}; + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert_eq!(Members::::get(&new_who).unwrap().rank, 1); + assert_eq!(Members::::get(&who), None); + assert_last_event::(Event::MemberExchanged { who, new_who }.into()); + } + impl_benchmark_test_suite!(RankedCollective, crate::tests::new_test_ext(), crate::tests::Test); } diff --git a/substrate/frame/ranked-collective/src/lib.rs b/substrate/frame/ranked-collective/src/lib.rs index 51ee7d7144b1..88631884c5a7 100644 --- a/substrate/frame/ranked-collective/src/lib.rs +++ b/substrate/frame/ranked-collective/src/lib.rs @@ -161,6 +161,7 @@ impl, I: 'static, M: GetMaxVoters>> crate::Pallet::::do_add_member_to_rank( who, T::MinRankOfClass::convert(class.clone()), + true, ) .expect("could not add members for benchmarks"); } @@ -299,7 +300,7 @@ impl, I: 'static> EnsureOriginWithArg for E #[cfg(feature = "runtime-benchmarks")] fn try_successful_origin(min_rank: &Rank) -> Result { let who = frame_benchmarking::account::("successful_origin", 0, 0); - crate::Pallet::::do_add_member_to_rank(who.clone(), *min_rank) + crate::Pallet::::do_add_member_to_rank(who.clone(), *min_rank, true) .expect("Could not add members for benchmarks"); Ok(frame_system::RawOrigin::Signed(who).into()) } @@ -352,7 +353,7 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin Result { let who = frame_benchmarking::account::("successful_origin", 0, 0); - crate::Pallet::::do_add_member_to_rank(who.clone(), MIN_RANK) + crate::Pallet::::do_add_member_to_rank(who.clone(), MIN_RANK, true) .expect("Could not add members for benchmarks"); Ok(frame_system::RawOrigin::Signed(who).into()) } @@ -390,6 +391,9 @@ pub mod pallet { /// maximum rank *from which* the demotion/removal may be. type DemoteOrigin: EnsureOrigin; + /// The origin that can swap the account of a member. + type ExchangeOrigin: EnsureOrigin; + /// The polling system used for our voting. type Polls: Polling, Votes = Votes, Moment = BlockNumberFor>; @@ -454,6 +458,8 @@ pub mod pallet { /// The member `who` has voted for the `poll` with the given `vote` leading to an updated /// `tally`. Voted { who: T::AccountId, poll: PollIndexOf, vote: VoteRecord, tally: TallyOf }, + /// The member `who` had their `AccountId` changed to `new_who`. + MemberExchanged { who: T::AccountId, new_who: T::AccountId }, } #[pallet::error] @@ -476,6 +482,8 @@ pub mod pallet { InvalidWitness, /// The origin is not sufficiently privileged to do the operation. NoPermission, + /// The new member to exchange is the same as the old member + SameMember, } #[pallet::call] @@ -492,7 +500,7 @@ pub mod pallet { pub fn add_member(origin: OriginFor, who: AccountIdLookupOf) -> DispatchResult { let _ = T::PromoteOrigin::ensure_origin(origin)?; let who = T::Lookup::lookup(who)?; - Self::do_add_member(who) + Self::do_add_member(who, true) } /// Increment the rank of an existing member by one. @@ -506,7 +514,7 @@ pub mod pallet { pub fn promote_member(origin: OriginFor, who: AccountIdLookupOf) -> DispatchResult { let max_rank = T::PromoteOrigin::ensure_origin(origin)?; let who = T::Lookup::lookup(who)?; - Self::do_promote_member(who, Some(max_rank)) + Self::do_promote_member(who, Some(max_rank), true) } /// Decrement the rank of an existing member by one. If the member is already at rank zero, @@ -544,10 +552,7 @@ pub mod pallet { ensure!(min_rank >= rank, Error::::InvalidWitness); ensure!(max_rank >= rank, Error::::NoPermission); - for r in 0..=rank { - Self::remove_from_rank(&who, r)?; - } - Members::::remove(&who); + Self::do_remove_member_from_rank(&who, rank)?; Self::deposit_event(Event::MemberRemoved { who, rank }); Ok(PostDispatchInfo { actual_weight: Some(T::WeightInfo::remove_member(rank as u32)), @@ -650,6 +655,33 @@ pub mod pallet { pays_fee: Pays::No, }) } + + /// Exchanges a member with a new account and the same existing rank. + /// + /// - `origin`: Must be the `ExchangeOrigin`. + /// - `who`: Account of existing member of rank greater than zero to be exchanged. + /// - `new_who`: New Account of existing member of rank greater than zero to exchanged to. + #[pallet::call_index(6)] + #[pallet::weight(T::WeightInfo::exchange_member())] + pub fn exchange_member( + origin: OriginFor, + who: AccountIdLookupOf, + new_who: AccountIdLookupOf, + ) -> DispatchResult { + T::ExchangeOrigin::ensure_origin(origin)?; + let who = T::Lookup::lookup(who)?; + let new_who = T::Lookup::lookup(new_who)?; + + ensure!(who != new_who, Error::::SameMember); + + let MemberRecord { rank, .. } = Self::ensure_member(&who)?; + + Self::do_remove_member_from_rank(&who, rank)?; + Self::do_add_member_to_rank(new_who.clone(), rank, false)?; + + Self::deposit_event(Event::MemberExchanged { who, new_who }); + Ok(()) + } } impl, I: 'static> Pallet { @@ -683,7 +715,7 @@ pub mod pallet { /// Adds a member into the ranked collective at level 0. /// /// No origin checks are executed. - pub fn do_add_member(who: T::AccountId) -> DispatchResult { + pub fn do_add_member(who: T::AccountId, emit_event: bool) -> DispatchResult { ensure!(!Members::::contains_key(&who), Error::::AlreadyMember); let index = MemberCount::::get(0); let count = index.checked_add(1).ok_or(Overflow)?; @@ -692,7 +724,9 @@ pub mod pallet { IdToIndex::::insert(0, &who, index); IndexToId::::insert(0, index, &who); MemberCount::::insert(0, count); - Self::deposit_event(Event::MemberAdded { who }); + if emit_event { + Self::deposit_event(Event::MemberAdded { who }); + } Ok(()) } @@ -703,6 +737,7 @@ pub mod pallet { pub fn do_promote_member( who: T::AccountId, maybe_max_rank: Option, + emit_event: bool, ) -> DispatchResult { let record = Self::ensure_member(&who)?; let rank = record.rank.checked_add(1).ok_or(Overflow)?; @@ -714,7 +749,9 @@ pub mod pallet { IdToIndex::::insert(rank, &who, index); IndexToId::::insert(rank, index, &who); Members::::insert(&who, MemberRecord { rank }); - Self::deposit_event(Event::RankChanged { who, rank }); + if emit_event { + Self::deposit_event(Event::RankChanged { who, rank }); + } Ok(()) } @@ -747,10 +784,14 @@ pub mod pallet { /// Add a member to the rank collective, and continue to promote them until a certain rank /// is reached. - pub fn do_add_member_to_rank(who: T::AccountId, rank: Rank) -> DispatchResult { - Self::do_add_member(who.clone())?; + pub fn do_add_member_to_rank( + who: T::AccountId, + rank: Rank, + emit_event: bool, + ) -> DispatchResult { + Self::do_add_member(who.clone(), emit_event)?; for _ in 0..rank { - Self::do_promote_member(who.clone(), None)?; + Self::do_promote_member(who.clone(), None, emit_event)?; } Ok(()) } @@ -763,6 +804,15 @@ pub mod pallet { use frame_support::traits::CallerTrait; o.as_signed().and_then(Self::rank_of) } + + /// Removes a member from the rank collective + pub fn do_remove_member_from_rank(who: &T::AccountId, rank: Rank) -> DispatchResult { + for r in 0..=rank { + Self::remove_from_rank(&who, r)?; + } + Members::::remove(&who); + Ok(()) + } } impl, I: 'static> RankedMembers for Pallet { @@ -778,11 +828,11 @@ pub mod pallet { } fn induct(who: &Self::AccountId) -> DispatchResult { - Self::do_add_member(who.clone()) + Self::do_add_member(who.clone(), true) } fn promote(who: &Self::AccountId) -> DispatchResult { - Self::do_promote_member(who.clone(), None) + Self::do_promote_member(who.clone(), None, true) } fn demote(who: &Self::AccountId) -> DispatchResult { diff --git a/substrate/frame/ranked-collective/src/tests.rs b/substrate/frame/ranked-collective/src/tests.rs index b3605169339a..f5e83c48acd0 100644 --- a/substrate/frame/ranked-collective/src/tests.rs +++ b/substrate/frame/ranked-collective/src/tests.rs @@ -164,6 +164,12 @@ impl Config for Test { // Members can demote up to the rank of 3 below them. MapSuccess, ReduceBy>>, >; + type ExchangeOrigin = EitherOf< + // Root can exchange arbitrarily. + frame_system::EnsureRootWithSuccess>, + // Members can exchange up to the rank of 2 below them. + MapSuccess, ReduceBy>>, + >; type Polls = TestPolls; type MinRankOfClass = MinRankOfClass; type VoteWeight = Geometric; @@ -516,8 +522,8 @@ fn ensure_ranked_works() { fn do_add_member_to_rank_works() { new_test_ext().execute_with(|| { let max_rank = 9u16; - assert_ok!(Club::do_add_member_to_rank(69, max_rank / 2)); - assert_ok!(Club::do_add_member_to_rank(1337, max_rank)); + assert_ok!(Club::do_add_member_to_rank(69, max_rank / 2, true)); + assert_ok!(Club::do_add_member_to_rank(1337, max_rank, true)); for i in 0..=max_rank { if i <= max_rank / 2 { assert_eq!(member_count(i), 2); @@ -568,3 +574,31 @@ fn tally_support_correct() { MinRankOfClassDelta::set(0); }); } + +#[test] +fn exchange_member_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_eq!(member_count(0), 1); + + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); + + let member_record = MemberRecord { rank: 1 }; + assert_eq!(Members::::get(1), Some(member_record.clone())); + assert_eq!(Members::::get(2), None); + + assert_ok!(Club::exchange_member(RuntimeOrigin::root(), 1, 2)); + assert_eq!(member_count(0), 1); + + assert_eq!(Members::::get(1), None); + assert_eq!(Members::::get(2), Some(member_record)); + + assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + + assert_noop!( + Club::exchange_member(RuntimeOrigin::signed(3), 2, 1), + DispatchError::BadOrigin + ); + }); +} diff --git a/substrate/frame/ranked-collective/src/weights.rs b/substrate/frame/ranked-collective/src/weights.rs index 9f1a0a818044..4ff0c3337d50 100644 --- a/substrate/frame/ranked-collective/src/weights.rs +++ b/substrate/frame/ranked-collective/src/weights.rs @@ -58,6 +58,7 @@ pub trait WeightInfo { fn demote_member(r: u32, ) -> Weight; fn vote() -> Weight; fn cleanup_poll(n: u32, ) -> Weight; + fn exchange_member() -> Weight; } /// Weights for pallet_ranked_collective using the Substrate node and recommended hardware. @@ -180,6 +181,25 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(n.into()))) .saturating_add(Weight::from_parts(0, 2540).saturating_mul(n.into())) } + + /// Storage: `RankedCollective::Members` (r:2 w:2) + /// Proof: `RankedCollective::Members` (`max_values`: None, `max_size`: Some(42), added: 2517, mode: `MaxEncodedLen`) + /// Storage: `RankedCollective::MemberCount` (r:2 w:2) + /// Proof: `RankedCollective::MemberCount` (`max_values`: None, `max_size`: Some(14), added: 2489, mode: `MaxEncodedLen`) + /// Storage: `RankedCollective::IdToIndex` (r:2 w:2) + /// Proof: `RankedCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) + /// Storage: `RankedCollective::IndexToId` (r:0 w:2) + /// Proof: `RankedCollective::IndexToId` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) + fn exchange_member() -> Weight { + // Proof Size summary in bytes: + // Measured: `437` + // Estimated: `6048` + // Minimum execution time: 138_000_000 picoseconds. + Weight::from_parts(141_000_000, 0) + .saturating_add(Weight::from_parts(0, 6048)) + .saturating_add(T::DbWeight::get().reads(6)) + .saturating_add(T::DbWeight::get().writes(8)) + } } // For backwards compatibility and tests @@ -301,4 +321,23 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(n.into()))) .saturating_add(Weight::from_parts(0, 2540).saturating_mul(n.into())) } + + /// Storage: `RankedCollective::Members` (r:2 w:2) + /// Proof: `RankedCollective::Members` (`max_values`: None, `max_size`: Some(42), added: 2517, mode: `MaxEncodedLen`) + /// Storage: `RankedCollective::MemberCount` (r:2 w:2) + /// Proof: `RankedCollective::MemberCount` (`max_values`: None, `max_size`: Some(14), added: 2489, mode: `MaxEncodedLen`) + /// Storage: `RankedCollective::IdToIndex` (r:2 w:2) + /// Proof: `RankedCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) + /// Storage: `RankedCollective::IndexToId` (r:0 w:2) + /// Proof: `RankedCollective::IndexToId` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) + fn exchange_member() -> Weight { + // Proof Size summary in bytes: + // Measured: `437` + // Estimated: `6048` + // Minimum execution time: 138_000_000 picoseconds. + Weight::from_parts(141_000_000, 0) + .saturating_add(Weight::from_parts(0, 6048)) + .saturating_add(RocksDbWeight::get().reads(6)) + .saturating_add(RocksDbWeight::get().writes(8)) + } }