Use a single total delay, rename stuff

frame/recover/src/lib.rs

@@ -19,20 +19,24 @@
 
 use sp_std::prelude::*;
 use sp_runtime::{
-	traits::{StaticLookup, Dispatchable, SaturatedConversion, Zero},
+	traits::{StaticLookup, Dispatchable, SaturatedConversion, Zero, CheckedAdd, CheckedMul},
 	DispatchError, DispatchResult
 };
 use codec::{Encode, Decode};
 
 use frame_support::{
 	decl_module, decl_event, decl_storage, decl_error, ensure,
 	Parameter, RuntimeDebug,
-	weights::{SimpleDispatchInfo, GetDispatchInfo, PaysFee, WeighData, Weight, ClassifyDispatch, DispatchClass},
+	weights::{
+		SimpleDispatchInfo, GetDispatchInfo, PaysFee, WeighData, Weight,
+		ClassifyDispatch, DispatchClass
+	},
 	traits::{Currency, ReservableCurrency, Get},
 };
 use frame_system::{self as system, ensure_signed, ensure_root};
 
-type BalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::Balance;
+type BalanceOf<T> =
+	<<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::Balance;
 
 /// Configuration trait.
 pub trait Trait: frame_system::Trait {
@@ -45,18 +49,18 @@ pub trait Trait: frame_system::Trait {
 	/// The currency mechanism.
 	type Currency: ReservableCurrency<Self::AccountId>;
 
-	/// The base amount of currency needed to reserve for creating a recovery setup.
+	/// The base amount of currency needed to reserve for creating a recovery configuration.
 	///
 	/// This is held for an additional storage item whose value size is
 	/// TODO bytes.
-	type SetupDepositBase: Get<BalanceOf<Self>>;
+	type ConfigDepositBase: Get<BalanceOf<Self>>;
 
-	/// The amount of currency needed per additional user when creating a recovery setup.
+	/// The amount of currency needed per additional user when creating a recovery configuration.
 	///
 	/// This is held for adding TODO bytes more into a pre-existing storage value.
-	type SetupDepositFactor: Get<BalanceOf<Self>>;
+	type FriendDepositFactor: Get<BalanceOf<Self>>;
 
-	/// The maximum amount of friends allowed in a recovery setup.
+	/// The maximum amount of friends allowed in a recovery configuration.
 	type MaxFriends: Get<u16>;
 
 	/// The base amount of currency needed to reserve for starting a recovery.
@@ -66,23 +70,26 @@ pub trait Trait: frame_system::Trait {
 	type RecoveryDeposit: Get<BalanceOf<Self>>;
 }
 
-/// An open multisig operation.
+/// An active recovery process.
 #[derive(Clone, Eq, PartialEq, Encode, Decode, Default, RuntimeDebug)]
-pub struct RecoveryStatus<BlockNumber, Balance, AccountId> {
-	/// The last block when someone has tried to recover the account
-	last: BlockNumber,
-	/// The amount held in reserve of the `depositor`, to be returned once this setup is closed.
+pub struct ActiveRecovery<BlockNumber, Balance, AccountId> {
+	/// The block number when the recovery process started.
+	created: BlockNumber,
+	/// The amount held in reserve of the `depositor`,
+	/// To be returned once this recovery process is closed.
 	deposit: Balance,
-	/// The approvals achieved so far, including the depositor. Always sorted.
+	/// The friends which have vouched so far. Always sorted.
 	friends: Vec<AccountId>,
 }
 
-/// An open multisig operation.
+/// Configuration for recovering an account.
 #[derive(Clone, Eq, PartialEq, Encode, Decode, Default, RuntimeDebug)]
-pub struct RecoverySetup<BlockNumber, Balance, AccountId> {
-	/// The minimum amount of time between friend approvals.
+pub struct RecoveryConfig<BlockNumber, Balance, AccountId> {
+	/// The minimum number of blocks since the start of the recovery process before the account
+	/// can be recovered.
 	delay_period: BlockNumber,
-	/// The amount held in reserve of the `depositor`, to be returned once this setup is closed.
+	/// The amount held in reserve of the `depositor`,
+	/// to be returned once this configuration is removed.
 	deposit: Balance,
 	/// The list of friends which can help recover an account. Always sorted.
 	friends: Vec<AccountId>,
@@ -92,16 +99,16 @@ pub struct RecoverySetup<BlockNumber, Balance, AccountId> {
 
 decl_storage! {
 	trait Store for Module<T: Trait> as Utility {
-		/// The set of recovery setups.
-		pub RecoverySetups get(fn recovery_setup):
-			map T::AccountId => Option<RecoverySetup<T::BlockNumber, BalanceOf<T>, T::AccountId>>;
+		/// The set of recoverable accounts and their recovery configuration.
+		pub Recoverable get(fn recovery_config):
+			map T::AccountId => Option<RecoveryConfig<T::BlockNumber, BalanceOf<T>, T::AccountId>>;
 		/// Active recovery attempts.
 		///
-		/// First account is the account to be recovered, and the second account is the user trying to
-		/// recover the account.
+		/// First account is the account to be recovered, and the second account
+		/// is the user trying to recover the account.
 		pub ActiveRecoveries get(fn active_recovery):
 			double_map hasher(twox_64_concat) T::AccountId, twox_64_concat(T::AccountId) =>
-			Option<RecoveryStatus<T::BlockNumber, BalanceOf<T>, T::AccountId>>;
+			Option<ActiveRecovery<T::BlockNumber, BalanceOf<T>, T::AccountId>>;
 		/// The final list of recovered accounts.
 		///
 		/// Map from the recovered account to the user who can access it.
@@ -140,9 +147,9 @@ decl_error! {
 		/// Friends list must be sorted
 		NotSorted,
 		/// This account is not set up for recovery
-		NotSetup,
+		NotRecoverable,
 		/// This account is already set up for recovery
-		AlreadySetup,
+		AlreadyRecoverable,
 		/// A recovery process has already started for this account
 		AlreadyStarted,
 		/// A recovery process has not started for this rescuer
@@ -157,6 +164,8 @@ decl_error! {
 		Threshold,
 		/// There are still active recovery attempts that need to be closed
 		StillActive,
+		/// There was an overflow in a calculation
+		Overflow,
 	}
 }
 
@@ -174,20 +183,24 @@ decl_module! {
 		}
 
 		/// Allow Sudo to bypass the recovery process and set an alias account.
-		fn set_recovered_account(origin, rescuee: T::AccountId, rescuer: T::AccountId) {
+		fn set_recovered_account(origin, lost: T::AccountId, rescuer: T::AccountId) {
 			ensure_root(origin)?;
 
 			// Create the recovery storage item.
-			<Recovered<T>>::insert(&rescuee, &rescuer);
+			<Recovered<T>>::insert(&lost, &rescuer);
 
-			Self::deposit_event(RawEvent::AccountRecovered(rescuer, rescuee));
+			Self::deposit_event(RawEvent::AccountRecovered(rescuer, lost));
 		}
 
 		/// Create a recovery process for your account.
-		fn create_recovery(origin, friends: Vec<T::AccountId>, threshold: u16, delay_period: T::BlockNumber) {
+		fn create_recovery(origin,
+			friends: Vec<T::AccountId>,
+			threshold: u16,
+			delay_period: T::BlockNumber
+		) {
 			let who = ensure_signed(origin)?;
 			// Check account is not already set up for recovery
-			ensure!(<RecoverySetups<T>>::exists(&who), Error::<T>::AlreadySetup);
+			ensure!(<Recoverable<T>>::exists(&who), Error::<T>::AlreadyRecoverable);
 			// Check user input is valid
 			ensure!(threshold >= 1, Error::<T>::ZeroThreshold);
 			ensure!(!friends.is_empty(), Error::<T>::ZeroFriends);
@@ -196,35 +209,40 @@ decl_module! {
 			ensure!(Self::is_sorted(&friends), Error::<T>::NotSorted);
 
 			// Total deposit is base fee + number of friends * factor fee
-			let total_deposit = T::SetupDepositBase::get() + T::SetupDepositFactor::get() * friends.len().saturated_into();
+			let friend_deposit = T::FriendDepositFactor::get()
+				.checked_mul(&friends.len().saturated_into())
+				.ok_or(Error::<T>::Overflow)?;
+			let total_deposit = T::ConfigDepositBase::get()
+				.checked_add(&friend_deposit)
+				.ok_or(Error::<T>::Overflow)?;
 			// Reserve the deposit
 			T::Currency::reserve(&who, total_deposit)?;
 
-			// Create the recovery setup
-			let recovery_setup = RecoverySetup {
+			// Create the recovery configuration
+			let recovery_config = RecoveryConfig {
 				delay_period,
 				deposit: total_deposit,
 				friends,
 				threshold,
 			};
 
-			<RecoverySetups<T>>::insert(&who, recovery_setup);
+			<Recoverable<T>>::insert(&who, recovery_config);
 
 			Self::deposit_event(RawEvent::RecoveryCreated(who));
 		}
 
 		fn initiate_recovery(origin, account: T::AccountId) {
 			let who = ensure_signed(origin)?;
-			// Check that the account has a recovery setup
-			ensure!(<RecoverySetups<T>>::exists(&account), Error::<T>::NotSetup);
+			// Check that the account is recoverable
+			ensure!(<Recoverable<T>>::exists(&account), Error::<T>::NotRecoverable);
 			// Check that the recovery process has not already been started
 			ensure!(!<ActiveRecoveries<T>>::exists(&account, &who), Error::<T>::AlreadyStarted);
 			// Take recovery deposit
 			let recovery_deposit = T::RecoveryDeposit::get();
 			T::Currency::reserve(&who, recovery_deposit)?;
 			// Create an active recovery status
-			let recovery_status = RecoveryStatus {
-				last: T::BlockNumber::zero(),
+			let recovery_status = ActiveRecovery {
+				created: <system::Module<T>>::block_number(),
 				deposit: recovery_deposit,
 				friends: vec![],
 			};
@@ -234,50 +252,49 @@ decl_module! {
 			Self::deposit_event(RawEvent::RecoveryInitiated(who, account));
 		}
 
-		fn vouch_recovery(origin, rescuee: T::AccountId, rescuer: T::AccountId) {
+		fn vouch_recovery(origin, lost: T::AccountId, rescuer: T::AccountId) {
 			let who = ensure_signed(origin)?;
 
-			// Check that there is a recovery setup for this rescuee
-			if let Some(recovery_setup) = Self::recovery_setup(&rescuee) {
+			// Check that the lost account is recoverable
+			if let Some(recovery_config) = Self::recovery_config(&lost) {
 				// Check that the recovery process has been initiated for this rescuer
-				if let Some(mut active_recovery) = Self::active_recovery(&rescuee, &rescuer) {
+				if let Some(mut active_recovery) = Self::active_recovery(&lost, &rescuer) {
 					// Make sure the voter is a friend
-					ensure!(Self::is_friend(&recovery_setup.friends, &who), Error::<T>::NotFriend);
-					// Make sure the delay period has passed
-					let current_block_number = <system::Module<T>>::block_number();
-					ensure!(
-						active_recovery.last + recovery_setup.delay_period >= current_block_number,
-						Error::<T>::DelayPeriod
-					);
+					ensure!(Self::is_friend(&recovery_config.friends, &who), Error::<T>::NotFriend);
 					// Either insert the vouch, or return an error that the user already vouched.
 					match active_recovery.friends.binary_search(&who) {
 						Ok(_pos) => Err(Error::<T>::AlreadyVouched)?,
 						Err(pos) => active_recovery.friends.insert(pos, who.clone()),
 					}
-					// Update the last vote time
-					active_recovery.last = current_block_number;
+
 					// Update storage with the latest details
-					<ActiveRecoveries<T>>::insert(&rescuee, &rescuer, active_recovery);
+					<ActiveRecoveries<T>>::insert(&lost, &rescuer, active_recovery);
 
-					Self::deposit_event(RawEvent::RecoveryVouched(rescuer, rescuee, who));
+					Self::deposit_event(RawEvent::RecoveryVouched(rescuer, lost, who));
 				} else {
 					Err(Error::<T>::NotStarted)?
 				}
 			} else {
-				Err(Error::<T>::NotSetup)?
+				Err(Error::<T>::NotRecoverable)?
 			}
 		}
 
 		/// Allow a rescuer to claim their recovered account.
 		fn claim_recovery(origin, account: T::AccountId) {
 			let who = ensure_signed(origin)?;
-			// Check that there is a recovery setup for this rescuee
-			if let Some(recovery_setup) = Self::recovery_setup(&account) {
+			// Check that the lost account is recoverable
+			if let Some(recovery_config) = Self::recovery_config(&account) {
 				// Check that the recovery process has been initiated for this rescuer
 				if let Some(active_recovery) = Self::active_recovery(&account, &who) {
+					// Make sure the delay period has passed
+					let current_block_number = <system::Module<T>>::block_number();
+					ensure!(
+						active_recovery.created + recovery_config.delay_period >= current_block_number,
+						Error::<T>::DelayPeriod
+					);
 					// Make sure the threshold is met
 					ensure!(
-						recovery_setup.threshold as usize <= active_recovery.friends.len(),
+						recovery_config.threshold as usize <= active_recovery.friends.len(),
 						Error::<T>::Threshold
 					);
 
@@ -289,7 +306,7 @@ decl_module! {
 					Err(Error::<T>::NotStarted)?
 				}
 			} else {
-				Err(Error::<T>::NotSetup)?
+				Err(Error::<T>::NotRecoverable)?
 			}
 		}
 
@@ -299,7 +316,7 @@ decl_module! {
 		fn close_recovery(origin, rescuer: T::AccountId) {
 			let who = ensure_signed(origin)?;
 			if let Some(active_recovery) = <ActiveRecoveries<T>>::take(&who, &rescuer) {
-				// Move the reserved funds from the rescuer to the rescuee account.
+				// Move the reserved funds from the rescuer to the rescued account.
 				// Acts like a slashing mechanism for those who try to maliciously recover accounts.
 				let _ = T::Currency::repatriate_reserved(&rescuer, &who, active_recovery.deposit);
 			} else {
@@ -316,9 +333,9 @@ decl_module! {
 			// Check there are no active recoveries
 			let mut active_recoveries = <ActiveRecoveries<T>>::iter_prefix(&who);
 			ensure!(active_recoveries.next().is_none(), Error::<T>::StillActive);
-			// Check account has recovery setup
-			if let Some(recovery_setup) = <RecoverySetups<T>>::take(&who) {
-				T::Currency::unreserve(&who, recovery_setup.deposit);
+			// Check account is recoverable
+			if let Some(recovery_config) = <Recoverable<T>>::take(&who) {
+				T::Currency::unreserve(&who, recovery_config.deposit);
 
 				Self::deposit_event(RawEvent::RecoveryRemoved(who));
 			}