Removed pallet::getter usage from pallet-alliance (#3738)

Part of #3326 

cc @kianenigma @ggwpez @liamaharon 

polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

---------

Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

prdoc/pr_3738.prdoc

@@ -0,0 +1,14 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Removed `pallet::getter` usage from `pallet-alliance`
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      This PR removes `pallet::getter` usage from `pallet-alliance`, and updates dependant code accordingly.
+      The syntax `StorageItem::<T, I>::get()` should be used instead.
+
+crates: 
+  - name: pallet-alliance
+    bump: major

substrate/frame/alliance/src/benchmarking.rs

@@ -505,8 +505,8 @@ mod benchmarks {
 		assert_last_event::<T, I>(
 			Event::MembersInitialized { fellows: fellows.clone(), allies: allies.clone() }.into(),
 		);
-		assert_eq!(Alliance::<T, I>::members(MemberRole::Fellow), fellows);
-		assert_eq!(Alliance::<T, I>::members(MemberRole::Ally), allies);
+		assert_eq!(Members::<T, I>::get(MemberRole::Fellow), fellows);
+		assert_eq!(Members::<T, I>::get(MemberRole::Ally), allies);
 		Ok(())
 	}
 
@@ -563,7 +563,7 @@ mod benchmarks {
 		{
 			call.dispatch_bypass_filter(origin)?;
 		}
-		assert_eq!(Alliance::<T, I>::rule(), Some(rule.clone()));
+		assert_eq!(Rule::<T, I>::get(), Some(rule.clone()));
 		assert_last_event::<T, I>(Event::NewRuleSet { rule }.into());
 		Ok(())
 	}
@@ -583,7 +583,7 @@ mod benchmarks {
 			call.dispatch_bypass_filter(origin)?;
 		}
 
-		assert!(Alliance::<T, I>::announcements().contains(&announcement));
+		assert!(Announcements::<T, I>::get().contains(&announcement));
 		assert_last_event::<T, I>(Event::Announced { announcement }.into());
 		Ok(())
 	}
@@ -606,7 +606,7 @@ mod benchmarks {
 			call.dispatch_bypass_filter(origin)?;
 		}
 
-		assert!(!Alliance::<T, I>::announcements().contains(&announcement));
+		assert!(!Announcements::<T, I>::get().contains(&announcement));
 		assert_last_event::<T, I>(Event::AnnouncementRemoved { announcement }.into());
 		Ok(())
 	}

substrate/frame/alliance/src/lib.rs

@@ -442,24 +442,20 @@ pub mod pallet {
 	/// The IPFS CID of the alliance rule.
 	/// Fellows can propose a new rule with a super-majority.
 	#[pallet::storage]
-	#[pallet::getter(fn rule)]
 	pub type Rule<T: Config<I>, I: 'static = ()> = StorageValue<_, Cid, OptionQuery>;
 
 	/// The current IPFS CIDs of any announcements.
 	#[pallet::storage]
-	#[pallet::getter(fn announcements)]
 	pub type Announcements<T: Config<I>, I: 'static = ()> =
 		StorageValue<_, BoundedVec<Cid, T::MaxAnnouncementsCount>, ValueQuery>;
 
 	/// Maps members to their candidacy deposit.
 	#[pallet::storage]
-	#[pallet::getter(fn deposit_of)]
 	pub type DepositOf<T: Config<I>, I: 'static = ()> =
 		StorageMap<_, Blake2_128Concat, T::AccountId, BalanceOf<T, I>, OptionQuery>;
 
 	/// Maps member type to members of each type.
 	#[pallet::storage]
-	#[pallet::getter(fn members)]
 	pub type Members<T: Config<I>, I: 'static = ()> = StorageMap<
 		_,
 		Twox64Concat,
@@ -471,20 +467,17 @@ pub mod pallet {
 	/// A set of members who gave a retirement notice. They can retire after the end of retirement
 	/// period stored as a future block number.
 	#[pallet::storage]
-	#[pallet::getter(fn retiring_members)]
 	pub type RetiringMembers<T: Config<I>, I: 'static = ()> =
 		StorageMap<_, Blake2_128Concat, T::AccountId, BlockNumberFor<T>, OptionQuery>;
 
 	/// The current list of accounts deemed unscrupulous. These accounts non grata cannot submit
 	/// candidacy.
 	#[pallet::storage]
-	#[pallet::getter(fn unscrupulous_accounts)]
 	pub type UnscrupulousAccounts<T: Config<I>, I: 'static = ()> =
 		StorageValue<_, BoundedVec<T::AccountId, T::MaxUnscrupulousItems>, ValueQuery>;
 
 	/// The current list of websites deemed unscrupulous.
 	#[pallet::storage]
-	#[pallet::getter(fn unscrupulous_websites)]
 	pub type UnscrupulousWebsites<T: Config<I>, I: 'static = ()> =
 		StorageValue<_, BoundedVec<UrlOf<T, I>, T::MaxUnscrupulousItems>, ValueQuery>;
 

substrate/frame/alliance/src/migration.rs

@@ -162,18 +162,18 @@ pub(crate) mod v1_to_v2 {
 #[cfg(test)]
 mod test {
 	use super::*;
-	use crate::{mock::*, MemberRole};
+	use crate::{mock::*, MemberRole, Members};
 
 	#[test]
 	fn migration_v1_to_v2_works() {
 		new_test_ext().execute_with(|| {
 			assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(4)));
-			assert_eq!(Alliance::members(MemberRole::Ally), vec![4]);
-			assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]);
+			assert_eq!(Members::<Test>::get(MemberRole::Ally), vec![4]);
+			assert_eq!(Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3]);
 			v1_to_v2::migrate::<Test, ()>();
-			assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3, 4]);
-			assert_eq!(Alliance::members(MemberRole::Ally), vec![]);
-			assert_eq!(Alliance::members(MemberRole::Retiring), vec![]);
+			assert_eq!(Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3, 4]);
+			assert_eq!(Members::<Test>::get(MemberRole::Ally), vec![]);
+			assert_eq!(Members::<Test>::get(MemberRole::Retiring), vec![]);
 		});
 	}
 }

