diff --git a/prdoc/pr_3722.prdoc b/prdoc/pr_3722.prdoc new file mode 100644 index 000000000000..7e2d7d38795b --- /dev/null +++ b/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 diff --git a/substrate/frame/authority-discovery/src/lib.rs b/substrate/frame/authority-discovery/src/lib.rs index 2b4dfaf1aea8..0cde61a97b50 100644 --- a/substrate/frame/authority-discovery/src/lib.rs +++ b/substrate/frame/authority-discovery/src/lib.rs @@ -144,19 +144,21 @@ impl OneSessionHandler for Pallet { ); Keys::::put(bounded_keys); + } - let next_keys = queued_validators.map(|x| x.1).collect::>(); + // `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::>(); - 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::::put(next_bounded_keys); - } + NextKeys::::put(next_bounded_keys); } fn on_disabled(_i: u32) { @@ -270,7 +272,7 @@ mod tests { .map(|id| (&account_id, id)) .collect::>(); - let mut third_authorities: Vec = vec![4, 5] + let third_authorities: Vec = 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::>(); + let mut fourth_authorities: Vec = 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::>(); + // Build genesis. let mut t = frame_system::GenesisConfig::::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::>(); + 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::>(); - 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." ); diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs index 7d8128dbc1fe..17b6aa7a4640 100644 --- a/substrate/frame/session/src/lib.rs +++ b/substrate/frame/session/src/lib.rs @@ -285,7 +285,11 @@ pub trait SessionHandler { /// 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( changed: bool, validators: &[(ValidatorId, Ks)],