From 298931a4681b4d1692c172d62b85699c1d6c79cc Mon Sep 17 00:00:00 2001 From: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Date: Tue, 31 Oct 2023 13:24:19 +0100 Subject: [PATCH] Reduce Impact on Identity Pallet in Migration (#2088) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses https://github.com/paritytech/polkadot-sdk/pull/1814#pullrequestreview-1701818579 - Removes `lock`/`unlock` extrinsics - Moves `reap_identity` and `poke_deposit` into special migration pallet to avoid adding new one-time calls to the Identity pallet --------- Co-authored-by: Bastian Köcher --- Cargo.lock | 1 + polkadot/runtime/common/Cargo.toml | 4 + .../runtime/common/src/identity_migrator.rs | 137 +++++++++++ polkadot/runtime/common/src/lib.rs | 1 + polkadot/runtime/rococo/src/impls.rs | 14 +- polkadot/runtime/rococo/src/lib.rs | 18 +- .../rococo/src/weights/pallet_identity.rs | 22 -- polkadot/runtime/westend/src/lib.rs | 3 - .../westend/src/weights/pallet_identity.rs | 22 -- substrate/bin/node/runtime/src/lib.rs | 3 - substrate/frame/alliance/src/mock.rs | 3 - substrate/frame/identity/src/benchmarking.rs | 60 ++--- substrate/frame/identity/src/lib.rs | 230 ++++++------------ substrate/frame/identity/src/tests.rs | 103 +------- substrate/frame/identity/src/weights.rs | 46 ---- 15 files changed, 254 insertions(+), 413 deletions(-) create mode 100644 polkadot/runtime/common/src/identity_migrator.rs diff --git a/Cargo.lock b/Cargo.lock index a8d679c6ce8b..9ff20b1e1017 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12721,6 +12721,7 @@ dependencies = [ "pallet-balances", "pallet-election-provider-multi-phase", "pallet-fast-unstake", + "pallet-identity", "pallet-session", "pallet-staking", "pallet-staking-reward-fn", diff --git a/polkadot/runtime/common/Cargo.toml b/polkadot/runtime/common/Cargo.toml index 18332156d99a..f34fd71af28e 100644 --- a/polkadot/runtime/common/Cargo.toml +++ b/polkadot/runtime/common/Cargo.toml @@ -30,6 +30,7 @@ sp-npos-elections = { path = "../../../substrate/primitives/npos-elections", def pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false } pallet-balances = { path = "../../../substrate/frame/balances", default-features = false } pallet-fast-unstake = { path = "../../../substrate/frame/fast-unstake", default-features = false } +pallet-identity = { path = "../../../substrate/frame/identity", default-features = false } pallet-session = { path = "../../../substrate/frame/session", default-features = false } frame-support = { path = "../../../substrate/frame/support", default-features = false } pallet-staking = { path = "../../../substrate/frame/staking", default-features = false } @@ -85,6 +86,7 @@ std = [ "pallet-balances/std", "pallet-election-provider-multi-phase/std", "pallet-fast-unstake/std", + "pallet-identity/std", "pallet-session/std", "pallet-staking-reward-fn/std", "pallet-staking/std", @@ -124,6 +126,7 @@ runtime-benchmarks = [ "pallet-balances/runtime-benchmarks", "pallet-election-provider-multi-phase/runtime-benchmarks", "pallet-fast-unstake/runtime-benchmarks", + "pallet-identity/runtime-benchmarks", "pallet-staking/runtime-benchmarks", "pallet-timestamp/runtime-benchmarks", "pallet-treasury/runtime-benchmarks", @@ -147,6 +150,7 @@ try-runtime = [ "pallet-balances/try-runtime", "pallet-election-provider-multi-phase/try-runtime", "pallet-fast-unstake/try-runtime", + "pallet-identity/try-runtime", "pallet-session/try-runtime", "pallet-staking/try-runtime", "pallet-timestamp/try-runtime", diff --git a/polkadot/runtime/common/src/identity_migrator.rs b/polkadot/runtime/common/src/identity_migrator.rs new file mode 100644 index 000000000000..d3ccc8df7803 --- /dev/null +++ b/polkadot/runtime/common/src/identity_migrator.rs @@ -0,0 +1,137 @@ +// 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. + +//! This pallet is designed to go into a source chain and destination chain to migrate data. The +//! design motivations are: +//! +//! - Call some function on the source chain that executes some migration (clearing state, +//! forwarding an XCM program). +//! - Call some function (probably from an XCM program) on the destination chain. +//! - Avoid cluttering the source pallet with new dispatchables that are unrelated to its +//! functionality and only used for migration. +//! +//! After the migration is complete, the pallet may be removed from both chains' runtimes. + +use frame_support::{dispatch::DispatchResult, traits::Currency}; +pub use pallet::*; +use pallet_identity::{self, WeightInfo}; +use sp_core::Get; + +type BalanceOf = <::Currency as Currency< + ::AccountId, +>>::Balance; + +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::{ + dispatch::{DispatchResultWithPostInfo, PostDispatchInfo}, + pallet_prelude::*, + traits::EnsureOrigin, + }; + use frame_system::pallet_prelude::*; + + #[pallet::pallet] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config + pallet_identity::Config { + /// Overarching event type. + type RuntimeEvent: From> + IsType<::RuntimeEvent>; + + /// The origin that can reap identities. Expected to be `EnsureSigned` on the + /// source chain such that anyone can all this function. + type Reaper: EnsureOrigin; + + /// A handler for what to do when an identity is reaped. + type ReapIdentityHandler: OnReapIdentity; + + /// Weight information for the extrinsics in the pallet. + type WeightInfo: pallet_identity::WeightInfo; + } + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// The identity and all sub accounts were reaped for `who`. + IdentityReaped { who: T::AccountId }, + /// The deposits held for `who` were updated. `identity` is the new deposit held for + /// identity info, and `subs` is the new deposit held for the sub-accounts. + DepositUpdated { who: T::AccountId, identity: BalanceOf, subs: BalanceOf }, + } + + #[pallet::call] + impl Pallet { + /// Reap the Identity Info of `who` from the Relay Chain, unreserving any deposits held and + /// removing storage items associated with `who`. + #[pallet::call_index(0)] + #[pallet::weight(::WeightInfo::reap_identity( + T::MaxRegistrars::get(), + T::MaxSubAccounts::get() + ))] + pub fn reap_identity( + origin: OriginFor, + who: T::AccountId, + ) -> DispatchResultWithPostInfo { + T::Reaper::ensure_origin(origin)?; + let (registrars, fields, subs) = pallet_identity::Pallet::::reap_identity(&who)?; + T::ReapIdentityHandler::on_reap_identity(&who, fields, subs)?; + Self::deposit_event(Event::IdentityReaped { who }); + let post = PostDispatchInfo { + actual_weight: Some(::WeightInfo::reap_identity( + registrars, subs, + )), + pays_fee: Pays::No, + }; + Ok(post) + } + + /// Update the deposit of `who`. Meant to be called by the system with an XCM `Transact` + /// Instruction. + #[pallet::call_index(1)] + #[pallet::weight(::WeightInfo::poke_deposit())] + pub fn poke_deposit(origin: OriginFor, who: T::AccountId) -> DispatchResultWithPostInfo { + ensure_root(origin)?; + let (id_deposit, subs_deposit) = pallet_identity::Pallet::::poke_deposit(&who)?; + Self::deposit_event(Event::DepositUpdated { + who, + identity: id_deposit, + subs: subs_deposit, + }); + Ok(Pays::No.into()) + } + } +} + +/// Trait to handle reaping identity from state. +pub trait OnReapIdentity { + /// What to do when an identity is reaped. For example, the implementation could send an XCM + /// program to another chain. Concretely, a type implementing this trait in the Polkadot + /// runtime would teleport enough DOT to the People Chain to cover the Identity deposit there. + /// + /// This could also directly include `Transact { poke_deposit(..), ..}`. + /// + /// Inputs + /// - `who`: Whose identity was reaped. + /// - `fields`: The number of `additional_fields` they had. + /// - `subs`: The number of sub-accounts they had. + fn on_reap_identity(who: &AccountId, fields: u32, subs: u32) -> DispatchResult; +} + +impl OnReapIdentity for () { + fn on_reap_identity(_who: &AccountId, _fields: u32, _subs: u32) -> DispatchResult { + Ok(()) + } +} diff --git a/polkadot/runtime/common/src/lib.rs b/polkadot/runtime/common/src/lib.rs index 46a9f7d12cd4..2788c35a7409 100644 --- a/polkadot/runtime/common/src/lib.rs +++ b/polkadot/runtime/common/src/lib.rs @@ -23,6 +23,7 @@ pub mod auctions; pub mod claims; pub mod crowdloan; pub mod elections; +pub mod identity_migrator; pub mod impls; pub mod paras_registrar; pub mod paras_sudo_wrapper; diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs index 7252f79cb2ea..0f33979f128d 100644 --- a/polkadot/runtime/rococo/src/impls.rs +++ b/polkadot/runtime/rococo/src/impls.rs @@ -17,10 +17,10 @@ use crate::xcm_config; use frame_support::pallet_prelude::DispatchResult; use frame_system::RawOrigin; -use pallet_identity::OnReapIdentity; use parity_scale_codec::{Decode, Encode}; use primitives::Balance; use rococo_runtime_constants::currency::*; +use runtime_common::identity_migrator::OnReapIdentity; use sp_std::{marker::PhantomData, prelude::*}; use xcm::{latest::prelude::*, VersionedMultiLocation, VersionedXcm}; use xcm_executor::traits::TransactAsset; @@ -29,14 +29,14 @@ use xcm_executor::traits::TransactAsset; /// remote calls. #[derive(Encode, Decode)] enum PeopleRuntimePallets { - #[codec(index = 50)] - Identity(IdentityCalls), + #[codec(index = 248)] + IdentityMigrator(IdentityMigratorCalls), } /// Call encoding for the calls needed from the Identity pallet. #[derive(Encode, Decode)] -enum IdentityCalls { - #[codec(index = 16)] +enum IdentityMigratorCalls { + #[codec(index = 1)] PokeDeposit(AccountId), } @@ -78,7 +78,7 @@ where AccountId: Into<[u8; 32]> + Clone + Encode, { fn on_reap_identity(who: &AccountId, fields: u32, subs: u32) -> DispatchResult { - use crate::impls::IdentityCalls::PokeDeposit; + use crate::impls::IdentityMigratorCalls::PokeDeposit; let total_to_send = Self::calculate_remote_deposit(fields, subs); @@ -114,7 +114,7 @@ where }] .into(); - let poke = PeopleRuntimePallets::::Identity(PokeDeposit(who.clone())); + let poke = PeopleRuntimePallets::::IdentityMigrator(PokeDeposit(who.clone())); // Actual program to execute on People Chain. let program: Xcm<()> = Xcm(vec![ diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 2790690c9f52..8be97ce67a0a 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -30,7 +30,7 @@ use primitives::{ ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, PARACHAIN_KEY_TYPE_ID, }; use runtime_common::{ - assigned_slots, auctions, claims, crowdloan, impl_runtime_weights, + assigned_slots, auctions, claims, crowdloan, identity_migrator, impl_runtime_weights, impls::{ LocatableAssetConverter, ToAuthor, VersionedLocatableAsset, VersionedMultiLocationConverter, }, @@ -620,11 +620,6 @@ impl pallet_identity::Config for Runtime { type Slashed = Treasury; type ForceOrigin = EitherOf, GeneralAdmin>; type RegistrarOrigin = EitherOf, GeneralAdmin>; - // Should be `EnsureRoot` until the parachain launches with Identity state. Then, it will be - // `EnsureSigned` to allow deposit migration. - type ReapOrigin = EnsureRoot; - type ReapIdentityHandler = ToParachainIdentityReaper; - type LockerOrigin = EnsureRoot; type WeightInfo = weights::pallet_identity::WeightInfo; } @@ -1080,6 +1075,14 @@ impl auctions::Config for Runtime { type WeightInfo = weights::runtime_common_auctions::WeightInfo; } +impl identity_migrator::Config for Runtime { + type RuntimeEvent = RuntimeEvent; + // To be changed to `EnsureSigned` once there is a People Chain to migrate to. + type Reaper = EnsureRoot; + type ReapIdentityHandler = ToParachainIdentityReaper; + type WeightInfo = weights::pallet_identity::WeightInfo; +} + type NisCounterpartInstance = pallet_balances::Instance2; impl pallet_balances::Config for Runtime { type Balance = Balance; @@ -1368,6 +1371,9 @@ construct_runtime! { // Pallet for sending XCM. XcmPallet: pallet_xcm::{Pallet, Call, Storage, Event, Origin, Config} = 99, + // Pallet for migrating Identity to a parachain. To be removed post-migration. + IdentityMigrator: identity_migrator::{Pallet, Call, Event} = 248, + ParasSudoWrapper: paras_sudo_wrapper::{Pallet, Call} = 250, AssignedSlots: assigned_slots::{Pallet, Call, Storage, Event, Config} = 251, diff --git a/polkadot/runtime/rococo/src/weights/pallet_identity.rs b/polkadot/runtime/rococo/src/weights/pallet_identity.rs index 8da3ce4c239c..e2d33ebb3e0c 100644 --- a/polkadot/runtime/rococo/src/weights/pallet_identity.rs +++ b/polkadot/runtime/rococo/src/weights/pallet_identity.rs @@ -402,26 +402,4 @@ impl pallet_identity::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().writes(3)) } - /// Storage: `Identity::Locked` (r:0 w:1) - /// Proof: `Identity::Locked` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) - fn lock_pallet() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 43_286_000 picoseconds. - Weight::from_parts(51_523_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } - /// Storage: `Identity::Locked` (r:0 w:1) - /// Proof: `Identity::Locked` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) - fn unlock_pallet() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 42_243_000 picoseconds. - Weight::from_parts(52_907_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 76affb68337f..9ee4f3cf23e5 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -881,9 +881,6 @@ impl pallet_identity::Config for Runtime { type MaxRegistrars = MaxRegistrars; type ForceOrigin = EitherOf, GeneralAdmin>; type RegistrarOrigin = EitherOf, GeneralAdmin>; - type ReapOrigin = EnsureRoot; - type ReapIdentityHandler = (); - type LockerOrigin = EnsureRoot; type WeightInfo = weights::pallet_identity::WeightInfo; } diff --git a/polkadot/runtime/westend/src/weights/pallet_identity.rs b/polkadot/runtime/westend/src/weights/pallet_identity.rs index 8cc91af62d49..d120b0fc5c1f 100644 --- a/polkadot/runtime/westend/src/weights/pallet_identity.rs +++ b/polkadot/runtime/westend/src/weights/pallet_identity.rs @@ -407,26 +407,4 @@ impl pallet_identity::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().writes(3)) } - /// Storage: `Identity::Locked` (r:0 w:1) - /// Proof: `Identity::Locked` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) - fn lock_pallet() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 43_286_000 picoseconds. - Weight::from_parts(51_523_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } - /// Storage: `Identity::Locked` (r:0 w:1) - /// Proof: `Identity::Locked` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) - fn unlock_pallet() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 42_243_000 picoseconds. - Weight::from_parts(52_907_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } } diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 9262bde7f048..f4c8a5940a3c 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1480,9 +1480,6 @@ impl pallet_identity::Config for Runtime { type Slashed = Treasury; type ForceOrigin = EnsureRootOrHalfCouncil; type RegistrarOrigin = EnsureRootOrHalfCouncil; - type ReapOrigin = EnsureRoot; - type ReapIdentityHandler = (); - type LockerOrigin = EnsureRoot; type WeightInfo = pallet_identity::weights::SubstrateWeight; } diff --git a/substrate/frame/alliance/src/mock.rs b/substrate/frame/alliance/src/mock.rs index b0fa378556b4..a5970bc7af67 100644 --- a/substrate/frame/alliance/src/mock.rs +++ b/substrate/frame/alliance/src/mock.rs @@ -125,10 +125,7 @@ impl pallet_identity::Config for Test { type MaxRegistrars = MaxRegistrars; type Slashed = (); type RegistrarOrigin = EnsureOneOrRoot; - type ReapOrigin = EnsureOneOrRoot; - type ReapIdentityHandler = (); type ForceOrigin = EnsureTwoOrRoot; - type LockerOrigin = EnsureTwoOrRoot; type WeightInfo = (); } diff --git a/substrate/frame/identity/src/benchmarking.rs b/substrate/frame/identity/src/benchmarking.rs index df8bc7dd6dc8..906c185e1cc6 100644 --- a/substrate/frame/identity/src/benchmarking.rs +++ b/substrate/frame/identity/src/benchmarking.rs @@ -556,10 +556,6 @@ mod benchmarks { let target_lookup = T::Lookup::unlookup(target.clone()); let _ = T::Currency::make_free_balance_be(&target, BalanceOf::::max_value()); - //origin - let origin = - T::ReapOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - // set identity let info = T::IdentityInformation::create_identity_info(1); Identity::::set_identity( @@ -586,19 +582,20 @@ mod benchmarks { )?; } - #[extrinsic_call] - _(origin as T::RuntimeOrigin, target_lookup); + #[block] + { + let _ = Identity::::reap_identity(&target); + } - assert_last_event::(Event::::IdentityReaped { who: target }.into()); + assert!(IdentityOf::::get(&target).is_none()); + assert_eq!(SubsOf::::get(&target).0, Zero::zero()); Ok(()) } #[benchmark] fn poke_deposit() -> Result<(), BenchmarkError> { - let caller: T::AccountId = whitelisted_caller(); let target: T::AccountId = account("target", 0, SEED); - let target_lookup = T::Lookup::unlookup(target.clone()); let _ = T::Currency::make_free_balance_be(&target, BalanceOf::::max_value()); let additional_fields = 0; @@ -626,45 +623,16 @@ mod benchmarks { .saturating_add(T::BasicDeposit::get()); let expected_sub_deposit = T::SubAccountDeposit::get(); // only 1 - #[extrinsic_call] - _(RawOrigin::Signed(caller), target_lookup); - - assert_last_event::( - Event::::DepositUpdated { - who: target, - identity: expected_id_deposit, - subs: expected_sub_deposit, - } - .into(), - ); - - Ok(()) - } - - #[benchmark] - fn lock_pallet() -> Result<(), BenchmarkError> { - let origin = - T::LockerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - Identity::::unlock_pallet(origin.clone())?; - - #[extrinsic_call] - _(origin as T::RuntimeOrigin); - - assert!(Locked::::get()); - - Ok(()) - } - - #[benchmark] - fn unlock_pallet() -> Result<(), BenchmarkError> { - let origin = - T::LockerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - Identity::::lock_pallet(origin.clone())?; + #[block] + { + let _ = Identity::::poke_deposit(&target); + } - #[extrinsic_call] - _(origin as T::RuntimeOrigin); + let info = IdentityOf::::get(&target).unwrap(); + assert_eq!(info.deposit, expected_id_deposit); - assert!(!Locked::::get()); + let subs = SubsOf::::get(&target); + assert_eq!(subs.0, expected_sub_deposit); Ok(()) } diff --git a/substrate/frame/identity/src/lib.rs b/substrate/frame/identity/src/lib.rs index 8ec05c786bf8..17c9d7789f3d 100644 --- a/substrate/frame/identity/src/lib.rs +++ b/substrate/frame/identity/src/lib.rs @@ -80,7 +80,8 @@ mod types; pub mod weights; use frame_support::{ - pallet_prelude::DispatchResult, + ensure, + pallet_prelude::{DispatchError, DispatchResult, DispatchResultWithPostInfo}, traits::{BalanceStatus, Currency, Get, OnUnbalanced, ReservableCurrency}, }; use sp_runtime::traits::{AppendZerosInput, Hash, Saturating, StaticLookup, Zero}; @@ -154,15 +155,6 @@ pub mod pallet { /// The origin which may add or remove registrars. Root can always do this. type RegistrarOrigin: EnsureOrigin; - /// The origin that can reap an account's identity info. - type ReapOrigin: EnsureOrigin; - - /// A handler for what to do when an identity is reaped. - type ReapIdentityHandler: OnReapIdentity; - - /// The origin that can lock calls to the pallet. - type LockerOrigin: EnsureOrigin; - /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -297,11 +289,6 @@ pub mod pallet { /// A sub-identity was cleared, and the given deposit repatriated from the /// main identity account to the sub-identity account. SubIdentityRevoked { sub: T::AccountId, main: T::AccountId, deposit: BalanceOf }, - /// The identity and all sub accounts were reaped for `who`. - IdentityReaped { who: T::AccountId }, - /// The deposits held for `who` were updated. `identity` is the new deposit held for - /// identity info, and `subs` is the new deposit held for the sub-accounts. - DepositUpdated { who: T::AccountId, identity: BalanceOf, subs: BalanceOf }, } #[pallet::call] @@ -961,129 +948,6 @@ pub mod pallet { }); Ok(()) } - - /// Reap an identity, clearing associated storage items and refunding any deposits. Calls - /// the `ReapIdentityHandler`, which can be implemented in the runtime. This function is - /// very similar to (a) `clear_identity`, but called on a `target` account instead of self; - /// and (b) `kill_identity`, but without imposing a slash. - /// - /// Parameters: - /// - `target`: The account for which to reap identity state. - /// - /// Origin must be the `ReapOrigin`. - #[pallet::call_index(15)] - #[pallet::weight(T::WeightInfo::reap_identity( - T::MaxRegistrars::get(), - T::MaxSubAccounts::get() - ))] - pub fn reap_identity( - origin: OriginFor, - target: AccountIdLookupOf, - ) -> DispatchResultWithPostInfo { - // No locked check: used for migration. - // `ReapOrigin` to be set to `EnsureSigned` (i.e. anyone) on chain where we - // want to reap data (i.e. Relay) and `EnsureRoot` on chains where we want "normal" - // functionality (i.e. the People Chain). - T::ReapOrigin::ensure_origin(origin)?; - let who = T::Lookup::lookup(target)?; - - // `take` any storage items keyed by `target` - let id = >::take(&who).ok_or(Error::::NotNamed)?; - let registrars = id.judgements.len() as u32; - #[allow(deprecated)] - let fields = id.info.additional() as u32; - let (subs_deposit, sub_ids) = >::take(&who); - // check witness data - let actual_subs = sub_ids.len() as u32; - for sub in sub_ids.iter() { - >::remove(sub); - } - - // unreserve any deposits - let deposit = id.total_deposit().saturating_add(subs_deposit); - let err_amount = T::Currency::unreserve(&who, deposit); - debug_assert!(err_amount.is_zero()); - - // Finally, call the handler. - T::ReapIdentityHandler::on_reap_identity(&who, fields, actual_subs)?; - Self::deposit_event(Event::IdentityReaped { who }); - Ok(Some(T::WeightInfo::reap_identity(registrars, actual_subs)).into()) - } - - /// Update the deposits held by `target` for its identity info. - /// - /// Parameters: - /// - `target`: The account for which to update deposits. - /// - /// May be called by any signed origin. - #[pallet::call_index(16)] - #[pallet::weight(T::WeightInfo::poke_deposit())] - pub fn poke_deposit(origin: OriginFor, target: AccountIdLookupOf) -> DispatchResult { - // No locked check: used for migration. - // anyone or root (so that the system can call it for identity migration) - let _ = ensure_signed_or_root(origin)?; - let target = T::Lookup::lookup(target)?; - - // Identity Deposit - ensure!(IdentityOf::::contains_key(&target), Error::::NoIdentity); - let new_id_deposit = IdentityOf::::try_mutate( - &target, - |registration| -> Result, DispatchError> { - let reg = registration.as_mut().ok_or(Error::::NoIdentity)?; - // Calculate what deposit should be - #[allow(deprecated)] - let field_deposit = T::FieldDeposit::get() - .saturating_mul(BalanceOf::::from(reg.info.additional() as u32)); - let new_id_deposit = T::BasicDeposit::get().saturating_add(field_deposit); - - // Update account - Self::rejig_deposit(&target, reg.deposit, new_id_deposit)?; - - reg.deposit = new_id_deposit; - Ok(new_id_deposit) - }, - )?; - - // Subs Deposit - let new_subs_deposit = SubsOf::::try_mutate( - &target, - |(current_subs_deposit, subs_of)| -> Result, DispatchError> { - let new_subs_deposit = Self::subs_deposit(subs_of.len() as u32); - Self::rejig_deposit(&target, *current_subs_deposit, new_subs_deposit)?; - *current_subs_deposit = new_subs_deposit; - Ok(new_subs_deposit) - }, - )?; - - Self::deposit_event(Event::DepositUpdated { - who: target, - identity: new_id_deposit, - subs: new_subs_deposit, - }); - Ok(()) - } - - /// Lock the pallet. - /// - /// Origin must be the `LockerOrigin`. - #[pallet::call_index(17)] - #[pallet::weight(T::WeightInfo::lock_pallet())] - pub fn lock_pallet(origin: OriginFor) -> DispatchResult { - T::LockerOrigin::ensure_origin(origin)?; - Locked::::put(true); - Ok(()) - } - - /// Unlock the pallet. - /// - /// Origin must be the `LockerOrigin`. - #[pallet::call_index(18)] - #[pallet::weight(T::WeightInfo::unlock_pallet())] - pub fn unlock_pallet(origin: OriginFor) -> DispatchResult { - T::LockerOrigin::ensure_origin(origin)?; - Locked::::put(false); - Ok(()) - } } } @@ -1122,25 +986,83 @@ impl Pallet { IdentityOf::::get(who) .map_or(false, |registration| (registration.info.has_identity(fields))) } -} -/// Trait to handle reaping identity from state. -pub trait OnReapIdentity { - /// What to do when an identity is reaped. For example, the implementation could send an XCM - /// program to another chain. Concretely, a type implementing this trait in the Polkadot - /// runtime would teleport enough DOT to the People Chain to cover the Identity deposit there. + /// Reap an identity, clearing associated storage items and refunding any deposits. This + /// function is very similar to (a) `clear_identity`, but called on a `target` account instead + /// of self; and (b) `kill_identity`, but without imposing a slash. /// - /// This could also directly include `Transact { poke_deposit(..), ..}`. + /// Parameters: + /// - `target`: The account for which to reap identity state. /// - /// Inputs - /// - `who`: Whose identity was reaped. - /// - `fields`: The number of `additional_fields` they had. - /// - `subs`: The number of sub-accounts they had. - fn on_reap_identity(who: &AccountId, fields: u32, subs: u32) -> DispatchResult; -} + /// Return type is a tuple of the number of registrars, additional fields, and sub accounts, + /// respectively. + /// + /// NOTE: This function is here temporarily for migration of Identity info from the Polkadot + /// Relay Chain into a system parachain. It will be removed after the migration. + pub fn reap_identity(who: &T::AccountId) -> Result<(u32, u32, u32), DispatchError> { + // `take` any storage items keyed by `target` + // identity + let id = >::take(&who).ok_or(Error::::NotNamed)?; + let registrars = id.judgements.len() as u32; + #[allow(deprecated)] + let fields = id.info.additional() as u32; + + // subs + let (subs_deposit, sub_ids) = >::take(&who); + let actual_subs = sub_ids.len() as u32; + for sub in sub_ids.iter() { + >::remove(sub); + } -impl OnReapIdentity for () { - fn on_reap_identity(_who: &AccountId, _fields: u32, _subs: u32) -> DispatchResult { - Ok(()) + // unreserve any deposits + let deposit = id.total_deposit().saturating_add(subs_deposit); + let err_amount = T::Currency::unreserve(&who, deposit); + debug_assert!(err_amount.is_zero()); + Ok((registrars, fields, actual_subs)) + } + + /// Update the deposits held by `target` for its identity info. + /// + /// Parameters: + /// - `target`: The account for which to update deposits. + /// + /// Return type is a tuple of the new Identity and Subs deposits, respectively. + /// + /// NOTE: This function is here temporarily for migration of Identity info from the Polkadot + /// Relay Chain into a system parachain. It will be removed after the migration. + pub fn poke_deposit( + target: &T::AccountId, + ) -> Result<(BalanceOf, BalanceOf), DispatchError> { + // Identity Deposit + ensure!(IdentityOf::::contains_key(&target), Error::::NoIdentity); + let new_id_deposit = IdentityOf::::try_mutate( + &target, + |registration| -> Result, DispatchError> { + let reg = registration.as_mut().ok_or(Error::::NoIdentity)?; + // Calculate what deposit should be + #[allow(deprecated)] + let field_deposit = T::FieldDeposit::get() + .saturating_mul(BalanceOf::::from(reg.info.additional() as u32)); + let new_id_deposit = T::BasicDeposit::get().saturating_add(field_deposit); + + // Update account + Self::rejig_deposit(&target, reg.deposit, new_id_deposit)?; + + reg.deposit = new_id_deposit; + Ok(new_id_deposit) + }, + )?; + + // Subs Deposit + let new_subs_deposit = SubsOf::::try_mutate( + &target, + |(current_subs_deposit, subs_of)| -> Result, DispatchError> { + let new_subs_deposit = Self::subs_deposit(subs_of.len() as u32); + Self::rejig_deposit(&target, *current_subs_deposit, new_subs_deposit)?; + *current_subs_deposit = new_subs_deposit; + Ok(new_subs_deposit) + }, + )?; + Ok((new_id_deposit, new_subs_deposit)) } } diff --git a/substrate/frame/identity/src/tests.rs b/substrate/frame/identity/src/tests.rs index 172f83e5fceb..3f21ee29913f 100644 --- a/substrate/frame/identity/src/tests.rs +++ b/substrate/frame/identity/src/tests.rs @@ -113,10 +113,7 @@ impl pallet_identity::Config for Test { type IdentityInformation = IdentityInfo; type MaxRegistrars = MaxRegistrars; type RegistrarOrigin = EnsureOneOrRoot; - type ReapOrigin = EnsureOneOrRoot; - type ReapIdentityHandler = (); type ForceOrigin = EnsureTwoOrRoot; - type LockerOrigin = EnsureOneOrRoot; type WeightInfo = (); } @@ -677,7 +674,7 @@ fn reap_identity_works() { )); // 10 for identity, 10 for sub assert_eq!(Balances::free_balance(10), 80); - assert_ok!(Identity::reap_identity(RuntimeOrigin::signed(1), 10)); + assert_ok!(Identity::reap_identity(&10)); // no identity or subs assert!(Identity::identity(10).is_none()); assert!(Identity::super_of(20).is_none()); @@ -705,7 +702,7 @@ fn poke_deposit_works() { assert_eq!(Balances::free_balance(10), 100); // poke - assert_ok!(Identity::poke_deposit(RuntimeOrigin::signed(1), 10)); + assert_ok!(Identity::poke_deposit(&10)); // free balance reduced by 20 assert_eq!(Balances::free_balance(10), 80); @@ -718,99 +715,3 @@ fn poke_deposit_works() { assert_eq!(Identity::subs_of(10), (10, vec![20].try_into().unwrap())); }); } - -#[test] -fn pallet_locks_and_unlocks() { - new_test_ext().execute_with(|| { - // Can add registrars and reap - let one_as_origin = RuntimeOrigin::signed(1); - // Killer - let two_as_origin = RuntimeOrigin::signed(2); - // Registrar - let three_as_origin = RuntimeOrigin::signed(3); - // Sets identity - let ten_as_origin = RuntimeOrigin::signed(10); - // Sets identity - let twenty_as_origin = RuntimeOrigin::signed(20); - // Sub data to use - let data = Data::Raw(vec![40; 1].try_into().unwrap()); - - // Set some state before locking so that calls are sensible - assert_ok!(Identity::add_registrar(one_as_origin.clone(), 3)); - assert_ok!(Identity::set_identity(ten_as_origin.clone(), Box::new(ten()))); - assert_ok!(Identity::request_judgement(ten_as_origin.clone(), 0, 10)); - - // Lock - assert_ok!(Identity::lock_pallet(one_as_origin.clone())); - - // Almost everything is uncallable - assert_noop!( - Identity::add_registrar(one_as_origin.clone(), 1), - Error::::PalletLocked - ); - assert_noop!( - Identity::set_identity(twenty_as_origin.clone(), Box::new(twenty())), - Error::::PalletLocked - ); - assert_noop!( - Identity::set_subs(ten_as_origin.clone(), vec![(20, data.clone())]), - Error::::PalletLocked - ); - assert_noop!(Identity::clear_identity(ten_as_origin.clone()), Error::::PalletLocked); - assert_noop!( - Identity::request_judgement(twenty_as_origin.clone(), 0, 10), - Error::::PalletLocked - ); - assert_noop!( - Identity::cancel_request(twenty_as_origin.clone(), 0), - Error::::PalletLocked - ); - assert_noop!( - Identity::set_fee(three_as_origin.clone(), 0, 10), - Error::::PalletLocked - ); - assert_noop!( - Identity::set_account_id(three_as_origin.clone(), 0, 4), - Error::::PalletLocked - ); - assert_noop!( - Identity::set_fields( - three_as_origin.clone(), - 0, - IdentityFields(SimpleIdentityField::Display | SimpleIdentityField::Legal) - ), - Error::::PalletLocked - ); - assert_noop!( - Identity::provide_judgement( - three_as_origin, - 0, - 10, - Judgement::Reasonable, - BlakeTwo256::hash_of(&ten()) - ), - Error::::PalletLocked - ); - assert_noop!(Identity::kill_identity(two_as_origin, 10), Error::::PalletLocked); - assert_noop!( - Identity::add_sub(ten_as_origin.clone(), 1, data.clone()), - Error::::PalletLocked - ); - assert_noop!( - Identity::rename_sub(ten_as_origin.clone(), 1, data), - Error::::PalletLocked - ); - assert_noop!(Identity::remove_sub(ten_as_origin.clone(), 1), Error::::PalletLocked); - assert_noop!(Identity::quit_sub(one_as_origin.clone()), Error::::PalletLocked); - - // Reap and Poke are still callable for migration - assert_ok!(Identity::poke_deposit(one_as_origin.clone(), 10)); - assert_ok!(Identity::reap_identity(one_as_origin.clone(), 10)); - - // Unlock still needs to be callable, obviously - assert_ok!(Identity::unlock_pallet(one_as_origin)); - - // And pallet works - assert_ok!(Identity::set_identity(twenty_as_origin, Box::new(twenty()))); - }); -} diff --git a/substrate/frame/identity/src/weights.rs b/substrate/frame/identity/src/weights.rs index 91adede22e18..07ab48ccd899 100644 --- a/substrate/frame/identity/src/weights.rs +++ b/substrate/frame/identity/src/weights.rs @@ -70,8 +70,6 @@ pub trait WeightInfo { fn quit_sub(s: u32, ) -> Weight; fn reap_identity(r: u32, s: u32, ) -> Weight; fn poke_deposit() -> Weight; - fn lock_pallet() -> Weight; - fn unlock_pallet() -> Weight; } /// Weights for pallet_identity using the Substrate node and recommended hardware. @@ -418,28 +416,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().writes(3)) } - /// Storage: `Identity::Locked` (r:0 w:1) - /// Proof: `Identity::Locked` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) - fn lock_pallet() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 43_286_000 picoseconds. - Weight::from_parts(51_523_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } - /// Storage: `Identity::Locked` (r:0 w:1) - /// Proof: `Identity::Locked` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) - fn unlock_pallet() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 42_243_000 picoseconds. - Weight::from_parts(52_907_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } } // For backwards compatibility and tests @@ -785,26 +761,4 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(3)) .saturating_add(RocksDbWeight::get().writes(3)) } - /// Storage: `Identity::Locked` (r:0 w:1) - /// Proof: `Identity::Locked` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) - fn lock_pallet() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 43_286_000 picoseconds. - Weight::from_parts(51_523_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(RocksDbWeight::get().writes(1)) - } - /// Storage: `Identity::Locked` (r:0 w:1) - /// Proof: `Identity::Locked` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) - fn unlock_pallet() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 42_243_000 picoseconds. - Weight::from_parts(52_907_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(RocksDbWeight::get().writes(1)) - } }