Introduce frozen indices. (#6307)

* Introduce frozen indices.

* Fix.

* Bump runtime

* Benchmark for freeze

* Fix

* fix benchmarks

* update freeze weights

* remove copy pasta

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

substrate/bin/node/runtime/src/lib.rs

@@ -95,8 +95,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 251,
+	spec_version: 252,
-	impl_version: 2,
+	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,
 };

substrate/frame/indices/src/benchmarking.rs

@@ -83,11 +83,25 @@ benchmarks! {
 		T::Currency::make_free_balance_be(&recipient, BalanceOf::<T>::max_value());
 		// Claim the index
 		Indices::<T>::claim(RawOrigin::Signed(original).into(), account_index)?;
-	}: _(RawOrigin::Root, recipient.clone(), account_index)
+	}: _(RawOrigin::Root, recipient.clone(), account_index, false)
 	verify {
 		assert_eq!(Accounts::<T>::get(account_index).unwrap().0, recipient);
 	}
 
+	freeze {
+		// Index being claimed
+		let i in 0 .. 1000;
+		let account_index = T::AccountIndex::from(i);
+		// Setup accounts
+		let caller: T::AccountId = account("caller", 0, SEED);
+		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
+		// Claim the index
+		Indices::<T>::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?;
+	}: _(RawOrigin::Signed(caller.clone()), account_index)
+	verify {
+		assert_eq!(Accounts::<T>::get(account_index).unwrap().2, true);
+	}
+
 	// TODO in another PR: lookup and unlookup trait weights (not critical)
 }
 
@@ -104,6 +118,7 @@ mod tests {
 			assert_ok!(test_benchmark_transfer::<Test>());
 			assert_ok!(test_benchmark_free::<Test>());
 			assert_ok!(test_benchmark_force_transfer::<Test>());
+			assert_ok!(test_benchmark_freeze::<Test>());
 		});
 	}
 }

substrate/frame/indices/src/lib.rs

@@ -26,7 +26,7 @@ use sp_runtime::traits::{
 	StaticLookup, Member, LookupError, Zero, Saturating, AtLeast32Bit
 };
 use frame_support::{Parameter, decl_module, decl_error, decl_event, decl_storage, ensure};
-use frame_support::dispatch::DispatchResult;
+use frame_support::dispatch::{DispatchResult, Weight};
 use frame_support::traits::{Currency, ReservableCurrency, Get, BalanceStatus::Reserved};
 use frame_support::weights::constants::WEIGHT_PER_MICROS;
 use frame_system::{ensure_signed, ensure_root};
@@ -62,9 +62,9 @@ decl_storage! {
 		pub Accounts build(|config: &GenesisConfig<T>|
 			config.indices.iter()
 				.cloned()
-				.map(|(a, b)| (a, (b, Zero::zero())))
+				.map(|(a, b)| (a, (b, Zero::zero(), false)))
 				.collect::<Vec<_>>()
-		): map hasher(blake2_128_concat) T::AccountIndex => Option<(T::AccountId, BalanceOf<T>)>;
+		): map hasher(blake2_128_concat) T::AccountIndex => Option<(T::AccountId, BalanceOf<T>, bool)>;
 	}
 	add_extra_genesis {
 		config(indices): Vec<(T::AccountIndex, T::AccountId)>;
@@ -80,6 +80,8 @@ decl_event!(
 		IndexAssigned(AccountId, AccountIndex),
 		/// A account index has been freed up (unassigned).
 		IndexFreed(AccountIndex),
+		/// A account index has been frozen to its current account ID.
+		IndexFrozen(AccountIndex, AccountId),
 	}
 );
 
@@ -93,6 +95,8 @@ decl_error! {
 		InUse,
 		/// The source and destination accounts are identical.
 		NotTransfer,
+		/// The index is permanent and may not be freed/changed.
+		Permanent,
 	}
 }
 
