collective pallet: sort genesis members and enforce max len constraint (

paritytech#13988)

* insert members in sorted order

* improve variable name

* enforce genesis members length constraint

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)
 		}
@@ -1107,6 +1111,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);
 		}
 	}

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_members = vec![3, 2, 4, 1];
+	let expected_members = vec![1, 2, 3, 4];
+	ExtBuilder::default()
+		.set_collective_members(unsorted_members)
+		.build_and_execute(|| {
+			assert_eq!(Collective::members(), expected_members);
+		});
+}
+
 #[test]
 fn proposal_weight_limit_works() {
 	ExtBuilder::default().build_and_execute(|| {
@@ -1424,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() {