Removed pallet::getter usage from pallet-collective (#3456)

Part of #3326 This one is easier as all the storage items are public. @ggwpez @kianenigma @shawntabrizi --------- Signed-off-by: Matteo Muraca <[email protected]> Co-authored-by: Liam Aharon <[email protected]> Co-authored-by: command-bot <> Co-authored-by: Kian Paimani <[email protected]>
paritytech · Mar 5, 2024 · a71f018 · a71f018
1 parent 3ff78a7
commit a71f018
Show file tree

Hide file tree

Showing 8 changed files with 94 additions and 85 deletions.
diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/impls.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/impls.rs
@@ -81,7 +81,7 @@ where
 	}
 
 	fn proposal_of(proposal_hash: HashOf<T>) -> Option<ProposalOf<T, I>> {
-		pallet_collective::Pallet::<T, I>::proposal_of(proposal_hash)
+		pallet_collective::ProposalOf::<T, I>::get(proposal_hash)
 	}
 }
 

diff --git a/prdoc/pr_3456.prdoc b/prdoc/pr_3456.prdoc
@@ -0,0 +1,15 @@
+# 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-collective`
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      This PR removes `pallet::getter` usage from `pallet-collective`, and updates dependant code accordingly.
+      The syntax `StorageItem::<T, I>::get()` should be used instead.
+
+crates: 
+  - name: pallet-collective
+  - name: kitchensink-runtime
+  - name: collectives-westend-runtime
diff --git a/substrate/bin/node/runtime/src/impls.rs b/substrate/bin/node/runtime/src/impls.rs
@@ -30,8 +30,8 @@ use pallet_identity::legacy::IdentityField;
 use sp_std::prelude::*;
 
 use crate::{
-	AccountId, AllianceMotion, Assets, Authorship, Balances, Hash, NegativeImbalance, Runtime,
-	RuntimeCall,
+	AccountId, AllianceCollective, AllianceMotion, Assets, Authorship, Balances, Hash,
+	NegativeImbalance, Runtime, RuntimeCall,
 };
 
 pub struct Author;
@@ -107,7 +107,7 @@ impl ProposalProvider<AccountId, Hash, RuntimeCall> for AllianceProposalProvider
 	}
 
 	fn proposal_of(proposal_hash: Hash) -> Option<RuntimeCall> {
-		AllianceMotion::proposal_of(proposal_hash)
+		pallet_collective::ProposalOf::<Runtime, AllianceCollective>::get(proposal_hash)
 	}
 }
 
@@ -276,7 +276,7 @@ mod multiplier_tests {
 				let next = runtime_multiplier_update(fm);
 				fm = next;
 				if fm == min_multiplier() {
-					break
+					break;
 				}
 				iterations += 1;
 			}

diff --git a/substrate/frame/alliance/src/mock.rs b/substrate/frame/alliance/src/mock.rs
@@ -208,7 +208,7 @@ impl ProposalProvider<AccountId, H256, RuntimeCall> for AllianceProposalProvider
 	}
 
 	fn proposal_of(proposal_hash: H256) -> Option<RuntimeCall> {
-		AllianceMotion::proposal_of(proposal_hash)
+		pallet_collective::ProposalOf::<Test, Instance1>::get(proposal_hash)
 	}
 }
 

diff --git a/substrate/frame/alliance/src/tests.rs b/substrate/frame/alliance/src/tests.rs
@@ -187,8 +187,8 @@ fn propose_works() {
 			Box::new(proposal.clone()),
 			proposal_len
 		));
-		assert_eq!(*AllianceMotion::proposals(), vec![hash]);
-		assert_eq!(AllianceMotion::proposal_of(&hash), Some(proposal));
+		assert_eq!(*pallet_collective::Proposals::<Test, Instance1>::get(), vec![hash]);
+		assert_eq!(pallet_collective::ProposalOf::<Test, Instance1>::get(&hash), Some(proposal));
 		assert_eq!(
 			System::events(),
 			vec![EventRecord {

diff --git a/substrate/frame/collective/src/benchmarking.rs b/substrate/frame/collective/src/benchmarking.rs
@@ -105,7 +105,7 @@ benchmarks_instance_pallet! {
 	}: _(SystemOrigin::Root, new_members.clone(), new_members.last().cloned(), T::MaxMembers::get())
 	verify {
 		new_members.sort();
-		assert_eq!(Collective::<T, I>::members(), new_members);
+		assert_eq!(Members::<T, I>::get(), new_members);
 	}
 
 	execute {
@@ -199,14 +199,14 @@ benchmarks_instance_pallet! {
 			)?;
 		}
 
-		assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
 
 		let proposal: T::Proposal = SystemCall::<T>::remark { remark: id_to_remark_data(p, b as usize) }.into();
 
 	}: propose(SystemOrigin::Signed(caller.clone()), threshold, Box::new(proposal.clone()), bytes_in_storage)
 	verify {
 		// New proposal is recorded
-		assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), p as usize);
 		let proposal_hash = T::Hashing::hash_of(&proposal);
 		assert_last_event::<T, I>(Event::Proposed { account: caller, proposal_index: p - 1, proposal_hash, threshold }.into());
 	}
