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

remove getter from babe pallet #4912

Merged
Show file tree
Hide file tree
Changes from 9 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
14 changes: 14 additions & 0 deletions prdoc/pr_4912.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# 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 the pallet-babe

doc:
- audience: Runtime Dev
description: |
This PR removed `pallet::getter`s from `pallet-babe`s storage items.
When accessed inside the pallet, use the syntax `StorageItem::<T>::get()`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update this please?
It just uses less FRAME macro magic now as it seems.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, its probably minor because of the type that is being made public. Then please ignore my other comments about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thank you


crates:
- name: pallet-babe
bump: minor
Aideepakchaudhary marked this conversation as resolved.
Show resolved Hide resolved
99 changes: 65 additions & 34 deletions substrate/frame/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ pub struct SameAuthoritiesForever;

impl EpochChangeTrigger for SameAuthoritiesForever {
fn trigger<T: Config>(now: BlockNumberFor<T>) {
if <Pallet<T>>::should_epoch_change(now) {
let authorities = <Pallet<T>>::authorities();
if Pallet::<T>::should_epoch_change(now) {
let authorities = Authorities::<T>::get();
let next_authorities = authorities.clone();

<Pallet<T>>::enact_epoch_change(authorities, next_authorities, None);
Pallet::<T>::enact_epoch_change(authorities, next_authorities, None);
}
}
}
Expand Down Expand Up @@ -185,12 +185,10 @@ pub mod pallet {

/// Current epoch index.
#[pallet::storage]
#[pallet::getter(fn epoch_index)]
pub type EpochIndex<T> = StorageValue<_, u64, ValueQuery>;

/// Current epoch authorities.
#[pallet::storage]
#[pallet::getter(fn authorities)]
pub type Authorities<T: Config> = StorageValue<
_,
WeakBoundedVec<(AuthorityId, BabeAuthorityWeight), T::MaxAuthorities>,
Expand All @@ -200,12 +198,10 @@ pub mod pallet {
/// The slot at which the first epoch actually started. This is 0
/// until the first block of the chain.
#[pallet::storage]
#[pallet::getter(fn genesis_slot)]
pub type GenesisSlot<T> = StorageValue<_, Slot, ValueQuery>;

/// Current slot number.
#[pallet::storage]
#[pallet::getter(fn current_slot)]
pub type CurrentSlot<T> = StorageValue<_, Slot, ValueQuery>;

/// The epoch randomness for the *current* epoch.
Expand All @@ -222,20 +218,19 @@ pub mod pallet {
// array size because the metadata API currently doesn't resolve the
// variable to its underlying value.
#[pallet::storage]
#[pallet::getter(fn randomness)]
pub type Randomness<T> = StorageValue<_, BabeRandomness, ValueQuery>;

/// Pending epoch configuration change that will be applied when the next epoch is enacted.
#[pallet::storage]
pub(super) type PendingEpochConfigChange<T> = StorageValue<_, NextConfigDescriptor>;
pub type PendingEpochConfigChange<T> = StorageValue<_, NextConfigDescriptor>;

/// Next epoch randomness.
#[pallet::storage]
pub(super) type NextRandomness<T> = StorageValue<_, BabeRandomness, ValueQuery>;
pub type NextRandomness<T> = StorageValue<_, BabeRandomness, ValueQuery>;

/// Next epoch authorities.
#[pallet::storage]
pub(super) type NextAuthorities<T: Config> = StorageValue<
pub type NextAuthorities<T: Config> = StorageValue<
_,
WeakBoundedVec<(AuthorityId, BabeAuthorityWeight), T::MaxAuthorities>,
ValueQuery,
Expand All @@ -251,11 +246,11 @@ pub mod pallet {
/// We reset all segments and return to `0` at the beginning of every
/// epoch.
#[pallet::storage]
pub(super) type SegmentIndex<T> = StorageValue<_, u32, ValueQuery>;
pub type SegmentIndex<T> = StorageValue<_, u32, ValueQuery>;

/// TWOX-NOTE: `SegmentIndex` is an increasing integer, so this is okay.
#[pallet::storage]
pub(super) type UnderConstruction<T: Config> = StorageMap<
pub type UnderConstruction<T: Config> = StorageMap<
_,
Twox64Concat,
u32,
Expand All @@ -266,16 +261,14 @@ pub mod pallet {
/// Temporary value (cleared at block finalization) which is `Some`
/// if per-block initialization has already been called for current block.
#[pallet::storage]
#[pallet::getter(fn initialized)]
pub(super) type Initialized<T> = StorageValue<_, Option<PreDigest>>;
pub type Initialized<T> = StorageValue<_, Option<PreDigest>>;

/// This field should always be populated during block processing unless
/// secondary plain slots are enabled (which don't contain a VRF output).
///
/// It is set in `on_finalize`, before it will contain the value from the last block.
#[pallet::storage]
#[pallet::getter(fn author_vrf_randomness)]
pub(super) type AuthorVrfRandomness<T> = StorageValue<_, Option<BabeRandomness>, ValueQuery>;
pub type AuthorVrfRandomness<T> = StorageValue<_, Option<BabeRandomness>, ValueQuery>;

/// The block numbers when the last and current epoch have started, respectively `N-1` and
/// `N`.
Expand All @@ -292,19 +285,17 @@ pub mod pallet {
/// on block finalization. Querying this storage entry outside of block
/// execution context should always yield zero.
#[pallet::storage]
#[pallet::getter(fn lateness)]
pub(super) type Lateness<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
pub type Lateness<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;

/// The configuration for the current epoch. Should never be `None` as it is initialized in
/// genesis.
#[pallet::storage]
#[pallet::getter(fn epoch_config)]
pub(super) type EpochConfig<T> = StorageValue<_, BabeEpochConfiguration>;
pub type EpochConfig<T> = StorageValue<_, BabeEpochConfiguration>;

/// The configuration for the next epoch, `None` if the config will not change
/// (you can fallback to `EpochConfig` instead in that case).
#[pallet::storage]
pub(super) type NextEpochConfig<T> = StorageValue<_, BabeEpochConfiguration>;
pub type NextEpochConfig<T> = StorageValue<_, BabeEpochConfiguration>;

/// A list of the last 100 skipped epochs and the corresponding session index
/// when the epoch was skipped.
Expand All @@ -315,8 +306,7 @@ pub mod pallet {
/// a validator was the owner of a given key on a given session, and what the
/// active epoch index was during that session.
#[pallet::storage]
#[pallet::getter(fn skipped_epochs)]
pub(super) type SkippedEpochs<T> =
pub type SkippedEpochs<T> =
StorageValue<_, BoundedVec<(u64, SessionIndex), ConstU32<100>>, ValueQuery>;

#[derive(frame_support::DefaultNoBound)]
Expand Down Expand Up @@ -368,7 +358,7 @@ pub mod pallet {
.and_then(|(authority, _)| {
let public = authority.as_inner_ref();
let transcript = sp_consensus_babe::make_vrf_transcript(
&Self::randomness(),
&Randomness::<T>::get(),
CurrentSlot::<T>::get(),
EpochIndex::<T>::get(),
);
Expand Down Expand Up @@ -510,7 +500,7 @@ impl<T: Config> FindAuthor<u32> for Pallet<T> {

impl<T: Config> IsMember<AuthorityId> for Pallet<T> {
fn is_member(authority_id: &AuthorityId) -> bool {
<Pallet<T>>::authorities().iter().any(|id| &id.0 == authority_id)
Authorities::<T>::get().iter().any(|id| &id.0 == authority_id)
}
}

Expand All @@ -526,6 +516,47 @@ impl<T: Config> pallet_session::ShouldEndSession<BlockNumberFor<T>> for Pallet<T
}

impl<T: Config> Pallet<T> {
/// Public function to access epoch_index storage.
pub fn epoch_index() -> u64 {
EpochIndex::<T>::get()
}
/// Public function to access authorities storage.
pub fn authorities() -> WeakBoundedVec<(AuthorityId, BabeAuthorityWeight), T::MaxAuthorities> {
Authorities::<T>::get()
}
/// Public function to access genesis_slot storage.
pub fn genesis_slot() -> Slot {
GenesisSlot::<T>::get()
}
/// Public function to access current_slot storage.
pub fn current_slot() -> Slot {
CurrentSlot::<T>::get()
}
/// Public function to access randomness storage.
pub fn randomness() -> BabeRandomness {
Randomness::<T>::get()
}
/// Public function to access initialized storage.
pub fn initialized() -> Option<Option<PreDigest>> {
Initialized::<T>::get()
}
/// Public function to access author_vrf_randomness storage.
pub fn author_vrf_randomness() -> Option<BabeRandomness> {
AuthorVrfRandomness::<T>::get()
}
/// Public function to access lateness storage.
pub fn lateness() -> BlockNumberFor<T> {
Lateness::<T>::get()
}
/// Public function to access epoch_config storage.
pub fn epoch_config() -> Option<BabeEpochConfiguration> {
EpochConfig::<T>::get()
}
/// Public function to access skipped_epochs storage.
pub fn skipped_epochs() -> BoundedVec<(u64, SessionIndex), ConstU32<100>> {
SkippedEpochs::<T>::get()
}

/// Determine the BABE slot duration based on the Timestamp module configuration.
pub fn slot_duration() -> T::Moment {
// we double the minimum block-period so each author can always propose within
Expand Down Expand Up @@ -588,7 +619,7 @@ impl<T: Config> Pallet<T> {
) {
// PRECONDITION: caller has done initialization and is guaranteed
// by the session module to be called before this.
debug_assert!(Self::initialized().is_some());
debug_assert!(Initialized::<T>::get().is_some());

if authorities.is_empty() {
log::warn!(target: LOG_TARGET, "Ignoring empty epoch change.");
Expand Down Expand Up @@ -655,7 +686,7 @@ impl<T: Config> Pallet<T> {
NextAuthorities::<T>::put(&next_authorities);

// Update the start blocks of the previous and new current epoch.
<EpochStart<T>>::mutate(|(previous_epoch_start_block, current_epoch_start_block)| {
EpochStart::<T>::mutate(|(previous_epoch_start_block, current_epoch_start_block)| {
*previous_epoch_start_block = sp_std::mem::take(current_epoch_start_block);
*current_epoch_start_block = <frame_system::Pallet<T>>::block_number();
});
Expand Down Expand Up @@ -701,8 +732,8 @@ impl<T: Config> Pallet<T> {
epoch_index: EpochIndex::<T>::get(),
start_slot: Self::current_epoch_start(),
duration: T::EpochDuration::get(),
authorities: Self::authorities().into_inner(),
randomness: Self::randomness(),
authorities: Authorities::<T>::get().into_inner(),
randomness: Randomness::<T>::get(),
config: EpochConfig::<T>::get()
.expect("EpochConfig is initialized in genesis; we never `take` or `kill` it; qed"),
}
Expand Down Expand Up @@ -779,8 +810,8 @@ impl<T: Config> Pallet<T> {
// we use the same values as genesis because we haven't collected any
// randomness yet.
let next = NextEpochDescriptor {
authorities: Self::authorities().into_inner(),
randomness: Self::randomness(),
authorities: Authorities::<T>::get().into_inner(),
randomness: Randomness::<T>::get(),
};

Self::deposit_consensus(ConsensusLog::NextEpochData(next));
Expand All @@ -789,7 +820,7 @@ impl<T: Config> Pallet<T> {
fn initialize(now: BlockNumberFor<T>) {
// since `initialize` can be called twice (e.g. if session module is present)
// let's ensure that we only do the initialization once per block
let initialized = Self::initialized().is_some();
let initialized = Initialized::<T>::get().is_some();
if initialized {
return
}
Expand Down Expand Up @@ -940,7 +971,7 @@ impl<T: Config> frame_support::traits::EstimateNextSessionRotation<BlockNumberFo

impl<T: Config> frame_support::traits::Lateness<BlockNumberFor<T>> for Pallet<T> {
fn lateness(&self) -> BlockNumberFor<T> {
Self::lateness()
Lateness::<T>::get()
}
}

Expand Down
5 changes: 3 additions & 2 deletions substrate/frame/babe/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ pub fn go_to_block(n: u64, s: u64) {

/// Slots will grow accordingly to blocks
pub fn progress_to_block(n: u64) {
let mut slot = u64::from(Babe::current_slot()) + 1;
let mut slot = u64::from(CurrentSlot::<Test>::get()) + 1;
for i in System::block_number() + 1..=n {
go_to_block(i, slot);
slot += 1;
Expand Down Expand Up @@ -272,7 +272,8 @@ pub fn make_vrf_signature_and_randomness(
slot: Slot,
pair: &sp_consensus_babe::AuthorityPair,
) -> (VrfSignature, Randomness) {
let transcript = sp_consensus_babe::make_vrf_transcript(&Babe::randomness(), slot, 0);
let transcript =
sp_consensus_babe::make_vrf_transcript(&pallet_babe::Randomness::<Test>::get(), slot, 0);

let randomness =
pair.as_ref().make_bytes(sp_consensus_babe::RANDOMNESS_VRF_CONTEXT, &transcript);
Expand Down
Loading
Loading