diff --git a/prdoc/pr_11573.prdoc b/prdoc/pr_11573.prdoc new file mode 100644 index 0000000000000..4816a62be1331 --- /dev/null +++ b/prdoc/pr_11573.prdoc @@ -0,0 +1,8 @@ +title: Fix can_inc_consumer check blocking session key rotation in pallet_session +doc: +- audience: Runtime Dev + description: |- + Check consumer capacity only when we actually increment the consumer count (first-time local registration or external-to-local transition), not on key rotation. +crates: +- name: pallet-session + bump: patch diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs index 940d2ecebc8eb..63b36764dd92e 100644 --- a/substrate/frame/session/src/lib.rs +++ b/substrate/frame/session/src/lib.rs @@ -924,7 +924,14 @@ impl Pallet { let who = T::ValidatorIdOf::convert(account.clone()) .ok_or(Error::::NoAssociatedValidatorId)?; - ensure!(frame_system::Pallet::::can_inc_consumer(account), Error::::NoAccount); + // Only check consumer capacity when we will actually increment the consumer + // count: first-time local registration or external-to-local transition. + // Key rotation for an existing locally-managed validator does not need this. + let needs_new_consumer = + !NextKeys::::contains_key(&who) || ExternallySetKeys::::contains_key(account); + if needs_new_consumer { + ensure!(frame_system::Pallet::::can_inc_consumer(account), Error::::NoAccount); + } let old_keys = Self::inner_set_keys(&who, keys)?; diff --git a/substrate/frame/session/src/tests.rs b/substrate/frame/session/src/tests.rs index 038a0d00580aa..c6122372661b6 100644 --- a/substrate/frame/session/src/tests.rs +++ b/substrate/frame/session/src/tests.rs @@ -762,6 +762,76 @@ mod externally_set_keys_tracking { assert_local_state(consumers_before); }); } + + #[test] + fn key_rotation_succeeds_at_max_consumers() { + new_test_ext().execute_with(|| { + // Given: an account with existing locally-managed session keys. + setup_account(); + set_local(ACCOUNT); + + // Saturate the consumer count so `can_inc_consumer` returns false. + while frame_system::Pallet::::can_inc_consumer(&ACCOUNT) { + frame_system::Pallet::::inc_consumers(&ACCOUNT).unwrap(); + } + assert!( + !frame_system::Pallet::::can_inc_consumer(&ACCOUNT), + "pre-condition: consumer slots exhausted" + ); + + // When: the validator rotates keys (not a first-time registration). + let new_key = ACCOUNT + 1; + let keys = UintAuthorityId(new_key).into(); + let proof = create_set_keys_proof(ACCOUNT, &UintAuthorityId(new_key)); + + // Then: the rotation succeeds because it does not need a new consumer ref. + assert_ok!(Session::set_keys(RuntimeOrigin::signed(ACCOUNT), keys, proof)); + }); + } + + #[test] + fn first_registration_fails_at_max_consumers() { + new_test_ext().execute_with(|| { + // Given: a fresh account with no session keys and exhausted consumer slots. + const FRESH_ACCOUNT: u64 = 2000; + frame_system::Pallet::::inc_providers(&FRESH_ACCOUNT); + ValidatorAccounts::mutate(|m| { + m.insert(FRESH_ACCOUNT, FRESH_ACCOUNT); + }); + while frame_system::Pallet::::can_inc_consumer(&FRESH_ACCOUNT) { + frame_system::Pallet::::inc_consumers(&FRESH_ACCOUNT).unwrap(); + } + + // When/Then: first-time registration is rejected. + let keys = UintAuthorityId(FRESH_ACCOUNT).into(); + let proof = create_set_keys_proof(FRESH_ACCOUNT, &UintAuthorityId(FRESH_ACCOUNT)); + assert_noop!( + Session::set_keys(RuntimeOrigin::signed(FRESH_ACCOUNT), keys, proof), + Error::::NoAccount, + ); + }); + } + + #[test] + fn external_to_local_transition_fails_at_max_consumers() { + new_test_ext().execute_with(|| { + // Given: an account with externally-set keys and exhausted consumer slots. + setup_account(); + set_remote(ACCOUNT); + while frame_system::Pallet::::can_inc_consumer(&ACCOUNT) { + frame_system::Pallet::::inc_consumers(&ACCOUNT).unwrap(); + } + + // When/Then: transitioning from external to local is rejected because it + // needs a new consumer reference. + let keys = UintAuthorityId(ACCOUNT + 1).into(); + let proof = create_set_keys_proof(ACCOUNT, &UintAuthorityId(ACCOUNT + 1)); + assert_noop!( + Session::set_keys(RuntimeOrigin::signed(ACCOUNT), keys, proof), + Error::::NoAccount, + ); + }); + } } mod disabling_byzantine_threshold {