Remove pallet::getter usage from the bounties and child-bounties pall…

…ets (paritytech#4392) As per paritytech#3326, removes pallet::getter usage from the bounties and child-bounties pallets. The syntax `StorageItem::<T, I>::get()` should be used instead. Changes to one pallet involved changes in the other, so I figured it'd be best to combine these two. cc @muraca --------- Co-authored-by: Bastian Köcher <[email protected]>
moonbeam-foundation · Aug 2, 2024 · 848620a · 848620a
1 parent 2a38895
commit 848620a
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 75 deletions.
diff --git a/prdoc/pr_4392.prdoc b/prdoc/pr_4392.prdoc
@@ -0,0 +1,16 @@
+# 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: Remove `pallet::getter` usage from both bounties and child bounties pallet
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      This PR removes `pallet::getter`s from `pallet-bounties` and `pallet-child-bounties`.
+      The syntax `StorageItem::<T, I>::get()` should be used instead.
+
+crates:
+  - name: pallet-bounties
+    bump: major
+  - name: pallet-child-bounties
+    bump: major
diff --git a/substrate/frame/bounties/src/lib.rs b/substrate/frame/bounties/src/lib.rs
@@ -303,12 +303,10 @@ pub mod pallet {
 
 	/// Number of bounty proposals that have been made.
 	#[pallet::storage]
-	#[pallet::getter(fn bounty_count)]
 	pub type BountyCount<T: Config<I>, I: 'static = ()> = StorageValue<_, BountyIndex, ValueQuery>;
 
 	/// Bounties that have been made.
 	#[pallet::storage]
-	#[pallet::getter(fn bounties)]
 	pub type Bounties<T: Config<I>, I: 'static = ()> = StorageMap<
 		_,
 		Twox64Concat,
@@ -318,13 +316,11 @@ pub mod pallet {
 
 	/// The description of each bounty.
 	#[pallet::storage]
-	#[pallet::getter(fn bounty_descriptions)]
 	pub type BountyDescriptions<T: Config<I>, I: 'static = ()> =
 		StorageMap<_, Twox64Concat, BountyIndex, BoundedVec<u8, T::MaximumReasonLength>>;
 
 	/// Bounty indices that have been approved but not yet funded.
 	#[pallet::storage]
-	#[pallet::getter(fn bounty_approvals)]
 	pub type BountyApprovals<T: Config<I>, I: 'static = ()> =
 		StorageValue<_, BoundedVec<BountyIndex, T::MaxApprovals>, ValueQuery>;
 
@@ -849,7 +845,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			description.try_into().map_err(|_| Error::<T, I>::ReasonTooBig)?;
 		ensure!(value >= T::BountyValueMinimum::get(), Error::<T, I>::InvalidValue);
 
-		let index = Self::bounty_count();
+		let index = BountyCount::<T, I>::get();
 
 		// reserve deposit for new bounty
 		let bond = T::BountyDepositBase::get() +

diff --git a/substrate/frame/bounties/src/tests.rs b/substrate/frame/bounties/src/tests.rs
@@ -534,7 +534,7 @@ fn propose_bounty_works() {
 		assert_eq!(Balances::free_balance(0), 100 - deposit);
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee: 0,
@@ -545,9 +545,12 @@ fn propose_bounty_works() {
 			}
 		);
 
-		assert_eq!(Bounties::bounty_descriptions(0).unwrap(), b"1234567890".to_vec());
+		assert_eq!(
+			pallet_bounties::BountyDescriptions::<Test>::get(0).unwrap(),
+			b"1234567890".to_vec()
+		);
 
-		assert_eq!(Bounties::bounty_count(), 1);
+		assert_eq!(pallet_bounties::BountyCount::<Test>::get(), 1);
 	});
 }
 
@@ -598,10 +601,10 @@ fn close_bounty_works() {
 		assert_eq!(Balances::reserved_balance(0), 0);
 		assert_eq!(Balances::free_balance(0), 100 - deposit);
 
-		assert_eq!(Bounties::bounties(0), None);
+		assert_eq!(pallet_bounties::Bounties::<Test>::get(0), None);
 		assert!(!pallet_treasury::Proposals::<Test>::contains_key(0));
 
-		assert_eq!(Bounties::bounty_descriptions(0), None);
+		assert_eq!(pallet_bounties::BountyDescriptions::<Test>::get(0), None);
 	});
 }
 
@@ -622,7 +625,7 @@ fn approve_bounty_works() {
 		let deposit: u64 = 80 + 5;
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee: 0,
@@ -632,7 +635,7 @@ fn approve_bounty_works() {
 				status: BountyStatus::Approved,
 			}
 		);
