Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Introduce frozen indices. #6307

Merged
merged 11 commits into from
Jun 9, 2020
4 changes: 2 additions & 2 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
17 changes: 16 additions & 1 deletion frame/indices/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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>());
});
}
}
70 changes: 60 additions & 10 deletions frame/indices/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)>;
Expand All @@ -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),
}
);

Expand All @@ -93,13 +95,25 @@ decl_error! {
InUse,
/// The source and destination accounts are identical.
NotTransfer,
/// The index is permanent and may not be freed/changed.
Permanent,
}
}

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.
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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(())
Expand All @@ -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.
///
Expand All @@ -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));
}
}
}

Expand Down
18 changes: 16 additions & 2 deletions frame/indices/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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));
Expand All @@ -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));
});
Expand Down