Skip to content

Commit

Permalink
[Communities] Refactor pallet structure (#312)
Browse files Browse the repository at this point in the history
* refactor(pallets/communities): resolve ambiguity between CommunityMetadata storage type and struct

* refactor(pallets/communities): split logical modules within pallet

* refactor(communities:pallet): use pallet::call(weight) option

* refactor(pallet/communities): remove excessive verbosity

* change(pallet/communities): update documentation regarding tokens

* refactor(pallets/communities): split types into multiple files
  • Loading branch information
pandres95 authored Oct 6, 2023
1 parent cc217c5 commit 3cacc62
Show file tree
Hide file tree
Showing 16 changed files with 631 additions and 701 deletions.
17 changes: 7 additions & 10 deletions pallets/communities/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,11 @@ facilitating its participants to have governance over the community entity
the community. Can receive [payments][5], transfers, or payment [fees][6].
It can transfer funds via a privileged call executed by the community
_admin_ or a call dispatched from a proposal.
- **Governance Token:** A [non-sufficient fungible asset][7] issued and
administered by the _Treasury Account_ of the community. Customarily, it's
given among community members and can be used to vote depending on the
voting mechanism set by the community.
- **Economic Token:** A [sufficient fungible asset][7] issued and
administered by the _Treasury Account_ of the community. Generally used
for monetary purposes, it can be transferred among members and non-members
of the community, used to pay network fees, and for [payments][5] and its
corresponding [fees][6].
- **Community Token:** A Community might create and manage tokens by
dispatching the respective call from the corresponding origin. These tokens
also might be used to vote if set via Voting Mechanism.
- **Voting Method:** Can be either rank weighed, member-counted, or asset-weighed
and determines how the votes of proposals will be tallied.

## Goals

Expand Down Expand Up @@ -160,8 +156,9 @@ dispatched through an approved proposal. !

- `community`: Stores the basic information of the community. If a value exists for a
specified [`ComumunityId`][t00], this means a community exists.
- `asset_id`: Stores the ID of the [asset][7] for a given [`CommunityId`][t00].
- `metadata`: Stores the metadata regarding a community.
- `member_information`: Stores the information of a community (specified by its
- `membership`: Stores the information of a community (specified by its
[`CommunityId`][t00]) member (specified by it's [`AccountId`][1]).
- `members_count`: Store the count of community members. This simplifies the process of keeping track of members' count.

Expand Down
5 changes: 2 additions & 3 deletions pallets/communities/src/functions/challenges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use super::*;

impl<T: Config> Pallet<T> {
pub(crate) fn ensure_active(community_id: &CommunityIdOf<T>) -> DispatchResult {
let community_info =
<CommunityInfo<T>>::try_get(community_id).map_err(|_| Error::<T>::CommunityDoesNotExist)?;
let community_info = Self::community(community_id).ok_or(Error::<T>::CommunityDoesNotExist)?;

if community_info.state != CommunityState::Active {
Err(Error::<T>::CommunityNotActive)?
Expand All @@ -14,7 +13,7 @@ impl<T: Config> Pallet<T> {

#[allow(dead_code)]
pub(crate) fn do_force_complete_challenge(community_id: &CommunityIdOf<T>) -> DispatchResult {
<CommunityInfo<T>>::try_mutate_exists(community_id, |value| {
Info::<T>::try_mutate_exists(community_id, |value| {
let Some(community_info) = value else {
return Err::<(), DispatchError>(Error::<T>::CommunityDoesNotExist.into());
};
Expand Down
4 changes: 1 addition & 3 deletions pallets/communities/src/functions/getters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ impl<T: Config> Pallet<T> {
}

pub(crate) fn get_community_admin(community_id: &CommunityIdOf<T>) -> Result<AccountIdOf<T>, DispatchError> {
let Some(community) = <CommunityInfo<T>>::get(community_id) else {
Err(Error::<T>::CommunityDoesNotExist)?
};
let community = Self::community(community_id).ok_or(Error::<T>::CommunityDoesNotExist)?;

Ok(community.admin)
}
Expand Down
10 changes: 5 additions & 5 deletions pallets/communities/src/functions/membership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ impl<T: Config> Pallet<T> {
) -> Result<(), DispatchError> {
let caller = ensure_signed(origin)?;

if Self::member_information(community_id, caller).is_none() {
if Self::membership(community_id, caller).is_none() {
return Err(DispatchError::BadOrigin);
}

Expand All @@ -28,7 +28,7 @@ impl<T: Config> Pallet<T> {
}

pub(crate) fn do_insert_member(community_id: &CommunityIdOf<T>, who: &AccountIdOf<T>) -> DispatchResult {
<CommunityMembers<T>>::try_mutate_exists(community_id, who, |value| {
Members::<T>::try_mutate_exists(community_id, who, |value| {
if value.is_some() {
return Err(Error::<T>::AlreadyAMember.into());
}
Expand All @@ -38,14 +38,14 @@ impl<T: Config> Pallet<T> {

// Increases member count
let members_count = Self::members_count(community_id).unwrap_or_default();
<CommunityMembersCount<T>>::set(community_id, members_count.checked_add(1));
MembersCount::<T>::set(community_id, members_count.checked_add(1));

Ok(())
})
}

pub(crate) fn do_remove_member(community_id: &T::CommunityId, who: &T::AccountId) -> DispatchResult {
<CommunityMembers<T>>::try_mutate_exists(community_id, who, |value| {
Members::<T>::try_mutate_exists(community_id, who, |value| {
if value.is_none() {
return Err(Error::<T>::NotAMember.into());
}
Expand All @@ -63,7 +63,7 @@ impl<T: Config> Pallet<T> {

// Decreases member count
let members_count = Self::members_count(community_id).unwrap_or_default();
<CommunityMembersCount<T>>::set(community_id, members_count.checked_sub(1));
MembersCount::<T>::set(community_id, members_count.checked_sub(1));

Ok(())
})
Expand Down
11 changes: 4 additions & 7 deletions pallets/communities/src/functions/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ impl<T: Config> Pallet<T> {
return Err(Error::<T>::CommunityAlreadyExists.into());
}

<CommunityInfo<T>>::insert(
Info::<T>::insert(
community_id.clone(),
Community {
CommunityInfo {
admin: who.clone(),
state: Default::default(),
sufficient_asset_id: None,
Expand All @@ -28,11 +28,8 @@ impl<T: Config> Pallet<T> {
Ok(())
}

pub(crate) fn do_set_metadata(
community_id: &CommunityIdOf<T>,
value: types::CommunityMetadata<T>,
) -> DispatchResult {
<pallet::CommunityMetadata<T>>::try_mutate(community_id, |metadata| {
pub(crate) fn do_set_metadata(community_id: &CommunityIdOf<T>, value: CommunityMetadata<T>) -> DispatchResult {
Metadata::<T>::try_mutate(community_id, |metadata| {
*metadata = Some(value);

Ok(())
Expand Down
55 changes: 23 additions & 32 deletions pallets/communities/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,11 @@
//! the community. Can receive [payments][5], transfers, or payment [fees][6].
//! It can transfer funds via a privileged call executed by the community
//! _admin_ or a call dispatched from a proposal.
//! - **Governance Token:** A [non-sufficient fungible asset][7] issued and
//! administered by the _Treasury Account_ of the community. Customarily, it's
//! given among community members and can be used to vote depending on the
//! voting mechanism set by the community.
//! - **Economic Token:** A [sufficient fungible asset][7] issued and
//! administered by the _Treasury Account_ of the community. Generally used
//! for monetary purposes, it can be transferred among members and non-members
//! of the community, used to pay network fees, and for [payments][5] and its
//! corresponding [fees][6].
//! - **Community Token:** A Community might create and manage tokens by
//! dispatching the respective call from the corresponding origin. These
//! tokens also might be used to vote if set via Voting Mechanism.
//! - **Voting Method:** Can be either rank weighed, member-counted, or
//! asset-weighed and determines how the votes of proposals will be tallied.
//!
//! ## Goals
//!
Expand Down Expand Up @@ -167,10 +163,9 @@
//! value exists for a specified [`ComumunityId`][t00], this means a community
//! exists.
//! - [`metadata`][g01]: Stores the metadata regarding a community.
//! - [`member_information`][g02]: Stores the information of a community
//! (specified by its [`CommunityId`][t00]) member (specified by it's
//! [`AccountId`][1]).
//! - [`members_count`][g03]: Store the count of community members. This
//! - [`membership`][g02]: Stores the information of a community (specified by
//! its [`CommunityId`][t00]) member (specified by it's [`AccountId`][1]).
//! - [`members_count`][g03]: Stores the count of community members. This
//! simplifies the process of keeping track of members' count.
//!
//! <!-- References -->
Expand All @@ -195,13 +190,10 @@
//!
//! [g00]: `crate::Pallet::community`
//! [g01]: `crate::Pallet::metadata`
//! [g02]: `crate::Pallet::member_information`
//! [g02]: `crate::Pallet::membership`
//! [g03]: `crate::Pallet::members_count`
pub use pallet::*;

#[cfg(test)]
mod mock;

#[cfg(test)]
mod tests;

Expand Down Expand Up @@ -286,20 +278,19 @@ pub mod pallet {
/// community exists.
#[pallet::storage]
#[pallet::getter(fn community)]
pub(super) type CommunityInfo<T> = StorageMap<_, Blake2_128Concat, CommunityIdOf<T>, Community<T>>;
pub(super) type Info<T> = StorageMap<_, Blake2_128Concat, CommunityIdOf<T>, CommunityInfo<T>>;

/// Stores the metadata regarding a community.
#[pallet::storage]
#[pallet::getter(fn metadata)]
pub(super) type CommunityMetadata<T: Config> =
StorageMap<_, Blake2_128Concat, CommunityIdOf<T>, types::CommunityMetadata<T>>;
pub(super) type Metadata<T: Config> = StorageMap<_, Blake2_128Concat, CommunityIdOf<T>, CommunityMetadata<T>>;

/// Stores the information of a community (specified by its
/// [`CommunityId`][`Config::CommunityId`]) member (specified by it's
/// [`AccountId`][`frame_system::Config::AccountId`]).
#[pallet::storage]
#[pallet::getter(fn member_information)]
pub(super) type CommunityMembers<T> = StorageDoubleMap<
#[pallet::getter(fn membership)]
pub(super) type Members<T> = StorageDoubleMap<
_,
Blake2_128Concat,
CommunityIdOf<T>,
Expand All @@ -312,7 +303,7 @@ pub mod pallet {
/// keeping track of members' count.
#[pallet::storage]
#[pallet::getter(fn members_count)]
pub(super) type CommunityMembersCount<T> = StorageMap<_, Blake2_128Concat, CommunityIdOf<T>, u128>;
pub(super) type MembersCount<T> = StorageMap<_, Blake2_128Concat, CommunityIdOf<T>, u128>;

// Pallets use events to inform users when important changes are made.
// https://docs.substrate.io/main-docs/build/events-errors/
Expand Down Expand Up @@ -359,14 +350,15 @@ pub mod pallet {
// state changes. These functions materialize as "extrinsics", which are often
// compared to transactions. Dispatchable functions must be annotated with a
// weight and must return a DispatchResult.
#[pallet::call]
#[pallet::call(weight(<T as Config>::WeightInfo))]
impl<T: Config> Pallet<T> {
// === Registry management ===

/// Registers an appliation as a new community, taking an
/// [existential deposit][8] used to create the community account.
///
/// [8]: https://docs.substrate.io/reference/glossary/#existential-deposit
#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::apply())]
pub fn apply(origin: OriginFor<T>, community_id: T::CommunityId) -> DispatchResult {
let who = ensure_signed(origin)?;

Expand All @@ -385,7 +377,6 @@ pub mod pallet {
///
/// [11]: `types::CommunityMetadata`
#[pallet::call_index(1)]
#[pallet::weight(T::WeightInfo::set_metadata())]
pub fn set_metadata(
origin: OriginFor<T>,
community_id: T::CommunityId,
Expand All @@ -397,12 +388,12 @@ pub mod pallet {
// Ensures caller is a privileged origin
Self::ensure_origin_privileged(origin, &community_id)?;

let metadata = <CommunityMetadata<T>>::get(&community_id).unwrap_or_default();
let metadata = Self::metadata(&community_id).unwrap_or_default();

// Deposits metadata
Self::do_set_metadata(
&community_id,
types::CommunityMetadata {
CommunityMetadata {
name: name.clone().unwrap_or(metadata.name),
description: description.clone().unwrap_or(metadata.description),
urls: urls.clone().unwrap_or(metadata.urls),
Expand All @@ -423,11 +414,12 @@ pub mod pallet {
Ok(())
}

// === Memberships management ===

/// Enroll an account as a community member. In theory,
/// any community member should be able to add a member. However, this
/// can be changed to ensure it is a privileged function.
#[pallet::call_index(2)]
#[pallet::weight(T::WeightInfo::add_member())]
pub fn add_member(
origin: OriginFor<T>,
community_id: T::CommunityId,
Expand All @@ -449,7 +441,6 @@ pub mod pallet {
/// to arbitrarily remove the community admin, as some privileged calls
/// would be impossible to execute thereafter.
#[pallet::call_index(3)]
#[pallet::weight(T::WeightInfo::remove_member())]
pub fn remove_member(
origin: OriginFor<T>,
community_id: T::CommunityId,
Expand All @@ -464,10 +455,11 @@ pub mod pallet {
Ok(())
}

// === Treasury management ===

/// Transfers an amount of a given asset from the treasury account to a
/// beneficiary.
#[pallet::call_index(4)]
#[pallet::weight(T::WeightInfo::assets_transfer())]
pub fn assets_transfer(
origin: OriginFor<T>,
community_id: T::CommunityId,
Expand All @@ -490,7 +482,6 @@ pub mod pallet {

/// Transfers funds from the treasury account to a beneficiary
#[pallet::call_index(5)]
#[pallet::weight(T::WeightInfo::balance_transfer())]
pub fn balance_transfer(
origin: OriginFor<T>,
community_id: T::CommunityId,
Expand Down
Loading

0 comments on commit 3cacc62

Please sign in to comment.