Alliance pallet: add force_set_members instead of init_members function

bin/node/runtime/src/impls.rs

@@ -112,6 +112,10 @@ 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()
+	}
 }
 
 #[cfg(test)]

bin/node/runtime/src/lib.rs

@@ -1699,6 +1699,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` - Initialize the founding members.

frame/alliance/src/benchmarking.rs

@@ -121,7 +121,7 @@ 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![])?;
 
 		let threshold = m;
 		// Add previous proposals.
@@ -167,7 +167,7 @@ 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![])?;
 
 		// Threshold is 1 less than the number of members so that one person can vote nay
 		let threshold = m - 1;
@@ -230,7 +230,7 @@ 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![])?;
 
 		// Threshold is one less than total members so that two nays will disapprove the vote
 		let threshold = m - 1;
@@ -276,7 +276,7 @@ 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![])?;
 
 		let proposer = members[0].clone();
 		let voter = members[1].clone();
@@ -356,7 +356,7 @@ 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![])?;
 
 		let proposer = members[0].clone();
 		let voter = members[1].clone();
@@ -442,7 +442,7 @@ 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![])?;
 
 		let proposer = members[0].clone();
 		let voter = members[1].clone();
@@ -513,7 +513,7 @@ 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![])?;
 
 		let proposer = members[0].clone();
 		let voter = members[1].clone();
@@ -568,16 +568,45 @@ benchmarks_instance_pallet! {
 		assert_eq!(T::ProposalProvider::proposal_of(last_hash), None);
 	}
 
-	init_members {
+	force_set_members {
+		let b in 1 .. MAX_BYTES;
 		// at least 2 founders
 		let x in 2 .. T::MaxFounders::get();
 		let y in 0 .. T::MaxFellows::get();
 		let z in 0 .. T::MaxAllies::get();
+		let p in 1 .. T::MaxProposals::get();
 
-		let mut founders = (2 .. x).map(founder::<T, I>).collect::<Vec<_>>();
+		let mut founders = (0 .. x).map(founder::<T, I>).collect::<Vec<_>>();
+		let 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 m = x + y;
+		let bytes_in_storage = b + size_of::<Cid>() as u32 + 32;
+
+		// set members before benchmarked call to include alliance reset to the benchmark
+		Alliance::<T, I>::force_set_members(
+			SystemOrigin::Root.into(),
+			founders.clone(),
+			fellows.clone(),
+			allies.clone()
+		)?;
+
+		let threshold = m;
+		// Add previous proposals.
+		for i in 0 .. p - 1 {
+			// 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; b 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())
 	verify {
 		founders.sort();

frame/alliance/src/lib.rs

@@ -82,7 +82,7 @@
 //!
 //! #### Root Calls
 //!
-//! - `init_founders` - Initialize the founding members.
+//! - `force_set_members` - Initialize the founding members.
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
@@ -166,30 +166,41 @@ 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, close, and remove 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>;
 }
 
 /// The various roles that a member can hold.
@@ -310,8 +321,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.
@@ -355,6 +364,8 @@ pub mod pallet {
 		TooManyMembers,
 		/// Number of announcements exceeds `MaxAnnouncementsCount`.
 		TooManyAnnouncements,
+		/// Failed to reset alliance.
+		AllianceNotReset,
 	}
 
 	#[pallet::event]
@@ -604,25 +615,46 @@ 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(
+		#[pallet::weight(T::WeightInfo::force_set_members(
 			T::MaxFounders::get(),
 			T::MaxFellows::get(),
 			T::MaxAllies::get()
 		))]
-		pub fn init_members(
+		pub fn force_set_members(
 			origin: OriginFor<T>,
 			founders: Vec<T::AccountId>,
 			fellows: Vec<T::AccountId>,
 			allies: Vec<T::AccountId>,
 		) -> 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 like `UnscrupulousWebsites`
+				// managed within the pallet left for new Alliance to cleanup or keep.
+
+				T::ProposalProvider::proposals().into_iter().for_each(|hash| {
+					T::ProposalProvider::veto_proposal(hash);
+				});
+
+				T::MembershipChanged::change_members_sorted(&[], &Self::votable_members(), &[]);
+				ensure!(
+					// `MemberRole` variants, the key of the `StorageMap`, is not expected to grow
+					// to 255.
+					Members::<T, I>::clear(255, None).maybe_cursor.is_none(),
+					Error::<T, I>::AllianceNotReset
+				);
+				ensure!(
+					UpForKicking::<T, I>::clear(T::MaxMembersCount::get(), 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)?;
@@ -929,12 +961,18 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	}
 
 	/// 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.

frame/alliance/src/mock.rs

@@ -18,6 +18,7 @@
 //! Test utilities
 
 pub use sp_core::H256;
+use sp_runtime::traits::Hash;
 pub use sp_runtime::{
 	testing::Header,
 	traits::{BlakeTwo256, IdentityLookup},
@@ -192,6 +193,10 @@ impl ProposalProvider<u64, H256, Call> for AllianceProposalProvider {
 	fn proposal_of(proposal_hash: H256) -> Option<Call> {
 		AllianceMotion::proposal_of(proposal_hash)
 	}
+
+	fn proposals() -> Vec<H256> {
+		AllianceMotion::proposals().into_inner()
+	}
 }
 
 parameter_types! {
@@ -248,7 +253,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 	let mut t = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
 
 	pallet_balances::GenesisConfig::<Test> {
-		balances: vec![(1, 50), (2, 50), (3, 50), (4, 50), (5, 30), (6, 50), (7, 50)],
+		balances: vec![(1, 50), (2, 50), (3, 50), (4, 50), (5, 30), (6, 50), (7, 50), (8, 50)],
 	}
 	.assimilate_storage(&mut t)
 	.unwrap();
@@ -290,14 +295,16 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 		assert_ok!(Identity::set_identity(Origin::signed(5), Box::new(info.clone())));
 		assert_ok!(Identity::provide_judgement(Origin::signed(1), 0, 5, Judgement::KnownGood));
 		assert_ok!(Identity::set_identity(Origin::signed(6), Box::new(info.clone())));
+		assert_ok!(Identity::set_identity(Origin::signed(8), Box::new(info.clone())));
+		assert_ok!(Identity::provide_judgement(Origin::signed(1), 0, 8, Judgement::KnownGood));
 
 		// Joining before init should fail.
 		assert_noop!(
 			Alliance::join_alliance(Origin::signed(1)),
 			Error::<Test, ()>::AllianceNotYetInitialized
 		);
 
-		assert_ok!(Alliance::init_members(Origin::root(), vec![1, 2], vec![3], vec![]));
+		assert_ok!(Alliance::force_set_members(Origin::root(), vec![1, 2], vec![3], vec![]));
 
 		System::set_block_number(1);
 	});
@@ -317,14 +324,20 @@ pub fn test_cid() -> Cid {
 	Cid::new_v0(&*result)
 }
 
-pub fn make_proposal(value: u64) -> Call {
-	Call::System(frame_system::Call::remark { remark: value.encode() })
+pub fn make_remark_proposal(value: u64) -> (Call, u32, H256) {
+	make_proposal(Call::System(frame_system::Call::remark { remark: value.encode() }))
+}
+
+pub fn make_set_rule_proposal(rule: Cid) -> (Call, u32, H256) {
+	make_proposal(Call::Alliance(pallet_alliance::Call::set_rule { rule }))
 }
 
-pub fn make_set_rule_proposal(rule: Cid) -> Call {
-	Call::Alliance(pallet_alliance::Call::set_rule { rule })
+pub fn make_kick_member_proposal(who: u64) -> (Call, u32, H256) {
+	make_proposal(Call::Alliance(pallet_alliance::Call::kick_member { who }))
 }
 
-pub fn make_kick_member_proposal(who: u64) -> Call {
-	Call::Alliance(pallet_alliance::Call::kick_member { who })
+pub fn make_proposal(proposal: Call) -> (Call, u32, H256) {
+	let len: u32 = proposal.using_encoded(|p| p.len() as u32);
+	let hash = BlakeTwo256::hash_of(&proposal);
+	(proposal, len, hash)
 }