Skip to content

Commit

Permalink
Remove pallet-getter usage from pallet-session (paritytech#4972)
Browse files Browse the repository at this point in the history
As per paritytech#3326, removes usage of the `pallet::getter` macro from the
`session` pallet. The syntax `StorageItem::<T, I>::get()` should be used
instead.

Also, adds public functions for compatibility.

NOTE: The `./historical` directory has not been modified.

cc @muraca

polkadot address: 5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
  • Loading branch information
3 people authored and jpserrat committed Jul 18, 2024
1 parent 1c412d1 commit e8af1d7
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 53 deletions.
14 changes: 14 additions & 0 deletions prdoc/pr_4972.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: "Remove `pallet::getter` usage from pallet-session"

doc:
- audience: Runtime Dev
description: |
This PR removes the `pallet::getter`s from `pallet-session`.
The syntax `StorageItem::<T, I>::get()` should be used instead.

crates:
- name: pallet-session
bump: minor
2 changes: 1 addition & 1 deletion substrate/frame/session/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ set.
use pallet_session as session;

fn validators<T: pallet_session::Config>() -> Vec<<T as pallet_session::Config>::ValidatorId> {
<pallet_session::Pallet<T>>::validators()
pallet_session::Validators::<T>::get()
}
```

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/session/benchmarking/src/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn check_membership_proof_setup<T: Config>(
Pallet::<T>::on_initialize(frame_system::pallet_prelude::BlockNumberFor::<T>::one());

// skip sessions until the new validator set is enacted
while Session::<T>::validators().len() < n as usize {
while Validators::<T>::get().len() < n as usize {
Session::<T>::rotate_session();
}

Expand Down
74 changes: 45 additions & 29 deletions substrate/frame/session/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
//! use pallet_session as session;
//!
//! fn validators<T: pallet_session::Config>() -> Vec<<T as pallet_session::Config>::ValidatorId> {
//! <pallet_session::Pallet<T>>::validators()
//! pallet_session::Validators::<T>::get()
//! }
//! # fn main(){}
//! ```
Expand Down Expand Up @@ -447,7 +447,7 @@ pub mod pallet {
});

