[Communities] Remove DecisionMethod from Origin (#356)

* chore(pallet-communities): remove `decision_method` from `Origin` * Benchmarking: Calculate weights for b838122 (#357) * [ci] calculate weights * change(pallet-communities): update weight sanity checks --------- Co-authored-by: pandres95 <[email protected]> Co-authored-by: Pablo Andrés Dorado Suárez <[email protected]> * change(pallet-communities): create `admin_origin` directly into `benchmarking` --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
virto-network · Apr 8, 2024 · a9ca1ca · a9ca1ca
1 parent c093eac
commit a9ca1ca
Show file tree

Hide file tree

Showing 10 changed files with 130 additions and 155 deletions.
diff --git a/pallets/communities/src/benchmarking.rs b/pallets/communities/src/benchmarking.rs
@@ -8,7 +8,7 @@ use self::{
 		AccountIdOf, CommunityIdOf, DecisionMethodFor, MembershipIdOf, NativeBalanceOf, PalletsOriginOf, PollIndexOf,
 		RuntimeCallFor, Vote,
 	},
-	Event, HoldReason, Pallet as Communities,
+	CommunityDecisionMethod, Event, HoldReason, Pallet as Communities,
 };
 use fc_traits_memberships::{Inspect, Rank};
 use frame_benchmarking::v2::*;
@@ -56,17 +56,15 @@ fn setup_accounts<T: Config>() -> Result<Vec<AccountIdOf<T>>, BenchmarkError> {
 
 fn community_params<T: Config>(
 	maybe_decision_method: Option<DecisionMethodFor<T>>,
-) -> (
-	CommunityIdOf<T>,
-	DecisionMethodFor<T>,
-	T::RuntimeOrigin,
-	PalletsOriginOf<T>,
-) {
+) -> (CommunityIdOf<T>, DecisionMethodFor<T>, OriginFor<T>, PalletsOriginOf<T>)
+where
+	OriginFor<T>: From<Origin<T>>,
+{
 	let community_id = T::BenchmarkHelper::community_id();
 
 	let decision_method = maybe_decision_method.unwrap_or(DecisionMethod::Rank);
-	let admin_origin: T::RuntimeOrigin = T::BenchmarkHelper::community_origin(decision_method.clone());
-	let admin_origin_caller: PalletsOriginOf<T> = admin_origin.clone().into_caller();
+	let admin_origin: OriginFor<T> = Origin::<T>::new(community_id).into();
+	let admin_origin_caller = admin_origin.clone().into_caller();
 
 	(community_id, decision_method, admin_origin, admin_origin_caller)
 }
@@ -76,7 +74,10 @@ fn community_params<T: Config>(
 fn create_community<T: Config>(
 	origin: OriginFor<T>,
 	maybe_decision_method: Option<DecisionMethodFor<T>>,
-) -> Result<(CommunityIdOf<T>, OriginFor<T>), BenchmarkError> {
+) -> Result<(CommunityIdOf<T>, OriginFor<T>), BenchmarkError>
+where
+	OriginFor<T>: From<Origin<T>>,
+{
 	T::BenchmarkHelper::initialize_memberships_collection()?;
 	let (community_id, decision_method, admin_origin, admin_origin_caller) =
 		community_params::<T>(maybe_decision_method);
@@ -136,6 +137,7 @@ where
 #[benchmarks(
 	where
 		T: frame_system::Config + crate::Config,
+		OriginFor<T>: From<Origin<T>>,
 		<T as Config>::RuntimeEvent: From<frame_system::Event<T>>,
 		MembershipIdOf<T>: From<u32>,
 		BlockNumberFor<T>: From<u32>
@@ -177,9 +179,9 @@ mod benchmarks {
 	#[benchmark]
 	fn set_decision_method() -> Result<(), BenchmarkError> {
 		// setup code
-		let (id, _, _, admin_origin) = community_params::<T>(Default::default());
+		let (id, decision_method, _, admin_origin) = community_params::<T>(None);
 		Communities::<T>::create(RawOrigin::Root.into(), admin_origin, id)?;
-		crate::pallet::CommunityDecisionMethod::<T>::set(id, DecisionMethod::Rank);
+		CommunityDecisionMethod::<T>::set(id, decision_method);
 
 		#[extrinsic_call]
 		_(

diff --git a/pallets/communities/src/mock.rs b/pallets/communities/src/mock.rs
@@ -297,8 +297,8 @@ type MembershipCollection = ItemOf<Nfts, MembershipsManagerCollectionId, Account
 
 #[cfg(feature = "runtime-benchmarks")]
 use crate::{
-	types::{AssetIdOf, CommunityIdOf, DecisionMethodFor, MembershipIdOf, PollIndexOf},
-	BenchmarkHelper, Origin,
+	types::{AssetIdOf, CommunityIdOf, MembershipIdOf, PollIndexOf},
+	BenchmarkHelper,
 };
 
 #[cfg(feature = "runtime-benchmarks")]
@@ -327,13 +327,6 @@ impl BenchmarkHelper<Test> for CommunityBenchmarkHelper {
 		u8::MAX as u32
 	}
 
-	fn community_origin(decision_method: DecisionMethodFor<Test>) -> OriginFor<Test> {
-		let mut origin = Origin::<Test>::new(Self::community_id());
-		origin.with_decision_method(decision_method);
-
-		origin.into()
-	}
-
 	fn initialize_memberships_collection() -> Result<(), frame_benchmarking::BenchmarkError> {
 		TestEnvBuilder::initialize_memberships_manager_collection()?;
 		TestEnvBuilder::initialize_community_memberships_collection(&Self::community_id())?;
@@ -546,7 +539,7 @@ impl TestEnvBuilder {
 					.decision_methods
 					.get(community_id)
 					.expect("should include decision_method on add_community");
-				let community_origin: RuntimeOrigin = Self::create_community_origin(community_id, decision_method);
+				let community_origin: RuntimeOrigin = Self::create_community_origin(community_id);
 
 				Communities::create(RuntimeOrigin::root(), community_origin.caller().clone(), *community_id)
 					.expect("can add community");
@@ -618,13 +611,7 @@ impl TestEnvBuilder {
 		)
 	}
 
-	pub fn create_community_origin(
-		community_id: &CommunityId,
-		decision_method: &DecisionMethod<AssetId>,
-	) -> RuntimeOrigin {
-		let mut origin = pallet_communities::Origin::<Test>::new(*community_id);
-		origin.with_decision_method(decision_method.clone());
-
-		origin.into()
+	pub fn create_community_origin(community_id: &CommunityId) -> RuntimeOrigin {
+		pallet_communities::Origin::<Test>::new(*community_id).into()
 	}
 }
diff --git a/pallets/communities/src/origin.rs b/pallets/communities/src/origin.rs
@@ -1,5 +1,5 @@
 use crate::{
-	types::{AssetIdOf, CommunityIdOf, CommunityState::Active, MembershipIdOf},
+	types::{CommunityIdOf, CommunityState::Active, MembershipIdOf},
 	CommunityIdFor, Config, Info, Pallet,
 };
 use core::marker::PhantomData;
@@ -78,15 +78,13 @@ where
 #[derive(TypeInfo, Encode, Decode, MaxEncodedLen, Clone, Eq, PartialEq, Debug)]
 pub struct RawOrigin<T: Config> {
 	community_id: CommunityIdOf<T>,
-	method: DecisionMethod<AssetIdOf<T>>,
 	subset: Option<Subset<T>>,
 }
 
 impl<T: Config> RawOrigin<T> {
 	pub const fn new(community_id: CommunityIdOf<T>) -> Self {
 		RawOrigin {
 			community_id,
-			method: DecisionMethod::Membership,
 			subset: None,
 		}
 	}
@@ -95,17 +93,9 @@ impl<T: Config> RawOrigin<T> {
 		self.subset = Some(s);
 	}
 
-	pub fn with_decision_method(&mut self, m: DecisionMethod<AssetIdOf<T>>) {
-		self.method = m;
-	}
-
 	pub fn id(&self) -> CommunityIdOf<T> {
 		self.community_id
 	}
-
-	pub fn decision_method(&self) -> DecisionMethod<AssetIdOf<T>> {
-		self.method.clone()
-	}
 }
 
 /// Subsets of the community can also have a voice

diff --git a/pallets/communities/src/tests/governance.rs b/pallets/communities/src/tests/governance.rs
@@ -45,13 +45,13 @@ const CHARLIE: AccountId = AccountId::new([3; 32]);
 
 parameter_types! {
 	pub OriginForCommunityA: Box<OriginCaller> =
-		Box::new(TestEnvBuilder::create_community_origin(&COMMUNITY_A, &DecisionMethod::Membership).caller().clone());
+		Box::new(TestEnvBuilder::create_community_origin(&COMMUNITY_A).caller().clone());
 	pub OriginForCommunityB: Box<OriginCaller> =
-		Box::new(TestEnvBuilder::create_community_origin(&COMMUNITY_B, &DecisionMethod::CommunityAsset(COMMUNITY_B_ASSET_ID)).caller().clone());
+		Box::new(TestEnvBuilder::create_community_origin(&COMMUNITY_B).caller().clone());
 	pub OriginForCommunityC: Box<OriginCaller> =
-		Box::new(TestEnvBuilder::create_community_origin(&COMMUNITY_C, &DecisionMethod::NativeToken).caller().clone());
+		Box::new(TestEnvBuilder::create_community_origin(&COMMUNITY_C).caller().clone());
 	pub OriginForCommunityD: Box<OriginCaller> =
-		Box::new(TestEnvBuilder::create_community_origin(&COMMUNITY_D, &DecisionMethod::Rank).caller().clone());
+		Box::new(TestEnvBuilder::create_community_origin(&COMMUNITY_D).caller().clone());
 
 	pub CommunityTrack: TrackInfoOf<Test> = TrackInfo {
 		name: s("Community"),

diff --git a/pallets/communities/src/types.rs b/pallets/communities/src/types.rs
@@ -163,10 +163,6 @@ pub trait BenchmarkHelper<T: Config> {
 	/// effects of benchmark testing
 	fn community_desired_size() -> u32;
 
-	/// Returns the origin for the community
-	/// as well as the caller
-	fn community_origin(decision_method: DecisionMethodFor<T>) -> OriginFor<T>;
-
 	/// Initializes the membership collection of a community.
 	fn initialize_memberships_collection() -> Result<(), frame_benchmarking::BenchmarkError>;