Use specific Rank type instead of generic

pallets/communities/src/functions/membership.rs

@@ -1,5 +1,4 @@
 use super::*;
-use crate::traits::member_rank::Mutate;
 
 impl<T: Config> Pallet<T> {
 	pub(crate) fn ensure_origin_member(
@@ -26,10 +25,12 @@ impl<T: Config> Pallet<T> {
 		community_id: &CommunityIdOf<T>,
 	) -> Result<Option<AccountIdOf<T>>, DispatchError> {
 		if let Some(caller) = ensure_signed_or_root(origin)? {
-			if let Some(admin) = Self::get_community_admin(community_id) && admin == caller {
-				return Ok(Some(admin))
+			if let Some(admin) = Self::get_community_admin(community_id)
+				&& admin == caller
+			{
+				return Ok(Some(admin));
 			} else {
-				return Err(DispatchError::BadOrigin)
+				return Err(DispatchError::BadOrigin);
 			}
 		}
 
@@ -45,7 +46,7 @@ impl<T: Config> Pallet<T> {
 
 			// Inserts the member
 			*value = Some(Default::default());
-			MemberRanks::<T>::set(community_id, who, Some(Default::default()));
+			MemberRanks::<T>::set(community_id, who, Default::default());
 
 			// Increases member count
 			let members_count = Self::members_count(community_id).unwrap_or_default();
@@ -55,45 +56,15 @@ impl<T: Config> Pallet<T> {
 		})
 	}
 
-	pub(crate) fn do_promote_member(community_id: &CommunityIdOf<T>, who: &AccountIdOf<T>) -> DispatchResult {
-		MemberRanks::<T>::try_mutate(community_id, who, |maybe_rank| {
-			let Some(rank) = maybe_rank else {
-				return Err(Error::<T>::NotAMember)?;
-			};
-
-			*maybe_rank = rank.promote();
-
-			if maybe_rank.is_none() {
-				return Err(Error::<T>::ExceededPromoteBound)?;
-			}
-
-			Ok(())
-		})
-	}
-
-	pub(crate) fn do_demote_member(community_id: &CommunityIdOf<T>, who: &AccountIdOf<T>) -> DispatchResult {
-		MemberRanks::<T>::try_mutate(community_id, who, |maybe_rank| {
-			let Some(rank) = maybe_rank else {
-				return Err(Error::<T>::NotAMember)?;
-			};
-
-			*maybe_rank = rank.demote();
-
-			if maybe_rank.is_none() {
-				return Err(Error::<T>::ExceededDemoteBound)?;
-			}
-
-			Ok(())
-		})
-	}
-
 	pub(crate) fn do_remove_member(community_id: &T::CommunityId, who: &T::AccountId) -> DispatchResult {
 		Members::<T>::try_mutate_exists(community_id, who, |value| {
 			if value.is_none() {
 				return Err(Error::<T>::NotAMember.into());
 			}
 
-			if let Some(community_admin) = Self::get_community_admin(community_id) && community_admin == *who {
+			if let Some(community_admin) = Self::get_community_admin(community_id)
+				&& community_admin == *who
+			{
 				return Err(Error::<T>::CannotRemoveAdmin.into());
 			}
 

pallets/communities/src/lib.rs

@@ -176,7 +176,6 @@ mod benchmarking;
 
 mod functions;
 
-pub mod traits;
 pub mod types;
 pub mod weights;
 pub use weights::*;
@@ -185,7 +184,7 @@ pub use weights::*;
 pub mod pallet {
 	use super::*;
 	use frame_support::{
-		pallet_prelude::{DispatchResult, *},
+		pallet_prelude::{DispatchResult, ValueQuery, *},
 		traits::{
 			fungible::{Inspect, Mutate},
 			fungibles,
@@ -199,7 +198,6 @@ pub mod pallet {
 		pallet_prelude::{OriginFor, *},
 	};
 	use sp_runtime::traits::StaticLookup;
-	use traits::member_rank;
 	use types::*;
 
 	#[pallet::pallet]
@@ -215,8 +213,6 @@ pub mod pallet {
 		/// This type represents a rank for a member in a community
 		type Membership: Default + Parameter + MaxEncodedLen;
 
-		type MemberRank: Default + Parameter + MaxEncodedLen + member_rank::Inspect + member_rank::Mutate;
-
 		/// The asset used for governance
 		type Assets: fungibles::Inspect<Self::AccountId>;
 
@@ -267,7 +263,7 @@ pub mod pallet {
 	#[pallet::storage]
 	#[pallet::getter(fn member_rank)]
 	pub(super) type MemberRanks<T> =
-		StorageDoubleMap<_, Blake2_128Concat, CommunityIdOf<T>, Blake2_128Concat, AccountIdOf<T>, MemberRankOf<T>>;
+		StorageDoubleMap<_, Blake2_128Concat, CommunityIdOf<T>, Blake2_128Concat, AccountIdOf<T>, Rank, ValueQuery>;
 
 	/// Stores the count of community members. This simplifies the process of
 	/// keeping track of members' count.
@@ -340,13 +336,6 @@ pub mod pallet {
 		/// The community has exceeded the max amount of enqueded proposals at
 		/// this moment.
 		ExceededMaxProposals,
-		/// A member cannot be promoted since their rank is already at the upper
-		/// bound. The only option available so far is creating a higher rank.
-		ExceededPromoteBound,
-		/// A member cannot be demoted since their rank is already at the upper
-		/// bound. The options available so far are creating a lower rank, or
-		/// removing the member.
-		ExceededDemoteBound,
 		/// It is not possible to dequeue a proposal
 		CannotDequeueProposal,
 		/// A call for the spciefied [Hash][`frame_system::Config::Hash`] is not
@@ -472,9 +461,8 @@ pub mod pallet {
 		) -> DispatchResult {
 			Self::ensure_origin_privileged(origin, &community_id)?;
 			Self::ensure_active(&community_id)?;
-
-			let who = <<T as frame_system::Config>::Lookup as StaticLookup>::lookup(who)?;
-			Self::do_promote_member(&community_id, &who)
+			let who = T::Lookup::lookup(who)?;
+			Ok(MemberRanks::<T>::mutate(community_id, who, |rank| rank.promote()))
 		}
 
 		/// Decreases the rank of a member in the community
@@ -486,9 +474,8 @@ pub mod pallet {
 		) -> DispatchResult {
 			Self::ensure_origin_privileged(origin, &community_id)?;
 			Self::ensure_active(&community_id)?;
-
-			let who = <<T as frame_system::Config>::Lookup as StaticLookup>::lookup(who)?;
-			Self::do_demote_member(&community_id, &who)
+			let who = T::Lookup::lookup(who)?;
+			Ok(MemberRanks::<T>::mutate(community_id, who, |rank| rank.demote()))
 		}
 
 		// === Governance ===

pallets/communities/src/tests/membership.rs

@@ -210,6 +210,8 @@ mod member_rank {
 	}
 
 	mod promote_member {
+		use crate::MemberRanks;
+
 		use super::*;
 
 		#[test]
@@ -254,27 +256,23 @@ mod member_rank {
 					COMMUNITY,
 					COMMUNITY_MEMBER_1
 				));
-				assert_eq!(Communities::member_rank(COMMUNITY, COMMUNITY_MEMBER_1), Some(1));
+				assert_eq!(Communities::member_rank(COMMUNITY, COMMUNITY_MEMBER_1), 1.into());
 			});
 		}
 
 		#[test]
-		fn should_fail_when_exceeding_rank_upper_bound() {
+		fn should_stay_at_max_rank() {
 			new_test_ext().execute_with(|| {
 				setup();
 
-				while Communities::member_rank(COMMUNITY, COMMUNITY_MEMBER_1) < Some(u8::MAX) {
-					assert_ok!(Communities::promote_member(
-						RuntimeOrigin::signed(COMMUNITY_ADMIN),
-						COMMUNITY,
-						COMMUNITY_MEMBER_1
-					));
-				}
+				MemberRanks::<Test>::set(COMMUNITY, COMMUNITY_MEMBER_1, Rank::MAX);
+				assert_ok!(Communities::promote_member(
+					RuntimeOrigin::signed(COMMUNITY_ADMIN),
+					COMMUNITY,
+					COMMUNITY_MEMBER_1
+				));
 
-				assert_noop!(
-					Communities::promote_member(RuntimeOrigin::signed(COMMUNITY_ADMIN), COMMUNITY, COMMUNITY_MEMBER_1),
-					Error::ExceededPromoteBound,
-				);
+				assert_eq!(Communities::member_rank(COMMUNITY, COMMUNITY_MEMBER_1), Rank::MAX);
 			});
 		}
 	}
@@ -317,26 +315,29 @@ mod member_rank {
 			new_test_ext().execute_with(|| {
 				setup();
 
-				MemberRanks::<Test>::set(COMMUNITY, COMMUNITY_MEMBER_1, Some(2));
+				MemberRanks::<Test>::set(COMMUNITY, COMMUNITY_MEMBER_1, 2.into());
 
 				assert_ok!(Communities::demote_member(
 					RuntimeOrigin::signed(COMMUNITY_ADMIN),
 					COMMUNITY,
 					COMMUNITY_MEMBER_1
 				));
-				assert_eq!(Communities::member_rank(COMMUNITY, COMMUNITY_MEMBER_1), Some(1));
+				assert_eq!(Communities::member_rank(COMMUNITY, COMMUNITY_MEMBER_1), 1.into());
 			});
 		}
 
 		#[test]
-		fn should_fail_when_exceeding_rank_upper_bound() {
+		fn should_remain_at_min_rank() {
 			new_test_ext().execute_with(|| {
 				setup();
 
-				assert_noop!(
-					Communities::demote_member(RuntimeOrigin::signed(COMMUNITY_ADMIN), COMMUNITY, COMMUNITY_MEMBER_1),
-					Error::ExceededDemoteBound,
-				);
+				MemberRanks::<Test>::set(COMMUNITY, COMMUNITY_MEMBER_1, 0.into());
+				assert_ok!(Communities::demote_member(
+					RuntimeOrigin::signed(COMMUNITY_ADMIN),
+					COMMUNITY,
+					COMMUNITY_MEMBER_1
+				));
+				assert_eq!(Communities::member_rank(COMMUNITY, COMMUNITY_MEMBER_1), Rank::MIN);
 			});
 		}
 	}

pallets/communities/src/tests/mock.rs

@@ -187,7 +187,6 @@ impl pallet_communities::Config for Test {
 	type Balances = Balances;
 	type CommunityId = CommunityId;
 	type Membership = MembershipPassport;
-	type MemberRank = MemberRank;
 	type PalletId = CommunitiesPalletId;
 	type Polls = Referenda;
 }

pallets/communities/src/types/mod.rs

@@ -1,3 +1,5 @@
+use core::ops::Deref;
+
 use frame_support::pallet_prelude::{Decode, Encode};
 use frame_support::traits::fungible::Inspect;
 use frame_support::{sp_runtime::BoundedVec, traits::ConstU32};
@@ -11,11 +13,43 @@ pub(crate) use frame_system::Config as SystemConfig;
 pub use governance::*;
 pub use origin::*;
 pub use parameters::*;
-pub use primitives::*;
 pub use registry::*;
 
 mod governance;
 mod origin;
 mod parameters;
-mod primitives;
 mod registry;
+
+pub type SizedField<S> = BoundedVec<u8, S>;
+pub type ConstSizedField<const S: u32> = SizedField<ConstU32<S>>;
+
+/// A general purpose rank in the range 0-100
+#[derive(Debug, Default, Clone, Copy, PartialEq, PartialOrd, Encode, Decode, TypeInfo, MaxEncodedLen)]
+pub struct Rank(u8);
+
+impl Rank {
+	pub const MAX: Self = Rank(100);
+	pub const MIN: Self = Rank(0);
+
+	#[inline]
+	pub fn promote(&mut self) {
+		*self = self.0.saturating_add(1).min(Self::MAX.0).into()
+	}
+
+	#[inline]
+	pub fn demote(&mut self) {
+		*self = self.0.saturating_sub(1).max(Self::MIN.0).into()
+	}
+}
+
+impl From<u8> for Rank {
+	fn from(rank: u8) -> Self {
+		Rank(rank)
+	}
+}
+impl Deref for Rank {
+	type Target = u8;
+	fn deref(&self) -> &Self::Target {
+		&(self.0)
+	}
+}

pallets/communities/src/types/parameters.rs

@@ -10,7 +10,6 @@ pub type AccountIdLookupOf<T> = <<T as SystemConfig>::Lookup as StaticLookup>::S
 pub type CommunityIdOf<T> = <T as Config>::CommunityId;
 pub type MemberListOf<T> = Vec<AccountIdOf<T>>;
 pub type MembershipOf<T> = <T as Config>::Membership;
-pub type MemberRankOf<T> = <T as Config>::MemberRank;
 pub type VoteOf<T> = Vote<AssetIdOf<T>, AssetBalanceOf<T>>;
 pub type PollIndexOf<T> = <<T as Config>::Polls as Polling<Tally<T>>>::Index;
 pub type RuntimeOriginOf<T> = <<T as SystemConfig>::RuntimeOrigin as OriginTrait>::PalletsOrigin;