diff --git a/Cargo.lock b/Cargo.lock index dd9fe26c8135..eb2dc22d1e48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10101,6 +10101,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "log", "pallet-balances", "parity-scale-codec", "scale-info", diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 2d49f0b99e95..9f1ab9ac8dd8 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -283,13 +283,21 @@ impl pallet_babe::Config for Runtime { } parameter_types! { + pub const IndicesHoldReason: RuntimeHoldReason = RuntimeHoldReason::Indices(pallet_indices::HoldReason::ClaimedIndex); pub const IndexDeposit: Balance = 100 * CENTS; } impl pallet_indices::Config for Runtime { type AccountIndex = AccountIndex; type Currency = Balances; - type Deposit = IndexDeposit; + type RuntimeHoldReason = RuntimeHoldReason; + type Consideration = HoldConsideration< + AccountId, + Balances, + IndicesHoldReason, + // TODO: check if a different converter is needed + LinearStoragePrice, + >; type RuntimeEvent = RuntimeEvent; type WeightInfo = weights::pallet_indices::WeightInfo; } diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index 51c199b7a054..9e1cdfdefcb4 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -191,15 +191,23 @@ impl pallet_babe::Config for Runtime { } parameter_types! { - pub storage IndexDeposit: Balance = 1 * DOLLARS; + pub const IndicesHoldReason: RuntimeHoldReason = RuntimeHoldReason::Indices(pallet_indices::HoldReason::ClaimedIndex); + pub const IndexDeposit: Balance = 1 * ; } impl pallet_indices::Config for Runtime { type AccountIndex = AccountIndex; type Currency = Balances; - type Deposit = IndexDeposit; + type RuntimeHoldReason = RuntimeHoldReason; + type Consideration = HoldConsideration< + AccountId, + Balances, + IndicesHoldReason, + // TODO: check if a different converter is needed + LinearStoragePrice, + >; type RuntimeEvent = RuntimeEvent; - type WeightInfo = (); + type WeightInfo = weights::pallet_indices::WeightInfo; } parameter_types! { diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 17406f7ae9fc..a42ef9dee47f 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -273,13 +273,21 @@ impl pallet_babe::Config for Runtime { } parameter_types! { + pub const IndicesHoldReason: RuntimeHoldReason = RuntimeHoldReason::Indices(pallet_indices::HoldReason::ClaimedIndex); pub const IndexDeposit: Balance = 100 * CENTS; } impl pallet_indices::Config for Runtime { type AccountIndex = AccountIndex; type Currency = Balances; - type Deposit = IndexDeposit; + type RuntimeHoldReason = RuntimeHoldReason; + type Consideration = HoldConsideration< + AccountId, + Balances, + IndicesHoldReason, + // TODO: check if a different converter is needed + LinearStoragePrice, + >; type RuntimeEvent = RuntimeEvent; type WeightInfo = weights::pallet_indices::WeightInfo; } diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index d7598adb5a22..96b016086281 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -503,10 +503,21 @@ parameter_types! { pub const IndexDeposit: Balance = 1 * DOLLARS; } +parameter_types! { + pub const IndicesHoldReason: RuntimeHoldReason = RuntimeHoldReason::Indices(pallet_indices::HoldReason::ClaimedIndex); +} + impl pallet_indices::Config for Runtime { type AccountIndex = AccountIndex; type Currency = Balances; - type Deposit = IndexDeposit; + type RuntimeHoldReason = RuntimeHoldReason; + type Consideration = HoldConsideration< + AccountId, + Balances, + IndicesHoldReason, + // TODO: check if a different converter is needed + LinearStoragePrice, + >; type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_indices::weights::SubstrateWeight; } diff --git a/substrate/frame/indices/Cargo.toml b/substrate/frame/indices/Cargo.toml index 4f0c780c6af3..6df7ed979abe 100644 --- a/substrate/frame/indices/Cargo.toml +++ b/substrate/frame/indices/Cargo.toml @@ -16,6 +16,7 @@ workspace = true targets = ["x86_64-unknown-linux-gnu"] [dependencies] +log = { version = "0.4.20", default-features = false } codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false, features = ["derive"] } scale-info = { version = "2.10.0", default-features = false, features = ["derive"] } frame-benchmarking = { path = "../benchmarking", default-features = false, optional = true } diff --git a/substrate/frame/indices/src/lib.rs b/substrate/frame/indices/src/lib.rs index ff12d092cfb8..ebc792ccd8f1 100644 --- a/substrate/frame/indices/src/lib.rs +++ b/substrate/frame/indices/src/lib.rs @@ -24,9 +24,15 @@ mod benchmarking; mod mock; mod tests; pub mod weights; +pub mod migrations; use codec::Codec; -use frame_support::traits::{BalanceStatus::Reserved, Currency, ReservableCurrency}; +use frame_support::traits::{ + fungible::{InspectHold, MutateHold, HoldConsiderationFromLegacy}, + Consideration, + Footprint, + StorageVersion, +}; use sp_runtime::{ traits::{AtLeast32Bit, LookupError, Saturating, StaticLookup, Zero}, MultiAddress, @@ -35,11 +41,20 @@ use sp_std::prelude::*; pub use weights::WeightInfo; type BalanceOf = - <::Currency as Currency<::AccountId>>::Balance; + <::Currency as frame_support::traits::fungible::Inspect< + ::AccountId, + >>::Balance; +type TicketOf = ::Consideration; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; pub use pallet::*; +/// The logging target of this pallet. +pub const LOG_TARGET: &'static str = "runtime::indices"; + +/// The current storage version. +const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[frame_support::pallet] pub mod pallet { use super::*; @@ -60,12 +75,17 @@ pub mod pallet { + Copy + MaxEncodedLen; + // TODO: How to inspect held balance only with Considerations? + // Possible to add Currency when #[cfg(any(feature = "try-runtime", test))]? /// The currency trait. - type Currency: ReservableCurrency; + type Currency: MutateHold + + InspectHold; - /// The deposit needed for reserving an index. - #[pallet::constant] - type Deposit: Get>; + /// The overarching runtime hold reason. + type RuntimeHoldReason: From; + + /// A means of providing some cost while data is stored on-chain. + type Consideration: Consideration + HoldConsiderationFromLegacy; /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -75,13 +95,22 @@ pub mod pallet { } #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); + /// A reason for this pallet placing a hold on funds. + #[pallet::composite_enum] + pub enum HoldReason { + /// The funds are held as deposit for claiming an index. + #[codec(index = 0)] + ClaimedIndex, + } + #[pallet::call] impl Pallet { /// Assign an previously unassigned index. /// - /// Payment: `Deposit` is reserved from the sender account. + /// Payment: `Consideration` is held from the sender account. /// /// The dispatch origin for this call must be _Signed_. /// @@ -96,10 +125,18 @@ pub mod pallet { pub fn claim(origin: OriginFor, index: T::AccountIndex) -> DispatchResult { let who = ensure_signed(origin)?; - Accounts::::try_mutate(index, |maybe_value| { + Accounts::::try_mutate(index, |maybe_value| -> DispatchResult { ensure!(maybe_value.is_none(), Error::::InUse); - *maybe_value = Some((who.clone(), T::Deposit::get(), false)); - T::Currency::reserve(&who, T::Deposit::get()) + let ticket = + T::Consideration::new( + &who, + Footprint::from_parts( + 1, + sp_std::mem::size_of::<::AccountId>() as usize + ) + )?; + *maybe_value = Some((who.clone(), Some(ticket), false)); + Ok(()) })?; Self::deposit_event(Event::IndexAssigned { who, index }); Ok(()) @@ -129,11 +166,27 @@ pub mod pallet { ensure!(who != new, Error::::NotTransfer); Accounts::::try_mutate(index, |maybe_value| -> DispatchResult { - let (account, amount, perm) = maybe_value.take().ok_or(Error::::NotAssigned)?; + let (account_id, maybe_ticket, 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), false)); + ensure!(account_id == who, Error::::NotOwner); + + // Done in two steps as `transfer` does not exist for `HoldConsideration` + let maybe_new_ticket = if let Some(ticket) = maybe_ticket { + let new_ticket = + T::Consideration::new( + &new, + Footprint::from_parts( + 1, + sp_std::mem::size_of::<::AccountId>() as usize + ) + )?; + ticket.drop(&account_id)?; + Some(new_ticket) + } else { None }; + + *maybe_value = Some((new.clone(), maybe_new_ticket, false)); + Ok(()) })?; Self::deposit_event(Event::IndexAssigned { who: new, index }); @@ -158,10 +211,13 @@ pub mod pallet { let who = ensure_signed(origin)?; Accounts::::try_mutate(index, |maybe_value| -> DispatchResult { - let (account, amount, perm) = maybe_value.take().ok_or(Error::::NotAssigned)?; + let (account_id, maybe_ticket, perm) = maybe_value.take().ok_or(Error::::NotAssigned)?; ensure!(!perm, Error::::Permanent); - ensure!(account == who, Error::::NotOwner); - T::Currency::unreserve(&who, amount); + ensure!(account_id == who, Error::::NotOwner); + + if let Some(ticket) = maybe_ticket { + ticket.drop(&account_id)?; + }; Ok(()) })?; Self::deposit_event(Event::IndexFreed { index }); @@ -192,12 +248,15 @@ pub mod pallet { ensure_root(origin)?; let new = T::Lookup::lookup(new)?; - Accounts::::mutate(index, |maybe_value| { - if let Some((account, amount, _)) = maybe_value.take() { - T::Currency::unreserve(&account, amount); + Accounts::::try_mutate(index, |maybe_value| -> DispatchResult { + if let Some((account_id, maybe_ticket, _)) = maybe_value.take() { + if let Some(ticket) = maybe_ticket { + ticket.drop(&account_id)?; + } } - *maybe_value = Some((new.clone(), Zero::zero(), freeze)); - }); + *maybe_value = Some((new.clone(), None, freeze)); + Ok(()) + })?; Self::deposit_event(Event::IndexAssigned { who: new, index }); Ok(()) } @@ -220,11 +279,13 @@ pub mod pallet { let who = ensure_signed(origin)?; Accounts::::try_mutate(index, |maybe_value| -> DispatchResult { - let (account, amount, perm) = maybe_value.take().ok_or(Error::::NotAssigned)?; + let (account_id, maybe_ticket, perm) = maybe_value.take().ok_or(Error::::NotAssigned)?; ensure!(!perm, Error::::Permanent); - ensure!(account == who, Error::::NotOwner); - let _ = T::Currency::slash_reserved(&who, amount); - *maybe_value = Some((account, Zero::zero(), true)); + ensure!(account_id == who, Error::::NotOwner); + if let Some(ticket) = maybe_ticket { + ticket.burn(&account_id); + } + *maybe_value = Some((account_id, None, true)); Ok(()) })?; Self::deposit_event(Event::IndexFrozen { index, who }); @@ -260,7 +321,7 @@ pub mod pallet { /// The lookup from index to account. #[pallet::storage] pub type Accounts = - StorageMap<_, Blake2_128Concat, T::AccountIndex, (T::AccountId, BalanceOf, bool)>; + StorageMap<_, Blake2_128Concat, T::AccountIndex, (T::AccountId, Option>, bool)>; #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] @@ -272,10 +333,19 @@ pub mod pallet { impl BuildGenesisConfig for GenesisConfig { fn build(&self) { for (a, b) in &self.indices { - >::insert(a, (b, >::zero(), false)) + >::insert(a, (b, Option::>::None, false)) } } } + + #[pallet::hooks] + impl Hooks> for Pallet { + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + Self::do_try_state()?; + Ok(()) + } + } } impl Pallet { @@ -294,6 +364,27 @@ impl Pallet { _ => None, } } + + /// Ensure the correctness of the state of this pallet. + /// + /// The following assertions must always apply. + /// + /// Invariants: + /// - If the index has been frozen, the ticket should be `None` + #[cfg(any(feature = "try-runtime", test))] + fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { + // Check held amount is zero when an index is frozen + let zero_held = Accounts::::iter() + .filter(|(_, (_, _, perm))| *perm == true ) + .all( + |(_, (_, ticket, _))| + ticket.is_none() + ); + + frame_support::ensure!(zero_held, "Frozen indexes should hold zero balance"); + + Ok(()) + } } impl StaticLookup for Pallet { diff --git a/substrate/frame/indices/src/migrations.rs b/substrate/frame/indices/src/migrations.rs new file mode 100644 index 000000000000..d9a9e8fd0f5f --- /dev/null +++ b/substrate/frame/indices/src/migrations.rs @@ -0,0 +1,181 @@ +// This file is part of Substrate. + +// Copyright (C) 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. + +//! A module that is responsible for migration of storage for Indices pallet + +use super::*; +use frame_support::{ + traits::{OnRuntimeUpgrade, ReservableCurrency}, + Blake2_128Concat, + storage_alias, +}; +use log; + +pub mod v0 { + use super::*; + + pub type BalanceOf = ::AccountId, + >>::Balance; + + // The old `Accounts`, removed in , used in v0 + #[storage_alias] + pub type Accounts = + StorageMap< + Pallet, + Blake2_128Concat, + ::AccountIndex, + (::AccountId, BalanceOf, bool) + >; +} + + +/// Version 1 Migration +/// This migration ensures that: +/// - Deposits are properly removed from `Accounts` +/// - Hold reasons are stored in pallet Balances +pub mod v1 { + use super::*; + use frame_support::pallet_prelude::*; + #[cfg(feature = "try-runtime")] + use sp_std::prelude::*; + + pub struct MigrateToV1 { + _marker: sp_std::marker::PhantomData<(T, OldCurrency)> + } + + impl OnRuntimeUpgrade for MigrateToV1 + where + T: Config, + OldCurrency: 'static + ReservableCurrency<::AccountId>, + BalanceOf: From, + { + fn on_runtime_upgrade() -> Weight { + let current = Pallet::::current_storage_version(); + let onchain = Pallet::::on_chain_storage_version(); + + if current == 1 && onchain == 0 { + // update the version nonetheless. + current.put::>(); + + // TODO: Replace unbound storage iteration by lazy migration or multiblock migration + v0::Accounts::::iter().for_each(|(account_index, (account_id, deposit, perm))| { + let remaining = OldCurrency::unreserve(&account_id, deposit); + + if remaining > Zero::zero() { + log::warn!( + target: LOG_TARGET, + "Account {:?} has some non-unreservable deposit {:?} from a total of {:?} that will remain in reserved.", + account_id, + remaining, + deposit, + ); + } + + let unreserved = deposit.saturating_sub(remaining); + let amount = BalanceOf::::from(unreserved); + + // TODO: is there a way of calculating exactly the same deposit with a Footprint? + let ticket = T::Consideration::new_from_exact( + &account_id, + amount + ).map_err(|err| { + log::error!( + target: LOG_TARGET, + "Failed creating a new Consideration for the account {:?}, reason: {:?}.", + account_id, + err + ); + err + }).ok(); + + Accounts::::set(account_index, Some((account_id, ticket, perm))); + }); + + // TODO: Fix weight when lazy migration or multi block migration is in place + T::DbWeight::get().reads_writes(2, 3) + } else { + log::info!( + target: LOG_TARGET, + "Migration did not execute. This probably should be removed" + ); + T::DbWeight::get().reads(2) + } + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::DispatchError> { + ensure!( + Pallet::::on_chain_storage_version() == 0, + "The onchain storage version must be zero for the migration to execute." + ); + // Count the number of `Accounts` and calculate the total reserved balance + let accounts_info = v0::Accounts::::iter().fold((0, Zero::zero()), |count: (u32, v0::BalanceOf), account| { + let (_, (account_id, deposit, _)) = account; + let (accounts_count, total_reserved) = count; + // Try to unreserve the deposit + // + // TODO: does the state persists between `pre_upgrade` and `post_upgrade`? + // If that's the case I should reserve back `unreserved_deposit` as `can_unreserve()` method + // does not exists + let remaining = OldCurrency::unreserve(&account_id, deposit); + let unreserved_deposit = deposit.saturating_sub(remaining); + + (accounts_count + 1, total_reserved.saturating_add(unreserved_deposit)) + }); + let (accounts_count, total_reserved) = accounts_info; + + Ok((accounts_count, total_reserved).encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::DispatchError> { + ensure!( + Pallet::::on_chain_storage_version() == 1, + "The onchain version must be updated after the migration." + ); + + let (pre_accounts_count, pre_total_reserved) = <(u32, v0::BalanceOf)>::decode(&mut &state[..]).expect("pre_upgrade provides a valid state; qed"); + + // Count the number of `Accounts` and calculate the total held balance + let accounts_info = Accounts::::iter().fold((0, Zero::zero()), |count: (u32, BalanceOf), account| { + let (_, (account_id, _, _)) = account; + let (accounts_count, total_held) = count; + + let held = T::Currency::balance_on_hold(&HoldReason::ClaimedIndex.into(), &account_id); + (accounts_count + 1, total_held.saturating_add(held)) + }); + + let (post_accounts_count, post_total_held) = accounts_info; + + // Number of accounts should remain the same + ensure!( + pre_accounts_count == post_accounts_count, + "The number of migrated accounts should remain" + ); + + // TODO: Need a way of transforming both balances to the same type + // // Total reserved/held amount should remain the same + // ensure!( + // pre_total_reserved == post_total_held, + // "Total real reserved/held amount should remain" + // ); + + Ok(()) + } + } +} diff --git a/substrate/frame/support/src/traits/tokens/fungible/mod.rs b/substrate/frame/support/src/traits/tokens/fungible/mod.rs index ba4a2e5e21a2..7a64537ab1d7 100644 --- a/substrate/frame/support/src/traits/tokens/fungible/mod.rs +++ b/substrate/frame/support/src/traits/tokens/fungible/mod.rs @@ -67,7 +67,7 @@ pub use regular::{ }; use sp_arithmetic::traits::Zero; use sp_core::Get; -use sp_runtime::{traits::Convert, DispatchError}; +use sp_runtime::{traits::Convert, DispatchError, TokenError}; pub use union_of::{NativeFromLeft, NativeOrWithId, UnionOf}; use crate::{ @@ -75,6 +75,47 @@ use crate::{ traits::{Consideration, Footprint}, }; +/// Extension for the `Consideration` trait to migrate legacy `Currency` deposits. +/// +/// Provides a `new_from_exact` method for those types using a `fungible` balance frozen, +/// This method is useful when a new ticket needs to be created with a precise balance, instead of +/// deriving it from a footprint. +pub trait FreezeConsiderationFromLegacy: Consideration +where + A: 'static, + F: 'static + MutateFreeze, +{ + /// Create a ticket for a `new` balance attributable to `who`. This ticket *must* ultimately + /// be consumed through `update` or `drop` once a footprint changes or is removed. + fn new_from_exact(_who: &A, _new: F::Balance) -> Result + where + Self: Sized, + { + // TODO: Choose the right error + Err(DispatchError::Token(TokenError::Unsupported)) + } +} + +/// Extension for `Consideration` trait. +/// Provides a `new_from_exact` method for those types using a `fungible` balance placed on hold. +/// This method is useful when a new ticket needs to be created with a precise balance, instead of +/// deriving it from a footprint. +pub trait HoldConsiderationFromLegacy: Consideration +where + A: 'static, + F: 'static + MutateHold, +{ + /// Create a ticket for a `new` balance attributable to `who`. This ticket *must* ultimately + /// be consumed through `update` or `drop` once a footprint changes or is removed. + fn new_from_exact(_who: &A, _new: F::Balance) -> Result + where + Self: Sized, + { + // TODO: Choose the right error + Err(DispatchError::Other("Unsupported")) + } +} + /// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint. /// /// The aggregate amount frozen under `R::get()` for any account which has multiple tickets, @@ -120,7 +161,21 @@ impl< } } -/// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint. +impl< + A: 'static, + F: 'static + MutateFreeze, + R: 'static + Get, + D: 'static + Convert, + > FreezeConsiderationFromLegacy for FreezeConsideration +{ + fn new_from_exact(who: &A, new: F::Balance) -> Result { + F::increase_frozen(&R::get(), who, new)?; + Ok(Self(new, PhantomData)) + } +} + +/// Consideration method using a `fungible` balance placed on hold as the cost exacted for the +/// footprint. #[derive( CloneNoBound, EqNoBound, @@ -165,6 +220,19 @@ impl< } } +impl< + A: 'static, + F: 'static + MutateHold, + R: 'static + Get, + D: 'static + Convert, + > HoldConsiderationFromLegacy for HoldConsideration +{ + fn new_from_exact(who: &A, new: F::Balance) -> Result { + F::hold(&R::get(), who, new)?; + Ok(Self(new, PhantomData)) + } +} + /// Basic consideration method using a `fungible` balance frozen as the cost exacted for the /// footprint. /// @@ -204,6 +272,19 @@ impl< } } +impl< + A: 'static, + Fx: 'static + MutateFreeze, + Rx: 'static + Get, + D: 'static + Convert, + > FreezeConsiderationFromLegacy for LoneFreezeConsideration +{ + fn new_from_exact(who: &A, new: Fx::Balance) -> Result { + ensure!(Fx::balance_frozen(&Rx::get(), who).is_zero(), DispatchError::Unavailable); + Fx::set_frozen(&Rx::get(), who, new, Polite).map(|_| Self(PhantomData)) + } +} + /// Basic consideration method using a `fungible` balance placed on hold as the cost exacted for the /// footprint. /// @@ -245,3 +326,16 @@ impl< let _ = F::burn_all_held(&R::get(), who, BestEffort, Force); } } + +impl< + A: 'static, + F: 'static + MutateHold, + R: 'static + Get, + D: 'static + Convert, + > HoldConsiderationFromLegacy for LoneHoldConsideration +{ + fn new_from_exact(who: &A, new: F::Balance) -> Result { + ensure!(F::balance_on_hold(&R::get(), who).is_zero(), DispatchError::Unavailable); + F::set_on_hold(&R::get(), who, new).map(|_| Self(PhantomData)) + } +}