Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions prdoc/pr_11573.prdoc
Original file line number Diff line number Diff line change
@@ -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
9 changes: 8 additions & 1 deletion substrate/frame/session/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,14 @@ impl<T: Config> Pallet<T> {
let who = T::ValidatorIdOf::convert(account.clone())
.ok_or(Error::<T>::NoAssociatedValidatorId)?;

ensure!(frame_system::Pallet::<T>::can_inc_consumer(account), Error::<T>::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::<T>::contains_key(&who) || ExternallySetKeys::<T>::contains_key(account);
if needs_new_consumer {
ensure!(frame_system::Pallet::<T>::can_inc_consumer(account), Error::<T>::NoAccount);
}

let old_keys = Self::inner_set_keys(&who, keys)?;

Expand Down
70 changes: 70 additions & 0 deletions substrate/frame/session/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,76 @@ mod externally_set_keys_tracking {
assert_local_state(consumers_before);
});
}

#[test]
fn key_rotation_succeeds_at_max_consumers() {
Comment thread
sigurpol marked this conversation as resolved.
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::<Test>::can_inc_consumer(&ACCOUNT) {
frame_system::Pallet::<Test>::inc_consumers(&ACCOUNT).unwrap();
}
assert!(
!frame_system::Pallet::<Test>::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::<Test>::inc_providers(&FRESH_ACCOUNT);
ValidatorAccounts::mutate(|m| {
m.insert(FRESH_ACCOUNT, FRESH_ACCOUNT);
});
while frame_system::Pallet::<Test>::can_inc_consumer(&FRESH_ACCOUNT) {
frame_system::Pallet::<Test>::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::<Test>::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::<Test>::can_inc_consumer(&ACCOUNT) {
frame_system::Pallet::<Test>::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::<Test>::NoAccount,
);
});
}
}

mod disabling_byzantine_threshold {
Expand Down
Loading