-		assert_eq!(Bounties::bounty_approvals(), vec![0]);
+		assert_eq!(pallet_bounties::BountyApprovals::<Test>::get(), vec![0]);
 
 		assert_noop!(
 			Bounties::close_bounty(RuntimeOrigin::root(), 0),
@@ -650,7 +653,7 @@ fn approve_bounty_works() {
 		assert_eq!(Balances::free_balance(0), 100);
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee: 0,
@@ -693,7 +696,7 @@ fn assign_curator_works() {
 		assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, fee));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee,
@@ -720,7 +723,7 @@ fn assign_curator_works() {
 		let expected_deposit = Bounties::calculate_curator_deposit(&fee);
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee,
@@ -755,7 +758,7 @@ fn unassign_curator_works() {
 		assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(4), 0));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee,
@@ -773,7 +776,7 @@ fn unassign_curator_works() {
 		assert_ok!(Bounties::unassign_curator(RuntimeOrigin::root(), 0));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee,
@@ -817,7 +820,7 @@ fn award_and_claim_bounty_works() {
 		assert_ok!(Bounties::award_bounty(RuntimeOrigin::signed(4), 0, 3));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee,
@@ -851,8 +854,8 @@ fn award_and_claim_bounty_works() {
 		assert_eq!(Balances::free_balance(3), 56);
 		assert_eq!(Balances::free_balance(Bounties::bounty_account_id(0)), 0);
 
-		assert_eq!(Bounties::bounties(0), None);
-		assert_eq!(Bounties::bounty_descriptions(0), None);
+		assert_eq!(pallet_bounties::Bounties::<Test>::get(0), None);
+		assert_eq!(pallet_bounties::BountyDescriptions::<Test>::get(0), None);
 	});
 }
 
@@ -892,8 +895,8 @@ fn claim_handles_high_fee() {
 		assert_eq!(Balances::free_balance(3), 0);
 		assert_eq!(Balances::free_balance(Bounties::bounty_account_id(0)), 0);
 
-		assert_eq!(Bounties::bounties(0), None);
-		assert_eq!(Bounties::bounty_descriptions(0), None);
+		assert_eq!(pallet_bounties::Bounties::<Test>::get(0), None);
+		assert_eq!(pallet_bounties::BountyDescriptions::<Test>::get(0), None);
 	});
 }
 
@@ -918,7 +921,7 @@ fn cancel_and_refund() {
 		));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee: 0,
@@ -978,8 +981,8 @@ fn award_and_cancel() {
 		assert_eq!(Balances::free_balance(0), 95);
 		assert_eq!(Balances::reserved_balance(0), 0);
 
-		assert_eq!(Bounties::bounties(0), None);
-		assert_eq!(Bounties::bounty_descriptions(0), None);
+		assert_eq!(pallet_bounties::Bounties::<Test>::get(0), None);
+		assert_eq!(pallet_bounties::BountyDescriptions::<Test>::get(0), None);
 	});
 }
 
@@ -1015,7 +1018,7 @@ fn expire_and_unassign() {
 		assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(0), 0));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee: 10,