for (account, val, keys) in self.keys.iter().cloned() {
<Pallet<T>>::inner_set_keys(&val, keys)
Pallet::<T>::inner_set_keys(&val, keys)
.expect("genesis config must not contain duplicates; qed");
if frame_system::Pallet::<T>::inc_consumers_without_limit(&account).is_err() {
// This will leak a provider reference, however it only happens once (at
Expand Down Expand Up @@ -479,20 +479,18 @@ pub mod pallet {
T::SessionHandler::on_genesis_session::<T::Keys>(&queued_keys);

Validators::<T>::put(initial_validators_0);
<QueuedKeys<T>>::put(queued_keys);
QueuedKeys::<T>::put(queued_keys);

T::SessionManager::start_session(0);
}
}

/// The current set of validators.
#[pallet::storage]
#[pallet::getter(fn validators)]
pub type Validators<T: Config> = StorageValue<_, Vec<T::ValidatorId>, ValueQuery>;

/// Current index of the session.
#[pallet::storage]
#[pallet::getter(fn current_index)]
pub type CurrentIndex<T> = StorageValue<_, SessionIndex, ValueQuery>;

/// True if the underlying economic identities or weighting behind the validators
Expand All @@ -503,7 +501,6 @@ pub mod pallet {
/// The queued keys for the next session. When the next session begins, these keys
/// will be used to determine the validator's session keys.
#[pallet::storage]
#[pallet::getter(fn queued_keys)]
pub type QueuedKeys<T: Config> = StorageValue<_, Vec<(T::ValidatorId, T::Keys)>, ValueQuery>;

/// Indices of disabled validators.
Expand All @@ -512,7 +509,6 @@ pub mod pallet {
/// disabled using binary search. It gets cleared when `on_session_ending` returns
/// a new set of identities.
#[pallet::storage]
#[pallet::getter(fn disabled_validators)]
pub type DisabledValidators<T> = StorageValue<_, Vec<u32>, ValueQuery>;

/// The next session keys for a validator.
Expand Down Expand Up @@ -609,33 +605,53 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
/// Public function to access the current set of validators.
pub fn validators() -> Vec<T::ValidatorId> {
Validators::<T>::get()
}

/// Public function to access the current session index.
pub fn current_index() -> SessionIndex {
CurrentIndex::<T>::get()
}

/// Public function to access the queued keys.
pub fn queued_keys() -> Vec<(T::ValidatorId, T::Keys)> {
QueuedKeys::<T>::get()
}

/// Public function to access the disabled validators.
pub fn disabled_validators() -> Vec<u32> {
DisabledValidators::<T>::get()
}

/// Move on to next session. Register new validator set and session keys. Changes to the
/// validator set have a session of delay to take effect. This allows for equivocation
/// punishment after a fork.
pub fn rotate_session() {
let session_index = <CurrentIndex<T>>::get();
let session_index = CurrentIndex::<T>::get();
log::trace!(target: "runtime::session", "rotating session {:?}", session_index);

let changed = <QueuedChanged<T>>::get();
let changed = QueuedChanged::<T>::get();

// Inform the session handlers that a session is going to end.
T::SessionHandler::on_before_session_ending();
T::SessionManager::end_session(session_index);

// Get queued session keys and validators.
let session_keys = <QueuedKeys<T>>::get();
let session_keys = QueuedKeys::<T>::get();
let validators =
session_keys.iter().map(|(validator, _)| validator.clone()).collect::<Vec<_>>();
Validators::<T>::put(&validators);

if changed {
// reset disabled validators if active set was changed
<DisabledValidators<T>>::take();
DisabledValidators::<T>::take();
}

// Increment session index.
let session_index = session_index + 1;
<CurrentIndex<T>>::put(session_index);
CurrentIndex::<T>::put(session_index);

T::SessionManager::start_session(session_index);

Expand Down Expand Up @@ -683,8 +699,8 @@ impl<T: Config> Pallet<T> {
(queued_amalgamated, changed)
};

<QueuedKeys<T>>::put(queued_amalgamated.clone());
<QueuedChanged<T>>::put(next_changed);
QueuedKeys::<T>::put(queued_amalgamated.clone());
QueuedChanged::<T>::put(next_changed);

// Record that this happened.
Self::deposit_event(Event::NewSession { session_index });
Expand All @@ -699,7 +715,7 @@ impl<T: Config> Pallet<T> {
return false
}

<DisabledValidators<T>>::mutate(|disabled| {
DisabledValidators::<T>::mutate(|disabled| {
if let Err(index) = disabled.binary_search(&i) {
disabled.insert(index, i);
T::SessionHandler::on_disabled(i);
Expand All @@ -716,7 +732,7 @@ impl<T: Config> Pallet<T> {
/// Returns `false` either if the validator could not be found or it was already
/// disabled.
pub fn disable(c: &T::ValidatorId) -> bool {
Self::validators()
Validators::<T>::get()
.iter()
.position(|i| i == c)
.map(|i| Self::disable_index(i as u32))
Expand Down Expand Up @@ -747,7 +763,7 @@ impl<T: Config> Pallet<T> {
let new_ids = T::Keys::key_ids();

// Translate NextKeys, and key ownership relations at the same time.
<NextKeys<T>>::translate::<Old, _>(|val, old_keys| {
NextKeys::<T>::translate::<Old, _>(|val, old_keys| {
// Clear all key ownership relations. Typically the overlap should
// stay the same, but no guarantees by the upgrade function.
for i in old_ids.iter() {
Expand All @@ -764,7 +780,7 @@ impl<T: Config> Pallet<T> {
Some(new_keys)
});

let _ = <QueuedKeys<T>>::translate::<Vec<(T::ValidatorId, Old)>, _>(|k| {
let _ = QueuedKeys::<T>::translate::<Vec<(T::ValidatorId, Old)>, _>(|k| {
k.map(|k| {
k.into_iter()
.map(|(val, old_keys)| (val.clone(), upgrade(val, old_keys)))
Expand Down Expand Up @@ -850,28 +866,28 @@ impl<T: Config> Pallet<T> {
}

fn load_keys(v: &T::ValidatorId) -> Option<T::Keys> {
<NextKeys<T>>::get(v)
NextKeys::<T>::get(v)
}

fn take_keys(v: &T::ValidatorId) -> Option<T::Keys> {
<NextKeys<T>>::take(v)
NextKeys::<T>::take(v)
}

fn put_keys(v: &T::ValidatorId, keys: &T::Keys) {
<NextKeys<T>>::insert(v, keys);
NextKeys::<T>::insert(v, keys);
}

/// Query the owner of a session key by returning the owner's validator ID.
pub fn key_owner(id: KeyTypeId, key_data: &[u8]) -> Option<T::ValidatorId> {
<KeyOwner<T>>::get((id, key_data))
KeyOwner::<T>::get((id, key_data))
}

fn put_key_owner(id: KeyTypeId, key_data: &[u8], v: &T::ValidatorId) {
<KeyOwner<T>>::insert((id, key_data), v)
KeyOwner::<T>::insert((id, key_data), v)
}

fn clear_key_owner(id: KeyTypeId, key_data: &[u8]) {
<KeyOwner<T>>::remove((id, key_data));
KeyOwner::<T>::remove((id, key_data));
}
}

Expand All @@ -886,11 +902,11 @@ impl<T: Config> ValidatorSet<T::AccountId> for Pallet<T> {
type ValidatorIdOf = T::ValidatorIdOf;

fn session_index() -> sp_staking::SessionIndex {
Pallet::<T>::current_index()
CurrentIndex::<T>::get()
}

fn validators() -> Vec<Self::ValidatorId> {
Pallet::<T>::validators()
Validators::<T>::get()
}
}

Expand All @@ -908,11 +924,11 @@ impl<T: Config> EstimateNextNewSession<BlockNumberFor<T>> for Pallet<T> {

impl<T: Config> frame_support::traits::DisabledValidators for Pallet<T> {
fn is_disabled(index: u32) -> bool {
<Pallet<T>>::disabled_validators().binary_search(&index).is_ok()
DisabledValidators::<T>::get().binary_search(&index).is_ok()
}

fn disabled_validators() -> Vec<u32> {
<Pallet<T>>::disabled_validators()
DisabledValidators::<T>::get()
}
}

Expand All @@ -930,7 +946,7 @@ impl<T: Config, Inner: FindAuthor<u32>> FindAuthor<T::ValidatorId>
{
let i = Inner::find_author(digests)?;

let validators = <Pallet<T>>::validators();
let validators = Validators::<T>::get();
validators.get(i as usize).cloned()
}
}
Loading

0 comments on commit e8af1d7

Please sign in to comment.