fix(authority-members): MaxAuthorities constraint must be applied

pallets/authority-members/src/lib.rs

@@ -66,6 +66,9 @@ pub mod pallet {
         type KeysWrapper: Parameter + Into<Self::Keys>;
         type IsMember: IsMember<Self::MemberId>;
         type OnRemovedMember: OnRemovedMember<Self::MemberId>;
+        /// Max number of authorities allowed
+        #[pallet::constant]
+        type MaxAuthorities: Get<u32>;
         #[pallet::constant]
         type MaxKeysLife: Get<SessionIndex>;
         #[pallet::constant]
@@ -113,6 +116,7 @@ pub mod pallet {
                 .collect::<Vec<T::MemberId>>();
             members_ids.sort();
 
+            AuthoritiesCounter::<T>::put(members_ids.len() as u32);
             OnlineAuthorities::<T>::put(members_ids.clone());
             MustRotateKeysBefore::<T>::insert(T::MaxKeysLife::get(), members_ids);
         }
@@ -125,6 +129,10 @@ pub mod pallet {
     pub type AccountIdOf<T: Config> =
         StorageMap<_, Twox64Concat, T::MemberId, T::AccountId, OptionQuery>;
 
+    #[pallet::storage]
+    #[pallet::getter(fn authorities_counter)]
+    pub type AuthoritiesCounter<T: Config> = StorageValue<_, u32, ValueQuery>;
+
     #[pallet::storage]
     #[pallet::getter(fn incoming)]
     pub type IncomingAuthorities<T: Config> = StorageValue<_, Vec<T::MemberId>, ValueQuery>;
@@ -198,6 +206,8 @@ pub mod pallet {
         NotMember,
         /// Session keys not provided
         SessionKeysNotProvided,
+        /// Too man aAuthorities
+        TooManyAuthorities,
     }
 
     // CALLS //
@@ -258,6 +268,9 @@ pub mod pallet {
             if Self::is_online(member_id) && !is_outgoing {
                 return Err(Error::<T>::AlreadyOnline.into());
             }
+            if AuthoritiesCounter::<T>::get() >= T::MaxAuthorities::get() {
+                return Err(Error::<T>::TooManyAuthorities.into());
+            }
 
             // Apply phase //
             if is_outgoing {
@@ -398,6 +411,7 @@ pub mod pallet {
                 }
             });
             if not_already_inserted {
+                AuthoritiesCounter::<T>::mutate(|counter| *counter += 1);
                 AccountIdOf::<T>::insert(member_id, account_id);
                 Self::deposit_event(Event::MemberGoOnline(member_id));
             }
@@ -413,6 +427,11 @@ pub mod pallet {
                 }
             });
             if not_already_inserted {
+                AuthoritiesCounter::<T>::mutate(|counter| {
+                    if *counter > 0 {
+                        *counter -= 1
+                    }
+                });
                 Self::deposit_event(Event::MemberGoOffline(member_id));
             }
             not_already_inserted
@@ -433,6 +452,7 @@ pub mod pallet {
                 .is_ok()
         }
         fn remove_in(member_id: T::MemberId) {
+            AuthoritiesCounter::<T>::mutate(|counter| counter.saturating_sub(1));
             IncomingAuthorities::<T>::mutate(|members_ids| {
                 if let Ok(index) = members_ids.binary_search(&member_id) {
                     members_ids.remove(index);
@@ -440,6 +460,7 @@ pub mod pallet {
             })
         }
         fn remove_online(member_id: T::MemberId) {
+            AuthoritiesCounter::<T>::mutate(|counter| counter.saturating_add(1));
             OnlineAuthorities::<T>::mutate(|members_ids| {
                 if let Ok(index) = members_ids.binary_search(&member_id) {
                     members_ids.remove(index);

pallets/authority-members/src/mock.rs

@@ -151,6 +151,7 @@ impl pallet_authority_members::Config for Test {
     type Event = Event;
     type KeysWrapper = MockSessionKeys;
     type IsMember = TestIsSmithMember;
+    type MaxAuthorities = ConstU32<4>;
     type MaxKeysLife = ConstU32<5>;
     type MaxOfflineSessions = ConstU32<2>;
     type MemberId = u64;
@@ -175,8 +176,9 @@ pub fn new_test_ext(initial_authorities_len: u64) -> sp_io::TestExternalities {
         for (ref k, ..) in &keys {
             frame_system::Pallet::<Test>::inc_providers(k);
         }
-        // A dedicated test account
+        // Some dedicated test account
         frame_system::Pallet::<Test>::inc_providers(&12);
+        frame_system::Pallet::<Test>::inc_providers(&15);
     });
     pallet_authority_members::GenesisConfig::<Test> {
         initial_authorities,

pallets/authority-members/src/tests.rs

@@ -17,7 +17,7 @@
 use super::*;
 use crate::mock::*;
 use crate::MemberData;
-use frame_support::assert_ok;
+use frame_support::{assert_err, assert_ok};
 use sp_runtime::testing::UintAuthorityId;
 
 const EMPTY: Vec<u64> = Vec::new();
@@ -224,6 +224,40 @@ fn test_go_online() {
     });
 }
 
+#[test]
+fn test_too_many_authorities() {
+    new_test_ext(3).execute_with(|| {
+        run_to_block(1);
+
+        // Member 12 set his session keys then go online
+        assert_ok!(AuthorityMembers::set_session_keys(
+            Origin::signed(12),
+            12,
+            UintAuthorityId(12).into(),
+        ));
+        assert_eq!(AuthorityMembers::authorities_counter(), 3);
+        assert_ok!(AuthorityMembers::go_online(Origin::signed(12), 12),);
+
+        // Member 15 can't go online because there is already 4 authorities "planned"
+        assert_ok!(AuthorityMembers::set_session_keys(
+            Origin::signed(15),
+            15,
+            UintAuthorityId(15).into(),
+        ));
+        assert_eq!(AuthorityMembers::authorities_counter(), 4);
+        assert_err!(
+            AuthorityMembers::go_online(Origin::signed(15), 15),
+            Error::<Test>::TooManyAuthorities,
+        );
+
+        // If member 3 go_offline, member 15 can go_online
+        assert_ok!(AuthorityMembers::go_offline(Origin::signed(3), 3),);
+        assert_eq!(AuthorityMembers::authorities_counter(), 3);
+        assert_ok!(AuthorityMembers::go_online(Origin::signed(15), 15),);
+        assert_eq!(AuthorityMembers::authorities_counter(), 4);
+    });
+}
+
 #[test]
 fn test_go_online_then_go_offline_in_same_session() {
     new_test_ext(3).execute_with(|| {

runtime/common/src/pallets_config.rs

@@ -183,6 +183,7 @@ macro_rules! pallets_config {
 			type OnRemovedMember = OnRemovedAuthorityMemberHandler<Runtime>;
 			type MemberId = IdtyIndex;
 			type MemberIdOf = Identity;
+			type MaxAuthorities = MaxAuthorities;
 			type MaxKeysLife = frame_support::pallet_prelude::ConstU32<1_000>;
 			type MaxOfflineSessions = frame_support::pallet_prelude::ConstU32<100>;
 			type RemoveMemberOrigin = EnsureRoot<Self::AccountId>;