@@ -1065,7 +1068,7 @@ fn extend_expiry() {
 		assert_ok!(Bounties::extend_bounty_expiry(RuntimeOrigin::signed(4), 0, Vec::new()));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee: 10,
@@ -1079,7 +1082,7 @@ fn extend_expiry() {
 		assert_ok!(Bounties::extend_bounty_expiry(RuntimeOrigin::signed(4), 0, Vec::new()));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee: 10,
@@ -1190,7 +1193,7 @@ fn unassign_curator_self() {
 		assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(1), 0));
 
 		assert_eq!(
-			Bounties::bounties(0).unwrap(),
+			pallet_bounties::Bounties::<Test>::get(0).unwrap(),
 			Bounty {
 				proposer: 0,
 				fee: 10,

diff --git a/substrate/frame/child-bounties/src/benchmarking.rs b/substrate/frame/child-bounties/src/benchmarking.rs
@@ -109,7 +109,7 @@ fn activate_bounty<T: Config>(
 		child_bounty_setup.reason.clone(),
 	)?;
 
-	child_bounty_setup.bounty_id = Bounties::<T>::bounty_count() - 1;
+	child_bounty_setup.bounty_id = pallet_bounties::BountyCount::<T>::get() - 1;
 
 	let approve_origin =
 		T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;

diff --git a/substrate/frame/child-bounties/src/lib.rs b/substrate/frame/child-bounties/src/lib.rs
@@ -181,19 +181,16 @@ pub mod pallet {
 
 	/// Number of total child bounties.
 	#[pallet::storage]
-	#[pallet::getter(fn child_bounty_count)]
 	pub type ChildBountyCount<T: Config> = StorageValue<_, BountyIndex, ValueQuery>;
 
 	/// Number of child bounties per parent bounty.
 	/// Map of parent bounty index to number of child bounties.
 	#[pallet::storage]
-	#[pallet::getter(fn parent_child_bounties)]
 	pub type ParentChildBounties<T: Config> =
 		StorageMap<_, Twox64Concat, BountyIndex, u32, ValueQuery>;
 
 	/// Child bounties that have been added.
 	#[pallet::storage]
-	#[pallet::getter(fn child_bounties)]
 	pub type ChildBounties<T: Config> = StorageDoubleMap<
 		_,
 		Twox64Concat,
@@ -205,13 +202,11 @@ pub mod pallet {
 
 	/// The description of each child-bounty.
 	#[pallet::storage]
-	#[pallet::getter(fn child_bounty_descriptions)]
 	pub type ChildBountyDescriptions<T: Config> =
 		StorageMap<_, Twox64Concat, BountyIndex, BoundedVec<u8, T::MaximumReasonLength>>;
 
 	/// The cumulative child-bounty curator fee for each parent bounty.
 	#[pallet::storage]
-	#[pallet::getter(fn children_curator_fees)]
 	pub type ChildrenCuratorFees<T: Config> =
 		StorageMap<_, Twox64Concat, BountyIndex, BalanceOf<T>, ValueQuery>;
 
@@ -251,7 +246,7 @@ pub mod pallet {
 				description.try_into().map_err(|_| BountiesError::<T>::ReasonTooBig)?;
 			ensure!(value >= T::ChildBountyValueMinimum::get(), BountiesError::<T>::InvalidValue);
 			ensure!(
-				Self::parent_child_bounties(parent_bounty_id) <=
+				ParentChildBounties::<T>::get(parent_bounty_id) <=
 					T::MaxActiveChildBountyCount::get() as u32,
 				Error::<T>::TooManyChildBounties,
 			);
@@ -276,15 +271,15 @@ pub mod pallet {
 			)?;
 
 			// Get child-bounty ID.
-			let child_bounty_id = Self::child_bounty_count();
+			let child_bounty_id = ChildBountyCount::<T>::get();
 			let child_bounty_account = Self::child_bounty_account_id(child_bounty_id);
 
 			// Transfer funds from parent bounty to child-bounty.
 			T::Currency::transfer(&parent_bounty_account, &child_bounty_account, value, KeepAlive)?;
 
 			// Increment the active child-bounty count.
-			<ParentChildBounties<T>>::mutate(parent_bounty_id, |count| count.saturating_inc());
-			<ChildBountyCount<T>>::put(child_bounty_id.saturating_add(1));
+			ParentChildBounties::<T>::mutate(parent_bounty_id, |count| count.saturating_inc());
+			ChildBountyCount::<T>::put(child_bounty_id.saturating_add(1));
 
 			// Create child-bounty instance.
 			Self::create_child_bounty(
@@ -710,12 +705,12 @@ pub mod pallet {
 						});
 
 						// Update the active child-bounty tracking count.
-						<ParentChildBounties<T>>::mutate(parent_bounty_id, |count| {
+						ParentChildBounties::<T>::mutate(parent_bounty_id, |count| {
 							count.saturating_dec()
 						});
 
 						// Remove the child-bounty description.
-						<ChildBountyDescriptions<T>>::remove(child_bounty_id);
+						ChildBountyDescriptions::<T>::remove(child_bounty_id);
 
 						// Remove the child-bounty instance from the state.
 						*maybe_child_bounty = None;
@@ -817,7 +812,7 @@ impl<T: Config> Pallet<T> {
 	fn ensure_bounty_active(
 		bounty_id: BountyIndex,
 	) -> Result<(T::AccountId, BlockNumberFor<T>), DispatchError> {
-		let parent_bounty = pallet_bounties::Pallet::<T>::bounties(bounty_id)
+		let parent_bounty = pallet_bounties::Bounties::<T>::get(bounty_id)
 			.ok_or(BountiesError::<T>::InvalidIndex)?;
 		if let BountyStatus::Active { curator, update_due } = parent_bounty.get_status() {
 			Ok((curator, update_due))
@@ -862,7 +857,7 @@ impl<T: Config> Pallet<T> {
 				ChildrenCuratorFees::<T>::mutate(parent_bounty_id, |value| {
 					*value = value.saturating_sub(child_bounty.fee)
 				});
-				<ParentChildBounties<T>>::mutate(parent_bounty_id, |count| {
+				ParentChildBounties::<T>::mutate(parent_bounty_id, |count| {
 					*count = count.saturating_sub(1)
 				});
 
@@ -880,7 +875,7 @@ impl<T: Config> Pallet<T> {
 				debug_assert!(transfer_result.is_ok());
 
 				// Remove the child-bounty description.
-				<ChildBountyDescriptions<T>>::remove(child_bounty_id);
+				ChildBountyDescriptions::<T>::remove(child_bounty_id);
 
 				*maybe_child_bounty = None;
 
@@ -901,14 +896,14 @@ impl<T: Config> pallet_bounties::ChildBountyManager<BalanceOf<T>> for Pallet<T>
 	fn child_bounties_count(
 		bounty_id: pallet_bounties::BountyIndex,
 	) -> pallet_bounties::BountyIndex {
-		Self::parent_child_bounties(bounty_id)
+		ParentChildBounties::<T>::get(bounty_id)
 	}
 
 	fn children_curator_fees(bounty_id: pallet_bounties::BountyIndex) -> BalanceOf<T> {
 		// This is asked for when the parent bounty is being claimed. No use of
 		// keeping it in state after that. Hence removing.
-		let children_fee_total = Self::children_curator_fees(bounty_id);
-		<ChildrenCuratorFees<T>>::remove(bounty_id);
+		let children_fee_total = ChildrenCuratorFees::<T>::get(bounty_id);
+		ChildrenCuratorFees::<T>::remove(bounty_id);
 		children_fee_total
 	}
 }