substrate/frame/alliance/src/tests.rs

@@ -21,7 +21,7 @@ use frame_support::{assert_noop, assert_ok, error::BadOrigin};
 use frame_system::{EventRecord, Phase};
 
 use super::*;
-use crate::mock::*;
+use crate::{self as alliance, mock::*};
 
 type AllianceMotionEvent = pallet_collective::Event<Test, pallet_collective::Instance1>;
 
@@ -118,7 +118,7 @@ fn disband_works() {
 		// join alliance and reserve funds
 		assert_eq!(Balances::free_balance(9), 1000 - id_deposit);
 		assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(9)));
-		assert_eq!(Alliance::deposit_of(9), Some(expected_join_deposit));
+		assert_eq!(alliance::DepositOf::<Test>::get(9), Some(expected_join_deposit));
 		assert_eq!(Balances::free_balance(9), 1000 - id_deposit - expected_join_deposit);
 		assert!(Alliance::is_member_of(&9, MemberRole::Ally));
 
@@ -314,7 +314,7 @@ fn set_rule_works() {
 	new_test_ext().execute_with(|| {
 		let cid = test_cid();
 		assert_ok!(Alliance::set_rule(RuntimeOrigin::signed(1), cid.clone()));
-		assert_eq!(Alliance::rule(), Some(cid.clone()));
+		assert_eq!(alliance::Rule::<Test>::get(), Some(cid.clone()));
 
 		System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::NewRuleSet {
 			rule: cid,
@@ -330,7 +330,7 @@ fn announce_works() {
 		assert_noop!(Alliance::announce(RuntimeOrigin::signed(2), cid.clone()), BadOrigin);
 
 		assert_ok!(Alliance::announce(RuntimeOrigin::signed(3), cid.clone()));
-		assert_eq!(Alliance::announcements(), vec![cid.clone()]);
+		assert_eq!(alliance::Announcements::<Test>::get(), vec![cid.clone()]);
 
 		System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::Announced {
 			announcement: cid,
@@ -343,15 +343,15 @@ fn remove_announcement_works() {
 	new_test_ext().execute_with(|| {
 		let cid = test_cid();
 		assert_ok!(Alliance::announce(RuntimeOrigin::signed(3), cid.clone()));
-		assert_eq!(Alliance::announcements(), vec![cid.clone()]);
+		assert_eq!(alliance::Announcements::<Test>::get(), vec![cid.clone()]);
 		System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::Announced {
 			announcement: cid.clone(),
 		}));
 
 		System::set_block_number(2);
 
 		assert_ok!(Alliance::remove_announcement(RuntimeOrigin::signed(3), cid.clone()));
-		assert_eq!(Alliance::announcements(), vec![]);
+		assert_eq!(alliance::Announcements::<Test>::get(), vec![]);
 		System::assert_last_event(mock::RuntimeEvent::Alliance(
 			crate::Event::AnnouncementRemoved { announcement: cid },
 		));
@@ -394,8 +394,8 @@ fn join_alliance_works() {
 		// success to submit
 		assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(4)));
 		assert_eq!(Balances::free_balance(4), 1000 - id_deposit - join_deposit);
-		assert_eq!(Alliance::deposit_of(4), Some(25));
-		assert_eq!(Alliance::members(MemberRole::Ally), vec![4]);
+		assert_eq!(alliance::DepositOf::<Test>::get(4), Some(25));
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Ally), vec![4]);
 
 		// check already member
 		assert_noop!(
@@ -449,8 +449,8 @@ fn nominate_ally_works() {
 
 		// success to nominate
 		assert_ok!(Alliance::nominate_ally(RuntimeOrigin::signed(1), 4));
-		assert_eq!(Alliance::deposit_of(4), None);
-		assert_eq!(Alliance::members(MemberRole::Ally), vec![4]);
+		assert_eq!(alliance::DepositOf::<Test>::get(4), None);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Ally), vec![4]);
 
 		// check already member
 		assert_noop!(
@@ -482,12 +482,12 @@ fn elevate_ally_works() {
 		);
 
 		assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(4)));
-		assert_eq!(Alliance::members(MemberRole::Ally), vec![4]);
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Ally), vec![4]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3]);
 
 		assert_ok!(Alliance::elevate_ally(RuntimeOrigin::signed(2), 4));
