Make sure that srml-collective does not initialize Members twice (#3379)

* Make sure that `srml-collective` does not initialize `Members` twice

* Implement trait for `()`

* Fix test

substrate/srml/collective/src/lib.rs

@@ -28,8 +28,9 @@ use primitives::u32_trait::Value as U32;
 use sr_primitives::traits::{Hash, EnsureOrigin};
 use sr_primitives::weights::SimpleDispatchInfo;
 use srml_support::{
-	dispatch::{Dispatchable, Parameter}, codec::{Encode, Decode}, traits::ChangeMembers,
-	StorageValue, StorageMap, decl_module, decl_event, decl_storage, ensure
+	dispatch::{Dispatchable, Parameter}, codec::{Encode, Decode},
+	traits::{ChangeMembers, InitializeMembers}, StorageValue, StorageMap, decl_module, decl_event,
+	decl_storage, ensure,
 };
 use system::{self, ensure_signed, ensure_root};
 
@@ -93,10 +94,20 @@ decl_storage! {
 		/// Proposals so far.
 		pub ProposalCount get(proposal_count): u32;
 		/// The current members of the collective. This is stored sorted (just by value).
-		pub Members get(members) config(): Vec<T::AccountId>;
+		pub Members get(members): Vec<T::AccountId>;
 	}
 	add_extra_genesis {
 		config(phantom): rstd::marker::PhantomData<I>;
+		config(members): Vec<T::AccountId>;
+		build(|
+			storage: &mut (sr_primitives::StorageOverlay, sr_primitives::ChildrenStorageOverlay),
+			config: &Self,
+		| {
+			runtime_io::with_storage(
+				storage,
+				|| Module::<T, I>::initialize_members(&config.members),
+			);
+		})
 	}
 }
 
@@ -282,6 +293,15 @@ impl<T: Trait<I>, I: Instance> ChangeMembers<T::AccountId> for Module<T, I> {
 	}
 }
 
+impl<T: Trait<I>, I: Instance> InitializeMembers<T::AccountId> for Module<T, I> {
+	fn initialize_members(members: &[T::AccountId]) {
+		if !members.is_empty() {
+			assert!(<Members<T, I>>::get().is_empty(), "Members are already initialized!");
+			<Members<T, I>>::put_ref(members);
+		}
+	}
+}
+
 /// Ensure that the origin `o` represents at least `n` members. Returns `Ok` or an `Err`
 /// otherwise.
 pub fn ensure_members<OuterOrigin, AccountId, I>(o: OuterOrigin, n: MemberCount)

substrate/srml/membership/src/lib.rs

@@ -24,8 +24,7 @@
 
 use sr_std::prelude::*;
 use srml_support::{
-	StorageValue, decl_module, decl_storage, decl_event,
-	traits::{ChangeMembers}
+	StorageValue, decl_module, decl_storage, decl_event, traits::{ChangeMembers, InitializeMembers},
 };
 use system::ensure_root;
 use sr_primitives::{traits::EnsureOrigin, weights::SimpleDispatchInfo};
@@ -49,7 +48,7 @@ pub trait Trait<I=DefaultInstance>: system::Trait {
 	/// The receiver of the signal for when the membership has been initialized. This happens pre-
 	/// genesis and will usually be the same as `MembershipChanged`. If you need to do something
 	/// different on initialization, then you can change this accordingly.
-	type MembershipInitialized: ChangeMembers<Self::AccountId>;
+	type MembershipInitialized: InitializeMembers<Self::AccountId>;
 
 	/// The receiver of the signal for when the membership has changed.
 	type MembershipChanged: ChangeMembers<Self::AccountId>;
@@ -65,12 +64,12 @@ decl_storage! {
 		config(phantom): sr_std::marker::PhantomData<I>;
 		build(|
 			storage: &mut (sr_primitives::StorageOverlay, sr_primitives::ChildrenStorageOverlay),
-			config: &GenesisConfig<T, I>
+			config: &Self,
 		| {
 			sr_io::with_storage(storage, || {
 				let mut members = config.members.clone();
 				members.sort();
-				T::MembershipInitialized::set_members_sorted(&members[..], &[]);
+				T::MembershipInitialized::initialize_members(&members);
 				<Members<T, I>>::put(members);
 			});
 		})
@@ -266,6 +265,11 @@ mod tests {
 			MEMBERS.with(|m| *m.borrow_mut() = new.to_vec());
 		}
 	}
+	impl InitializeMembers<u64> for TestChangeMembers {
+		fn initialize_members(members: &[u64]) {
+			MEMBERS.with(|m| *m.borrow_mut() = members.to_vec());
+		}
+	}
 
 	impl Trait for Test {
 		type Event = ();

substrate/srml/support/src/traits.rs

@@ -689,3 +689,13 @@ impl<T: Clone + Ord> ChangeMembers<T> for () {
 	fn change_members_sorted(_: &[T], _: &[T], _: &[T]) {}
 	fn set_members_sorted(_: &[T], _: &[T]) {}
 }
+
+/// Trait for type that can handle the initialization of account IDs at genesis.
+pub trait InitializeMembers<AccountId> {
+	/// Initialize the members to the given `members`.
+	fn initialize_members(members: &[AccountId]);
+}
+
+impl<T> InitializeMembers<T> for () {
+	fn initialize_members(_: &[T]) {}
+}