From 0c941d66cb806dea441ebbe7223c16a845bb9038 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com>
Date: Tue, 13 Aug 2019 20:40:30 +0200
Subject: [PATCH] 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 | 26 +++++++++++++++++++++++---
 substrate/srml/membership/src/lib.rs | 14 +++++++++-----
 substrate/srml/support/src/traits.rs | 10 ++++++++++
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/substrate/srml/collective/src/lib.rs b/substrate/srml/collective/src/lib.rs
index 9dafa6721c..76320d1117 100644
--- a/substrate/srml/collective/src/lib.rs
+++ b/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)
diff --git a/substrate/srml/membership/src/lib.rs b/substrate/srml/membership/src/lib.rs
index f56a0ca042..8e8feec4ee 100644
--- a/substrate/srml/membership/src/lib.rs
+++ b/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 = ();
diff --git a/substrate/srml/support/src/traits.rs b/substrate/srml/support/src/traits.rs
index 2766ba0a98..a7d8c9e6ba 100644
--- a/substrate/srml/support/src/traits.rs
+++ b/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]) {}
+}
\ No newline at end of file