Removed pallet::getter usage from Beefy and MMR pallets (#3740)

Part of #3326 cc @kianenigma @ggwpez @liamaharon polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp --------- Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
paritytech · Mar 19, 2024 · 817870e · 817870e
1 parent abd3f0c
commit 817870e
Show file tree

Hide file tree

Showing 12 changed files with 81 additions and 72 deletions.
diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs
@@ -1991,7 +1991,7 @@ sp_api::impl_runtime_apis! {
 	#[api_version(3)]
 	impl beefy_primitives::BeefyApi<Block, BeefyId> for Runtime {
 		fn beefy_genesis() -> Option<BlockNumber> {
-			Beefy::genesis_block()
+			pallet_beefy::GenesisBlock::<Runtime>::get()
 		}
 
 		fn validator_set() -> Option<beefy_primitives::ValidatorSet<BeefyId>> {
@@ -2029,11 +2029,11 @@ sp_api::impl_runtime_apis! {
 	#[api_version(2)]
 	impl mmr::MmrApi<Block, mmr::Hash, BlockNumber> for Runtime {
 		fn mmr_root() -> Result<mmr::Hash, mmr::Error> {
-			Ok(Mmr::mmr_root())
+			Ok(pallet_mmr::RootHash::<Runtime>::get())
 		}
 
 		fn mmr_leaf_count() -> Result<mmr::LeafIndex, mmr::Error> {
-			Ok(Mmr::mmr_leaves())
+			Ok(pallet_mmr::NumberOfLeaves::<Runtime>::get())
 		}
 
 		fn generate_proof(

diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs
@@ -2015,7 +2015,7 @@ sp_api::impl_runtime_apis! {
 
 	impl beefy_primitives::BeefyApi<Block, BeefyId> for Runtime {
 		fn beefy_genesis() -> Option<BlockNumber> {
-			Beefy::genesis_block()
+			pallet_beefy::GenesisBlock::<Runtime>::get()
 		}
 
 		fn validator_set() -> Option<beefy_primitives::ValidatorSet<BeefyId>> {
@@ -2052,11 +2052,11 @@ sp_api::impl_runtime_apis! {
 
 	impl mmr::MmrApi<Block, Hash, BlockNumber> for Runtime {
 		fn mmr_root() -> Result<mmr::Hash, mmr::Error> {
-			Ok(Mmr::mmr_root())
+			Ok(pallet_mmr::RootHash::<Runtime>::get())
 		}
 
 		fn mmr_leaf_count() -> Result<mmr::LeafIndex, mmr::Error> {
-			Ok(Mmr::mmr_leaves())
+			Ok(pallet_mmr::NumberOfLeaves::<Runtime>::get())
 		}
 
 		fn generate_proof(

diff --git a/prdoc/pr_3740.prdoc b/prdoc/pr_3740.prdoc
@@ -0,0 +1,18 @@
+# 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 Beefy and MMR pallets
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      This PR removes `pallet::getter` usage from `pallet-beefy`, `pallet-beefy-mmr` and `pallet-mmr`, and updates dependant code and runtimes accordingly.
+      The syntax `StorageItem::<T, I>::get()` should be used instead.
+
+crates: 
+  - name: pallet-beefy
+  - name: pallet-beefy-mmr
+  - name: pallet-mmr
+  - name: kitchensink-runtime
+  - name: rococo-runtime
+  - name: westend-runtime
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
@@ -2979,7 +2979,7 @@ impl_runtime_apis! {
 	#[api_version(3)]
 	impl sp_consensus_beefy::BeefyApi<Block, BeefyId> for Runtime {
 		fn beefy_genesis() -> Option<BlockNumber> {
-			Beefy::genesis_block()
+			pallet_beefy::GenesisBlock::<Runtime>::get()
 		}
 
 		fn validator_set() -> Option<sp_consensus_beefy::ValidatorSet<BeefyId>> {
@@ -3018,11 +3018,11 @@ impl_runtime_apis! {
 		BlockNumber,
 	> for Runtime {
 		fn mmr_root() -> Result<mmr::Hash, mmr::Error> {
-			Ok(Mmr::mmr_root())
+			Ok(pallet_mmr::RootHash::<Runtime>::get())
 		}
 
 		fn mmr_leaf_count() -> Result<mmr::LeafIndex, mmr::Error> {
-			Ok(Mmr::mmr_leaves())
+			Ok(pallet_mmr::NumberOfLeaves::<Runtime>::get())
 		}
 
 		fn generate_proof(

diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs
@@ -68,7 +68,7 @@ where
 				<T as pallet_beefy::Config>::BeefyId,
 			>::MmrRoot(*root)),
 		);
-		<frame_system::Pallet<T>>::deposit_log(digest);
+		frame_system::Pallet::<T>::deposit_log(digest);
 	}
 }
 
@@ -126,15 +126,13 @@ pub mod pallet {
 
 	/// Details of current BEEFY authority set.
 	#[pallet::storage]
-	#[pallet::getter(fn beefy_authorities)]
 	pub type BeefyAuthorities<T: Config> =
 		StorageValue<_, BeefyAuthoritySet<MerkleRootOf<T>>, ValueQuery>;
 
 	/// Details of next BEEFY authority set.
 	///
 	/// This storage entry is used as cache for calls to `update_beefy_next_authority_set`.
 	#[pallet::storage]
-	#[pallet::getter(fn beefy_next_authorities)]
 	pub type BeefyNextAuthorities<T: Config> =
 		StorageValue<_, BeefyNextAuthoritySet<MerkleRootOf<T>>, ValueQuery>;
 }
@@ -152,7 +150,7 @@ impl<T: Config> LeafDataProvider for Pallet<T> {
 			version: T::LeafVersion::get(),
 			parent_number_and_hash: ParentNumberAndHash::<T>::leaf_data(),
 			leaf_extra: T::BeefyDataProvider::extra_data(),
-			beefy_next_authority_set: Pallet::<T>::beefy_next_authorities(),
+			beefy_next_authority_set: BeefyNextAuthorities::<T>::get(),
 		}
 	}
 }
@@ -177,12 +175,12 @@ where
 impl<T: Config> Pallet<T> {
 	/// Return the currently active BEEFY authority set proof.
 	pub fn authority_set_proof() -> BeefyAuthoritySet<MerkleRootOf<T>> {
-		Pallet::<T>::beefy_authorities()
+		BeefyAuthorities::<T>::get()
 	}
 
 	/// Return the next/queued BEEFY authority set proof.
 	pub fn next_authority_set_proof() -> BeefyNextAuthoritySet<MerkleRootOf<T>> {
-		Pallet::<T>::beefy_next_authorities()
+		BeefyNextAuthorities::<T>::get()
 	}
 
 	/// Returns details of a BEEFY authority set.

diff --git a/substrate/frame/beefy-mmr/src/tests.rs b/substrate/frame/beefy-mmr/src/tests.rs
@@ -107,7 +107,7 @@ fn should_contain_valid_leaf_data() {
 	let mut ext = new_test_ext(vec![1, 2, 3, 4]);
 	let parent_hash = ext.execute_with(|| {
 		init_block(1);
-		<frame_system::Pallet<Test>>::parent_hash()
+		frame_system::Pallet::<Test>::parent_hash()
 	});
 
 	let mmr_leaf = read_mmr_leaf(&mut ext, node_offchain_key(0, parent_hash));
@@ -132,7 +132,7 @@ fn should_contain_valid_leaf_data() {
 	// build second block on top
 	let parent_hash = ext.execute_with(|| {
 		init_block(2);
-		<frame_system::Pallet<Test>>::parent_hash()
+		frame_system::Pallet::<Test>::parent_hash()
 	});
 
 	let mmr_leaf = read_mmr_leaf(&mut ext, node_offchain_key(1, parent_hash));

diff --git a/substrate/frame/beefy/src/equivocation.rs b/substrate/frame/beefy/src/equivocation.rs
@@ -190,7 +190,7 @@ where
 		evidence: EquivocationEvidenceFor<T>,
 	) -> Result<(), DispatchError> {
 		let (equivocation_proof, key_owner_proof) = evidence;
-		let reporter = reporter.or_else(|| <pallet_authorship::Pallet<T>>::author());
+		let reporter = reporter.or_else(|| pallet_authorship::Pallet::<T>::author());
 		let offender = equivocation_proof.offender_id().clone();
 
 		// We check the equivocation within the context of its set id (and

diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs
@@ -120,20 +120,17 @@ pub mod pallet {
 
 	/// The current authorities set
 	#[pallet::storage]
-	#[pallet::getter(fn authorities)]
-	pub(super) type Authorities<T: Config> =
+	pub type Authorities<T: Config> =
 		StorageValue<_, BoundedVec<T::BeefyId, T::MaxAuthorities>, ValueQuery>;
 
 	/// The current validator set id
 	#[pallet::storage]
-	#[pallet::getter(fn validator_set_id)]
-	pub(super) type ValidatorSetId<T: Config> =
+	pub type ValidatorSetId<T: Config> =
 		StorageValue<_, sp_consensus_beefy::ValidatorSetId, ValueQuery>;
 
 	/// Authorities set scheduled to be used with the next session
 	#[pallet::storage]
-	#[pallet::getter(fn next_authorities)]
-	pub(super) type NextAuthorities<T: Config> =
+	pub type NextAuthorities<T: Config> =
 		StorageValue<_, BoundedVec<T::BeefyId, T::MaxAuthorities>, ValueQuery>;
 
 	/// A mapping from BEEFY set ID to the index of the *most recent* session for which its
@@ -147,17 +144,14 @@ pub mod pallet {
 	///
 	/// TWOX-NOTE: `ValidatorSetId` is not under user control.
 	#[pallet::storage]
-	#[pallet::getter(fn session_for_set)]
-	pub(super) type SetIdSession<T: Config> =
+	pub type SetIdSession<T: Config> =
 		StorageMap<_, Twox64Concat, sp_consensus_beefy::ValidatorSetId, SessionIndex>;
 
 	/// Block number where BEEFY consensus is enabled/started.
 	/// By changing this (through privileged `set_new_genesis()`), BEEFY consensus is effectively
 	/// restarted from the newly set block number.
 	#[pallet::storage]
-	#[pallet::getter(fn genesis_block)]
-	pub(super) type GenesisBlock<T: Config> =
-		StorageValue<_, Option<BlockNumberFor<T>>, ValueQuery>;
+	pub type GenesisBlock<T: Config> = StorageValue<_, Option<BlockNumberFor<T>>, ValueQuery>;
 
 	#[pallet::genesis_config]
 	pub struct GenesisConfig<T: Config> {
@@ -186,7 +180,7 @@ pub mod pallet {
 				// we panic here as runtime maintainers can simply reconfigure genesis and restart
 				// the chain easily
 				.expect("Authorities vec too big");
-			<GenesisBlock<T>>::put(&self.genesis_block);
+			GenesisBlock::<T>::put(&self.genesis_block);
 		}
 	}
 
@@ -303,8 +297,8 @@ pub mod pallet {
 impl<T: Config> Pallet<T> {
 	/// Return the current active BEEFY validator set.
 	pub fn validator_set() -> Option<ValidatorSet<T::BeefyId>> {
-		let validators: BoundedVec<T::BeefyId, T::MaxAuthorities> = Self::authorities();
-		let id: sp_consensus_beefy::ValidatorSetId = Self::validator_set_id();
+		let validators: BoundedVec<T::BeefyId, T::MaxAuthorities> = Authorities::<T>::get();
+		let id: sp_consensus_beefy::ValidatorSetId = ValidatorSetId::<T>::get();
 		ValidatorSet::<T::BeefyId>::new(validators, id)
 	}
 
@@ -326,19 +320,19 @@ impl<T: Config> Pallet<T> {
 		new: BoundedVec<T::BeefyId, T::MaxAuthorities>,
 		queued: BoundedVec<T::BeefyId, T::MaxAuthorities>,
 	) {
-		<Authorities<T>>::put(&new);
+		Authorities::<T>::put(&new);
 
-		let new_id = Self::validator_set_id() + 1u64;
-		<ValidatorSetId<T>>::put(new_id);
+		let new_id = ValidatorSetId::<T>::get() + 1u64;
+		ValidatorSetId::<T>::put(new_id);
 
-		<NextAuthorities<T>>::put(&queued);
+		NextAuthorities::<T>::put(&queued);
 
 		if let Some(validator_set) = ValidatorSet::<T::BeefyId>::new(new, new_id) {
 			let log = DigestItem::Consensus(
 				BEEFY_ENGINE_ID,
 				ConsensusLog::AuthoritiesChange(validator_set.clone()).encode(),
 			);
-			<frame_system::Pallet<T>>::deposit_log(log);
+			frame_system::Pallet::<T>::deposit_log(log);
 
 			let next_id = new_id + 1;
 			if let Some(next_validator_set) = ValidatorSet::<T::BeefyId>::new(queued, next_id) {
@@ -355,7 +349,7 @@ impl<T: Config> Pallet<T> {
 			return Ok(())
 		}
 
-		if !<Authorities<T>>::get().is_empty() {
+		if !Authorities::<T>::get().is_empty() {
 			return Err(())
 		}
 
@@ -364,10 +358,10 @@ impl<T: Config> Pallet<T> {
 				.map_err(|_| ())?;
 
 		let id = GENESIS_AUTHORITY_SET_ID;
-		<Authorities<T>>::put(bounded_authorities);
-		<ValidatorSetId<T>>::put(id);
+		Authorities::<T>::put(bounded_authorities);
+		ValidatorSetId::<T>::put(id);
 		// Like `pallet_session`, initialize the next validator set as well.
-		<NextAuthorities<T>>::put(bounded_authorities);
+		NextAuthorities::<T>::put(bounded_authorities);
 
 		if let Some(validator_set) = ValidatorSet::<T::BeefyId>::new(authorities.clone(), id) {
 			let next_id = id + 1;
@@ -442,9 +436,9 @@ where
 		// We want to have at least one BEEFY mandatory block per session.
 		Self::change_authorities(bounded_next_authorities, bounded_next_queued_authorities);
 
-		let validator_set_id = Self::validator_set_id();
+		let validator_set_id = ValidatorSetId::<T>::get();
 		// Update the mapping for the new set id that corresponds to the latest session (i.e. now).
-		let session_index = <pallet_session::Pallet<T>>::current_index();
+		let session_index = pallet_session::Pallet::<T>::current_index();
 		SetIdSession::<T>::insert(validator_set_id, &session_index);
 		// Prune old entry if limit reached.
 		let max_set_id_session_entries = T::MaxSetIdSessionEntries::get().max(1);
@@ -459,13 +453,13 @@ where
 			ConsensusLog::<T::BeefyId>::OnDisabled(i as AuthorityIndex).encode(),
 		);
 
-		<frame_system::Pallet<T>>::deposit_log(log);
+		frame_system::Pallet::<T>::deposit_log(log);
 	}
 }
 
 impl<T: Config> IsMember<T::BeefyId> for Pallet<T> {
 	fn is_member(authority_id: &T::BeefyId) -> bool {
-		Self::authorities().iter().any(|id| id == authority_id)
+		Authorities::<T>::get().iter().any(|id| id == authority_id)
 	}
 }