From fb0d1572cdac2a1f248952bfcb6b3cb98de341b6 Mon Sep 17 00:00:00 2001
From: librelois <c@elo.tf>
Date: Wed, 26 Jan 2022 21:00:41 +0100
Subject: [PATCH] fix(authority-members): MaxAuthorities constraint must be
applied
---
pallets/authority-members/src/lib.rs | 21 +++++++++++++++
pallets/authority-members/src/mock.rs | 4 ++-
pallets/authority-members/src/tests.rs | 36 +++++++++++++++++++++++++-
runtime/common/src/pallets_config.rs | 1 +
4 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs
index d3f9c51b0..fb3f77fbb 100644
--- a/pallets/authority-members/src/lib.rs
+++ b/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);
diff --git a/pallets/authority-members/src/mock.rs b/pallets/authority-members/src/mock.rs
index 6362d638a..6f499362f 100644
--- a/pallets/authority-members/src/mock.rs
+++ b/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,
diff --git a/pallets/authority-members/src/tests.rs b/pallets/authority-members/src/tests.rs
index 9ed210742..f92c27e05 100644
--- a/pallets/authority-members/src/tests.rs
+++ b/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(|| {
diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs
index 53a3ff52b..79c92c3eb 100644
--- a/runtime/common/src/pallets_config.rs
+++ b/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>;
--
GitLab