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 pallet::getter from pallet-staking #6184

Merged
merged 20 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from 16 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
2 changes: 1 addition & 1 deletion polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ impl Get<u32> for MaybeSignedPhase {
fn get() -> u32 {
// 1 day = 4 eras -> 1 week = 28 eras. We want to disable signed phase once a week to test
// the fallback unsigned phase is able to compute elections on Westend.
if Staking::current_era().unwrap_or(1) % 28 == 0 {
if pallet_staking::CurrentEra::<Runtime>::get().unwrap_or(1) % 28 == 0 {
0
} else {
SignedPhase::get()
Expand Down
24 changes: 24 additions & 0 deletions prdoc/pr_6184.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
title: Remove pallet::getter from pallet-staking
doc:
- audience: Runtime Dev
description: |
This PR removes all pallet::getter occurrences from pallet-staking and replaces them with explicit implementations.
It also adds tests to verify that retrieval of affected entities works as expected so via storage::getter.

crates:
- name: pallet-babe
bump: patch
- name: pallet-beefy
bump: patch
- name: pallet-election-provider-multi-phase
bump: patch
- name: pallet-grandpa
bump: patch
- name: pallet-nomination-pools
bump: patch
- name: pallet-root-offences
bump: patch
- name: westend-runtime
bump: patch
- name: pallet-staking
bump: patch
2 changes: 1 addition & 1 deletion substrate/frame/babe/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ pub fn start_session(session_index: SessionIndex) {
/// Progress to the first block at the given era
pub fn start_era(era_index: EraIndex) {
start_session((era_index * 3).into());
assert_eq!(Staking::current_era(), Some(era_index));
assert_eq!(pallet_staking::CurrentEra::<Test>::get(), Some(era_index));
}

pub fn make_primary_pre_digest(
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ fn disabled_validators_cannot_author_blocks() {
// so we should still be able to author blocks
start_era(2);

assert_eq!(Staking::current_era().unwrap(), 2);
assert_eq!(pallet_staking::CurrentEra::<Test>::get().unwrap(), 2);

// let's disable the validator at index 0
Session::disable_index(0);
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/beefy/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,5 +366,5 @@ pub fn start_session(session_index: SessionIndex) {

pub fn start_era(era_index: EraIndex) {
start_session((era_index * 3).into());
assert_eq!(Staking::current_era(), Some(era_index));
assert_eq!(pallet_staking::CurrentEra::<Test>::get(), Some(era_index));
}
4 changes: 2 additions & 2 deletions substrate/frame/beefy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ fn report_equivocation_current_set_works(mut f: impl ReportEquivocationFn) {
let authorities = test_authorities();

ExtBuilder::default().add_authorities(authorities).build_and_execute(|| {
assert_eq!(Staking::current_era(), Some(0));
assert_eq!(pallet_staking::CurrentEra::<Test>::get(), Some(0));
assert_eq!(Session::current_index(), 0);

start_era(1);
Expand Down Expand Up @@ -906,7 +906,7 @@ fn report_fork_voting_invalid_context() {

let mut era = 1;
let block_num = ext.execute_with(|| {
assert_eq!(Staking::current_era(), Some(0));
assert_eq!(pallet_staking::CurrentEra::<Test>::get(), Some(0));
assert_eq!(Session::current_index(), 0);
start_era(era);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn log_current_time() {
"block: {:?}, session: {:?}, era: {:?}, EPM phase: {:?} ts: {:?}",
System::block_number(),
Session::current_index(),
Staking::current_era(),
pallet_staking::CurrentEra::<Runtime>::get(),
CurrentPhase::<Runtime>::get(),
Now::<Runtime>::get()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use pallet_election_provider_multi_phase::{
unsigned::MinerConfig, Call, CurrentPhase, ElectionCompute, GeometricDepositBase,
QueuedSolution, SolutionAccuracyOf,
};
use pallet_staking::StakerStatus;
use pallet_staking::{ActiveEra, CurrentEra, ErasStartSessionIndex, StakerStatus};
use parking_lot::RwLock;
use std::sync::Arc;

Expand Down Expand Up @@ -806,11 +806,11 @@ pub(crate) fn start_active_era(
}

pub(crate) fn active_era() -> EraIndex {
Staking::active_era().unwrap().index
ActiveEra::<Runtime>::get().unwrap().index
}

pub(crate) fn current_era() -> EraIndex {
Staking::current_era().unwrap()
CurrentEra::<Runtime>::get().unwrap()
}

// Fast forward until EPM signed phase.
Expand Down Expand Up @@ -862,11 +862,11 @@ pub(crate) fn on_offence_now(
>],
slash_fraction: &[Perbill],
) {
let now = Staking::active_era().unwrap().index;
let now = ActiveEra::<Runtime>::get().unwrap().index;
let _ = Staking::on_offence(
offenders,
slash_fraction,
Staking::eras_start_session_index(now).unwrap(),
ErasStartSessionIndex::<Runtime>::get(now).unwrap(),
);
}

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/grandpa/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ pub fn start_session(session_index: SessionIndex) {

pub fn start_era(era_index: EraIndex) {
start_session((era_index * 3).into());
assert_eq!(Staking::current_era(), Some(era_index));
assert_eq!(pallet_staking::CurrentEra::<Test>::get(), Some(era_index));
}

pub fn initialize_block(number: u64, parent_hash: H256) {
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/grandpa/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ fn report_equivocation_current_set_works() {
let authorities = test_authorities();

new_test_ext_raw_authorities(authorities).execute_with(|| {
assert_eq!(Staking::current_era(), Some(0));
assert_eq!(pallet_staking::CurrentEra::<Test>::get(), Some(0));
assert_eq!(Session::current_index(), 0);

start_era(1);
Expand Down
18 changes: 9 additions & 9 deletions substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use sp_staking::Agent;
fn pool_lifecycle_e2e() {
new_test_ext().execute_with(|| {
assert_eq!(Balances::minimum_balance(), 5);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 50, 10, 10, 10));
Expand Down Expand Up @@ -204,7 +204,7 @@ fn pool_lifecycle_e2e() {
fn pool_chill_e2e() {
new_test_ext().execute_with(|| {
assert_eq!(Balances::minimum_balance(), 5);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 50, 10, 10, 10));
Expand Down Expand Up @@ -330,7 +330,7 @@ fn pool_slash_e2e() {
new_test_ext().execute_with(|| {
ExistentialDeposit::set(1);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
Expand Down Expand Up @@ -540,7 +540,7 @@ fn pool_slash_proportional() {
ExistentialDeposit::set(1);
BondingDuration::set(28);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
Expand Down Expand Up @@ -758,7 +758,7 @@ fn pool_slash_non_proportional_only_bonded_pool() {
ExistentialDeposit::set(1);
BondingDuration::set(28);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
Expand Down Expand Up @@ -837,7 +837,7 @@ fn pool_slash_non_proportional_bonded_pool_and_chunks() {
ExistentialDeposit::set(1);
BondingDuration::set(28);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
Expand Down Expand Up @@ -914,7 +914,7 @@ fn pool_migration_e2e() {
new_test_ext().execute_with(|| {
LegacyAdapter::set(true);
assert_eq!(Balances::minimum_balance(), 5);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool with TransferStake strategy.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 50, 10, 10, 10));
Expand Down Expand Up @@ -1192,7 +1192,7 @@ fn disable_pool_operations_on_non_migrated() {
new_test_ext().execute_with(|| {
LegacyAdapter::set(true);
assert_eq!(Balances::minimum_balance(), 5);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool with TransferStake strategy.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 50, 10, 10, 10));
Expand Down Expand Up @@ -1369,7 +1369,7 @@ fn pool_no_dangling_delegation() {
new_test_ext().execute_with(|| {
ExistentialDeposit::set(1);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);
// pool creator
let alice = 10;
let bob = 20;
Expand Down
12 changes: 6 additions & 6 deletions substrate/frame/nomination-pools/test-transfer-stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use sp_runtime::{bounded_btree_map, traits::Zero};
fn pool_lifecycle_e2e() {
new_test_ext().execute_with(|| {
assert_eq!(Balances::minimum_balance(), 5);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 50, 10, 10, 10));
Expand Down Expand Up @@ -286,7 +286,7 @@ fn destroy_pool_with_erroneous_consumer() {
fn pool_chill_e2e() {
new_test_ext().execute_with(|| {
assert_eq!(Balances::minimum_balance(), 5);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 50, 10, 10, 10));
Expand Down Expand Up @@ -412,7 +412,7 @@ fn pool_slash_e2e() {
new_test_ext().execute_with(|| {
ExistentialDeposit::set(1);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
Expand Down Expand Up @@ -622,7 +622,7 @@ fn pool_slash_proportional() {
ExistentialDeposit::set(1);
BondingDuration::set(28);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
Expand Down Expand Up @@ -759,7 +759,7 @@ fn pool_slash_non_proportional_only_bonded_pool() {
ExistentialDeposit::set(1);
BondingDuration::set(28);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
Expand Down Expand Up @@ -838,7 +838,7 @@ fn pool_slash_non_proportional_bonded_pool_and_chunks() {
ExistentialDeposit::set(1);
BondingDuration::set(28);
assert_eq!(Balances::minimum_balance(), 1);
assert_eq!(Staking::current_era(), None);
assert_eq!(CurrentEra::<T>::get(), None);

// create the pool, we know this has id 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/root-offences/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub mod pallet {
fn get_offence_details(
offenders: Vec<(T::AccountId, Perbill)>,
) -> Result<Vec<OffenceDetails<T>>, DispatchError> {
let now = Staking::<T>::active_era()
let now = pallet_staking::ActiveEra::<T>::get()
.map(|e| e.index)
.ok_or(Error::<T>::FailedToGetActiveEra)?;

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/root-offences/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,5 +296,5 @@ pub(crate) fn run_to_block(n: BlockNumber) {
}

pub(crate) fn active_era() -> EraIndex {
Staking::active_era().unwrap().index
pallet_staking::ActiveEra::<Test>::get().unwrap().index
}
2 changes: 1 addition & 1 deletion substrate/frame/staking/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ mod benchmarks {
<ErasValidatorPrefs<T>>::insert(
current_era,
validator.clone(),
<Staking<T>>::validators(&validator),
Validators::<T>::get(&validator),
);

let caller = whitelisted_caller();
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,7 @@ impl<T: Config> Convert<T::AccountId, Option<Exposure<T::AccountId, BalanceOf<T>
for ExposureOf<T>
{
fn convert(validator: T::AccountId) -> Option<Exposure<T::AccountId, BalanceOf<T>>> {
<Pallet<T>>::active_era()
ActiveEra::<T>::get()
.map(|active_era| <Pallet<T>>::eras_stakers(active_era.index, &validator))
}
}
Expand Down Expand Up @@ -1326,7 +1326,7 @@ impl<T: Config, const DISABLING_LIMIT_FACTOR: usize> DisablingStrategy<T>
log!(
debug,
"Won't disable: current_era {:?} > slash_era {:?}",
Pallet::<T>::current_era().unwrap_or_default(),
CurrentEra::<T>::get().unwrap_or_default(),
slash_era
);
return None
Expand Down
18 changes: 9 additions & 9 deletions substrate/frame/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,11 +568,11 @@ impl ExtBuilder {
}

pub(crate) fn active_era() -> EraIndex {
Staking::active_era().unwrap().index
pallet_staking::ActiveEra::<Test>::get().unwrap().index
}

pub(crate) fn current_era() -> EraIndex {
Staking::current_era().unwrap()
pallet_staking::CurrentEra::<Test>::get().unwrap()
}

pub(crate) fn bond(who: AccountId, val: Balance) {
Expand Down Expand Up @@ -663,7 +663,7 @@ pub(crate) fn start_active_era(era_index: EraIndex) {

pub(crate) fn current_total_payout_for_duration(duration: u64) -> Balance {
let (payout, _rest) = <Test as Config>::EraPayout::era_payout(
Staking::eras_total_stake(active_era()),
pallet_staking::ErasTotalStake::<Test>::get(active_era()),
pallet_balances::TotalIssuance::<Test>::get(),
duration,
);
Expand All @@ -673,7 +673,7 @@ pub(crate) fn current_total_payout_for_duration(duration: u64) -> Balance {

pub(crate) fn maximum_payout_for_duration(duration: u64) -> Balance {
let (payout, rest) = <Test as Config>::EraPayout::era_payout(
Staking::eras_total_stake(active_era()),
pallet_staking::ErasTotalStake::<Test>::get(active_era()),
pallet_balances::TotalIssuance::<Test>::get(),
duration,
);
Expand Down Expand Up @@ -732,11 +732,11 @@ pub(crate) fn on_offence_in_era(
}
}

if Staking::active_era().unwrap().index == era {
if pallet_staking::ActiveEra::<Test>::get().unwrap().index == era {
let _ = Staking::on_offence(
offenders,
slash_fraction,
Staking::eras_start_session_index(era).unwrap(),
pallet_staking::ErasStartSessionIndex::<Test>::get(era).unwrap(),
);
} else {
panic!("cannot slash in era {}", era);
Expand All @@ -750,7 +750,7 @@ pub(crate) fn on_offence_now(
>],
slash_fraction: &[Perbill],
) {
let now = Staking::active_era().unwrap().index;
let now = pallet_staking::ActiveEra::<Test>::get().unwrap().index;
on_offence_in_era(offenders, slash_fraction, now)
}

Expand Down Expand Up @@ -889,10 +889,10 @@ macro_rules! assert_session_era {
$session,
);
assert_eq!(
Staking::current_era().unwrap(),
CurrentEra::<T>::get().unwrap(),
$era,
"wrong current era {} != {}",
Staking::current_era().unwrap(),
CurrentEra::<T>::get().unwrap(),
$era,
);
};
Expand Down
Loading
Loading