From 82cbd76245d6ad323cd83d0785f339db49f09657 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 2 Jun 2024 18:47:09 +0200 Subject: [PATCH 1/9] clear judgement --- .../people/people-kusama/src/identity_ops.rs | 185 ++++++++++++++++++ .../people/people-kusama/src/lib.rs | 7 + 2 files changed, 192 insertions(+) create mode 100644 system-parachains/people/people-kusama/src/identity_ops.rs diff --git a/system-parachains/people/people-kusama/src/identity_ops.rs b/system-parachains/people/people-kusama/src/identity_ops.rs new file mode 100644 index 0000000000..dd29c47216 --- /dev/null +++ b/system-parachains/people/people-kusama/src/identity_ops.rs @@ -0,0 +1,185 @@ +// 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. + +#[frame_support::pallet] +pub mod pallet_identity_ops { + use crate::*; + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use pallet_identity::Judgement; + + type IdentityPallet = pallet_identity::Pallet; + + #[pallet::pallet] + pub struct Pallet(PhantomData); + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::error] + pub enum Error { + /// No judgement to clear. + NotFound, + } + + #[frame_support::storage_alias(pallet_name)] + pub type IdentityMap = StorageMap< + IdentityPallet, + Twox64Concat, + AccountId, + ( + pallet_identity::Registration< + ::Balance, + ::MaxRegistrars, + ::IdentityInformation, + >, + Option>, + ), + OptionQuery, + >; + + #[pallet::call] + impl Pallet { + /// Clear requested judgements that do not have a corresponding deposit reserved. + /// + /// This is successful only if the `target` account has judgements to clear. The transaction + /// fee is refunded to the caller if successful. + #[pallet::call_index(0)] + #[pallet::weight(Pallet::::weight_clear_judgement())] + pub fn clear_judgement( + _origin: OriginFor, + target: AccountId, + ) -> DispatchResultWithPostInfo { + let removed = Self::do_clear_judgement(&target); + ensure!(removed > 0, Error::::NotFound); + + Ok(Pays::No.into()) + } + } + + impl Pallet { + fn do_clear_judgement(target: &AccountId) -> u32 { + let Some((mut identity, _)) = IdentityPallet::identity(target) else { + return 0; + }; + // `subs_of`'s query kind is value option, if non subs the deposit is zero. + let (subs_deposit, _) = IdentityPallet::subs_of(target); + // deposit without deposits for judgement request. + let identity_deposit = identity.deposit.saturating_add(subs_deposit); + // total reserved balance. + let reserved = Balances::reserved_balance(target); + // expected deposit with judgement deposits. + let mut expected_total_deposit = identity_deposit; + + let judgements_count = identity.judgements.len(); + + identity.judgements.retain(|(_, judgement)| { + if let Judgement::FeePaid(deposit) = judgement { + expected_total_deposit = expected_total_deposit.saturating_add(*deposit); + reserved >= expected_total_deposit + } else { + true + } + }); + + (judgements_count - identity.judgements.len()) as u32 + } + + /// Weight calculation for the worst-case scenario of `clear_judgement`. + /// Equal to 20 registrars reads/writes + 1 identity read + 1 subs read + 1 reserve + /// balance read. + fn weight_clear_judgement() -> Weight { + let max_registrars = + <::MaxRegistrars as Get>::get(); + ::DbWeight::get() + .reads_writes((max_registrars + 3).into(), max_registrars.into()) + } + } + + #[pallet::hooks] + impl Hooks> for Pallet { + #[cfg(feature = "try-runtime")] + fn try_state(_: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + use crate::Balance; + use sp_core::crypto::Ss58Codec; + use sp_runtime::traits::Zero; + + let mut invalid_identity_count = 0; + let mut invalid_judgement_count = 0; + let mut identity_count = 0; + IdentityMap::iter().for_each(|(account_id, _)| { + let (identity, _) = IdentityPallet::identity(&account_id).unwrap(); + + let (paid_judgement_count, judgement_deposit) = identity.judgements.iter().fold( + (0, Zero::zero()), + |(count, total_deposit): (u32, Balance), (_, judgement)| { + if let Judgement::FeePaid(deposit) = judgement { + (count + 1, total_deposit.saturating_add(*deposit)) + } else { + (count, total_deposit) + } + }, + ); + + let (subs_deposit, _) = IdentityPallet::subs_of(&account_id); + + let deposit = + identity.deposit.saturating_add(judgement_deposit).saturating_add(subs_deposit); + let deposit_wo_judgement = identity.deposit.saturating_add(subs_deposit); + let reserved = Balances::reserved_balance(&account_id); + + if deposit > reserved && paid_judgement_count > 0 { + invalid_identity_count += 1; + invalid_judgement_count += paid_judgement_count; + + log::info!( + "account with invalid state: {:?}, expected reserve at least: {:?}, actual: {:?}", + account_id.clone().to_ss58check(), + deposit, + reserved, + ); + + assert_eq!(paid_judgement_count, Self::do_clear_judgement(&account_id)); + + if deposit_wo_judgement != reserved { + log::warn!( + "unexpected state: {:?}, deposit w/o judgement: {:?}, not equal to the total reserved: {:?}", + account_id.clone().to_ss58check(), + deposit_wo_judgement, + reserved, + ); + } + } else { + assert_eq!(0, Self::do_clear_judgement(&account_id)); + } + if deposit_wo_judgement > reserved { + log::warn!( + "unexpected state: {:?}, deposit w/o judgement: {:?}, greater than the total reserved: {:?}", + account_id.clone().to_ss58check(), + deposit_wo_judgement, + reserved, + ); + } + identity_count += 1; + }); + + log::info!("total identities processed: {:?}", identity_count); + log::info!("invalid identities: {:?}", invalid_identity_count); + log::info!("invalid judgements: {:?}", invalid_judgement_count); + + Ok(()) + } + } +} diff --git a/system-parachains/people/people-kusama/src/lib.rs b/system-parachains/people/people-kusama/src/lib.rs index 2b53323aeb..0d8fb7ae77 100644 --- a/system-parachains/people/people-kusama/src/lib.rs +++ b/system-parachains/people/people-kusama/src/lib.rs @@ -18,6 +18,7 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); +mod identity_ops; pub mod people; mod weights; pub mod xcm_config; @@ -41,6 +42,7 @@ use frame_system::{ limits::{BlockLength, BlockWeights}, EnsureRoot, }; +use identity_ops::pallet_identity_ops; use pallet_xcm::{EnsureXcm, IsVoiceOfBody}; use parachains_common::{ message_queue::{NarrowOriginToSibling, ParaIdToSibling}, @@ -511,6 +513,8 @@ impl identity_migrator::Config for Runtime { type WeightInfo = weights::polkadot_runtime_common_identity_migrator::WeightInfo; } +impl pallet_identity_ops::Config for Runtime {} + // Create the runtime by composing the FRAME pallets that were previously configured. construct_runtime!( pub enum Runtime @@ -548,6 +552,9 @@ construct_runtime!( // To migrate deposits IdentityMigrator: identity_migrator = 248, + + // Identity operations pallet. + IdentityOps: pallet_identity_ops = 247, } ); From 1f32485cd148240cc507d967333cf55a53dfccfa Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 3 Jun 2024 09:23:16 +0200 Subject: [PATCH 2/9] fixes --- .../people/people-kusama/src/identity_ops.rs | 69 +++++++++---------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/system-parachains/people/people-kusama/src/identity_ops.rs b/system-parachains/people/people-kusama/src/identity_ops.rs index dd29c47216..c5d7f41a98 100644 --- a/system-parachains/people/people-kusama/src/identity_ops.rs +++ b/system-parachains/people/people-kusama/src/identity_ops.rs @@ -34,21 +34,19 @@ pub mod pallet_identity_ops { NotFound, } + pub(crate) type Identity = ( + pallet_identity::Registration< + ::Balance, + ::MaxRegistrars, + ::IdentityInformation, + >, + Option::MaxUsernameLength>>, + ); + + /// Alias for `IdentityOf` from `pallet_identity`. #[frame_support::storage_alias(pallet_name)] - pub type IdentityMap = StorageMap< - IdentityPallet, - Twox64Concat, - AccountId, - ( - pallet_identity::Registration< - ::Balance, - ::MaxRegistrars, - ::IdentityInformation, - >, - Option>, - ), - OptionQuery, - >; + pub(crate) type IdentityOf = + StorageMap; #[pallet::call] impl Pallet { @@ -62,30 +60,30 @@ pub mod pallet_identity_ops { _origin: OriginFor, target: AccountId, ) -> DispatchResultWithPostInfo { - let removed = Self::do_clear_judgement(&target); + let identity = IdentityPallet::identity(&target).ok_or(Error::::NotFound)?; + let (removed, identity) = Self::do_clear_judgement(&target, identity); ensure!(removed > 0, Error::::NotFound); + IdentityOf::insert(&target, identity); + Ok(Pays::No.into()) } } impl Pallet { - fn do_clear_judgement(target: &AccountId) -> u32 { - let Some((mut identity, _)) = IdentityPallet::identity(target) else { - return 0; - }; + fn do_clear_judgement(account_id: &AccountId, mut identity: Identity) -> (u32, Identity) { // `subs_of`'s query kind is value option, if non subs the deposit is zero. - let (subs_deposit, _) = IdentityPallet::subs_of(target); + let (subs_deposit, _) = IdentityPallet::subs_of(account_id); // deposit without deposits for judgement request. - let identity_deposit = identity.deposit.saturating_add(subs_deposit); + let identity_deposit = identity.0.deposit.saturating_add(subs_deposit); // total reserved balance. - let reserved = Balances::reserved_balance(target); + let reserved = Balances::reserved_balance(account_id); // expected deposit with judgement deposits. let mut expected_total_deposit = identity_deposit; + // count before cleaning up the judgements. + let judgements_count = identity.0.judgements.len(); - let judgements_count = identity.judgements.len(); - - identity.judgements.retain(|(_, judgement)| { + identity.0.judgements.retain(|(_, judgement)| { if let Judgement::FeePaid(deposit) = judgement { expected_total_deposit = expected_total_deposit.saturating_add(*deposit); reserved >= expected_total_deposit @@ -94,17 +92,13 @@ pub mod pallet_identity_ops { } }); - (judgements_count - identity.judgements.len()) as u32 + ((judgements_count - identity.0.judgements.len()) as u32, identity) } /// Weight calculation for the worst-case scenario of `clear_judgement`. - /// Equal to 20 registrars reads/writes + 1 identity read + 1 subs read + 1 reserve - /// balance read. + /// Equal to 1 identity read/write + 1 subs read + 1 reserve balance read. fn weight_clear_judgement() -> Weight { - let max_registrars = - <::MaxRegistrars as Get>::get(); - ::DbWeight::get() - .reads_writes((max_registrars + 3).into(), max_registrars.into()) + ::DbWeight::get().reads_writes(3, 1) } } @@ -119,8 +113,8 @@ pub mod pallet_identity_ops { let mut invalid_identity_count = 0; let mut invalid_judgement_count = 0; let mut identity_count = 0; - IdentityMap::iter().for_each(|(account_id, _)| { - let (identity, _) = IdentityPallet::identity(&account_id).unwrap(); + IdentityOf::iter().for_each(|(account_id, _)| { + let (identity, username) = IdentityPallet::identity(&account_id).unwrap(); let (paid_judgement_count, judgement_deposit) = identity.judgements.iter().fold( (0, Zero::zero()), @@ -151,7 +145,10 @@ pub mod pallet_identity_ops { reserved, ); - assert_eq!(paid_judgement_count, Self::do_clear_judgement(&account_id)); + assert_eq!( + paid_judgement_count, + Self::do_clear_judgement(&account_id, (identity, username)).0 + ); if deposit_wo_judgement != reserved { log::warn!( @@ -162,7 +159,7 @@ pub mod pallet_identity_ops { ); } } else { - assert_eq!(0, Self::do_clear_judgement(&account_id)); + assert_eq!(0, Self::do_clear_judgement(&account_id, (identity, username)).0); } if deposit_wo_judgement > reserved { log::warn!( From 4d6808aba652b9d2343f1135cef0ff2c5d810483 Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 3 Jun 2024 09:28:33 +0200 Subject: [PATCH 3/9] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f34f76d1bd..2d9a029b0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Handle extra erroneous consumer reference when a nomination pool is destroying ([polkadot-fellows/runtimes#318](https://github.com/polkadot-fellows/runtimes/pull/318)) - Introduce [Encointer](https://encointer.org) collator selection and send fees to authors instead of treasury ([polkadot-fellows/runtimes#270](https://github.com/polkadot-fellows/runtimes/pull/270)) +- Kusama People: clear requested judgements that do not have corresponding deposits reserved ([polkadot-fellows/runtimes#339](https://github.com/polkadot-fellows/runtimes/pull/339)) ## [1.2.4] 20.05.2024 From 5c52a3a64a976f4833e1c67cd2aafd136821e148 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 4 Jun 2024 13:49:33 +0200 Subject: [PATCH 4/9] benchmarks --- .../people/people-kusama/src/identity_ops.rs | 64 ++++++++++++++++--- .../people/people-kusama/src/lib.rs | 5 +- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/system-parachains/people/people-kusama/src/identity_ops.rs b/system-parachains/people/people-kusama/src/identity_ops.rs index c5d7f41a98..3b0df6ee99 100644 --- a/system-parachains/people/people-kusama/src/identity_ops.rs +++ b/system-parachains/people/people-kusama/src/identity_ops.rs @@ -26,7 +26,10 @@ pub mod pallet_identity_ops { pub struct Pallet(PhantomData); #[pallet::config] - pub trait Config: frame_system::Config {} + pub trait Config: frame_system::Config { + /// Overarching event type. + type RuntimeEvent: From> + IsType<::RuntimeEvent>; + } #[pallet::error] pub enum Error { @@ -34,6 +37,13 @@ pub mod pallet_identity_ops { NotFound, } + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + /// The invalid judgements have been cleared. + JudgementsCleared { target: AccountId }, + } + pub(crate) type Identity = ( pallet_identity::Registration< ::Balance, @@ -55,7 +65,7 @@ pub mod pallet_identity_ops { /// This is successful only if the `target` account has judgements to clear. The transaction /// fee is refunded to the caller if successful. #[pallet::call_index(0)] - #[pallet::weight(Pallet::::weight_clear_judgement())] + #[pallet::weight({ 0 })] // TODO generate weights pub fn clear_judgement( _origin: OriginFor, target: AccountId, @@ -66,6 +76,8 @@ pub mod pallet_identity_ops { IdentityOf::insert(&target, identity); + Self::deposit_event(Event::JudgementsCleared { target }); + Ok(Pays::No.into()) } } @@ -94,12 +106,6 @@ pub mod pallet_identity_ops { ((judgements_count - identity.0.judgements.len()) as u32, identity) } - - /// Weight calculation for the worst-case scenario of `clear_judgement`. - /// Equal to 1 identity read/write + 1 subs read + 1 reserve balance read. - fn weight_clear_judgement() -> Weight { - ::DbWeight::get().reads_writes(3, 1) - } } #[pallet::hooks] @@ -180,3 +186,45 @@ pub mod pallet_identity_ops { } } } + +#[cfg(feature = "runtime-benchmarks")] +#[frame_benchmarking::v2::benchmarks(where T: pallet_identity_ops::Config)] +mod benchmarks { + use crate::{people::IdentityInfo, *}; + use frame_benchmarking::BenchmarkError; + use frame_system::RawOrigin; + use pallet_identity::{IdentityInformationProvider, Judgement}; + use pallet_identity_ops::{Event, Identity, *}; + use parachains_common::{AccountId, Balance}; + use sp_core::Get; + use sp_runtime::traits::One; + + #[benchmark] + fn clear_judgement() -> Result<(), BenchmarkError> { + let max_registrars = + <::MaxRegistrars as Get>::get(); + let mut judgements = Vec::<(u32, Judgement)>::new(); + for i in 0..max_registrars { + judgements.push((i, Judgement::FeePaid(Balance::one()))); + } + let identity: Identity = ( + pallet_identity::Registration { + deposit: Balance::one(), + judgements: judgements.try_into().unwrap(), + info: IdentityInfo::create_identity_info(), + }, + None, + ); + + let target: AccountId = [1u8; 32].into(); + + IdentityOf::insert(&target, identity); + + #[extrinsic_call] + _(RawOrigin::None, target.clone()); + + crate::System::assert_last_event(Event::::JudgementsCleared { target }.into()); + + Ok(()) + } +} diff --git a/system-parachains/people/people-kusama/src/lib.rs b/system-parachains/people/people-kusama/src/lib.rs index 0d8fb7ae77..05b2b5d580 100644 --- a/system-parachains/people/people-kusama/src/lib.rs +++ b/system-parachains/people/people-kusama/src/lib.rs @@ -513,7 +513,9 @@ impl identity_migrator::Config for Runtime { type WeightInfo = weights::polkadot_runtime_common_identity_migrator::WeightInfo; } -impl pallet_identity_ops::Config for Runtime {} +impl pallet_identity_ops::Config for Runtime { + type RuntimeEvent = RuntimeEvent; +} // Create the runtime by composing the FRAME pallets that were previously configured. construct_runtime!( @@ -579,6 +581,7 @@ mod benches { [pallet_xcm, PalletXcmExtrinsiscsBenchmark::] [pallet_xcm_benchmarks::fungible, XcmBalances] [pallet_xcm_benchmarks::generic, XcmGeneric] + [pallet_identity_ops, IdentityOps] ); } From a0a05c1e20731d1dc160e95f6a035702c31f05ac Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 4 Jun 2024 15:00:17 +0200 Subject: [PATCH 5/9] local weights --- .../people/people-kusama/src/identity_ops.rs | 8 ++- .../people/people-kusama/src/weights/mod.rs | 1 + .../src/weights/pallet_identity_ops.rs | 65 +++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs diff --git a/system-parachains/people/people-kusama/src/identity_ops.rs b/system-parachains/people/people-kusama/src/identity_ops.rs index 3b0df6ee99..5b21c1310b 100644 --- a/system-parachains/people/people-kusama/src/identity_ops.rs +++ b/system-parachains/people/people-kusama/src/identity_ops.rs @@ -20,6 +20,12 @@ pub mod pallet_identity_ops { use frame_system::pallet_prelude::*; use pallet_identity::Judgement; + /// Weight information for extrinsics in this pallet. + pub trait WeightInfo { + /// Weight for clearing judgement. + fn clear_judgement() -> Weight; + } + type IdentityPallet = pallet_identity::Pallet; #[pallet::pallet] @@ -65,7 +71,7 @@ pub mod pallet_identity_ops { /// This is successful only if the `target` account has judgements to clear. The transaction /// fee is refunded to the caller if successful. #[pallet::call_index(0)] - #[pallet::weight({ 0 })] // TODO generate weights + #[pallet::weight(weights::pallet_identity_ops::WeightInfo::::clear_judgement())] pub fn clear_judgement( _origin: OriginFor, target: AccountId, diff --git a/system-parachains/people/people-kusama/src/weights/mod.rs b/system-parachains/people/people-kusama/src/weights/mod.rs index dce959e817..0773aa66f5 100644 --- a/system-parachains/people/people-kusama/src/weights/mod.rs +++ b/system-parachains/people/people-kusama/src/weights/mod.rs @@ -23,6 +23,7 @@ pub mod frame_system; pub mod pallet_balances; pub mod pallet_collator_selection; pub mod pallet_identity; +pub mod pallet_identity_ops; pub mod pallet_message_queue; pub mod pallet_multisig; pub mod pallet_proxy; diff --git a/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs b/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs new file mode 100644 index 0000000000..0d0f9ee38e --- /dev/null +++ b/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs @@ -0,0 +1,65 @@ +// Copyright (C) Parity Technologies and the various Polkadot contributors, see Contributions.md +// for a list of specific contributors. +// 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. +//! Autogenerated weights for `pallet_identity_ops` +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 +//! DATE: 2024-06-04, STEPS: `10`, REPEAT: `50`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `cob`, CPU: `` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("./people-kusama-chain-spec.json")`, DB CACHE: 1024 + +// Executed Command: +// ./polkadot +// benchmark +// pallet +// --chain=./people-kusama-chain-spec.json +// --steps=10 +// --repeat=50 +// --pallet=pallet_identity_ops +// --extrinsic=* +// --wasm-execution=compiled +// --heap-pages=4096 +// --output=./people-kusama-weights/ +// --header=./file_header.txt + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] + +use frame_support::{traits::Get, weights::Weight}; +use core::marker::PhantomData; + +/// Weight functions for `pallet_identity_ops`. +pub struct WeightInfo(PhantomData); +impl crate::pallet_identity_ops::WeightInfo for WeightInfo { + /// Storage: `Identity::IdentityOf` (r:1 w:1) + /// Proof: `Identity::IdentityOf` (`max_values`: None, `max_size`: Some(838), added: 3313, mode: `MaxEncodedLen`) + /// Storage: `Identity::SubsOf` (r:1 w:0) + /// Proof: `Identity::SubsOf` (`max_values`: None, `max_size`: Some(3258), added: 5733, mode: `MaxEncodedLen`) + /// Storage: `System::Account` (r:1 w:0) + /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) + fn clear_judgement() -> Weight { + // Proof Size summary in bytes: + // Measured: `828` + // Estimated: `6723` + // Minimum execution time: 15_000_000 picoseconds. + Weight::from_parts(16_000_000, 0) + .saturating_add(Weight::from_parts(0, 6723)) + .saturating_add(T::DbWeight::get().reads(3)) + .saturating_add(T::DbWeight::get().writes(1)) + } +} From af5149fc89ea2e0b792c5fbce171cccac01eb1c9 Mon Sep 17 00:00:00 2001 From: Muharem Date: Tue, 4 Jun 2024 15:39:10 +0200 Subject: [PATCH 6/9] Apply suggestions from code review Co-authored-by: Branislav Kontur --- .../src/weights/pallet_identity_ops.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs b/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs index 0d0f9ee38e..daac69090b 100644 --- a/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs +++ b/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs @@ -13,21 +13,22 @@ // 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. + //! Autogenerated weights for `pallet_identity_ops` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 -//! DATE: 2024-06-04, STEPS: `10`, REPEAT: `50`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-06-04, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `cob`, CPU: `` +//! HOSTNAME: `ggwpez-ref-hw`, CPU: `AMD EPYC 7232P 8-Core Processor` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("./people-kusama-chain-spec.json")`, DB CACHE: 1024 // Executed Command: -// ./polkadot +// ./target/production/polkadot // benchmark // pallet // --chain=./people-kusama-chain-spec.json -// --steps=10 -// --repeat=50 +// --steps=50 +// --repeat=20 // --pallet=pallet_identity_ops // --extrinsic=* // --wasm-execution=compiled @@ -45,7 +46,7 @@ use core::marker::PhantomData; /// Weight functions for `pallet_identity_ops`. pub struct WeightInfo(PhantomData); -impl crate::pallet_identity_ops::WeightInfo for WeightInfo { +impl pallet_identity_ops::WeightInfo for WeightInfo { /// Storage: `Identity::IdentityOf` (r:1 w:1) /// Proof: `Identity::IdentityOf` (`max_values`: None, `max_size`: Some(838), added: 3313, mode: `MaxEncodedLen`) /// Storage: `Identity::SubsOf` (r:1 w:0) @@ -56,8 +57,8 @@ impl crate::pallet_identity_ops::WeightInfo for WeightI // Proof Size summary in bytes: // Measured: `828` // Estimated: `6723` - // Minimum execution time: 15_000_000 picoseconds. - Weight::from_parts(16_000_000, 0) + // Minimum execution time: 22_170_000 picoseconds. + Weight::from_parts(22_770_000, 0) .saturating_add(Weight::from_parts(0, 6723)) .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().writes(1)) From a012e5cf4eb50adc857871d42b7f9861cdf50187 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 4 Jun 2024 15:43:28 +0200 Subject: [PATCH 7/9] fix weights --- .../people/people-kusama/src/weights/pallet_identity_ops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs b/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs index daac69090b..f26f114e49 100644 --- a/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs +++ b/system-parachains/people/people-kusama/src/weights/pallet_identity_ops.rs @@ -46,7 +46,7 @@ use core::marker::PhantomData; /// Weight functions for `pallet_identity_ops`. pub struct WeightInfo(PhantomData); -impl pallet_identity_ops::WeightInfo for WeightInfo { +impl crate::pallet_identity_ops::WeightInfo for WeightInfo { /// Storage: `Identity::IdentityOf` (r:1 w:1) /// Proof: `Identity::IdentityOf` (`max_values`: None, `max_size`: Some(838), added: 3313, mode: `MaxEncodedLen`) /// Storage: `Identity::SubsOf` (r:1 w:0) From 1a7d89b7eb928febed789213db587dc2949da5d8 Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 10 Jun 2024 12:55:47 +0200 Subject: [PATCH 8/9] remove fee paid with zero deposit --- .../people/people-kusama/src/identity_ops.rs | 63 ++++++++++--------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/system-parachains/people/people-kusama/src/identity_ops.rs b/system-parachains/people/people-kusama/src/identity_ops.rs index 5b21c1310b..1c0a5a4352 100644 --- a/system-parachains/people/people-kusama/src/identity_ops.rs +++ b/system-parachains/people/people-kusama/src/identity_ops.rs @@ -104,7 +104,7 @@ pub mod pallet_identity_ops { identity.0.judgements.retain(|(_, judgement)| { if let Judgement::FeePaid(deposit) = judgement { expected_total_deposit = expected_total_deposit.saturating_add(*deposit); - reserved >= expected_total_deposit + reserved >= expected_total_deposit && *deposit > 0 } else { true } @@ -120,52 +120,53 @@ pub mod pallet_identity_ops { fn try_state(_: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { use crate::Balance; use sp_core::crypto::Ss58Codec; - use sp_runtime::traits::Zero; - let mut invalid_identity_count = 0; - let mut invalid_judgement_count = 0; + let mut total_invalid_identity_count = 0; + let mut total_invalid_judgement_count = 0; let mut identity_count = 0; - IdentityOf::iter().for_each(|(account_id, _)| { - let (identity, username) = IdentityPallet::identity(&account_id).unwrap(); - - let (paid_judgement_count, judgement_deposit) = identity.judgements.iter().fold( - (0, Zero::zero()), - |(count, total_deposit): (u32, Balance), (_, judgement)| { - if let Judgement::FeePaid(deposit) = judgement { - (count + 1, total_deposit.saturating_add(*deposit)) - } else { - (count, total_deposit) - } - }, - ); + IdentityOf::iter().for_each(|(account_id, registration)| { + let (identity, username) = registration; let (subs_deposit, _) = IdentityPallet::subs_of(&account_id); - - let deposit = - identity.deposit.saturating_add(judgement_deposit).saturating_add(subs_deposit); let deposit_wo_judgement = identity.deposit.saturating_add(subs_deposit); let reserved = Balances::reserved_balance(&account_id); - if deposit > reserved && paid_judgement_count > 0 { - invalid_identity_count += 1; - invalid_judgement_count += paid_judgement_count; + let (invalid_judgement_count, expected_total_deposit) = + identity.judgements.iter().fold( + (0, deposit_wo_judgement), + |(count, total_deposit): (u32, Balance), (_, judgement)| { + if let Judgement::FeePaid(deposit) = judgement { + if total_deposit.saturating_add(*deposit) > reserved || + *deposit == 0 + { + return (count + 1, total_deposit.saturating_add(*deposit)) + } + } + (count, total_deposit) + }, + ); + + if expected_total_deposit >= reserved && invalid_judgement_count > 0 { + total_invalid_identity_count += 1; + total_invalid_judgement_count += invalid_judgement_count; log::info!( - "account with invalid state: {:?}, expected reserve at least: {:?}, actual: {:?}", - account_id.clone().to_ss58check(), - deposit, + "account with invalid state: {:?}, expected reserve at least: {:?}, actual: {:?}, invalid judgements: {:?}", + account_id.clone().to_ss58check_with_version(2u8.into()), + expected_total_deposit, reserved, + invalid_judgement_count, ); assert_eq!( - paid_judgement_count, + invalid_judgement_count, Self::do_clear_judgement(&account_id, (identity, username)).0 ); if deposit_wo_judgement != reserved { log::warn!( "unexpected state: {:?}, deposit w/o judgement: {:?}, not equal to the total reserved: {:?}", - account_id.clone().to_ss58check(), + account_id.clone().to_ss58check_with_version(2u8.into()), deposit_wo_judgement, reserved, ); @@ -176,7 +177,7 @@ pub mod pallet_identity_ops { if deposit_wo_judgement > reserved { log::warn!( "unexpected state: {:?}, deposit w/o judgement: {:?}, greater than the total reserved: {:?}", - account_id.clone().to_ss58check(), + account_id.clone().to_ss58check_with_version(2u8.into()), deposit_wo_judgement, reserved, ); @@ -185,8 +186,8 @@ pub mod pallet_identity_ops { }); log::info!("total identities processed: {:?}", identity_count); - log::info!("invalid identities: {:?}", invalid_identity_count); - log::info!("invalid judgements: {:?}", invalid_judgement_count); + log::info!("invalid identities: {:?}", total_invalid_identity_count); + log::info!("invalid judgements: {:?}", total_invalid_judgement_count); Ok(()) } From 2f031cf9d33de7267374924f7d85d4e44119b882 Mon Sep 17 00:00:00 2001 From: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Date: Tue, 11 Jun 2024 13:14:39 +0200 Subject: [PATCH 9/9] Update CHANGELOG.md --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 074dac111b..5198be8618 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ Changelog for the runtimes governed by the Polkadot Fellowship. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [Unreleased] + +### Fixed + +- Kusama People: clear requested judgements that do not have corresponding deposits reserved ([polkadot-fellows/runtimes#339](https://github.com/polkadot-fellows/runtimes/pull/339)) + ## [1.2.5] 06.06.2024 ### Added @@ -22,7 +28,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Handle extra erroneous consumer reference when a nomination pool is destroying ([polkadot-fellows/runtimes#318](https://github.com/polkadot-fellows/runtimes/pull/318)) - Introduce [Encointer](https://encointer.org) collator selection and send fees to authors instead of treasury ([polkadot-fellows/runtimes#270](https://github.com/polkadot-fellows/runtimes/pull/270)) -- Kusama People: clear requested judgements that do not have corresponding deposits reserved ([polkadot-fellows/runtimes#339](https://github.com/polkadot-fellows/runtimes/pull/339)) ## [1.2.4] 20.05.2024