From a9a2fdca7b6c9eeb7890331d3280166902930d95 Mon Sep 17 00:00:00 2001
From: Muharem Ismailov <ismailov.m.h@gmail.com>
Date: Fri, 2 Sep 2022 18:54:30 +0200
Subject: [PATCH] Alliance pallet: fix func name, fix migration weights
 (#12174)

* Alliance pallet: fix func name, fix migration weights

* update comment order
---
 substrate/frame/alliance/src/benchmarking.rs |  2 +-
 substrate/frame/alliance/src/lib.rs          | 16 ++++++++--------
 substrate/frame/alliance/src/migration.rs    | 19 +++++++++++++------
 substrate/frame/alliance/src/tests.rs        |  8 ++++----
 4 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs
index 60df1e1a84..d3917b9d2e 100644
--- a/substrate/frame/alliance/src/benchmarking.rs
+++ b/substrate/frame/alliance/src/benchmarking.rs
@@ -685,7 +685,7 @@ benchmarks_instance_pallet! {
 				).unwrap();
 			}
 
-			assert_eq!(Alliance::<T, I>::votable_members_count(), cc);
+			assert_eq!(Alliance::<T, I>::voting_members_count(), cc);
 			assert_eq!(Alliance::<T, I>::ally_members_count(), m);
 		}
 
diff --git a/substrate/frame/alliance/src/lib.rs b/substrate/frame/alliance/src/lib.rs
index 99cb46c9ef..b60077c4ef 100644
--- a/substrate/frame/alliance/src/lib.rs
+++ b/substrate/frame/alliance/src/lib.rs
@@ -684,7 +684,7 @@ pub mod pallet {
 					Error::<T, I>::BadWitness
 				);
 				ensure!(
-					Self::votable_members_count() <= witness.voting_members,
+					Self::voting_members_count() <= witness.voting_members,
 					Error::<T, I>::BadWitness
 				);
 				ensure!(
@@ -697,7 +697,7 @@ pub mod pallet {
 					T::ProposalProvider::veto_proposal(*hash);
 				}
 
-				let mut members = Self::votable_members();
+				let mut members = Self::voting_members();
 				T::MembershipChanged::change_members_sorted(&[], &members, &[]);
 
 				members.append(&mut Self::members_of(MemberRole::Ally));
@@ -1052,7 +1052,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	}
 
 	/// Count of all members who have voting rights.
-	fn votable_members_count() -> u32 {
+	fn voting_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
@@ -1064,7 +1064,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	}
 
 	/// Collect all members who have voting rights into one list.
-	fn votable_members() -> Vec<T::AccountId> {
+	fn voting_members() -> Vec<T::AccountId> {
 		let mut founders = Self::members_of(MemberRole::Founder);
 		let mut fellows = Self::members_of(MemberRole::Fellow);
 		founders.append(&mut fellows);
@@ -1072,8 +1072,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	}
 
 	/// Collect all members who have voting rights into one sorted list.
-	fn votable_members_sorted() -> Vec<T::AccountId> {
-		let mut members = Self::votable_members();
+	fn voting_members_sorted() -> Vec<T::AccountId> {
+		let mut members = Self::voting_members();
 		members.sort();
 		members
 	}
@@ -1089,7 +1089,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		})?;
 
 		if role == MemberRole::Founder || role == MemberRole::Fellow {
-			let members = Self::votable_members_sorted();
+			let members = Self::voting_members_sorted();
 			T::MembershipChanged::change_members_sorted(&[who.clone()], &[], &members[..]);
 		}
 		Ok(())
@@ -1104,7 +1104,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		})?;
 
 		if matches!(role, MemberRole::Founder | MemberRole::Fellow) {
-			let members = Self::votable_members_sorted();
+			let members = Self::voting_members_sorted();
 			T::MembershipChanged::change_members_sorted(&[], &[who.clone()], &members[..]);
 		}
 		Ok(())
diff --git a/substrate/frame/alliance/src/migration.rs b/substrate/frame/alliance/src/migration.rs
index 7e3df5219f..8f98484240 100644
--- a/substrate/frame/alliance/src/migration.rs
+++ b/substrate/frame/alliance/src/migration.rs
@@ -51,22 +51,29 @@ mod v0_to_v1 {
 	use super::*;
 
 	pub fn migrate<T: Config<I>, I: 'static>() -> Weight {
-		if migration::clear_storage_prefix(
+		log::info!(target: LOG_TARGET, "Running migration v0_to_v1.");
+
+		let res = migration::clear_storage_prefix(
 			<Pallet<T, I>>::name().as_bytes(),
 			b"UpForKicking",
 			b"",
 			None,
 			None,
-		)
-		.maybe_cursor
-		.is_some()
-		{
+		);
+
+		log::info!(
+			target: LOG_TARGET,
+			"Cleared '{}' entries from 'UpForKicking' storage prefix",
+			res.unique
+		);
+
+		if res.maybe_cursor.is_some() {
 			log::error!(
 				target: LOG_TARGET,
 				"Storage prefix 'UpForKicking' is not completely cleared."
 			);
 		}
 
-		T::DbWeight::get().writes(1)
+		T::DbWeight::get().writes(res.unique.into())
 	}
 }
diff --git a/substrate/frame/alliance/src/tests.rs b/substrate/frame/alliance/src/tests.rs
index 3bd82a8870..9c61561d7d 100644
--- a/substrate/frame/alliance/src/tests.rs
+++ b/substrate/frame/alliance/src/tests.rs
@@ -29,7 +29,7 @@ type AllianceMotionEvent = pallet_collective::Event<Test, pallet_collective::Ins
 fn force_set_members_works() {
 	new_test_ext().execute_with(|| {
 		// ensure alliance is set
-		assert_eq!(Alliance::votable_members_sorted(), vec![1, 2, 3]);
+		assert_eq!(Alliance::voting_members_sorted(), vec![1, 2, 3]);
 
 		// creating and proposing proposals
 		let (proposal, proposal_len, hash) = make_remark_proposal(42);
@@ -141,7 +141,7 @@ fn force_set_members_works() {
 		));
 
 		// assert new set of voting members
-		assert_eq!(Alliance::votable_members_sorted(), vec![4, 5, 8]);
+		assert_eq!(Alliance::voting_members_sorted(), vec![4, 5, 8]);
 		// assert new ally member
 		assert!(Alliance::is_ally(&2));
 		// assert a retiring member from previous Alliance not removed
@@ -176,7 +176,7 @@ fn propose_works() {
 	new_test_ext().execute_with(|| {
 		let (proposal, proposal_len, hash) = make_remark_proposal(42);
 
-		// only votable member can propose proposal, 4 is ally not have vote rights
+		// only voting member can propose proposal, 4 is ally not have vote rights
 		assert_noop!(
 			Alliance::propose(Origin::signed(4), 3, Box::new(proposal.clone()), proposal_len),
 			Error::<Test, ()>::NoVotingRights
@@ -465,7 +465,7 @@ fn nominate_ally_works() {
 			Error::<Test, ()>::AlreadyMember
 		);
 
-		// only votable member(founder/fellow) have nominate right
+		// only voting member(founder/fellow) have nominate right
 		assert_noop!(
 			Alliance::nominate_ally(Origin::signed(5), 4),
 			Error::<Test, ()>::NoVotingRights