From 4b1d86201b07ca6ebc2bfc2fd39262c2f24102b3 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Tue, 9 Jun 2020 16:31:18 +0200 Subject: [PATCH] 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 --- bin/node/runtime/src/lib.rs | 4 +- frame/indices/src/benchmarking.rs | 17 +++++++- frame/indices/src/lib.rs | 70 ++++++++++++++++++++++++++----- frame/indices/src/tests.rs | 18 +++++++- 4 files changed, 94 insertions(+), 15 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 5a56ca4a7b5ea..f92189e3c920a 100644 --- a/bin/node/runtime/src/lib.rs +++ b/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, - impl_version: 2, + spec_version: 252, + impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, }; diff --git a/frame/indices/src/benchmarking.rs b/frame/indices/src/benchmarking.rs index 843e4e2faa592..a6b543bb43f4a 100644 --- a/frame/indices/src/benchmarking.rs +++ b/frame/indices/src/benchmarking.rs @@ -83,11 +83,25 @@ benchmarks! { T::Currency::make_free_balance_be(&recipient, BalanceOf::::max_value()); // Claim the index Indices::::claim(RawOrigin::Signed(original).into(), account_index)?; - }: _(RawOrigin::Root, recipient.clone(), account_index) + }: _(RawOrigin::Root, recipient.clone(), account_index, false) verify { assert_eq!(Accounts::::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::::max_value()); + // Claim the index + Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; + }: _(RawOrigin::Signed(caller.clone()), account_index) + verify { + assert_eq!(Accounts::::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::()); assert_ok!(test_benchmark_free::()); assert_ok!(test_benchmark_force_transfer::()); + assert_ok!(test_benchmark_freeze::()); }); } } diff --git a/frame/indices/src/lib.rs b/frame/indices/src/lib.rs index ef4e0082f4fb6..048a5b9936ad4 100644 --- a/frame/indices/src/lib.rs +++ b/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| config.indices.iter() .cloned() - .map(|(a, b)| (a, (b, Zero::zero()))) + .map(|(a, b)| (a, (b, Zero::zero(), false))) .collect::>() - ): map hasher(blake2_128_concat) T::AccountIndex => Option<(T::AccountId, BalanceOf)>; + ): map hasher(blake2_128_concat) T::AccountIndex => Option<(T::AccountId, BalanceOf, 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 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) + >::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::::try_mutate(index, |maybe_value| { ensure!(maybe_value.is_none(), Error::::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::::NotTransfer); Accounts::::try_mutate(index, |maybe_value| -> DispatchResult { - let (account, amount) = maybe_value.take().ok_or(Error::::NotAssigned)?; + let (account, amount, perm) = maybe_value.take().ok_or(Error::::NotAssigned)?; + ensure!(!perm, Error::::Permanent); ensure!(&account == &who, Error::::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::::try_mutate(index, |maybe_value| -> DispatchResult { - let (account, amount) = maybe_value.take().ok_or(Error::::NotAssigned)?; + let (account, amount, perm) = maybe_value.take().ok_or(Error::::NotAssigned)?; + ensure!(!perm, Error::::Permanent); ensure!(&account == &who, Error::::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 = 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::::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. + /// + /// # + /// - `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 = T::DbWeight::get().reads_writes(1, 1) + 30 * WEIGHT_PER_MICROS] + fn freeze(origin, index: T::AccountIndex) { + let who = ensure_signed(origin)?; + + Accounts::::try_mutate(index, |maybe_value| -> DispatchResult { + let (account, amount, perm) = maybe_value.take().ok_or(Error::::NotAssigned)?; + ensure!(!perm, Error::::Permanent); + ensure!(&account == &who, Error::::NotOwner); + T::Currency::slash_reserved(&who, amount); + *maybe_value = Some((account, Zero::zero(), true)); + Ok(()) + })?; + Self::deposit_event(RawEvent::IndexFrozen(index, who)); + } } } diff --git a/frame/indices/src/tests.rs b/frame/indices/src/tests.rs index 7f416afbd3392..c5fc2b4735d16 100644 --- a/frame/indices/src/tests.rs +++ b/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::::NotAssigned); + assert_noop!(Indices::freeze(Some(2).into(), 0), Error::::NotOwner); + assert_ok!(Indices::freeze(Some(1).into(), 0)); + assert_noop!(Indices::freeze(Some(1).into(), 0), Error::::Permanent); + + assert_noop!(Indices::free(Some(1).into(), 0), Error::::Permanent); + assert_noop!(Indices::transfer(Some(1).into(), 2, 0), Error::::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)); });