Start an additional session to accelerate usage of the new committee (#1038)

Author: LGLO

changelog.md

@@ -7,6 +7,8 @@ This changelog is based on [Keep A Changelog](https://keepachangelog.com/en/1.1.
 ## Changed
 
 * Updated partner-chains-smart-contracts (raw-scripts) dependency to v8.2.0. Not breaking.
+* `SessionManager` and `ShouldEndSession` implementations of `pallet-session-validator-management` rotate one additional
+session in order to make `pallet_session` use authorities with less delay.
 
 ## Removed
 

demo/runtime/src/lib.rs

@@ -1201,49 +1201,46 @@ mod tests {
 			// Needs to be run to initialize first slot and epoch numbers;
 			advance_block();
 
-			// Scheduled committee goes into effect after a 2-epoch delay
+			// Committee goes into effect 1-epoch and 1-block after selection
 			set_committee_through_inherent_data(&[alice()]);
 			until_epoch_after_finalizing(1, &|| {
-				assert_current_epoch!(0);
 				assert_grandpa_weights();
 				assert_grandpa_authorities!([alice(), bob()]);
 			});
-
+			for_next_n_blocks_after_finalizing(1, &|| {
+				assert_grandpa_weights();
+				assert_grandpa_authorities!([alice(), bob()]);
+			});
 			set_committee_through_inherent_data(&[bob()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(1);
 				assert_grandpa_weights();
-				assert_grandpa_authorities!([alice(), bob()]);
+				assert_grandpa_authorities!([alice()]);
 			});
 			set_committee_through_inherent_data(&[alice()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(2);
 				assert_grandpa_weights();
-				assert_grandpa_authorities!([alice()]);
+				assert_grandpa_authorities!([bob()]);
 			});
 			set_committee_through_inherent_data(&[alice(), bob()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(3);
 				assert_grandpa_weights();
-				assert_grandpa_authorities!([bob()]);
+				assert_grandpa_authorities!([alice()]);
 			});
 			set_committee_through_inherent_data(&[bob(), alice()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(4);
 				assert_grandpa_weights();
-				assert_grandpa_authorities!([alice()]);
+				assert_grandpa_authorities!([alice(), bob()]);
 			});
 			set_committee_through_inherent_data(&[alice()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(5);
 				assert_grandpa_weights();
-				assert_grandpa_authorities!([alice(), bob()]);
+				assert_grandpa_authorities!([bob(), alice()]);
 			});
 
 			// When there's no new committees being scheduled, the last committee stays in power
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH * 3, &|| {
 				assert_grandpa_weights();
-				assert_grandpa_authorities!([bob(), alice()]);
+				assert_grandpa_authorities!([alice()]);
 			});
 		});
 
@@ -1260,39 +1257,33 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			// Needs to be run to initialize first slot and epoch numbers;
 			advance_block();
-
-			// Scheduled committee goes into effect after a 2-epoch delay
+			// Committee goes into effect 1-epoch and 1-block after selection
 			set_committee_through_inherent_data(&[alice()]);
 			until_epoch_after_finalizing(1, &|| {
-				assert_current_epoch!(0);
 				assert_aura_authorities!([alice(), bob()]);
 			});
-
-			set_committee_through_inherent_data(&[bob()]);
-			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(1);
+			for_next_n_blocks_after_finalizing(1, &|| {
 				assert_aura_authorities!([alice(), bob()]);
 			});
-			set_committee_through_inherent_data(&[alice()]);
+			set_committee_through_inherent_data(&[bob()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(2);
 				assert_aura_authorities!([alice()]);
 			});
-			set_committee_through_inherent_data(&[alice(), bob()]);
+			set_committee_through_inherent_data(&[alice()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(3);
 				assert_aura_authorities!([bob()]);
 			});
-			set_committee_through_inherent_data(&[bob(), alice()]);
+			set_committee_through_inherent_data(&[alice(), bob()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(4);
 				assert_aura_authorities!([alice()]);
 			});
