Skip to content

Commit

Permalink
Removed pallet::getter usage from Beefy and MMR pallets (paritytech…
Browse files Browse the repository at this point in the history
…#3740)

Part of paritytech#3326 

cc @kianenigma @ggwpez @liamaharon 

polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

---------

Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
  • Loading branch information
muraca authored and dharjeezy committed Mar 24, 2024
1 parent 80c551a commit f963f95
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 72 deletions.
6 changes: 3 additions & 3 deletions polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Expand Down Expand Up @@ -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(
Expand Down
18 changes: 18 additions & 0 deletions prdoc/pr_3740.prdoc
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 4 additions & 6 deletions substrate/frame/beefy-mmr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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>;
}
Expand All @@ -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(),
}
}
}
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/beefy-mmr/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/beefy/src/equivocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 21 additions & 27 deletions substrate/frame/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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> {
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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)
}

Expand All @@ -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) {
Expand All @@ -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(())
}

Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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)
}
}

Expand Down
Loading

0 comments on commit f963f95

Please sign in to comment.