Skip to content

Commit

Permalink
remove getter from babe pallet (paritytech#4912)
Browse files Browse the repository at this point in the history
### ISSUE
Link to the issue:
paritytech#3326
cc @muraca 

Deliverables
 - [Deprecation] remove pallet::getter usage from all pallet-babe

### Test Outcomes
___
Successful tests by running `cargo test -p pallet-babe --features
runtime-benchmarks`


running 32 tests
test
mock::__pallet_staking_reward_curve_test_module::reward_curve_piece_count
... ok
test mock::__construct_runtime_integrity_test::runtime_integrity_tests
... ok
test mock::test_genesis_config_builds ... ok
2024-06-28T17:02:11.158812Z ERROR runtime::storage: Corrupted state at
`0x1cb6f36e027abb2091cfb5110ab5087f9aab0a5b63b359512deee557c9f4cf63`:
Error { cause: Some(Error { cause: None, desc: "Could not decode
`NextConfigDescriptor`, variant doesn't exist" }), desc: "Could not
decode `Option::Some(T)`" }
2024-06-28T17:02:11.159752Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::add_epoch_configurations_migration_works ... ok
test tests::author_vrf_output_for_secondary_vrf ... ok
test benchmarking::bench_check_equivocation_proof ... ok
2024-06-28T17:02:11.160537Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::can_estimate_current_epoch_progress ... ok
test tests::author_vrf_output_for_primary ... ok
test tests::authority_index ... ok
2024-06-28T17:02:11.162327Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::empty_randomness_is_correct ... ok
test tests::check_module ... ok
2024-06-28T17:02:11.163492Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::current_slot_is_processed_on_initialization ... ok
test tests::can_enact_next_config ... ok
2024-06-28T17:02:11.164987Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.165007Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::can_predict_next_epoch_change ... ok
test tests::first_block_epoch_zero_start ... ok
test tests::initial_values ... ok
2024-06-28T17:02:11.168430Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.168685Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.170982Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.171220Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::only_root_can_enact_config_change ... ok
test tests::no_author_vrf_output_for_secondary_plain ... ok
test tests::can_fetch_current_and_next_epoch_data ... ok
2024-06-28T17:02:11.172960Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::report_equivocation_has_valid_weight ... ok
2024-06-28T17:02:11.173873Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.177084Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::report_equivocation_after_skipped_epochs_works ...
2024-06-28T17:02:11.177694Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.177703Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.177925Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.177927Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
ok
2024-06-28T17:02:11.179678Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.181446Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.183665Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.183874Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.185732Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.185951Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.189332Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.189559Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.189587Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::generate_equivocation_report_blob ... ok
test tests::disabled_validators_cannot_author_blocks - should panic ...
ok
2024-06-28T17:02:11.190552Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.192279Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.194735Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.196136Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.197240Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::skipping_over_epochs_works ... ok
2024-06-28T17:02:11.202783Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.202846Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.203029Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.205242Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::tracks_block_numbers_when_current_and_previous_epoch_started
... ok
2024-06-28T17:02:11.208965Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::report_equivocation_current_session_works ... ok
test tests::report_equivocation_invalid_key_owner_proof ... ok
2024-06-28T17:02:11.216431Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
2024-06-28T17:02:11.216855Z ERROR runtime::timestamp:
`pallet_timestamp::UnixTime::now` is called at genesis, invalid value
returned: 0
test tests::report_equivocation_validate_unsigned_prevents_duplicates
... ok
test tests::report_equivocation_invalid_equivocation_proof ... ok
test tests::valid_equivocation_reports_dont_pay_fees ... ok
test tests::report_equivocation_old_session_works ... ok
test
mock::__pallet_staking_reward_curve_test_module::reward_curve_precision
... ok

test result: ok. 32 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
out; finished in 0.20s

   Doc-tests pallet-babe

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
out; finished in 0.00s

---

Polkadot Address: 16htXkeVhfroBhL6nuqiwknfXKcT6WadJPZqEi2jRf9z4XPY
  • Loading branch information
Aideepakchaudhary authored and TarekkMA committed Aug 2, 2024
1 parent ce1783b commit 81055f3
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 72 deletions.
15 changes: 15 additions & 0 deletions prdoc/pr_4912.prdoc
Original file line number Diff line number Diff line change
@@ -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 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()`.
When accessed outside the pallet, use the public functions of storage.

crates:
- name: pallet-babe
bump: minor
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

0 comments on commit 81055f3

Please sign in to comment.