Fix kusama validators getting 0 backing rewards the first session they enter the active set

prdoc/pr_3722.prdoc

@@ -0,0 +1,13 @@
+# 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: Fix kusama 0 backing rewards when entering active set
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      This PR fixes getting 0 backing rewards the first session when
+      a node enters the active set.
+
+crates:
+ - name: pallet-authority-discovery

substrate/frame/authority-discovery/src/lib.rs

@@ -144,19 +144,21 @@ impl<T: Config> OneSessionHandler<T::AccountId> for Pallet<T> {
 			);
 
 			Keys::<T>::put(bounded_keys);
+		}
 
-			let next_keys = queued_validators.map(|x| x.1).collect::<Vec<_>>();
+		// `changed` represents if queued_validators changed in the previous session not in the
+		// current one.
+		let next_keys = queued_validators.map(|x| x.1).collect::<Vec<_>>();
 
-			let next_bounded_keys = WeakBoundedVec::<_, T::MaxAuthorities>::force_from(
-				next_keys,
-				Some(
-					"Warning: The session has more queued validators than expected. \
-				A runtime configuration adjustment may be needed.",
-				),
-			);
+		let next_bounded_keys = WeakBoundedVec::<_, T::MaxAuthorities>::force_from(
+			next_keys,
+			Some(
+				"Warning: The session has more queued validators than expected. \
+			A runtime configuration adjustment may be needed.",
+			),
+		);
 
-			NextKeys::<T>::put(next_bounded_keys);
-		}
+		NextKeys::<T>::put(next_bounded_keys);
 	}
 
 	fn on_disabled(_i: u32) {
@@ -270,7 +272,7 @@ mod tests {
 			.map(|id| (&account_id, id))
 			.collect::<Vec<(&AuthorityId, AuthorityId)>>();
 
-		let mut third_authorities: Vec<AuthorityId> = vec![4, 5]
+		let third_authorities: Vec<AuthorityId> = vec![4, 5]
 			.into_iter()
 			.map(|i| AuthorityPair::from_seed_slice(vec![i; 32].as_ref()).unwrap().public())
 			.map(AuthorityId::from)
@@ -282,6 +284,18 @@ mod tests {
 			.map(|id| (&account_id, id))
 			.collect::<Vec<(&AuthorityId, AuthorityId)>>();
 
+		let mut fourth_authorities: Vec<AuthorityId> = vec![6, 7]
+			.into_iter()
+			.map(|i| AuthorityPair::from_seed_slice(vec![i; 32].as_ref()).unwrap().public())
+			.map(AuthorityId::from)
+			.collect();
+		// Needed for `pallet_session::OneSessionHandler::on_new_session`.
+		let fourth_authorities_and_account_ids = fourth_authorities
+			.clone()
+			.into_iter()
+			.map(|id| (&account_id, id))
+			.collect::<Vec<(&AuthorityId, AuthorityId)>>();
+
 		// Build genesis.
 		let mut t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
 
@@ -310,25 +324,33 @@ mod tests {
 				third_authorities_and_account_ids.clone().into_iter(),
 			);
 			let authorities_returned = AuthorityDiscovery::authorities();
+			let mut first_and_third_authorities = first_authorities
+				.iter()
+				.chain(third_authorities.iter())
+				.cloned()
+				.collect::<Vec<AuthorityId>>();
+			first_and_third_authorities.sort();
+
 			assert_eq!(
-				first_authorities, authorities_returned,
+				first_and_third_authorities, authorities_returned,
 				"Expected authority set not to change as `changed` was set to false.",
 			);
 
 			// When `changed` set to true, the authority set should be updated.
 			AuthorityDiscovery::on_new_session(
 				true,
-				second_authorities_and_account_ids.into_iter(),
-				third_authorities_and_account_ids.clone().into_iter(),
+				third_authorities_and_account_ids.into_iter(),
+				fourth_authorities_and_account_ids.clone().into_iter(),
 			);
-			let mut second_and_third_authorities = second_authorities
+
+			let mut third_and_fourth_authorities = third_authorities
 				.iter()
-				.chain(third_authorities.iter())
+				.chain(fourth_authorities.iter())
 				.cloned()
 				.collect::<Vec<AuthorityId>>();
-			second_and_third_authorities.sort();
+			third_and_fourth_authorities.sort();
 			assert_eq!(
-				second_and_third_authorities,
+				third_and_fourth_authorities,
 				AuthorityDiscovery::authorities(),
 				"Expected authority set to contain both the authorities of the new as well as the \
 				 next session."
@@ -337,12 +359,12 @@ mod tests {
 			// With overlapping authority sets, `authorities()` should return a deduplicated set.
 			AuthorityDiscovery::on_new_session(
 				true,
-				third_authorities_and_account_ids.clone().into_iter(),
-				third_authorities_and_account_ids.clone().into_iter(),
+				fourth_authorities_and_account_ids.clone().into_iter(),
+				fourth_authorities_and_account_ids.clone().into_iter(),
 			);
-			third_authorities.sort();
+			fourth_authorities.sort();
 			assert_eq!(
-				third_authorities,
+				fourth_authorities,
 				AuthorityDiscovery::authorities(),
 				"Expected authority set to be deduplicated."
 			);

substrate/frame/session/src/lib.rs

@@ -285,7 +285,11 @@ pub trait SessionHandler<ValidatorId> {
 	/// before initialization of your pallet.
 	///
 	/// `changed` is true whenever any of the session keys or underlying economic
-	/// identities or weightings behind those keys has changed.
+	/// identities or weightings behind `validators` keys has changed. `queued_validators`
+	/// could change without `validators` changing. Example of possible sequent calls:
+	///     Session N: on_new_session(false, unchanged_validators, unchanged_queued_validators)
+	///     Session N + 1: on_new_session(false, unchanged_validators, new_queued_validators)
+	/// 	Session N + 2: on_new_session(true, new_queued_validators, new_queued_validators)
 	fn on_new_session<Ks: OpaqueKeys>(
 		changed: bool,
 		validators: &[(ValidatorId, Ks)],