From abe68375f922bd57a9d44942989f89234890f298 Mon Sep 17 00:00:00 2001 From: Daniel Olano Date: Sat, 11 May 2024 17:16:36 +0200 Subject: [PATCH 1/2] Automatically create the asset of a community if it doesnt exist yet --- pallets/communities/src/functions.rs | 7 ++- pallets/communities/src/lib.rs | 21 +++++-- pallets/communities/src/mock.rs | 7 ++- pallets/communities/src/origin.rs | 10 --- pallets/communities/src/tests/governance.rs | 68 +++++++++++++-------- pallets/communities/src/types.rs | 15 ++++- 6 files changed, 80 insertions(+), 48 deletions(-) diff --git a/pallets/communities/src/functions.rs b/pallets/communities/src/functions.rs index bb827995..6ae79a96 100644 --- a/pallets/communities/src/functions.rs +++ b/pallets/communities/src/functions.rs @@ -1,4 +1,4 @@ -use super::{origin::DecisionMethod, *}; +use super::*; use fc_traits_memberships::{GenericRank, Inspect, Rank}; use frame_support::{ dispatch::PostDispatchInfo, @@ -95,7 +95,10 @@ impl Pallet { }; let say = *match (vote, decision_method) { - (Vote::AssetBalance(say, asset, ..), DecisionMethod::CommunityAsset(a)) if asset == a => say, + (Vote::AssetBalance(say, asset, amount), DecisionMethod::CommunityAsset(a, min)) if asset == a => { + ensure!(amount > min, Error::::VoteBelowMinimum); + say + } (Vote::NativeBalance(say, ..), DecisionMethod::NativeToken) | (Vote::Standard(say), DecisionMethod::Membership | DecisionMethod::Rank) => say, _ => fail!(Error::::InvalidVoteType), diff --git a/pallets/communities/src/lib.rs b/pallets/communities/src/lib.rs index 0ae489fb..9e924b19 100644 --- a/pallets/communities/src/lib.rs +++ b/pallets/communities/src/lib.rs @@ -146,10 +146,8 @@ pub mod pallet { Blake2_128Concat, Parameter, }; use frame_system::pallet_prelude::{ensure_signed, BlockNumberFor, OriginFor}; - use sp_runtime::{ - traits::{Dispatchable, StaticLookup}, - TokenError, - }; + use sp_runtime::traits::AccountIdConversion; + use sp_runtime::traits::{Dispatchable, StaticLookup}; use sp_std::prelude::Box; const ONE: NonZeroU8 = NonZeroU8::MIN; @@ -193,7 +191,7 @@ pub mod pallet { >; /// Type represents interactions between fungibles (i.e. assets) - type Assets: fungibles::Inspect + type Assets: fungibles::Inspect> + fungibles::Mutate + fungibles::Create + fungibles::hold::Inspect @@ -351,6 +349,8 @@ pub mod pallet { NoLocksInPlace, /// The origin already controls another community AlreadyAdmin, + /// The vote is below the minimum requried + VoteBelowMinimum, } // Dispatchable functions allows users to interact with the pallet and invoke @@ -477,6 +477,15 @@ pub mod pallet { decision_method: DecisionMethodFor, ) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; + if let DecisionMethod::CommunityAsset(ref asset, min_vote) = decision_method { + // best effort attemt to create the asset if it doesn't exist + let _ = >::create( + asset.clone(), + T::PalletId::get().into_account_truncating(), + false, + min_vote, + ); + } CommunityDecisionMethod::::set(community_id, decision_method); Self::deposit_event(Event::DecisionMethodSet { id: community_id }); Ok(()) @@ -490,7 +499,7 @@ pub mod pallet { #[pallet::compact] poll_index: PollIndexOf, vote: VoteOf, ) -> DispatchResult { - ensure!(VoteWeight::from(&vote).gt(&0), TokenError::BelowMinimum); + ensure!(VoteWeight::from(&vote).gt(&0), Error::::VoteBelowMinimum); let who = ensure_signed(origin)?; let community_id = T::MemberMgmt::check_membership(&who, &membership_id).ok_or(Error::::NotAMember)?; let decision_method = CommunityDecisionMethod::::get(community_id); diff --git a/pallets/communities/src/mock.rs b/pallets/communities/src/mock.rs index 92c90815..7bd8b1ca 100644 --- a/pallets/communities/src/mock.rs +++ b/pallets/communities/src/mock.rs @@ -24,8 +24,9 @@ pub use virto_common::{CommunityId, MembershipId}; use crate::{ self as pallet_communities, - origin::{DecisionMethod, EnsureCommunity, EnsureSignedPays}, + origin::{EnsureCommunity, EnsureSignedPays}, types::{Tally, VoteWeight}, + DecisionMethod, }; // Weights constants @@ -455,7 +456,7 @@ pub(crate) struct TestEnvBuilder { assets_config: AssetsConfig, balances: Vec<(AccountId, Balance)>, communities: Vec, - decision_methods: sp_std::collections::btree_map::BTreeMap>, + decision_methods: sp_std::collections::btree_map::BTreeMap>, members: Vec<(CommunityId, AccountId)>, memberships: Vec<(CommunityId, MembershipId)>, tracks: Vec<(TrackIdOf, TrackInfoOf)>, @@ -498,7 +499,7 @@ impl TestEnvBuilder { pub(crate) fn add_community( mut self, community_id: CommunityId, - decision_method: DecisionMethod, + decision_method: DecisionMethod, members: &[AccountId], memberships: &[MembershipId], maybe_track: Option>, diff --git a/pallets/communities/src/origin.rs b/pallets/communities/src/origin.rs index adf2b0b5..57ea01fa 100644 --- a/pallets/communities/src/origin.rs +++ b/pallets/communities/src/origin.rs @@ -126,16 +126,6 @@ pub enum Subset { AtLeastRank(GenericRank), } -/// The mechanism used by the community or one of its subsets to make decisions -#[derive(Clone, Debug, Decode, Default, Encode, Eq, MaxEncodedLen, PartialEq, TypeInfo)] -pub enum DecisionMethod { - #[default] - Membership, - NativeToken, - CommunityAsset(AssetId), - Rank, -} - #[cfg(feature = "xcm")] impl TryConvert, xcm::v3::MultiLocation> for RawOrigin where diff --git a/pallets/communities/src/tests/governance.rs b/pallets/communities/src/tests/governance.rs index b27c94bb..e4a5d5e0 100644 --- a/pallets/communities/src/tests/governance.rs +++ b/pallets/communities/src/tests/governance.rs @@ -6,9 +6,8 @@ use parity_scale_codec::Encode; use sp_runtime::{str_array as s, BoundedVec, TokenError}; use crate::{ - origin::DecisionMethod, types::{Tally, Vote}, - Call, + Call, DecisionMethod, }; use frame_support::assert_noop; use pallet_referenda::TrackInfo; @@ -108,7 +107,7 @@ fn new_test_ext() -> sp_io::TestExternalities { // Community-asset based .add_community( COMMUNITY_B, - DecisionMethod::CommunityAsset(COMMUNITY_B_ASSET_ID), + DecisionMethod::CommunityAsset(COMMUNITY_B_ASSET_ID, 10), &[BOB, CHARLIE], memberships_of(COMMUNITY_B), Some(CommunityTrack::get()), @@ -119,7 +118,7 @@ fn new_test_ext() -> sp_io::TestExternalities { true, 1, None, - Some(vec![(BOB, 10), (CHARLIE, 10)]), + Some(vec![(BOB, 50), (CHARLIE, 50)]), ) // Native-asset based .add_community( @@ -210,7 +209,7 @@ mod vote { 1, Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 0) ), - TokenError::BelowMinimum + Error::VoteBelowMinimum ); }); } @@ -537,13 +536,34 @@ mod vote { RuntimeOrigin::signed(CHARLIE), membership(COMMUNITY_B, 2), 1, - Vote::AssetBalance(false, COMMUNITY_B_ASSET_ID, 10) + Vote::AssetBalance(false, COMMUNITY_B_ASSET_ID, 50) ), TokenError::FundsUnavailable ); }); } + #[test] + fn fails_if_asset_vote_weight_is_under_minimum() { + new_test_ext().execute_with(|| { + assert_noop!( + Communities::vote( + RuntimeOrigin::signed(BOB), + membership(COMMUNITY_B, 1), + 1, + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 10) + ), + Error::VoteBelowMinimum + ); + assert_ok!(Communities::vote( + RuntimeOrigin::signed(BOB), + membership(COMMUNITY_B, 1), + 1, + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 11) + )); + }); + } + #[test] fn holds_cannot_overlap() { new_test_ext().execute_with(|| { @@ -552,7 +572,7 @@ mod vote { COMMUNITY_B_ASSET_ID, &pallet_preimage::HoldReason::Preimage.into(), &CHARLIE, - 6, + 40, )); // Before voting, the poll is ongoing @@ -561,7 +581,7 @@ mod vote { RuntimeOrigin::signed(CHARLIE), membership(COMMUNITY_B, 2), 1, - Vote::AssetBalance(false, COMMUNITY_B_ASSET_ID, 5) + Vote::AssetBalance(false, COMMUNITY_B_ASSET_ID, 15) ), TokenError::FundsUnavailable ); @@ -589,7 +609,7 @@ mod vote { RuntimeOrigin::signed(BOB), membership(COMMUNITY_B, 1), 1, - Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 6) + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 30) )); tick_block(); @@ -598,7 +618,7 @@ mod vote { RuntimeOrigin::signed(CHARLIE), membership(COMMUNITY_B, 2), 1, - Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 6) + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 30) )); tick_block(); @@ -611,9 +631,9 @@ mod vote { pallet_referenda::Event::::Confirmed { index: 1, tally: Tally { - ayes: 12, + ayes: 60, nays: 0, - bare_ayes: 12, + bare_ayes: 60, ..Default::default() }, } @@ -643,7 +663,7 @@ mod vote { RuntimeOrigin::signed(BOB), membership(COMMUNITY_B, 1), 1, - Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 2) + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 12) )); tick_block(); @@ -652,7 +672,7 @@ mod vote { RuntimeOrigin::signed(CHARLIE), membership(COMMUNITY_B, 2), 1, - Vote::AssetBalance(false, COMMUNITY_B_ASSET_ID, 1) + Vote::AssetBalance(false, COMMUNITY_B_ASSET_ID, 11) )); tick_blocks(4); @@ -665,9 +685,9 @@ mod vote { pallet_referenda::Event::::Confirmed { index: 1, tally: Tally { - ayes: 2, - nays: 1, - bare_ayes: 2, + ayes: 12, + nays: 11, + bare_ayes: 12, ..Default::default() }, } @@ -699,7 +719,7 @@ mod vote { RuntimeOrigin::signed(CHARLIE), membership(COMMUNITY_B, 2), 1, - Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 6) + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 11) )); tick_block(); @@ -708,7 +728,7 @@ mod vote { RuntimeOrigin::signed(BOB), membership(COMMUNITY_B, 1), 1, - Vote::AssetBalance(false, COMMUNITY_B_ASSET_ID, 7) + Vote::AssetBalance(false, COMMUNITY_B_ASSET_ID, 12) )); tick_block(); @@ -717,7 +737,7 @@ mod vote { RuntimeOrigin::signed(CHARLIE), membership(COMMUNITY_B, 2), 1, - Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 8) + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 13) )); tick_blocks(3); @@ -726,9 +746,9 @@ mod vote { pallet_referenda::Event::::Confirmed { index: 1, tally: Tally { - ayes: 8, - nays: 7, - bare_ayes: 8, + ayes: 13, + nays: 12, + bare_ayes: 13, ..Default::default() }, } @@ -1156,7 +1176,7 @@ mod unlock { RuntimeOrigin::signed(BOB), membership(COMMUNITY_B, 1), 1, - Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 9) + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 15) )); assert_ok!(Communities::vote( diff --git a/pallets/communities/src/types.rs b/pallets/communities/src/types.rs index 2a9b80ef..da43ba66 100644 --- a/pallets/communities/src/types.rs +++ b/pallets/communities/src/types.rs @@ -1,4 +1,3 @@ -use crate::origin::DecisionMethod; use crate::{CommunityDecisionMethod, Config}; use fc_traits_memberships::{Inspect, Rank}; use frame_support::pallet_prelude::*; @@ -16,7 +15,7 @@ pub type NativeBalanceOf = <::Balances as fungible::Inspect = ::AccountId; pub type CommunityIdOf = ::CommunityId; pub type VoteOf = Vote, AssetBalanceOf, NativeBalanceOf>; -pub type DecisionMethodFor = DecisionMethod>; +pub type DecisionMethodFor = DecisionMethod, AssetBalanceOf>; pub type PollIndexOf = <::Polls as Polling>>::Index; pub type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; pub type PalletsOriginOf = @@ -54,6 +53,16 @@ pub enum CommunityState { Blocked, } +/// The mechanism used by the community or one of its subsets to make decisions +#[derive(Clone, Debug, Decode, Default, Encode, Eq, MaxEncodedLen, PartialEq, TypeInfo)] +pub enum DecisionMethod { + #[default] + Membership, + NativeToken, + CommunityAsset(AssetId, MinVote), + Rank, +} + // Governance pub type VoteWeight = u32; @@ -126,7 +135,7 @@ impl Tally { DecisionMethod::Membership => T::MemberMgmt::members_total(&community_id), DecisionMethod::Rank => T::MemberMgmt::ranks_total(&community_id), DecisionMethod::NativeToken => T::Balances::total_issuance().saturated_into::(), - DecisionMethod::CommunityAsset(asset_id) => { + DecisionMethod::CommunityAsset(asset_id, _) => { T::Assets::total_issuance(asset_id).saturated_into::() } } From d630577f12449598b111d1d3ab7c86a0570e2d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 14 May 2024 17:27:13 -0500 Subject: [PATCH 2/2] change(pallet-communities): solve testing issues --- pallets/communities/src/benchmarking.rs | 12 ++++-------- pallets/communities/src/functions.rs | 2 +- pallets/communities/src/tests/governance.rs | 4 ++-- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/pallets/communities/src/benchmarking.rs b/pallets/communities/src/benchmarking.rs index df3a5e3a..4eddd486 100644 --- a/pallets/communities/src/benchmarking.rs +++ b/pallets/communities/src/benchmarking.rs @@ -3,18 +3,17 @@ use super::*; use self::{ - origin::DecisionMethod, types::{ AccountIdOf, AssetIdOf, CommunityIdOf, DecisionMethodFor, MembershipIdOf, NativeBalanceOf, PalletsOriginOf, PollIndexOf, RuntimeCallFor, Vote, }, - CommunityDecisionMethod, Event, HoldReason, Pallet as Communities, + CommunityDecisionMethod, DecisionMethod, Event, HoldReason, Pallet as Communities, }; use fc_traits_memberships::{Inspect, Rank}; use frame_benchmarking::v2::*; use frame_support::traits::{ fungible::{InspectFreeze, Mutate}, - fungibles::{Create, Mutate as FunsMutate}, + fungibles::Mutate as FunsMutate, OriginTrait, }; use frame_system::{ @@ -197,7 +196,7 @@ mod benchmarks { _( admin_origin, id, - DecisionMethod::CommunityAsset(T::BenchmarkHelper::community_asset_id()), + DecisionMethod::CommunityAsset(T::BenchmarkHelper::community_asset_id(), 10u128.into()), ); // verification code @@ -316,7 +315,7 @@ mod benchmarks { // setup code let (id, origin) = create_community::( RawOrigin::Root.into(), - Some(DecisionMethodFor::::CommunityAsset(1u32.into())), + Some(DecisionMethodFor::::CommunityAsset(1u32.into(), 1u128.into())), )?; let members = setup_members::(origin.clone(), id)?; @@ -325,9 +324,6 @@ mod benchmarks { .expect("desired size of community to be equal or greather than 1") .clone(); - let community_account = Communities::::community_account(&id); - - T::Assets::create(1u32.into(), community_account, false, 1u128.into())?; T::Assets::mint_into(1u32.into(), &who, 4u128.into())?; prepare_track_and_prepare_poll::(origin.into_caller(), who.clone())?; diff --git a/pallets/communities/src/functions.rs b/pallets/communities/src/functions.rs index 6ae79a96..2da90ca7 100644 --- a/pallets/communities/src/functions.rs +++ b/pallets/communities/src/functions.rs @@ -96,7 +96,7 @@ impl Pallet { let say = *match (vote, decision_method) { (Vote::AssetBalance(say, asset, amount), DecisionMethod::CommunityAsset(a, min)) if asset == a => { - ensure!(amount > min, Error::::VoteBelowMinimum); + ensure!(amount >= min, Error::::VoteBelowMinimum); say } (Vote::NativeBalance(say, ..), DecisionMethod::NativeToken) diff --git a/pallets/communities/src/tests/governance.rs b/pallets/communities/src/tests/governance.rs index e4a5d5e0..c9f09c19 100644 --- a/pallets/communities/src/tests/governance.rs +++ b/pallets/communities/src/tests/governance.rs @@ -551,7 +551,7 @@ mod vote { RuntimeOrigin::signed(BOB), membership(COMMUNITY_B, 1), 1, - Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 10) + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 9) ), Error::VoteBelowMinimum ); @@ -559,7 +559,7 @@ mod vote { RuntimeOrigin::signed(BOB), membership(COMMUNITY_B, 1), 1, - Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 11) + Vote::AssetBalance(true, COMMUNITY_B_ASSET_ID, 10) )); }); }