-			set_committee_through_inherent_data(&[alice()]);
+			set_committee_through_inherent_data(&[bob(), alice()]);
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
-				assert_current_epoch!(5);
 				assert_aura_authorities!([alice(), bob()]);
 			});
+			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH, &|| {
+				assert_aura_authorities!([bob(), alice()]);
+			});
 
 			// When there's no new committees being scheduled, the last committee stays in power
 			for_next_n_blocks_after_finalizing(SLOTS_PER_EPOCH * 3, &|| {
@@ -1326,9 +1317,7 @@ mod tests {
 		});
 	}
 
-	pub fn set_committee_through_inherent_data(
-		expected_authorities: &[TestKeys],
-	) -> PostDispatchInfo {
+	fn set_committee_through_inherent_data(expected_authorities: &[TestKeys]) -> PostDispatchInfo {
 		let epoch = Sidechain::current_epoch_number();
 		let slot = *pallet_aura::CurrentSlot::<Test>::get();
 		println!(

e2e-tests/tests/committee/test_committee.py

@@ -486,7 +486,7 @@ def test_authorities_matching_committee(self, api: BlockchainApi, config: ApiCon
 
         # There's a delay between selecting a committee and it being in power, which means the current
         # committee was selected based on inputs from the previous epoch
-        committee = api.get_epoch_committee(current_epoch - 1).result['committee']
+        committee = api.get_epoch_committee(current_epoch).result['committee']
         authorities = api.get_authorities()
 
         if api.get_pc_epoch() > current_epoch:

toolkit/committee-selection/pallet/src/lib.rs

@@ -33,6 +33,8 @@ mod tests;
 
 pub mod weights;
 
+use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
+use scale_info::TypeInfo;
 pub use sp_session_validator_management::CommitteeMember;
 pub use weights::WeightInfo;
 
@@ -165,6 +167,12 @@ pub mod pallet {
 		OptionQuery,
 	>;
 
+	/// Stores the stage of handling the inputs change. Used by session manager, to decide
+	/// if the session should be ended quickly, to speed up using the newly selected committee.
+	#[pallet::storage]
+	pub type CommitteeRotationStage<T: Config> =
+		StorageValue<_, CommitteeRotationStages, ValueQuery>;
+
 	#[pallet::storage]
 	pub type MainChainScriptsConfiguration<T: Config> =
 		StorageValue<_, MainChainScripts, ValueQuery>;
@@ -448,3 +456,15 @@ pub mod pallet {
 		}
 	}
 }
+
+/// For session state machine
+#[derive(Encode, Decode, Default, Debug, MaxEncodedLen, TypeInfo, PartialEq, Eq)]
+pub enum CommitteeRotationStages {
+	/// No action is required until the current committee becomes obsolete
+	#[default]
+	AwaitEpochChange,
+	/// Session ended because of epoch change
+	NewSessionDueEpochChange,
+	/// Session ended to accelerate use of validators queued in the previous block
+	AdditionalSession,
+}

toolkit/committee-selection/pallet/src/mock.rs

@@ -280,3 +280,18 @@ pub(crate) fn start_session(session_index: u32) {
 
 	assert_eq!(Session::current_index(), session_index);
 }
+
+pub(crate) fn advance_one_block() {
+	let block_number = System::block_number();
+	System::on_finalize(block_number);
+	Session::on_finalize(block_number);
+	let parent_hash =
+		if block_number > 1 { System::finalize().hash() } else { System::parent_hash() };
+	System::reset_events();
+	let next_block_number = block_number as u64 + 1;
+	System::initialize(&next_block_number, &parent_hash, &Default::default());
+	System::set_block_number(next_block_number.into());
+
+	System::on_initialize(next_block_number);
+	Session::on_initialize(next_block_number);
+}

toolkit/committee-selection/pallet/src/pallet_session_support.rs

@@ -3,7 +3,7 @@
 //! This implementation has lag of one additional PC epoch when applying committees to sessions.
 //!
 //! To use it, wire [crate::Pallet] in runtime configuration of [`pallet_session`].
-use crate::CommitteeMember;
+use crate::{CommitteeMember, CommitteeRotationStage, CommitteeRotationStages};
 use frame_support::traits::UnfilteredDispatchable;
 use frame_system::RawOrigin;
 use frame_system::pallet_prelude::BlockNumberFor;
@@ -32,7 +32,14 @@ where
 	/// Updates the session index of [`pallet_session`].
 	// Instead of Some((*).expect) we could just use (*). However, we rather panic in presence of important programming errors.
 	fn new_session(new_index: SessionIndex) -> Option<Vec<T::AccountId>> {
-		info!("💼 Session manager: new_session {new_index}");
+		if CommitteeRotationStage::<T>::get() == CommitteeRotationStages::AdditionalSession {
+			info!("💼 Session manager: new additional session {new_index}");
+			CommitteeRotationStage::<T>::put(CommitteeRotationStages::AwaitEpochChange);
+			let committee = crate::Pallet::<T>::current_committee_storage().committee;
+			return Some(committee.iter().map(|member| member.authority_id().into()).collect());
+		}
+
+		info!("💼 Session manager: new_session {new_index}, rotating the committee");
 		let new_committee = crate::Pallet::<T>::rotate_committee_to_next_epoch().expect(
 			"Session should never end without current epoch validators defined. \
 				Check ShouldEndSession implementation or if it is used before starting new session",
@@ -130,6 +137,7 @@ where
 		if current_epoch_number > current_committee_epoch {
 			if next_committee_is_defined {
 				info!("Session manager: should_end_session({n:?}) = true");
+				CommitteeRotationStage::<T>::put(CommitteeRotationStages::NewSessionDueEpochChange);
 				true
 			} else {
 				warn!(
@@ -138,16 +146,23 @@ where
 				false
 			}
 		} else {
-			debug!("Session manager: should_end_session({n:?}) = false");
-			false
+			let stage = CommitteeRotationStage::<T>::get();
+			if stage == CommitteeRotationStages::NewSessionDueEpochChange {
+				CommitteeRotationStage::<T>::put(CommitteeRotationStages::AdditionalSession);
+				info!("Session manager: should_end_session({n:?}) to force the new committee");
+				true
+			} else {
+				false
+			}
 		}
 	}
 }
 
 #[cfg(test)]
 mod tests {
 	use crate::{
-		CommitteeInfo, CurrentCommittee, NextCommittee, mock::mock_pallet::CurrentEpoch, mock::*,
+		CommitteeInfo, CurrentCommittee, NextCommittee,
+		mock::{mock_pallet::CurrentEpoch, *},
 		tests::increment_epoch,
 	};
 	use pallet_session::ShouldEndSession;
@@ -200,4 +215,32 @@ mod tests {
 			);
 		});
 	}
+
+	#[test]
+	fn ends_two_sessions_and_rotates_once_when_committee_changes() {
+		new_test_ext().execute_with(|| {
+			assert_eq!(Session::current_index(), 0);
+			assert_eq!(SessionCommitteeManagement::current_committee_storage().epoch, 0);
+			increment_epoch();
+			set_validators_directly(&[CHARLIE, DAVE], 1).unwrap();
+
+			advance_one_block();
+			assert_eq!(Session::current_index(), 1);
+			// pallet_session needs additional session to apply CHARLIE and DAVE as validators
+			assert_eq!(Session::validators(), Vec::<u64>::new());
+			assert_eq!(SessionCommitteeManagement::current_committee_storage().epoch, 1);
+
+			advance_one_block();
+			assert_eq!(Session::current_index(), 2);
+			assert_eq!(Session::validators(), vec![CHARLIE.authority_id, DAVE.authority_id]);
+			assert_eq!(SessionCommitteeManagement::current_committee_storage().epoch, 1);
+
+			for _i in 0..10 {
+				advance_one_block();
+				assert_eq!(Session::current_index(), 2);
+				assert_eq!(Session::validators(), vec![CHARLIE.authority_id, DAVE.authority_id]);
+				assert_eq!(SessionCommitteeManagement::current_committee_storage().epoch, 1);
+			}
+		});
+	}
 }