@@ -100,6 +104,16 @@ decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin, system = frame_system {
 		fn deposit_event() = default;
 
+		fn on_runtime_upgrade() -> Weight {
+			use frame_support::migration::{StorageIterator, put_storage_value};
+			for (key, value) in StorageIterator::<
+				(T::AccountId, BalanceOf<T>)
+			>::new(b"Indices", b"Accounts").drain() {
+				put_storage_value(b"Indices", b"Accounts", &key, (value.0, value.1, false));
+			}
+			1_000_000_000
+		}
+
 		/// Assign an previously unassigned index.
 		///
 		/// Payment: `Deposit` is reserved from the sender account.
@@ -125,7 +139,7 @@ decl_module! {
 
 			Accounts::<T>::try_mutate(index, |maybe_value| {
 				ensure!(maybe_value.is_none(), Error::<T>::InUse);
-				*maybe_value = Some((who.clone(), T::Deposit::get()));
+				*maybe_value = Some((who.clone(), T::Deposit::get(), false));
 				T::Currency::reserve(&who, T::Deposit::get())
 			})?;
 			Self::deposit_event(RawEvent::IndexAssigned(who, index));
@@ -158,10 +172,11 @@ decl_module! {
 			ensure!(who != new, Error::<T>::NotTransfer);
 
 			Accounts::<T>::try_mutate(index, |maybe_value| -> DispatchResult {
-				let (account, amount) = maybe_value.take().ok_or(Error::<T>::NotAssigned)?;
+				let (account, amount, perm) = maybe_value.take().ok_or(Error::<T>::NotAssigned)?;
+				ensure!(!perm, Error::<T>::Permanent);
 				ensure!(&account == &who, Error::<T>::NotOwner);
 				let lost = T::Currency::repatriate_reserved(&who, &new, amount, Reserved)?;
-				*maybe_value = Some((new.clone(), amount.saturating_sub(lost)));
+				*maybe_value = Some((new.clone(), amount.saturating_sub(lost), false));
 				Ok(())
 			})?;
 			Self::deposit_event(RawEvent::IndexAssigned(new, index));
@@ -191,7 +206,8 @@ decl_module! {
 			let who = ensure_signed(origin)?;
 
 			Accounts::<T>::try_mutate(index, |maybe_value| -> DispatchResult {
-				let (account, amount) = maybe_value.take().ok_or(Error::<T>::NotAssigned)?;
+				let (account, amount, perm) = maybe_value.take().ok_or(Error::<T>::NotAssigned)?;
+				ensure!(!perm, Error::<T>::Permanent);
 				ensure!(&account == &who, Error::<T>::NotOwner);
 				T::Currency::unreserve(&who, amount);
 				Ok(())
@@ -206,6 +222,7 @@ decl_module! {
 		///
 		/// - `index`: the index to be (re-)assigned.
 		/// - `new`: the new owner of the index. This function is a no-op if it is equal to sender.
+		/// - `freeze`: if set to `true`, will freeze the index so it cannot be transferred.
 		///
 		/// Emits `IndexAssigned` if successful.
 		///
@@ -221,17 +238,50 @@ decl_module! {
 		///    - Writes: Indices Accounts, System Account (original owner)
 		/// # </weight>
 		#[weight = T::DbWeight::get().reads_writes(2, 2) + 25 * WEIGHT_PER_MICROS]
-		fn force_transfer(origin, new: T::AccountId, index: T::AccountIndex) {
+		fn force_transfer(origin, new: T::AccountId, index: T::AccountIndex, freeze: bool) {
 			ensure_root(origin)?;
 
 			Accounts::<T>::mutate(index, |maybe_value| {
-				if let Some((account, amount)) = maybe_value.take() {
+				if let Some((account, amount, _)) = maybe_value.take() {
 					T::Currency::unreserve(&account, amount);
 				}
-				*maybe_value = Some((new.clone(), Zero::zero()));
+				*maybe_value = Some((new.clone(), Zero::zero(), freeze));
 			});
 			Self::deposit_event(RawEvent::IndexAssigned(new, index));
 		}
+
+		/// Freeze an index so it will always point to the sender account. This consumes the deposit.
+		///
+		/// The dispatch origin for this call must be _Signed_ and the signing account must have a
+		/// non-frozen account `index`.
+		///
+		/// - `index`: the index to be frozen in place.
+		///
+		/// Emits `IndexFrozen` if successful.
+		///
+		/// # <weight>
+		/// - `O(1)`.
+		/// - One storage mutation (codec `O(1)`).
+		/// - Up to one slash operation.
+		/// - One event.
+		/// -------------------
+		/// - Base Weight: 30.86 µs
+		/// - DB Weight: 1 Read/Write (Accounts)
+		/// # </weight>
+		#[weight = T::DbWeight::get().reads_writes(1, 1) + 30 * WEIGHT_PER_MICROS]
+		fn freeze(origin, index: T::AccountIndex) {
+			let who = ensure_signed(origin)?;
+
+			Accounts::<T>::try_mutate(index, |maybe_value| -> DispatchResult {
+				let (account, amount, perm) = maybe_value.take().ok_or(Error::<T>::NotAssigned)?;
+				ensure!(!perm, Error::<T>::Permanent);
+				ensure!(&account == &who, Error::<T>::NotOwner);
+				T::Currency::slash_reserved(&who, amount);
+				*maybe_value = Some((account, Zero::zero(), true));
+				Ok(())
+			})?;
+			Self::deposit_event(RawEvent::IndexFrozen(index, who));
+		}
 	}
 }
 

substrate/frame/indices/src/tests.rs

@@ -48,6 +48,20 @@ fn freeing_should_work() {
 	});
 }
 
+#[test]
+fn freezing_should_work() {
+	new_test_ext().execute_with(|| {
+		assert_ok!(Indices::claim(Some(1).into(), 0));
+		assert_noop!(Indices::freeze(Some(1).into(), 1), Error::<Test>::NotAssigned);
+		assert_noop!(Indices::freeze(Some(2).into(), 0), Error::<Test>::NotOwner);
+		assert_ok!(Indices::freeze(Some(1).into(), 0));
+		assert_noop!(Indices::freeze(Some(1).into(), 0), Error::<Test>::Permanent);
+
+		assert_noop!(Indices::free(Some(1).into(), 0), Error::<Test>::Permanent);
+		assert_noop!(Indices::transfer(Some(1).into(), 2, 0), Error::<Test>::Permanent);
+	});
+}
+
 #[test]
 fn indexing_lookup_should_work() {
 	new_test_ext().execute_with(|| {
@@ -87,7 +101,7 @@ fn transfer_index_on_accounts_should_work() {
 fn force_transfer_index_on_preowned_should_work() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Indices::claim(Some(1).into(), 0));
-		assert_ok!(Indices::force_transfer(Origin::ROOT, 3, 0));
+		assert_ok!(Indices::force_transfer(Origin::ROOT, 3, 0, false));
 		assert_eq!(Balances::reserved_balance(1), 0);
 		assert_eq!(Balances::reserved_balance(3), 0);
 		assert_eq!(Indices::lookup_index(0), Some(3));
@@ -97,7 +111,7 @@ fn force_transfer_index_on_preowned_should_work() {
 #[test]
 fn force_transfer_index_on_free_should_work() {
 	new_test_ext().execute_with(|| {
-		assert_ok!(Indices::force_transfer(Origin::ROOT, 3, 0));
+		assert_ok!(Indices::force_transfer(Origin::ROOT, 3, 0, false));
 		assert_eq!(Balances::reserved_balance(3), 0);
 		assert_eq!(Indices::lookup_index(0), Some(3));
 	});