From 1798b603f620eb698e4cd6bc2f4bfb630b122c83 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 18 May 2022 15:56:26 +0200 Subject: [PATCH 01/25] Ranked Collective pallet --- Cargo.toml | 1 + frame/ranked-collective/Cargo.toml | 49 +++ frame/ranked-collective/README.md | 25 ++ frame/ranked-collective/src/benchmarking.rs | 85 ++++ frame/ranked-collective/src/lib.rs | 431 ++++++++++++++++++++ frame/ranked-collective/src/tests.rs | 287 +++++++++++++ frame/ranked-collective/src/weights.rs | 326 +++++++++++++++ 7 files changed, 1204 insertions(+) create mode 100644 frame/ranked-collective/Cargo.toml create mode 100644 frame/ranked-collective/README.md create mode 100644 frame/ranked-collective/src/benchmarking.rs create mode 100644 frame/ranked-collective/src/lib.rs create mode 100644 frame/ranked-collective/src/tests.rs create mode 100644 frame/ranked-collective/src/weights.rs diff --git a/Cargo.toml b/Cargo.toml index 41739fe6f1ebc..74e7aae7949c5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,6 +115,7 @@ members = [ "frame/nomination-pools", "frame/nomination-pools/benchmarking", "frame/randomness-collective-flip", + "frame/ranked-collective", "frame/recovery", "frame/referenda", "frame/remark", diff --git a/frame/ranked-collective/Cargo.toml b/frame/ranked-collective/Cargo.toml new file mode 100644 index 0000000000000..cb43b9ea4c831 --- /dev/null +++ b/frame/ranked-collective/Cargo.toml @@ -0,0 +1,49 @@ +[package] +name = "pallet-ranked-collective" +version = "4.0.0-dev" +authors = ["Parity Technologies "] +edition = "2021" +license = "Apache-2.0" +homepage = "https://substrate.io" +repository = "https://github.com/paritytech/substrate/" +description = "Ranked collective system: Members of a set of account IDs can make their collective feelings known through dispatched calls from one of two specialized origins." +readme = "README.md" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } +log = { version = "0.4.16", default-features = false } +scale-info = { version = "2.0.1", default-features = false, features = ["derive"] } +frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" } +frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } +frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } +sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../primitives/arithmetic" } +sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } +sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } +sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } +sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } + +[features] +default = ["std"] +std = [ + "codec/std", + "frame-benchmarking/std", + "frame-support/std", + "frame-system/std", + "log/std", + "scale-info/std", + "sp-arithmetic/std", + "sp-core/std", + "sp-io/std", + "sp-runtime/std", + "sp-std/std", +] +runtime-benchmarks = [ + "frame-benchmarking/runtime-benchmarks", + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks", +] +try-runtime = ["frame-support/try-runtime"] diff --git a/frame/ranked-collective/README.md b/frame/ranked-collective/README.md new file mode 100644 index 0000000000000..444927e51da22 --- /dev/null +++ b/frame/ranked-collective/README.md @@ -0,0 +1,25 @@ +Collective system: Members of a set of account IDs can make their collective feelings known +through dispatched calls from one of two specialized origins. + +The membership can be provided in one of two ways: either directly, using the Root-dispatchable +function `set_members`, or indirectly, through implementing the `ChangeMembers`. +The pallet assumes that the amount of members stays at or below `MaxMembers` for its weight +calculations, but enforces this neither in `set_members` nor in `change_members_sorted`. + +A "prime" member may be set to help determine the default vote behavior based on chain +config. If `PrimeDefaultVote` is used, the prime vote acts as the default vote in case of any +abstentions after the voting period. If `MoreThanMajorityThenPrimeDefaultVote` is used, then +abstentations will first follow the majority of the collective voting, and then the prime +member. + +Voting happens through motions comprising a proposal (i.e. a dispatchable) plus a +number of approvals required for it to pass and be called. Motions are open for members to +vote on for a minimum period given by `MotionDuration`. As soon as the required number of +approvals is given, the motion is closed and executed. If the number of approvals is not reached +during the voting period, then `close` may be called by any account in order to force the end +the motion explicitly. If a prime member is defined, then their vote is used instead of any +abstentions and the proposal is executed if there are enough approvals counting the new votes. + +If there are not, or if no prime member is set, then the motion is dropped without being executed. + +License: Apache-2.0 diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs new file mode 100644 index 0000000000000..4d972e211fea9 --- /dev/null +++ b/frame/ranked-collective/src/benchmarking.rs @@ -0,0 +1,85 @@ +// This file is part of Substrate. + +// Copyright (C) 2020-2022 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. + +//! Staking pallet benchmarking. + +use super::*; +use crate::Pallet as Collective; + +use sp_runtime::traits::Bounded; +use sp_std::mem::size_of; + +use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller}; +use frame_support::dispatch::UnfilteredDispatchable; +use frame_system::{Call as SystemCall, Pallet as System, RawOrigin as SystemOrigin}; + +const SEED: u32 = 0; + +const MAX_BYTES: u32 = 1_024; + +fn assert_last_event, I: 'static>(generic_event: >::Event) { + frame_system::Pallet::::assert_last_event(generic_event.into()); +} + +fn make_member, I: 'static>(rank: Rank) -> T::AccountId { + let who = account::("member", MemberCount::::get().0, SEED); + Pallet::::add_member(T::AdminOrigin::successful_origin(), who.clone(), rank); + who +} + +benchmarks_instance_pallet! { + add_member { + let old = MemberCount::::get().0; + let who = account::("member", 0, SEED); + let rank = 1; + let origin = T::AdminOrigin::successful_origin(); + let call = Call::::add_member { who: who.clone(), rank }; + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert_eq!(MemberCount::::get().0, old + 1); + assert_last_event::(Event::MemberAdded { who, rank }.into()); + } + + remove_member { + let rank = 1; + let who = make_member::(rank); + let other = make_member::(rank); + let old = MemberCount::::get().0; + let other_index = Members::::get(&other).unwrap().index; + let origin = T::AdminOrigin::successful_origin(); + let call = Call::::remove_member { who: who.clone() }; + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert_eq!(MemberCount::::get().0, old - 1); + assert_ne!(other_index, Members::::get(&other).unwrap().index); + assert_last_event::(Event::MemberRemoved { who }.into()); + } + + set_member_rank { + let old_rank = 1; + let rank = 2; + let who = make_member::(old_rank); + let origin = T::AdminOrigin::successful_origin(); + let call = Call::::set_member_rank { who: who.clone(), rank }; + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert_eq!(Members::::get(&who).unwrap().rank, rank); + assert_last_event::(Event::RankChanged { who, rank }.into()); + } + + impl_benchmark_test_suite!(Collective, crate::tests::new_test_ext(), crate::tests::Test); +} diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs new file mode 100644 index 0000000000000..ef23cdf648c28 --- /dev/null +++ b/frame/ranked-collective/src/lib.rs @@ -0,0 +1,431 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 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. + +//! Ranked collective system: Members of a set of account IDs can make their collective feelings +//! known through dispatched calls from one of two specialized origins. +//! +//! The membership can be provided in one of two ways: either directly, using the Root-dispatchable +//! function `set_members`, or indirectly, through implementing the `ChangeMembers`. +//! The pallet assumes that the amount of members stays at or below `MaxMembers` for its weight +//! calculations, but enforces this neither in `set_members` nor in `change_members_sorted`. +//! +//! A "prime" member may be set to help determine the default vote behavior based on chain +//! config. If `PrimeDefaultVote` is used, the prime vote acts as the default vote in case of any +//! abstentions after the voting period. If `MoreThanMajorityThenPrimeDefaultVote` is used, then +//! abstentions will first follow the majority of the collective voting, and then the prime +//! member. +//! +//! Voting happens through motions comprising a proposal (i.e. a curried dispatchable) plus a +//! number of approvals required for it to pass and be called. Motions are open for members to +//! vote on for a minimum period given by `MotionDuration`. As soon as the needed number of +//! approvals is given, the motion is closed and executed. If the number of approvals is not reached +//! during the voting period, then `close` may be called by any account in order to force the end +//! the motion explicitly. If a prime member is defined then their vote is used in place of any +//! abstentions and the proposal is executed if there are enough approvals counting the new votes. +//! +//! If there are not, or if no prime is set, then the motion is dropped without being executed. + +#![cfg_attr(not(feature = "std"), no_std)] +#![recursion_limit = "128"] + +use scale_info::TypeInfo; +use sp_arithmetic::traits::Saturating; +use sp_runtime::{ + ArithmeticError::{Overflow, Underflow}, + Perbill, RuntimeDebug, +}; +use sp_std::{marker::PhantomData, prelude::*}; + +use frame_support::{ + codec::{Decode, Encode, MaxEncodedLen}, + dispatch::{DispatchError, DispatchResultWithPostInfo}, + ensure, + traits::{EnsureOrigin, Get, PollStatus, Polling, VoteTally}, + CloneNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, +}; + +#[cfg(test)] +mod tests; + +#[cfg(feature = "runtime-benchmarks")] +mod benchmarking; +pub mod weights; + +pub use pallet::*; +pub use weights::WeightInfo; + +/// A number of members. +pub type MemberIndex = u32; + +/// Member rank. +pub type Rank = u16; + +/// Votes. +pub type Votes = u32; + +/// Aggregated votes for an ongoing poll. +#[derive( + CloneNoBound, + DefaultNoBound, + PartialEqNoBound, + EqNoBound, + RuntimeDebugNoBound, + TypeInfo, + Encode, + Decode, + MaxEncodedLen, +)] +#[scale_info(skip_type_params(M))] +pub struct Tally> { + ayes: Votes, + nays: Votes, + dummy: PhantomData, +} + +impl> Tally { + fn from_parts(ayes: Votes, nays: Votes) -> Self { + Tally { ayes, nays, dummy: PhantomData } + } +} + +pub type TallyOf = Tally>; +pub type PollIndexOf = <>::Polls as Polling>>::Index; + +impl> VoteTally for Tally { + fn ayes(&self) -> Votes { + self.ayes + } + fn support(&self) -> Perbill { + Perbill::from_rational(self.ayes, M::get()) + } + fn approval(&self) -> Perbill { + Perbill::from_rational(self.ayes, 1.max(self.ayes + self.nays)) + } + #[cfg(feature = "runtime-benchmarks")] + fn unanimity() -> Self { + Self { ayes: M::get(), nays: 0, dummy: PhantomData } + } + #[cfg(feature = "runtime-benchmarks")] + fn rejection() -> Self { + Self { ayes: 0, nays: M::get(), dummy: PhantomData } + } + #[cfg(feature = "runtime-benchmarks")] + fn from_requirements(support: Perbill, approval: Perbill) -> Self { + let c = M::get(); + let ayes = support * c; + let nays = ((ayes as u64) * 1_000_000_000u64 / approval.deconstruct() as u64) as u32 - ayes; + Self { ayes, nays, dummy: PhantomData } + } +} + +/// Record needed for every member. +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] +pub struct MemberRecord { + /// The index of the member. + index: MemberIndex, + /// The rank of the member. + rank: Rank, +} + +/// Record needed for every vote. +#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug, TypeInfo)] +pub enum VoteRecord { + /// Vote was an aye with given vote weight. + Aye(Votes), + /// Vote was a nay with given vote weight. + Nay(Votes), +} + +impl From<(bool, Votes)> for VoteRecord { + fn from((aye, votes): (bool, Votes)) -> Self { + match aye { + true => VoteRecord::Aye(votes), + false => VoteRecord::Nay(votes), + } + } +} + +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + #[pallet::without_storage_info] + pub struct Pallet(PhantomData<(T, I)>); + + #[pallet::config] + pub trait Config: frame_system::Config { + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; + + /// The outer event type. + type Event: From> + IsType<::Event>; + + /// The origin required to add, promote or remove a member. + type AdminOrigin: EnsureOrigin; + + /// The polling system used for our voting. + type Polls: Polling, Votes = Votes, Moment = Self::BlockNumber>; + } + + /// The number of members in the collective. + #[pallet::storage] + pub type MemberCount, I: 'static = ()> = + StorageValue<_, (MemberIndex, Votes), ValueQuery>; + + /// The current members of the collective. + #[pallet::storage] + pub type Members, I: 'static = ()> = + StorageMap<_, Twox64Concat, T::AccountId, MemberRecord>; + + /// The members in the collective by index. All indices in the range `0..MemberCount` will + /// return `Some`, however a member's index is not guaranteed to remain unchanged over time. + #[pallet::storage] + pub type MemberByIndex, I: 'static = ()> = + StorageMap<_, Twox64Concat, MemberIndex, T::AccountId>; + + /// Votes on a given proposal, if it is ongoing. + #[pallet::storage] + pub type Voting, I: 'static = ()> = StorageDoubleMap< + _, + Blake2_128Concat, + PollIndexOf, + Twox64Concat, + T::AccountId, + VoteRecord, + >; + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event, I: 'static = ()> { + /// A member has been added. + MemberAdded { who: T::AccountId, rank: Rank }, + /// A member's rank has been changed. + RankChanged { who: T::AccountId, rank: Rank }, + /// A member has been removed. + MemberRemoved { who: T::AccountId }, + /// A motion (given hash) has been voted on by given account, leaving + /// a tally (yes votes and no votes given respectively as `MemberIndex`). + Voted { who: T::AccountId, poll: PollIndexOf, vote: VoteRecord, tally: TallyOf }, + } + + #[pallet::error] + pub enum Error { + /// Account is already a member. + AlreadyMember, + /// Account is not a member. + NotMember, + /// The given poll index is unknown or has closed. + NotPolling, + /// The given poll is still ongoing. + Ongoing, + /// There are no further records to be removed. + NoneRemaining, + /// Unexpected error in state. + Corruption, + } + + #[pallet::call] + impl, I: 'static> Pallet { + /// Introduce a new member. + /// + /// - `origin`: Must be the `AdminOrigin`. + /// - `who`: Account of non-member which will become a member. + /// - `rank`: The rank to give the new member. + /// + /// Weight: `O(1)` + #[pallet::weight(T::BlockWeights::get().max_block / 10)] + pub fn add_member(origin: OriginFor, who: T::AccountId, rank: Rank) -> DispatchResult { + T::AdminOrigin::ensure_origin(origin)?; + ensure!(!Members::::contains_key(&who), Error::::AlreadyMember); + let (index, mut votes) = MemberCount::::get(); + let count = index.checked_add(1).ok_or(Overflow)?; + votes = votes.checked_add(Self::rank_to_votes(rank)).ok_or(Overflow)?; + + Members::::insert(&who, MemberRecord { rank, index }); + MemberByIndex::::insert(index, &who); + MemberCount::::put((count, votes)); + Self::deposit_event(Event::MemberAdded { who, rank }); + + Ok(()) + } + + /// Alter the rank of an existing member. + /// + /// - `origin`: Must be the `AdminOrigin`. + /// - `who`: Account of existing member. + /// - `rank`: The new rank to give the member. + /// + /// Weight: `O(1)` + #[pallet::weight(T::BlockWeights::get().max_block / 10)] + pub fn set_member_rank( + origin: OriginFor, + who: T::AccountId, + rank: Rank, + ) -> DispatchResult { + T::AdminOrigin::ensure_origin(origin)?; + let mut record = Self::ensure_member(&who)?; + let (count, mut votes) = MemberCount::::get(); + votes = votes + .checked_sub(Self::rank_to_votes(record.rank)) + .ok_or(Underflow)? + .checked_add(Self::rank_to_votes(rank)) + .ok_or(Overflow)?; + record.rank = rank; + + MemberCount::::put((count, votes)); + Members::::insert(&who, record); + Self::deposit_event(Event::RankChanged { who, rank }); + + Ok(()) + } + + /// Remove a member. + /// + /// - `origin`: Must be the `AdminOrigin`. + /// - `who`: Account of existing member to be removed. + /// + /// Weight: `O(1)`, less if the member's index is highest. + #[pallet::weight(T::BlockWeights::get().max_block / 10)] + pub fn remove_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { + T::AdminOrigin::ensure_origin(origin)?; + let record = Self::ensure_member(&who)?; + let (count, votes) = MemberCount::::get(); + let count = count.checked_sub(1).ok_or(Underflow)?; + let votes = votes.checked_sub(Self::rank_to_votes(record.rank)).ok_or(Underflow)?; + + let index = record.index; + if index != count { + let last = MemberByIndex::::get(count).ok_or(Error::::Corruption)?; + Members::::mutate(&last, |r| { + if let Some(ref mut r) = r { + r.index = index + } + }); + MemberByIndex::::insert(index, &last); + } + MemberByIndex::::remove(count); + Members::::remove(&who); + MemberCount::::put((count, votes)); + Self::deposit_event(Event::MemberRemoved { who }); + + Ok(()) + } + + /// Add an aye or nay vote for the sender to the given proposal. + /// + /// - `origin`: Must be `Signed` by a member account. + /// - `poll`: Index of a poll which is ongoing. + /// - `aye`: `true` if the vote is to approve the proposal, `false` otherwise. + /// + /// Transaction fees are be waived if the member is voting on any particular proposal + /// for the first time and the call is successful. Subsequent vote changes will charge a + /// fee. + /// + /// Weight: `O(1)`, less if there was no previous vote on the poll by the member. + #[pallet::weight(T::BlockWeights::get().max_block / 10)] + pub fn vote( + origin: OriginFor, + poll: PollIndexOf, + aye: bool, + ) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin)?; + let record = Self::ensure_member(&who)?; + use VoteRecord::*; + let votes = Self::rank_to_votes(record.rank); + let vote = VoteRecord::from((aye, votes)); + let mut pays = Pays::Yes; + + T::Polls::try_access_poll(poll, |mut status| -> DispatchResult { + match status { + PollStatus::None | PollStatus::Completed(..) => Err(Error::::NotPolling)?, + PollStatus::Ongoing(ref mut tally, _) => { + match Voting::::get(&poll, &who) { + Some(Aye(votes)) => tally.ayes.saturating_reduce(votes), + Some(Nay(votes)) => tally.nays.saturating_reduce(votes), + None => pays = Pays::No, + } + match aye { + true => tally.ayes.saturating_accrue(votes), + false => tally.nays.saturating_accrue(votes), + } + Voting::::insert(&poll, &who, &vote); + Self::deposit_event(Event::Voted { who, poll, vote, tally: tally.clone() }); + Ok(()) + }, + } + })?; + Ok(pays.into()) + } + + /// Remove votes from the given poll. It must have ended. + /// + /// - `origin`: Must be `Signed` by any account. + /// - `poll_index`: Index of a poll which is completed and for which votes continue to + /// exist. + /// - `max`: Maximum number of vote items from remove in this call. + /// + /// Transaction fees are waived if the operation is successful. + /// + /// Weight `O(max)` (less if there are fewer items to remove than `max`). + #[pallet::weight(T::BlockWeights::get().max_block / 10)] + pub fn cleanup_poll( + origin: OriginFor, + poll_index: PollIndexOf, + max: u32, + ) -> DispatchResultWithPostInfo { + ensure_signed(origin)?; + ensure!(T::Polls::as_ongoing(poll_index).is_none(), Error::::Ongoing); + + use sp_io::KillStorageResult::*; + let _count = match Voting::::remove_prefix(poll_index, Some(max)) { + AllRemoved(0) => Err(Error::::NoneRemaining)?, + AllRemoved(n) | SomeRemaining(n) => n, + }; + // TODO: weight from count. + Ok(Pays::No.into()) + } + } + + impl, I: 'static> Pallet { + fn ensure_member(who: &T::AccountId) -> Result { + Members::::get(who).ok_or(Error::::NotMember.into()) + } + + fn rank_to_votes(r: Rank) -> Votes { + let r = r as Votes; + r * (r + 1) / 2 + } + + pub(crate) fn member_count() -> MemberIndex { + MemberCount::::get().0 + } + + pub(crate) fn max_turnout() -> Votes { + MemberCount::::get().1 + } + } + + impl, I: 'static> Get for Pallet { + fn get() -> Votes { + MemberCount::::get().1 + } + } +} diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs new file mode 100644 index 0000000000000..6b559bc1064fd --- /dev/null +++ b/frame/ranked-collective/src/tests.rs @@ -0,0 +1,287 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 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. + +//! The crate's tests. + +use std::collections::BTreeMap; + +use frame_support::{ + assert_noop, assert_ok, parameter_types, + traits::{ConstU32, ConstU64, Everything, Polling}, +}; +use sp_core::H256; +use sp_runtime::{ + testing::Header, + traits::{BlakeTwo256, IdentityLookup}, +}; + +use super::*; +use crate as pallet_ranked_collective; + +type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; +type Block = frame_system::mocking::MockBlock; + +frame_support::construct_runtime!( + pub enum Test where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic, + { + System: frame_system::{Pallet, Call, Config, Storage, Event}, + Club: pallet_ranked_collective::{Pallet, Call, Storage, Event}, + } +); + +parameter_types! { + pub BlockWeights: frame_system::limits::BlockWeights = + frame_system::limits::BlockWeights::simple_max(1_000_000); +} +impl frame_system::Config for Test { + type BaseCallFilter = Everything; + type BlockWeights = (); + type BlockLength = (); + type DbWeight = (); + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Call = Call; + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type Event = Event; + type BlockHashCount = ConstU64<250>; + type Version = (); + type PalletInfo = PalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type SS58Prefix = (); + type OnSetCode = (); + type MaxConsumers = ConstU32<16>; +} + +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum TestPollState { + Ongoing(TallyOf, u8), + Completed(u64, bool), +} +use TestPollState::*; + +parameter_types! { + pub static Polls: BTreeMap = vec![ + (1, Completed(1, true)), + (2, Completed(2, false)), + (3, Ongoing(Tally::from_parts(0, 0), 0)), + ].into_iter().collect(); +} + +pub struct TestPolls; +impl Polling> for TestPolls { + type Index = u8; + type Votes = Votes; + type Moment = u64; + type Class = u8; + fn classes() -> Vec { + vec![0, 1, 2] + } + fn as_ongoing(index: u8) -> Option<(TallyOf, Self::Class)> { + Polls::get().remove(&index).and_then(|x| { + if let TestPollState::Ongoing(t, c) = x { + Some((t, c)) + } else { + None + } + }) + } + fn access_poll( + index: Self::Index, + f: impl FnOnce(PollStatus<&mut TallyOf, Self::Moment, Self::Class>) -> R, + ) -> R { + let mut polls = Polls::get(); + let entry = polls.get_mut(&index); + let r = match entry { + Some(Ongoing(ref mut tally_mut_ref, class)) => + f(PollStatus::Ongoing(tally_mut_ref, *class)), + Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)), + None => f(PollStatus::None), + }; + Polls::set(polls); + r + } + fn try_access_poll( + index: Self::Index, + f: impl FnOnce( + PollStatus<&mut TallyOf, Self::Moment, Self::Class>, + ) -> Result, + ) -> Result { + let mut polls = Polls::get(); + let entry = polls.get_mut(&index); + let r = match entry { + Some(Ongoing(ref mut tally_mut_ref, class)) => + f(PollStatus::Ongoing(tally_mut_ref, *class)), + Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)), + None => f(PollStatus::None), + }?; + Polls::set(polls); + Ok(r) + } + + #[cfg(feature = "runtime-benchmarks")] + fn create_ongoing(class: Self::Class) -> Result { + let mut polls = Polls::get(); + let i = polls.keys().rev().next().map_or(0, |x| x + 1); + polls.insert(i, Ongoing(Tally::default(), class)); + Polls::set(polls); + Ok(i) + } + + #[cfg(feature = "runtime-benchmarks")] + fn end_ongoing(index: Self::Index, approved: bool) -> Result<(), ()> { + let mut polls = Polls::get(); + match polls.get(&index) { + Some(Ongoing(..)) => {}, + _ => return Err(()), + } + let now = frame_system::Pallet::::block_number(); + polls.insert(index, Completed(now, approved)); + Polls::set(polls); + Ok(()) + } +} + +impl Config for Test { + type WeightInfo = (); + type Event = Event; + type AdminOrigin = frame_system::EnsureRoot; + type Polls = TestPolls; +} + +pub fn new_test_ext() -> sp_io::TestExternalities { + let t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + let mut ext = sp_io::TestExternalities::new(t); + ext.execute_with(|| System::set_block_number(1)); + ext +} + +fn next_block() { + System::set_block_number(System::block_number() + 1); +} + +#[allow(dead_code)] +fn run_to(n: u64) { + while System::block_number() < n { + next_block(); + } +} + +fn tally(index: u8) -> TallyOf { + >>::as_ongoing(index).expect("No poll").0 +} + +#[test] +#[ignore] +#[should_panic(expected = "No poll")] +fn unknown_poll_should_panic() { + let _ = tally(0); +} + +#[test] +#[ignore] +#[should_panic(expected = "No poll")] +fn completed_poll_should_panic() { + let _ = tally(1); +} + +#[test] +fn basic_stuff() { + new_test_ext().execute_with(|| { + assert_eq!(tally(3), Tally::from_parts(0, 0)); + }); +} + +#[test] +fn membership_works() { + new_test_ext().execute_with(|| { + assert_noop!(Club::add_member(Origin::signed(1), 1, 1), DispatchError::BadOrigin); + assert_ok!(Club::add_member(Origin::root(), 1, 1)); + assert_eq!(Club::member_count(), 1); + assert_eq!(Club::max_turnout(), 1); + + assert_ok!(Club::add_member(Origin::root(), 2, 4)); + assert_eq!(Club::member_count(), 2); + assert_eq!(Club::max_turnout(), 11); + + assert_ok!(Club::set_member_rank(Origin::root(), 1, 4)); + assert_eq!(Club::member_count(), 2); + assert_eq!(Club::max_turnout(), 20); + + assert_noop!(Club::remove_member(Origin::signed(1), 1), DispatchError::BadOrigin); + assert_ok!(Club::remove_member(Origin::root(), 1)); + assert_eq!(Club::member_count(), 1); + assert_eq!(Club::max_turnout(), 10); + }); +} + +#[test] +fn voting_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(Origin::root(), 1, 1)); + assert_ok!(Club::add_member(Origin::root(), 2, 2)); + assert_ok!(Club::add_member(Origin::root(), 3, 3)); + + assert_ok!(Club::vote(Origin::signed(1), 3, true)); + assert_eq!(tally(3), Tally::from_parts(1, 0)); + assert_ok!(Club::vote(Origin::signed(1), 3, false)); + assert_eq!(tally(3), Tally::from_parts(0, 1)); + + assert_ok!(Club::vote(Origin::signed(2), 3, true)); + assert_eq!(tally(3), Tally::from_parts(3, 1)); + assert_ok!(Club::vote(Origin::signed(2), 3, false)); + assert_eq!(tally(3), Tally::from_parts(0, 4)); + + assert_ok!(Club::vote(Origin::signed(3), 3, true)); + assert_eq!(tally(3), Tally::from_parts(6, 4)); + assert_ok!(Club::vote(Origin::signed(3), 3, false)); + assert_eq!(tally(3), Tally::from_parts(0, 10)); + }); +} + +#[test] +fn cleanup_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(Origin::root(), 1, 1)); + assert_ok!(Club::add_member(Origin::root(), 2, 2)); + assert_ok!(Club::add_member(Origin::root(), 3, 3)); + + assert_ok!(Club::vote(Origin::signed(1), 3, true)); + assert_ok!(Club::vote(Origin::signed(2), 3, false)); + assert_ok!(Club::vote(Origin::signed(3), 3, true)); + + assert_noop!(Club::cleanup_poll(Origin::signed(4), 3, 10), Error::::Ongoing); + Polls::set( + vec![(1, Completed(1, true)), (2, Completed(2, false)), (3, Completed(3, true))] + .into_iter() + .collect(), + ); + // NOTE: This will fail until #10016 is merged. + // assert_ok!(Club::cleanup_poll(Origin::signed(4), 3, 10)); + assert_noop!(Club::cleanup_poll(Origin::signed(4), 3, 10), Error::::NoneRemaining); + }); +} diff --git a/frame/ranked-collective/src/weights.rs b/frame/ranked-collective/src/weights.rs new file mode 100644 index 0000000000000..1280ced89eeea --- /dev/null +++ b/frame/ranked-collective/src/weights.rs @@ -0,0 +1,326 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 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. + +//! Autogenerated weights for pallet_collective +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev +//! DATE: 2022-01-30, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 + +// Executed Command: +// ./target/production/substrate +// benchmark +// --chain=dev +// --steps=50 +// --repeat=20 +// --pallet=pallet_collective +// --extrinsic=* +// --execution=wasm +// --wasm-execution=compiled +// --heap-pages=4096 +// --output=./frame/collective/src/weights.rs +// --template=.maintain/frame-weight-template.hbs +// --header=HEADER-APACHE2 +// --raw + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] + +use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; +use sp_std::marker::PhantomData; + +/// Weight functions needed for pallet_collective. +pub trait WeightInfo { + fn set_members(m: u32, n: u32, p: u32, ) -> Weight; + fn execute(b: u32, m: u32, ) -> Weight; + fn propose_execute(b: u32, m: u32, ) -> Weight; + fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight; + fn vote(m: u32, ) -> Weight; + fn close_early_disapproved(m: u32, p: u32, ) -> Weight; + fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight; + fn close_disapproved(m: u32, p: u32, ) -> Weight; + fn close_approved(b: u32, m: u32, p: u32, ) -> Weight; + fn disapprove_proposal(p: u32, ) -> Weight; +} + +/// Weights for pallet_collective using the Substrate node and recommended hardware. +pub struct SubstrateWeight(PhantomData); +impl WeightInfo for SubstrateWeight { + // Storage: Council Members (r:1 w:1) + // Storage: Council Proposals (r:1 w:0) + // Storage: Council Voting (r:100 w:100) + // Storage: Council Prime (r:0 w:1) + fn set_members(m: u32, n: u32, p: u32, ) -> Weight { + (0 as Weight) + // Standard Error: 10_000 + .saturating_add((14_493_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 10_000 + .saturating_add((23_000 as Weight).saturating_mul(n as Weight)) + // Standard Error: 10_000 + .saturating_add((16_909_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(p as Weight))) + .saturating_add(T::DbWeight::get().writes(2 as Weight)) + .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(p as Weight))) + } + // Storage: Council Members (r:1 w:0) + fn execute(b: u32, m: u32, ) -> Weight { + (12_790_000 as Weight) + // Standard Error: 0 + .saturating_add((2_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 0 + .saturating_add((73_000 as Weight).saturating_mul(m as Weight)) + .saturating_add(T::DbWeight::get().reads(1 as Weight)) + } + // Storage: Council Members (r:1 w:0) + // Storage: Council ProposalOf (r:1 w:0) + fn propose_execute(b: u32, m: u32, ) -> Weight { + (15_087_000 as Weight) + // Standard Error: 0 + .saturating_add((2_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 0 + .saturating_add((135_000 as Weight).saturating_mul(m as Weight)) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + } + // Storage: Council Members (r:1 w:0) + // Storage: Council ProposalOf (r:1 w:1) + // Storage: Council Proposals (r:1 w:1) + // Storage: Council ProposalCount (r:1 w:1) + // Storage: Council Voting (r:0 w:1) + fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight { + (18_269_000 as Weight) + // Standard Error: 0 + .saturating_add((8_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 1_000 + .saturating_add((77_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 1_000 + .saturating_add((203_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } + // Storage: Council Members (r:1 w:0) + // Storage: Council Voting (r:1 w:1) + fn vote(m: u32, ) -> Weight { + (26_624_000 as Weight) + // Standard Error: 2_000 + .saturating_add((161_000 as Weight).saturating_mul(m as Weight)) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } + // Storage: Council Voting (r:1 w:1) + // Storage: Council Members (r:1 w:0) + // Storage: Council Proposals (r:1 w:1) + // Storage: Council ProposalOf (r:0 w:1) + fn close_early_disapproved(m: u32, p: u32, ) -> Weight { + (26_527_000 as Weight) + // Standard Error: 1_000 + .saturating_add((127_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 1_000 + .saturating_add((155_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(T::DbWeight::get().reads(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) + } + // Storage: Council Voting (r:1 w:1) + // Storage: Council Members (r:1 w:0) + // Storage: Council ProposalOf (r:1 w:1) + // Storage: Council Proposals (r:1 w:1) + fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight { + (26_352_000 as Weight) + // Standard Error: 0 + .saturating_add((6_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 1_000 + .saturating_add((154_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 1_000 + .saturating_add((203_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) + } + // Storage: Council Voting (r:1 w:1) + // Storage: Council Members (r:1 w:0) + // Storage: Council Prime (r:1 w:0) + // Storage: Council Proposals (r:1 w:1) + // Storage: Council ProposalOf (r:0 w:1) + fn close_disapproved(m: u32, p: u32, ) -> Weight { + (28_638_000 as Weight) + // Standard Error: 1_000 + .saturating_add((133_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 1_000 + .saturating_add((162_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) + } + // Storage: Council Voting (r:1 w:1) + // Storage: Council Members (r:1 w:0) + // Storage: Council Prime (r:1 w:0) + // Storage: Council ProposalOf (r:1 w:1) + // Storage: Council Proposals (r:1 w:1) + fn close_approved(b: u32, m: u32, p: u32, ) -> Weight { + (29_946_000 as Weight) + // Standard Error: 0 + .saturating_add((5_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 2_000 + .saturating_add((151_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 2_000 + .saturating_add((201_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(T::DbWeight::get().reads(5 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) + } + // Storage: Council Proposals (r:1 w:1) + // Storage: Council Voting (r:0 w:1) + // Storage: Council ProposalOf (r:0 w:1) + fn disapprove_proposal(p: u32, ) -> Weight { + (15_778_000 as Weight) + // Standard Error: 1_000 + .saturating_add((206_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(T::DbWeight::get().reads(1 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) + } +} + +// For backwards compatibility and tests +impl WeightInfo for () { + // Storage: Council Members (r:1 w:1) + // Storage: Council Proposals (r:1 w:0) + // Storage: Council Voting (r:100 w:100) + // Storage: Council Prime (r:0 w:1) + fn set_members(m: u32, n: u32, p: u32, ) -> Weight { + (0 as Weight) + // Standard Error: 10_000 + .saturating_add((14_493_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 10_000 + .saturating_add((23_000 as Weight).saturating_mul(n as Weight)) + // Standard Error: 10_000 + .saturating_add((16_909_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(p as Weight))) + .saturating_add(RocksDbWeight::get().writes(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(p as Weight))) + } + // Storage: Council Members (r:1 w:0) + fn execute(b: u32, m: u32, ) -> Weight { + (12_790_000 as Weight) + // Standard Error: 0 + .saturating_add((2_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 0 + .saturating_add((73_000 as Weight).saturating_mul(m as Weight)) + .saturating_add(RocksDbWeight::get().reads(1 as Weight)) + } + // Storage: Council Members (r:1 w:0) + // Storage: Council ProposalOf (r:1 w:0) + fn propose_execute(b: u32, m: u32, ) -> Weight { + (15_087_000 as Weight) + // Standard Error: 0 + .saturating_add((2_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 0 + .saturating_add((135_000 as Weight).saturating_mul(m as Weight)) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + } + // Storage: Council Members (r:1 w:0) + // Storage: Council ProposalOf (r:1 w:1) + // Storage: Council Proposals (r:1 w:1) + // Storage: Council ProposalCount (r:1 w:1) + // Storage: Council Voting (r:0 w:1) + fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight { + (18_269_000 as Weight) + // Standard Error: 0 + .saturating_add((8_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 1_000 + .saturating_add((77_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 1_000 + .saturating_add((203_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + } + // Storage: Council Members (r:1 w:0) + // Storage: Council Voting (r:1 w:1) + fn vote(m: u32, ) -> Weight { + (26_624_000 as Weight) + // Standard Error: 2_000 + .saturating_add((161_000 as Weight).saturating_mul(m as Weight)) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + } + // Storage: Council Voting (r:1 w:1) + // Storage: Council Members (r:1 w:0) + // Storage: Council Proposals (r:1 w:1) + // Storage: Council ProposalOf (r:0 w:1) + fn close_early_disapproved(m: u32, p: u32, ) -> Weight { + (26_527_000 as Weight) + // Standard Error: 1_000 + .saturating_add((127_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 1_000 + .saturating_add((155_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(RocksDbWeight::get().reads(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + } + // Storage: Council Voting (r:1 w:1) + // Storage: Council Members (r:1 w:0) + // Storage: Council ProposalOf (r:1 w:1) + // Storage: Council Proposals (r:1 w:1) + fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight { + (26_352_000 as Weight) + // Standard Error: 0 + .saturating_add((6_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 1_000 + .saturating_add((154_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 1_000 + .saturating_add((203_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + } + // Storage: Council Voting (r:1 w:1) + // Storage: Council Members (r:1 w:0) + // Storage: Council Prime (r:1 w:0) + // Storage: Council Proposals (r:1 w:1) + // Storage: Council ProposalOf (r:0 w:1) + fn close_disapproved(m: u32, p: u32, ) -> Weight { + (28_638_000 as Weight) + // Standard Error: 1_000 + .saturating_add((133_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 1_000 + .saturating_add((162_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + } + // Storage: Council Voting (r:1 w:1) + // Storage: Council Members (r:1 w:0) + // Storage: Council Prime (r:1 w:0) + // Storage: Council ProposalOf (r:1 w:1) + // Storage: Council Proposals (r:1 w:1) + fn close_approved(b: u32, m: u32, p: u32, ) -> Weight { + (29_946_000 as Weight) + // Standard Error: 0 + .saturating_add((5_000 as Weight).saturating_mul(b as Weight)) + // Standard Error: 2_000 + .saturating_add((151_000 as Weight).saturating_mul(m as Weight)) + // Standard Error: 2_000 + .saturating_add((201_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + } + // Storage: Council Proposals (r:1 w:1) + // Storage: Council Voting (r:0 w:1) + // Storage: Council ProposalOf (r:0 w:1) + fn disapprove_proposal(p: u32, ) -> Weight { + (15_778_000 as Weight) + // Standard Error: 1_000 + .saturating_add((206_000 as Weight).saturating_mul(p as Weight)) + .saturating_add(RocksDbWeight::get().reads(1 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + } +} From a6e2367e4986dec27e5744ec64b3338c1b629898 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 18 May 2022 16:15:08 +0200 Subject: [PATCH 02/25] Fixes --- Cargo.lock | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index ff14da370db9c..ec4d4b9c859e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5980,6 +5980,23 @@ dependencies = [ "sp-std", ] +[[package]] +name = "pallet-ranked-collective" +version = "4.0.0-dev" +dependencies = [ + "frame-benchmarking", + "frame-support", + "frame-system", + "log", + "parity-scale-codec", + "scale-info", + "sp-arithmetic", + "sp-core", + "sp-io", + "sp-runtime", + "sp-std", +] + [[package]] name = "pallet-recovery" version = "4.0.0-dev" From bf125a88a9f39436509a3d24d92884f2e3e4b041 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 18 May 2022 17:14:09 +0200 Subject: [PATCH 03/25] benchmarks --- frame/ranked-collective/src/benchmarking.rs | 46 ++++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs index 4d972e211fea9..26dc83ed0382c 100644 --- a/frame/ranked-collective/src/benchmarking.rs +++ b/frame/ranked-collective/src/benchmarking.rs @@ -24,7 +24,7 @@ use sp_runtime::traits::Bounded; use sp_std::mem::size_of; use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller}; -use frame_support::dispatch::UnfilteredDispatchable; +use frame_support::{assert_ok, dispatch::UnfilteredDispatchable}; use frame_system::{Call as SystemCall, Pallet as System, RawOrigin as SystemOrigin}; const SEED: u32 = 0; @@ -37,7 +37,7 @@ fn assert_last_event, I: 'static>(generic_event: >:: fn make_member, I: 'static>(rank: Rank) -> T::AccountId { let who = account::("member", MemberCount::::get().0, SEED); - Pallet::::add_member(T::AdminOrigin::successful_origin(), who.clone(), rank); + assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), who.clone(), rank)); who } @@ -81,5 +81,47 @@ benchmarks_instance_pallet! { assert_last_event::(Event::RankChanged { who, rank }.into()); } + vote { + let rank = 1; + let caller: T::AccountId = whitelisted_caller(); + Pallet::::add_member(T::AdminOrigin::successful_origin(), caller.clone(), rank); + // Create a poll + let class = T::Polls::classes().into_iter().next().expect("Must always be at least one class"); + let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll"); + + // Vote once. + assert_ok!(Pallet::::vote(SystemOrigin::Signed(caller.clone()).into(), poll, true)); + }: _(SystemOrigin::Signed(caller.clone()), poll, false) + verify { + let tally = Tally::from_parts(0, 1); + let ev = Event::Voted { who: caller, poll, vote: VoteRecord::Nay(1), tally }; + assert_last_event::(ev.into()); + } + + cleanup_poll { + let rank = 1; + let n in 1 .. 100; + + dbg!(n); + // Create a poll + let class = T::Polls::classes().into_iter().next().expect("Must always be at least one class"); + let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll"); + + // Vote in the poll by each of `n` members + for i in 0..n { + let who = make_member::(rank); + assert_ok!(Pallet::::vote(SystemOrigin::Signed(who).into(), poll, true)); + } + + // End the poll. + T::Polls::end_ongoing(poll, false).expect("Must always be able to end a poll"); + + assert_eq!(Voting::::iter_prefix(poll).count(), n as usize); + dbg!(Voting::::iter_prefix(poll).count()); + }: _(SystemOrigin::Signed(whitelisted_caller()), poll, n) + verify { + assert_eq!(Voting::::iter().count(), 0); + } + impl_benchmark_test_suite!(Collective, crate::tests::new_test_ext(), crate::tests::Test); } From b8f6f2cba355c7cc9cebba623e2513ad79894bc9 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 18 May 2022 17:19:12 +0100 Subject: [PATCH 04/25] Weights --- Cargo.lock | 1 + bin/node/runtime/Cargo.toml | 4 + bin/node/runtime/src/lib.rs | 30 +- frame/ranked-collective/src/benchmarking.rs | 16 +- frame/ranked-collective/src/lib.rs | 36 +-- frame/ranked-collective/src/tests.rs | 28 +- frame/ranked-collective/src/weights.rs | 333 +++++--------------- 7 files changed, 149 insertions(+), 299 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec4d4b9c859e0..55c71f45457c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4755,6 +4755,7 @@ dependencies = [ "pallet-preimage", "pallet-proxy", "pallet-randomness-collective-flip", + "pallet-ranked-collective", "pallet-recovery", "pallet-referenda", "pallet-remark", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 1b3e9083d1a1d..5ecd6ccedaf01 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -83,6 +83,7 @@ pallet-offences-benchmarking = { version = "4.0.0-dev", path = "../../../frame/o pallet-preimage = { version = "4.0.0-dev", default-features = false, path = "../../../frame/preimage" } pallet-proxy = { version = "4.0.0-dev", default-features = false, path = "../../../frame/proxy" } pallet-randomness-collective-flip = { version = "4.0.0-dev", default-features = false, path = "../../../frame/randomness-collective-flip" } +pallet-ranked-collective = { version = "4.0.0-dev", default-features = false, path = "../../../frame/ranked-collective" } pallet-recovery = { version = "4.0.0-dev", default-features = false, path = "../../../frame/recovery" } pallet-referenda = { version = "4.0.0-dev", default-features = false, path = "../../../frame/referenda" } pallet-remark = { version = "4.0.0-dev", default-features = false, path = "../../../frame/remark" } @@ -176,6 +177,7 @@ std = [ "pallet-utility/std", "sp-version/std", "pallet-society/std", + "pallet-ranked-collective/std", "pallet-referenda/std", "pallet-remark/std", "pallet-recovery/std", @@ -218,6 +220,7 @@ runtime-benchmarks = [ "pallet-preimage/runtime-benchmarks", "pallet-proxy/runtime-benchmarks", "pallet-scheduler/runtime-benchmarks", + "pallet-ranked-collective/runtime-benchmarks", "pallet-referenda/runtime-benchmarks", "pallet-recovery/runtime-benchmarks", "pallet-remark/runtime-benchmarks", @@ -265,6 +268,7 @@ try-runtime = [ "pallet-offences/try-runtime", "pallet-preimage/try-runtime", "pallet-proxy/try-runtime", + "pallet-ranked-collective/try-runtime", "pallet-randomness-collective-flip/try-runtime", "pallet-recovery/try-runtime", "pallet-referenda/try-runtime", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index de6f8eeb9f1c0..480fca7972011 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -20,7 +20,7 @@ #![cfg_attr(not(feature = "std"), no_std)] // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256. -#![recursion_limit = "256"] +#![recursion_limit = "512"] use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::{ @@ -835,6 +835,31 @@ impl pallet_referenda::Config for Runtime { type Tracks = TracksInfo; } +impl pallet_referenda::Config for Runtime { + type WeightInfo = pallet_referenda::weights::SubstrateWeight; + type Call = Call; + type Event = Event; + type Scheduler = Scheduler; + type Currency = pallet_balances::Pallet; + type CancelOrigin = EnsureRoot; + type KillOrigin = EnsureRoot; + type Slash = (); + type Votes = pallet_ranked_collective::Votes; + type Tally = pallet_ranked_collective::TallyOf; + type SubmissionDeposit = SubmissionDeposit; + type MaxQueued = ConstU32<100>; + type UndecidingTimeout = UndecidingTimeout; + type AlarmInterval = AlarmInterval; + type Tracks = TracksInfo; +} + +impl pallet_ranked_collective::Config for Runtime { + type WeightInfo = pallet_ranked_collective::weights::SubstrateWeight; + type Event = Event; + type AdminOrigin = EnsureRoot; + type Polls = RankedPolls; +} + impl pallet_remark::Config for Runtime { type WeightInfo = pallet_remark::weights::SubstrateWeight; type Event = Event; @@ -1531,6 +1556,8 @@ construct_runtime!( ConvictionVoting: pallet_conviction_voting, Whitelist: pallet_whitelist, NominationPools: pallet_nomination_pools, + RankedPolls: pallet_referenda::, + RankedCollective: pallet_ranked_collective, } ); @@ -1619,6 +1646,7 @@ mod benches { [pallet_offences, OffencesBench::] [pallet_preimage, Preimage] [pallet_proxy, Proxy] + [pallet_ranked_collective, RankedCollective] [pallet_referenda, Referenda] [pallet_recovery, Recovery] [pallet_remark, Remark] diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs index 26dc83ed0382c..2900773813a4a 100644 --- a/frame/ranked-collective/src/benchmarking.rs +++ b/frame/ranked-collective/src/benchmarking.rs @@ -18,19 +18,15 @@ //! Staking pallet benchmarking. use super::*; -use crate::Pallet as Collective; - -use sp_runtime::traits::Bounded; -use sp_std::mem::size_of; +#[allow(unused_imports)] +use crate::Pallet as RankedCollective; use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller}; use frame_support::{assert_ok, dispatch::UnfilteredDispatchable}; -use frame_system::{Call as SystemCall, Pallet as System, RawOrigin as SystemOrigin}; +use frame_system::RawOrigin as SystemOrigin; const SEED: u32 = 0; -const MAX_BYTES: u32 = 1_024; - fn assert_last_event, I: 'static>(generic_event: >::Event) { frame_system::Pallet::::assert_last_event(generic_event.into()); } @@ -84,7 +80,7 @@ benchmarks_instance_pallet! { vote { let rank = 1; let caller: T::AccountId = whitelisted_caller(); - Pallet::::add_member(T::AdminOrigin::successful_origin(), caller.clone(), rank); + assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), caller.clone(), rank)); // Create a poll let class = T::Polls::classes().into_iter().next().expect("Must always be at least one class"); let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll"); @@ -102,7 +98,6 @@ benchmarks_instance_pallet! { let rank = 1; let n in 1 .. 100; - dbg!(n); // Create a poll let class = T::Polls::classes().into_iter().next().expect("Must always be at least one class"); let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll"); @@ -117,11 +112,10 @@ benchmarks_instance_pallet! { T::Polls::end_ongoing(poll, false).expect("Must always be able to end a poll"); assert_eq!(Voting::::iter_prefix(poll).count(), n as usize); - dbg!(Voting::::iter_prefix(poll).count()); }: _(SystemOrigin::Signed(whitelisted_caller()), poll, n) verify { assert_eq!(Voting::::iter().count(), 0); } - impl_benchmark_test_suite!(Collective, crate::tests::new_test_ext(), crate::tests::Test); + impl_benchmark_test_suite!(RankedCollective, crate::tests::new_test_ext(), crate::tests::Test); } diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index ef23cdf648c28..9f3d9caba7c17 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -55,6 +55,7 @@ use frame_support::{ dispatch::{DispatchError, DispatchResultWithPostInfo}, ensure, traits::{EnsureOrigin, Get, PollStatus, Polling, VoteTally}, + weights::PostDispatchInfo, CloneNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; @@ -251,7 +252,7 @@ pub mod pallet { /// - `rank`: The rank to give the new member. /// /// Weight: `O(1)` - #[pallet::weight(T::BlockWeights::get().max_block / 10)] + #[pallet::weight(T::WeightInfo::add_member())] pub fn add_member(origin: OriginFor, who: T::AccountId, rank: Rank) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; ensure!(!Members::::contains_key(&who), Error::::AlreadyMember); @@ -274,7 +275,7 @@ pub mod pallet { /// - `rank`: The new rank to give the member. /// /// Weight: `O(1)` - #[pallet::weight(T::BlockWeights::get().max_block / 10)] + #[pallet::weight(T::WeightInfo::set_member_rank())] pub fn set_member_rank( origin: OriginFor, who: T::AccountId, @@ -303,7 +304,7 @@ pub mod pallet { /// - `who`: Account of existing member to be removed. /// /// Weight: `O(1)`, less if the member's index is highest. - #[pallet::weight(T::BlockWeights::get().max_block / 10)] + #[pallet::weight(T::WeightInfo::remove_member())] pub fn remove_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; let record = Self::ensure_member(&who)?; @@ -340,7 +341,7 @@ pub mod pallet { /// fee. /// /// Weight: `O(1)`, less if there was no previous vote on the poll by the member. - #[pallet::weight(T::BlockWeights::get().max_block / 10)] + #[pallet::weight(T::WeightInfo::vote())] pub fn vote( origin: OriginFor, poll: PollIndexOf, @@ -353,7 +354,7 @@ pub mod pallet { let vote = VoteRecord::from((aye, votes)); let mut pays = Pays::Yes; - T::Polls::try_access_poll(poll, |mut status| -> DispatchResult { + let tally = T::Polls::try_access_poll(poll, |mut status| -> Result, DispatchError> { match status { PollStatus::None | PollStatus::Completed(..) => Err(Error::::NotPolling)?, PollStatus::Ongoing(ref mut tally, _) => { @@ -367,11 +368,11 @@ pub mod pallet { false => tally.nays.saturating_accrue(votes), } Voting::::insert(&poll, &who, &vote); - Self::deposit_event(Event::Voted { who, poll, vote, tally: tally.clone() }); - Ok(()) + Ok(tally.clone()) }, } })?; + Self::deposit_event(Event::Voted { who, poll, vote, tally }); Ok(pays.into()) } @@ -385,7 +386,7 @@ pub mod pallet { /// Transaction fees are waived if the operation is successful. /// /// Weight `O(max)` (less if there are fewer items to remove than `max`). - #[pallet::weight(T::BlockWeights::get().max_block / 10)] + #[pallet::weight(T::WeightInfo::cleanup_poll(*max))] pub fn cleanup_poll( origin: OriginFor, poll_index: PollIndexOf, @@ -395,12 +396,15 @@ pub mod pallet { ensure!(T::Polls::as_ongoing(poll_index).is_none(), Error::::Ongoing); use sp_io::KillStorageResult::*; - let _count = match Voting::::remove_prefix(poll_index, Some(max)) { - AllRemoved(0) => Err(Error::::NoneRemaining)?, + let count = match Voting::::remove_prefix(poll_index, Some(max)) { +// AllRemoved(0) => Err(Error::::NoneRemaining)?, + AllRemoved(0) => return Ok(Pays::Yes.into()), AllRemoved(n) | SomeRemaining(n) => n, }; - // TODO: weight from count. - Ok(Pays::No.into()) + Ok(PostDispatchInfo { + actual_weight: Some(T::WeightInfo::cleanup_poll(count)), + pays_fee: Pays::No, + }) } } @@ -413,14 +417,6 @@ pub mod pallet { let r = r as Votes; r * (r + 1) / 2 } - - pub(crate) fn member_count() -> MemberIndex { - MemberCount::::get().0 - } - - pub(crate) fn max_turnout() -> Votes { - MemberCount::::get().1 - } } impl, I: 'static> Get for Pallet { diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index 6b559bc1064fd..948387f35833d 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -184,6 +184,14 @@ fn next_block() { System::set_block_number(System::block_number() + 1); } +fn member_count() -> MemberIndex { + MemberCount::::get().0 +} + +fn max_turnout() -> Votes { + MemberCount::::get().1 +} + #[allow(dead_code)] fn run_to(n: u64) { while System::block_number() < n { @@ -221,21 +229,21 @@ fn membership_works() { new_test_ext().execute_with(|| { assert_noop!(Club::add_member(Origin::signed(1), 1, 1), DispatchError::BadOrigin); assert_ok!(Club::add_member(Origin::root(), 1, 1)); - assert_eq!(Club::member_count(), 1); - assert_eq!(Club::max_turnout(), 1); + assert_eq!(member_count(), 1); + assert_eq!(max_turnout(), 1); assert_ok!(Club::add_member(Origin::root(), 2, 4)); - assert_eq!(Club::member_count(), 2); - assert_eq!(Club::max_turnout(), 11); + assert_eq!(member_count(), 2); + assert_eq!(max_turnout(), 11); assert_ok!(Club::set_member_rank(Origin::root(), 1, 4)); - assert_eq!(Club::member_count(), 2); - assert_eq!(Club::max_turnout(), 20); + assert_eq!(member_count(), 2); + assert_eq!(max_turnout(), 20); assert_noop!(Club::remove_member(Origin::signed(1), 1), DispatchError::BadOrigin); assert_ok!(Club::remove_member(Origin::root(), 1)); - assert_eq!(Club::member_count(), 1); - assert_eq!(Club::max_turnout(), 10); + assert_eq!(member_count(), 1); + assert_eq!(max_turnout(), 10); }); } @@ -280,8 +288,8 @@ fn cleanup_works() { .into_iter() .collect(), ); + assert_ok!(Club::cleanup_poll(Origin::signed(4), 3, 10)); // NOTE: This will fail until #10016 is merged. - // assert_ok!(Club::cleanup_poll(Origin::signed(4), 3, 10)); - assert_noop!(Club::cleanup_poll(Origin::signed(4), 3, 10), Error::::NoneRemaining); +// assert_noop!(Club::cleanup_poll(Origin::signed(4), 3, 10), Error::::NoneRemaining); }); } diff --git a/frame/ranked-collective/src/weights.rs b/frame/ranked-collective/src/weights.rs index 1280ced89eeea..ed08564774c1d 100644 --- a/frame/ranked-collective/src/weights.rs +++ b/frame/ranked-collective/src/weights.rs @@ -15,27 +15,27 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Autogenerated weights for pallet_collective +//! Autogenerated weights for pallet_ranked_collective //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-01-30, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2022-05-18, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: -// ./target/production/substrate +// ../../../target/release/substrate // benchmark +// pallet // --chain=dev // --steps=50 // --repeat=20 -// --pallet=pallet_collective +// --pallet=pallet-ranked-collective // --extrinsic=* // --execution=wasm -// --wasm-execution=compiled // --heap-pages=4096 -// --output=./frame/collective/src/weights.rs -// --template=.maintain/frame-weight-template.hbs -// --header=HEADER-APACHE2 -// --raw +// --output=../../../frame/ranked-collective/src/weights.rs +// --template=../../../.maintain/frame-weight-template.hbs +// --header=../../../HEADER-APACHE2 +// --record-proof #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -44,283 +44,102 @@ use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; use sp_std::marker::PhantomData; -/// Weight functions needed for pallet_collective. +/// Weight functions needed for pallet_ranked_collective. pub trait WeightInfo { - fn set_members(m: u32, n: u32, p: u32, ) -> Weight; - fn execute(b: u32, m: u32, ) -> Weight; - fn propose_execute(b: u32, m: u32, ) -> Weight; - fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight; - fn vote(m: u32, ) -> Weight; - fn close_early_disapproved(m: u32, p: u32, ) -> Weight; - fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight; - fn close_disapproved(m: u32, p: u32, ) -> Weight; - fn close_approved(b: u32, m: u32, p: u32, ) -> Weight; - fn disapprove_proposal(p: u32, ) -> Weight; + fn add_member() -> Weight; + fn remove_member() -> Weight; + fn set_member_rank() -> Weight; + fn vote() -> Weight; + fn cleanup_poll(n: u32, ) -> Weight; } -/// Weights for pallet_collective using the Substrate node and recommended hardware. +/// Weights for pallet_ranked_collective using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - // Storage: Council Members (r:1 w:1) - // Storage: Council Proposals (r:1 w:0) - // Storage: Council Voting (r:100 w:100) - // Storage: Council Prime (r:0 w:1) - fn set_members(m: u32, n: u32, p: u32, ) -> Weight { - (0 as Weight) - // Standard Error: 10_000 - .saturating_add((14_493_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 10_000 - .saturating_add((23_000 as Weight).saturating_mul(n as Weight)) - // Standard Error: 10_000 - .saturating_add((16_909_000 as Weight).saturating_mul(p as Weight)) + // Storage: RankedCollective Members (r:1 w:1) + // Storage: RankedCollective MemberCount (r:1 w:1) + // Storage: RankedCollective MemberByIndex (r:0 w:1) + fn add_member() -> Weight { + (15_000_000 as Weight) .saturating_add(T::DbWeight::get().reads(2 as Weight)) - .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(p as Weight))) - .saturating_add(T::DbWeight::get().writes(2 as Weight)) - .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(p as Weight))) - } - // Storage: Council Members (r:1 w:0) - fn execute(b: u32, m: u32, ) -> Weight { - (12_790_000 as Weight) - // Standard Error: 0 - .saturating_add((2_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 0 - .saturating_add((73_000 as Weight).saturating_mul(m as Weight)) - .saturating_add(T::DbWeight::get().reads(1 as Weight)) - } - // Storage: Council Members (r:1 w:0) - // Storage: Council ProposalOf (r:1 w:0) - fn propose_execute(b: u32, m: u32, ) -> Weight { - (15_087_000 as Weight) - // Standard Error: 0 - .saturating_add((2_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 0 - .saturating_add((135_000 as Weight).saturating_mul(m as Weight)) - .saturating_add(T::DbWeight::get().reads(2 as Weight)) - } - // Storage: Council Members (r:1 w:0) - // Storage: Council ProposalOf (r:1 w:1) - // Storage: Council Proposals (r:1 w:1) - // Storage: Council ProposalCount (r:1 w:1) - // Storage: Council Voting (r:0 w:1) - fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight { - (18_269_000 as Weight) - // Standard Error: 0 - .saturating_add((8_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 1_000 - .saturating_add((77_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 1_000 - .saturating_add((203_000 as Weight).saturating_mul(p as Weight)) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(4 as Weight)) - } - // Storage: Council Members (r:1 w:0) - // Storage: Council Voting (r:1 w:1) - fn vote(m: u32, ) -> Weight { - (26_624_000 as Weight) - // Standard Error: 2_000 - .saturating_add((161_000 as Weight).saturating_mul(m as Weight)) - .saturating_add(T::DbWeight::get().reads(2 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) - } - // Storage: Council Voting (r:1 w:1) - // Storage: Council Members (r:1 w:0) - // Storage: Council Proposals (r:1 w:1) - // Storage: Council ProposalOf (r:0 w:1) - fn close_early_disapproved(m: u32, p: u32, ) -> Weight { - (26_527_000 as Weight) - // Standard Error: 1_000 - .saturating_add((127_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 1_000 - .saturating_add((155_000 as Weight).saturating_mul(p as Weight)) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } - // Storage: Council Voting (r:1 w:1) - // Storage: Council Members (r:1 w:0) - // Storage: Council ProposalOf (r:1 w:1) - // Storage: Council Proposals (r:1 w:1) - fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight { - (26_352_000 as Weight) - // Standard Error: 0 - .saturating_add((6_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 1_000 - .saturating_add((154_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 1_000 - .saturating_add((203_000 as Weight).saturating_mul(p as Weight)) + // Storage: RankedCollective Members (r:2 w:2) + // Storage: RankedCollective MemberCount (r:1 w:1) + // Storage: RankedCollective MemberByIndex (r:1 w:2) + fn remove_member() -> Weight { + (23_000_000 as Weight) .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(5 as Weight)) } - // Storage: Council Voting (r:1 w:1) - // Storage: Council Members (r:1 w:0) - // Storage: Council Prime (r:1 w:0) - // Storage: Council Proposals (r:1 w:1) - // Storage: Council ProposalOf (r:0 w:1) - fn close_disapproved(m: u32, p: u32, ) -> Weight { - (28_638_000 as Weight) - // Standard Error: 1_000 - .saturating_add((133_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 1_000 - .saturating_add((162_000 as Weight).saturating_mul(p as Weight)) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(3 as Weight)) + // Storage: RankedCollective Members (r:1 w:1) + // Storage: RankedCollective MemberCount (r:1 w:1) + fn set_member_rank() -> Weight { + (15_000_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(2 as Weight)) } - // Storage: Council Voting (r:1 w:1) - // Storage: Council Members (r:1 w:0) - // Storage: Council Prime (r:1 w:0) - // Storage: Council ProposalOf (r:1 w:1) - // Storage: Council Proposals (r:1 w:1) - fn close_approved(b: u32, m: u32, p: u32, ) -> Weight { - (29_946_000 as Weight) - // Standard Error: 0 - .saturating_add((5_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 2_000 - .saturating_add((151_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 2_000 - .saturating_add((201_000 as Weight).saturating_mul(p as Weight)) + // Storage: RankedCollective Members (r:1 w:0) + // Storage: RankedPolls ReferendumInfoFor (r:1 w:1) + // Storage: RankedCollective Voting (r:1 w:1) + // Storage: Scheduler Agenda (r:2 w:2) + fn vote() -> Weight { + (35_000_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) - .saturating_add(T::DbWeight::get().writes(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) } - // Storage: Council Proposals (r:1 w:1) - // Storage: Council Voting (r:0 w:1) - // Storage: Council ProposalOf (r:0 w:1) - fn disapprove_proposal(p: u32, ) -> Weight { - (15_778_000 as Weight) + // Storage: RankedPolls ReferendumInfoFor (r:1 w:0) + // Storage: RankedCollective Voting (r:0 w:1) + fn cleanup_poll(n: u32, ) -> Weight { + (7_800_000 as Weight) // Standard Error: 1_000 - .saturating_add((206_000 as Weight).saturating_mul(p as Weight)) + .saturating_add((860_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(1 as Weight)) - .saturating_add(T::DbWeight::get().writes(3 as Weight)) + .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(n as Weight))) } } // For backwards compatibility and tests impl WeightInfo for () { - // Storage: Council Members (r:1 w:1) - // Storage: Council Proposals (r:1 w:0) - // Storage: Council Voting (r:100 w:100) - // Storage: Council Prime (r:0 w:1) - fn set_members(m: u32, n: u32, p: u32, ) -> Weight { - (0 as Weight) - // Standard Error: 10_000 - .saturating_add((14_493_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 10_000 - .saturating_add((23_000 as Weight).saturating_mul(n as Weight)) - // Standard Error: 10_000 - .saturating_add((16_909_000 as Weight).saturating_mul(p as Weight)) + // Storage: RankedCollective Members (r:1 w:1) + // Storage: RankedCollective MemberCount (r:1 w:1) + // Storage: RankedCollective MemberByIndex (r:0 w:1) + fn add_member() -> Weight { + (15_000_000 as Weight) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(p as Weight))) - .saturating_add(RocksDbWeight::get().writes(2 as Weight)) - .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(p as Weight))) - } - // Storage: Council Members (r:1 w:0) - fn execute(b: u32, m: u32, ) -> Weight { - (12_790_000 as Weight) - // Standard Error: 0 - .saturating_add((2_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 0 - .saturating_add((73_000 as Weight).saturating_mul(m as Weight)) - .saturating_add(RocksDbWeight::get().reads(1 as Weight)) - } - // Storage: Council Members (r:1 w:0) - // Storage: Council ProposalOf (r:1 w:0) - fn propose_execute(b: u32, m: u32, ) -> Weight { - (15_087_000 as Weight) - // Standard Error: 0 - .saturating_add((2_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 0 - .saturating_add((135_000 as Weight).saturating_mul(m as Weight)) - .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - } - // Storage: Council Members (r:1 w:0) - // Storage: Council ProposalOf (r:1 w:1) - // Storage: Council Proposals (r:1 w:1) - // Storage: Council ProposalCount (r:1 w:1) - // Storage: Council Voting (r:0 w:1) - fn propose_proposed(b: u32, m: u32, p: u32, ) -> Weight { - (18_269_000 as Weight) - // Standard Error: 0 - .saturating_add((8_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 1_000 - .saturating_add((77_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 1_000 - .saturating_add((203_000 as Weight).saturating_mul(p as Weight)) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(4 as Weight)) - } - // Storage: Council Members (r:1 w:0) - // Storage: Council Voting (r:1 w:1) - fn vote(m: u32, ) -> Weight { - (26_624_000 as Weight) - // Standard Error: 2_000 - .saturating_add((161_000 as Weight).saturating_mul(m as Weight)) - .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) - } - // Storage: Council Voting (r:1 w:1) - // Storage: Council Members (r:1 w:0) - // Storage: Council Proposals (r:1 w:1) - // Storage: Council ProposalOf (r:0 w:1) - fn close_early_disapproved(m: u32, p: u32, ) -> Weight { - (26_527_000 as Weight) - // Standard Error: 1_000 - .saturating_add((127_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 1_000 - .saturating_add((155_000 as Weight).saturating_mul(p as Weight)) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } - // Storage: Council Voting (r:1 w:1) - // Storage: Council Members (r:1 w:0) - // Storage: Council ProposalOf (r:1 w:1) - // Storage: Council Proposals (r:1 w:1) - fn close_early_approved(b: u32, m: u32, p: u32, ) -> Weight { - (26_352_000 as Weight) - // Standard Error: 0 - .saturating_add((6_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 1_000 - .saturating_add((154_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 1_000 - .saturating_add((203_000 as Weight).saturating_mul(p as Weight)) + // Storage: RankedCollective Members (r:2 w:2) + // Storage: RankedCollective MemberCount (r:1 w:1) + // Storage: RankedCollective MemberByIndex (r:1 w:2) + fn remove_member() -> Weight { + (23_000_000 as Weight) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes(5 as Weight)) } - // Storage: Council Voting (r:1 w:1) - // Storage: Council Members (r:1 w:0) - // Storage: Council Prime (r:1 w:0) - // Storage: Council Proposals (r:1 w:1) - // Storage: Council ProposalOf (r:0 w:1) - fn close_disapproved(m: u32, p: u32, ) -> Weight { - (28_638_000 as Weight) - // Standard Error: 1_000 - .saturating_add((133_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 1_000 - .saturating_add((162_000 as Weight).saturating_mul(p as Weight)) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + // Storage: RankedCollective Members (r:1 w:1) + // Storage: RankedCollective MemberCount (r:1 w:1) + fn set_member_rank() -> Weight { + (15_000_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } - // Storage: Council Voting (r:1 w:1) - // Storage: Council Members (r:1 w:0) - // Storage: Council Prime (r:1 w:0) - // Storage: Council ProposalOf (r:1 w:1) - // Storage: Council Proposals (r:1 w:1) - fn close_approved(b: u32, m: u32, p: u32, ) -> Weight { - (29_946_000 as Weight) - // Standard Error: 0 - .saturating_add((5_000 as Weight).saturating_mul(b as Weight)) - // Standard Error: 2_000 - .saturating_add((151_000 as Weight).saturating_mul(m as Weight)) - // Standard Error: 2_000 - .saturating_add((201_000 as Weight).saturating_mul(p as Weight)) + // Storage: RankedCollective Members (r:1 w:0) + // Storage: RankedPolls ReferendumInfoFor (r:1 w:1) + // Storage: RankedCollective Voting (r:1 w:1) + // Storage: Scheduler Agenda (r:2 w:2) + fn vote() -> Weight { + (35_000_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) - .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } - // Storage: Council Proposals (r:1 w:1) - // Storage: Council Voting (r:0 w:1) - // Storage: Council ProposalOf (r:0 w:1) - fn disapprove_proposal(p: u32, ) -> Weight { - (15_778_000 as Weight) + // Storage: RankedPolls ReferendumInfoFor (r:1 w:0) + // Storage: RankedCollective Voting (r:0 w:1) + fn cleanup_poll(n: u32, ) -> Weight { + (7_800_000 as Weight) // Standard Error: 1_000 - .saturating_add((206_000 as Weight).saturating_mul(p as Weight)) + .saturating_add((860_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) - .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(n as Weight))) } } From c44e41af9b4f70100185b11906c6875848f18c8b Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 19 May 2022 12:17:49 +0100 Subject: [PATCH 05/25] Allow class voting in rank Use bare ayes for calculating support. Allow only promotion/demotion by one rank only. Allow removal of member with rank zero only. Use new Tally API --- frame/ranked-collective/src/benchmarking.rs | 68 ++++---- frame/ranked-collective/src/lib.rs | 172 ++++++++++++-------- frame/ranked-collective/src/tests.rs | 135 ++++++++++----- 3 files changed, 244 insertions(+), 131 deletions(-) diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs index 2900773813a4a..f571d35debf7d 100644 --- a/frame/ranked-collective/src/benchmarking.rs +++ b/frame/ranked-collective/src/benchmarking.rs @@ -32,79 +32,89 @@ fn assert_last_event, I: 'static>(generic_event: >:: } fn make_member, I: 'static>(rank: Rank) -> T::AccountId { - let who = account::("member", MemberCount::::get().0, SEED); - assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), who.clone(), rank)); + let who = account::("member", MemberCount::::get(0), SEED); + assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), who.clone())); + for _ in 0..rank { + promote_member::(&who); + } who } +fn promote_member, I: 'static>(who: &T::AccountId) { + assert_ok!(Pallet::::promote(T::AdminOrigin::successful_origin(), who.clone())); +} + benchmarks_instance_pallet! { add_member { - let old = MemberCount::::get().0; let who = account::("member", 0, SEED); - let rank = 1; let origin = T::AdminOrigin::successful_origin(); - let call = Call::::add_member { who: who.clone(), rank }; + let call = Call::::add_member { who: who.clone() }; }: { call.dispatch_bypass_filter(origin)? } verify { - assert_eq!(MemberCount::::get().0, old + 1); - assert_last_event::(Event::MemberAdded { who, rank }.into()); + assert_eq!(MemberCount::::get(0), 1); + assert_last_event::(Event::MemberAdded { who }.into()); } remove_member { - let rank = 1; - let who = make_member::(rank); - let other = make_member::(rank); - let old = MemberCount::::get().0; - let other_index = Members::::get(&other).unwrap().index; + let first = make_member::(0); + let who = make_member::(0); + let last = make_member::(0); + let last_index = Members::::get(&last).unwrap().index; let origin = T::AdminOrigin::successful_origin(); let call = Call::::remove_member { who: who.clone() }; }: { call.dispatch_bypass_filter(origin)? } verify { - assert_eq!(MemberCount::::get().0, old - 1); - assert_ne!(other_index, Members::::get(&other).unwrap().index); + assert_eq!(MemberCount::::get(0), 2); + assert_ne!(last_index, Members::::get(&last).unwrap().index); assert_last_event::(Event::MemberRemoved { who }.into()); } - set_member_rank { - let old_rank = 1; - let rank = 2; - let who = make_member::(old_rank); + promote { + let who = make_member::(0); let origin = T::AdminOrigin::successful_origin(); - let call = Call::::set_member_rank { who: who.clone(), rank }; + let call = Call::::promote { who: who.clone() }; }: { call.dispatch_bypass_filter(origin)? } verify { - assert_eq!(Members::::get(&who).unwrap().rank, rank); - assert_last_event::(Event::RankChanged { who, rank }.into()); + assert_eq!(Members::::get(&who).unwrap().rank, 1); + assert_last_event::(Event::RankChanged { who, rank: 1 }.into()); + } + + demote { + let who = make_member::(1); + let origin = T::AdminOrigin::successful_origin(); + let call = Call::::demote { who: who.clone() }; + }: { call.dispatch_bypass_filter(origin)? } + verify { + assert_eq!(Members::::get(&who).unwrap().rank, 0); + assert_last_event::(Event::RankChanged { who, rank: 0 }.into()); } vote { - let rank = 1; let caller: T::AccountId = whitelisted_caller(); - assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), caller.clone(), rank)); + assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), caller.clone())); // Create a poll - let class = T::Polls::classes().into_iter().next().expect("Must always be at least one class"); - let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll"); + let class = 0; + let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll for rank 0"); // Vote once. assert_ok!(Pallet::::vote(SystemOrigin::Signed(caller.clone()).into(), poll, true)); }: _(SystemOrigin::Signed(caller.clone()), poll, false) verify { - let tally = Tally::from_parts(0, 1); + let tally = Tally::from_parts(0, 0, 1); let ev = Event::Voted { who: caller, poll, vote: VoteRecord::Nay(1), tally }; assert_last_event::(ev.into()); } cleanup_poll { - let rank = 1; let n in 1 .. 100; // Create a poll - let class = T::Polls::classes().into_iter().next().expect("Must always be at least one class"); + let class = 0; let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll"); // Vote in the poll by each of `n` members for i in 0..n { - let who = make_member::(rank); + let who = make_member::(0); assert_ok!(Pallet::::vote(SystemOrigin::Signed(who).into(), poll, true)); } diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 9f3d9caba7c17..91b91c75cd346 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -54,9 +54,9 @@ use frame_support::{ codec::{Decode, Encode, MaxEncodedLen}, dispatch::{DispatchError, DispatchResultWithPostInfo}, ensure, - traits::{EnsureOrigin, Get, PollStatus, Polling, VoteTally}, + traits::{EnsureOrigin, PollStatus, Polling, VoteTally}, weights::PostDispatchInfo, - CloneNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, + CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; #[cfg(test)] @@ -81,7 +81,6 @@ pub type Votes = u32; /// Aggregated votes for an ongoing poll. #[derive( CloneNoBound, - DefaultNoBound, PartialEqNoBound, EqNoBound, RuntimeDebugNoBound, @@ -91,45 +90,62 @@ pub type Votes = u32; MaxEncodedLen, )] #[scale_info(skip_type_params(M))] -pub struct Tally> { +pub struct Tally { + bare_ayes: MemberIndex, ayes: Votes, nays: Votes, dummy: PhantomData, } -impl> Tally { - fn from_parts(ayes: Votes, nays: Votes) -> Self { - Tally { ayes, nays, dummy: PhantomData } +impl Tally { + fn from_parts(bare_ayes: MemberIndex, ayes: Votes, nays: Votes) -> Self { + Tally { bare_ayes, ayes, nays, dummy: PhantomData } } } +// Use (non-rank-weighted) ayes for calculating support. +// Allow only promotion/demotion by one rank only. +// Allow removal of member with rank zero only. +// This keeps everything O(1) while still allowing arbitrary number of ranks. + +// All functions of VoteTally now include the class as a param. +// TODO: ** BEFORE COMMIT ** split and move into gg2t branch. + pub type TallyOf = Tally>; pub type PollIndexOf = <>::Polls as Polling>>::Index; -impl> VoteTally for Tally { - fn ayes(&self) -> Votes { - self.ayes +impl VoteTally for Tally { + fn new(_: Rank) -> Self { + Self { bare_ayes: 0, ayes: 0, nays: 0, dummy: PhantomData } + } + fn ayes(&self, _: Rank) -> Votes { + self.bare_ayes } - fn support(&self) -> Perbill { - Perbill::from_rational(self.ayes, M::get()) + fn support(&self, class: Rank) -> Perbill { + Perbill::from_rational(self.bare_ayes, M::get_max_voters(class)) } - fn approval(&self) -> Perbill { + fn approval(&self, _: Rank) -> Perbill { Perbill::from_rational(self.ayes, 1.max(self.ayes + self.nays)) } #[cfg(feature = "runtime-benchmarks")] - fn unanimity() -> Self { - Self { ayes: M::get(), nays: 0, dummy: PhantomData } + fn unanimity(class: Rank) -> Self { + Self { + bare_ayes: M::get_max_voters(class), + ayes: M::get_max_voters(class), + nays: 0, + dummy: PhantomData, + } } #[cfg(feature = "runtime-benchmarks")] - fn rejection() -> Self { - Self { ayes: 0, nays: M::get(), dummy: PhantomData } + fn rejection(class: Rank) -> Self { + Self { bare_ayes: 0, ayes: 0, nays: M::get_max_voters(class), dummy: PhantomData } } #[cfg(feature = "runtime-benchmarks")] - fn from_requirements(support: Perbill, approval: Perbill) -> Self { - let c = M::get(); + fn from_requirements(support: Perbill, approval: Perbill, class: Rank) -> Self { + let c = M::get_max_voters(class); let ayes = support * c; let nays = ((ayes as u64) * 1_000_000_000u64 / approval.deconstruct() as u64) as u32 - ayes; - Self { ayes, nays, dummy: PhantomData } + Self { bare_ayes: ayes, ayes, nays, dummy: PhantomData } } } @@ -183,13 +199,14 @@ pub mod pallet { type AdminOrigin: EnsureOrigin; /// The polling system used for our voting. - type Polls: Polling, Votes = Votes, Moment = Self::BlockNumber>; + type Polls: Polling, Votes = Votes, Class = Rank, Moment = Self::BlockNumber>; } - /// The number of members in the collective. + /// The number of members in the collective who have at least the rank according to the index + /// of the vec. #[pallet::storage] pub type MemberCount, I: 'static = ()> = - StorageValue<_, (MemberIndex, Votes), ValueQuery>; + StorageMap<_, Twox64Concat, Rank, MemberIndex, ValueQuery>; /// The current members of the collective. #[pallet::storage] @@ -217,7 +234,7 @@ pub mod pallet { #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { /// A member has been added. - MemberAdded { who: T::AccountId, rank: Rank }, + MemberAdded { who: T::AccountId }, /// A member's rank has been changed. RankChanged { who: T::AccountId, rank: Rank }, /// A member has been removed. @@ -241,6 +258,11 @@ pub mod pallet { NoneRemaining, /// Unexpected error in state. Corruption, + /// The member's rank is too low to vote. + RankTooLow, + /// The member's rank is too high to be removed. + RankTooHigh, + } #[pallet::call] @@ -253,47 +275,58 @@ pub mod pallet { /// /// Weight: `O(1)` #[pallet::weight(T::WeightInfo::add_member())] - pub fn add_member(origin: OriginFor, who: T::AccountId, rank: Rank) -> DispatchResult { + pub fn add_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; ensure!(!Members::::contains_key(&who), Error::::AlreadyMember); - let (index, mut votes) = MemberCount::::get(); + let index = MemberCount::::get(0); let count = index.checked_add(1).ok_or(Overflow)?; - votes = votes.checked_add(Self::rank_to_votes(rank)).ok_or(Overflow)?; - Members::::insert(&who, MemberRecord { rank, index }); + Members::::insert(&who, MemberRecord { rank: 0, index }); MemberByIndex::::insert(index, &who); - MemberCount::::put((count, votes)); - Self::deposit_event(Event::MemberAdded { who, rank }); + MemberCount::::insert(0, count); + Self::deposit_event(Event::MemberAdded { who }); Ok(()) } - /// Alter the rank of an existing member. + /// Increment the rank of an existing member by one. /// /// - `origin`: Must be the `AdminOrigin`. /// - `who`: Account of existing member. - /// - `rank`: The new rank to give the member. /// /// Weight: `O(1)` #[pallet::weight(T::WeightInfo::set_member_rank())] - pub fn set_member_rank( + pub fn promote( + origin: OriginFor, + who: T::AccountId, + ) -> DispatchResult { + T::AdminOrigin::ensure_origin(origin)?; + let mut record = Self::ensure_member(&who)?; + record.rank = record.rank.checked_add(1).ok_or(Overflow)?; + MemberCount::::mutate(record.rank, |r| r.saturating_inc()); + Members::::insert(&who, &record); + Self::deposit_event(Event::RankChanged { who, rank: record.rank }); + + Ok(()) + } + + /// Decrement the rank of an existing member by one. + /// + /// - `origin`: Must be the `AdminOrigin`. + /// - `who`: Account of existing member of rank greater than zero. + /// + /// Weight: `O(1)` + #[pallet::weight(T::WeightInfo::set_member_rank())] + pub fn demote( origin: OriginFor, who: T::AccountId, - rank: Rank, ) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; let mut record = Self::ensure_member(&who)?; - let (count, mut votes) = MemberCount::::get(); - votes = votes - .checked_sub(Self::rank_to_votes(record.rank)) - .ok_or(Underflow)? - .checked_add(Self::rank_to_votes(rank)) - .ok_or(Overflow)?; - record.rank = rank; - - MemberCount::::put((count, votes)); - Members::::insert(&who, record); - Self::deposit_event(Event::RankChanged { who, rank }); + record.rank = record.rank.checked_sub(1).ok_or(Underflow)?; + MemberCount::::mutate(record.rank + 1, |r| r.saturating_dec()); + Members::::insert(&who, &record); + Self::deposit_event(Event::RankChanged { who, rank: record.rank }); Ok(()) } @@ -308,13 +341,12 @@ pub mod pallet { pub fn remove_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; let record = Self::ensure_member(&who)?; - let (count, votes) = MemberCount::::get(); - let count = count.checked_sub(1).ok_or(Underflow)?; - let votes = votes.checked_sub(Self::rank_to_votes(record.rank)).ok_or(Underflow)?; + ensure!(record.rank == 0, Error::::RankTooHigh); + let last_index = MemberCount::::get(0).saturating_sub(1); let index = record.index; - if index != count { - let last = MemberByIndex::::get(count).ok_or(Error::::Corruption)?; + if index != last_index { + let last = MemberByIndex::::get(last_index).ok_or(Error::::Corruption)?; Members::::mutate(&last, |r| { if let Some(ref mut r) = r { r.index = index @@ -322,9 +354,9 @@ pub mod pallet { }); MemberByIndex::::insert(index, &last); } - MemberByIndex::::remove(count); + MemberCount::::insert(0, last_index); + MemberByIndex::::remove(last_index); Members::::remove(&who); - MemberCount::::put((count, votes)); Self::deposit_event(Event::MemberRemoved { who }); Ok(()) @@ -350,25 +382,31 @@ pub mod pallet { let who = ensure_signed(origin)?; let record = Self::ensure_member(&who)?; use VoteRecord::*; - let votes = Self::rank_to_votes(record.rank); - let vote = VoteRecord::from((aye, votes)); let mut pays = Pays::Yes; - let tally = T::Polls::try_access_poll(poll, |mut status| -> Result, DispatchError> { + let (tally, vote) = T::Polls::try_access_poll(poll, |mut status| -> Result<(TallyOf, VoteRecord), DispatchError> { match status { PollStatus::None | PollStatus::Completed(..) => Err(Error::::NotPolling)?, - PollStatus::Ongoing(ref mut tally, _) => { + PollStatus::Ongoing(ref mut tally, min_rank) => { match Voting::::get(&poll, &who) { - Some(Aye(votes)) => tally.ayes.saturating_reduce(votes), + Some(Aye(votes)) => { + tally.bare_ayes.saturating_dec(); + tally.ayes.saturating_reduce(votes); + }, Some(Nay(votes)) => tally.nays.saturating_reduce(votes), None => pays = Pays::No, } + let votes = Self::rank_to_votes(record.rank, min_rank)?; + let vote = VoteRecord::from((aye, votes)); match aye { - true => tally.ayes.saturating_accrue(votes), + true => { + tally.bare_ayes.saturating_inc(); + tally.ayes.saturating_accrue(votes); + }, false => tally.nays.saturating_accrue(votes), } Voting::::insert(&poll, &who, &vote); - Ok(tally.clone()) + Ok((tally.clone(), vote)) }, } })?; @@ -413,15 +451,19 @@ pub mod pallet { Members::::get(who).ok_or(Error::::NotMember.into()) } - fn rank_to_votes(r: Rank) -> Votes { - let r = r as Votes; - r * (r + 1) / 2 + fn rank_to_votes(rank: Rank, min: Rank) -> Result { + let excess = rank.checked_sub(min).ok_or(Error::::RankTooLow)?; + let v = (excess + 1) as Votes; + Ok(v * (v + 1) / 2) } } - impl, I: 'static> Get for Pallet { - fn get() -> Votes { - MemberCount::::get().1 + pub trait GetMaxVoters { + fn get_max_voters(r: Rank) -> MemberIndex; + } + impl, I: 'static> GetMaxVoters for Pallet { + fn get_max_voters(r: Rank) -> MemberIndex { + MemberCount::::get(r) } } } diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index 948387f35833d..44cc8c9b331ed 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -79,7 +79,7 @@ impl frame_system::Config for Test { #[derive(Clone, PartialEq, Eq, Debug)] pub enum TestPollState { - Ongoing(TallyOf, u8), + Ongoing(TallyOf, Rank), Completed(u64, bool), } use TestPollState::*; @@ -88,7 +88,7 @@ parameter_types! { pub static Polls: BTreeMap = vec![ (1, Completed(1, true)), (2, Completed(2, false)), - (3, Ongoing(Tally::from_parts(0, 0), 0)), + (3, Ongoing(Tally::from_parts(0, 0, 0), 1)), ].into_iter().collect(); } @@ -97,8 +97,8 @@ impl Polling> for TestPolls { type Index = u8; type Votes = Votes; type Moment = u64; - type Class = u8; - fn classes() -> Vec { + type Class = Rank; + fn classes() -> Vec { vec![0, 1, 2] } fn as_ongoing(index: u8) -> Option<(TallyOf, Self::Class)> { @@ -147,7 +147,7 @@ impl Polling> for TestPolls { fn create_ongoing(class: Self::Class) -> Result { let mut polls = Polls::get(); let i = polls.keys().rev().next().map_or(0, |x| x + 1); - polls.insert(i, Ongoing(Tally::default(), class)); + polls.insert(i, Ongoing(Tally::new(class), class)); Polls::set(polls); Ok(i) } @@ -184,12 +184,8 @@ fn next_block() { System::set_block_number(System::block_number() + 1); } -fn member_count() -> MemberIndex { - MemberCount::::get().0 -} - -fn max_turnout() -> Votes { - MemberCount::::get().1 +fn member_count(r: Rank) -> MemberIndex { + MemberCount::::get(r) } #[allow(dead_code)] @@ -220,63 +216,128 @@ fn completed_poll_should_panic() { #[test] fn basic_stuff() { new_test_ext().execute_with(|| { - assert_eq!(tally(3), Tally::from_parts(0, 0)); + assert_eq!(tally(3), Tally::from_parts(0, 0, 0)); }); } + #[test] -fn membership_works() { +fn member_lifecycle_works() { new_test_ext().execute_with(|| { - assert_noop!(Club::add_member(Origin::signed(1), 1, 1), DispatchError::BadOrigin); - assert_ok!(Club::add_member(Origin::root(), 1, 1)); - assert_eq!(member_count(), 1); - assert_eq!(max_turnout(), 1); + assert_ok!(Club::add_member(Origin::root(), 1)); + assert_ok!(Club::promote(Origin::root(), 1)); + assert_ok!(Club::demote(Origin::root(), 1)); + assert_ok!(Club::remove_member(Origin::root(), 1)); + assert_eq!(member_count(0), 0); + assert_eq!(member_count(1), 0); + }); +} + +#[test] +fn add_remove_works() { + new_test_ext().execute_with(|| { + assert_noop!(Club::add_member(Origin::signed(1), 1), DispatchError::BadOrigin); + assert_ok!(Club::add_member(Origin::root(), 1)); + assert_eq!(member_count(0), 1); + + assert_ok!(Club::remove_member(Origin::root(), 1)); + assert_eq!(member_count(0), 0); + + assert_ok!(Club::add_member(Origin::root(), 1)); + assert_eq!(member_count(0), 1); + + assert_ok!(Club::add_member(Origin::root(), 2)); + assert_eq!(member_count(0), 2); + + assert_ok!(Club::add_member(Origin::root(), 3)); + assert_eq!(member_count(0), 3); + + assert_ok!(Club::remove_member(Origin::root(), 3)); + assert_eq!(member_count(0), 2); + + assert_ok!(Club::remove_member(Origin::root(), 1)); + assert_eq!(member_count(0), 1); + + assert_ok!(Club::remove_member(Origin::root(), 2)); + assert_eq!(member_count(0), 0); + }); +} + +#[test] +fn promote_demote_works() { + new_test_ext().execute_with(|| { + assert_noop!(Club::add_member(Origin::signed(1), 1), DispatchError::BadOrigin); + assert_ok!(Club::add_member(Origin::root(), 1)); + assert_eq!(member_count(0), 1); + assert_eq!(member_count(1), 0); + + assert_ok!(Club::add_member(Origin::root(), 2)); + assert_eq!(member_count(0), 2); + assert_eq!(member_count(1), 0); + + assert_ok!(Club::promote(Origin::root(), 1)); + assert_eq!(member_count(0), 2); + assert_eq!(member_count(1), 1); - assert_ok!(Club::add_member(Origin::root(), 2, 4)); - assert_eq!(member_count(), 2); - assert_eq!(max_turnout(), 11); + assert_ok!(Club::promote(Origin::root(), 2)); + assert_eq!(member_count(0), 2); + assert_eq!(member_count(1), 2); - assert_ok!(Club::set_member_rank(Origin::root(), 1, 4)); - assert_eq!(member_count(), 2); - assert_eq!(max_turnout(), 20); + assert_ok!(Club::demote(Origin::root(), 1)); + assert_eq!(member_count(0), 2); + assert_eq!(member_count(1), 1); assert_noop!(Club::remove_member(Origin::signed(1), 1), DispatchError::BadOrigin); + assert_noop!(Club::remove_member(Origin::root(), 2), Error::::RankTooHigh); assert_ok!(Club::remove_member(Origin::root(), 1)); - assert_eq!(member_count(), 1); - assert_eq!(max_turnout(), 10); + assert_eq!(member_count(0), 1); + assert_eq!(member_count(1), 1); }); } #[test] fn voting_works() { new_test_ext().execute_with(|| { - assert_ok!(Club::add_member(Origin::root(), 1, 1)); - assert_ok!(Club::add_member(Origin::root(), 2, 2)); - assert_ok!(Club::add_member(Origin::root(), 3, 3)); + assert_ok!(Club::add_member(Origin::root(), 0)); + assert_ok!(Club::add_member(Origin::root(), 1)); + assert_ok!(Club::promote(Origin::root(), 1)); + assert_ok!(Club::add_member(Origin::root(), 2)); + assert_ok!(Club::promote(Origin::root(), 2)); + assert_ok!(Club::promote(Origin::root(), 2)); + assert_ok!(Club::add_member(Origin::root(), 3)); + assert_ok!(Club::promote(Origin::root(), 3)); + assert_ok!(Club::promote(Origin::root(), 3)); + assert_ok!(Club::promote(Origin::root(), 3)); + + assert_noop!(Club::vote(Origin::signed(0), 3, true), Error::::RankTooLow); + assert_eq!(tally(3), Tally::from_parts(0, 0, 0)); assert_ok!(Club::vote(Origin::signed(1), 3, true)); - assert_eq!(tally(3), Tally::from_parts(1, 0)); + assert_eq!(tally(3), Tally::from_parts(1, 1, 0)); assert_ok!(Club::vote(Origin::signed(1), 3, false)); - assert_eq!(tally(3), Tally::from_parts(0, 1)); + assert_eq!(tally(3), Tally::from_parts(0, 0, 1)); assert_ok!(Club::vote(Origin::signed(2), 3, true)); - assert_eq!(tally(3), Tally::from_parts(3, 1)); + assert_eq!(tally(3), Tally::from_parts(1, 3, 1)); assert_ok!(Club::vote(Origin::signed(2), 3, false)); - assert_eq!(tally(3), Tally::from_parts(0, 4)); + assert_eq!(tally(3), Tally::from_parts(0, 0, 4)); assert_ok!(Club::vote(Origin::signed(3), 3, true)); - assert_eq!(tally(3), Tally::from_parts(6, 4)); + assert_eq!(tally(3), Tally::from_parts(1, 6, 4)); assert_ok!(Club::vote(Origin::signed(3), 3, false)); - assert_eq!(tally(3), Tally::from_parts(0, 10)); + assert_eq!(tally(3), Tally::from_parts(0, 0, 10)); }); } #[test] fn cleanup_works() { new_test_ext().execute_with(|| { - assert_ok!(Club::add_member(Origin::root(), 1, 1)); - assert_ok!(Club::add_member(Origin::root(), 2, 2)); - assert_ok!(Club::add_member(Origin::root(), 3, 3)); + assert_ok!(Club::add_member(Origin::root(), 1)); + assert_ok!(Club::promote(Origin::root(), 1)); + assert_ok!(Club::add_member(Origin::root(), 2)); + assert_ok!(Club::promote(Origin::root(), 2)); + assert_ok!(Club::add_member(Origin::root(), 3)); + assert_ok!(Club::promote(Origin::root(), 3)); assert_ok!(Club::vote(Origin::signed(1), 3, true)); assert_ok!(Club::vote(Origin::signed(2), 3, false)); From 0822cd65198dfbba61175d11d35a8b7f5ce16e8e Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 19 May 2022 14:10:15 +0100 Subject: [PATCH 06/25] Index by rank, still O(1). --- bin/node/runtime/src/lib.rs | 6 +- frame/ranked-collective/src/benchmarking.rs | 58 +++++---- frame/ranked-collective/src/lib.rs | 134 ++++++++++++-------- frame/ranked-collective/src/tests.rs | 43 +++---- frame/ranked-collective/src/weights.rs | 116 +++++++++++------ 5 files changed, 218 insertions(+), 139 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 480fca7972011..7ac7ed2efa1b6 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -780,11 +780,11 @@ parameter_types! { pub struct TracksInfo; impl pallet_referenda::TracksInfo for TracksInfo { - type Id = u8; + type Id = u16; type Origin = ::PalletsOrigin; fn tracks() -> &'static [(Self::Id, pallet_referenda::TrackInfo)] { - static DATA: [(u8, pallet_referenda::TrackInfo); 1] = [( - 0u8, + static DATA: [(u16, pallet_referenda::TrackInfo); 1] = [( + 0u16, pallet_referenda::TrackInfo { name: "root", max_deciding: 1, diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs index f571d35debf7d..c8f3c0ba02652 100644 --- a/frame/ranked-collective/src/benchmarking.rs +++ b/frame/ranked-collective/src/benchmarking.rs @@ -35,15 +35,11 @@ fn make_member, I: 'static>(rank: Rank) -> T::AccountId { let who = account::("member", MemberCount::::get(0), SEED); assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), who.clone())); for _ in 0..rank { - promote_member::(&who); + assert_ok!(Pallet::::promote_member(T::AdminOrigin::successful_origin(), who.clone())); } who } -fn promote_member, I: 'static>(who: &T::AccountId) { - assert_ok!(Pallet::::promote(T::AdminOrigin::successful_origin(), who.clone())); -} - benchmarks_instance_pallet! { add_member { let who = account::("member", 0, SEED); @@ -56,37 +52,53 @@ benchmarks_instance_pallet! { } remove_member { - let first = make_member::(0); - let who = make_member::(0); - let last = make_member::(0); - let last_index = Members::::get(&last).unwrap().index; + let r in 0 .. 10; + let rank = r as u16; + let first = make_member::(rank); + let who = make_member::(rank); + let last = make_member::(rank); + let last_index = (0..=rank).map(|r| IdToIndex::::get(r, &last).unwrap()).collect::>(); let origin = T::AdminOrigin::successful_origin(); - let call = Call::::remove_member { who: who.clone() }; + let call = Call::::remove_member { who: who.clone(), min_rank: rank }; }: { call.dispatch_bypass_filter(origin)? } verify { - assert_eq!(MemberCount::::get(0), 2); - assert_ne!(last_index, Members::::get(&last).unwrap().index); - assert_last_event::(Event::MemberRemoved { who }.into()); + for r in 0..=rank { + assert_eq!(MemberCount::::get(r), 2); + assert_ne!(last_index[r as usize], IdToIndex::::get(r, &last).unwrap()); + } + assert_last_event::(Event::MemberRemoved { who, rank }.into()); } - promote { - let who = make_member::(0); + promote_member { + let r in 0 .. 10; + let rank = r as u16; + let who = make_member::(rank); let origin = T::AdminOrigin::successful_origin(); - let call = Call::::promote { who: who.clone() }; + let call = Call::::promote_member { who: who.clone() }; }: { call.dispatch_bypass_filter(origin)? } verify { - assert_eq!(Members::::get(&who).unwrap().rank, 1); - assert_last_event::(Event::RankChanged { who, rank: 1 }.into()); + assert_eq!(Members::::get(&who).unwrap().rank, rank + 1); + assert_last_event::(Event::RankChanged { who, rank: rank + 1 }.into()); } - demote { - let who = make_member::(1); + demote_member { + let r in 0 .. 10; + let rank = r as u16; + let first = make_member::(rank); + let who = make_member::(rank); + let last = make_member::(rank); + let last_index = IdToIndex::::get(rank, &last).unwrap(); let origin = T::AdminOrigin::successful_origin(); - let call = Call::::demote { who: who.clone() }; + let call = Call::::demote_member { who: who.clone() }; }: { call.dispatch_bypass_filter(origin)? } verify { - assert_eq!(Members::::get(&who).unwrap().rank, 0); - assert_last_event::(Event::RankChanged { who, rank: 0 }.into()); + assert_eq!(Members::::get(&who).map(|x| x.rank), rank.checked_sub(1)); + assert_eq!(MemberCount::::get(rank), 2); + assert_ne!(last_index, IdToIndex::::get(rank, &last).unwrap()); + assert_last_event::(match rank { + 0 => Event::MemberRemoved { who, rank: 0 }, + r => Event::RankChanged { who, rank: r - 1 }, + }.into()); } vote { diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 91b91c75cd346..f33e01d867461 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -45,7 +45,7 @@ use scale_info::TypeInfo; use sp_arithmetic::traits::Saturating; use sp_runtime::{ - ArithmeticError::{Overflow, Underflow}, + ArithmeticError::Overflow, Perbill, RuntimeDebug, }; use sp_std::{marker::PhantomData, prelude::*}; @@ -152,8 +152,6 @@ impl VoteTally for Tally { /// Record needed for every member. #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] pub struct MemberRecord { - /// The index of the member. - index: MemberIndex, /// The rank of the member. rank: Rank, } @@ -213,11 +211,16 @@ pub mod pallet { pub type Members, I: 'static = ()> = StorageMap<_, Twox64Concat, T::AccountId, MemberRecord>; + /// The index of each ranks's member into the group of members who have at least that rank. + #[pallet::storage] + pub type IdToIndex, I: 'static = ()> = + StorageDoubleMap<_, Twox64Concat, Rank, Twox64Concat, T::AccountId, MemberIndex>; + /// The members in the collective by index. All indices in the range `0..MemberCount` will /// return `Some`, however a member's index is not guaranteed to remain unchanged over time. #[pallet::storage] - pub type MemberByIndex, I: 'static = ()> = - StorageMap<_, Twox64Concat, MemberIndex, T::AccountId>; + pub type IndexToId, I: 'static = ()> = + StorageDoubleMap<_, Twox64Concat, Rank, Twox64Concat, MemberIndex, T::AccountId>; /// Votes on a given proposal, if it is ongoing. #[pallet::storage] @@ -233,14 +236,14 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { - /// A member has been added. + /// A member `who` has been added. MemberAdded { who: T::AccountId }, - /// A member's rank has been changed. + /// The member `who`'s rank has been changed to the given `rank`. RankChanged { who: T::AccountId, rank: Rank }, - /// A member has been removed. - MemberRemoved { who: T::AccountId }, - /// A motion (given hash) has been voted on by given account, leaving - /// a tally (yes votes and no votes given respectively as `MemberIndex`). + /// The member `who` of given `rank` has been removed from the collective. + MemberRemoved { who: T::AccountId, rank: Rank }, + /// 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 }, } @@ -260,9 +263,8 @@ pub mod pallet { Corruption, /// The member's rank is too low to vote. RankTooLow, - /// The member's rank is too high to be removed. - RankTooHigh, - + /// The information provided is incorrect. + InvalidWitness, } #[pallet::call] @@ -281,8 +283,9 @@ pub mod pallet { let index = MemberCount::::get(0); let count = index.checked_add(1).ok_or(Overflow)?; - Members::::insert(&who, MemberRecord { rank: 0, index }); - MemberByIndex::::insert(index, &who); + Members::::insert(&who, MemberRecord { rank: 0 }); + IdToIndex::::insert(0, &who, index); + IndexToId::::insert(0, index, &who); MemberCount::::insert(0, count); Self::deposit_event(Event::MemberAdded { who }); @@ -295,71 +298,82 @@ pub mod pallet { /// - `who`: Account of existing member. /// /// Weight: `O(1)` - #[pallet::weight(T::WeightInfo::set_member_rank())] - pub fn promote( + #[pallet::weight(T::WeightInfo::promote_member(0))] + pub fn promote_member( origin: OriginFor, who: T::AccountId, ) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; - let mut record = Self::ensure_member(&who)?; - record.rank = record.rank.checked_add(1).ok_or(Overflow)?; - MemberCount::::mutate(record.rank, |r| r.saturating_inc()); - Members::::insert(&who, &record); - Self::deposit_event(Event::RankChanged { who, rank: record.rank }); + let record = Self::ensure_member(&who)?; + let rank = record.rank.checked_add(1).ok_or(Overflow)?; + let index = MemberCount::::get(rank); + MemberCount::::insert(rank, index.checked_add(1).ok_or(Overflow)?); + IdToIndex::::insert(rank, &who, index); + IndexToId::::insert(rank, index, &who); + Members::::insert(&who, MemberRecord { rank, .. record }); + Self::deposit_event(Event::RankChanged { who, rank }); Ok(()) } - /// Decrement the rank of an existing member by one. + /// Decrement the rank of an existing member by one. If the member is already at rank zero, + /// then they are removed entirely. /// /// - `origin`: Must be the `AdminOrigin`. /// - `who`: Account of existing member of rank greater than zero. /// - /// Weight: `O(1)` - #[pallet::weight(T::WeightInfo::set_member_rank())] - pub fn demote( + /// Weight: `O(1)`, less if the member's index is highest in its rank. + #[pallet::weight(T::WeightInfo::demote_member(0))] + pub fn demote_member( origin: OriginFor, who: T::AccountId, ) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; let mut record = Self::ensure_member(&who)?; - record.rank = record.rank.checked_sub(1).ok_or(Underflow)?; - MemberCount::::mutate(record.rank + 1, |r| r.saturating_dec()); - Members::::insert(&who, &record); - Self::deposit_event(Event::RankChanged { who, rank: record.rank }); - + let rank = record.rank; + + Self::remove_from_rank(&who, rank)?; + let maybe_rank = rank.checked_sub(1); + match maybe_rank { + None => { + Members::::remove(&who); + Self::deposit_event(Event::MemberRemoved { who, rank: 0 }); + } + Some(rank) => { + record.rank = rank; + Members::::insert(&who, &record); + Self::deposit_event(Event::RankChanged { who, rank }); + } + } Ok(()) } - /// Remove a member. + /// Remove the member entirely. /// /// - `origin`: Must be the `AdminOrigin`. - /// - `who`: Account of existing member to be removed. + /// - `who`: Account of existing member of rank greater than zero. + /// - `rank`: The rank of the member. /// - /// Weight: `O(1)`, less if the member's index is highest. - #[pallet::weight(T::WeightInfo::remove_member())] - pub fn remove_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { + /// Weight: `O(rank)`. + #[pallet::weight(T::WeightInfo::remove_member(*min_rank as u32))] + pub fn remove_member( + origin: OriginFor, + who: T::AccountId, + min_rank: Rank, + ) -> DispatchResultWithPostInfo { T::AdminOrigin::ensure_origin(origin)?; - let record = Self::ensure_member(&who)?; - ensure!(record.rank == 0, Error::::RankTooHigh); + let MemberRecord { rank, .. } = Self::ensure_member(&who)?; + ensure!(min_rank >= rank, Error::::InvalidWitness); - let last_index = MemberCount::::get(0).saturating_sub(1); - let index = record.index; - if index != last_index { - let last = MemberByIndex::::get(last_index).ok_or(Error::::Corruption)?; - Members::::mutate(&last, |r| { - if let Some(ref mut r) = r { - r.index = index - } - }); - MemberByIndex::::insert(index, &last); + for r in 0..=rank { + Self::remove_from_rank(&who, r)?; } - MemberCount::::insert(0, last_index); - MemberByIndex::::remove(last_index); Members::::remove(&who); - Self::deposit_event(Event::MemberRemoved { who }); - - Ok(()) + Self::deposit_event(Event::MemberRemoved { who, rank }); + Ok(PostDispatchInfo { + actual_weight: Some(T::WeightInfo::remove_member(rank as u32)), + pays_fee: Pays::Yes, + }) } /// Add an aye or nay vote for the sender to the given proposal. @@ -456,6 +470,18 @@ pub mod pallet { let v = (excess + 1) as Votes; Ok(v * (v + 1) / 2) } + + fn remove_from_rank(who: &T::AccountId, rank: Rank) -> DispatchResult { + let last_index = MemberCount::::get(rank).saturating_sub(1); + let index = IdToIndex::::get(rank, &who).ok_or(Error::::Corruption)?; + if index != last_index { + let last = IndexToId::::get(rank, last_index).ok_or(Error::::Corruption)?; + IdToIndex::::insert(rank, &last, index); + IndexToId::::insert(rank, index, &last); + } + MemberCount::::mutate(rank, |r| r.saturating_dec()); + Ok(()) + } } pub trait GetMaxVoters { diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index 44cc8c9b331ed..bd5fe1c6ba11a 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -225,9 +225,9 @@ fn basic_stuff() { fn member_lifecycle_works() { new_test_ext().execute_with(|| { assert_ok!(Club::add_member(Origin::root(), 1)); - assert_ok!(Club::promote(Origin::root(), 1)); - assert_ok!(Club::demote(Origin::root(), 1)); - assert_ok!(Club::remove_member(Origin::root(), 1)); + assert_ok!(Club::promote_member(Origin::root(), 1)); + assert_ok!(Club::demote_member(Origin::root(), 1)); + assert_ok!(Club::demote_member(Origin::root(), 1)); assert_eq!(member_count(0), 0); assert_eq!(member_count(1), 0); }); @@ -240,7 +240,7 @@ fn add_remove_works() { assert_ok!(Club::add_member(Origin::root(), 1)); assert_eq!(member_count(0), 1); - assert_ok!(Club::remove_member(Origin::root(), 1)); + assert_ok!(Club::demote_member(Origin::root(), 1)); assert_eq!(member_count(0), 0); assert_ok!(Club::add_member(Origin::root(), 1)); @@ -252,13 +252,13 @@ fn add_remove_works() { assert_ok!(Club::add_member(Origin::root(), 3)); assert_eq!(member_count(0), 3); - assert_ok!(Club::remove_member(Origin::root(), 3)); + assert_ok!(Club::demote_member(Origin::root(), 3)); assert_eq!(member_count(0), 2); - assert_ok!(Club::remove_member(Origin::root(), 1)); + assert_ok!(Club::demote_member(Origin::root(), 1)); assert_eq!(member_count(0), 1); - assert_ok!(Club::remove_member(Origin::root(), 2)); + assert_ok!(Club::demote_member(Origin::root(), 2)); assert_eq!(member_count(0), 0); }); } @@ -275,21 +275,20 @@ fn promote_demote_works() { assert_eq!(member_count(0), 2); assert_eq!(member_count(1), 0); - assert_ok!(Club::promote(Origin::root(), 1)); + assert_ok!(Club::promote_member(Origin::root(), 1)); assert_eq!(member_count(0), 2); assert_eq!(member_count(1), 1); - assert_ok!(Club::promote(Origin::root(), 2)); + assert_ok!(Club::promote_member(Origin::root(), 2)); assert_eq!(member_count(0), 2); assert_eq!(member_count(1), 2); - assert_ok!(Club::demote(Origin::root(), 1)); + assert_ok!(Club::demote_member(Origin::root(), 1)); assert_eq!(member_count(0), 2); assert_eq!(member_count(1), 1); - assert_noop!(Club::remove_member(Origin::signed(1), 1), DispatchError::BadOrigin); - assert_noop!(Club::remove_member(Origin::root(), 2), Error::::RankTooHigh); - assert_ok!(Club::remove_member(Origin::root(), 1)); + assert_noop!(Club::demote_member(Origin::signed(1), 1), DispatchError::BadOrigin); + assert_ok!(Club::demote_member(Origin::root(), 1)); assert_eq!(member_count(0), 1); assert_eq!(member_count(1), 1); }); @@ -300,14 +299,14 @@ fn voting_works() { new_test_ext().execute_with(|| { assert_ok!(Club::add_member(Origin::root(), 0)); assert_ok!(Club::add_member(Origin::root(), 1)); - assert_ok!(Club::promote(Origin::root(), 1)); + assert_ok!(Club::promote_member(Origin::root(), 1)); assert_ok!(Club::add_member(Origin::root(), 2)); - assert_ok!(Club::promote(Origin::root(), 2)); - assert_ok!(Club::promote(Origin::root(), 2)); + assert_ok!(Club::promote_member(Origin::root(), 2)); + assert_ok!(Club::promote_member(Origin::root(), 2)); assert_ok!(Club::add_member(Origin::root(), 3)); - assert_ok!(Club::promote(Origin::root(), 3)); - assert_ok!(Club::promote(Origin::root(), 3)); - assert_ok!(Club::promote(Origin::root(), 3)); + assert_ok!(Club::promote_member(Origin::root(), 3)); + assert_ok!(Club::promote_member(Origin::root(), 3)); + assert_ok!(Club::promote_member(Origin::root(), 3)); assert_noop!(Club::vote(Origin::signed(0), 3, true), Error::::RankTooLow); assert_eq!(tally(3), Tally::from_parts(0, 0, 0)); @@ -333,11 +332,11 @@ fn voting_works() { fn cleanup_works() { new_test_ext().execute_with(|| { assert_ok!(Club::add_member(Origin::root(), 1)); - assert_ok!(Club::promote(Origin::root(), 1)); + assert_ok!(Club::promote_member(Origin::root(), 1)); assert_ok!(Club::add_member(Origin::root(), 2)); - assert_ok!(Club::promote(Origin::root(), 2)); + assert_ok!(Club::promote_member(Origin::root(), 2)); assert_ok!(Club::add_member(Origin::root(), 3)); - assert_ok!(Club::promote(Origin::root(), 3)); + assert_ok!(Club::promote_member(Origin::root(), 3)); assert_ok!(Club::vote(Origin::signed(1), 3, true)); assert_ok!(Club::vote(Origin::signed(2), 3, false)); diff --git a/frame/ranked-collective/src/weights.rs b/frame/ranked-collective/src/weights.rs index ed08564774c1d..f287137fedfdd 100644 --- a/frame/ranked-collective/src/weights.rs +++ b/frame/ranked-collective/src/weights.rs @@ -18,20 +18,19 @@ //! Autogenerated weights for pallet_ranked_collective //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-05-18, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 +//! DATE: 2022-05-19, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: -// ../../../target/release/substrate +// /Users/gav/Core/substrate/target/release/substrate // benchmark // pallet +// --pallet +// pallet-ranked-collective +// --extrinsic=* // --chain=dev // --steps=50 // --repeat=20 -// --pallet=pallet-ranked-collective -// --extrinsic=* -// --execution=wasm -// --heap-pages=4096 // --output=../../../frame/ranked-collective/src/weights.rs // --template=../../../.maintain/frame-weight-template.hbs // --header=../../../HEADER-APACHE2 @@ -47,8 +46,9 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_ranked_collective. pub trait WeightInfo { fn add_member() -> Weight; - fn remove_member() -> Weight; - fn set_member_rank() -> Weight; + fn remove_member(r: u32, ) -> Weight; + fn promote_member(r: u32, ) -> Weight; + fn demote_member(r: u32, ) -> Weight; fn vote() -> Weight; fn cleanup_poll(n: u32, ) -> Weight; } @@ -58,42 +58,63 @@ pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { // Storage: RankedCollective Members (r:1 w:1) // Storage: RankedCollective MemberCount (r:1 w:1) - // Storage: RankedCollective MemberByIndex (r:0 w:1) + // Storage: RankedCollective IndexToId (r:0 w:1) + // Storage: RankedCollective IdToIndex (r:0 w:1) fn add_member() -> Weight { - (15_000_000 as Weight) + (11_000_000 as Weight) .saturating_add(T::DbWeight::get().reads(2 as Weight)) - .saturating_add(T::DbWeight::get().writes(3 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) } - // Storage: RankedCollective Members (r:2 w:2) + // Storage: RankedCollective Members (r:1 w:1) // Storage: RankedCollective MemberCount (r:1 w:1) - // Storage: RankedCollective MemberByIndex (r:1 w:2) - fn remove_member() -> Weight { - (23_000_000 as Weight) + // Storage: RankedCollective IdToIndex (r:1 w:1) + // Storage: RankedCollective IndexToId (r:1 w:1) + fn remove_member(r: u32, ) -> Weight { + (16_814_000 as Weight) + // Standard Error: 29_000 + .saturating_add((8_128_000 as Weight).saturating_mul(r as Weight)) .saturating_add(T::DbWeight::get().reads(4 as Weight)) - .saturating_add(T::DbWeight::get().writes(5 as Weight)) + .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(r as Weight))) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + .saturating_add(T::DbWeight::get().writes((3 as Weight).saturating_mul(r as Weight))) } // Storage: RankedCollective Members (r:1 w:1) // Storage: RankedCollective MemberCount (r:1 w:1) - fn set_member_rank() -> Weight { - (15_000_000 as Weight) + // Storage: RankedCollective IndexToId (r:0 w:1) + // Storage: RankedCollective IdToIndex (r:0 w:1) + fn promote_member(r: u32, ) -> Weight { + (12_036_000 as Weight) + // Standard Error: 7_000 + .saturating_add((7_000 as Weight).saturating_mul(r as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) - .saturating_add(T::DbWeight::get().writes(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) + } + // Storage: RankedCollective Members (r:1 w:1) + // Storage: RankedCollective MemberCount (r:1 w:1) + // Storage: RankedCollective IdToIndex (r:1 w:1) + // Storage: RankedCollective IndexToId (r:1 w:1) + fn demote_member(r: u32, ) -> Weight { + (17_605_000 as Weight) + // Standard Error: 17_000 + .saturating_add((165_000 as Weight).saturating_mul(r as Weight)) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(4 as Weight)) } // Storage: RankedCollective Members (r:1 w:0) // Storage: RankedPolls ReferendumInfoFor (r:1 w:1) // Storage: RankedCollective Voting (r:1 w:1) // Storage: Scheduler Agenda (r:2 w:2) fn vote() -> Weight { - (35_000_000 as Weight) + (22_000_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } // Storage: RankedPolls ReferendumInfoFor (r:1 w:0) // Storage: RankedCollective Voting (r:0 w:1) fn cleanup_poll(n: u32, ) -> Weight { - (7_800_000 as Weight) + (6_273_000 as Weight) // Standard Error: 1_000 - .saturating_add((860_000 as Weight).saturating_mul(n as Weight)) + .saturating_add((847_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(n as Weight))) } @@ -103,42 +124,63 @@ impl WeightInfo for SubstrateWeight { impl WeightInfo for () { // Storage: RankedCollective Members (r:1 w:1) // Storage: RankedCollective MemberCount (r:1 w:1) - // Storage: RankedCollective MemberByIndex (r:0 w:1) + // Storage: RankedCollective IndexToId (r:0 w:1) + // Storage: RankedCollective IdToIndex (r:0 w:1) fn add_member() -> Weight { - (15_000_000 as Weight) + (11_000_000 as Weight) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } - // Storage: RankedCollective Members (r:2 w:2) + // Storage: RankedCollective Members (r:1 w:1) // Storage: RankedCollective MemberCount (r:1 w:1) - // Storage: RankedCollective MemberByIndex (r:1 w:2) - fn remove_member() -> Weight { - (23_000_000 as Weight) + // Storage: RankedCollective IdToIndex (r:1 w:1) + // Storage: RankedCollective IndexToId (r:1 w:1) + fn remove_member(r: u32, ) -> Weight { + (16_814_000 as Weight) + // Standard Error: 29_000 + .saturating_add((8_128_000 as Weight).saturating_mul(r as Weight)) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) - .saturating_add(RocksDbWeight::get().writes(5 as Weight)) + .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(r as Weight))) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes((3 as Weight).saturating_mul(r as Weight))) } // Storage: RankedCollective Members (r:1 w:1) // Storage: RankedCollective MemberCount (r:1 w:1) - fn set_member_rank() -> Weight { - (15_000_000 as Weight) + // Storage: RankedCollective IndexToId (r:0 w:1) + // Storage: RankedCollective IdToIndex (r:0 w:1) + fn promote_member(r: u32, ) -> Weight { + (12_036_000 as Weight) + // Standard Error: 7_000 + .saturating_add((7_000 as Weight).saturating_mul(r as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - .saturating_add(RocksDbWeight::get().writes(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) + } + // Storage: RankedCollective Members (r:1 w:1) + // Storage: RankedCollective MemberCount (r:1 w:1) + // Storage: RankedCollective IdToIndex (r:1 w:1) + // Storage: RankedCollective IndexToId (r:1 w:1) + fn demote_member(r: u32, ) -> Weight { + (17_605_000 as Weight) + // Standard Error: 17_000 + .saturating_add((165_000 as Weight).saturating_mul(r as Weight)) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } // Storage: RankedCollective Members (r:1 w:0) // Storage: RankedPolls ReferendumInfoFor (r:1 w:1) // Storage: RankedCollective Voting (r:1 w:1) // Storage: Scheduler Agenda (r:2 w:2) fn vote() -> Weight { - (35_000_000 as Weight) + (22_000_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } // Storage: RankedPolls ReferendumInfoFor (r:1 w:0) // Storage: RankedCollective Voting (r:0 w:1) fn cleanup_poll(n: u32, ) -> Weight { - (7_800_000 as Weight) + (6_273_000 as Weight) // Standard Error: 1_000 - .saturating_add((860_000 as Weight).saturating_mul(n as Weight)) + .saturating_add((847_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(n as Weight))) } From 5688cb357546c082295f80fa3bef601f30315ba3 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 19 May 2022 14:34:05 +0100 Subject: [PATCH 07/25] Custom vote weights --- bin/node/runtime/src/lib.rs | 1 + frame/ranked-collective/src/lib.rs | 49 ++++++++++++++++++++++++-- frame/ranked-collective/src/tests.rs | 1 + frame/ranked-collective/src/weights.rs | 44 +++++++++++------------ 4 files changed, 71 insertions(+), 24 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 7ac7ed2efa1b6..980d20626513b 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -858,6 +858,7 @@ impl pallet_ranked_collective::Config for Runtime { type Event = Event; type AdminOrigin = EnsureRoot; type Polls = RankedPolls; + type VoteWeight = pallet_ranked_collective::Geometric; } impl pallet_remark::Config for Runtime { diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index f33e01d867461..49d0e0c4d1935 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -45,6 +45,7 @@ use scale_info::TypeInfo; use sp_arithmetic::traits::Saturating; use sp_runtime::{ + traits::Convert, ArithmeticError::Overflow, Perbill, RuntimeDebug, }; @@ -174,6 +175,45 @@ impl From<(bool, Votes)> for VoteRecord { } } +/// Vote-weight scheme where all voters get one vote regardless of rank. +pub struct Unit; +impl Convert for Unit { + fn convert(_: Rank) -> Votes { + 1 + } +} + +/// Vote-weight scheme where all voters get one vote plus an additional vote for every excess rank +/// they have. I.e.: +/// +/// - Each member with no excess rank gets 1 vote; +/// - ...with an excess rank of 1 gets 2 votes; +/// - ...with an excess rank of 2 gets 2 votes; +/// - ...with an excess rank of 3 gets 3 votes; +/// - ...with an excess rank of 4 gets 4 votes. +pub struct Linear; +impl Convert for Linear { + fn convert(r: Rank) -> Votes { + (r + 1) as Votes + } +} + +/// Vote-weight scheme where all voters get one vote plus additional votes for every excess rank +/// they have incrementing by one vote for each excess rank. I.e.: +/// +/// - Each member with no excess rank gets 1 vote; +/// - ...with an excess rank of 1 gets 2 votes; +/// - ...with an excess rank of 2 gets 3 votes; +/// - ...with an excess rank of 3 gets 6 votes; +/// - ...with an excess rank of 4 gets 10 votes. +pub struct Geometric; +impl Convert for Geometric { + fn convert(r: Rank) -> Votes { + let v = (r + 1) as Votes; + v * (v + 1) / 2 + } +} + #[frame_support::pallet] pub mod pallet { use super::*; @@ -198,6 +238,12 @@ pub mod pallet { /// The polling system used for our voting. type Polls: Polling, Votes = Votes, Class = Rank, Moment = Self::BlockNumber>; + + /// Convert a rank_delta into a number of votes the rank gets. + /// + /// Rank_delta is defined as the number of ranks above the minimum required to take part + /// in the poll. + type VoteWeight: Convert; } /// The number of members in the collective who have at least the rank according to the index @@ -467,8 +513,7 @@ pub mod pallet { fn rank_to_votes(rank: Rank, min: Rank) -> Result { let excess = rank.checked_sub(min).ok_or(Error::::RankTooLow)?; - let v = (excess + 1) as Votes; - Ok(v * (v + 1) / 2) + Ok(T::VoteWeight::convert(excess)) } fn remove_from_rank(who: &T::AccountId, rank: Rank) -> DispatchResult { diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index bd5fe1c6ba11a..f92027e6ace85 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -171,6 +171,7 @@ impl Config for Test { type Event = Event; type AdminOrigin = frame_system::EnsureRoot; type Polls = TestPolls; + type VoteWeight = Geometric; } pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/frame/ranked-collective/src/weights.rs b/frame/ranked-collective/src/weights.rs index f287137fedfdd..3048dd804a5e2 100644 --- a/frame/ranked-collective/src/weights.rs +++ b/frame/ranked-collective/src/weights.rs @@ -70,9 +70,9 @@ impl WeightInfo for SubstrateWeight { // Storage: RankedCollective IdToIndex (r:1 w:1) // Storage: RankedCollective IndexToId (r:1 w:1) fn remove_member(r: u32, ) -> Weight { - (16_814_000 as Weight) - // Standard Error: 29_000 - .saturating_add((8_128_000 as Weight).saturating_mul(r as Weight)) + (16_855_000 as Weight) + // Standard Error: 27_000 + .saturating_add((8_107_000 as Weight).saturating_mul(r as Weight)) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(r as Weight))) .saturating_add(T::DbWeight::get().writes(4 as Weight)) @@ -83,9 +83,9 @@ impl WeightInfo for SubstrateWeight { // Storage: RankedCollective IndexToId (r:0 w:1) // Storage: RankedCollective IdToIndex (r:0 w:1) fn promote_member(r: u32, ) -> Weight { - (12_036_000 as Weight) - // Standard Error: 7_000 - .saturating_add((7_000 as Weight).saturating_mul(r as Weight)) + (11_936_000 as Weight) + // Standard Error: 3_000 + .saturating_add((9_000 as Weight).saturating_mul(r as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } @@ -94,9 +94,9 @@ impl WeightInfo for SubstrateWeight { // Storage: RankedCollective IdToIndex (r:1 w:1) // Storage: RankedCollective IndexToId (r:1 w:1) fn demote_member(r: u32, ) -> Weight { - (17_605_000 as Weight) - // Standard Error: 17_000 - .saturating_add((165_000 as Weight).saturating_mul(r as Weight)) + (17_582_000 as Weight) + // Standard Error: 14_000 + .saturating_add((142_000 as Weight).saturating_mul(r as Weight)) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } @@ -112,9 +112,9 @@ impl WeightInfo for SubstrateWeight { // Storage: RankedPolls ReferendumInfoFor (r:1 w:0) // Storage: RankedCollective Voting (r:0 w:1) fn cleanup_poll(n: u32, ) -> Weight { - (6_273_000 as Weight) + (6_188_000 as Weight) // Standard Error: 1_000 - .saturating_add((847_000 as Weight).saturating_mul(n as Weight)) + .saturating_add((867_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(n as Weight))) } @@ -136,9 +136,9 @@ impl WeightInfo for () { // Storage: RankedCollective IdToIndex (r:1 w:1) // Storage: RankedCollective IndexToId (r:1 w:1) fn remove_member(r: u32, ) -> Weight { - (16_814_000 as Weight) - // Standard Error: 29_000 - .saturating_add((8_128_000 as Weight).saturating_mul(r as Weight)) + (16_855_000 as Weight) + // Standard Error: 27_000 + .saturating_add((8_107_000 as Weight).saturating_mul(r as Weight)) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(r as Weight))) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) @@ -149,9 +149,9 @@ impl WeightInfo for () { // Storage: RankedCollective IndexToId (r:0 w:1) // Storage: RankedCollective IdToIndex (r:0 w:1) fn promote_member(r: u32, ) -> Weight { - (12_036_000 as Weight) - // Standard Error: 7_000 - .saturating_add((7_000 as Weight).saturating_mul(r as Weight)) + (11_936_000 as Weight) + // Standard Error: 3_000 + .saturating_add((9_000 as Weight).saturating_mul(r as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } @@ -160,9 +160,9 @@ impl WeightInfo for () { // Storage: RankedCollective IdToIndex (r:1 w:1) // Storage: RankedCollective IndexToId (r:1 w:1) fn demote_member(r: u32, ) -> Weight { - (17_605_000 as Weight) - // Standard Error: 17_000 - .saturating_add((165_000 as Weight).saturating_mul(r as Weight)) + (17_582_000 as Weight) + // Standard Error: 14_000 + .saturating_add((142_000 as Weight).saturating_mul(r as Weight)) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } @@ -178,9 +178,9 @@ impl WeightInfo for () { // Storage: RankedPolls ReferendumInfoFor (r:1 w:0) // Storage: RankedCollective Voting (r:0 w:1) fn cleanup_poll(n: u32, ) -> Weight { - (6_273_000 as Weight) + (6_188_000 as Weight) // Standard Error: 1_000 - .saturating_add((847_000 as Weight).saturating_mul(n as Weight)) + .saturating_add((867_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(n as Weight))) } From 919589d0ef74471c18c21393a40bda39ad63af79 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 24 May 2022 11:29:31 +0200 Subject: [PATCH 08/25] Formatting --- frame/ranked-collective/src/benchmarking.rs | 5 +- frame/ranked-collective/src/lib.rs | 90 ++++++++++----------- frame/ranked-collective/src/tests.rs | 3 +- 3 files changed, 50 insertions(+), 48 deletions(-) diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs index c8f3c0ba02652..042287900852c 100644 --- a/frame/ranked-collective/src/benchmarking.rs +++ b/frame/ranked-collective/src/benchmarking.rs @@ -35,7 +35,10 @@ fn make_member, I: 'static>(rank: Rank) -> T::AccountId { let who = account::("member", MemberCount::::get(0), SEED); assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), who.clone())); for _ in 0..rank { - assert_ok!(Pallet::::promote_member(T::AdminOrigin::successful_origin(), who.clone())); + assert_ok!(Pallet::::promote_member( + T::AdminOrigin::successful_origin(), + who.clone() + )); } who } diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 49d0e0c4d1935..54114805f02fa 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -44,11 +44,7 @@ use scale_info::TypeInfo; use sp_arithmetic::traits::Saturating; -use sp_runtime::{ - traits::Convert, - ArithmeticError::Overflow, - Perbill, RuntimeDebug, -}; +use sp_runtime::{traits::Convert, ArithmeticError::Overflow, Perbill, RuntimeDebug}; use sp_std::{marker::PhantomData, prelude::*}; use frame_support::{ @@ -237,7 +233,12 @@ pub mod pallet { type AdminOrigin: EnsureOrigin; /// The polling system used for our voting. - type Polls: Polling, Votes = Votes, Class = Rank, Moment = Self::BlockNumber>; + type Polls: Polling< + TallyOf, + Votes = Votes, + Class = Rank, + Moment = Self::BlockNumber, + >; /// Convert a rank_delta into a number of votes the rank gets. /// @@ -345,10 +346,7 @@ pub mod pallet { /// /// Weight: `O(1)` #[pallet::weight(T::WeightInfo::promote_member(0))] - pub fn promote_member( - origin: OriginFor, - who: T::AccountId, - ) -> DispatchResult { + pub fn promote_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; let record = Self::ensure_member(&who)?; let rank = record.rank.checked_add(1).ok_or(Overflow)?; @@ -356,7 +354,7 @@ pub mod pallet { MemberCount::::insert(rank, index.checked_add(1).ok_or(Overflow)?); IdToIndex::::insert(rank, &who, index); IndexToId::::insert(rank, index, &who); - Members::::insert(&who, MemberRecord { rank, .. record }); + Members::::insert(&who, MemberRecord { rank, ..record }); Self::deposit_event(Event::RankChanged { who, rank }); Ok(()) @@ -370,10 +368,7 @@ pub mod pallet { /// /// Weight: `O(1)`, less if the member's index is highest in its rank. #[pallet::weight(T::WeightInfo::demote_member(0))] - pub fn demote_member( - origin: OriginFor, - who: T::AccountId, - ) -> DispatchResult { + pub fn demote_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; let mut record = Self::ensure_member(&who)?; let rank = record.rank; @@ -384,12 +379,12 @@ pub mod pallet { None => { Members::::remove(&who); Self::deposit_event(Event::MemberRemoved { who, rank: 0 }); - } + }, Some(rank) => { record.rank = rank; Members::::insert(&who, &record); Self::deposit_event(Event::RankChanged { who, rank }); - } + }, } Ok(()) } @@ -444,32 +439,36 @@ pub mod pallet { use VoteRecord::*; let mut pays = Pays::Yes; - let (tally, vote) = T::Polls::try_access_poll(poll, |mut status| -> Result<(TallyOf, VoteRecord), DispatchError> { - match status { - PollStatus::None | PollStatus::Completed(..) => Err(Error::::NotPolling)?, - PollStatus::Ongoing(ref mut tally, min_rank) => { - match Voting::::get(&poll, &who) { - Some(Aye(votes)) => { - tally.bare_ayes.saturating_dec(); - tally.ayes.saturating_reduce(votes); - }, - Some(Nay(votes)) => tally.nays.saturating_reduce(votes), - None => pays = Pays::No, - } - let votes = Self::rank_to_votes(record.rank, min_rank)?; - let vote = VoteRecord::from((aye, votes)); - match aye { - true => { - tally.bare_ayes.saturating_inc(); - tally.ayes.saturating_accrue(votes); - }, - false => tally.nays.saturating_accrue(votes), - } - Voting::::insert(&poll, &who, &vote); - Ok((tally.clone(), vote)) - }, - } - })?; + let (tally, vote) = T::Polls::try_access_poll( + poll, + |mut status| -> Result<(TallyOf, VoteRecord), DispatchError> { + match status { + PollStatus::None | PollStatus::Completed(..) => + Err(Error::::NotPolling)?, + PollStatus::Ongoing(ref mut tally, min_rank) => { + match Voting::::get(&poll, &who) { + Some(Aye(votes)) => { + tally.bare_ayes.saturating_dec(); + tally.ayes.saturating_reduce(votes); + }, + Some(Nay(votes)) => tally.nays.saturating_reduce(votes), + None => pays = Pays::No, + } + let votes = Self::rank_to_votes(record.rank, min_rank)?; + let vote = VoteRecord::from((aye, votes)); + match aye { + true => { + tally.bare_ayes.saturating_inc(); + tally.ayes.saturating_accrue(votes); + }, + false => tally.nays.saturating_accrue(votes), + } + Voting::::insert(&poll, &who, &vote); + Ok((tally.clone(), vote)) + }, + } + }, + )?; Self::deposit_event(Event::Voted { who, poll, vote, tally }); Ok(pays.into()) } @@ -495,7 +494,7 @@ pub mod pallet { use sp_io::KillStorageResult::*; let count = match Voting::::remove_prefix(poll_index, Some(max)) { -// AllRemoved(0) => Err(Error::::NoneRemaining)?, + // AllRemoved(0) => Err(Error::::NoneRemaining)?, AllRemoved(0) => return Ok(Pays::Yes.into()), AllRemoved(n) | SomeRemaining(n) => n, }; @@ -520,7 +519,8 @@ pub mod pallet { let last_index = MemberCount::::get(rank).saturating_sub(1); let index = IdToIndex::::get(rank, &who).ok_or(Error::::Corruption)?; if index != last_index { - let last = IndexToId::::get(rank, last_index).ok_or(Error::::Corruption)?; + let last = + IndexToId::::get(rank, last_index).ok_or(Error::::Corruption)?; IdToIndex::::insert(rank, &last, index); IndexToId::::insert(rank, index, &last); } diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index f92027e6ace85..88b61228ffeff 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -221,7 +221,6 @@ fn basic_stuff() { }); } - #[test] fn member_lifecycle_works() { new_test_ext().execute_with(|| { @@ -351,6 +350,6 @@ fn cleanup_works() { ); assert_ok!(Club::cleanup_poll(Origin::signed(4), 3, 10)); // NOTE: This will fail until #10016 is merged. -// assert_noop!(Club::cleanup_poll(Origin::signed(4), 3, 10), Error::::NoneRemaining); + // assert_noop!(Club::cleanup_poll(Origin::signed(4), 3, 10), Error::::NoneRemaining); }); } From 4deff132fdf58fe7555f194935f720b9d9b2ef25 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Tue, 31 May 2022 13:22:03 +0100 Subject: [PATCH 09/25] Update frame/ranked-collective/src/lib.rs --- frame/ranked-collective/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 54114805f02fa..eeeb72f696089 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -106,7 +106,6 @@ impl Tally { // This keeps everything O(1) while still allowing arbitrary number of ranks. // All functions of VoteTally now include the class as a param. -// TODO: ** BEFORE COMMIT ** split and move into gg2t branch. pub type TallyOf = Tally>; pub type PollIndexOf = <>::Polls as Polling>>::Index; From d5a21669bcdb31aef3d6c530dc9196e024e4641b Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 15:33:47 +0100 Subject: [PATCH 10/25] Broken :( --- frame/ranked-collective/src/lib.rs | 120 ++++++++++++------ frame/ranked-collective/src/tests.rs | 4 +- frame/support/src/storage/bounded_vec.rs | 10 +- frame/support/src/storage/mod.rs | 6 +- frame/support/src/storage/types/double_map.rs | 21 ++- frame/support/src/storage/types/map.rs | 18 ++- 6 files changed, 134 insertions(+), 45 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 54114805f02fa..8c9db292f4643 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -15,29 +15,28 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Ranked collective system: Members of a set of account IDs can make their collective feelings -//! known through dispatched calls from one of two specialized origins. +//! Ranked collective system. //! -//! The membership can be provided in one of two ways: either directly, using the Root-dispatchable -//! function `set_members`, or indirectly, through implementing the `ChangeMembers`. -//! The pallet assumes that the amount of members stays at or below `MaxMembers` for its weight -//! calculations, but enforces this neither in `set_members` nor in `change_members_sorted`. +//! This is a membership pallet providing a `Tally` implementation ready for use with polling +//! systems such as the Referenda pallet. Members each have a rank, with zero being the lowest. +//! There is no complexity limitation on either the number of members at a rank or the number of +//! ranks in the system thus allowing potentially public membership. A member of at least a given +//! rank can be selected at random in O(1) time, allowing for various games to constructed using +//! this as a primitive. Members may only be promoted and demoted by one rank at a time, however +//! all operations (save one) are O(1) in complexity. The only operation which is not O(1) is the +//! `remove_member` since they must be removed from all ranks from the present down to zero. //! -//! A "prime" member may be set to help determine the default vote behavior based on chain -//! config. If `PrimeDefaultVote` is used, the prime vote acts as the default vote in case of any -//! abstentions after the voting period. If `MoreThanMajorityThenPrimeDefaultVote` is used, then -//! abstentions will first follow the majority of the collective voting, and then the prime -//! member. +//! Different ranks have different voting power, and are able to vote in different polls. In general +//! rank privileges are cumulative. Higher ranks are able to vote in any polls open to lower ranks. +//! Similarly, higher ranks always have at least as much voting power in any given poll as lower +//! ranks. //! -//! Voting happens through motions comprising a proposal (i.e. a curried dispatchable) plus a -//! number of approvals required for it to pass and be called. Motions are open for members to -//! vote on for a minimum period given by `MotionDuration`. As soon as the needed number of -//! approvals is given, the motion is closed and executed. If the number of approvals is not reached -//! during the voting period, then `close` may be called by any account in order to force the end -//! the motion explicitly. If a prime member is defined then their vote is used in place of any -//! abstentions and the proposal is executed if there are enough approvals counting the new votes. +//! Two `Config` trait items control these "rank privileges": `MinRankOfClass` and `VoteWeight`. +//! The first controls which ranks are allowed to vote on a particular class of poll. The second +//! controls the weight of a vote given the voters rank compared to the minimum rank of the poll. //! -//! If there are not, or if no prime is set, then the motion is dropped without being executed. +//! An origin control, `EnsureRank`, ensures that the origin is a member of the collective of at +//! least a particular rank. #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "128"] @@ -51,7 +50,7 @@ use frame_support::{ codec::{Decode, Encode, MaxEncodedLen}, dispatch::{DispatchError, DispatchResultWithPostInfo}, ensure, - traits::{EnsureOrigin, PollStatus, Polling, VoteTally}, + traits::{EnsureOrigin, OriginTrait, PollStatus, Polling, VoteTally}, weights::PostDispatchInfo, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; @@ -106,7 +105,6 @@ impl Tally { // This keeps everything O(1) while still allowing arbitrary number of ranks. // All functions of VoteTally now include the class as a param. -// TODO: ** BEFORE COMMIT ** split and move into gg2t branch. pub type TallyOf = Tally>; pub type PollIndexOf = <>::Polls as Polling>>::Index; @@ -210,10 +208,43 @@ impl Convert for Geometric { } } +pub trait GetMaxVoters { + fn get_max_voters(r: Rank) -> MemberIndex; +} +impl, I: 'static> GetMaxVoters for Pallet { + fn get_max_voters(r: Rank) -> MemberIndex { + MemberCount::::get(r) + } +} + +pub struct EnsureRanked(PhantomData<(T, I)>); +impl< + T: Config, + I: 'static, + const MinRank: u16, +> EnsureOrigin for EnsureRanked { + type Success = T::AccountId; + + fn try_origin(o: T::Origin) -> Result { + let who = frame_system::EnsureSigned::try_origin(o)?; + match Members::::get(&who) { + Some(MemberRecord { rank, .. }) if rank >= MinRank => Ok(who), + _ => Err(frame_system::RawOrigin::Signed(who).into()), + } + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> T::Origin { + let who = IndexToId::::get(MinRank, 0) + .expect("Must be at least one member at rank for a successful origin"); + frame_system::RawOrigin::Signed(who).into() + } +} + #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::pallet_prelude::*; + use frame_support::{pallet_prelude::*, storage::KeyLenOf}; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -236,15 +267,22 @@ pub mod pallet { type Polls: Polling< TallyOf, Votes = Votes, - Class = Rank, Moment = Self::BlockNumber, >; + /// Convert the tally class into the minimum rank required to vote on the poll. If + /// `Polls::Class` is the same type as `Rank`, then `Identity` can be used here to mean + /// "a rank of at least the poll class". + type MinRankOfClass: Convert<>>::Class, Rank>; + /// Convert a rank_delta into a number of votes the rank gets. /// /// Rank_delta is defined as the number of ranks above the minimum required to take part /// in the poll. type VoteWeight: Convert; + + /// Dummy Get impl. + type TestGetter: Get; } /// The number of members in the collective who have at least the rank according to the index @@ -280,6 +318,14 @@ pub mod pallet { VoteRecord, >; + #[pallet::storage] + pub type VotingCleanup, I: 'static = ()> = StorageMap< + _, + Blake2_128Concat, + PollIndexOf, + BoundedVec> >, + >; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { @@ -445,7 +491,7 @@ pub mod pallet { match status { PollStatus::None | PollStatus::Completed(..) => Err(Error::::NotPolling)?, - PollStatus::Ongoing(ref mut tally, min_rank) => { + PollStatus::Ongoing(ref mut tally, class) => { match Voting::::get(&poll, &who) { Some(Aye(votes)) => { tally.bare_ayes.saturating_dec(); @@ -454,6 +500,7 @@ pub mod pallet { Some(Nay(votes)) => tally.nays.saturating_reduce(votes), None => pays = Pays::No, } + let min_rank = T::MinRankOfClass::convert(class); let votes = Self::rank_to_votes(record.rank, min_rank)?; let vote = VoteRecord::from((aye, votes)); match aye { @@ -492,14 +539,16 @@ pub mod pallet { ensure_signed(origin)?; ensure!(T::Polls::as_ongoing(poll_index).is_none(), Error::::Ongoing); - use sp_io::KillStorageResult::*; - let count = match Voting::::remove_prefix(poll_index, Some(max)) { - // AllRemoved(0) => Err(Error::::NoneRemaining)?, - AllRemoved(0) => return Ok(Pays::Yes.into()), - AllRemoved(n) | SomeRemaining(n) => n, - }; + let r = Voting::::clear_prefix(poll_index, max, VotingCleanup::::take(poll_index).as_ref().map(|c| &c[..])); + if r.unique == 0 { + // return Err(Error::::NoneRemaining) + return Ok(Pays::Yes.into()) + } + if let Some(cursor) = r.maybe_cursor { + VotingCleanup::::insert(poll_index, BoundedVec::truncate_from(cursor)); + } Ok(PostDispatchInfo { - actual_weight: Some(T::WeightInfo::cleanup_poll(count)), + actual_weight: Some(T::WeightInfo::cleanup_poll(r.unique)), pays_fee: Pays::No, }) } @@ -528,13 +577,4 @@ pub mod pallet { Ok(()) } } - - pub trait GetMaxVoters { - fn get_max_voters(r: Rank) -> MemberIndex; - } - impl, I: 'static> GetMaxVoters for Pallet { - fn get_max_voters(r: Rank) -> MemberIndex { - MemberCount::::get(r) - } - } } diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index 88b61228ffeff..faad32f5711c6 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -26,7 +26,7 @@ use frame_support::{ use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, IdentityLookup}, + traits::{BlakeTwo256, IdentityLookup, Identity}, }; use super::*; @@ -171,7 +171,9 @@ impl Config for Test { type Event = Event; type AdminOrigin = frame_system::EnsureRoot; type Polls = TestPolls; + type MinRankOfClass = Identity; type VoteWeight = Geometric; + type TestGetter = frame_support::storage::KeyLenOf>; } pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index d3f9bfdd21d3b..0b633f5043452 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -47,7 +47,9 @@ use sp_std::{marker::PhantomData, prelude::*}; #[scale_info(skip_type_params(S))] pub struct BoundedVec( Vec, - #[cfg_attr(feature = "std", serde(skip_serializing))] PhantomData, + #[cfg_attr(feature = "std", serde(skip_serializing))] + #[codec(skip)] + PhantomData, ); #[cfg(feature = "std")] @@ -286,6 +288,12 @@ impl> BoundedVec { Self::with_bounded_capacity(Self::bound()) } + /// Consume and truncate the vector `v` in order to create a new instance of `Self` from it. + pub fn truncate_from(mut v: Vec) -> Self { + v.truncate(Self::bound()); + Self::unchecked_from(v) + } + /// Get the bound of the type in `usize`. pub fn bound() -> usize { S::get() as usize diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 8b6f198b03f96..f8f78ce535330 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -27,7 +27,7 @@ use crate::{ use codec::{Decode, Encode, EncodeLike, FullCodec, FullEncode}; use sp_core::storage::ChildInfo; use sp_runtime::generic::{Digest, DigestItem}; -use sp_std::prelude::*; +use sp_std::{prelude::*, marker::PhantomData}; pub use self::{ transactional::{ @@ -51,6 +51,10 @@ pub mod types; pub mod unhashed; pub mod weak_bounded_vec; +/// Utility type for converting a storage map into a `Get` impl which returns the maximum +/// key size. +pub struct KeyLenOf(PhantomData); + /// A trait for working with macro-generated storage values under the substrate storage API. /// /// Details on implementation can be found at [`generator::StorageValue`]. diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 5662087cc896f..a977eab411a56 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -22,7 +22,7 @@ use crate::{ metadata::{StorageEntryMetadata, StorageEntryType}, storage::{ types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, - StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, + StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, KeyLenOf, }, traits::{Get, GetDefault, StorageInfo, StorageInstance}, }; @@ -70,6 +70,25 @@ pub struct StorageDoubleMap< )>, ); +impl Get +for KeyLenOf> +where + Prefix: StorageInstance, + Hasher1: crate::hash::StorageHasher, + Hasher2: crate::hash::StorageHasher, + Key1: MaxEncodedLen, + Key2: MaxEncodedLen, +{ + fn get() -> u32 { + let z = Hasher1::max_len::() + + Hasher2::max_len::() + + Prefix::pallet_prefix().len() + + Prefix::STORAGE_PREFIX.len(); + z as u32 + } +} + + impl crate::storage::generator::StorageDoubleMap for StorageDoubleMap diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index 6dac5b3756598..f041a71c9f659 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -22,7 +22,7 @@ use crate::{ metadata::{StorageEntryMetadata, StorageEntryType}, storage::{ types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, - StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, + StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, KeyLenOf, }, traits::{Get, GetDefault, StorageInfo, StorageInstance}, }; @@ -53,6 +53,22 @@ pub struct StorageMap< MaxValues = GetDefault, >(core::marker::PhantomData<(Prefix, Hasher, Key, Value, QueryKind, OnEmpty, MaxValues)>); +impl + Get + for KeyLenOf> +where + Prefix: StorageInstance, + Hasher: crate::hash::StorageHasher, + Key: FullCodec + MaxEncodedLen, +{ + fn get() -> u32 { + let z = Hasher::max_len::() + + Prefix::pallet_prefix().len() + + Prefix::STORAGE_PREFIX.len(); + z as u32 + } +} + impl crate::storage::generator::StorageMap for StorageMap From fd172b804593387c3e46e0b4b7c6b89fc13cdfa5 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 16:02:10 +0100 Subject: [PATCH 11/25] origin guard; cleanup uses new API --- frame/ranked-collective/src/lib.rs | 17 ++++------ frame/ranked-collective/src/tests.rs | 34 ++++++++++++++++++- .../src/construct_runtime/expand/origin.rs | 9 ++++- frame/support/src/traits/dispatch.rs | 3 ++ frame/support/src/traits/voting.rs | 8 ++--- 5 files changed, 55 insertions(+), 16 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 8c9db292f4643..7bb81912eda92 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -50,7 +50,7 @@ use frame_support::{ codec::{Decode, Encode, MaxEncodedLen}, dispatch::{DispatchError, DispatchResultWithPostInfo}, ensure, - traits::{EnsureOrigin, OriginTrait, PollStatus, Polling, VoteTally}, + traits::{EnsureOrigin, PollStatus, Polling, VoteTally}, weights::PostDispatchInfo, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; @@ -217,25 +217,25 @@ impl, I: 'static> GetMaxVoters for Pallet { } } -pub struct EnsureRanked(PhantomData<(T, I)>); +pub struct EnsureRanked(PhantomData<(T, I)>); impl< T: Config, I: 'static, - const MinRank: u16, -> EnsureOrigin for EnsureRanked { + const MIN_RANK: u16, +> EnsureOrigin for EnsureRanked { type Success = T::AccountId; fn try_origin(o: T::Origin) -> Result { let who = frame_system::EnsureSigned::try_origin(o)?; match Members::::get(&who) { - Some(MemberRecord { rank, .. }) if rank >= MinRank => Ok(who), + Some(MemberRecord { rank, .. }) if rank >= MIN_RANK => Ok(who), _ => Err(frame_system::RawOrigin::Signed(who).into()), } } #[cfg(feature = "runtime-benchmarks")] fn successful_origin() -> T::Origin { - let who = IndexToId::::get(MinRank, 0) + let who = IndexToId::::get(MIN_RANK, 0) .expect("Must be at least one member at rank for a successful origin"); frame_system::RawOrigin::Signed(who).into() } @@ -280,9 +280,6 @@ pub mod pallet { /// Rank_delta is defined as the number of ranks above the minimum required to take part /// in the poll. type VoteWeight: Convert; - - /// Dummy Get impl. - type TestGetter: Get; } /// The number of members in the collective who have at least the rank according to the index @@ -323,7 +320,7 @@ pub mod pallet { _, Blake2_128Concat, PollIndexOf, - BoundedVec> >, + BoundedVec>>, >; #[pallet::event] diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index faad32f5711c6..4bd1be7901715 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -173,7 +173,6 @@ impl Config for Test { type Polls = TestPolls; type MinRankOfClass = Identity; type VoteWeight = Geometric; - type TestGetter = frame_support::storage::KeyLenOf>; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -355,3 +354,36 @@ fn cleanup_works() { // assert_noop!(Club::cleanup_poll(Origin::signed(4), 3, 10), Error::::NoneRemaining); }); } + +#[test] +fn ensure_ranked_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(Origin::root(), 1)); + assert_ok!(Club::promote_member(Origin::root(), 1)); + assert_ok!(Club::add_member(Origin::root(), 2)); + assert_ok!(Club::promote_member(Origin::root(), 2)); + assert_ok!(Club::promote_member(Origin::root(), 2)); + assert_ok!(Club::add_member(Origin::root(), 3)); + assert_ok!(Club::promote_member(Origin::root(), 3)); + assert_ok!(Club::promote_member(Origin::root(), 3)); + assert_ok!(Club::promote_member(Origin::root(), 3)); + + use frame_support::traits::OriginTrait; + type Rank1 = EnsureRanked::; + type Rank2 = EnsureRanked::; + type Rank3 = EnsureRanked::; + type Rank4 = EnsureRanked::; + assert_eq!(Rank1::try_origin(Origin::signed(1)).unwrap(), 1); + assert_eq!(Rank1::try_origin(Origin::signed(2)).unwrap(), 2); + assert_eq!(Rank1::try_origin(Origin::signed(3)).unwrap(), 3); + assert_eq!(Rank2::try_origin(Origin::signed(1)).unwrap_err().as_signed().unwrap(), 1); + assert_eq!(Rank2::try_origin(Origin::signed(2)).unwrap(), 2); + assert_eq!(Rank2::try_origin(Origin::signed(3)).unwrap(), 3); + assert_eq!(Rank3::try_origin(Origin::signed(1)).unwrap_err().as_signed().unwrap(), 1); + assert_eq!(Rank3::try_origin(Origin::signed(2)).unwrap_err().as_signed().unwrap(), 2); + assert_eq!(Rank3::try_origin(Origin::signed(3)).unwrap(), 3); + assert_eq!(Rank4::try_origin(Origin::signed(1)).unwrap_err().as_signed().unwrap(), 1); + assert_eq!(Rank4::try_origin(Origin::signed(2)).unwrap_err().as_signed().unwrap(), 2); + assert_eq!(Rank4::try_origin(Origin::signed(3)).unwrap_err().as_signed().unwrap(), 3); + }); +} \ No newline at end of file diff --git a/frame/support/procedural/src/construct_runtime/expand/origin.rs b/frame/support/procedural/src/construct_runtime/expand/origin.rs index 342a002b52b9d..d347818d65c38 100644 --- a/frame/support/procedural/src/construct_runtime/expand/origin.rs +++ b/frame/support/procedural/src/construct_runtime/expand/origin.rs @@ -186,9 +186,16 @@ pub fn expand_outer_origin( #system_path::RawOrigin::Root.into() } - fn signed(by: <#runtime as #system_path::Config>::AccountId) -> Self { + fn signed(by: Self::AccountId) -> Self { #system_path::RawOrigin::Signed(by).into() } + + fn as_signed(self) -> Option { + match self.caller { + OriginCaller::system(#system_path::RawOrigin::Signed(by)) => Some(by), + _ => None, + } + } } #[derive( diff --git a/frame/support/src/traits/dispatch.rs b/frame/support/src/traits/dispatch.rs index b1bd52ca960da..f1b59ecb5f065 100644 --- a/frame/support/src/traits/dispatch.rs +++ b/frame/support/src/traits/dispatch.rs @@ -174,6 +174,9 @@ pub trait OriginTrait: Sized { /// Create with system signed origin and `frame_system::Config::BaseCallFilter`. fn signed(by: Self::AccountId) -> Self; + + /// Extract the signer from the message if it is a `Signed` origin. + fn as_signed(self) -> Option; } /// "OR gate" implementation of `EnsureOrigin` allowing for different `Success` types for `L` diff --git a/frame/support/src/traits/voting.rs b/frame/support/src/traits/voting.rs index 6c802a6112246..7a3ff87e5940e 100644 --- a/frame/support/src/traits/voting.rs +++ b/frame/support/src/traits/voting.rs @@ -19,7 +19,7 @@ //! votes. use crate::dispatch::{DispatchError, Parameter}; -use codec::HasCompact; +use codec::{HasCompact, MaxEncodedLen}; use sp_arithmetic::{ traits::{SaturatedConversion, UniqueSaturatedFrom, UniqueSaturatedInto}, Perbill, @@ -123,9 +123,9 @@ impl PollStatus { } pub trait Polling { - type Index: Parameter + Member + Ord + PartialOrd + Copy + HasCompact; - type Votes: Parameter + Member + Ord + PartialOrd + Copy + HasCompact; - type Class: Parameter + Member + Ord + PartialOrd; + type Index: Parameter + Member + Ord + PartialOrd + Copy + HasCompact + MaxEncodedLen; + type Votes: Parameter + Member + Ord + PartialOrd + Copy + HasCompact + MaxEncodedLen; + type Class: Parameter + Member + Ord + PartialOrd + MaxEncodedLen; type Moment; /// Provides a vec of values that `T` may take. From 6b781b7cb82ab3bc16fbbb8cb4bef696a8b2e310 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 16:02:21 +0100 Subject: [PATCH 12/25] Formatting --- frame/ranked-collective/src/lib.rs | 28 ++++++++----------- frame/ranked-collective/src/tests.rs | 12 ++++---- frame/support/src/storage/mod.rs | 2 +- frame/support/src/storage/types/double_map.rs | 26 +++++++++++------ frame/support/src/storage/types/map.rs | 10 +++---- 5 files changed, 40 insertions(+), 38 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 7bb81912eda92..10db21ab3a8d3 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -218,11 +218,9 @@ impl, I: 'static> GetMaxVoters for Pallet { } pub struct EnsureRanked(PhantomData<(T, I)>); -impl< - T: Config, - I: 'static, - const MIN_RANK: u16, -> EnsureOrigin for EnsureRanked { +impl, I: 'static, const MIN_RANK: u16> EnsureOrigin + for EnsureRanked +{ type Success = T::AccountId; fn try_origin(o: T::Origin) -> Result { @@ -264,11 +262,7 @@ pub mod pallet { type AdminOrigin: EnsureOrigin; /// The polling system used for our voting. - type Polls: Polling< - TallyOf, - Votes = Votes, - Moment = Self::BlockNumber, - >; + type Polls: Polling, Votes = Votes, Moment = Self::BlockNumber>; /// Convert the tally class into the minimum rank required to vote on the poll. If /// `Polls::Class` is the same type as `Rank`, then `Identity` can be used here to mean @@ -316,12 +310,8 @@ pub mod pallet { >; #[pallet::storage] - pub type VotingCleanup, I: 'static = ()> = StorageMap< - _, - Blake2_128Concat, - PollIndexOf, - BoundedVec>>, - >; + pub type VotingCleanup, I: 'static = ()> = + StorageMap<_, Blake2_128Concat, PollIndexOf, BoundedVec>>>; #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -536,7 +526,11 @@ pub mod pallet { ensure_signed(origin)?; ensure!(T::Polls::as_ongoing(poll_index).is_none(), Error::::Ongoing); - let r = Voting::::clear_prefix(poll_index, max, VotingCleanup::::take(poll_index).as_ref().map(|c| &c[..])); + let r = Voting::::clear_prefix( + poll_index, + max, + VotingCleanup::::take(poll_index).as_ref().map(|c| &c[..]), + ); if r.unique == 0 { // return Err(Error::::NoneRemaining) return Ok(Pays::Yes.into()) diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index 4bd1be7901715..88d24e5852362 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -26,7 +26,7 @@ use frame_support::{ use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, IdentityLookup, Identity}, + traits::{BlakeTwo256, Identity, IdentityLookup}, }; use super::*; @@ -369,10 +369,10 @@ fn ensure_ranked_works() { assert_ok!(Club::promote_member(Origin::root(), 3)); use frame_support::traits::OriginTrait; - type Rank1 = EnsureRanked::; - type Rank2 = EnsureRanked::; - type Rank3 = EnsureRanked::; - type Rank4 = EnsureRanked::; + type Rank1 = EnsureRanked; + type Rank2 = EnsureRanked; + type Rank3 = EnsureRanked; + type Rank4 = EnsureRanked; assert_eq!(Rank1::try_origin(Origin::signed(1)).unwrap(), 1); assert_eq!(Rank1::try_origin(Origin::signed(2)).unwrap(), 2); assert_eq!(Rank1::try_origin(Origin::signed(3)).unwrap(), 3); @@ -386,4 +386,4 @@ fn ensure_ranked_works() { assert_eq!(Rank4::try_origin(Origin::signed(2)).unwrap_err().as_signed().unwrap(), 2); assert_eq!(Rank4::try_origin(Origin::signed(3)).unwrap_err().as_signed().unwrap(), 3); }); -} \ No newline at end of file +} diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index f8f78ce535330..c878766643667 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -27,7 +27,7 @@ use crate::{ use codec::{Decode, Encode, EncodeLike, FullCodec, FullEncode}; use sp_core::storage::ChildInfo; use sp_runtime::generic::{Digest, DigestItem}; -use sp_std::{prelude::*, marker::PhantomData}; +use sp_std::{marker::PhantomData, prelude::*}; pub use self::{ transactional::{ diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index a977eab411a56..bf957dc0ff928 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -22,7 +22,7 @@ use crate::{ metadata::{StorageEntryMetadata, StorageEntryType}, storage::{ types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, - StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, KeyLenOf, + KeyLenOf, StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, }, traits::{Get, GetDefault, StorageInfo, StorageInstance}, }; @@ -71,8 +71,19 @@ pub struct StorageDoubleMap< ); impl Get -for KeyLenOf> -where + for KeyLenOf< + StorageDoubleMap< + Prefix, + Hasher1, + Key1, + Hasher2, + Key2, + Value, + QueryKind, + OnEmpty, + MaxValues, + >, + > where Prefix: StorageInstance, Hasher1: crate::hash::StorageHasher, Hasher2: crate::hash::StorageHasher, @@ -80,15 +91,14 @@ where Key2: MaxEncodedLen, { fn get() -> u32 { - let z = Hasher1::max_len::() - + Hasher2::max_len::() - + Prefix::pallet_prefix().len() - + Prefix::STORAGE_PREFIX.len(); + let z = Hasher1::max_len::() + + Hasher2::max_len::() + + Prefix::pallet_prefix().len() + + Prefix::STORAGE_PREFIX.len(); z as u32 } } - impl crate::storage::generator::StorageDoubleMap for StorageDoubleMap diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index f041a71c9f659..1a5500e589d3a 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -22,7 +22,7 @@ use crate::{ metadata::{StorageEntryMetadata, StorageEntryType}, storage::{ types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, - StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, KeyLenOf, + KeyLenOf, StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend, }, traits::{Get, GetDefault, StorageInfo, StorageInstance}, }; @@ -53,8 +53,7 @@ pub struct StorageMap< MaxValues = GetDefault, >(core::marker::PhantomData<(Prefix, Hasher, Key, Value, QueryKind, OnEmpty, MaxValues)>); -impl - Get +impl Get for KeyLenOf> where Prefix: StorageInstance, @@ -62,9 +61,8 @@ where Key: FullCodec + MaxEncodedLen, { fn get() -> u32 { - let z = Hasher::max_len::() - + Prefix::pallet_prefix().len() - + Prefix::STORAGE_PREFIX.len(); + let z = + Hasher::max_len::() + Prefix::pallet_prefix().len() + Prefix::STORAGE_PREFIX.len(); z as u32 } } From 0ee3e2808dff49df9bf6e2baf37ad2ab65bb0bdc Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 17:29:38 +0100 Subject: [PATCH 13/25] Promote/demote by rank --- frame/ranked-collective/src/lib.rs | 66 ++++++++++++++++++++++++--- frame/ranked-collective/src/tests.rs | 67 +++++++++++++++++++++++++++- frame/support/src/traits.rs | 3 +- frame/support/src/traits/dispatch.rs | 55 ++++++++++++++++++++++- primitives/runtime/src/traits.rs | 37 +++++++++++++++ 5 files changed, 218 insertions(+), 10 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 10db21ab3a8d3..b7d94ddeb02ae 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -220,6 +220,28 @@ impl, I: 'static> GetMaxVoters for Pallet { pub struct EnsureRanked(PhantomData<(T, I)>); impl, I: 'static, const MIN_RANK: u16> EnsureOrigin for EnsureRanked +{ + type Success = Rank; + + fn try_origin(o: T::Origin) -> Result { + let who = frame_system::EnsureSigned::try_origin(o)?; + match Members::::get(&who) { + Some(MemberRecord { rank, .. }) if rank >= MIN_RANK => Ok(rank), + _ => Err(frame_system::RawOrigin::Signed(who).into()), + } + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> T::Origin { + let who = IndexToId::::get(MIN_RANK, 0) + .expect("Must be at least one member at rank for a successful origin"); + frame_system::RawOrigin::Signed(who).into() + } +} + +pub struct EnsureMember(PhantomData<(T, I)>); +impl, I: 'static, const MIN_RANK: u16> EnsureOrigin + for EnsureMember { type Success = T::AccountId; @@ -239,6 +261,28 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin } } +pub struct EnsureRankedMember(PhantomData<(T, I)>); +impl, I: 'static, const MIN_RANK: u16> EnsureOrigin + for EnsureRankedMember +{ + type Success = (T::AccountId, Rank); + + fn try_origin(o: T::Origin) -> Result { + let who = frame_system::EnsureSigned::try_origin(o)?; + match Members::::get(&who) { + Some(MemberRecord { rank, .. }) if rank >= MIN_RANK => Ok((who, rank)), + _ => Err(frame_system::RawOrigin::Signed(who).into()), + } + } + + #[cfg(feature = "runtime-benchmarks")] + fn successful_origin() -> T::Origin { + let who = IndexToId::::get(MIN_RANK, 0) + .expect("Must be at least one member at rank for a successful origin"); + frame_system::RawOrigin::Signed(who).into() + } +} + #[frame_support::pallet] pub mod pallet { use super::*; @@ -258,8 +302,13 @@ pub mod pallet { /// The outer event type. type Event: From> + IsType<::Event>; - /// The origin required to add, promote or remove a member. - type AdminOrigin: EnsureOrigin; + /// The origin required to add or promote a mmember. The success value indicates the + /// maximum rank *to which* the promotion may be. + type PromoteOrigin: EnsureOrigin; + + /// The origin required to demote or remove a member. The success value indicates the + /// maximum rank *from which* the demotion/removal may be. + type DemoteOrigin: EnsureOrigin; /// The polling system used for our voting. type Polls: Polling, Votes = Votes, Moment = Self::BlockNumber>; @@ -345,6 +394,8 @@ pub mod pallet { RankTooLow, /// The information provided is incorrect. InvalidWitness, + /// The origin is not sufficiently privileged to do the operation. + NoPermission, } #[pallet::call] @@ -358,7 +409,7 @@ pub mod pallet { /// Weight: `O(1)` #[pallet::weight(T::WeightInfo::add_member())] pub fn add_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { - T::AdminOrigin::ensure_origin(origin)?; + let _ = T::PromoteOrigin::ensure_origin(origin)?; ensure!(!Members::::contains_key(&who), Error::::AlreadyMember); let index = MemberCount::::get(0); let count = index.checked_add(1).ok_or(Overflow)?; @@ -380,9 +431,10 @@ pub mod pallet { /// Weight: `O(1)` #[pallet::weight(T::WeightInfo::promote_member(0))] pub fn promote_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { - T::AdminOrigin::ensure_origin(origin)?; + let max_rank = T::PromoteOrigin::ensure_origin(origin)?; let record = Self::ensure_member(&who)?; let rank = record.rank.checked_add(1).ok_or(Overflow)?; + ensure!(max_rank >= rank, Error::::NoPermission); let index = MemberCount::::get(rank); MemberCount::::insert(rank, index.checked_add(1).ok_or(Overflow)?); IdToIndex::::insert(rank, &who, index); @@ -402,9 +454,10 @@ pub mod pallet { /// Weight: `O(1)`, less if the member's index is highest in its rank. #[pallet::weight(T::WeightInfo::demote_member(0))] pub fn demote_member(origin: OriginFor, who: T::AccountId) -> DispatchResult { - T::AdminOrigin::ensure_origin(origin)?; + let max_rank = T::DemoteOrigin::ensure_origin(origin)?; let mut record = Self::ensure_member(&who)?; let rank = record.rank; + ensure!(max_rank >= rank, Error::::NoPermission); Self::remove_from_rank(&who, rank)?; let maybe_rank = rank.checked_sub(1); @@ -435,9 +488,10 @@ pub mod pallet { who: T::AccountId, min_rank: Rank, ) -> DispatchResultWithPostInfo { - T::AdminOrigin::ensure_origin(origin)?; + let max_rank = T::DemoteOrigin::ensure_origin(origin)?; let MemberRecord { rank, .. } = Self::ensure_member(&who)?; ensure!(min_rank >= rank, Error::::InvalidWitness); + ensure!(max_rank >= rank, Error::::NoPermission); for r in 0..=rank { Self::remove_from_rank(&who, r)?; diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index 88d24e5852362..9a0d6412664af 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -21,7 +21,7 @@ use std::collections::BTreeMap; use frame_support::{ assert_noop, assert_ok, parameter_types, - traits::{ConstU32, ConstU64, Everything, Polling}, + traits::{ConstU32, ConstU64, Everything, Polling, ConstU16, EitherOf, MapSuccess, ReduceBy}, error::BadOrigin, }; use sp_core::H256; use sp_runtime::{ @@ -169,7 +169,16 @@ impl Polling> for TestPolls { impl Config for Test { type WeightInfo = (); type Event = Event; - type AdminOrigin = frame_system::EnsureRoot; + type PromoteOrigin = EitherOf< + // Members can promote up to the rank of 2 below them. + MapSuccess, ReduceBy>>, + frame_system::EnsureRootWithSuccess> + >; + type DemoteOrigin = EitherOf< + // Members can demote up to the rank of 3 below them. + MapSuccess, ReduceBy>>, + frame_system::EnsureRootWithSuccess> + >; type Polls = TestPolls; type MinRankOfClass = Identity; type VoteWeight = Geometric; @@ -295,6 +304,59 @@ fn promote_demote_works() { }); } +#[test] +fn promote_demote_by_rank_works() { + new_test_ext().execute_with(|| { + assert_ok!(Club::add_member(Origin::root(), 1)); + for _ in 0..7 { assert_ok!(Club::promote_member(Origin::root(), 1)); }; + + // #1 can add #2 and promote to rank 1 + assert_ok!(Club::add_member(Origin::signed(1), 2)); + assert_ok!(Club::promote_member(Origin::signed(1), 2)); + // #2 as rank 1 cannot do anything privileged + assert_noop!(Club::add_member(Origin::signed(2), 3), BadOrigin); + + assert_ok!(Club::promote_member(Origin::signed(1), 2)); + // #2 as rank 2 can add #3. + assert_ok!(Club::add_member(Origin::signed(2), 3)); + + // #2 as rank 2 cannot promote #3 to rank 1 + assert_noop!(Club::promote_member(Origin::signed(2), 3), Error::::NoPermission); + + // #1 as rank 7 can promote #2 only up to rank 5 and once there cannot demote them. + assert_ok!(Club::promote_member(Origin::signed(1), 2)); + assert_ok!(Club::promote_member(Origin::signed(1), 2)); + assert_ok!(Club::promote_member(Origin::signed(1), 2)); + assert_noop!(Club::promote_member(Origin::signed(1), 2), Error::::NoPermission); + assert_noop!(Club::demote_member(Origin::signed(1), 2), Error::::NoPermission); + + // #2 as rank 5 can promote #3 only up to rank 3 and once there cannot demote them. + assert_ok!(Club::promote_member(Origin::signed(2), 3)); + assert_ok!(Club::promote_member(Origin::signed(2), 3)); + assert_ok!(Club::promote_member(Origin::signed(2), 3)); + assert_noop!(Club::promote_member(Origin::signed(2), 3), Error::::NoPermission); + assert_noop!(Club::demote_member(Origin::signed(2), 3), Error::::NoPermission); + + // #2 can add #4 & #5 as rank 0 and #6 & #7 as rank 1. + assert_ok!(Club::add_member(Origin::signed(2), 4)); + assert_ok!(Club::add_member(Origin::signed(2), 5)); + assert_ok!(Club::add_member(Origin::signed(2), 6)); + assert_ok!(Club::promote_member(Origin::signed(2), 6)); + assert_ok!(Club::add_member(Origin::signed(2), 7)); + assert_ok!(Club::promote_member(Origin::signed(2), 7)); + + // #3 as rank 3 can demote/remove #4 & #5 but not #6 & #7 + assert_ok!(Club::demote_member(Origin::signed(3), 4)); + assert_ok!(Club::remove_member(Origin::signed(3), 5, 0)); + assert_noop!(Club::demote_member(Origin::signed(3), 6), Error::::NoPermission); + assert_noop!(Club::remove_member(Origin::signed(3), 7, 1), Error::::NoPermission); + + // #2 as rank 5 can demote/remove #6 & #7 + assert_ok!(Club::demote_member(Origin::signed(2), 6)); + assert_ok!(Club::remove_member(Origin::signed(2), 7, 1)); + }); +} + #[test] fn voting_works() { new_test_ext().execute_with(|| { @@ -387,3 +449,4 @@ fn ensure_ranked_works() { assert_eq!(Rank4::try_origin(Origin::signed(3)).unwrap_err().as_signed().unwrap(), 3); }); } + diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index d1872544e024d..4eac916abdc3e 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -97,7 +97,8 @@ mod dispatch; pub use dispatch::EnsureOneOf; pub use dispatch::{ AsEnsureOriginWithArg, DispatchableWithStorageLayer, EitherOf, EitherOfDiverse, EnsureOrigin, - EnsureOriginWithArg, NeverEnsureOrigin, OriginTrait, UnfilteredDispatchable, + EnsureOriginWithArg, NeverEnsureOrigin, OriginTrait, UnfilteredDispatchable, MapSuccess, TryMapSuccess, + ReduceBy, }; mod voting; diff --git a/frame/support/src/traits/dispatch.rs b/frame/support/src/traits/dispatch.rs index f1b59ecb5f065..ce10340f17c1d 100644 --- a/frame/support/src/traits/dispatch.rs +++ b/frame/support/src/traits/dispatch.rs @@ -17,12 +17,17 @@ //! Traits for dealing with dispatching calls and the origin from which they are dispatched. +use std::marker::PhantomData; + use crate::dispatch::{DispatchResultWithPostInfo, Parameter, RawOrigin}; +use sp_arithmetic::traits::{Zero, CheckedSub}; use sp_runtime::{ - traits::{BadOrigin, Member}, + traits::{BadOrigin, Member, TryMorph, Morph}, Either, }; +use super::TypedGet; + /// Some sort of check on the origin is performed by this object. pub trait EnsureOrigin { /// A return type. @@ -102,6 +107,54 @@ impl> } } +/// A derivative `EnsureOrigin` implementation. It mutates the `Success` result of an `Original` +/// implementation with a given `Mutator`. +pub struct MapSuccess(PhantomData<(Original, Mutator)>); +impl< + O, + Original: EnsureOrigin, + Mutator: Morph, +> EnsureOrigin for MapSuccess { + type Success = Mutator::Outcome; + fn try_origin(o: O) -> Result { + Ok(Mutator::morph(Original::try_origin(o)?)) + } +} + +/// A derivative `EnsureOrigin` implementation. It mutates the `Success` result of an `Original` +/// implementation with a given `Mutator`, allowing the possibility of an error to be returned +/// from the mutator. +/// +/// NOTE: This is strictly worse performance than `MapSuccess` since it clones the original origin +/// value. If possible, use `MapSuccess` instead. +pub struct TryMapSuccess(PhantomData<(Orig, Mutator)>); +impl< + O: Clone, + Original: EnsureOrigin, + Mutator: TryMorph, +> EnsureOrigin for TryMapSuccess { + type Success = Mutator::Outcome; + fn try_origin(o: O) -> Result { + let orig = o.clone(); + Mutator::try_morph(Original::try_origin(o)?).map_err(|()| orig) + } +} + +pub struct ReduceBy(PhantomData); +impl TryMorph for ReduceBy where N::Type: CheckedSub { + type Outcome = N::Type; + fn try_morph(r: N::Type) -> Result { + r.checked_sub(&N::get()).ok_or(()) + } +} +impl Morph for ReduceBy where N::Type: CheckedSub + Zero { + type Outcome = N::Type; + fn morph(r: N::Type) -> N::Type { + r.checked_sub(&N::get()).unwrap_or(Zero::zero()) + } +} + + /// Type that can be dispatched with an origin but without checking the origin filter. /// /// Implemented for pallet dispatchable type by `decl_module` and for runtime dispatchable by diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 1b21e7c65ddf7..821b56a786632 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -274,6 +274,43 @@ where } } +/// Extensible conversion trait. Generic over only source type, with destination type being +/// associated. +pub trait Morph { + /// The type into which `A` is mutated. + type Outcome; + + /// Make conversion. + fn morph(a: A) -> Self::Outcome; +} + +/// A structure that performs identity conversion. +impl Morph for Identity { + type Outcome = T; + fn morph(a: T) -> T { + a + } +} + + +/// Extensible conversion trait. Generic over only source type, with destination type being +/// associated. +pub trait TryMorph { + /// The type into which `A` is mutated. + type Outcome; + + /// Make conversion. + fn try_morph(a: A) -> Result; +} + +/// A structure that performs identity conversion. +impl TryMorph for Identity { + type Outcome = T; + fn try_morph(a: T) -> Result { + Ok(a) + } +} + /// Extensible conversion trait. Generic over both source and destination types. pub trait Convert { /// Make conversion. From 255420a1f5be56e1f6ae1942ae3bf267ce501630 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 17:29:45 +0100 Subject: [PATCH 14/25] Formatting --- frame/ranked-collective/src/tests.rs | 15 ++++++++------ frame/support/src/traits.rs | 4 ++-- frame/support/src/traits/dispatch.rs | 31 ++++++++++++++-------------- primitives/runtime/src/traits.rs | 1 - 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index 9a0d6412664af..ae56b9f0be24b 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -20,8 +20,10 @@ use std::collections::BTreeMap; use frame_support::{ - assert_noop, assert_ok, parameter_types, - traits::{ConstU32, ConstU64, Everything, Polling, ConstU16, EitherOf, MapSuccess, ReduceBy}, error::BadOrigin, + assert_noop, assert_ok, + error::BadOrigin, + parameter_types, + traits::{ConstU16, ConstU32, ConstU64, EitherOf, Everything, MapSuccess, Polling, ReduceBy}, }; use sp_core::H256; use sp_runtime::{ @@ -172,12 +174,12 @@ impl Config for Test { type PromoteOrigin = EitherOf< // Members can promote up to the rank of 2 below them. MapSuccess, ReduceBy>>, - frame_system::EnsureRootWithSuccess> + frame_system::EnsureRootWithSuccess>, >; type DemoteOrigin = EitherOf< // Members can demote up to the rank of 3 below them. MapSuccess, ReduceBy>>, - frame_system::EnsureRootWithSuccess> + frame_system::EnsureRootWithSuccess>, >; type Polls = TestPolls; type MinRankOfClass = Identity; @@ -308,7 +310,9 @@ fn promote_demote_works() { fn promote_demote_by_rank_works() { new_test_ext().execute_with(|| { assert_ok!(Club::add_member(Origin::root(), 1)); - for _ in 0..7 { assert_ok!(Club::promote_member(Origin::root(), 1)); }; + for _ in 0..7 { + assert_ok!(Club::promote_member(Origin::root(), 1)); + } // #1 can add #2 and promote to rank 1 assert_ok!(Club::add_member(Origin::signed(1), 2)); @@ -449,4 +453,3 @@ fn ensure_ranked_works() { assert_eq!(Rank4::try_origin(Origin::signed(3)).unwrap_err().as_signed().unwrap(), 3); }); } - diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 4eac916abdc3e..fe983f5c292e3 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -97,8 +97,8 @@ mod dispatch; pub use dispatch::EnsureOneOf; pub use dispatch::{ AsEnsureOriginWithArg, DispatchableWithStorageLayer, EitherOf, EitherOfDiverse, EnsureOrigin, - EnsureOriginWithArg, NeverEnsureOrigin, OriginTrait, UnfilteredDispatchable, MapSuccess, TryMapSuccess, - ReduceBy, + EnsureOriginWithArg, MapSuccess, NeverEnsureOrigin, OriginTrait, ReduceBy, TryMapSuccess, + UnfilteredDispatchable, }; mod voting; diff --git a/frame/support/src/traits/dispatch.rs b/frame/support/src/traits/dispatch.rs index ce10340f17c1d..5dd22faab278a 100644 --- a/frame/support/src/traits/dispatch.rs +++ b/frame/support/src/traits/dispatch.rs @@ -20,9 +20,9 @@ use std::marker::PhantomData; use crate::dispatch::{DispatchResultWithPostInfo, Parameter, RawOrigin}; -use sp_arithmetic::traits::{Zero, CheckedSub}; +use sp_arithmetic::traits::{CheckedSub, Zero}; use sp_runtime::{ - traits::{BadOrigin, Member, TryMorph, Morph}, + traits::{BadOrigin, Member, Morph, TryMorph}, Either, }; @@ -110,11 +110,9 @@ impl> /// A derivative `EnsureOrigin` implementation. It mutates the `Success` result of an `Original` /// implementation with a given `Mutator`. pub struct MapSuccess(PhantomData<(Original, Mutator)>); -impl< - O, - Original: EnsureOrigin, - Mutator: Morph, -> EnsureOrigin for MapSuccess { +impl, Mutator: Morph> EnsureOrigin + for MapSuccess +{ type Success = Mutator::Outcome; fn try_origin(o: O) -> Result { Ok(Mutator::morph(Original::try_origin(o)?)) @@ -128,11 +126,9 @@ impl< /// NOTE: This is strictly worse performance than `MapSuccess` since it clones the original origin /// value. If possible, use `MapSuccess` instead. pub struct TryMapSuccess(PhantomData<(Orig, Mutator)>); -impl< - O: Clone, - Original: EnsureOrigin, - Mutator: TryMorph, -> EnsureOrigin for TryMapSuccess { +impl, Mutator: TryMorph> EnsureOrigin + for TryMapSuccess +{ type Success = Mutator::Outcome; fn try_origin(o: O) -> Result { let orig = o.clone(); @@ -141,20 +137,25 @@ impl< } pub struct ReduceBy(PhantomData); -impl TryMorph for ReduceBy where N::Type: CheckedSub { +impl TryMorph for ReduceBy +where + N::Type: CheckedSub, +{ type Outcome = N::Type; fn try_morph(r: N::Type) -> Result { r.checked_sub(&N::get()).ok_or(()) } } -impl Morph for ReduceBy where N::Type: CheckedSub + Zero { +impl Morph for ReduceBy +where + N::Type: CheckedSub + Zero, +{ type Outcome = N::Type; fn morph(r: N::Type) -> N::Type { r.checked_sub(&N::get()).unwrap_or(Zero::zero()) } } - /// Type that can be dispatched with an origin but without checking the origin filter. /// /// Implemented for pallet dispatchable type by `decl_module` and for runtime dispatchable by diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 821b56a786632..ee6a526e95e1e 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -292,7 +292,6 @@ impl Morph for Identity { } } - /// Extensible conversion trait. Generic over only source type, with destination type being /// associated. pub trait TryMorph { From a8217fac964e20bcf089721ec27a499c68dda35e Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 18:08:23 +0100 Subject: [PATCH 15/25] Use new API --- frame/ranked-collective/src/benchmarking.rs | 29 ++++++++++++++------- frame/ranked-collective/src/lib.rs | 21 +++++++-------- frame/ranked-collective/src/tests.rs | 6 +++-- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs index 042287900852c..ab1a5dc283ca5 100644 --- a/frame/ranked-collective/src/benchmarking.rs +++ b/frame/ranked-collective/src/benchmarking.rs @@ -33,10 +33,10 @@ fn assert_last_event, I: 'static>(generic_event: >:: fn make_member, I: 'static>(rank: Rank) -> T::AccountId { let who = account::("member", MemberCount::::get(0), SEED); - assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), who.clone())); + assert_ok!(Pallet::::add_member(T::PromoteOrigin::successful_origin(), who.clone())); for _ in 0..rank { assert_ok!(Pallet::::promote_member( - T::AdminOrigin::successful_origin(), + T::PromoteOrigin::successful_origin(), who.clone() )); } @@ -46,7 +46,7 @@ fn make_member, I: 'static>(rank: Rank) -> T::AccountId { benchmarks_instance_pallet! { add_member { let who = account::("member", 0, SEED); - let origin = T::AdminOrigin::successful_origin(); + let origin = T::PromoteOrigin::successful_origin(); let call = Call::::add_member { who: who.clone() }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -61,7 +61,7 @@ benchmarks_instance_pallet! { let who = make_member::(rank); let last = make_member::(rank); let last_index = (0..=rank).map(|r| IdToIndex::::get(r, &last).unwrap()).collect::>(); - let origin = T::AdminOrigin::successful_origin(); + let origin = T::DemoteOrigin::successful_origin(); let call = Call::::remove_member { who: who.clone(), min_rank: rank }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -76,7 +76,7 @@ benchmarks_instance_pallet! { let r in 0 .. 10; let rank = r as u16; let who = make_member::(rank); - let origin = T::AdminOrigin::successful_origin(); + let origin = T::PromoteOrigin::successful_origin(); let call = Call::::promote_member { who: who.clone() }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -91,7 +91,7 @@ benchmarks_instance_pallet! { let who = make_member::(rank); let last = make_member::(rank); let last_index = IdToIndex::::get(rank, &last).unwrap(); - let origin = T::AdminOrigin::successful_origin(); + let origin = T::DemoteOrigin::successful_origin(); let call = Call::::demote_member { who: who.clone() }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -106,9 +106,17 @@ benchmarks_instance_pallet! { vote { let caller: T::AccountId = whitelisted_caller(); - assert_ok!(Pallet::::add_member(T::AdminOrigin::successful_origin(), caller.clone())); + assert_ok!(Pallet::::add_member(T::PromoteOrigin::successful_origin(), caller.clone())); // Create a poll - let class = 0; + let class = T::Polls::classes().into_iter().next().unwrap(); + let rank = T::MinRankOfClass::convert(class.clone()); + for _ in 0..rank { + assert_ok!(Pallet::::promote_member( + T::PromoteOrigin::successful_origin(), + caller.clone() + )); + } + let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll for rank 0"); // Vote once. @@ -124,12 +132,13 @@ benchmarks_instance_pallet! { let n in 1 .. 100; // Create a poll - let class = 0; + let class = T::Polls::classes().into_iter().next().unwrap(); + let rank = T::MinRankOfClass::convert(class.clone()); let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll"); // Vote in the poll by each of `n` members for i in 0..n { - let who = make_member::(0); + let who = make_member::(rank); assert_ok!(Pallet::::vote(SystemOrigin::Signed(who).into(), poll, true)); } diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index b7d94ddeb02ae..217000d53d79e 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -232,10 +232,9 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin } #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> T::Origin { - let who = IndexToId::::get(MIN_RANK, 0) - .expect("Must be at least one member at rank for a successful origin"); - frame_system::RawOrigin::Signed(who).into() + fn try_successful_origin() -> Result { + let who = IndexToId::::get(MIN_RANK, 0).ok_or(())?; + Ok(frame_system::RawOrigin::Signed(who).into()) } } @@ -254,10 +253,9 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin } #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> T::Origin { - let who = IndexToId::::get(MIN_RANK, 0) - .expect("Must be at least one member at rank for a successful origin"); - frame_system::RawOrigin::Signed(who).into() + fn try_successful_origin() -> Result { + let who = IndexToId::::get(MIN_RANK, 0).ok_or(())?; + Ok(frame_system::RawOrigin::Signed(who).into()) } } @@ -276,10 +274,9 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin } #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> T::Origin { - let who = IndexToId::::get(MIN_RANK, 0) - .expect("Must be at least one member at rank for a successful origin"); - frame_system::RawOrigin::Signed(who).into() + fn try_successful_origin() -> Result { + let who = IndexToId::::get(MIN_RANK, 0).ok_or(())?; + Ok(frame_system::RawOrigin::Signed(who).into()) } } diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index ae56b9f0be24b..1426012b63cea 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -172,14 +172,16 @@ impl Config for Test { type WeightInfo = (); type Event = Event; type PromoteOrigin = EitherOf< + // Root can promote arbitrarily. + frame_system::EnsureRootWithSuccess>, // Members can promote up to the rank of 2 below them. MapSuccess, ReduceBy>>, - frame_system::EnsureRootWithSuccess>, >; type DemoteOrigin = EitherOf< + // Root can demote arbitrarily. + frame_system::EnsureRootWithSuccess>, // Members can demote up to the rank of 3 below them. MapSuccess, ReduceBy>>, - frame_system::EnsureRootWithSuccess>, >; type Polls = TestPolls; type MinRankOfClass = Identity; From 63dd2703a0c213f1628d2864245b8485971c5188 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 18:58:48 +0100 Subject: [PATCH 16/25] Remove code in another PR --- frame/support/src/traits/dispatch.rs | 56 +--------------------------- primitives/runtime/src/traits.rs | 36 ------------------ 2 files changed, 1 insertion(+), 91 deletions(-) diff --git a/frame/support/src/traits/dispatch.rs b/frame/support/src/traits/dispatch.rs index 5dd22faab278a..f1b59ecb5f065 100644 --- a/frame/support/src/traits/dispatch.rs +++ b/frame/support/src/traits/dispatch.rs @@ -17,17 +17,12 @@ //! Traits for dealing with dispatching calls and the origin from which they are dispatched. -use std::marker::PhantomData; - use crate::dispatch::{DispatchResultWithPostInfo, Parameter, RawOrigin}; -use sp_arithmetic::traits::{CheckedSub, Zero}; use sp_runtime::{ - traits::{BadOrigin, Member, Morph, TryMorph}, + traits::{BadOrigin, Member}, Either, }; -use super::TypedGet; - /// Some sort of check on the origin is performed by this object. pub trait EnsureOrigin { /// A return type. @@ -107,55 +102,6 @@ impl> } } -/// A derivative `EnsureOrigin` implementation. It mutates the `Success` result of an `Original` -/// implementation with a given `Mutator`. -pub struct MapSuccess(PhantomData<(Original, Mutator)>); -impl, Mutator: Morph> EnsureOrigin - for MapSuccess -{ - type Success = Mutator::Outcome; - fn try_origin(o: O) -> Result { - Ok(Mutator::morph(Original::try_origin(o)?)) - } -} - -/// A derivative `EnsureOrigin` implementation. It mutates the `Success` result of an `Original` -/// implementation with a given `Mutator`, allowing the possibility of an error to be returned -/// from the mutator. -/// -/// NOTE: This is strictly worse performance than `MapSuccess` since it clones the original origin -/// value. If possible, use `MapSuccess` instead. -pub struct TryMapSuccess(PhantomData<(Orig, Mutator)>); -impl, Mutator: TryMorph> EnsureOrigin - for TryMapSuccess -{ - type Success = Mutator::Outcome; - fn try_origin(o: O) -> Result { - let orig = o.clone(); - Mutator::try_morph(Original::try_origin(o)?).map_err(|()| orig) - } -} - -pub struct ReduceBy(PhantomData); -impl TryMorph for ReduceBy -where - N::Type: CheckedSub, -{ - type Outcome = N::Type; - fn try_morph(r: N::Type) -> Result { - r.checked_sub(&N::get()).ok_or(()) - } -} -impl Morph for ReduceBy -where - N::Type: CheckedSub + Zero, -{ - type Outcome = N::Type; - fn morph(r: N::Type) -> N::Type { - r.checked_sub(&N::get()).unwrap_or(Zero::zero()) - } -} - /// Type that can be dispatched with an origin but without checking the origin filter. /// /// Implemented for pallet dispatchable type by `decl_module` and for runtime dispatchable by diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index ee6a526e95e1e..1b21e7c65ddf7 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -274,42 +274,6 @@ where } } -/// Extensible conversion trait. Generic over only source type, with destination type being -/// associated. -pub trait Morph { - /// The type into which `A` is mutated. - type Outcome; - - /// Make conversion. - fn morph(a: A) -> Self::Outcome; -} - -/// A structure that performs identity conversion. -impl Morph for Identity { - type Outcome = T; - fn morph(a: T) -> T { - a - } -} - -/// Extensible conversion trait. Generic over only source type, with destination type being -/// associated. -pub trait TryMorph { - /// The type into which `A` is mutated. - type Outcome; - - /// Make conversion. - fn try_morph(a: A) -> Result; -} - -/// A structure that performs identity conversion. -impl TryMorph for Identity { - type Outcome = T; - fn try_morph(a: T) -> Result { - Ok(a) - } -} - /// Extensible conversion trait. Generic over both source and destination types. pub trait Convert { /// Make conversion. From 1530149845a5c9ed3544bf073f423e48bcabe735 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 18:59:21 +0100 Subject: [PATCH 17/25] Remove code in another PR --- frame/support/src/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index fe983f5c292e3..d31fbf4ab1f43 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -97,7 +97,7 @@ mod dispatch; pub use dispatch::EnsureOneOf; pub use dispatch::{ AsEnsureOriginWithArg, DispatchableWithStorageLayer, EitherOf, EitherOfDiverse, EnsureOrigin, - EnsureOriginWithArg, MapSuccess, NeverEnsureOrigin, OriginTrait, ReduceBy, TryMapSuccess, + EnsureOriginWithArg, NeverEnsureOrigin, OriginTrait, UnfilteredDispatchable, }; From 10f293b0c2ae2a032f3603c56892de142a7c5017 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 18:59:42 +0100 Subject: [PATCH 18/25] Formatting --- frame/support/src/traits.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index d31fbf4ab1f43..d1872544e024d 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -97,8 +97,7 @@ mod dispatch; pub use dispatch::EnsureOneOf; pub use dispatch::{ AsEnsureOriginWithArg, DispatchableWithStorageLayer, EitherOf, EitherOfDiverse, EnsureOrigin, - EnsureOriginWithArg, NeverEnsureOrigin, OriginTrait, - UnfilteredDispatchable, + EnsureOriginWithArg, NeverEnsureOrigin, OriginTrait, UnfilteredDispatchable, }; mod voting; From b5a0aeda1e5957d2133b8bb9c2695412aa8f0ae0 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 19:00:56 +0100 Subject: [PATCH 19/25] Remove code in another PR --- frame/support/src/storage/bounded_vec.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 0b633f5043452..f1f4330ab2960 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -47,9 +47,7 @@ use sp_std::{marker::PhantomData, prelude::*}; #[scale_info(skip_type_params(S))] pub struct BoundedVec( Vec, - #[cfg_attr(feature = "std", serde(skip_serializing))] - #[codec(skip)] - PhantomData, + #[cfg_attr(feature = "std", serde(skip_serializing))] PhantomData, ); #[cfg(feature = "std")] From ad7d645fdddf02591c10a146705583869ae699ad Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 19:03:08 +0100 Subject: [PATCH 20/25] Docs --- frame/ranked-collective/README.md | 39 ++++++++++++++----------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/frame/ranked-collective/README.md b/frame/ranked-collective/README.md index 444927e51da22..b5fe65ef34920 100644 --- a/frame/ranked-collective/README.md +++ b/frame/ranked-collective/README.md @@ -1,25 +1,22 @@ -Collective system: Members of a set of account IDs can make their collective feelings known -through dispatched calls from one of two specialized origins. +# Ranked collective system. -The membership can be provided in one of two ways: either directly, using the Root-dispatchable -function `set_members`, or indirectly, through implementing the `ChangeMembers`. -The pallet assumes that the amount of members stays at or below `MaxMembers` for its weight -calculations, but enforces this neither in `set_members` nor in `change_members_sorted`. +This is a membership pallet providing a `Tally` implementation ready for use with polling +systems such as the Referenda pallet. Members each have a rank, with zero being the lowest. +There is no complexity limitation on either the number of members at a rank or the number of +ranks in the system thus allowing potentially public membership. A member of at least a given +rank can be selected at random in O(1) time, allowing for various games to constructed using +this as a primitive. Members may only be promoted and demoted by one rank at a time, however +all operations (save one) are O(1) in complexity. The only operation which is not O(1) is the +`remove_member` since they must be removed from all ranks from the present down to zero. -A "prime" member may be set to help determine the default vote behavior based on chain -config. If `PrimeDefaultVote` is used, the prime vote acts as the default vote in case of any -abstentions after the voting period. If `MoreThanMajorityThenPrimeDefaultVote` is used, then -abstentations will first follow the majority of the collective voting, and then the prime -member. +Different ranks have different voting power, and are able to vote in different polls. In general +rank privileges are cumulative. Higher ranks are able to vote in any polls open to lower ranks. +Similarly, higher ranks always have at least as much voting power in any given poll as lower +ranks. -Voting happens through motions comprising a proposal (i.e. a dispatchable) plus a -number of approvals required for it to pass and be called. Motions are open for members to -vote on for a minimum period given by `MotionDuration`. As soon as the required number of -approvals is given, the motion is closed and executed. If the number of approvals is not reached -during the voting period, then `close` may be called by any account in order to force the end -the motion explicitly. If a prime member is defined, then their vote is used instead of any -abstentions and the proposal is executed if there are enough approvals counting the new votes. +Two `Config` trait items control these "rank privileges": `MinRankOfClass` and `VoteWeight`. +The first controls which ranks are allowed to vote on a particular class of poll. The second +controls the weight of a vote given the voters rank compared to the minimum rank of the poll. -If there are not, or if no prime member is set, then the motion is dropped without being executed. - -License: Apache-2.0 +An origin control, `EnsureRank`, ensures that the origin is a member of the collective of at +least a particular rank. From bec4d6e6ac4a8a0bea86e40ca1e86e2dfb031ae6 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 19:11:04 +0100 Subject: [PATCH 21/25] Docs --- frame/ranked-collective/src/lib.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 217000d53d79e..5143b8c07c466 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -74,7 +74,7 @@ pub type Rank = u16; /// Votes. pub type Votes = u32; -/// Aggregated votes for an ongoing poll. +/// Aggregated votes for an ongoing poll by members of the ranked collective. #[derive( CloneNoBound, PartialEqNoBound, @@ -208,7 +208,9 @@ impl Convert for Geometric { } } +/// Trait for getting the maximum number of voters for a given rank. pub trait GetMaxVoters { + /// Return the maximum number of voters for the rank `r`. fn get_max_voters(r: Rank) -> MemberIndex; } impl, I: 'static> GetMaxVoters for Pallet { @@ -217,6 +219,8 @@ impl, I: 'static> GetMaxVoters for Pallet { } } +/// Guard to ensure that the given origin is a member of the collective. The rank of the member is +/// the `Success` value. pub struct EnsureRanked(PhantomData<(T, I)>); impl, I: 'static, const MIN_RANK: u16> EnsureOrigin for EnsureRanked @@ -238,6 +242,8 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin } } +/// Guard to ensure that the given origin is a member of the collective. The account ID of the +/// member is the `Success` value. pub struct EnsureMember(PhantomData<(T, I)>); impl, I: 'static, const MIN_RANK: u16> EnsureOrigin for EnsureMember @@ -259,6 +265,8 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin } } +/// Guard to ensure that the given origin is a member of the collective. The pair of including both +/// the account ID and the rank of the member is the `Success` value. pub struct EnsureRankedMember(PhantomData<(T, I)>); impl, I: 'static, const MIN_RANK: u16> EnsureOrigin for EnsureRankedMember @@ -476,9 +484,9 @@ pub mod pallet { /// /// - `origin`: Must be the `AdminOrigin`. /// - `who`: Account of existing member of rank greater than zero. - /// - `rank`: The rank of the member. + /// - `min_rank`: The rank of the member or greater. /// - /// Weight: `O(rank)`. + /// Weight: `O(min_rank)`. #[pallet::weight(T::WeightInfo::remove_member(*min_rank as u32))] pub fn remove_member( origin: OriginFor, From 5168c7b9e6bd455f53b065abe6054c68eabfeb04 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 19:14:21 +0100 Subject: [PATCH 22/25] Bump --- frame/support/src/traits.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index d1872544e024d..fe983f5c292e3 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -97,7 +97,8 @@ mod dispatch; pub use dispatch::EnsureOneOf; pub use dispatch::{ AsEnsureOriginWithArg, DispatchableWithStorageLayer, EitherOf, EitherOfDiverse, EnsureOrigin, - EnsureOriginWithArg, NeverEnsureOrigin, OriginTrait, UnfilteredDispatchable, + EnsureOriginWithArg, MapSuccess, NeverEnsureOrigin, OriginTrait, ReduceBy, TryMapSuccess, + UnfilteredDispatchable, }; mod voting; From 3a84fa2d054be928bb1491c90ea64495c20741da Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 19:23:50 +0100 Subject: [PATCH 23/25] Fixes --- bin/node/runtime/src/lib.rs | 6 ++++-- frame/referenda/src/lib.rs | 2 +- frame/referenda/src/types.rs | 2 +- frame/support/src/dispatch.rs | 3 +++ test-utils/runtime/src/lib.rs | 5 ++++- 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 56b213efb0d26..3a8d55e27aec6 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -43,7 +43,7 @@ use frame_support::{ }; use frame_system::{ limits::{BlockLength, BlockWeights}, - EnsureRoot, EnsureSigned, + EnsureRoot, EnsureSigned, EnsureRootWithSuccess, }; pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; @@ -858,8 +858,10 @@ impl pallet_referenda::Config for Runtime { impl pallet_ranked_collective::Config for Runtime { type WeightInfo = pallet_ranked_collective::weights::SubstrateWeight; type Event = Event; - type AdminOrigin = EnsureRoot; + type PromoteOrigin = EnsureRootWithSuccess>; + type DemoteOrigin = EnsureRootWithSuccess>; type Polls = RankedPolls; + type MinRankOfClass = traits::Identity; type VoteWeight = pallet_ranked_collective::Geometric; } diff --git a/frame/referenda/src/lib.rs b/frame/referenda/src/lib.rs index e3f31bc411328..3fd54b54d331e 100644 --- a/frame/referenda/src/lib.rs +++ b/frame/referenda/src/lib.rs @@ -150,7 +150,7 @@ pub mod pallet { /// Handler for the unbalanced reduction when slashing a preimage deposit. type Slash: OnUnbalanced>; /// The counting type for votes. Usually just balance. - type Votes: AtLeast32BitUnsigned + Copy + Parameter + Member; + type Votes: AtLeast32BitUnsigned + Copy + Parameter + Member + MaxEncodedLen; /// The tallying type. type Tally: VoteTally> + Clone diff --git a/frame/referenda/src/types.rs b/frame/referenda/src/types.rs index 27b3c6a5a9d59..3eba783246e10 100644 --- a/frame/referenda/src/types.rs +++ b/frame/referenda/src/types.rs @@ -136,7 +136,7 @@ pub struct TrackInfo { /// Information on the voting tracks. pub trait TracksInfo { /// The identifier for a track. - type Id: Copy + Parameter + Ord + PartialOrd + Send + Sync + 'static; + type Id: Copy + Parameter + Ord + PartialOrd + Send + Sync + 'static + MaxEncodedLen; /// The origin type from which a track is implied. type Origin; diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 0a5092b411930..d399340f0470e 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2748,6 +2748,9 @@ mod tests { fn signed(_by: ::AccountId) -> Self { unimplemented!("Not required in tests!") } + fn as_signed(self) -> Option { + unimplemented!("Not required in tests!") + } } impl system::Config for TraitImpl { diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index aef061d952a96..97e060cb2a9bd 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -491,7 +491,10 @@ impl frame_support::traits::OriginTrait for Origin { fn root() -> Self { unimplemented!("Not required in tests!") } - fn signed(_by: ::AccountId) -> Self { + fn signed(_by: Self::AccountId) -> Self { + unimplemented!("Not required in tests!") + } + fn as_signed(self) -> Option { unimplemented!("Not required in tests!") } } From f9d1ce235e241e8278eb5fb40709e40512134923 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 19:30:59 +0100 Subject: [PATCH 24/25] Formatting --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 3a8d55e27aec6..bfac30db6c66f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -43,7 +43,7 @@ use frame_support::{ }; use frame_system::{ limits::{BlockLength, BlockWeights}, - EnsureRoot, EnsureSigned, EnsureRootWithSuccess, + EnsureRoot, EnsureRootWithSuccess, EnsureSigned, }; pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; From 51f59ee2c4921a8183d92d2e1dfc4c358076a3c5 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 May 2022 20:52:24 +0100 Subject: [PATCH 25/25] Fixes --- frame/ranked-collective/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index 5143b8c07c466..521dfb6aad4e3 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -94,7 +94,7 @@ pub struct Tally { } impl Tally { - fn from_parts(bare_ayes: MemberIndex, ayes: Votes, nays: Votes) -> Self { + pub fn from_parts(bare_ayes: MemberIndex, ayes: Votes, nays: Votes) -> Self { Tally { bare_ayes, ayes, nays, dummy: PhantomData } } }