Skip to content

Commit

Permalink
pallet-initializer new session is applied immediately (#288)
Browse files Browse the repository at this point in the history
* New session is now immediately applied

* fmt

* Remove BufferedSessionChange struct

* Updated ts types

* Get registered paras using session_index. Review
  • Loading branch information
fgamundi authored Oct 17, 2023
1 parent c0b88e7 commit 51f6ca6
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 213 deletions.
73 changes: 7 additions & 66 deletions pallets/initializer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@
//!
//! This pallet is in charge of organizing what happens on session changes.
//! In particular this pallet has implemented the OneSessionHandler trait
//! which will be called upon a session change. This pallet will then store
//! the bufferedSessionChanges (collators, new session index, etc) in the
//! BufferedSessionChanges storage item. This storage item gets read on_finalize
//! and calls the SessionHandler config trait
//! which will be called upon a session change. There it will call the
//! SessionHandler config trait
#![cfg_attr(not(feature = "std"), no_std)]

Expand All @@ -34,30 +32,16 @@ mod tests;
pub use pallet::*;
use {
frame_support::{pallet_prelude::*, traits::OneSessionHandler},
frame_system::pallet_prelude::*,
parity_scale_codec::{Decode, Encode},
scale_info::TypeInfo,
sp_runtime::{
traits::{AtLeast32BitUnsigned, Zero},
RuntimeAppPublic,
},
sp_runtime::{traits::AtLeast32BitUnsigned, RuntimeAppPublic},
sp_std::prelude::*,
};

#[frame_support::pallet]
pub mod pallet {
use super::*;

#[derive(Encode, Decode, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct BufferedSessionChange<T: Config> {
pub changed: bool,
pub validators: Vec<(T::AccountId, T::AuthorityId)>,
pub queued: Vec<(T::AccountId, T::AuthorityId)>,
pub session_index: T::SessionIndex,
}

// The apply_new_sseion trait. We need to comply with this
// The apply_new_session trait. We need to comply with this
pub trait ApplyNewSession<T: Config> {
fn apply_new_session(
changed: bool,
Expand All @@ -84,42 +68,11 @@ pub mod pallet {

type SessionHandler: ApplyNewSession<Self>;
}

/// Buffered session changes along with the block number at which they should be applied.
///
/// Typically this will be empty or one element long. Apart from that this item never hits
/// the storage.
///
/// However this is a `Vec` regardless to handle various edge cases that may occur at runtime
/// upgrade boundaries or if governance intervenes.
#[pallet::storage]
pub(super) type BufferedSessionChanges<T: Config> =
StorageValue<_, BufferedSessionChange<T>, OptionQuery>;

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_finalize(_now: BlockNumberFor<T>) {
// Apply buffered session changes as the last thing. This way the runtime APIs and the
// next block will observe the next session.
//
// Note that we only apply the last session as all others lasted less than a block (weirdly).
if let Some(BufferedSessionChange {
changed,
session_index,
validators,
queued,
}) = BufferedSessionChanges::<T>::take()
{
// Changes to be applied on new session
T::SessionHandler::apply_new_session(changed, session_index, validators, queued);
}
}
}
}

impl<T: Config> Pallet<T> {
/// Should be called when a new session occurs. Buffers the session notification to be applied
/// at the end of the block. If `queued` is `None`, the `validators` are considered queued.
/// Should be called when a new session occurs. If `queued` is `None`,
/// the `validators` are considered queued.
fn on_new_session<'a, I: 'a>(
changed: bool,
session_index: T::SessionIndex,
Expand All @@ -135,19 +88,7 @@ impl<T: Config> Pallet<T> {
validators.clone()
};

if session_index == T::SessionIndex::zero() {
// Genesis session should be immediately enacted.
T::SessionHandler::apply_new_session(false, 0u32.into(), validators, queued);
} else {
BufferedSessionChanges::<T>::mutate(|v| {
*v = Some(BufferedSessionChange {
changed,
validators,
queued,
session_index,
})
});
}
T::SessionHandler::apply_new_session(changed, session_index, validators, queued);
}

/// Should be called when a new session occurs. Buffers the session notification to be applied
Expand Down
35 changes: 3 additions & 32 deletions pallets/initializer/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

use {
super::*,
crate::mock::{new_test_ext, session_change_validators, Initializer, System, Test},
crate::mock::{new_test_ext, session_change_validators, Initializer},
};

#[test]
Expand All @@ -29,15 +29,12 @@ fn session_0_is_instantly_applied() {
Some(Vec::new().into_iter()),
);

let v = BufferedSessionChanges::<Test>::get();
assert!(v.is_none());

assert_eq!(session_change_validators(), Some((0, Vec::new())));
});
}