@@ -269,7 +269,7 @@ benchmarks_instance_pallet! {
 			approve,
 		)?;
 
-		assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), p as usize);
 
 		// Voter switches vote to nay, but does not kill the vote, just updates + inserts
 		let approve = false;
@@ -280,8 +280,8 @@ benchmarks_instance_pallet! {
 	}: _(SystemOrigin::Signed(voter), last_hash, index, approve)
 	verify {
 		// All proposals exist and the last proposal has just been updated.
-		assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
-		let voting = Collective::<T, I>::voting(&last_hash).ok_or("Proposal Missing")?;
+		assert_eq!(Proposals::<T, I>::get().len(), p as usize);
+		let voting = Voting::<T, I>::get(&last_hash).ok_or("Proposal Missing")?;
 		assert_eq!(voting.ayes.len(), (m - 3) as usize);
 		assert_eq!(voting.nays.len(), 1);
 	}
@@ -344,7 +344,7 @@ benchmarks_instance_pallet! {
 			approve,
 		)?;
 
-		assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), p as usize);
 
 		// Voter switches vote to nay, which kills the vote
 		let approve = false;
@@ -361,7 +361,7 @@ benchmarks_instance_pallet! {
 	}: close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage)
 	verify {
 		// The last proposal is removed.
-		assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
 		assert_last_event::<T, I>(Event::Disapproved { proposal_hash: last_hash }.into());
 	}
 
@@ -428,7 +428,7 @@ benchmarks_instance_pallet! {
 			true,
 		)?;
 
-		assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), p as usize);
 
 		// Caller switches vote to aye, which passes the vote
 		let index = p - 1;
@@ -442,7 +442,7 @@ benchmarks_instance_pallet! {
 	}: close(SystemOrigin::Signed(caller), last_hash, index, Weight::MAX, bytes_in_storage)
 	verify {
 		// The last proposal is removed.
-		assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
 		assert_last_event::<T, I>(Event::Executed { proposal_hash: last_hash, result: Ok(()) }.into());
 	}
 
@@ -519,12 +519,12 @@ benchmarks_instance_pallet! {
 		)?;
 
 		System::<T>::set_block_number(BlockNumberFor::<T>::max_value());
-		assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), p as usize);
 
 		// Prime nay will close it as disapproved
 	}: close(SystemOrigin::Signed(caller), last_hash, index, Weight::MAX, bytes_in_storage)
 	verify {
-		assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
 		assert_last_event::<T, I>(Event::Disapproved { proposal_hash: last_hash }.into());
 	}
 
@@ -591,12 +591,12 @@ benchmarks_instance_pallet! {
 
 		// caller is prime, prime already votes aye by creating the proposal
 		System::<T>::set_block_number(BlockNumberFor::<T>::max_value());
-		assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), p as usize);
 
 		// Prime aye will close it as approved
 	}: close(SystemOrigin::Signed(caller), last_hash, p - 1, Weight::MAX, bytes_in_storage)
 	verify {
-		assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
 		assert_last_event::<T, I>(Event::Executed { proposal_hash: last_hash, result: Ok(()) }.into());
 	}
 
@@ -640,11 +640,11 @@ benchmarks_instance_pallet! {
 		}
 
 		System::<T>::set_block_number(BlockNumberFor::<T>::max_value());
-		assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), p as usize);
 
 	}: _(SystemOrigin::Root, last_hash)
 	verify {
-		assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
+		assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
 		assert_last_event::<T, I>(Event::Disapproved { proposal_hash: last_hash }.into());
 	}