From 21f5906536182ff34479ccab2d0733668624aa58 Mon Sep 17 00:00:00 2001
From: Liam Aharon <liam.aharon@hotmail.com>
Date: Mon, 24 Apr 2023 16:30:23 +0400
Subject: [PATCH 1/3] insert members in sorted order

---
 frame/collective/src/lib.rs   |  2 ++
 frame/collective/src/tests.rs | 27 +++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs
index 0ecf008fee80e..0a50005c58052 100644
--- a/frame/collective/src/lib.rs
+++ b/frame/collective/src/lib.rs
@@ -1107,6 +1107,8 @@ impl<T: Config<I>, I: 'static> InitializeMembers<T::AccountId> for Pallet<T, I>
 	fn initialize_members(members: &[T::AccountId]) {
 		if !members.is_empty() {
 			assert!(<Members<T, I>>::get().is_empty(), "Members are already initialized!");
+			let mut members = members.to_vec();
+			members.sort();
 			<Members<T, I>>::put(members);
 		}
 	}
diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs
index 4775133ffa2f6..9f8ec1200105e 100644
--- a/frame/collective/src/tests.rs
+++ b/frame/collective/src/tests.rs
@@ -86,6 +86,7 @@ mod mock_democracy {
 }
 
 pub type MaxMembers = ConstU32<100>;
+type AccountId = u64;
 
 parameter_types! {
 	pub const MotionDuration: u64 = 3;
@@ -105,7 +106,7 @@ impl frame_system::Config for Test {
 	type RuntimeCall = RuntimeCall;
 	type Hash = H256;
 	type Hashing = BlakeTwo256;
-	type AccountId = u64;
+	type AccountId = AccountId;
 	type Lookup = IdentityLookup<Self::AccountId>;
 	type Header = Header;
 	type RuntimeEvent = RuntimeEvent;
@@ -161,19 +162,26 @@ impl Config for Test {
 	type MaxProposalWeight = MaxProposalWeight;
 }
 
-pub struct ExtBuilder {}
+pub struct ExtBuilder {
+	collective_members: Vec<AccountId>,
+}
 
 impl Default for ExtBuilder {
 	fn default() -> Self {
-		Self {}
+		Self { collective_members: vec![1, 2, 3] }
 	}
 }
 
 impl ExtBuilder {
+	fn set_collective_members(mut self, collective_members: Vec<AccountId>) -> Self {
+		self.collective_members = collective_members;
+		self
+	}
+
 	pub fn build(self) -> sp_io::TestExternalities {
 		let mut ext: sp_io::TestExternalities = GenesisConfig {
 			collective: pallet_collective::GenesisConfig {
-				members: vec![1, 2, 3],
+				members: self.collective_members,
 				phantom: Default::default(),
 			},
 			collective_majority: pallet_collective::GenesisConfig {
@@ -219,6 +227,17 @@ fn motions_basic_environment_works() {
 	});
 }
 
+#[test]
+fn initialize_members_sorts_members() {
+	let unsorted_accounts = vec![3, 2, 4, 1];
+	let expected_accounts = vec![1, 2, 3, 4];
+	ExtBuilder::default()
+		.set_collective_members(unsorted_accounts)
+		.build_and_execute(|| {
+			assert_eq!(Collective::members(), expected_accounts);
+		});
+}
+
 #[test]
 fn proposal_weight_limit_works() {
 	ExtBuilder::default().build_and_execute(|| {

From 24c4d5591e4456d6359984ff765703a0f26f7b67 Mon Sep 17 00:00:00 2001
From: Liam Aharon <liam.aharon@hotmail.com>
Date: Mon, 24 Apr 2023 16:44:26 +0400
Subject: [PATCH 2/3] improve variable name

---
 frame/collective/src/tests.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs
index 9f8ec1200105e..1ed30a44f1b60 100644
--- a/frame/collective/src/tests.rs
+++ b/frame/collective/src/tests.rs
@@ -229,12 +229,12 @@ fn motions_basic_environment_works() {
 
 #[test]
 fn initialize_members_sorts_members() {
-	let unsorted_accounts = vec![3, 2, 4, 1];
-	let expected_accounts = vec![1, 2, 3, 4];
+	let unsorted_members = vec![3, 2, 4, 1];
+	let expected_members = vec![1, 2, 3, 4];
 	ExtBuilder::default()
-		.set_collective_members(unsorted_accounts)
+		.set_collective_members(unsorted_members)
 		.build_and_execute(|| {
-			assert_eq!(Collective::members(), expected_accounts);
+			assert_eq!(Collective::members(), expected_members);
 		});
 }
 

From 166e53f483d1f885fc0226b6a97b539e039dc9cc Mon Sep 17 00:00:00 2001
From: Liam Aharon <liam.aharon@hotmail.com>
Date: Thu, 27 Apr 2023 12:11:22 +0400
Subject: [PATCH 3/3] enforce genesis members length constraint

---
 frame/collective/src/lib.rs   |  4 ++++
 frame/collective/src/tests.rs | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs
index 0a50005c58052..fd89a998ad97e 100644
--- a/frame/collective/src/lib.rs
+++ b/frame/collective/src/lib.rs
@@ -246,6 +246,10 @@ pub mod pallet {
 				self.members.len(),
 				"Members cannot contain duplicate accounts."
 			);
+			assert!(
+				self.members.len() <= T::MaxMembers::get() as usize,
+				"Members length cannot exceed MaxMembers.",
+			);
 
 			Pallet::<T, I>::initialize_members(&self.members)
 		}
diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs
index 1ed30a44f1b60..1165ebcec2287 100644
--- a/frame/collective/src/tests.rs
+++ b/frame/collective/src/tests.rs
@@ -1443,6 +1443,19 @@ fn disapprove_proposal_works() {
 	})
 }
 
+#[should_panic(expected = "Members length cannot exceed MaxMembers.")]
+#[test]
+fn genesis_build_panics_with_too_many_members() {
+	let max_members: u32 = MaxMembers::get();
+	let too_many_members = (1..=max_members as u64 + 1).collect::<Vec<AccountId>>();
+	pallet_collective::GenesisConfig::<Test> {
+		members: too_many_members,
+		phantom: Default::default(),
+	}
+	.build_storage()
+	.unwrap();
+}
+
 #[test]
 #[should_panic(expected = "Members cannot contain duplicate accounts.")]
 fn genesis_build_panics_with_duplicate_members() {