#[test]
fn session_change_before_initialize_is_still_buffered_after() {
fn session_change_applied() {
new_test_ext().execute_with(|| {
Initializer::test_trigger_on_new_session(
false,
Expand All @@ -46,33 +43,7 @@ fn session_change_before_initialize_is_still_buffered_after() {
Some(Vec::new().into_iter()),
);

let now = System::block_number();
Initializer::on_initialize(now);

// Session change validators are applied after on_finalize
assert_eq!(session_change_validators(), None);

let v = BufferedSessionChanges::<Test>::get();
assert!(v.is_some());
});
}

#[test]
fn session_change_applied_on_finalize() {
new_test_ext().execute_with(|| {
Initializer::on_initialize(1);
Initializer::test_trigger_on_new_session(
false,
1,
Vec::new().into_iter(),
Some(Vec::new().into_iter()),
);

Initializer::on_finalize(1);

// Session change validators are applied after on_finalize
// Session change validators are applied
assert_eq!(session_change_validators(), Some((1, Vec::new())));

assert!(BufferedSessionChanges::<Test>::get().is_none());
});
}
14 changes: 13 additions & 1 deletion runtime/dancebox/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ use {
},
sp_std::{marker::PhantomData, prelude::*},
sp_version::RuntimeVersion,
tp_traits::GetSessionContainerChains,
};
pub use {
sp_runtime::{MultiAddress, Perbill, Permill},
Expand Down Expand Up @@ -1332,7 +1333,18 @@ impl_runtime_apis! {
impl pallet_registrar_runtime_api::RegistrarApi<Block, ParaId, MaxLengthTokenSymbol> for Runtime {
/// Return the registered para ids
fn registered_paras() -> Vec<ParaId> {
Registrar::registered_para_ids().to_vec()
// We should return the container-chains for the session in which we are kicking in
let parent_number = System::block_number();
let should_end_session = <Runtime as pallet_session::Config>::ShouldEndSession::should_end_session(parent_number + 1);

let session_index = if should_end_session {
Session::current_index() +1
}
else {
Session::current_index()
};

Registrar::session_container_chains(session_index).to_vec()
}

/// Fetch genesis data for this para id
Expand Down
19 changes: 0 additions & 19 deletions typescript-api/src/dancebox/interfaces/augment-api-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import type {
PalletBalancesIdAmount,
PalletBalancesReserveData,
PalletConfigurationHostConfiguration,
PalletInitializerBufferedSessionChange,
PalletPooledStakingCandidateEligibleCandidate,
PalletPooledStakingPendingOperationKey,
PalletPooledStakingPoolsKey,
Expand Down Expand Up @@ -263,24 +262,6 @@ declare module "@polkadot/api-base/types/storage" {
/** Generic query */
[key: string]: QueryableStorageEntry<ApiType>;
};
initializer: {
/**
* Buffered session changes along with the block number at which they should be applied.
*
* Typically this will be empty or one element long. Apart from that this item never hits the storage.
*
* However this is a `Vec` regardless to handle various edge cases that may occur at runtime upgrade boundaries or
* if governance intervenes.
*/
bufferedSessionChanges: AugmentedQuery<
ApiType,
() => Observable<Option<PalletInitializerBufferedSessionChange>>,
[]
> &
QueryableStorageEntry<ApiType, []>;
/** Generic query */
[key: string]: QueryableStorageEntry<ApiType>;
};
invulnerables: {
/** The invulnerable, permissioned collators. This list must be sorted. */
invulnerables: AugmentedQuery<ApiType, () => Observable<Vec<AccountId32>>, []> &
Expand Down
Loading

0 comments on commit 51f6ca6

Please sign in to comment.