-		assert_eq!(Alliance::members(MemberRole::Ally), Vec::<u64>::new());
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3, 4]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Ally), Vec::<u64>::new());
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3, 4]);
 	});
 }
 
@@ -499,10 +499,10 @@ fn give_retirement_notice_work() {
 			Error::<Test, ()>::NotMember
 		);
 
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3]);
 		assert_ok!(Alliance::give_retirement_notice(RuntimeOrigin::signed(3)));
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2]);
-		assert_eq!(Alliance::members(MemberRole::Retiring), vec![3]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Retiring), vec![3]);
 		System::assert_last_event(mock::RuntimeEvent::Alliance(
 			crate::Event::MemberRetirementPeriodStarted { member: (3) },
 		));
@@ -527,15 +527,15 @@ fn retire_works() {
 			Error::<Test, ()>::RetirementNoticeNotGiven
 		);
 
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3]);
 		assert_ok!(Alliance::give_retirement_notice(RuntimeOrigin::signed(3)));
 		assert_noop!(
 			Alliance::retire(RuntimeOrigin::signed(3)),
 			Error::<Test, ()>::RetirementPeriodNotPassed
 		);
 		System::set_block_number(System::block_number() + RetirementPeriod::get());
 		assert_ok!(Alliance::retire(RuntimeOrigin::signed(3)));
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2]);
 		System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::MemberRetired {
 			member: (3),
 			unreserved: None,
@@ -551,7 +551,7 @@ fn retire_works() {
 #[test]
 fn abdicate_works() {
 	new_test_ext().execute_with(|| {
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3]);
 		assert_ok!(Alliance::abdicate_fellow_status(RuntimeOrigin::signed(3)));
 
 		System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::FellowAbdicated {
@@ -573,9 +573,9 @@ fn kick_member_works() {
 		);
 
 		<DepositOf<Test, ()>>::insert(2, 25);
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3]);
 		assert_ok!(Alliance::kick_member(RuntimeOrigin::signed(2), 2));
-		assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 3]);
+		assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 3]);
 		assert_eq!(<DepositOf<Test, ()>>::get(2), None);
 		System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::MemberKicked {
 			member: (2),
@@ -596,8 +596,11 @@ fn add_unscrupulous_items_works() {
 				UnscrupulousItem::Website("abc".as_bytes().to_vec().try_into().unwrap())
 			]
 		));
-		assert_eq!(Alliance::unscrupulous_accounts().into_inner(), vec![3]);
-		assert_eq!(Alliance::unscrupulous_websites().into_inner(), vec!["abc".as_bytes().to_vec()]);
+		assert_eq!(alliance::UnscrupulousAccounts::<Test>::get().into_inner(), vec![3]);
+		assert_eq!(
+			alliance::UnscrupulousWebsites::<Test>::get().into_inner(),
+			vec!["abc".as_bytes().to_vec()]
+		);
 
 		assert_noop!(
 			Alliance::add_unscrupulous_items(
@@ -629,12 +632,12 @@ fn remove_unscrupulous_items_works() {
 			RuntimeOrigin::signed(3),
 			vec![UnscrupulousItem::AccountId(3)]
 		));
-		assert_eq!(Alliance::unscrupulous_accounts(), vec![3]);
+		assert_eq!(alliance::UnscrupulousAccounts::<Test>::get(), vec![3]);
 		assert_ok!(Alliance::remove_unscrupulous_items(
 			RuntimeOrigin::signed(3),
 			vec![UnscrupulousItem::AccountId(3)]
 		));
-		assert_eq!(Alliance::unscrupulous_accounts(), Vec::<u64>::new());
+		assert_eq!(alliance::UnscrupulousAccounts::<Test>::get(), Vec::<u64>::new());
 	});
 }