From 03f50eda9ea9c2dd156f869c7e9b373dc8accf0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Fri, 9 Feb 2024 23:18:08 -0500 Subject: [PATCH] feat(pallet-communities): implement voting for DecisionMethod::Rank --- pallets/communities/src/functions.rs | 58 +++++++-- pallets/communities/src/impls.rs | 2 +- pallets/communities/src/lib.rs | 70 +++++++---- pallets/communities/src/tests/governance.rs | 128 +++++++++++++++++++- pallets/communities/src/types.rs | 6 +- 5 files changed, 224 insertions(+), 40 deletions(-) diff --git a/pallets/communities/src/functions.rs b/pallets/communities/src/functions.rs index f95b22c8..3e39eea4 100644 --- a/pallets/communities/src/functions.rs +++ b/pallets/communities/src/functions.rs @@ -79,24 +79,36 @@ impl Pallet { pub(crate) fn do_vote( who: &AccountIdOf, - community_id: &CommunityIdOf, + membership_id: MembershipIdOf, poll_index: PollIndexOf, vote: VoteOf, ) -> DispatchResult { + let info = T::MemberMgmt::get_membership(membership_id.clone(), &who).ok_or(Error::::NotAMember)?; + let community_id = CommunityIdOf::::from(membership_id.clone()); + if VoteWeight::from(vote.clone()) == 0 { return Err(TokenError::BelowMinimum.into()); } T::Polls::try_access_poll(poll_index, |poll_status| { let (tally, class) = poll_status.ensure_ongoing().ok_or(Error::::NotOngoing)?; - ensure!(community_id == &class, Error::::InvalidTrack); + ensure!(community_id == class, Error::::InvalidTrack); let decision_method = CommunityDecisionMethod::::get(community_id); let maybe_vote = Self::community_vote_of(who, poll_index); if let Some(vote) = maybe_vote { Self::do_unlock_for_vote(who, &poll_index, &vote)?; - tally.remove_vote(vote.clone().into(), vote.into()); + + let multiplied_vote = match CommunityDecisionMethod::::get(community_id) { + DecisionMethod::Rank => Into::::into(info.rank()) as u32, + _ => 1, + }; + tally.remove_vote( + vote.clone().into(), + multiplied_vote * Into::::into(vote.clone()), + vote.into(), + ); } let say = match vote.clone() { @@ -127,7 +139,16 @@ impl Pallet { }; Self::do_lock_for_vote(who, &poll_index, &vote)?; - tally.add_vote(say, vote.clone().into()); + + let multiplied_vote = match CommunityDecisionMethod::::get(community_id) { + DecisionMethod::Rank => Into::::into(info.rank()) as u32, + _ => 1, + }; + tally.add_vote( + say, + multiplied_vote * Into::::into(vote.clone()), + vote.clone().into(), + ); Self::deposit_event(Event::::VoteCasted { who: who.clone(), @@ -141,15 +162,26 @@ impl Pallet { pub(crate) fn do_remove_vote( who: &AccountIdOf, - community_id: &CommunityIdOf, + membership_id: MembershipIdOf, poll_index: PollIndexOf, ) -> DispatchResult { + let info = T::MemberMgmt::get_membership(membership_id.clone(), &who).ok_or(Error::::NotAMember)?; + let community_id = CommunityIdOf::::from(membership_id.clone()); + T::Polls::try_access_poll(poll_index, |poll_status| { if let Some((tally, class)) = poll_status.ensure_ongoing() { - ensure!(community_id == &class, Error::::InvalidTrack); + ensure!(community_id == class, Error::::InvalidTrack); let vote = Self::community_vote_of(who, poll_index).ok_or(Error::::NoVoteCasted)?; - tally.remove_vote(vote.clone().into(), vote.clone().into()); + let multiplied_vote = match CommunityDecisionMethod::::get(community_id) { + DecisionMethod::Rank => Into::::into(info.rank()) as u32, + _ => 1, + }; + tally.remove_vote( + vote.clone().into(), + multiplied_vote * Into::::into(vote.clone()), + vote.clone().into(), + ); let reason = HoldReason::VoteCasted(poll_index).into(); CommunityVotes::::remove(who, poll_index); @@ -199,26 +231,26 @@ impl Pallet { } impl Tally { - pub(self) fn add_vote(&mut self, say: bool, weight: VoteWeight) { + pub(self) fn add_vote(&mut self, say: bool, multiplied_weight: VoteWeight, weight: VoteWeight) { match say { true => { - self.ayes = self.ayes.saturating_add(weight); + self.ayes = self.ayes.saturating_add(multiplied_weight); self.bare_ayes = self.bare_ayes.saturating_add(weight); } false => { - self.nays = self.nays.saturating_add(weight); + self.nays = self.nays.saturating_add(multiplied_weight); } } } - pub(self) fn remove_vote(&mut self, say: bool, weight: VoteWeight) { + pub(self) fn remove_vote(&mut self, say: bool, multiplied_weight: VoteWeight, weight: VoteWeight) { match say { true => { - self.ayes = self.ayes.saturating_sub(weight); + self.ayes = self.ayes.saturating_sub(multiplied_weight); self.bare_ayes = self.bare_ayes.saturating_sub(weight); } false => { - self.nays = self.nays.saturating_sub(weight); + self.nays = self.nays.saturating_sub(multiplied_weight); } } } diff --git a/pallets/communities/src/impls.rs b/pallets/communities/src/impls.rs index 65c663c6..244b9a09 100644 --- a/pallets/communities/src/impls.rs +++ b/pallets/communities/src/impls.rs @@ -16,7 +16,7 @@ impl VoteTally> for Tally { } fn support(&self, community_id: CommunityIdOf) -> sp_runtime::Perbill { - Perbill::from_rational(self.bare_ayes, Self::max_ayes(community_id)) + Perbill::from_rational(self.bare_ayes, Self::max_support(community_id)) } fn approval(&self, _cid: CommunityIdOf) -> sp_runtime::Perbill { diff --git a/pallets/communities/src/lib.rs b/pallets/communities/src/lib.rs index a3ab1eb0..889eab6b 100644 --- a/pallets/communities/src/lib.rs +++ b/pallets/communities/src/lib.rs @@ -279,6 +279,10 @@ pub mod pallet { #[pallet::storage] pub(super) type CommunityMembersCount = StorageMap<_, Blake2_128Concat, CommunityIdOf, u32, ValueQuery>; + /// Stores the sum of members' ranks, managed by promote_member and demote_member + #[pallet::storage] + pub(super) type CommunityRanksSum = StorageMap<_, Blake2_128Concat, CommunityIdOf, u32, ValueQuery>; + /// Stores the decision method for a community #[pallet::storage] pub(super) type CommunityDecisionMethod = @@ -428,6 +432,13 @@ pub mod pallet { )?; CommunityMembersCount::::mutate(community_id, |count| { *count += 1; + CommunityRanksSum::::mutate(community_id, |sum| { + let rank = T::MemberMgmt::get_membership(membership_id.clone(), &who) + .ok_or(Error::::NotAMember) + .expect("a member has just been inserted; qed") + .rank(); + *sum += Into::::into(rank) as u32; + }); }); Self::deposit_event(Event::MemberAdded { who, membership_id }); @@ -463,6 +474,11 @@ pub mod pallet { )?; CommunityMembersCount::::mutate(community_id, |count| { *count -= 1; + + CommunityRanksSum::::mutate(community_id, |sum| { + let rank = info.rank(); + *sum -= Into::::into(rank) as u32; + }); }); Self::deposit_event(Event::MemberRemoved { who, membership_id }); @@ -476,16 +492,24 @@ pub mod pallet { who: AccountIdLookupOf, membership_id: MembershipIdOf, ) -> DispatchResult { - let _community_id = T::MemberMgmtOrigin::ensure_origin(origin)?; + let community_id = T::MemberMgmtOrigin::ensure_origin(origin)?; let who = T::Lookup::lookup(who)?; - let mut m = T::MemberMgmt::get_membership(membership_id.clone(), &who).ok_or(Error::::NotAMember)?; - let rank = m.rank(); - m.set_rank(rank.promote_by(1.try_into().expect("can promote by 1"))); - T::MemberMgmt::update(membership_id.clone(), m, None)?; + CommunityRanksSum::::try_mutate(community_id, |sum| { + let mut m = T::MemberMgmt::get_membership(membership_id.clone(), &who).ok_or(Error::::NotAMember)?; + let rank = m.rank(); - Self::deposit_event(Event::MembershipRankUpdated { membership_id, rank }); - Ok(()) + *sum = sum.saturating_sub(Into::::into(rank) as u32); + + let rank = rank.promote_by(1.try_into().expect("can demote by 1")); + m.set_rank(rank); + T::MemberMgmt::update(membership_id.clone(), m, None)?; + + *sum += Into::::into(rank) as u32; + + Self::deposit_event(Event::MembershipRankUpdated { membership_id, rank }); + Ok(()) + }) } /// Decreases the rank of a member in the community @@ -495,16 +519,24 @@ pub mod pallet { who: AccountIdLookupOf, membership_id: MembershipIdOf, ) -> DispatchResult { - let _community_id = T::MemberMgmtOrigin::ensure_origin(origin)?; + let community_id = T::MemberMgmtOrigin::ensure_origin(origin)?; let who = T::Lookup::lookup(who)?; - let mut m = T::MemberMgmt::get_membership(membership_id.clone(), &who).ok_or(Error::::NotAMember)?; - let rank = m.rank(); - m.set_rank(rank.demote_by(1.try_into().expect("can promote by 1"))); - T::MemberMgmt::update(membership_id.clone(), m, None)?; + CommunityRanksSum::::try_mutate(community_id, |sum| { + let mut m = T::MemberMgmt::get_membership(membership_id.clone(), &who).ok_or(Error::::NotAMember)?; + let rank = m.rank(); - Self::deposit_event(Event::MembershipRankUpdated { membership_id, rank }); - Ok(()) + *sum = sum.saturating_sub(Into::::into(rank) as u32); + + let rank = rank.demote_by(1.try_into().expect("can demote by 1")); + m.set_rank(rank); + T::MemberMgmt::update(membership_id.clone(), m, None)?; + + *sum += Into::::into(rank) as u32; + + Self::deposit_event(Event::MembershipRankUpdated { membership_id, rank }); + Ok(()) + }) } // === Governance === @@ -532,10 +564,7 @@ pub mod pallet { vote: VoteOf, ) -> DispatchResult { let who = ensure_signed(origin)?; - let _info = T::MemberMgmt::get_membership(membership_id.clone(), &who).ok_or(Error::::NotAMember)?; - let community_id = CommunityIdOf::::from(membership_id); - - Self::do_vote(&who, &community_id, poll_index, vote) + Self::do_vote(&who, membership_id, poll_index, vote) } /// @@ -546,10 +575,7 @@ pub mod pallet { #[pallet::compact] poll_index: PollIndexOf, ) -> DispatchResult { let who = ensure_signed(origin)?; - let _info = T::MemberMgmt::get_membership(membership_id.clone(), &who).ok_or(Error::::NotAMember)?; - let community_id = CommunityIdOf::::from(membership_id); - - Self::do_remove_vote(&who, &community_id, poll_index) + Self::do_remove_vote(&who, membership_id, poll_index) } /// diff --git a/pallets/communities/src/tests/governance.rs b/pallets/communities/src/tests/governance.rs index c6016012..58e0a331 100644 --- a/pallets/communities/src/tests/governance.rs +++ b/pallets/communities/src/tests/governance.rs @@ -831,7 +831,133 @@ mod vote { } mod rank { - // TODO: Implement rank-based voting first + use frame_support::traits::Polling; + + use super::*; + + fn new_test_ext() -> sp_io::TestExternalities { + let mut ext = super::new_test_ext(); + + ext.execute_with(|| { + assert_ok!(Communities::promote_member( + Into::::into(*OriginForCommunityD::get()), + ALICE, + MembershipId(COMMUNITY_D, 1) + )); + assert_ok!(Communities::promote_member( + Into::::into(*OriginForCommunityD::get()), + BOB, + MembershipId(COMMUNITY_D, 2) + )); + assert_ok!(Communities::promote_member( + Into::::into(*OriginForCommunityD::get()), + CHARLIE, + MembershipId(COMMUNITY_D, 3) + )); + + assert_ok!(Referenda::submit( + RuntimeOrigin::signed(CHARLIE), + OriginForCommunityD::get(), + ProposalCallPromoteCharlie::get(), + frame_support::traits::schedule::DispatchTime::After(1), + )); + + System::assert_has_event( + pallet_referenda::Event::::Submitted { + index: 3, + proposal: ProposalCallPromoteCharlie::get(), + track: COMMUNITY_D, + } + .into(), + ); + + assert_ok!(Referenda::place_decision_deposit(RuntimeOrigin::signed(CHARLIE), 3)); + + tick_block(); + }); + + ext + } + + #[test] + fn it_works() { + new_test_ext().execute_with(|| { + assert_ok!(Communities::vote( + RuntimeOrigin::signed(ALICE), + MembershipId(COMMUNITY_D, 1), + 3, + Vote::Standard(true) + )); + + tick_block(); + + assert_ok!(Communities::vote( + RuntimeOrigin::signed(BOB), + MembershipId(COMMUNITY_D, 2), + 3, + Vote::Standard(false) + )); + + tick_block(); + + assert_ok!(Communities::vote( + RuntimeOrigin::signed(CHARLIE), + MembershipId(COMMUNITY_D, 3), + 3, + Vote::Standard(true) + )); + + tick_blocks(3); + + System::assert_has_event(pallet_referenda::Event::::ConfirmStarted { index: 3 }.into()); + }); + } + + #[test] + fn it_works_with_different_ranks() { + new_test_ext().execute_with(|| { + assert_ok!(Communities::promote_member( + Into::::into(*OriginForCommunityD::get()), + ALICE, + MembershipId(COMMUNITY_D, 1) + )); + + assert_ok!(Communities::vote( + RuntimeOrigin::signed(ALICE), + MembershipId(COMMUNITY_D, 1), + 3, + Vote::Standard(false) + )); + + tick_block(); + + assert_ok!(Communities::vote( + RuntimeOrigin::signed(BOB), + MembershipId(COMMUNITY_D, 2), + 3, + Vote::Standard(true) + )); + + tick_block(); + + assert_ok!(Communities::vote( + RuntimeOrigin::signed(CHARLIE), + MembershipId(COMMUNITY_D, 3), + 3, + Vote::Standard(true) + )); + + assert_eq!( + Referenda::as_ongoing(3).expect("the poll was initiated; qed").0, + Tally { + ayes: 2, + nays: 2, + bare_ayes: 2, + ..Default::default() + } + ) + }); + } } } diff --git a/pallets/communities/src/types.rs b/pallets/communities/src/types.rs index 79db6df8..61889373 100644 --- a/pallets/communities/src/types.rs +++ b/pallets/communities/src/types.rs @@ -1,5 +1,5 @@ use crate::origin::DecisionMethod; -use crate::{CommunityDecisionMethod, CommunityMembersCount, Config}; +use crate::{CommunityDecisionMethod, CommunityMembersCount, CommunityRanksSum, Config}; use frame_support::pallet_prelude::*; use frame_support::traits::{ fungible::{self, Inspect as FunInspect}, @@ -123,10 +123,10 @@ impl Default for Tally { } impl Tally { - pub(crate) fn max_ayes(community_id: CommunityIdOf) -> VoteWeight { + pub(crate) fn max_support(community_id: CommunityIdOf) -> VoteWeight { match CommunityDecisionMethod::::get(community_id) { DecisionMethod::Membership => CommunityMembersCount::::get(community_id), - DecisionMethod::Rank => todo!("max_ayes is the sum of each rank of each membership"), + DecisionMethod::Rank => CommunityRanksSum::::get(community_id), DecisionMethod::NativeToken => T::Balances::total_issuance().saturated_into::(), DecisionMethod::CommunityAsset(asset_id) => { T::Assets::total_issuance(asset_id).saturated_into::()