From fd4e7d5abc0389eb4c8f9baff10f94d864acbb3c Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 5 Feb 2023 05:46:51 +0100 Subject: [PATCH 1/5] max class voters for vote tally --- frame/ranked-collective/src/lib.rs | 70 +++++++++++++++++++----------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index a65d184d6bf06..c4814def2b15f 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -90,14 +90,14 @@ pub type Votes = u32; )] #[scale_info(skip_type_params(T, I, M))] #[codec(mel_bound())] -pub struct Tally { +pub struct Tally { bare_ayes: MemberIndex, ayes: Votes, nays: Votes, dummy: PhantomData<(T, I, M)>, } -impl, I: 'static, M: GetMaxVoters> Tally { +impl, I: 'static, M: MaxClassVoters> Tally { pub fn from_parts(bare_ayes: MemberIndex, ayes: Votes, nays: Votes) -> Self { Tally { bare_ayes, ayes, nays, dummy: PhantomData } } @@ -112,53 +112,59 @@ impl, I: 'static, M: GetMaxVoters> Tally { pub type TallyOf = Tally>; pub type PollIndexOf = <>::Polls as Polling>>::Index; +pub type ClassOf = <>::Polls as Polling>>::Class; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; -impl, I: 'static, M: GetMaxVoters> VoteTally for Tally { - fn new(_: Rank) -> Self { +impl, I: 'static, M: MaxClassVoters>> + VoteTally> for Tally +{ + fn new(_: ClassOf) -> Self { Self { bare_ayes: 0, ayes: 0, nays: 0, dummy: PhantomData } } - fn ayes(&self, _: Rank) -> Votes { + fn ayes(&self, _: ClassOf) -> Votes { self.bare_ayes } - fn support(&self, class: Rank) -> Perbill { - Perbill::from_rational(self.bare_ayes, M::get_max_voters(class)) + fn support(&self, class: ClassOf) -> Perbill { + Perbill::from_rational(self.bare_ayes, M::max_class_voters(class)) } - fn approval(&self, _: Rank) -> Perbill { + fn approval(&self, _: ClassOf) -> Perbill { Perbill::from_rational(self.ayes, 1.max(self.ayes + self.nays)) } #[cfg(feature = "runtime-benchmarks")] - fn unanimity(class: Rank) -> Self { + fn unanimity(class: ClassOf) -> Self { Self { - bare_ayes: M::get_max_voters(class), - ayes: M::get_max_voters(class), + bare_ayes: M::max_class_voters(class), + ayes: M::max_class_voters(class), nays: 0, dummy: PhantomData, } } #[cfg(feature = "runtime-benchmarks")] - fn rejection(class: Rank) -> Self { - Self { bare_ayes: 0, ayes: 0, nays: M::get_max_voters(class), dummy: PhantomData } + fn rejection(class: ClassOf) -> Self { + Self { bare_ayes: 0, ayes: 0, nays: M::max_class_voters(class), dummy: PhantomData } } #[cfg(feature = "runtime-benchmarks")] - fn from_requirements(support: Perbill, approval: Perbill, class: Rank) -> Self { - let c = M::get_max_voters(class); + fn from_requirements(support: Perbill, approval: Perbill, class: ClassOf) -> Self { + let c = M::max_class_voters(class); let ayes = support * c; let nays = ((ayes as u64) * 1_000_000_000u64 / approval.deconstruct() as u64) as u32 - ayes; Self { bare_ayes: ayes, ayes, nays, dummy: PhantomData } } #[cfg(feature = "runtime-benchmarks")] - fn setup(class: Rank, granularity: Perbill) { - if M::get_max_voters(class) == 0 { + fn setup(class: ClassOf, granularity: Perbill) { + if M::max_class_voters(class) == 0 { let max_voters = granularity.saturating_reciprocal_mul(1u32); for i in 0..max_voters { let who: T::AccountId = frame_benchmarking::account("ranked_collective_benchmarking", i, 0); - crate::Pallet::::do_add_member_to_rank(who, class) - .expect("could not add members for benchmarks"); + crate::Pallet::::do_add_member_to_rank( + who, + T::MinRankOfClass::convert(class), + ) + .expect("could not add members for benchmarks"); } - assert_eq!(M::get_max_voters(class), max_voters); + assert_eq!(M::max_class_voters(class), max_voters); } } } @@ -228,16 +234,30 @@ impl Convert for Geometric { } /// Trait for getting the maximum number of voters for a given rank. -pub trait GetMaxVoters { +pub trait MaxRankVoters { /// Return the maximum number of voters for the rank `r`. - fn get_max_voters(r: Rank) -> MemberIndex; + fn max_rank_voters(r: Rank) -> MemberIndex; } -impl, I: 'static> GetMaxVoters for Pallet { - fn get_max_voters(r: Rank) -> MemberIndex { +impl, I: 'static> MaxRankVoters for Pallet { + fn max_rank_voters(r: Rank) -> MemberIndex { MemberCount::::get(r) } } +/// Trait for getting the maximum number of voters for a given poll class. +pub trait MaxClassVoters { + /// Poll class type. + type Class; + /// Return the maximum number of voters for the class `c`. + fn max_class_voters(c: Self::Class) -> MemberIndex; +} +impl, I: 'static> MaxClassVoters for Pallet { + type Class = ClassOf; + fn max_class_voters(c: Self::Class) -> MemberIndex { + ::max_rank_voters(T::MinRankOfClass::convert(c)) + } +} + /// 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)>); @@ -340,7 +360,7 @@ pub mod pallet { /// 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>; + type MinRankOfClass: Convert, Rank>; /// Convert a rank_delta into a number of votes the rank gets. /// From 9b881e41859dfc47cda85feb8fd73d3a6cabd43c Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 5 Feb 2023 06:33:31 +0100 Subject: [PATCH 2/5] fix move --- frame/ranked-collective/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index c4814def2b15f..a460f69846285 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -133,7 +133,7 @@ impl, I: 'static, M: MaxClassVoters>> #[cfg(feature = "runtime-benchmarks")] fn unanimity(class: ClassOf) -> Self { Self { - bare_ayes: M::max_class_voters(class), + bare_ayes: M::max_class_voters(class.clone()), ayes: M::max_class_voters(class), nays: 0, dummy: PhantomData, @@ -153,14 +153,14 @@ impl, I: 'static, M: MaxClassVoters>> #[cfg(feature = "runtime-benchmarks")] fn setup(class: ClassOf, granularity: Perbill) { - if M::max_class_voters(class) == 0 { + if M::max_class_voters(class.clone()) == 0 { let max_voters = granularity.saturating_reciprocal_mul(1u32); for i in 0..max_voters { let who: T::AccountId = frame_benchmarking::account("ranked_collective_benchmarking", i, 0); crate::Pallet::::do_add_member_to_rank( who, - T::MinRankOfClass::convert(class), + T::MinRankOfClass::convert(class.clone()), ) .expect("could not add members for benchmarks"); } From 916d346f1c2f1f32979d11367a0971763981098c Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 29 Mar 2023 13:16:09 +0200 Subject: [PATCH 3/5] tests --- frame/ranked-collective/src/tests.rs | 62 ++++++++++++++++++++++++++-- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index c11d4877d3577..7e0fdb4b04639 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -25,10 +25,10 @@ use frame_support::{ parameter_types, traits::{ConstU16, ConstU32, ConstU64, EitherOf, Everything, MapSuccess, Polling}, }; -use sp_core::H256; +use sp_core::{Get, H256}; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, Identity, IdentityLookup, ReduceBy}, + traits::{BlakeTwo256, IdentityLookup, ReduceBy}, }; use super::*; @@ -36,6 +36,7 @@ use crate as pallet_ranked_collective; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; +type Class = Rank; frame_support::construct_runtime!( pub enum Test where @@ -95,7 +96,7 @@ impl Polling> for TestPolls { type Index = u8; type Votes = Votes; type Moment = u64; - type Class = Rank; + type Class = Class; fn classes() -> Vec { vec![0, 1, 2] } @@ -164,6 +165,19 @@ impl Polling> for TestPolls { } } +/// Convert the tally class into the minimum rank required to vote on the poll. +/// MinRank(Class) = Class - Delta +pub struct MinRankOfClass(PhantomData); +impl> Convert for MinRankOfClass { + fn convert(a: Class) -> Rank { + a - Delta::get() + } +} + +parameter_types! { + pub static MinRankOfClassDelta: Rank = 0; +} + impl Config for Test { type WeightInfo = (); type RuntimeEvent = RuntimeEvent; @@ -180,7 +194,7 @@ impl Config for Test { MapSuccess, ReduceBy>>, >; type Polls = TestPolls; - type MinRankOfClass = Identity; + type MinRankOfClass = MinRankOfClass; type VoteWeight = Geometric; } @@ -499,3 +513,43 @@ fn do_add_member_to_rank_works() { assert_eq!(member_count(max_rank + 1), 0); }) } + +#[test] +fn tally_support_correct() { + new_test_ext().execute_with(|| { + // add members, + // rank 1: accounts 1, 2, 3 + // rank 2: accounts 2, 3 + // rank 3: accounts 3. + assert_ok!(Club::add_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 2)); + assert_ok!(Club::add_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + assert_ok!(Club::promote_member(RuntimeOrigin::root(), 3)); + + // init tally with 1 aye vote. + let tally: TallyOf = Tally::from_parts(1, 1, 0); + + // with minRank(Class) = Class + // for class 3, 100% support. + MinRankOfClassDelta::set(0); + assert_eq!(tally.support(3), Perbill::from_rational(1u32, 1)); + + // with minRank(Class) = (Class - 1) + // for class 3, ~50% support. + MinRankOfClassDelta::set(1); + assert_eq!(tally.support(3), Perbill::from_rational(1u32, 2)); + + // with minRank(Class) = (Class - 2) + // for class 3, ~33% support. + MinRankOfClassDelta::set(2); + assert_eq!(tally.support(3), Perbill::from_rational(1u32, 3)); + + // reset back. + MinRankOfClassDelta::set(0); + }); +} From 7756173389a1ba7ded8750727109ef0a44bccf62 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 29 Mar 2023 13:16:36 +0200 Subject: [PATCH 4/5] rename to GetMaxVoters --- frame/ranked-collective/src/lib.rs | 43 +++++++++++------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index a460f69846285..2af14d0c9c657 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -90,14 +90,14 @@ pub type Votes = u32; )] #[scale_info(skip_type_params(T, I, M))] #[codec(mel_bound())] -pub struct Tally { +pub struct Tally { bare_ayes: MemberIndex, ayes: Votes, nays: Votes, dummy: PhantomData<(T, I, M)>, } -impl, I: 'static, M: MaxClassVoters> Tally { +impl, I: 'static, M: GetMaxVoters> Tally { pub fn from_parts(bare_ayes: MemberIndex, ayes: Votes, nays: Votes) -> Self { Tally { bare_ayes, ayes, nays, dummy: PhantomData } } @@ -115,7 +115,7 @@ pub type PollIndexOf = <>::Polls as Polling = <>::Polls as Polling>>::Class; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; -impl, I: 'static, M: MaxClassVoters>> +impl, I: 'static, M: GetMaxVoters>> VoteTally> for Tally { fn new(_: ClassOf) -> Self { @@ -125,7 +125,7 @@ impl, I: 'static, M: MaxClassVoters>> self.bare_ayes } fn support(&self, class: ClassOf) -> Perbill { - Perbill::from_rational(self.bare_ayes, M::max_class_voters(class)) + Perbill::from_rational(self.bare_ayes, M::get_max_voters(class)) } fn approval(&self, _: ClassOf) -> Perbill { Perbill::from_rational(self.ayes, 1.max(self.ayes + self.nays)) @@ -133,19 +133,19 @@ impl, I: 'static, M: MaxClassVoters>> #[cfg(feature = "runtime-benchmarks")] fn unanimity(class: ClassOf) -> Self { Self { - bare_ayes: M::max_class_voters(class.clone()), - ayes: M::max_class_voters(class), + bare_ayes: M::get_max_voters(class.clone()), + ayes: M::get_max_voters(class), nays: 0, dummy: PhantomData, } } #[cfg(feature = "runtime-benchmarks")] fn rejection(class: ClassOf) -> Self { - Self { bare_ayes: 0, ayes: 0, nays: M::max_class_voters(class), dummy: PhantomData } + 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, class: ClassOf) -> Self { - let c = M::max_class_voters(class); + 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 { bare_ayes: ayes, ayes, nays, dummy: PhantomData } @@ -153,7 +153,7 @@ impl, I: 'static, M: MaxClassVoters>> #[cfg(feature = "runtime-benchmarks")] fn setup(class: ClassOf, granularity: Perbill) { - if M::max_class_voters(class.clone()) == 0 { + if M::get_max_voters(class.clone()) == 0 { let max_voters = granularity.saturating_reciprocal_mul(1u32); for i in 0..max_voters { let who: T::AccountId = @@ -164,7 +164,7 @@ impl, I: 'static, M: MaxClassVoters>> ) .expect("could not add members for benchmarks"); } - assert_eq!(M::max_class_voters(class), max_voters); + assert_eq!(M::get_max_voters(class), max_voters); } } } @@ -233,28 +233,17 @@ impl Convert for Geometric { } } -/// Trait for getting the maximum number of voters for a given rank. -pub trait MaxRankVoters { - /// Return the maximum number of voters for the rank `r`. - fn max_rank_voters(r: Rank) -> MemberIndex; -} -impl, I: 'static> MaxRankVoters for Pallet { - fn max_rank_voters(r: Rank) -> MemberIndex { - MemberCount::::get(r) - } -} - /// Trait for getting the maximum number of voters for a given poll class. -pub trait MaxClassVoters { +pub trait GetMaxVoters { /// Poll class type. type Class; - /// Return the maximum number of voters for the class `c`. - fn max_class_voters(c: Self::Class) -> MemberIndex; + /// Return the maximum number of voters for the poll class `c`. + fn get_max_voters(c: Self::Class) -> MemberIndex; } -impl, I: 'static> MaxClassVoters for Pallet { +impl, I: 'static> GetMaxVoters for Pallet { type Class = ClassOf; - fn max_class_voters(c: Self::Class) -> MemberIndex { - ::max_rank_voters(T::MinRankOfClass::convert(c)) + fn get_max_voters(c: Self::Class) -> MemberIndex { + MemberCount::::get(T::MinRankOfClass::convert(c)) } } From 868aa3db751d9312412abf75631f01f6058bb4e1 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 17 May 2023 12:11:58 +0200 Subject: [PATCH 5/5] saturating sub --- frame/ranked-collective/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/ranked-collective/src/tests.rs b/frame/ranked-collective/src/tests.rs index 0040a45f8c1de..04519bc0f8e22 100644 --- a/frame/ranked-collective/src/tests.rs +++ b/frame/ranked-collective/src/tests.rs @@ -170,7 +170,7 @@ impl Polling> for TestPolls { pub struct MinRankOfClass(PhantomData); impl> Convert for MinRankOfClass { fn convert(a: Class) -> Rank { - a - Delta::get() + a.saturating_sub(Delta::get()) } }