Alliance pallet: retirement notice call

bin/node/runtime/src/lib.rs

@@ -1514,8 +1514,10 @@ impl pallet_state_trie_migration::Config for Runtime {
 	type WeightInfo = ();
 }
 
+const ALLIANCE_MOTION_DURATION: BlockNumber = 5 * DAYS;
+
 parameter_types! {
-	pub const AllianceMotionDuration: BlockNumber = 5 * DAYS;
+	pub const AllianceMotionDuration: BlockNumber = ALLIANCE_MOTION_DURATION;
 	pub const AllianceMaxProposals: u32 = 100;
 	pub const AllianceMaxMembers: u32 = 100;
 }
@@ -1537,6 +1539,7 @@ parameter_types! {
 	pub const MaxFellows: u32 = AllianceMaxMembers::get() - MaxFounders::get();
 	pub const MaxAllies: u32 = 100;
 	pub const AllyDeposit: Balance = 10 * DOLLARS;
+	pub const RetirementPeriod: BlockNumber = ALLIANCE_MOTION_DURATION + (1 * DAYS);
 }
 
 impl pallet_alliance::Config for Runtime {
@@ -1573,6 +1576,7 @@ impl pallet_alliance::Config for Runtime {
 	type MaxMembersCount = AllianceMaxMembers;
 	type AllyDeposit = AllyDeposit;
 	type WeightInfo = pallet_alliance::weights::SubstrateWeight<Runtime>;
+	type RetirementPeriod = RetirementPeriod;
 }
 
 construct_runtime!(
@@ -1678,9 +1682,16 @@ pub type Executive = frame_executive::Executive<
 	frame_system::ChainContext<Runtime>,
 	Runtime,
 	AllPalletsWithSystem,
-	pallet_nomination_pools::migration::v2::MigrateToV2<Runtime>,
+	Migrations,
 >;
 
+// All migrations executed on runtime upgrade as a nested tuple of types implementing
+// `OnRuntimeUpgrade`.
+type Migrations = (
+	pallet_nomination_pools::migration::v2::MigrateToV2<Runtime>,
+	pallet_alliance::migration::Migration<Runtime>,
+);
+
 /// MMR helper types.
 mod mmr {
 	use super::Runtime;
@@ -1699,6 +1710,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

@@ -43,6 +43,7 @@ to update the Alliance's rule and make announcements.
 
 #### For Members (All)
 
+- `give_retirement_notice` - Give a retirement notice and start a retirement period required to pass in order to retire.
 - `retire` - Retire from the Alliance and release the caller's deposit.
 
 #### For Members (Founders/Fellows)

frame/alliance/src/benchmarking.rs

@@ -691,12 +691,37 @@ benchmarks_instance_pallet! {
 		assert_last_event::<T, I>(Event::AllyElevated { ally: ally1 }.into());
 	}
 
+	give_retirement_notice {
+		set_members::<T, I>();
+		let fellow2 = fellow::<T, I>(2);
+
+		assert!(Alliance::<T, I>::is_fellow(&fellow2));
+	}: _(SystemOrigin::Signed(fellow2.clone()))
+	verify {
+		assert!(Alliance::<T, I>::is_member_of(&fellow2, MemberRole::Retiring));
+
+		assert_eq!(
+			RetiringMembers::<T, I>::get(&fellow2),
+			Some(System::<T>::block_number() + T::RetirementPeriod::get())
+		);
+		assert_last_event::<T, I>(
+			Event::MemberRetirementPeriodStarted {member: fellow2}.into()
+		);
+	}
+
 	retire {
 		set_members::<T, I>();
 
 		let fellow2 = fellow::<T, I>(2);
 		assert!(Alliance::<T, I>::is_fellow(&fellow2));
-		assert!(!Alliance::<T, I>::is_up_for_kicking(&fellow2));
+
+		assert_eq!(
+			Alliance::<T, I>::give_retirement_notice(
+				SystemOrigin::Signed(fellow2.clone()).into()
+			),
+			Ok(())
+		);
+		System::<T>::set_block_number(System::<T>::block_number() + T::RetirementPeriod::get());
 
 		assert_eq!(DepositOf::<T, I>::get(&fellow2), Some(T::AllyDeposit::get()));
 	}: _(SystemOrigin::Signed(fellow2.clone()))
@@ -713,11 +738,7 @@ benchmarks_instance_pallet! {
 		set_members::<T, I>();
 
 		let fellow2 = fellow::<T, I>(2);
-		UpForKicking::<T, I>::insert(&fellow2, true);
-
 		assert!(Alliance::<T, I>::is_member_of(&fellow2, MemberRole::Fellow));
-		assert!(Alliance::<T, I>::is_up_for_kicking(&fellow2));
-
 		assert_eq!(DepositOf::<T, I>::get(&fellow2), Some(T::AllyDeposit::get()));
 
 		let fellow2_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(fellow2.clone());

frame/alliance/src/lib.rs

@@ -60,6 +60,8 @@
 //!
 //! #### For Members (All)
 //!
+//! - `give_retirement_notice` - Give a retirement notice and start a retirement period required to
+//!   pass in order to retire.
 //! - `retire` - Retire from the Alliance and release the caller's deposit.
 //!
 //! #### For Members (Founders/Fellows)
@@ -93,6 +95,7 @@ mod tests;
 
 #[cfg(feature = "runtime-benchmarks")]
 mod benchmarking;
+pub mod migration;
 mod types;
 pub mod weights;
 
@@ -124,6 +127,9 @@ pub use pallet::*;
 pub use types::*;
 pub use weights::*;
 
+/// The log target of this pallet.
+pub const LOG_TARGET: &str = "runtime::alliance";
+
 /// Simple index type for proposal counting.
 pub type ProposalIndex = u32;
 
@@ -198,6 +204,7 @@ pub enum MemberRole {
 	Founder,
 	Fellow,
 	Ally,
+	Retiring,
 }
 
 /// The type of item that may be deemed unscrupulous.
@@ -216,6 +223,7 @@ pub mod pallet {
 
 	#[pallet::pallet]
 	#[pallet::generate_store(pub (super) trait Store)]
+	#[pallet::storage_version(migration::STORAGE_VERSION)]
 	pub struct Pallet<T, I = ()>(PhantomData<(T, I)>);
 
 	#[pallet::config]
@@ -306,6 +314,10 @@ pub mod pallet {
 
 		/// Weight information for extrinsics in this pallet.
 		type WeightInfo: WeightInfo;
+
+		/// The period required to pass from retirement notice for a member to retire.
+		/// Supposed to be greater than time required to `kick_member`.
+		type RetirementPeriod: Get<Self::BlockNumber>;
 	}
 
 	#[pallet::error]
@@ -322,8 +334,6 @@ pub mod pallet {
 		NotAlly,
 		/// Account is not a founder.
 		NotFounder,
-		/// This member is up for being kicked from the Alliance and cannot perform this operation.
-		UpForKicking,
 		/// Account does not have voting rights.
 		NoVotingRights,
 		/// Account is already an elevated (fellow) member.
@@ -355,6 +365,12 @@ pub mod pallet {
 		TooManyMembers,
 		/// Number of announcements exceeds `MaxAnnouncementsCount`.
 		TooManyAnnouncements,
+		/// Account already gave retirement notice
+		AlreadyRetiring,
+		/// Account did not give a retirement notice required to retire.
+		RetirementNoticeNotGiven,
+		/// Retirement period has not passed.
+		RetirementPeriodNotPassed,
 	}
 
 	#[pallet::event]
@@ -380,6 +396,8 @@ pub mod pallet {
 		},
 		/// An ally has been elevated to Fellow.
 		AllyElevated { ally: T::AccountId },
+		/// A member gave retirement notice and their retirement period started.
+		MemberRetirementPeriodStarted { member: T::AccountId },
 		/// A member has retired with its deposit unreserved.
 		MemberRetired { member: T::AccountId, unreserved: Option<BalanceOf<T, I>> },
 		/// A member has been kicked out with its deposit slashed.
@@ -483,12 +501,12 @@ pub mod pallet {
 		ValueQuery,
 	>;
 
-	/// A set of members that are (potentially) being kicked out. They cannot retire until the
-	/// motion is settled.
+	/// A set of members who gave a retirement notice. They can retire after the end of retirement
+	/// period stored as a future block number.
 	#[pallet::storage]
-	#[pallet::getter(fn up_for_kicking)]
-	pub type UpForKicking<T: Config<I>, I: 'static = ()> =
-		StorageMap<_, Blake2_128Concat, T::AccountId, bool, ValueQuery>;
+	#[pallet::getter(fn retiring_members)]
+	pub type RetiringMembers<T: Config<I>, I: 'static = ()> =
+		StorageMap<_, Blake2_128Concat, T::AccountId, T::BlockNumber, OptionQuery>;
 
 	/// The current list of accounts deemed unscrupulous. These accounts non grata cannot submit
 	/// candidacy.
@@ -523,11 +541,6 @@ pub mod pallet {
 			let proposor = ensure_signed(origin)?;
 			ensure!(Self::has_voting_rights(&proposor), Error::<T, I>::NoVotingRights);
 
-			if let Some(Call::kick_member { who }) = proposal.is_sub_type() {
-				let strike = T::Lookup::lookup(who.clone())?;
-				<UpForKicking<T, I>>::insert(strike, true);
-			}
-
 			T::ProposalProvider::propose_proposal(proposor, threshold, proposal, length_bound)?;
 			Ok(())
 		}
@@ -787,15 +800,39 @@ pub mod pallet {
 			Ok(())
 		}
 
+		/// As a member, give a retirement notice and start a retirement period required to pass in
+		/// order to retire.
+		#[pallet::weight(T::WeightInfo::give_retirement_notice())]
+		pub fn give_retirement_notice(origin: OriginFor<T>) -> DispatchResult {
+			let who = ensure_signed(origin)?;
+			let role = Self::member_role_of(&who).ok_or(Error::<T, I>::NotMember)?;
+			ensure!(!role.eq(&MemberRole::Retiring), Error::<T, I>::AlreadyRetiring);
+
+			Self::remove_member(&who, role)?;
+			Self::add_member(&who, MemberRole::Retiring)?;
+			<RetiringMembers<T, I>>::insert(
+				&who,
+				frame_system::Pallet::<T>::block_number() + T::RetirementPeriod::get(),
+			);
+
+			Self::deposit_event(Event::MemberRetirementPeriodStarted { member: who });
+			Ok(())
+		}
+
 		/// As a member, retire from the alliance and unreserve the deposit.
+		/// This can only be done once you have `give_retirement_notice` and it has expired.
 		#[pallet::weight(T::WeightInfo::retire())]
 		pub fn retire(origin: OriginFor<T>) -> DispatchResult {
 			let who = ensure_signed(origin)?;
-			// A member up for kicking cannot retire.
-			ensure!(!Self::is_up_for_kicking(&who), Error::<T, I>::UpForKicking);
+			let retirement_period_end = RetiringMembers::<T, I>::get(&who)
+				.ok_or(Error::<T, I>::RetirementNoticeNotGiven)?;
+			ensure!(
+				frame_system::Pallet::<T>::block_number() >= retirement_period_end,
+				Error::<T, I>::RetirementPeriodNotPassed
+			);
 
-			let role = Self::member_role_of(&who).ok_or(Error::<T, I>::NotMember)?;
-			Self::remove_member(&who, role)?;
+			Self::remove_member(&who, MemberRole::Retiring)?;
+			<RetiringMembers<T, I>>::remove(&who);
 			let deposit = DepositOf::<T, I>::take(&who);
 			if let Some(deposit) = deposit {
 				let err_amount = T::Currency::unreserve(&who, deposit);
@@ -821,8 +858,6 @@ pub mod pallet {
 				T::Slashed::on_unbalanced(T::Currency::slash_reserved(&member, deposit).0);
 			}
 
-			<UpForKicking<T, I>>::remove(&member);
-
 			Self::deposit_event(Event::MemberKicked { member, slashed: deposit });
 			Ok(())
 		}
@@ -937,11 +972,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		founders.into()
 	}
 
-	/// Check if an account's forced removal is up for consideration.
-	fn is_up_for_kicking(who: &T::AccountId) -> bool {
-		<UpForKicking<T, I>>::contains_key(&who)
-	}
-
 	/// Add a user to the sorted alliance member set.
 	fn add_member(who: &T::AccountId, role: MemberRole) -> DispatchResult {
 		<Members<T, I>>::try_mutate(role, |members| -> DispatchResult {

frame/alliance/src/migration.rs

@@ -0,0 +1,72 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2022 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::{Config, Pallet, Weight, LOG_TARGET};
+use frame_support::{pallet_prelude::*, storage::migration, traits::OnRuntimeUpgrade};
+use log;
+
+/// The current storage version.
+pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+
+/// Wrapper for all migrations of this pallet.
+pub fn migrate<T: Config<I>, I: 'static>() -> Weight {
+	let onchain_version = Pallet::<T, I>::on_chain_storage_version();
+	let mut weight: Weight = 0;
+
+	if onchain_version < 1 {
+		weight = weight.saturating_add(v0_to_v1::migrate::<T, I>());
+	}
+
+	STORAGE_VERSION.put::<Pallet<T, I>>();
+	weight = weight.saturating_add(T::DbWeight::get().writes(1));
+
+	weight
+}
+
+/// Implements `OnRuntimeUpgrade` trait.
+pub struct Migration<T, I = ()>(PhantomData<(T, I)>);
+
+impl<T: Config<I>, I: 'static> OnRuntimeUpgrade for Migration<T, I> {
+	fn on_runtime_upgrade() -> Weight {
+		migrate::<T, I>()
+	}
+}
+
+/// v0_to_v1: `UpForKicking` is replaced by a retirement period.
+mod v0_to_v1 {
+	use super::*;
+
+	pub fn migrate<T: Config<I>, I: 'static>() -> Weight {
+		if migration::clear_storage_prefix(
+			<Pallet<T, I>>::name().as_bytes(),
+			b"UpForKicking",
+			b"",
+			None,
+			None,
+		)
+		.maybe_cursor
+		.is_some()
+		{
+			log::error!(
+				target: LOG_TARGET,
+				"Storage prefix 'UpForKicking' is not completely cleared."
+			);
+		}
+
+		T::DbWeight::get().writes(1)
+	}
+}