Alliance pallet: add force_set_members instead of init_members function

bin/node/runtime/src/impls.rs

@@ -18,7 +18,8 @@
 //! Some configurable implementations as associated type for the substrate runtime.
 
 use crate::{
-	AccountId, AllianceMotion, Assets, Authorship, Balances, Call, Hash, NegativeImbalance, Runtime,
+	AccountId, AllianceCollective, AllianceMotion, Assets, Authorship, Balances, Call, Hash,
+	NegativeImbalance, Runtime,
 };
 use frame_support::{
 	pallet_prelude::*,
@@ -112,6 +113,15 @@ impl ProposalProvider<AccountId, Hash, Call> for AllianceProposalProvider {
 	fn proposal_of(proposal_hash: Hash) -> Option<Call> {
 		AllianceMotion::proposal_of(proposal_hash)
 	}
+
+	fn proposals() -> Vec<Hash> {
+		AllianceMotion::proposals().into_inner()
+	}
+
+	fn proposals_count() -> u32 {
+		pallet_collective::Proposals::<Runtime, AllianceCollective>::decode_len().unwrap_or(0)
+			as u32
+	}
 }
 
 #[cfg(test)]

bin/node/runtime/src/lib.rs

@@ -1703,6 +1703,7 @@ extern crate frame_benchmarking;
 mod benches {
 	define_benchmarks!(
 		[frame_benchmarking, BaselineBench::<Runtime>]
+		[pallet_alliance, Alliance]
 		[pallet_assets, Assets]
 		[pallet_babe, Babe]
 		[pallet_bags_list, BagsList]

frame/alliance/README.md

@@ -65,4 +65,4 @@ to update the Alliance's rule and make announcements.
 
 #### Root Calls
 
-- `init_founders` - Initialize the founding members.
+- `force_set_members` - Set the members via chain governance.

frame/alliance/src/benchmarking.rs

@@ -121,7 +121,13 @@ benchmarks_instance_pallet! {
 		let proposer = founders[0].clone();
 		let fellows = (0 .. y).map(fellow::<T, I>).collect::<Vec<_>>();
 
-		Alliance::<T, I>::init_members(SystemOrigin::Root.into(), founders, fellows, vec![])?;
+		Alliance::<T, I>::force_set_members(
+			SystemOrigin::Root.into(),
+			founders,
+			fellows,
+			vec![],
+			Default::default(),
+		)?;
 
 		let threshold = m;
 		// Add previous proposals.
@@ -167,7 +173,13 @@ benchmarks_instance_pallet! {
 		members.extend(founders.clone());
 		members.extend(fellows.clone());
 
-		Alliance::<T, I>::init_members(SystemOrigin::Root.into(), founders, fellows, vec![])?;
+		Alliance::<T, I>::force_set_members(
+			SystemOrigin::Root.into(),
+			founders,
+			fellows,
+			vec![],
+			Default::default(),
+		)?;
 
 		// Threshold is 1 less than the number of members so that one person can vote nay
 		let threshold = m - 1;
@@ -230,7 +242,13 @@ benchmarks_instance_pallet! {
 		let founders = (0 .. m).map(founder::<T, I>).collect::<Vec<_>>();
 		let vetor = founders[0].clone();
 
-		Alliance::<T, I>::init_members(SystemOrigin::Root.into(), founders, vec![], vec![])?;
+		Alliance::<T, I>::force_set_members(
+			SystemOrigin::Root.into(),
+			founders,
+			vec![],
+			vec![],
+			Default::default(),
+		)?;
 
 		// Threshold is one less than total members so that two nays will disapprove the vote
 		let threshold = m - 1;
@@ -276,7 +294,13 @@ benchmarks_instance_pallet! {
 		members.extend(founders.clone());
 		members.extend(fellows.clone());
 
-		Alliance::<T, I>::init_members(SystemOrigin::Root.into(), founders, fellows, vec![])?;
+		Alliance::<T, I>::force_set_members(
+			SystemOrigin::Root.into(),
+			founders,
+			fellows,
+			vec![],
+			Default::default(),
+		)?;
 
 		let proposer = members[0].clone();
 		let voter = members[1].clone();
@@ -356,7 +380,13 @@ benchmarks_instance_pallet! {
 		members.extend(founders.clone());
 		members.extend(fellows.clone());
 
-		Alliance::<T, I>::init_members(SystemOrigin::Root.into(), founders, fellows, vec![])?;
+		Alliance::<T, I>::force_set_members(
+			SystemOrigin::Root.into(),
+			founders,
+			fellows,
+			vec![],
+			Default::default(),
+		)?;
 
 		let proposer = members[0].clone();
 		let voter = members[1].clone();
@@ -442,7 +472,13 @@ benchmarks_instance_pallet! {
 		members.extend(founders.clone());
 		members.extend(fellows.clone());
 
-		Alliance::<T, I>::init_members(SystemOrigin::Root.into(), founders, fellows, vec![])?;
+		Alliance::<T, I>::force_set_members(
+			SystemOrigin::Root.into(),
+			founders,
+			fellows,
+			vec![],
+			Default::default(),
+		)?;
 
 		let proposer = members[0].clone();
 		let voter = members[1].clone();
@@ -513,7 +549,13 @@ benchmarks_instance_pallet! {
 		members.extend(founders.clone());
 		members.extend(fellows.clone());
 
-		Alliance::<T, I>::init_members(SystemOrigin::Root.into(), founders, fellows, vec![])?;
+		Alliance::<T, I>::force_set_members(
+			SystemOrigin::Root.into(),
+			founders,
+			fellows,
+			vec![],
+			Default::default(),
+		)?;
 
 		let proposer = members[0].clone();
 		let voter = members[1].clone();
@@ -568,17 +610,53 @@ benchmarks_instance_pallet! {
 		assert_eq!(T::ProposalProvider::proposal_of(last_hash), None);
 	}
 
-	init_members {
-		// at least 2 founders
-		let x in 2 .. T::MaxFounders::get();
+	force_set_members {
+		// at least 1 founders
+		let x in 1 .. T::MaxFounders::get();
 		let y in 0 .. T::MaxFellows::get();
 		let z in 0 .. T::MaxAllies::get();
+		let p in 0 .. T::MaxProposals::get();
+		let c in 0 .. T::MaxMembersCount::get();
 
-		let mut founders = (2 .. x).map(founder::<T, I>).collect::<Vec<_>>();
+		let mut founders = (0 .. x).map(founder::<T, I>).collect::<Vec<_>>();
+		let mut proposer = founders[0].clone();
 		let mut fellows = (0 .. y).map(fellow::<T, I>).collect::<Vec<_>>();
 		let mut allies = (0 .. z).map(ally::<T, I>).collect::<Vec<_>>();
+		let witness = ForceSetWitness{
+			proposals: p,
+			members: c,
+		};
+
+		if c > 0 {
+			let old_founders = (0..c).map(founder::<T, I>).collect::<Vec<_>>();
+			proposer = old_founders[0].clone();
+			// set members before benchmarked call to perform alliance reset.
+			Alliance::<T, I>::force_set_members(
+				SystemOrigin::Root.into(),
+				old_founders,
+				vec![],
+				vec![],
+				Default::default(),
+			)?;
+		}
 
-	}: _(SystemOrigin::Root, founders.clone(), fellows.clone(), allies.clone())
+		if p > 0 && c > 0 {
+			// Add previous proposals.
+			for i in 0..p - 1 {
+				let threshold = c;
+				let bytes_in_storage = i + size_of::<Cid>() as u32 + 32;
+				// Proposals should be different so that different proposal hashes are generated
+				let proposal: T::Proposal =
+					AllianceCall::<T, I>::set_rule { rule: rule(vec![i as u8; i as usize]) }.into();
+				Alliance::<T, I>::propose(
+					SystemOrigin::Signed(proposer.clone()).into(),
+					threshold,
+					Box::new(proposal),
+					bytes_in_storage,
+				)?;
+			}
+		}
+	}: _(SystemOrigin::Root, founders.clone(), fellows.clone(), allies.clone(), witness)
 	verify {
 		founders.sort();
 		fellows.sort();

frame/alliance/src/lib.rs

@@ -82,7 +82,7 @@
 //!
 //! #### Root Calls
 //!
-//! - `init_founders` - Initialize the founding members.
+//! - `force_set_members` - Set the members via chain governance.
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
@@ -166,30 +166,46 @@ impl<AccountId> IdentityVerifier<AccountId> for () {
 
 /// The provider of a collective action interface, for example an instance of `pallet-collective`.
 pub trait ProposalProvider<AccountId, Hash, Proposal> {
+	/// Add a new proposal.
+	/// Returns a proposal length and active proposals count if successful.
 	fn propose_proposal(
 		who: AccountId,
 		threshold: u32,
 		proposal: Box<Proposal>,
 		length_bound: u32,
 	) -> Result<(u32, u32), DispatchError>;
 
+	/// Add an aye or nay vote for the sender to the given proposal.
+	/// Returns true if the sender votes first time if successful.
 	fn vote_proposal(
 		who: AccountId,
 		proposal: Hash,
 		index: ProposalIndex,
 		approve: bool,
 	) -> Result<bool, DispatchError>;
 
+	/// Veto a proposal, closing and removing it from the system, regardless of its current state.
+	/// Returns an active proposals count, which includes removed proposal.
 	fn veto_proposal(proposal_hash: Hash) -> u32;
 
+	/// Close a proposal that is either approved, disapproved, or whose voting period has ended.
 	fn close_proposal(
 		proposal_hash: Hash,
 		index: ProposalIndex,
 		proposal_weight_bound: Weight,
 		length_bound: u32,
 	) -> DispatchResultWithPostInfo;
 
+	/// Return a proposal of the given hash.
 	fn proposal_of(proposal_hash: Hash) -> Option<Proposal>;
+
+	/// Return hashes of all active proposals.
+	fn proposals() -> Vec<Hash>;
+
+	// Return count of all active proposals.
+	//
+	// Used to check a witness data for an extrinsic.
+	fn proposals_count() -> u32;
 }
 
 /// The various roles that a member can hold.
@@ -312,8 +328,6 @@ pub mod pallet {
 
 	#[pallet::error]
 	pub enum Error<T, I = ()> {
-		/// The founders/fellows/allies have already been initialized.
-		AllianceAlreadyInitialized,
 		/// The Alliance has not been initialized yet, therefore accounts cannot join it.
 		AllianceNotYetInitialized,
 		/// Account is already a member.
@@ -357,6 +371,10 @@ pub mod pallet {
 		TooManyMembers,
 		/// Number of announcements exceeds `MaxAnnouncementsCount`.
 		TooManyAnnouncements,
+		/// Failed to reset alliance.
+		AllianceNotReset,
+		/// Invalid witness data given.
+		BadWitness,
 	}
 
 	#[pallet::event]
@@ -606,25 +624,56 @@ pub mod pallet {
 		}
 
 		/// Initialize the founders, fellows, and allies.
+		/// Resets members and removes all active proposals if alliance is already initialized.
 		///
-		/// This should only be called once, and must be called by the Root origin.
-		#[pallet::weight(T::WeightInfo::init_members(
+		/// Must be called by the Root origin.
+		/// To force reset the Alliance, witness data must be provided.
+		#[pallet::weight(T::WeightInfo::force_set_members(
 			T::MaxFounders::get(),
 			T::MaxFellows::get(),
-			T::MaxAllies::get()
+			T::MaxAllies::get(),
+			witness.proposals,
+			witness.members,
 		))]
-		pub fn init_members(
+		pub fn force_set_members(
 			origin: OriginFor<T>,
 			founders: Vec<T::AccountId>,
 			fellows: Vec<T::AccountId>,
 			allies: Vec<T::AccountId>,
+			witness: ForceSetWitness,
 		) -> DispatchResult {
 			ensure_root(origin)?;
 
-			// Cannot be called if the Alliance already has Founders or Fellows.
-			// TODO: Remove check and allow Root to set members at any time.
-			// https://github.com/paritytech/substrate/issues/11928
-			ensure!(!Self::is_initialized(), Error::<T, I>::AllianceAlreadyInitialized);
+			if Self::is_initialized() {
+				// Reset Alliance by removing all members.
+				// Veto and remove all active proposals to avoid any unexpected behavior from
+				// actionable items managed outside of the pallet. Items managed within the pallet,
+				// like `UnscrupulousWebsites`, are left for the new Alliance to clean up or keep.
+
+				ensure!(
+					T::ProposalProvider::proposals_count() <= witness.proposals,
+					Error::<T, I>::BadWitness
+				);
+				ensure!(
+					Self::votable_members_count() <= witness.members,
+					Error::<T, I>::BadWitness
+				);
+
+				T::ProposalProvider::proposals().into_iter().for_each(|hash| {
+					T::ProposalProvider::veto_proposal(hash);
+				});
+
+				T::MembershipChanged::change_members_sorted(&[], &Self::votable_members(), &[]);
+				ensure!(
+					// `MemberRole` variants count never greater than members count.
+					Members::<T, I>::clear(witness.members, None).maybe_cursor.is_none(),
+					Error::<T, I>::AllianceNotReset
+				);
+				ensure!(
+					UpForKicking::<T, I>::clear(witness.members, None).maybe_cursor.is_none(),
+					Error::<T, I>::AllianceNotReset
+				);
+			}
 
 			let mut founders: BoundedVec<T::AccountId, T::MaxMembersCount> =
 				founders.try_into().map_err(|_| Error::<T, I>::TooManyMembers)?;
@@ -921,13 +970,26 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Self::is_founder(who) || Self::is_fellow(who)
 	}
 
+	/// Count of all members who have voting rights.
+	fn votable_members_count() -> u32 {
+		Members::<T, I>::decode_len(MemberRole::Founder)
+			.unwrap_or(0)
+			.saturating_add(Members::<T, I>::decode_len(MemberRole::Fellow).unwrap_or(0)) as u32
+	}
+
 	/// Collect all members who have voting rights into one list.
-	fn votable_members_sorted() -> Vec<T::AccountId> {
+	fn votable_members() -> Vec<T::AccountId> {
 		let mut founders = Members::<T, I>::get(MemberRole::Founder).into_inner();
 		let mut fellows = Members::<T, I>::get(MemberRole::Fellow).into_inner();
 		founders.append(&mut fellows);
-		founders.sort();
-		founders.into()
+		founders
+	}
+
+	/// Collect all members who have voting rights into one sorted list.
+	fn votable_members_sorted() -> Vec<T::AccountId> {
+		let mut members = Self::votable_members();
+		members.sort();
+		members
 	}
 
 	/// Check if an account's forced removal is up for consideration.